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/11/14 15:40:10 UTC

(jackrabbit-filevault) branch master updated: JCRVLT-730 Fix race condition in VaultSyncServiceImplIT.testAddRemoveFileFromNonVltCheckoutFolder (#323)

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 39877a52 JCRVLT-730 Fix race condition in VaultSyncServiceImplIT.testAddRemoveFileFromNonVltCheckoutFolder (#323)
39877a52 is described below

commit 39877a52118da3c7f764868a86bb1d452e727e42
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Tue Nov 14 16:40:05 2023 +0100

    JCRVLT-730 Fix race condition in VaultSyncServiceImplIT.testAddRemoveFileFromNonVltCheckoutFolder (#323)
    
    Make sure to have a proper filter file prior to starting the sync service
    
    Increase logging level to debug for Vault sync classes
    Remove JCR observation listener once service is deactivated
    Improved logging for JCR->FS operation and JCR event
    Cleanup tests
---
 .../vault/sync/impl/VaultSyncServiceImplIT.java    | 28 ++++++++++------------
 .../src/main/resources/logback-only-errors.xml     |  1 +
 .../src/main/resources/logback.xml                 |  1 +
 .../jackrabbit/vault/sync/impl/SyncHandler.java    |  5 +++-
 .../vault/sync/impl/VaultSyncServiceImpl.java      |  5 +++-
 5 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/vault-core-it/vault-core-integration-tests/src/main/java/org/apache/jackrabbit/vault/sync/impl/VaultSyncServiceImplIT.java b/vault-core-it/vault-core-integration-tests/src/main/java/org/apache/jackrabbit/vault/sync/impl/VaultSyncServiceImplIT.java
index b993db23..96b88087 100644
--- a/vault-core-it/vault-core-integration-tests/src/main/java/org/apache/jackrabbit/vault/sync/impl/VaultSyncServiceImplIT.java
+++ b/vault-core-it/vault-core-integration-tests/src/main/java/org/apache/jackrabbit/vault/sync/impl/VaultSyncServiceImplIT.java
@@ -16,8 +16,6 @@
  */
 package org.apache.jackrabbit.vault.sync.impl;
 
-import static org.junit.Assert.assertTrue;
-
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.file.Files;
@@ -25,7 +23,6 @@ import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.nio.file.StandardCopyOption;
 import java.nio.file.attribute.FileTime;
-import java.time.Duration;
 import java.util.Calendar;
 import java.util.Collections;
 
@@ -53,19 +50,18 @@ public class VaultSyncServiceImplIT extends IntegrationTestBase {
     public void testAddRemoveFileFromNonVltCheckoutFolder() throws RepositoryException, IOException, InterruptedException {
         Path syncRootDirectory1 = tmpFolder.newFolder().toPath();
         Session newAdminSession = repository.login(new SimpleCredentials("admin", "admin".toCharArray()));
+        // setup .vlt-sync-filter.xml in advance, otherwise changes to it are detected potentially later than the ones to the actual file
+        Path filterFile = syncRootDirectory1.resolve(".vlt-sync-filter.xml");
+        // modify filter
+        try (InputStream input = this.getClass().getResourceAsStream("filter.xml")) {
+            Files.copy(input, filterFile, StandardCopyOption.REPLACE_EXISTING);
+        }
+        // check for file not yet being there in repo
+        assertNodeMissing("/testroot/testfile.txt");
+       
         // create new session
         VaultSyncServiceImpl service = new VaultSyncServiceImpl(newAdminSession, true, 250, Collections.singleton(syncRootDirectory1.toFile()));
         try {
-            Awaitility.setDefaultTimeout(Duration.ofSeconds(20));
-            // verify filter is created automatically
-            Path filterFile = syncRootDirectory1.resolve(".vlt-sync-filter.xml");
-            assertTrue(Files.exists(filterFile));
-            // check for file not yet being there in repo
-            assertNodeMissing("/testroot/testfile.txt");
-            // modify filter
-            try (InputStream input = this.getClass().getResourceAsStream("filter.xml")) {
-                Files.copy(input, filterFile, StandardCopyOption.REPLACE_EXISTING);
-            }
             Path syncDirectory = syncRootDirectory1.resolve("testroot");
             try (InputStream input = this.getClass().getResourceAsStream("testfile1.txt")) {
                 Files.createDirectories(syncDirectory);
@@ -78,6 +74,7 @@ public class VaultSyncServiceImplIT extends IntegrationTestBase {
             });
             Node fileNode = admin.getNode("/testroot/testfile.txt");
             Calendar lastModified1 = JcrUtils.getLastModified(fileNode);
+
             // now change file
             try (InputStream input = this.getClass().getResourceAsStream("testfile2.txt")) {
                 Files.copy(input, syncDirectory.resolve("testfile.txt"), StandardCopyOption.REPLACE_EXISTING);
@@ -113,11 +110,12 @@ public class VaultSyncServiceImplIT extends IntegrationTestBase {
         // create new session
         VaultSyncServiceImpl service = new VaultSyncServiceImpl(newAdminSession, true, 250, Collections.singleton(syncRootDirectory1.toFile()));
         try {
-            // wait a bit
-            Thread.sleep(5000);
+           
             Path syncFile = syncRootDirectory1.resolve(Paths.get("testroot", "testfile"));
             Awaitility.await().until(() -> Files.exists(syncFile));
             FileTime lastModified1 = Files.getLastModifiedTime(syncFile);
+            // wait at least 1 second to really have a different date
+            Thread.sleep(1000);
             // now modify node
             try (InputStream input = this.getClass().getResourceAsStream("testfile2.txt")) {
                 JcrUtils.putFile(testRootNode, "testfile", MimeTypes.APPLICATION_OCTET_STREAM, input);
diff --git a/vault-core-it/vault-core-integration-tests/src/main/resources/logback-only-errors.xml b/vault-core-it/vault-core-integration-tests/src/main/resources/logback-only-errors.xml
index 3b142e39..defc8433 100644
--- a/vault-core-it/vault-core-integration-tests/src/main/resources/logback-only-errors.xml
+++ b/vault-core-it/vault-core-integration-tests/src/main/resources/logback-only-errors.xml
@@ -27,6 +27,7 @@
 
   <logger name="org.apache.jackrabbit.oak.query.QueryImpl" level="WARN"/>
   <logger name="org.apache.jackrabbit.oak.plugins.index.IndexUpdate" level="WARN" />
+  <logger name="org.apache.jackrabbit.vault.sync.impl" level="DEBUG" />
 
   <root level="INFO">
     <appender-ref ref="STDOUT" />
diff --git a/vault-core-it/vault-core-integration-tests/src/main/resources/logback.xml b/vault-core-it/vault-core-integration-tests/src/main/resources/logback.xml
index cefc6b85..9f81398b 100644
--- a/vault-core-it/vault-core-integration-tests/src/main/resources/logback.xml
+++ b/vault-core-it/vault-core-integration-tests/src/main/resources/logback.xml
@@ -23,6 +23,7 @@
 
   <logger name="org.apache.jackrabbit.oak.query.QueryImpl" level="WARN"/>
   <logger name="org.apache.jackrabbit.oak.plugins.index.IndexUpdate" level="WARN" />
+  <logger name="org.apache.jackrabbit.vault.sync.impl" level="DEBUG" />
 
   <root level="INFO">
     <appender-ref ref="STDOUT" />
diff --git a/vault-sync/src/main/java/org/apache/jackrabbit/vault/sync/impl/SyncHandler.java b/vault-sync/src/main/java/org/apache/jackrabbit/vault/sync/impl/SyncHandler.java
index c93c1db0..e173dff6 100644
--- a/vault-sync/src/main/java/org/apache/jackrabbit/vault/sync/impl/SyncHandler.java
+++ b/vault-sync/src/main/java/org/apache/jackrabbit/vault/sync/impl/SyncHandler.java
@@ -271,19 +271,22 @@ public class SyncHandler implements FilesystemAlterationListener {
                 continue;
             }
             File file = getFileForJcrPath(path);
-            log.debug("**** about sync jcr:/{} -> file://{}", path, file.getAbsolutePath());
             Node node;
             Node parentNode;
+            final String operation;
             if (session.nodeExists(path)) {
                 node = session.getNode(path);
                 parentNode = node.getParent();
+                operation = "updated/created";
             } else {
                 node = null;
                 String parentPath = Text.getRelativeParent(path, 1);
                 parentNode = session.nodeExists(parentPath)
                         ? session.getNode(parentPath)
                         : null;
+                operation = "deleted";
             }
+            log.debug("**** about sync jcr:/{} -> file://{} ({})", path, file.getAbsolutePath(), operation);
             TreeSync tree = createTreeSync(SyncMode.JCR2FS);
             res.merge(tree.syncSingle(parentNode, node, file, recursive));
         }
diff --git a/vault-sync/src/main/java/org/apache/jackrabbit/vault/sync/impl/VaultSyncServiceImpl.java b/vault-sync/src/main/java/org/apache/jackrabbit/vault/sync/impl/VaultSyncServiceImpl.java
index c176c360..bbcd4780 100644
--- a/vault-sync/src/main/java/org/apache/jackrabbit/vault/sync/impl/VaultSyncServiceImpl.java
+++ b/vault-sync/src/main/java/org/apache/jackrabbit/vault/sync/impl/VaultSyncServiceImpl.java
@@ -32,6 +32,7 @@ import java.util.stream.Collectors;
 
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
+import javax.jcr.UnsupportedRepositoryOperationException;
 import javax.jcr.observation.Event;
 import javax.jcr.observation.EventIterator;
 import javax.jcr.observation.EventListener;
@@ -144,7 +145,7 @@ public class VaultSyncServiceImpl implements EventListener, Runnable {
     }
 
     @Deactivate
-    protected void deactivate() {
+    protected void deactivate() throws RepositoryException {
         waitLock.lock();
         try {
             enabled = false;
@@ -163,6 +164,7 @@ public class VaultSyncServiceImpl implements EventListener, Runnable {
         }
         // session is still accessed in fsCheckThread, therefore close it last
         if (session != null) {
+            session.getWorkspace().getObservationManager().removeEventListener(this);
             session.logout();
             session = null;
         }
@@ -231,6 +233,7 @@ public class VaultSyncServiceImpl implements EventListener, Runnable {
                 } else {
                     modified.add(path);
                 }
+                log.debug("Received JCR event {} leading to the following modifications: {}", evt, String.join(",", modified));
             }
             waitLock.lock();
             try {