Skip to content

Commit 7c00cfa

Browse files
dmealingclaude
andcommitted
fix(naming): Python — refuse a generation run with two entities colliding on one API-surface name
Mirrors the TS/C#/Java fix: assert_no_collection_name_collisions (new, metaobjects.codegen.collection_name_collision) is the single choke point run_gen calls, over the full entity set, before any generator runs — refusing (ValueError carrying the new ERR_COLLECTION_NAME_COLLISION code) a model where two distinct entities/projections pluralize to the same API-surface collection name (route path, generated router module/finder names) — e.g. Address + Addresses, both landing on "Addresses". Reachable now that apidocs.naming.pluralize no longer doubles an already-plural word. Scoped to every object except object.value. ERR_COLLECTION_NAME_COLLISION registered in errors.py's ErrorCode enum — required here (unlike C#/Java, which use a local/plain-exception convention for codegen-time errors): Python's own error-ledger test (test_errors.py) asserts the enum is a superset of every code in the shared fixtures/conformance/ERROR-CODES.json, which the TS fix already added this code to. New test_collection_name_collision.py (distinct plurals, Address/Addresses, Order/Orders, object.value exclusion, the shared fixtures/naming- conformance/ collisionCases) plus an integration-level test in test_runner.py proving the gate is wired into run_gen itself. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M1mRRB1K2WdYy82kntMQdx
1 parent 8b5cb81 commit 7c00cfa

5 files changed

Lines changed: 199 additions & 0 deletions

File tree

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
"""Collection-name collision refusal — the single choke point every route/router/
2+
finder generator path goes through (called once, in ``run_gen``, over the full
3+
entity set BEFORE any generator runs).
4+
5+
Two DISTINCT entities/projections pluralizing to the SAME API-surface collection
6+
name (REST path, generated router module/finder names) is reachable from stock
7+
metadata now that :func:`metaobjects.apidocs.naming.pluralize` no longer doubles an
8+
already-plural word: ``Address`` (not already-plural: legacy-pluralizes to
9+
``Addresses``) and ``Addresses`` (already-plural: left unchanged, ALSO
10+
``Addresses``) collide, where before the fix they did not (``Addresses`` legacy-
11+
pluralized to ``Addresseses``). Silently letting two entities share one collection
12+
name would mean whichever generator runs second silently wins, or a straight
13+
duplicate-module/route registration two files downstream of the real cause. This
14+
refuses at the one place that can see the WHOLE set, with a message naming both
15+
entities and the colliding name.
16+
"""
17+
from __future__ import annotations
18+
19+
from metaobjects.apidocs.naming import pluralize
20+
from metaobjects.errors import ErrorCode
21+
from metaobjects.meta.core.object.meta_object import MetaObject
22+
from metaobjects.meta.core.object.object_constants import OBJECT_SUBTYPE_VALUE
23+
24+
ERR_COLLECTION_NAME_COLLISION = ErrorCode.ERR_COLLECTION_NAME_COLLISION.value
25+
26+
27+
def assert_no_collection_name_collisions(entities: list[MetaObject]) -> None:
28+
"""Refuse a generation run in which two DISTINCT entities/projections pluralize
29+
to the same API-surface collection name.
30+
31+
Scope: every object EXCEPT ``object.value`` (a value object never gets a route,
32+
router module, or finder name — it is pure shape, so it cannot collide on any of
33+
those). A TPH subtype IS included: it gets its own generated names against the
34+
shared base table, so two subtypes (or a subtype and an unrelated top-level
35+
entity) sharing a pluralized name collide on those generated symbols exactly as
36+
two top-level entities would.
37+
38+
A PURE function of the entity set — never of traversal order: the reported pair
39+
is deterministic (first-seen-by-input-order "owner" of a plural; raised as soon
40+
as a second, different entity claims the same plural).
41+
"""
42+
owner_of_plural: dict[str, MetaObject] = {}
43+
for obj in entities:
44+
if obj.sub_type == OBJECT_SUBTYPE_VALUE:
45+
continue
46+
plural = pluralize(obj.name)
47+
existing = owner_of_plural.get(plural)
48+
if existing is not None:
49+
if existing.resolution_key() != obj.resolution_key():
50+
raise ValueError(
51+
f"{ERR_COLLECTION_NAME_COLLISION}: \"{existing.name}\" and \"{obj.name}\" both "
52+
f'pluralize to the API-surface collection name "{plural}" (REST path, generated '
53+
"router module/finder names) — rename one entity so its pluralized name is "
54+
"distinct. Default PHYSICAL table names are unaffected by this rule and are not "
55+
"involved in the collision."
56+
)
57+
else:
58+
owner_of_plural[plural] = obj

