From 65865abfd551696c5c8ac59f0cf713d2f79f0b85 Mon Sep 17 00:00:00 2001 From: Iwan Aucamp Date: Wed, 13 Jul 2022 22:51:02 +0200 Subject: [PATCH] Fallback to old `Store.bind` signature on `TypeError` If `Store.bind` raises a `TypeError`, and the string coversion of this TypeError contains `override`, then log a warning and call `Store.bind` without an override. This is done so that stores that do not accept `override` on `Store.bind` still work, but at the cost of still having the bug that was fixed by introducing the `override` parameter. Also added a private flag which can be used to disable the fix entirely and never use `override` when calling `Store.bind`. --- rdflib/namespace/__init__.py | 26 +++- test/test_graph/test_graph_store.py | 213 ++++++++++++++++++++++++++++ 2 files changed, 235 insertions(+), 4 deletions(-) create mode 100644 test/test_graph/test_graph_store.py diff --git a/rdflib/namespace/__init__.py b/rdflib/namespace/__init__.py index 0f64806d1..087d4a7e1 100644 --- a/rdflib/namespace/__init__.py +++ b/rdflib/namespace/__init__.py @@ -346,6 +346,8 @@ def _ipython_key_completions_(self) -> List[str]: if TYPE_CHECKING: from rdflib._type_checking import _NamespaceSetString +_with_bind_override_fix = True + class NamespaceManager(object): """Class for managing prefix => namespace mappings @@ -618,6 +620,22 @@ def expand_curie(self, curie: str) -> Union[URIRef, None]: f"Prefix \"{curie.split(':')[0]}\" not bound to any namespace." ) + def _store_bind(self, prefix: str, namespace: URIRef, override: bool) -> None: + if not _with_bind_override_fix: + return self.store.bind(prefix, namespace) + try: + return self.store.bind(prefix, namespace, override=override) + except TypeError as error: + if "override" in str(error): + logger.warning( + "caught a TypeError, " + "retrying call to %s.bind without override, " + "see https://github.com/RDFLib/rdflib/issues/1880 for more info", + type(self.store), + exc_info=True, + ) + return self.store.bind(prefix, namespace) + def bind( self, prefix: Optional[str], @@ -652,7 +670,7 @@ def bind( if bound_namespace and bound_namespace != namespace: if replace: - self.store.bind(prefix, namespace, override=override) + self._store_bind(prefix, namespace, override=override) insert_trie(self.__trie, str(namespace)) return # prefix already in use for different namespace @@ -673,16 +691,16 @@ def bind( if not self.store.namespace(new_prefix): break num += 1 - self.store.bind(new_prefix, namespace, override=override) + self._store_bind(new_prefix, namespace, override=override) else: bound_prefix = self.store.prefix(namespace) if bound_prefix is None: - self.store.bind(prefix, namespace, override=override) + self._store_bind(prefix, namespace, override=override) elif bound_prefix == prefix: pass # already bound else: if override or bound_prefix.startswith("_"): # or a generated prefix - self.store.bind(prefix, namespace, override=override) + self._store_bind(prefix, namespace, override=override) insert_trie(self.__trie, str(namespace)) diff --git a/test/test_graph/test_graph_store.py b/test/test_graph/test_graph_store.py new file mode 100644 index 000000000..144434cc6 --- /dev/null +++ b/test/test_graph/test_graph_store.py @@ -0,0 +1,213 @@ +""" +Tests for usage of the Store interface from Graph/NamespaceManager. +""" + +import itertools +import logging +from typing import ( + TYPE_CHECKING, + Any, + Callable, + Dict, + Iterable, + Optional, + Sequence, + Tuple, + Type, + Union, +) + +import pytest + +import rdflib.namespace +from rdflib.graph import Graph +from rdflib.namespace import Namespace +from rdflib.plugins.stores.memory import Memory +from rdflib.store import Store +from rdflib.term import URIRef + +if TYPE_CHECKING: + from _pytest.mark.structures import ParameterSet + +NamespaceBindings = Dict[str, URIRef] + + +def check_ns(graph: Graph, expected_bindings: NamespaceBindings) -> None: + actual_graph_bindings = list(graph.namespaces()) + logging.info("actual_bindings = %s", actual_graph_bindings) + assert list(expected_bindings.items()) == actual_graph_bindings + store: Store = graph.store + actual_store_bindings = list(store.namespaces()) + assert list(expected_bindings.items()) == actual_store_bindings + for prefix, uri in expected_bindings.items(): + assert store.prefix(uri) == prefix + assert store.namespace(prefix) == uri + + +class MemoryWithoutBindOverride(Memory): + def bind(self, prefix, namespace) -> None: # type: ignore[override] + return super().bind(prefix, namespace, False) + + +class GraphWithoutBindOverrideFix(Graph): + def bind(self, prefix, namespace, override=True, replace=False) -> None: + old_value = rdflib.namespace._with_bind_override_fix + rdflib.namespace._with_bind_override_fix = False + try: + return super().bind(prefix, namespace, override, replace) + finally: + rdflib.namespace._with_bind_override_fix = old_value + + +GraphFactory = Callable[[], Graph] +GraphOperation = Callable[[Graph], None] +GraphOperations = Sequence[GraphOperation] + +EGNS = Namespace("http://example.com/namespace#") +EGNS_V0 = EGNS["v0"] +EGNS_V1 = EGNS["v1"] +EGNS_V2 = EGNS["v2"] + + +def make_test_graph_store_bind_cases( + store_type: Type[Store] = Memory, + graph_type: Type[Graph] = Graph, +) -> Iterable[Union[Tuple[Any, ...], "ParameterSet"]]: + """ + Generate test cases for test_graph_store_bind. + """ + + def graph_factory(): + return graph_type(bind_namespaces="none", store=store_type()) + + id_prefix = f"{store_type.__name__}-{graph_type.__name__}" + + def _p( + graph_factory: GraphFactory, + ops: GraphOperations, + expected_bindings: NamespaceBindings, + expected_bindings_overrides: Optional[ + Dict[Tuple[Type[Graph], Type[Store]], NamespaceBindings] + ] = None, + *, + id: Optional[str] = None, + ): + if expected_bindings_overrides is not None: + expected_bindings = expected_bindings_overrides.get( + (graph_type, store_type), expected_bindings + ) + if id is not None: + return pytest.param(graph_factory, ops, expected_bindings, id=id) + else: + return (graph_factory, ops, expected_bindings) + + yield from [ + _p( + graph_factory, + [ + lambda g: g.bind("eg", EGNS_V0), + ], + {"eg": EGNS_V0}, + id=f"{id_prefix}-default-args", + ), + # reused-prefix + _p( + graph_factory, + [ + lambda g: g.bind("eg", EGNS_V0), + lambda g: g.bind("eg", EGNS_V1, override=False), + ], + {"eg": EGNS_V0, "eg1": EGNS_V1}, + id=f"{id_prefix}-reused-prefix-override-false-replace-false", + ), + _p( + graph_factory, + [ + lambda g: g.bind("eg", EGNS_V0), + lambda g: g.bind("eg", EGNS_V1), + ], + {"eg": EGNS_V0, "eg1": EGNS_V1}, + id=f"{id_prefix}-reused-prefix-override-true-replace-false", + ), + _p( + graph_factory, + [ + lambda g: g.bind("eg", EGNS_V0), + lambda g: g.bind("eg", EGNS_V1, override=False, replace=True), + ], + {"eg": EGNS_V0}, + {(GraphWithoutBindOverrideFix, Memory): {"eg": EGNS_V1}}, + id=f"{id_prefix}-reused-prefix-override-false-replace-true", + ), + _p( + graph_factory, + [ + lambda g: g.bind("eg", EGNS_V0), + lambda g: g.bind("eg", EGNS_V1, replace=True), + ], + {"eg": EGNS_V1}, + {(Graph, MemoryWithoutBindOverride): {"eg": EGNS_V0}}, + id=f"{id_prefix}-reused-prefix-override-true-replace-true", + ), + # reused-namespace + _p( + graph_factory, + [ + lambda g: g.bind("eg", EGNS_V0), + lambda g: g.bind("egv0", EGNS_V0, override=False), + ], + {"eg": EGNS_V0}, + id=f"{id_prefix}-reused-namespace-override-false-replace-false", + ), + _p( + graph_factory, + [ + lambda g: g.bind("eg", EGNS_V0), + lambda g: g.bind("egv0", EGNS_V0), + ], + {"egv0": EGNS_V0}, + {(Graph, MemoryWithoutBindOverride): {"eg": EGNS_V0}}, + id=f"{id_prefix}-reused-namespace-override-true-replace-false", + ), + _p( + graph_factory, + [ + lambda g: g.bind("eg", EGNS_V0), + lambda g: g.bind("egv0", EGNS_V0, override=False, replace=True), + ], + {"eg": EGNS_V0}, + id=f"{id_prefix}-reused-namespace-override-false-replace-true", + ), + _p( + graph_factory, + [ + lambda g: g.bind("eg", EGNS_V0), + lambda g: g.bind("egv0", EGNS_V0, replace=True), + ], + {"egv0": EGNS_V0}, + {(Graph, MemoryWithoutBindOverride): {"eg": EGNS_V0}}, + id=f"{id_prefix}-reused-namespace-override-true-replace-true", + ), + ] + + +@pytest.mark.parametrize( + ["graph_factory", "ops", "expected_bindings"], + itertools.chain( + make_test_graph_store_bind_cases(), + make_test_graph_store_bind_cases(store_type=MemoryWithoutBindOverride), + make_test_graph_store_bind_cases(graph_type=GraphWithoutBindOverrideFix), + ), +) +def test_graph_store_bind( + graph_factory: GraphFactory, + ops: GraphOperations, + expected_bindings: NamespaceBindings, +) -> None: + """ + The expected sequence of graph operations results in the expected namespace bindings. + """ + graph = graph_factory() + for op in ops: + op(graph) + check_ns(graph, expected_bindings)