Skip to content

Commit

Permalink
XWIKI-22323: Refactoring operation should wait for the Solr index to …
Browse files Browse the repository at this point in the history
…be empty before proceeding (#3403)

* Introduce a new ReadyIndicator interface that allows waiting for the
  link index to become ready while getting a progress percentage.
* In the BackLinkUpdaterListener, wait for the index to become ready
  when a job is active and display the indexing progress.
* Provide a ready indicator including indexing progress in the Solr
  indexer.
* Modernize the jobRunner JavaScript code
* Continue polling the job status when the job is waiting to detect when
  a question is answered in the background (by another browser tab or on
  the server).
* Add support in entity requests to indicate if the job should wait for indexing to finish.
* Ask the user after 10 seconds if the refactoring should wait for link
  indexing to finish.
* Wait for link indexing before adapting links after moving attachments
* Add unit and integration tests.
* Adapt the code to Java 11 and older Mockito.
* Backport TestUtils#serializeLocalReference.

(cherry picked from commit 00b8440)
  • Loading branch information
michitux committed Oct 14, 2024
1 parent a1cbd8d commit b16309d
Show file tree
Hide file tree
Showing 30 changed files with 1,595 additions and 78 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,12 @@
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.function.Predicate;

import javax.inject.Inject;
import javax.inject.Named;
import javax.inject.Provider;
import javax.inject.Singleton;

import org.slf4j.Logger;
Expand All @@ -41,6 +43,7 @@
import org.xwiki.refactoring.RefactoringException;
import org.xwiki.refactoring.internal.ModelBridge;
import org.xwiki.refactoring.internal.ReferenceUpdater;
import org.xwiki.refactoring.internal.listener.LinkIndexingWaitingHelper;
import org.xwiki.security.authorization.AuthorizationManager;
import org.xwiki.security.authorization.Right;

Expand Down Expand Up @@ -75,6 +78,10 @@ public class MovedAttachmentListener implements EventListener
@Inject
private Logger logger;

// Use a Provider to avoid early initialization of dependencies.
@Inject
private Provider<LinkIndexingWaitingHelper> linkIndexingHelper;

@Override
public String getName()
{
Expand Down Expand Up @@ -111,6 +118,8 @@ private void updateBackLinks(AttachmentMovedEvent event, Predicate<EntityReferen
{
this.logger.info("Updating the back-links for attachment [{}].", event.getSourceReference());

this.linkIndexingHelper.get().maybeWaitForLinkIndexingWithLog(10, TimeUnit.SECONDS);

// TODO: it's possible to optimize a bit the actual entities to modify (especially which translation of the
// document to load and parse) since we have the information in the store
Set<DocumentReference> documentsList =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.xwiki.attachment.internal.listener;

import java.util.Set;
import java.util.concurrent.TimeUnit;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
Expand All @@ -31,6 +32,7 @@
import org.xwiki.refactoring.RefactoringException;
import org.xwiki.refactoring.internal.ModelBridge;
import org.xwiki.refactoring.internal.ReferenceUpdater;
import org.xwiki.refactoring.internal.listener.LinkIndexingWaitingHelper;
import org.xwiki.security.authorization.AuthorizationManager;
import org.xwiki.security.authorization.Right;
import org.xwiki.test.LogLevel;
Expand Down Expand Up @@ -79,6 +81,9 @@ class MovedAttachmentListenerTest
@MockComponent
private AuthorizationManager authorization;

@MockComponent
private LinkIndexingWaitingHelper linkIndexingHelper;

@RegisterExtension
private LogCaptureExtension logCapture = new LogCaptureExtension(LogLevel.INFO);

Expand All @@ -92,6 +97,7 @@ void onEventNoUpdateReference()
verifyNoInteractions(this.progressManager);
verifyNoInteractions(this.modelBridge);
verifyNoInteractions(this.authorization);
verifyNoInteractions(this.linkIndexingHelper);
}

@Test
Expand All @@ -114,6 +120,7 @@ void onEvent() throws RefactoringException
verify(this.linkRefactoring).update(d1, SOURCE_ATTACHMENT, TARGET_ATTACHMENT);
verify(this.linkRefactoring).update(d2, SOURCE_ATTACHMENT, TARGET_ATTACHMENT);
verify(this.linkRefactoring).update(DOCUMENT_REFERENCE, SOURCE_ATTACHMENT, TARGET_ATTACHMENT);
verify(this.linkIndexingHelper).maybeWaitForLinkIndexingWithLog(10, TimeUnit.SECONDS);
assertEquals(1, this.logCapture.size());
assertEquals("Updating the back-links for attachment [Attachment wiki:space.page@oldname].",
this.logCapture.getMessage(0));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* 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.test;

import javax.inject.Named;
import javax.inject.Singleton;

import org.xwiki.component.annotation.Component;
import org.xwiki.script.service.ScriptService;

/**
* A script service that provides a sleep method for testing purposes.
*
* @version $Id$
*/
@Component
@Singleton
@Named("sleep")
public class SleepScriptService implements ScriptService
{
/**
* Sleep for a given number of seconds.
*
* @param seconds the number of seconds to sleep
*/
public void sleepInSolr(long seconds)
{
// Check if we're in the Solr indexing thread.
if (Thread.currentThread().getName().equals("XWiki Solr index thread")) {
try {
Thread.sleep(seconds * 1000);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
org.xwiki.test.CustomUserUpdatedDocumentEventListener
org.xwiki.test.TestMacro
org.xwiki.test.SleepScriptService
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Order;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.NullSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.xwiki.flamingo.skin.test.po.AttachmentsPane;
import org.xwiki.flamingo.skin.test.po.AttachmentsViewPage;
import org.xwiki.flamingo.skin.test.po.JobQuestionPane;
Expand Down Expand Up @@ -520,4 +523,76 @@ void renamePageRelativeLinkPageAndTranslation(TestUtils setup, TestReference tes
assertEquals("fr [[" + parent + ".OtherPage.WebHome]]", newPage.getContent());

}

@Order(8)
@ParameterizedTest
@ValueSource(strings = { "true", "false" })
@NullSource
void renameWithIndexingWaiting(String strategy, TestUtils setup, TestReference testReference,
TestConfiguration testConfiguration) throws Exception
{
// Create two pages that link to each other.
SpaceReference spaceReference = testReference.getLastSpaceReference();
SpaceReference sourceSpace = new SpaceReference("Source", spaceReference);
SpaceReference targetSpace = new SpaceReference("Target", spaceReference);
DocumentReference sourceReference = new DocumentReference("WebHome", sourceSpace);
DocumentReference targetReference = new DocumentReference("WebHome", targetSpace);

SpaceReference otherSpace = new SpaceReference("Other", spaceReference);
DocumentReference otherReference = new DocumentReference("WebHome", otherSpace);

setup.rest().delete(sourceReference);
setup.rest().delete(targetReference);
setup.rest().delete(otherReference);

// 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(setup, computedHostURL(testConfiguration)).waitEmptyQueue();

// Make the other page slow to index by sleeping when we're in Solr.
String slowTitle = "$services.sleep.sleepInSolr(20) Slow Title";

setup.rest().savePage(sourceReference,
String.format("Source, see [[%s]].", setup.serializeLocalReference(otherReference)), "Source");
String otherFormat = "Other, see [[%s]]";
String otherContent = String.format(otherFormat, setup.serializeLocalReference(sourceReference));
setup.rest().savePage(otherReference, otherContent, slowTitle);

ViewPage viewPage = setup.gotoPage(sourceReference);
RenamePage renamePage = viewPage.rename();
renamePage.getDocumentPicker().setTitle("Target");
CopyOrRenameOrDeleteStatusPage statusPage = renamePage.clickRenameButton();

int currentTimeout = setup.getDriver().getTimeout();
try {
// Increase the timeout as the question pane will only appear after 10 seconds and then we need to wait
// possibly for another 10 seconds until indexing finished.
setup.getDriver().setTimeout(20);
JobQuestionPane jobQuestionPane = new JobQuestionPane().waitForQuestionPane();
assertEquals("Continue waiting for the link indexing to finish?", jobQuestionPane.getQuestionTitle());
if (strategy != null) {
jobQuestionPane.clickButton("qproperty_continueWaiting", strategy);
}
statusPage.waitUntilFinished();

assertEquals("Done.", statusPage.getInfoMessage());
assertTrue(setup.rest().exists(targetReference));
assertFalse(setup.rest().exists(sourceReference));

String updatedOtherContent = setup.gotoPage(otherReference).editWiki().getContent();
if ("false".equals(strategy)) {
// Make sure the other page hasn't been changed.
assertEquals(otherContent, updatedOtherContent);
} else {
// Both when the question isn't answered and when the continue waiting button is clicked, the other page
// should be updated.
assertEquals(String.format(otherFormat, setup.serializeLocalReference(targetReference)),
updatedOtherContent);
}
} finally {
setup.getDriver().setTimeout(currentTimeout);
// Make sure we delete that strange page again.
setup.rest().delete(otherReference);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,19 @@ public CopyOrRenameOrDeleteStatusPage confirmQuestion()
return new CopyOrRenameOrDeleteStatusPage();
}

/**
* Click on the button with the given name and value.
*
* @param name the name of the button
* @param value the value of the button
* @return the status page
*/
public CopyOrRenameOrDeleteStatusPage clickButton(String name, String value)
{
this.questionPane.findElement(By.cssSelector("button[name='" + name + "'][value='" + value + "']")).click();
return new CopyOrRenameOrDeleteStatusPage();
}

/**
* @return true if the current job is blocked by another one.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,51 +19,73 @@
*/
define(['jquery'], function($) {
'use strict';
var createCallback = function(config, promise) {
var answerJobQuestion = function(data) {
const createCallback = function(config, promise) {
// Store if we were already waiting for a question answer
let waitingForAnswer = false;

const answerJobQuestion = function(data) {
// Ensure that we update the status of the job after answering the question even if the job should wait for
// another answer.
waitingForAnswer = false;

// 'this' is the job status.
var request = config.createAnswerRequest(this.id, data);
const request = config.createAnswerRequest(this.id, data);

// Create a POST request
var promise = $.post(request.url, request.data);
const promise = $.post(request.url, request.data);

// Automated progress and failure
promise.then(onProgress, onFailure);

return promise;
};

var onFailure = promise.reject.bind(promise);
const onFailure = promise.reject.bind(promise);

var refresh = function(job) {
var request = config.createStatusRequest(job.id);
const refresh = function(job) {
const request = config.createStatusRequest(job.id);
$.get(request.url, request.data).then(onProgress, onFailure);
};

var onProgress = function(job) {
if (job && job.id && job.state && job.progress) {
if (job.state == 'WAITING') {
promise.notify(job, answerJobQuestion.bind(job));

// Restart the progress if the question timeout is reached
var timeout = job.questionTimeLeft;
if (timeout && timeout > -1) {
setTimeout(function() {
refresh(job);
}, timeout / 1000000); // The JSON contains nanoseconds
function computeNextRefreshInterval(job)
{
let timeout = config.updateInterval || 1000;
// If we are waiting for a question answer and the timeout is earlier than the job status update interval,
// then we should wait for the question timeout instead to ensure a prompt refresh when the question expires.
if (waitingForAnswer && job.questionTimeLeft && job.questionTimeLeft > -1 &&
job.questionTimeLeft / 1000000 <= timeout)
{
timeout = job.questionTimeLeft / 1000000; // The JSON contains nanoseconds
waitingForAnswer = false; // We will refresh the job status after the question timeout
}
return timeout;
}

const onProgress = function(job) {
if (job?.id && job?.state && job?.progress) {
if (job.state === 'WAITING') {
if (!waitingForAnswer) {
promise.notify(job, answerJobQuestion.bind(job));
waitingForAnswer = true;
}
} else {
// The status could have been updated without answering the question, e.g., when the question was answered
// in a different browser tab or when the job was resumed server-side.
waitingForAnswer = false;
// Even if the job is finished we still need to notify the last progress update.
promise.notify(job);
if (job.state == 'FINISHED') {
promise.resolve(job);
} else {
// The job is still running. Wait before asking for a job status update.
setTimeout(function() {
refresh(job);
}, config.updateInterval || 1000);
}
}

if (job.state === 'FINISHED') {
promise.resolve(job);
} else {
// The job is still running. Wait before asking for a job status update.
setTimeout(function () {
refresh(job);
}, computeNextRefreshInterval(job));
}


} else {
promise.resolve(job);
}
Expand All @@ -86,16 +108,16 @@ define(['jquery'], function($) {
*/
return function(config) {
this.resume = function(jobId) {
var promise = $.Deferred();
var callback = createCallback(config, promise);
var request = config.createStatusRequest(jobId);
const promise = $.Deferred();
const callback = createCallback(config, promise);
const request = config.createStatusRequest(jobId);
$.get(request.url, request.data).then(callback.onProgress, callback.onFailure);
return promise;
};

this.run = function(url, data) {
var promise = $.Deferred();
var callback = createCallback(config, promise);
const promise = $.Deferred();
const callback = createCallback(config, promise);
$.post(url, data).then(callback.onProgress, callback.onFailure);
return promise;
};
Expand Down
5 changes: 5 additions & 0 deletions xwiki-platform-core/xwiki-platform-link/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,10 @@
<artifactId>xwiki-platform-model-api</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.xwiki.platform</groupId>
<artifactId>xwiki-platform-store-api</artifactId>
<version>${project.version}</version>
</dependency>
</dependencies>
</project>
Loading

0 comments on commit b16309d

Please sign in to comment.