OAK-9369 : UserImporter should obtain UserManager from configuration

git-svn-id: https://svn.apache.org/repos/asf/jackrabbit/oak/trunk@1887145 13f79535-47bb-0310-9956-ffa450edef68
adubrov · Mar 3, 2021 · 1f072b8 · 1f072b8
1 parent 9631c2c
commit 1f072b8
Show file tree

Hide file tree

Showing 6 changed files with 111 additions and 56 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
@@ -16,22 +16,6 @@
  */
 package org.apache.jackrabbit.oak.security.user;
 
-import java.security.Principal;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.TreeSet;
-import javax.jcr.ImportUUIDBehavior;
-import javax.jcr.PropertyType;
-import javax.jcr.RepositoryException;
-import javax.jcr.Session;
-import javax.jcr.nodetype.ConstraintViolationException;
-import javax.jcr.nodetype.PropertyDefinition;
-
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
@@ -43,18 +27,21 @@
 import org.apache.jackrabbit.api.security.user.Group;
 import org.apache.jackrabbit.api.security.user.Impersonation;
 import org.apache.jackrabbit.api.security.user.User;
+import org.apache.jackrabbit.api.security.user.UserManager;
 import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.api.Root;
 import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.namepath.NamePathMapper;
 import org.apache.jackrabbit.oak.plugins.identifier.IdentifierManager;
 import org.apache.jackrabbit.oak.plugins.memory.PropertyStates;
-import org.apache.jackrabbit.oak.plugins.value.jcr.PartialValueFactory;
+import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
+import org.apache.jackrabbit.oak.security.user.autosave.AutoSaveEnabledManager;
 import org.apache.jackrabbit.oak.spi.nodetype.NodeTypeConstants;
 import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
 import org.apache.jackrabbit.oak.spi.security.SecurityProvider;
 import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl;
+import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration;
 import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
 import org.apache.jackrabbit.oak.spi.security.user.util.UserUtil;
 import org.apache.jackrabbit.oak.spi.xml.ImportBehavior;
@@ -64,11 +51,27 @@
 import org.apache.jackrabbit.oak.spi.xml.ProtectedPropertyImporter;
 import org.apache.jackrabbit.oak.spi.xml.ReferenceChangeTracker;
 import org.apache.jackrabbit.oak.spi.xml.TextValue;
-import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import javax.jcr.ImportUUIDBehavior;
+import javax.jcr.PropertyType;
+import javax.jcr.RepositoryException;
+import javax.jcr.Session;
+import javax.jcr.nodetype.ConstraintViolationException;
+import javax.jcr.nodetype.PropertyDefinition;
+import java.security.Principal;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeSet;
+
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 import static org.apache.jackrabbit.oak.api.Type.STRINGS;
@@ -194,7 +197,10 @@ public boolean init(@NotNull Session session, @NotNull Root root, @NotNull NameP
             return false;
         }
 
-        userManager = new UserManagerImpl(root, new PartialValueFactory(namePathMapper), securityProvider);
+        userManager = initUserManager(securityProvider, root, namePathMapper);
+        if (userManager == null) {
+            return false;
+        }
 
         initialized = true;
         return initialized;
@@ -215,6 +221,20 @@ private static boolean canInitUserManager(@NotNull JackrabbitSession session, bo
         return true;
     }
 
