Skip to content

Commit f4c7e01

Browse files
committed
fix(core): prefer a registered class over a generated one
- load_entity looked up classes by name in osw.model.entity only - a packaged class registered for the category IRI was invisible - the generated class then took over the oold type registry slot - warn instead of silently replacing a foreign registration - closes #138
1 parent a867b65 commit f4c7e01

2 files changed

Lines changed: 282 additions & 16 deletions

File tree

‎src/osw/core.py‎

Lines changed: 52 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
set_resolver,
3232
)
3333
from oold.generator import Generator
34+
from oold.model.v1 import _types as oold_type_registry
3435
from oold.utils.codegen import OOLDJsonSchemaParser
3536
from opensemantic.v1 import OswBaseModel
3637
from pydantic import PydanticDeprecatedSince20
@@ -1234,6 +1235,10 @@ def load_entity(
12341235
entity = None
12351236
schemas = []
12361237
schemas_fetched = True
1238+
# maps a category (page title, e.g. "Category:OSW...") to the class
1239+
# to use for it, either an already registered class (e.g. a packaged
1240+
# model class) or a freshly compiled one
1241+
category_to_cls: Dict[str, Type[model.Entity]] = {}
12371242
jsondata = page.get_slot_content("jsondata")
12381243
if param.remove_empty:
12391244
remove_empty(jsondata)
@@ -1255,21 +1260,50 @@ def load_entity(
12551260
# If a schema_to_use is provided, we do not need to check if the
12561261
# model exists
12571262
if not param.model_to_use:
1258-
if not hasattr(model, cls_name):
1259-
if param.autofetch_schema:
1260-
self.fetch_schema(
1261-
OSW.FetchSchemaParam(
1262-
schema_title=category,
1263-
mode="append",
1264-
offline_pages=param.offline_pages,
1263+
# Prefer a class already registered for this category IRI
1264+
# (e.g. a packaged model class) over compiling a new one.
1265+
# Compiling one anyway would take over the oold type
1266+
# registry entry for this category and hide the packaged
1267+
# class' typed fields/helpers.
1268+
registered_cls = oold_type_registry.get(category)
1269+
if registered_cls is not None:
1270+
category_to_cls[category] = registered_cls
1271+
else:
1272+
if not hasattr(model, cls_name):
1273+
if param.autofetch_schema:
1274+
self.fetch_schema(
1275+
OSW.FetchSchemaParam(
1276+
schema_title=category,
1277+
mode="append",
1278+
offline_pages=param.offline_pages,
1279+
)
12651280
)
1281+
if not hasattr(model, cls_name):
1282+
schemas_fetched = False
1283+
print(
1284+
f"Error: Model {cls_name} not found. Schema "
1285+
f"{category} needs to be fetched first."
12661286
)
1267-
if not hasattr(model, cls_name):
1268-
schemas_fetched = False
1269-
print(
1270-
f"Error: Model {cls_name} not found. Schema {category} "
1271-
f"needs to be fetched first."
1272-
)
1287+
else:
1288+
generated_cls = getattr(model, cls_name)
1289+
# The class we are about to use may have just
1290+
# claimed (or may already hold) the registry slot
1291+
# for this category. If a different class is
1292+
# registered for it, someone's registration was
1293+
# silently overwritten - do not raise, but make
1294+
# sure this does not pass silently.
1295+
conflicting_cls = oold_type_registry.get(category)
1296+
if (
1297+
conflicting_cls is not None
1298+
and conflicting_cls is not generated_cls
1299+
):
1300+
_logger.warning(
1301+
f"Class '{generated_cls}' generated for "
1302+
f"category '{category}' claims the oold "
1303+
f"type registry slot already held by a "
1304+
f"different class '{conflicting_cls}'."
1305+
)
1306+
category_to_cls[category] = generated_cls
12731307
if not schemas_fetched:
12741308
continue
12751309

@@ -1281,13 +1315,15 @@ def load_entity(
12811315
_logger.error("Error: no schema defined")
12821316

12831317
elif len(schemas) == 1:
1284-
cls: Type[model.Entity] = getattr(model, schemas[0]["title"])
1318+
# category_to_cls is fully populated for every category that
1319+
# reached this point (see the loop above)
1320+
cls: Type[model.Entity] = category_to_cls[jsondata["type"][0]]
12851321
entity: model.Entity = cls(**jsondata)
12861322

12871323
else:
12881324
bases = []
1289-
for schema in schemas:
1290-
bases.append(getattr(model, schema["title"]))
1325+
for category in jsondata["type"]:
1326+
bases.append(category_to_cls[category])
12911327
cls = create_model("Test", __base__=tuple(bases))
12921328
entity: model.Entity = cls(**jsondata)
12931329
except Exception as e:
Lines changed: 230 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,230 @@
1+
"""Unit tests for OSW.load_entity() preferring an already registered class.
2+
3+
Regression guard for #138: load_entity() used to decide whether to compile a
4+
class for a category by checking ``hasattr(model, cls_name)``, keyed by class
5+
name and only looking inside ``osw.model.entity``. This missed classes from
6+
packaged modules (e.g. ``opensemantic.base.v1._model.Database``) that are
7+
already registered for that category IRI in oold's type registry
8+
(``oold.model.v1._types``), causing load_entity() to silently compile and use
9+
a different, incomplete class instead.
10+
11+
These tests run fully offline: WtSite.get_page() is fed pages through
12+
``offline_pages`` (see ``osw.wtsite.WtSite.GetPageParam``), so no network or
13+
wiki credentials are required.
14+
"""
15+
16+
import json
17+
import threading
18+
import uuid as uuid_module
19+
from typing import Any, Dict, Union
20+
21+
from oold.model.v1 import _types as oold_type_registry
22+
from opensemantic.base.v1 import Database
23+
24+
import osw.model.entity as model
25+
from osw.core import OSW
26+
from osw.utils.wiki import remove_empty
27+
from osw.wtsite import WtPage, WtSite
28+
29+
30+
class OfflineWtPage(WtPage):
31+
"""A WtPage that pretends to exist without touching a wiki."""
32+
33+
def __init__(self, wtSite: Any = None, title: str = None):
34+
self.wtSite = wtSite
35+
self.title = title
36+
self.exists = True
37+
self._original_content = ""
38+
self.changed: bool = False
39+
self._dict = []
40+
self._slots: Dict[str, Union[str, dict]] = {"main": ""}
41+
self._slots_changed: Dict[str, bool] = {"main": False}
42+
self._content_model: Dict[str, str] = {"main": "wikitext"}
43+
44+
45+
class _FakeConnection:
46+
"""Just enough of a requests session for WtSite._clear_cookies()."""
47+
48+
cookies = []
49+
50+
51+
class _FakeMwSite:
52+
connection = _FakeConnection()
53+
54+
55+
def make_offline_wtsite() -> WtSite:
56+
"""A WtSite that never touches the network (bypasses __init__)."""
57+
ws = WtSite.__new__(WtSite)
58+
ws._page_cache = {}
59+
ws._cache_enabled = False
60+
ws._session_lock = threading.RLock()
61+
ws._site = _FakeMwSite()
62+
return ws
63+
64+
65+
def make_page_for_entity(entity) -> OfflineWtPage:
66+
"""Build an offline page whose jsondata slot holds the serialized entity."""
67+
page = OfflineWtPage(title=f"Item:{OSW.get_osw_id(entity.uuid)}")
68+
jsondata = json.loads(entity.json(exclude_none=True))
69+
remove_empty(jsondata)
70+
page.set_slot_content("jsondata", jsondata)
71+
return page
72+
73+
74+
def make_schema_page(category: str, cls_name: str) -> OfflineWtPage:
75+
"""Build an offline page holding the jsonschema slot for a category."""
76+
page = OfflineWtPage(title=category)
77+
page.set_slot_content("jsonschema", {"title": cls_name})
78+
return page
79+
80+
81+
def make_isolated_cls(name: str, base=model.Item):
82+
"""Build a model.Item subclass registered under its own private category IRI.
83+
84+
Overriding schema_extra's title/uuid makes get_cls_iri() derive a fresh
85+
"Category:OSW<uuid>" IRI for this class alone, so defining it cannot
86+
clobber the registration of any real category (e.g. "Category:Item").
87+
"""
88+
namespace = {
89+
"Config": type(
90+
"Config",
91+
(base.Config,),
92+
{
93+
"schema_extra": {
94+
**base.Config.schema_extra,
95+
"title": name,
96+
"uuid": str(uuid_module.uuid4()),
97+
}
98+
},
99+
),
100+
"__qualname__": name,
101+
}
102+
return type(base)(name, (base,), namespace)
103+
104+
105+
def test_load_entity_prefers_registered_class_over_generated_one():
106+
"""A class already registered for the category IRI is used as-is, and no
107+
replacement class is compiled into osw.model.entity for it."""
108+
assert not hasattr(model, "Database")
109+
110+
db = Database(name="TestDb", label=[model.Label(text="Test Db")])
111+
category = db.type[0]
112+
entity_page = make_page_for_entity(db)
113+
schema_page = make_schema_page(category, "Database")
114+
115+
osw_obj = OSW(site=make_offline_wtsite())
116+
117+
result = osw_obj.load_entity(
118+
OSW.LoadEntityParam(
119+
titles=[entity_page.title],
120+
autofetch_schema=True,
121+
offline_pages={
122+
entity_page.title: entity_page,
123+
category: schema_page,
124+
},
125+
)
126+
)
127+
128+
entity = result.entities[0]
129+
assert type(entity) is Database
130+
# the packaged class was used directly, nothing was compiled
131+
assert not hasattr(model, "Database")
132+
133+
134+
def test_load_entity_falls_back_to_generated_class_when_nothing_registered():
135+
"""A category with nothing registered in oold's type registry still gets
136+
the class already present in osw.model.entity, exactly like before."""
137+
category = "Category:OSWFakeCategoryNotRegistered00000000000000"
138+
cls_name = "FakeGeneratedClass"
139+
assert oold_type_registry.get(category) is None
140+
141+
fake_cls = make_isolated_cls(cls_name)
142+
setattr(model, cls_name, fake_cls)
143+
try:
144+
entity_page = OfflineWtPage(title="Item:OSWFakeEntity0000000000000000000000000")
145+
jsondata = {
146+
"type": [category],
147+
"uuid": "00000000-0000-0000-0000-000000000000",
148+
"name": "x",
149+
"label": [{"text": "x"}],
150+
}
151+
remove_empty(jsondata)
152+
entity_page.set_slot_content("jsondata", jsondata)
153+
schema_page = make_schema_page(category, cls_name)
154+
155+
osw_obj = OSW(site=make_offline_wtsite())
156+
157+
result = osw_obj.load_entity(
158+
OSW.LoadEntityParam(
159+
titles=[entity_page.title],
160+
autofetch_schema=True,
161+
offline_pages={
162+
entity_page.title: entity_page,
163+
category: schema_page,
164+
},
165+
)
166+
)
167+
168+
entity = result.entities[0]
169+
assert type(entity) is fake_cls
170+
finally:
171+
delattr(model, cls_name)
172+
173+
174+
def test_load_entity_warns_on_registry_conflict(monkeypatch, caplog):
175+
"""If the class about to be used for a category differs from whatever is
176+
now registered for that IRI, load_entity() logs a warning instead of
177+
silently letting the mismatch pass."""
178+
category = "Category:OSWConflictTest000000000000000000000000"
179+
cls_name = "ConflictGeneratedClass"
180+
assert oold_type_registry.get(category) is None
181+
182+
generated_cls = make_isolated_cls(cls_name)
183+
other_cls = make_isolated_cls("OtherRegisteredClass")
184+
185+
def fake_fetch_schema(self, fetchSchemaParam=None):
186+
# Simulate fetch_schema() compiling a class and importing it into
187+
# osw.model.entity, while a *different* class ends up holding the
188+
# oold registry slot for the same category.
189+
setattr(model, cls_name, generated_cls)
190+
oold_type_registry[category] = other_cls
191+
192+
monkeypatch.setattr(OSW, "fetch_schema", fake_fetch_schema)
193+
194+
try:
195+
entity_page = OfflineWtPage(
196+
title="Item:OSWConflictEntity00000000000000000000000000"
197+
)
198+
jsondata = {
199+
"type": [category],
200+
"uuid": "11111111-1111-1111-1111-111111111111",
201+
"name": "x",
202+
"label": [{"text": "x"}],
203+
}
204+
remove_empty(jsondata)
205+
entity_page.set_slot_content("jsondata", jsondata)
206+
schema_page = make_schema_page(category, cls_name)
207+
208+
osw_obj = OSW(site=make_offline_wtsite())
209+
210+
result = osw_obj.load_entity(
211+
OSW.LoadEntityParam(
212+
titles=[entity_page.title],
213+
autofetch_schema=True,
214+
offline_pages={
215+
entity_page.title: entity_page,
216+
category: schema_page,
217+
},
218+
)
219+
)
220+
221+
entity = result.entities[0]
222+
assert type(entity) is generated_cls
223+
assert any(
224+
"claims the oold type registry slot" in record.message
225+
for record in caplog.records
226+
)
227+
finally:
228+
if hasattr(model, cls_name):
229+
delattr(model, cls_name)
230+
oold_type_registry.pop(category, None)

0 commit comments

Comments
 (0)