Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XWIKI-12987: Relative links are made absolute or even broken after moving a page #3634

Merged
merged 12 commits into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -366,18 +366,16 @@ void renamePageUsedInMacroContentAndParameters(TestUtils setup, TestReference te

setup.rest().delete(testReference);

ViewPage standardLinkPage = setup.createPage(sourcePageReference1, "Some content to be linked. number 1");
ViewPage standardMacroLinkPage =
setup.createPage(sourcePageReference2, "Some content to be linked in macro. number 2");
ViewPage nestedMacroLinkPage =
setup.createPage(sourcePageReference3, "Some content to be linked in nested macro. number 3");
setup.createPage(sourcePageReference4, "A page with image to be linked. number 4");
setup.rest().savePage(sourcePageReference1, "Some content to be linked. number 1", "sourcePage1");
setup.rest().savePage(sourcePageReference2, "Some content to be linked in macro. number 2", "sourcePage2");
setup.rest().savePage(sourcePageReference3, "Some content to be linked in nested macro. number 3",
"sourcePage3");
setup.rest().savePage(sourcePageReference4, "A page with image to be linked. number 4", "sourcePage4");
AttachmentsPane attachmentsPane = new AttachmentsViewPage().openAttachmentsDocExtraPane();
File image = new File(testConfiguration.getBrowser().getTestResourcesPath(), "AttachmentIT/image.gif");
attachmentsPane.setFileToUpload(image.getAbsolutePath());
attachmentsPane.waitForUploadToFinish("image.gif");

ViewPage includeLinkPage = setup.createPage(sourcePageReference5, "A page to be included. number 5");
setup.rest().savePage(sourcePageReference5, "A page to be included. number 5", "sourcePage5");

String testPageContent = "Check out this page: [[type the link label>>doc:%1$s]]\n" + "\n" + "{{warning}}\n"
+ "Withing a macro: Check out this page: [[type the link label>>doc:%2$s]]\n" + "\n" + "{{error}}\n"
Expand All @@ -390,8 +388,9 @@ void renamePageUsedInMacroContentAndParameters(TestUtils setup, TestReference te
+ "Withing a macro: Check out this page: [[type the link label>>doc:%1$s]]\n"
+ "{{/warning}}\n\n"
+ "Final line.";
setup.createPage(testReference,
String.format(testPageContent, sourcePage1, sourcePage2, sourcePage3, sourcePage4, sourcePage5));
setup.rest().savePage(testReference,
String.format(testPageContent, sourcePage1, sourcePage2, sourcePage3, sourcePage4, sourcePage5),
"testPage");

// Wait for the solr indexing to be completed before doing any rename
new SolrTestUtils(setup, testConfiguration.getServletEngine()).waitEmptyQueue();
Expand Down Expand Up @@ -511,10 +510,10 @@ void renamePageRelativeLinkPageAndTranslation(TestUtils setup, TestReference tes

// Make sure the link was refactored in both the page and its translation
Page newPage = setup.rest().get(new LocalDocumentReference(newSpace, newName));
assertEquals("[[" + parent + ".OtherPage.WebHome]]", newPage.getContent());
assertEquals("[[OtherPage]]", newPage.getContent());

newPage = setup.rest().get(new LocalDocumentReference(newSpace, newName, Locale.FRENCH));
assertEquals("fr [[" + parent + ".OtherPage.WebHome]]", newPage.getContent());
assertEquals("fr [[OtherPage]]", newPage.getContent());
}

@Order(6)
Expand Down Expand Up @@ -668,4 +667,54 @@ void renameWithIndexingWaiting(String strategy, TestUtils setup, TestReference t
setup.rest().delete(otherReference);
}
}

@Order(9)
@Test
void renameWithRelativeLinks(TestUtils testUtils, TestReference testReference, TestConfiguration testConfiguration)
throws Exception
{
testUtils.rest().savePage(testReference, "[[Alice]]\n[[Bob]]\n[[Eve]]", "Test relative links");
SpaceReference rootSpaceReference = testReference.getLastSpaceReference();
SpaceReference aliceSpace = new SpaceReference("Alice", rootSpaceReference);
testUtils.rest().savePage(new DocumentReference("WebHome", aliceSpace), "Alice page", "Alice");
SpaceReference bobSpace = new SpaceReference("Bob", rootSpaceReference);
testUtils.rest().savePage(new DocumentReference("WebHome", bobSpace), "[[Alice]]", "Bob");

// Wait for an empty queue here to ensure that the deleted page has been removed from the index and links
// won't be updated just because the page is still in the index.
new SolrTestUtils(testUtils, testConfiguration.getServletEngine()).waitEmptyQueue();

ViewPage viewPage = testUtils.gotoPage(testReference);
RenamePage rename = viewPage.rename();
rename.getDocumentPicker().setName(rootSpaceReference.getName() + "Foo");
CopyOrRenameOrDeleteStatusPage statusPage = rename.clickRenameButton().waitUntilFinished();
assertEquals("Done.", statusPage.getInfoMessage());

WikiEditPage wikiEditPage = statusPage.gotoNewPage().editWiki();
assertEquals("[[Alice]]\n[[Bob]]\n[[Eve]]", wikiEditPage.getContent());

SpaceReference newRootSpace =
new SpaceReference(rootSpaceReference.getName() + "Foo", rootSpaceReference.getParent());
SpaceReference newBobSpace = new SpaceReference("Bob", newRootSpace);
wikiEditPage = WikiEditPage.gotoPage(new DocumentReference("WebHome", newBobSpace));
assertEquals("[[Alice]]", wikiEditPage.getContent());

SpaceReference newAliceSpace = new SpaceReference("Alice", newRootSpace);
DocumentReference newAliceReference = new DocumentReference("WebHome", newAliceSpace);
viewPage = testUtils.gotoPage(newAliceReference);
rename = viewPage.rename();
rename.getDocumentPicker().setName("Alice2");
statusPage = rename.clickRenameButton().waitUntilFinished();
assertEquals("Done.", statusPage.getInfoMessage());

SpaceReference alice2Space = new SpaceReference("Alice2", newRootSpace);
DocumentReference alice2Reference = new DocumentReference("WebHome", alice2Space);
wikiEditPage = WikiEditPage.gotoPage(new DocumentReference("WebHome", newRootSpace));
String serializedAlice2Reference = testUtils.serializeLocalReference(alice2Reference);
assertEquals(String.format("[[%s]]%n[[Bob]]%n[[Eve]]", serializedAlice2Reference), wikiEditPage.getContent());

// FIXME: ideally this one should be refactored too, however it's not a regression.
//wikiEditPage = WikiEditPage.gotoPage(new DocumentReference("WebHome", newBobSpace));
//assertEquals(String.format("[[%s]]", serializedAlice2Reference), wikiEditPage.getContent());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@
import org.xwiki.extension.job.internal.UninstallJob;
import org.xwiki.extension.repository.CoreExtensionRepository;
import org.xwiki.job.Job;
import org.xwiki.job.JobContext;
import org.xwiki.job.JobException;
import org.xwiki.job.JobExecutor;
import org.xwiki.job.annotation.Serializable;
Expand Down Expand Up @@ -151,6 +152,7 @@
import org.xwiki.query.QueryFilter;
import org.xwiki.refactoring.batch.BatchOperationExecutor;
import org.xwiki.refactoring.internal.ReferenceUpdater;
import org.xwiki.refactoring.internal.job.AbstractCopyOrMoveJob;
import org.xwiki.rendering.async.AsyncContext;
import org.xwiki.rendering.block.Block;
import org.xwiki.rendering.internal.transformation.MutableRenderingContext;
Expand Down Expand Up @@ -4968,9 +4970,19 @@ private void updateLinksForRename(XWikiDocument sourceDoc, DocumentReference new
List<DocumentReference> backlinkDocumentReferences, List<DocumentReference> childDocumentReferences,
XWikiContext context) throws XWikiException
{
// FIXME: that's ugly we should use something else.
JobContext jobContext = Utils.getComponent(JobContext.class);
Job currentJob = jobContext.getCurrentJob();

Map<EntityReference, EntityReference> updatedReferences =
Map.of(sourceDoc.getDocumentReference(), newDocumentReference);
if (currentJob instanceof AbstractCopyOrMoveJob) {
updatedReferences = ((AbstractCopyOrMoveJob) currentJob).getSelectedEntities();
}
// Step 1: Refactor the relative links contained in the document to make sure they are relative to the new
// document's location.
getReferenceUpdater().update(newDocumentReference, sourceDoc.getDocumentReference(), newDocumentReference);
getReferenceUpdater().update(newDocumentReference, sourceDoc.getDocumentReference(), newDocumentReference,
updatedReferences);

// Step 2: For each child document, update its parent reference.
if (childDocumentReferences != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
oldDocumentReference, newDocumentReference, false,
Map.of(oldDocumentReference, newDocumentReference));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1093,13 +1093,14 @@ void atomicRename() throws Exception
new DocumentReference(targetReference, Locale.GERMAN), xWikiContext);

// Test links
verify(this.referenceUpdater).update(targetReference, sourceReference, targetReference);
verify(this.referenceUpdater).update(targetReference, sourceReference, targetReference,
Map.of(sourceReference, targetReference));
verify(this.referenceRenamer).renameReferences(doc1.getXDOM(), reference1, sourceReference,
targetReference, false);
targetReference, false, Map.of(sourceReference, targetReference));
verify(this.referenceRenamer).renameReferences(doc2.getXDOM(), reference2, sourceReference,
targetReference, false);
targetReference, false, Map.of(sourceReference, targetReference));
verify(this.referenceRenamer).renameReferences(doc3.getXDOM(), reference3, sourceReference,
targetReference, false);
targetReference, false, Map.of(sourceReference, targetReference));

