Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support merging with disassociated source namespace #130

Draft
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

NebelNidas
Copy link
Member

@NebelNidas NebelNidas commented Mar 14, 2025

Currently, if a tree has

nsA nsB
cls1NsAName cls1NsBName

and you want to merge

nsB nsC
cls1NsBName cls1NsCName

cls1NsCName never finds its way into the tree, as the visit pass doesn't supply a nsA name. Even worse, if you visit with nsC on the source and nsB on the destination side, an exception is thrown, since the incoming source namespace (nsC) doesn't exist in the tree yet.

Both of these cases can be solved by iterating all incoming namespaces that actually are present in the tree (nsB in this case), searching for entries with the same name (here: cls1NsBName) and if found, merging the new data into that existing entry, leading to the following result:

nsA nsB nsC
cls1NsAName cls1NsBName cls1NsCName

That's exactly what this PR does, however there's an edge case I wasn't sure how to handle: What if we find a match in more than one namespace, but each one belonging to a different entry?
Tree:

nsA nsB nsC
cls1NsAName someNsBName
cls2NsAName someNsCName

To be merged:

nsB nsC nsD
someNsBName someNsCName someNsDName

This would match both cls1 and cls2. As of now, this PR returns after merging with the first match, but we could of course merge with the remaining matches too or throw an exception instead. What are your opinions on this?

Also introduces a fallback for cases where pending elements haven't received a tree-side source name, but an overlap with other tree-side namespaces exists.
Mainly for use in tests to ensure our test data doesn't violate any API contracts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant