Skip to content

Commit f86ac4e

Browse files
dmealingclaude
andcommitted
feat(#214): entity read-view codegen READ half — Python port
Fan the FR-024 §7 read-half (TS reference b4eee45) out to the Python port. It splits into a codegen half and a runtime half (schema stays TS-owned, ADR-0015). Shared metadata predicates (SSOT, mirror Java/Kotlin/C#): - MetaField.is_derived() — the field carries an origin.* child (own-only; matched by type string so the core field module needs no persistence-layer import). - MetaObject.is_write_through() — owns BOTH a writable-kind and a read-only-kind source.rdb (role-agnostic own-source classification — a replica view carries @ROLE:replica, so no primary-role accessor). Codegen: - entity_model.py: <Name>Create and <Name>Patch EXCLUDE derived fields (view- computed, not client input); the read model still carries them. - router_generator.py + filter_allowlist_generator.py: a write-through entity is writable, so both its CRUD router AND the filter allowlist its router imports are emitted — via an order-independent gate (else a view-source-first write-through router imports a never-generated allowlist module → FastAPI startup crash). Runtime (ObjectManager — the analogue of the TS queries-file): - reads (find_many / find_by_id / count) route to the replica VIEW (carrying the derived columns) for a write-through entity, else the entity's own source. - writes (create / update) target the table with derived fields excluded from BOTH the INSERT/UPDATE column set (a read-modify-write round-trip POSTing back the read model must not try to write a view-computed column) AND the RETURNING set (the table has no such column), then RE-READ the row through the view by PK so the returned dict carries the derived fields (read-your-writes). delete is unchanged. - vanilla entities and projections are byte-identical (they have no derived fields / are not write-through). Gated by test_write_through.py (read model carries derived, Create/Patch exclude it, router + filter-allowlist emitted regardless of source order) and test_object_manager_write_through.py (a fake driver asserts reads→view, writes→table with derived excluded from INSERT/UPDATE + RETURNING, and the by-PK view re-read). A high-effort review caught the filter-allowlist gate coupling and the write-column-set derived leak (both fixed above). Single-PK is the documented ObjectManager scope; a row-scoped replica view that does not surface a just-written row falls back to the table row (its derived fields absent) — an accepted edge for the 1:1-replica common case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NGQ7oSuNcjhsMHWwZzhBwr
1 parent 2345e8f commit f86ac4e

8 files changed

Lines changed: 397 additions & 13 deletions

File tree

server/python/src/metaobjects/codegen/generators/entity_model.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,8 @@ def _emit_create_lines(
496496
continue # FR-036 #2 — a server-generated PK is never in the create body
497497
if f.attrs().get(fc.FIELD_ATTR_READ_ONLY) is True:
498498
continue # FR-013 — a read-only column is DB/owner-written, not create input
499+
if f.is_derived():
500+
continue # FR-024 §7 (#214) — a derived (origin.*) field is view-computed, not create input
499501
line, used = _create_field_line(f, imports, cfg)
500502
uses_field = uses_field or used
501503
lines.append(line)
@@ -529,6 +531,8 @@ def _emit_patch_lines(
529531
for f in entity.fields():
530532
if f.name in pk_fields:
531533
continue # FR-036 #4 — the PK is route-authoritative, never patched
534+
if f.is_derived():
535+
continue # FR-024 §7 (#214) — a derived (origin.*) field is view-computed, never patched
532536
line, used = _patch_field_line(f, imports, cfg)
533537
uses_field = uses_field or used
534538
lines.append(line)

server/python/src/metaobjects/codegen/generators/filter_allowlist_generator.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,12 @@ def render_filter_allowlist(
250250
src = _primary_source_rdb(entity)
251251
if src is None:
252252
return None
253-
if src.effective_kind() != SOURCE_KIND_TABLE:
253+
# FR-024 §7 (#214): a write-through entity read-view is writable and its generated
254+
# router imports this allowlist — so it MUST be emitted for write-through regardless of
255+
# source declaration order (matching the router gate; else a view-source-first
256+
# write-through router imports a never-generated module and FastAPI startup crashes). A
257+
# projection (read-only source only) is not write-through, so it still skips.
258+
if not entity.is_write_through() and src.effective_kind() != SOURCE_KIND_TABLE:
254259
return None
255260

256261
short_name = entity.name

server/python/src/metaobjects/codegen/generators/router_generator.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -645,7 +645,12 @@ def render_router(
645645
src = _primary_source_rdb(entity)
646646
if src is None:
647647
return None
648-
if src.effective_kind() != SOURCE_KIND_TABLE:
648+
# FR-024 §7 (#214): a write-through entity read-view is writable (owns a table
649+
# source) and MUST emit its CRUD router regardless of source declaration order —
650+
# the first-source gate below would wrongly skip a view-source-first write-through
651+
# entity. Reads route to the replica view in the ObjectManager (not here). A
652+
# projection (read-only source only) is not write-through, so it still skips.
653+
if not entity.is_write_through() and src.effective_kind() != SOURCE_KIND_TABLE:
649654
return None
650655

651656
# FR-017 TPH: a discriminator base emits a polymorphic collection at the base

server/python/src/metaobjects/meta/core/field/meta_field.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from __future__ import annotations
33

44
from ....datatype import DataType
5+
from ....shared.base_types import TYPE_ORIGIN
56
from ...meta_data import MetaData
67
from . import field_constants as fc
78

@@ -53,6 +54,20 @@ def object_ref(self) -> str | None:
5354
v = self.get_meta_attr(fc.FIELD_ATTR_OBJECT_REF)
5455
return str(v) if v is not None else None
5556

57+
def is_derived(self) -> bool:
58+
"""Whether this field is DERIVED — it carries an ``origin.*`` child
59+
(``passthrough`` / ``aggregate`` / …), so its value is computed from a read
60+
source (a view join or aggregate), not stored on the writable table.
61+
62+
Derived ⇒ read-only wherever the field lives (ADR-0028): excluded from the
63+
write table + create/update inputs, carried only on the read shape (FR-024 §7,
64+
#214). Mirrors the TS/Java ``MetaField.isDerived()``.
65+
66+
OWN-only (ADR-0029/0039): an ``origin.*`` never inherits via ``extends``, so
67+
this reads own children (matched by ``type`` so no ``MetaOrigin`` import is
68+
needed — the core field module must not depend on the persistence layer)."""
69+
return any(c.type == TYPE_ORIGIN for c in self.own_children())
70+
5671
def get_value(self, obj: object, name: str | None = None) -> object:
5772
"""Read this field's value from a backing object.
5873

server/python/src/metaobjects/meta/core/object/meta_object.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""MetaObject — typed accessors over children + runtime instantiation."""
22
from __future__ import annotations
33

4+
from ....shared.base_types import TYPE_SOURCE
45
from ...meta_data import MetaData
56
from ..field.meta_field import MetaField
67
from .object_class_registry import (
@@ -32,6 +33,27 @@ def get_meta_field(self, name: str) -> MetaField | None:
3233
"""Alias for find_field — Java/TS parity (``getMetaField``)."""
3334
return self.find_field(name)
3435

36+
def is_write_through(self) -> bool:
37+
"""Whether this object is a WRITE-THROUGH entity read-view (FR-024 §7, #214) —
38+
it owns BOTH a writable-kind ``source.rdb`` (a ``@kind: table``) AND a
39+
read-only-kind ``source.rdb`` (a ``@kind: view`` / …). Writes route to the
40+
table; reads route to the (derived-field-carrying) view.
41+
42+
OWN-only + role-agnostic (mirrors the TS/Java ``isWriteThrough``): classified by
43+
the source ``@kind`` of this object's OWN ``source.*`` children — a replica view
44+
carries ``@role: replica``, so this must NOT go through a primary-role accessor.
45+
Duck-typed on ``type == TYPE_SOURCE`` + ``is_read_only()`` so the core object
46+
module needs no persistence-layer import."""
47+
has_writable = has_read_only = False
48+
for c in self.own_children():
49+
if c.type != TYPE_SOURCE:
50+
continue
51+
if c.is_read_only():
52+
has_read_only = True
53+
else:
54+
has_writable = True
55+
return has_writable and has_read_only
56+
3557
def new_instance(self, registry: ObjectClassRegistry | None = None) -> object:
3658
"""Instantiate a backing object for this MetaObject (Java/TS parity:
3759
``newInstance``). Resolution order:

server/python/src/metaobjects/runtime/object_manager.py

Lines changed: 65 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -273,14 +273,22 @@ def _insert_row(
273273
raise ValueError(
274274
f"create('{entity_name}'): no field '{field_name}' in metadata"
275275
)
276+
# #214: a DERIVED (origin.*) field has no column on the write table — it is
277+
# view-computed. A read-modify-write round-trip (a client POSTs back the read
278+
# model, which carries derived fields) must not try to INSERT it; drop it here.
279+
if f.is_derived():
280+
continue
276281
insert_cols.append(_column_of(f))
277282
params.append(_coerce_write_value(f, raw))
278283

279-
# Always RETURNING the full physical column set so the inserted row —
280-
# including any server-generated PK / default — comes back, then map
281-
# columns → metadata field names for cross-port row-shape parity.
282-
all_cols = [_column_of(f) for f in entity.fields()]
283-
col_to_field = {_column_of(f): f.name for f in entity.fields()}
284+
# RETURNING the physical column set so the inserted row — including any
285+
# server-generated PK / default — comes back, then map columns → metadata field
286+
# names for cross-port row-shape parity. #214: a DERIVED (origin.*) field has no
287+
# column on the write table (it is computed by the replica view), so it is
288+
# excluded from RETURNING; the write-through re-read below fetches it via the view.
289+
returning_fields = [f for f in entity.fields() if not f.is_derived()]
290+
all_cols = [_column_of(f) for f in returning_fields]
291+
col_to_field = {_column_of(f): f.name for f in returning_fields}
284292

285293
if insert_cols:
286294
col_list = ", ".join(_q(c) for c in insert_cols)
@@ -302,7 +310,15 @@ def _insert_row(
302310
col_to_field.get(c, c): oid for c, oid in result.column_oids.items()
303311
}
304312
row = result.rows[0]
305-
return {col_to_field.get(k, k): v for k, v in row.items()}
313+
mapped = {col_to_field.get(k, k): v for k, v in row.items()}
314+
# #214: a write-through entity's INSERT RETURNING covers only the table (non-derived)
315+
# columns; re-read the persisted row through the replica VIEW by PK so the returned row
316+
# carries the derived origin.* fields (read-your-writes). find_by_id routes to the view.
317+
if entity.is_write_through():
318+
reread = self.find_by_id(entity_name, mapped[self._primary_pk_field(entity)])
319+
if reread is not None:
320+
return reread
321+
return mapped
306322

307323
def update(
308324
self,
@@ -366,15 +382,24 @@ def update(
366382
raise ValueError(
367383
f"update('{entity_name}'): no field '{field_name}' in metadata"
368384
)
385+
# #214: a DERIVED (origin.*) field has no column on the write table (view-computed).
386+
# A read-modify-write round-trip PATCHing back the whole read model must not try to
387+
# UPDATE it; drop it from the SET list.
388+
if f.is_derived():
389+
continue
369390
set_cols.append(_column_of(f))
370391
params.append(_coerce_write_value(f, raw))
371392
if not set_cols:
372393
# No columns to set → just read the (scoped) row back by PK (no-op update).
373394
row = self.find_by_id(entity_name, id_value)
374395
return self._on_missing_update(entity_name, pk_field, id_value, if_missing) if row is None else row
375396

376-
all_cols = [_column_of(f) for f in entity.fields()]
377-
col_to_field = {_column_of(f): f.name for f in entity.fields()}
397+
# #214: exclude DERIVED (origin.*) fields from RETURNING — they have no column on
398+
# the write table (computed by the replica view); the write-through re-read below
399+
# fetches them via the view.
400+
returning_fields = [f for f in entity.fields() if not f.is_derived()]
401+
all_cols = [_column_of(f) for f in returning_fields]
402+
col_to_field = {_column_of(f): f.name for f in returning_fields}
378403
assignments = ", ".join(f"{_q(c)} = %s" for c in set_cols)
379404
params.append(_coerce_write_value(entity.find_field(pk_field), id_value))
380405
where = f"{_q(pk_col)} = %s"
@@ -397,7 +422,14 @@ def update(
397422
col_to_field.get(c, c): oid for c, oid in result.column_oids.items()
398423
}
399424
row = result.rows[0]
400-
return {col_to_field.get(k, k): v for k, v in row.items()}
425+
mapped = {col_to_field.get(k, k): v for k, v in row.items()}
426+
# #214: re-read the updated row through the replica VIEW by PK so the returned row
427+
# carries the derived origin.* fields (write targeted the table, which lacks them).
428+
if entity.is_write_through():
429+
reread = self.find_by_id(entity_name, id_value)
430+
if reread is not None:
431+
return reread
432+
return mapped
401433

402434
@staticmethod
403435
def _on_missing_update(
@@ -442,7 +474,10 @@ def find_many(
442474
offset: int | None = None,
443475
) -> list[dict[str, Any]]:
444476
entity = self._require_entity(entity_name)
445-
table = self._table_name(entity)
477+
# #214: reads route to the replica VIEW for a write-through entity (carrying the
478+
# derived origin.* columns), else the entity's own primary source. All effective
479+
# fields (incl. derived) are selected — the view exposes them.
480+
table = self._read_source_name(entity)
446481
cols = [_column_of(f) for f in entity.fields()]
447482
sql = f'SELECT {", ".join(_q(c) for c in cols)} FROM {_q(table)}'
448483
params: list[Any] = []
@@ -480,7 +515,9 @@ def find_many(
480515

481516
def count(self, entity_name: str, filter: Filter | None = None) -> int:
482517
entity = self._require_entity(entity_name)
483-
table = self._table_name(entity)
518+
# #214: count reads through the same source as find_many (the replica view for a
519+
# write-through entity — a 1:1 replica, so the count is unchanged).
520+
table = self._read_source_name(entity)
484521
sql = f"SELECT COUNT(*) FROM {_q(table)}"
485522
params: list[Any] = []
486523
filter = self._scope_filter(filter, tph_subtype_of(entity)) # FR-017 TPH subtype scope
@@ -591,6 +628,23 @@ def _table_name(self, entity: MetaObject) -> str:
591628
return pn
592629
return entity.name
593630

631+
def _read_source_name(self, entity: MetaObject) -> str:
632+
"""The physical relation READS run against. FR-024 §7 (#214): a write-through
633+
entity read-view reads through its read-only REPLICA VIEW (which carries the
634+
derived ``origin.*`` columns), while writes still target the table
635+
(:meth:`_table_name`). Every other object reads its own primary source (=
636+
``_table_name``), so vanilla/projection behavior is unchanged.
637+
638+
OWN + role-agnostic: the replica view carries ``@role: replica``, so it is found
639+
by read-only kind among the entity's own sources, not a primary-role accessor."""
640+
if entity.is_write_through():
641+
for c in entity.own_children():
642+
if isinstance(c, MetaSource) and c.is_read_only():
643+
pn = c.physical_name()
644+
if pn:
645+
return pn
646+
return self._table_name(entity)
647+
594648
@staticmethod
595649
def _scope_filter(filter: Filter | None, tph: TphSubtype | None) -> Filter | None:
596650
"""AND the TPH discriminator predicate into a filter (subtype-scoped reads).
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
"""FR-024 §7 (#214) — write-through entity read-view codegen (Python port).
2+
3+
A write-through entity declares BOTH a writable ``source.rdb`` (@kind:table) and a
4+
read-only replica ``source.rdb`` (@role:replica @kind:view) plus DERIVED fields (a
5+
field carrying an ``origin.*`` child). The codegen contract mirrors the TS reference
6+
(``b4eee456``): the read model ``<Name>`` carries the derived field, while the write
7+
models ``<Name>Create`` / ``<Name>Patch`` EXCLUDE it (it is view-computed, not client
8+
input). The router IS emitted (a write-through entity is writable), regardless of
9+
source declaration order.
10+
11+
Canonical fixture: Customer + write-through Order (replica view ``v_order_with_customer``,
12+
derived ``customerName`` passthrough from ``Customer.name`` via the ``customer`` join).
13+
"""
14+
from __future__ import annotations
15+
16+
import json
17+
18+
import metaobjects.core_types # noqa: F401 — side-effect: registers attr classes
19+
from metaobjects import load_string
20+
from metaobjects.codegen.generators.entity_model import render_entity_model
21+
from metaobjects.codegen.generators.filter_allowlist_generator import render_filter_allowlist
22+
from metaobjects.codegen.generators.router_generator import render_router
23+
from metaobjects.meta.core.object.meta_object import MetaObject
24+
25+
# Table source declared FIRST.
26+
_META = {
27+
"metadata.root": {
28+
"package": "acme::sales",
29+
"children": [
30+
{
31+
"object.entity": {
32+
"name": "Customer",
33+
"children": [
34+
{"source.rdb": {"@table": "customers"}},
35+
{"field.long": {"name": "id"}},
36+
{"field.string": {"name": "name", "@required": True}},
37+
{"identity.primary": {"name": "pk", "@fields": ["id"], "@generation": "increment"}},
38+
],
39+
}
40+
},
41+
{
42+
"object.entity": {
43+
"name": "Order",
44+
"children": [
45+
{"source.rdb": {"@role": "primary", "@table": "orders"}},
46+
{"source.rdb": {"@role": "replica", "@kind": "view", "@view": "v_order_with_customer"}},
47+
{"field.long": {"name": "id"}},
48+
{"field.long": {"name": "customerId", "@required": True}},
49+
{"field.string": {"name": "customerName", "children": [
50+
{"origin.passthrough": {"@from": "Customer.name", "@via": "Order.customer"}}]}},
51+
{"relationship.association": {"name": "customer", "@objectRef": "Customer", "@cardinality": "one"}},
52+
{"identity.primary": {"name": "pk", "@fields": ["id"], "@generation": "increment"}},
53+
{"identity.reference": {"name": "ref_customer", "@fields": ["customerId"], "@references": "Customer"}},
54+
],
55+
}
56+
},
57+
],
58+
}
59+
}
60+
61+
# Same, but with the replica VIEW source declared BEFORE the table — guards that
62+
# write-through detection / router emission is source-order-independent.
63+
_META_VIEW_FIRST = json.loads(json.dumps(_META))
64+
_order = _META_VIEW_FIRST["metadata.root"]["children"][1]["object.entity"]["children"]
65+
_order[0], _order[1] = _order[1], _order[0] # swap the two source.rdb declarations
66+
67+
68+
def _order_obj(meta: dict) -> MetaObject:
69+
root = load_string(json.dumps(meta)).root
70+
return next(c for c in root.own_children()
71+
if isinstance(c, MetaObject) and c.name == "Order")
72+
73+
74+
def test_read_model_carries_derived_write_models_exclude_it() -> None:
75+
order = _order_obj(_META)
76+
assert order.is_write_through()
77+
78+
# render_entity_model emits the whole module: the read model `Order` plus the
79+
# write shapes `OrderCreate` / `OrderPatch`. EXEC it (a syntax/name/import defect
80+
# fails here), then introspect each Pydantic model's field set.
81+
src = render_entity_model(order)
82+
ns: dict[str, object] = {}
83+
exec(compile(src, "<Order model>", "exec"), ns) # noqa: S102
84+
85+
read_fields = set(ns["Order"].model_fields)
86+
create_fields = set(ns["OrderCreate"].model_fields)
87+
patch_fields = set(ns["OrderPatch"].model_fields)
88+
89+
# the read model carries the derived field; the write shapes EXCLUDE it.
90+
assert "customerName" in read_fields
91+
assert "customerName" not in create_fields
92+
assert "customerName" not in patch_fields
93+
# the stored writable field is present everywhere.
94+
assert "customerId" in read_fields
95+
assert "customerId" in create_fields
96+
assert "customerId" in patch_fields
97+
98+
99+
def test_write_through_router_is_emitted_regardless_of_source_order() -> None:
100+
# A write-through entity is writable → it gets a CRUD router (a read-only
101+
# projection returns None). Order-independent: the view-first fixture too.
102+
assert render_router(_order_obj(_META)) is not None
103+
assert render_router(_order_obj(_META_VIEW_FIRST)) is not None
104+
assert _order_obj(_META_VIEW_FIRST).is_write_through()
105+
106+
107+
def test_write_through_filter_allowlist_is_emitted_with_the_router() -> None:
108+
# The generated router imports the entity's filter allowlist, so the allowlist
109+
# generator MUST emit for a write-through entity too — regardless of source order,
110+
# else a view-first router imports a never-generated module (FastAPI startup crash).
111+
assert render_filter_allowlist(_order_obj(_META)) is not None
112+
assert render_filter_allowlist(_order_obj(_META_VIEW_FIRST)) is not None

0 commit comments

Comments
 (0)