Skip to content

Commit a86f874

Browse files
boubakersofyenne
authored andcommitted
feat: Enhance Nested Group API - Meeds-io/MIPs#240
Prior to this change, the NestedMemberships was systematically added into the Group DAO while it's not used when reading the Tree of Group inheritance. This is useful only when linking/unlinking a group. Thus a dedicated API Method has been defined to make it possible to access this information.
1 parent ca861d2 commit a86f874

File tree

4 files changed

+65
-72
lines changed

4 files changed

+65
-72
lines changed

component/identity/src/main/java/io/meeds/services/organization/listener/GroupLinkGroupListener.java

Lines changed: 19 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -18,28 +18,24 @@
1818
*/
1919
package io.meeds.services.organization.listener;
2020

21-
import jakarta.annotation.PostConstruct;
22-
import org.apache.commons.collections.CollectionUtils;
23-
import org.exoplatform.services.log.ExoLogger;
24-
import org.exoplatform.services.log.Log;
21+
import java.util.concurrent.CompletableFuture;
22+
23+
import org.springframework.beans.factory.annotation.Autowired;
24+
import org.springframework.stereotype.Component;
25+
2526
import org.exoplatform.services.organization.GroupEventListener;
2627
import org.exoplatform.services.organization.NestedMembership;
2728
import org.exoplatform.services.organization.OrganizationService;
2829
import org.exoplatform.services.security.ConversationRegistry;
2930
import org.exoplatform.services.security.Identity;
3031
import org.exoplatform.services.security.IdentityRegistry;
31-
import org.springframework.beans.factory.annotation.Autowired;
32-
import org.springframework.stereotype.Component;
3332

34-
import java.util.ArrayList;
35-
import java.util.List;
36-
import java.util.concurrent.CompletableFuture;
33+
import jakarta.annotation.PostConstruct;
34+
import lombok.SneakyThrows;
3735

3836
@Component
3937
public class GroupLinkGroupListener extends GroupEventListener {
4038

41-
protected static final Log LOG = ExoLogger.getLogger(GroupLinkGroupListener.class);
42-
4339
@Autowired
4440
private IdentityRegistry identityRegistry;
4541

@@ -64,23 +60,13 @@ public void unlinkGroups(NestedMembership nestedMembership) throws Exception {
6460
CompletableFuture.runAsync(() -> refreshIdentitiesMemberships(nestedMembership));
6561
}
6662

63+
@SneakyThrows
6764
private void refreshIdentitiesMemberships(NestedMembership nestedMembership) {
68-
List<Identity> identities = new ArrayList<>();
69-
try {
70-
identities = identityRegistry.getIdentities()
71-
.stream()
72-
.filter(identity -> hasMatchingMembership(identity, nestedMembership))
73-
.toList();
74-
} catch (Exception exception) {
75-
LOG.error("Error while fetching cached identities", exception);
76-
return;
77-
}
78-
if (CollectionUtils.isNotEmpty(identities)) {
79-
for (Identity identity : identities) {
80-
identityRegistry.unregister(identity.getUserId());
81-
conversationRegistry.unregisterByUserId(identity.getUserId());
82-
}
83-
}
65+
identityRegistry.getIdentities()
66+
.stream()
67+
.filter(identity -> hasMatchingMembership(identity, nestedMembership))
68+
.map(Identity::getUserId)
69+
.forEach(this::clearIdentityCache);
8470
}
8571

8672
private boolean hasMatchingMembership(Identity identity, NestedMembership nestedMembership) {
@@ -93,4 +79,10 @@ private boolean hasMatchingMembership(Identity identity, NestedMembership nested
9379
&& (nestedMembership.isIncludeAllMembershipTypes()
9480
|| m.getMembershipType().equals(nestedMembership.getNestedMembershipType())));
9581
}
82+
83+
private void clearIdentityCache(String userId) {
84+
identityRegistry.unregister(userId);
85+
conversationRegistry.unregisterByUserId(userId);
86+
}
87+
9688
}

component/identity/src/main/java/org/exoplatform/services/organization/idm/GroupDAOImpl.java

Lines changed: 31 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -503,27 +503,9 @@ public Collection<Group> findGroupsOfUser(String user) throws Exception {
503503
}
504504

505505
if (user == null) {
506-
// julien : integration bug
507-
// need to look at that later
508-
//
509-
// Caused by: java.lang.IllegalArgumentException: User name cannot be null
510-
// at
511-
// org.picketlink.idm.impl.api.session.managers.AbstractManager.checkNotNullArgument(AbstractManager.java:267)
512-
// at
513-
// org.picketlink.idm.impl.api.session.managers.RelationshipManagerImpl.findRelatedGroups(RelationshipManagerImpl.java:753)
514-
// at
515-
// org.exoplatform.services.organization.idm.GroupDAOImpl.findGroupsOfUser(GroupDAOImpl.java:225)
516-
// at
517-
// org.exoplatform.organization.webui.component.GroupManagement.isMemberOfGroup(GroupManagement.java:72)
518-
// at
519-
// org.exoplatform.organization.webui.component.GroupManagement.isAdministrator(GroupManagement.java:125)
520-
// at
521-
// org.exoplatform.organization.webui.component.UIGroupExplorer.<init>(UIGroupExplorer.java:57)
522-
523506
if (log.isTraceEnabled()) {
524507
Tools.logMethodOut(log, LogLevel.TRACE, "findGroupsOfUser", Collections.emptyList());
525508
}
526-
527509
return Collections.emptyList();
528510
}
529511

