[issue-305, review] refactor parse_field_or_log_error

Signed-off-by: Meret Behrens <meret.behrens@tngtech.com>

src/parser/json/annotation_parser.py

@@ -51,9 +51,8 @@ def parse_annotations_from_object(self, annotations: List[Annotation], element_l
             element_spdx_id: Optional[str] = element.get("SPDXID")
             element_annotations: List[Dict] = element.get("annotations", [])
             annotations.extend(parse_field_or_log_error(self.logger, element_annotations,
-                                                        lambda x: parse_list_of_elements(
-                                                            lambda y: self.parse_annotation(y, spdx_id=element_spdx_id),
-                                                            x, self.logger), default=[]))
+                                                        lambda y: self.parse_annotation(y, spdx_id=element_spdx_id),
+                                                        [], True))
 
     def parse_annotation(self, annotation_dict: Dict, spdx_id: Optional[str] = None) -> Annotation:
         logger = Logger()

src/parser/json/dict_parsing_functions.py

@@ -43,11 +43,15 @@ def construct_or_raise_parsing_error(object_to_construct: Any, args_for_construc
     return constructed_object
 
 
-def parse_field_or_log_error(logger: Logger, field: Any, parsing_method: Callable = lambda x: x, default=None) -> Any:
+def parse_field_or_log_error(logger: Logger, field: Any, parsing_method: Callable = lambda x: x, default: Any = None,
+                             field_is_list: bool = False) -> Any:
     if not field:
         return default
     try:
-        return parsing_method(field)
+        if field_is_list:
+            return parse_list_of_elements(field, parsing_method)
+        else:
+            return parsing_method(field)
     except SPDXParsingError as err:
         logger.extend(err.get_messages())
         return default
@@ -81,14 +85,15 @@ def parse_field_or_no_assertion_or_none(field: Optional[str], method_for_field:
         return method_for_field(field)
 
 
-def parse_field_or_no_assertion(field: Optional[str], method_for_field: Callable = lambda x: x) -> Union[SpdxNoAssertion, Any]:
+def parse_field_or_no_assertion(field: Optional[str], method_for_field: Callable = lambda x: x) -> Union[
+    SpdxNoAssertion, Any]:
     if field == SpdxNoAssertion().__str__():
         return SpdxNoAssertion()
     else:
         return method_for_field(field)
 
 
-def parse_list_of_elements(method_to_parse_element, list_of_elements, logger=None):
+def parse_list_of_elements(list_of_elements: List[Dict], method_to_parse_element: Callable, logger=None):
     if not logger:
         logger = Logger()
     parsed_elements = []

src/parser/json/file_parser.py

@@ -38,9 +38,7 @@ def parse_file(self, file_dict: Dict) -> Optional[File]:
         name: Optional[str] = file_dict.get("fileName")
         spdx_id: Optional[str] = file_dict.get("SPDXID")
         checksums_list: List[Dict] = file_dict.get("checksums")
-        checksums: List[Checksum] = parse_field_or_log_error(logger, checksums_list,
-                                                             lambda x: parse_list_of_elements(
-                                                                 self.checksum_parser.parse_checksum, x))
+        checksums: List[Checksum] = parse_field_or_log_error(logger, checksums_list, self.checksum_parser.parse_checksum, field_is_list=True)
 
         attribution_texts: List[str] = file_dict.get("attributionTexts", [])
         comment: Optional[str] = file_dict.get("comment")

src/parser/json/json_parser.py

@@ -50,13 +50,21 @@ def parse(self, filename: str) -> Document:
             input_doc_as_dict = json.load(file)
 
         fields_to_parse = [("creation_info", input_doc_as_dict, self.creation_info_parser.parse_creation_info, False),
-                           ("packages", input_doc_as_dict.get("packages"), lambda x: parse_list_of_elements(self.package_parser.parse_package, x, self.package_parser.logger), True),
-                           ("files", input_doc_as_dict.get("files"), lambda x: parse_list_of_elements(self.file_parser.parse_file, x, self.file_parser.logger), True),
+                           ("packages", input_doc_as_dict.get("packages"), lambda x: parse_list_of_elements(x,
+                                                                                                            self.package_parser.parse_package,
+                                                                                                            self.package_parser.logger), True),
+                           ("files", input_doc_as_dict.get("files"), lambda x: parse_list_of_elements(x,
+                                                                                                      self.file_parser.parse_file,
+                                                                                                      self.file_parser.logger), True),
                            ("annotations", input_doc_as_dict, self.annotation_parser.parse_all_annotations, True),
-                           ("snippets", input_doc_as_dict.get("snippets"), lambda x: parse_list_of_elements(self.snippet_parser.parse_snippet, x, self.snippet_parser.logger), True),
+                           ("snippets", input_doc_as_dict.get("snippets"), lambda x: parse_list_of_elements(x,
+                                                                                                            self.snippet_parser.parse_snippet,
+                                                                                                            self.snippet_parser.logger), True),
                            ("relationships", input_doc_as_dict, self.relationship_parser.parse_all_relationships, True),
                            ("extracted_licensing_info", input_doc_as_dict.get("hasExtractedLicensingInfos"),
-                            lambda x: parse_list_of_elements(self.extracted_licensing_info_parser.parse_extracted_licensing_info, x, self.extracted_licensing_info_parser.logger), True)]
+                            lambda x: parse_list_of_elements(x,
+                                                             self.extracted_licensing_info_parser.parse_extracted_licensing_info,
+                                                             self.extracted_licensing_info_parser.logger), True)]
 
         parsed_fields = {}
 

