[issue-47] implement review comments

- rename get_paths method
- make add_path return a mutated copy to comply with frozen property of Resource dataclass

Signed-off-by: Meret Behrens <[email protected]>

src/opossum_lib/file_generation.py

@@ -123,7 +123,7 @@ def generate_json_file_from_tree(tree: DiGraph) -> OpossumInformation:
             ] = _replace_node_ids_with_labels_and_add_resource_type(
                 path, connected_subgraph
             )
-            resources.add_path(path_with_labels)
+            resources = resources.add_path(path_with_labels)
             file_path: str = _create_file_path_from_graph_path(path, connected_subgraph)
             if _node_represents_a_spdx_element(connected_subgraph, node):
                 create_attribution_and_link_with_resource(

src/opossum_lib/merger.py

@@ -89,8 +89,8 @@ def expand_opossum_package_identifier(
 def _merge_resources(resources: List[Resource]) -> Resource:
     merged_resource = Resource(ResourceType.TOP_LEVEL)
     for resource in resources:
-        for path in resource.get_paths_with_resource_types():
-            merged_resource.add_path(path)
+        for path in resource.get_paths_of_all_leaf_nodes_with_types():
+            merged_resource = merged_resource.add_path(path)
     return merged_resource
 
 

src/opossum_lib/opossum_file.py

@@ -3,6 +3,7 @@
 # SPDX-License-Identifier: Apache-2.0
 from __future__ import annotations
 
+from copy import deepcopy
 from dataclasses import dataclass, field
 from enum import Enum, auto
 from typing import Dict, List, Literal, Optional, Tuple, Union
@@ -75,9 +76,10 @@ class Resource:
 
     def add_path(
         self, path_with_resource_types: List[Tuple[str, ResourceType]]
-    ) -> None:
+    ) -> Resource:
+        resource = deepcopy(self)
         if len(path_with_resource_types) == 0:
-            return
+            return resource
         (first, resource_type), rest = (
             path_with_resource_types[0],
             path_with_resource_types[1:],
@@ -88,8 +90,10 @@ def add_path(
                 " the same path differ."
             )
         if first not in self.children:
-            self.children[first] = Resource(type=resource_type)
-        self.children[first].add_path(rest)
+            resource.children[first] = Resource(type=resource_type)
+        resource.children[first] = resource.children[first].add_path(rest)
+
+        return resource
 
     def element_exists_but_resource_type_differs(
         self, element: str, resource_type: ResourceType
@@ -101,7 +105,7 @@ def element_exists_but_resource_type_differs(
     def drop_element(
         self, path_to_element_to_drop: List[Tuple[str, ResourceType]]
     ) -> Resource:
-        paths_in_resource = self.get_paths_with_resource_types()
+        paths_in_resource = self.get_paths_of_all_leaf_nodes_with_types()
         if path_to_element_to_drop not in paths_in_resource:
             raise ValueError(
                 f"Element {path_to_element_to_drop} doesn't exist in resource!"
@@ -113,7 +117,7 @@ def drop_element(
             paths_in_resource.append(path_to_element_to_drop[:-1])
 
             for path_to_element_to_drop in paths_in_resource:
-                resource.add_path(path_to_element_to_drop)
+                resource = resource.add_path(path_to_element_to_drop)
 
             return resource
 
@@ -128,15 +132,17 @@ def to_dict(self) -> Union[int, Dict]:
                 name: resource.to_dict() for name, resource in self.children.items()
             }
 
-    def get_paths_with_resource_types(self) -> List[List[Tuple[str, ResourceType]]]:
+    def get_paths_of_all_leaf_nodes_with_types(
+        self,
+    ) -> List[List[Tuple[str, ResourceType]]]:
         paths = []
         for name, resource in self.children.items():
             path = [(name, resource.type)]
             if resource.has_children():
                 paths.extend(
                     [
                         path + element
-                        for element in resource.get_paths_with_resource_types()
+                        for element in resource.get_paths_of_all_leaf_nodes_with_types()
                     ]
                 )
             else:

tests/test_merge.py

@@ -103,7 +103,7 @@ def test_merge_resources() -> None:
 
     resource = Resource(ResourceType.TOP_LEVEL)
     for path in list_of_paths_with_resource_types:
-        resource.add_path(path)
+        resource = resource.add_path(path)
 
     list_of_paths_with_resource_type = [
         [("A", ResourceType.FOLDER)],
@@ -121,7 +121,7 @@ def test_merge_resources() -> None:
     ]
     resource2 = Resource(ResourceType.TOP_LEVEL)
     for path in list_of_paths_with_resource_type:
-        resource2.add_path(path)
+        resource2 = resource2.add_path(path)
 
     resources = [resource, resource2]
     merged_resource = _merge_resources(resources)

tests/test_opossum_file.py

@@ -20,7 +20,7 @@ def test_resource_to_dict_with_file_as_leaf() -> None:
     resource = Resource(ResourceType.TOP_LEVEL)
 
     for path in list_of_paths:
-        resource.add_path(path)
+        resource = resource.add_path(path)
 
     assert resource.to_dict() == {"A": {"B": {"C": 1}, "D": 1}}
 
@@ -39,7 +39,7 @@ def test_resource_to_dict_with_package_as_leaf() -> None:
     resource = Resource(ResourceType.TOP_LEVEL)
 
     for path in list_of_paths:
-        resource.add_path(path)
+        resource = resource.add_path(path)
 
     assert resource.to_dict() == {"A": {"B": {"C": {}}, "D": {}}}
 
@@ -64,9 +64,9 @@ def test_resource_get_path() -> None:
     resource = Resource(ResourceType.TOP_LEVEL)
 
     for path in list_of_paths:
-        resource.add_path(path)
+        resource = resource.add_path(path)
 
-    assert resource.get_paths_with_resource_types() == [
+    assert resource.get_paths_of_all_leaf_nodes_with_types() == [
         [
             ("A", ResourceType.FOLDER),
             ("B", ResourceType.FILE),
@@ -88,7 +88,7 @@ def test_resource_add_path_throws_err_if_leaf_element_exists_with_different_type
     None
 ):
     resource = Resource(ResourceType.TOP_LEVEL)
-    resource.add_path(
+    resource = resource.add_path(
         [
             ("A", ResourceType.FOLDER),
             ("B", ResourceType.FILE),
@@ -108,7 +108,7 @@ def test_resource_add_path_throws_err_if_leaf_element_exists_with_different_type
 
 def test_resource_add_path_throws_err_if_element_exists_with_different_type() -> None:
     resource = Resource(ResourceType.TOP_LEVEL)
-    resource.add_path(
+    resource = resource.add_path(
         [
             ("A", ResourceType.FOLDER),
             ("B", ResourceType.FILE),
@@ -129,7 +129,7 @@ def test_resource_add_path_throws_err_if_element_exists_with_different_type() ->
 
 def test_resource_drop_element_error() -> None:
     resource = Resource(ResourceType.TOP_LEVEL)
-    resource.add_path(
+    resource = resource.add_path(
         [
             ("A", ResourceType.FOLDER),
             ("B", ResourceType.FILE),
@@ -150,7 +150,7 @@ def test_resource_drop_element_error() -> None:
 
 def test_resource_drop_element_error_not_leaf() -> None:
     resource = Resource(ResourceType.TOP_LEVEL)
-    resource.add_path(
+    resource = resource.add_path(
         [
             ("A", ResourceType.FOLDER),
             ("B", ResourceType.FILE),
@@ -171,7 +171,7 @@ def test_resource_drop_element_error_not_leaf() -> None:
 
 def test_resource_drop_element() -> None:
     resource = Resource(ResourceType.TOP_LEVEL)
-    resource.add_path(
+    resource = resource.add_path(
         [
             ("A", ResourceType.FOLDER),
             ("B", ResourceType.FILE),
@@ -187,6 +187,6 @@ def test_resource_drop_element() -> None:
         ]
     )
 
-    assert resource_without_element.get_paths_with_resource_types() == [
+    assert resource_without_element.get_paths_of_all_leaf_nodes_with_types() == [
         [("A", ResourceType.FOLDER), ("B", ResourceType.FILE)]
     ]