assertTrue(this.xwiki
.getDocument(new DocumentReference(DOCWIKI, DOCSPACE, DOCNAME), this.oldcore.getXWikiContext()).isNew());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,6 @@
"http://www.puppycrawl.com/dtds/suppressions_1_0.dtd">

<suppressions>
<!-- Fan out of 22 on a max of 20 -->
<suppress checks="ClassFanOutComplexity" files="DefaultDocumentSplitter.java"/>
</suppressions>
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,14 @@
*/
package org.xwiki.refactoring;

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;

/**
* Allow to replace references during rename/move refactoring operations.
Expand All @@ -46,6 +50,27 @@ public interface ReferenceRenamer
boolean renameReferences(Block block, DocumentReference currentDocumentReference, DocumentReference oldTarget,
DocumentReference newTarget, boolean relative);

/**
* Change references of the given block so that the references pointing to the old target points to the new target.
*
* @param block the {@link Block} to modify
* @param currentDocumentReference the current document reference
* @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 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,
Map<EntityReference, EntityReference> updatedEntities)
{
return renameReferences(block, currentDocumentReference, oldTarget, newTarget, relative);
}

/**
* Change references of the given block so that the references pointing to the old target points to the new target.
*
Expand All @@ -63,4 +88,25 @@ default boolean renameReferences(Block block, DocumentReference currentDocumentR
return false;
}

/**
* Change references of the given block so that the references pointing to the old target points to the new target.
*
* @param block the {@link Block} to modify
* @param currentDocumentReference the current document reference
* @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 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,
Map<EntityReference, EntityReference> updatedEntities)
{
return renameReferences(block, currentDocumentReference, oldTarget, newTarget, relative);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
*/
package org.xwiki.refactoring.internal;

import java.util.Map;

import org.xwiki.component.annotation.Role;
import org.xwiki.model.reference.DocumentReference;
import org.xwiki.model.reference.EntityReference;
Expand All @@ -32,6 +34,20 @@
@Role
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 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, Map<EntityReference, EntityReference> updatedEntities)
{
update(documentReference, oldTargetReference, newTargetReference);
}

/**
* @param documentReference the reference of the document in which to update the references
* @param oldTargetReference the previous reference of the renamed entity
Expand Down
Loading
Loading