diff --git a/fairgraph/caching.py b/fairgraph/caching.py index 79189d9a..ae5e4a7b 100644 --- a/fairgraph/caching.py +++ b/fairgraph/caching.py @@ -34,8 +34,9 @@ def generate_cache_key(qd: Dict[str, str]) -> Tuple: if isinstance(sub_value, dict): sub_key.append(generate_cache_key(sub_value)) else: - sub_key.append(str(value)) - cache_key.append(tuple(sub_key)) + sub_key.append(str(sub_value)) + # the order of the items does not matter in a query + cache_key.append((key, tuple(sorted(sub_key, key=str)))) elif isinstance(value, dict): cache_key.append((key, str(value))) elif value.__class__.__name__ == "IRI": # a bit hacky diff --git a/fairgraph/node.py b/fairgraph/node.py index 6f699d31..5cfaf228 100644 --- a/fairgraph/node.py +++ b/fairgraph/node.py @@ -183,23 +183,43 @@ def normalize_filter(cls, filter_dict: Dict[str, Any], check_validity: bool = Tr Normalize a dict containing filter key:value pairs so that it can be used in a call to the KG query API. - Example: + The dict must already be nested (see `expand_filter()`): a filter on a property of a linked + or embedded node is given as a dict under the name of the linking property. + Values are converted into the form used in queries, e.g. a node is replaced by its ID. + A list of values is kept as a list, each item being normalized in the same way. + + Examples: >>> import fairgraph.openminds.core as omcore - >>> person = omcore.Person.from_uuid("045f846f-f010-4db8-97b9-b95b20970bf2", kg_client) - >>> filter_dict = {"custodians": person, "name": "Virtual"} - >>> omcore.Dataset.normalize_filter(filter_dict) - {'name': 'Virtual', - 'custodians': 'https://kg.ebrains.eu/api/instances/045f846f-f010-4db8-97b9-b95b20970bf2'} + >>> person = omcore.Person(id="https://kg.ebrains.eu/api/instances/00000000-0000-0000-0000-000000000001") + >>> omcore.Dataset.normalize_filter({"custodians": person, "name": "Virtual"}) + {'custodians': 'https://kg.ebrains.eu/api/instances/00000000-0000-0000-0000-000000000001', + 'full_name': 'Virtual'} + + Here the linked node is replaced by its ID, and the alias "name" by the property it stands for. + + >>> from datetime import date + >>> from fairgraph.utility import expand_filter + >>> filters = expand_filter({"developers__family_name": ["Baggins", "Oakenshield"], + ... "release_date": date(2024, 1, 1)}) + >>> omcore.SoftwareVersion.normalize_filter(filters) + {'developers': {'family_name': ['Baggins', 'Oakenshield']}, 'release_date': '2024-01-01'} + + Here the filter on the developers' family names is normalized by the Person class + (one of the possible types of `developers`), and the date is converted to a string. """ normalized = {} filter_dict_copy = filter_dict.copy() - # handle aliases + # replace aliases (alternative names for properties, e.g. "name" for "full_name") + # by the names of the properties they stand for for name_, alias_ in cls.aliases.items(): if name_ in filter_dict_copy: filter_dict_copy[alias_] = filter_dict_copy.pop(name_) def _check_validity(filters, property_names): + """ + Raise a ValueError if any of the keys of `filters` is not in `property_names`. + """ invalid_filters = set(filters).difference(property_names) if invalid_filters: if len(invalid_filters) == 1: @@ -208,28 +228,46 @@ def _check_validity(filters, property_names): msg = f"Invalid filters: " raise ValueError(msg + ", ".join(sorted(invalid_filters))) + # the check is skipped when this method is called by `_normalize_dict()` for one of several + # possible child classes, since a filter key need only be valid for one of them if check_validity: _check_validity(filter_dict_copy, cls.all_property_names + list(cls.aliases.keys())) + def _normalize_dict(prop, value): + """ + Normalize `value`, a dict of filters on the properties of the node(s) linked or embedded via `prop`. + + A property may have several possible types (e.g. Person or Organization), + so each filter key must be a property of at least one of them. + Each type normalizes the filters on its own properties, and the results are merged. + """ + _check_validity( + value, + set( + chain(*(child_cls.all_property_names + list(child_cls.aliases.keys()) for child_cls in prop.types)) + ), + ) + normalized_value = {} + for child_cls in prop.types: + normalized_value.update(child_cls.normalize_filter(value, check_validity=False)) + return normalized_value + for prop in cls.all_properties: if prop.name in filter_dict_copy: value = filter_dict_copy[prop.name] if isinstance(value, dict): - _check_validity( - value, - set( - chain( - *( - child_cls.all_property_names + list(child_cls.aliases.keys()) - for child_cls in prop.types - ) - ) - ), - ) - normalized[prop.name] = {} - for child_cls in prop.types: - normalized[prop.name].update(child_cls.normalize_filter(value, check_validity=False)) + # filters on the properties of a linked or embedded node + normalized[prop.name] = _normalize_dict(prop, value) + elif isinstance(value, list) and any(isinstance(item, dict) for item in value): + # a list in which some items are filters on the properties of a linked or embedded node + # (as in existence queries for nodes with no ID), and others may be plain values, + # so each item is normalized separately + normalized[prop.name] = [ + _normalize_dict(prop, item) if isinstance(item, dict) else get_filter_value(prop, item) + for item in value + ] else: + # a plain value, or a list of plain values normalized[prop.name] = get_filter_value(prop, value) return normalized @@ -516,6 +554,9 @@ def _build_existence_query(self, existence_match: str = "equals") -> Union[None, With `existence_match="equals"`, string values must match exactly (ignoring case); with `"contains"`, it is enough for the value in the KG to contain the local value. + + For a property holding a list, every local item must be present in the KG, + but the order of the items is ignored, as are any additional items in the KG. """ check_existence_match(existence_match) if self.existence_query_properties is None: @@ -533,28 +574,52 @@ def _build_existence_query(self, existence_match: str = "equals") -> Union[None, for property in query_properties: query_property_name = property.name value = getattr(self, property.name) - 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: - sub_query = value._build_existence_query(existence_match) - query.update({f"{query_property_name}__{key}": val for key, val in sub_query.items()}) - elif isinstance(value, (list, tuple)): - raise CannotBuildExistenceQuery("not implemented yet") - elif value is None: - raise CannotBuildExistenceQuery(f"Required value for '{query_property_name}' is missing") - else: - query_val = value_to_jsonld(value, include_empty_properties=False, embed_linked_nodes=LinkedNodeEmbedding.NEVER) - if query_val is None: + if isinstance(value, (list, tuple)): + if len(value) == 0: raise CannotBuildExistenceQuery(f"Required value for '{query_property_name}' is missing") - if isinstance(query_val, str) and existence_match == "equals": - # match exactly, so that we don't identify this object with one whose value merely contains ours - query_val = Equals(query_val) - query[query_property_name] = query_val + # sub-queries for nodes in the list are kept as separate dicts, so that each is matched as a whole + query[query_property_name] = [ + self._existence_query_value(query_property_name, item, existence_match) for item in value + ] + else: + query_val = self._existence_query_value(query_property_name, value, existence_match) + if isinstance(query_val, dict): + query.update({f"{query_property_name}__{key}": val for key, val in query_val.items()}) + else: + query[query_property_name] = query_val return query + @staticmethod + def _existence_query_value(property_name: str, value: Any, existence_match: str) -> Any: + """ + Return the value to use in an existence query for a single property value (not a list): + an ID for a linked node that has one, a sub-query dict for any other node, or else a JSON-LD value. + """ + if isinstance(value, KGProxy): + return value.id + elif isinstance(value, KGNode): + if hasattr(value, "id") and value.id: + return value.id + sub_query = value._build_existence_query(existence_match) + if sub_query is None: + raise CannotBuildExistenceQuery( + f"Cannot build an existence query for '{property_name}', " + f"since {value.__class__.__name__} has no existence query" + ) + return sub_query + elif value is None: + raise CannotBuildExistenceQuery(f"Required value for '{property_name}' is missing") + else: + query_val = value_to_jsonld( + value, include_empty_properties=False, embed_linked_nodes=LinkedNodeEmbedding.NEVER + ) + if query_val is None: + raise CannotBuildExistenceQuery(f"Required value for '{property_name}' is missing") + if isinstance(query_val, str) and existence_match == "equals": + # match exactly, so that we don't identify this object with one whose value merely contains ours + query_val = Equals(query_val) + return query_val + # Deprecated alias ContainsMetadata = KGNode diff --git a/fairgraph/utility/jsonld.py b/fairgraph/utility/jsonld.py index a7b3ce72..f7e7d208 100644 --- a/fairgraph/utility/jsonld.py +++ b/fairgraph/utility/jsonld.py @@ -216,6 +216,9 @@ def expand_filter(filter_dict: Dict[str, Any]): for key, value in filter_dict.items(): if hasattr(value, "items"): raise TypeError("Filter specifications should be a single-level dict, without nesting") + if isinstance(value, list): + # each dict in a list is a separate filter specification, e.g. for one of several embedded nodes + value = [expand_filter(item) if isinstance(item, dict) else item for item in value] local_path = expanded parts = key.split("__") for part in parts[:-1]: diff --git a/test/test_base.py b/test/test_base.py index 8f76f68b..1c1c92ba 100644 --- a/test/test_base.py +++ b/test/test_base.py @@ -16,6 +16,7 @@ from fairgraph.errors import CannotBuildExistenceQuery from fairgraph.base import ErrorHandling from fairgraph.utility import ActivityLog +from fairgraph.queries import Equals from .utils import MockKGResponse, clear_caches, mock_client import pytest @@ -458,6 +459,81 @@ def test_save__with_proxy_in_existence_query(self, mock_client, clear_caches, mo cache_key = generate_cache_key(resolved_obj._build_existence_query()) assert save_cache[MockKGObject] == {cache_key: proxy_obj.id} + @pytest.fixture() + def list_existence_query(self, monkeypatch): + monkeypatch.setattr( + MockKGObject, + "existence_query_properties", + ( + "a_required_list_of_strings", + "a_required_list_of_datetimes", + "a_required_list_of_linked_objects", + "a_required_list_of_embedded_objects", + ), + ) + + def test_build_existence_query__with_lists(self, list_existence_query): + obj = self._construct_object_all_properties() + linked_id = "https://kg.ebrains.eu/api/instances/00000000-0000-0000-0000-000000000002" + query = obj._build_existence_query() + assert query == { + "a_required_list_of_strings": ["banana", "pear"], + "a_required_list_of_datetimes": ["1900-01-01T00:00:00", "2000-01-01T00:00:00"], + "a_required_list_of_linked_objects": [linked_id, linked_id], + # each embedded object is kept as a separate dict, so that it is matched as a whole + "a_required_list_of_embedded_objects": [{"a_number": 100.0}, {"a_number": 200.0}], + } + assert all(isinstance(item, Equals) for item in query["a_required_list_of_strings"]) + + def test_build_existence_query__with_lists__contains(self, list_existence_query): + obj = self._construct_object_all_properties() + query = obj._build_existence_query("contains") + assert query["a_required_list_of_strings"] == ["banana", "pear"] + assert not any(isinstance(item, Equals) for item in query["a_required_list_of_strings"]) + + def test_build_existence_query__with_empty_list(self, list_existence_query): + obj = self._construct_object_all_properties() + orig_error_handling = obj.__class__.error_handling + obj.__class__.error_handling = ErrorHandling.none + obj.a_required_list_of_strings = [] + try: + with pytest.raises(CannotBuildExistenceQuery) as exc_info: + obj._build_existence_query() + finally: + obj.__class__.error_handling = orig_error_handling + assert exc_info.value.args[0] == "Required value for 'a_required_list_of_strings' is missing" + + def test_build_existence_query__with_linked_object_without_existence_query( + self, list_existence_query, monkeypatch + ): + monkeypatch.setattr(MockKGObject2, "existence_query_properties", None) + obj = self._construct_object_all_properties() + obj.a_required_list_of_linked_objects = [MockKGObject2(a=1)] + with pytest.raises(CannotBuildExistenceQuery, match="MockKGObject2 has no existence query"): + obj._build_existence_query() + + def test_exists_and_save__with_lists_in_existence_query( + self, list_existence_query, mock_client, clear_caches, mocker + ): + obj = self._construct_object_all_properties() + obj.id = None + query = mocker.patch.object(mock_client, "query", return_value=MockKGResponse([])) + + assert not obj.exists(mock_client) + + # previously, no query was made, since the existence query could not be built + query.assert_called_once() + expected_query = MockKGObject.generate_minimal_query(client=mock_client, filters=obj._build_existence_query()) + assert query.call_args.kwargs["query"] == expected_query + + obj.save(mock_client, space="mock", recursive=False) # also checks existence + n_queries = query.call_count + new_obj = self._construct_object_all_properties() + new_obj.id = None + assert new_obj.exists(mock_client) # found in the save cache, without a further query + assert new_obj.id == obj.id + assert query.call_count == n_queries + def test_build_data_all_properties(self): obj = self._construct_object_all_properties() expected = { @@ -818,6 +894,14 @@ def test_generate_cache_key(): generate_cache_key(None) +def test_generate_cache_key__with_lists(): + # the key depends on which property holds a list, but not on the order of its items + assert generate_cache_key({"a": ["x", "y"], "b": "z"}) != generate_cache_key({"a": "z", "b": ["x", "y"]}) + assert generate_cache_key({"a": ["x", "y"]}) == generate_cache_key({"a": ["y", "x"]}) + assert generate_cache_key({"a": [{"b": "x"}, {"b": "y"}]}) == generate_cache_key({"a": [{"b": "y"}, {"b": "x"}]}) + assert generate_cache_key({"a": [{"b": "x"}]}) != generate_cache_key({"a": [{"b": "y"}]}) + + class TestKGProxy: def test_initialization_with_class(self): uri = "https://kg.ebrains.eu/api/instances/00000000-0000-0000-0000-000000001234" diff --git a/test/test_queries.py b/test/test_queries.py index af4639e3..d761dfac 100644 --- a/test/test_queries.py +++ b/test/test_queries.py @@ -1,5 +1,7 @@ import os import json +from datetime import date +from openminds import IRI import pytest from kg_core.request import Stage, Pagination from fairgraph.queries import Query, QueryProperty, Filter, PathElement, Regex, Equals @@ -847,6 +849,65 @@ def test_existence_query_rejects_an_invalid_match(): omcore.Dataset(short_name="FOO")._build_existence_query("fuzzy") +def _new_chapter(authors): + import fairgraph.openminds.publications as ompub + + return ompub.Chapter( + name="There and Back Again", + is_part_of=ompub.Book(id="https://kg.ebrains.eu/api/instances/00000000-0000-0000-0000-000000000001"), + publication_date=date(1937, 9, 21), + authors=authors, + ) + + +def _existence_filter_properties(cls, obj, mock_client, property_name): + query = cls.generate_minimal_query(client=mock_client, filters=obj._build_existence_query()) + return [prop for prop in query["structure"] if prop.get("propertyName", "").startswith(f"Q{property_name}")] + + +def test_existence_query_with_list_of_links(mock_client): + ids = [f"https://kg.ebrains.eu/api/instances/00000000-0000-0000-0000-00000000000{i}" for i in (2, 3)] + chapter = _new_chapter([omcore.Person(id=id, given_name="A", family_name="B") for id in ids]) + props = _existence_filter_properties(type(chapter), chapter, mock_client, "authors") + assert [prop["propertyName"] for prop in props] == ["Qauthors1", "Qauthors2"] + assert [prop["structure"] for prop in props] == [ + [{"path": "@id", "filter": {"op": "CONTAINS", "value": id}}] for id in ids + ] + + +def test_existence_query_with_list_of_embedded_nodes(mock_client): + file = omcore.File( + iri=IRI("https://example.org/data.nwb"), + hashes=[omcore.Hash(algorithm="MD5", digest="abc"), omcore.Hash(algorithm="SHA1", digest="def")], + ) + props = _existence_filter_properties(omcore.File, file, mock_client, "hashes") + assert [prop["propertyName"] for prop in props] == ["Qhashes1", "Qhashes2"] + # the filters for each hash are kept together, so that they must match the same hash in the KG + assert [ + {sub_prop["propertyName"]: sub_prop["filter"] for sub_prop in prop["structure"]} for prop in props + ] == [ + {"Qalgorithm": {"op": "EQUALS", "value": algorithm}, "Qdigest": {"op": "EQUALS", "value": digest}} + for algorithm, digest in (("MD5", "abc"), ("SHA1", "def")) + ] + + +def test_existence_query_with_list_of_saved_and_unsaved_links(mock_client): + saved_id = "https://kg.ebrains.eu/api/instances/00000000-0000-0000-0000-000000000002" + chapter = _new_chapter( + [ + omcore.Person(id=saved_id, given_name="A", family_name="B"), + omcore.Person(given_name="Bilbo", family_name="Baggins"), + ] + ) + props = _existence_filter_properties(type(chapter), chapter, mock_client, "authors") + assert [prop["propertyName"] for prop in props] == ["Qauthors1", "Qauthors2"] + assert props[0]["structure"] == [{"path": "@id", "filter": {"op": "CONTAINS", "value": saved_id}}] + assert {sub_prop["propertyName"]: sub_prop["filter"] for sub_prop in props[1]["structure"]} == { + "Qgiven_name": {"op": "EQUALS", "value": "Bilbo"}, + "Qfamily_name": {"op": "EQUALS", "value": "Baggins"}, + } + + def _filter_properties(query, path): return [prop for prop in query["structure"] if prop.get("path") == path and prop.get("required")] diff --git a/test/test_utility.py b/test/test_utility.py index a31c600f..1a5d5740 100644 --- a/test/test_utility.py +++ b/test/test_utility.py @@ -35,6 +35,13 @@ def test_expand_filter_keeps_siblings(): assert expand_filter(filter) == {"developers": {"given_name": "Bilbo", "family_name": "Baggins"}} +def test_expand_filter_expands_dicts_in_lists(): + filter = {"coordinates": [{"value": 1.0, "unit__name": "mm"}, {"value": 2.0, "unit__name": "mm"}]} + assert expand_filter(filter) == { + "coordinates": [{"value": 1.0, "unit": {"name": "mm"}}, {"value": 2.0, "unit": {"name": "mm"}}] + } + + def test_compact_uri(): context = {"foaf": "http://xmlns.com/foaf/0.1/"} uri_list = "http://xmlns.com/foaf/0.1/Person"