From 973c017482b5af30eb0d7f95de65a15553c07585 Mon Sep 17 00:00:00 2001 From: liuyuefei Date: Fri, 8 Dec 2023 23:37:55 +0800 Subject: [PATCH 1/4] fixed membership hashmap bug when task enable concurrent setting fixed membership hashmap bug when task enable concurrent setting fixed membership hashmap bug when task enable concurrent setting --- .../pushpull/LDAPMembershipPullActions.java | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActions.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActions.java index 4db705e3f42..3d06496fbc3 100644 --- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActions.java +++ b/core/provisioning-java/src/main/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> membershipsBefore = new HashMap<>(); + protected final Map> membershipsBefore = new ConcurrentHashMap<>(); - protected final Map> membershipsAfter = new HashMap<>(); + protected final Map> membershipsAfter = new ConcurrentHashMap<>(); /** * Allows easy subclassing for the ConnId AD connector bundle. @@ -155,8 +156,10 @@ public void beforeUpdate( groupDAO.findUMemberships(groupDAO.find(entity.getKey())).forEach(uMembership -> { Set memb = membershipsBefore.get(uMembership.getLeftEnd().getKey()); if (memb == null) { - memb = new HashSet<>(); - membershipsBefore.put(uMembership.getLeftEnd().getKey(), memb); + memb = Collections.synchronizedSet(new HashSet<>()); + memb = Optional.ofNullable(membershipsBefore + .putIfAbsent(uMembership.getLeftEnd().getKey(), memb)) + .orElse(memb); } memb.add(entity.getKey()); }); @@ -194,8 +197,10 @@ public void after( if (match.isPresent()) { Set memb = membershipsAfter.get(match.get().getAny().getKey()); if (memb == null) { - memb = new HashSet<>(); - membershipsAfter.put(match.get().getAny().getKey(), memb); + memb = Collections.synchronizedSet(new HashSet<>()); + memb = Optional.ofNullable(membershipsAfter + .putIfAbsent(match.get().getAny().getKey(), memb)) + .orElse(memb); } memb.add(entity.getKey()); } else { From bb53369d0cac956d7a2940fccb7e70261890780d Mon Sep 17 00:00:00 2001 From: liuyuefei Date: Sat, 9 Dec 2023 18:28:56 +0800 Subject: [PATCH 2/4] use ConcurrentMap computeIfAbsent api --- .../pushpull/LDAPMembershipPullActions.java | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActions.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActions.java index 3d06496fbc3..cde48720da2 100644 --- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActions.java +++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActions.java @@ -154,13 +154,8 @@ public void beforeUpdate( } groupDAO.findUMemberships(groupDAO.find(entity.getKey())).forEach(uMembership -> { - Set memb = membershipsBefore.get(uMembership.getLeftEnd().getKey()); - if (memb == null) { - memb = Collections.synchronizedSet(new HashSet<>()); - memb = Optional.ofNullable(membershipsBefore - .putIfAbsent(uMembership.getLeftEnd().getKey(), memb)) - .orElse(memb); - } + Set memb = membershipsBefore.computeIfAbsent(uMembership.getLeftEnd().getKey(), + k -> Collections.synchronizedSet(new HashSet<>())); memb.add(entity.getKey()); }); } @@ -195,13 +190,9 @@ public void after( profile.getTask().getResource(), profile.getConnector()); if (match.isPresent()) { - Set memb = membershipsAfter.get(match.get().getAny().getKey()); - if (memb == null) { - memb = Collections.synchronizedSet(new HashSet<>()); - memb = Optional.ofNullable(membershipsAfter - .putIfAbsent(match.get().getAny().getKey(), memb)) - .orElse(memb); - } + Set 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); From bde2e7aa0a026fbc9cf1db970f29a4a593abfc07 Mon Sep 17 00:00:00 2001 From: liuyuefei Date: Sun, 10 Dec 2023 00:58:58 +0800 Subject: [PATCH 3/4] fix unit test --- .../java/pushpull/LDAPMembershipPullActions.java | 3 ++- .../java/pushpull/LDAPMembershipPullActionsTest.java | 5 ++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActions.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActions.java index cde48720da2..509ab4c9f6f 100644 --- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActions.java +++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActions.java @@ -154,7 +154,8 @@ public void beforeUpdate( } groupDAO.findUMemberships(groupDAO.find(entity.getKey())).forEach(uMembership -> { - Set memb = membershipsBefore.computeIfAbsent(uMembership.getLeftEnd().getKey(), + Set memb = membershipsBefore.computeIfAbsent( + uMembership.getLeftEnd().getKey(), k -> Collections.synchronizedSet(new HashSet<>())); memb.add(entity.getKey()); }); diff --git a/core/provisioning-java/src/test/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActionsTest.java b/core/provisioning-java/src/test/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActionsTest.java index 26339123e92..4d1cf882487 100644 --- a/core/provisioning-java/src/test/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActionsTest.java +++ b/core/provisioning-java/src/test/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActionsTest.java @@ -98,7 +98,6 @@ public class LDAPMembershipPullActionsTest extends AbstractTest { @Mock private ProvisioningReport result; - @Mock private Map> membershipsAfter; @Mock @@ -139,6 +138,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 +208,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()); } } From de90a78dec623b632d69a81ce5319bd8f3e518b9 Mon Sep 17 00:00:00 2001 From: liuyuefei Date: Sun, 10 Dec 2023 14:40:03 +0800 Subject: [PATCH 4/4] fix checkstyle --- .../java/pushpull/LDAPMembershipPullActionsTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/core/provisioning-java/src/test/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActionsTest.java b/core/provisioning-java/src/test/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActionsTest.java index 4d1cf882487..f89210118a9 100644 --- a/core/provisioning-java/src/test/java/org/apache/syncope/core/provisioning/java/pushpull/LDAPMembershipPullActionsTest.java +++ b/core/provisioning-java/src/test/java/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;