Add valid URI checks in mapping service graph (#45)

Bioregistry has some synonyms that contain spaces, which currently get
propagated to the `curies.Converter` instance in the Bioregistry's
default `bioregistry.Manager`. This is an issue since it results in
`https://bioregistry.io/<prefix>:<identifier>` variant URIs with spaces.

This PR makes sure that the ones containing spaces get filtered out. It
also does some refactoring of the logic for the custom `triples()`
function to streamline it and reduce code duplication.

Author: cthoyt

src/curies/mapping_service/__init__.py

@@ -107,6 +107,7 @@
 from typing import TYPE_CHECKING, Any, Collection, Iterable, List, Set, Tuple, Union, cast
 
 from rdflib import OWL, Graph, URIRef
+from rdflib.term import _is_valid_uri
 
 from .rdflib_custom import MappingServiceSPARQLProcessor  # type: ignore
 from ..api import Converter
@@ -201,37 +202,29 @@ def __init__(
         self.predicates = _prepare_predicates(predicates)
         super().__init__(*args, **kwargs)
 
+    def _expand_pair_all(self, uri_in: str) -> List[URIRef]:
+        prefix, identifier = self.converter.parse_uri(uri_in)
+        if prefix is None or identifier is None:
+            return []
+        uris = cast(Collection[str], self.converter.expand_pair_all(prefix, identifier))
+        # do _is_valid_uri check because some configurations e.g. from Bioregistry might
+        # produce invalid URIs e.g., containing spaces
+        return [URIRef(uri) for uri in uris if _is_valid_uri(uri)]
+
     def triples(
         self, triple: Tuple[URIRef, URIRef, URIRef]
     ) -> Iterable[Tuple[URIRef, URIRef, URIRef]]:
         """Generate triples, overriden to dynamically generate mappings based on this graph's converter."""
         subj_query, pred_query, obj_query = triple
-        if pred_query not in self.predicates:
-            return
-        if subj_query is None and obj_query is None:
-            return  # can't generate based on this
-        if subj_query is None and obj_query is not None:
-            prefix, identifier = self.converter.parse_uri(obj_query)
-            if prefix is None or identifier is None:
-                return
-            subjects = [
-                URIRef(sub)
-                for sub in cast(Collection[str], self.converter.expand_pair_all(prefix, identifier))
-            ]
-            for subj, pred in itt.product(subjects, self.predicates):
-                yield subj, pred, obj_query
-        elif subj_query is not None and obj_query is None:
-            prefix, identifier = self.converter.parse_uri(subj_query)
-            if prefix is None or identifier is None:
-                return
-            objects = [
-                URIRef(obj)
-                for obj in cast(Collection[str], self.converter.expand_pair_all(prefix, identifier))
-            ]
-            for obj, pred in itt.product(objects, self.predicates):
-                yield subj_query, pred, obj
-        else:  # subj_query is not None and obj_query is not None
-            return  # too much specification? maybe just return one triple then?
+        if pred_query in self.predicates:
+            if subj_query is None and obj_query is not None:
+                subjects = self._expand_pair_all(obj_query)
+                for subj, pred in itt.product(subjects, self.predicates):
+                    yield subj, pred, obj_query
+            elif subj_query is not None and obj_query is None:
+                objects = self._expand_pair_all(subj_query)
+                for obj, pred in itt.product(objects, self.predicates):
+                    yield subj_query, pred, obj
 
 
 def get_flask_mapping_blueprint(

tests/test_mapping_service.py

@@ -151,6 +151,22 @@ def test_missing(self):
         """
         self.assertEqual([], list(self.graph.query(sparql, processor=self.processor)))
 
+    def test_safe_expand(self):
+        """Test that expansion to invalid prefixes doesn't happen."""
+        ppm = {
+            "CHEBI": [
+                "http://purl.obolibrary.org/obo/CHEBI_",
+                "http://identifiers.org/chebi/",
+                "http://identifiers.org/chebi/nope nope:",
+            ],
+        }
+        converter = Converter.from_priority_prefix_map(ppm)
+        graph = MappingServiceGraph(converter=converter)
+        self.assertEqual(
+            {"http://purl.obolibrary.org/obo/CHEBI_1", "http://identifiers.org/chebi/1"},
+            set(map(str, graph._expand_pair_all("http://purl.obolibrary.org/obo/CHEBI_1"))),
+        )
+
 
 class ConverterMixin(unittest.TestCase):
     """A mixin that has a converter."""