XWIKI-12987: Relative links are made absolute or even broken after mo…

…ving a page

  * Change APIs to use a Map<EntityReference, EntityReference>
    corresponding to the source and target of refactorings in renamers
  * Change some logic of AbstractCopyOrMoveJob to compute the actual
    couple source/destination before performing any operation and store
the info in EntitySelection
  * Add a log in RenameJob if it's not executed because of the number of
    entities (not needed for this issue, but felt better to understand
what's happening)

xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/XWiki.java

@@ -4974,7 +4974,8 @@ private void updateLinksForRename(XWikiDocument sourceDoc, DocumentReference new
         JobContext jobContext = Utils.getComponent(JobContext.class);
         Job currentJob = jobContext.getCurrentJob();
 
-        Set<DocumentReference> updatedReferences = Set.of();
+        Map<EntityReference, EntityReference> updatedReferences =
+            Map.of(sourceDoc.getDocumentReference(), newDocumentReference);
         if (currentJob instanceof AbstractCopyOrMoveJob) {
             updatedReferences = ((AbstractCopyOrMoveJob) currentJob).getSelectedEntities();
         }

xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/render/DefaultOldRendering.java

@@ -20,6 +20,7 @@
 package com.xpn.xwiki.internal.render;
 
 import java.io.StringWriter;
+import java.util.Map;
 import java.util.Set;
 
 import javax.inject.Inject;
@@ -117,7 +118,8 @@ private void refactorDocumentLinks(XWikiDocument document, DocumentReference old
         DocumentReference newDocumentReference, XWikiContext context) throws XWikiException
     {
         this.referenceRenamer.renameReferences(document.getXDOM(), document.getDocumentReference(),
-            oldDocumentReference, newDocumentReference, false, Set.of());
+            oldDocumentReference, newDocumentReference, false,
+            Map.of(oldDocumentReference, newDocumentReference));
     }
 
     @Override

xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/XWikiTest.java

@@ -29,7 +29,6 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
-import java.util.Set;
 
 import javax.inject.Provider;
 import javax.servlet.http.Cookie;
@@ -1094,13 +1093,14 @@ void atomicRename() throws Exception
             new DocumentReference(targetReference, Locale.GERMAN), xWikiContext);
 
         // Test links
-        verify(this.referenceUpdater).update(targetReference, sourceReference, targetReference, Set.of());
+        verify(this.referenceUpdater).update(targetReference, sourceReference, targetReference,
+            Map.of(sourceReference, targetReference));
         verify(this.referenceRenamer).renameReferences(doc1.getXDOM(), reference1, sourceReference,
-            targetReference, false, Set.of());
+            targetReference, false, Map.of(sourceReference, targetReference));
         verify(this.referenceRenamer).renameReferences(doc2.getXDOM(), reference2, sourceReference,
-            targetReference, false, Set.of());
+            targetReference, false, Map.of(sourceReference, targetReference));
         verify(this.referenceRenamer).renameReferences(doc3.getXDOM(), reference3, sourceReference,
-            targetReference, false, Set.of());
+            targetReference, false, Map.of(sourceReference, targetReference));
 
         assertTrue(this.xwiki
             .getDocument(new DocumentReference(DOCWIKI, DOCSPACE, DOCNAME), this.oldcore.getXWikiContext()).isNew());

xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/ReferenceRenamer.java

@@ -19,11 +19,12 @@
  */
 package org.xwiki.refactoring;
 
-import java.util.Set;
+import java.util.Map;
 
 import org.xwiki.component.annotation.Role;
 import org.xwiki.model.reference.AttachmentReference;
 import org.xwiki.model.reference.DocumentReference;
+import org.xwiki.model.reference.EntityReference;
 import org.xwiki.rendering.block.Block;
 import org.xwiki.stability.Unstable;
 
@@ -57,15 +58,15 @@ boolean renameReferences(Block block, DocumentReference currentDocumentReference
      * @param oldTarget the previous reference of the renamed entity (attachment or document)
      * @param newTarget the new reference of the renamed entity (attachment or document)
      * @param relative {@code true} if the link should be serialized relatively to the current document
-     * @param updatedDocuments the list of documents that have been renamed in the same job: this list contains the
-     *                          old references before the rename
+     * @param updatedEntities the map of entities that are or are going to be updated: the map contains the source
+     *      and target destination.
      * @return {@code true} if the given {@link Block} was modified
      * @since 16.10.0RC1
      */
     @Unstable
     default boolean renameReferences(Block block, DocumentReference currentDocumentReference,
-        DocumentReference oldTarget,
-        DocumentReference newTarget, boolean relative, Set<DocumentReference> updatedDocuments)
+        DocumentReference oldTarget, DocumentReference newTarget, boolean relative,
+        Map<EntityReference, EntityReference> updatedEntities)
     {
         return renameReferences(block, currentDocumentReference, oldTarget, newTarget, relative);
     }
