You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by kw...@apache.org on 2023/02/19 19:40:47 UTC

[jackrabbit-filevault] branch bugfix/clarify-importer-session-save created (now 4e9795d0)

This is an automated email from the ASF dual-hosted git repository.

kwin pushed a change to branch bugfix/clarify-importer-session-save
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault.git


      at 4e9795d0 JCRVLT-690 Allow to prevent Session.save() and Session.refresh(false) in Importer.run(...)

This branch includes the following new commits:

     new 4e9795d0 JCRVLT-690 Allow to prevent Session.save() and Session.refresh(false) in Importer.run(...)

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[jackrabbit-filevault] 01/01: JCRVLT-690 Allow to prevent Session.save() and Session.refresh(false) in Importer.run(...)

Posted by kw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

kwin pushed a commit to branch bugfix/clarify-importer-session-save
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault.git

commit 4e9795d0c96a19ebba783fe654bddf65e38e34f0
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Sun Feb 19 20:40:41 2023 +0100

    JCRVLT-690 Allow to prevent Session.save() and Session.refresh(false) in
    Importer.run(...)
    
    Clarify javadoc around persisting the session.
---
 .../apache/jackrabbit/vault/fs/io/AutoSave.java    | 19 +++++++++------
 .../jackrabbit/vault/fs/io/ImportOptions.java      | 24 +++++++++++--------
 .../apache/jackrabbit/vault/fs/io/Importer.java    | 27 +++++++++++++---------
 .../jackrabbit/vault/fs/io/AutoSaveTest.java       |  9 ++++++++
 4 files changed, 51 insertions(+), 28 deletions(-)

diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/AutoSave.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/AutoSave.java
index f9c05dd5..f7884342 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/AutoSave.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/AutoSave.java
@@ -17,7 +17,6 @@
 
 package org.apache.jackrabbit.vault.fs.io;
 
-import javax.jcr.InvalidItemStateException;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
 import javax.jcr.nodetype.ConstraintViolationException;
