Merge pull request #839 from simphony/dev

Merge release 3.9.0

.github/workflows/ci.yml

@@ -4,7 +4,7 @@ on: pull_request
 
 jobs:
   PEP8:
-    runs-on: self-hosted
+    runs-on: [self-hosted, simphony-osp]
     steps:
       - uses: actions/checkout@v3
 
@@ -16,7 +16,7 @@ jobs:
         run: pre-commit run --all-files
 
   complexity:
-    runs-on: self-hosted
+    runs-on: [self-hosted, simphony-osp]
     steps:
       - uses: actions/checkout@v3
 
@@ -26,15 +26,15 @@ jobs:
           radon mi -s .
 
   security:
-    runs-on: self-hosted
+    runs-on: [self-hosted, simphony-osp]
     steps:
       - uses: actions/checkout@v3
 
       - name: Bandit
         run: bandit -r osp --skip B101
 
   testing:
-    runs-on: self-hosted
+    runs-on: [self-hosted, simphony-osp]
     steps:
       - uses: actions/checkout@v3
       - name: tox

.pre-commit-config.yaml

@@ -1,6 +1,6 @@
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.1.0
+    rev: v4.4.0
     hooks:
       - id: check-json
       - id: check-yaml
@@ -17,7 +17,7 @@ repos:
         args: [--profile, black, --filter-files, --line-length, "79"]
 
   - repo: https://github.com/psf/black
-    rev: 22.3.0
+    rev: 22.10.0
     hooks:
       - id: black
         args: [--line-length, "79"]

examples/example_rdf_import_export.py

@@ -38,7 +38,7 @@
 )
 
 # Export from Core Session
-export_cuds(path="test.rdf", format="ttl")
+export_cuds(file="test.rdf", format="ttl")
 
 # Check output
 with open("test.rdf", encoding="utf-8") as f:
@@ -50,7 +50,7 @@
 with SqliteSession(path="test.db") as session:
     w = city.CityWrapper(session=session)
     w.add(c)
-    export_cuds(session, path="test.rdf", format="ttl")
+    export_cuds(session, file="test.rdf", format="ttl")
 
     # Check output
     with open("test.rdf", encoding="utf-8") as f:

osp/core/ontology/docs/foaf.yml

@@ -1,6 +1,6 @@
 ---
 identifier: foaf
-ontology_file: http://xmlns.com/foaf/spec/index.rdf
+ontology_file: "https://web.archive.org/web/20220614185720if_/http://xmlns.com/foaf/spec/index.rdf"
 reference_by_label: False
 namespaces:
     foaf: "http://xmlns.com/foaf/0.1/"

osp/core/utils/schema_validation.py

@@ -5,6 +5,8 @@
 import yaml
 
 from osp.core.namespaces import get_entity
+from osp.core.ontology import OntologyAttribute, OntologyRelationship
+from osp.core.ontology.datatypes import YML_DATATYPES
 
 logger = logging.getLogger(__name__)
 
@@ -17,7 +19,7 @@ class CardinalityError(Exception):
     """A cardinality constraint is violated."""
 
 