@@ -95,15 +96,15 @@ default boolean renameReferences(Block block, DocumentReference currentDocumentR
      * @param oldTarget the previous reference of the renamed entity (attachment or document)
      * @param newTarget the new reference of the renamed entity (attachment or document)
      * @param relative {@code true} if the link should be serialized relatively to the current document
-     * @param updatedDocuments the list of documents that have been renamed in the same job: this list contains the
-     *     old references before the rename
+     * @param updatedEntities the map of entities that are or are going to be updated: the map contains the source
+     *      and target destination.
      * @return {@code true} if the given {@link Block} was modified
      * @since 16.10.0RC1
      */
     @Unstable
     default boolean renameReferences(Block block, DocumentReference currentDocumentReference,
         AttachmentReference oldTarget, AttachmentReference newTarget, boolean relative,
-        Set<DocumentReference> updatedDocuments)
+        Map<EntityReference, EntityReference> updatedEntities)
     {
         return renameReferences(block, currentDocumentReference, oldTarget, newTarget, relative);
     }

xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/ReferenceUpdater.java

@@ -19,7 +19,7 @@
  */
 package org.xwiki.refactoring.internal;
 
-import java.util.Set;
+import java.util.Map;
 
 import org.xwiki.component.annotation.Role;
 import org.xwiki.model.reference.DocumentReference;
@@ -38,11 +38,12 @@ public interface ReferenceUpdater
      * @param documentReference the reference of the document in which to update the references
      * @param oldTargetReference the previous reference of the renamed entity
      * @param newTargetReference the new reference of the renamed entity
-     * @param updatedEntities the set of entities that might have been updated.
+     * @param updatedEntities the map of entities that are or are going to be updated: the map contains the source
+     * and target destination.
      * @since 16.10.0RC1
      */
     default void update(DocumentReference documentReference, EntityReference oldTargetReference,
-        EntityReference newTargetReference, Set<DocumentReference> updatedEntities)
+        EntityReference newTargetReference, Map<EntityReference, EntityReference> updatedEntities)
     {
         update(documentReference, oldTargetReference, newTargetReference);
     }

xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractCopyOrMoveJob.java

@@ -21,6 +21,9 @@
 
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
+import java.util.function.BiConsumer;
+import java.util.stream.Collectors;
 
 import org.xwiki.model.EntityType;
 import org.xwiki.model.reference.DocumentReference;
@@ -100,26 +103,14 @@ protected void process(EntityReference source)
 
         EntityReference destination = this.request.getDestination();
 
-        if (processOnlySameSourceDestinationTypes()) {
-            if (source.getType() != destination.getType()) {
-                this.logger.error("You cannot change the entity type (from [{}] to [{}]).", source.getType(),
-                    destination.getType());
-                return;
-            }
-        }
-
-        if (isDescendantOrSelf(destination, source)) {
-            this.logger.error("Cannot make [{}] a descendant of itself.", source);
-            return;
-        }
-
-        if (source.getParent() != null && source.getParent().equals(destination)) {
-            this.logger.error("Cannot move [{}] into [{}], it's already there.", source, destination);
+        try {
+            checkSourceDestination(source, destination);
+        } catch (InternalCopyOrMoveJobException e) {
+            this.logger.error(e.getMessage());
             return;
         }
 
         // Dispatch the move operation based on the source entity type.
-
         switch (source.getType()) {
             case DOCUMENT:
                 try {
@@ -129,13 +120,34 @@ protected void process(EntityReference source)
                 }
                 break;
             case SPACE:
-                process(new SpaceReference(source), destination);
+                visitSpace(new SpaceReference(source), destination, this::maybePerformRefactoring);
                 break;
             default:
                 this.logger.error("Unsupported source entity type [{}].", source.getType());
         }
     }
 