@@ -49,7 +48,8 @@ public class AutoSave {
     private int lastSave;
 
     /**
-     * number of modified nodes that trigger a save. default is 1024
+     * Number of modified nodes that trigger a save. Default is 1024.
+     * When {@link Integer#MAX_VALUE} is used, no save should be triggered at all (neither intermediate nor final)
      */
     private int threshold = 1024;
 
@@ -85,6 +85,10 @@ public class AutoSave {
         this.threshold = threshold;
     }
 
+    boolean isDisabled() {
+        return threshold == Integer.MAX_VALUE;
+    }
+
     public AutoSave copy() {
         AutoSave ret = new AutoSave();
         ret.threshold = threshold;
@@ -137,17 +141,18 @@ public class AutoSave {
     }
 
     /**
-     * saves the changes under the given node and resets the counter
+     * Saves the changes under the given node and resets the counter.
      * @param session the session to save. can be {@code null}
+     * @param isIntermediate {@code false} when this is the final attempt, otherwise {@code true}.
      * @throws RepositoryException if an error occurs.
      */
     public void save(@Nullable Session session, boolean isIntermediate) throws RepositoryException {
+        if (isDisabled()) {
+            log.trace("Save disabled.");
+            return;
+        }
         int diff = numModified - lastSave;
         if (isIntermediate) {
-            if (threshold == Integer.MAX_VALUE) {
-                log.trace("Intermediate save disabled.");
-                return;
-            }
             log.debug("Threshold of {} reached. {} approx {} transient changes.", 
                     threshold,
                     dryRun ? "dry run, reverting" : "saving",
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/ImportOptions.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/ImportOptions.java
index e5c4c491..36fde847 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/ImportOptions.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/ImportOptions.java
@@ -22,6 +22,8 @@ import java.io.IOException;
 import java.util.regex.Pattern;
 import java.util.regex.PatternSyntaxException;
 
+import javax.jcr.Session;
+
 import org.apache.jackrabbit.vault.fs.api.IdConflictPolicy;
 import org.apache.jackrabbit.vault.fs.api.ImportMode;
 import org.apache.jackrabbit.vault.fs.api.PathMapping;
@@ -49,7 +51,7 @@ public class ImportOptions {
 
     private boolean dryRun;
 
-    private int autoSave = -1;
+    private int autoSaveThreshold = -1;
 
     private AccessControlHandling acHandling = null;
 
@@ -92,7 +94,7 @@ public class ImportOptions {
             patchKeepInRepo = base.patchKeepInRepo;
             nonRecursive = base.nonRecursive;
             dryRun = base.dryRun;
-            autoSave = base.autoSave;
+            autoSaveThreshold = base.autoSaveThreshold;
             acHandling = base.acHandling;
             cugHandling = base.cugHandling;
             importMode = base.importMode;
@@ -119,7 +121,7 @@ public class ImportOptions {
         ret.patchKeepInRepo = patchKeepInRepo;
         ret.nonRecursive = nonRecursive;
         ret.dryRun = dryRun;
-        ret.autoSave = autoSave;
+        ret.autoSaveThreshold = autoSaveThreshold;
         ret.acHandling = acHandling;
         ret.cugHandling = cugHandling;
         ret.importMode = importMode;
@@ -342,20 +344,22 @@ public class ImportOptions {
 
     /**
      * Sets the auto-save threshold. See {@link AutoSave}
-     * @param threshold the threshold in number of nodes.
+     * @param autoSaveThreshold the threshold in number of nodes.
      * @since 2.2.16
      */
-    public void setAutoSaveThreshold(int threshold) {
-        this.autoSave = threshold;
+    public void setAutoSaveThreshold(int autoSaveThreshold) {
+        this.autoSaveThreshold = autoSaveThreshold;
     }
 
     /**
      * Returns the auto-save threshold.
+     * If {@link Integer#MAX_VALUE} both {@link Session#save()} and {@link Session#refresh(boolean)} must not be
+     * executed during {@link Importer#run(Archive, Session, String)}.
      * @return the auto-save threshold.
      * @since 2.2.16
      */
     public int getAutoSaveThreshold() {
-        return autoSave;
+        return autoSaveThreshold;
     }
 
     /**
@@ -481,7 +485,7 @@ public class ImportOptions {
         final int prime = 31;
         int result = 1;
         result = prime * result + ((acHandling == null) ? 0 : acHandling.hashCode());
-        result = prime * result + autoSave;
+        result = prime * result + autoSaveThreshold;
         result = prime * result + ((cndPattern == null) ? 0 : cndPattern.hashCode());
         result = prime * result + ((cugHandling == null) ? 0 : cugHandling.hashCode());
         result = prime * result + ((dependencyHandling == null) ? 0 : dependencyHandling.hashCode());
@@ -513,7 +517,7 @@ public class ImportOptions {
         ImportOptions other = (ImportOptions) obj;
         if (acHandling != other.acHandling)
             return false;
-        if (autoSave != other.autoSave)
+        if (autoSaveThreshold != other.autoSaveThreshold)
             return false;
         if (cndPattern == null) {
             if (other.cndPattern != null)
@@ -579,7 +583,7 @@ public class ImportOptions {
         return "ImportOptions [strict=" + strict + ", " + (listener != null ? "listener=" + listener + ", " : "")
                 + (patchParentPath != null ? "patchParentPath=" + patchParentPath + ", " : "")
                 + (patchDirectory != null ? "patchDirectory=" + patchDirectory + ", " : "") + "patchKeepInRepo=" + patchKeepInRepo
-                + ", nonRecursive=" + nonRecursive + ", dryRun=" + dryRun + ", autoSave=" + autoSave + ", "
+                + ", nonRecursive=" + nonRecursive + ", dryRun=" + dryRun + ", autoSave=" + autoSaveThreshold + ", "
                 + (acHandling != null ? "acHandling=" + acHandling + ", " : "")
                 + (cugHandling != null ? "cugHandling=" + cugHandling + ", " : "")
                 + (importMode != null ? "importMode=" + importMode + ", " : "")
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/Importer.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/Importer.java
index a5d0afdd..88316a0c 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/Importer.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/Importer.java
@@ -373,8 +373,8 @@ public class Importer {
     }
 
     /**
-     * Runs the importer
-     *
+     * Runs the importer from the given root node.
+     * Shortcut for {@link #run(Archive, Session, String)} with the session and path from the given node.
      * @param archive the archive to import
      * @param importRoot the root node to import
      *
@@ -391,6 +391,9 @@ public class Importer {
 
     /**
      * Runs the importer with the given session.
+     * {@link Session#save()} and potentially {@link Session#refresh(boolean)} are automatically called during the import
+     * in the given session, except when {@link ImportOptions#setAutoSaveThreshold(int)} with value {@link Integer#MAX_VALUE} has been set.
+     * In all other cases the changes are automatically persisted (potentially in batches) potentially after advanced retry mechanisms.
      *
      * @param archive the archive to import
      * @param session the session importing the archive
@@ -401,7 +404,7 @@ public class Importer {
      *
      * @since 2.7.0
      */
-    public void run(Archive archive, Session session,  String parentPath)
+    public void run(Archive archive, Session session, String parentPath)
             throws IOException, RepositoryException, ConfigurationException {
         this.archive = archive;
 
@@ -509,7 +512,7 @@ public class Importer {
                 autoSave.save(session, false);
                 break;
             } catch (RepositoryException e) {
-                if (recoveryRetryCounter == 10) {
+                if (autoSave.isDisabled() || recoveryRetryCounter == 10) {
                     log.error("Error while committing changes. Aborting.");
                     throw e;
                 } else {
@@ -1169,14 +1172,16 @@ public class Importer {
                 track("U", String.format("%s", authPath));
             }
         }
-        try {
-            session.save();
-        } catch (RepositoryException e) {
-            log.error("Error while updating memberships.", e);
+        if (!autoSave.isDisabled()) {
             try {
-                session.refresh(false);
-            } catch (RepositoryException e1) {
-                // ignore
+                session.save();
+            } catch (RepositoryException e) {
+                log.error("Error while updating memberships.", e);
+                try {
+                    session.refresh(false);
+                } catch (RepositoryException e1) {
+                    // ignore
+                }
             }
         }
         memberships.clear();
diff --git a/vault-core/src/test/java/org/apache/jackrabbit/vault/fs/io/AutoSaveTest.java b/vault-core/src/test/java/org/apache/jackrabbit/vault/fs/io/AutoSaveTest.java
index 6b326d87..37bdd5e7 100644
--- a/vault-core/src/test/java/org/apache/jackrabbit/vault/fs/io/AutoSaveTest.java
+++ b/vault-core/src/test/java/org/apache/jackrabbit/vault/fs/io/AutoSaveTest.java
@@ -84,4 +84,13 @@ public class AutoSaveTest {
         // next regular save after 100 more nodes
         assertTrue(autoSave.needsSave());
     }
+
+    @Test
+    public void testSaveWithMaxThreshold() throws RepositoryException {
+        Mockito.lenient().doThrow(new RepositoryException("Forced exception")).when(session).save();
+        AutoSave autoSave = new AutoSave(Integer.MAX_VALUE);
+        autoSave.modified(2);
+        autoSave.save(session, true);
+        autoSave.save(session, false);
+    }
 }