From 3291161954c2c5af66c49114823ca150cb558c9e Mon Sep 17 00:00:00 2001 From: Michael Cooney Date: Wed, 15 Apr 2020 15:47:10 -0400 Subject: [PATCH] KEYCLOAK-13818: Addressing performance issues with adding client scopes during realm creation. Removing redundant lookups by passing all scopes that need to be created at once. --- .../cache/infinispan/ClientAdapter.java | 7 ++++ .../keycloak/models/jpa/ClientAdapter.java | 12 +++++++ .../AbstractLoginProtocolFactory.java | 33 ++++++++++++------- .../AbstractReadOnlyClientStorageAdapter.java | 6 ++++ .../java/org/keycloak/models/ClientModel.java | 8 +++++ 5 files changed, 55 insertions(+), 11 deletions(-) diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/ClientAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/ClientAdapter.java index dfb379fbe7..6e7462b770 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/ClientAdapter.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/ClientAdapter.java @@ -102,6 +102,13 @@ public class ClientAdapter implements ClientModel, CachedObject { updated.addClientScope(clientScope, defaultScope); } + @Override + public void addClientScopes(Set clientScopes, boolean defaultScope) { + for (ClientScopeModel clientScope : clientScopes) { + addClientScope(clientScope, defaultScope); + } + } + @Override public void removeClientScope(ClientScopeModel clientScope) { getDelegateForUpdate(); diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/ClientAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/ClientAdapter.java index 2f25fee94f..e0b01ca4ea 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/ClientAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/ClientAdapter.java @@ -355,6 +355,18 @@ public class ClientAdapter implements ClientModel, JpaModel { public void addClientScope(ClientScopeModel clientScope, boolean defaultScope) { if (getClientScopes(defaultScope, false).containsKey(clientScope.getName())) return; + persist(clientScope, defaultScope); + } + + @Override + public void addClientScopes(Set clientScopes, boolean defaultScope) { + Map existingClientScopes = getClientScopes(defaultScope, false); + clientScopes.stream() + .filter(clientScope -> !existingClientScopes.containsKey(clientScope.getName())) + .forEach(clientScope -> persist(clientScope, defaultScope)); + } + + private void persist(ClientScopeModel clientScope, boolean defaultScope) { ClientScopeClientMappingEntity entity = new ClientScopeClientMappingEntity(); entity.setClientScope(ClientScopeAdapter.toClientScopeEntity(clientScope, em)); entity.setClient(getEntity()); diff --git a/server-spi-private/src/main/java/org/keycloak/protocol/AbstractLoginProtocolFactory.java b/server-spi-private/src/main/java/org/keycloak/protocol/AbstractLoginProtocolFactory.java index 106f073c96..b26e470924 100755 --- a/server-spi-private/src/main/java/org/keycloak/protocol/AbstractLoginProtocolFactory.java +++ b/server-spi-private/src/main/java/org/keycloak/protocol/AbstractLoginProtocolFactory.java @@ -25,6 +25,12 @@ import org.keycloak.models.RealmModel; import org.keycloak.provider.ProviderEvent; import org.keycloak.provider.ProviderEventListener; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; + /** * @author Bill Burke * @version $Revision: 1 $ @@ -56,9 +62,7 @@ public abstract class AbstractLoginProtocolFactory implements LoginProtocolFacto // Create default client scopes for realm built-in clients too if (addScopesToExistingClients) { - for (ClientModel client : newRealm.getClients()) { - addDefaultClientScopes(newRealm, client); - } + addDefaultClientScopes(newRealm, newRealm.getClients()); } } @@ -69,15 +73,22 @@ public abstract class AbstractLoginProtocolFactory implements LoginProtocolFacto protected void addDefaultClientScopes(RealmModel realm, ClientModel newClient) { - for (ClientScopeModel clientScope : realm.getDefaultClientScopes(true)) { - if (getId().equals(clientScope.getProtocol())) { - newClient.addClientScope(clientScope, true); - } + addDefaultClientScopes(realm, Arrays.asList(newClient)); + } + + protected void addDefaultClientScopes(RealmModel realm, List newClients) { + Set defaultClientScopes = realm.getDefaultClientScopes(true).stream() + .filter(clientScope -> getId().equals(clientScope.getProtocol())) + .collect(Collectors.toSet()); + for (ClientModel newClient : newClients) { + newClient.addClientScopes(defaultClientScopes, true); } - for (ClientScopeModel clientScope : realm.getDefaultClientScopes(false)) { - if (getId().equals(clientScope.getProtocol())) { - newClient.addClientScope(clientScope, false); - } + + Set nonDefaultClientScopes = realm.getDefaultClientScopes(false).stream() + .filter(clientScope -> getId().equals(clientScope.getProtocol())) + .collect(Collectors.toSet()); + for (ClientModel newClient : newClients) { + newClient.addClientScopes(nonDefaultClientScopes, false); } } diff --git a/server-spi-private/src/main/java/org/keycloak/storage/client/AbstractReadOnlyClientStorageAdapter.java b/server-spi-private/src/main/java/org/keycloak/storage/client/AbstractReadOnlyClientStorageAdapter.java index 3e50971da2..52131e7549 100644 --- a/server-spi-private/src/main/java/org/keycloak/storage/client/AbstractReadOnlyClientStorageAdapter.java +++ b/server-spi-private/src/main/java/org/keycloak/storage/client/AbstractReadOnlyClientStorageAdapter.java @@ -23,6 +23,7 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; import org.keycloak.storage.ReadOnlyException; +import java.util.List; import java.util.Set; /** @@ -224,6 +225,11 @@ public abstract class AbstractReadOnlyClientStorageAdapter extends AbstractClien throw new ReadOnlyException("client is read only for this update"); } + @Override + public void addClientScopes(Set clientScopes, boolean defaultScope) { + throw new ReadOnlyException("client is read only for this update"); + } + @Override public void removeClientScope(ClientScopeModel clientScope) { throw new ReadOnlyException("client is read only for this update"); diff --git a/server-spi/src/main/java/org/keycloak/models/ClientModel.java b/server-spi/src/main/java/org/keycloak/models/ClientModel.java index 1a2337e0ad..801b8abba0 100755 --- a/server-spi/src/main/java/org/keycloak/models/ClientModel.java +++ b/server-spi/src/main/java/org/keycloak/models/ClientModel.java @@ -17,6 +17,7 @@ package org.keycloak.models; +import java.util.List; import java.util.Map; import java.util.Set; @@ -170,6 +171,13 @@ public interface ClientModel extends ClientScopeModel, RoleContainerModel, Prot */ void addClientScope(ClientScopeModel clientScope, boolean defaultScope); + /** + * Add clientScopes with this client. Add as default scopes (if parameter 'defaultScope' is true) or optional scopes (if parameter 'defaultScope' is false) + * @param clientScopes + * @param defaultScope + */ + void addClientScopes(Set clientScopes, boolean defaultScope); + void removeClientScope(ClientScopeModel clientScope); /**