-def validate_tree_against_schema(root_obj, schema_file):
+def validate_tree_against_schema(root_obj, schema_file, strict_check=False):
     """Test cardinality constraints on given CUDS tree.
 
     The tree that starts at root_obj.
@@ -27,6 +29,8 @@ def validate_tree_against_schema(root_obj, schema_file):
         root_obj (Cuds): The root CUDS object of the tree
         schema_file (str): The path to the schema file that
             defines the constraints
+        strict_check (bool): whether extra cuds not listed in
+            the schema_file should be tolerated or not
 
     Raise:
         Exception: Tells the user which constraint was violated
@@ -67,9 +71,12 @@ def validate_tree_against_schema(root_obj, schema_file):
         try:
             relationships = data_model_dict["model"][oclass]
         except KeyError:
-            # TODO ask Yoav: is it ok when there is an object
-            # in the tree that is not part of the datamodel?
-            continue
+            if strict_check:
+                message = f"An entity for {oclass} was found,"
+                " but it is not part of the provided schema"
+                raise ConsistencyError(message)
+            else:
+                continue
         if relationships is None:
             # if there are no relationships defined,
             # the only constraint is that the object exists
@@ -96,15 +103,28 @@ def _load_data_model_from_yaml(data_model_file):
 
 
 def _check_cuds_object_cardinality(origin_cuds, dest_oclass, rel, constraints):
+    rel_entity = get_entity(rel)
 
-    actual_cardinality = len(
-        origin_cuds.get(rel=get_entity(rel), oclass=get_entity(dest_oclass))
-    )
+    if type(rel_entity) == OntologyRelationship:
+        actual_cardinality = len(
+            origin_cuds.get(rel=rel_entity, oclass=get_entity(dest_oclass))
+        )
+    elif type(rel_entity) == OntologyAttribute:
+        # No datatype checking since this is already done when Cuds are
+        # instantiated or imported from a file
+        actual_cardinality = (
+            1 if rel_entity in origin_cuds.get_attributes() else 0
+        )
+    else:
+        raise ConsistencyError(
+            f"Relation '{rel}' not supported for {origin_cuds.oclass}."
+        )
 
     min, max = _interpret_cardinality_value_from_constraints(constraints)
     if actual_cardinality < min or actual_cardinality > max:
-        message = """Found invalid cardinality between {} and {} with relationship {}.
-        The constraint says it should be between {} and {}, but we found {}.
+        message = """Found invalid cardinality between {} and {}
+        with relationship {}. The constraint says it should be
+        between {} and {}, but we found {}.
         The uid of the affected cuds_object is: {}""".format(
             str(origin_cuds.oclass),
             dest_oclass,
@@ -116,6 +136,64 @@ def _check_cuds_object_cardinality(origin_cuds, dest_oclass, rel, constraints):
         )
         raise CardinalityError(message)
 
+    _check_attribute_contraints(
+        origin_cuds, rel_entity, dest_oclass, constraints
+    )
+
+
+def _check_attribute_contraints(
+    origin_cuds, rel_entity, dest_oclass, constraints
+):
+    attribute = origin_cuds.get_attributes().get(rel_entity)
+    value = constraints.get("value")
+    if attribute:
+        if value and attribute != value:
+            message = """Found invalid attribute value
+            between {} and {} with relationship {}.
+            The constraint says it should be valued '{}',
+            but we found '{}'. The uid of the affected
+            cuds_object is: {}""".format(
+                str(origin_cuds.oclass),
+                dest_oclass,
+                rel_entity,
+                value,
+                attribute,
+                origin_cuds.uid,
+            )
+            raise ConsistencyError(message)
+
+        if type(attribute) == str:
+            attribute = len(attribute)
+            target = "length"
+        else:
+            target = "range"
+        min, max = _interpret_attribute_from_constraints(constraints, target)
+        if attribute < min or attribute > max:
+            message = """Found invalid attribute value {} between {} and {}
+            relationship {}. The constraint says it should be between {}
+            and {}, but we found {}. The uid of the affected
+            cuds_object is: {}""".format(
+                target,
+                str(origin_cuds.oclass),
+                dest_oclass,
+                rel_entity,
+                min,
+                max,
+                attribute,
+                origin_cuds.uid,
+            )
+            raise CardinalityError(message)
+
+
+def _interpret_attribute_from_constraints(constraints, range_or_len: str):
+    min = -float("inf")
+    if constraints is not None:
+        value = constraints.get(range_or_len)
+        min, max = _interpret_cardinality_value_from_constraints(
+            dict(cardinality=value)
+        )
+    return min, max
+
 
 def _interpret_cardinality_value_from_constraints(constraints):
     # default is arbitrary
@@ -126,11 +204,12 @@ def _interpret_cardinality_value_from_constraints(constraints):
         if isinstance(cardinality_value, int):
             min = cardinality_value
             max = cardinality_value
-        elif "-" in cardinality_value:
-            min = int(cardinality_value.split("-")[0])
-            max = int(cardinality_value.split("-")[1])
-        elif "+" in cardinality_value:
-            min = int(cardinality_value.split("+")[0])
+        elif isinstance(cardinality_value, str):
+            if "-" in cardinality_value:
+                min = int(cardinality_value.split("-")[0])
+                max = int(cardinality_value.split("-")[1])
+            elif "+" in cardinality_value:
+                min = int(cardinality_value.split("+")[0])
     return min, max
 
 
