Issue #756: honor inlined entity references on schema create-by-upload#1007
Open
bjagg wants to merge 3 commits into
Open
Issue #756: honor inlined entity references on schema create-by-upload#1007bjagg wants to merge 3 commits into
bjagg wants to merge 3 commits into
Conversation
…reate-by-upload
MDR's schema generator inlines entity references as objects under a "Ref"-infix
property key ("Ref<Child>" / "<relationship>Ref<Child>", 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<Something>". Add unit
tests for the helpers and the reference post-pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ound-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 <noreply@anthropic.com>
Contributor
Author
|
Added the end-to-end round-trip test flagged in the description ( Verified it fails against pre-fix |
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 <noreply@anthropic.com>
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of Change
Problem. MDR's schema generator (
schema_generation_service.add_ref) encodes a reference to another entity as an inlined object under a property key carrying aRefinfix —Ref<Child>for a plain reference,<relationship>Ref<Child>when the association has a relationship — rather than emitting an OpenAPI$ref. The create-by-upload reader only looked for$ref, so re-uploading an MDR-exported schema silently dropped every reference, and the entity-creation pass then materialized each reference as a bogus child entity.Solution.
parse_reference_key/is_inlined_referencehelpers to detect the inlined-reference convention alongside genuine$ref.is_inlined_referencerequires a PascalCase entity name after the marker so an embedded child merely namedReferenceis not misread.create_reference_associations_for_childrento handle both$refand inlined references: resolve the referenced and parent entities (UniqueName from embedded metadata, falling back to the name parsed from the key — entities are created in the first pass with UniqueName defaulting to their schema-key name), and create thePlacement="Reference"EntityAssociationwith the correct relationship. It now receives the entity's own key name for robust parent resolution and treats a reference as a leaf (no spurious recursion into its narrowed fields).create_entity_and_children_if_neededso they are no longer created as embedded child entities.Side effects / limitations. Detection keys off the
Refinfix per the issue report. That is inherently ambiguous for an entity legitimately namedRef<Something>; an in-codeNOTEdocuments the preferred two-sided fix — have the generator stamp an explicit marker (e.g.x-lif-reference-to: <UniqueName>) and key off that instead of string-parsing the property name. Resolution assumes the uploaded schema was generated with entity metadata (the realistic export case); the key-name fallback covers the no-metadata case.How reviewers should test.
uv run pytest test/components/lif/mdr_services/test_schema_upload_service.py— covers key parsing, reference/child discrimination, relationship extraction, plain-reference (no relationship), and the embedded-child non-match. The post-pass test asserts aReference-placement association is created with the resolved parent/child ids. End-to-end validation against a live MDR-generated schema is still worth doing before relying on this in production (see Additional Notes).Related Issues
Closes #756
Type of Change
Project Area(s) Affected
Checklist
uv run ruff check)uv run ruff format)uv run ty check) — no new diagnostics introducedTesting
Additional Notes
mdr_servicessuite passes (102 tests);tydiagnostics unchanged vsmain(7→7, pre-existing SQLAlchemy/pydantic library typing noise).schema_upload_service.py, so whichever merges first leaves the other a trivial conflict — Issue #668: fix MDR import_datamodel reference and constraint handling #1006 only adds a forward-looking doc comment there, which this PR replaces with the actual implementation.schema_generation_serviceand round-trips it through the upload path.🤖 Generated with Claude Code