Support lists of values in query filters

fairgraph/node.py

@@ -26,7 +26,14 @@
 from .base import Resolvable, ErrorHandling, Releasable
 from .kgproxy import KGProxy
 from .kgquery import KGQuery
-from .queries import Equals, 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,
+    to_single_value_filters,
+)
 from .errors import ResolutionFailure, CannotBuildExistenceQuery
 from .utility import (
     as_list,  # temporary for backwards compatibility (a lot of code imports it from here)
@@ -303,7 +310,16 @@ def generate_query_filter_properties(
         properties = []
         for prop in cls.all_properties:
             if prop.name in filters:
-                properties.append(get_query_filter_property(prop, cls.context, filters[prop.name]))
+                # the KG accepts one value per filter, so a filter containing a list of values
+                # becomes one copy of the property per value, each of which must match
+                single_value_filters = to_single_value_filters(filters[prop.name])
+                if len(single_value_filters) == 1:
+                    properties.append(get_query_filter_property(prop, cls.context, single_value_filters[0]))
+                else:
+                    for i, single_value_filter in enumerate(single_value_filters, start=1):
+                        properties.append(
+                            get_query_filter_property(prop, cls.context, single_value_filter, name=f"Q{prop.name}{i}")
+                        )
         return properties
 
     @classmethod

fairgraph/queries.py

@@ -75,7 +75,8 @@ class Equals(str):
     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".
+    As with any filter value, a list of `Equals` values matches only if every item is matched,
+    each with "EQUALS".
 
     Args:
         value (str): the value the property must be equal to.
@@ -483,12 +484,50 @@ def get_query_properties(
     return properties
 
 
-def get_query_filter_property(property, context, filter: Any) -> QueryProperty:
+def to_single_value_filters(filter: Any) -> List[Any]:
+    """
+    Return one or more filters equivalent to `filter`, each with a single value at every leaf.
+
+    A filter that contains no lists is returned unchanged, as the only element of the result.
+    The KG query API accepts only a single value per filter, so a filter containing a list of values
+    is expressed by repeating the property once per item, each copy being required: in that case
+    one filter is returned per item. The list may be nested within a dict of filters for a linked
+    or embedded node, in which case each copy of the dict contains one of the items.
+
+    Example:
+        >>> to_single_value_filters({"family_name": ["Baggins", "Oakenshield"], "given_name": "A"})
+        [{"family_name": "Baggins", "given_name": "A"}, {"family_name": "Oakenshield", "given_name": "A"}]
+    """
+    if isinstance(filter, (list, tuple)):
+        if len(filter) == 0:
+            raise ValueError("Cannot filter on an empty list of values")
+        return list(filter)
+    if isinstance(filter, dict):
+        split = {key: to_single_value_filters(value) for key, value in filter.items()}
+        keys_with_lists = [key for key, values in split.items() if len(values) > 1]
+        if len(keys_with_lists) > 1:
+            raise ValueError(
+                f"Only one list of filter values is supported within a single property, not {len(keys_with_lists)}"
+            )
+        base = {key: values[0] for key, values in split.items()}
+        if keys_with_lists:
+            key = keys_with_lists[0]
+            return [{**base, key: item} for item in split[key]]
+        return [base]
+    return [filter]
+
+
+def get_query_filter_property(property, context, filter: Any, name: Optional[str] = None) -> QueryProperty:
     """
     Generate a QueryProperty instance containing a filter,
     for use in constructing a KG query definition.
+
+    The filter must contain a single value: use `to_single_value_filters()` for filters containing lists.
+    The name of the QueryProperty defaults to "Q" followed by the property name.
     """
     assert filter is not None
+    if name is None:
+        name = f"Q{property.name}"
     if isinstance(filter, dict):
         # we pass the filter through to the next level
         filter_obj = None
@@ -508,7 +547,7 @@ def get_query_filter_property(property, context, filter: Any) -> QueryProperty:
 
     if any(issubclass(_type, Node) for _type in property.types):
         assert all(issubclass(_type, Node) for _type in property.types)
-        prop = QueryProperty(expanded_path, name=f"Q{property.name}", required=True, reverse=property.reverse)
+        prop = QueryProperty(expanded_path, name=name, required=True, reverse=property.reverse)
         if filter_obj:
             if filter_obj.value.startswith("https://kg.ebrains.eu/api/instances"):
                 filter_path = "@id"
@@ -525,7 +564,7 @@ def get_query_filter_property(property, context, filter: Any) -> QueryProperty:
                     prop.properties.extend(child_properties)
                     break
     else:
-        prop = QueryProperty(expanded_path, name=f"Q{property.name}", filter=filter_obj, required=True)
+        prop = QueryProperty(expanded_path, name=name, filter=filter_obj, required=True)
     return prop
 
 
@@ -564,7 +603,9 @@ def is_valid(val):
             isinstance(val, KGProxy) and not set(val.classes).isdisjoint(property.types)
         )
 
-    if isinstance(value, list) and len(value) > 0:
+    if isinstance(value, (list, tuple)) and len(value) == 0:
+        raise ValueError(f"Cannot filter {property.name} on an empty list of values")
+    if isinstance(value, list):
         valid_type = all(is_valid(item) for item in value)
         have_multiple = True
     else:
@@ -573,7 +614,7 @@ def is_valid(val):
     if not valid_type:
         if property.name == "hash":  # bit of a hack
             filter_value = value
-        elif isinstance(value, str) and value.startswith("http"):  # for @id
+        elif all(isinstance(item, str) and item.startswith("http") for item in as_list(value)):  # for @id
             filter_value = value
         else:
             raise TypeError("{} must be of type {}, not {}".format(property.name, property.types, type(value)))

fairgraph/utility/jsonld.py

@@ -219,7 +219,6 @@ def expand_filter(filter_dict: Dict[str, Any]):
         local_path = expanded
         parts = key.split("__")
         for part in parts[:-1]:
-            local_path[part] = {}
-            local_path = local_path[part]
+            local_path = local_path.setdefault(part, {})
         local_path[parts[-1]] = value
     return expanded

test/test_queries.py

@@ -847,6 +847,101 @@ def test_existence_query_rejects_an_invalid_match():
         omcore.Dataset(short_name="FOO")._build_existence_query("fuzzy")
 
 
+def _filter_properties(query, path):
+    return [prop for prop in query["structure"] if prop.get("path") == path and prop.get("required")]
+
+
+def test_list_filter_repeats_the_property_once_per_value(mock_client):
+    # each value must be matched by some developer, as in a hand-written KG query
+    query = omcore.SoftwareVersion.generate_query(
+        client=mock_client,
+        space=None,
+        filters={"developers__family_name": [Equals("Baggins"), Equals("Oakenshield")]},
+    )
+    props = _filter_properties(query, "https://openminds.om-i.org/props/developer")
+    assert [prop["propertyName"] for prop in props] == ["Qdevelopers1", "Qdevelopers2"]
+    assert [prop["structure"] for prop in props] == [
+        [
+            {
+                "path": "https://openminds.om-i.org/props/familyName",
+                "propertyName": "Qfamily_name",
+                "filter": {"op": "EQUALS", "value": family_name},
+                "required": True,
+            }
+        ]
+        for family_name in ("Baggins", "Oakenshield")
+    ]
+
+
+def test_list_filter_keeps_sibling_filters_in_each_copy(mock_client):
+    query = omcore.SoftwareVersion.generate_query(
+        client=mock_client,
+        space=None,
+        filters={"developers__given_name": "Bilbo", "developers__family_name": ["Baggins", "Oakenshield"]},
+    )
+    props = _filter_properties(query, "https://openminds.om-i.org/props/developer")
+    assert len(props) == 2
+    for prop, family_name in zip(props, ("Baggins", "Oakenshield")):
+        filters = {sub_prop["propertyName"]: sub_prop["filter"]["value"] for sub_prop in prop["structure"]}
+        assert filters == {"Qgiven_name": "Bilbo", "Qfamily_name": family_name}
+
+
+@pytest.mark.parametrize("as_objects", [True, False])
+def test_list_filter_on_links(mock_client, as_objects):
+    import fairgraph.openminds.publications as ompub
+
+    ids = [f"https://kg.ebrains.eu/api/instances/00000000-0000-0000-0000-00000000000{i}" for i in (1, 2)]
+    if as_objects:
+        values = [omcore.Person(id=id, given_name="A", family_name="B") for id in ids]
+    else:
+        values = ids
+    query = ompub.Chapter.generate_query(client=mock_client, space=None, filters={"authors": values})
+    props = _filter_properties(query, "https://openminds.om-i.org/props/author")
+    assert [prop["propertyName"] for prop in props] == ["Qauthors1", "Qauthors2"]
+    assert [prop["structure"][0]["path"] for prop in props] == ["@id", "@id"]
+    assert [prop["structure"][0]["filter"]["value"] for prop in props] == ids
+
+
+def test_list_filter_with_one_value_is_the_same_as_a_single_value(mock_client):
+    single = omcore.SoftwareVersion.generate_query(
+        client=mock_client, space=None, filters={"developers__family_name": "Baggins"}
+    )
+    as_list = omcore.SoftwareVersion.generate_query(
+        client=mock_client, space=None, filters={"developers__family_name": ["Baggins"]}
+    )
+    assert as_list == single
+
+
+def test_list_filter_rejects_an_empty_list(mock_client):
+    with pytest.raises(ValueError, match="empty list"):
+        omcore.SoftwareVersion.generate_query(client=mock_client, space=None, filters={"developers__family_name": []})
+
+
+def test_list_filter_rejects_two_lists_within_one_property(mock_client):
+    with pytest.raises(ValueError, match="Only one list"):
+        omcore.SoftwareVersion.generate_query(
+            client=mock_client,
+            space=None,
+            filters={"developers__family_name": ["Baggins", "Oakenshield"], "developers__given_name": ["A", "B"]},
+        )
+
+
+@skip_if_no_connection
+def test_execute_query_with_list_filter(kg_client):
+    # software versions developed by both people should be those found by filtering on each separately
+    family_names = ["Davison", "Appukuttan"]
+
+    def software_version_ids(family_name):
+        software_versions = omcore.SoftwareVersion.list(
+            kg_client, release_status="released", size=1000, developers__family_name=family_name
+        )
+        return {software_version.id for software_version in software_versions}
+
+    expected_ids = set.intersection(*(software_version_ids(Equals(family_name)) for family_name in family_names))
+    assert len(expected_ids) > 0
+    assert software_version_ids([Equals(family_name) for family_name in family_names]) == expected_ids
+
+
 def _all_kg_classes():
     import fairgraph.openminds.v4 as v4
     import fairgraph.openminds.v5 as v5

test/test_utility.py

@@ -27,6 +27,14 @@ def test_expand_filter():
     assert result == expected
 
 
+def test_expand_filter_keeps_siblings():
+    filter = {
+        "developers__given_name": "Bilbo",
+        "developers__family_name": "Baggins",
+    }
+    assert expand_filter(filter) == {"developers": {"given_name": "Bilbo", "family_name": "Baggins"}}
+
+
 def test_compact_uri():
     context = {"foaf": "http://xmlns.com/foaf/0.1/"}
     uri_list = "http://xmlns.com/foaf/0.1/Person"