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/20 08:14:41 UTC

[jackrabbit-filevault] branch master updated: JCRVLT-690 Allow to prevent Session.save() and Session.refresh(false) in (#278)

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

kwin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault.git


The following commit(s) were added to refs/heads/master by this push:
     new bf9d2ff2 JCRVLT-690 Allow to prevent Session.save() and Session.refresh(false) in (#278)
bf9d2ff2 is described below

commit bf9d2ff235b2f44b847d6c262e0878507eb4380d
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Mon Feb 20 09:14:35 2023 +0100

    JCRVLT-690 Allow to prevent Session.save() and Session.refresh(false) in (#278)
    
    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);
+    }
 }