XRENDERING-802: Prepared macro content leads to duplicated IDs when inserted#374
Open
XRENDERING-802: Prepared macro content leads to duplicated IDs when inserted#374
Conversation
…nserted * Add IdGenerator#adaptId(String) to adapt an existing id (make it unique in the scope of this id generator) * Add XDOM#setIdGenerator(IdGenerator, boolean) to allow adapting the existing ids when changing the id generator of an XDOM * Adapt the existing ids from the prepared macro content XDOM clone before inserting it into the target document * Add unit tests
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.
Jira URL
https://jira.xwiki.org/browse/XRENDERING-802
Changes
Description
IdGenerator#adaptId(String)to adapt an existing ID (make it unique in the scope of this ID generator)XDOM#setIdGenerator(IdGenerator, boolean)to allow adapting the existing IDs when changing the ID generator of an XDOMClarifications
IdGenerator#adaptId(String)I kept the method name that @michitux introduced (I basically moved the private method fromDocumentContentAsyncExecutor). I find the name to be OK, but I'm open to change it if you have better suggestionsXDOM#setIdGenerator(IdGenerator, boolean), I initially thought we could adapt existing IDs every time a different ID generator is set, but this breaks the XDOM cloning use case where we set on the clone a copy of the ID generator from the source XDOM, but we don't want to adapt the IDs, otherwise the clone would not equal the source.XDOM#makeIdsUnique()because I don't see other use case for it than when you change the ID generatorIdGenerator#equals()and use it to avoid adapting the existing IDs if the ID generators are equal; my worry with this is that we might have cases where the ID generators have the same generated IDs but thy were used on different content, so we might get duplicated IDs. I'm not sure if it's worth taking this risk just to have continuous IDs.Expected merging strategy