@@ -819,12 +801,10 @@ public Group convertGroup(org.picketlink.idm.api.Group jbidGroup) throws Excepti
819801
Tools.logMethodIn(log, LogLevel.TRACE, "convertGroup", new Object[] { "jbidGroup", jbidGroup });
820802
}
821803

822-
Map<String, Attribute> attrs = new HashMap<String, Attribute>();
823-
804+
Map<String, Attribute> attrs = new HashMap<>();
824805
try {
825806
attrs = getIdentitySession().getAttributesManager().getAttributes(jbidGroup);
826807
} catch (Exception e) {
827-
// TODO:
828808
handleException("Identity operation error: ", e);
829809
}
830810

@@ -864,15 +844,6 @@ public Group convertGroup(org.picketlink.idm.api.Group jbidGroup) throws Excepti
864844
group.setParentId(id.substring(0, id.lastIndexOf("/")));
865845
}
866846

867-
if (attrs.containsKey(NESTED_GROUPS) && attrs.get(NESTED_GROUPS).getValue() != null) {
868-
Set<NestedMembership> nestedMemberships = attrs.get(NESTED_GROUPS)
869-
.getValues()
870-
.stream()
871-
.map(String::valueOf)
872-
.map(m -> NestedMembership.parseNestedMembership(m, id))
873-
.collect(Collectors.toSet());
874-
group.setNestedMemberships(nestedMemberships);
875-
}
876847
if (attrs.containsKey(ENCLOSING_GROUPS) && attrs.get(ENCLOSING_GROUPS).getValue() != null) {
877848
Set<NestedMembership> enclosingMemberships = attrs.get(ENCLOSING_GROUPS)
878849
.getValues()
@@ -894,6 +865,29 @@ public Group convertGroup(org.picketlink.idm.api.Group jbidGroup) throws Excepti
894865
return result;
895866
}
896867

