From f843e98bab30cbf96144862e54def7891d225126 Mon Sep 17 00:00:00 2001
From: Thomas Mortagne <thomas.mortagne@gmail.com>
Date: Tue, 16 Jan 2024 16:33:28 +0100
Subject: [PATCH] XWIKI-21801: Duplicate versions in history of documents not
 based on JRCS Co-authored-by: trrenty <trrenty@gmail.com>

---
 .../DocumentInstanceOutputFilterStream.java   |  11 --
 .../com/xpn/xwiki/test/MockitoOldcore.java    | 149 ++++++++++--------
 .../filter/documentwithattachment1content.xml |   9 +-
 .../filter/documentwithattachment2content.xml |   9 +-
 4 files changed, 100 insertions(+), 78 deletions(-)

diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/filter/output/DocumentInstanceOutputFilterStream.java b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/filter/output/DocumentInstanceOutputFilterStream.java
index d8e6a5e2b41..ec2ed1c2e34 100644
--- a/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/filter/output/DocumentInstanceOutputFilterStream.java
+++ b/xwiki-platform-core/xwiki-platform-oldcore/src/main/java/com/xpn/xwiki/internal/filter/output/DocumentInstanceOutputFilterStream.java
@@ -46,7 +46,6 @@
 import com.xpn.xwiki.doc.XWikiAttachment;
 import com.xpn.xwiki.doc.XWikiDocument;
 import com.xpn.xwiki.doc.XWikiDocumentArchive;
-import com.xpn.xwiki.store.XWikiVersioningStoreInterface;
 
 /**
  * @version $Id$
@@ -252,16 +251,6 @@ private void maybeSaveDocument() throws FilterException
 
                 xcontext.getWiki().saveDocument(document, inputDocument.getComment(), inputDocument.isMinorEdit(),
                     xcontext);
-
-                if (!hasJRCSHistory) {
-                    // Not a JRCS based history document
-                    // Explicitly update the history because the store won't do it automatically (because
-                    // metadata/content dirty is false)
-                    XWikiVersioningStoreInterface versioningStore = document.getVersioningStore(xcontext);
-                    if (versioningStore != null) {
-                        versioningStore.updateXWikiDocArchive(document, true, xcontext);
-                    }
-                }
             } else {
                 // Forget the input history to let the store do its standard job
                 document.setDocumentArchive((XWikiDocumentArchive) null);
diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/test/MockitoOldcore.java b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/test/MockitoOldcore.java
index 44ff8f2c001..99cab4371cd 100644
--- a/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/test/MockitoOldcore.java
+++ b/xwiki-platform-core/xwiki-platform-oldcore/src/test/java/com/xpn/xwiki/test/MockitoOldcore.java
@@ -695,35 +695,7 @@ public Void answer(InvocationOnMock invocation) throws Throwable
                     reference = reference.setWikiReference(xcontext.getWikiReference());
                 }
 
-                if (document.isContentDirty() || document.isMetaDataDirty()) {
-                    document.setDate(new Date());
-                    if (document.isContentDirty()) {
-                        document.setContentUpdateDate(new Date());
-                        document.setContentAuthorReference(document.getAuthorReference());
-                    }
-                    document.incrementVersion();
-
-                    document.setContentDirty(false);
-                    document.setMetaDataDirty(false);
-
-                    if (supportRevisionStore) {
-                        // Save the document in the document archive.
-                        getMockVersioningStore().updateXWikiDocArchive(document, true, xcontext);
-                    }
-                }
-                document.setNew(false);
-                document.setStore(getMockStore());
-
-                // Make sure the document is not restricted.
-                document.setRestricted(false);
-
-                XWikiDocument savedDocument = document.clone();
-
-                documents.put(document.getDocumentReferenceWithLocale(), savedDocument);
-
-
-                // Set the document as it's original document
-                savedDocument.setOriginalDocument(savedDocument.clone());
+                saveDocument(reference, document, xcontext);
 
                 return null;
             }
@@ -881,27 +853,6 @@ public Void answer(InvocationOnMock invocation) throws Throwable
                     document.setComment(StringUtils.defaultString(comment));
                     document.setMinorEdit(minorEdit);
 
-                    if (document.isContentDirty() || document.isMetaDataDirty()) {
-                        Date ndate = new Date();
-                        document.setDate(ndate);
-                        if (document.isContentDirty()) {
-                            document.setContentUpdateDate(ndate);
-                            DocumentAuthors authors = document.getAuthors();
-                            authors.setContentAuthor(authors.getEffectiveMetadataAuthor());
-                        }
-                        document.incrementVersion();
-
-                        document.setContentDirty(false);
-                        document.setMetaDataDirty(false);
-
-                        // Save the document in the document archive.
-                        if (supportRevisionStore) {
-                            getMockVersioningStore().updateXWikiDocArchive(document, true, xcontext);
-                        }
-                    }
-                    document.setNew(false);
-                    document.setStore(getMockStore());
-
                     XWikiDocument previousDocument = documents.get(document.getDocumentReferenceWithLocale());
 
                     if (previousDocument != null && previousDocument != document) {
@@ -920,28 +871,28 @@ public Void answer(InvocationOnMock invocation) throws Throwable
                         document.setOriginalDocument(originalDocument);
                     }
 
-                    // Make sure the document is not restricted.
-                    document.setRestricted(false);
+                    saveDocument(document.getDocumentReferenceWithLocale(), document, xcontext);
 
-                    XWikiDocument savedDocument = document.clone();
+                    XWikiDocument newOriginal = document.getOriginalDocument();
 
-                    documents.put(document.getDocumentReferenceWithLocale(), savedDocument);
+                    try {
+                        document.setOriginalDocument(originalDocument);
 
-                    if (isNew) {
                         if (notifyDocumentCreatedEvent) {
-                            getObservationManager().notify(new DocumentCreatedEvent(document.getDocumentReference()),
-                                document, getXWikiContext());
-                        }
-                    } else {
-                        if (notifyDocumentUpdatedEvent) {
-                            getObservationManager().notify(new DocumentUpdatedEvent(document.getDocumentReference()),
-                                document, getXWikiContext());
+                            if (isNew) {
+                                getObservationManager().notify(
+                                    new DocumentCreatedEvent(document.getDocumentReference()), document,
+                                    getXWikiContext());
+                            } else {
+                                getObservationManager().notify(
+                                    new DocumentUpdatedEvent(document.getDocumentReference()), document,
+                                    getXWikiContext());
+                            }
                         }
+                    } finally {
+                        document.setOriginalDocument(newOriginal);
                     }
 
-                    // Set the document as it's original document
-                    savedDocument.setOriginalDocument(savedDocument.clone());
-
                     return null;
                 }
             }).when(getSpyXWiki()).saveDocument(anyXWikiDocument(), any(String.class), anyBoolean(), anyXWikiContext());
@@ -1181,6 +1132,74 @@ public String answer(InvocationOnMock invocation) throws Throwable
         }
     }
 
+    private void saveDocument(DocumentReference reference, XWikiDocument document, XWikiContext xcontext)
+        throws XWikiException
+    {
+        boolean supportRevisionStore = this.componentManager.hasComponent(XWikiDocumentFilterUtils.class);
+
+        if (document.isContentDirty() || document.isMetaDataDirty()) {
+            document.setDate(new Date());
+            if (document.isContentDirty()) {
+                document.setContentUpdateDate(new Date());
+                DocumentAuthors authors = document.getAuthors();
+                authors.setContentAuthor(authors.getEffectiveMetadataAuthor());
+            }
+            document.incrementVersion();
+
+            document.setContentDirty(false);
+            document.setMetaDataDirty(false);
+
+            if (supportRevisionStore) {
+                // Save the document in the document archive.
+                getMockVersioningStore().updateXWikiDocArchive(document, true, xcontext);
+            }
+        } else {
+            if (supportRevisionStore) {
+                if (document.getDocumentArchive() != null) {
+                    getMockVersioningStore().saveXWikiDocArchive(document.getDocumentArchive(), false, xcontext);
+
+                    if (!containsVersion(document, document.getRCSVersion(), xcontext)) {
+                        getMockVersioningStore().updateXWikiDocArchive(document, false, xcontext);
+                    }
+                } else {
+                    try {
+                        document.getDocumentArchive(xcontext);
+
+                        if (!containsVersion(document, document.getRCSVersion(), xcontext)) {
+                            getMockVersioningStore().updateXWikiDocArchive(document, false, xcontext);
+                        }
+                    } catch (XWikiException e) {
+                        // this is a non critical error
+                    }
+                }
+            }
+        }
+        document.setNew(false);
+        document.setStore(getMockStore());
+
+        // Make sure the document is not restricted.
+        document.setRestricted(false);
+
+        XWikiDocument savedDocument = document.clone();
+
+        documents.put(document.getDocumentReferenceWithLocale(), savedDocument);
+
+        // Set the document as it's original document
+        savedDocument.setOriginalDocument(savedDocument.clone());
+    }
+
+    private boolean containsVersion(XWikiDocument doc, Version targetversion, XWikiContext context)
+        throws XWikiException
+    {
+        for (Version version : doc.getRevisions(context)) {
+            if (version.equals(targetversion)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     protected DocumentReference resolveDocument(String documentName) throws ComponentLookupException
     {
         DocumentReferenceResolver<String> resolver =
diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/test/resources/filter/documentwithattachment1content.xml b/xwiki-platform-core/xwiki-platform-oldcore/src/test/resources/filter/documentwithattachment1content.xml
index 28c6d435927..3d5891effb6 100644
--- a/xwiki-platform-core/xwiki-platform-oldcore/src/test/resources/filter/documentwithattachment1content.xml
+++ b/xwiki-platform-core/xwiki-platform-oldcore/src/test/resources/filter/documentwithattachment1content.xml
@@ -36,7 +36,14 @@
                 </entry>
                 <entry>
                   <string>syntax</string>
-                  <null/>
+                  <org.xwiki.rendering.syntax.Syntax>
+                    <type>
+                      <name>XWiki</name>
+                      <id>xwiki</id>
+                      <variants class="empty-list"/>
+                    </type>
+                    <version>2.1</version>
+                  </org.xwiki.rendering.syntax.Syntax>
                 </entry>
                 <entry>
                   <string>hidden</string>
diff --git a/xwiki-platform-core/xwiki-platform-oldcore/src/test/resources/filter/documentwithattachment2content.xml b/xwiki-platform-core/xwiki-platform-oldcore/src/test/resources/filter/documentwithattachment2content.xml
index 40393fe4b7c..77e524ef589 100644
--- a/xwiki-platform-core/xwiki-platform-oldcore/src/test/resources/filter/documentwithattachment2content.xml
+++ b/xwiki-platform-core/xwiki-platform-oldcore/src/test/resources/filter/documentwithattachment2content.xml
@@ -36,7 +36,14 @@
                 </entry>
                 <entry>
                   <string>syntax</string>
-                  <null/>
+                  <org.xwiki.rendering.syntax.Syntax>
+                    <type>
+                      <name>XWiki</name>
+                      <id>xwiki</id>
+                      <variants class="empty-list"/>
+                    </type>
+                    <version>2.1</version>
+                  </org.xwiki.rendering.syntax.Syntax>
                 </entry>
                 <entry>
                   <string>hidden</string>
-- 
GitLab