You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ossie.apache.org by "tripleaceme (via GitHub)" <gi...@apache.org> on 2026/10/14 21:36:14 UTC
[PR] fix(wisdom): store relationship types in a WISDOM custom extension [ossie]
tripleaceme opened a new pull request, #585:
URL: https://github.com/apache/ossie/pull/585
## 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 restore `relationshipType`. As #507 describes, that made the note text a wire format, misread or ignored hand-written notes, and put a `one-to-many relationship` note on a relationship whose `from` is the many side.
This moves the type into a `WISDOM` entry in `custom_extensions`, following the plan agreed on the issue.
**Wisdom → Ossie**
- Every relationship that isn't `MANY_TO_ONE` gets a `WISDOM` entry with its original `relationshipType` and the `from`/`to` datasets it was written with:
```yaml
custom_extensions:
- vendor_name: WISDOM
data: '{"_v": 1, "normalizedEndpoints": ["orders", "customers"], "relationshipType": "ONE_TO_MANY"}'
```
The key names follow the Microsoft converter (`normalizedEndpoints`, `_v`) and Wisdom's own `relationshipType`.
- The `one-to-many relationship` note is no longer written. The `one-to-one` and `many-to-many` notes stay as plain `ai_context` for readers, but the export no longer depends on them.
**Ossie → Wisdom**
- The type, and the left/right and column swap for `ONE_TO_MANY`, are restored from the `WISDOM` entry.
- If the entry is malformed, has an unknown `_v`, or its endpoints no longer match `from`/`to` (the relationship was edited after import), the edge is exported as `MANY_TO_ONE` with a new `RELATIONSHIP_TYPE_UNRESOLVED` warning. None of these cases raise.
- Entries the converter reads are no longer reported as `CUSTOM_EXTENSION_DROPPED`. Other vendors' entries still are.
- Without a `WISDOM` entry, the exact (whitespace-stripped) notes from `RELATIONSHIP_TYPE_NOTES` are still read, including the old `one-to-many` note, so documents generated before this change still round-trip. A note that states the exported type is no longer reported as `AI_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 changes `custom_extensions[].data` to an object, only those two functions change.
The README's cardinality table and export section are updated.
**For reviewers to decide**
1. A stale or malformed entry is reported once, as `RELATIONSHIP_TYPE_UNRESOLVED`, not also as `CUSTOM_EXTENSION_DROPPED`.
2. When an entry and a conflicting note are both present, the entry wins and the note is reported as `AI_CONTEXT_DROPPED`.
3. `normalizedEndpoints` stores 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_ONE` and `MANY_TO_MANY` get the entry with the exact stored payload; `MANY_TO_ONE` gets none
- `ONE_TO_MANY` has no note; `ONE_TO_ONE` and `MANY_TO_MANY` keep theirs
- a non-string `relationshipType` still converts as before
Ossie → Wisdom:
- the entry restores each type, and the `ONE_TO_MANY` swap with distinct column names on each side
- stale endpoints, malformed entries (bad JSON, non-object, unknown type, bad endpoints, missing endpoints, `_v` of 2 or `"1"`) export `MANY_TO_ONE` with the warning
- a 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.json` passes unchanged
- `cd converters/wisdom && uv run pytest`: 59 passed (37 on `main`)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(wisdom): store relationship types in a WISDOM custom extension [ossie]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on PR #585:
URL: https://github.com/apache/ossie/pull/585#issuecomment-6314232717
@tripleaceme as you probably know (and it has been shared on the dev mailing list), the converters have moved to a dedicated repository: https://github.com/apache/ossie-converters
I propose you move this PR (create) on the new repo and we will resume the review together there.
Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(wisdom): store relationship types in a WISDOM custom extension [ossie]
Posted by "tripleaceme (via GitHub)" <gi...@apache.org>.
tripleaceme commented on PR #585:
URL: https://github.com/apache/ossie/pull/585#issuecomment-6317343787
Thanks @jbonofre. Moved to apache/ossie-converters#9. It has the same two commits with the `converters/` prefix dropped, and the wisdom tests pass there (63) against the pinned `ossie` submodule.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(wisdom): store relationship types in a WISDOM custom extension [ossie]
Posted by "tripleaceme (via GitHub)" <gi...@apache.org>.
tripleaceme commented on code in PR #585:
URL: https://github.com/apache/ossie/pull/585#discussion_r4307197967
##########
converters/wisdom/src/ossie_wisdom/ossie_to_wisdom.py:
##########
@@ -402,3 +427,24 @@ def _convert_relationships(
}
)
return edges
+
+ def _wisdom_extension(self, relationship: OssieRelationship) -> OssieCustomExtension | None:
+ for extension in relationship.custom_extensions or []:
+ if extension.vendor_name == OssieVendor.WISDOM.value:
+ return extension
+ return None
+
+ def _stored_relationship_type(
+ self, extension: OssieCustomExtension, relationship: OssieRelationship
+ ) -> str | None:
+ """Returns the type stored by wisdom_to_ossie, or None when the entry is malformed or stale."""
+ data = decode_extension_data(extension.data)
+ if data is None:
+ return None
+ relationship_type = data.get("relationshipType")
+ if not isinstance(relationship_type, str) or relationship_type not in _RELATIONSHIP_TYPES:
+ return None
+ # 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
Review 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.
##########
converters/wisdom/src/ossie_wisdom/wisdom_to_ossie.py:
##########
@@ -53,14 +56,41 @@
_SIMPLE_IDENTIFIER = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$")
# Relationship `ai_context` notes for the Wisdom types Ossie direction can't express.
-# ossie_to_wisdom matches these exactly, so a free-text note is never read as a marker.
+# The type itself is kept in a WISDOM custom extension; the notes are for readers.
+# ONE_TO_MANY's note is no longer written (the edge is flipped, so it would contradict
+# the direction), but ossie_to_wisdom still reads all three, matched exactly, from
+# documents that carry no WISDOM extension.
RELATIONSHIP_TYPE_NOTES = {
"ONE_TO_MANY": "one-to-many relationship",
"ONE_TO_ONE": "one-to-one relationship",
"MANY_TO_MANY": "many-to-many relationship; cardinality is not representable in Ossie",
}
+# Format version of the WISDOM custom extension payload, stored as `_v`.
+EXTENSION_VERSION = 1
+
+
+def encode_extension_data(data: dict) -> str:
+ """Serializes a WISDOM custom extension payload; inverse of :func:`decode_extension_data`."""
+ return json.dumps({"_v": EXTENSION_VERSION, **data}, sort_keys=True)
+
+
+def decode_extension_data(data: str) -> dict | None:
+ """Parses a WISDOM custom extension payload, or returns None when it is not a JSON object
+ or was written in a format version this converter does not understand."""
+ try:
+ decoded = json.loads(data)
+ except (TypeError, ValueError):
Review 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(wisdom): store relationship types in a WISDOM custom extension [ossie]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #585:
URL: https://github.com/apache/ossie/pull/585#discussion_r4307013964
##########
converters/wisdom/src/ossie_wisdom/ossie_to_wisdom.py:
##########
@@ -402,3 +427,24 @@ def _convert_relationships(
}
)
return edges
+
+ def _wisdom_extension(self, relationship: OssieRelationship) -> OssieCustomExtension | None:
+ for extension in relationship.custom_extensions or []:
+ if extension.vendor_name == OssieVendor.WISDOM.value:
+ return extension
+ return None
+
+ def _stored_relationship_type(
+ self, extension: OssieCustomExtension, relationship: OssieRelationship
+ ) -> str | None:
+ """Returns the type stored by wisdom_to_ossie, or None when the entry is malformed or stale."""
+ data = decode_extension_data(extension.data)
+ if data is None:
+ return None
+ relationship_type = data.get("relationshipType")
+ if not isinstance(relationship_type, str) or relationship_type not in _RELATIONSHIP_TYPES:
+ return None
+ # 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
Review 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:
```suggestion
# 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`?
##########
converters/wisdom/src/ossie_wisdom/wisdom_to_ossie.py:
##########
@@ -53,14 +56,41 @@
_SIMPLE_IDENTIFIER = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$")
# Relationship `ai_context` notes for the Wisdom types Ossie direction can't express.
-# ossie_to_wisdom matches these exactly, so a free-text note is never read as a marker.
+# The type itself is kept in a WISDOM custom extension; the notes are for readers.
+# ONE_TO_MANY's note is no longer written (the edge is flipped, so it would contradict
+# the direction), but ossie_to_wisdom still reads all three, matched exactly, from
+# documents that carry no WISDOM extension.
RELATIONSHIP_TYPE_NOTES = {
"ONE_TO_MANY": "one-to-many relationship",
"ONE_TO_ONE": "one-to-one relationship",
"MANY_TO_MANY": "many-to-many relationship; cardinality is not representable in Ossie",
}
+# Format version of the WISDOM custom extension payload, stored as `_v`.
+EXTENSION_VERSION = 1
+
+
+def encode_extension_data(data: dict) -> str:
+ """Serializes a WISDOM custom extension payload; inverse of :func:`decode_extension_data`."""
+ return json.dumps({"_v": EXTENSION_VERSION, **data}, sort_keys=True)
+
+
+def decode_extension_data(data: str) -> dict | None:
+ """Parses a WISDOM custom extension payload, or returns None when it is not a JSON object
+ or was written in a format version this converter does not understand."""
+ try:
+ decoded = json.loads(data)
+ except (TypeError, ValueError):
Review 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:
```suggestion
except (TypeError, ValueError, RecursionError):
```
Could we add `"[" * 200_000` to the cases in `test_malformed_wisdom_extension_exports_many_to_one`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(wisdom): store relationship types in a WISDOM custom extension [ossie]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre closed pull request #585: fix(wisdom): store relationship types in a WISDOM custom extension
URL: https://github.com/apache/ossie/pull/585
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ossie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org