Skip to content

Commit e2df424

Browse files
committed
Fixes, improvements and test for namespace (re)-binding on stores.
- Added store specific tests for namespace binding and rebinding. - Copied @gjhiggins fix for `rdflib.plugins.stores.memory.Memory.bind` to `rdflib.plugins.stores.memory.SimpleMemory.bind`. - Changed `bind` on `Memory`, `SimpleMemory`, `BerkleyDB` stores to explicitly check for `is not None`/`is None` instead of falsey/truthy. - Added `pytest.util._coalesce` to do null coalescing, for more info see deferred PEP-505 https://peps.python.org/pep-0505/.
1 parent 021e18e commit e2df424

File tree

7 files changed

+330
-16
lines changed

7 files changed

+330
-16
lines changed

rdflib/graph.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,6 +1019,13 @@ def bind(self, prefix, namespace, override=True, replace=False) -> None:
10191019
for example: graph.bind("foaf", "http://xmlns.com/foaf/0.1/")
10201020
10211021
"""
1022+
# TODO FIXME: This method's behaviour should be simplified and made
1023+
# more robust. If the method cannot do what it is asked it should raise
1024+
# an exception, it is also unclear why this method has all the
1025+
# different modes. It seems to just make it more complex to use, maybe
1026+
# it should be clarified when someone will need to use override=False
1027+
# and replace=False. And also why silent failure here is preferred over
1028+
# raising an excpetion.
10221029
return self.namespace_manager.bind(
10231030
prefix, namespace, override=override, replace=replace
10241031
)

rdflib/plugins/stores/memory.py

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#
22
#
33
from rdflib.store import Store
4+
from rdflib.util import _coalesce
45

56
__all__ = ["SimpleMemory", "Memory"]
67

@@ -149,11 +150,26 @@ def __len__(self, context=None):
149150
return i
150151

151152
def bind(self, prefix, namespace, override=True):
152-
bound_prefix = self.__prefix.get(namespace)
153-
if override and bound_prefix:
154-
del self.__namespace[bound_prefix]
155-
self.__prefix[namespace] = prefix
156-
self.__namespace[prefix] = namespace
153+
# should be identical to `Memory.bind`
154+
bound_namespace = self.__namespace.get(prefix)
155+
bound_prefix = _coalesce(
156+
self.__prefix.get(namespace),
157+
self.__prefix.get(bound_namespace),
158+
)
159+
if override:
160+
if bound_prefix is not None:
161+
del self.__namespace[bound_prefix]
162+
if bound_namespace is not None:
163+
del self.__prefix[bound_namespace]
164+
self.__prefix[namespace] = prefix
165+
self.__namespace[prefix] = namespace
166+
else:
167+
self.__prefix[_coalesce(bound_namespace, namespace)] = _coalesce(
168+
bound_prefix, prefix
169+
)
170+
self.__namespace[_coalesce(bound_prefix, prefix)] = _coalesce(
171+
bound_namespace, namespace
172+
)
157173

158174
def namespace(self, prefix):
159175
return self.__namespace.get(prefix, None)
@@ -403,21 +419,26 @@ def triples(self, triple_pattern, context=None):
403419
yield triple, self.__contexts(triple)
404420

405421
def bind(self, prefix, namespace, override=True):
422+
# should be identical to `SimpleMemory.bind`
406423
bound_namespace = self.__namespace.get(prefix)
407-
bound_prefix = self.__prefix.get(namespace) or self.__prefix.get(
408-
bound_namespace
424+
bound_prefix = _coalesce(
425+
self.__prefix.get(namespace),
426+
self.__prefix.get(bound_namespace),
409427
)
410428
if override:
411-
if bound_prefix:
429+
if bound_prefix is not None:
412430
del self.__namespace[bound_prefix]
413-
if bound_namespace:
431+
if bound_namespace is not None:
414432
del self.__prefix[bound_namespace]
415-
416433
self.__prefix[namespace] = prefix
417434
self.__namespace[prefix] = namespace
418435
else:
419-
self.__prefix[bound_namespace or namespace] = bound_prefix or prefix
420-
self.__namespace[bound_prefix or prefix] = bound_namespace or namespace
436+
self.__prefix[_coalesce(bound_namespace, namespace)] = _coalesce(
437+
bound_prefix, prefix
438+
)
439+
self.__namespace[_coalesce(bound_prefix, prefix)] = _coalesce(
440+
bound_namespace, namespace
441+
)
421442

422443
def namespace(self, prefix):
423444
return self.__namespace.get(prefix, None)

rdflib/util.py

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525

2626
# from time import daylight
2727
from time import altzone, gmtime, localtime, time, timezone
28-
from typing import Optional
28+
from typing import Optional, TypeVar
2929

3030
import rdflib.graph # avoid circular dependency
3131
from rdflib.compat import sign
@@ -44,6 +44,7 @@
4444
"guess_format",
4545
"find_roots",
4646
"get_tree",
47+
"_coalesce",
4748
]
4849

4950

@@ -430,6 +431,29 @@ def get_tree(
430431
return (mapper(root), sorted(tree, key=sortkey))
431432

432433

434+
_AnyT = TypeVar("_AnyT")
435+
436+
437+
def _coalesce(*args: Optional[_AnyT]) -> Optional[_AnyT]:
438+
"""
439+
This is a null coalescing function, it will return the first non-`None`
440+
argument passed to it, otherwise it will return `None`.
441+
442+
For more info regarding the rationale of this function see deferred `PEP
443+
505 <https://peps.python.org/pep-0505/>`_.
444+
445+
:param args: Values to consider as candidates to return, the first arg that
446+
is not `None` will be returned. If no argument is passed this function
447+
will return None.
448+
:return: The first ``arg`` that is not `None`, otherwise `None` if there
449+
are no args or if all args are `None`.
450+
"""
451+
for arg in args:
452+
if arg is not None:
453+
return arg
454+
return None
455+
456+
433457
def test():
434458
import doctest
435459

test/test_graph/test_namespace_rebinding.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
FOAF1 = Namespace(foaf1_uri)
1414
FOAF2 = Namespace(foaf2_uri)
1515

16-
# @pytest.mark.xfail(reason="'foaf=>FOAF2' binding improperly removed from namespaces")
16+
1717
def test_binding_replace():
1818

1919
# The confusion here is in the two arguments “override” and “replace” and
@@ -234,7 +234,11 @@ def test_parse_rebinds_prefix():
234234

235235

236236
@pytest.mark.xfail(
237-
reason="Automatic handling of unknown predicates not automatically registered with namespace manager"
237+
reason="""
238+
Automatic handling of unknown predicates not automatically registered with namespace manager
239+
240+
NOTE: This is not a bug, but more of a feature request.
241+
"""
238242
)
239243
def test_automatic_handling_of_unknown_predicates():
240244

@@ -393,7 +397,6 @@ def test_new_namespace_override_false():
393397
]
394398

395399

396-
# @pytest.mark.xfail()
397400
def test_change_namespace_and_prefix():
398401

399402
# A more extensive illustration of attempted rebinds of
Lines changed: 210 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,210 @@
1+
import enum
2+
import itertools
3+
import logging
4+
from dataclasses import dataclass, field
5+
from pathlib import Path
6+
from test.testutils import pytest_mark_filter
7+
from typing import Any, Callable, Dict, Set, Union
8+
9+
import pytest
10+
11+
from rdflib import Graph
12+
from rdflib.namespace import Namespace
13+
from rdflib.plugins.stores.berkeleydb import has_bsddb
14+
from rdflib.store import Store
15+
from rdflib.term import IdentifiedNode, URIRef
16+
17+
18+
class StoreTrait(enum.Enum):
19+
WRAPPER = enum.auto()
20+
DISK_BACKED = enum.auto()
21+
22+
23+
@dataclass
24+
class StoreInfo:
25+
name: str
26+
traits: Set[StoreTrait] = field(default_factory=set)
27+
28+
def make_graph(
29+
self, tmp_path: Path, identifier: Union[IdentifiedNode, str, None] = None
30+
) -> Graph:
31+
graph = Graph(store=self.name, bind_namespaces="none", identifier=identifier)
32+
if StoreTrait.DISK_BACKED in self.traits:
33+
use_path = tmp_path / f"{self.name}.storedata"
34+
use_path.mkdir(exist_ok=True, parents=True)
35+
logging.debug("use_path = %s", use_path)
36+
graph.open(f"{use_path}", create=True)
37+
return graph
38+
39+
40+
def make_store_info_dict(*result_format: StoreInfo) -> Dict[str, StoreInfo]:
41+
result = {}
42+
for item in result_format:
43+
result[item.name] = item
44+
return result
45+
46+
47+
store_info_dict = make_store_info_dict(
48+
StoreInfo("Memory"),
49+
StoreInfo("SimpleMemory"),
50+
StoreInfo("SPARQLStore"),
51+
StoreInfo("SPARQLUpdateStore"),
52+
*((StoreInfo("BerkeleyDB", {StoreTrait.DISK_BACKED}),) if has_bsddb else ()),
53+
)
54+
55+
56+
@pytest.fixture(
57+
scope="module",
58+
params=store_info_dict.keys(),
59+
)
60+
def store_name(request) -> str:
61+
assert isinstance(request.param, str)
62+
return request.param
63+
64+
65+
GraphMutator = Callable[[Graph], Any]
66+
67+
EGNSSUB = Namespace("http://example.com/sub/")
68+
EGNSSUB_V0 = EGNSSUB["v0"]
69+
EGNSSUB_V1 = EGNSSUB["v1"]
70+
EGNSSUB_V2 = EGNSSUB["v2"]
71+
72+
NamespaceBindings = Dict[str, URIRef]
73+
74+
75+
def make_graph(tmp_path: Path, store_name: str) -> Graph:
76+
logging.info("store_info_dict.keys() = %s", store_info_dict.keys())
77+
store_info = store_info_dict[store_name]
78+
return store_info.make_graph(tmp_path)
79+
80+
81+
def check_ns(graph: Graph, expected_bindings: NamespaceBindings) -> None:
82+
actual_graph_bindings = list(graph.namespaces())
83+
logging.info("actual_bindings = %s", actual_graph_bindings)
84+
assert list(expected_bindings.items()) == actual_graph_bindings
85+
store: Store = graph.store
86+
actual_store_bindings = list(store.namespaces())
87+
assert list(expected_bindings.items()) == actual_store_bindings
88+
for prefix, uri in expected_bindings.items():
89+
assert store.prefix(uri) == prefix
90+
assert store.namespace(prefix) == uri
91+
92+
93+
@pytest.mark.parametrize(
94+
["override", "replace"], itertools.product([True, False], [True, False])
95+
)
96+
def test_simple_bind(
97+
tmp_path: Path, store_name: str, override: bool, replace: bool
98+
) -> None:
99+
"""
100+
URI binds to a prefix regardless of override and replace values.
101+
"""
102+
graph = make_graph(tmp_path, store_name)
103+
graph.bind("egsub", EGNSSUB_V0, override=override, replace=replace)
104+
check_ns(graph, {"egsub": EGNSSUB_V0})
105+
106+
107+
@pytest.mark.parametrize(
108+
["override", "replace"], itertools.product([True, False], [True, False])
109+
)
110+
def test_bind_two_bind(
111+
tmp_path: Path, store_name: str, override: bool, replace: bool
112+
) -> None:
113+
"""
114+
A second prefix with a different URI binds correctly regardless of override
115+
and replace values.
116+
"""
117+
graph = make_graph(tmp_path, store_name)
118+
graph.bind("egsub", EGNSSUB_V0)
119+
graph.bind("egsubv1", EGNSSUB_V1, override=override, replace=replace)
120+
check_ns(graph, {"egsub": EGNSSUB_V0, "egsubv1": EGNSSUB_V1})
121+
122+
123+
@pytest.mark.parametrize("replace", [True, False])
124+
def test_rebind_uri_override(tmp_path: Path, store_name: str, replace: bool) -> None:
125+
"""
126+
URI binds to new prefix if override is `True` regardless of the value of `replace`.
127+
"""
128+
graph = make_graph(tmp_path, store_name)
129+
graph.bind("egsub", EGNSSUB_V0)
130+
graph.bind("egsubv0", EGNSSUB_V0, override=True, replace=replace)
131+
check_ns(graph, {"egsubv0": EGNSSUB_V0})
132+
133+
134+
@pytest.mark.parametrize("replace", [True, False])
135+
def test_rebind_uri_no_override(tmp_path: Path, store_name: str, replace: bool) -> None:
136+
"""
137+
URI does not bind to new prefix if override is `False` regardless of the value of `replace`.
138+
"""
139+
graph = make_graph(tmp_path, store_name)
140+
graph.bind("egsub", EGNSSUB_V0)
141+
graph.bind("egsubv0", EGNSSUB_V0, override=False, replace=replace)
142+
check_ns(graph, {"egsub": EGNSSUB_V0})
143+
144+
145+
@pytest.mark.parametrize(
146+
["store_name", "override"],
147+
pytest_mark_filter(
148+
itertools.product(store_info_dict.keys(), [True, False]),
149+
{
150+
("SPARQLStore", False): (
151+
pytest.mark.xfail(
152+
raises=AssertionError,
153+
reason="""
154+
SPARQLStore's namespace bindings work on a fundementally different way than
155+
the other stores, which is both simpler, but requires some additional work
156+
to make it behave like the other stores.
157+
""",
158+
),
159+
),
160+
("SPARQLUpdateStore", False): (
161+
pytest.mark.xfail(
162+
raises=AssertionError,
163+
reason="""
164+
SPARQLStore's namespace bindings work on a fundementally different way than
165+
the other stores, which is both simpler, but requires some additional work
166+
to make it behave like the other stores.
167+
""",
168+
),
169+
),
170+
},
171+
),
172+
)
173+
def test_rebind_prefix_replace(tmp_path: Path, store_name: str, override: bool) -> None:
174+
"""
175+
If `replace` is `True`,
176+
Prefix binds to a new URI and `override` is `True`,
177+
but does not bind to a new URI if `override` is `False`.
178+
179+
NOTE re logic, this is mainly just taking what most stores does and saying
180+
that is the right thing, not sure it really makes sense. This method is
181+
quite complicated and it is unlcear which of replace and override should
182+
take precedence in this case, once we sorted it out we should clarify in
183+
the documentation.
184+
"""
185+
graph = make_graph(tmp_path, store_name)
186+
graph.bind("egsub", EGNSSUB_V0)
187+
if override:
188+
graph.bind("egsub", EGNSSUB_V1, override=override, replace=True)
189+
check_ns(graph, {"egsub": EGNSSUB_V1})
190+
else:
191+
graph.bind("egsub", EGNSSUB_V1, override=override, replace=True)
192+
check_ns(graph, {"egsub": EGNSSUB_V0})
193+
194+
195+
@pytest.mark.parametrize(
196+
["reuse_override", "reuse_replace"], itertools.product([True, False], [True, False])
197+
)
198+
def test_rebind_prefix_reuse_uri_replace(
199+
tmp_path: Path, store_name: str, reuse_override: bool, reuse_replace: bool
200+
) -> None:
201+
"""
202+
Prefix binds to a new URI and the old URI can be reused with a new prefix
203+
regardless of the value of override or replace used when reusing the old
204+
URI.
205+
"""
206+
graph = make_graph(tmp_path, store_name)
207+
graph.bind("egsub", EGNSSUB_V0)
208+
graph.bind("egsub", EGNSSUB_V1, override=True, replace=True)
209+
graph.bind("egsubv0", EGNSSUB_V0, override=reuse_override, replace=reuse_replace)
210+
check_ns(graph, {"egsub": EGNSSUB_V1, "egsubv0": EGNSSUB_V0})

0 commit comments

Comments
 (0)