fix(create): improve validation to prevent endless loop if properties…

… are undefined (DEV-1720) (#297)

src/dsp_tools/utils/project_validate.py

@@ -13,6 +13,99 @@
 from dsp_tools.utils.excel_to_json_lists import expand_lists_from_excel
 
 
+def  check_for_undefined_superproperty(project_definition: dict[str, Any]) -> bool:
+    """
+    Check the superproperties that claim to point to a property defined in the same JSON project.
+    Check if the property they point to actually exists.
+    (DSP base properties and properties from other ontologies are not considered.)
+
+    Args:
+        project_definition: parsed JSON project definition
+
+    Raises:
+        BaseError: detailed error message if a superproperty is not existent
+
+    Returns:
+        True if the superproperties are valid
+    """
+    errors: dict[str, list[str]] = dict()
+    for onto in project_definition["project"]["ontologies"]:
+        ontoname = onto["name"]
+        propnames = [p["name"] for p in onto["properties"]]
+        for prop in onto["properties"]:
+            supers = prop["super"]
+            # form of supers:
+            #  - isSequenceOf  # DSP base property
+            #  - other:prop    # other onto
+            #  - same:prop     # same onto
+            #  - :prop         # same onto (short form)
+
+            # filter out DSP base properties
+            supers = [s for s in supers if ":" in s]
+            # extend short form
+            supers = [regex.sub(r"^:", f"{ontoname}:", s) for s in supers]
+            # filter out other ontos
+            supers = [s for s in supers if regex.search(f"^{ontoname}:", s)]
+            # convert to short form
+            supers = [regex.sub(ontoname, "", s) for s in supers]
+
+            invalid_references = [s for s in supers if regex.sub(":", "", s) not in propnames]
+            if invalid_references:
+                errors[f"Ontology '{ontoname}', property '{prop['name']}'"] = invalid_references
+
+    if errors:
+        err_msg = "Your data model contains properties that are derived from an invalid super-property:\n"
+        err_msg += "\n".join(f" - {loc}: {invalids}" for loc, invalids in errors.items())
+        raise BaseError(err_msg)
+    return True
+
+
+def check_for_undefined_cardinalities(project_definition: dict[str, Any]) -> bool:
+    """
+    Check if the propnames that are used in the cardinalities of each resource are defined in the "properties" 
+    section. (DSP base properties and properties from other ontologies are not considered.)
+
+    Args:
+        project_definition: parsed JSON project definition
+
+    Raises:
+        BaseError: detailed error message if a cardinality is used that is not defined
+
+    Returns:
+        True if all cardinalities are defined in the "properties" section
+    """
+    errors: dict[str, list[str]] = dict()
+    for onto in project_definition["project"]["ontologies"]:
+        ontoname = onto["name"]
+        propnames = [prop["name"] for prop in onto["properties"]]
+        for res in onto["resources"]:
+            cardnames = [card["propname"] for card in res["cardinalities"]]
+            # form of the cardnames:
+            #  - isSequenceOf  # DSP base property
+            #  - other:prop    # other onto
+            #  - same:prop     # same onto
+            #  - :prop         # same onto (short form)
+
+            # filter out DSP base properties
+            cardnames = [card for card in cardnames if ":" in card]
+            # extend short form
+            cardnames = [regex.sub(r"^:", f"{ontoname}:", card) for card in cardnames]
+            # filter out other ontos
+            cardnames = [card for card in cardnames if regex.search(f"^{ontoname}:", card)]
+            # convert to short form
+            cardnames = [regex.sub(ontoname, "", card) for card in cardnames]
+
+            invalid_cardnames = [card for card in cardnames if regex.sub(":", "", card) not in propnames]
+            if invalid_cardnames:
+                errors[f"Ontology '{ontoname}', resource '{res['name']}'"] = invalid_cardnames
+
+    if errors:
+        err_msg = "Your data model contains cardinalities with invalid propnames:\n"
+        err_msg += "\n".join(f" - {loc}: {invalids}" for loc, invalids in errors.items())
+        raise BaseError(err_msg)
+    return True
+
+
 def validate_project(
     input_file_or_json: Union[dict[str, Any], str],
     expand_lists: bool = True
@@ -28,8 +121,11 @@ def validate_project(
         input_file_or_json: the project to be validated, can either be a file path or a parsed JSON file
         expand_lists: if True, the Excel file references in the "lists" section will be expanded
 
+    Raises:
+        BaseError: detailed error report if the validation doesn't pass
+
     Returns:
-        True if the project passed validation. Otherwise, a BaseError with a detailed error report is raised.
+        True if the project passed validation. 
     """
 
     if isinstance(input_file_or_json, dict) and "project" in input_file_or_json:
@@ -52,11 +148,17 @@ def validate_project(
         project_schema = json.load(schema_file)
     try:
         jsonschema.validate(instance=project_definition, schema=project_schema)
-    except jsonschema.exceptions.ValidationError as err:
+    except jsonschema.ValidationError as err:
         raise BaseError(f"The JSON project file cannot be created due to the following validation error: {err.message}.\n"
                         f"The error occurred at {err.json_path}:\n"
                         f"{err.instance}")
 
+    # make sure that there is no undefined superproperty
+    check_for_undefined_superproperty(project_definition)
+
+    # make sure that every cardinality was defined in the properties section
+    check_for_undefined_cardinalities(project_definition)
+
     # cardinalities check for circular references
     return _check_cardinalities_of_circular_references(project_definition)
 
@@ -69,10 +171,12 @@ def _check_cardinalities_of_circular_references(project_definition: dict[Any, An
 
     Args:
         project_definition: dictionary with a DSP project (as defined in a JSON project file)
+    
+    Raises:
+        BaseError: if there is a circle with at least one element that has a cardinality of "1" or "1-n"
 
     Returns:
         True if no circle was detected, or if all elements of all circles are of cardinality "0-1" or "0-n".
-        False if there is a circle with at least one element that has a cardinality of "1" or "1-n".
     """
 
     link_properties = _collect_link_properties(project_definition)

test/unittests/test_create_project.py

@@ -6,17 +6,27 @@
 from dsp_tools.models.helpers import BaseError
 from dsp_tools.utils.project_create import _sort_resources, _sort_prop_classes
 from dsp_tools.utils.project_validate import _collect_link_properties, _identify_problematic_cardinalities, \
-    validate_project
+    validate_project, check_for_undefined_cardinalities, check_for_undefined_superproperty
 
 
 class TestProjectCreation(unittest.TestCase):
+
     test_project_systematic_file = "testdata/test-project-systematic.json"
-    with open(test_project_systematic_file, "r") as json_file:
+    with open(test_project_systematic_file) as json_file:
         test_project_systematic: dict[str, Any] = json.load(json_file)
         test_project_systematic_ontology: dict[str, Any] = test_project_systematic["project"]["ontologies"][0]
+
     test_project_circular_ontology_file = "testdata/invalid_testdata/test-project-circular-ontology.json"
-    with open(test_project_circular_ontology_file, "r") as json_file:
+    with open(test_project_circular_ontology_file) as json_file:
         test_project_circular_ontology: dict[str, Any] = json.load(json_file)
+
+    test_project_undefined_cardinality_file = "testdata/invalid_testdata/test-project-cardinalities-that-were-not-defined-in-properties-section.json"
+    with open(test_project_undefined_cardinality_file) as json_file:
+        test_project_undefined_cardinality = json.load(json_file)
+
+    test_project_undefined_super_property_file = "testdata/invalid_testdata/test-project-super-property-that-was-not-defined-in-properties-section.json"
+    with open(test_project_undefined_super_property_file) as json_file:
+        test_project_undefined_super_property = json.load(json_file)
 
 
     def test_sort_resources(self) -> None:
@@ -72,5 +82,25 @@ def test_circular_references_in_onto(self) -> None:
         self.assertListEqual(sorted(errors), sorted(expected_errors))
 
 
+    def test_check_for_undefined_cardinalities(self) -> None:
+        self.assertTrue(check_for_undefined_cardinalities(self.test_project_systematic))
+        with self.assertRaisesRegex(
+            BaseError, 
+            r"Your data model contains cardinalities with invalid propnames:\n"
+            r" - Ontology 'testonto', resource 'TestThing': \[':hasText'\]"
+        ):
+            check_for_undefined_cardinalities(self.test_project_undefined_cardinality)
+
+
+    def test_check_for_undefined_superproperty(self) -> None:
+        self.assertTrue(check_for_undefined_superproperty(self.test_project_systematic))
+        with self.assertRaisesRegex(
+            BaseError, 
+            r"Your data model contains properties that are derived from an invalid super-property:\n"
+            r" - Ontology 'testonto', property 'hasSimpleText': \[':hasText'\]"
+        ):
+            check_for_undefined_superproperty(self.test_project_undefined_super_property)
+
+
 if __name__ == "__main__":
     unittest.main()

testdata/invalid_testdata/test-project-cardinalities-that-were-not-defined-in-properties-section.json

@@ -0,0 +1,54 @@
+{
+    "$schema": "../../src/dsp_tools/schemas/project.json",
+    "project": {
+        "shortcode": "4123",
+        "shortname": "tp",
+        "longname": "test project",
+        "descriptions": {
+            "en": "A systematic test project"
+        },
+        "keywords": [
+            "test"
+        ],
+        "ontologies": [
+            {
+                "name": "testonto",
+                "label": "Test ontology",
+                "properties": [
+                    {
+                        "name": "hasSimpleText",
+                        "super": [
+                            "hasValue"
+                        ],
+                        "object": "TextValue",
+                        "labels": {
+                            "en": "hasSimpleText"
+                        },
+                        "gui_element": "SimpleText"
+                    }
+                ],
+                "resources": [
+                    {
+                        "name": "TestThing",
+                        "super": [
+                            "Resource"
+                        ],
+                        "labels": {
+                            "en": "TestThing"
+                        },
+                        "cardinalities": [
+                            {
+                                "propname": ":hasText",
+                                "cardinality": "1-n"
+                            },
+                            {
+                                "propname": ":hasSimpleText",
+                                "cardinality": "0-n"
+                            }
+                        ]
+                    }
+                ]
+            }
+        ]
+    }
+}

testdata/invalid_testdata/test-project-super-property-that-was-not-defined-in-properties-section.json

@@ -0,0 +1,65 @@
+{
+    "$schema": "../../src/dsp_tools/schemas/project.json",
+    "project": {
+        "shortcode": "4123",
+        "shortname": "tp",
+        "longname": "test project",
+        "descriptions": {
+            "en": "A systematic test project"
+        },
+        "keywords": [
+            "test"
+        ],
+        "ontologies": [
+            {
+                "name": "testonto",
+                "label": "Test ontology",
+                "properties": [
+                    {
+                        "name": "hasSimpleText",
+                        "super": [
+                            ":hasText"
+                        ],
+                        "object": "TextValue",
+                        "labels": {
+                            "en": "hasSimpleText"
+                        },
+                        "gui_element": "SimpleText"
+                    },
+                    {
+                        "name": "hasRichtext",
+                        "super": [
+                            "hasValue"
+                        ],
+                        "object": "TextValue",
+                        "labels": {
+                            "en": "hasRichtext"
+                        },
+                        "gui_element": "Richtext"
+                    }
+                ],
+                "resources": [
+                    {
+                        "name": "TestThing",
+                        "super": [
+                            "Resource"
+                        ],
+                        "labels": {
+                            "en": "TestThing"
+                        },
+                        "cardinalities": [
+                            {
+                                "propname": ":hasSimpleText",
+                                "cardinality": "1-n"
+                            },
+                            {
+                                "propname": ":hasRichtext",
+                                "cardinality": "0-n"
+                            }
+                        ]
+                    }
+                ]
+            }
+        ]
+    }
+}

testdata/test-project-systematic.json

@@ -523,7 +523,7 @@
                         },
                         "cardinalities": [
                             {
-                                "propname": ":hasText",
+                                "propname": "testonto:hasText",
                                 "cardinality": "1-n"
                             },
                             {
@@ -914,6 +914,11 @@
                                 "propname": ":hasText",
                                 "gui_order": 1,
                                 "cardinality": "0-n"
+                            },
+                            {
+                                "propname": "testonto:hasText",
+                                "gui_order": 2,
+                                "cardinality": "0-n"
                             }
                         ]
                     },