Skip to content

Commit 6c6ee6d

Browse files
committed
fix(core): return a list from load_entity and pair iris by title
- LoadEntityResult.entities is now List[OswBaseModel]; pydantic v1 tried the Union's first member, so an empty result became a truthy OswBaseModel that is neither subscriptable nor sized - resolve() keys the loaded entities by full page title instead of zipping them against request.iris; load_entity skips pages it cannot build, which shifted every later iri onto the wrong entity - an iri with no matching entity is logged and left out of nodes - export_entity_jsonld drops the now dead isinstance branch that hid the empty result from its NotFound check Closes #198
1 parent c21ad24 commit 6c6ee6d

3 files changed

Lines changed: 114 additions & 7 deletions

File tree

‎src/osw/core.py‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -268,10 +268,19 @@ def resolve(self, request: ResolveParam):
268268
entities = osw_obj.load_entity(
269269
OSW.LoadEntityParam(titles=request.iris)
270270
).entities
271-
# create a dict with request.iris as keys and the loaded entities as values
272-
# by iterating over both lists
271+
# load_entity() skips pages it cannot build, so the returned
272+
# list can be shorter than request.iris. Pair by full title
273+
# instead of by position, so a skipped page does not shift
274+
# every following entity onto the wrong iri.
275+
entities_by_title = {
276+
get_full_title(entity): entity for entity in entities
277+
}
273278
nodes = {}
274-
for iri, entity in zip(request.iris, entities):
279+
for iri in request.iris:
280+
entity = entities_by_title.get(iri)
281+
if entity is None:
282+
_logger.warning(f"Could not resolve iri '{iri}'")
283+
continue
275284
nodes[iri] = entity
276285
return ResolveResult(nodes=nodes)
277286

@@ -1280,8 +1289,8 @@ def __init__(self, **data):
12801289
class LoadEntityResult(BaseModel):
12811290
"""Result of load_entity()"""
12821291

1283-
entities: Union[model.OswBaseModel, List[model.OswBaseModel]]
1284-
"""The dataclass instance(s)"""
1292+
entities: List[model.OswBaseModel]
1293+
"""The list of dataclass instances"""
12851294

12861295
# fmt: off
12871296
@overload

‎src/osw/service/ops/entities.py‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,6 @@ def export_entity_jsonld(
9292
OSW.LoadEntityParam(titles=[title], autofetch_schema=True)
9393
)
9494
entities = result.entities
95-
if not isinstance(entities, list):
96-
entities = [entities]
9795
if not entities:
9896
raise errors.NotFound(f"Entity '{title}' not found.")
9997
export = ctx.osw.export_jsonld(
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
"""Unit tests for load_entity()'s result shape and resolve()'s iri pairing.
2+
3+
Regression guard for #198: https://github.com/OpenSemanticLab/osw-python/issues/198
4+
LoadEntityResult.entities was declared as Union[OswBaseModel, List[OswBaseModel]],
5+
so pydantic v1 tried the bare OswBaseModel variant first and an empty list
6+
validated as OswBaseModel() instead of staying an empty list. Separately,
7+
OswDefaultBackend.resolve() zipped request.iris against load_entity()'s result
8+
positionally, so a page load_entity() skipped shifted every later iri onto the
9+
wrong entity.
10+
11+
These run fully offline: no site touches the network.
12+
"""
13+
14+
from unittest.mock import MagicMock
15+
16+
from oold.backend.interface import ResolveParam
17+
18+
import osw.core
19+
import osw.model.entity as model
20+
from osw.core import OSW
21+
from osw.wtsite import WtSite
22+
23+
24+
def _make_entity(namespace: str, title: str) -> model.Item:
25+
entity = model.Item(label=[model.Label(text=title)])
26+
entity.meta = model.Meta(wiki_page=model.WikiPage(namespace=namespace, title=title))
27+
return entity
28+
29+
30+
def _build_backend(monkeypatch):
31+
"""Constructs an OSW instance and returns its OswDefaultBackend.
32+
33+
set_resolver/set_backend are replaced so construction does not touch the
34+
real global oold registry; set_backend is used to capture the backend
35+
instance it would otherwise register.
36+
"""
37+
captured = {}
38+
39+
def _capture_set_backend(param):
40+
captured["backend"] = param.backend
41+
42+
monkeypatch.setattr(osw.core, "set_resolver", lambda param: None)
43+
monkeypatch.setattr(osw.core, "set_backend", _capture_set_backend)
44+
45+
site = MagicMock(spec=WtSite)
46+
OSW(site=site)
47+
48+
return captured["backend"]
49+
50+
51+
def test_load_entity_result_keeps_empty_list_as_list():
52+
result = OSW.LoadEntityResult(entities=[])
53+
54+
assert result.entities == []
55+
assert bool(result.entities) is False
56+
assert len(result.entities) == 0
57+
58+
59+
def test_load_entity_result_keeps_non_empty_list_and_subclass():
60+
item = model.Item(label=[model.Label(text="x")])
61+
62+
result = OSW.LoadEntityResult(entities=[item])
63+
64+
assert isinstance(result.entities, list)
65+
assert len(result.entities) == 1
66+
assert isinstance(result.entities[0], model.Item)
67+
68+
69+
def test_resolve_pairs_iris_by_title_not_position(monkeypatch):
70+
backend = _build_backend(monkeypatch)
71+
good = _make_entity("Item", "OSWAlignGood")
72+
73+
def _stub_load_entity(self, param):
74+
# "Item:OSWAlignBad" could not be built and load_entity() silently
75+
# skips it, so the returned list is shorter than request.iris.
76+
return OSW.LoadEntityResult(entities=[good])
77+
78+
monkeypatch.setattr(OSW, "load_entity", _stub_load_entity)
79+
80+
result = backend.resolve(
81+
ResolveParam(iris=["Item:OSWAlignBad", "Item:OSWAlignGood"])
82+
)
83+
84+
assert result.nodes == {"Item:OSWAlignGood": good}
85+
assert "Item:OSWAlignBad" not in result.nodes
86+
87+
88+
def test_resolve_returns_all_entities_when_every_iri_resolves(monkeypatch):
89+
backend = _build_backend(monkeypatch)
90+
first = _make_entity("Item", "OSWFirst")
91+
second = _make_entity("Item", "OSWSecond")
92+
93+
def _stub_load_entity(self, param):
94+
return OSW.LoadEntityResult(entities=[first, second])
95+
96+
monkeypatch.setattr(OSW, "load_entity", _stub_load_entity)
97+
98+
result = backend.resolve(ResolveParam(iris=["Item:OSWFirst", "Item:OSWSecond"]))
99+
100+
assert result.nodes == {"Item:OSWFirst": first, "Item:OSWSecond": second}

0 commit comments

Comments
 (0)