Skip to content

Commit d32a737

Browse files
committed
fix: address CodeRabbit MAJOR review comments (PR #20)
- entities/store: rename graph relation key `schema_id` -> `type_id` to match GTS spec v0.10+ terminology (v0.13 rebase); update tests - x_gts_ref: avoid redundant `_without_x_gts_ref` recomputation in the combinator-stripping path to prevent exponential recursion on deep single-branch allOf/anyOf/oneOf chains - Makefile: set `SHELL := /bin/bash` so POSIX recipes don't fall back to cmd.exe via COMSPEC on Windows Signed-off-by: Artfizer <artifizer@gmail.com>
1 parent 82f18f2 commit d32a737

6 files changed

Lines changed: 24 additions & 11 deletions

File tree

‎Makefile‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
CI := 1
22

3+
# These recipes rely on POSIX tools (command -v, touch, rm -rf, sleep, kill,
4+
# cat) and POSIX syntax (background jobs, inline env assignments). Require Bash
5+
# explicitly so GNU Make does not fall back to cmd.exe via COMSPEC on Windows.
6+
# On Windows, run these targets from a Bash environment (e.g. Git Bash / MSYS2).
7+
SHELL := /bin/bash
8+
39
# Python: PYTHON_BOOTSTRAP is used only to create the virtual environment;
410
# PYTHON is the venv interpreter used by all other targets.
511
PYTHON_BOOTSTRAP ?= $(shell command -v python3 2>/dev/null || command -v python 2>/dev/null || echo python3)

‎gts/src/gts/entities.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -379,4 +379,4 @@ def get_graph(self) -> dict[str, set[str]]:
379379
refs = {}
380380
for r in self.gts_refs:
381381
refs[r["sourcePath"]] = r["id"]
382-
return {"id": self.gts_id.id, "schema_id": self.type_id, "refs": refs}
382+
return {"id": self.gts_id.id, "type_id": self.type_id, "refs": refs}

‎gts/src/gts/store.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -867,7 +867,7 @@ def gts2node(gts_id: str, seen_gts_ids: set[str]) -> str:
867867
if not entity.type_id.startswith(
868868
"http://json-schema.org"
869869
) and not entity.type_id.startswith("https://json-schema.org"):
870-
ret["schema_id"] = gts2node(entity.type_id, seen_gts_ids)
870+
ret["type_id"] = gts2node(entity.type_id, seen_gts_ids)
871871
else:
872872
ret["errors"] = ret.get("errors", []) + ["Schema not recognized"]
873873
else:

‎gts/src/gts/x_gts_ref.py‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,12 @@ def _without_x_gts_ref(schema: Any) -> Any:
2727
if key != "x-gts-ref"
2828
}
2929
for keyword in ("oneOf", "anyOf", "allOf"):
30-
branches = schema.get(keyword)
31-
if isinstance(branches, list) and _is_x_gts_ref_only_combinator(branches):
30+
branches = stripped.get(keyword)
31+
if (
32+
isinstance(branches, list)
33+
and branches
34+
and all(isinstance(branch, dict) and not branch for branch in branches)
35+
):
3236
stripped.pop(keyword, None)
3337
return stripped
3438
if isinstance(schema, list):
@@ -37,10 +41,13 @@ def _without_x_gts_ref(schema: Any) -> Any:
3741

3842

3943
def _is_x_gts_ref_only_combinator(branches: list[Any]) -> bool:
40-
return bool(branches) and all(
41-
isinstance(_without_x_gts_ref(branch), dict) and not _without_x_gts_ref(branch)
42-
for branch in branches
43-
)
44+
if not branches:
45+
return False
46+
for branch in branches:
47+
stripped = _without_x_gts_ref(branch)
48+
if not isinstance(stripped, dict) or stripped:
49+
return False
50+
return True
4451

4552

4653
def _is_structurally_valid(instance: Any, schema: Any) -> bool:

‎tests/test_entities.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,5 +335,5 @@ def test_get_graph_basic(self):
335335

336336
graph = entity.get_graph()
337337
assert graph["id"] == "gts.vendor.package.namespace.type.v1~"
338-
assert graph["schema_id"] == "gts.vendor.package.namespace.type.v1~"
338+
assert graph["type_id"] == "gts.vendor.package.namespace.type.v1~"
339339
assert "refs" in graph

‎tests/test_store_extra.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ def test_is_minor_compatible_valid(self):
306306

307307

308308
class TestBuildSchemaGraphWithRefs:
309-
def test_graph_includes_refs_and_schema_id(self):
309+
def test_graph_includes_refs_and_type_id(self):
310310
schema = _schema_entity("gts.x.test._.foo.v1~")
311311
instance = GtsEntity(
312312
content={
@@ -321,7 +321,7 @@ def test_graph_includes_refs_and_schema_id(self):
321321
store.register(instance)
322322
graph = store.build_schema_graph(instance.gts_id.id)
323323
assert graph["id"] == instance.gts_id.id
324-
assert "schema_id" in graph
324+
assert "type_id" in graph
325325

326326
def test_graph_skips_json_schema_org_refs(self):
327327
schema = _schema_entity("gts.x.test._.foo.v1~")

0 commit comments

Comments
 (0)