@@ -167,9 +246,9 @@ def _get_optional_and_mandatory_subtrees(data_model_dict):
                 min, max = _interpret_cardinality_value_from_constraints(
                     constraints
                 )
-                if min == 0:
+                if min == 0 and neighbor not in YML_DATATYPES.keys():
                     optional_subtrees.add(neighbor)
-                if min > 0:
+                if min > 0 and neighbor not in YML_DATATYPES.keys():
                     mandatory_subtrees.add(neighbor)
 
     if optional_subtrees & mandatory_subtrees:

packageinfo.py

@@ -1,4 +1,4 @@
 """Information about the package."""
 
 NAME = "osp-core"
-VERSION = "3.8.0"
+VERSION = "3.9.0"

setup.py

@@ -66,7 +66,7 @@
         "PyYaml",
         "rdflib >= 6.0.0, < 7.0.0; python_version >= '3.7'",
         "requests",
-        "websockets < 11",
+        "websockets >= 9, < 11",
         "websockets >= 10; python_version >= '3.10'",
         # ↓ --- Python 3.6 support. --- ↓ #
         "pyparsing < 3.0.0; python_version < '3.7'",

tests/test_installation.py

@@ -189,9 +189,13 @@ def test_conflicting_labels(self):
         label for multiple IRIs. An error is only raised on installation if the
         reference_by_label option is set to Tue in the yml file.
         """
-        FOAF = """
+        FOAF_URL = (
+            "https://web.archive.org/web/20220614185720if_/"
+            "http://xmlns.com/foaf/spec/index.rdf"
+        )
+        FOAF = f"""
         identifier: foaf_TEST
-        ontology_file: http://xmlns.com/foaf/spec/index.rdf
+        ontology_file: "{FOAF_URL}"
         reference_by_label: True
         namespaces:
             foaf_TEST: "http://xmlns.com/foaf/0.1/"

tests/test_utils.py

@@ -134,7 +134,17 @@ def test_validate_tree_against_schema(self):
             "test_validation_schema_city_with_optional_subtree.yml",
         )
 
-        c = city.City(name="freiburg")
+        schema_file_with_attribute = os.path.join(
+            os.path.dirname(__file__),
+            "test_validation_schema_city_with_attribute.yml",
+        )
+
+        schema_file_with_attribute_value = os.path.join(
+            os.path.dirname(__file__),
+            "test_validation_schema_city_with_attribute_value.yml",
+        )
+
+        c = city.City(name="Freiburg")
 
         # empty city is not valid
         self.assertRaises(
@@ -188,6 +198,26 @@ def test_validate_tree_against_schema(self):
             schema_file_with_missing_entity,
         )
 
+        # now we validate the attributes and their cardinality
+        validate_tree_against_schema(c, schema_file_with_attribute)
+
+        # additionally we check the length and the value of the attribute
+        validate_tree_against_schema(c, schema_file_with_attribute_value)
+
+        # and if there are more objects in tree than in the schema
+        # it can be specified if the test should be done strictly
+        c.add(wrong_object, rel=city.hasPart)
+        # first no strict check - additional cuds is tolerated:
+        validate_tree_against_schema(c, schema_file_with_attribute_value)
+        # second with strict check - additional cuds is not tolerated:
+        self.assertRaises(
+            ConsistencyError,
+            validate_tree_against_schema,
+            c,
+            schema_file_with_attribute_value,
+            strict_check=True,
+        )
+
     def test_branch(self):
         """Test the branch function."""
         x = branch(

tests/test_validation_schema_city_with_attribute.yml

@@ -0,0 +1,33 @@
+version: "1.0.0"
+
+oclass: city.City
+
+model:
+    city.City:
+        city.name:
+            STRING:
+                cardinality: 1
+        city.hasInhabitant:
+            city.Citizen:
+                cardinality: 1-2
+        city.hasPart:
+            city.Neighborhood:
+                cardinality: 1
+
+    city.Neighborhood:
+        city.name:
+            STRING:
+                cardinality: 1
+        city.hasPart:
+            city.Street:
+                cardinality: 1+
+
+    city.Street:
+        city.name:
+            STRING:
+                cardinality: 1
+
+    city.Citizen:
+        city.name:
+            STRING:
+                cardinality: 1