Skip to content

Commit 9c2ad6e

Browse files
committed
fix(core): resolve bare $refs before dropping schema definitions
- register_schema only rewrote refs reachable via the $..allOf jsonpath - a nested model field without Field() metadata emits a bare $ref - those were left dangling once definitions was deleted - keep definitions when a local ref cannot be resolved - closes #101
1 parent 3a4a0f2 commit 9c2ad6e

2 files changed

Lines changed: 132 additions & 1 deletion

File tree

‎src/osw/core.py‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,29 @@ def register_schema(self, schema_registration: SchemaRegistration):
372372
else:
373373
result_array.append(subschema)
374374
match.full_path.update_or_create(schema, result_array)
375-
if "definitions" in schema:
375+
376+
all_refs_resolved = True
377+
bare_ref_jsonpath_expr = parse("$..dollarref")
378+
# pydantic only wraps a $ref in allOf when the field carries extra
379+
# metadata (e.g. Field(description=...)). A plain nested-model
380+
# field produces a bare {"$ref": ...} that is not reachable via
381+
# the "$..allOf" jsonpath above, so replace those local
382+
# definitions (#/definitions/...) with embedded definitions too,
383+
# to prevent a dangling ref once "definitions" is removed below
384+
for match in bare_ref_jsonpath_expr.find(schema):
385+
value = match.value
386+
if value.startswith("#"):
387+
definition_jsonpath_expr = parse(
388+
value.replace("#", "$").replace("/", ".")
389+
)
390+
def_matches = definition_jsonpath_expr.find(schema)
391+
for def_match in def_matches:
392+
match.context.full_path.update_or_create(
393+
schema, def_match.value
394+
)
395+
if not def_matches:
396+
all_refs_resolved = False
397+
if "definitions" in schema and all_refs_resolved:
376398
del schema["definitions"]
377399

378400
if "allOf" not in schema:
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
"""Unit tests for register_schema()'s $ref post-processing (issue #101).
2+
3+
Regression guard: pydantic v1 only wraps a nested-model $ref in "allOf" when
4+
the field carries extra metadata (e.g. Field(description=...)). A plain
5+
nested-model field produces a bare {"$ref": ...} that the old code never
6+
rewrote before deleting "definitions", leaving a dangling ref behind.
7+
8+
These run fully offline: WtPage.init and WtPage.edit are stubbed so no
9+
network is required, following the pattern in test_store_entity_failure.py.
10+
"""
11+
12+
import uuid
13+
14+
import pytest
15+
from pydantic.v1 import Field
16+
17+
import osw.model.entity as model
18+
from osw.core import OSW
19+
from osw.wtsite import WtPage
20+
21+
22+
class Inner(model.OswBaseModel):
23+
a: str = "x"
24+
25+
26+
class OuterBare(model.OswBaseModel):
27+
"""Nested field without Field(...) metadata: pydantic emits a bare $ref."""
28+
29+
inner: Inner
30+
31+
32+
class OuterAllOf(model.OswBaseModel):
33+
"""Nested field with Field(description=...): pydantic wraps the $ref in allOf."""
34+
35+
inner: Inner = Field(description="the inner thing")
36+
37+
38+
class _FakeMwSite:
39+
host = "example.org"
40+
41+
42+
class _FakeSite:
43+
mw_site = _FakeMwSite()
44+
45+
46+
@pytest.fixture
47+
def offline_osw(monkeypatch):
48+
# no network when a WtPage is constructed with do_init=True; mimic the
49+
# do_init=False branch of WtPage.__init__ which sets .exists
50+
monkeypatch.setattr(WtPage, "init", lambda self: setattr(self, "exists", False))
51+
captured_schemas = []
52+
53+
def fake_edit(self, *args, **kwargs):
54+
captured_schemas.append(self._slots.get("jsonschema"))
55+
56+
monkeypatch.setattr(WtPage, "edit", fake_edit)
57+
return OSW.construct(site=_FakeSite()), captured_schemas
58+
59+
60+
def _register_and_get_schema(offline_osw, model_cls, name):
61+
osw, captured_schemas = offline_osw
62+
osw.register_schema(
63+
OSW.SchemaRegistration(
64+
model_cls=model_cls,
65+
schema_uuid=str(uuid.uuid4()),
66+
schema_name=name,
67+
)
68+
)
69+
return captured_schemas[-1]
70+
71+
72+
def _has_dangling_ref(node):
73+
"""Recursively look for a "dollarref" (register_schema's stand-in for
74+
"$ref") anywhere in the schema. Since register_schema only ever emits
75+
local (#/definitions/...) refs, any leftover one is dangling once
76+
"definitions" is removed.
77+
"""
78+
if isinstance(node, dict):
79+
if "dollarref" in node:
80+
return True
81+
return any(_has_dangling_ref(v) for v in node.values())
82+
if isinstance(node, list):
83+
return any(_has_dangling_ref(v) for v in node)
84+
return False
85+
86+
87+
def test_bare_nested_ref_is_resolved_like_allof(offline_osw):
88+
"""A nested field with no Field(...) metadata must not leave a dangling
89+
ref behind, and must be embedded the same way an allOf-wrapped ref is.
90+
"""
91+
schema = _register_and_get_schema(offline_osw, OuterBare, "OuterBare")
92+
93+
assert not _has_dangling_ref(schema)
94+
assert "definitions" not in schema
95+
assert schema["properties"]["inner"]["title"] == "Inner"
96+
assert schema["properties"]["inner"]["properties"]["a"]["type"] == "string"
97+
98+
99+
def test_allof_wrapped_ref_is_still_resolved(offline_osw):
100+
"""Existing behaviour for allOf-wrapped refs must be unchanged."""
101+
schema = _register_and_get_schema(offline_osw, OuterAllOf, "OuterAllOf")
102+
103+
assert not _has_dangling_ref(schema)
104+
assert "definitions" not in schema
105+
assert schema["properties"]["inner"]["description"] == "the inner thing"
106+
assert schema["properties"]["inner"]["allOf"][0]["title"] == "Inner"
107+
assert (
108+
schema["properties"]["inner"]["allOf"][0]["properties"]["a"]["type"] == "string"
109+
)

0 commit comments

Comments
 (0)