Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,8 @@ public Stream<GroupModel> getSubGroupsStream(String search, Boolean exact, Integ
@Override
public Long getSubGroupsCount() {
if (isUpdated()) return updated.getSubGroupsCount();
return getGroupModel().getSubGroupsCount();
GroupModel model = modelSupplier.get();
return model == null ? null : model.getSubGroupsCount();
}
Comment on lines +274 to 276
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: This pattern differs from other methods like getSubGroupsStream() which call getDelegateForUpdate() when encountering null models. Consider consistency with the existing error handling pattern.


@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ public CachedGroup(Long revision, RealmModel realm, GroupModel group) {
this.type = group.getType();
}

@Override
public String getRealm() {
return realm;
}
Expand Down
10 changes: 0 additions & 10 deletions services/src/main/java/org/keycloak/utils/GroupUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,4 @@ public static GroupRepresentation toRepresentation(GroupPermissionEvaluator grou
rep.setAccess(groupsEvaluator.getAccess(groupTree));
return rep;
}

private static boolean groupMatchesSearchOrIsPathElement(GroupModel group, String search) {
if (StringUtil.isBlank(search)) {
return true;
}
if (group.getName().contains(search)) {
return true;
}
return group.getSubGroupsStream().findAny().isPresent();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.impl.client.CloseableHttpClient;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.keycloak.admin.client.Keycloak;
Expand Down Expand Up @@ -76,6 +77,9 @@
import java.util.List;
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.IntStream;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.anEmptyMap;
Expand All @@ -90,6 +94,7 @@
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;

/**
* @author <a href="mailto:[email protected]">Marko Strukelj</a>
Expand All @@ -109,6 +114,49 @@ public class GroupTest extends AbstractGroupTest {
@InjectHttpClient
CloseableHttpClient httpClient;


@Test
public void createMultiDeleteMultiReadMulti() {
// create multiple groups
List<String> groupUuuids = new ArrayList<>();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: Variable name has typo: groupUuuids should be groupUuids

IntStream.range(0, 100).forEach(groupIndex -> {
GroupRepresentation group = new GroupRepresentation();
group.setName("Test Group " + groupIndex);
try (Response response = managedRealm.admin().groups().add(group)) {
boolean created = response.getStatusInfo().getFamily() == Response.Status.Family.SUCCESSFUL;
if (created) {
final String groupUuid = ApiUtil.getCreatedId(response);
groupUuuids.add(groupUuid);
} else {
fail("Failed to create group: " + response.getStatusInfo().getReasonPhrase());
}
}
});

AtomicBoolean deletedAll = new AtomicBoolean(false);
List<Exception> caughtExceptions = new CopyOnWriteArrayList<>();
// read groups in a separate thread
new Thread(() -> {
while (!deletedAll.get()) {
try {
// just loading briefs
managedRealm.admin().groups().groups(null, 0, Integer.MAX_VALUE, true);
} catch (Exception e) {

caughtExceptions.add(e);
}
}
}).start();
Comment on lines +139 to +149
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Thread is started but not joined, creating a race condition where the test might complete before the reader thread finishes executing

Suggested change
new Thread(() -> {
while (!deletedAll.get()) {
try {
// just loading briefs
managedRealm.admin().groups().groups(null, 0, Integer.MAX_VALUE, true);
} catch (Exception e) {
caughtExceptions.add(e);
}
}
}).start();
Thread readerThread = new Thread(() -> {
while (!deletedAll.get()) {
try {
// just loading briefs
managedRealm.admin().groups().groups(null, 0, Integer.MAX_VALUE, true);
} catch (Exception e) {
caughtExceptions.add(e);
}
}
});
readerThread.start();
// delete groups
groupUuuids.forEach(groupUuid -> {
managedRealm.admin().groups().group(groupUuid).remove();
});
deletedAll.set(true);
try {
readerThread.join(5000); // Wait up to 5 seconds for thread to complete
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
fail("Test interrupted while waiting for reader thread");
}


// delete groups
groupUuuids.forEach(groupUuid -> {
managedRealm.admin().groups().group(groupUuid).remove();
});
deletedAll.set(true);

assertThat(caughtExceptions, Matchers.empty());
}

// KEYCLOAK-2716 Can't delete client if its role is assigned to a group
@Test
public void testClientRemoveWithClientRoleGroupMapping() {
Expand Down
Loading