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

…ving a page

  * Find proper conditions to perform or not link renames
  * Fix unit tests to add missing conditions
  * WIP: need to fix coverage and check on subwikis / with more
    conditions (e.g. with holes in hierarchy)

xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java

@@ -134,7 +134,7 @@ public boolean updateResourceReference(ResourceReference resourceReference,
 
     private boolean updateAbsoluteResourceReference(ResourceReference resourceReference,
         DocumentReference oldReference, DocumentReference newReference, DocumentReference currentDocumentReference,
-        Set<? extends EntityReference> movedDocuments)
+        Set<? extends EntityReference> movedReferences)
     {
         boolean result = false;
 
@@ -146,12 +146,14 @@ private boolean updateAbsoluteResourceReference(ResourceReference resourceRefere
             this.defaultReferenceDocumentReferenceResolver.resolve(linkEntityReference);
         EntityReference newTargetReference = newReference;
         ResourceType newResourceType = resourceReference.getType();
+        // TODO: test in subwiki
+        EntityReference absoluteResolvedReference = this.entityReferenceResolver.resolve(resourceReference, null);
 
-        // If the link targets the old (renamed) document reference we must update it,
-        // except if the target link document is also part of the moved documents, in which case the hierarchy is moved.
-        boolean shouldBeUpdated = linkTargetDocumentReference.equals(oldReference)
-            // FIXME: this oracle is not good, it currently works with test mostly by luck
-            && !movedDocuments.contains(currentDocumentReference);
+        // If the link targets the old (renamed) document reference and it's an absolute reference
+        // (i.e. its resolution without any given parameter gives same result than its resolution with the
+        // currentDocument) then we must update it
+        boolean shouldBeUpdated =
+            linkTargetDocumentReference.equals(oldReference) && absoluteResolvedReference.equals(linkEntityReference);
 
         if (shouldBeUpdated) {
             // If the link was resolved to a space...
@@ -200,15 +202,7 @@ private <T extends EntityReference> boolean updateRelativeResourceReference(Reso
         // current link, use the old document's reference to fill in blanks.
         EntityReference oldLinkReference =
             this.entityReferenceResolver.resolve(resourceReference, null, oldReference);
-
-        boolean anyMatchInMovedReferences = oldLinkReference.hasParent(oldReference)
-            || movedReferences.contains(oldLinkReference)
-            // TODO: check if that oracle is good in case of holes in the hierarchy
-            || movedReferences.stream().anyMatch(oldLinkReference::hasParent);
-
-        if (anyMatchInMovedReferences) {
-            return false;
-        }
+        EntityReference absoluteResolvedReference = this.entityReferenceResolver.resolve(resourceReference, null);
 
         boolean docExists = false;
         EntityType entityType = linkEntityReference.getType();
@@ -220,10 +214,23 @@ private <T extends EntityReference> boolean updateRelativeResourceReference(Reso
             docExists = true;
         }
 
-        boolean shouldBeUpdated =
-            !linkEntityReference.equals(oldLinkReference) && docExists;
+        boolean anyMatchInMovedReferences =
+            (oldLinkReference.hasParent(oldReference)
+                || movedReferences.contains(oldLinkReference)
+                // TODO: check if that oracle is good in case of holes in the hierarchy
+                || movedReferences.stream().anyMatch(oldLinkReference::hasParent));
+
+        // We should update the reference if:
+        //  - it's relative: the resolution of the reference without any parameter, and the resolution of the reference
+        //                   with the new reference should give different results
+        //  - it doesn't match any reference moved in the same job, i.e. in the same hierarchy
+        //  - the new and old link references don`t match
+        //  - the link refers to a doc that exists
+        boolean shouldBeUpdated = !absoluteResolvedReference.equals(linkEntityReference)
+            && !anyMatchInMovedReferences
+            && !linkEntityReference.equals(oldLinkReference)
+            && docExists;
 
-        // If the new and old link references don`t match, then we must update the relative link.
         if (shouldBeUpdated) {
             // Serialize the old (original) link relative to the new document's location, in compact form.
             String serializedLinkReference =

xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultReferenceUpdaterTest.java

@@ -233,6 +233,8 @@ void updateRelativeLinks() throws Exception
         AttachmentReference oldImageTargetAttachment = new AttachmentReference("attachment.txt", oldReference);
         DocumentReference newReference = new DocumentReference("wiki", "X", "Y");
         AttachmentReference newImageTargetAttachment = new AttachmentReference("attachment.txt", newReference);
+        AttachmentReference absoluteTargetAttachment = new AttachmentReference("attachment.txt",
+            new DocumentReference("wiki", "Main", "WebHome"));
 
         XWikiDocument newDocument = mock(XWikiDocument.class);
         when(this.xcontext.getWiki().getDocument(newReference, this.xcontext)).thenReturn(newDocument);
@@ -260,14 +262,23 @@ void updateRelativeLinks() throws Exception
         setTextarea(newDocument,
             new XDOM(Arrays.asList(xobjectDocLinkBlock, xobjectSpaceLinkBlock, xobjectImageBlock)));
 
-        DocumentReference originalDocLinkReference = new DocumentReference("C", oldReference.getLastSpaceReference());
+        DocumentReference originalDocLinkReference = new DocumentReference("WebHome",
+            new SpaceReference("C", oldReference.getLastSpaceReference()));
+        DocumentReference absoluteDocLinkReference = new DocumentReference("WebHome", new SpaceReference("wiki", "C"));
+        when(this.resourceReferenceResolver.resolve(docLinkReference, null))
+            .thenReturn(absoluteDocLinkReference);
         when(this.resourceReferenceResolver.resolve(docLinkReference, null, oldReference))
             .thenReturn(originalDocLinkReference);
+        when(this.resourceReferenceResolver.resolve(xobjectDocLinkReference, null))
+            .thenReturn(absoluteDocLinkReference);
         when(this.resourceReferenceResolver.resolve(xobjectDocLinkReference, null, oldReference))
             .thenReturn(originalDocLinkReference);
+        when(this.resourceReferenceResolver.resolve(imageReference, null))
+            .thenReturn(absoluteTargetAttachment);
         when(this.resourceReferenceResolver.resolve(imageReference, null, oldReference))
             .thenReturn(oldImageTargetAttachment);
-        DocumentReference newDocLinkReference = new DocumentReference("C", newReference.getLastSpaceReference());
+        DocumentReference newDocLinkReference = new DocumentReference("WebHome",
+            new SpaceReference("C", newReference.getLastSpaceReference()));
         when(this.resourceReferenceResolver.resolve(docLinkReference, null, newReference))
             .thenReturn(newDocLinkReference);
         when(this.resourceReferenceResolver.resolve(xobjectDocLinkReference, null, newReference))
@@ -276,8 +287,12 @@ void updateRelativeLinks() throws Exception
             .thenReturn(newImageTargetAttachment);
 
         SpaceReference originalSpaceReference = new SpaceReference("wiki", "Z");
+        when(this.resourceReferenceResolver.resolve(spaceLinkReference, null))
+            .thenReturn(originalSpaceReference);
         when(this.resourceReferenceResolver.resolve(spaceLinkReference, null, oldReference))
             .thenReturn(originalSpaceReference);
+        when(this.resourceReferenceResolver.resolve(xobjectSpaceLinkReference, null))
+            .thenReturn(originalSpaceReference);
         when(this.resourceReferenceResolver.resolve(xobjectSpaceLinkReference, null, oldReference))
             .thenReturn(originalSpaceReference);
         when(this.resourceReferenceResolver.resolve(spaceLinkReference, null, newReference))
@@ -334,13 +349,17 @@ void updateRelativeLinksAcrossWikis() throws Exception
             .thenReturn(Arrays.asList(docLinkBlock, spaceLinkBlock));
 
         DocumentReference originalDocLinkReference = new DocumentReference("C", oldReference.getLastSpaceReference());
+        DocumentReference absoluteDocLinkReference = new DocumentReference("C", new SpaceReference("xwiki", "Main"));
+        when(this.resourceReferenceResolver.resolve(docLinkReference, null)).thenReturn(absoluteDocLinkReference);
         when(this.resourceReferenceResolver.resolve(docLinkReference, null, oldReference))
             .thenReturn(originalDocLinkReference);
         DocumentReference newDocLinkReference = new DocumentReference("C", newReference.getLastSpaceReference());
         when(this.resourceReferenceResolver.resolve(docLinkReference, null, newReference))
             .thenReturn(newDocLinkReference);
 
         SpaceReference originalSpaceReference = new SpaceReference("wiki1", "Z");
+        SpaceReference absoluteSpaceReference = new SpaceReference("xwiki", "Z");
+        when(this.resourceReferenceResolver.resolve(spaceLinkReference, null)).thenReturn(absoluteSpaceReference);
         when(this.resourceReferenceResolver.resolve(spaceLinkReference, null, oldReference))
             .thenReturn(originalSpaceReference);
         SpaceReference newSpaceReference = new SpaceReference("wiki2", "Z");
@@ -388,7 +407,10 @@ void update() throws Exception
         XDOM xobjectXDOM = new XDOM(Collections.singletonList(xobjectLinkBlock));
         setTextarea(document, xobjectXDOM);
 
+        when(this.resourceReferenceResolver.resolve(linkReference, null)).thenReturn(oldLinkTarget);
         when(this.resourceReferenceResolver.resolve(linkReference, null, documentReference)).thenReturn(oldLinkTarget);
+        when(this.resourceReferenceResolver.resolve(xobjectLinkReference, null))
+            .thenReturn(oldLinkTarget);
         when(this.resourceReferenceResolver.resolve(xobjectLinkReference, null, documentReference))
             .thenReturn(oldLinkTarget);
         when(this.defaultReferenceDocumentReferenceResolver.resolve(oldLinkTarget)).thenReturn(oldLinkTarget);
@@ -425,6 +447,7 @@ void renameImage() throws Exception
         ImageBlock imageBlock = new ImageBlock(imageReference, false);
         when(xdom.getBlocks(any(), eq(Block.Axes.DESCENDANT))).thenReturn(Arrays.asList(imageBlock));
 
+        when(this.resourceReferenceResolver.resolve(imageReference, null)).thenReturn(oldImageTargetAttachment);
         when(this.resourceReferenceResolver.resolve(imageReference, null, documentReference))
             .thenReturn(oldImageTargetAttachment);
         when(this.defaultReferenceDocumentReferenceResolver.resolve(oldImageTargetAttachment))
@@ -462,6 +485,8 @@ public void renameAttachment() throws Exception
         LinkBlock linkBlock = new LinkBlock(Collections.emptyList(), linkReference, false);
         when(xdom.getBlocks(any(), eq(Block.Axes.DESCENDANT))).thenReturn(Arrays.asList(linkBlock));
 
+        when(this.resourceReferenceResolver.resolve(linkReference, null))
+            .thenReturn(oldLinkTargetAttachment);
         when(this.resourceReferenceResolver.resolve(linkReference, null, documentReference))
             .thenReturn(oldLinkTargetAttachment);
         when(this.defaultReferenceDocumentReferenceResolver.resolve(oldLinkTargetAttachment)).thenReturn(oldLinkTarget);
@@ -502,13 +527,17 @@ void renameNonTerminalDocumentLinks() throws Exception
             .thenReturn(Arrays.asList(documentLinkBlock, spaceLinkBlock));
 
         // Doc link
+        when(this.resourceReferenceResolver.resolve(docLinkReference, null))
+            .thenReturn(oldLinkTarget);
         when(this.resourceReferenceResolver.resolve(docLinkReference, null, documentReference))
             .thenReturn(oldLinkTarget);
         when(this.defaultReferenceDocumentReferenceResolver.resolve(oldLinkTarget)).thenReturn(oldLinkTarget);
         when(this.compactEntityReferenceSerializer.serialize(newLinkTarget, documentReference)).thenReturn("X.WebHome");
 
         // Space link
         SpaceReference spaceReference = oldLinkTarget.getLastSpaceReference();
+        when(this.resourceReferenceResolver.resolve(spaceLinkReference, null))
+            .thenReturn(spaceReference);
         when(this.resourceReferenceResolver.resolve(spaceLinkReference, null, documentReference))
             .thenReturn(spaceReference);
         when(this.defaultReferenceDocumentReferenceResolver.resolve(spaceReference)).thenReturn(oldLinkTarget);
@@ -550,13 +579,17 @@ void renameNonTerminalToTerminalDocumentLinks() throws Exception
             .thenReturn(Arrays.asList(documentLinkBlock, spaceLinkBlock));
 
         // Doc link
+        when(this.resourceReferenceResolver.resolve(docLinkReference, null))
+            .thenReturn(oldLinkTarget);
         when(this.resourceReferenceResolver.resolve(docLinkReference, null, documentReference))
             .thenReturn(oldLinkTarget);
         when(this.defaultReferenceDocumentReferenceResolver.resolve(oldLinkTarget)).thenReturn(oldLinkTarget);
         when(this.compactEntityReferenceSerializer.serialize(newLinkTarget, documentReference)).thenReturn("X.Y");
 
         // Space link
         SpaceReference spaceReference = oldLinkTarget.getLastSpaceReference();
+        when(this.resourceReferenceResolver.resolve(spaceLinkReference, null))
+            .thenReturn(spaceReference);
         when(this.resourceReferenceResolver.resolve(spaceLinkReference, null, documentReference))
             .thenReturn(spaceReference);
         when(this.defaultReferenceDocumentReferenceResolver.resolve(spaceReference)).thenReturn(oldLinkTarget);
@@ -610,6 +643,8 @@ void updateFromMacros(MockitoComponentManager componentManager) throws Exception
 
         ResourceReference macroResourceReference = new ResourceReference("A.B", ResourceType.DOCUMENT);
 
+        when(this.resourceReferenceResolver.resolve(macroResourceReference, null))
+            .thenReturn(oldLinkTarget);
         when(this.resourceReferenceResolver.resolve(macroResourceReference, null, documentReference))
             .thenReturn(oldLinkTarget);
         when(this.defaultReferenceDocumentReferenceResolver.resolve(oldLinkTarget)).thenReturn(oldLinkTarget);
@@ -657,6 +692,8 @@ void updateAttachments() throws Exception
         when(document.getSyntax()).thenReturn(Syntax.XWIKI_2_1);
         when(document.getXDOM()).thenReturn(xdom);
         when(xdom.getBlocks(any(), eq(Block.Axes.DESCENDANT))).thenReturn(List.of(documentLinkBlock));
+        when(this.resourceReferenceResolver.resolve(resourceReference, null)).thenReturn(
+            new AttachmentReference("oldname.txt", new DocumentReference("wiki", "Main", "WebHome")));
         when(this.resourceReferenceResolver.resolve(resourceReference, null, documentReference)).thenReturn(
             oldLinkTarget);
         when(this.compactEntityReferenceSerializer.serialize(newLinkTarget, documentReference)).thenReturn(
@@ -693,7 +730,7 @@ void updateFromLinksAndMacros(MockitoComponentManager componentManager) throws E
         XDOM xdom = mock(XDOM.class);
         when(document.getXDOM()).thenReturn(xdom);
 
-        Map<String, String> includeParameters = new HashMap<String, String>();
+        Map<String, String> includeParameters = new HashMap<>();
         includeParameters.put("reference", "A.B");
         MacroBlock includeMacroBlock = new MacroBlock("include", includeParameters, false);
 
@@ -703,6 +740,8 @@ void updateFromLinksAndMacros(MockitoComponentManager componentManager) throws E
         when(xdom.getBlocks(any(), eq(Block.Axes.DESCENDANT)))
             .thenReturn(Arrays.asList(includeMacroBlock, documentLinkBlock));
 
+        when(this.resourceReferenceResolver.resolve(resourceReference, null))
+            .thenReturn(oldLinkTarget);
         when(this.resourceReferenceResolver.resolve(resourceReference, null, documentReference))
             .thenReturn(oldLinkTarget);
         when(this.defaultReferenceDocumentReferenceResolver.resolve(oldLinkTarget)).thenReturn(oldLinkTarget);
@@ -771,8 +810,10 @@ void updateAndTranslations() throws Exception
 
             ResourceReference linkReference = new ResourceReference("A.B", ResourceType.DOCUMENT);
             LinkBlock linkBlock = new LinkBlock(Collections.emptyList(), linkReference, false);
-            when(xdom.getBlocks(any(), eq(Block.Axes.DESCENDANT))).thenReturn(Arrays.asList(linkBlock));
+            when(xdom.getBlocks(any(), eq(Block.Axes.DESCENDANT))).thenReturn(List.of(linkBlock));
 
+            when(this.resourceReferenceResolver.resolve(linkReference, null))
+                .thenReturn(oldLinkTarget);
             when(this.resourceReferenceResolver.resolve(linkReference, null, documentReference))
                 .thenReturn(oldLinkTarget);
             when(this.defaultReferenceDocumentReferenceResolver.resolve(oldLinkTarget)).thenReturn(oldLinkTarget);

xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/ResourceReferenceRenamerTest.java

@@ -99,6 +99,7 @@ void updateResourceReferenceRelative()
             new AttachmentReference("file.txt", new DocumentReference("wiki", "space", "page"));
         AttachmentReference newReference =
             new AttachmentReference("file2.txt", new DocumentReference("wiki", "space", "page"));
+        when(this.entityReferenceResolver.resolve(resourceReference, null)).thenReturn(oldReference);
         when(this.entityReferenceResolver.resolve(resourceReference, null, newReference)).thenReturn(newReference);
         when(this.entityReferenceResolver.resolve(resourceReference, null, oldReference)).thenReturn(oldReference);
         when(this.compactEntityReferenceSerializer.serialize(oldReference, newReference)).thenReturn("file2.txt");
@@ -119,8 +120,12 @@ void updateResourceReferenceNotRelative()
             new AttachmentReference("file.txt", new DocumentReference("wiki", "space", "page"));
         AttachmentReference newReference =
             new AttachmentReference("file2.txt", new DocumentReference("wiki", "space", "page"));
+        AttachmentReference absoluteReference =
+            new AttachmentReference("image.png", new DocumentReference("xwiki", "Main", "WebHome"));
         DocumentReference currentDocumentReference = new DocumentReference("xwiki", "Space", "Page");
 
+        when(this.entityReferenceResolver.resolve(resourceReference, null))
+            .thenReturn(absoluteReference);
         when(this.entityReferenceResolver.resolve(resourceReference, null, currentDocumentReference))
             .thenReturn(oldReference);
         when(this.compactEntityReferenceSerializer.serialize(newReference, currentDocumentReference))

xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-macros/xwiki-platform-rendering-macro-include/src/main/java/org/xwiki/rendering/internal/macro/include/IncludeMacroRefactoring.java

@@ -103,6 +103,24 @@ public Optional<MacroBlock> replaceReference(MacroBlock macroBlock, DocumentRefe
             updatedDocuments);
     }
 
+    @Override
+    public Optional<MacroBlock> replaceReference(MacroBlock macroBlock, DocumentReference currentDocumentReference,
+        DocumentReference sourceReference, DocumentReference targetReference, boolean relative)
+        throws MacroRefactoringException
+    {
+        return getMacroBlock(macroBlock, currentDocumentReference, sourceReference, targetReference, relative,
+            Set.of());
+    }
+
+    @Override
+    public Optional<MacroBlock> replaceReference(MacroBlock macroBlock, DocumentReference currentDocumentReference,
+        AttachmentReference sourceReference, AttachmentReference targetReference, boolean relative)
+        throws MacroRefactoringException
+    {
+        return getMacroBlock(macroBlock, currentDocumentReference, sourceReference, targetReference, relative,
+            Set.of());
+    }
+
     private <T extends EntityReference> Optional<MacroBlock> getMacroBlock(MacroBlock macroBlock,
         DocumentReference currentDocumentReference, T sourceReference, T targetReference, boolean relative,
         Set<DocumentReference> updatedDocuments)