868+
@Override
869+
public Set<NestedMembership> getNestedMemberships(String groupId) {
870+
try {
871+
org.picketlink.idm.api.Group jbidGroup = orgService.getJBIDMGroup(groupId);
872+
if (jbidGroup != null) {
873+
Map<String, Attribute> attrs = getIdentitySession().getAttributesManager().getAttributes(jbidGroup);
874+
if (attrs != null
875+
&& attrs.containsKey(NESTED_GROUPS)
876+
&& attrs.get(NESTED_GROUPS).getValue() != null) {
877+
return attrs.get(NESTED_GROUPS)
878+
.getValues()
879+
.stream()
880+
.map(String::valueOf)
881+
.map(m -> NestedMembership.parseNestedMembership(m, groupId))
882+
.collect(Collectors.toSet());
883+
}
884+
}
885+
} catch (Exception e) {
886+
handleException("Identity operation error: ", e);
887+
}
888+
return Collections.emptySet();
889+
}
890+
897891
/**
898892
* Calculates group id by checking all parents up to the root group or group
899893
* type mapping from the configuration.
@@ -1171,12 +1165,8 @@ public void linkGroups(NestedMembership nestedMembership) throws Exception {
11711165
throw new IllegalStateException("A circular dependency has been detected between groups '%s' and '%s'".formatted(enclosingGroupId,
11721166
nestedGroupId));
11731167
}
1174-
Set<NestedMembership> nestedMemberships = parentGroup.getNestedMemberships();
1175-
if (nestedMemberships == null) {
1176-
nestedMemberships = new HashSet<>();
1177-
} else {
1178-
nestedMemberships = new HashSet<>(nestedMemberships);
1179-
}
1168+
Set<NestedMembership> existingNestedMemberships = getNestedMemberships(enclosingGroupId);
1169+
Set<NestedMembership> nestedMemberships = new HashSet<>(existingNestedMemberships);
11801170
Set<NestedMembership> enclosingMemberships = nestedGroup.getEnclosingMemberships();
11811171
if (enclosingMemberships == null) {
11821172
enclosingMemberships = new HashSet<>();
@@ -1218,9 +1208,9 @@ public void unlinkGroups(NestedMembership nestedMembership) throws Exception {
12181208
boolean updated = false;
12191209
Group parentGroup = findGroupById(nestedMembership.getGroupId());
12201210
if (parentGroup != null) {
1221-
Set<NestedMembership> nestedMemberships = parentGroup.getNestedMemberships();
1222-
if (nestedMemberships != null && nestedMemberships.contains(nestedMembership)) {
1223-
nestedMemberships = new HashSet<>(nestedMemberships);
1211+
Set<NestedMembership> existingNestedMemberships = getNestedMemberships(nestedMembership.getGroupId());
1212+
Set<NestedMembership> nestedMemberships = new HashSet<>(existingNestedMemberships);
1213+
if (nestedMemberships.contains(nestedMembership)) {
12241214
nestedMemberships.remove(nestedMembership);
12251215
updateEnclosingMembershipAttributes(nestedMembership.getGroupId(), nestedMemberships);
12261216
updated = true;
@@ -1250,9 +1240,8 @@ public void addDecoratorPlugin(GroupDecoratorPlugin decoratorPlugin) {
12501240

12511241
@SneakyThrows
12521242
private boolean isNestedIn(String parentGroupId, String memberGroupId) {
1253-
Group group = findGroupById(memberGroupId);
1254-
Set<NestedMembership> nestedMemberships = group == null ? null : group.getNestedMemberships();
1255-
if (group == null || CollectionUtils.isEmpty(nestedMemberships)) {
1243+
Set<NestedMembership> nestedMemberships = getNestedMemberships(memberGroupId);
1244+
if (CollectionUtils.isEmpty(nestedMemberships)) {
12561245
return false;
12571246
} else {
12581247
return nestedMemberships.stream()

component/identity/src/main/java/org/exoplatform/services/organization/idm/PicketLinkIDMOrganizationServiceImpl.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@
4646
import org.exoplatform.services.organization.idm.cache.CacheableUserProfileHandlerImpl;
4747

4848
import io.meeds.services.organization.plugin.GroupDecoratorPlugin;
49+
import io.meeds.services.organization.plugin.UserDecoratorPlugin;
50+
import io.meeds.services.organization.plugin.UserProfileDecoratorPlugin;
4951

5052
/**
5153
* OrganizationService implementation using PicketLink
@@ -307,8 +309,16 @@ public void addDecoratorPlugin(ComponentPlugin plugin) {
307309
((GroupDAOImpl) groupDAO_).addDecoratorPlugin(groupDecoratorPlugin);
308310
organizationCacheHandler.getGroupCache().clearCache();
309311
}
312+
case UserDecoratorPlugin userDecoratorPlugin -> {
313+
((UserDAOImpl) userDAO_).addDecoratorPlugin(userDecoratorPlugin);
314+
organizationCacheHandler.getUserCache().clearCache();
315+
}
316+
case UserProfileDecoratorPlugin userProfileDecoratorPlugin -> {
317+
((UserProfileDAOImpl) userDAO_).addDecoratorPlugin(userProfileDecoratorPlugin);
318+
organizationCacheHandler.getUserProfileCache().clearCache();
319+
}
310320
default -> throw new IllegalArgumentException("Unexpected plugin class: %s".formatted(plugin));
311-
};
321+
}
312322
}
313323

314324
private void initConfiguration(InitParams params) {

component/identity/src/test/java/io/meeds/portal/identity/TestGroupAsMembership.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import java.util.Collection;
2424
import java.util.Set;
2525

26+
import org.apache.commons.collections.CollectionUtils;
27+
2628
import org.exoplatform.component.test.AbstractKernelTest;
2729
import org.exoplatform.component.test.ConfigurationUnit;
2830
import org.exoplatform.component.test.ConfiguredBy;
@@ -82,7 +84,7 @@ public void testLinkGroupAsGroupMember() throws Exception {
8284
.build());
8385

8486
// Then
85-
Set<NestedMembership> updatedNestedMemberships = groupDao.findGroupById(parentGroup.getId()).getNestedMemberships();
87+
Set<NestedMembership> updatedNestedMemberships = groupDao.getNestedMemberships(parentGroup.getId());
8688
assertNotNull(updatedNestedMemberships);
8789
assertTrue(updatedNestedMemberships.stream().anyMatch(m -> m.getNestedGroupId().equals(nestedGroup.getId())));
8890
assertTrue(updatedNestedMemberships.stream()
@@ -98,7 +100,7 @@ public void testLinkGroupAsGroupMember() throws Exception {
98100
.anyMatch(m -> m.getGroupId().equals(parentGroup.getId())
99101
&& m.isIncludeAllMembershipTypes()
100102
&& m.isInheritMembershipType()));
101-
assertNull(groupDao.findGroupById(nestedGroup.getId()).getNestedMemberships());
103+
assertTrue(CollectionUtils.isEmpty(groupDao.getNestedMemberships(nestedGroup.getId())));
102104

103105
memberships = membershipDao.findMembershipsByUser("root", true);
104106
assertTrue(memberships.stream()

0 commit comments

Comments
 (0)