src/parser/json/package_parser.py

@@ -48,8 +48,7 @@ def parse_package(self, package_dict: Dict) -> Package:
         built_date: Optional[datetime] = parse_field_or_log_error(logger, package_dict.get("builtDate"),
                                                                   datetime_from_str)
 
-        checksums = parse_field_or_log_error(logger, package_dict.get("checksums"),
-                                             lambda x: parse_list_of_elements(self.checksum_parser.parse_checksum, x))
+        checksums = parse_field_or_log_error(logger, package_dict.get("checksums"), self.checksum_parser.parse_checksum, field_is_list=True)
         comment: Optional[str] = package_dict.get("comment")
         copyright_text: Optional[str] = package_dict.get("copyrightText")
         description: Optional[str] = package_dict.get("description")

src/parser/json/relationship_parser.py

@@ -29,8 +29,7 @@ def parse_all_relationships(self, input_doc_dict: Dict) -> List[Relationship]:
         relationships = []
         relationship_dicts: List[Dict] = input_doc_dict.get("relationships", [])
         relationships.extend(
-            parse_field_or_log_error(self.logger, relationship_dicts,
-                                     lambda x: parse_list_of_elements(self.parse_relationship, x), []))
+            parse_field_or_log_error(self.logger, relationship_dicts, self.parse_relationship, [], True))
 
         document_describes: List[str] = input_doc_dict.get("documentDescribes", [])
         doc_spdx_id: Optional[str] = input_doc_dict.get("SPDXID")

tests/parser/test_file_parser.py

@@ -103,7 +103,7 @@ def test_parse_invalid_files():
              ]
 
     with pytest.raises(SPDXParsingError) as err:
-        parse_list_of_elements(file_parser.parse_file, files)
+        parse_list_of_elements(files, file_parser.parse_file)
     TestCase().assertCountEqual(err.value.get_messages(), [
         "Error while constructing File: ['SetterError File: type of argument " '"checksums" must be a list; got NoneType instead: None\']',
         'Error while constructing File: [\'SetterError File: type of argument "name" ' "must be str; got NoneType instead: None']",

tests/parser/test_package_parser.py

@@ -180,7 +180,7 @@ def test_parse_packages():
     ]
 
     with pytest.raises(SPDXParsingError) as err:
-        parse_list_of_elements(package_parser.parse_package, packages_list)
+        parse_list_of_elements(packages_list, package_parser.parse_package)
 
     TestCase().assertCountEqual(err.value.get_messages(),
                                 ['Error while parsing Package: ["Error while parsing Checksum: '