Repository navigation
fix(wisdom): store relationship types in a WISDOM custom extension - #585
tripleaceme wants to merge 2 commits into
Conversation
wisdom-to-ossie now records a non-MANY_TO_ONE relationshipType, with the from/to datasets it was written with, in a WISDOM custom extension on the relationship. ossie-to-wisdom restores the type, and the ONE_TO_MANY side swap, from that entry instead of matching ai_context text. If the entry is malformed, has an unknown format version (_v), or its datasets no longer match from/to, the edge is exported as MANY_TO_ONE with a new RELATIONSHIP_TYPE_UNRESOLVED warning. Entries read this way are no longer reported as CUSTOM_EXTENSION_DROPPED. The one-to-many note is no longer written, since it contradicts the flipped direction. The one-to-one and many-to-many notes stay as readable ai_context. Without a WISDOM entry, the exact notes are still read, so documents generated before this change keep round-tripping. Closes apache#507
| # Edited endpoints may no longer match the stored type (e.g. a ONE_TO_MANY swap), so it is not applied. | ||
| if data.get("normalizedEndpoints") != [relationship.from_dataset, relationship.to]: | ||
| return None |
There was a problem hiding this comment.
Because this compares the endpoints in order, a reversed relationships count as stale. For ONE_TO_ONE and MANY_TO_MANY that's a regression. The README says ONE_TO_ONE direction is arbitrary, but it you swap from/to on an imported one, it now exports as MANY_TO_ONE with RELATIONSHIP_TYPE_UNRESOLVED, plus AI_CONTEXT_DROPPED for the note. On main, the note alone kept ONE_TO_ONE after the same edit.
A reversed ONE_TO_MANY isn't stale either. Applying it and swapping the sides back means the same as MANY_TO_ONE in the new direction, so the warning reports a loss that didn't happen.
Accepting both orders fixes all three cases, and an endpoint moved to a different dataset is still caught:
| # Edited endpoints may no longer match the stored type (e.g. a ONE_TO_MANY swap), so it is not applied. | |
| if data.get("normalizedEndpoints") != [relationship.from_dataset, relationship.to]: | |
| return None | |
| # Reversing from/to keeps the meaning: ONE_TO_ONE and MANY_TO_MANY are symmetric and | |
| # ONE_TO_MANY is swapped back below. Only endpoints moved to other datasets are stale. | |
| endpoints = [relationship.from_dataset, relationship.to] | |
| if data.get("normalizedEndpoints") not in (endpoints, endpoints[::-1]): | |
| return None |
test_wisdom_extension_with_stale_endpoints_exports_many_to_one then needs endpoints that point to a different dataset (e.g. ["orders", "users"], as in test_note_is_not_read_when_wisdom_extension_is_stale).
Could we also add a test for a reversed ONE_TO_ONE and MANY_TO_MANY?
There was a problem hiding this comment.
You're right, and it was a regression against main for the symmetric types. Applied your suggestion in 1c9bb5c, so the recorded endpoints are accepted in either order. The stale test now uses ["orders", "users"]. I added tests for a reversed ONE_TO_ONE and MANY_TO_MANY, each with its note, that keep the type with no issues, and one for a reversed ONE_TO_MANY that exports customers ONE_TO_MANY orders with no warning. I changed "swapped back below" in the comment to "swapped back on export", since the swap happens in _convert_relationships, not in this helper. The README and PR description are updated to match.
| or was written in a format version this converter does not understand.""" | ||
| try: | ||
| decoded = json.loads(data) | ||
| except (TypeError, ValueError): |
There was a problem hiding this comment.
json.loads raises RecursionError on deeply nested input, and that isn't a ValueError. So data = "[" * 200_000 crashes the export with a traceback, instead of giving the RELATIONSHIP_TYPE_UNRESOLVED warning that other malformed input gets:
| except (TypeError, ValueError): | |
| except (TypeError, ValueError, RecursionError): |
Could we add "[" * 200_000 to the cases in test_malformed_wisdom_extension_exports_many_to_one?
There was a problem hiding this comment.
Applied in 1c9bb5c, and "[" * 200_000 is now one of the malformed cases. I gave it the id deeply-nested so pytest doesn't print the 200,000-character string as the test name.
Reversing a relationship's from/to after import made its WISDOM entry look stale, so ONE_TO_ONE and MANY_TO_MANY exported as MANY_TO_ONE with RELATIONSHIP_TYPE_UNRESOLVED, where main kept them through the note. A reversed ONE_TO_MANY still describes the relationship once its sides are swapped back. Accept the recorded endpoints in either order; an endpoint moved to a different dataset is still treated as stale. json.loads raises RecursionError on deeply nested input, which crashed the export. Treat it as malformed data like other bad JSON.
Summary
The Wisdom converter kept relationship types that Ossie's direction can't carry as text in the relationship's
ai_context, and read them back to restorerelationshipType. As #507 describes, that made the note text a wire format, misread or ignored hand-written notes, and put aone-to-many relationshipnote on a relationship whosefromis the many side.This moves the type into a
WISDOMentry incustom_extensions, following the plan agreed on the issue.Wisdom → Ossie
MANY_TO_ONEgets aWISDOMentry with its originalrelationshipTypeand thefrom/todatasets it was written with:normalizedEndpoints,_v) and Wisdom's ownrelationshipType.one-to-many relationshipnote is no longer written. Theone-to-oneandmany-to-manynotes stay as plainai_contextfor readers, but the export no longer depends on them.Ossie → Wisdom
ONE_TO_MANY, are restored from theWISDOMentry._v, or one of its recorded endpoints is no longer the relationship'sfromorto(it was moved to another dataset after import), the edge is exported asMANY_TO_ONEwith a newRELATIONSHIP_TYPE_UNRESOLVEDwarning. None of these cases raise. Reversingfrom/tois not stale:ONE_TO_ONEandMANY_TO_MANYare symmetric, and a reversedONE_TO_MANYstill describes the relationship once its sides are swapped back.CUSTOM_EXTENSION_DROPPED. Other vendors' entries still are.WISDOMentry, the exact (whitespace-stripped) notes fromRELATIONSHIP_TYPE_NOTESare still read, including the oldone-to-manynote, so documents generated before this change still round-trip. A note that states the exported type is no longer reported asAI_CONTEXT_DROPPED; any other note still is.The payload is encoded and decoded in one pair of helpers (
encode_extension_data/decode_extension_data), so if #522 changescustom_extensions[].datato an object, only those two functions change.The README's cardinality table and export section are updated.
For reviewers to decide
RELATIONSHIP_TYPE_UNRESOLVED, not also asCUSTOM_EXTENSION_DROPPED.AI_CONTEXT_DROPPED.normalizedEndpointsstores dataset names only. An edit that changes only the join columns isn't treated as stale; it doesn't affect which side is swapped.If apache/ossie-converters#1 lands before this, I'll re-open it there.
Related Issues
Closes #507
Tests
Wisdom → Ossie:
ONE_TO_MANY,ONE_TO_ONEandMANY_TO_MANYget the entry with the exact stored payload;MANY_TO_ONEgets noneONE_TO_MANYhas no note;ONE_TO_ONEandMANY_TO_MANYkeep theirsrelationshipTypestill converts as beforeOssie → Wisdom:
the entry restores each type, and the
ONE_TO_MANYswap with distinct column names on each sidereversed
ONE_TO_ONE,MANY_TO_MANYandONE_TO_MANYkeep their type with no warningan endpoint moved to another dataset, and malformed entries (bad JSON, deeply nested JSON, non-object, unknown type, bad endpoints, missing endpoints,
_vof 2 or"1") exportMANY_TO_ONEwith the warninga read entry isn't reported as dropped, while an unrelated vendor's entry still is
legacy notes with no entry restore all three types
the entry wins over a conflicting note, and a stale entry doesn't fall back to the note
the existing full round trip on
sample_export.jsonpasses unchangedcd converters/wisdom && uv run pytest: 63 passed (37 onmain)