From f6950ad1c9cb0530efb711ccbca91c874d44bab5 Mon Sep 17 00:00:00 2001
From: Konrad Windszus <kwin@apache.org>
Date: Fri, 29 Jul 2022 09:42:43 +0200
Subject: [PATCH] JCRVLT-646 Potential
 javax.jcr.nodetype.ConstraintViolationException while deserializing extended
 file aggregates (#240)

improve logging for failed intermediate saves
fix backoff behaviour (really retry after 10 more nodes)
---
 .../jackrabbit/vault/fs/io/AutoSave.java      | 13 +--
 .../jackrabbit/vault/fs/io/Importer.java      |  2 +-
 .../vault/packaging/integration/ImportIT.java | 21 +++++
 .../META-INF/vault/config.xml                 | 93 +++++++++++++++++++
 .../META-INF/vault/filter.xml                 |  4 +
 .../META-INF/vault/properties.xml             | 13 +++
 .../META-INF/vault/settings.xml               | 21 +++++
 .../jcr_root/testroot/.content.xml            |  9 ++
 .../jcr_root/testroot/tika/config.xml         | 11 +++
 9 files changed, 180 insertions(+), 7 deletions(-)
 create mode 100644 vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/META-INF/vault/config.xml
 create mode 100644 vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/META-INF/vault/filter.xml
 create mode 100644 vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/META-INF/vault/properties.xml
 create mode 100644 vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/META-INF/vault/settings.xml
 create mode 100644 vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/jcr_root/testroot/.content.xml
 create mode 100644 vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/jcr_root/testroot/tika/config.xml

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 69d7109f0..d0536a359 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
@@ -172,11 +172,12 @@ public void save(@Nullable Session session, boolean isIntermediate) throws Repos
                 // either retry after some more nodes have been modified or after throttle 
                 // retry with next save() after another 10 nodes have been modified
                 failedSaveThreshold = 10;
-                log.warn("Retry auto-save after {} modified nodes", failedSaveThreshold);
+                log.warn("Retry auto-save after {} more modified nodes", failedSaveThreshold);
+            } else {
+                lastSave = numModified;
+                failedSaveThreshold = 0;
             }
         }
