From dffa7a31cbdaec67a74570af28bb152e3fbc0be1 Mon Sep 17 00:00:00 2001 From: Todor Staykovski Date: Mon, 7 Aug 2023 22:18:09 +0300 Subject: [PATCH] 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 --- .../models/cache/infinispan/GroupAdapter.java | 2 +- .../models/jpa/entities/GroupEntity.java | 2 +- .../models/map/group/MapGroupProvider.java | 2 +- .../java/org/keycloak/models/GroupModel.java | 2 ++ .../testsuite/model/group/GroupModelTest.java | 29 +++++++++++++++++++ 5 files changed, 34 insertions(+), 3 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 index cfe602031ba..0493b668d2e 100755 --- 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 @@ -234,7 +234,7 @@ public class GroupAdapter implements GroupModel { } subGroups.add(subGroup); } - return subGroups.stream(); + return subGroups.stream().sorted(GroupModel.COMPARE_BY_NAME); } diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/GroupEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/GroupEntity.java index ba44986efaf..b02c1b3b0cf 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/GroupEntity.java +++ b/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"), 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 index a4c32b77346..7bf9c7db7a7 100644 --- 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 @@ -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)); } } diff --git a/server-spi/src/main/java/org/keycloak/models/GroupModel.java b/server-spi/src/main/java/org/keycloak/models/GroupModel.java index 932e6627c20..0665adf11ca 100755 --- a/server-spi/src/main/java/org/keycloak/models/GroupModel.java +++ b/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 getSubGroupsStream(); diff --git a/testsuite/model/src/test/java/org/keycloak/testsuite/model/group/GroupModelTest.java b/testsuite/model/src/test/java/org/keycloak/testsuite/model/group/GroupModelTest.java index 497d653fc25..bcaa8795fa3 100644 --- a/testsuite/model/src/test/java/org/keycloak/testsuite/model/group/GroupModelTest.java +++ b/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 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; + }); + } + }