Cache empty results for role-by-name lookup

Closes #36919

Signed-off-by: Alexander Schwartz <aschwart@redhat.com>
This commit is contained in:
Alexander Schwartz 2025-01-29 21:27:52 +01:00 committed by Pedro Igor
parent a9080f8b63
commit eee2805cef
4 changed files with 71 additions and 13 deletions

View File

@ -64,8 +64,9 @@ public class RealmCacheManager extends CacheManager {
addInvalidations(InRealmPredicate.create().realm(id), invalidations);
}
public void roleAdded(String roleContainerId, Set<String> invalidations) {
public void roleAdded(String roleContainerId, String roleName, Set<String> invalidations) {
invalidations.add(RealmCacheSession.getRolesCacheKey(roleContainerId));
invalidations.add(RealmCacheSession.getRoleByNameCacheKey(roleContainerId, roleName));
}
public void roleUpdated(String roleContainerId, String roleName, Set<String> invalidations) {

View File

@ -19,6 +19,7 @@ package org.keycloak.models.cache.infinispan;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
@ -320,13 +321,13 @@ public class RealmCacheSession implements CacheRealmProvider {
if (adapter != null) adapter.invalidate();
}
private void addedRole(String roleId, String roleContainerId) {
private void addedRole(String roleId, String roleContainerId, String roleName) {
// this is needed so that a new role that hasn't been committed isn't cached in a query
listInvalidations.add(roleContainerId);
invalidateRole(roleId);
cache.roleAdded(roleContainerId, invalidations);
invalidationEvents.add(RoleAddedEvent.create(roleId, roleContainerId));
cache.roleAdded(roleContainerId, roleName, invalidations);
invalidationEvents.add(RoleAddedEvent.create(roleId, roleContainerId, roleName));
}
@Override
@ -719,7 +720,7 @@ public class RealmCacheSession implements CacheRealmProvider {
@Override
public RoleModel addRealmRole(RealmModel realm, String id, String name) {
RoleModel role = getRoleDelegate().addRealmRole(realm, id, name);
addedRole(role.getId(), realm.getId());
addedRole(role.getId(), realm.getId(), name);
return role;
}
@ -836,7 +837,7 @@ public class RealmCacheSession implements CacheRealmProvider {
@Override
public RoleModel addClientRole(ClientModel client, String id, String name) {
RoleModel role = getRoleDelegate().addClientRole(client, id, name);
addedRole(role.getId(), client.getId());
addedRole(role.getId(), client.getId(), name);
return role;
}
@ -856,13 +857,21 @@ public class RealmCacheSession implements CacheRealmProvider {
if (query == null) {
Long loaded = cache.getCurrentRevision(cacheKey);
RoleModel model = getRoleDelegate().getRealmRole(realm, name);
if (model == null) return null;
query = new RoleListQuery(loaded, cacheKey, realm, model.getId());
if (model == null) {
// caching empty results will speed up the policy evaluation which tries to look up the role by name and ID
query = new RoleListQuery(loaded, cacheKey, realm, Set.of());
} else {
query = new RoleListQuery(loaded, cacheKey, realm, model.getId());
}
logger.tracev("adding realm role cache miss: client {0} key {1}", realm.getName(), cacheKey);
cache.addRevisioned(query, startupRevision);
return model;
}
RoleModel role = getRoleById(realm, query.getRoles().iterator().next());
Iterator<String> iterator = query.getRoles().iterator();
if (!iterator.hasNext()) {
return null;
}
RoleModel role = getRoleById(realm, iterator.next());
if (role == null) {
invalidations.add(cacheKey);
return getRoleDelegate().getRealmRole(realm, name);

View File

@ -20,6 +20,7 @@ package org.keycloak.models.cache.infinispan.events;
import java.util.Set;
import org.infinispan.protostream.annotations.ProtoFactory;
import org.infinispan.protostream.annotations.ProtoField;
import org.infinispan.protostream.annotations.ProtoTypeId;
import org.keycloak.marshalling.Marshalling;
import org.keycloak.models.cache.infinispan.RealmCacheManager;
@ -30,17 +31,21 @@ import org.keycloak.models.cache.infinispan.RealmCacheManager;
@ProtoTypeId(Marshalling.ROLE_ADDED_EVENT)
public class RoleAddedEvent extends BaseRoleEvent {
@ProtoField(3)
final String roleName;
@ProtoFactory
RoleAddedEvent(String id, String containerId) {
RoleAddedEvent(String id, String containerId, String roleName) {
super(id, containerId);
this.roleName = roleName;
}
public static RoleAddedEvent create(String roleId, String containerId) {
return new RoleAddedEvent(roleId, containerId);
public static RoleAddedEvent create(String roleId, String containerId, String roleName) {
return new RoleAddedEvent(roleId, containerId, roleName);
}
@Override
public void addInvalidations(RealmCacheManager realmCache, Set<String> invalidations) {
realmCache.roleAdded(containerId, invalidations);
realmCache.roleAdded(containerId, roleName, invalidations);
}
}

View File

@ -15,6 +15,7 @@ import org.keycloak.testsuite.model.RequireProvider;
import java.util.Collection;
import java.util.List;
import java.util.Random;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Collectors;
@ -28,6 +29,7 @@ import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
@RequireProvider(RealmProvider.class)
@ -305,6 +307,47 @@ public class RoleModelTest extends KeycloakModelTest {
});
}
@Test
public void getRoleByNameFromTheDatabaseAndTheCache() {
String roleName = "role-" + new Random().nextInt();
// Look up a non-existent role from the database
withRealm(realmId, (session, realm) -> {
RoleModel role = session.roles().getRealmRole(realm, roleName);
assertThat(role, nullValue());
return null;
});
// Look up a non-existent role from the cache
withRealm(realmId, (session, realm) -> {
RoleModel role = session.roles().getRealmRole(realm, roleName);
assertThat(role, nullValue());
return null;
});
// Create the role, and invalidate the cache
withRealm(realmId, (session, realm) -> {
RoleModel role = session.roles().addRealmRole(realm, roleName);
assertThat(role, notNullValue());
return null;
});
// Find the role from the database
withRealm(realmId, (session, realm) -> {
RoleModel role = session.roles().getRealmRole(realm, roleName);
assertThat(role, notNullValue());
return null;
});
// Find the role from the cache
withRealm(realmId, (session, realm) -> {
RoleModel role = session.roles().getRealmRole(realm, roleName);
assertThat(role, notNullValue());
return null;
});
}
public void testRolesWithIdsPaginationSearchQueries(GetResult resultProvider) {
// test all parameters together
List<RoleModel> result = resultProvider.getResult("1", 4, 3);