+    @Nullable
+    private static UserManagerImpl initUserManager(@NotNull SecurityProvider securityProvider, @NotNull Root root, @NotNull NamePathMapper namePathMapper) {
+        UserManager umgr = securityProvider.getConfiguration(UserConfiguration.class).getUserManager(root, namePathMapper);
+        if (umgr instanceof AutoSaveEnabledManager) {
+            umgr = ((AutoSaveEnabledManager) umgr).unwrap();
+        }
+        if (umgr instanceof UserManagerImpl) {
+            return (UserManagerImpl) umgr;
+        } else {
+            log.error("Unexpected UserManager implementation {}, expected {}", umgr.getClass(), UserManagerImpl.class);
+            return null;
+        }
+    }
+
     // -----------------------------------------< ProtectedPropertyImporter >---
     @Override
     public boolean handlePropInfo(@NotNull Tree parent, @NotNull PropInfo propInfo, @NotNull PropertyDefinition def) throws RepositoryException {

diff --git a/...rc/main/java/org/apache/jackrabbit/oak/security/user/autosave/AutoSaveEnabledManager.java b/...rc/main/java/org/apache/jackrabbit/oak/security/user/autosave/AutoSaveEnabledManager.java
@@ -64,6 +64,10 @@ public AutoSaveEnabledManager(UserManager dlg, Root root) {
         this.root = root;
     }
 
+    public UserManager unwrap() {
+        return dlg;
+    }
+
     @Nullable
     @Override
     public Authorizable getAuthorizable(@NotNull String id) throws RepositoryException {

diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterBaseTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterBaseTest.java
@@ -122,8 +122,12 @@ boolean isWorkspaceImport() {
         return false;
     }
 
-    void init() throws Exception {
-        init(false);
+    boolean isAutosave() {
+        return false;
+    }
+
+    boolean init() throws Exception {
+        return init(false);
     }
 
     boolean init(boolean createAction, Class<?>... extraInterfaces) throws Exception {

diff --git a/...rc/test/java/org/apache/jackrabbit/oak/security/user/UserImporterSessionAutosaveTest.java b/...rc/test/java/org/apache/jackrabbit/oak/security/user/UserImporterSessionAutosaveTest.java
@@ -51,6 +51,11 @@ ProtectedItemImporter.PARAM_IMPORT_BEHAVIOR, getImportBehavior(),
         return ConfigurationParameters.of(UserConfiguration.NAME, userParams);
     }
 
+    @Override
+    boolean isAutosave() {
+        return true;
+    }
+
     @Override
     public void after() throws Exception {
         try {
@@ -70,24 +75,4 @@ boolean init(boolean createAction, Class<?>... extraInterfaces) throws Exception
         getUserManager(root).autoSave(true);
         return b;
     }
-
-    @Test
-    public void testInitImportUUIDBehaviorRemove() throws Exception {
-        assertFalse(importer.init(mockJackrabbitSession(), root, getNamePathMapper(), isWorkspaceImport(), ImportUUIDBehavior.IMPORT_UUID_COLLISION_REMOVE_EXISTING, new ReferenceChangeTracker(), getSecurityProvider()));
-    }
-
-    @Test
-    public void testInitImportUUIDBehaviorReplace() throws Exception {
-        assertFalse(importer.init(mockJackrabbitSession(), root, getNamePathMapper(), isWorkspaceImport(), ImportUUIDBehavior.IMPORT_UUID_COLLISION_REPLACE_EXISTING, new ReferenceChangeTracker(), getSecurityProvider()));
-    }
-
-    @Test
-    public void testInitImportUUIDBehaviorThrow() throws Exception {
-        assertFalse(importer.init(mockJackrabbitSession(), root, getNamePathMapper(), isWorkspaceImport(), ImportUUIDBehavior.IMPORT_UUID_COLLISION_THROW, new ReferenceChangeTracker(), getSecurityProvider()));
-    }
-
-    @Test
-    public void testInitImportUUIDBehaviourCreateNew() throws Exception {
-        assertFalse(importer.init(mockJackrabbitSession(), root, getNamePathMapper(), isWorkspaceImport(), ImportUUIDBehavior.IMPORT_UUID_CREATE_NEW, new ReferenceChangeTracker(), getSecurityProvider()));
-    }
 }
diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterTest.java
@@ -23,15 +23,19 @@
 import org.apache.jackrabbit.api.security.user.AuthorizableExistsException;
 import org.apache.jackrabbit.api.security.user.Group;
 import org.apache.jackrabbit.api.security.user.User;
+import org.apache.jackrabbit.api.security.user.UserManager;
 import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.api.Root;
 import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.commons.PathUtils;
 import org.apache.jackrabbit.oak.namepath.NamePathMapper;
 import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
+import org.apache.jackrabbit.oak.security.user.autosave.AutoSaveEnabledManager;
 import org.apache.jackrabbit.oak.spi.nodetype.NodeTypeConstants;
+import org.apache.jackrabbit.oak.spi.security.SecurityProvider;
 import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
+import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration;
 import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
 import org.apache.jackrabbit.oak.spi.security.user.action.AuthorizableAction;
 import org.apache.jackrabbit.oak.spi.security.user.util.PasswordUtil;
@@ -57,6 +61,7 @@
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.nullable;
+import static org.mockito.Mockito.clearInvocations;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
@@ -79,25 +84,45 @@ public void testInitGetUserManagerFails() throws Exception {
         assertFalse(importer.init(s, root, getNamePathMapper(), false, ImportUUIDBehavior.IMPORT_UUID_COLLISION_THROW, new ReferenceChangeTracker(), getSecurityProvider()));
     }
 
+    @Test
+    public void testInitAutosaveEnabledUserManager() throws Exception {
+        UserManager umgr = new AutoSaveEnabledManager(getUserManager(root), root);
+        SecurityProvider sp = mock(SecurityProvider.class);
+        UserConfiguration uc = when(mock(UserConfiguration.class).getUserManager(root, getNamePathMapper())).thenReturn(umgr).getMock();
+        when(sp.getConfiguration(UserConfiguration.class)).thenReturn(uc);
+
+        assertEquals(!isAutosave(), importer.init(mockJackrabbitSession(), root, getNamePathMapper(), false, ImportUUIDBehavior.IMPORT_UUID_COLLISION_REMOVE_EXISTING, new ReferenceChangeTracker(), sp));
+    }
+
+    @Test
+    public void testInitInvalidUserManager() throws Exception {
+        UserManager umgr = mock(UserManager.class);
+        SecurityProvider sp = mock(SecurityProvider.class);
+        UserConfiguration uc = when(mock(UserConfiguration.class).getUserManager(root, getNamePathMapper())).thenReturn(umgr).getMock();
+        when(sp.getConfiguration(UserConfiguration.class)).thenReturn(uc);
+
+        assertFalse(importer.init(mockJackrabbitSession(), root, getNamePathMapper(), false, ImportUUIDBehavior.IMPORT_UUID_COLLISION_REMOVE_EXISTING, new ReferenceChangeTracker(), sp));
+    }
+
     @Test(expected = IllegalStateException.class)
     public void testInitAlreadyInitialized() throws Exception {
-        init();
+        assertTrue(init());
         importer.init(mockJackrabbitSession(), root, getNamePathMapper(), isWorkspaceImport(), ImportUUIDBehavior.IMPORT_UUID_COLLISION_REMOVE_EXISTING, new ReferenceChangeTracker(), getSecurityProvider());
     }
 
     @Test
     public void testInitImportUUIDBehaviorRemove() throws Exception {
-        assertTrue(importer.init(mockJackrabbitSession(), root, getNamePathMapper(), isWorkspaceImport(), ImportUUIDBehavior.IMPORT_UUID_COLLISION_REMOVE_EXISTING, new ReferenceChangeTracker(), getSecurityProvider()));
+        assertEquals(!isAutosave(), importer.init(mockJackrabbitSession(), root, getNamePathMapper(), isWorkspaceImport(), ImportUUIDBehavior.IMPORT_UUID_COLLISION_REMOVE_EXISTING, new ReferenceChangeTracker(), getSecurityProvider()));
     }
 
     @Test
     public void testInitImportUUIDBehaviorReplace() throws Exception {
-        assertTrue(importer.init(mockJackrabbitSession(), root, getNamePathMapper(), isWorkspaceImport(), ImportUUIDBehavior.IMPORT_UUID_COLLISION_REPLACE_EXISTING, new ReferenceChangeTracker(), getSecurityProvider()));
+        assertEquals(!isAutosave(), importer.init(mockJackrabbitSession(), root, getNamePathMapper(), isWorkspaceImport(), ImportUUIDBehavior.IMPORT_UUID_COLLISION_REPLACE_EXISTING, new ReferenceChangeTracker(), getSecurityProvider()));
     }
 
     @Test
     public void testInitImportUUIDBehaviorThrow() throws Exception {
-        assertTrue(importer.init(mockJackrabbitSession(), root, getNamePathMapper(), isWorkspaceImport(), ImportUUIDBehavior.IMPORT_UUID_COLLISION_THROW, new ReferenceChangeTracker(), getSecurityProvider()));
+        assertEquals(!isAutosave(), importer.init(mockJackrabbitSession(), root, getNamePathMapper(), isWorkspaceImport(), ImportUUIDBehavior.IMPORT_UUID_COLLISION_THROW, new ReferenceChangeTracker(), getSecurityProvider()));
     }
 
     @Test
@@ -113,7 +138,7 @@ public void testHandlePropInfoNotInitialized() throws Exception {
 
     @Test
     public void testHandlePropInfoParentNotAuthorizable() throws Exception {
-        init();
+        assertTrue(init());
         assertFalse(importer.handlePropInfo(root.getTree(PathUtils.ROOT_PATH), mock(PropInfo.class), mock(PropertyDefinition.class)));
     }
 
@@ -128,14 +153,14 @@ public void testHandleAuthorizableId() throws Exception {
 
     @Test(expected = ConstraintViolationException.class)
     public void testHandleAuthorizableIdMismatch() throws Exception {
-        init();
+        assertTrue(init());
         Tree userTree = createUserTree();
         importer.handlePropInfo(userTree, createPropInfo(REP_AUTHORIZABLE_ID, "mismatch"), mockPropertyDefinition(NT_REP_AUTHORIZABLE, false));
     }
 
     @Test(expected = AuthorizableExistsException.class)
     public void testHandleAuthorizableIdConflictExisting() throws Exception {
-        init();
+        assertTrue(init());
         Tree userTree = createUserTree();
         importer.handlePropInfo(userTree, createPropInfo(REP_AUTHORIZABLE_ID, testUser.getID()), mockPropertyDefinition(NT_REP_AUTHORIZABLE, false));
     }
@@ -173,14 +198,14 @@ public void testHandlePrincipalName() throws Exception {
 
     @Test(expected = IllegalArgumentException.class)
     public void testHandleEmptyPrincipalName() throws Exception {
-        init();
+        assertTrue(init());
         Tree userTree = createUserTree();
         importer.handlePropInfo(userTree, createPropInfo(REP_PRINCIPAL_NAME, ""), mockPropertyDefinition(NT_REP_AUTHORIZABLE, false));
     }
 
     @Test(expected = IllegalArgumentException.class)
     public void testHandleEveryonePrincipalNameOnUser() throws Exception {
-        init();
+        assertTrue(init());
         Tree userTree = createUserTree();
         importer.handlePropInfo(userTree, createPropInfo(REP_PRINCIPAL_NAME, EveryonePrincipal.NAME), mockPropertyDefinition(NT_REP_AUTHORIZABLE, false));
     }
@@ -439,7 +464,7 @@ public void testPropertiesCompletedClearsCache() throws Exception {
 
     @Test
     public void testPropertiesCompletedParentNotAuthorizable() throws Exception {
-        init();
+        assertTrue(init());
         importer.propertiesCompleted(root.getTree("/"));
     }
 
@@ -618,7 +643,7 @@ public void testStartChildInfoWithoutRepMembersProperty() throws Exception {
         Tree memberRefList = groupTree.addChild(REP_MEMBERS_LIST);
         memberRefList.setProperty(JcrConstants.JCR_PRIMARYTYPE, NT_REP_MEMBER_REFERENCES_LIST);
 
-        importer.start(memberRefList);
+        assertTrue(importer.start(memberRefList));
         importer.startChildInfo(createNodeInfo("memberRef", NT_REP_MEMBER_REFERENCES), ImmutableList.of());
     }
 
@@ -629,7 +654,7 @@ public void testStartChildInfoWithRepMembersProperty() throws Exception {
         Tree memberRefList = groupTree.addChild(REP_MEMBERS_LIST);
         memberRefList.setProperty(JcrConstants.JCR_PRIMARYTYPE, NT_REP_MEMBER_REFERENCES_LIST);
 
-        importer.start(memberRefList);
+        assertTrue(importer.start(memberRefList));
         importer.startChildInfo(createNodeInfo("memberRef", NT_REP_MEMBER_REFERENCES), ImmutableList.of(createPropInfo(REP_MEMBERS, "member1")));
     }
 
@@ -656,7 +681,7 @@ public void testStartRepMembersChildInfo() throws Exception {
 
         Tree repMembers = groupTree.addChild("memberTree");
         repMembers.setProperty(JcrConstants.JCR_PRIMARYTYPE, NT_REP_MEMBERS);
-        importer.start(repMembers);
+        assertTrue(importer.start(repMembers));
         importer.startChildInfo(createNodeInfo("memberTree", NT_REP_MEMBERS), ImmutableList.of(createPropInfo("anyProp", "memberValue")));
     }
 
@@ -667,14 +692,25 @@ public void testStartOtherChildInfo() throws Exception {
         Tree memberRefList = groupTree.addChild(REP_MEMBERS_LIST);
         memberRefList.setProperty(JcrConstants.JCR_PRIMARYTYPE, NT_REP_MEMBER_REFERENCES_LIST);
 
-        importer.start(memberRefList);
+        assertTrue(importer.start(memberRefList));
         importer.startChildInfo(createNodeInfo("memberRef", NT_OAK_UNSTRUCTURED), ImmutableList.of(createPropInfo(REP_MEMBERS, "member1")));
     }
 
     //-------------------------------------------------------< endChildInfo >---
     @Test
-    public void testEndChildInfoIsNoop() {
+    public void testEndChildInfoIsNoop() throws Exception {
+        Root r = mock(Root.class);
+        NamePathMapper npmapper = mock(NamePathMapper.class);
+        ReferenceChangeTracker reftracker = mock(ReferenceChangeTracker.class);
+        SecurityProvider sp = mock(SecurityProvider.class);
+        UserConfiguration uc = when(mock(UserConfiguration.class).getUserManager(r, npmapper)).thenReturn(getUserManager(root)).getMock();
+        when(sp.getConfiguration(UserConfiguration.class)).thenReturn(uc);
+
+        importer.init(mockJackrabbitSession(), r, npmapper, false, ImportUUIDBehavior.IMPORT_UUID_COLLISION_REMOVE_EXISTING, reftracker, sp);
+        clearInvocations(r, npmapper, reftracker, sp);
+
         importer.endChildInfo();
+        verifyNoInteractions(r, npmapper, reftracker, sp);
     }
 
     //----------------------------------------------------------------< end >---

diff --git a/...est/java/org/apache/jackrabbit/oak/security/user/autosave/AutoSaveEnabledManagerTest.java b/...est/java/org/apache/jackrabbit/oak/security/user/autosave/AutoSaveEnabledManagerTest.java
@@ -36,6 +36,7 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
@@ -352,4 +353,9 @@ public void testRemoveMembers() throws Exception {
         assertTrue(g.removeMembers(u.getID()).isEmpty());
         assertFalse(root.hasPendingChanges());
     }
+
+    @Test
+    public void testUnwrap() {
+        assertSame(mgrDlg, autosaveMgr.unwrap());
+    }
 }