+    private void checkSourceDestination(EntityReference source, EntityReference destination)
+        throws InternalCopyOrMoveJobException
+    {
+        if (processOnlySameSourceDestinationTypes() && source.getType() != destination.getType()) {
+            throw new InternalCopyOrMoveJobException(
+                String.format("You cannot change the entity type (from [%s] to [%s]).",
+                    source.getType(),
+                    destination.getType()));
+        }
+
+        if (isDescendantOrSelf(destination, source)) {
+            throw new InternalCopyOrMoveJobException(
+                String.format("Cannot make [%s] a descendant of itself.", source));
+        }
+
+        if (source.getParent() != null && source.getParent().equals(destination)) {
+            throw new InternalCopyOrMoveJobException(
+                String.format("Cannot move [%s] into [%s], it's already there.", source, destination));
+        }
+    }
+
     private boolean isDescendantOrSelf(EntityReference alice, EntityReference bob)
     {
         EntityReference parent = alice;
@@ -145,6 +157,71 @@ private boolean isDescendantOrSelf(EntityReference alice, EntityReference bob)
         return parent != null;
     }
 
+    @Override
+    protected void getEntities(DocumentReference documentReference)
+    {
+        this.putInConcernedEntities(documentReference);
+    }
+
+    @Override
+    protected void putInConcernedEntities(DocumentReference documentReference)
+    {
+        DocumentReference source = cleanLocale(documentReference);
+        try {
+            EntityReference destination = this.request.getDestination();
+            checkSourceDestination(source, destination);
+            DocumentReference destinationDocumentReference;
+            if (processOnlySameSourceDestinationTypes()) {
+                putInConcernedEntitiesOnlySameSource(source, destination);
+            } else {
+                if (this.request.isDeep() && isSpaceHomeReference(source)) {
+                    visitSpace(source.getLastSpaceReference(), destination, this::putInConcernedEntities);
+                } else if (destination.getType() == EntityType.SPACE) {
+                    destinationDocumentReference =
+                        new DocumentReference(source.getName(), new SpaceReference(destination));
+                    this.putInConcernedEntities(source, destinationDocumentReference);
+                } else if (destination.getType() == EntityType.DOCUMENT
+                    && isSpaceHomeReference(new DocumentReference(destination))) {
+                    destinationDocumentReference =
+                        new DocumentReference(source.getName(), new SpaceReference(destination.getParent()));
+                    this.putInConcernedEntities(source, destinationDocumentReference);
+                } else if (destination.getType() == EntityType.WIKI) {
+                    visitSpace(source.getLastSpaceReference(), destination, this::putInConcernedEntities);
+                }
+            }
+        } catch (InternalCopyOrMoveJobException e) {
+            this.logger.debug(e.getMessage());
+        }
+    }
+
+    private void putInConcernedEntitiesOnlySameSource(DocumentReference source, EntityReference destination)
+    {
+        DocumentReference destinationDocumentReference = new DocumentReference(destination);
+        if (this.request.isDeep() && isSpaceHomeReference(source)) {
+            if (isSpaceHomeReference(destinationDocumentReference)) {
+                visitSpace(source.getLastSpaceReference(), destinationDocumentReference.getLastSpaceReference(),
+                    this::putInConcernedEntities);
+            }
+        } else {
+            this.putInConcernedEntities(source, destinationDocumentReference);
+        }
+    }
+
+    private void putInConcernedEntities(DocumentReference sourceDocument, DocumentReference destination)
+    {
+        try {
+            if (!this.modelBridge.exists(sourceDocument)) {
+                this.logger.warn("Skipping [{}] because it doesn't exist.", sourceDocument);
+            } else if (this.checkAllRights(sourceDocument, destination)) {
+                this.concernedEntities.put(sourceDocument, new EntitySelection(sourceDocument, destination));
+            }
+        } catch (Exception e) {
+            logger.error("Failed to perform the refactoring from document with reference [{}] to [{}]",
+                sourceDocument, destination, e);
+        }
+    }
+
+    // FIXME: this should be factorized with the code from getTargetReference
     protected void process(DocumentReference source, EntityReference destination) throws Exception
     {
         if (processOnlySameSourceDestinationTypes()) {
@@ -153,7 +230,7 @@ protected void process(DocumentReference source, EntityReference destination) th
             this.process(source, destinationDocumentReference);
         } else {
             if (this.request.isDeep() && isSpaceHomeReference(source)) {
-                process(source.getLastSpaceReference(), destination);
+                visitSpace(source.getLastSpaceReference(), destination, this::maybePerformRefactoring);
             } else if (destination.getType() == EntityType.SPACE) {
                 maybePerformRefactoring(source,
                     new DocumentReference(source.getName(), new SpaceReference(destination)));
@@ -182,39 +259,35 @@ protected void process(DocumentReference source, DocumentReference destination)
         }
     }
 
-    protected void process(SpaceReference source, EntityReference destination)
+    private void visitSpace(final SpaceReference source, final EntityReference destination,
+        BiConsumer<DocumentReference, DocumentReference> callback)
     {
+        SpaceReference spaceDestination;
         if (processOnlySameSourceDestinationTypes()) {
             // We know the destination is a space (see above).
-            process(source, new SpaceReference(destination));
+            spaceDestination = new SpaceReference(destination);
         } else {
             if (destination.getType() == EntityType.SPACE || destination.getType() == EntityType.WIKI) {
-                process(source, new SpaceReference(source.getName(), destination));
+                spaceDestination = new SpaceReference(source.getName(), destination);
             } else if (destination.getType() == EntityType.DOCUMENT
                 && isSpaceHomeReference(new DocumentReference(destination))) {
-                process(source, new SpaceReference(source.getName(), destination.getParent()));
+                spaceDestination = new SpaceReference(source.getName(), destination.getParent());
             } else {
+                spaceDestination = null;
                 this.logger.error("Unsupported destination entity type [{}] for a space.", destination.getType());
             }
         }
+        if (spaceDestination != null) {
+            visitDocuments(source, oldChildReference -> {
+                DocumentReference newChildReference = oldChildReference.replaceParent(source, spaceDestination);
+                callback.accept(oldChildReference, newChildReference);
+            });
+        }
     }
 
     protected void process(final SpaceReference source, final SpaceReference destination)
     {
-        visitDocuments(source, new Visitor<DocumentReference>()
-        {
-            @Override
-            public void visit(DocumentReference oldChildReference)
-            {
-                DocumentReference newChildReference = oldChildReference.replaceParent(source, destination);
-                try {
-                    maybePerformRefactoring(oldChildReference, newChildReference);
-                } catch (Exception e) {
-                    logger.error("Failed to perform the refactoring from document with reference [{}] to [{}]",
-                        oldChildReference, newChildReference, e);
-                }
-            }
-        });
+        visitSpace(source, destination, this::maybePerformRefactoring);
     }
 
     protected boolean checkAllRights(DocumentReference oldReference, DocumentReference newReference) throws Exception
@@ -232,18 +305,14 @@ protected boolean checkAllRights(DocumentReference oldReference, DocumentReferen
     }
 
     protected void maybePerformRefactoring(DocumentReference oldReference, DocumentReference newReference)
-        throws Exception
     {
         // Perform checks that are specific to the document source/destination type.
-
         EntitySelection entitySelection = this.getConcernedEntitiesEntitySelection(oldReference);
         if (entitySelection == null) {
             this.logger.info("Skipping [{}] because it does not match any entity selection.", oldReference);
         } else if (!entitySelection.isSelected()) {
             this.logger.info("Skipping [{}] because it has been unselected.", oldReference);
-        } else if (!this.modelBridge.exists(oldReference)) {
-            this.logger.warn("Skipping [{}] because it doesn't exist.", oldReference);
-        } else if (this.checkAllRights(oldReference, newReference)) {
+        } else {
             performRefactoring(oldReference, newReference);
         }
     }
@@ -330,6 +399,19 @@ protected EntityReference getCommonParent()
         return getCommonParent(entityReferences);
     }
 
+    /**
+     * @return the list of references that have been selected to be refactored.
+     * @since 16.10.0RC1
+     */
+    public Map<EntityReference, EntityReference> getSelectedEntities()
+    {
+        return this.concernedEntities.values().stream()
+            .filter(EntitySelection::isSelected)
+            .filter(entity -> entity.getTargetEntityReference().isPresent())
+            .collect(Collectors.toMap(EntitySelection::getEntityReference,
+                entity -> entity.getTargetEntityReference().get()));
+    }
+
     /**
      * Atomic operation to perform: should be a rename for Rename/Move and copy for Copy.
      * @param source the source reference