fix(onto creation): prevent sorting algorith from modifying original …

…ontology (#169) * bugfix * rename unsorted_resources_copy * improve code readability, add multiple inherited resources and properties to test data
dasch-swiss · Mar 25, 2022 · 9a9e5f0 · 9a9e5f0
1 parent 2ebece3
commit 9a9e5f0
Show file tree

Hide file tree

Showing 3 changed files with 81 additions and 17 deletions.
diff --git a/knora/dsplib/utils/onto_create_ontology.py b/knora/dsplib/utils/onto_create_ontology.py
@@ -311,11 +311,15 @@ def sort_resources(unsorted_resources: list[dict[str, Any]], onto_name: str) ->
     Returns:
         sorted list of resource classes
     """
-
+
+    # do not modify the original unsorted_resources, which points to the original onto file
+    resources_to_sort = unsorted_resources.copy()
     sorted_resources: list[dict[str, Any]] = list()
     ok_resource_names: list[str] = list()
-    while len(unsorted_resources) > 0:
-        for res in unsorted_resources.copy():
+    while len(resources_to_sort) > 0:
+        # inside the for loop, resources_to_sort is modified, so a copy must be made
+        # to iterate over
+        for res in resources_to_sort.copy():
             res_name = f'{onto_name}:{res["name"]}'
             parent_classes = res['super']
             if isinstance(parent_classes, str):
@@ -325,7 +329,7 @@ def sort_resources(unsorted_resources: list[dict[str, Any]], onto_name: str) ->
             if all(parent_classes_ok):
                 sorted_resources.append(res)
                 ok_resource_names.append(res_name)
-                unsorted_resources.remove(res)
+                resources_to_sort.remove(res)
     return sorted_resources
 
 
@@ -342,10 +346,14 @@ def sort_prop_classes(unsorted_prop_classes: list[dict[str, Any]], onto_name: st
             sorted list of properties
         """
 
+    # do not modify the original unsorted_prop_classes, which points to the original onto file
+    prop_classes_to_sort = unsorted_prop_classes.copy()
     sorted_prop_classes: list[dict[str, Any]] = list()
     ok_propclass_names: list[str] = list()
-    while len(unsorted_prop_classes) > 0:
-        for prop in unsorted_prop_classes.copy():
+    while len(prop_classes_to_sort) > 0:
+        # inside the for loop, resources_to_sort is modified, so a copy must be made
+        # to iterate over
+        for prop in prop_classes_to_sort.copy():
             prop_name = f'{onto_name}:{prop["name"]}'
             parent_classes = prop.get('super', 'hasValue')
             if isinstance(parent_classes, str):
@@ -355,7 +363,7 @@ def sort_prop_classes(unsorted_prop_classes: list[dict[str, Any]], onto_name: st
             if all(parent_classes_ok):
                 sorted_prop_classes.append(prop)
                 ok_propclass_names.append(prop_name)
-                unsorted_prop_classes.remove(prop)
+                prop_classes_to_sort.remove(prop)
     return sorted_prop_classes
 
 
@@ -626,6 +634,8 @@ def create_ontology(input_file: str,
                                 cardinality=cardinality,
                                 gui_order=card_info.get("gui_order"),
                                 last_modification_date=last_modification_date)
+                            if verbose:
+                                print(f'{res_class["name"]}: Added property {prop_name_for_card}')
 
                         except BaseError as err:
                             print(

diff --git a/testdata/test-data.xml b/testdata/test-data.xml
@@ -47,18 +47,11 @@
             <text permissions="prop-restricted" encoding="utf8">Nochmals ein einfacher Text</text>
         </text-prop>
         <text-prop name=":hasRichtext">
-            <text permissions="prop-default" encoding="xml">This is
-                <em>bold and
-                    <strong>string</strong>
-                </em>
-                text!
-            </text>
+            <text permissions="prop-default" encoding="xml">This is<em>bold and<strong>string</strong></em>text!</text>
         </text-prop>
-
         <uri-prop name=":hasUri">
             <uri permissions="prop-default">https://dasch.swiss</uri>
         </uri-prop>
-
         <boolean-prop name=":hasBoolean">
             <boolean permissions="prop-default">true</boolean>
         </boolean-prop>
@@ -147,7 +140,6 @@
         </integer-prop>
     </resource>
 
-
     <resource label="Partofthing-3"
               restype=":PartOfThing"
               id="obj_0006"
@@ -201,4 +193,16 @@
         </text-prop>
     </resource>
 
+    <resource label="TestThing2Child1" restype=":TestThing2Child1" id="obj_0011">
+        <text-prop name=":hasTextChild1">
+            <text encoding="utf8">Some text</text>
+        </text-prop>
+    </resource>
+
+    <resource label="TestThing2Child2" restype=":TestThing2Child2" id="obj_0012">
+        <text-prop name=":hasTextChild2">
+            <text encoding="utf8">Some text</text>
+        </text-prop>
+    </resource>
+
 </knora>
diff --git a/testdata/test-onto.json b/testdata/test-onto.json
@@ -168,6 +168,28 @@
                 "name": "testonto",
                 "label": "Test ontology",
                 "properties": [
+                    {
+                        "name": "hasTextChild2",
+                        "super": [
+                            ":hasTextChild1"
+                        ],
+                        "object": "TextValue",
+                        "labels": {
+                            "en": "Text"
+                        },
+                        "gui_element": "SimpleText"
+                    },
+                    {
+                        "name": "hasTextChild1",
+                        "super": [
+                            ":hasText"
+                        ],
+                        "object": "TextValue",
+                        "labels": {
+                            "en": "Text"
+                        },
+                        "gui_element": "SimpleText"
+                    },
                     {
                         "name": "hasText",
                         "super": [
@@ -187,7 +209,7 @@
                             "size": 80
                         }
                     },
-                    {
+                                        {
                         "name": "hasRichtext",
                         "super": [
                             "hasValue"
@@ -426,6 +448,34 @@
                             }
                         ]
                     },
+                    {
+                        "name": "TestThing2Child2",
+                        "super": ":TestThing2Child1",
+                        "labels": {
+                            "en": "TestThing2Child2"
+                        },
+                        "cardinalities": [
+                            {
+                                "propname": ":hasTextChild2",
+                                "gui_order": 1,
+                                "cardinality": "1"
+                            }
+                        ]
+                    },
+                    {
+                        "name": "TestThing2Child1",
+                        "super": ":TestThing2",
+                        "labels": {
+                            "en": "TestThing2Child1"
+                        },
+                        "cardinalities": [
+                            {
+                                "propname": ":hasTextChild1",
+                                "gui_order": 1,
+                                "cardinality": "1"
+                            }
+                        ]
+                    },
                     {
                         "name": "TestThing2",
                         "super": "Resource",