‎server/python/src/metaobjects/codegen/runner.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from metaobjects.meta.meta_data import MetaData
1111
from metaobjects.meta.core.object.meta_object import MetaObject
1212
from metaobjects.shared.base_types import TYPE_OBJECT
13+
from .collection_name_collision import assert_no_collection_name_collisions
1314
from .config import GenConfig
1415
from .constants import generated_package_init
1516
from .generator import GenContext, Generator
@@ -107,6 +108,10 @@ def run_gen(
107108
if not safe:
108109
return result
109110

111+
# Refuse before any generator runs — the one choke point that sees the WHOLE
112+
# entity set every route/router-emitting generator below reads from.
113+
assert_no_collection_name_collisions(safe)
114+
110115
emitted: dict[str, tuple[str, str]] = {} # full_path -> (content, generated_by)
111116
for gen in generators:
112117
ctx = GenContext(

‎server/python/src/metaobjects/errors.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,12 @@ class ErrorCode(str, Enum):
200200
# Codegen (ADR-0044): two value-object FQNs in one payload artifact derive the same
201201
# emitted record name even after package-qualification. Peer of ERR_VAR_NOT_ON_PAYLOAD.
202202
ERR_PAYLOAD_NAME_COLLISION = "ERR_PAYLOAD_NAME_COLLISION"
203+
# Codegen: two DISTINCT entities/projections in one generation run pluralize to the
204+
# SAME API-surface collection name (route path, generated router/finder names) —
205+
# e.g. "Address" and "Addresses" both resolving to "Addresses". A codegen-time (not
206+
# loader) error, peer of ERR_PAYLOAD_NAME_COLLISION: refused rather than silently
207+
# letting one entity's route win.
208+
ERR_COLLECTION_NAME_COLLISION = "ERR_COLLECTION_NAME_COLLISION"
203209
ERR_OUTPUT_TAG_MISSING = "ERR_OUTPUT_TAG_MISSING"
204210
# SP-H Unit9 — @filterable: true on a field subtype with no filter-operator
205211
# band (e.g. field.object). Would silently generate an empty-ops filter.
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
"""assert_no_collection_name_collisions — the single choke point every route/router
2+
generator path goes through (called once, in run_gen, over the full entity set
3+
BEFORE any generator runs). See collection_name_collision.py's module docstring.
4+
"""
5+
from __future__ import annotations
6+
7+
import json
8+
from pathlib import Path
9+
10+
import pytest
11+
12+
from metaobjects import MetaDataLoader
13+
from metaobjects.meta.core.object.meta_object import MetaObject
14+
from metaobjects.shared.base_types import TYPE_OBJECT
15+
from metaobjects.codegen.collection_name_collision import (
16+
ERR_COLLECTION_NAME_COLLISION,
17+
assert_no_collection_name_collisions,
18+
)
19+
20+
_NAMING_CONFORMANCE_FIXTURE = (
21+
Path(__file__).parents[4] / "fixtures" / "naming-conformance" / "already-plural-pluralize.json"
22+
)
23+
24+
25+
def _entities(tmp_path: Path, *names: str) -> list:
26+
children = [
27+
{
28+
"object.entity": {
29+
"name": name,
30+
"children": [
31+
{"source.rdb": {"@table": name.lower()}},
32+
{"field.long": {"name": "id"}},
33+
{"identity.primary": {"@fields": "id"}},
34+
],
35+
}
36+
}
37+
for name in names
38+
]
39+
meta_dir = tmp_path / "meta"
40+
meta_dir.mkdir(parents=True, exist_ok=True)
41+
(meta_dir / "meta.json").write_text(
42+
json.dumps({"metadata.root": {"package": "acme", "children": children}})
43+
)
44+
root = MetaDataLoader.from_directory(meta_dir).root
45+
return [c for c in root.own_children() if c.type == TYPE_OBJECT and isinstance(c, MetaObject)]
46+
47+
48+
def _entity_and_value(tmp_path: Path, entity_name: str, value_name: str) -> list:
49+
doc = {
50+
"metadata.root": {
51+
"package": "acme",
52+
"children": [
53+
{
54+
"object.entity": {
55+
"name": entity_name,
56+
"children": [
57+
{"source.rdb": {"@table": entity_name.lower()}},
58+
{"field.long": {"name": "id"}},
59+
{"identity.primary": {"@fields": "id"}},
60+
],
61+
}
62+
},
63+
{
64+
"object.value": {
65+
"name": value_name,
66+
"children": [{"field.string": {"name": "text"}}],
67+
}
68+
},
69+
],
70+
}
71+
}
72+
meta_dir = tmp_path / "meta"
73+
meta_dir.mkdir(parents=True, exist_ok=True)
74+
(meta_dir / "meta.json").write_text(json.dumps(doc))
75+
root = MetaDataLoader.from_directory(meta_dir).root
76+
return [c for c in root.own_children() if c.type == TYPE_OBJECT and isinstance(c, MetaObject)]
77+
78+
79+
def test_does_not_raise_for_distinct_plurals(tmp_path: Path) -> None:
80+
assert_no_collection_name_collisions(_entities(tmp_path, "Post", "Author", "Category"))
81+
82+
83+
def test_raises_for_address_and_addresses(tmp_path: Path) -> None:
84+
with pytest.raises(ValueError) as exc_info:
85+
assert_no_collection_name_collisions(_entities(tmp_path, "Address", "Addresses"))
86+
msg = str(exc_info.value)
87+
assert ERR_COLLECTION_NAME_COLLISION in msg
88+
assert "Address" in msg
89+
assert "Addresses" in msg
90+
91+
92+
def test_raises_for_order_and_orders(tmp_path: Path) -> None:
93+
with pytest.raises(ValueError):
94+
assert_no_collection_name_collisions(_entities(tmp_path, "Order", "Orders"))
95+
96+
97+
def test_excludes_value_objects(tmp_path: Path) -> None:
98+
# "Address" (entity) pluralizes to "Addresses"; an unrelated object.value named
99+
# "Addresses" never gets a route, so it must not trip this.
100+
assert_no_collection_name_collisions(_entity_and_value(tmp_path, "Address", "Addresses"))
101+
102+
103+
def test_naming_conformance_collision_cases_are_refused(tmp_path: Path) -> None:
104+
cases = json.loads(_NAMING_CONFORMANCE_FIXTURE.read_text())["collisionCases"]
105+
for i, case in enumerate(cases):
106+
with pytest.raises(ValueError):
107+
assert_no_collection_name_collisions(
108+
_entities(tmp_path / str(i), case["entityA"], case["entityB"])
109+
)

‎server/python/tests/codegen/test_runner.py‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,3 +210,24 @@ def test_the_no_manifest_refusal_leads_with_the_performable_remedy(tmp_path: Pat
210210
assert "--baseline=adopt" in msgs[0]
211211
# What it must no longer do: prescribe committing a file this project cannot produce.
212212
assert "Commit '.metaobjects/.gen-state/.hashes.json' and re-run" not in msgs[0]
213+
214+
215+
def test_run_gen_refuses_a_model_with_colliding_collection_names(tmp_path: Path) -> None:
216+
"""Integration-level: proves the collision gate is wired into the real entry
217+
point (run_gen), not just callable as a standalone function. Address +
218+
Addresses both pluralize to "Addresses" — reachable now that an already-
219+
plural entity name is no longer double-pluralized."""
220+
root = _load(tmp_path / "meta", {"metadata.root": {"package": "acme", "children": [
221+
{"object.entity": {"name": "Address", "children": [
222+
{"source.rdb": {"@table": "address"}},
223+
{"field.long": {"name": "id"}},
224+
{"identity.primary": {"@fields": "id"}},
225+
]}},
226+
{"object.entity": {"name": "Addresses", "children": [
227+
{"source.rdb": {"@table": "addresses"}},
228+
{"field.long": {"name": "id"}},
229+
{"identity.primary": {"@fields": "id"}},
230+
]}},
231+
]}})
232+
with pytest.raises(ValueError):
233+
run_gen(GenConfig(out_dir=str(tmp_path / "out")), root, generators=[entity_model()])

0 commit comments

Comments
 (0)