Issue-58: Ensure safety usage of the init_data methods (#72)

* Ensure safety usage of the init_data methods

CHANGELOG.md

@@ -1,11 +1,15 @@
 # Changelog
 
-## v2.0.0 - 
+## v2.0.0 -
 
 ### Added
 
 - #68 - Added new provider `HGC` using `Html` and `EmailSubjectParser`
 
+### Fixed
+
+- #72 - Ensure `NotificationData` init methods for library client do not raise exceptions and just return `None`.
+
 ## v2.0.0-beta - 2021-09-07
 
 ### Added

circuit_maintenance_parser/__init__.py

@@ -1,8 +1,8 @@
-"""Notifications parser init."""
-
+"""Circuit-maintenance-parser init."""
 from typing import Type, Optional
 
 from .data import NotificationData
+from .output import Maintenance
 from .errors import NonexistentProviderError, ProviderError
 from .provider import (
     GenericProvider,
@@ -60,21 +60,6 @@ def init_provider(provider_type=None) -> Optional[GenericProvider]:
         return None
 
 
-def init_data_raw(data_type: str, data_content: bytes) -> NotificationData:
-    """Returns an instance of NotificationData from one combination of data type and content."""
-    return NotificationData.init(data_type, data_content)
-
-
-def init_data_email(raw_email_bytes: bytes) -> NotificationData:
-    """Returns an instance of NotificationData from a raw email content."""
-    return NotificationData.init_from_email_bytes(raw_email_bytes)
-
-
-def init_data_emailmessage(email_message) -> NotificationData:
-    """Returns an instance of NotificationData from an email message."""
-    return NotificationData.init_from_emailmessage(email_message)
-
-
 def get_provider_class(provider_name: str) -> Type[GenericProvider]:
     """Returns the Provider parser class for a specific provider_type."""
     provider_name = provider_name.lower()
@@ -107,11 +92,10 @@ def get_provider_class_from_sender(email_sender: str) -> Type[GenericProvider]:
 
 __all__ = [
     "init_provider",
-    "init_data_raw",
-    "init_data_email",
-    "init_data_emailmessage",
+    "NotificationData",
     "get_provider_class",
     "get_provider_class_from_sender",
     "ProviderError",
     "NonexistentProviderError",
+    "Maintenance",
 ]

circuit_maintenance_parser/cli.py

@@ -4,8 +4,9 @@
 import email
 import click
 
-from . import SUPPORTED_PROVIDERS, init_provider, init_data_raw, init_data_emailmessage
+from . import SUPPORTED_PROVIDERS, init_provider
 from .provider import ProviderError
+from .data import NotificationData
 
 
 @click.command()
@@ -32,15 +33,15 @@ def main(provider_type, data_file, data_type, verbose):
         if str.lower(data_file[-3:]) == "eml":
             with open(data_file) as email_file:
                 msg = email.message_from_file(email_file)
-            data = init_data_emailmessage(msg)
+            data = NotificationData.init_from_emailmessage(msg)
         else:
             click.echo("File format not supported, only *.eml", err=True)
             sys.exit(1)
 
     else:
         with open(data_file, "rb") as raw_filename:
             raw_bytes = raw_filename.read()
-        data = init_data_raw(data_type, raw_bytes)
+        data = NotificationData.init_from_raw(data_type, raw_bytes)
 
     try:
         parsed_notifications = provider.get_maintenances(data)

circuit_maintenance_parser/data.py

@@ -1,9 +1,12 @@
 """Definition of Data classes."""
-from typing import List, NamedTuple
+import logging
+from typing import List, NamedTuple, Optional, Type, Set
 
 import email
 from pydantic import BaseModel, Extra
 
+logger = logging.getLogger(__name__)
+
 
 class DataPart(NamedTuple):
     """Simplest data unit to be parsed."""
@@ -23,16 +26,26 @@ def add_data_part(self, data_type: str, data_content: bytes):
         self.data_parts.append(DataPart(data_type, data_content))
 
     @classmethod
-    def init(cls, data_type: str, data_content: bytes):
+    def init_from_raw(
+        cls: Type["NotificationData"], data_type: str, data_content: bytes
+    ) -> Optional["NotificationData"]:
         """Initialize the data_parts with only one DataPart object."""
-        return cls(data_parts=[DataPart(data_type, data_content)])
+        try:
+            return cls(data_parts=[DataPart(data_type, data_content)])
+        except Exception:  # pylint: disable=broad-except
+            logger.exception("Error found initializing data raw: %s, %s", data_type, data_content)
+        return None
 
     @classmethod
-    def init_from_email_bytes(cls, raw_email_bytes: bytes):
+    def init_from_email_bytes(cls: Type["NotificationData"], raw_email_bytes: bytes) -> Optional["NotificationData"]:
         """Initialize the data_parts from an email defined as raw bytes.."""
-        raw_email_string = raw_email_bytes.decode("utf-8")
-        email_message = email.message_from_string(raw_email_string)
-        return cls.init_from_emailmessage(email_message)
+        try:
+            raw_email_string = raw_email_bytes.decode("utf-8")
+            email_message = email.message_from_string(raw_email_string)
+            return cls.init_from_emailmessage(email_message)
+        except Exception:  # pylint: disable=broad-except
+            logger.exception("Error found initializing data from email raw bytes: %s", raw_email_bytes)
+        return None
 
     @classmethod
     def walk_email(cls, email_message, data_parts):
@@ -53,13 +66,17 @@ def walk_email(cls, email_message, data_parts):
                 data_parts.add(DataPart(part.get_content_type(), part.get_payload(decode=True)))
 
     @classmethod
-    def init_from_emailmessage(cls, email_message):
+    def init_from_emailmessage(cls: Type["NotificationData"], email_message) -> Optional["NotificationData"]:
         """Initialize the data_parts from an email.message.Email object."""
-        data_parts = set()
-        cls.walk_email(email_message, data_parts)
+        try:
+            data_parts: Set[DataPart] = set()
+            cls.walk_email(email_message, data_parts)
 
-        # Adding extra headers that are interesting to be parsed
-        data_parts.add(DataPart("email-header-subject", email_message["Subject"].encode()))
-        # TODO: Date could be used to extend the "Stamp" time of a notification when not available, but we need a parser
-        data_parts.add(DataPart("email-header-date", email_message["Date"].encode()))
-        return cls(data_parts=list(data_parts))
+            # Adding extra headers that are interesting to be parsed
+            data_parts.add(DataPart("email-header-subject", email_message["Subject"].encode()))
+            # TODO: Date could be used to extend the "Stamp" time of a notification when not available, but we need a parser
+            data_parts.add(DataPart("email-header-date", email_message["Date"].encode()))
+            return cls(data_parts=list(data_parts))
+        except Exception:  # pylint: disable=broad-except
+            logger.exception("Error found initializing data from email message: %s", email_message)
+        return None

tests/unit/test_data.py

@@ -9,15 +9,21 @@
 dir_path = os.path.dirname(os.path.realpath(__file__))
 
 
-def test_init():
-    """Test the simple init class."""
-    data = NotificationData.init("my_type", b"my_content")
+def test_init_from_raw():
+    """Test the init_data_raw function."""
+    data = NotificationData.init_from_raw("my_type", b"my_content")
     assert isinstance(data, NotificationData)
     assert len(data.data_parts) == 1
     assert data.data_parts[0].type == "my_type"
     assert data.data_parts[0].content == b"my_content"
 
 
+def test_init_from_raw_with_issue():
+    """Test the init_data_raw function with issue."""
+    data = NotificationData.init_from_raw({}, {})
+    assert data is None
+
+
 def test_init_from_email_bytes():
     """Test the email data load."""
     with open(Path(dir_path, "data", "email", "test_sample_message.eml"), "rb") as email_file:
@@ -27,6 +33,12 @@ def test_init_from_email_bytes():
     assert len(data.data_parts) == 5
 
 
+def test_init_from_email_with_issue():
+    """Test the init_data_email function with issue."""
+    data = NotificationData.init_from_email_bytes("")
+    assert data is None
+
+
 def test_init_from_emailmessage():
     """Test the emailmessage data load."""
     with open(Path(dir_path, "data", "email", "test_sample_message.eml"), "rb") as email_file:
@@ -36,3 +48,9 @@ def test_init_from_emailmessage():
     data = NotificationData.init_from_emailmessage(email_message)
     assert isinstance(data, NotificationData)
     assert len(data.data_parts) == 5
+
+
+def test_init_from_emailmessage_with_issue():
+    """Test the init_data_emailmessage function with issue."""
+    data = NotificationData.init_from_emailmessage("")
+    assert data is None

tests/unit/test_e2e.py

@@ -295,7 +295,7 @@ def test_provider_get_maintenances(provider_class, test_data_files, result_parse
         with open(data_file, "rb") as file_obj:
             if not data:
                 if data_type in ["ical", "html"]:
-                    data = NotificationData.init(data_type, file_obj.read())
+                    data = NotificationData.init_from_raw(data_type, file_obj.read())
                 elif data_type in ["email"]:
                     data = NotificationData.init_from_email_bytes(file_obj.read())
             else:
@@ -433,7 +433,7 @@ def test_errored_provider_process(provider_class, data_type, data_file, exceptio
 
     with open(data_file, "rb") as file_obj:
         if data_type in ["ical", "html"]:
-            data = NotificationData.init(data_type, file_obj.read())
+            data = NotificationData.init_from_raw(data_type, file_obj.read())
         elif data_type in ["email"]:
             data = NotificationData.init_from_email_bytes(file_obj.read())
 

tests/unit/test_init.py

@@ -1,19 +1,13 @@
 """Tests for generic parser."""
 import os
-from pathlib import Path
-import email
 
 import pytest
 
 from circuit_maintenance_parser import (
     init_provider,
     get_provider_class,
     get_provider_class_from_sender,
-    init_data_raw,
-    init_data_email,
-    init_data_emailmessage,
 )
-from circuit_maintenance_parser.data import NotificationData
 from circuit_maintenance_parser.errors import NonexistentProviderError
 from circuit_maintenance_parser.provider import (
     GenericProvider,
@@ -27,35 +21,6 @@
 dir_path = os.path.dirname(os.path.realpath(__file__))
 
 
-def test_init_data_raw():
-    """Test the init_data_raw function."""
-    data = init_data_raw("my_type", b"my_content")
-    assert isinstance(data, NotificationData)
-    assert len(data.data_parts) == 1
-    assert data.data_parts[0].type == "my_type"
-    assert data.data_parts[0].content == b"my_content"
-
-
-def test_init_data_email():
-    """Test the email data load."""
-    with open(Path(dir_path, "data", "email", "test_sample_message.eml"), "rb") as email_file:
-        email_raw_data = email_file.read()
-    data = init_data_email(email_raw_data)
-    assert isinstance(data, NotificationData)
-    assert len(data.data_parts) == 5
-
-
-def test_init_data_emailmessage():
-    """Test the emailmessage data load."""
-    with open(Path(dir_path, "data", "email", "test_sample_message.eml"), "rb") as email_file:
-        email_raw_data = email_file.read()
-    raw_email_string = email_raw_data.decode("utf-8")
-    email_message = email.message_from_string(raw_email_string)
-    data = init_data_emailmessage(email_message)
-    assert isinstance(data, NotificationData)
-    assert len(data.data_parts) == 5
-
-
 @pytest.mark.parametrize(
     "provider_type, result_type",
     [

tests/unit/test_processors.py

@@ -47,10 +47,11 @@ class FakeParser1(FakeParser):
 
 
 # Fake data used for SimpleProcessor
-fake_data = NotificationData.init("fake_type", b"fake data")
+fake_data = NotificationData.init_from_raw("fake_type", b"fake data")
 # Fake data used for CombinedProcessor
-fake_data_for_combined = NotificationData.init("fake_type_0", b"fake data")
-fake_data_for_combined.data_parts.append(DataPart("fake_type_1", b"fake data"))
+fake_data_for_combined = NotificationData.init_from_raw("fake_type_0", b"fake data")
+if fake_data_for_combined:
+    fake_data_for_combined.data_parts.append(DataPart("fake_type_1", b"fake data"))
 
 
 def test_simpleprocessor():

tests/unit/test_providers.py

@@ -10,7 +10,7 @@
 from circuit_maintenance_parser.parser import Parser
 
 
-fake_data = NotificationData.init("fake_type", b"fake data")
+fake_data = NotificationData.init_from_raw("fake_type", b"fake data")
 
 
 class ProviderWithOneProcessor(GenericProvider):