(jackrabbit-oak) branch trunk updated: OAK-11810 resolve UserMonitor lazily (#2395)

[jo...@apache.org]

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

joerghoh pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git


The following commit(s) were added to refs/heads/trunk by this push:
     new aa64fdfc56 OAK-11810 resolve UserMonitor lazily (#2395)
aa64fdfc56 is described below

commit aa64fdfc56cefd0b99b8c3723b70ee43371fd5d1
Author: Jörg Hoh <jo...@users.noreply.github.com>
AuthorDate: Fri Jul 18 21:17:14 2025 +0200

    OAK-11810 resolve UserMonitor lazily (#2395)
    
    * OAK-11810 resolve UserMonitor lazily
    * OAK-11810 move the lamdba into its own private method to reduce Cyclomatic Complexity
---
 .../jackrabbit/oak/security/user/UserImporter.java | 37 ++++++++++++++++------
 .../user/UserImporterMembershipMonitoringTest.java |  7 ++--
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImporter.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImporter.java
index d711e583d9..d209bead16 100644
--- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImporter.java
+++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImporter.java
@@ -77,6 +77,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
+import java.util.function.Supplier;
 
 import static java.util.Objects.requireNonNull;
 import static java.util.concurrent.TimeUnit.NANOSECONDS;
@@ -169,7 +170,8 @@ class UserImporter implements ProtectedPropertyImporter, ProtectedNodeImporter,
      */
     private final Map<String, Principal> principals = new HashMap<>();
 
-    private UserMonitor userMonitor = UserMonitor.NOOP;
+    private UserMonitor userMonitor; // do not access directly, but only via the userMonitorSupplier
+    private Supplier<UserMonitor> userMonitorSupplier;
 
     UserImporter(ConfigurationParameters config) {
         importBehavior = UserUtil.getImportBehavior(config);
@@ -207,18 +209,35 @@ class UserImporter implements ProtectedPropertyImporter, ProtectedNodeImporter,
             return false;
         }
 
-        if (securityProvider instanceof WhiteboardAware) {
-            Whiteboard whiteboard = ((WhiteboardAware) securityProvider).getWhiteboard();
-            UserMonitor monitor = WhiteboardUtils.getService(whiteboard, UserMonitor.class);
-            if (monitor != null) {
-                userMonitor = monitor;
-            }
-        }
+        /*
+         * resolve the userMonitor lazily, as resolving that service via the OSGi service registry
+         * can be costly; this is often a redundant operation, because a UserImporter is typically much more
+         * frequent initialized than actually used.
+         */
+        userMonitorSupplier = getUserMonitorSupplier(securityProvider);
 
         initialized = true;
         return initialized;
     }
 
+    private Supplier<UserMonitor> getUserMonitorSupplier(SecurityProvider securityProvider) {
+        return () -> {
+            if (userMonitor == null) {
+                if (securityProvider instanceof WhiteboardAware) {
+                    Whiteboard whiteboard = ((WhiteboardAware) securityProvider).getWhiteboard();
+                    UserMonitor monitor = WhiteboardUtils.getService(whiteboard, UserMonitor.class);
+                    if (monitor != null) {
+                        userMonitor = monitor;
+                    }
+                }
+                if (userMonitor == null) { // always fall back to the NOOP version
+                    userMonitor = UserMonitor.NOOP;
+                }
+            }
+            return userMonitor;
+        };
+    }
+
     private static boolean canInitUserManager(@NotNull JackrabbitSession session, boolean isWorkspaceImport) {
         try {
             if (!isWorkspaceImport && session.getUserManager().isAutoSave()) {
@@ -647,7 +666,7 @@ class UserImporter implements ProtectedPropertyImporter, ProtectedNodeImporter,
                 memberContentIds.removeAll(failedContentIds);
 
                 userManager.onGroupUpdate(gr, false, true, memberContentIds, failedContentIds);
-                userMonitor.doneUpdateMembers(watch.elapsed(NANOSECONDS), totalSize, failedContentIds.size(), false);
+                userMonitorSupplier.get().doneUpdateMembers(watch.elapsed(NANOSECONDS), totalSize, failedContentIds.size(), false);
             }
         }
 
diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterMembershipMonitoringTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterMembershipMonitoringTest.java
index d3516e456d..7c4525e066 100644
--- a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterMembershipMonitoringTest.java
+++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterMembershipMonitoringTest.java
@@ -26,7 +26,7 @@ import org.junit.After;
 
 import java.lang.reflect.Field;
 
-import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertNull;
 import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.Mockito.atLeastOnce;
@@ -84,11 +84,10 @@ public class UserImporterMembershipMonitoringTest extends UserImporterMembership
     boolean init(boolean createAction, Class<?>... extraInterfaces) throws Exception {
         boolean b = super.init(createAction, extraInterfaces);
 
-        // verify that the usermonitor has been properly initialized and replace it with the spied instance
+        // verify that the userMonitor is not initialized on init, but only on demand
         Field f = UserImporter.class.getDeclaredField("userMonitor");
         f.setAccessible(true);
-        assertSame(userMonitor, f.get(importer));
-
+        assertNull(f.get(importer));
         return b;
     }
 }
\ No newline at end of file