JCRVLT-697: count moves of stashed child nodes towards autosave limit (#288)
* JCRVLT-697: count stash ops towards autosave limit
* JCRVLT-697: add test coverage
* JCRVLT-697: avoid major version change by adding default impl
* JCRVLT-697: add 'since' annotation to new API
* JCRVLT-697: explain change in the test
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/api/ImportInfo.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/api/ImportInfo.java
index 116d206..bcc9e85 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/api/ImportInfo.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/api/ImportInfo.java
@@ -52,6 +52,15 @@
void onCreated(String path);
/**
+ * Marks that the child node at {@code path} was moved due to stashing.
+ * @param path the path
+ * @since 3.6.10
+ */
+ default void onStashed(String path) {
+ // by default do nothing
+ };
+
+ /**
* Marks that the node at {@code path} was deleted.
* @param path the path
*/
@@ -125,7 +134,7 @@
* @return a new, merged info.
*/
ImportInfo merge(ImportInfo info);
-
+
/**
* returns the number of non-NOP entries.
* @return the number of modified entries.
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/api/package-info.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/api/package-info.java
index 625d01f..f05a7ad 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/api/package-info.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/api/package-info.java
@@ -15,7 +15,7 @@
* limitations under the License.
*/
-@Version("2.11.0")
+@Version("2.12.0")
package org.apache.jackrabbit.vault.fs.api;
import org.osgi.annotation.versioning.Version;
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewImporter.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewImporter.java
index 7ac3f3a..97a01b7 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewImporter.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewImporter.java
@@ -971,7 +971,7 @@
// try to set uuid via sysview import if it differs from existing one
if (identifier.isPresent() && !node.getIdentifier().equals(identifier.get()) && !"rep:root".equals(ni.getPrimaryType().orElse(""))) {
NodeStash stash = new NodeStash(session, node.getPath());
- stash.stash();
+ stash.stash(importInfo);
Node parent = node.getParent();
removeReferences(node);
node.remove();
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/ImportInfoImpl.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/ImportInfoImpl.java
index e09f797..2106cb2 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/ImportInfoImpl.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/ImportInfoImpl.java
@@ -158,6 +158,12 @@
numErrors++;
}
+ @Override
+ public void onStashed(String path) {
+ // path currently unused
+ numModified += 1;
+ }
+
/**
* remembers that a package path was remapped during import. e.g. when the importer follows and existing
* authorizable for MERGE and UPDATE modes.
@@ -211,7 +217,7 @@
public Collection<String> getToVersion() {
return toVersion;
}
-
+
public void registerToVersion(String path) {
toVersion.add(path);
}
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/JcrSysViewTransformer.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/JcrSysViewTransformer.java
index 22b75c8..e7e18a9 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/JcrSysViewTransformer.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/JcrSysViewTransformer.java
@@ -110,7 +110,7 @@
if (existingPath != null) {
// check if there is an existing node with the name
recovery = new NodeStash(session, existingPath).excludeName("rep:cache");
- recovery.stash();
+ recovery.stash(null);
}
excludeNode(NAME_REP_CACHE);
this.importMode = importMode;
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/NodeStash.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/NodeStash.java
index d6a4afc..8629a53 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/NodeStash.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/NodeStash.java
@@ -114,7 +114,7 @@
* Moves the child nodes and optionally properties of the path to a temporary location.
* @return the stashed node's primary type (if it needs to be kept)
*/
- public @Nullable String stash() {
+ public @Nullable String stash(@Nullable ImportInfo importInfo) {
try {
Node parent = session.getNode(path);
Node tmp = getOrCreateTemporaryNode();
@@ -131,8 +131,12 @@
continue;
}
try {
- session.move(child.getPath(), tmp.getPath() + "/" + name);
+ String path = child.getPath();
+ session.move(path, tmp.getPath() + "/" + name);
childNodeCount += 1;
+ if (importInfo != null) {
+ importInfo.onStashed(path);
+ }
} catch (RepositoryException e) {
log.error("Error while moving child node to temporary location. Child will be removed.", e);
}
@@ -183,6 +187,7 @@
Node parent = session.getNode(path);
NodeIterator iter = tmpNode.getNodes();
boolean hasErrors = false;
+
while (iter.hasNext()) {
Node child = iter.nextNode();
String newPath = parent.getPath() + "/" + child.getName();
@@ -190,7 +195,11 @@
if (session.nodeExists(newPath)) {
log.debug("Skipping restore from temporary location {} as node already exists at {}", child.getPath(), newPath);
} else {
- session.move(child.getPath(), newPath);
+ String path = child.getPath();
+ session.move(path, newPath);
+ if (importInfo != null) {
+ importInfo.onStashed(path);
+ }
}
} catch (RepositoryException e) {
log.warn(
diff --git a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/NodeStashingIT.java b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/NodeStashingIT.java
index 4ab759b..1b7d029 100644
--- a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/NodeStashingIT.java
+++ b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/NodeStashingIT.java
@@ -22,11 +22,15 @@
import static org.junit.Assume.assumeTrue;
import java.io.IOException;
+import java.util.LinkedList;
+import java.util.List;
import javax.jcr.Node;
import javax.jcr.RepositoryException;
import org.apache.jackrabbit.vault.fs.api.ImportMode;
+import org.apache.jackrabbit.vault.fs.api.ProgressTrackerListener;
+import org.apache.jackrabbit.vault.fs.api.ProgressTrackerListener.Mode;
import org.apache.jackrabbit.vault.fs.io.ImportOptions;
import org.apache.jackrabbit.vault.packaging.PackageException;
import org.junit.Test;
@@ -65,6 +69,9 @@
// update same path but without mixin allowing child nodes and different
// UUID so that node stashing kicks in
+ Collector col = new Collector();
+ options.setListener(col);
+
extractVaultPackage("/test-packages/stashing/update.zip", options);
// child node should be retained
@@ -79,5 +86,22 @@
// make sure mixin type was restored
assertTrue(node2.isNodeType("{" + TESTNS + "}hasMandatoryChildNode"));
+
+ // before JCRVLT-697, this would have been only one node
+ String expected = "saving approx 3 nodes...";
+ assertTrue("Expected message '" + expected + "' not seen in: " + col.actions, col.actions.contains(expected));
+ }
+
+ private static class Collector implements ProgressTrackerListener {
+ private final List<String> actions = new LinkedList<>();
+
+ public void onMessage(Mode mode, String action, String path) {
+ if (Mode.PATHS == mode) {
+ actions.add(action);
+ }
+ }
+
+ public void onError(Mode mode, String path, Exception e) {
+ }
}
}
\ No newline at end of file
