XRENDERING-802: Prepared macro content leads to duplicated IDs when inserted

xwiki-platform-core/xwiki-platform-display/xwiki-platform-display-api/pom.xml

@@ -33,7 +33,7 @@
   <description>Provides APIs to display XWiki entities (documents, objects, etc.). You can use for instance the
     DocumentDisplayer to display the title and the content of XWiki documents.</description>
   <properties>
-    <xwiki.jacoco.instructionRatio>0.34</xwiki.jacoco.instructionRatio>
+    <xwiki.jacoco.instructionRatio>0.47</xwiki.jacoco.instructionRatio>
     </properties>
   <dependencies>
     <dependency>

xwiki-platform-core/xwiki-platform-display/xwiki-platform-display-api/src/main/java/org/xwiki/display/internal/DocumentContentAsyncExecutor.java

@@ -24,7 +24,6 @@
 
 import javax.inject.Inject;
 
-import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.exception.ExceptionUtils;
 import org.slf4j.Logger;
 import org.xwiki.bridge.DocumentAccessBridge;
@@ -40,7 +39,6 @@
 import org.xwiki.rendering.RenderingException;
 import org.xwiki.rendering.block.Block;
 import org.xwiki.rendering.block.HeaderBlock;
-import org.xwiki.rendering.block.ImageBlock;
 import org.xwiki.rendering.block.XDOM;
 import org.xwiki.rendering.block.match.ClassBlockMatcher;
 import org.xwiki.rendering.block.match.CompositeBlockMatcher;
@@ -150,51 +148,12 @@ private XDOM getPreparedContent(DocumentModelBridge document, final DocumentDisp
 
         IdGenerator idGenerator = parameters.getIdGenerator();
         if (idGenerator != null) {
-            content.setIdGenerator(idGenerator);
-            makeIdsUnique(content, idGenerator);
+            content.setIdGenerator(idGenerator, true);
         }
 
         return content;
     }
 
-    /**
-     * Replace ids of heading and images by unique ids.
-     *
-     * @param content the XDOM to modify in-place
-     * @param idGenerator the id generator for unique ids
-     * @since 14.2RC1
-     */
-    private void makeIdsUnique(XDOM content, IdGenerator idGenerator)
-    {
-        // Traverse the XDOM and adapt all image and heading blocks.
-        content.getBlocks(block -> {
-            if (block instanceof ImageBlock) {
-                ImageBlock imageBlock = (ImageBlock) block;
-                imageBlock.setId(adaptId(idGenerator, imageBlock.getId()));
-            } else if (block instanceof HeaderBlock) {
-                HeaderBlock headerBlock = (HeaderBlock) block;
-                headerBlock.setId(adaptId(idGenerator, headerBlock.getId()));
-            }
-            return false;
-        }, Block.Axes.DESCENDANT);
-    }
-
-    /**
-     * @param idGenerator the id generator to use
-     * @param id the id to adapt to make it unique
-     * @return the unique id, can be the input if was already unique
-     * @since 14.2RC1
-     */
-    private String adaptId(IdGenerator idGenerator, String id)
-    {
-        if (StringUtils.isNotBlank(id)) {
-            String prefix = id.substring(0, 1);
-            String suffix = id.substring(1);
-            return idGenerator.generateUniqueId(prefix, suffix);
-        }
-        return id;
-    }
-
     /**
      * Get the translated content of the given document as XDOM tree. If the language of the given document matches the
      * context language (meaning that the given document is the current translation) then we use the content of the

xwiki-platform-core/xwiki-platform-display/xwiki-platform-display-api/src/test/java/org/xwiki/display/internal/DocumentContentAsyncExecutorTest.java

@@ -0,0 +1,104 @@
+/*
+ * See the NOTICE file distributed with this work for additional
+ * information regarding copyright ownership.
+ *
+ * This is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as
+ * published by the Free Software Foundation; either version 2.1 of
+ * the License, or (at your option) any later version.
+ *
+ * This software is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this software; if not, write to the Free
+ * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA, or see the FSF site: http://www.fsf.org.
+ */
+package org.xwiki.display.internal;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mock;
+import org.xwiki.bridge.DocumentModelBridge;
+import org.xwiki.context.Execution;
+import org.xwiki.context.ExecutionContext;
+import org.xwiki.rendering.block.ImageBlock;
+import org.xwiki.rendering.block.XDOM;
+import org.xwiki.rendering.util.IdGenerator;
+import org.xwiki.test.junit5.mockito.ComponentTest;
+import org.xwiki.test.junit5.mockito.InjectMockComponents;
+import org.xwiki.test.junit5.mockito.MockComponent;
+import org.xwiki.velocity.VelocityEngine;
+import org.xwiki.velocity.VelocityManager;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+/**
+ * Unit tests for {@link DocumentContentAsyncExecutor}.
+ *
+ * @version $Id$
+ */
+@ComponentTest
+class DocumentContentAsyncExecutorTest
+{
+    @InjectMockComponents
+    private DocumentContentAsyncExecutor documentContentAsyncExecutor;
+
+    @MockComponent
+    private Execution execution;
+
+    @MockComponent
+    private VelocityManager velocityManager;
+
+    @Mock
+    private DocumentModelBridge document;
+
+    private Map<Object, Object> xcontext = new HashMap<>();
+
+    private DocumentDisplayerParameters parameters = new DocumentDisplayerParameters();
+
+    @BeforeEach
+    void beforeEach() throws Exception
+    {
+        ExecutionContext executionContext = new ExecutionContext();
+        executionContext.setProperty("xwikicontext", this.xcontext);
+        when(this.execution.getContext()).thenReturn(executionContext);
+
+        VelocityEngine velocityEngine = mock(VelocityEngine.class);
+        when(this.velocityManager.getVelocityEngine()).thenReturn(velocityEngine);
+    }
+
+    @Test
+    void initializeAndExecuteAsync() throws Exception
+    {
+        IdGenerator idGenerator = new IdGenerator();
+        // Simulate the fact that the id generator has already generated some ids.
+        idGenerator.generateUniqueId("logo");
+        this.parameters.setIdGenerator(idGenerator);
+
+        ImageBlock image = new ImageBlock(null, true);
+        // Use an id that was already generated by the id generator we're going to pass in the display parameters.
+        image.setId("Ilogo");
+        XDOM preparedXDOM = new XDOM(List.of(image));
+        when(this.document.getPreparedXDOM()).thenReturn(preparedXDOM);
+
+        this.documentContentAsyncExecutor.initialize("test", this.document, this.parameters);
+        XDOM xdom = this.documentContentAsyncExecutor.execute(true);
+
+        // Verify the returned XDOM is using the id generator provided in the display parameters.
+        assertSame(idGenerator, xdom.getIdGenerator());
+
+        // Verify the image id has been adapted.
+        assertEquals("Ilogo-1", image.getId());
+    }
+}