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

Author: joerghoh

oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImporter.java

@@ -77,6 +77,7 @@
 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 @@ public boolean init(@NotNull Session session, @NotNull Root root, @NotNull NameP
             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 @@ void process() throws RepositoryException {
                 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);
             }
         }
 

oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterMembershipMonitoringTest.java

@@ -26,7 +26,7 @@
 
 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 @@ private static void replaceUserMonitor(@NotNull UserConfigurationImpl uc, @NotNu
     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;
     }
 }