[SYNCOPE-1716] fixed membership hashmap multi-threaded problem with c…

…oncurrent pull (#571)
apache · Dec 10, 2023 · 97d2ecd · 97d2ecd
1 parent dd93c87
commit 97d2ecd
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 17 deletions.
diff --git a/...in/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActions.java b/...in/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActions.java
@@ -19,12 +19,13 @@
 package org.apache.syncope.core.provisioning.java.pushpull;
 
 import java.util.ArrayList;
-import java.util.HashMap;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 import org.apache.syncope.common.lib.request.AnyUR;
 import org.apache.syncope.common.lib.request.MembershipUR;
 import org.apache.syncope.common.lib.request.UserUR;
@@ -78,9 +79,9 @@ public class LDAPMembershipPullActions implements PullActions {
     @Autowired
     protected UserProvisioningManager userProvisioningManager;
 
-    protected final Map<String, Set<String>> membershipsBefore = new HashMap<>();
+    protected final Map<String, Set<String>> membershipsBefore = new ConcurrentHashMap<>();
 
-    protected final Map<String, Set<String>> membershipsAfter = new HashMap<>();
+    protected final Map<String, Set<String>> membershipsAfter = new ConcurrentHashMap<>();
 
     /**
      * Allows easy subclassing for the ConnId AD connector bundle.
@@ -153,11 +154,9 @@ public void beforeUpdate(
         }
 
         groupDAO.findUMemberships(groupDAO.find(entity.getKey())).forEach(uMembership -> {
-            Set<String> memb = membershipsBefore.get(uMembership.getLeftEnd().getKey());
-            if (memb == null) {
-                memb = new HashSet<>();
-                membershipsBefore.put(uMembership.getLeftEnd().getKey(), memb);
-            }
+            Set<String> memb = membershipsBefore.computeIfAbsent(
+                uMembership.getLeftEnd().getKey(),
+                k -> Collections.synchronizedSet(new HashSet<>()));
             memb.add(entity.getKey());
         });
     }
@@ -192,11 +191,9 @@ public void after(
                     profile.getTask().getResource(),
                     profile.getConnector());
             if (match.isPresent()) {
-                Set<String> memb = membershipsAfter.get(match.get().getAny().getKey());
-                if (memb == null) {
-                    memb = new HashSet<>();
-                    membershipsAfter.put(match.get().getAny().getKey(), memb);
-                }
+                Set<String> memb = membershipsAfter.computeIfAbsent(
+                    match.get().getAny().getKey(),
+                    k -> Collections.synchronizedSet(new HashSet<>()));
                 memb.add(entity.getKey());
             } else {
                 LOG.warn("Could not find matching user for {}", membValue);

diff --git a/...ava/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActionsTest.java b/...ava/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActionsTest.java
@@ -25,7 +25,6 @@
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import java.util.HashMap;
@@ -98,7 +97,6 @@ public class LDAPMembershipPullActionsTest extends AbstractTest {
     @Mock
     private ProvisioningReport result;
 
-    @Mock
     private Map<String, Set<String>> membershipsAfter;
 
     @Mock
@@ -139,6 +137,7 @@ public void initTest() {
 
         anyReq = new UserUR();
         membershipsBefore = new HashMap<>();
+        membershipsAfter = new HashMap<>();
         ReflectionTestUtils.setField(ldapMembershipPullActions, "membershipsBefore", membershipsBefore);
         ReflectionTestUtils.setField(ldapMembershipPullActions, "membershipsAfter", membershipsAfter);
 
@@ -208,8 +207,7 @@ public void after() throws JobExecutionException {
 
         ldapMembershipPullActions.after(profile, syncDelta, entity, result);
 
-        verify(membershipsAfter).get(anyString());
-        verify(membershipsAfter).put(anyString(), any());
+        assertEquals(1, membershipsAfter.get(user.getKey()).size());
         assertEquals(expected, attribute.getValue());
     }
 }