From 925f089d628c50e977830da2e8c4326389eb4b36 Mon Sep 17 00:00:00 2001 From: Hynek Mlnarik Date: Mon, 26 Oct 2020 12:19:28 +0100 Subject: [PATCH] KEYCLOAK-16077 Remove need for MapStorage.replace --- .../client/ClientPolicyProviderFactory.java | 2 +- ...nispanPublicKeyStorageProviderFactory.java | 9 +++--- .../keycloak/models/jpa/ClientAdapter.java | 3 +- .../keycloak/models/jpa/JpaRealmProvider.java | 16 +++++----- .../models/map/client/MapClientProvider.java | 10 +++---- .../map/storage/MapKeycloakTransaction.java | 15 ---------- .../models/map/storage/MapStorage.java | 2 -- .../store/AuthorizationStoreFactory.java | 2 +- .../ClientApplicationSynchronizer.java | 2 +- .../AbstractLoginProtocolFactory.java | 4 +-- .../client/AbstractClientStorageAdapter.java | 2 +- .../java/org/keycloak/models/ClientModel.java | 29 ++++++++++++++++++- .../java/org/keycloak/models/RealmModel.java | 15 ---------- .../admin/permissions/AdminPermissions.java | 4 +-- 14 files changed, 53 insertions(+), 62 deletions(-) diff --git a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/client/ClientPolicyProviderFactory.java b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/client/ClientPolicyProviderFactory.java index c48dacdf96..b47fea1e4d 100644 --- a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/client/ClientPolicyProviderFactory.java +++ b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/client/ClientPolicyProviderFactory.java @@ -23,7 +23,7 @@ import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; import org.keycloak.models.RealmModel; -import org.keycloak.models.RealmModel.ClientRemovedEvent; +import org.keycloak.models.ClientModel.ClientRemovedEvent; import org.keycloak.representations.idm.authorization.ClientPolicyRepresentation; import org.keycloak.representations.idm.authorization.PolicyRepresentation; import org.keycloak.util.JsonSerialization; diff --git a/model/infinispan/src/main/java/org/keycloak/keys/infinispan/InfinispanPublicKeyStorageProviderFactory.java b/model/infinispan/src/main/java/org/keycloak/keys/infinispan/InfinispanPublicKeyStorageProviderFactory.java index b977214302..c11b0d702f 100644 --- a/model/infinispan/src/main/java/org/keycloak/keys/infinispan/InfinispanPublicKeyStorageProviderFactory.java +++ b/model/infinispan/src/main/java/org/keycloak/keys/infinispan/InfinispanPublicKeyStorageProviderFactory.java @@ -32,6 +32,7 @@ import org.keycloak.jose.jwk.JWK; import org.keycloak.keys.PublicKeyStorageProvider; import org.keycloak.keys.PublicKeyStorageProviderFactory; import org.keycloak.keys.PublicKeyStorageUtils; +import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; import org.keycloak.models.RealmModel; @@ -117,15 +118,15 @@ public class InfinispanPublicKeyStorageProviderFactory implements PublicKeyStora private SessionAndKeyHolder getCacheKeyToInvalidate(ProviderEvent event) { ArrayList cacheKeys = new ArrayList<>(); String cacheKey = null; - if (event instanceof RealmModel.ClientUpdatedEvent) { - RealmModel.ClientUpdatedEvent eventt = (RealmModel.ClientUpdatedEvent) event; + if (event instanceof ClientModel.ClientUpdatedEvent) { + ClientModel.ClientUpdatedEvent eventt = (ClientModel.ClientUpdatedEvent) event; cacheKey = PublicKeyStorageUtils.getClientModelCacheKey(eventt.getUpdatedClient().getRealm().getId(), eventt.getUpdatedClient().getId(), JWK.Use.SIG); cacheKeys.add(cacheKey); cacheKey = PublicKeyStorageUtils.getClientModelCacheKey(eventt.getUpdatedClient().getRealm().getId(), eventt.getUpdatedClient().getId(), JWK.Use.ENCRYPTION); cacheKeys.add(cacheKey); return new SessionAndKeyHolder(eventt.getKeycloakSession(), cacheKeys); - } else if (event instanceof RealmModel.ClientRemovedEvent) { - RealmModel.ClientRemovedEvent eventt = (RealmModel.ClientRemovedEvent) event; + } else if (event instanceof ClientModel.ClientRemovedEvent) { + ClientModel.ClientRemovedEvent eventt = (ClientModel.ClientRemovedEvent) event; cacheKey = PublicKeyStorageUtils.getClientModelCacheKey(eventt.getClient().getRealm().getId(), eventt.getClient().getId(), JWK.Use.SIG); cacheKeys.add(cacheKey); cacheKey = PublicKeyStorageUtils.getClientModelCacheKey(eventt.getClient().getRealm().getId(), eventt.getClient().getId(), JWK.Use.ENCRYPTION); 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 aece277059..4202327032 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 @@ -513,8 +513,7 @@ public class ClientAdapter implements ClientModel, JpaModel { @Override public void updateClient() { - em.flush(); - session.getKeycloakSessionFactory().publish(new RealmModel.ClientUpdatedEvent() { + session.getKeycloakSessionFactory().publish(new ClientModel.ClientUpdatedEvent() { @Override public ClientModel getUpdatedClient() { diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java index 7bcca946bf..b727ad82cc 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java @@ -624,6 +624,10 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, GroupPro @Override public ClientModel addClient(RealmModel realm, String id, String clientId) { + if (id == null) { + id = KeycloakModelUtils.generateId(); + } + if (clientId == null) { clientId = id; } @@ -638,16 +642,10 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, GroupPro RealmEntity realmRef = em.getReference(RealmEntity.class, realm.getId()); entity.setRealm(realmRef); em.persist(entity); - em.flush(); + final ClientModel resource = new ClientAdapter(realm, em, session, entity); - em.flush(); - session.getKeycloakSessionFactory().publish(new RealmModel.ClientCreationEvent() { - @Override - public ClientModel getCreatedClient() { - return resource; - } - }); + session.getKeycloakSessionFactory().publish((ClientModel.ClientCreationEvent) () -> resource); return resource; } @@ -745,7 +743,7 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, GroupPro ClientEntity clientEntity = em.find(ClientEntity.class, id, LockModeType.PESSIMISTIC_WRITE); - session.getKeycloakSessionFactory().publish(new RealmModel.ClientRemovedEvent() { + session.getKeycloakSessionFactory().publish(new ClientModel.ClientRemovedEvent() { @Override public ClientModel getClient() { return client; diff --git a/model/map/src/main/java/org/keycloak/models/map/client/MapClientProvider.java b/model/map/src/main/java/org/keycloak/models/map/client/MapClientProvider.java index 96668389e8..a11ee5a26a 100644 --- a/model/map/src/main/java/org/keycloak/models/map/client/MapClientProvider.java +++ b/model/map/src/main/java/org/keycloak/models/map/client/MapClientProvider.java @@ -19,12 +19,12 @@ package org.keycloak.models.map.client; import org.jboss.logging.Logger; import org.keycloak.models.ClientModel; +import org.keycloak.models.ClientModel.ClientUpdatedEvent; import org.keycloak.models.ClientProvider; import org.keycloak.models.KeycloakSession; import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.RealmModel; -import org.keycloak.models.RealmModel.ClientUpdatedEvent; import org.keycloak.models.map.storage.MapKeycloakTransaction; import org.keycloak.models.map.common.Serialization; import java.util.Comparator; @@ -71,7 +71,7 @@ public class MapClientProvider implements ClientProvider { } private ClientUpdatedEvent clientUpdatedEvent(ClientModel c) { - return new RealmModel.ClientUpdatedEvent() { + return new ClientModel.ClientUpdatedEvent() { @Override public ClientModel getUpdatedClient() { return c; @@ -96,8 +96,6 @@ public class MapClientProvider implements ClientProvider { return origEntity -> new MapClientAdapter(session, realm, registerEntityForChanges(origEntity)) { @Override public void updateClient() { - // commit - MapClientProvider.this.tx.replace(entity.getId(), this.entity); session.getKeycloakSessionFactory().publish(clientUpdatedEvent(this)); } @@ -178,7 +176,7 @@ public class MapClientProvider implements ClientProvider { final ClientModel resource = entityToAdapterFunc(realm).apply(entity); // TODO: Sending an event should be extracted to store layer - session.getKeycloakSessionFactory().publish((RealmModel.ClientCreationEvent) () -> resource); + session.getKeycloakSessionFactory().publish((ClientModel.ClientCreationEvent) () -> resource); resource.updateClient(); // This is actualy strange contract - it should be the store code to call updateClient return resource; @@ -214,7 +212,7 @@ public class MapClientProvider implements ClientProvider { session.users().preRemove(realm, client); session.roles().removeRoles(client); - session.getKeycloakSessionFactory().publish(new RealmModel.ClientRemovedEvent() { + session.getKeycloakSessionFactory().publish(new ClientModel.ClientRemovedEvent() { @Override public ClientModel getClient() { return client; diff --git a/model/map/src/main/java/org/keycloak/models/map/storage/MapKeycloakTransaction.java b/model/map/src/main/java/org/keycloak/models/map/storage/MapKeycloakTransaction.java index 7b24d3dd06..55605bd9fd 100644 --- a/model/map/src/main/java/org/keycloak/models/map/storage/MapKeycloakTransaction.java +++ b/model/map/src/main/java/org/keycloak/models/map/storage/MapKeycloakTransaction.java @@ -65,17 +65,6 @@ public class MapKeycloakTransaction implements KeycloakTransaction { }; } }, - REPLACE { - @Override - protected MapTaskWithValue taskFor(K key, V value) { - return new MapTaskWithValue(value) { - @Override - public void execute(MapStorage map) { - map.replace(key, getValue()); - } - }; - } - }, ; protected abstract MapTaskWithValue taskFor(K key, V value); @@ -179,10 +168,6 @@ public class MapKeycloakTransaction implements KeycloakTransaction { tasks.merge(taskKey, op, MapTaskCompose::new); } - public void replace(K key, V value) { - addTask(MapOperation.REPLACE, key, value); - } - public void remove(K key) { addTask(MapOperation.REMOVE, key, null); } diff --git a/model/map/src/main/java/org/keycloak/models/map/storage/MapStorage.java b/model/map/src/main/java/org/keycloak/models/map/storage/MapStorage.java index f39adc1143..b7935f758f 100644 --- a/model/map/src/main/java/org/keycloak/models/map/storage/MapStorage.java +++ b/model/map/src/main/java/org/keycloak/models/map/storage/MapStorage.java @@ -34,8 +34,6 @@ public interface MapStorage { V remove(K key); - V replace(K key, V value); - Set keySet(); Set> entrySet(); diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/store/AuthorizationStoreFactory.java b/server-spi-private/src/main/java/org/keycloak/authorization/store/AuthorizationStoreFactory.java index a54f1950a6..b624b94a36 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/store/AuthorizationStoreFactory.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/store/AuthorizationStoreFactory.java @@ -28,7 +28,7 @@ import org.keycloak.authorization.store.syncronization.Synchronizer; import org.keycloak.authorization.store.syncronization.UserSynchronizer; import org.keycloak.models.GroupModel; import org.keycloak.models.KeycloakSessionFactory; -import org.keycloak.models.RealmModel.ClientRemovedEvent; +import org.keycloak.models.ClientModel.ClientRemovedEvent; import org.keycloak.models.RealmModel.RealmRemovedEvent; import org.keycloak.models.UserModel.UserRemovedEvent; import org.keycloak.provider.ProviderEvent; diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/store/syncronization/ClientApplicationSynchronizer.java b/server-spi-private/src/main/java/org/keycloak/authorization/store/syncronization/ClientApplicationSynchronizer.java index dfc069ea1c..f6e055504a 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/store/syncronization/ClientApplicationSynchronizer.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/store/syncronization/ClientApplicationSynchronizer.java @@ -30,7 +30,7 @@ import org.keycloak.authorization.policy.provider.PolicyProviderFactory; import org.keycloak.authorization.store.ResourceServerStore; import org.keycloak.authorization.store.StoreFactory; import org.keycloak.models.KeycloakSessionFactory; -import org.keycloak.models.RealmModel.ClientRemovedEvent; +import org.keycloak.models.ClientModel.ClientRemovedEvent; import org.keycloak.provider.ProviderFactory; import org.keycloak.representations.idm.authorization.ClientPolicyRepresentation; 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 29de53a643..117d4cb71b 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 @@ -46,8 +46,8 @@ public abstract class AbstractLoginProtocolFactory implements LoginProtocolFacto factory.register(new ProviderEventListener() { @Override public void onEvent(ProviderEvent event) { - if (event instanceof RealmModel.ClientCreationEvent) { - ClientModel client = ((RealmModel.ClientCreationEvent)event).getCreatedClient(); + if (event instanceof ClientModel.ClientCreationEvent) { + ClientModel client = ((ClientModel.ClientCreationEvent)event).getCreatedClient(); addDefaultClientScopes(client.getRealm(), client); addDefaults(client); } diff --git a/server-spi-private/src/main/java/org/keycloak/storage/client/AbstractClientStorageAdapter.java b/server-spi-private/src/main/java/org/keycloak/storage/client/AbstractClientStorageAdapter.java index db2a9adeac..4cced449c9 100644 --- a/server-spi-private/src/main/java/org/keycloak/storage/client/AbstractClientStorageAdapter.java +++ b/server-spi-private/src/main/java/org/keycloak/storage/client/AbstractClientStorageAdapter.java @@ -120,7 +120,7 @@ public abstract class AbstractClientStorageAdapter extends UnsupportedOperations */ @Override public void updateClient() { - session.getKeycloakSessionFactory().publish(new RealmModel.ClientUpdatedEvent() { + session.getKeycloakSessionFactory().publish(new ClientModel.ClientUpdatedEvent() { @Override public ClientModel getUpdatedClient() { 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 a7a6112608..e40328e76c 100755 --- a/server-spi/src/main/java/org/keycloak/models/ClientModel.java +++ b/server-spi/src/main/java/org/keycloak/models/ClientModel.java @@ -21,6 +21,8 @@ import java.util.Map; import java.util.Set; import org.keycloak.common.util.ObjectUtil; +import org.keycloak.provider.ProviderEvent; +import org.keycloak.provider.ProviderEventManager; /** * @author Bill Burke @@ -34,10 +36,35 @@ public interface ClientModel extends ClientScopeModel, RoleContainerModel, Prot String PUBLIC_KEY = "publicKey"; String X509CERTIFICATE = "X509Certificate"; + interface ClientCreationEvent extends ProviderEvent { + ClientModel getCreatedClient(); + } + + // Called also during client creation after client is fully initialized (including all attributes etc) + interface ClientUpdatedEvent extends ProviderEvent { + ClientModel getUpdatedClient(); + KeycloakSession getKeycloakSession(); + } + + interface ClientRemovedEvent extends ProviderEvent { + ClientModel getClient(); + KeycloakSession getKeycloakSession(); + } + /** - * Stores the current state of the client immediately to the underlying store, similarly to a commit. + * Notifies other providers that this client has been updated. + *

+ * After a client is updated, providers can register for {@link ClientUpdatedEvent}. + * The setters in this model do not send an update for individual updates of the model. + * This method is here to allow for sending this event for this client, + * allowsing for to group multiple changes of a client and signal that + * all the changes in this client have been performed. * * @deprecated Do not use, to be removed + * + * @see ProviderEvent + * @see ProviderEventManager + * @see ClientUpdatedEvent */ void updateClient(); diff --git a/server-spi/src/main/java/org/keycloak/models/RealmModel.java b/server-spi/src/main/java/org/keycloak/models/RealmModel.java index 649d9b7104..925a7fb0c5 100755 --- a/server-spi/src/main/java/org/keycloak/models/RealmModel.java +++ b/server-spi/src/main/java/org/keycloak/models/RealmModel.java @@ -52,21 +52,6 @@ public interface RealmModel extends RoleContainerModel { KeycloakSession getKeycloakSession(); } - interface ClientCreationEvent extends ProviderEvent { - ClientModel getCreatedClient(); - } - - // Called also during client creation after client is fully initialized (including all attributes etc) - interface ClientUpdatedEvent extends ProviderEvent { - ClientModel getUpdatedClient(); - KeycloakSession getKeycloakSession(); - } - - interface ClientRemovedEvent extends ProviderEvent { - ClientModel getClient(); - KeycloakSession getKeycloakSession(); - } - interface IdentityProviderUpdatedEvent extends ProviderEvent { RealmModel getRealm(); IdentityProviderModel getUpdatedIdentityProvider(); diff --git a/services/src/main/java/org/keycloak/services/resources/admin/permissions/AdminPermissions.java b/services/src/main/java/org/keycloak/services/resources/admin/permissions/AdminPermissions.java index 705b258c4b..2b3dd29e87 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/permissions/AdminPermissions.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/permissions/AdminPermissions.java @@ -69,8 +69,8 @@ public class AdminPermissions { realm = (RealmModel)role.getContainer(); } management(cast.getKeycloakSession(), realm).roles().setPermissionsEnabled(role, false); - } else if (event instanceof RealmModel.ClientRemovedEvent) { - RealmModel.ClientRemovedEvent cast = (RealmModel.ClientRemovedEvent)event; + } else if (event instanceof ClientModel.ClientRemovedEvent) { + ClientModel.ClientRemovedEvent cast = (ClientModel.ClientRemovedEvent)event; management(cast.getKeycloakSession(), cast.getClient().getRealm()).clients().setPermissionsEnabled(cast.getClient(), false); } else if (event instanceof GroupModel.GroupRemovedEvent) { GroupModel.GroupRemovedEvent cast = (GroupModel.GroupRemovedEvent)event;