From de2cdef4e1e018697b4fd1e7b428d379a0cab7c2 Mon Sep 17 00:00:00 2001 From: Andrew Davison Date: Mon, 28 Sep 2026 15:23:01 +0200 Subject: [PATCH] Match existence queries exactly by default, and add an existence_match option The existence query used by exists() and save() filtered string properties with the KG's CONTAINS operator, so a new Dataset called FOO was identified with an existing one called FOO-BAR and silently overwrote it (#149). Existence queries now use EQUALS, which the KG applies case-insensitively but not whitespace-insensitively, so a stored name with a trailing space is no longer found by a local name without one. exists() and save() take a new existence_match argument, either "equals" (the default, matching name_matching.MATCH_TYPES) or "contains" to restore the old behaviour; save() passes it on to child objects. The save cache holds a match found with "contains" under a separate key, so that it is not later returned for an "equals" lookup. Collection.upload() does not expose the option and always matches exactly. The exact match is implemented by a new Equals filter value, the counterpart of Regex, which selects the EQUALS operator in place of CONTAINS. It is exported from fairgraph and documented in the API reference and the queries guide. A list of Equals values is not supported and falls back to CONTAINS, as for Regex (#150). MockKGClient in test/utils.py gained matching case-insensitive EQUALS handling. --- doc/api_reference.rst | 3 ++ doc/creatingupdating.rst | 7 +++ doc/queries.rst | 14 +++++- doc/release_notes.rst | 28 +++++++++++ fairgraph/__init__.py | 2 +- fairgraph/embedded.py | 9 +++- fairgraph/kgobject.py | 85 ++++++++++++++++++++++++--------- fairgraph/node.py | 24 ++++++++-- fairgraph/queries.py | 24 ++++++++++ test/test_openminds_core.py | 94 +++++++++++++++++++++++++++++++++++++ test/test_queries.py | 80 ++++++++++++++++++++++++++++++- test/test_whitespace.py | 15 ++++++ test/utils.py | 7 ++- 13 files changed, 360 insertions(+), 32 deletions(-) diff --git a/doc/api_reference.rst b/doc/api_reference.rst index 27b4c452..be6abd7a 100644 --- a/doc/api_reference.rst +++ b/doc/api_reference.rst @@ -83,6 +83,9 @@ Queries .. autoclass:: fairgraph.queries.Regex :show-inheritance: +.. autoclass:: fairgraph.queries.Equals + :show-inheritance: + Utility classes and functions ============================= diff --git a/doc/creatingupdating.rst b/doc/creatingupdating.rst index e1f5071d..3dbd347f 100644 --- a/doc/creatingupdating.rst +++ b/doc/creatingupdating.rst @@ -40,6 +40,13 @@ attribute, e.g.:: >>> SoftwareVersion.existence_query_properties ('short_name', 'version_identifier') +String properties must match exactly (ignoring case). +If you want a value in the Knowledge Graph to match when it merely *contains* the local value, +pass ``existence_match="contains"`` to :meth:`save()` or :meth:`exists()`. +Use this with care: the object you save will overwrite any that is found:: + + >>> dataset.save(client, space="myspace", existence_match="contains") + Saving child nodes ================== diff --git a/doc/queries.rst b/doc/queries.rst index 9415863e..fb121b36 100644 --- a/doc/queries.rst +++ b/doc/queries.rst @@ -143,7 +143,19 @@ For example, to see only datasets whose name contain the phrase 'patch-clamp':: e.g., ``DatasetVersion.property_names`` or consult the inline help (``help(omcore.DatasetVersion)``). -For a more precise search, pass a :class:`Regex` instead of a plain string:: +A plain string matches any property value that *contains* it. +To require the whole value to match, pass an :class:`Equals` instead:: + + from fairgraph import Equals + + datasets = Dataset.list(client, short_name=Equals("FOO")) + +This finds datasets whose short name is "FOO", but not those named "FOO-BAR". +Case is ignored, but whitespace is not, so "FOO " would not match. +For a case-sensitive search on a property such as ``name``, see :meth:`by_name` below. +Only a single value is supported. + +For a more flexible search, pass a :class:`Regex` instead of a plain string:: from fairgraph import Regex diff --git a/doc/release_notes.rst b/doc/release_notes.rst index 148d1b7c..d500c33c 100644 --- a/doc/release_notes.rst +++ b/doc/release_notes.rst @@ -6,6 +6,14 @@ Release notes Version 0.16.0 ============== +New features +------------ + +- Filters may now be given as an :class:`~fairgraph.queries.Equals` instead of a plain string, + which selects the KG's "EQUALS" operator in place of "CONTAINS", so that the whole property value + must match. For example, ``Dataset.list(client, short_name=Equals("FOO"))`` does not return + a dataset called "FOO-BAR". As with :class:`~fairgraph.queries.Regex`, matching ignores case. + Changes in behaviour -------------------- @@ -13,6 +21,26 @@ Changes in behaviour Text loaded from the Knowledge Graph is left as it is until saved, so saving a fetched object corrects any untrimmed text that is stored. +- The existence query that :meth:`~fairgraph.kgobject.KGObject.exists` and + :meth:`~fairgraph.kgobject.KGObject.save` use to decide whether an object is already in the KG + now matches string properties exactly, ignoring case, where it previously matched any value that + merely *contained* the local one. Saving a :class:`Dataset` with the short name "FOO" therefore + no longer finds, and overwrites, an existing one called "FOO-BAR"; likewise, a + :class:`DatasetVersion` with the version identifier "1.0" no longer matches "1.0.1" + (`#149 `_). + + Whitespace is significant in the match, so an existing object whose name has a leading or trailing space + will not be found by a local object whose name lacks it, and a new object will be created. + Since :meth:`~fairgraph.kgobject.KGObject.save` now removes leading and trailing whitespace from text properties + (see above), this applies to any existing object whose text has stray whitespace. + Pipelines that re-save objects and previously relied on such near-matches to update + existing nodes may therefore now create new ones. + + To restore the previous behaviour for a call, pass ``existence_match="contains"`` to + :meth:`~fairgraph.kgobject.KGObject.exists` or :meth:`~fairgraph.kgobject.KGObject.save` + (the default is ``existence_match="equals"``). With ``recursive=True`` the setting applies to + child objects as well. + Bug fixes --------- diff --git a/fairgraph/__init__.py b/fairgraph/__init__.py index f996efe3..2d7b5bae 100644 --- a/fairgraph/__init__.py +++ b/fairgraph/__init__.py @@ -25,7 +25,7 @@ from .embedded import KGEmbedded, EmbeddedMetadata # EmbeddedMetadata is a deprecated alias from .kgproxy import KGProxy from .kgquery import KGQuery -from .queries import Regex +from .queries import Regex, Equals from .collection import Collection from . import client, errors, openminds, utility diff --git a/fairgraph/embedded.py b/fairgraph/embedded.py index 2ec6b8f6..b9863fdc 100644 --- a/fairgraph/embedded.py +++ b/fairgraph/embedded.py @@ -89,6 +89,7 @@ def save( activity_log: Optional[ActivityLog] = None, replace: bool = False, ignore_duplicates: bool = False, + existence_match: str = "equals", ): """ Save to the KG any sub-components of the metadata object that are KGObjects. @@ -101,7 +102,7 @@ def save( target_space = value.space elif ( value.__class__.default_space == "controlled" - and value.exists(client, ignore_duplicates=ignore_duplicates) + and value.exists(client, ignore_duplicates=ignore_duplicates, existence_match=existence_match) and value.space == "controlled" ): continue @@ -111,7 +112,10 @@ def save( assert space is not None # for type checking target_space = space if target_space == "controlled": - if value.exists(client, ignore_duplicates=ignore_duplicates) and value.space == "controlled": + if ( + value.exists(client, ignore_duplicates=ignore_duplicates, existence_match=existence_match) + and value.space == "controlled" + ): continue else: raise Exception("Cannot write to controlled space") @@ -121,6 +125,7 @@ def save( recursive=recursive, activity_log=activity_log, ignore_duplicates=ignore_duplicates, + existence_match=existence_match, ) diff --git a/fairgraph/kgobject.py b/fairgraph/kgobject.py index c8e43b83..f479b34a 100644 --- a/fairgraph/kgobject.py +++ b/fairgraph/kgobject.py @@ -44,7 +44,7 @@ from .caching import object_cache, save_cache, generate_cache_key from .base import ErrorHandling, Releasable, JSONdict from .utility.name_matching import KG_NAMELIKE_PROPERTIES, MATCH_TYPES, build_name_regex, matches_name -from .node import KGNode +from .node import KGNode, check_existence_match from .kgproxy import KGProxy from .kgquery import KGQuery @@ -558,7 +558,13 @@ def diff(self, other): differences["properties"][prop.name] = (val_self, val_other) return differences - def exists(self, client: KGClient, ignore_duplicates: bool = False, in_spaces: Optional[List[str]] = None) -> bool: + def exists( + self, + client: KGClient, + ignore_duplicates: bool = False, + in_spaces: Optional[List[str]] = None, + existence_match: str = "equals", + ) -> bool: """ Check if this object already exists in the KnowledgeGraph. @@ -567,17 +573,34 @@ def exists(self, client: KGClient, ignore_duplicates: bool = False, in_spaces: O ignore_duplicates (bool, optional): Whether to ignore the existence of multiple objects with the same properties (and consider only the first in the list), or to raise an Exception. Defaults to False. in_spaces (list of str, optional): If provided, only look for the object in these spaces. + existence_match (str, optional): How string properties are compared when looking for the object. + Either "equals" (the value in the KG must be the same as the local value, ignoring case - the default), + or "contains" (it is enough for the value in the KG to contain the local value, + so an object with the name "FOO" will match an existing object with the name "FOO-BAR"). """ - obj_exists = self._exists_without_query(client) + check_existence_match(existence_match) + obj_exists = self._exists_without_query(client, existence_match) if obj_exists is not None: return obj_exists - instances = self._query_matching_instances(client, in_spaces=in_spaces) + instances = self._query_matching_instances(client, in_spaces=in_spaces, existence_match=existence_match) if not instances: return False - self._check_for_duplicates(instances, ignore_duplicates) - return self._use_matching_instance(client, instances[0]) + self._check_for_duplicates(instances, ignore_duplicates, existence_match) + return self._use_matching_instance(client, instances[0], existence_match) + + def _existence_cache_keys(self, existence_match: str) -> List[Tuple]: + """ + The keys under which the save cache may hold the ID of an object matching this one. - def _exists_without_query(self, client: KGClient) -> Optional[bool]: + An object that matches exactly also matches "contains", but not the other way round, + so a "contains" lookup may use either key while an "equals" lookup may use only the first. + """ + equals_key = generate_cache_key(self._build_existence_query("equals")) + if existence_match == "equals": + return [equals_key] + return [equals_key, equals_key + (existence_match,)] + + def _exists_without_query(self, client: KGClient, existence_match: str = "equals") -> Optional[bool]: """ Check if this object exists in the KG, where this can be determined without an existence query, i.e. if the object has an ID, if there is no existence query, or if the object is found in the save cache. @@ -598,7 +621,7 @@ def _exists_without_query(self, client: KGClient) -> Optional[bool]: return obj_exists try: - query_filter = self._build_existence_query() + query_filter = self._build_existence_query(existence_match) except CannotBuildExistenceQuery: return False if query_filter is None: @@ -606,8 +629,10 @@ def _exists_without_query(self, client: KGClient) -> Optional[bool]: # duplicate entries return False - query_cache_key = generate_cache_key(query_filter) - if query_cache_key in save_cache[self.__class__]: + query_cache_key = next( + (key for key in self._existence_cache_keys(existence_match) if key in save_cache[self.__class__]), None + ) + if query_cache_key is not None: # Because the KnowledgeGraph is only eventually consistent, an instance # that has just been written to the KG may not appear in the query. # Therefore we cache the query when creating an instance and @@ -624,14 +649,16 @@ def _exists_without_query(self, client: KGClient) -> Optional[bool]: return True return None - def _query_matching_instances(self, client: KGClient, in_spaces: Optional[List[str]] = None) -> List[JSONdict]: + def _query_matching_instances( + self, client: KGClient, in_spaces: Optional[List[str]] = None, existence_match: str = "equals" + ) -> List[JSONdict]: """ Run the existence query for this object, and return all matching instances (their "@id" and space only). If the connection is lost while querying, an empty list is returned, with a warning. """ - query_filter = self._build_existence_query() + query_filter = self._build_existence_query(existence_match) query = self.__class__.generate_minimal_query(client=client, filters=query_filter) try: response = client.query( @@ -653,15 +680,16 @@ def _query_matching_instances(self, client: KGClient, in_spaces: Optional[List[s raise return instances - def _check_for_duplicates(self, instances: List[JSONdict], ignore_duplicates: bool): + def _check_for_duplicates( + self, instances: List[JSONdict], ignore_duplicates: bool, existence_match: str = "equals" + ): if len(instances) > 1 and not ignore_duplicates: - # we might want to consider running a second query with "equals" rather than "contains" raise Exception( f"Existence query is not specific enough. Type: {self.__class__.__name__}; " - f"filters: {self._build_existence_query()}" + f"filters: {self._build_existence_query(existence_match)}" ) - def _use_matching_instance(self, client: KGClient, match: JSONdict) -> bool: + def _use_matching_instance(self, client: KGClient, match: JSONdict, existence_match: str = "equals") -> bool: """ Identify this object with an instance found by the existence query. @@ -679,7 +707,7 @@ def _use_matching_instance(self, client: KGClient, match: JSONdict) -> bool: # the instance's actual location takes precedence over any space set locally if "https://schema.hbp.eu/myQuery/space" in match: self._space = match["https://schema.hbp.eu/myQuery/space"] - save_cache[self.__class__][generate_cache_key(self._build_existence_query())] = self.id + save_cache[self.__class__][self._existence_cache_keys(existence_match)[-1]] = self.id self._update_empty_properties(instance) # also updates `remote_data` return True @@ -728,6 +756,7 @@ def save( replace: bool = False, ignore_auth_errors: bool = False, ignore_duplicates: bool = False, + existence_match: str = "equals", ): """ Store the current object in the Knowledge Graph, either updating an existing instance @@ -744,11 +773,16 @@ def save( ignore_auth_errors (bool, optional): Whether to continue silently when encountering authentication errors. Defaults to False. ignore_duplicates (bool, optional): Whether to ignore the existence of multiple objects with the same properties (and consider only the first in the list), or to raise an Exception. Defaults to False. + existence_match (str, optional): How string properties are compared when looking for an existing object + to update. Either "equals" (the value in the KG must be the same as the local value, ignoring case - the default), + or "contains" (it is enough for the value in the KG to contain the local value, + so an object with the name "FOO" will overwrite an existing object with the name "FOO-BAR"). Raises: - An `AuthorizationError` if the current user is not authorized to perform the requested operation. """ + check_existence_match(existence_match) # Done first, so that the existence query, the data that is sent and the comparison with # the remote data all use the same values. The object is changed in place on purpose: # afterwards it is identical to what is stored in the KG. @@ -765,7 +799,9 @@ def save( if ( isinstance(value, KGObject) and value.__class__.default_space == "controlled" - and value.exists(client, ignore_duplicates=ignore_duplicates) + and value.exists( + client, ignore_duplicates=ignore_duplicates, existence_match=existence_match + ) and value.space == "controlled" ): continue @@ -778,7 +814,9 @@ def save( if target_space == "controlled": assert isinstance(value, KGObject) # for type checking if ( - value.exists(client, ignore_duplicates=ignore_duplicates) + value.exists( + client, ignore_duplicates=ignore_duplicates, existence_match=existence_match + ) and value.space == "controlled" ): continue @@ -790,6 +828,7 @@ def save( recursive=True, activity_log=activity_log, ignore_duplicates=ignore_duplicates, + existence_match=existence_match, ) if space is None: if self.space is None: @@ -797,17 +836,17 @@ def save( else: space = self.space logger.info(f"Saving a {self.__class__.__name__} in space {space}") - found = self._exists_without_query(client) + found = self._exists_without_query(client, existence_match) if found is None: # We look for the object in all spaces, not only the one we are saving to, to avoid creating duplicates, # but if it exists both in the target space and elsewhere, we use the instance in the target space. - instances = self._query_matching_instances(client) + instances = self._query_matching_instances(client, existence_match=existence_match) candidates = [ instance for instance in instances if instance.get("https://schema.hbp.eu/myQuery/space") == space ] or instances if candidates: - self._check_for_duplicates(candidates, ignore_duplicates) - found = self._use_matching_instance(client, candidates[0]) + self._check_for_duplicates(candidates, ignore_duplicates, existence_match) + found = self._use_matching_instance(client, candidates[0], existence_match) else: found = False if found and self.space is not None and self.space != space: diff --git a/fairgraph/node.py b/fairgraph/node.py index 34c6f312..1969084b 100644 --- a/fairgraph/node.py +++ b/fairgraph/node.py @@ -26,7 +26,7 @@ from .base import Resolvable, ErrorHandling, Releasable from .kgproxy import KGProxy from .kgquery import KGQuery -from .queries import QueryProperty, get_query_properties, get_query_filter_property, get_filter_value +from .queries import Equals, QueryProperty, get_query_properties, get_query_filter_property, get_filter_value from .errors import ResolutionFailure, CannotBuildExistenceQuery from .utility import ( as_list, # temporary for backwards compatibility (a lot of code imports it from here) @@ -44,6 +44,16 @@ JSONdict = Dict[str, Any] # see https://github.com/python/typing/issues/182 for some possible improvements +EXISTENCE_MATCH_MODES = ("equals", "contains") + + +def check_existence_match(existence_match: str): + if existence_match not in EXISTENCE_MATCH_MODES: + raise ValueError( + f"Invalid value for existence_match: {existence_match!r}. Must be one of {', '.join(EXISTENCE_MATCH_MODES)}" + ) + + class KGNode(Resolvable, metaclass=NodeMeta): # KGObject and KGEmbedded properties: List[Property] reverse_properties: List[Property] = [] @@ -134,6 +144,7 @@ def save( replace: bool = False, ignore_auth_errors: bool = False, ignore_duplicates: bool = False, + existence_match: str = "equals", ): raise NotImplementedError("This should be implemented by subclasses") @@ -482,11 +493,15 @@ def _normalize_text(self) -> None: if isinstance(item, KGEmbedded): item._normalize_text() - def _build_existence_query(self) -> Union[None, Dict[str, Any]]: + def _build_existence_query(self, existence_match: str = "equals") -> Union[None, Dict[str, Any]]: """ Generate a KG query definition (as a JSON-LD document) that can be used to check whether a locally-defined object (with no ID) already exists in the KG. + + 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. """ + check_existence_match(existence_match) if self.existence_query_properties is None: return None @@ -508,7 +523,7 @@ def _build_existence_query(self) -> Union[None, Dict[str, Any]]: if hasattr(value, "id") and value.id: query[query_property_name] = value.id else: - sub_query = value._build_existence_query() + 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") @@ -518,6 +533,9 @@ def _build_existence_query(self) -> Union[None, Dict[str, Any]]: 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 '{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 return query diff --git a/fairgraph/queries.py b/fairgraph/queries.py index 1b6879e8..2888289a 100644 --- a/fairgraph/queries.py +++ b/fairgraph/queries.py @@ -65,6 +65,28 @@ def __new__(cls, pattern: str): return super().__new__(cls, pattern) +class Equals(str): + """ + A string, for use as a filter value in place of a plain string, that must match the + property value exactly rather than as a substring. + + Where a plain string filters with the KG's "CONTAINS" operator, an `Equals` filters with "EQUALS". + The whole property value must match, so `Equals("FOO")` does not match "FOO-BAR". + The KG ignores case when matching, but not whitespace, so `Equals("FOO")` does match "foo", + but not "FOO ". + + Only a single value is supported: a list of `Equals` values is filtered with "CONTAINS". + + Args: + value (str): the value the property must be equal to. + + Example: + >>> import fairgraph.openminds.core as omcore + >>> from fairgraph import Equals + >>> omcore.Dataset.list(client, short_name=Equals("FOO")) + """ + + class PathElement: """ A single element in a multi-element query path, carrying optional @@ -475,6 +497,8 @@ def get_query_filter_property(property, context, filter: Any) -> QueryProperty: # we have a filter value for this property if isinstance(filter, Regex): op = "REGEX" + elif isinstance(filter, Equals): + op = "EQUALS" elif property.types[0] in (int, float, bool, datetime, date): op = "EQUALS" else: diff --git a/test/test_openminds_core.py b/test/test_openminds_core.py index 16add03b..c7c49d56 100644 --- a/test/test_openminds_core.py +++ b/test/test_openminds_core.py @@ -883,6 +883,100 @@ def _seed_person(mock_client, space, uuid="12345678-90ab-cdef-0123-4567890abcde" return person_id +def _seed_dataset(mock_client, short_name, space="myspace", uuid="12345678-90ab-cdef-0123-4567890abcdf"): + dataset_id = f"https://kg.ebrains.eu/api/instances/{uuid}" + mock_client.instances[dataset_id] = { + "@id": dataset_id, + "@type": ["https://openminds.om-i.org/types/Dataset"], + "https://core.kg.ebrains.eu/vocab/meta/space": space, + "https://openminds.om-i.org/props/shortName": short_name, + } + return dataset_id + + +def test_exists_does_not_match_a_longer_name(mock_client, clear_caches): + """An object must not be identified with an existing one whose name merely contains its own""" + _seed_dataset(mock_client, "FOO-BAR") + dataset = omcore.Dataset(short_name="FOO") + assert not dataset.exists(mock_client) + assert dataset.id is None + assert not dataset.exists(mock_client, existence_match="equals") + + +def test_exists_matches_the_same_name(mock_client, clear_caches): + dataset_id = _seed_dataset(mock_client, "FOO-BAR") + dataset = omcore.Dataset(short_name="FOO-BAR") + assert dataset.exists(mock_client) + assert dataset.id == dataset_id + + +def test_exists_ignores_case_when_matching(mock_client, clear_caches): + dataset_id = _seed_dataset(mock_client, "FOO-BAR") + dataset = omcore.Dataset(short_name="foo-bar") + assert dataset.exists(mock_client) + assert dataset.id == dataset_id + + +def test_exists_does_not_ignore_whitespace_when_matching(mock_client, clear_caches): + _seed_dataset(mock_client, "FOO-BAR ") + dataset = omcore.Dataset(short_name="FOO-BAR") + assert not dataset.exists(mock_client) + + +def test_exists_with_contains_matches_a_longer_name(mock_client, clear_caches): + dataset_id = _seed_dataset(mock_client, "FOO-BAR") + dataset = omcore.Dataset(short_name="FOO") + assert dataset.exists(mock_client, existence_match="contains") + assert dataset.id == dataset_id + + +def test_exists_with_contains_does_not_affect_later_exact_lookups(mock_client, clear_caches): + """Adopting a partial match must not make a later exact lookup find it, via the save cache""" + _seed_dataset(mock_client, "FOO-BAR") + assert omcore.Dataset(short_name="FOO").exists(mock_client, existence_match="contains") + assert not omcore.Dataset(short_name="FOO").exists(mock_client) + assert omcore.Dataset(short_name="FOO").exists(mock_client, existence_match="contains") + + +def test_exists_with_contains_uses_exact_matches_cached_by_earlier_saves(mock_client, clear_caches): + dataset = omcore.Dataset(short_name="FOO") + dataset.save(mock_client, space="myspace", recursive=False) + other = omcore.Dataset(short_name="FOO") + assert other.exists(mock_client, existence_match="contains") + assert other.id == dataset.id + + +def test_exists_rejects_an_invalid_existence_match(mock_client, clear_caches): + dataset = omcore.Dataset(short_name="FOO") + with pytest.raises(ValueError, match="existence_match"): + dataset.exists(mock_client, existence_match="fuzzy") + with pytest.raises(ValueError, match="existence_match"): + dataset.save(mock_client, space="myspace", recursive=False, existence_match="fuzzy") + + +def test_save_with_contains_updates_a_longer_name(mock_client, clear_caches): + dataset_id = _seed_dataset(mock_client, "FOO-BAR") + dataset = omcore.Dataset(short_name="FOO") + dataset.save(mock_client, space="myspace", recursive=False, existence_match="contains") + assert dataset.id == dataset_id + assert len(mock_client.instances) == 1 + + +def test_save_creates_a_new_object_when_only_a_longer_name_exists(mock_client, clear_caches): + dataset_id = _seed_dataset(mock_client, "FOO-BAR") + dataset = omcore.Dataset(short_name="FOO") + dataset.save(mock_client, space="myspace", recursive=False) + assert dataset.id != dataset_id + assert len(mock_client.instances) == 2 + + +def test_save_passes_existence_match_to_children(mock_client, clear_caches): + child_id = _seed_person(mock_client, "myspace") + parent = omcore.Dataset(short_name="FOO", custodians=[omcore.Person(given_name="Bil", family_name="Bag")]) + parent.save(mock_client, space="myspace", recursive=True, existence_match="contains") + assert parent.custodians[0].id == child_id + + def _spy_on_query(mock_client, monkeypatch): """Record the `restrict_to_spaces` argument of each query sent to the mock client""" restrictions = [] diff --git a/test/test_queries.py b/test/test_queries.py index e9c02d83..1b029056 100644 --- a/test/test_queries.py +++ b/test/test_queries.py @@ -2,7 +2,7 @@ import json import pytest from kg_core.request import Stage, Pagination -from fairgraph.queries import Query, QueryProperty, Filter, PathElement, Regex +from fairgraph.queries import Query, QueryProperty, Filter, PathElement, Regex, Equals import fairgraph.openminds.core as omcore import fairgraph.openminds.controlled_terms as omterms from .utils import kg_client, mock_client, skip_if_no_connection @@ -767,3 +767,81 @@ def test_regex_accepts_a_valid_pattern(): pattern = Regex("^M[uü]ller$") assert isinstance(pattern, str) assert pattern == "^M[uü]ller$" + + +def test_equals_marker_filters_with_equals_operator(mock_client): + query = omcore.Dataset.generate_query(client=mock_client, space=None, filters={"short_name": Equals("FOO")}) + filters = [prop["filter"] for prop in query["structure"] if prop.get("propertyName", None) == "Qshort_name"] + assert filters == [{"op": "EQUALS", "value": "FOO"}] + + +def test_equals_is_exported_from_the_package(): + import fairgraph + + assert fairgraph.Equals is Equals + + +@pytest.mark.parametrize( + "cls, filters, path_expected", + [ + (omcore.Dataset, {"short_name": Equals("FOO")}, "Qshort_name"), + (omcore.Dataset, {"digital_identifier__identifier": Equals("https://doi.org/10.1/x")}, "Qidentifier"), + (omcore.WebResource, {"iri": Equals("https://example.org/a")}, "Qiri"), + ], +) +def _find_filters(structure, property_name=None): + """All the filters in a query structure, at any depth, optionally only on the named property""" + for prop in structure: + if "filter" in prop and property_name in (None, prop.get("propertyName")): + yield prop["filter"] + yield from _find_filters(prop.get("structure", []), property_name) + + +@pytest.mark.parametrize( + "cls, filters, property_name", + [ + (omcore.Dataset, {"short_name": Equals("FOO")}, "Qshort_name"), + (omcore.Dataset, {"digital_identifier__identifier": Equals("https://doi.org/10.1/x")}, "Qidentifier"), + (omcore.WebResource, {"iri": Equals("https://example.org/a")}, "Qiri"), + ], +) +def test_equals_filters_string_and_iri_properties_with_equals(mock_client, cls, filters, property_name): + query = cls.generate_query(client=mock_client, space=None, filters=filters) + assert [f["op"] for f in _find_filters(query["structure"], property_name)] == ["EQUALS"] + + +def test_equals_filters_a_link_by_id_with_equals(mock_client): + instance_id = "https://kg.ebrains.eu/api/instances/00000000-0000-0000-0000-000000000001" + query = omcore.DatasetVersion.generate_query( + client=mock_client, space=None, filters={"is_new_version_of": Equals(instance_id)} + ) + filters = [f for f in _find_filters(query["structure"]) if f.get("value") == instance_id] + assert filters == [{"op": "EQUALS", "value": instance_id}] + + +def test_equals_rejects_a_non_id_string_for_a_link(mock_client): + # as for a plain string or a Regex + with pytest.raises(TypeError): + omcore.DatasetVersion.generate_query(client=mock_client, space=None, filters={"is_new_version_of": Equals("FOO")}) + + +def test_existence_query_filters_strings_with_equals(mock_client): + dataset = omcore.Dataset(short_name="FOO") + query = omcore.Dataset.generate_minimal_query(client=mock_client, filters=dataset._build_existence_query()) + filters = [prop["filter"] for prop in query["structure"] if prop.get("propertyName", None) == "Qshort_name"] + assert filters == [{"op": "EQUALS", "value": "FOO"}] + + +def test_existence_query_filters_strings_with_contains_if_requested(mock_client): + dataset = omcore.Dataset(short_name="FOO") + filters = dataset._build_existence_query("contains") + assert filters == {"short_name": "FOO"} + assert not isinstance(filters["short_name"], Equals) + query = omcore.Dataset.generate_minimal_query(client=mock_client, filters=filters) + query_filters = [prop["filter"] for prop in query["structure"] if prop.get("propertyName", None) == "Qshort_name"] + assert query_filters == [{"op": "CONTAINS", "value": "FOO"}] + + +def test_existence_query_rejects_an_invalid_match(): + with pytest.raises(ValueError, match="existence_match"): + omcore.Dataset(short_name="FOO")._build_existence_query("fuzzy") diff --git a/test/test_whitespace.py b/test/test_whitespace.py index becf4e76..f50c4b98 100644 --- a/test/test_whitespace.py +++ b/test/test_whitespace.py @@ -151,6 +151,21 @@ def test_untrimmed_new_object_finds_stored_object_and_creates_no_duplicate(self, assert [entry.type for entry in log.entries] == ["no-op"] assert mock_client.updates == [] + def test_stored_untrimmed_value_is_not_matched_exactly(self, mock_client, clear_caches): + _seed_bilbo(mock_client, given_name="Bilbo ") + # the value is stripped before saving, and an exact match does not ignore whitespace in the KG + person = omcore.Person(given_name="Bilbo", family_name="Baggins") + person.save(mock_client, space="common") + assert person.id != BILBO_ID + assert len(mock_client.instances) == 2 + + def test_stored_untrimmed_value_is_matched_with_existence_match_contains(self, mock_client, clear_caches): + _seed_bilbo(mock_client, given_name="Bilbo ") + person = omcore.Person(given_name=" Bilbo", family_name="Baggins") + person.save(mock_client, space="common", existence_match="contains") + assert person.id == BILBO_ID + assert len(mock_client.instances) == 1 + def test_loaded_object_with_untrimmed_stored_value_is_repaired(self, mock_client, clear_caches): _seed_bilbo(mock_client, given_name="Bilbo ") person = omcore.Person.from_id(BILBO_ID, mock_client) diff --git a/test/utils.py b/test/utils.py index f77d60eb..cea7834f 100644 --- a/test/utils.py +++ b/test/utils.py @@ -114,7 +114,7 @@ def _filter_selects(spec, candidate): """ Whether a query filter would select an instance whose name-like property is `candidate`. - `by_name()` filters with a regular expression, while existence queries filter with a plain string, + `by_name()` filters with a regular expression, while existence queries filter with an exact match (which the KG applies case-insensitively), so both have to be understood here. The regex is applied case-insensitively, to match the KG. """ value = spec.get("value", None) @@ -122,6 +122,8 @@ def _filter_selects(spec, candidate): return False if spec.get("op", None) == "REGEX": return re.search(value, candidate, re.IGNORECASE) is not None + if spec.get("op", None) == "EQUALS": + return candidate.lower() == value.lower() return candidate in value def query( @@ -229,6 +231,9 @@ def _value_matches(stored, op, value): continue if item == value: return True + if op == "EQUALS" and isinstance(item, str) and isinstance(value, str) and item.lower() == value.lower(): + # the KG ignores case, but not whitespace, when matching + return True if op == "CONTAINS" and isinstance(item, str) and isinstance(value, str) and value in item: return True return False