XWIKI-22187: Cannot get revisions with a criteria on a document not stored in DB (#3135)

  * apply the criteria over the archive in cache when possible instead
    of always loading it from database
  * add an integration test to test that there's no regression on
    getRevision API even if the document is not stored
  * Provide some comments
  * Reuse the filtering implementation that is already provided in
    XWikiVersioningStoreInterface
  * Fix unit tests
  * Improve integration test

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/VersionIT.java

@@ -29,9 +29,11 @@
 import org.xwiki.flamingo.skin.test.po.AttachmentsViewPage;
 import org.xwiki.model.reference.AttachmentReference;
 import org.xwiki.model.reference.DocumentReference;
+import org.xwiki.rendering.syntax.Syntax;
 import org.xwiki.rest.model.jaxb.Page;
 import org.xwiki.test.docker.junit5.TestReference;
 import org.xwiki.test.docker.junit5.UITest;
+import org.xwiki.test.integration.junit.LogCaptureConfiguration;
 import org.xwiki.test.ui.TestUtils;
 import org.xwiki.test.ui.po.HistoryPane;
 import org.xwiki.test.ui.po.ViewPage;
@@ -52,7 +54,11 @@
  */
 @UITest(properties = {
     // Add the FileUploadPlugin which is needed by the test to upload attachment files
-    "xwikiCfgPlugins=com.xpn.xwiki.plugin.fileupload.FileUploadPlugin"})
+    "xwikiCfgPlugins=com.xpn.xwiki.plugin.fileupload.FileUploadPlugin",
+    // The script needs PR right.
+    "xwikiPropertiesAdditionalProperties=test.prchecker.excludePattern="
+        + ".*:Test\\.Execute\\..+"
+})
 class VersionIT
 {
     private static final String TITLE = "Page Title";
@@ -645,4 +651,58 @@ void testDeleteVersionDontMessUpRightsWithCancellingEvent(TestUtils setup)
             historyPane.rollbackToVersion(latestVersionBeforeChanges);
         }
     }
+
+    @Test
+    @Order(10)
+    void getRevisionsWithCriteria(TestUtils testUtils, TestReference testReference,
+        LogCaptureConfiguration logCaptureConfiguration) throws Exception
+    {
+        testUtils.rest().delete(testReference);
+        testUtils.rest().savePage(testReference, "Some content", "Title");
+        testUtils.rest().savePage(testReference, "Some content 1", "Title");
+        testUtils.rest().savePage(testReference, "Some content 1 2", "Title");
+        testUtils.rest().savePage(testReference, "Some content 1 2 3", "Title");
+        testUtils.rest().savePage(testReference, "Some content 1 2 3 4", "Title");
+
+        ViewPage viewPage = testUtils.gotoPage(testReference);
+        HistoryPane historyPane = viewPage.openHistoryDocExtraPane();
+        assertEquals("5.1", historyPane.getCurrentVersion());
+        assertEquals(5, historyPane.getNumberOfVersions());
+
+        String currentTestReference = testUtils.serializeReference(testReference);
+        String targetTestReference = "xwiki:Test.getRevisionsWithCriteriaFoo.WebHome";
+        String script = String.format("""
+        {{velocity}}
+        #set ($myTest = "%s")
+        #set ($startAt = 0)
+        #set ($endAt = -1)
+        #set ($criteria = $xwiki.criteriaService.revisionCriteriaFactory.createRevisionCriteria('', $minorVersions))
+        #set ($range = $xwiki.criteriaService.rangeFactory.createRange($startAt, $endAt))
+        #set ($discard = $criteria.setRange($range))
+        #set ($myDoc = $xwiki.getDocument($myTest))
+        #set ($xwikiDoc = $myDoc.document)
+        #set ($discard = $myDoc.document.loadArchive($xcontext.context))
+        XWiki Doc: $xwikiDoc
+        #set ($revisions = $xwikiDoc.getRevisions($criteria, $xcontext.context))
+        Revision: $revisions
+        #set ($newRef = $services.model.resolveDocument("%s"))
+        #set ($discard = $xwikiDoc.setDocumentReference($newRef))
+        XWiki Doc: $xwikiDoc
+        #set ($revisions = $xwikiDoc.getRevisions($criteria, $xcontext.context))
+        Revision: $revisions
+        {{/velocity}}
+        """, currentTestReference, targetTestReference);
+
+        String obtainedResult = testUtils.executeWikiPlain(script, Syntax.XWIKI_2_1);
+        String expectedResult = String.format("""
+        XWiki Doc: %s
+        Revision: [5.1]
+        XWiki Doc: %s
+        Revision: [5.1]""",
+            currentTestReference.substring("xwiki:".length()),
+            targetTestReference.substring("xwiki:".length()));
+        assertEquals(expectedResult, obtainedResult);
+        logCaptureConfiguration.registerExpectedRegexes("^.*\\QDeprecated usage of method "
+            + "[com.xpn.xwiki.doc.XWikiDocument.setDocumentReference] in xwiki:Test.Execute\\E.*$");
+    }
 }

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

