update cache implementation for groups to allow for handling of searc…

…hes in an efficient manner before attempting to load a full resource and calculating a cache-miss
alice-wondered · Aug 21, 2023 · e8116ad · e8116ad
1 parent dcf50c1
commit e8116ad
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 10 deletions.
diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/GroupAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/GroupAdapter.java
@@ -17,6 +17,8 @@
 
 package org.keycloak.models.cache.infinispan;
 
+import java.util.Map.Entry;
+import java.util.stream.Collectors;
 import org.keycloak.models.ClientModel;
 import org.keycloak.models.GroupModel;
 import org.keycloak.models.KeycloakSession;
@@ -224,33 +226,41 @@ public String getParentId() {
     public Stream<GroupModel> getSubGroupsStream() {
         if (isUpdated()) return updated.getSubGroupsStream();
         Set<GroupModel> subGroups = new HashSet<>();
-        for (String id : cached.getSubGroups(modelSupplier)) {
-            GroupModel subGroup = keycloakSession.groups().getGroupById(realm, id);
+
+        for (Map.Entry<String, String> entry : cached.getSubGroups(modelSupplier).entrySet()) {
+            GroupModel subGroup = keycloakSession.groups().getGroupById(realm, entry.getValue());
             if (subGroup == null) {
                 // chance that role was removed, so just delegate to persistence and get user invalidated
                 getDelegateForUpdate();
                 return updated.getSubGroupsStream();
-
             }
             subGroups.add(subGroup);
         }
-        return subGroups.stream().sorted(GroupModel.COMPARE_BY_NAME);
+        return subGroups.stream();
     }
 
     @Override
     public Stream<GroupModel> getSubGroupsStream(String search, Integer firstResult, Integer maxResults) {
         if (isUpdated()) return updated.getSubGroupsStream(search, firstResult, maxResults);
         Set<GroupModel> subGroups = new HashSet<>();
-        for (String id : cached.getSubGroups(modelSupplier)) {
+
+        Set<String> groupIds = cached.getSubGroups(modelSupplier).entrySet().stream()
+            .filter(entry -> entry.getKey().contains(search))
+            .skip(firstResult)
+            .limit(maxResults)
+            .map(Entry::getValue)
+            .collect(Collectors.toSet());
+
+        for (String id : groupIds) {
             GroupModel subGroup = keycloakSession.groups().getGroupById(realm, id);
             if (subGroup == null) {
                 // chance that role was removed, so just delegate to persistence and get user invalidated
                 getDelegateForUpdate();
-                return updated.getSubGroupsStream(search, firstResult, maxResults);
+                return updated.getSubGroupsStream();
             }
             subGroups.add(subGroup);
         }
-        return subGroups.stream().sorted(GroupModel.COMPARE_BY_NAME);
+        return subGroups.stream();
     }
 
 

diff --git a/...l/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedGroup.java b/...l/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedGroup.java
@@ -17,6 +17,8 @@
 
 package org.keycloak.models.cache.infinispan.entities;
 
+import java.util.Map;
+import java.util.TreeMap;
 import org.keycloak.common.util.MultivaluedHashMap;
 import org.keycloak.models.GroupModel;
 import org.keycloak.models.RealmModel;
@@ -40,7 +42,7 @@ public class CachedGroup extends AbstractRevisioned implements InRealm {
     private final String parentId;
     private final LazyLoader<GroupModel, MultivaluedHashMap<String, String>> attributes;
     private final LazyLoader<GroupModel, Set<String>> roleMappings;
-    private final LazyLoader<GroupModel, Set<String>> subGroups;
+    private final LazyLoader<GroupModel, Map<String, String>> subGroups;
 
     public CachedGroup(Long revision, RealmModel realm, GroupModel group) {
         super(revision, group.getId());
@@ -49,7 +51,7 @@ public CachedGroup(Long revision, RealmModel realm, GroupModel group) {
         this.parentId = group.getParentId();
         this.attributes = new DefaultLazyLoader<>(source -> new MultivaluedHashMap<>(source.getAttributes()), MultivaluedHashMap::new);
         this.roleMappings = new DefaultLazyLoader<>(source -> source.getRoleMappingsStream().map(RoleModel::getId).collect(Collectors.toSet()), Collections::emptySet);
-        this.subGroups = new DefaultLazyLoader<>(source -> source.getSubGroupsStream().map(GroupModel::getId).collect(Collectors.toSet()), Collections::emptySet);
+        this.subGroups = new DefaultLazyLoader<>(source -> source.getSubGroupsStream().collect(Collectors.toMap(GroupModel::getName, GroupModel::getId, (g, g1) -> g1, TreeMap::new)), Collections::emptyMap);
     }
 
     public String getRealm() {
@@ -76,7 +78,7 @@ public String getParentId() {
         return parentId;
     }
 
-    public Set<String> getSubGroups(Supplier<GroupModel> group) {
+    public Map<String, String> getSubGroups(Supplier<GroupModel> group) {
         return subGroups.get(group);
     }
 }