From c8f33eb3860e809b404b38244c9ba0f41f872861 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Thu, 10 Oct 2024 17:47:25 +0200 Subject: [PATCH 01/12] XWIKI-12987: Relative links are made absolute or even broken after moving a page WIP The idea of this work is to: 1. Provide a way to access all documents that are moved as part of a move job 2. Use that information when performing a call to ReferenceRenamer to define if a relative untyped link should be handled or not On top of it, the idea is also to check if the doc exists in case of refactoring of a link to avoid refactoring unexisting relative links. One problem is remaining about relative link pointing to sibling pages (e.g. the link to Alice in Bob page in the ticket): we rely apparently to an old mechanism for backward compatibility reason for this to work in the UI, we might need same thing in the check, or to decide to ignore that UC. I started to add an integration tests but for some reason it's not passing, though it seemed to be working locally for the scenario described in the ticket (except for the link in Bob page). --- .../flamingo/test/docker/RenamePageIT.java | 20 ++++++ .../src/main/java/com/xpn/xwiki/XWiki.java | 13 +++- .../internal/render/DefaultOldRendering.java | 2 +- .../test/java/com/xpn/xwiki/XWikiTest.java | 7 ++- .../checkstyle/checkstyle-suppressions.xml | 1 + .../xwiki/refactoring/ReferenceRenamer.java | 7 ++- .../internal/ReferenceUpdater.java | 9 +++ .../internal/job/AbstractCopyOrMoveJob.java | 31 +++++++++- .../job/AbstractEntityJobWithChecks.java | 11 ++++ .../refactoring/internal/job/CopyJob.java | 42 +++++-------- .../refactoring/internal/job/MoveJob.java | 32 +++------- .../listener/BackLinkUpdaterListener.java | 15 +++-- .../internal/DefaultMacroRefactoring.java | 10 +-- .../internal/DefaultReferenceRenamer.java | 18 +++--- .../internal/DefaultReferenceUpdater.java | 25 +++++--- .../internal/ResourceReferenceRenamer.java | 61 +++++++++++++++---- .../internal/DefaultMacroRefactoringTest.java | 16 ++--- .../internal/DefaultReferenceUpdaterTest.java | 12 ++-- .../ResourceReferenceRenamerTest.java | 6 +- .../rendering/macro/MacroRefactoring.java | 6 +- 20 files changed, 236 insertions(+), 108 deletions(-) diff --git a/xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-test/xwiki-platform-flamingo-skin-test-docker/src/test/it/org/xwiki/flamingo/test/docker/RenamePageIT.java b/xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-test/xwiki-platform-flamingo-skin-test-docker/src/test/it/org/xwiki/flamingo/test/docker/RenamePageIT.java index 9bc9027b5980..1e8f28a0cb76 100644 --- a/xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-test/xwiki-platform-flamingo-skin-test-docker/src/test/it/org/xwiki/flamingo/test/docker/RenamePageIT.java +++ b/xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-test/xwiki-platform-flamingo-skin-test-docker/src/test/it/org/xwiki/flamingo/test/docker/RenamePageIT.java @@ -668,4 +668,24 @@ void renameWithIndexingWaiting(String strategy, TestUtils setup, TestReference t setup.rest().delete(otherReference); } } + + @Order(9) + @Test + void renameWithRelativeLinks(TestUtils testUtils, TestReference testReference) + { + testUtils.createPage(testReference, "[[Alice]]\n[[Bob]]\n[[Eve]]", "Test relative links"); + testUtils.createPage(new DocumentReference("Alice", testReference.getLastSpaceReference()), "Alice page", + "Alice"); + testUtils.createPage(new DocumentReference("Bob", testReference.getLastSpaceReference()), "[[Alice]]", + "Alice"); + + ViewPage viewPage = testUtils.gotoPage(testReference); + RenamePage rename = viewPage.rename(); + rename.getDocumentPicker().setName("Foo"); + CopyOrRenameOrDeleteStatusPage statusPage = rename.clickRenameButton().waitUntilFinished(); + assertEquals("Done.", statusPage.getInfoMessage()); + + WikiEditPage wikiEditPage = statusPage.gotoNewPage().editWiki(); + assertEquals("[[Alice]]\n[[Bob]]\n[[Eve]]", wikiEditPage.getContent()); + } } diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/XWiki.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/XWiki.java index 68bb8e850a12..f2099f7dbe47 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/XWiki.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/XWiki.java @@ -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; @@ -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; @@ -4968,9 +4970,18 @@ private void updateLinksForRename(XWikiDocument sourceDoc, DocumentReference new List backlinkDocumentReferences, List childDocumentReferences, XWikiContext context) throws XWikiException { + // FIXME: that's ugly we should use something else. + JobContext jobContext = Utils.getComponent(JobContext.class); + Job currentJob = jobContext.getCurrentJob(); + + Set updatedReferences = Set.of(); + 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) { diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/render/DefaultOldRendering.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/render/DefaultOldRendering.java index e6bbc153efb7..bc0d1f0d81ac 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/render/DefaultOldRendering.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/render/DefaultOldRendering.java @@ -117,7 +117,7 @@ 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, Set.of()); } @Override diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/XWikiTest.java b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/XWikiTest.java index edfe0a9e4ce0..e5364f34e28e 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/XWikiTest.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/XWikiTest.java @@ -29,6 +29,7 @@ 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; @@ -1095,11 +1096,11 @@ void atomicRename() throws Exception // Test links verify(this.referenceUpdater).update(targetReference, sourceReference, targetReference); verify(this.referenceRenamer).renameReferences(doc1.getXDOM(), reference1, sourceReference, - targetReference, false); + targetReference, false, Set.of()); verify(this.referenceRenamer).renameReferences(doc2.getXDOM(), reference2, sourceReference, - targetReference, false); + targetReference, false, Set.of()); verify(this.referenceRenamer).renameReferences(doc3.getXDOM(), reference3, sourceReference, - targetReference, false); + targetReference, false, Set.of()); assertTrue(this.xwiki .getDocument(new DocumentReference(DOCWIKI, DOCSPACE, DOCNAME), this.oldcore.getXWikiContext()).isNew()); diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/checkstyle/checkstyle-suppressions.xml b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/checkstyle/checkstyle-suppressions.xml index 6a89b7c3cf34..99562d1512f8 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/checkstyle/checkstyle-suppressions.xml +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/checkstyle/checkstyle-suppressions.xml @@ -25,5 +25,6 @@ "http://www.puppycrawl.com/dtds/suppressions_1_0.dtd"> + diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/ReferenceRenamer.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/ReferenceRenamer.java index f7786998d5e1..8691a6d4e423 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/ReferenceRenamer.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/ReferenceRenamer.java @@ -19,6 +19,8 @@ */ package org.xwiki.refactoring; +import java.util.Set; + import org.xwiki.component.annotation.Role; import org.xwiki.model.reference.AttachmentReference; import org.xwiki.model.reference.DocumentReference; @@ -44,7 +46,7 @@ public interface ReferenceRenamer * @return {@code true} if the given {@link Block} was modified */ boolean renameReferences(Block block, DocumentReference currentDocumentReference, DocumentReference oldTarget, - DocumentReference newTarget, boolean relative); + DocumentReference newTarget, boolean relative, Set updatedDocuments); /** * Change references of the given block so that the references pointing to the old target points to the new target. @@ -58,7 +60,8 @@ boolean renameReferences(Block block, DocumentReference currentDocumentReference * @since 14.2RC1 */ default boolean renameReferences(Block block, DocumentReference currentDocumentReference, - AttachmentReference oldTarget, AttachmentReference newTarget, boolean relative) + AttachmentReference oldTarget, AttachmentReference newTarget, boolean relative, + Set updatedDocuments) { return false; } diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/ReferenceUpdater.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/ReferenceUpdater.java index a1e7b8aa32e8..631509edef80 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/ReferenceUpdater.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/ReferenceUpdater.java @@ -19,6 +19,8 @@ */ package org.xwiki.refactoring.internal; +import java.util.Set; + import org.xwiki.component.annotation.Role; import org.xwiki.model.reference.DocumentReference; import org.xwiki.model.reference.EntityReference; @@ -36,7 +38,14 @@ 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. */ + default void update(DocumentReference documentReference, EntityReference oldTargetReference, + EntityReference newTargetReference, Set updatedEntities) + { + update(documentReference, oldTargetReference, newTargetReference); + } + void update(DocumentReference documentReference, EntityReference oldTargetReference, EntityReference newTargetReference); } diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractCopyOrMoveJob.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractCopyOrMoveJob.java index aea21744bec5..3ede00ed5f09 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractCopyOrMoveJob.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractCopyOrMoveJob.java @@ -26,6 +26,9 @@ import org.xwiki.model.reference.DocumentReference; import org.xwiki.model.reference.EntityReference; import org.xwiki.model.reference.SpaceReference; +import org.xwiki.observation.event.BeginFoldEvent; +import org.xwiki.observation.event.CancelableEvent; +import org.xwiki.observation.event.EndFoldEvent; import org.xwiki.refactoring.internal.event.AbstractEntityCopyOrRenameEvent; import org.xwiki.refactoring.job.AbstractCopyOrMoveRequest; import org.xwiki.refactoring.job.EntityJobStatus; @@ -53,8 +56,29 @@ public abstract class AbstractCopyOrMoveJob @Override protected void runInternal() throws Exception { - if (this.request.getDestination() != null) { - super.runInternal(); + this.progressManager.pushLevelProgress(3, this); + + try { + this.progressManager.startStep(this); + BeginFoldEvent beginEvent = getBeginEvent(); + this.observationManager.notify(beginEvent, this, this.getRequest()); + if (((CancelableEvent) beginEvent).isCanceled()) { + return; + } + this.progressManager.endStep(this); + + this.progressManager.startStep(this); + if (this.request.getDestination() != null) { + super.runInternal(); + } + this.progressManager.endStep(this); + + this.progressManager.startStep(this); + EndFoldEvent endEvent = getEndEvent(); + this.observationManager.notify(endEvent, this, this.getRequest()); + this.progressManager.endStep(this); + } finally { + this.progressManager.popLevelProgress(this); } } @@ -313,4 +337,7 @@ protected EntityReference getCommonParent() * @return {@code true} if the operation worked well. */ protected abstract boolean atomicOperation(DocumentReference source, DocumentReference target); + protected abstract B getBeginEvent(); + + protected abstract EndFoldEvent getEndEvent(); } diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractEntityJobWithChecks.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractEntityJobWithChecks.java index e80728f66225..d900a5756c9d 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractEntityJobWithChecks.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractEntityJobWithChecks.java @@ -21,8 +21,11 @@ import java.util.Collection; import java.util.HashMap; +import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; import org.xwiki.bridge.event.DocumentsDeletingEvent; import org.xwiki.model.reference.DocumentReference; @@ -165,4 +168,12 @@ protected EntitySelection getConcernedEntitiesEntitySelection(EntityReference re } return entitySelection; } + + public Set getSelectedEntities() + { + return this.concernedEntities.values().stream() + .filter(EntitySelection::isSelected) + .map(EntitySelection::getEntityReference) + .collect(Collectors.toSet()); + } } diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/CopyJob.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/CopyJob.java index 749a2fa3704b..f922cf75b6b0 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/CopyJob.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/CopyJob.java @@ -23,6 +23,9 @@ import org.xwiki.component.annotation.Component; import org.xwiki.model.reference.DocumentReference; +import org.xwiki.observation.event.BeginFoldEvent; +import org.xwiki.observation.event.CancelableEvent; +import org.xwiki.observation.event.EndFoldEvent; import org.xwiki.refactoring.event.DocumentCopiedEvent; import org.xwiki.refactoring.event.DocumentCopyingEvent; import org.xwiki.refactoring.event.EntitiesCopiedEvent; @@ -46,33 +49,6 @@ public String getType() return RefactoringJobs.COPY; } - @Override - protected void runInternal() throws Exception - { - this.progressManager.pushLevelProgress(3, this); - - try { - this.progressManager.startStep(this); - EntitiesCopyingEvent entitiesCopyingEvent = new EntitiesCopyingEvent(); - this.observationManager.notify(entitiesCopyingEvent, this, this.getRequest()); - if (entitiesCopyingEvent.isCanceled()) { - return; - } - this.progressManager.endStep(this); - - this.progressManager.startStep(this); - super.runInternal(); - this.progressManager.endStep(this); - - this.progressManager.startStep(this); - EntitiesCopiedEvent entitiesCopiedEvent = new EntitiesCopiedEvent(); - this.observationManager.notify(entitiesCopiedEvent, this, this.getRequest()); - this.progressManager.endStep(this); - } finally { - this.progressManager.popLevelProgress(this); - } - } - @Override protected void performRefactoring(DocumentReference sourceReference, DocumentReference targetReference) { @@ -90,4 +66,16 @@ protected boolean atomicOperation(DocumentReference source, DocumentReference ta { return this.modelBridge.copy(source, target); } + + @Override + protected T getBeginEvent() + { + return (T) new EntitiesCopyingEvent(); + } + + @Override + protected EndFoldEvent getEndEvent() + { + return new EntitiesCopiedEvent(); + } } diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/MoveJob.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/MoveJob.java index 3e3234473632..faaad4533cd3 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/MoveJob.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/MoveJob.java @@ -26,6 +26,9 @@ import org.xwiki.component.annotation.Component; import org.xwiki.model.reference.DocumentReference; import org.xwiki.model.reference.EntityReference; +import org.xwiki.observation.event.BeginFoldEvent; +import org.xwiki.observation.event.CancelableEvent; +import org.xwiki.observation.event.EndFoldEvent; import org.xwiki.refactoring.event.DocumentRenamedEvent; import org.xwiki.refactoring.event.DocumentRenamingEvent; import org.xwiki.refactoring.event.EntitiesRenamedEvent; @@ -51,30 +54,15 @@ public String getType() } @Override - protected void runInternal() throws Exception + protected T getBeginEvent() { - this.progressManager.pushLevelProgress(3, this); - - try { - this.progressManager.startStep(this); - EntitiesRenamingEvent entitiesRenamingEvent = new EntitiesRenamingEvent(); - this.observationManager.notify(entitiesRenamingEvent, this, this.getRequest()); - if (entitiesRenamingEvent.isCanceled()) { - return; - } - this.progressManager.endStep(this); - - this.progressManager.startStep(this); - super.runInternal(); - this.progressManager.endStep(this); + return (T) new EntitiesRenamingEvent(); + } - this.progressManager.startStep(this); - EntitiesRenamedEvent entitiesRenamedEvent = new EntitiesRenamedEvent(); - this.observationManager.notify(entitiesRenamedEvent, this, this.getRequest()); - this.progressManager.endStep(this); - } finally { - this.progressManager.popLevelProgress(this); - } + @Override + protected EndFoldEvent getEndEvent() + { + return new EntitiesRenamedEvent(); } @Override diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/listener/BackLinkUpdaterListener.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/listener/BackLinkUpdaterListener.java index 9a8183df61d0..0dd9cf2041ff 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/listener/BackLinkUpdaterListener.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/listener/BackLinkUpdaterListener.java @@ -41,9 +41,12 @@ import org.xwiki.refactoring.event.DocumentRenamedEvent; import org.xwiki.refactoring.internal.ModelBridge; import org.xwiki.refactoring.internal.ReferenceUpdater; +import org.xwiki.refactoring.internal.job.AbstractEntityJobWithChecks; import org.xwiki.refactoring.internal.job.DeleteJob; import org.xwiki.refactoring.internal.job.MoveJob; import org.xwiki.refactoring.job.DeleteRequest; +import org.xwiki.refactoring.job.EntityJobStatus; +import org.xwiki.refactoring.job.EntityRequest; import org.xwiki.refactoring.job.MoveRequest; import org.xwiki.security.authorization.ContextualAuthorizationManager; import org.xwiki.security.authorization.Right; @@ -117,7 +120,7 @@ private void maybeUpdateLinksAfterDelete(Event event) throws RefactoringExceptio DocumentReference newTarget = request.getNewBacklinkTargets().get(deletedEvent.getDocumentReference()); if (request.isUpdateLinks() && newTarget != null) { - updateBackLinks(deletedEvent.getDocumentReference(), newTarget, canEdit); + updateBackLinks(deletedEvent.getDocumentReference(), newTarget, canEdit, Set.of()); } } @@ -127,20 +130,24 @@ private void maybeUpdateLinksAfterRename(Event event, Object source, Object data Predicate canEdit = entityReference -> this.authorization.hasAccess(Right.EDIT, entityReference); + Set movedDocuments = Set.of(); if (source instanceof MoveJob) { MoveRequest request = (MoveRequest) data; updateLinks = request.isUpdateLinks(); // Check access rights taking into account the move request. canEdit = entityReference -> ((MoveJob) source).hasAccess(Right.EDIT, entityReference); + movedDocuments = ((AbstractEntityJobWithChecks) source).getSelectedEntities(); } if (updateLinks) { DocumentRenamedEvent renameEvent = (DocumentRenamedEvent) event; - updateBackLinks(renameEvent.getSourceReference(), renameEvent.getTargetReference(), canEdit); + updateBackLinks(renameEvent.getSourceReference(), renameEvent.getTargetReference(), canEdit, + movedDocuments); } } - private void updateBackLinks(DocumentReference source, DocumentReference target, Predicate canEdit) + private void updateBackLinks(DocumentReference source, DocumentReference target, + Predicate canEdit, Set movedDocuments) throws RefactoringException { this.logger.info("Updating the back-links for document [{}].", source); @@ -157,7 +164,7 @@ private void updateBackLinks(DocumentReference source, DocumentReference target, for (DocumentReference backlinkDocumentReference : backlinkDocumentReferences) { this.progressManager.startStep(this); if (canEdit.test(backlinkDocumentReference)) { - this.updater.update(backlinkDocumentReference, source, target); + this.updater.update(backlinkDocumentReference, source, target, movedDocuments); } this.progressManager.endStep(this); } diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultMacroRefactoring.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultMacroRefactoring.java index 55b432892732..d77906fd4282 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultMacroRefactoring.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultMacroRefactoring.java @@ -149,22 +149,24 @@ private XDOM parseMacro(MacroBlock macroBlock) throws MacroRefactoringException @Override public Optional replaceReference(MacroBlock macroBlock, DocumentReference currentDocumentReference, - DocumentReference sourceReference, DocumentReference targetReference, boolean relative) + DocumentReference sourceReference, DocumentReference targetReference, boolean relative, + Set updatedDocuments) throws MacroRefactoringException { return innerReplaceReference(macroBlock, xdom -> this.referenceRenamerProvider.get().renameReferences(xdom, currentDocumentReference, - sourceReference, targetReference, relative)); + sourceReference, targetReference, relative, updatedDocuments)); } @Override public Optional replaceReference(MacroBlock macroBlock, DocumentReference currentDocumentReference, - AttachmentReference sourceReference, AttachmentReference targetReference, boolean relative) + AttachmentReference sourceReference, AttachmentReference targetReference, boolean relative, + Set updatedDocuments) throws MacroRefactoringException { return innerReplaceReference(macroBlock, xdom -> this.referenceRenamerProvider.get().renameReferences(xdom, currentDocumentReference, - sourceReference, targetReference, relative)); + sourceReference, targetReference, relative, updatedDocuments)); } private Optional innerReplaceReference(MacroBlock macroBlock, Predicate lambda) diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultReferenceRenamer.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultReferenceRenamer.java index e90a8cf83e1c..d66a0ee52e34 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultReferenceRenamer.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultReferenceRenamer.java @@ -114,26 +114,28 @@ private interface RenameResourceLambda @Override public boolean renameReferences(Block block, DocumentReference currentDocumentReference, - DocumentReference oldTarget, DocumentReference newTarget, boolean relative) + DocumentReference oldTarget, DocumentReference newTarget, boolean relative, + Set updatedDocuments) { return innerRenameReferences(block, currentDocumentReference, oldTarget, newTarget, SUPPORTED_RESOURCE_TYPES_FOR_DOCUMENTS, (MacroRefactoring macroRefactoring, MacroBlock macroBlock) -> macroRefactoring.replaceReference(macroBlock, - currentDocumentReference, oldTarget, newTarget, relative), - reference -> this.resourceReferenceRenamer.updateResourceReference(reference, oldTarget, newTarget, - currentDocumentReference, relative)); + currentDocumentReference, oldTarget, newTarget, relative, updatedDocuments), + reference -> this.resourceReferenceRenamer.updateResourceReference(reference, oldTarget, + newTarget, currentDocumentReference, relative, updatedDocuments)); } @Override public boolean renameReferences(Block block, DocumentReference currentDocumentReference, - AttachmentReference oldTarget, AttachmentReference newTarget, boolean relative) + AttachmentReference oldTarget, AttachmentReference newTarget, boolean relative, + Set updatedDocuments) { return innerRenameReferences(block, currentDocumentReference, oldTarget, newTarget, SUPPORTED_RESOURCE_TYPES_FOR_ATTACHMENTS, (MacroRefactoring macroRefactoring, MacroBlock macroBlock) -> macroRefactoring.replaceReference(macroBlock, - currentDocumentReference, oldTarget, newTarget, relative), - reference -> this.resourceReferenceRenamer.updateResourceReference(reference, oldTarget, newTarget, - currentDocumentReference, relative)); + currentDocumentReference, oldTarget, newTarget, relative, updatedDocuments), + reference -> this.resourceReferenceRenamer.updateResourceReference(reference, + oldTarget, newTarget, currentDocumentReference, relative, updatedDocuments)); } private boolean innerRenameReferences(Block block, DocumentReference currentDocumentReference, diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultReferenceUpdater.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultReferenceUpdater.java index f737972f332c..a62c14038fe2 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultReferenceUpdater.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultReferenceUpdater.java @@ -21,6 +21,7 @@ import java.util.List; import java.util.Locale; +import java.util.Set; import javax.inject.Inject; import javax.inject.Named; @@ -265,17 +266,20 @@ private void renameLinks(XWikiDocument document, EntityReference oldTarget, Enti } private void renameLinks(DocumentReference documentReference, DocumentReference oldLinkTarget, - DocumentReference newLinkTarget, boolean relative) + DocumentReference newLinkTarget, boolean relative, Set updatedDocuments) { internalRenameLinks(documentReference, oldLinkTarget, newLinkTarget, relative, (xdom, currentDocumentReference, - r) -> this.renamer.renameReferences(xdom, currentDocumentReference, oldLinkTarget, newLinkTarget, r)); + r) -> this.renamer.renameReferences(xdom, currentDocumentReference, oldLinkTarget, newLinkTarget, r, + updatedDocuments)); } private void renameLinks(DocumentReference documentReference, AttachmentReference oldLinkTarget, - AttachmentReference newLinkTarget, boolean relative) + AttachmentReference newLinkTarget, boolean relative, Set updatedDocuments) { internalRenameLinks(documentReference, oldLinkTarget, newLinkTarget, relative, (xdom, currentDocumentReference, - r) -> this.renamer.renameReferences(xdom, currentDocumentReference, oldLinkTarget, newLinkTarget, r)); + r) -> + this.renamer.renameReferences(xdom, currentDocumentReference, oldLinkTarget, newLinkTarget, r, + updatedDocuments)); } private void internalRenameLinks(DocumentReference documentReference, EntityReference oldLinkTarget, @@ -331,7 +335,7 @@ private AttachmentReference toAttachmentReference(EntityReference entityReferenc @Override public void update(DocumentReference documentReference, EntityReference oldTargetReference, - EntityReference newTargetReference) + EntityReference newTargetReference, Set updatedDocuments) { // If the current document is the moved entity the links should be serialized relative to it boolean relative = newTargetReference.equals(documentReference); @@ -344,10 +348,17 @@ public void update(DocumentReference documentReference, EntityReference oldTarge // Only support documents and attachments targets if (oldTargetReference.getType() == EntityType.ATTACHMENT) { renameLinks(documentReference, toAttachmentReference(oldTargetReference), - toAttachmentReference(newTargetReference), relative); + toAttachmentReference(newTargetReference), relative, updatedDocuments); } else if (oldTargetReference.getType() == EntityType.DOCUMENT) { renameLinks(documentReference, toDocumentReference(oldTargetReference), - toDocumentReference(newTargetReference), relative); + toDocumentReference(newTargetReference), relative, updatedDocuments); } } + + @Override + public void update(DocumentReference documentReference, EntityReference oldTargetReference, + EntityReference newTargetReference) + { + update(documentReference, oldTargetReference, newTargetReference, Set.of()); + } } diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java index 28db4a72b893..d43a61b52b88 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java @@ -19,10 +19,15 @@ */ package org.xwiki.refactoring.internal; +import java.util.Map; +import java.util.Set; + import javax.inject.Inject; import javax.inject.Named; +import javax.inject.Provider; import javax.inject.Singleton; +import org.slf4j.Logger; import org.xwiki.component.annotation.Component; import org.xwiki.model.EntityType; import org.xwiki.model.reference.AttachmentReference; @@ -35,6 +40,9 @@ import org.xwiki.rendering.listener.reference.ResourceReference; import org.xwiki.rendering.listener.reference.ResourceType; +import com.xpn.xwiki.XWikiContext; +import com.xpn.xwiki.XWikiException; + import static com.xpn.xwiki.XWiki.DEFAULT_SPACE_HOMEPAGE; /** @@ -60,6 +68,12 @@ public class ResourceReferenceRenamer @Inject private PageReferenceResolver defaultReferencePageReferenceResolver; + @Inject + private Provider contextProvider; + + @Inject + private Logger logger; + /** * Update the given document reference so that if it targets the old document reference, the new document reference * is used instead. @@ -73,12 +87,13 @@ public class ResourceReferenceRenamer */ public boolean updateResourceReference(ResourceReference resourceReference, DocumentReference oldReference, DocumentReference newReference, DocumentReference currentDocumentReference, - boolean relative) + boolean relative, Set movedDocuments) { return (relative) - ? this.updateRelativeResourceReference(resourceReference, oldReference, newReference) + ? this.updateRelativeResourceReference(resourceReference, oldReference, newReference, + currentDocumentReference, movedDocuments) : this.updateAbsoluteResourceReference(resourceReference, oldReference, newReference, - currentDocumentReference); + currentDocumentReference, movedDocuments); } /** @@ -94,16 +109,18 @@ public boolean updateResourceReference(ResourceReference resourceReference, */ public boolean updateResourceReference(ResourceReference resourceReference, AttachmentReference oldReference, AttachmentReference newReference, DocumentReference currentDocumentReference, - boolean relative) + boolean relative, Set movedDocuments) { return (relative) - ? this.updateRelativeResourceReference(resourceReference, oldReference, newReference) + ? this.updateRelativeResourceReference(resourceReference, oldReference, newReference, + currentDocumentReference, movedDocuments) : this.updateAbsoluteResourceReference(resourceReference, oldReference, newReference, - currentDocumentReference); + currentDocumentReference, movedDocuments); } private boolean updateAbsoluteResourceReference(ResourceReference resourceReference, - DocumentReference oldReference, DocumentReference newReference, DocumentReference currentDocumentReference) + DocumentReference oldReference, DocumentReference newReference, DocumentReference currentDocumentReference, + Set movedDocuments) { boolean result = false; @@ -116,8 +133,23 @@ private boolean updateAbsoluteResourceReference(ResourceReference resourceRefere EntityReference newTargetReference = newReference; ResourceType newResourceType = resourceReference.getType(); + XWikiContext context = contextProvider.get(); + boolean docExists = false; + try { + docExists = context.getWiki().exists(linkTargetDocumentReference, context); + } catch (XWikiException e) { + this.logger.error("Error while checking if [{}] exists for link refactoring.", linkTargetDocumentReference, + e); + } + + boolean shouldBeUpdated = + linkTargetDocumentReference.equals(oldReference) + && !(!resourceReference.isTyped() && movedDocuments.contains(linkEntityReference) + && movedDocuments.contains(currentDocumentReference)) + && docExists; + // If the link targets the old (renamed) document reference, we must update it. - if (linkTargetDocumentReference.equals(oldReference)) { + if (shouldBeUpdated) { // If the link was resolved to a space... if (EntityType.SPACE.equals(linkEntityReference.getType())) { if (DEFAULT_SPACE_HOMEPAGE.equals(newReference.getName())) { @@ -155,7 +187,8 @@ private boolean updateAbsoluteResourceReference(ResourceReference resourceRefere } private boolean updateRelativeResourceReference(ResourceReference resourceReference, - T oldReference, T newReference) + T oldReference, T newReference, DocumentReference currentDocumentReference, + Set movedDocuments) { boolean result = false; @@ -171,8 +204,13 @@ private boolean updateRelativeResourceReference(Reso EntityReference oldLinkReference = this.entityReferenceResolver.resolve(resourceReference, null, oldReference); + boolean shouldBeUpdated = + !linkEntityReference.equals(oldLinkReference) + && !(!resourceReference.isTyped() && movedDocuments.contains(oldLinkReference) + && movedDocuments.contains(currentDocumentReference)); + // If the new and old link references don`t match, then we must update the relative link. - if (!linkEntityReference.equals(oldLinkReference)) { + if (shouldBeUpdated) { // Serialize the old (original) link relative to the new document's location, in compact form. String serializedLinkReference = this.compactEntityReferenceSerializer.serialize(oldLinkReference, newReference); @@ -183,7 +221,8 @@ private boolean updateRelativeResourceReference(Reso } private boolean updateAbsoluteResourceReference(ResourceReference resourceReference, - AttachmentReference oldReference, AttachmentReference newReference, DocumentReference currentDocumentReference) + AttachmentReference oldReference, AttachmentReference newReference, DocumentReference currentDocumentReference, + Set movedDocuments) { boolean result = false; diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultMacroRefactoringTest.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultMacroRefactoringTest.java index b76c9316c057..a9ed5dc66fa9 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultMacroRefactoringTest.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultMacroRefactoringTest.java @@ -22,6 +22,7 @@ import java.util.Collections; import java.util.Map; import java.util.Optional; +import java.util.Set; import javax.inject.Provider; @@ -152,11 +153,12 @@ void replaceReferenceUnparseableMacro() throws MacroRefactoringException }); assertEquals(Optional.empty(), this.macroRefactoring.replaceReference(this.macroBlock, this.currentDocumentReference, - this.sourceReference, this.targetReference, true)); + this.sourceReference, this.targetReference, true, Set.of())); assertEquals(Optional.empty(), this.macroRefactoring.replaceReference(this.macroBlock, this.currentDocumentReference, - this.sourceReference, this.targetReference, false)); - verify(this.referenceRenamer, never()).renameReferences(any(), any(), any(DocumentReference.class), any(), anyBoolean()); + this.sourceReference, this.targetReference, false, Set.of())); + verify(this.referenceRenamer, never()).renameReferences(any(), any(), any(DocumentReference.class), any(), + anyBoolean(), any()); } @Test @@ -186,10 +188,10 @@ void replaceReferenceNotUpdated() throws Exception return xdom; }); when(this.referenceRenamer.renameReferences(xdom, this.currentDocumentReference, this.sourceReference, - this.targetReference, true)).thenReturn(false); + this.targetReference, true, Set.of())).thenReturn(false); assertEquals(Optional.empty(), this.macroRefactoring.replaceReference(this.macroBlock, this.currentDocumentReference, this.sourceReference, - this.targetReference, true)); + this.targetReference, true, Set.of())); verify(this.blockRenderer, never()).render(any(Block.class), any()); } @@ -211,7 +213,7 @@ void replaceReferenceUpdated() throws Exception return xdom; }); when(this.referenceRenamer.renameReferences(xdom, this.currentDocumentReference, this.sourceReference, - this.targetReference, true)).thenReturn(true); + this.targetReference, true, Set.of())).thenReturn(true); String expectedContent = "the expected content"; doAnswer(invocationOnMock -> { WikiPrinter printer = invocationOnMock.getArgument(1); @@ -232,7 +234,7 @@ void replaceReferenceUpdated() throws Exception MacroBlock expectedBlock = new MacroBlock(this.macroId, parameters, expectedContent, false); assertEquals(Optional.of(expectedBlock), this.macroRefactoring.replaceReference(this.macroBlock, this.currentDocumentReference, this.sourceReference, - this.targetReference, true)); + this.targetReference, true, Set.of())); } @Test diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultReferenceUpdaterTest.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultReferenceUpdaterTest.java index 8383667d1f9d..263c36ee6961 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultReferenceUpdaterTest.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultReferenceUpdaterTest.java @@ -26,6 +26,7 @@ import java.util.Locale; import java.util.Map; import java.util.Optional; +import java.util.Set; import javax.inject.Named; import javax.inject.Provider; @@ -617,17 +618,18 @@ void updateFromMacros(MockitoComponentManager componentManager) throws Exception componentManager.registerMockComponent(MacroRefactoring.class, "include"); MacroRefactoring displayMacroRefactoring = componentManager.registerMockComponent(MacroRefactoring.class, "display"); - when(displayMacroRefactoring.replaceReference(any(), any(), any(DocumentReference.class), any(), anyBoolean())) + when(displayMacroRefactoring.replaceReference(any(), any(), any(DocumentReference.class), any(), anyBoolean() + , any())) .thenReturn(Optional.of(displayMacroBlock)); when(this.documentAccessBridge.getDocumentInstance(documentReference)).thenReturn(document); updater.update(documentReference, oldLinkTarget, newLinkTarget); verify(includeMacroRefactoring).replaceReference(includeMacroBlock1, documentReference, oldLinkTarget, - newLinkTarget, false); + newLinkTarget, false, Set.of()); verify(includeMacroRefactoring).replaceReference(includeMacroBlock2, documentReference, oldLinkTarget, - newLinkTarget, false); + newLinkTarget, false, Set.of()); verify(displayMacroRefactoring).replaceReference(displayMacroBlock, documentReference, oldLinkTarget, - newLinkTarget, false); + newLinkTarget, false, Set.of()); verify(this.mutableRenderingContext, times(3)).push(any(), any(), eq(Syntax.XWIKI_2_1), any(), anyBoolean(), any()); verify(this.mutableRenderingContext, times(3)).pop(); @@ -711,7 +713,7 @@ void updateFromLinksAndMacros(MockitoComponentManager componentManager) throws E updater.update(documentReference, oldLinkTarget, newLinkTarget); verify(includeMacroRefactoring).replaceReference(includeMacroBlock, documentReference, oldLinkTarget, - newLinkTarget, false); + newLinkTarget, false, Set.of()); assertEquals("X.Y", documentLinkBlock.getReference().getReference()); assertEquals(ResourceType.DOCUMENT, documentLinkBlock.getReference().getType()); verifyDocumentSave(document, "Renamed back-links.", false, false); diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/ResourceReferenceRenamerTest.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/ResourceReferenceRenamerTest.java index f3611227a829..c900af5ddfb5 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/ResourceReferenceRenamerTest.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/ResourceReferenceRenamerTest.java @@ -19,6 +19,8 @@ */ package org.xwiki.refactoring.internal; +import java.util.Set; + import javax.inject.Named; import org.junit.jupiter.api.Test; @@ -81,7 +83,7 @@ void updateResourceReferenceRelative() assertTrue(this.renamer.updateResourceReference(resourceReference, oldReference, newReference, - new DocumentReference("xwiki", "Space", "Page"), true)); + new DocumentReference("xwiki", "Space", "Page"), true, Set.of())); verify(this.compactEntityReferenceSerializer).serialize(oldReference, newReference); } @@ -103,7 +105,7 @@ void updateResourceReferenceNotRelative() assertTrue(this.renamer.updateResourceReference(resourceReference, oldReference, newReference, currentDocumentReference, - false)); + false, Set.of())); assertEquals(new AttachmentResourceReference("xwiki:Space.Page.file2.txt"), resourceReference); } } diff --git a/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-xwiki/src/main/java/org/xwiki/rendering/macro/MacroRefactoring.java b/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-xwiki/src/main/java/org/xwiki/rendering/macro/MacroRefactoring.java index bc24dc6bd3ea..70010c8faa8a 100644 --- a/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-xwiki/src/main/java/org/xwiki/rendering/macro/MacroRefactoring.java +++ b/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-xwiki/src/main/java/org/xwiki/rendering/macro/MacroRefactoring.java @@ -54,7 +54,8 @@ public interface MacroRefactoring * @throws MacroRefactoringException in case of problem to parse or render the macro content. */ Optional replaceReference(MacroBlock macroBlock, DocumentReference currentDocumentReference, - DocumentReference sourceReference, DocumentReference targetReference, boolean relative) + DocumentReference sourceReference, DocumentReference targetReference, boolean relative, + Set updatedDocuments) throws MacroRefactoringException; /** @@ -74,7 +75,8 @@ Optional replaceReference(MacroBlock macroBlock, DocumentReference c * @since 14.2RC1 */ Optional replaceReference(MacroBlock macroBlock, DocumentReference currentDocumentReference, - AttachmentReference sourceReference, AttachmentReference targetReference, boolean relative) + AttachmentReference sourceReference, AttachmentReference targetReference, boolean relative, + Set updatedDocuments) throws MacroRefactoringException; /** From 456e447a435d0a7994d9e39374de4437a163a713 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Wed, 23 Oct 2024 18:22:56 +0200 Subject: [PATCH 02/12] XWIKI-12987: Relative links are made absolute or even broken after moving a page * Fix integration test setup * Fix some signatures * Work on the conditions for performing link update: WIP --- .../flamingo/test/docker/RenamePageIT.java | 37 ++++++++++++------- .../internal/ResourceReferenceRenamer.java | 19 ++++++++-- .../include/IncludeMacroRefactoring.java | 15 +++++--- .../include/IncludeMacroRefactoringTest.java | 23 ++++++------ 4 files changed, 61 insertions(+), 33 deletions(-) diff --git a/xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-test/xwiki-platform-flamingo-skin-test-docker/src/test/it/org/xwiki/flamingo/test/docker/RenamePageIT.java b/xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-test/xwiki-platform-flamingo-skin-test-docker/src/test/it/org/xwiki/flamingo/test/docker/RenamePageIT.java index 1e8f28a0cb76..c85828591cfe 100644 --- a/xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-test/xwiki-platform-flamingo-skin-test-docker/src/test/it/org/xwiki/flamingo/test/docker/RenamePageIT.java +++ b/xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-test/xwiki-platform-flamingo-skin-test-docker/src/test/it/org/xwiki/flamingo/test/docker/RenamePageIT.java @@ -366,18 +366,15 @@ 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(sourcePageReference1, "Some content to be linked. number 1"); + setup.createPage(sourcePageReference2, "Some content to be linked in macro. number 2"); + 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"); 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.createPage(sourcePageReference5, "A page to be included. number 5"); 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" @@ -511,10 +508,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) @@ -671,21 +668,35 @@ void renameWithIndexingWaiting(String strategy, TestUtils setup, TestReference t @Order(9) @Test - void renameWithRelativeLinks(TestUtils testUtils, TestReference testReference) + void renameWithRelativeLinks(TestUtils testUtils, TestReference testReference, TestConfiguration testConfiguration) + throws Exception { testUtils.createPage(testReference, "[[Alice]]\n[[Bob]]\n[[Eve]]", "Test relative links"); - testUtils.createPage(new DocumentReference("Alice", testReference.getLastSpaceReference()), "Alice page", + SpaceReference rootSpaceReference = testReference.getLastSpaceReference(); + SpaceReference aliceSpace = new SpaceReference("Alice", rootSpaceReference); + testUtils.createPage(new DocumentReference("WebHome", aliceSpace), "Alice page", "Alice"); - testUtils.createPage(new DocumentReference("Bob", testReference.getLastSpaceReference()), "[[Alice]]", + SpaceReference bobSpace = new SpaceReference("Bob", rootSpaceReference); + testUtils.createPage(new DocumentReference("WebHome", bobSpace), "[[Alice]]", "Alice"); + // 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("Foo"); + 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()); } } diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java index d43a61b52b88..e2bddd4fce6d 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java @@ -146,7 +146,7 @@ private boolean updateAbsoluteResourceReference(ResourceReference resourceRefere linkTargetDocumentReference.equals(oldReference) && !(!resourceReference.isTyped() && movedDocuments.contains(linkEntityReference) && movedDocuments.contains(currentDocumentReference)) - && docExists; + && (docExists || movedDocuments.contains(linkEntityReference)); // If the link targets the old (renamed) document reference, we must update it. if (shouldBeUpdated) { @@ -195,7 +195,9 @@ private boolean updateRelativeResourceReference(Reso EntityReference linkEntityReference = this.entityReferenceResolver.resolve(resourceReference, null, newReference); - if (newReference.equals(linkEntityReference.extractReference(EntityType.DOCUMENT))) { + DocumentReference documentReference = + (DocumentReference) linkEntityReference.extractReference(EntityType.DOCUMENT); + if (newReference.equals(documentReference)) { // If the link is relative to the containing document we don't modify it return false; } @@ -204,10 +206,19 @@ private boolean updateRelativeResourceReference(Reso EntityReference oldLinkReference = this.entityReferenceResolver.resolve(resourceReference, null, oldReference); + XWikiContext context = contextProvider.get(); + boolean docExists = false; + try { + docExists = context.getWiki().exists(documentReference, context); + } catch (XWikiException e) { + this.logger.error("Error while checking if [{}] exists for link refactoring.", documentReference, + e); + } + boolean shouldBeUpdated = !linkEntityReference.equals(oldLinkReference) - && !(!resourceReference.isTyped() && movedDocuments.contains(oldLinkReference) - && movedDocuments.contains(currentDocumentReference)); + && !(!resourceReference.isTyped() && movedDocuments.contains(documentReference)) + && (docExists || movedDocuments.contains(documentReference)); // If the new and old link references don`t match, then we must update the relative link. if (shouldBeUpdated) { diff --git a/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 b/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 index 4bd9640e42b7..68b34209c83c 100644 --- a/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 +++ b/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 @@ -85,22 +85,27 @@ public class IncludeMacroRefactoring implements MacroRefactoring @Override public Optional replaceReference(MacroBlock macroBlock, DocumentReference currentDocumentReference, - DocumentReference sourceReference, DocumentReference targetReference, boolean relative) + DocumentReference sourceReference, DocumentReference targetReference, boolean relative, + Set updatedDocuments) throws MacroRefactoringException { - return getMacroBlock(macroBlock, currentDocumentReference, sourceReference, targetReference, relative); + return getMacroBlock(macroBlock, currentDocumentReference, sourceReference, targetReference, relative, + updatedDocuments); } @Override public Optional replaceReference(MacroBlock macroBlock, DocumentReference currentDocumentReference, - AttachmentReference sourceReference, AttachmentReference targetReference, boolean relative) + AttachmentReference sourceReference, AttachmentReference targetReference, boolean relative, + Set updatedDocuments) throws MacroRefactoringException { - return getMacroBlock(macroBlock, currentDocumentReference, sourceReference, targetReference, relative); + return getMacroBlock(macroBlock, currentDocumentReference, sourceReference, targetReference, relative, + updatedDocuments); } private Optional getMacroBlock(MacroBlock macroBlock, - DocumentReference currentDocumentReference, T sourceReference, T targetReference, boolean relative) + DocumentReference currentDocumentReference, T sourceReference, T targetReference, boolean relative, + Set updatedDocuments) throws MacroRefactoringException { Optional result; diff --git a/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-macros/xwiki-platform-rendering-macro-include/src/test/java/org/xwiki/rendering/internal/macro/include/IncludeMacroRefactoringTest.java b/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-macros/xwiki-platform-rendering-macro-include/src/test/java/org/xwiki/rendering/internal/macro/include/IncludeMacroRefactoringTest.java index a8751b6cbec0..9de3b607d9dc 100644 --- a/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-macros/xwiki-platform-rendering-macro-include/src/test/java/org/xwiki/rendering/internal/macro/include/IncludeMacroRefactoringTest.java +++ b/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-macros/xwiki-platform-rendering-macro-include/src/test/java/org/xwiki/rendering/internal/macro/include/IncludeMacroRefactoringTest.java @@ -110,7 +110,7 @@ void replaceDocumentReferenceWhenNoReferenceParameterSet() throws Exception { MacroBlock block = new MacroBlock("include", Collections.emptyMap(), false); assertEquals(Optional.empty(), this.includeMacroRefactoring.replaceReference(block, null, null, - (DocumentReference) null, false)); + (DocumentReference) null, false, Set.of())); } @Test @@ -119,7 +119,7 @@ void replaceDocumentReferenceWhenEmptyReferenceParameterSet() throws Exception MacroBlock block = new MacroBlock("include", Collections.emptyMap(), false); block.setParameter("reference", ""); assertEquals(Optional.empty(), this.includeMacroRefactoring.replaceReference(block, null, null, - (DocumentReference) null, false)); + (DocumentReference) null, false, Set.of())); } @Test @@ -128,7 +128,7 @@ void replaceDocumentReferenceWhenEmptyPageParameterSet() throws Exception MacroBlock block = new MacroBlock("include", Collections.emptyMap(), false); block.setParameter("page", ""); assertEquals(Optional.empty(), this.includeMacroRefactoring.replaceReference(block, null, null, - (DocumentReference) null, false)); + (DocumentReference) null, false, Set.of())); } @Test @@ -153,7 +153,7 @@ void replaceDocumentReferenceWhenIncludedReferenceIsRenamedUsingReferenceParamet new EntityReference("sourcewiki", EntityType.WIKI))).thenReturn("sourcespace.foo"); Optional result = this.includeMacroRefactoring.replaceReference(block, currentReference, - sourceReference, targetReference, false); + sourceReference, targetReference, false, Set.of()); assertFalse(result.isEmpty()); assertEquals("targetwiki:targetspace.targetfoo", result.get().getParameter("reference")); } @@ -189,7 +189,7 @@ void replaceDocumentReferenceWhenIncludingDocumentRenamedUsingReferenceParameter new EntityReference("sourcewiki", EntityType.WIKI))).thenReturn("sourcespace.foo"); Optional result = this.includeMacroRefactoring.replaceReference(block, null, - sourceReference, targetReference, false); + sourceReference, targetReference, false, Set.of()); assertFalse(result.isEmpty()); assertEquals("sourcewiki:sourcespace.foo", result.get().getParameter("reference")); } @@ -225,7 +225,7 @@ void replaceDocumentReferenceWhenIncludingDocumentRenamedUsingPageParameterAndNo new EntityReference("sourcewiki", EntityType.WIKI))).thenReturn("sourcespace/foo"); Optional result = this.includeMacroRefactoring.replaceReference(block, null, - sourceReference, targetReference, false); + sourceReference, targetReference, false, Set.of()); assertFalse(result.isEmpty()); assertEquals("sourcewiki:sourcespace.foo", result.get().getParameter("page")); } @@ -254,7 +254,7 @@ void replaceDocumentReferenceWhenIncludedReferenceIsRenamedUsingPageParameterAnd new EntityReference("sourcewiki", EntityType.WIKI))).thenReturn("sourcespace/foo"); Optional result = this.includeMacroRefactoring.replaceReference(block, currentReference, - sourceReference, targetReference, false); + sourceReference, targetReference, false, Set.of()); assertFalse(result.isEmpty()); assertEquals("targetwiki:targetspace.targetfoo", result.get().getParameter("page")); } @@ -282,7 +282,7 @@ void replaceDocumentReferenceWhenIncludingDocumentIsRenamedToSameReference() thr new EntityReference("sourcewiki", EntityType.WIKI))).thenReturn("sourcespace.foo"); Optional result = this.includeMacroRefactoring.replaceReference(block, null, sourceReference, - targetReference, false); + targetReference, false, Set.of()); assertTrue(result.isEmpty()); } @@ -312,7 +312,7 @@ void replaceAttachmentReferenceWhenIncludedAttachmentReferenceIsRenamedUsingRefe new EntityReference("sourcewiki", EntityType.WIKI))).thenReturn("sourcespace.foo"); Optional result = this.includeMacroRefactoring.replaceReference(block, currentReference, - sourceReference, targetAttachmentReference, false); + sourceReference, targetAttachmentReference, false, Set.of()); assertFalse(result.isEmpty()); assertEquals("targetwiki:targetspace.targetfoo@targetfile", result.get().getParameter("reference")); } @@ -355,7 +355,7 @@ void replaceDocumentReferenceWhenIncludingDocumentIsRenamedUsingAttachmentRefere new EntityReference("sourcewiki", EntityType.WIKI))).thenReturn("sourcespace.foopage@foofile"); Optional result = this.includeMacroRefactoring.replaceReference(block, null, - sourceReference, targetReference, false); + sourceReference, targetReference, false, Set.of()); assertFalse(result.isEmpty()); assertEquals("sourcewiki:sourcespace.foopage@foofile", result.get().getParameter("reference")); } @@ -369,7 +369,8 @@ void replaceDocumentReferenceWhenParametersPopulateError() throws Exception block.setParameter("type", "invalid"); Throwable exception = assertThrows(MacroRefactoringException.class, - () -> this.includeMacroRefactoring.replaceReference(block, null, null, (DocumentReference) null, false)); + () -> this.includeMacroRefactoring.replaceReference(block, null, null, (DocumentReference) null, false, + Set.of())); assertEquals("There's one or several invalid parameters for an [include] macro with parameters " + "[{type=invalid}]", exception.getMessage()); } From 60c169af60c38aa9c4c4b807ea99732dcb54bf6f Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Thu, 24 Oct 2024 10:38:41 +0200 Subject: [PATCH 03/12] XWIKI-12987: Relative links are made absolute or even broken after moving a page * Fix conditions to make all RenamePageIT passing * WIP: need to double check that some conditions are not redundant and double check side effects --- .../internal/ResourceReferenceRenamer.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java index e2bddd4fce6d..91b4b9e13908 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java @@ -127,6 +127,8 @@ private boolean updateAbsoluteResourceReference(ResourceReference resourceRefere // FIXME: the root cause of XWIKI-18634 is related to this call. EntityReference linkEntityReference = this.entityReferenceResolver.resolve(resourceReference, null, currentDocumentReference); + DocumentReference documentReference = + (DocumentReference) linkEntityReference.extractReference(EntityType.DOCUMENT); DocumentReference linkTargetDocumentReference = this.defaultReferenceDocumentReferenceResolver.resolve(linkEntityReference); @@ -144,9 +146,9 @@ private boolean updateAbsoluteResourceReference(ResourceReference resourceRefere boolean shouldBeUpdated = linkTargetDocumentReference.equals(oldReference) - && !(!resourceReference.isTyped() && movedDocuments.contains(linkEntityReference) + && !(!resourceReference.isTyped() && movedDocuments.contains(documentReference) && movedDocuments.contains(currentDocumentReference)) - && (docExists || movedDocuments.contains(linkEntityReference)); + && (docExists || movedDocuments.contains(documentReference)); // If the link targets the old (renamed) document reference, we must update it. if (shouldBeUpdated) { @@ -194,18 +196,18 @@ private boolean updateRelativeResourceReference(Reso EntityReference linkEntityReference = this.entityReferenceResolver.resolve(resourceReference, null, newReference); + // current link, use the old document's reference to fill in blanks. + EntityReference oldLinkReference = + this.entityReferenceResolver.resolve(resourceReference, null, oldReference); DocumentReference documentReference = - (DocumentReference) linkEntityReference.extractReference(EntityType.DOCUMENT); - if (newReference.equals(documentReference)) { + (DocumentReference) oldLinkReference.extractReference(EntityType.DOCUMENT); + if (newReference.equals(documentReference) + || (newReference.hasParent(documentReference) && movedDocuments.contains(documentReference))) { // If the link is relative to the containing document we don't modify it return false; } - // current link, use the old document's reference to fill in blanks. - EntityReference oldLinkReference = - this.entityReferenceResolver.resolve(resourceReference, null, oldReference); - XWikiContext context = contextProvider.get(); boolean docExists = false; try { From c8be6ae0a21f77ae8734cba6a1507608c7eb2f0f Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Thu, 24 Oct 2024 16:13:57 +0200 Subject: [PATCH 04/12] XWIKI-12987: Relative links are made absolute or even broken after moving a page * Simplify a bit the conditions in ResourceReferenceRenamer and ensure all unit tests are passing in refactoring module --- .../listener/BackLinkUpdaterListenerTest.java | 20 +++---- .../internal/ResourceReferenceRenamer.java | 56 ++++++++++--------- .../internal/DefaultReferenceUpdaterTest.java | 3 +- .../ResourceReferenceRenamerTest.java | 23 ++++++++ 4 files changed, 64 insertions(+), 38 deletions(-) diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/test/java/org/xwiki/refactoring/internal/listener/BackLinkUpdaterListenerTest.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/test/java/org/xwiki/refactoring/internal/listener/BackLinkUpdaterListenerTest.java index 8c57b8f41edf..0198ed1ca28a 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/test/java/org/xwiki/refactoring/internal/listener/BackLinkUpdaterListenerTest.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/test/java/org/xwiki/refactoring/internal/listener/BackLinkUpdaterListenerTest.java @@ -129,8 +129,8 @@ void onDocumentRenamedWithUpdateLinksOnFarm() this.listener.onEvent(documentRenamedEvent, renameJob, renameRequest); - verify(this.updater).update(carolReference, aliceReference, bobReference); - verify(this.updater).update(denisReference, aliceReference, bobReference); + verify(this.updater).update(carolReference, aliceReference, bobReference, Set.of()); + verify(this.updater).update(denisReference, aliceReference, bobReference, Set.of()); assertEquals("Updating the back-links for document [foo:Users.Alice].", logCapture.getMessage(0)); } @@ -147,7 +147,7 @@ void onDocumentRenamedWithUpdateLinksOnFarmAndNoEditRight() this.listener.onEvent(documentRenamedEvent, renameJob, renameRequest); - verify(this.updater).update(carolReference, aliceReference, bobReference); + verify(this.updater).update(carolReference, aliceReference, bobReference, Set.of()); verify(this.updater, never()).update(eq(denisReference), any(DocumentReference.class), any()); assertEquals("Updating the back-links for document [foo:Users.Alice].", logCapture.getMessage(0)); @@ -163,7 +163,7 @@ void onDocumentRenamedWithUpdateLinksOnWiki() this.listener.onEvent(documentRenamedEvent, renameJob, renameRequest); - verify(this.updater).update(carolReference, aliceReference, bobReference); + verify(this.updater).update(carolReference, aliceReference, bobReference, Set.of()); verify(this.updater, never()).update(eq(denisReference), any(DocumentReference.class), any()); assertEquals("Updating the back-links for document [foo:Users.Alice].", logCapture.getMessage(0)); @@ -189,8 +189,8 @@ void onDocumentRenamedWithoutRenameJob() this.listener.onEvent(documentRenamedEvent, null, null); - verify(this.updater).update(carolReference, aliceReference, bobReference); - verify(this.updater).update(denisReference, aliceReference, bobReference); + verify(this.updater).update(carolReference, aliceReference, bobReference, Set.of()); + verify(this.updater).update(denisReference, aliceReference, bobReference, Set.of()); assertEquals("Updating the back-links for document [foo:Users.Alice].", logCapture.getMessage(0)); } @@ -205,7 +205,7 @@ void onDocumentRenamedWithoutRenameJobAndNoEditRight() this.listener.onEvent(documentRenamedEvent, null, null); verify(this.updater, never()).update(eq(carolReference), any(DocumentReference.class), any()); - verify(this.updater).update(denisReference, aliceReference, bobReference); + verify(this.updater).update(denisReference, aliceReference, bobReference, Set.of()); assertEquals("Updating the back-links for document [foo:Users.Alice].", logCapture.getMessage(0)); } @@ -220,8 +220,8 @@ void onDocumentDeletedWithUpdateLinksOnFarm() this.listener.onEvent(documentDeletedEvent, null, null); - verify(this.updater).update(carolReference, aliceReference, bobReference); - verify(this.updater).update(denisReference, aliceReference, bobReference); + verify(this.updater).update(carolReference, aliceReference, bobReference, Set.of()); + verify(this.updater).update(denisReference, aliceReference, bobReference, Set.of()); assertEquals("Updating the back-links for document [foo:Users.Alice].", logCapture.getMessage(0)); } @@ -252,7 +252,7 @@ void onDocumentDeleteWithUpdateLinksOnFarmAndNoEditRight() this.listener.onEvent(documentDeletedEvent, null, null); - verify(this.updater).update(carolReference, aliceReference, bobReference); + verify(this.updater).update(carolReference, aliceReference, bobReference, Set.of()); verify(this.updater, never()).update(eq(denisReference), any(DocumentReference.class), any()); assertEquals("Updating the back-links for document [foo:Users.Alice].", this.logCapture.getMessage(0)); diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java index 91b4b9e13908..5fdd6b7d6036 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java @@ -19,7 +19,6 @@ */ package org.xwiki.refactoring.internal; -import java.util.Map; import java.util.Set; import javax.inject.Inject; @@ -87,13 +86,12 @@ public class ResourceReferenceRenamer */ public boolean updateResourceReference(ResourceReference resourceReference, DocumentReference oldReference, DocumentReference newReference, DocumentReference currentDocumentReference, - boolean relative, Set movedDocuments) + boolean relative, Set movedReferences) { return (relative) - ? this.updateRelativeResourceReference(resourceReference, oldReference, newReference, - currentDocumentReference, movedDocuments) + ? this.updateRelativeResourceReference(resourceReference, oldReference, newReference, movedReferences) : this.updateAbsoluteResourceReference(resourceReference, oldReference, newReference, - currentDocumentReference, movedDocuments); + currentDocumentReference, movedReferences); } /** @@ -109,18 +107,17 @@ public boolean updateResourceReference(ResourceReference resourceReference, */ public boolean updateResourceReference(ResourceReference resourceReference, AttachmentReference oldReference, AttachmentReference newReference, DocumentReference currentDocumentReference, - boolean relative, Set movedDocuments) + boolean relative, Set movedReferences) { return (relative) - ? this.updateRelativeResourceReference(resourceReference, oldReference, newReference, - currentDocumentReference, movedDocuments) + ? this.updateRelativeResourceReference(resourceReference, oldReference, newReference, movedReferences) : this.updateAbsoluteResourceReference(resourceReference, oldReference, newReference, - currentDocumentReference, movedDocuments); + currentDocumentReference, movedReferences); } private boolean updateAbsoluteResourceReference(ResourceReference resourceReference, DocumentReference oldReference, DocumentReference newReference, DocumentReference currentDocumentReference, - Set movedDocuments) + Set movedDocuments) { boolean result = false; @@ -189,8 +186,7 @@ private boolean updateAbsoluteResourceReference(ResourceReference resourceRefere } private boolean updateRelativeResourceReference(ResourceReference resourceReference, - T oldReference, T newReference, DocumentReference currentDocumentReference, - Set movedDocuments) + T oldReference, T newReference, Set movedReferences) { boolean result = false; @@ -200,27 +196,33 @@ private boolean updateRelativeResourceReference(Reso EntityReference oldLinkReference = this.entityReferenceResolver.resolve(resourceReference, null, oldReference); - DocumentReference documentReference = - (DocumentReference) oldLinkReference.extractReference(EntityType.DOCUMENT); - if (newReference.equals(documentReference) - || (newReference.hasParent(documentReference) && movedDocuments.contains(documentReference))) { - // If the link is relative to the containing document we don't modify it + 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; } - XWikiContext context = contextProvider.get(); boolean docExists = false; - try { - docExists = context.getWiki().exists(documentReference, context); - } catch (XWikiException e) { - this.logger.error("Error while checking if [{}] exists for link refactoring.", documentReference, - e); + EntityType entityType = linkEntityReference.getType(); + if (entityType == EntityType.DOCUMENT || entityType.getAllowedParents().contains(EntityType.DOCUMENT)) { + DocumentReference documentReference = + (DocumentReference) oldLinkReference.extractReference(EntityType.DOCUMENT); + XWikiContext context = contextProvider.get(); + try { + docExists = context.getWiki().exists(documentReference, context); + } catch (XWikiException e) { + this.logger.error("Error while checking if [{}] exists for link refactoring.", documentReference, + e); + } + } else { + docExists = true; } boolean shouldBeUpdated = - !linkEntityReference.equals(oldLinkReference) - && !(!resourceReference.isTyped() && movedDocuments.contains(documentReference)) - && (docExists || movedDocuments.contains(documentReference)); + !linkEntityReference.equals(oldLinkReference) && docExists; // If the new and old link references don`t match, then we must update the relative link. if (shouldBeUpdated) { @@ -235,7 +237,7 @@ private boolean updateRelativeResourceReference(Reso private boolean updateAbsoluteResourceReference(ResourceReference resourceReference, AttachmentReference oldReference, AttachmentReference newReference, DocumentReference currentDocumentReference, - Set movedDocuments) + Set movedReferences) { boolean result = false; diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultReferenceUpdaterTest.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultReferenceUpdaterTest.java index 263c36ee6961..c1c17791a3c3 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultReferenceUpdaterTest.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultReferenceUpdaterTest.java @@ -190,6 +190,7 @@ void beforeEach() throws Exception { XWiki xwiki = mock(XWiki.class); when(this.xcontext.getWiki()).thenReturn(xwiki); + when(xwiki.exists(any(DocumentReference.class), eq(this.xcontext))).thenReturn(true); when(this.xcontextProvider.get()).thenReturn(this.xcontext); when(this.componentManagerProvider.get()).thenReturn(this.componentManager); @@ -255,7 +256,7 @@ void updateRelativeLinks() throws Exception LinkBlock xobjectSpaceLinkBlock = new LinkBlock(Collections.emptyList(), xobjectSpaceLinkReference, false); ResourceReference xobjectImageReference = new AttachmentResourceReference("attachment.txt"); - ImageBlock xobjectImageBlock = new ImageBlock(imageReference, false); + ImageBlock xobjectImageBlock = new ImageBlock(xobjectImageReference, false); setTextarea(newDocument, new XDOM(Arrays.asList(xobjectDocLinkBlock, xobjectSpaceLinkBlock, xobjectImageBlock))); diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/ResourceReferenceRenamerTest.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/ResourceReferenceRenamerTest.java index c900af5ddfb5..bf9051af8955 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/ResourceReferenceRenamerTest.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/ResourceReferenceRenamerTest.java @@ -22,7 +22,9 @@ import java.util.Set; import javax.inject.Named; +import javax.inject.Provider; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.xwiki.model.reference.AttachmentReference; import org.xwiki.model.reference.DocumentReference; @@ -38,8 +40,15 @@ import org.xwiki.test.junit5.mockito.InjectMockComponents; import org.xwiki.test.junit5.mockito.MockComponent; +import com.xpn.xwiki.XWiki; +import com.xpn.xwiki.XWikiContext; +import com.xpn.xwiki.XWikiException; + import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -68,6 +77,20 @@ class ResourceReferenceRenamerTest @MockComponent private PageReferenceResolver defaultReferencePageReferenceResolver; + @MockComponent + private Provider contextProvider; + + @BeforeEach + void setup() throws XWikiException + { + XWikiContext context = mock(XWikiContext.class); + when(this.contextProvider.get()).thenReturn(context); + + XWiki xWiki = mock(XWiki.class); + when(context.getWiki()).thenReturn(xWiki); + when(xWiki.exists(any(DocumentReference.class), eq(context))).thenReturn(true); + } + @Test void updateResourceReferenceRelative() { From 4ff12e2ef2683368e79e378849b26cd04fbf7f57 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Fri, 25 Oct 2024 16:50:02 +0200 Subject: [PATCH 05/12] XWIKI-12987: Relative links are made absolute or even broken after moving a page * Fix checkstyle * WIP: try to find proper oracle for renaming absolute references, without success so far. --- .../test/java/com/xpn/xwiki/XWikiTest.java | 2 +- .../xwiki/refactoring/ReferenceRenamer.java | 46 ++++++++++++++- .../internal/ReferenceUpdater.java | 6 ++ .../job/AbstractEntityJobWithChecks.java | 5 +- .../listener/BackLinkUpdaterListener.java | 2 - .../internal/DefaultMacroRefactoring.java | 18 ++++++ .../internal/DefaultReferenceRenamer.java | 14 +++++ .../internal/ResourceReferenceRenamer.java | 47 ++++++++------- .../rendering/macro/MacroRefactoring.java | 59 ++++++++++++++++++- 9 files changed, 167 insertions(+), 32 deletions(-) diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/XWikiTest.java b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/XWikiTest.java index e5364f34e28e..4001bb8fb359 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/XWikiTest.java +++ b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/XWikiTest.java @@ -1094,7 +1094,7 @@ 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, Set.of()); verify(this.referenceRenamer).renameReferences(doc1.getXDOM(), reference1, sourceReference, targetReference, false, Set.of()); verify(this.referenceRenamer).renameReferences(doc2.getXDOM(), reference2, sourceReference, diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/ReferenceRenamer.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/ReferenceRenamer.java index 8691a6d4e423..48fd1ac93b09 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/ReferenceRenamer.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/ReferenceRenamer.java @@ -25,6 +25,7 @@ import org.xwiki.model.reference.AttachmentReference; import org.xwiki.model.reference.DocumentReference; import org.xwiki.rendering.block.Block; +import org.xwiki.stability.Unstable; /** * Allow to replace references during rename/move refactoring operations. @@ -46,7 +47,28 @@ public interface ReferenceRenamer * @return {@code true} if the given {@link Block} was modified */ boolean renameReferences(Block block, DocumentReference currentDocumentReference, DocumentReference oldTarget, - DocumentReference newTarget, boolean relative, Set updatedDocuments); + 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 updatedDocuments the list of documents that have been renamed in the same job: this list contains the + * old references before the rename + * @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 updatedDocuments) + { + 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. @@ -59,11 +81,31 @@ boolean renameReferences(Block block, DocumentReference currentDocumentReference * @return {@code true} if the given {@link Block} was modified * @since 14.2RC1 */ + default boolean renameReferences(Block block, DocumentReference currentDocumentReference, + AttachmentReference oldTarget, AttachmentReference newTarget, boolean relative) + { + 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 updatedDocuments the list of documents that have been renamed in the same job: this list contains the + * old references before the rename + * @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 updatedDocuments) { - return false; + return renameReferences(block, currentDocumentReference, oldTarget, newTarget, relative); } } diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/ReferenceUpdater.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/ReferenceUpdater.java index 631509edef80..f0a467ac926a 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/ReferenceUpdater.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/ReferenceUpdater.java @@ -39,6 +39,7 @@ public interface ReferenceUpdater * @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. + * @since 16.10.0RC1 */ default void update(DocumentReference documentReference, EntityReference oldTargetReference, EntityReference newTargetReference, Set updatedEntities) @@ -46,6 +47,11 @@ default void update(DocumentReference documentReference, EntityReference oldTarg 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 + * @param newTargetReference the new reference of the renamed entity + */ void update(DocumentReference documentReference, EntityReference oldTargetReference, EntityReference newTargetReference); } diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractEntityJobWithChecks.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractEntityJobWithChecks.java index d900a5756c9d..ecc442b5fd22 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractEntityJobWithChecks.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractEntityJobWithChecks.java @@ -21,7 +21,6 @@ import java.util.Collection; import java.util.HashMap; -import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; @@ -169,6 +168,10 @@ protected EntitySelection getConcernedEntitiesEntitySelection(EntityReference re return entitySelection; } + /** + * @return the list of references that have been selected to be refactored. + * @since 16.10.0RC1 + */ public Set getSelectedEntities() { return this.concernedEntities.values().stream() diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/listener/BackLinkUpdaterListener.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/listener/BackLinkUpdaterListener.java index 0dd9cf2041ff..f15091a5b1ce 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/listener/BackLinkUpdaterListener.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/listener/BackLinkUpdaterListener.java @@ -45,8 +45,6 @@ import org.xwiki.refactoring.internal.job.DeleteJob; import org.xwiki.refactoring.internal.job.MoveJob; import org.xwiki.refactoring.job.DeleteRequest; -import org.xwiki.refactoring.job.EntityJobStatus; -import org.xwiki.refactoring.job.EntityRequest; import org.xwiki.refactoring.job.MoveRequest; import org.xwiki.security.authorization.ContextualAuthorizationManager; import org.xwiki.security.authorization.Right; diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultMacroRefactoring.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultMacroRefactoring.java index d77906fd4282..68944f16e610 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultMacroRefactoring.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultMacroRefactoring.java @@ -169,6 +169,24 @@ public Optional replaceReference(MacroBlock macroBlock, DocumentRefe sourceReference, targetReference, relative, updatedDocuments)); } + @Override + public Optional replaceReference(MacroBlock macroBlock, DocumentReference currentDocumentReference, + DocumentReference sourceReference, DocumentReference targetReference, boolean relative) + throws MacroRefactoringException + { + return replaceReference(macroBlock, currentDocumentReference, sourceReference, targetReference, relative, + Set.of(sourceReference)); + } + + @Override + public Optional replaceReference(MacroBlock macroBlock, DocumentReference currentDocumentReference, + AttachmentReference sourceReference, AttachmentReference targetReference, boolean relative) + throws MacroRefactoringException + { + return replaceReference(macroBlock, currentDocumentReference, sourceReference, targetReference, relative, + Set.of()); + } + private Optional innerReplaceReference(MacroBlock macroBlock, Predicate lambda) throws MacroRefactoringException { diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultReferenceRenamer.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultReferenceRenamer.java index d66a0ee52e34..34cdec113928 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultReferenceRenamer.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultReferenceRenamer.java @@ -125,6 +125,13 @@ public boolean renameReferences(Block block, DocumentReference currentDocumentRe newTarget, currentDocumentReference, relative, updatedDocuments)); } + @Override + public boolean renameReferences(Block block, DocumentReference currentDocumentReference, + DocumentReference oldTarget, DocumentReference newTarget, boolean relative) + { + return renameReferences(block, currentDocumentReference, oldTarget, newTarget, relative, Set.of(oldTarget)); + } + @Override public boolean renameReferences(Block block, DocumentReference currentDocumentReference, AttachmentReference oldTarget, AttachmentReference newTarget, boolean relative, @@ -138,6 +145,13 @@ public boolean renameReferences(Block block, DocumentReference currentDocumentRe oldTarget, newTarget, currentDocumentReference, relative, updatedDocuments)); } + @Override + public boolean renameReferences(Block block, DocumentReference currentDocumentReference, + AttachmentReference oldTarget, AttachmentReference newTarget, boolean relative) + { + return renameReferences(block, currentDocumentReference, oldTarget, newTarget, relative, Set.of()); + } + private boolean innerRenameReferences(Block block, DocumentReference currentDocumentReference, EntityReference oldTarget, EntityReference newTarget, Set allowedResourceTypes, diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java index 5fdd6b7d6036..e495cca9dc94 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java @@ -82,6 +82,8 @@ public class ResourceReferenceRenamer * @param newReference the new document reference target * @param currentDocumentReference the current document where the resource reference is located * @param relative {@code true} if the reference should be kept relative to the current document + * @param movedReferences the list of references that have been moved in the same job: this list contains the old + * references before the move. * @return {@code true} if the resource reference has been updated */ public boolean updateResourceReference(ResourceReference resourceReference, @@ -94,6 +96,19 @@ public boolean updateResourceReference(ResourceReference resourceReference, currentDocumentReference, movedReferences); } + private boolean checkIfDocExists(DocumentReference documentReference) + { + XWikiContext context = contextProvider.get(); + boolean docExists = false; + try { + docExists = context.getWiki().exists(documentReference, context); + } catch (XWikiException e) { + this.logger.error("Error while checking if [{}] exists for link refactoring.", documentReference, + e); + } + return docExists; + } + /** * Update the given document reference so that if it targets the old attachment reference, the new attachment * reference is used instead. @@ -103,6 +118,8 @@ public boolean updateResourceReference(ResourceReference resourceReference, * @param newReference the new attachment reference target * @param currentDocumentReference the current document where the resource reference is located * @param relative {@code true} if the reference should be kept relative to the current document + * @param movedReferences the list of references that have been moved in the same job: this list contains the old + * references before the move. * @return {@code true} if the attachment reference has been updated */ public boolean updateResourceReference(ResourceReference resourceReference, @@ -124,30 +141,18 @@ private boolean updateAbsoluteResourceReference(ResourceReference resourceRefere // FIXME: the root cause of XWIKI-18634 is related to this call. EntityReference linkEntityReference = this.entityReferenceResolver.resolve(resourceReference, null, currentDocumentReference); - DocumentReference documentReference = - (DocumentReference) linkEntityReference.extractReference(EntityType.DOCUMENT); DocumentReference linkTargetDocumentReference = this.defaultReferenceDocumentReferenceResolver.resolve(linkEntityReference); EntityReference newTargetReference = newReference; ResourceType newResourceType = resourceReference.getType(); - XWikiContext context = contextProvider.get(); - boolean docExists = false; - try { - docExists = context.getWiki().exists(linkTargetDocumentReference, context); - } catch (XWikiException e) { - this.logger.error("Error while checking if [{}] exists for link refactoring.", linkTargetDocumentReference, - e); - } + // 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); - boolean shouldBeUpdated = - linkTargetDocumentReference.equals(oldReference) - && !(!resourceReference.isTyped() && movedDocuments.contains(documentReference) - && movedDocuments.contains(currentDocumentReference)) - && (docExists || movedDocuments.contains(documentReference)); - - // If the link targets the old (renamed) document reference, we must update it. if (shouldBeUpdated) { // If the link was resolved to a space... if (EntityType.SPACE.equals(linkEntityReference.getType())) { @@ -210,13 +215,7 @@ private boolean updateRelativeResourceReference(Reso if (entityType == EntityType.DOCUMENT || entityType.getAllowedParents().contains(EntityType.DOCUMENT)) { DocumentReference documentReference = (DocumentReference) oldLinkReference.extractReference(EntityType.DOCUMENT); - XWikiContext context = contextProvider.get(); - try { - docExists = context.getWiki().exists(documentReference, context); - } catch (XWikiException e) { - this.logger.error("Error while checking if [{}] exists for link refactoring.", documentReference, - e); - } + docExists = checkIfDocExists(documentReference); } else { docExists = true; } diff --git a/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-xwiki/src/main/java/org/xwiki/rendering/macro/MacroRefactoring.java b/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-xwiki/src/main/java/org/xwiki/rendering/macro/MacroRefactoring.java index 70010c8faa8a..8603a14e3eda 100644 --- a/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-xwiki/src/main/java/org/xwiki/rendering/macro/MacroRefactoring.java +++ b/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-xwiki/src/main/java/org/xwiki/rendering/macro/MacroRefactoring.java @@ -28,6 +28,7 @@ import org.xwiki.model.reference.DocumentReference; import org.xwiki.rendering.block.MacroBlock; import org.xwiki.rendering.listener.reference.ResourceReference; +import org.xwiki.stability.Unstable; /** * Component dedicated to perform refactoring of existing macros. @@ -54,9 +55,36 @@ public interface MacroRefactoring * @throws MacroRefactoringException in case of problem to parse or render the macro content. */ Optional replaceReference(MacroBlock macroBlock, DocumentReference currentDocumentReference, + DocumentReference sourceReference, DocumentReference targetReference, boolean relative) + throws MacroRefactoringException; + + /** + * Replace the given source reference by the given entity reference in the macro block. The method returns an + * optional containing a modified macro block if it needs to be updated, else it returns an empty optional. + * Depending on the macro implementation, this method might lead to parsing the macro content for finding the + * reference, or might just modify the macro parameters. + * + * @param macroBlock the macro block in which to replace the reference. + * @param currentDocumentReference the reference of the document in which the block is located + * @param sourceReference the reference to replace. + * @param targetReference the reference to use as replacement. + * @param relative if {@code true} indicate that the reference should be resolved 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 + * @return an optional containing the new macro block with proper information if it needs to be updated, else an + * empty optional. + * @throws MacroRefactoringException in case of problem to parse or render the macro content. + * @since 16.10.0RC1 + */ + @Unstable + default Optional replaceReference(MacroBlock macroBlock, DocumentReference currentDocumentReference, DocumentReference sourceReference, DocumentReference targetReference, boolean relative, Set updatedDocuments) - throws MacroRefactoringException; + throws MacroRefactoringException + { + return replaceReference(macroBlock, currentDocumentReference, sourceReference, targetReference, relative); + } /** * Replace the given source reference by the given entity reference in the macro block. The method returns an @@ -75,9 +103,36 @@ Optional replaceReference(MacroBlock macroBlock, DocumentReference c * @since 14.2RC1 */ Optional replaceReference(MacroBlock macroBlock, DocumentReference currentDocumentReference, + AttachmentReference sourceReference, AttachmentReference targetReference, boolean relative) + throws MacroRefactoringException; + + /** + * Replace the given source reference by the given entity reference in the macro block. The method returns an + * optional containing a modified macro block if it needs to be updated, else it returns an empty optional. + * Depending on the macro implementation, this method might lead to parsing the macro content for finding the + * reference, or might just modify the macro parameters. + * + * @param macroBlock the macro block in which to replace the reference. + * @param currentDocumentReference the reference of the document in which the block is located + * @param sourceReference the reference to replace. + * @param targetReference the reference to use as replacement + * @param relative if {@code true} indicate that the reference should be resolved 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 + * @return an optional containing the new macro block with proper information if it needs to be updated, else an + * empty optional. + * @throws MacroRefactoringException in case of problem to parse or render the macro content. + * @since 16.10.0RC1 + */ + @Unstable + default Optional replaceReference(MacroBlock macroBlock, DocumentReference currentDocumentReference, AttachmentReference sourceReference, AttachmentReference targetReference, boolean relative, Set updatedDocuments) - throws MacroRefactoringException; + throws MacroRefactoringException + { + return replaceReference(macroBlock, currentDocumentReference, sourceReference, targetReference, relative); + } /** * Extract references used in the macro so that they can be used for example for creating backlinks. From 1ac0e3a439abf2b7302422931298445b4ecaf1a6 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Mon, 28 Oct 2024 18:06:31 +0100 Subject: [PATCH 06/12] XWIKI-12987: Relative links are made absolute or even broken after moving 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) --- .../internal/ResourceReferenceRenamer.java | 43 +++++++++------- .../internal/DefaultReferenceUpdaterTest.java | 49 +++++++++++++++++-- .../ResourceReferenceRenamerTest.java | 5 ++ .../include/IncludeMacroRefactoring.java | 18 +++++++ 4 files changed, 93 insertions(+), 22 deletions(-) diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java index e495cca9dc94..0da7bc8b9b42 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java +++ b/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 movedDocuments) + Set 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 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 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 = diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultReferenceUpdaterTest.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultReferenceUpdaterTest.java index c1c17791a3c3..3ec23251e625 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultReferenceUpdaterTest.java +++ b/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,6 +349,8 @@ 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()); @@ -341,6 +358,8 @@ void updateRelativeLinksAcrossWikis() throws Exception .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,6 +527,8 @@ 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); @@ -509,6 +536,8 @@ void renameNonTerminalDocumentLinks() throws Exception // 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,6 +579,8 @@ 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); @@ -557,6 +588,8 @@ void renameNonTerminalToTerminalDocumentLinks() throws Exception // 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 includeParameters = new HashMap(); + Map 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); diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/ResourceReferenceRenamerTest.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/ResourceReferenceRenamerTest.java index bf9051af8955..231b0cf2cdbd 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/ResourceReferenceRenamerTest.java +++ b/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)) diff --git a/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 b/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 index 68b34209c83c..bc208d9eaf22 100644 --- a/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 +++ b/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 replaceReference(MacroBlock macroBlock, DocumentRefe updatedDocuments); } + @Override + public Optional 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 replaceReference(MacroBlock macroBlock, DocumentReference currentDocumentReference, + AttachmentReference sourceReference, AttachmentReference targetReference, boolean relative) + throws MacroRefactoringException + { + return getMacroBlock(macroBlock, currentDocumentReference, sourceReference, targetReference, relative, + Set.of()); + } + private Optional getMacroBlock(MacroBlock macroBlock, DocumentReference currentDocumentReference, T sourceReference, T targetReference, boolean relative, Set updatedDocuments) From efd557d5c29252854c0c51ab42771c6afd41d056 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Tue, 29 Oct 2024 17:54:24 +0100 Subject: [PATCH 07/12] XWIKI-12987: Relative links are made absolute or even broken after moving a page * Fix a regression and provide a test to cover it --- .../flamingo/test/docker/RenamePageIT.java | 18 ++++++++++++++++++ .../internal/ResourceReferenceRenamer.java | 8 +++++--- .../internal/DefaultModelBridgeTest.java | 12 +++++++++++- 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-test/xwiki-platform-flamingo-skin-test-docker/src/test/it/org/xwiki/flamingo/test/docker/RenamePageIT.java b/xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-test/xwiki-platform-flamingo-skin-test-docker/src/test/it/org/xwiki/flamingo/test/docker/RenamePageIT.java index c85828591cfe..1c7d563d0d67 100644 --- a/xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-test/xwiki-platform-flamingo-skin-test-docker/src/test/it/org/xwiki/flamingo/test/docker/RenamePageIT.java +++ b/xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-test/xwiki-platform-flamingo-skin-test-docker/src/test/it/org/xwiki/flamingo/test/docker/RenamePageIT.java @@ -698,5 +698,23 @@ void renameWithRelativeLinks(TestUtils testUtils, TestReference testReference, T 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()); } } diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java index 0da7bc8b9b42..79186bef8889 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java @@ -146,14 +146,16 @@ 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 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); + // We also update the link if it's not an absolute link but the current document is not part of the move job, + // as in this case there won't be any other call to perform the link refactoring. + boolean shouldBeUpdated = linkTargetDocumentReference.equals(oldReference) + && (absoluteResolvedReference.equals(linkEntityReference) + || !movedReferences.contains(currentDocumentReference)); if (shouldBeUpdated) { // If the link was resolved to a space... diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultModelBridgeTest.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultModelBridgeTest.java index f023515744a7..faf1e22b4aaa 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultModelBridgeTest.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultModelBridgeTest.java @@ -881,5 +881,15 @@ void permanentlyDeleteAllDocuments() throws Exception } } - + @Test + void rename() throws Exception + { + DocumentReference source = new DocumentReference("wiki", "space", "sourcePage"); + DocumentReference target = new DocumentReference("wiki", "space", "targetPage"); + + when(this.xwiki.renameDocument(source, target, true, List.of(), List.of(), this.xcontext)).thenReturn(true); + assertTrue(this.modelBridge.rename(source, target)); + + verify(this.xwiki).renameDocument(source, target, true, List.of(), List.of(), this.xcontext); + } } From 640972309e0fe39b7a4932e0a5901f91d94a96c1 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Thu, 31 Oct 2024 11:43:58 +0100 Subject: [PATCH 08/12] XWIKI-12987: Relative links are made absolute or even broken after moving a page * Provide subwiki integration tests * Minor improvment in RenamePageIT --- .../flamingo/test/docker/RenamePageIT.java | 2 +- .../it/org/xwiki/wiki/test/ui/SubWikiIT.java | 69 +++++++++++++++---- 2 files changed, 58 insertions(+), 13 deletions(-) diff --git a/xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-test/xwiki-platform-flamingo-skin-test-docker/src/test/it/org/xwiki/flamingo/test/docker/RenamePageIT.java b/xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-test/xwiki-platform-flamingo-skin-test-docker/src/test/it/org/xwiki/flamingo/test/docker/RenamePageIT.java index 1c7d563d0d67..84932ccdd8e8 100644 --- a/xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-test/xwiki-platform-flamingo-skin-test-docker/src/test/it/org/xwiki/flamingo/test/docker/RenamePageIT.java +++ b/xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-test/xwiki-platform-flamingo-skin-test-docker/src/test/it/org/xwiki/flamingo/test/docker/RenamePageIT.java @@ -711,7 +711,7 @@ void renameWithRelativeLinks(TestUtils testUtils, TestReference testReference, T 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()); + 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)); diff --git a/xwiki-platform-core/xwiki-platform-wiki/xwiki-platform-wiki-test/xwiki-platform-wiki-test-docker/src/test/it/org/xwiki/wiki/test/ui/SubWikiIT.java b/xwiki-platform-core/xwiki-platform-wiki/xwiki-platform-wiki-test/xwiki-platform-wiki-test-docker/src/test/it/org/xwiki/wiki/test/ui/SubWikiIT.java index de22e6bed7b8..4151e964ffab 100644 --- a/xwiki-platform-core/xwiki-platform-wiki/xwiki-platform-wiki-test/xwiki-platform-wiki-test-docker/src/test/it/org/xwiki/wiki/test/ui/SubWikiIT.java +++ b/xwiki-platform-core/xwiki-platform-wiki/xwiki-platform-wiki-test/xwiki-platform-wiki-test-docker/src/test/it/org/xwiki/wiki/test/ui/SubWikiIT.java @@ -114,15 +114,24 @@ void movePageToSubwiki(TestUtils setup, TestReference testReference, TestConfigu // Ensure that the page does not exist before the test. setup.rest().delete(mainWikiLinkPage); - - String space = testReference.getSpaceReferences() - .stream().map(SpaceReference::getName).collect(Collectors.joining(".")); - // The page that will be moved. - setup.createPage(testReference, "Some content", "My Page"); - - // For checking the link update. - setup.createPage(mainWikiLinkPage, String.format("[[%s.WebHome]]", space)); + // We'll check moving a hierarchy with relative links + setup.createPage(testReference, "[[Alice]]\n[[Bob]]\n[[Eve]]", "Test relative links"); + SpaceReference rootSpaceReference = testReference.getLastSpaceReference(); + SpaceReference aliceSpace = new SpaceReference("Alice", rootSpaceReference); + DocumentReference alicePage = new DocumentReference("WebHome", aliceSpace); + setup.createPage(alicePage, "Alice page", "Alice"); + SpaceReference bobSpace = new SpaceReference("Bob", rootSpaceReference); + DocumentReference bobPage = new DocumentReference("WebHome", bobSpace); + setup.createPage(bobPage, "[[Alice]]", + "Alice"); + + // For checking the link update in an external page. + setup.createPage(mainWikiLinkPage, + String.format("[[%s]]%n[[%s]]%n[[%s]]", + setup.serializeLocalReference(testReference), + setup.serializeLocalReference(alicePage), + setup.serializeLocalReference(bobPage))); // Wait for the Solr indexing to be completed before moving the page new SolrTestUtils(setup, testConfiguration.getServletEngine()).waitEmptyQueue(); @@ -137,17 +146,53 @@ void movePageToSubwiki(TestUtils setup, TestReference testReference, TestConfigu // Ensure the move has been properly done. assertEquals("Done.", renameStatusPage.getInfoMessage()); DocumentReference movedPageReference = testReference.setWikiReference(new WikiReference(SUBWIKI_NAME)); + SpaceReference newRootSpace = movedPageReference.getLastSpaceReference(); assertTrue(setup.rest().exists(movedPageReference)); viewPage = renameStatusPage.gotoNewPage(); assertEquals( String.format("/%s/%s/%s", SUBWIKI_NAME, testReference.getLastSpaceReference().extractFirstReference( - EntityType.SPACE).getName(), "My Page"), viewPage.getBreadcrumbContent()); - assertEquals("Some content", viewPage.getContent()); + EntityType.SPACE).getName(), "Test relative links"), viewPage.getBreadcrumbContent()); + WikiEditPage wikiEditPage = viewPage.editWiki(); + assertEquals("[[Alice]]\n[[Bob]]\n[[Eve]]", wikiEditPage.getContent()); + + SpaceReference newBobSpace = new SpaceReference("Bob", newRootSpace); + DocumentReference newBobPage = new DocumentReference("WebHome", newBobSpace); + wikiEditPage = WikiEditPage.gotoPage(newBobPage); + assertEquals("[[Alice]]", wikiEditPage.getContent()); + + SpaceReference newAliceSpace = new SpaceReference("Alice", newRootSpace); + DocumentReference newAliceReference = new DocumentReference("WebHome", newAliceSpace); // Check the link is updated. viewPage = setup.gotoPage(mainWikiLinkPage); - WikiEditPage wikiEditPage = viewPage.editWiki(); - assertEquals(String.format("[[subwiki:%s.WebHome]]", space), wikiEditPage.getContent()); + wikiEditPage = viewPage.editWiki(); + assertEquals( + String.format("[[%s]]%n[[%s]]%n[[%s]]", + setup.serializeReference(movedPageReference), + setup.serializeReference(newAliceReference), + setup.serializeReference(newBobPage)), wikiEditPage.getContent()); + + + viewPage = setup.gotoPage(newAliceReference); + renamePage = viewPage.rename(); + renamePage.getDocumentPicker().setName("Alice2"); + renameStatusPage = renamePage.clickRenameButton().waitUntilFinished(); + assertEquals("Done.", renameStatusPage.getInfoMessage()); + + SpaceReference Alice2Space = new SpaceReference("Alice2", newRootSpace); + DocumentReference Alice2Reference = new DocumentReference("WebHome", Alice2Space); + wikiEditPage = WikiEditPage.gotoPage(new DocumentReference("WebHome", newRootSpace)); + String serializedlocalAlice2Reference = setup.serializeLocalReference(Alice2Reference); + assertEquals(String.format("[[%s]]%n[[Bob]]%n[[Eve]]", serializedlocalAlice2Reference), + wikiEditPage.getContent()); + + viewPage = setup.gotoPage(mainWikiLinkPage); + wikiEditPage = viewPage.editWiki(); + assertEquals( + String.format("[[%s]]%n[[%s]]%n[[%s]]", + setup.serializeReference(movedPageReference), + setup.serializeReference(Alice2Reference), + setup.serializeReference(newBobPage)), wikiEditPage.getContent()); deleteSubWiki(setup); } From 0b077f85cc1eb9bd8cb32e5aebe702b1a3847c65 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Thu, 31 Oct 2024 14:36:02 +0100 Subject: [PATCH 09/12] XWIKI-12987: Relative links are made absolute or even broken after moving a page * Improve SubWikiIT to add more checks --- .../it/org/xwiki/wiki/test/ui/SubWikiIT.java | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/xwiki-platform-core/xwiki-platform-wiki/xwiki-platform-wiki-test/xwiki-platform-wiki-test-docker/src/test/it/org/xwiki/wiki/test/ui/SubWikiIT.java b/xwiki-platform-core/xwiki-platform-wiki/xwiki-platform-wiki-test/xwiki-platform-wiki-test-docker/src/test/it/org/xwiki/wiki/test/ui/SubWikiIT.java index 4151e964ffab..3a34a5ab74cd 100644 --- a/xwiki-platform-core/xwiki-platform-wiki/xwiki-platform-wiki-test/xwiki-platform-wiki-test-docker/src/test/it/org/xwiki/wiki/test/ui/SubWikiIT.java +++ b/xwiki-platform-core/xwiki-platform-wiki/xwiki-platform-wiki-test/xwiki-platform-wiki-test-docker/src/test/it/org/xwiki/wiki/test/ui/SubWikiIT.java @@ -19,8 +19,6 @@ */ package org.xwiki.wiki.test.ui; -import java.util.stream.Collectors; - import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; import org.xwiki.livedata.test.po.TableLayoutElement; @@ -180,11 +178,14 @@ void movePageToSubwiki(TestUtils setup, TestReference testReference, TestConfigu assertEquals("Done.", renameStatusPage.getInfoMessage()); SpaceReference Alice2Space = new SpaceReference("Alice2", newRootSpace); + DocumentReference newrootPage = new DocumentReference("WebHome", newRootSpace); DocumentReference Alice2Reference = new DocumentReference("WebHome", Alice2Space); - wikiEditPage = WikiEditPage.gotoPage(new DocumentReference("WebHome", newRootSpace)); + wikiEditPage = WikiEditPage.gotoPage(newrootPage); String serializedlocalAlice2Reference = setup.serializeLocalReference(Alice2Reference); assertEquals(String.format("[[%s]]%n[[Bob]]%n[[Eve]]", serializedlocalAlice2Reference), wikiEditPage.getContent()); + wikiEditPage.setContent(String.format("[[Alice2]]%n[[%s]]%n[[Bob]]%n[[Eve]]",serializedlocalAlice2Reference)); + wikiEditPage.clickSaveAndView(); viewPage = setup.gotoPage(mainWikiLinkPage); wikiEditPage = viewPage.editWiki(); @@ -194,6 +195,25 @@ void movePageToSubwiki(TestUtils setup, TestReference testReference, TestConfigu setup.serializeReference(Alice2Reference), setup.serializeReference(newBobPage)), wikiEditPage.getContent()); + viewPage = setup.gotoPage(Alice2Reference); + renamePage = viewPage.rename(); + renamePage.getDocumentPicker().setWiki("xwiki"); + renameStatusPage = renamePage.clickRenameButton().waitUntilFinished(); + assertEquals("Done.", renameStatusPage.getInfoMessage()); + + Alice2Reference = Alice2Reference.setWikiReference(new WikiReference("xwiki")); + String serializedAliceReference = setup.serializeReference(Alice2Reference); + wikiEditPage = WikiEditPage.gotoPage(newrootPage); + assertEquals(String.format("[[%1$s]]%n[[%1$s]]%n[[Bob]]%n[[Eve]]", serializedAliceReference), + wikiEditPage.getContent()); + viewPage = setup.gotoPage(mainWikiLinkPage); + wikiEditPage = viewPage.editWiki(); + assertEquals( + String.format("[[%s]]%n[[%s]]%n[[%s]]", + setup.serializeReference(movedPageReference), + setup.serializeLocalReference(Alice2Reference), + setup.serializeReference(newBobPage)), wikiEditPage.getContent()); + deleteSubWiki(setup); } From 96eb739d9fe782f984fb8f04a0cae3c8074792d2 Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Mon, 4 Nov 2024 10:20:38 +0100 Subject: [PATCH 10/12] XWIKI-12987: Relative links are made absolute or even broken after moving a page * Few improvments following review --- .../flamingo/test/docker/RenamePageIT.java | 30 +++++++++---------- .../internal/job/AbstractCopyOrMoveJob.java | 11 ++++--- .../refactoring/internal/job/CopyJob.java | 7 ++--- .../refactoring/internal/job/MoveJob.java | 7 ++--- 4 files changed, 26 insertions(+), 29 deletions(-) diff --git a/xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-test/xwiki-platform-flamingo-skin-test-docker/src/test/it/org/xwiki/flamingo/test/docker/RenamePageIT.java b/xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-test/xwiki-platform-flamingo-skin-test-docker/src/test/it/org/xwiki/flamingo/test/docker/RenamePageIT.java index 84932ccdd8e8..8d5889b3d022 100644 --- a/xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-test/xwiki-platform-flamingo-skin-test-docker/src/test/it/org/xwiki/flamingo/test/docker/RenamePageIT.java +++ b/xwiki-platform-core/xwiki-platform-flamingo/xwiki-platform-flamingo-skin/xwiki-platform-flamingo-skin-test/xwiki-platform-flamingo-skin-test-docker/src/test/it/org/xwiki/flamingo/test/docker/RenamePageIT.java @@ -366,15 +366,16 @@ void renamePageUsedInMacroContentAndParameters(TestUtils setup, TestReference te setup.rest().delete(testReference); - setup.createPage(sourcePageReference1, "Some content to be linked. number 1"); - setup.createPage(sourcePageReference2, "Some content to be linked in macro. number 2"); - 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"); - 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" @@ -387,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(); @@ -671,14 +673,12 @@ void renameWithIndexingWaiting(String strategy, TestUtils setup, TestReference t void renameWithRelativeLinks(TestUtils testUtils, TestReference testReference, TestConfiguration testConfiguration) throws Exception { - testUtils.createPage(testReference, "[[Alice]]\n[[Bob]]\n[[Eve]]", "Test relative links"); + testUtils.rest().savePage(testReference, "[[Alice]]\n[[Bob]]\n[[Eve]]", "Test relative links"); SpaceReference rootSpaceReference = testReference.getLastSpaceReference(); SpaceReference aliceSpace = new SpaceReference("Alice", rootSpaceReference); - testUtils.createPage(new DocumentReference("WebHome", aliceSpace), "Alice page", - "Alice"); + testUtils.rest().savePage(new DocumentReference("WebHome", aliceSpace), "Alice page", "Alice"); SpaceReference bobSpace = new SpaceReference("Bob", rootSpaceReference); - testUtils.createPage(new DocumentReference("WebHome", bobSpace), "[[Alice]]", - "Alice"); + 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. @@ -707,10 +707,10 @@ void renameWithRelativeLinks(TestUtils testUtils, TestReference testReference, T statusPage = rename.clickRenameButton().waitUntilFinished(); assertEquals("Done.", statusPage.getInfoMessage()); - SpaceReference Alice2Space = new SpaceReference("Alice2", newRootSpace); - DocumentReference Alice2Reference = new DocumentReference("WebHome", Alice2Space); + SpaceReference alice2Space = new SpaceReference("Alice2", newRootSpace); + DocumentReference alice2Reference = new DocumentReference("WebHome", alice2Space); wikiEditPage = WikiEditPage.gotoPage(new DocumentReference("WebHome", newRootSpace)); - String serializedAlice2Reference = testUtils.serializeLocalReference(Alice2Reference); + 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. diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractCopyOrMoveJob.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractCopyOrMoveJob.java index 3ede00ed5f09..02d7168e7648 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractCopyOrMoveJob.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractCopyOrMoveJob.java @@ -60,9 +60,9 @@ protected void runInternal() throws Exception try { this.progressManager.startStep(this); - BeginFoldEvent beginEvent = getBeginEvent(); + BeginFoldEvent beginEvent = createBeginEvent(); this.observationManager.notify(beginEvent, this, this.getRequest()); - if (((CancelableEvent) beginEvent).isCanceled()) { + if (beginEvent instanceof CancelableEvent && ((CancelableEvent) beginEvent).isCanceled()) { return; } this.progressManager.endStep(this); @@ -74,7 +74,7 @@ protected void runInternal() throws Exception this.progressManager.endStep(this); this.progressManager.startStep(this); - EndFoldEvent endEvent = getEndEvent(); + EndFoldEvent endEvent = createEndEvent(); this.observationManager.notify(endEvent, this, this.getRequest()); this.progressManager.endStep(this); } finally { @@ -337,7 +337,6 @@ protected EntityReference getCommonParent() * @return {@code true} if the operation worked well. */ protected abstract boolean atomicOperation(DocumentReference source, DocumentReference target); - protected abstract B getBeginEvent(); - - protected abstract EndFoldEvent getEndEvent(); + protected abstract BeginFoldEvent createBeginEvent(); + protected abstract EndFoldEvent createEndEvent(); } diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/CopyJob.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/CopyJob.java index f922cf75b6b0..d38e04c83751 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/CopyJob.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/CopyJob.java @@ -24,7 +24,6 @@ import org.xwiki.component.annotation.Component; import org.xwiki.model.reference.DocumentReference; import org.xwiki.observation.event.BeginFoldEvent; -import org.xwiki.observation.event.CancelableEvent; import org.xwiki.observation.event.EndFoldEvent; import org.xwiki.refactoring.event.DocumentCopiedEvent; import org.xwiki.refactoring.event.DocumentCopyingEvent; @@ -68,13 +67,13 @@ protected boolean atomicOperation(DocumentReference source, DocumentReference ta } @Override - protected T getBeginEvent() + protected BeginFoldEvent createBeginEvent() { - return (T) new EntitiesCopyingEvent(); + return new EntitiesCopyingEvent(); } @Override - protected EndFoldEvent getEndEvent() + protected EndFoldEvent createEndEvent() { return new EntitiesCopiedEvent(); } diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/MoveJob.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/MoveJob.java index faaad4533cd3..a5f2513b5c9e 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/MoveJob.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/MoveJob.java @@ -27,7 +27,6 @@ import org.xwiki.model.reference.DocumentReference; import org.xwiki.model.reference.EntityReference; import org.xwiki.observation.event.BeginFoldEvent; -import org.xwiki.observation.event.CancelableEvent; import org.xwiki.observation.event.EndFoldEvent; import org.xwiki.refactoring.event.DocumentRenamedEvent; import org.xwiki.refactoring.event.DocumentRenamingEvent; @@ -54,13 +53,13 @@ public String getType() } @Override - protected T getBeginEvent() + protected BeginFoldEvent createBeginEvent() { - return (T) new EntitiesRenamingEvent(); + return new EntitiesRenamingEvent(); } @Override - protected EndFoldEvent getEndEvent() + protected EndFoldEvent createEndEvent() { return new EntitiesRenamedEvent(); } From 079bb3cdd1af499332b0be267fd859ddd524a31e Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Wed, 6 Nov 2024 14:54:18 +0100 Subject: [PATCH 11/12] XWIKI-12987: Relative links are made absolute or even broken after moving a page * Change APIs to use a Map 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) --- .../src/main/java/com/xpn/xwiki/XWiki.java | 3 +- .../internal/render/DefaultOldRendering.java | 4 +- .../test/java/com/xpn/xwiki/XWikiTest.java | 10 +- .../xwiki/refactoring/ReferenceRenamer.java | 17 +- .../internal/ReferenceUpdater.java | 7 +- .../internal/job/AbstractCopyOrMoveJob.java | 164 +++++++++++++----- .../job/AbstractEntityJobWithChecks.java | 24 +-- .../job/InternalCopyOrMoveJobException.java | 38 ++++ .../refactoring/internal/job/RenameJob.java | 2 + .../listener/BackLinkUpdaterListener.java | 14 +- .../job/question/EntitySelection.java | 63 ++++++- .../refactoring/internal/job/MoveJobTest.java | 23 ++- .../internal/job/RenameJobTest.java | 7 + .../listener/BackLinkUpdaterListenerTest.java | 21 +-- .../internal/DefaultMacroRefactoring.java | 14 +- .../internal/DefaultReferenceRenamer.java | 18 +- .../internal/DefaultReferenceUpdater.java | 19 +- .../internal/ResourceReferenceRenamer.java | 42 +++-- .../internal/DefaultMacroRefactoringTest.java | 13 +- .../internal/DefaultReferenceUpdaterTest.java | 9 +- .../ResourceReferenceRenamerTest.java | 6 +- .../include/IncludeMacroRefactoring.java | 16 +- .../include/IncludeMacroRefactoringTest.java | 23 +-- .../rendering/macro/MacroRefactoring.java | 14 +- 24 files changed, 383 insertions(+), 188 deletions(-) create mode 100644 xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/InternalCopyOrMoveJobException.java diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/XWiki.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/XWiki.java index f2099f7dbe47..0023a55697f1 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/XWiki.java +++ b/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 updatedReferences = Set.of(); + Map updatedReferences = + Map.of(sourceDoc.getDocumentReference(), newDocumentReference); if (currentJob instanceof AbstractCopyOrMoveJob) { updatedReferences = ((AbstractCopyOrMoveJob) currentJob).getSelectedEntities(); } diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/render/DefaultOldRendering.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/render/DefaultOldRendering.java index bc0d1f0d81ac..eaf71afdfade 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/render/DefaultOldRendering.java +++ b/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 diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/XWikiTest.java b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/XWikiTest.java index 4001bb8fb359..e4200bbe2d43 100644 --- a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/XWikiTest.java +++ b/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()); diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/ReferenceRenamer.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/ReferenceRenamer.java index 48fd1ac93b09..e50bb3117de4 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/ReferenceRenamer.java +++ b/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 updatedDocuments) + DocumentReference oldTarget, DocumentReference newTarget, boolean relative, + Map 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 updatedDocuments) + Map updatedEntities) { return renameReferences(block, currentDocumentReference, oldTarget, newTarget, relative); } diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/ReferenceUpdater.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/ReferenceUpdater.java index f0a467ac926a..38a87cb95da4 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/ReferenceUpdater.java +++ b/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 updatedEntities) + EntityReference newTargetReference, Map updatedEntities) { update(documentReference, oldTargetReference, newTargetReference); } diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractCopyOrMoveJob.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractCopyOrMoveJob.java index 02d7168e7648..37b71d86cccc 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractCopyOrMoveJob.java +++ b/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 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() - { - @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 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 diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractEntityJobWithChecks.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractEntityJobWithChecks.java index ecc442b5fd22..5bbf26f9daf3 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractEntityJobWithChecks.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/AbstractEntityJobWithChecks.java @@ -23,8 +23,6 @@ import java.util.HashMap; import java.util.Locale; import java.util.Map; -import java.util.Set; -import java.util.stream.Collectors; import org.xwiki.bridge.event.DocumentsDeletingEvent; import org.xwiki.model.reference.DocumentReference; @@ -49,7 +47,7 @@ public abstract class AbstractEntityJobWithChecks concernedEntities = new HashMap<>(); + protected final Map concernedEntities = new HashMap<>(); @Override protected void runInternal() throws Exception @@ -106,7 +104,7 @@ protected void getEntities(EntityReference entityReference) } } - private DocumentReference cleanLocale(DocumentReference documentReference) + protected DocumentReference cleanLocale(DocumentReference documentReference) { // We don't want to have locale information for root locale in the reference to not have problems with // the questions. @@ -123,13 +121,13 @@ private DocumentReference cleanLocale(DocumentReference documentReference) } } - private void putInConcernedEntities(DocumentReference documentReference) + protected void putInConcernedEntities(DocumentReference documentReference) { DocumentReference cleanDocumentReference = cleanLocale(documentReference); this.concernedEntities.put(cleanDocumentReference, new EntitySelection(cleanDocumentReference)); } - private void getEntities(DocumentReference documentReference) + protected void getEntities(DocumentReference documentReference) { if (this.request.isDeep() && isSpaceHomeReference(documentReference)) { getEntities(documentReference.getLastSpaceReference()); @@ -138,7 +136,7 @@ private void getEntities(DocumentReference documentReference) } } - private void getEntities(SpaceReference spaceReference) + protected void getEntities(SpaceReference spaceReference) { visitDocuments(spaceReference, this::putInConcernedEntities); } @@ -167,16 +165,4 @@ protected EntitySelection getConcernedEntitiesEntitySelection(EntityReference re } return entitySelection; } - - /** - * @return the list of references that have been selected to be refactored. - * @since 16.10.0RC1 - */ - public Set getSelectedEntities() - { - return this.concernedEntities.values().stream() - .filter(EntitySelection::isSelected) - .map(EntitySelection::getEntityReference) - .collect(Collectors.toSet()); - } } diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/InternalCopyOrMoveJobException.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/InternalCopyOrMoveJobException.java new file mode 100644 index 000000000000..2653e69b2b90 --- /dev/null +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/InternalCopyOrMoveJobException.java @@ -0,0 +1,38 @@ +/* + * 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.refactoring.internal.job; + +/** + * Internal exception to be used in {@link AbstractCopyOrMoveJob}. + * + * @version $Id$ + * @since 16.10.0RC1 + */ +public class InternalCopyOrMoveJobException extends Exception +{ + /** + * Default constructor. + * @param message the message of the exception. + */ + public InternalCopyOrMoveJobException(String message) + { + super(message); + } +} diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/RenameJob.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/RenameJob.java index dfac6f9d42e1..eb4123c4a35d 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/RenameJob.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/job/RenameJob.java @@ -48,6 +48,8 @@ protected void process(Collection entityReferences) { if (entityReferences.size() == 1) { process(entityReferences.iterator().next()); + } else { + this.logger.warn("Cannot rename multiple entities."); } } diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/listener/BackLinkUpdaterListener.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/listener/BackLinkUpdaterListener.java index f15091a5b1ce..0cf8138c3f65 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/listener/BackLinkUpdaterListener.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/internal/listener/BackLinkUpdaterListener.java @@ -19,6 +19,7 @@ */ package org.xwiki.refactoring.internal.listener; +import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.function.Predicate; @@ -41,7 +42,6 @@ import org.xwiki.refactoring.event.DocumentRenamedEvent; import org.xwiki.refactoring.internal.ModelBridge; import org.xwiki.refactoring.internal.ReferenceUpdater; -import org.xwiki.refactoring.internal.job.AbstractEntityJobWithChecks; import org.xwiki.refactoring.internal.job.DeleteJob; import org.xwiki.refactoring.internal.job.MoveJob; import org.xwiki.refactoring.job.DeleteRequest; @@ -118,7 +118,7 @@ private void maybeUpdateLinksAfterDelete(Event event) throws RefactoringExceptio DocumentReference newTarget = request.getNewBacklinkTargets().get(deletedEvent.getDocumentReference()); if (request.isUpdateLinks() && newTarget != null) { - updateBackLinks(deletedEvent.getDocumentReference(), newTarget, canEdit, Set.of()); + updateBackLinks(deletedEvent.getDocumentReference(), newTarget, canEdit, Map.of()); } } @@ -128,24 +128,24 @@ private void maybeUpdateLinksAfterRename(Event event, Object source, Object data Predicate canEdit = entityReference -> this.authorization.hasAccess(Right.EDIT, entityReference); - Set movedDocuments = Set.of(); + Map updatedEntities = Map.of(); if (source instanceof MoveJob) { MoveRequest request = (MoveRequest) data; updateLinks = request.isUpdateLinks(); // Check access rights taking into account the move request. canEdit = entityReference -> ((MoveJob) source).hasAccess(Right.EDIT, entityReference); - movedDocuments = ((AbstractEntityJobWithChecks) source).getSelectedEntities(); + updatedEntities = ((MoveJob) source).getSelectedEntities(); } if (updateLinks) { DocumentRenamedEvent renameEvent = (DocumentRenamedEvent) event; updateBackLinks(renameEvent.getSourceReference(), renameEvent.getTargetReference(), canEdit, - movedDocuments); + updatedEntities); } } private void updateBackLinks(DocumentReference source, DocumentReference target, - Predicate canEdit, Set movedDocuments) + Predicate canEdit, Map updatedEntities) throws RefactoringException { this.logger.info("Updating the back-links for document [{}].", source); @@ -162,7 +162,7 @@ private void updateBackLinks(DocumentReference source, DocumentReference target, for (DocumentReference backlinkDocumentReference : backlinkDocumentReferences) { this.progressManager.startStep(this); if (canEdit.test(backlinkDocumentReference)) { - this.updater.update(backlinkDocumentReference, source, target, movedDocuments); + this.updater.update(backlinkDocumentReference, source, target, updatedEntities); } this.progressManager.endStep(this); } diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/job/question/EntitySelection.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/job/question/EntitySelection.java index 24ee96ac0751..68ee2ab4e8de 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/job/question/EntitySelection.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/main/java/org/xwiki/refactoring/job/question/EntitySelection.java @@ -19,9 +19,13 @@ */ package org.xwiki.refactoring.job.question; +import java.util.Optional; + import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; +import org.apache.commons.lang3.builder.ToStringBuilder; import org.xwiki.model.reference.EntityReference; +import org.xwiki.stability.Unstable; /** * Represent an entity with an information about either or not the entity is selected to perform some refactoring. @@ -55,7 +59,8 @@ public enum State /** * Reference to the entity to select for the refactoring. */ - private EntityReference entityReference; + private final EntityReference sourceEntityReference; + private final EntityReference targetEntityReference; /** * Indicate if the entity is selected or not by the user. @@ -68,7 +73,20 @@ public enum State */ public EntitySelection(EntityReference entityReference) { - this.entityReference = entityReference; + this(entityReference, null); + } + + /** + * Constructor of an EntitySelection when there is a target destination. + * @param sourceEntityReference the original reference + * @param targetEntityReference the target reference of the refactoring + * @since 16.10.0RC1 + */ + @Unstable + public EntitySelection(EntityReference sourceEntityReference, EntityReference targetEntityReference) + { + this.sourceEntityReference = sourceEntityReference; + this.targetEntityReference = targetEntityReference; } /** @@ -76,7 +94,17 @@ public EntitySelection(EntityReference entityReference) */ public EntityReference getEntityReference() { - return entityReference; + return sourceEntityReference; + } + + /** + * @return the target reference of the refactoring if any. + * @since 16.10.0RC1 + */ + @Unstable + public Optional getTargetEntityReference() + { + return (targetEntityReference != null) ? Optional.of(targetEntityReference) : Optional.empty(); } /** @@ -111,7 +139,11 @@ public void setSelected(boolean selected) @Override public int hashCode() { - return new HashCodeBuilder(3, 13).append(getEntityReference()).append(isSelected).toHashCode(); + return new HashCodeBuilder(3, 13) + .append(getEntityReference()) + .append(getTargetEntityReference()) + .append(isSelected) + .toHashCode(); } @Override @@ -127,8 +159,21 @@ public boolean equals(Object object) return false; } EntitySelection entitySelection = (EntitySelection) object; - return new EqualsBuilder().append(getEntityReference(), entitySelection.getEntityReference()) - .append(isSelected(), entitySelection.isSelected()).isEquals(); + return new EqualsBuilder() + .append(getEntityReference(), entitySelection.getEntityReference()) + .append(getTargetEntityReference(), entitySelection.getTargetEntityReference()) + .append(isSelected(), entitySelection.isSelected()) + .isEquals(); + } + + @Override + public String toString() + { + return new ToStringBuilder(this) + .append("sourceEntityReference", sourceEntityReference) + .append("targetEntityReference", targetEntityReference) + .append("isSelected", isSelected) + .toString(); } @Override @@ -142,15 +187,15 @@ public int compareTo(EntitySelection entitySelection) return 0; } - if (entityReference == null) { + if (sourceEntityReference == null) { return -1; } - if (entitySelection.entityReference == null) { + if (entitySelection.sourceEntityReference == null) { return 1; } - int result = entityReference.compareTo(entitySelection.entityReference); + int result = sourceEntityReference.compareTo(entitySelection.sourceEntityReference); if (result == 0) { return isSelected.compareTo(entitySelection.isSelected); diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/test/java/org/xwiki/refactoring/internal/job/MoveJobTest.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/test/java/org/xwiki/refactoring/internal/job/MoveJobTest.java index f7e6a2cb2db1..6f611010bbaa 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/test/java/org/xwiki/refactoring/internal/job/MoveJobTest.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/test/java/org/xwiki/refactoring/internal/job/MoveJobTest.java @@ -451,14 +451,22 @@ void moveDocumentToSpaceHome() throws Throwable verify(this.modelBridge).rename(source, new DocumentReference("wiki", "C", "B")); } + /* + * This test scenario checks performing a move of the space chess:A.B.C that contains docs X et Y to wiki tennis. + * The resulting references are tennis:C.X and tennis:C.Y. Note that it's a space move because the request is using + * deep flag set to true. + */ @Test void moveSpaceHomeDeep() throws Throwable { DocumentReference spaceHome = new DocumentReference("chess", List.of("A", "B", "C"), "WebHome"); DocumentReference docFromSpace = new DocumentReference("X", spaceHome.getLastSpaceReference()); + DocumentReference otherDocFromSpace = new DocumentReference("Y", spaceHome.getLastSpaceReference()); when(this.modelBridge.getDocumentReferences(spaceHome.getLastSpaceReference())) - .thenReturn(List.of(docFromSpace)); + .thenReturn(List.of(spaceHome, docFromSpace, otherDocFromSpace)); + when(this.modelBridge.exists(spaceHome)).thenReturn(false); when(this.modelBridge.exists(docFromSpace)).thenReturn(true); + when(this.modelBridge.exists(otherDocFromSpace)).thenReturn(true); WikiReference newWiki = new WikiReference("tennis"); @@ -468,10 +476,19 @@ void moveSpaceHomeDeep() throws Throwable request.setDeep(true); run(request); - verify(this.modelBridge).rename(docFromSpace, new DocumentReference("tennis", "C", "X")); + DocumentReference targetDoc1 = new DocumentReference("tennis", "C", "X"); + verify(this.modelBridge).rename(docFromSpace, targetDoc1); + + DocumentReference targetDoc2 = new DocumentReference("tennis", "C", "Y"); + verify(this.modelBridge).rename(otherDocFromSpace, targetDoc2); verify(this.observationManager).notify(any(DocumentsDeletingEvent.class), any(MoveJob.class), - eq(Map.of(docFromSpace, new EntitySelection(docFromSpace)))); + eq(Map.of( + docFromSpace, new EntitySelection(docFromSpace, targetDoc1), + otherDocFromSpace, new EntitySelection(otherDocFromSpace, targetDoc2) + ))); + assertEquals(1, getLogCapture().size()); + assertEquals("Skipping [chess:A.B.C.WebHome] because it doesn't exist.", getLogCapture().getMessage(0)); } @Test diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/test/java/org/xwiki/refactoring/internal/job/RenameJobTest.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/test/java/org/xwiki/refactoring/internal/job/RenameJobTest.java index fbefa091638f..895c9cf7c770 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/test/java/org/xwiki/refactoring/internal/job/RenameJobTest.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/test/java/org/xwiki/refactoring/internal/job/RenameJobTest.java @@ -61,12 +61,19 @@ void renameMultipleEntities() throws Throwable DocumentReference whiteReference = new DocumentReference("wiki", "Color", "White"); DocumentReference orangeReference = new DocumentReference("wiki", "Color", "Orange"); + when(this.modelBridge.exists(blackReference)).thenReturn(true); + when(this.modelBridge.exists(whiteReference)).thenReturn(true); + MoveRequest request = new MoveRequest(); request.setEntityReferences(List.of(blackReference, whiteReference)); request.setDestination(orangeReference); + request.setCheckAuthorRights(false); + request.setCheckRights(false); run(request); verifyNoMove(); + assertEquals(1, getLogCapture().size()); + assertEquals("Cannot rename multiple entities.", getLogCapture().getMessage(0)); } @Test diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/test/java/org/xwiki/refactoring/internal/listener/BackLinkUpdaterListenerTest.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/test/java/org/xwiki/refactoring/internal/listener/BackLinkUpdaterListenerTest.java index 0198ed1ca28a..6d1ed10d6216 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/test/java/org/xwiki/refactoring/internal/listener/BackLinkUpdaterListenerTest.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-api/src/test/java/org/xwiki/refactoring/internal/listener/BackLinkUpdaterListenerTest.java @@ -20,6 +20,7 @@ package org.xwiki.refactoring.internal.listener; import java.util.Collections; +import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -129,8 +130,8 @@ void onDocumentRenamedWithUpdateLinksOnFarm() this.listener.onEvent(documentRenamedEvent, renameJob, renameRequest); - verify(this.updater).update(carolReference, aliceReference, bobReference, Set.of()); - verify(this.updater).update(denisReference, aliceReference, bobReference, Set.of()); + verify(this.updater).update(carolReference, aliceReference, bobReference, Map.of()); + verify(this.updater).update(denisReference, aliceReference, bobReference, Map.of()); assertEquals("Updating the back-links for document [foo:Users.Alice].", logCapture.getMessage(0)); } @@ -147,7 +148,7 @@ void onDocumentRenamedWithUpdateLinksOnFarmAndNoEditRight() this.listener.onEvent(documentRenamedEvent, renameJob, renameRequest); - verify(this.updater).update(carolReference, aliceReference, bobReference, Set.of()); + verify(this.updater).update(carolReference, aliceReference, bobReference, Map.of()); verify(this.updater, never()).update(eq(denisReference), any(DocumentReference.class), any()); assertEquals("Updating the back-links for document [foo:Users.Alice].", logCapture.getMessage(0)); @@ -163,7 +164,7 @@ void onDocumentRenamedWithUpdateLinksOnWiki() this.listener.onEvent(documentRenamedEvent, renameJob, renameRequest); - verify(this.updater).update(carolReference, aliceReference, bobReference, Set.of()); + verify(this.updater).update(carolReference, aliceReference, bobReference, Map.of()); verify(this.updater, never()).update(eq(denisReference), any(DocumentReference.class), any()); assertEquals("Updating the back-links for document [foo:Users.Alice].", logCapture.getMessage(0)); @@ -189,8 +190,8 @@ void onDocumentRenamedWithoutRenameJob() this.listener.onEvent(documentRenamedEvent, null, null); - verify(this.updater).update(carolReference, aliceReference, bobReference, Set.of()); - verify(this.updater).update(denisReference, aliceReference, bobReference, Set.of()); + verify(this.updater).update(carolReference, aliceReference, bobReference, Map.of()); + verify(this.updater).update(denisReference, aliceReference, bobReference, Map.of()); assertEquals("Updating the back-links for document [foo:Users.Alice].", logCapture.getMessage(0)); } @@ -205,7 +206,7 @@ void onDocumentRenamedWithoutRenameJobAndNoEditRight() this.listener.onEvent(documentRenamedEvent, null, null); verify(this.updater, never()).update(eq(carolReference), any(DocumentReference.class), any()); - verify(this.updater).update(denisReference, aliceReference, bobReference, Set.of()); + verify(this.updater).update(denisReference, aliceReference, bobReference, Map.of()); assertEquals("Updating the back-links for document [foo:Users.Alice].", logCapture.getMessage(0)); } @@ -220,8 +221,8 @@ void onDocumentDeletedWithUpdateLinksOnFarm() this.listener.onEvent(documentDeletedEvent, null, null); - verify(this.updater).update(carolReference, aliceReference, bobReference, Set.of()); - verify(this.updater).update(denisReference, aliceReference, bobReference, Set.of()); + verify(this.updater).update(carolReference, aliceReference, bobReference, Map.of()); + verify(this.updater).update(denisReference, aliceReference, bobReference, Map.of()); assertEquals("Updating the back-links for document [foo:Users.Alice].", logCapture.getMessage(0)); } @@ -252,7 +253,7 @@ void onDocumentDeleteWithUpdateLinksOnFarmAndNoEditRight() this.listener.onEvent(documentDeletedEvent, null, null); - verify(this.updater).update(carolReference, aliceReference, bobReference, Set.of()); + verify(this.updater).update(carolReference, aliceReference, bobReference, Map.of()); verify(this.updater, never()).update(eq(denisReference), any(DocumentReference.class), any()); assertEquals("Updating the back-links for document [foo:Users.Alice].", this.logCapture.getMessage(0)); diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultMacroRefactoring.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultMacroRefactoring.java index 68944f16e610..36b755d8e113 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultMacroRefactoring.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultMacroRefactoring.java @@ -20,6 +20,7 @@ package org.xwiki.refactoring.internal; import java.util.Collections; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.function.Predicate; @@ -36,6 +37,7 @@ import org.xwiki.component.manager.ComponentManager; import org.xwiki.model.reference.AttachmentReference; import org.xwiki.model.reference.DocumentReference; +import org.xwiki.model.reference.EntityReference; import org.xwiki.refactoring.ReferenceRenamer; import org.xwiki.rendering.block.Block; import org.xwiki.rendering.block.MacroBlock; @@ -150,23 +152,23 @@ private XDOM parseMacro(MacroBlock macroBlock) throws MacroRefactoringException @Override public Optional replaceReference(MacroBlock macroBlock, DocumentReference currentDocumentReference, DocumentReference sourceReference, DocumentReference targetReference, boolean relative, - Set updatedDocuments) + Map updatedEntities) throws MacroRefactoringException { return innerReplaceReference(macroBlock, xdom -> this.referenceRenamerProvider.get().renameReferences(xdom, currentDocumentReference, - sourceReference, targetReference, relative, updatedDocuments)); + sourceReference, targetReference, relative, updatedEntities)); } @Override public Optional replaceReference(MacroBlock macroBlock, DocumentReference currentDocumentReference, AttachmentReference sourceReference, AttachmentReference targetReference, boolean relative, - Set updatedDocuments) + Map updatedEntities) throws MacroRefactoringException { return innerReplaceReference(macroBlock, xdom -> this.referenceRenamerProvider.get().renameReferences(xdom, currentDocumentReference, - sourceReference, targetReference, relative, updatedDocuments)); + sourceReference, targetReference, relative, updatedEntities)); } @Override @@ -175,7 +177,7 @@ public Optional replaceReference(MacroBlock macroBlock, DocumentRefe throws MacroRefactoringException { return replaceReference(macroBlock, currentDocumentReference, sourceReference, targetReference, relative, - Set.of(sourceReference)); + Map.of(sourceReference, targetReference)); } @Override @@ -184,7 +186,7 @@ public Optional replaceReference(MacroBlock macroBlock, DocumentRefe throws MacroRefactoringException { return replaceReference(macroBlock, currentDocumentReference, sourceReference, targetReference, relative, - Set.of()); + Map.of(sourceReference.getDocumentReference(), targetReference.getDocumentReference())); } private Optional innerReplaceReference(MacroBlock macroBlock, Predicate lambda) diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultReferenceRenamer.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultReferenceRenamer.java index 34cdec113928..c252b3741dd0 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultReferenceRenamer.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultReferenceRenamer.java @@ -21,6 +21,7 @@ import java.util.Arrays; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; @@ -115,41 +116,42 @@ private interface RenameResourceLambda @Override public boolean renameReferences(Block block, DocumentReference currentDocumentReference, DocumentReference oldTarget, DocumentReference newTarget, boolean relative, - Set updatedDocuments) + Map updatedEntities) { return innerRenameReferences(block, currentDocumentReference, oldTarget, newTarget, SUPPORTED_RESOURCE_TYPES_FOR_DOCUMENTS, (MacroRefactoring macroRefactoring, MacroBlock macroBlock) -> macroRefactoring.replaceReference(macroBlock, - currentDocumentReference, oldTarget, newTarget, relative, updatedDocuments), + currentDocumentReference, oldTarget, newTarget, relative, updatedEntities), reference -> this.resourceReferenceRenamer.updateResourceReference(reference, oldTarget, - newTarget, currentDocumentReference, relative, updatedDocuments)); + newTarget, currentDocumentReference, relative, updatedEntities)); } @Override public boolean renameReferences(Block block, DocumentReference currentDocumentReference, DocumentReference oldTarget, DocumentReference newTarget, boolean relative) { - return renameReferences(block, currentDocumentReference, oldTarget, newTarget, relative, Set.of(oldTarget)); + return renameReferences(block, currentDocumentReference, oldTarget, newTarget, relative, + Map.of(oldTarget, newTarget)); } @Override public boolean renameReferences(Block block, DocumentReference currentDocumentReference, AttachmentReference oldTarget, AttachmentReference newTarget, boolean relative, - Set updatedDocuments) + Map updatedEntities) { return innerRenameReferences(block, currentDocumentReference, oldTarget, newTarget, SUPPORTED_RESOURCE_TYPES_FOR_ATTACHMENTS, (MacroRefactoring macroRefactoring, MacroBlock macroBlock) -> macroRefactoring.replaceReference(macroBlock, - currentDocumentReference, oldTarget, newTarget, relative, updatedDocuments), + currentDocumentReference, oldTarget, newTarget, relative, updatedEntities), reference -> this.resourceReferenceRenamer.updateResourceReference(reference, - oldTarget, newTarget, currentDocumentReference, relative, updatedDocuments)); + oldTarget, newTarget, currentDocumentReference, relative, updatedEntities)); } @Override public boolean renameReferences(Block block, DocumentReference currentDocumentReference, AttachmentReference oldTarget, AttachmentReference newTarget, boolean relative) { - return renameReferences(block, currentDocumentReference, oldTarget, newTarget, relative, Set.of()); + return renameReferences(block, currentDocumentReference, oldTarget, newTarget, relative, Map.of()); } private boolean innerRenameReferences(Block block, DocumentReference currentDocumentReference, diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultReferenceUpdater.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultReferenceUpdater.java index a62c14038fe2..e3bfabbe28a3 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultReferenceUpdater.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/DefaultReferenceUpdater.java @@ -21,7 +21,7 @@ import java.util.List; import java.util.Locale; -import java.util.Set; +import java.util.Map; import javax.inject.Inject; import javax.inject.Named; @@ -266,20 +266,20 @@ private void renameLinks(XWikiDocument document, EntityReference oldTarget, Enti } private void renameLinks(DocumentReference documentReference, DocumentReference oldLinkTarget, - DocumentReference newLinkTarget, boolean relative, Set updatedDocuments) + DocumentReference newLinkTarget, boolean relative, Map updatedEntities) { internalRenameLinks(documentReference, oldLinkTarget, newLinkTarget, relative, (xdom, currentDocumentReference, r) -> this.renamer.renameReferences(xdom, currentDocumentReference, oldLinkTarget, newLinkTarget, r, - updatedDocuments)); + updatedEntities)); } private void renameLinks(DocumentReference documentReference, AttachmentReference oldLinkTarget, - AttachmentReference newLinkTarget, boolean relative, Set updatedDocuments) + AttachmentReference newLinkTarget, boolean relative, Map updatedEntities) { internalRenameLinks(documentReference, oldLinkTarget, newLinkTarget, relative, (xdom, currentDocumentReference, r) -> this.renamer.renameReferences(xdom, currentDocumentReference, oldLinkTarget, newLinkTarget, r, - updatedDocuments)); + updatedEntities)); } private void internalRenameLinks(DocumentReference documentReference, EntityReference oldLinkTarget, @@ -335,7 +335,7 @@ private AttachmentReference toAttachmentReference(EntityReference entityReferenc @Override public void update(DocumentReference documentReference, EntityReference oldTargetReference, - EntityReference newTargetReference, Set updatedDocuments) + EntityReference newTargetReference, Map updatedEntities) { // If the current document is the moved entity the links should be serialized relative to it boolean relative = newTargetReference.equals(documentReference); @@ -348,10 +348,10 @@ public void update(DocumentReference documentReference, EntityReference oldTarge // Only support documents and attachments targets if (oldTargetReference.getType() == EntityType.ATTACHMENT) { renameLinks(documentReference, toAttachmentReference(oldTargetReference), - toAttachmentReference(newTargetReference), relative, updatedDocuments); + toAttachmentReference(newTargetReference), relative, updatedEntities); } else if (oldTargetReference.getType() == EntityType.DOCUMENT) { renameLinks(documentReference, toDocumentReference(oldTargetReference), - toDocumentReference(newTargetReference), relative, updatedDocuments); + toDocumentReference(newTargetReference), relative, updatedEntities); } } @@ -359,6 +359,7 @@ public void update(DocumentReference documentReference, EntityReference oldTarge public void update(DocumentReference documentReference, EntityReference oldTargetReference, EntityReference newTargetReference) { - update(documentReference, oldTargetReference, newTargetReference, Set.of()); + update(documentReference, oldTargetReference, newTargetReference, + Map.of(oldTargetReference, newTargetReference)); } } diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java index 79186bef8889..74108ff78741 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/main/java/org/xwiki/refactoring/internal/ResourceReferenceRenamer.java @@ -19,7 +19,7 @@ */ package org.xwiki.refactoring.internal; -import java.util.Set; +import java.util.Map; import javax.inject.Inject; import javax.inject.Named; @@ -82,18 +82,18 @@ public class ResourceReferenceRenamer * @param newReference the new document reference target * @param currentDocumentReference the current document where the resource reference is located * @param relative {@code true} if the reference should be kept relative to the current document - * @param movedReferences the list of references that have been moved in the same job: this list contains the old - * references before the move. + * @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 resource reference has been updated */ public boolean updateResourceReference(ResourceReference resourceReference, DocumentReference oldReference, DocumentReference newReference, DocumentReference currentDocumentReference, - boolean relative, Set movedReferences) + boolean relative, Map updatedEntities) { return (relative) - ? this.updateRelativeResourceReference(resourceReference, oldReference, newReference, movedReferences) + ? this.updateRelativeResourceReference(resourceReference, oldReference, newReference, updatedEntities) : this.updateAbsoluteResourceReference(resourceReference, oldReference, newReference, - currentDocumentReference, movedReferences); + currentDocumentReference, updatedEntities); } private boolean checkIfDocExists(DocumentReference documentReference) @@ -118,23 +118,23 @@ private boolean checkIfDocExists(DocumentReference documentReference) * @param newReference the new attachment reference target * @param currentDocumentReference the current document where the resource reference is located * @param relative {@code true} if the reference should be kept relative to the current document - * @param movedReferences the list of references that have been moved in the same job: this list contains the old - * references before the move. + * @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 attachment reference has been updated */ public boolean updateResourceReference(ResourceReference resourceReference, AttachmentReference oldReference, AttachmentReference newReference, DocumentReference currentDocumentReference, - boolean relative, Set movedReferences) + boolean relative, Map updatedEntities) { return (relative) - ? this.updateRelativeResourceReference(resourceReference, oldReference, newReference, movedReferences) + ? this.updateRelativeResourceReference(resourceReference, oldReference, newReference, updatedEntities) : this.updateAbsoluteResourceReference(resourceReference, oldReference, newReference, - currentDocumentReference, movedReferences); + currentDocumentReference, updatedEntities); } private boolean updateAbsoluteResourceReference(ResourceReference resourceReference, DocumentReference oldReference, DocumentReference newReference, DocumentReference currentDocumentReference, - Set movedReferences) + Map updatedEntities) { boolean result = false; @@ -155,7 +155,7 @@ private boolean updateAbsoluteResourceReference(ResourceReference resourceRefere // as in this case there won't be any other call to perform the link refactoring. boolean shouldBeUpdated = linkTargetDocumentReference.equals(oldReference) && (absoluteResolvedReference.equals(linkEntityReference) - || !movedReferences.contains(currentDocumentReference)); + || !updatedEntities.containsKey(currentDocumentReference)); if (shouldBeUpdated) { // If the link was resolved to a space... @@ -195,7 +195,7 @@ private boolean updateAbsoluteResourceReference(ResourceReference resourceRefere } private boolean updateRelativeResourceReference(ResourceReference resourceReference, - T oldReference, T newReference, Set movedReferences) + T oldReference, T newReference, Map updatedEntities) { boolean result = false; @@ -208,9 +208,13 @@ private boolean updateRelativeResourceReference(Reso boolean docExists = false; EntityType entityType = linkEntityReference.getType(); + DocumentReference documentReference = null; + DocumentReference documentReferenceTarget = null; if (entityType == EntityType.DOCUMENT || entityType.getAllowedParents().contains(EntityType.DOCUMENT)) { - DocumentReference documentReference = + documentReference = (DocumentReference) oldLinkReference.extractReference(EntityType.DOCUMENT); + documentReferenceTarget = + (DocumentReference) linkEntityReference.extractReference(EntityType.DOCUMENT); docExists = checkIfDocExists(documentReference); } else { docExists = true; @@ -218,9 +222,8 @@ private boolean updateRelativeResourceReference(Reso 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)); + || (documentReferenceTarget != null + && documentReferenceTarget.equals(updatedEntities.get(documentReference)))); // We should update the reference if: // - it's relative: the resolution of the reference without any parameter, and the resolution of the reference @@ -243,9 +246,10 @@ private boolean updateRelativeResourceReference(Reso return result; } + // FIXME: Check if we shouldn't use the updatedEntities here. private boolean updateAbsoluteResourceReference(ResourceReference resourceReference, AttachmentReference oldReference, AttachmentReference newReference, DocumentReference currentDocumentReference, - Set movedReferences) + Map updatedEntities) { boolean result = false; diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultMacroRefactoringTest.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultMacroRefactoringTest.java index a9ed5dc66fa9..980e310f265f 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultMacroRefactoringTest.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultMacroRefactoringTest.java @@ -22,7 +22,6 @@ import java.util.Collections; import java.util.Map; import java.util.Optional; -import java.util.Set; import javax.inject.Provider; @@ -153,10 +152,10 @@ void replaceReferenceUnparseableMacro() throws MacroRefactoringException }); assertEquals(Optional.empty(), this.macroRefactoring.replaceReference(this.macroBlock, this.currentDocumentReference, - this.sourceReference, this.targetReference, true, Set.of())); + this.sourceReference, this.targetReference, true, Map.of())); assertEquals(Optional.empty(), this.macroRefactoring.replaceReference(this.macroBlock, this.currentDocumentReference, - this.sourceReference, this.targetReference, false, Set.of())); + this.sourceReference, this.targetReference, false, Map.of())); verify(this.referenceRenamer, never()).renameReferences(any(), any(), any(DocumentReference.class), any(), anyBoolean(), any()); } @@ -188,10 +187,10 @@ void replaceReferenceNotUpdated() throws Exception return xdom; }); when(this.referenceRenamer.renameReferences(xdom, this.currentDocumentReference, this.sourceReference, - this.targetReference, true, Set.of())).thenReturn(false); + this.targetReference, true, Map.of())).thenReturn(false); assertEquals(Optional.empty(), this.macroRefactoring.replaceReference(this.macroBlock, this.currentDocumentReference, this.sourceReference, - this.targetReference, true, Set.of())); + this.targetReference, true, Map.of())); verify(this.blockRenderer, never()).render(any(Block.class), any()); } @@ -213,7 +212,7 @@ void replaceReferenceUpdated() throws Exception return xdom; }); when(this.referenceRenamer.renameReferences(xdom, this.currentDocumentReference, this.sourceReference, - this.targetReference, true, Set.of())).thenReturn(true); + this.targetReference, true, Map.of())).thenReturn(true); String expectedContent = "the expected content"; doAnswer(invocationOnMock -> { WikiPrinter printer = invocationOnMock.getArgument(1); @@ -234,7 +233,7 @@ void replaceReferenceUpdated() throws Exception MacroBlock expectedBlock = new MacroBlock(this.macroId, parameters, expectedContent, false); assertEquals(Optional.of(expectedBlock), this.macroRefactoring.replaceReference(this.macroBlock, this.currentDocumentReference, this.sourceReference, - this.targetReference, true, Set.of())); + this.targetReference, true, Map.of())); } @Test diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultReferenceUpdaterTest.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultReferenceUpdaterTest.java index 3ec23251e625..3c5fe356493f 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultReferenceUpdaterTest.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultReferenceUpdaterTest.java @@ -26,7 +26,6 @@ import java.util.Locale; import java.util.Map; import java.util.Optional; -import java.util.Set; import javax.inject.Named; import javax.inject.Provider; @@ -661,11 +660,11 @@ void updateFromMacros(MockitoComponentManager componentManager) throws Exception updater.update(documentReference, oldLinkTarget, newLinkTarget); verify(includeMacroRefactoring).replaceReference(includeMacroBlock1, documentReference, oldLinkTarget, - newLinkTarget, false, Set.of()); + newLinkTarget, false, Map.of(oldLinkTarget, newLinkTarget)); verify(includeMacroRefactoring).replaceReference(includeMacroBlock2, documentReference, oldLinkTarget, - newLinkTarget, false, Set.of()); + newLinkTarget, false, Map.of(oldLinkTarget, newLinkTarget)); verify(displayMacroRefactoring).replaceReference(displayMacroBlock, documentReference, oldLinkTarget, - newLinkTarget, false, Set.of()); + newLinkTarget, false, Map.of(oldLinkTarget, newLinkTarget)); verify(this.mutableRenderingContext, times(3)).push(any(), any(), eq(Syntax.XWIKI_2_1), any(), anyBoolean(), any()); verify(this.mutableRenderingContext, times(3)).pop(); @@ -753,7 +752,7 @@ void updateFromLinksAndMacros(MockitoComponentManager componentManager) throws E updater.update(documentReference, oldLinkTarget, newLinkTarget); verify(includeMacroRefactoring).replaceReference(includeMacroBlock, documentReference, oldLinkTarget, - newLinkTarget, false, Set.of()); + newLinkTarget, false, Map.of(oldLinkTarget, newLinkTarget)); assertEquals("X.Y", documentLinkBlock.getReference().getReference()); assertEquals(ResourceType.DOCUMENT, documentLinkBlock.getReference().getType()); verifyDocumentSave(document, "Renamed back-links.", false, false); diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/ResourceReferenceRenamerTest.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/ResourceReferenceRenamerTest.java index 231b0cf2cdbd..63cd5422cc40 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/ResourceReferenceRenamerTest.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/ResourceReferenceRenamerTest.java @@ -19,7 +19,7 @@ */ package org.xwiki.refactoring.internal; -import java.util.Set; +import java.util.Map; import javax.inject.Named; import javax.inject.Provider; @@ -107,7 +107,7 @@ void updateResourceReferenceRelative() assertTrue(this.renamer.updateResourceReference(resourceReference, oldReference, newReference, - new DocumentReference("xwiki", "Space", "Page"), true, Set.of())); + new DocumentReference("xwiki", "Space", "Page"), true, Map.of())); verify(this.compactEntityReferenceSerializer).serialize(oldReference, newReference); } @@ -133,7 +133,7 @@ void updateResourceReferenceNotRelative() assertTrue(this.renamer.updateResourceReference(resourceReference, oldReference, newReference, currentDocumentReference, - false, Set.of())); + false, Map.of())); assertEquals(new AttachmentResourceReference("xwiki:Space.Page.file2.txt"), resourceReference); } } diff --git a/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 b/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 index bc208d9eaf22..52ec5c307ee4 100644 --- a/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 +++ b/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 @@ -20,6 +20,7 @@ package org.xwiki.rendering.internal.macro.include; import java.util.Collections; +import java.util.Map; import java.util.Optional; import java.util.Set; @@ -86,21 +87,21 @@ public class IncludeMacroRefactoring implements MacroRefactoring @Override public Optional replaceReference(MacroBlock macroBlock, DocumentReference currentDocumentReference, DocumentReference sourceReference, DocumentReference targetReference, boolean relative, - Set updatedDocuments) + Map updatedEntities) throws MacroRefactoringException { return getMacroBlock(macroBlock, currentDocumentReference, sourceReference, targetReference, relative, - updatedDocuments); + updatedEntities); } @Override public Optional replaceReference(MacroBlock macroBlock, DocumentReference currentDocumentReference, AttachmentReference sourceReference, AttachmentReference targetReference, boolean relative, - Set updatedDocuments) + Map updatedEntities) throws MacroRefactoringException { return getMacroBlock(macroBlock, currentDocumentReference, sourceReference, targetReference, relative, - updatedDocuments); + updatedEntities); } @Override @@ -109,7 +110,7 @@ public Optional replaceReference(MacroBlock macroBlock, DocumentRefe throws MacroRefactoringException { return getMacroBlock(macroBlock, currentDocumentReference, sourceReference, targetReference, relative, - Set.of()); + Map.of(sourceReference, targetReference)); } @Override @@ -118,12 +119,13 @@ public Optional replaceReference(MacroBlock macroBlock, DocumentRefe throws MacroRefactoringException { return getMacroBlock(macroBlock, currentDocumentReference, sourceReference, targetReference, relative, - Set.of()); + Map.of(sourceReference.getDocumentReference(), targetReference.getDocumentReference())); } + // FIXME: double check we don't need to use updated documents parameters here. private Optional getMacroBlock(MacroBlock macroBlock, DocumentReference currentDocumentReference, T sourceReference, T targetReference, boolean relative, - Set updatedDocuments) + Map updatedEntities) throws MacroRefactoringException { Optional result; diff --git a/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-macros/xwiki-platform-rendering-macro-include/src/test/java/org/xwiki/rendering/internal/macro/include/IncludeMacroRefactoringTest.java b/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-macros/xwiki-platform-rendering-macro-include/src/test/java/org/xwiki/rendering/internal/macro/include/IncludeMacroRefactoringTest.java index 9de3b607d9dc..93c0192c8c26 100644 --- a/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-macros/xwiki-platform-rendering-macro-include/src/test/java/org/xwiki/rendering/internal/macro/include/IncludeMacroRefactoringTest.java +++ b/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-macros/xwiki-platform-rendering-macro-include/src/test/java/org/xwiki/rendering/internal/macro/include/IncludeMacroRefactoringTest.java @@ -20,6 +20,7 @@ package org.xwiki.rendering.internal.macro.include; import java.util.Collections; +import java.util.Map; import java.util.Optional; import java.util.Set; @@ -110,7 +111,7 @@ void replaceDocumentReferenceWhenNoReferenceParameterSet() throws Exception { MacroBlock block = new MacroBlock("include", Collections.emptyMap(), false); assertEquals(Optional.empty(), this.includeMacroRefactoring.replaceReference(block, null, null, - (DocumentReference) null, false, Set.of())); + (DocumentReference) null, false, Map.of())); } @Test @@ -119,7 +120,7 @@ void replaceDocumentReferenceWhenEmptyReferenceParameterSet() throws Exception MacroBlock block = new MacroBlock("include", Collections.emptyMap(), false); block.setParameter("reference", ""); assertEquals(Optional.empty(), this.includeMacroRefactoring.replaceReference(block, null, null, - (DocumentReference) null, false, Set.of())); + (DocumentReference) null, false, Map.of())); } @Test @@ -128,7 +129,7 @@ void replaceDocumentReferenceWhenEmptyPageParameterSet() throws Exception MacroBlock block = new MacroBlock("include", Collections.emptyMap(), false); block.setParameter("page", ""); assertEquals(Optional.empty(), this.includeMacroRefactoring.replaceReference(block, null, null, - (DocumentReference) null, false, Set.of())); + (DocumentReference) null, false, Map.of())); } @Test @@ -153,7 +154,7 @@ void replaceDocumentReferenceWhenIncludedReferenceIsRenamedUsingReferenceParamet new EntityReference("sourcewiki", EntityType.WIKI))).thenReturn("sourcespace.foo"); Optional result = this.includeMacroRefactoring.replaceReference(block, currentReference, - sourceReference, targetReference, false, Set.of()); + sourceReference, targetReference, false, Map.of()); assertFalse(result.isEmpty()); assertEquals("targetwiki:targetspace.targetfoo", result.get().getParameter("reference")); } @@ -189,7 +190,7 @@ void replaceDocumentReferenceWhenIncludingDocumentRenamedUsingReferenceParameter new EntityReference("sourcewiki", EntityType.WIKI))).thenReturn("sourcespace.foo"); Optional result = this.includeMacroRefactoring.replaceReference(block, null, - sourceReference, targetReference, false, Set.of()); + sourceReference, targetReference, false, Map.of()); assertFalse(result.isEmpty()); assertEquals("sourcewiki:sourcespace.foo", result.get().getParameter("reference")); } @@ -225,7 +226,7 @@ void replaceDocumentReferenceWhenIncludingDocumentRenamedUsingPageParameterAndNo new EntityReference("sourcewiki", EntityType.WIKI))).thenReturn("sourcespace/foo"); Optional result = this.includeMacroRefactoring.replaceReference(block, null, - sourceReference, targetReference, false, Set.of()); + sourceReference, targetReference, false, Map.of()); assertFalse(result.isEmpty()); assertEquals("sourcewiki:sourcespace.foo", result.get().getParameter("page")); } @@ -254,7 +255,7 @@ void replaceDocumentReferenceWhenIncludedReferenceIsRenamedUsingPageParameterAnd new EntityReference("sourcewiki", EntityType.WIKI))).thenReturn("sourcespace/foo"); Optional result = this.includeMacroRefactoring.replaceReference(block, currentReference, - sourceReference, targetReference, false, Set.of()); + sourceReference, targetReference, false, Map.of()); assertFalse(result.isEmpty()); assertEquals("targetwiki:targetspace.targetfoo", result.get().getParameter("page")); } @@ -282,7 +283,7 @@ void replaceDocumentReferenceWhenIncludingDocumentIsRenamedToSameReference() thr new EntityReference("sourcewiki", EntityType.WIKI))).thenReturn("sourcespace.foo"); Optional result = this.includeMacroRefactoring.replaceReference(block, null, sourceReference, - targetReference, false, Set.of()); + targetReference, false, Map.of()); assertTrue(result.isEmpty()); } @@ -312,7 +313,7 @@ void replaceAttachmentReferenceWhenIncludedAttachmentReferenceIsRenamedUsingRefe new EntityReference("sourcewiki", EntityType.WIKI))).thenReturn("sourcespace.foo"); Optional result = this.includeMacroRefactoring.replaceReference(block, currentReference, - sourceReference, targetAttachmentReference, false, Set.of()); + sourceReference, targetAttachmentReference, false, Map.of()); assertFalse(result.isEmpty()); assertEquals("targetwiki:targetspace.targetfoo@targetfile", result.get().getParameter("reference")); } @@ -355,7 +356,7 @@ void replaceDocumentReferenceWhenIncludingDocumentIsRenamedUsingAttachmentRefere new EntityReference("sourcewiki", EntityType.WIKI))).thenReturn("sourcespace.foopage@foofile"); Optional result = this.includeMacroRefactoring.replaceReference(block, null, - sourceReference, targetReference, false, Set.of()); + sourceReference, targetReference, false, Map.of()); assertFalse(result.isEmpty()); assertEquals("sourcewiki:sourcespace.foopage@foofile", result.get().getParameter("reference")); } @@ -370,7 +371,7 @@ void replaceDocumentReferenceWhenParametersPopulateError() throws Exception Throwable exception = assertThrows(MacroRefactoringException.class, () -> this.includeMacroRefactoring.replaceReference(block, null, null, (DocumentReference) null, false, - Set.of())); + Map.of())); assertEquals("There's one or several invalid parameters for an [include] macro with parameters " + "[{type=invalid}]", exception.getMessage()); } diff --git a/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-xwiki/src/main/java/org/xwiki/rendering/macro/MacroRefactoring.java b/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-xwiki/src/main/java/org/xwiki/rendering/macro/MacroRefactoring.java index 8603a14e3eda..e752a0981ec7 100644 --- a/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-xwiki/src/main/java/org/xwiki/rendering/macro/MacroRefactoring.java +++ b/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-xwiki/src/main/java/org/xwiki/rendering/macro/MacroRefactoring.java @@ -20,12 +20,14 @@ package org.xwiki.rendering.macro; import java.util.Collections; +import java.util.Map; import java.util.Optional; import java.util.Set; 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.MacroBlock; import org.xwiki.rendering.listener.reference.ResourceReference; import org.xwiki.stability.Unstable; @@ -70,8 +72,8 @@ Optional replaceReference(MacroBlock macroBlock, DocumentReference c * @param targetReference the reference to use as replacement. * @param relative if {@code true} indicate that the reference should be resolved 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 an optional containing the new macro block with proper information if it needs to be updated, else an * empty optional. * @throws MacroRefactoringException in case of problem to parse or render the macro content. @@ -80,7 +82,7 @@ Optional replaceReference(MacroBlock macroBlock, DocumentReference c @Unstable default Optional replaceReference(MacroBlock macroBlock, DocumentReference currentDocumentReference, DocumentReference sourceReference, DocumentReference targetReference, boolean relative, - Set updatedDocuments) + Map updatedEntities) throws MacroRefactoringException { return replaceReference(macroBlock, currentDocumentReference, sourceReference, targetReference, relative); @@ -118,8 +120,8 @@ Optional replaceReference(MacroBlock macroBlock, DocumentReference c * @param targetReference the reference to use as replacement * @param relative if {@code true} indicate that the reference should be resolved 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 an optional containing the new macro block with proper information if it needs to be updated, else an * empty optional. * @throws MacroRefactoringException in case of problem to parse or render the macro content. @@ -128,7 +130,7 @@ Optional replaceReference(MacroBlock macroBlock, DocumentReference c @Unstable default Optional replaceReference(MacroBlock macroBlock, DocumentReference currentDocumentReference, AttachmentReference sourceReference, AttachmentReference targetReference, boolean relative, - Set updatedDocuments) + Map updatedEntities) throws MacroRefactoringException { return replaceReference(macroBlock, currentDocumentReference, sourceReference, targetReference, relative); From 1d8b51bfcace93f68fa33fc1bc22fcac67d1aa9f Mon Sep 17 00:00:00 2001 From: Simon Urli Date: Wed, 6 Nov 2024 17:38:38 +0100 Subject: [PATCH 12/12] XWIKI-12987: Relative links are made absolute or even broken after moving a page * Fix remaining coverage problems --- .../internal/DefaultModelBridgeTest.java | 32 +++++++++++++++++++ .../include/IncludeMacroRefactoringTest.java | 2 +- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultModelBridgeTest.java b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultModelBridgeTest.java index faf1e22b4aaa..d2ed895892b9 100644 --- a/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultModelBridgeTest.java +++ b/xwiki-platform-core/xwiki-platform-refactoring/xwiki-platform-refactoring-default/src/test/java/org/xwiki/refactoring/internal/DefaultModelBridgeTest.java @@ -23,6 +23,7 @@ import java.util.Collections; import java.util.List; import java.util.Locale; +import java.util.Set; import javax.inject.Named; import javax.inject.Provider; @@ -34,9 +35,11 @@ import org.xwiki.job.AbstractJobStatus; import org.xwiki.job.api.AbstractCheckRightsRequest; import org.xwiki.job.event.status.JobProgressManager; +import org.xwiki.link.LinkStore; import org.xwiki.model.EntityType; import org.xwiki.model.reference.DocumentReference; import org.xwiki.model.reference.DocumentReferenceResolver; +import org.xwiki.model.reference.EntityReference; import org.xwiki.model.reference.EntityReferenceProvider; import org.xwiki.model.reference.EntityReferenceResolver; import org.xwiki.model.reference.EntityReferenceSerializer; @@ -119,6 +122,12 @@ class DefaultModelBridgeTest @Named("document") private UserReferenceResolver userReferenceResolver; + @MockComponent + private Provider linkStoreProvider; + + @MockComponent + private DocumentReferenceResolver documentReferenceResolver; + @InjectComponentManager private MockitoComponentManager componentManager; @@ -140,6 +149,9 @@ class DefaultModelBridgeTest @Mock private XWikiRightService xWikiRightService; + @Mock + private LinkStore linkStore; + @BeforeEach void configure(MockitoComponentManager componentManager) throws Exception { @@ -158,6 +170,7 @@ void configure(MockitoComponentManager componentManager) throws Exception .thenReturn(new DocumentReference("what", "ever", "WebHome")); when(entityReferenceProvider.getDefaultReference(EntityType.SPACE)) .thenReturn(new SpaceReference("whatever", "Main")); + when(this.linkStoreProvider.get()).thenReturn(this.linkStore); } private void assertLog(Level level, String message, Object... arguments) @@ -892,4 +905,23 @@ void rename() throws Exception verify(this.xwiki).renameDocument(source, target, true, List.of(), List.of(), this.xcontext); } + + @Test + void getBackLinkedDocuments() throws Exception + { + EntityReference source = mock(EntityReference.class); + EntityReference ref1 = mock(EntityReference.class, "ref1"); + EntityReference ref2 = mock(EntityReference.class, "ref2"); + EntityReference ref3 = mock(EntityReference.class, "ref3"); + when(this.linkStore.resolveBackLinkedEntities(source)).thenReturn(Set.of(ref1, ref2, ref3)); + + DocumentReference docRef1 = mock(DocumentReference.class, "docRef1"); + DocumentReference docRef2 = mock(DocumentReference.class, "docRef2"); + DocumentReference docRef3 = mock(DocumentReference.class, "docRef3"); + when(this.documentReferenceResolver.resolve(ref1, this.xcontext)).thenReturn(docRef1); + when(this.documentReferenceResolver.resolve(ref2, this.xcontext)).thenReturn(docRef2); + when(this.documentReferenceResolver.resolve(ref3, this.xcontext)).thenReturn(docRef3); + + assertEquals(Set.of(docRef1, docRef2, docRef3), this.modelBridge.getBackLinkedDocuments(source)); + } } diff --git a/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-macros/xwiki-platform-rendering-macro-include/src/test/java/org/xwiki/rendering/internal/macro/include/IncludeMacroRefactoringTest.java b/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-macros/xwiki-platform-rendering-macro-include/src/test/java/org/xwiki/rendering/internal/macro/include/IncludeMacroRefactoringTest.java index 93c0192c8c26..67b78ddd6e63 100644 --- a/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-macros/xwiki-platform-rendering-macro-include/src/test/java/org/xwiki/rendering/internal/macro/include/IncludeMacroRefactoringTest.java +++ b/xwiki-platform-core/xwiki-platform-rendering/xwiki-platform-rendering-macros/xwiki-platform-rendering-macro-include/src/test/java/org/xwiki/rendering/internal/macro/include/IncludeMacroRefactoringTest.java @@ -226,7 +226,7 @@ void replaceDocumentReferenceWhenIncludingDocumentRenamedUsingPageParameterAndNo new EntityReference("sourcewiki", EntityType.WIKI))).thenReturn("sourcespace/foo"); Optional result = this.includeMacroRefactoring.replaceReference(block, null, - sourceReference, targetReference, false, Map.of()); + sourceReference, targetReference, false); assertFalse(result.isEmpty()); assertEquals("sourcewiki:sourcespace.foo", result.get().getParameter("page")); }