-        lastSave = numModified;
-        failedSaveThreshold = 0;
     }
 
     /**
@@ -193,7 +194,7 @@ private boolean saveWithBackoff(@NotNull Session session, boolean isIntermediate
                 try {
                     session.save();
                 } catch (RepositoryException e) {
-                    log.error("error during auto save: {} - retrying after refresh...", e.getMessage());
+                    log.error("Error during auto save: {} - retrying after refresh...", e.getMessage());
                     session.refresh(true);
                     session.save();
                 }
@@ -201,8 +202,8 @@ private boolean saveWithBackoff(@NotNull Session session, boolean isIntermediate
             }
         } catch (RepositoryException e) {
             if (isPotentiallyTransientException(e) && isIntermediate) {
-                log.warn("could not auto-save due to potentially transient exception {}", e.getCause());
-                log.debug("auto save exception", e);
+                log.warn("Could not auto-save even after refresh due to potentially transient exception: {}", e.getMessage());
+                log.debug("Auto save exception", e);
                 return false;
             } else {
                 throw e;
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 3bb69fd46..8f0a32e28 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
@@ -844,7 +844,7 @@ private void commit(Session session, TxInfo info, LinkedList<TxInfo> skipList) t
             }
 
             if (autoSave.needsSave()) {
-                autoSave.save(session, false);
+                autoSave.save(session, true); // this is only intermediate
                 // save checkpoint
                 cpTxInfo = info;
                 cpAutosave = autoSave.copy();
diff --git a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/ImportIT.java b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/ImportIT.java
index f439f8c51..3ca7b36b6 100644
--- a/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/ImportIT.java
+++ b/vault-core/src/test/java/org/apache/jackrabbit/vault/packaging/integration/ImportIT.java
@@ -23,13 +23,20 @@
 import java.io.IOException;
 import java.security.Principal;
 
+import javax.jcr.AccessDeniedException;
+import javax.jcr.InvalidItemStateException;
+import javax.jcr.ItemExistsException;
 import javax.jcr.Node;
+import javax.jcr.ReferentialIntegrityException;
 import javax.jcr.Repository;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
 import javax.jcr.SimpleCredentials;
+import javax.jcr.lock.LockException;
 import javax.jcr.nodetype.ConstraintViolationException;
+import javax.jcr.nodetype.NoSuchNodeTypeException;
 import javax.jcr.nodetype.NodeType;
+import javax.jcr.version.VersionException;
 
 import org.apache.commons.lang3.exception.ExceptionUtils;
 import org.apache.jackrabbit.api.JackrabbitSession;
@@ -422,4 +429,18 @@ public void testKeepNodeTypeForFolderAggregate() throws IOException, RepositoryE
         assertNodeHasPrimaryType("/testroot/myfolder", JcrConstants.NT_UNSTRUCTURED);
         assertNodeHasPrimaryType("/testroot/myfolder/mychild", JcrConstants.NT_UNSTRUCTURED);
     }
+
+    @Test
+    public void testEnhancedFileAggregatePackageWithIntermediateSaves() throws IOException, ConfigurationException, AccessDeniedException, ItemExistsException, ReferentialIntegrityException, ConstraintViolationException, InvalidItemStateException, VersionException, LockException, NoSuchNodeTypeException, RepositoryException {
+        ImportOptions opts = getDefaultOptions();
+        opts.setAutoSaveThreshold(1); // auto-save after each deserialized aggregator
+        Importer importer = new Importer(opts);
+        try (Archive archive = getFileArchive("/test-packages/enhanced_file_aggregate.zip")) {
+            archive.open(true);
+            importer.run(archive, admin.getRootNode());
+            admin.save();
+        }
+        assertPropertyExists("/testroot/tika/config.xml/jcr:content/jcr:data");
+        assertProperty("/testroot/tika/config.xml/jcr:content/jcr:mimeType", "text/xml");
+    }
 }
\ No newline at end of file
diff --git a/vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/META-INF/vault/config.xml b/vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/META-INF/vault/config.xml
new file mode 100644
index 000000000..59e3a4a78
--- /dev/null
+++ b/vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/META-INF/vault/config.xml
@@ -0,0 +1,93 @@
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+  -->
+<vaultfs version="1.1">
+    <!--
+        Defines the content aggregation. The order of the defined aggregates
+        is important for finding the correct aggregator.
+    -->
+    <aggregates>
+        <!--
+            Defines an aggregate that handles nt:file and nt:resource nodes.
+        -->
+        <aggregate type="file" title="File Aggregate"/>
+
+        <!--
+            Defines an aggregate that handles file/folder like nodes. It matches
+            all nt:hierarchyNode nodes that have or define a jcr:content
+            child node and excludes child nodes that are nt:hierarchyNodes.
+        -->
+        <aggregate type="filefolder" title="File/Folder Aggregate"/>
+
+        <!--
+            Defines an aggregate that handles nt:nodeType nodes and serializes
+            them into .cnd notation.
+        -->
+        <aggregate type="nodetype" title="Node Type Aggregate" />
+
+        <!--
+            Defines an aggregate that defines full coverage for certain node
+            types that cannot be covered by the default aggregator.
+        -->
+        <aggregate type="full" title="Full Coverage Aggregate">
+            <matches>
+                <include nodeType="rep:AccessControl" respectSupertype="true" />
+                <include nodeType="cq:Widget" respectSupertype="true" />
+                <include nodeType="cq:WidgetCollection" respectSupertype="true" />
+                <include nodeType="cq:EditConfig" respectSupertype="true" />
+                <include nodeType="cq:WorkflowModel" respectSupertype="true" />
+                <include nodeType="vlt:FullCoverage" respectSupertype="true" />
+                <include nodeType="mix:language" respectSupertype="true" />
+                <include nodeType="sling:OsgiConfig" respectSupertype="true" />
+            </matches>
+        </aggregate>
+
+        <!--
+            Defines an aggregate that handles nt:folder like nodes.
+        -->
+        <aggregate type="generic" title="Folder Aggregate">
+            <matches>
+                <include nodeType="nt:folder" respectSupertype="true" />
+            </matches>
+            <contains>
+                <exclude isNode="true" />
+            </contains>
+        </aggregate>
+
+        <!--
+            Defines the default aggregate
+        -->
+        <aggregate type="generic" title="Default Aggregator" isDefault="true">
+            <contains>
+                <exclude nodeType="nt:hierarchyNode" respectSupertype="true" />
+            </contains>
+            <matches>
+                <!-- all -->
+            </matches>
+        </aggregate>
+
+    </aggregates>
+
+    <!--
+      defines the input handlers
+    -->
+    <handlers>
+        <handler type="folder"/>
+        <handler type="file"/>
+        <handler type="nodetype"/>
+        <handler type="generic"/>
+    </handlers>
+</vaultfs>
\ No newline at end of file
diff --git a/vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/META-INF/vault/filter.xml b/vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/META-INF/vault/filter.xml
new file mode 100644
index 000000000..bbbd616d4
--- /dev/null
+++ b/vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/META-INF/vault/filter.xml
@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<workspaceFilter version="1.0">
+    <filter root="/testroot"/>
+</workspaceFilter>
diff --git a/vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/META-INF/vault/properties.xml b/vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/META-INF/vault/properties.xml
new file mode 100644
index 000000000..f57b010d2
--- /dev/null
+++ b/vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/META-INF/vault/properties.xml
@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<!DOCTYPE properties SYSTEM "http://java.sun.com/dtd/properties.dtd">
+<properties>
+<entry key="createdBy">root</entry>
+<entry key="allowIndexDefinitions">true</entry>
+<entry key="name">enhanced-file-aggregate-package</entry>
+<entry key="created">2022-05-26T10:38:55.609Z</entry>
+<entry key="version">1.0.0</entry>
+<entry key="packageType">application</entry>
+<entry key="requiresRoot">false</entry>
+<entry key="group">testpackages</entry>
+<entry key="description">test package containing enhanced file aggregate</entry>
+</properties>
diff --git a/vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/META-INF/vault/settings.xml b/vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/META-INF/vault/settings.xml
new file mode 100644
index 000000000..61c1bea29
--- /dev/null
+++ b/vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/META-INF/vault/settings.xml
@@ -0,0 +1,21 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+  -->
+<vault version="1.0">
+  <ignore name=".svn"/>
+  <ignore name=".DS_Store"/>
+</vault>
\ No newline at end of file
diff --git a/vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/jcr_root/testroot/.content.xml b/vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/jcr_root/testroot/.content.xml
new file mode 100644
index 000000000..48279148b
--- /dev/null
+++ b/vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/jcr_root/testroot/.content.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<jcr:root xmlns:jcr="http://www.jcp.org/jcr/1.0" xmlns:nt="http://www.jcp.org/jcr/nt/1.0"
+          jcr:primaryType="nt:unstructured">
+    <tika jcr:primaryType="nt:unstructured">
+        <config.xml jcr:primaryType="nt:file">
+            <jcr:content jcr:primaryType="nt:resource" jcr:mimeType="text/xml"/>
+        </config.xml>
+    </tika>
+</jcr:root>
diff --git a/vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/jcr_root/testroot/tika/config.xml b/vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/jcr_root/testroot/tika/config.xml
new file mode 100644
index 000000000..b6900ba3a
--- /dev/null
+++ b/vault-core/src/test/resources/test-packages/enhanced_file_aggregate.zip/jcr_root/testroot/tika/config.xml
@@ -0,0 +1,11 @@
+<properties>
+    <detectors>
+        <detector class="org.apache.tika.detect.TypeDetector"/>
+    </detectors>
+    <parsers>
+        <parser class="org.apache.tika.parser.DefaultParser">
+            <mime>text/plain</mime>
+        </parser>
+    </parsers>
+    <service-loader initializableProblemHandler="ignore" dynamic="true"/>
+</properties>
\ No newline at end of file