Skip to content

Commit 9b76a8f

Browse files
committed
fix: keep the stored jsonschema when store_entity regenerates it
- pass target_schema= instead of the ignored schema= (#215) - replace $defs/generated in DEFINITIONS_SECTION mode instead of merging - add offline regression tests through store_entity
1 parent 98320d1 commit 9b76a8f

2 files changed

Lines changed: 214 additions & 1 deletion

File tree

‎src/osw/core.py‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,29 @@ def read_file_if_exists(path: str) -> str:
210210
return f.read()
211211

212212

213+
def remove_generated_schema_section(
214+
schema: dict, def_key: str = "$defs", gen_def_key: str = "generated"
215+
) -> dict:
216+
"""Removes the generated section and the allOf entry that points to it.
217+
218+
aggregate_generated_schemas() merges into an existing generated section and
219+
merge_deep() concatenates arrays, so a stale 'required' entry or 'type' would
220+
survive every regeneration. Mirrors osl.util.postProcessPage in PageBot.js of
221+
mediawiki-extensions-OpenSemanticLab. Modifies schema in place.
222+
"""
223+
gen_def_pointer = f"#/{def_key}/{gen_def_key}"
224+
defs = schema.get(def_key)
225+
if isinstance(defs, dict):
226+
defs.pop(gen_def_key, None)
227+
if isinstance(schema.get("allOf"), list):
228+
schema["allOf"] = [
229+
entry
230+
for entry in schema["allOf"]
231+
if not (isinstance(entry, dict) and entry.get("$ref") == gen_def_pointer)
232+
]
233+
return schema
234+
235+
213236
# Reusable type definitions
214237
class OverwriteOptions(Enum):
215238
"""Options for overwriting properties"""
@@ -2084,9 +2107,17 @@ def store_entity_(
20842107
if characteristic_types and isinstance(entity_, characteristic_types):
20852108
mode = AggregateGeneratedSchemasParamMode.DEFINITIONS_SECTION
20862109

2110+
stored_schema = page.get_slot_content("jsonschema") or {}
2111+
# Regenerate the generated section from scratch. Skip this when
2112+
# the generation failed, so the stored section is kept.
2113+
if (
2114+
mode == AggregateGeneratedSchemasParamMode.DEFINITIONS_SECTION
2115+
and len(generated_schemas) > 0
2116+
):
2117+
remove_generated_schema_section(stored_schema)
20872118
new_schema = aggregate_generated_schemas(
20882119
AggregateGeneratedSchemasParam(
2089-
schema=page.get_slot_content("jsonschema"),
2120+
target_schema=stored_schema,
20902121
generated_schemas=generated_schemas,
20912122
mode=mode,
20922123
)
Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
"""Unit tests for the jsonschema slot regeneration in store_entity().
2+
3+
Regression guard for two defects in the schema aggregation of store_entity():
4+
5+
1. store_entity() passed the stored schema as `schema=`, a field that
6+
AggregateGeneratedSchemasParam does not have. Pydantic ignored it, so the
7+
aggregation always started from `{}` and every save dropped the hand-written
8+
part of the jsonschema slot.
9+
2. In DEFINITIONS_SECTION mode the new generated schema was merged into the
10+
existing `$defs/generated` instead of replacing it, so a `required` entry or a
11+
`type` from an earlier save survived every regeneration.
12+
13+
These run fully offline: store_entity() runs with offline=True, the meta category
14+
is served by a fake site and the overwrite policy is wrapped to put a stored
15+
jsonschema slot on the page, so no network is required.
16+
"""
17+
18+
import json
19+
20+
import pytest
21+
from opensemantic.core.v1 import CharacteristicType
22+
23+
import osw.model.entity as model
24+
from osw.core import OSW
25+
from osw.wtsite import WtPage
26+
27+
META_CATEGORY = "Category:OSWMetaCategory"
28+
GEN_POINTER = "#/$defs/generated"
29+
30+
31+
class _FakeMetaCategoryPage:
32+
def __init__(self, title, template):
33+
self.title = title
34+
self._slots = {"schema_template": template, "jsondata": {}}
35+
36+
def get_slot_content(self, slot_key):
37+
return self._slots[slot_key]
38+
39+
40+
class _FakeGetPageResult:
41+
def __init__(self, pages):
42+
self.pages = pages
43+
44+
45+
class _FakeSite:
46+
def __init__(self, template):
47+
self.template = template
48+
49+
def get_page(self, param):
50+
return _FakeGetPageResult([
51+
_FakeMetaCategoryPage(t, self.template) for t in param.titles
52+
])
53+
54+
55+
@pytest.fixture
56+
def store(monkeypatch):
57+
"""Runs store_entity() offline on a page whose jsonschema slot holds `stored`.
58+
59+
Returns the jsonschema slot that store_entity() wrote.
60+
"""
61+
original_policy = OSW._apply_overwrite_policy
62+
63+
def _store(entity, template, stored):
64+
def policy_with_stored_schema(param):
65+
if stored is not None:
66+
param.page.set_slot_content(
67+
"jsonschema", json.loads(json.dumps(stored))
68+
)
69+
return original_policy(param)
70+
71+
monkeypatch.setattr(
72+
OSW, "_apply_overwrite_policy", staticmethod(policy_with_stored_schema)
73+
)
74+
osw_obj = OSW.construct(site=_FakeSite(template))
75+
result = osw_obj.store_entity(
76+
OSW.StoreEntityParam(
77+
entities=[entity],
78+
namespace="Category",
79+
meta_category_title=META_CATEGORY,
80+
offline=True,
81+
)
82+
)
83+
(page,) = result.pages.values()
84+
assert isinstance(page, WtPage)
85+
return page.get_slot_content("jsonschema")
86+
87+
return _store
88+
89+
90+
def _characteristic():
91+
return CharacteristicType(
92+
uuid="efad8086-4a76-47ca-b278-f5f944e5754b",
93+
name="TestCharacteristic",
94+
label=[{"text": "Test Characteristic"}],
95+
# the default resolves a superclass from a wiki
96+
subclass_of=[],
97+
)
98+
99+
100+
def _category():
101+
# store_entity() selects ROOT_LEVEL mode for any entity that is not a
102+
# characteristic, so a plain Item stored in the Category namespace suffices
103+
return model.Item(
104+
uuid="0d0c1c35-4c4d-4e3a-9c9a-6f4a1b2c3d4e",
105+
name="TestCategory",
106+
label=[model.Label(text="Test Category")],
107+
)
108+
109+
110+
def test_root_level_keeps_hand_written_schema(store):
111+
stored = {"title": "hand", "properties": {"manual": {"type": "string"}}}
112+
template = '{"title": "{{name}}", "properties": {"gen": {"type": "number"}}}'
113+
114+
schema = store(_category(), template, stored)
115+
116+
assert schema["properties"]["manual"] == {"type": "string"}
117+
assert schema["properties"]["gen"] == {"type": "number"}
118+
assert schema["title"] == "TestCategory"
119+
120+
121+
def test_definitions_section_keeps_hand_written_schema(store):
122+
stored = {"title": "hand", "$defs": {"manual": {"type": "string"}}}
123+
template = '{"title": "{{name}}", "type": "object"}'
124+
125+
schema = store(_characteristic(), template, stored)
126+
127+
assert schema["$defs"]["manual"] == {"type": "string"}
128+
assert schema["$defs"]["generated"]["type"] == "object"
129+
assert schema["allOf"] == [{"$ref": GEN_POINTER}]
130+
131+
132+
def test_definitions_section_replaces_stale_generated_section(store):
133+
stored = {"title": "hand", "$defs": {"manual": {"type": "string"}}}
134+
first_template = (
135+
'{"title": "{{name}}", "type": "object", "required": ["type"],'
136+
' "properties": {"type": {"type": "string"}}}'
137+
)
138+
second_template = (
139+
'{"title": "{{name}}", "properties": {"value": {"type": "number"}}}'
140+
)
141+
142+
first = store(_characteristic(), first_template, stored)
143+
assert first["$defs"]["generated"]["required"] == ["type"]
144+
145+
second = store(_characteristic(), second_template, first)
146+
147+
generated = second["$defs"]["generated"]
148+
assert "required" not in generated
149+
assert "type" not in generated
150+
assert "type" not in generated["properties"]
151+
assert generated["properties"]["value"] == {"type": "number"}
152+
assert generated["$comment"].count(META_CATEGORY) == 1
153+
assert second["allOf"] == [{"$ref": GEN_POINTER}]
154+
assert second["$defs"]["manual"] == {"type": "string"}
155+
156+
157+
def test_failed_generation_keeps_generated_section(store):
158+
stored = {
159+
"title": "hand",
160+
"allOf": [{"$ref": GEN_POINTER}],
161+
"$defs": {"generated": {"type": "object", "required": ["type"]}},
162+
}
163+
# not valid JSON, so generation fails and produces no generated schema
164+
template = '{"title": {{name}}'
165+
166+
schema = store(_characteristic(), template, stored)
167+
168+
assert schema == stored
169+
170+
171+
def test_root_level_second_save_does_not_duplicate(store):
172+
stored = {"title": "hand", "properties": {"manual": {"type": "string"}}}
173+
template = (
174+
'{"title": "{{name}}", "required": ["gen"],'
175+
' "properties": {"gen": {"type": "number"}}}'
176+
)
177+
178+
first = store(_category(), template, stored)
179+
second = store(_category(), template, first)
180+
181+
assert second == first
182+
assert second["required"] == ["gen"]

0 commit comments

Comments
 (0)