Skip to content

OAK-11810 resolve UserMonitor lazily #2395

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
joerghoh merged 2 commits into from
Jul 18, 2025
Merged

OAK-11810 resolve UserMonitor lazily #2395

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
519c196
OAK-11810 resolve UserMonitor lazily
joerghoh Jul 18, 2025
cbe9ca9
OAK-11810 move the lamdba into its own private method to reduce Cyclo…
joerghoh Jul 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 28 additions & 9 deletions oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImporter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -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);
}
}

Expand Down
7 changes: 3 additions & 4 deletions ...st/java/org/apache/jackrabbit/oak/security/user/UserImporterMembershipMonitoringTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}