Resolve reverse links using the forward property's path, not its Python name

doc/release_notes.rst

@@ -65,6 +65,13 @@ Bug fixes
   Such a link now gives the same existence query as the object it points to
   (`#145 <https://github.com/HumanBrainProject/fairgraph/issues/145>`_).
 
+- Resolving or counting a reverse property, e.g. ``license.is_applied_to.resolve(client)``, no longer raises
+  :exc:`ValueError` ("Invalid filter") when the linking property has different names in different classes
+  (here ``license`` in :class:`DatasetVersion` but ``licenses`` in :class:`ModelVersion`),
+  and now finds the nodes linking through any of the forward properties a reverse property stands for,
+  where previously only the first was used
+  (`#92 <https://github.com/HumanBrainProject/fairgraph/issues/92>`_).
+
 Connection robustness
 ----------------------
 

fairgraph/kgobject.py

@@ -89,12 +89,6 @@ def __init__(
         self.release_status = release_status
         self.allow_update = True
         super().__init__(data=data, **properties)
-        for prop in self.reverse_properties:
-            if not hasattr(self, prop.name):
-                query = KGQuery(
-                    prop.types, {prop.reverse: self.id}, callback=lambda value: setattr(self, prop.name, value)
-                )
-                setattr(self, prop.name, query)
 
         self._raw_remote_data = None
         self.remote_data = {}

fairgraph/kgquery.py

@@ -22,6 +22,7 @@
 import logging
 from typing import Dict, List, Optional, Union, Any, TYPE_CHECKING
 
+from openminds.properties import Property
 from openminds.registry import lookup
 
 from .utility import as_list, expand_filter
@@ -75,6 +76,38 @@ def __init__(
                 self.classes.append(cls)
         self.filter = filter
         self.preferred_release_status = preferred_release_status
+        # set by from_reverse_property(), where the filter differs between classes
+        self._reverse_paths: Optional[List[str]] = None
+        self._node_id: Optional[str] = None
+
+    @classmethod
+    def from_reverse_property(
+        cls, prop: Property, node_id: str, preferred_release_status: str = "released"
+    ) -> "KGQuery":
+        """
+        Create a query for the nodes that link to the node with the given id,
+        as represented by the reverse property `prop` of that node.
+
+        The forward properties that make these links are identified by their path,
+        since the same path does not always have the same Python name in all classes
+        (e.g. "license" in DatasetVersion, "licenses" in ModelVersion),
+        and a class may link through more than one forward property.
+        """
+        query = cls(prop.types, {as_list(prop.reverse)[0]: node_id}, preferred_release_status)
+        query._reverse_paths = as_list(prop.path)
+        query._node_id = node_id
+        return query
+
+    def _class_filters(self):
+        """Return (class, filter) pairs, one per query to send."""
+        if self._reverse_paths is None:
+            return [(cls, self.filter) for cls in self.classes]
+        return [
+            (cls, {forward_prop.name: self._node_id})
+            for cls in self.classes
+            for forward_prop in cls.properties
+            if forward_prop.path in self._reverse_paths
+        ]
 
     def __repr__(self):
         return f"""{self.__class__.__name__}([{", ".join(cls.__name__ for cls in self.classes)}], filter="{self.filter!r}")""".replace(
@@ -111,12 +144,12 @@ def resolve(
         """
         release_status = release_status or self.preferred_release_status
         objects: List[KGObject] = []
-        for cls in self.classes:
+        for cls, filter in self._class_filters():
             if hasattr(cls, "generate_query"):
                 # if cls is EmbeddedMetadata we cannot query it
                 query = cls.generate_query(
                     client=client,
-                    filters=self.filter,
+                    filters=filter,
                     space=space,
                     follow_links=follow_links,
                     with_reverse_properties=with_reverse_properties,
@@ -128,6 +161,9 @@ def resolve(
                     release_status=release_status,
                 ).data
                 objects.extend(cls.from_jsonld(instance_data, client) for instance_data in instances)
+        if self._reverse_paths is not None:
+            # a node that links through more than one forward property is found more than once
+            objects = list({obj.id: obj for obj in objects}.values())
         for obj in objects:
             object_cache[obj.id] = obj
 
@@ -143,9 +179,12 @@ def resolve(
     def count(self, client: KGClient, space: Optional[str] = None, release_status: Optional[str] = None):
         """
         Return the number of objects that would be returned by resolving this query.
+
+        For a query created with `from_reverse_property()`, a node that links through
+        more than one forward property is counted once for each.
         """
         release_status = release_status or self.preferred_release_status
         n = 0
-        for cls in self.classes:
-            n += cls.count(client, api="query", release_status=release_status, space=space, **self.filter)
+        for cls, filter in self._class_filters():
+            n += cls.count(client, api="query", release_status=release_status, space=space, **filter)
         return n

fairgraph/node.py

@@ -441,12 +441,7 @@ def _normalize_type(data_item):
 
             if data_item is None:
                 if prop.reverse and "@id" in data:
-                    if isinstance(prop.reverse, list):
-                        # todo: handle all possible reverses
-                        #       for now, we just take the first
-                        deserialized_data[prop.name] = KGQuery(prop.types, {prop.reverse[0]: data["@id"]})
-                    else:
-                        deserialized_data[prop.name] = KGQuery(prop.types, {prop.reverse: data["@id"]})
+                    deserialized_data[prop.name] = KGQuery.from_reverse_property(prop, data["@id"])
                 else:
                     deserialized_data[prop.name] = None
             else:

test/test_openminds_core.py

@@ -654,6 +654,77 @@ def test_KGQuery_count(kg_client):
     assert n_models_q > 1
 
 
+REVERSE_TEST_ID = "https://kg.ebrains.eu/api/instances/00000000-0000-0000-0000-000000000001"
+
+
+def _all_reverse_properties():
+    import fairgraph.openminds.v4 as v4
+    import fairgraph.openminds.v5 as v5
+
+    for version in (v4, v5):
+        for module_name in dir(version):
+            module = getattr(version, module_name)
+            if hasattr(module, "list_kg_classes"):
+                for cls in module.list_kg_classes():
+                    for prop in cls.reverse_properties:
+                        yield cls, prop
+
+
+def test_reverse_property_queries_are_valid_for_every_target_class(mock_client):
+    # the forward property does not have the same Python name in all target classes (#92)
+    reverse_properties = list(_all_reverse_properties())
+    assert len(reverse_properties) > 1000
+    for cls, prop in reverse_properties:
+        class_filters = KGQuery.from_reverse_property(prop, REVERSE_TEST_ID)._class_filters()
+        assert {target for target, _ in class_filters} == set(prop.types), (cls, prop.name)
+        for target, filter in class_filters:
+            if not hasattr(target, "generate_query") or target.__name__ == "MRIScannerUsage":
+                continue  # embedded classes cannot be queried; MRIScannerUsage query generation fails separately
+            target.generate_query(client=mock_client, space=None, filters=filter)
+
+
+def test_reverse_property_query_uses_the_forward_property_name_of_each_class():
+    query = KGQuery.from_reverse_property(omcore.License.get_property("is_applied_to"), REVERSE_TEST_ID)
+    class_filters = dict((target.__name__, filter) for target, filter in query._class_filters())
+    assert class_filters["DatasetVersion"] == {"license": REVERSE_TEST_ID}
+    assert class_filters["ModelVersion"] == {"licenses": REVERSE_TEST_ID}
+
+
+def test_reverse_property_query_covers_every_forward_property_with_the_path():
+    import fairgraph.openminds.v5.core as v5core
+
+    query = KGQuery.from_reverse_property(v5core.BehavioralProtocol.get_property("used_in"), REVERSE_TEST_ID)
+    assert [filter for target, filter in query._class_filters() if target is v5core.ProtocolExecution] == [
+        {"behavioral_protocols": REVERSE_TEST_ID},
+        {"protocols": REVERSE_TEST_ID},
+    ]
+
+
+def test_resolving_reverse_property_query_returns_each_node_once():
+    import fairgraph.openminds.v5.core as v5core
+
+    execution_id = "https://kg.ebrains.eu/api/instances/00000000-0000-0000-0000-000000000002"
+
+    class ClientLinkingThroughBothProperties:
+        # the protocol execution links to the protocol through both "behavioralProtocol" and "protocol"
+        def query(self, query, size, from_index, release_status):
+            if query["meta"]["type"] == v5core.ProtocolExecution.type_:
+                return MockKGResponse([{"@id": execution_id, "@type": [v5core.ProtocolExecution.type_]}])
+            return MockKGResponse([])
+
+    query = KGQuery.from_reverse_property(v5core.BehavioralProtocol.get_property("used_in"), REVERSE_TEST_ID)
+    result = query.resolve(ClientLinkingThroughBothProperties())
+    assert isinstance(result, v5core.ProtocolExecution)
+    assert result.id == execution_id
+
+
+@skip_if_no_connection
+def test_count_reverse_link_with_different_forward_property_names(kg_client):
+    # DatasetVersion has "license", ModelVersion and SoftwareVersion have "licenses" (#92)
+    license = omcore.License.by_name("CC-BY-4.0", kg_client)
+    assert license.is_applied_to.count(kg_client) > 0
+
+
 def test_to_jsonld():
     person1 = omcore.Person(
         id="0000",