From 1cf51a700c8db6b9d750ea44a13ea7544e9f59af Mon Sep 17 00:00:00 2001
From: Alexander Schwartz <aschwart@redhat.com>
Date: Thu, 30 Jan 2025 22:25:42 +0100
Subject: [PATCH] Also cache client roles if looked up by name and not found

Closes #36919

Signed-off-by: Alexander Schwartz <aschwart@redhat.com>
---
 .../cache/infinispan/RealmCacheSession.java   | 12 ++++-
 .../testsuite/model/role/RoleModelTest.java   | 46 +++++++++++++++++++
 2 files changed, 56 insertions(+), 2 deletions(-)

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 ecebe26ddc7..2bcdb04942d 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
@@ -895,12 +895,20 @@ public class RealmCacheSession implements CacheRealmProvider {
         if (query == null) {
             Long loaded = cache.getCurrentRevision(cacheKey);
             RoleModel model = getRoleDelegate().getClientRole(client, name);
-            if (model == null) return null;
-            query = new RoleListQuery(loaded, cacheKey, client.getRealm(), model.getId(), client.getClientId());
+            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, client.getRealm(), Set.of());
+            } else {
+                query = new RoleListQuery(loaded, cacheKey, client.getRealm(), model.getId(), client.getClientId());
+            }
             logger.tracev("adding client role cache miss: client {0} key {1}", client.getClientId(), cacheKey);
             cache.addRevisioned(query, startupRevision);
             return model;
         }
+        Iterator<String> iterator = query.getRoles().iterator();
+        if (!iterator.hasNext()) {
+            return null;
+        }
         RoleModel role = getRoleById(client.getRealm(), query.getRoles().iterator().next());
         if (role == null) {
             invalidations.add(cacheKey);
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 2e6835db4a2..3f2a2c8fa39 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
@@ -348,6 +348,52 @@ public class RoleModelTest extends KeycloakModelTest {
 
     }
 
+    @Test
+    public void getClientRoleByNameFromTheDatabaseAndTheCache() {
+        String roleName = "role-" + new Random().nextInt();
+
+        // Look up a non-existent role from the database
+        withRealm(realmId, (session, realm) -> {
+            ClientModel client = session.clients().getClientByClientId(realm, CLIENT_NAME);
+            RoleModel role = session.roles().getClientRole(client, roleName);
+            assertThat(role, nullValue());
+            return null;
+        });
+
+        // Look up a non-existent role from the cache
+        withRealm(realmId, (session, realm) -> {
+            ClientModel client = session.clients().getClientByClientId(realm, CLIENT_NAME);
+            RoleModel role = session.roles().getClientRole(client, roleName);
+            assertThat(role, nullValue());
+            return null;
+        });
+
+        // Create the role, and invalidate the cache
+        withRealm(realmId, (session, realm) -> {
+            ClientModel client = session.clients().getClientByClientId(realm, CLIENT_NAME);
+            RoleModel role = session.roles().addClientRole(client, roleName);
+            assertThat(role, notNullValue());
+            return null;
+        });
+
+        // Find the role from the database
+        withRealm(realmId, (session, realm) -> {
+            ClientModel client = session.clients().getClientByClientId(realm, CLIENT_NAME);
+            RoleModel role = session.roles().getClientRole(client, roleName);
+            assertThat(role, notNullValue());
+            return null;
+        });
+
+        // Find the role from the cache
+        withRealm(realmId, (session, realm) -> {
+            ClientModel client = session.clients().getClientByClientId(realm, CLIENT_NAME);
+            RoleModel role = session.roles().getClientRole(client, roleName);
+            assertThat(role, notNullValue());
+            return null;
+        });
+
+    }
+
     public void testRolesWithIdsPaginationSearchQueries(GetResult resultProvider) {
         // test all parameters together
         List<RoleModel> result = resultProvider.getResult("1", 4, 3);