Optimize querying sub-groups of groups

Closes keycloak#12080
valkum · May 19, 2022 · d1a9268 · d1a9268
1 parent 2747729
commit d1a9268
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 8 deletions.
diff --git a/model/map-jpa/src/main/resources/META-INF/groups/jpa-groups-changelog-1.xml b/model/map-jpa/src/main/resources/META-INF/groups/jpa-groups-changelog-1.xml
@@ -48,6 +48,11 @@ limitations under the License.
             <column name="name"/>
             <column name="parentid"/>
         </createIndex>
+        <createIndex tableName="kc_group" indexName="group_realmId_parentId">
+            <!-- list column parentId first, as it is more selective than realmId -->
+            <column name="parentid"/>
+            <column name="realmid"/>
+        </createIndex>
 
         <createTable tableName="kc_group_attribute">
             <column name="id" type="UUID">

diff --git a/model/map/src/main/java/org/keycloak/models/map/group/MapGroupAdapter.java b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupAdapter.java
@@ -19,6 +19,7 @@
 
 import org.keycloak.models.ClientModel;
 import org.keycloak.models.GroupModel;
+import org.keycloak.models.GroupProvider;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.RoleModel;
@@ -31,7 +32,7 @@
 import java.util.stream.Stream;
 
 
-public class MapGroupAdapter extends AbstractGroupModel<MapGroupEntity> {
+public abstract class MapGroupAdapter extends AbstractGroupModel<MapGroupEntity> {
     public MapGroupAdapter(KeycloakSession session, RealmModel realm, MapGroupEntity entity) {
         super(session, realm, entity);
     }
@@ -99,12 +100,6 @@ public String getParentId() {
         return entity.getParentId();
     }
 
-    @Override
-    public Stream<GroupModel> getSubGroupsStream() {
-        return session.groups().getGroupsStream(realm)
-                .filter(groupModel -> getId().equals(groupModel.getParentId()));
-    }
-
     @Override
     public void setParent(GroupModel group) {
         if (group == null) {

diff --git a/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProvider.java b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProvider.java
@@ -59,7 +59,12 @@ public MapGroupProvider(KeycloakSession session, MapStorage<MapGroupEntity, Grou
 
     private Function<MapGroupEntity, GroupModel> entityToAdapterFunc(RealmModel realm) {
         // Clone entity before returning back, to avoid giving away a reference to the live object to the caller
-        return origEntity -> new MapGroupAdapter(session, realm, origEntity);
+        return origEntity -> new MapGroupAdapter(session, realm, origEntity) {
+            @Override
+            public Stream<GroupModel> getSubGroupsStream() {
+                return getGroupsByParentId(realm, this.getId());
+            }
+        };
     }
 
     @Override
@@ -297,4 +302,13 @@ public void preRemove(RealmModel realm) {
     public void close() {
     }
 
+    private Stream<GroupModel> getGroupsByParentId(RealmModel realm, String parentId) {
+        LOG.tracef("getGroupsByParentId(%s)%s", parentId, getShortStackTrace());
+        DefaultModelCriteria<GroupModel> mcb = criteria();
+        mcb = mcb
+                .compare(SearchableFields.REALM_ID, Operator.EQ, realm.getId())
+                .compare(SearchableFields.PARENT_ID, Operator.EQ, parentId);
+
+        return tx.read(withCriteria(mcb)).map(entityToAdapterFunc(realm));
+    }
 }