Handle non-openMINDS types in space_info() instead of raising

fairgraph/client.py

@@ -726,13 +726,16 @@ def space_info(
         space_name: str,
         release_status: str = "released",
         scope: Optional[str] = None,
-        ignore_errors: bool = False,
     ):
         """
         Return information about the types and number of instances in a space.
 
         The return format is a dictionary whose keys are classes and the values are the
         number of instances of each class in the given spaces.
+
+        A space may contain types that have no fairgraph class, such as the KG's own
+        internal types. These are not an error: they appear in the result keyed by their
+        type IRI (a string) rather than by a class.
         """
         release_status = handle_scope_keyword(scope, release_status)
         result = self._kg_client.types.list(space=space_name, stage=STAGE_MAP[release_status])
@@ -743,39 +746,37 @@ def space_info(
             type_iri = item.identifier
             try:
                 cls = lookup_type(type_iri, self.openminds_version)
-            except (KeyError, ValueError) as err:
-                ignore_list = [
-                    "https://core.kg.ebrains.eu/vocab/type/Bookmark",
-                    "https://core.kg.ebrains.eu/vocab/meta/type/Query",
-                    "https://openminds.om-i.org/types/Query",
-                    "https://openminds.om-i.org/types/URL"
-                ]
-                if ignore_errors or any(ignore in str(err) for ignore in ignore_list):
-                    pass
-                else:
-                    raise
-            else:
-                response[cls] = item.occurrences
+            except ValueError:
+                cls = type_iri
+            response[cls] = item.occurrences
         return response
 
     def clean_space(self, space_name):
         """Delete all instances from a given space."""
         # todo: check for released instances, they must be unreleased
         #       before deletion.
-        space_info = self.space_info(space_name, release_status="in progress", ignore_errors=True)
+        space_info = self.space_info(space_name, release_status="in progress")
         if sum(space_info.values()) > 0:
             print(f"The space '{space_name}' contains the following instances:\n")
             for cls, count in space_info.items():
                 if count > 0:
-                    print(cls.__name__, count)
+                    label = cls if isinstance(cls, str) else cls.__name__
+                    print(label, count)
             response = input("\nAre you sure you want to delete them? ")
             if response not in ("y", "Y", "yes", "YES"):
                 return
             error_messages = []
             for cls, count in space_info.items():
-                if count > 0 and hasattr(cls, "list"):  # exclude embedded metadata instances
-                    print(f"Deleting {cls.__name__} instances", end=" ")
-                    response = self.list(cls.type_, release_status="in progress", space=space_name, size=count)
+                if isinstance(cls, str):
+                    target_type = cls
+                elif hasattr(cls, "list"):  # exclude embedded metadata instances
+                    target_type = cls.type_
+                else:
+                    continue
+                if count > 0:
+                    label = cls if isinstance(cls, str) else cls.__name__
+                    print(f"Deleting {label} instances", end=" ")
+                    response = self.list(target_type, release_status="in progress", space=space_name, size=count)
                     assert response.total <= count
                     for instance in response.data:
                         assert instance["https://core.kg.ebrains.eu/vocab/meta/space"] == space_name
@@ -801,20 +802,30 @@ def move_all_to_space(self, source_space: str, destination_space: str):
             print(f"The space '{source_space}' contains the following instances:\n")
             for cls, count in space_info.items():
                 if count > 0:
-                    print(cls.__name__, count)
+                    label = cls if isinstance(cls, str) else cls.__name__
+                    print(label, count)
             response = input(f"\nAre you sure you want to move them to space '{destination_space}'? ")
             if response not in ("y", "Y", "yes", "YES"):
                 return
             for cls, count in space_info.items():
-                if count > 0 and hasattr(cls, "list"):  # exclude embedded metadata instances
-                    print(f"Moving {cls.__name__} instances", end="")
-                    instances = cls.list(self, release_status="in progress", space=source_space)
-                    assert len(instances) <= count
-                    for instance in instances:
-                        assert instance.space == source_space
-                        print(".", end="")
-                        self.move_to_space(instance.id, destination_space)
-                    print()
+                if count > 0:
+                    if isinstance(cls, str):
+                        label = cls
+                        print(f"Moving {label} instances", end="")
+                        response = self.list(cls, release_status="in progress", space=source_space, size=count)
+                        for instance in response.data:
+                            print(".", end="")
+                            self.move_to_space(self.uuid_from_uri(instance["@id"]), destination_space)
+                        print()
+                    elif hasattr(cls, "list"):  # exclude embedded metadata instances
+                        print(f"Moving {cls.__name__} instances", end="")
+                        instances = cls.list(self, release_status="in progress", space=source_space)
+                        assert len(instances) <= count
+                        for instance in instances:
+                            assert instance.space == source_space
+                            print(".", end="")
+                            self.move_to_space(instance.id, destination_space)
+                        print()
         else:
             print(f"The space '{source_space}' is empty, nothing to move.")
 

test/test_client.py

@@ -6,7 +6,14 @@
 from fairgraph.queries import Query, QueryProperty, Filter
 from fairgraph.errors import AuthenticationError, AuthorizationError, ResourceExistsError
 from fairgraph.base import OPENMINDS_VERSION
-from .utils import kg_client, kg_client_curator, skip_if_no_connection, MockKGResponse
+from fairgraph.client import KGClient
+from .utils import (
+    kg_client,
+    kg_client_curator,
+    mock_client,
+    skip_if_no_connection,
+    MockKGResponse,
+)
 
 
 @skip_if_no_connection
@@ -41,6 +48,56 @@ def test_spaces_names_only(kg_client):
     assert all(isinstance(space, str) for space in result)
 
 
+@skip_if_no_connection
+def test_space_info_tolerates_unknown_types(kg_client):
+    # Regression test for #113. A KG space may contain types that have no fairgraph
+    # class -- the KG's own internal types, for example
+    # "https://core.kg.ebrains.eu/doi/AdditionalDoiInformation". space_info() used to
+    # raise ValueError for those, so a single unrecognised type made the whole space
+    # listing fail. They are now returned keyed by their type IRI instead.
+    #
+    # "controlled" with release_status="in progress" is the case from the bug report.
+    info = kg_client.space_info("controlled", release_status="in progress")
+
+    assert isinstance(info, dict)
+    for key, count in info.items():
+        assert isinstance(count, int)
+        if isinstance(key, str):
+            # an unmapped type, keyed by its IRI
+            assert key.startswith("http"), f"unmapped type key is not an IRI: {key!r}"
+        else:
+            # a fairgraph/openMINDS class
+            assert isinstance(key, type), f"unexpected key type: {key!r}"
+            assert hasattr(key, "type_"), f"class key has no type_: {key!r}"
+
+
+@skip_if_no_connection
+def test_space_info_reports_unmapped_types_by_iri(kg_client):
+    # Companion to the test above: check that unmapped types really do occur and are
+    # handled, rather than the tolerance never being exercised. Scans the spaces the
+    # user can read until it finds one, since which spaces contain internal types
+    # depends on the deployment.
+    #
+    # Skips rather than fails if none is found: that means the KG served nothing
+    # unmapped, which is not a fairgraph bug.
+    unmapped = {}
+    for space_name in kg_client.spaces(names_only=True):
+        try:
+            info = kg_client.space_info(space_name, release_status="in progress")
+        except Exception as err:  # pragma: no cover - the bug this guards against
+            pytest.fail(f"space_info({space_name!r}) raised {type(err).__name__}: {err}")
+        found = [key for key in info if isinstance(key, str)]
+        if found:
+            unmapped[space_name] = found
+
+    if not unmapped:
+        pytest.skip("no unmapped types found in any accessible space")
+
+    for space_name, keys in unmapped.items():
+        for key in keys:
+            assert key.startswith("http"), f"in {space_name}: {key!r} is not an IRI"
+
+
 @skip_if_no_connection
 def test_query_filter_by_space(kg_client):
 
@@ -390,3 +447,58 @@ def contribute_to_partial_replacement(instance_id, payload, extended_response_co
         assert dsv._raw_remote_data is None, (
             "_raw_remote_data must be invalidated after a successful update"
         )
+
+
+class TestSpaceInfoOffline:
+    """space_info() and its callers, exercised without a KG connection.
+
+    Additional regression tests for #113, but no KG connection required.
+    """
+
+    known_type = "https://openminds.om-i.org/types/Person"
+    unknown_type = "https://core.kg.ebrains.eu/doi/AdditionalDoiInformation"
+
+    def _client_listing(self, mocker, mock_client, items):
+        """Equip the mock client with a types.list() returning `items`."""
+        data = [mocker.Mock(identifier=iri, occurrences=count) for iri, count in items]
+        mock_client._kg_client = mocker.Mock()
+        mock_client._kg_client.types.list.return_value = MockKGResponse(data)
+        return mock_client
+
+    def test_maps_known_types_to_classes_and_unknown_types_to_iris(self, mock_client, mocker):
+        client = self._client_listing(
+            mocker, mock_client, [(self.known_type, 3), (self.unknown_type, 7)]
+        )
+
+        info = KGClient.space_info(client, "myspace", release_status="in progress")
+
+        by_label = {(k if isinstance(k, str) else k.__name__): v for k, v in info.items()}
+        assert by_label == {"Person": 3, self.unknown_type: 7}
+
+        # the known type resolved to a class, the unknown one stayed a string
+        assert self.unknown_type in info
+        assert all(isinstance(k, type) for k in info if not isinstance(k, str))
+
+    def test_unknown_type_alone_does_not_raise(self, mock_client, mocker):
+        # The reported failure: a space whose only unrecognised type made the whole
+        # listing raise ValueError.
+        client = self._client_listing(mocker, mock_client, [(self.unknown_type, 1)])
+
+        assert KGClient.space_info(client, "myspace", release_status="in progress") == {
+            self.unknown_type: 1
+        }
+
+    def test_clean_space_lists_both_kinds_then_aborts(self, mock_client, mocker, capsys):
+        # clean_space() renders a label for every entry, so it has to cope with string
+        # keys as well as classes. Answering "n" exercises the listing without deleting.
+        from openminds.registry import lookup_type
+
+        person = lookup_type(self.known_type, mock_client.openminds_version)
+        mock_client.space_info = mocker.Mock(return_value={person: 3, self.unknown_type: 7})
+        mocker.patch("builtins.input", return_value="n")
+
+        KGClient.clean_space(mock_client, "myspace")
+
+        out = capsys.readouterr().out
+        assert "Person 3" in out
+        assert f"{self.unknown_type} 7" in out