Avoid caching the list of clientscopes in two places

ahus1 · martin-kanis · hmlnarik · commit e410a76c42bc · 2023-06-13T21:33:21.000+02:00
Closes keycloak#20426 Co-authored-by: Martin Kanis <mkanis@redhat.com>
diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java
@@ -1454,13 +1454,7 @@ public boolean removeGroup(GroupModel group) {
     @Override
     public Stream<ClientScopeModel> getClientScopesStream() {
         if (isUpdated()) return updated.getClientScopesStream();
-        return cached.getClientScopes().stream().map(scope -> {
-            ClientScopeModel model = cacheSession.getClientScopeById(this, scope);
-            if (model == null) {
-                throw new IllegalStateException("Cached clientScope not found: " + scope);
-            }
-            return model;
-        });
+        return cacheSession.getClientScopesStream(this);
     }
 
     @Override
diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedRealm.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedRealm.java
@@ -159,7 +159,6 @@ public Set<IdentityProviderMapperModel> getIdentityProviderMapperSet() {
     }
 
     protected List<String> defaultGroups;
-    protected List<String> clientScopes = new LinkedList<>();
     protected List<String> defaultDefaultClientScopes = new LinkedList<>();
     protected List<String> optionalDefaultClientScopes = new LinkedList<>();
     protected boolean internationalizationEnabled;
@@ -319,7 +318,6 @@ public CachedRealm(Long revision, RealmModel model) {
     }
 
     protected void cacheClientScopes(RealmModel model) {
-        clientScopes = model.getClientScopesStream().map(ClientScopeModel::getId).collect(Collectors.toList());
         defaultDefaultClientScopes = model.getDefaultClientScopesStream(true).map(ClientScopeModel::getId)
                 .collect(Collectors.toList());
         optionalDefaultClientScopes = model.getDefaultClientScopesStream(false).map(ClientScopeModel::getId)
@@ -687,10 +685,6 @@ public List<String> getDefaultGroups() {
         return defaultGroups;
     }
 
-    public List<String> getClientScopes() {
-        return clientScopes;
-    }
-
     public List<String> getDefaultDefaultClientScopes() {
         return defaultDefaultClientScopes;
     }
diff --git a/testsuite/model/src/test/java/org/keycloak/testsuite/model/clientscope/ClientScopeModelTest.java b/testsuite/model/src/test/java/org/keycloak/testsuite/model/clientscope/ClientScopeModelTest.java
@@ -16,6 +16,7 @@
  */
 package org.keycloak.testsuite.model.clientscope;
 
+import org.hamcrest.Matchers;
 import org.keycloak.models.ClientModel;
 import org.keycloak.models.ClientProvider;
 import org.keycloak.models.ClientScopeModel;
@@ -25,6 +26,7 @@
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.RealmProvider;
 import org.keycloak.models.RoleProvider;
+import org.keycloak.models.cache.CacheRealmProvider;
 import org.keycloak.testsuite.model.KeycloakModelTest;
 import org.keycloak.testsuite.model.RequireProvider;
 import java.util.LinkedList;
@@ -97,4 +99,33 @@ public void testClientScopes() {
             return null;
         });
     }
+
+    @Test
+    @RequireProvider(value=ClientScopeProvider.class, only="jpa")
+    @RequireProvider(value=CacheRealmProvider.class)
+    public void testClientScopesCaching() {
+        List<String> clientScopes = new LinkedList<>();
+        withRealm(realmId, (session, realm) -> {
+            ClientScopeModel clientScope = session.clientScopes().addClientScope(realm, "myClientScopeForCaching");
+            clientScopes.add(clientScope.getId());
+
+            assertionsForClientScopesCaching(clientScopes, session, realm);
+            return null;
+        });
+
+        withRealm(realmId, (session, realm) -> {
+            assertionsForClientScopesCaching(clientScopes, session, realm);
+            return null;
+        });
+
+    }
+
+    private static void assertionsForClientScopesCaching(List<String> clientScopes, KeycloakSession session, RealmModel realm) {
+        assertThat(clientScopes, Matchers.containsInAnyOrder(realm.getClientScopesStream()
+                .map(ClientScopeModel::getId).toArray(String[]::new)));
+
+        assertThat(clientScopes, Matchers.containsInAnyOrder(session.clientScopes().getClientScopesStream(realm)
+                .map(ClientScopeModel::getId).toArray(String[]::new)));
+    }
+
 }

Original file line number	Diff line number	Diff line change
`@@ -159,7 +159,6 @@ public Set<IdentityProviderMapperModel> getIdentityProviderMapperSet() {`
`159`	`159`	`}`
`160`	`160`
`161`	`161`	`protected List<String> defaultGroups;`
`162`		`- protected List<String> clientScopes = new LinkedList<>();`
`163`	`162`	`protected List<String> defaultDefaultClientScopes = new LinkedList<>();`
`164`	`163`	`protected List<String> optionalDefaultClientScopes = new LinkedList<>();`
`165`	`164`	`protected boolean internationalizationEnabled;`
`@@ -319,7 +318,6 @@ public CachedRealm(Long revision, RealmModel model) {`
`319`	`318`	`}`
`320`	`319`
`321`	`320`	`protected void cacheClientScopes(RealmModel model) {`
`322`		`- clientScopes = model.getClientScopesStream().map(ClientScopeModel::getId).collect(Collectors.toList());`
`323`	`321`	`defaultDefaultClientScopes = model.getDefaultClientScopesStream(true).map(ClientScopeModel::getId)`
`324`	`322`	`.collect(Collectors.toList());`
`325`	`323`	`optionalDefaultClientScopes = model.getDefaultClientScopesStream(false).map(ClientScopeModel::getId)`
`@@ -687,10 +685,6 @@ public List<String> getDefaultGroups() {`
`687`	`685`	`return defaultGroups;`
`688`	`686`	`}`
`689`	`687`
`690`		`- public List<String> getClientScopes() {`
`691`		`- return clientScopes;`
`692`		`- }`
`693`		`-`
`694`	`688`	`public List<String> getDefaultDefaultClientScopes() {`
`695`	`689`	`return defaultDefaultClientScopes;`
`696`	`690`	`}`