AMBARI-15013. Ldap Sync: Concurrent modification exception (Oliver Szabo via rlevas)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/0868a0fc Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/0868a0fc Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/0868a0fc Branch: refs/heads/branch-dev-patch-upgrade Commit: 0868a0fc1bce5a5d58ac44eba1af2aaa4c161ef6 Parents: 47c8d94 Author: Oliver Szabo <osz...@hortonworks.com> Authored: Tue Feb 16 10:28:27 2016 -0500 Committer: Robert Levas <rle...@hortonworks.com> Committed: Tue Feb 16 10:28:34 2016 -0500 ---------------------------------------------------------------------- .../security/ldap/AmbariLdapDataPopulator.java | 6 ++- .../ldap/AmbariLdapDataPopulatorTest.java | 53 ++++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/0868a0fc/ambari-server/src/main/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulator.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulator.java b/ambari-server/src/main/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulator.java index 801e43e..75df9cc 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulator.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulator.java @@ -38,6 +38,8 @@ import org.apache.ambari.server.security.authorization.User; import org.apache.ambari.server.security.authorization.Users; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; + +import com.google.common.collect.Sets; import org.springframework.ldap.control.PagedResultsDirContextProcessor; import org.springframework.ldap.core.AttributesMapper; import org.springframework.ldap.core.ContextMapper; @@ -293,7 +295,9 @@ public class AmbariLdapDataPopulator { final Map<String, Group> internalGroupsMap = getInternalGroups(); final Map<String, User> internalUsersMap = getInternalUsers(); - for (Group group : internalGroupsMap.values()) { + final Set<Group> internalGroupSet = Sets.newHashSet(internalGroupsMap.values()); + + for (Group group : internalGroupSet) { if (group.isLdapGroup()) { Set<LdapGroupDto> groupDtos = getLdapGroups(group.getGroupName()); if (groupDtos.isEmpty()) { http://git-wip-us.apache.org/repos/asf/ambari/blob/0868a0fc/ambari-server/src/test/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulatorTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulatorTest.java b/ambari-server/src/test/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulatorTest.java index 8ce6c5b..ffff3ea 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulatorTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/security/ldap/AmbariLdapDataPopulatorTest.java @@ -17,6 +17,7 @@ */ package org.apache.ambari.server.security.ldap; +import com.google.common.collect.Sets; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -274,6 +275,58 @@ public class AmbariLdapDataPopulatorTest { } @Test + public void testSynchronizeExistingLdapGroups_removeDuringIteration() throws Exception { + // GIVEN + Group group1 = createNiceMock(Group.class); + expect(group1.getGroupId()).andReturn(1).anyTimes(); + expect(group1.getGroupName()).andReturn("group1").anyTimes(); + expect(group1.isLdapGroup()).andReturn(true).anyTimes(); + + Group group2 = createNiceMock(Group.class); + expect(group2.getGroupId()).andReturn(2).anyTimes(); + expect(group2.getGroupName()).andReturn("group2").anyTimes(); + expect(group2.isLdapGroup()).andReturn(true).anyTimes(); + + Configuration configuration = createNiceMock(Configuration.class); + Users users = createNiceMock(Users.class); + expect(users.getAllGroups()).andReturn(Arrays.asList(group1, group2)); + expect(users.getAllUsers()).andReturn(Collections.EMPTY_LIST); + expect(configuration.getLdapServerProperties()).andReturn(new LdapServerProperties()).anyTimes(); + + Set<LdapGroupDto> groupDtos = Sets.newHashSet(); + LdapGroupDto group1Dto = new LdapGroupDto(); + group1Dto.setGroupName("group1"); + group1Dto.setMemberAttributes(Sets.newHashSet("group2")); + + LdapGroupDto group2Dto = new LdapGroupDto(); + group2Dto.setGroupName("group2"); + group2Dto.setMemberAttributes(Collections.EMPTY_SET); + groupDtos.add(group1Dto); + groupDtos.add(group2Dto); + + LdapBatchDto batchInfo = new LdapBatchDto(); + replay(configuration, users, group1, group2); + AmbariLdapDataPopulator dataPopulator = createMockBuilder(AmbariLdapDataPopulatorTestInstance.class) + .withConstructor(configuration, users) + .addMockedMethod("getLdapGroups") + .addMockedMethod("getLdapUserByMemberAttr") + .addMockedMethod("getLdapGroupByMemberAttr") + .createNiceMock(); + + expect(dataPopulator.getLdapUserByMemberAttr(anyString())).andReturn(null).anyTimes(); + expect(dataPopulator.getLdapGroupByMemberAttr("group2")).andReturn(group2Dto); + expect(dataPopulator.getLdapGroups("group1")).andReturn(groupDtos).anyTimes(); + expect(dataPopulator.getLdapGroups("group2")).andReturn(groupDtos).anyTimes(); + + replay(dataPopulator); + // WHEN + dataPopulator.synchronizeExistingLdapGroups(batchInfo); + // THEN + verify(dataPopulator, group1, group2); + + } + + @Test public void testSynchronizeLdapGroups_allExist() throws Exception { Group group1 = createNiceMock(Group.class);