From 19496f89236e4cfc46adb13bb818462ecfd5b502 Mon Sep 17 00:00:00 2001 From: Marius Dumitru Florea Date: Fri, 3 Apr 2026 11:41:21 +0300 Subject: [PATCH] XRENDERING-802: Prepared macro content leads to duplicated IDs when inserted * 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 --- xwiki-rendering-api/pom.xml | 2 +- .../java/org/xwiki/rendering/block/XDOM.java | 48 ++++++ .../org/xwiki/rendering/util/IdGenerator.java | 21 +++ .../org/xwiki/rendering/block/XDOMTest.java | 145 ++++++++++++++++++ .../xwiki/rendering/util/IdGeneratorTest.java | 42 +++-- .../macro/DefaultMacroContentParser.java | 46 ++++-- .../macro/DefaultMacroContentParserTest.java | 25 ++- 7 files changed, 293 insertions(+), 36 deletions(-) create mode 100644 xwiki-rendering-api/src/test/java/org/xwiki/rendering/block/XDOMTest.java diff --git a/xwiki-rendering-api/pom.xml b/xwiki-rendering-api/pom.xml index ecb4d69f0b..7b4d00fc0c 100644 --- a/xwiki-rendering-api/pom.xml +++ b/xwiki-rendering-api/pom.xml @@ -32,7 +32,7 @@ jar XWiki Rendering - Api - 0.62 + 0.64 true diff --git a/xwiki-rendering-api/src/main/java/org/xwiki/rendering/block/XDOM.java b/xwiki-rendering-api/src/main/java/org/xwiki/rendering/block/XDOM.java index fc276802d7..e01b36fe62 100644 --- a/xwiki-rendering-api/src/main/java/org/xwiki/rendering/block/XDOM.java +++ b/xwiki-rendering-api/src/main/java/org/xwiki/rendering/block/XDOM.java @@ -25,6 +25,7 @@ import org.xwiki.rendering.listener.Listener; import org.xwiki.rendering.listener.MetaData; import org.xwiki.rendering.util.IdGenerator; +import org.xwiki.stability.Unstable; /** * Contains the full tree of {@link Block} that represent a XWiki Document's content. @@ -101,7 +102,54 @@ public IdGenerator getIdGenerator() */ public void setIdGenerator(IdGenerator idGenerator) { + setIdGenerator(idGenerator, false); + } + + /** + * Sets a new id generator for this document and optionally adapts the existing ids to make them unique in the scope + * of the new id generator. Adapting the existing ids is needed if you plan to insert this document in a larger one, + * in which case you will have to reuse the id generator of the larger document for this document. On the other + * hand, if this document is a clone of another document, and you plan to use it alone then you don't need to adapt + * the existing ids. In this case, even if the id generator is different, it was created as a copy of the original + * id generator, so the existing ids are already unique. + * + * @param idGenerator a stateful id generator for the whole document + * @param adaptExistingIds whether to adapt the existing ids to make them unique in the scope of the new id + * generator; pass true if the new id generator is from a another document where you plan to insert this + * document; pass false if this document is a clone and the new id generator is a copy of the original id + * generator + * @since 17.10.6 + * @since 18.3.0RC1 + */ + @Unstable + public void setIdGenerator(IdGenerator idGenerator, boolean adaptExistingIds) + { + boolean changed = this.idGenerator != idGenerator; this.idGenerator = idGenerator; + if (this.idGenerator != null && changed && adaptExistingIds) { + // Make sure the existing ids are unique in the scope of the new id generator. + makeIdsUnique(); + } + } + + /** + * Make sure heading and image blocks have unique ids in the scope of the provided id generator. We target only + * heading and image blocks because these are currently the only blocks that can have generated ids. The ids of + * macro blocks are not generated. + */ + private void makeIdsUnique() + { + // Traverse the XDOM and adapt all image and heading blocks. + this.getBlocks(block -> { + // Would be nice to have an interface that marks blocks with generated ids, but for now we just check the + // known block types. + if (block instanceof ImageBlock imageBlock) { + imageBlock.setId(this.idGenerator.adaptId(imageBlock.getId())); + } else if (block instanceof HeaderBlock headerBlock) { + headerBlock.setId(this.idGenerator.adaptId(headerBlock.getId())); + } + return false; + }, Block.Axes.DESCENDANT); } @Override diff --git a/xwiki-rendering-api/src/main/java/org/xwiki/rendering/util/IdGenerator.java b/xwiki-rendering-api/src/main/java/org/xwiki/rendering/util/IdGenerator.java index e646bbe212..d29125c73e 100644 --- a/xwiki-rendering-api/src/main/java/org/xwiki/rendering/util/IdGenerator.java +++ b/xwiki-rendering-api/src/main/java/org/xwiki/rendering/util/IdGenerator.java @@ -24,6 +24,7 @@ import java.util.concurrent.ConcurrentHashMap; import org.apache.commons.lang3.StringUtils; +import org.xwiki.stability.Unstable; /** * Stateful generator of id attributes. It's stateful since it remembers the generated ids. Thus a new instance of it @@ -139,6 +140,26 @@ public String generateUniqueId(String prefix, String text) return id; } + /** + * Adapts the given id to make it unique in the scope of this id generator. Use this method to make existing ids + * unique after setting a new id generator. + * + * @param id the id to adapt to make it unique + * @return a unique id, in the scope of this id generator; it returns the same id if it's already unique + * @since 17.10.6 + * @since 18.3.0RC1 + */ + @Unstable + public String adaptId(String id) + { + if (StringUtils.isNotBlank(id)) { + String prefix = id.substring(0, 1); + String suffix = id.substring(1); + return generateUniqueId(prefix, suffix); + } + return id; + } + /** * Normalize passed string into valid string. *