@@ -147,11 +147,31 @@ private XWikiDocumentArchive getXWikiDocumentArchive(XWikiDocument doc, Revision
         XWikiContext inputxcontext) throws XWikiException
     {
         XWikiDocumentArchive archiveDoc = doc.getDocumentArchive();
-        // We only retrieve a cached archive if we want a complete one.
-        if (archiveDoc != null && criteria.isAllInclusive()) {
-            return archiveDoc;
+        if (archiveDoc == null) {
+            archiveDoc = getXWikiDocumentArchiveFromDatabase(doc, criteria, inputxcontext);
+        // if there's an archive doc and the criteria is to not return everything then we filter, else we just return
+        } else if (!criteria.isAllInclusive()) {
+            archiveDoc = filterArchiveFromCriteria(doc, archiveDoc, criteria);
         }
+        return archiveDoc;
+    }
 
+    private XWikiDocumentArchive filterArchiveFromCriteria(XWikiDocument doc, XWikiDocumentArchive archiveDoc,
+        RevisionCriteria criteria)
+    {
+        XWikiDocumentArchive result =
+            new XWikiDocumentArchive(doc.getDocumentReference().getWikiReference(), doc.getId());
+        Collection<String> versionsToKeep = filterVersions(archiveDoc, criteria);
+        // We retrieve the actual nodes from the versions we obtained
+        result.setNodes(archiveDoc.getNodes().stream()
+            .filter(node -> versionsToKeep.contains(node.getVersion().toString())).toList());
+        return result;
+    }
+
+    private XWikiDocumentArchive getXWikiDocumentArchiveFromDatabase(XWikiDocument doc, RevisionCriteria criteria,
+        XWikiContext inputxcontext) throws XWikiException
+    {
+        XWikiDocumentArchive archiveDoc = null;
         XWikiContext context = getExecutionXContext(inputxcontext, true);
 
         String db = context.getWikiId();

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

@@ -22,6 +22,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Date;
 import java.util.Iterator;
 import java.util.List;
@@ -69,24 +70,23 @@ private boolean matchCriteria(XWikiRCSNodeInfo nodeinfo, RevisionCriteria criter
         return false;
     }
 
-    private Collection<String> getXWikiDocStringVersions(XWikiDocument doc, RevisionCriteria criteria,
-        XWikiContext context) throws XWikiException
+    default Collection<String> filterVersions(XWikiDocumentArchive archive, RevisionCriteria criteria)
     {
         List<String> results = new ArrayList<>();
 
-        Version[] revisions = getXWikiDocVersions(doc, context);
-        XWikiDocumentArchive archive = getXWikiDocumentArchive(doc, context);
+        List<XWikiRCSNodeInfo> nodes = new ArrayList<>(archive.getNodes());
+        Collections.reverse(nodes);
 
-        Iterator<Version> revisionsIterator = Arrays.stream(revisions).iterator();
-        if (!revisionsIterator.hasNext()) {
+        Iterator<XWikiRCSNodeInfo> nodesIterator = nodes.iterator();
+        if (!nodesIterator.hasNext()) {
             return List.of();
         }
 
-        XWikiRCSNodeInfo nodeinfo = archive.getNode(revisionsIterator.next());
+        XWikiRCSNodeInfo nodeinfo = nodesIterator.next();
         XWikiRCSNodeInfo nextNodeinfo;
 
-        while (revisionsIterator.hasNext()) {
-            nextNodeinfo = archive.getNode(revisionsIterator.next());
+        while (nodesIterator.hasNext()) {
+            nextNodeinfo =nodesIterator.next();
 
             // Minor/Major version matching
             if ((criteria.getIncludeMinorVersions() || !nextNodeinfo.isMinorEdit())
@@ -118,7 +118,7 @@ private Collection<String> getXWikiDocStringVersions(XWikiDocument doc, Revision
     default Collection<Version> getXWikiDocVersions(XWikiDocument doc, RevisionCriteria criteria, XWikiContext context)
         throws XWikiException
     {
-        return getXWikiDocStringVersions(doc, criteria, context).stream()
+        return filterVersions(getXWikiDocumentArchive(doc, context), criteria).stream()
             .map(Version::new)
             .collect(Collectors.toList());
     }
@@ -137,7 +137,7 @@ default Collection<Version> getXWikiDocVersions(XWikiDocument doc, RevisionCrite
     default long getXWikiDocVersionsCount(XWikiDocument doc, RevisionCriteria criteria, XWikiContext context)
         throws XWikiException
     {
-        return getXWikiDocStringVersions(doc, criteria, context).size();
+        return filterVersions(getXWikiDocumentArchive(doc, context), criteria).size();
     }
 
     XWikiDocument loadXWikiDoc(XWikiDocument doc, String version, XWikiContext context) throws XWikiException;

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

@@ -19,6 +19,7 @@
  */
 package com.xpn.xwiki.store;
 
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Date;
@@ -56,7 +57,7 @@
  * @version $Id$
  */
 @ComponentTest
-public class XWikiVersioningStoreInterfaceTest
+class XWikiVersioningStoreInterfaceTest
 {
     @Spy
     private XWikiVersioningStoreInterface versioningStore;
@@ -87,19 +88,14 @@ void setUp() throws XWikiException
             archiveNodes.put((Version) arguments.get()[0], node);
         });
 
-        when(this.archive.getNode(any(Version.class))).thenAnswer(i -> {
-            Version version = i.getArgument(0);
-            return archiveNodes.getOrDefault(version, null);
-        });
-
-        List<Version> versions = archiveNodes.keySet().stream().sorted().collect(Collectors.toList());
-        Collections.reverse(versions);
-        when(this.versioningStore.getXWikiDocVersions(any(), any())).thenReturn(versions.toArray(new Version[0]));
+        List<XWikiRCSNodeInfo> nodes = new ArrayList<>(archiveNodes.values());
+        Collections.sort(nodes);
+        when(this.archive.getNodes()).thenReturn(nodes);
         when(this.versioningStore.getXWikiDocumentArchive(any(), any())).thenReturn(this.archive);
     }
 
     @Test
-    void testGetVersionsDefaultCriteria() throws XWikiException
+    void getVersionsDefaultCriteria() throws XWikiException
     {
         RevisionCriteria criteria = new RevisionCriteria();
         Collection<Version> versions = this.versioningStore.getXWikiDocVersions(null, criteria, null);
@@ -111,7 +107,7 @@ void testGetVersionsDefaultCriteria() throws XWikiException
     }
 
     @Test
-    void testGetVersionsFilterAuthor() throws XWikiException
+    void getVersionsFilterAuthor() throws XWikiException
     {
         RevisionCriteria criteria = new RevisionCriteriaFactory().createRevisionCriteria("Author1", true);
         Collection<Version> versions = this.versioningStore.getXWikiDocVersions(null, criteria, null);
@@ -123,7 +119,7 @@ void testGetVersionsFilterAuthor() throws XWikiException
     }
 
     @Test
-    void testGetVersionsFilterDate() throws XWikiException
+    void getVersionsFilterDate() throws XWikiException
     {
         RevisionCriteria criteria = new RevisionCriteriaFactory().createRevisionCriteria(new Period(1999L, 6001L),
             true);
@@ -136,7 +132,7 @@ void testGetVersionsFilterDate() throws XWikiException
     }
 
     @Test
-    void testGetLastVersion() throws XWikiException
+    void getLastVersion() throws XWikiException
     {
         RevisionCriteria criteria = new RevisionCriteriaFactory().createRevisionCriteria();
         criteria.setRange(RangeFactory.getLAST());