From ea199ae5e1e0c244ce961b4f57c48b4ccae0279e Mon Sep 17 00:00:00 2001 From: Andrew Davison Date: Wed, 16 Sep 2026 13:24:25 +0200 Subject: [PATCH] Use a KGProxy's id in existence queries (fixes #145) An unresolved link (KGProxy) in an existence-query property was serialized as {"@id": ...}, which expand_filter() rejects, so exists(), save() and Collection.upload() raised TypeError for objects without a full-URI id. The proxy's id is now used directly, giving the same query and save-cache key as a resolved node with that id. --- doc/release_notes.rst | 14 +++++++++++ fairgraph/node.py | 4 +++- test/test_base.py | 48 ++++++++++++++++++++++++++++++++++++- test/test_openminds_core.py | 38 +++++++++++++++++++++++++++++ 4 files changed, 102 insertions(+), 2 deletions(-) diff --git a/doc/release_notes.rst b/doc/release_notes.rst index c62d1cfa..d7ada6db 100644 --- a/doc/release_notes.rst +++ b/doc/release_notes.rst @@ -3,6 +3,20 @@ Release notes ============= +Version 0.16.0 +============== + +Bug fixes +--------- + +- :meth:`~fairgraph.kgobject.KGObject.exists`, and therefore :meth:`~fairgraph.kgobject.KGObject.save` and + :meth:`~fairgraph.collection.Collection.upload`, no longer raises :exc:`TypeError` when a property used + in the existence query holds an unresolved link (a :class:`~fairgraph.kgproxy.KGProxy`), + as happens when a link is taken from a fetched object or read from a JSON-LD file. + Such a link now gives the same existence query as the object it points to + (`#145 `_). + + Version 0.15.0 ============== diff --git a/fairgraph/node.py b/fairgraph/node.py index 3f89eac4..45278448 100644 --- a/fairgraph/node.py +++ b/fairgraph/node.py @@ -482,7 +482,9 @@ def _build_existence_query(self) -> Union[None, Dict[str, Any]]: for property in query_properties: query_property_name = property.name value = getattr(self, property.name) - if isinstance(value, KGNode): + if isinstance(value, KGProxy): + query[query_property_name] = value.id + elif isinstance(value, KGNode): if hasattr(value, "id") and value.id: query[query_property_name] = value.id else: diff --git a/test/test_base.py b/test/test_base.py index 114e89ae..8f76f68b 100644 --- a/test/test_base.py +++ b/test/test_base.py @@ -16,7 +16,7 @@ from fairgraph.errors import CannotBuildExistenceQuery from fairgraph.base import ErrorHandling from fairgraph.utility import ActivityLog -from .utils import clear_caches, mock_client +from .utils import MockKGResponse, clear_caches, mock_client import pytest @@ -412,6 +412,52 @@ def test_build_existence_query_with_missing_properties(self): assert exc_info.value.args[0] == f"Required value for '{prop_name}' is missing" obj.__class__.error_handling = orig_error_handling + def _construct_new_object_with_proxy_link(self): + """ + Return a new object (no id) whose linked object in the existence query is an + unresolved KGProxy, together with the equivalent object holding the resolved node. + """ + resolved_obj = self._construct_object_all_properties() + resolved_obj.id = None + proxy_obj = self._construct_object_all_properties() + proxy_obj.id = None + proxy_obj.a_required_linked_object = KGProxy(MockKGObject, resolved_obj.a_required_linked_object.id) + return proxy_obj, resolved_obj + + def test_build_existence_query__with_proxy(self): + """A link given as a KGProxy should give the same existence query as the resolved node.""" + proxy_obj, resolved_obj = self._construct_new_object_with_proxy_link() + query = proxy_obj._build_existence_query() + assert query["a_required_linked_object"] == ( + "https://kg.ebrains.eu/api/instances/00000000-0000-0000-0000-000000000002" + ) + assert query == resolved_obj._build_existence_query() + + def test_exists__with_proxy_in_existence_query(self, mock_client, clear_caches, mocker): + proxy_obj, resolved_obj = self._construct_new_object_with_proxy_link() + query = mocker.patch.object(mock_client, "query", return_value=MockKGResponse([])) + + assert not proxy_obj.exists(mock_client) + + query.assert_called_once() + expected_query = MockKGObject.generate_minimal_query( + client=mock_client, filters=resolved_obj._build_existence_query() + ) + assert query.call_args.kwargs["query"] == expected_query + + def test_save__with_proxy_in_existence_query(self, mock_client, clear_caches, mocker): + proxy_obj, resolved_obj = self._construct_new_object_with_proxy_link() + mocker.patch.object(mock_client, "query", return_value=MockKGResponse([])) + log = ActivityLog() + + proxy_obj.save(mock_client, space="mock", recursive=False, activity_log=log) + + assert [entry.type for entry in log.entries] == ["create"] + assert len(mock_client.instances) == 1 + # the save cache is keyed on the id of the linked object, as it is for a resolved node + cache_key = generate_cache_key(resolved_obj._build_existence_query()) + assert save_cache[MockKGObject] == {cache_key: proxy_obj.id} + def test_build_data_all_properties(self): obj = self._construct_object_all_properties() expected = { diff --git a/test/test_openminds_core.py b/test/test_openminds_core.py index b765fb08..16add03b 100644 --- a/test/test_openminds_core.py +++ b/test/test_openminds_core.py @@ -17,6 +17,7 @@ from openminds.base import LinkedNodeEmbedding from fairgraph.utility import as_list +from fairgraph.collection import Collection from fairgraph.kgproxy import KGProxy from fairgraph.kgquery import KGQuery from fairgraph.kgobject import KGObject, EXISTENCE_QUERY_SIZE @@ -593,6 +594,43 @@ def query_not_yet_consistent(query, **kwargs): assert len(queries) == 1 # the KG was not queried again +def test_collection_upload_with_link_to_existing_instance(mock_client, clear_caches, mocker, tmp_path): + """ + A node loaded from JSON-LD that links to an instance already in the KG (not in the collection) + holds a KGProxy for that link. Uploading it must not fail when the link is part of the + existence query (regression test for https://github.com/HumanBrainProject/fairgraph/issues/145). + """ + funder_id = "https://kg.ebrains.eu/api/instances/2cd6bfcd-6e3b-4b53-8b18-5a1eb6dc6f64" + path = tmp_path / "funding.jsonld" + path.write_text( + json.dumps( + { + "@context": {"@vocab": "https://openminds.om-i.org/props/"}, + # a blank-node id, so that the node's existence is checked by querying, + # rather than by looking up its id + "@id": "_:funding1", + "@type": "https://openminds.om-i.org/types/Funding", + "funder": {"@id": funder_id}, + } + ) + ) + collection = Collection() + collection.load(str(path)) + (funding,) = collection.nodes.values() + # the link leaves the collection, so it cannot be resolved to a node + assert isinstance(funding.funder, KGProxy) + # the mock client cannot run a query that filters on a linked node, + # so stub it out: an empty response means "no such instance in the KG" + query = mocker.patch.object(mock_client, "query", return_value=MockKGResponse([])) + + collection.upload(mock_client, default_space="myspace", upload_log_path=str(tmp_path / "upload_log.txt")) + + query.assert_called_once() # i.e. the existence query was built and run + assert len(mock_client.instances) == 1 + (instance,) = mock_client.instances.values() + assert instance["https://openminds.om-i.org/props/funder"] == {"@id": funder_id} + + @skip_if_no_connection def test_KGQuery_resolve(kg_client): ca1 = omterms.UBERONParcellation.by_name("CA1 field of hippocampus", kg_client)