From 05dcf98ebbf993a3311333bad0a48532144f8910 Mon Sep 17 00:00:00 2001 From: "Benito J. Gonzalez" Date: Mon, 22 Jun 2026 22:15:15 -0700 Subject: [PATCH 1/5] Issue #756: honor inlined entity references on schema create-by-upload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MDR's schema generator inlines entity references as objects under a "Ref"-infix property key ("Ref" / "Ref", see schema_generation_service.add_ref) rather than emitting an OpenAPI "$ref". The upload reader only looked for "$ref", so every reference in an MDR-exported schema was silently dropped on re-upload — and the create pass then materialized each reference as a bogus child entity. Detect inlined references (is_inlined_reference / parse_reference_key) alongside genuine "$ref", resolve the referenced and parent entities (UniqueName from embedded metadata, falling back to the name parsed from the key), and create the Reference-placement EntityAssociation with the correct relationship. Skip references in the entity-creation pass so they are no longer created as child entities. Pass the entity's own key name into the post-pass for robust parent resolution. Detection keys off the "Ref" infix per the issue report; an in-code NOTE records the preferred two-sided fix (an explicit generator-stamped marker) since the infix is ambiguous for entities legitimately named "Ref". Add unit tests for the helpers and the reference post-pass. Co-Authored-By: Claude Opus 4.8 --- .../lif/mdr_services/schema_upload_service.py | 122 ++++++++++++----- .../test_schema_upload_service.py | 125 ++++++++++++++++++ 2 files changed, 217 insertions(+), 30 deletions(-) create mode 100644 test/components/lif/mdr_services/test_schema_upload_service.py diff --git a/components/lif/mdr_services/schema_upload_service.py b/components/lif/mdr_services/schema_upload_service.py index 6e3388c9..fcf1a2d1 100644 --- a/components/lif/mdr_services/schema_upload_service.py +++ b/components/lif/mdr_services/schema_upload_service.py @@ -40,12 +40,48 @@ def parse_dt(val): raise TypeError(f"Unsupported date value: {val!r}") +# --- Entity-reference detection (#756) --------------------------------------------------- +# MDR's schema generator (schema_generation_service.add_ref) encodes a reference to another +# entity as an INLINED object — NOT an OpenAPI "$ref" — stored under a property key carrying a +# "Ref" infix: "Ref" for a plain reference, or "Ref" when the +# association has a relationship (see schema_generation_service.py:802-805). The inlined object +# is a deep copy of the referenced entity's schema narrowed to its required fields, so when the +# schema was generated with entity metadata it still carries the child's UniqueName/DataModelId. +# +# Before #756 this reader only looked for "$ref" and silently dropped every reference in an +# MDR-exported schema (and, worse, the create pass then materialized each reference as a bogus +# child entity — see create_entity_and_children_if_needed). +# +# NOTE — preferred long-term fix: keying off the "Ref" infix is inherently ambiguous (an entity +# legitimately named "Ref" would mis-parse). The robust fix is two-sided: have add_ref +# stamp an explicit marker on the inlined object (e.g. "x-lif-reference-to": "") and +# key off that here instead of string-parsing the property name. +REFERENCE_KEY_MARKER = "Ref" + + +def parse_reference_key(prop_name: str) -> tuple[Optional[str], str]: + """Split an inlined-reference key into (relationship, child_entity_name). + + "RefOrganization" -> (None, "Organization") + "issuedByRefOrganization" -> ("issuedBy", "Organization") + """ + relationship, _, child_name = prop_name.partition(REFERENCE_KEY_MARKER) + return (relationship or None), child_name + + +def is_inlined_reference(prop_name: str, prop) -> bool: + """True if `prop` is an MDR inlined entity reference rather than an embedded child or attribute.""" + if not isinstance(prop, dict) or "$ref" in prop or "ValueSetId" in prop: + return False + _, child_name = parse_reference_key(prop_name) + # The marker must be followed by a PascalCase entity name; this keeps an embedded child whose + # own name merely contains "Ref" (e.g. "Reference") from being misread as a reference. + return bool(child_name) and child_name[0].isupper() + + async def create_reference_entity_association_if_needed( - session: AsyncSession, ref_name, referenced_entity, parent_entity_id, data_model_id + session: AsyncSession, referenced_entity, parent_entity_id, relationship, data_model_id ): - # relationship = the text prepended on ref_name that is not part of the referenced entity name - # e.g. for "issuedByOrganization" if referenced_entity.Name is "Organization", relationship is "issuedBy" - relationship = ref_name[: ref_name.index(referenced_entity.Name)] # Check if an EntityAssociation already exists entity_association = await get_entity_association_by_parent_child_relationship( session, parent_entity_id, referenced_entity.Id, relationship, data_model_id @@ -69,39 +105,62 @@ async def create_reference_entity_association_if_needed( async def create_reference_associations_for_children( - session: AsyncSession, entity_md: Dict, data_model_id: int, openapi_schema: Dict, data_model_type: str + session: AsyncSession, + entity_name: str, + entity_md: Dict, + data_model_id: int, + openapi_schema: Dict, + data_model_type: str, ): entity_properties = entity_md.get("properties", {}) for prop_name, prop in entity_properties.items(): - if "$ref" in prop: # This is a reference to another entity - ref_path = prop[ - "$ref" - ] # Assume path is always in format like #/components/schemas/EntityName or #/components/schemas/ParentEntityName/properties/ChildEntityName (this could continue for deeper nesting) - # 1. Find where it is in the openapi_schema - referenced_entity_md = resolve_ref(openapi_schema, ref_path) - # 2. Determine its Entity Id + if not isinstance(prop, dict): + continue + + referenced_entity_md = None + relationship = None + child_name = None + if "$ref" in prop: # externally-authored OpenAPI reference + # Path is in a format like #/components/schemas/EntityName (possibly nested deeper). + referenced_entity_md = resolve_ref(openapi_schema, prop["$ref"]) + elif is_inlined_reference(prop_name, prop): # MDR-exported inlined reference (#756) + referenced_entity_md = prop + relationship, child_name = parse_reference_key(prop_name) + + if referenced_entity_md is not None: + # Resolve the referenced entity. Prefer UniqueName from embedded metadata; otherwise + # fall back to the child name parsed from the key (entities created in the first pass + # default their UniqueName to their schema-key name). + referenced_unique_name = referenced_entity_md.get("UniqueName") or child_name referenced_entity = await get_unique_entity( + session, referenced_unique_name, data_model_id, referenced_entity_md.get("DataModelId"), data_model_type + ) + parent_entity = await get_unique_entity( session, - referenced_entity_md.get("UniqueName"), + entity_md.get("UniqueName") or entity_name, data_model_id, - referenced_entity_md.get("DataModelId"), + entity_md.get("DataModelId"), data_model_type, ) - logger.info(f"Referenced entity unique name: {referenced_entity_md.get('UniqueName')}") - # Determine parent entity id - logger.info(f"Parent entity: {entity_md}") - parent_entity = await get_unique_entity( - session, entity_md.get("UniqueName"), data_model_id, entity_md.get("DataModelId"), data_model_type - ) - parent_entity_id = parent_entity.Id - # 3. Create an EntityAssociation if needed - await create_reference_entity_association_if_needed( - session, prop_name, referenced_entity, parent_entity_id, data_model_id - ) - # Go through this process recursively + if referenced_entity and parent_entity: + if child_name is None: + # $ref path: derive the relationship from the property-name prefix, e.g. + # "issuedByOrganization" -> "issuedBy" for referenced entity "Organization". + idx = prop_name.find(referenced_entity.Name) + relationship = prop_name[:idx] or None if idx > 0 else None + logger.info(f"Reference {prop_name!r} -> entity {referenced_unique_name!r} (rel={relationship!r})") + await create_reference_entity_association_if_needed( + session, referenced_entity, parent_entity.Id, relationship, data_model_id + ) + else: + logger.warning(f"Could not resolve reference {prop_name!r} on entity {entity_name!r}") + # An inlined reference is a leaf (narrowed required fields); do not recurse into it. + continue + + # Recurse into genuine child entities. if "properties" in prop: await create_reference_associations_for_children( - session, prop, data_model_id, openapi_schema, data_model_type + session, prop_name, prop, data_model_id, openapi_schema, data_model_type ) @@ -457,8 +516,11 @@ async def create_entity_and_children_if_needed( # Process child entities if any entity_properties = entity_md.get("properties", {}) for prop_name, prop in entity_properties.items(): - if "$ref" in prop: - continue # Skip $ref properties for now; we need to make sure all entities are created to reference to first. + if "$ref" in prop or is_inlined_reference(prop_name, prop): + # References (OpenAPI "$ref" or MDR inlined "Ref") point at entities created + # elsewhere; they are wired up in the post-pass (create_reference_associations_for_children), + # not materialized as embedded child entities here. (#756) + continue if "ValueSetId" not in prop: # It's a child entity child_entity = await create_entity_and_children_if_needed( session, prop_name, prop, data_model_id, contributor, contributor_organization, data_model_type @@ -594,7 +656,7 @@ async def create_data_model_from_openapi_schema( ## After everything has been created, process references for schema_name, schema in schemas.items(): await create_reference_associations_for_children( - session, schema, new_data_model.Id, openapi_schema, data_model_type + session, schema_name, schema, new_data_model.Id, openapi_schema, data_model_type ) await session.commit() diff --git a/test/components/lif/mdr_services/test_schema_upload_service.py b/test/components/lif/mdr_services/test_schema_upload_service.py new file mode 100644 index 00000000..ac8e129b --- /dev/null +++ b/test/components/lif/mdr_services/test_schema_upload_service.py @@ -0,0 +1,125 @@ +"""Tests for create-by-upload reference handling (Issue #756). + +MDR's schema generator inlines an entity reference as an object stored under a "Ref"-infix +property key ("Ref" / "Ref") rather than an OpenAPI "$ref". The +upload reader used to look only for "$ref", so references in an MDR-exported schema were +silently dropped (and materialized as bogus child entities). These tests cover the key parsing, +the reference/child discrimination, and the reference-association post-pass. +""" + +import types +import pytest +from unittest.mock import AsyncMock, MagicMock + +# asyncio_mode = auto (see pyproject) runs the async tests; sync helper tests stay sync. + +svc = pytest.importorskip("lif.mdr_services.schema_upload_service") + + +# --- pure helpers --------------------------------------------------------------------------- + + +def test_parse_reference_key_plain(): + assert svc.parse_reference_key("RefOrganization") == (None, "Organization") + + +def test_parse_reference_key_with_relationship(): + assert svc.parse_reference_key("issuedByRefOrganization") == ("issuedBy", "Organization") + + +@pytest.mark.parametrize( + "prop_name,prop,expected", + [ + ("RefOrganization", {"type": "object", "properties": {}}, True), + ("issuedByRefOrganization", {"type": "object", "properties": {}}, True), + # embedded child whose own name merely contains "Ref" must NOT be read as a reference + ("Reference", {"type": "object", "properties": {}}, False), + # a real $ref property is handled by its own branch, not as an inlined reference + ("RefOrganization", {"$ref": "#/components/schemas/Organization"}, False), + # attributes carry ValueSetId + ("someAttr", {"ValueSetId": 1}, False), + # no marker at all + ("hasOrganization", {"type": "object", "properties": {}}, False), + ], +) +def test_is_inlined_reference(prop_name, prop, expected): + assert svc.is_inlined_reference(prop_name, prop) is expected + + +# --- reference post-pass -------------------------------------------------------------------- + + +@pytest.fixture +def patched_lookups(monkeypatch): + """Resolve entities by UniqueName and report no pre-existing association.""" + entities = { + "Person": types.SimpleNamespace(Id=1, Name="Person"), + "Organization": types.SimpleNamespace(Id=2, Name="Organization"), + } + + async def fake_get_unique_entity(session, unique_name, data_model_id, base_data_model_id, data_model_type): + return entities.get(unique_name) + + monkeypatch.setattr(svc, "get_unique_entity", AsyncMock(side_effect=fake_get_unique_entity)) + monkeypatch.setattr(svc, "get_entity_association_by_parent_child_relationship", AsyncMock(return_value=None)) + return entities + + +def _added_associations(session): + return [c.args[0] for c in session.add.call_args_list if isinstance(c.args[0], svc.EntityAssociation)] + + +async def test_inlined_reference_creates_reference_association(patched_lookups): + session = MagicMock() + session.add = MagicMock() + entity_md = { + "UniqueName": "Person", + "DataModelId": 7, + "properties": { + "issuedByRefOrganization": { + "type": "object", + "UniqueName": "Organization", + "DataModelId": 7, + "properties": {"identifier": {"type": "string"}}, + } + }, + } + + await svc.create_reference_associations_for_children(session, "Person", entity_md, 7, {}, "SourceSchema") + + associations = _added_associations(session) + assert len(associations) == 1 + assoc = associations[0] + assert assoc.ParentEntityId == 1 + assert assoc.ChildEntityId == 2 + assert assoc.Relationship == "issuedBy" + assert assoc.Placement == "Reference" + + +async def test_plain_reference_has_no_relationship(patched_lookups): + session = MagicMock() + session.add = MagicMock() + entity_md = { + "UniqueName": "Person", + "properties": {"RefOrganization": {"type": "object", "UniqueName": "Organization", "properties": {}}}, + } + + await svc.create_reference_associations_for_children(session, "Person", entity_md, 7, {}, "SourceSchema") + + associations = _added_associations(session) + assert len(associations) == 1 + assert associations[0].Relationship is None + + +async def test_embedded_child_does_not_create_reference_association(patched_lookups): + """A non-reference child (no "Ref" marker) is recursed into, not turned into a reference.""" + session = MagicMock() + session.add = MagicMock() + entity_md = { + "UniqueName": "Person", + "properties": {"hasOrganization": {"type": "object", "UniqueName": "Organization", "properties": {}}}, + } + + await svc.create_reference_associations_for_children(session, "Person", entity_md, 7, {}, "SourceSchema") + + assert _added_associations(session) == [] From b14043f895116e8aeeb4edc8d1e90a40019b47dd Mon Sep 17 00:00:00 2001 From: "Benito J. Gonzalez" Date: Mon, 22 Jun 2026 23:28:39 -0700 Subject: [PATCH 2/5] Issue #756: add end-to-end generate->upload reference round-trip test Drives the real pipeline against a live Postgres DB: seed a source model with a Person --issuedBy(Reference)--> Organization association, run generate_openapi_schema, then round-trip the result back through create_data_model_from_openapi_schema. Asserts the inlined "issuedByRefOrganization" reference is recreated as a Reference-placement association (parent->child, relationship preserved) and is not materialized as a child entity. Fails against the pre-fix reader (the reference degrades to an Embedded association with no relationship). Co-Authored-By: Claude Opus 4.8 --- .../lif/mdr_restapi/test_schema_roundtrip.py | 113 ++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 test/bases/lif/mdr_restapi/test_schema_roundtrip.py diff --git a/test/bases/lif/mdr_restapi/test_schema_roundtrip.py b/test/bases/lif/mdr_restapi/test_schema_roundtrip.py new file mode 100644 index 00000000..f9a73c45 --- /dev/null +++ b/test/bases/lif/mdr_restapi/test_schema_roundtrip.py @@ -0,0 +1,113 @@ +"""End-to-end round-trip test for entity references (Issue #756). + +Drives both halves of the real pipeline against a live Postgres database: + + seed a source model with a Reference association + -> generate_openapi_schema (the export the UI/API hands back) + -> create_data_model_from_openapi_schema (the create-by-upload import) + +and asserts the reference survives the trip — i.e. the generator's inlined +"Ref" property is understood by the upload reader, recreated +as a Reference-placement EntityAssociation, and NOT materialized as a bogus child +entity. Before #756 the reference was silently dropped on upload. +""" + +from sqlmodel import select + +from lif.datatypes.mdr_sql_model import DataModel, DataModelType, Entity, EntityAssociation, EntityPlacementType +from lif.mdr_services.schema_generation_service import generate_openapi_schema +from lif.mdr_services.schema_upload_service import create_data_model_from_openapi_schema + + +async def _seed_source_model(session): + """A minimal source model: Person --issuedBy(Reference)--> Organization.""" + dm = DataModel( + Name="RoundTripSource", + Type=DataModelType.SourceSchema, + DataModelVersion="1.0", + ContributorOrganization="UniconQA", + Deleted=False, + ) + session.add(dm) + await session.commit() + await session.refresh(dm) + + person = Entity(Name="Person", UniqueName="Person", DataModelId=dm.Id, Array="No", Required="No", Deleted=False) + org = Entity( + Name="Organization", UniqueName="Organization", DataModelId=dm.Id, Array="No", Required="No", Deleted=False + ) + session.add(person) + session.add(org) + await session.commit() + await session.refresh(person) + await session.refresh(org) + + assoc = EntityAssociation( + ParentEntityId=person.Id, + ChildEntityId=org.Id, + Relationship="issuedBy", + Placement=EntityPlacementType.Reference, + Deleted=False, + ) + session.add(assoc) + await session.commit() + return dm + + +async def test_reference_survives_generate_then_upload(test_db_session): + session = test_db_session + source_dm = await _seed_source_model(session) + + # --- export: the schema the generator produces for the source model --- + schema = await generate_openapi_schema(session, source_dm.Id, include_attr_md=True, include_entity_md=True) + person_props = schema["components"]["schemas"]["Person"]["properties"] + # The reference is inlined under a "Ref"-infix key, not emitted as a "$ref". + assert "issuedByRefOrganization" in person_props + assert "$ref" not in person_props["issuedByRefOrganization"] + + # --- import: round-trip that schema back in through create-by-upload --- + target = await create_data_model_from_openapi_schema( + session=session, + openapi_schema=schema, + data_model_name="RoundTripTarget", + data_model_version="1.0", + data_model_type="SourceSchema", + data_model_description=None, + base_data_model_id=None, + use_considerations=None, + notes=None, + activation_date=None, + deprecation_date=None, + contributor=None, + contributor_organization="UniconQA", + ) + + # The reference must NOT have been materialized as a child entity. + entities = ( + (await session.execute(select(Entity).where(Entity.DataModelId == target.Id, Entity.Deleted == False))) + .scalars() + .all() + ) + names = sorted(e.Name for e in entities) + assert names == ["Organization", "Person"], names + assert not any("Ref" in n for n in names) + + # The reference association must have been recreated, parent->child, with its relationship. + by_name = {e.Name: e.Id for e in entities} + assocs = ( + ( + await session.execute( + select(EntityAssociation) + .join(Entity, Entity.Id == EntityAssociation.ParentEntityId) + .where(Entity.DataModelId == target.Id, EntityAssociation.Deleted == False) + ) + ) + .scalars() + .all() + ) + references = [a for a in assocs if a.Placement == EntityPlacementType.Reference] + assert len(references) == 1, [(a.Placement, a.Relationship) for a in assocs] + ref = references[0] + assert ref.ParentEntityId == by_name["Person"] + assert ref.ChildEntityId == by_name["Organization"] + assert ref.Relationship == "issuedBy" From eac040792a688516ddbd485a16cc6a6ea3c6e2d9 Mon Sep 17 00:00:00 2001 From: "Benito J. Gonzalez" Date: Mon, 29 Jun 2026 11:33:47 -0700 Subject: [PATCH 3/5] Issue #756: simplify $ref relationship-prefix derivation Address review nit: 'prop_name[:idx] or None if idx > 0 else None' relied on operator precedence and the 'or None' was dead code when idx > 0. Replace with the equivalent, clearer 'prop_name[:idx] if idx > 0 else None' plus a comment. No behavior change. Co-Authored-By: Claude Opus 4.8 --- components/lif/mdr_services/schema_upload_service.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/components/lif/mdr_services/schema_upload_service.py b/components/lif/mdr_services/schema_upload_service.py index fcf1a2d1..69d8477e 100644 --- a/components/lif/mdr_services/schema_upload_service.py +++ b/components/lif/mdr_services/schema_upload_service.py @@ -146,8 +146,10 @@ async def create_reference_associations_for_children( if child_name is None: # $ref path: derive the relationship from the property-name prefix, e.g. # "issuedByOrganization" -> "issuedBy" for referenced entity "Organization". + # idx > 0 means there is a non-empty prefix before the entity name; idx == 0 + # (no prefix) or idx == -1 (name not in the key) both mean no relationship. idx = prop_name.find(referenced_entity.Name) - relationship = prop_name[:idx] or None if idx > 0 else None + relationship = prop_name[:idx] if idx > 0 else None logger.info(f"Reference {prop_name!r} -> entity {referenced_unique_name!r} (rel={relationship!r})") await create_reference_entity_association_if_needed( session, referenced_entity, parent_entity.Id, relationship, data_model_id From edef175c3a01b990ab3d2f0276441ae5addcf7c6 Mon Sep 17 00:00:00 2001 From: "Benito J. Gonzalez" Date: Fri, 17 Jul 2026 15:35:16 -0700 Subject: [PATCH 4/5] Issue #756: fix Ref-containing relationship parse + document has/relevant loss MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses cbeach47's review's two silent-drop findings: - Finding B (fixed here): parse_reference_key split on the FIRST "Ref" (partition), so a relationship name containing "Ref" (isReferencedBy, refersTo) mis-parsed — "isReferencedByRefOrganization" -> ("is", "erencedByRefOrganization"), whose lowercase child failed the PascalCase guard and the reference was silently dropped. Switched to rpartition (split on the LAST "Ref") so the child entity name stays intact. Added a test. - Finding A (documented; generator-side, tracked in #1062): the generator drops has*/relevant* relationship names on export ("Ref"+child), so they round-trip to relationship=None. The reader can't recover a name never emitted. Added a unit test asserting the current loss so it's documented, not hidden; the generator decision (encode vs. formally document) is #1062. Also merged upstream/main (stale branch; resolved the schema_upload_service.py conflict in favor of #1007's reference-aware reader over the old $ref-only code). Co-Authored-By: Claude Opus 4.8 --- .../lif/mdr_services/schema_upload_service.py | 15 ++++++++++++--- .../mdr_services/test_schema_upload_service.py | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/components/lif/mdr_services/schema_upload_service.py b/components/lif/mdr_services/schema_upload_service.py index 69d8477e..661898de 100644 --- a/components/lif/mdr_services/schema_upload_service.py +++ b/components/lif/mdr_services/schema_upload_service.py @@ -62,10 +62,19 @@ def parse_dt(val): def parse_reference_key(prop_name: str) -> tuple[Optional[str], str]: """Split an inlined-reference key into (relationship, child_entity_name). - "RefOrganization" -> (None, "Organization") - "issuedByRefOrganization" -> ("issuedBy", "Organization") + "RefOrganization" -> (None, "Organization") + "issuedByRefOrganization" -> ("issuedBy", "Organization") + "isReferencedByRefOrganization" -> ("isReferencedBy", "Organization") + + Split on the LAST "Ref" (``rpartition``), not the first: a relationship name can itself + contain "Ref" (e.g. ``isReferencedBy``, ``refersTo``). Splitting on the first occurrence + would mis-parse ``isReferencedByRefOrganization`` as ("is", "erencedByRefOrganization"), + whose lowercase child then fails the PascalCase guard in ``is_inlined_reference`` and the + reference gets silently dropped. Splitting on the last "Ref" keeps the child entity name + (the trailing PascalCase token) intact. (A child entity legitimately named ``Ref...`` is the + residual ambiguity the module NOTE's two-sided marker fix would resolve.) """ - relationship, _, child_name = prop_name.partition(REFERENCE_KEY_MARKER) + relationship, _, child_name = prop_name.rpartition(REFERENCE_KEY_MARKER) return (relationship or None), child_name diff --git a/test/components/lif/mdr_services/test_schema_upload_service.py b/test/components/lif/mdr_services/test_schema_upload_service.py index ac8e129b..b1a00e67 100644 --- a/test/components/lif/mdr_services/test_schema_upload_service.py +++ b/test/components/lif/mdr_services/test_schema_upload_service.py @@ -27,6 +27,24 @@ def test_parse_reference_key_with_relationship(): assert svc.parse_reference_key("issuedByRefOrganization") == ("issuedBy", "Organization") +def test_parse_reference_key_relationship_containing_ref(): + # A relationship name can itself contain "Ref" (isReferencedBy, refersTo). rpartition splits + # on the LAST "Ref", so the child entity name stays intact; splitting on the first would yield + # ("is", "erencedByRefOrganization") whose lowercase child fails the PascalCase guard and the + # reference would be silently dropped (cbeach47 #1007 review). + assert svc.parse_reference_key("isReferencedByRefOrganization") == ("isReferencedBy", "Organization") + assert svc.is_inlined_reference("isReferencedByRefOrganization", {"type": "object"}) is True + + +def test_parse_reference_key_has_relevant_relationship_name_is_lost(): + # KNOWN, generator-side loss (tracked in #1062): the generator encodes has*/relevant* + # relationships WITHOUT the name (schema_generation_service.py:802 -> "Ref" + child), so + # e.g. hasManager exports as "RefEmployee". The reader can't recover a name that was never + # emitted, so it round-trips to relationship=None. Asserted here so the loss is documented, + # not hidden (the round-trip test only uses issuedBy, which survives). + assert svc.parse_reference_key("RefEmployee") == (None, "Employee") + + @pytest.mark.parametrize( "prop_name,prop,expected", [ From c476fb946134a92d1b1b41f2fec80210cfdc77ff Mon Sep 17 00:00:00 2001 From: "Benito J. Gonzalez" Date: Fri, 17 Jul 2026 16:10:08 -0700 Subject: [PATCH 5/5] Issue #756: parent_entity_id is not None (id-0 defensive fix) While in this file for the #756 review: the EntityAttributeAssociation create was guarded by `if parent_entity_id:`, which would skip a valid parent PK of 0 (same truthiness-vs-None class as the #1006 ElementId fix). Use `is not None`. Co-Authored-By: Claude Opus 4.8 --- components/lif/mdr_services/schema_upload_service.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/components/lif/mdr_services/schema_upload_service.py b/components/lif/mdr_services/schema_upload_service.py index 661898de..2678ffeb 100644 --- a/components/lif/mdr_services/schema_upload_service.py +++ b/components/lif/mdr_services/schema_upload_service.py @@ -345,8 +345,9 @@ async def create_attribute_if_needed( ) session.add(inclusion) - # If needed, create EntityAttributeAssociation - if parent_entity_id: + # If needed, create EntityAttributeAssociation. `is not None`, not truthiness: a parent + # entity id of 0 is a valid PK, not "no parent" (same class as the #1006 ElementId fix). + if parent_entity_id is not None: # Check if an EntityAttributeAssociation already exists association = await check_existing_association(session, parent_entity_id, attribute.Id, data_model_id) if not association: # If the EntityAttributeAssociation does not exist, create it