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:48 UTC
[jackrabbit-filevault] 01/01: JCRVLT-690 Allow to prevent Session.save() and Session.refresh(false) in Importer.run(...)
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);
+ }
}