Add subgroups sorting (#22295)

* Review comments to add a test, update the API description and adjust the map storage.

Closes #19348

Co-authored-by: Alexander Schwartz <aschwart@redhat.com>

model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/GroupAdapter.java

@@ -234,7 +234,7 @@ public class GroupAdapter implements GroupModel {
             }
             subGroups.add(subGroup);
         }
-        return subGroups.stream();
+        return subGroups.stream().sorted(GroupModel.COMPARE_BY_NAME);
     }
 
 

model/jpa/src/main/java/org/keycloak/models/jpa/entities/GroupEntity.java

@@ -28,7 +28,7 @@ import java.util.LinkedList;
  * @version $Revision: 1 $
  */
 @NamedQueries({
-        @NamedQuery(name="getGroupIdsByParent", query="select u.id from GroupEntity u where u.parentId = :parent"),
+        @NamedQuery(name="getGroupIdsByParent", query="select u.id from GroupEntity u where u.parentId = :parent order by u.name ASC"),
         @NamedQuery(name="getGroupIdsByRealm", query="select u.id from GroupEntity u where u.realm = :realm  order by u.name ASC"),
         @NamedQuery(name="getGroupIdsByNameContaining", query="select u.id from GroupEntity u where u.realm = :realm and u.name like concat('%',:search,'%') order by u.name ASC"),
         @NamedQuery(name="getGroupIdsByNameContainingFromIdList", query="select u.id from GroupEntity u where u.realm = :realm and lower(u.name) like lower(concat('%',:search,'%')) and u.id in :ids order by u.name ASC"),

model/map/src/main/java/org/keycloak/models/map/group/MapGroupProvider.java

@@ -366,6 +366,6 @@ public class MapGroupProvider implements GroupProvider {
                 .compare(SearchableFields.REALM_ID, Operator.EQ, realm.getId())
                 .compare(SearchableFields.PARENT_ID, Operator.EQ, parentId);
 
-        return storeWithRealm(realm).read(withCriteria(mcb)).map(entityToAdapterFunc(realm));
+        return storeWithRealm(realm).read(withCriteria(mcb).orderBy(SearchableFields.NAME, ASCENDING)).map(entityToAdapterFunc(realm));
     }
 }

server-spi/src/main/java/org/keycloak/models/GroupModel.java

@@ -104,6 +104,8 @@ public interface GroupModel extends RoleMapperModel {
 
     /**
      * Returns all sub groups for the parent group as a stream.
+     * The stream is sorted by the group name.
+     *
      * @return Stream of {@link GroupModel}. Never returns {@code null}.
      */
     Stream<GroupModel> getSubGroupsStream();

testsuite/model/src/test/java/org/keycloak/testsuite/model/group/GroupModelTest.java

@@ -24,6 +24,11 @@ import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.RealmModel;
 import org.keycloak.testsuite.model.KeycloakModelTest;
 
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
+
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.equalTo;
@@ -70,4 +75,28 @@ public class GroupModelTest extends KeycloakModelTest {
         });
     }
 
+    @Test
+    public void testSubGroupsSorted() {
+        List<String> subGroups = Arrays.asList("sub-group-1", "sub-group-2", "sub-group-3");
+
+        String groupId = withRealm(realmId, (session, realm) -> {
+            GroupModel group = session.groups().createGroup(realm, "my-group");
+
+            subGroups.stream().sorted(Collections.reverseOrder()).forEach(s -> {
+                GroupModel subGroup = session.groups().createGroup(realm, s);
+                group.addChild(subGroup);
+            });
+
+            return group.getId();
+        });
+        withRealm(realmId, (session, realm) -> {
+            GroupModel group = session.groups().getGroupById(realm, groupId);
+
+            assertThat(group.getSubGroupsStream().map(GroupModel::getName).collect(Collectors.toList()),
+                    contains(subGroups.toArray()));
+
+            return null;
+        });
+    }
+
 }