diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheManager.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheManager.java index 2e51914aead..cd662679248 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheManager.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheManager.java @@ -64,8 +64,9 @@ public class RealmCacheManager extends CacheManager { addInvalidations(InRealmPredicate.create().realm(id), invalidations); } - public void roleAdded(String roleContainerId, Set invalidations) { + public void roleAdded(String roleContainerId, String roleName, Set invalidations) { invalidations.add(RealmCacheSession.getRolesCacheKey(roleContainerId)); + invalidations.add(RealmCacheSession.getRoleByNameCacheKey(roleContainerId, roleName)); } public void roleUpdated(String roleContainerId, String roleName, Set invalidations) { diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java index 7ecc221de84..ecebe26ddc7 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java @@ -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 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); diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/events/RoleAddedEvent.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/events/RoleAddedEvent.java index d780006e66e..5b397835b78 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/events/RoleAddedEvent.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/events/RoleAddedEvent.java @@ -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 invalidations) { - realmCache.roleAdded(containerId, invalidations); + realmCache.roleAdded(containerId, roleName, invalidations); } } diff --git a/testsuite/model/src/test/java/org/keycloak/testsuite/model/role/RoleModelTest.java b/testsuite/model/src/test/java/org/keycloak/testsuite/model/role/RoleModelTest.java index 24827a71da1..2e6835db4a2 100644 --- a/testsuite/model/src/test/java/org/keycloak/testsuite/model/role/RoleModelTest.java +++ b/testsuite/model/src/test/java/org/keycloak/testsuite/model/role/RoleModelTest.java @@ -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 result = resultProvider.getResult("1", 4, 3);