From 44cd6cd5fb6811853405e9ba3b6d1624dc280f2e Mon Sep 17 00:00:00 2001 From: Hynek Mlnarik Date: Sat, 17 Jul 2021 16:18:04 +0200 Subject: [PATCH] KEYCLOAK-18824 Simplify MapStorageTransaction and move registerEntityForChanges to CHM transaction --- .../MapRootAuthenticationSessionEntity.java | 3 +- .../MapRootAuthenticationSessionProvider.java | 5 +- ...tAuthenticationSessionProviderFactory.java | 2 +- .../MapAuthorizationStoreFactory.java | 10 +-- .../MapPermissionTicketStore.java | 3 +- .../map/authorization/MapPolicyStore.java | 3 +- .../authorization/MapResourceServerStore.java | 3 +- .../map/authorization/MapResourceStore.java | 3 +- .../map/authorization/MapScopeStore.java | 3 +- .../entity/MapPermissionTicketEntity.java | 4 +- .../authorization/entity/MapPolicyEntity.java | 4 +- .../entity/MapResourceEntity.java | 4 +- .../entity/MapResourceServerEntity.java | 3 +- .../authorization/entity/MapScopeEntity.java | 3 +- .../models/map/client/MapClientEntity.java | 3 +- .../models/map/client/MapClientProvider.java | 3 +- .../map/client/MapClientProviderFactory.java | 2 +- .../map/clientscope/MapClientScopeEntity.java | 3 +- .../clientscope/MapClientScopeProvider.java | 3 +- .../MapClientScopeProviderFactory.java | 2 +- .../models/map/common/AbstractEntity.java | 2 +- .../common/AbstractMapProviderFactory.java | 7 +- .../models/map/common/MapStorageUtils.java | 42 ----------- .../models/map/group/MapGroupEntity.java | 3 +- .../models/map/group/MapGroupProvider.java | 3 +- .../map/group/MapGroupProviderFactory.java | 2 +- .../MapUserLoginFailureEntity.java | 3 +- .../MapUserLoginFailureProvider.java | 7 +- .../MapUserLoginFailureProviderFactory.java | 2 +- .../models/map/realm/MapRealmEntity.java | 3 +- .../models/map/realm/MapRealmProvider.java | 4 +- .../map/realm/MapRealmProviderFactory.java | 2 +- .../models/map/role/MapRoleEntity.java | 3 +- .../models/map/role/MapRoleProvider.java | 3 +- .../map/role/MapRoleProviderFactory.java | 2 +- .../map/storage/MapKeycloakTransaction.java | 54 +++----------- .../models/map/storage/MapStorage.java | 8 +-- .../map/storage/MapStorageProvider.java | 9 +-- .../ConcurrentHashMapKeycloakTransaction.java | 58 ++++++++------- .../storage/chm/ConcurrentHashMapStorage.java | 3 +- .../chm/ConcurrentHashMapStorageProvider.java | 8 ++- ...ncurrentHashMapStorageProviderFactory.java | 38 +++++++--- .../map/storage/chm/MapFieldPredicates.java | 3 +- .../UserSessionConcurrentHashMapStorage.java | 8 +-- .../models/map/user/MapUserEntity.java | 3 +- .../models/map/user/MapUserProvider.java | 71 ++++++------------- .../map/user/MapUserProviderFactory.java | 2 +- .../MapAuthenticatedClientSessionEntity.java | 3 +- .../map/userSession/MapUserSessionEntity.java | 3 +- .../userSession/MapUserSessionProvider.java | 16 ++--- .../MapUserSessionProviderFactory.java | 4 +- .../storage/SearchableModelField.java | 2 - .../testsuite/model/MapStorageTest.java | 12 ++-- 53 files changed, 191 insertions(+), 271 deletions(-) delete mode 100644 model/map/src/main/java/org/keycloak/models/map/common/MapStorageUtils.java diff --git a/model/map/src/main/java/org/keycloak/models/map/authSession/MapRootAuthenticationSessionEntity.java b/model/map/src/main/java/org/keycloak/models/map/authSession/MapRootAuthenticationSessionEntity.java index 60e4efa8ed..d16ae4f81c 100644 --- a/model/map/src/main/java/org/keycloak/models/map/authSession/MapRootAuthenticationSessionEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/authSession/MapRootAuthenticationSessionEntity.java @@ -18,6 +18,7 @@ package org.keycloak.models.map.authSession; import org.keycloak.models.map.common.AbstractEntity; +import org.keycloak.models.map.common.UpdatableEntity; import java.util.Map; import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; @@ -25,7 +26,7 @@ import java.util.concurrent.ConcurrentHashMap; /** * @author Martin Kanis */ -public class MapRootAuthenticationSessionEntity implements AbstractEntity { +public class MapRootAuthenticationSessionEntity implements AbstractEntity, UpdatableEntity { private K id; private String realmId; diff --git a/model/map/src/main/java/org/keycloak/models/map/authSession/MapRootAuthenticationSessionProvider.java b/model/map/src/main/java/org/keycloak/models/map/authSession/MapRootAuthenticationSessionProvider.java index 19b4cb2ad9..75175943a5 100644 --- a/model/map/src/main/java/org/keycloak/models/map/authSession/MapRootAuthenticationSessionProvider.java +++ b/model/map/src/main/java/org/keycloak/models/map/authSession/MapRootAuthenticationSessionProvider.java @@ -39,7 +39,6 @@ import java.util.function.Function; import java.util.function.Predicate; import static org.keycloak.common.util.StackUtil.getShortStackTrace; -import static org.keycloak.models.map.common.MapStorageUtils.registerEntityForChanges; import static org.keycloak.models.map.storage.QueryParameters.withCriteria; /** @@ -65,7 +64,7 @@ public class MapRootAuthenticationSessionProvider implements AuthenticationSe private Function, RootAuthenticationSessionModel> entityToAdapterFunc(RealmModel realm) { // Clone entity before returning back, to avoid giving away a reference to the live object to the caller - return origEntity -> new MapRootAuthenticationSessionAdapter(session, realm, registerEntityForChanges(tx, origEntity)) { + return origEntity -> new MapRootAuthenticationSessionAdapter(session, realm, origEntity) { @Override public String getId() { return sessionStore.getKeyConvertor().keyToString(entity.getId()); @@ -145,7 +144,7 @@ public class MapRootAuthenticationSessionProvider implements AuthenticationSe .compare(SearchableFields.REALM_ID, Operator.EQ, realm.getId()) .compare(SearchableFields.TIMESTAMP, Operator.LT, expired); - long deletedCount = tx.delete(sessionStore.getKeyConvertor().yieldNewUniqueKey(), withCriteria(mcb)); + long deletedCount = tx.delete(withCriteria(mcb)); LOG.debugf("Removed %d expired authentication sessions for realm '%s'", deletedCount, realm.getName()); } diff --git a/model/map/src/main/java/org/keycloak/models/map/authSession/MapRootAuthenticationSessionProviderFactory.java b/model/map/src/main/java/org/keycloak/models/map/authSession/MapRootAuthenticationSessionProviderFactory.java index 1d2f1d15ef..be9ce4e709 100644 --- a/model/map/src/main/java/org/keycloak/models/map/authSession/MapRootAuthenticationSessionProviderFactory.java +++ b/model/map/src/main/java/org/keycloak/models/map/authSession/MapRootAuthenticationSessionProviderFactory.java @@ -30,7 +30,7 @@ public class MapRootAuthenticationSessionProviderFactory extends AbstractMapP implements AuthenticationSessionProviderFactory { public MapRootAuthenticationSessionProviderFactory() { - super(MapRootAuthenticationSessionEntity.class, RootAuthenticationSessionModel.class); + super(RootAuthenticationSessionModel.class); } @Override diff --git a/model/map/src/main/java/org/keycloak/models/map/authorization/MapAuthorizationStoreFactory.java b/model/map/src/main/java/org/keycloak/models/map/authorization/MapAuthorizationStoreFactory.java index f5420da2a3..3a870b52a4 100644 --- a/model/map/src/main/java/org/keycloak/models/map/authorization/MapAuthorizationStoreFactory.java +++ b/model/map/src/main/java/org/keycloak/models/map/authorization/MapAuthorizationStoreFactory.java @@ -65,11 +65,11 @@ public class MapAuthorizationStoreFactory implements AmphibianProviderFactory MapStorage resourceStore; MapStorage scopeStore; - permissionTicketStore = mapStorageProvider.getStorage(MapPermissionTicketEntity.class, PermissionTicket.class); - policyStore = mapStorageProvider.getStorage(MapPolicyEntity.class, Policy.class); - resourceServerStore = mapStorageProvider.getStorage(MapResourceServerEntity.class, ResourceServer.class); - resourceStore = mapStorageProvider.getStorage(MapResourceEntity.class, Resource.class); - scopeStore = mapStorageProvider.getStorage(MapScopeEntity.class, Scope.class); + permissionTicketStore = mapStorageProvider.getStorage(PermissionTicket.class); + policyStore = mapStorageProvider.getStorage(Policy.class); + resourceServerStore = mapStorageProvider.getStorage(ResourceServer.class); + resourceStore = mapStorageProvider.getStorage(Resource.class); + scopeStore = mapStorageProvider.getStorage(Scope.class); return new MapAuthorizationStore(session, permissionTicketStore, diff --git a/model/map/src/main/java/org/keycloak/models/map/authorization/MapPermissionTicketStore.java b/model/map/src/main/java/org/keycloak/models/map/authorization/MapPermissionTicketStore.java index 7bdb83fae8..0aabd45b70 100644 --- a/model/map/src/main/java/org/keycloak/models/map/authorization/MapPermissionTicketStore.java +++ b/model/map/src/main/java/org/keycloak/models/map/authorization/MapPermissionTicketStore.java @@ -43,7 +43,6 @@ import java.util.function.Function; import java.util.stream.Collectors; import static org.keycloak.common.util.StackUtil.getShortStackTrace; -import static org.keycloak.models.map.common.MapStorageUtils.registerEntityForChanges; import static org.keycloak.models.map.storage.QueryParameters.Order.ASCENDING; import static org.keycloak.models.map.storage.QueryParameters.withCriteria; import static org.keycloak.utils.StreamsUtil.distinctByKey; @@ -66,7 +65,7 @@ public class MapPermissionTicketStore> implements Permis private PermissionTicket entityToAdapter(MapPermissionTicketEntity origEntity) { if (origEntity == null) return null; // Clone entity before returning back, to avoid giving away a reference to the live object to the caller - return new MapPermissionTicketAdapter(registerEntityForChanges(tx, origEntity), authorizationProvider.getStoreFactory()) { + return new MapPermissionTicketAdapter(origEntity, authorizationProvider.getStoreFactory()) { @Override public String getId() { return permissionTicketStore.getKeyConvertor().keyToString(entity.getId()); diff --git a/model/map/src/main/java/org/keycloak/models/map/authorization/MapPolicyStore.java b/model/map/src/main/java/org/keycloak/models/map/authorization/MapPolicyStore.java index 988ea50174..31f95f0c06 100644 --- a/model/map/src/main/java/org/keycloak/models/map/authorization/MapPolicyStore.java +++ b/model/map/src/main/java/org/keycloak/models/map/authorization/MapPolicyStore.java @@ -41,7 +41,6 @@ import java.util.function.Consumer; import java.util.stream.Collectors; import static org.keycloak.common.util.StackUtil.getShortStackTrace; -import static org.keycloak.models.map.common.MapStorageUtils.registerEntityForChanges; import static org.keycloak.models.map.storage.QueryParameters.withCriteria; public class MapPolicyStore implements PolicyStore { @@ -61,7 +60,7 @@ public class MapPolicyStore implements PolicyStore { private Policy entityToAdapter(MapPolicyEntity origEntity) { if (origEntity == null) return null; // Clone entity before returning back, to avoid giving away a reference to the live object to the caller - return new MapPolicyAdapter(registerEntityForChanges(tx, origEntity), authorizationProvider.getStoreFactory()) { + return new MapPolicyAdapter(origEntity, authorizationProvider.getStoreFactory()) { @Override public String getId() { return policyStore.getKeyConvertor().keyToString(entity.getId()); diff --git a/model/map/src/main/java/org/keycloak/models/map/authorization/MapResourceServerStore.java b/model/map/src/main/java/org/keycloak/models/map/authorization/MapResourceServerStore.java index fd3e7ee9c9..5b0dfc6907 100644 --- a/model/map/src/main/java/org/keycloak/models/map/authorization/MapResourceServerStore.java +++ b/model/map/src/main/java/org/keycloak/models/map/authorization/MapResourceServerStore.java @@ -39,7 +39,6 @@ import org.keycloak.models.map.storage.MapStorage; import org.keycloak.storage.StorageId; import static org.keycloak.common.util.StackUtil.getShortStackTrace; -import static org.keycloak.models.map.common.MapStorageUtils.registerEntityForChanges; public class MapResourceServerStore implements ResourceServerStore { @@ -58,7 +57,7 @@ public class MapResourceServerStore implements ResourceServerStore { private ResourceServer entityToAdapter(MapResourceServerEntity origEntity) { if (origEntity == null) return null; // Clone entity before returning back, to avoid giving away a reference to the live object to the caller - return new MapResourceServerAdapter(registerEntityForChanges(tx, origEntity), authorizationProvider.getStoreFactory()) { + return new MapResourceServerAdapter(origEntity, authorizationProvider.getStoreFactory()) { @Override public String getId() { return resourceServerStore.getKeyConvertor().keyToString(entity.getId()); diff --git a/model/map/src/main/java/org/keycloak/models/map/authorization/MapResourceStore.java b/model/map/src/main/java/org/keycloak/models/map/authorization/MapResourceStore.java index da62c4bd33..013150bcad 100644 --- a/model/map/src/main/java/org/keycloak/models/map/authorization/MapResourceStore.java +++ b/model/map/src/main/java/org/keycloak/models/map/authorization/MapResourceStore.java @@ -40,7 +40,6 @@ import java.util.function.Consumer; import java.util.stream.Collectors; import static org.keycloak.common.util.StackUtil.getShortStackTrace; -import static org.keycloak.models.map.common.MapStorageUtils.registerEntityForChanges; import static org.keycloak.models.map.storage.QueryParameters.withCriteria; public class MapResourceStore> implements ResourceStore { @@ -60,7 +59,7 @@ public class MapResourceStore> implements ResourceStore private Resource entityToAdapter(MapResourceEntity origEntity) { if (origEntity == null) return null; // Clone entity before returning back, to avoid giving away a reference to the live object to the caller - return new MapResourceAdapter(registerEntityForChanges(tx, origEntity), authorizationProvider.getStoreFactory()) { + return new MapResourceAdapter(origEntity, authorizationProvider.getStoreFactory()) { @Override public String getId() { return resourceStore.getKeyConvertor().keyToString(entity.getId()); diff --git a/model/map/src/main/java/org/keycloak/models/map/authorization/MapScopeStore.java b/model/map/src/main/java/org/keycloak/models/map/authorization/MapScopeStore.java index 1440926ac1..0e88209394 100644 --- a/model/map/src/main/java/org/keycloak/models/map/authorization/MapScopeStore.java +++ b/model/map/src/main/java/org/keycloak/models/map/authorization/MapScopeStore.java @@ -38,7 +38,6 @@ import java.util.Map; import java.util.stream.Collectors; import static org.keycloak.common.util.StackUtil.getShortStackTrace; -import static org.keycloak.models.map.common.MapStorageUtils.registerEntityForChanges; import static org.keycloak.models.map.storage.QueryParameters.withCriteria; public class MapScopeStore implements ScopeStore { @@ -58,7 +57,7 @@ public class MapScopeStore implements ScopeStore { private Scope entityToAdapter(MapScopeEntity origEntity) { if (origEntity == null) return null; // Clone entity before returning back, to avoid giving away a reference to the live object to the caller - return new MapScopeAdapter(registerEntityForChanges(tx, origEntity), authorizationProvider.getStoreFactory()) { + return new MapScopeAdapter(origEntity, authorizationProvider.getStoreFactory()) { @Override public String getId() { return scopeStore.getKeyConvertor().keyToString(entity.getId()); diff --git a/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapPermissionTicketEntity.java b/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapPermissionTicketEntity.java index 351b5cfa83..75f68605da 100644 --- a/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapPermissionTicketEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapPermissionTicketEntity.java @@ -19,10 +19,10 @@ package org.keycloak.models.map.authorization.entity; import org.keycloak.models.map.common.AbstractEntity; -import java.util.Comparator; +import org.keycloak.models.map.common.UpdatableEntity; import java.util.Objects; -public class MapPermissionTicketEntity implements AbstractEntity { +public class MapPermissionTicketEntity implements AbstractEntity, UpdatableEntity { private final K id; private String owner; diff --git a/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapPolicyEntity.java b/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapPolicyEntity.java index 35edf85372..bf6eb82eb0 100644 --- a/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapPolicyEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapPolicyEntity.java @@ -18,17 +18,17 @@ package org.keycloak.models.map.authorization.entity; import org.keycloak.models.map.common.AbstractEntity; +import org.keycloak.models.map.common.UpdatableEntity; import org.keycloak.representations.idm.authorization.DecisionStrategy; import org.keycloak.representations.idm.authorization.Logic; -import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; import java.util.Set; import java.util.Map; import java.util.Objects; -public class MapPolicyEntity implements AbstractEntity { +public class MapPolicyEntity implements AbstractEntity, UpdatableEntity { private final K id; private String name; diff --git a/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapResourceEntity.java b/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapResourceEntity.java index c1875365b7..900455b319 100644 --- a/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapResourceEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapResourceEntity.java @@ -19,7 +19,7 @@ package org.keycloak.models.map.authorization.entity; import org.keycloak.models.map.common.AbstractEntity; -import java.util.Comparator; +import org.keycloak.models.map.common.UpdatableEntity; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -27,7 +27,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; -public class MapResourceEntity implements AbstractEntity { +public class MapResourceEntity implements AbstractEntity, UpdatableEntity { private final K id; private String name; diff --git a/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapResourceServerEntity.java b/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapResourceServerEntity.java index 00776bf771..adbcd7d65b 100644 --- a/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapResourceServerEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapResourceServerEntity.java @@ -18,12 +18,13 @@ package org.keycloak.models.map.authorization.entity; import org.keycloak.models.map.common.AbstractEntity; +import org.keycloak.models.map.common.UpdatableEntity; import org.keycloak.representations.idm.authorization.DecisionStrategy; import org.keycloak.representations.idm.authorization.PolicyEnforcementMode; import java.util.Objects; -public class MapResourceServerEntity implements AbstractEntity { +public class MapResourceServerEntity implements AbstractEntity, UpdatableEntity { private final K id; private boolean updated = false; diff --git a/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapScopeEntity.java b/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapScopeEntity.java index b32461c4b5..9fed56372f 100644 --- a/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapScopeEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/authorization/entity/MapScopeEntity.java @@ -19,9 +19,10 @@ package org.keycloak.models.map.authorization.entity; import org.keycloak.models.map.common.AbstractEntity; +import org.keycloak.models.map.common.UpdatableEntity; import java.util.Objects; -public class MapScopeEntity implements AbstractEntity { +public class MapScopeEntity implements AbstractEntity, UpdatableEntity { private final K id; private String name; diff --git a/model/map/src/main/java/org/keycloak/models/map/client/MapClientEntity.java b/model/map/src/main/java/org/keycloak/models/map/client/MapClientEntity.java index 1ac88c177d..5a2aa9db76 100644 --- a/model/map/src/main/java/org/keycloak/models/map/client/MapClientEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/client/MapClientEntity.java @@ -18,6 +18,7 @@ package org.keycloak.models.map.client; import org.keycloak.models.ProtocolMapperModel; import org.keycloak.models.map.common.AbstractEntity; +import org.keycloak.models.map.common.UpdatableEntity; import java.util.Collection; import java.util.List; import java.util.Map; @@ -28,7 +29,7 @@ import java.util.stream.Stream; * * @author hmlnarik */ -public interface MapClientEntity extends AbstractEntity { +public interface MapClientEntity extends AbstractEntity, UpdatableEntity { void addClientScope(String id, Boolean defaultScope); 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 6b4a9c6ed1..a94ee8c508 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 @@ -44,7 +44,6 @@ import org.keycloak.models.map.storage.ModelCriteriaBuilder; import org.keycloak.models.map.storage.ModelCriteriaBuilder.Operator; import static org.keycloak.common.util.StackUtil.getShortStackTrace; import org.keycloak.models.ClientScopeModel; -import static org.keycloak.models.map.common.MapStorageUtils.registerEntityForChanges; import static org.keycloak.models.map.storage.QueryParameters.Order.ASCENDING; import static org.keycloak.models.map.storage.QueryParameters.withCriteria; @@ -84,7 +83,7 @@ public class MapClientProvider implements ClientProvider { private > Function entityToAdapterFunc(RealmModel realm) { // Clone entity before returning back, to avoid giving away a reference to the live object to the caller - return (T origEntity) -> new MapClientAdapter(session, realm, registerEntityForChanges(tx, origEntity)) { + return origEntity -> new MapClientAdapter(session, realm, origEntity) { @Override public String getId() { return clientStore.getKeyConvertor().keyToString(entity.getId()); diff --git a/model/map/src/main/java/org/keycloak/models/map/client/MapClientProviderFactory.java b/model/map/src/main/java/org/keycloak/models/map/client/MapClientProviderFactory.java index 60fa6a47f0..a9daeb1e04 100644 --- a/model/map/src/main/java/org/keycloak/models/map/client/MapClientProviderFactory.java +++ b/model/map/src/main/java/org/keycloak/models/map/client/MapClientProviderFactory.java @@ -42,7 +42,7 @@ public class MapClientProviderFactory extends AbstractMapProviderFactory implements AbstractEntity { +public class MapClientScopeEntity implements AbstractEntity, UpdatableEntity { private final K id; private final String realmId; diff --git a/model/map/src/main/java/org/keycloak/models/map/clientscope/MapClientScopeProvider.java b/model/map/src/main/java/org/keycloak/models/map/clientscope/MapClientScopeProvider.java index 7c72faac6a..77ddc2cfa5 100644 --- a/model/map/src/main/java/org/keycloak/models/map/clientscope/MapClientScopeProvider.java +++ b/model/map/src/main/java/org/keycloak/models/map/clientscope/MapClientScopeProvider.java @@ -36,7 +36,6 @@ import org.keycloak.models.map.storage.ModelCriteriaBuilder; import org.keycloak.models.map.storage.ModelCriteriaBuilder.Operator; import org.keycloak.models.utils.KeycloakModelUtils; -import static org.keycloak.models.map.common.MapStorageUtils.registerEntityForChanges; import static org.keycloak.models.map.storage.QueryParameters.Order.ASCENDING; import static org.keycloak.models.map.storage.QueryParameters.withCriteria; @@ -57,7 +56,7 @@ public class MapClientScopeProvider implements ClientScopeProvider { private Function, ClientScopeModel> entityToAdapterFunc(RealmModel realm) { // Clone entity before returning back, to avoid giving away a reference to the live object to the caller - return origEntity -> new MapClientScopeAdapter(session, realm, registerEntityForChanges(tx, origEntity)) { + return origEntity -> new MapClientScopeAdapter(session, realm, origEntity) { @Override public String getId() { return clientScopeStore.getKeyConvertor().keyToString(entity.getId()); diff --git a/model/map/src/main/java/org/keycloak/models/map/clientscope/MapClientScopeProviderFactory.java b/model/map/src/main/java/org/keycloak/models/map/clientscope/MapClientScopeProviderFactory.java index 6344db4d36..a01cbc3353 100644 --- a/model/map/src/main/java/org/keycloak/models/map/clientscope/MapClientScopeProviderFactory.java +++ b/model/map/src/main/java/org/keycloak/models/map/clientscope/MapClientScopeProviderFactory.java @@ -25,7 +25,7 @@ import org.keycloak.models.map.common.AbstractMapProviderFactory; public class MapClientScopeProviderFactory extends AbstractMapProviderFactory, ClientScopeModel> implements ClientScopeProviderFactory { public MapClientScopeProviderFactory() { - super(MapClientScopeEntity.class, ClientScopeModel.class); + super(ClientScopeModel.class); } @Override diff --git a/model/map/src/main/java/org/keycloak/models/map/common/AbstractEntity.java b/model/map/src/main/java/org/keycloak/models/map/common/AbstractEntity.java index 76786ea491..00395bf748 100644 --- a/model/map/src/main/java/org/keycloak/models/map/common/AbstractEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/common/AbstractEntity.java @@ -20,7 +20,7 @@ package org.keycloak.models.map.common; * * @author hmlnarik */ -public interface AbstractEntity extends UpdatableEntity { +public interface AbstractEntity { K getId(); diff --git a/model/map/src/main/java/org/keycloak/models/map/common/AbstractMapProviderFactory.java b/model/map/src/main/java/org/keycloak/models/map/common/AbstractMapProviderFactory.java index b30a97e1ef..05ce5bde7c 100644 --- a/model/map/src/main/java/org/keycloak/models/map/common/AbstractMapProviderFactory.java +++ b/model/map/src/main/java/org/keycloak/models/map/common/AbstractMapProviderFactory.java @@ -44,14 +44,11 @@ public abstract class AbstractMapProviderFactory modelType; - protected final Class entityType; - private Scope storageConfigScope; @SuppressWarnings("unchecked") - protected AbstractMapProviderFactory(Class entityType, Class modelType) { + protected AbstractMapProviderFactory(Class modelType) { this.modelType = modelType; - this.entityType = (Class) entityType; } @Override @@ -64,7 +61,7 @@ public abstract class AbstractMapProviderFactory - * Usually used before giving an entity from a source back to the caller, - * to prevent changing it directly in the data store, but to keep transactional properties. - * @param - * @param - * @param tx Transaction that is checked for existence of the entity before - * @param origEntity - * @return - */ - public static > V registerEntityForChanges(MapKeycloakTransaction tx, V origEntity) { - final V res = tx.read(origEntity.getId(), id -> Serialization.from(origEntity)); - return tx.updateIfChanged(res, AbstractEntity::isUpdated); - } -} diff --git a/model/map/src/main/java/org/keycloak/models/map/group/MapGroupEntity.java b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupEntity.java index e164dfb4e2..1e92776230 100644 --- a/model/map/src/main/java/org/keycloak/models/map/group/MapGroupEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupEntity.java @@ -19,6 +19,7 @@ package org.keycloak.models.map.group; import org.keycloak.models.map.common.AbstractEntity; +import org.keycloak.models.map.common.UpdatableEntity; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -30,7 +31,7 @@ import java.util.Set; * * @author mhajas */ -public class MapGroupEntity implements AbstractEntity { +public class MapGroupEntity implements AbstractEntity, UpdatableEntity { private final K id; private final String realmId; diff --git a/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProvider.java b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProvider.java index 105497e219..fd666483a3 100644 --- a/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProvider.java +++ b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProvider.java @@ -39,7 +39,6 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import static org.keycloak.common.util.StackUtil.getShortStackTrace; -import static org.keycloak.models.map.common.MapStorageUtils.registerEntityForChanges; import static org.keycloak.models.map.storage.QueryParameters.Order.ASCENDING; import static org.keycloak.models.map.storage.QueryParameters.withCriteria; @@ -59,7 +58,7 @@ public class MapGroupProvider implements GroupProvider { private Function, GroupModel> entityToAdapterFunc(RealmModel realm) { // Clone entity before returning back, to avoid giving away a reference to the live object to the caller - return origEntity -> new MapGroupAdapter(session, realm, registerEntityForChanges(tx, origEntity)) { + return origEntity -> new MapGroupAdapter(session, realm, origEntity) { @Override public String getId() { return groupStore.getKeyConvertor().keyToString(entity.getId()); diff --git a/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProviderFactory.java b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProviderFactory.java index fa6b873934..9658efe82a 100644 --- a/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProviderFactory.java +++ b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProviderFactory.java @@ -40,7 +40,7 @@ public class MapGroupProviderFactory extends AbstractMapProviderFactoryMartin Kanis */ -public class MapUserLoginFailureEntity implements AbstractEntity { +public class MapUserLoginFailureEntity implements AbstractEntity, UpdatableEntity { private K id; private String realmId; private String userId; diff --git a/model/map/src/main/java/org/keycloak/models/map/loginFailure/MapUserLoginFailureProvider.java b/model/map/src/main/java/org/keycloak/models/map/loginFailure/MapUserLoginFailureProvider.java index 73be5cf3e5..51186797cb 100644 --- a/model/map/src/main/java/org/keycloak/models/map/loginFailure/MapUserLoginFailureProvider.java +++ b/model/map/src/main/java/org/keycloak/models/map/loginFailure/MapUserLoginFailureProvider.java @@ -28,7 +28,6 @@ import org.keycloak.models.map.storage.ModelCriteriaBuilder; import java.util.function.Function; import static org.keycloak.common.util.StackUtil.getShortStackTrace; -import static org.keycloak.models.map.common.MapStorageUtils.registerEntityForChanges; import static org.keycloak.models.map.storage.QueryParameters.withCriteria; /** @@ -51,7 +50,7 @@ public class MapUserLoginFailureProvider implements UserLoginFailureProvider private Function, UserLoginFailureModel> userLoginFailureEntityToAdapterFunc(RealmModel realm) { // Clone entity before returning back, to avoid giving away a reference to the live object to the caller - return origEntity -> new MapUserLoginFailureAdapter(session, realm, registerEntityForChanges(userLoginFailureTx, origEntity)) { + return origEntity -> new MapUserLoginFailureAdapter(session, realm, origEntity) { @Override public String getId() { return userLoginFailureStore.getKeyConvertor().keyToString(entity.getId()); @@ -100,7 +99,7 @@ public class MapUserLoginFailureProvider implements UserLoginFailureProvider LOG.tracef("removeUserLoginFailure(%s, %s)%s", realm, userId, getShortStackTrace()); - userLoginFailureTx.delete(userLoginFailureStore.getKeyConvertor().yieldNewUniqueKey(), withCriteria(mcb)); + userLoginFailureTx.delete(withCriteria(mcb)); } @Override @@ -110,7 +109,7 @@ public class MapUserLoginFailureProvider implements UserLoginFailureProvider LOG.tracef("removeAllUserLoginFailures(%s)%s", realm, getShortStackTrace()); - userLoginFailureTx.delete(userLoginFailureStore.getKeyConvertor().yieldNewUniqueKey(), withCriteria(mcb)); + userLoginFailureTx.delete(withCriteria(mcb)); } @Override diff --git a/model/map/src/main/java/org/keycloak/models/map/loginFailure/MapUserLoginFailureProviderFactory.java b/model/map/src/main/java/org/keycloak/models/map/loginFailure/MapUserLoginFailureProviderFactory.java index e118d4b95d..40096c55fc 100644 --- a/model/map/src/main/java/org/keycloak/models/map/loginFailure/MapUserLoginFailureProviderFactory.java +++ b/model/map/src/main/java/org/keycloak/models/map/loginFailure/MapUserLoginFailureProviderFactory.java @@ -37,7 +37,7 @@ public class MapUserLoginFailureProviderFactory extends AbstractMapProviderFa private Runnable onClose; public MapUserLoginFailureProviderFactory() { - super(MapUserLoginFailureEntity.class, UserLoginFailureModel.class); + super(UserLoginFailureModel.class); } @Override diff --git a/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmEntity.java b/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmEntity.java index 2d3f9315b9..10947667e1 100644 --- a/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmEntity.java @@ -30,6 +30,7 @@ import org.keycloak.common.util.Time; import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.OTPPolicy; import org.keycloak.models.map.common.AbstractEntity; +import org.keycloak.models.map.common.UpdatableEntity; import org.keycloak.models.map.realm.entity.MapAuthenticationExecutionEntity; import org.keycloak.models.map.realm.entity.MapAuthenticationFlowEntity; import org.keycloak.models.map.realm.entity.MapAuthenticatorConfigEntity; @@ -42,7 +43,7 @@ import org.keycloak.models.map.realm.entity.MapRequiredActionProviderEntity; import org.keycloak.models.map.realm.entity.MapRequiredCredentialEntity; import org.keycloak.models.map.realm.entity.MapWebAuthnPolicyEntity; -public class MapRealmEntity implements AbstractEntity { +public class MapRealmEntity implements AbstractEntity, UpdatableEntity { private final K id; private String name; diff --git a/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmProvider.java b/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmProvider.java index 70a6031e89..9566974c95 100644 --- a/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmProvider.java +++ b/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmProvider.java @@ -37,7 +37,6 @@ import org.keycloak.models.map.storage.MapStorage; import org.keycloak.models.map.storage.ModelCriteriaBuilder; import org.keycloak.models.map.storage.ModelCriteriaBuilder.Operator; import org.keycloak.models.utils.KeycloakModelUtils; -import static org.keycloak.models.map.common.MapStorageUtils.registerEntityForChanges; import static org.keycloak.models.map.storage.QueryParameters.Order.ASCENDING; import static org.keycloak.models.map.storage.QueryParameters.withCriteria; @@ -56,7 +55,7 @@ public class MapRealmProvider implements RealmProvider { } private RealmModel entityToAdapter(MapRealmEntity entity) { - return new MapRealmAdapter(session, registerEntityForChanges(tx, entity)) { + return new MapRealmAdapter(session, entity) { @Override public String getId() { return realmStore.getKeyConvertor().keyToString(entity.getId()); @@ -176,7 +175,6 @@ public class MapRealmProvider implements RealmProvider { .compare(SearchableFields.CLIENT_INITIAL_ACCESS, Operator.EXISTS); tx.read(withCriteria(mcb)) - .map(e -> registerEntityForChanges(tx, e)) .forEach(MapRealmEntity::removeExpiredClientInitialAccesses); } diff --git a/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmProviderFactory.java b/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmProviderFactory.java index 525a334713..55b4af7c26 100644 --- a/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmProviderFactory.java +++ b/model/map/src/main/java/org/keycloak/models/map/realm/MapRealmProviderFactory.java @@ -25,7 +25,7 @@ import org.keycloak.models.RealmProviderFactory; public class MapRealmProviderFactory extends AbstractMapProviderFactory, RealmModel> implements RealmProviderFactory { public MapRealmProviderFactory() { - super(MapRealmEntity.class, RealmModel.class); + super(RealmModel.class); } @Override diff --git a/model/map/src/main/java/org/keycloak/models/map/role/MapRoleEntity.java b/model/map/src/main/java/org/keycloak/models/map/role/MapRoleEntity.java index 536771a9e6..1ce49f6529 100644 --- a/model/map/src/main/java/org/keycloak/models/map/role/MapRoleEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/role/MapRoleEntity.java @@ -23,8 +23,9 @@ import java.util.Map; import java.util.Objects; import java.util.Set; import org.keycloak.models.map.common.AbstractEntity; +import org.keycloak.models.map.common.UpdatableEntity; -public class MapRoleEntity implements AbstractEntity { +public class MapRoleEntity implements AbstractEntity, UpdatableEntity { private K id; private String realmId; diff --git a/model/map/src/main/java/org/keycloak/models/map/role/MapRoleProvider.java b/model/map/src/main/java/org/keycloak/models/map/role/MapRoleProvider.java index ce5d9e8c3c..8b8930795e 100644 --- a/model/map/src/main/java/org/keycloak/models/map/role/MapRoleProvider.java +++ b/model/map/src/main/java/org/keycloak/models/map/role/MapRoleProvider.java @@ -31,7 +31,6 @@ import java.util.function.Function; import java.util.stream.Stream; import org.keycloak.models.map.storage.MapStorage; import static org.keycloak.common.util.StackUtil.getShortStackTrace; -import static org.keycloak.models.map.common.MapStorageUtils.registerEntityForChanges; import static org.keycloak.models.map.storage.QueryParameters.Order.ASCENDING; import static org.keycloak.models.map.storage.QueryParameters.withCriteria; @@ -57,7 +56,7 @@ public class MapRoleProvider implements RoleProvider { private Function, RoleModel> entityToAdapterFunc(RealmModel realm) { // Clone entity before returning back, to avoid giving away a reference to the live object to the caller - return origEntity -> new MapRoleAdapter(session, realm, registerEntityForChanges(tx, origEntity)) { + return origEntity -> new MapRoleAdapter(session, realm, origEntity) { @Override public String getId() { return roleStore.getKeyConvertor().keyToString(entity.getId()); diff --git a/model/map/src/main/java/org/keycloak/models/map/role/MapRoleProviderFactory.java b/model/map/src/main/java/org/keycloak/models/map/role/MapRoleProviderFactory.java index 988ff305b9..b0606aa9f4 100644 --- a/model/map/src/main/java/org/keycloak/models/map/role/MapRoleProviderFactory.java +++ b/model/map/src/main/java/org/keycloak/models/map/role/MapRoleProviderFactory.java @@ -25,7 +25,7 @@ import org.keycloak.models.RoleProviderFactory; public class MapRoleProviderFactory extends AbstractMapProviderFactory, RoleModel> implements RoleProviderFactory { public MapRoleProviderFactory() { - super(MapRoleEntity.class, RoleModel.class); + super(RoleModel.class); } @Override 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 98a201f22d..b85768891a 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 @@ -27,6 +27,9 @@ public interface MapKeycloakTransaction, M> exten /** * Instructs this transaction to add a new value into the underlying store on commit. + *

+ * Updates to the returned instances of {@code V} would be visible in the current transaction + * and will propagate into the underlying store upon commit. * * @param value the value * @return Entity representing the {@code value} in the store. It may or may not be the same instance as {@code value} @@ -35,29 +38,22 @@ public interface MapKeycloakTransaction, M> exten /** * Provides possibility to lookup for values by a {@code key} in the underlying store with respect to changes done - * in current transaction. + * in current transaction. Updates to the returned instance would be visible in the current transaction + * and will propagate into the underlying store upon commit. * * @param key identifier of a value * @return a value associated with the given {@code key} */ V read(K key); - /** - * Looks up a value in the current transaction with corresponding key, returns {@code defaultValueFunc} when - * the transaction does not contain a value for the {@code key} identifier. - * - * @param key identifier of a value - * @param defaultValueFunc fallback function if the transaction does not contain a value that corresponds to {@code key} - * @return a value associated with the given {@code key}, or the result of {@code defaultValueFunc} - * - */ - V read(K key, Function defaultValueFunc); - /** * Returns a stream of values from underlying storage that are updated based on the current transaction changes; * i.e. the result contains updates and excludes of records that have been created, updated or deleted in this * transaction by methods {@link MapKeycloakTransaction#create}, {@link MapKeycloakTransaction#update}, * {@link MapKeycloakTransaction#delete}, etc. + *

+ * Updates to the returned instances of {@code V} would be visible in the current transaction + * and will propagate into the underlying store upon commit. * * @param queryParameters parameters for the query like firstResult, maxResult, requested ordering, etc. * @return values that fulfill the given criteria, that are updated based on changes in the current transaction @@ -73,38 +69,6 @@ public interface MapKeycloakTransaction, M> exten */ long getCount(QueryParameters queryParameters); - /** - * Instructs this transaction to force-update the {@code value} associated with the identifier {@code value.getId()} in the - * underlying store on commit. - * - * @param value updated version of the value - * @return Entity representing the {@code value} in the store. It may or may not be the same instance as {@code value} - */ - V update(V value); - - /** - * Returns an updated version of the {@code orig} object as updated in this transaction. - * - * If the underlying store handles transactions on its own, this can return {@code orig} directly. - * - * @param orig possibly stale version of some object from the underlying store - * @return the {@code orig} object as visible from this transaction, or {@code null} if the object has been removed. - */ - default V getUpdated(V orig) { - return orig; - } - - /** - * Instructs this transaction to update the {@code value} associated with the identifier {@code value.getId()} in the - * underlying store on commit, if by the time of {@code commit} the {@code shouldPut} predicate returns {@code true} - * - * @param value new version of the value. Must not alter the {@code id} of the entity - * @param shouldPut predicate to check in commit phase - * @return Entity representing the {@code value} in the store. It may or may not be the same instance as {@code value} - * @see AbstractEntity#getId() - */ - V updateIfChanged(V value, Predicate shouldPut); - /** * Instructs this transaction to delete a value associated with the identifier {@code key} from the underlying store * on commit. @@ -122,6 +86,6 @@ public interface MapKeycloakTransaction, M> exten * @param queryParameters parameters for the query like firstResult, maxResult, requested ordering, etc. * @return number of removed objects (might return {@code -1} if not supported) */ - long delete(K artificialKey, QueryParameters queryParameters); + long delete(QueryParameters queryParameters); } 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 fb2b2d3dfe..646ceb00c2 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 @@ -20,8 +20,6 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.map.common.AbstractEntity; import java.util.stream.Stream; -import static org.keycloak.models.map.storage.QueryParameters.withCriteria; - /** * Implementation of this interface interacts with a persistence storage storing various entities, e.g. users, realms. * It contains basic object CRUD operations as well as bulk {@link #read(org.keycloak.models.map.storage.QueryParameters)} @@ -41,10 +39,10 @@ import static org.keycloak.models.map.storage.QueryParameters.withCriteria; public interface MapStorage, M> { /** - * Creates an object in the store identified. The ID of the {@code value} should be non-{@code null}. - * If the ID is {@code null}, then the {@code value}'s ID will be returned + * Creates an object in the store. ID of the {@code value} may be prescribed in id of the {@code value}. + * If the id is {@code null}, then the {@code value}'s ID will be generated and returned in the id of the return value. * @param value Entity to create in the store - * @throws NullPointerException if object or its {@code key} is {@code null} + * @throws NullPointerException if {@code value} is {@code null} * @see AbstractEntity#getId() * @return Entity representing the {@code value} in the store. It may or may not be the same instance as {@code value} */ diff --git a/model/map/src/main/java/org/keycloak/models/map/storage/MapStorageProvider.java b/model/map/src/main/java/org/keycloak/models/map/storage/MapStorageProvider.java index cb5f4b92c5..4683bd5c05 100644 --- a/model/map/src/main/java/org/keycloak/models/map/storage/MapStorageProvider.java +++ b/model/map/src/main/java/org/keycloak/models/map/storage/MapStorageProvider.java @@ -27,13 +27,14 @@ import org.keycloak.provider.Provider; public interface MapStorageProvider extends Provider { /** - * Returns a key-value storage implementation for the particular types. + * Returns a key-value storage implementation for the given types. * @param type of the primary key * @param type of the value - * @param name Name of the storage - * @param flags + * @param type of the corresponding model (e.g. {@code UserModel}) + * @param modelType Model type + * @param flags Flags of the returned storage. Best effort, flags may be not honored by underlying implementation * @return * @throws IllegalArgumentException If some of the types is not supported by the underlying implementation. */ - , M> MapStorage getStorage(Class valueType, Class modelType, Flag... flags); + , M> MapStorage getStorage(Class modelType, Flag... flags); } diff --git a/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapKeycloakTransaction.java b/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapKeycloakTransaction.java index 6813a94e78..abf930d718 100644 --- a/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapKeycloakTransaction.java +++ b/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapKeycloakTransaction.java @@ -17,6 +17,8 @@ package org.keycloak.models.map.storage.chm; import org.keycloak.models.map.common.AbstractEntity; +import org.keycloak.models.map.common.Serialization; +import org.keycloak.models.map.common.UpdatableEntity; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.Map; @@ -33,7 +35,7 @@ import org.keycloak.models.map.storage.ModelCriteriaBuilder; import org.keycloak.models.map.storage.QueryParameters; import org.keycloak.utils.StreamsUtil; -public class ConcurrentHashMapKeycloakTransaction, M> implements MapKeycloakTransaction { +public class ConcurrentHashMapKeycloakTransaction & UpdatableEntity, M> implements MapKeycloakTransaction { private final static Logger log = Logger.getLogger(ConcurrentHashMapKeycloakTransaction.class); @@ -98,16 +100,37 @@ public class ConcurrentHashMapKeycloakTransaction tasks.merge(taskKey, task, MapTaskCompose::new); } + /** + * Returns a deep clone of an entity. If the clone is already in the transaction, returns this one. + *

+ * Usually used before giving an entity from a source back to the caller, + * to prevent changing it directly in the data store, but to keep transactional properties. + * @param origEntity Original entity + * @return + */ + public V registerEntityForChanges(V origEntity) { + final K key = origEntity.getId(); + // If the entity is listed in the transaction already, return it directly + if (tasks.containsKey(key)) { + MapTaskWithValue current = tasks.get(key); + return current.getValue(); + } + // Else enlist its copy in the transaction. Never return direct reference to the underlying map + final V res = Serialization.from(origEntity); + return updateIfChanged(res, e -> e.isUpdated()); + } + @Override public V read(K key) { - try { // TODO: Consider using Optional rather than handling NPE - return read(key, map::read); + try { + // TODO: Consider using Optional rather than handling NPE + final V entity = read(key, map::read); + return registerEntityForChanges(entity); } catch (NullPointerException ex) { return null; } } - @Override public V read(K key, Function defaultValueFunc) { MapTaskWithValue current = tasks.get(key); // If the key exists, then it has entered the "tasks" after bulk delete that could have @@ -152,7 +175,8 @@ public class ConcurrentHashMapKeycloakTransaction Stream updatedAndNotRemovedObjectsStream = this.map.read(queryParameters) .filter(filterOutAllBulkDeletedObjects) .map(this::getUpdated) // If the object has been removed, tx.get will return null, otherwise it will return me.getValue() - .filter(Objects::nonNull); + .filter(Objects::nonNull) + .map(this::registerEntityForChanges); // In case of created values stored in MapKeycloakTransaction, we need filter those according to the filter MapModelCriteriaBuilder mapMcb = mcb.unwrap(MapModelCriteriaBuilder.class); @@ -176,19 +200,11 @@ public class ConcurrentHashMapKeycloakTransaction return read(queryParameters).count(); } - @Override - public V getUpdated(V orig) { + private V getUpdated(V orig) { MapTaskWithValue current = orig == null ? null : tasks.get(orig.getId()); return current == null ? orig : current.getValue(); } - @Override - public V update(V value) { - K key = value.getId(); - addTask(key, new UpdateOperation(value)); - return value; - } - @Override public V create(V value) { K key = value.getId(); @@ -196,7 +212,6 @@ public class ConcurrentHashMapKeycloakTransaction return value; } - @Override public V updateIfChanged(V value, Predicate shouldPut) { K key = value.getId(); log.tracef("Adding operation UPDATE_IF_CHANGED for %s @ %08x", key, System.identityHashCode(value)); @@ -222,9 +237,11 @@ public class ConcurrentHashMapKeycloakTransaction @Override - public long delete(K artificialKey, QueryParameters queryParameters) { + public long delete(QueryParameters queryParameters) { log.tracef("Adding operation DELETE_BULK"); + K artificialKey = map.getKeyConvertor().yieldNewUniqueKey(); + // Remove all tasks that create / update / delete objects deleted by the bulk removal. final BulkDeleteOperation bdo = new BulkDeleteOperation(queryParameters); Predicate filterForNonDeletedObjects = bdo.getFilterForNonDeletedObjects(); @@ -344,15 +361,6 @@ public class ConcurrentHashMapKeycloakTransaction @Override public MapOperation getOperation() { return MapOperation.CREATE; } } - private class UpdateOperation extends MapTaskWithValue { - public UpdateOperation(V value) { - super(value); - } - - @Override public void execute() { map.update(getValue()); } - @Override public MapOperation getOperation() { return MapOperation.UPDATE; } - } - private class DeleteOperation extends MapTaskWithValue { private final K key; diff --git a/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapStorage.java b/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapStorage.java index 9856cedf19..2b07e39947 100644 --- a/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapStorage.java +++ b/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapStorage.java @@ -19,6 +19,7 @@ package org.keycloak.models.map.storage.chm; import org.keycloak.models.KeycloakSession; import org.keycloak.models.map.storage.MapKeycloakTransaction; import org.keycloak.models.map.common.AbstractEntity; +import org.keycloak.models.map.common.UpdatableEntity; import org.keycloak.models.map.storage.MapStorage; import org.keycloak.models.map.storage.ModelCriteriaBuilder; import org.keycloak.models.map.storage.QueryParameters; @@ -42,7 +43,7 @@ import static org.keycloak.utils.StreamsUtil.paginatedStream; * * @author hmlnarik */ -public class ConcurrentHashMapStorage, M> implements MapStorage { +public class ConcurrentHashMapStorage & UpdatableEntity, M> implements MapStorage { private final ConcurrentMap store = new ConcurrentHashMap<>(); diff --git a/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapStorageProvider.java b/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapStorageProvider.java index 2fdd7f79e0..18cc6ba390 100644 --- a/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapStorageProvider.java +++ b/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapStorageProvider.java @@ -17,6 +17,7 @@ package org.keycloak.models.map.storage.chm; import org.keycloak.models.map.common.AbstractEntity; +import org.keycloak.models.map.storage.MapStorage; import org.keycloak.models.map.storage.MapStorageProvider; import org.keycloak.models.map.storage.MapStorageProviderFactory.Flag; @@ -37,8 +38,9 @@ public class ConcurrentHashMapStorageProvider implements MapStorageProvider { } @Override - public , M> ConcurrentHashMapStorage getStorage( - Class valueType, Class modelType, Flag... flags) { - return factory.getStorage(valueType, modelType, flags); + @SuppressWarnings("unchecked") + public , M> MapStorage getStorage(Class modelType, Flag... flags) { + ConcurrentHashMapStorage storage = factory.getStorage(modelType, flags); + return (MapStorage) storage; } } diff --git a/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapStorageProviderFactory.java b/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapStorageProviderFactory.java index 4a5473ed2c..4f716a8950 100644 --- a/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapStorageProviderFactory.java +++ b/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapStorageProviderFactory.java @@ -46,6 +46,7 @@ import org.keycloak.models.map.client.MapClientEntityImpl; import org.keycloak.models.map.clientscope.MapClientScopeEntity; import org.keycloak.models.map.common.AbstractEntity; import org.keycloak.models.map.common.Serialization; +import org.keycloak.models.map.common.UpdatableEntity; import org.keycloak.models.map.group.MapGroupEntity; import org.keycloak.models.map.loginFailure.MapUserLoginFailureEntity; import org.keycloak.models.map.realm.MapRealmEntity; @@ -114,7 +115,28 @@ public class ConcurrentHashMapStorageProviderFactory implements AmphibianProvide MODEL_TO_NAME.put(Resource.class, "authz-resources"); MODEL_TO_NAME.put(org.keycloak.authorization.model.Scope.class, "authz-scopes"); } + + public static final Map, Class> MODEL_TO_VALUE_TYPE = new HashMap<>(); + static { + MODEL_TO_VALUE_TYPE.put(AuthenticatedClientSessionModel.class, MapAuthenticatedClientSessionEntity.class); + MODEL_TO_VALUE_TYPE.put(ClientScopeModel.class, MapClientScopeEntity.class); + MODEL_TO_VALUE_TYPE.put(ClientModel.class, MapClientEntity.class); + MODEL_TO_VALUE_TYPE.put(GroupModel.class, MapGroupEntity.class); + MODEL_TO_VALUE_TYPE.put(RealmModel.class, MapRealmEntity.class); + MODEL_TO_VALUE_TYPE.put(RoleModel.class, MapRoleEntity.class); + MODEL_TO_VALUE_TYPE.put(RootAuthenticationSessionModel.class, MapRootAuthenticationSessionEntity.class); + MODEL_TO_VALUE_TYPE.put(UserLoginFailureModel.class, MapUserLoginFailureEntity.class); + MODEL_TO_VALUE_TYPE.put(UserModel.class, MapUserEntity.class); + MODEL_TO_VALUE_TYPE.put(UserSessionModel.class, MapUserSessionEntity.class); + // authz + MODEL_TO_VALUE_TYPE.put(PermissionTicket.class, MapPermissionTicketEntity.class); + MODEL_TO_VALUE_TYPE.put(Policy.class, MapPolicyEntity.class); + MODEL_TO_VALUE_TYPE.put(ResourceServer.class, MapResourceServerEntity.class); + MODEL_TO_VALUE_TYPE.put(Resource.class, MapResourceEntity.class); + MODEL_TO_VALUE_TYPE.put(org.keycloak.authorization.model.Scope.class, MapScopeEntity.class); + } + public static final Map, Class> INTERFACE_TO_IMPL = new HashMap<>(); static { INTERFACE_TO_IMPL.put(MapClientEntity.class, MapClientEntityImpl.class); @@ -219,16 +241,16 @@ public class ConcurrentHashMapStorageProviderFactory implements AmphibianProvide } } - private , M> ConcurrentHashMapStorage loadMap(String mapName, - Class valueType, Class modelType, EnumSet flags) { + private & UpdatableEntity, M> ConcurrentHashMapStorage loadMap(String mapName, + Class modelType, EnumSet flags) { final StringKeyConvertor kc = keyConvertors.getOrDefault(mapName, defaultKeyConvertor); - + Class valueType = MODEL_TO_VALUE_TYPE.get(modelType); LOG.debugf("Initializing new map storage: %s", mapName); @SuppressWarnings("unchecked") ConcurrentHashMapStorage store; if (modelType == UserSessionModel.class) { - ConcurrentHashMapStorage clientSessionStore = getStorage(MapAuthenticatedClientSessionEntity.class, AuthenticatedClientSessionModel.class); + ConcurrentHashMapStorage clientSessionStore = getStorage(AuthenticatedClientSessionModel.class); store = new UserSessionConcurrentHashMapStorage(clientSessionStore, kc) { @Override public String toString() { @@ -269,8 +291,8 @@ public class ConcurrentHashMapStorageProviderFactory implements AmphibianProvide } @SuppressWarnings("unchecked") - public , M> ConcurrentHashMapStorage getStorage( - Class valueType, Class modelType, Flag... flags) { + public & UpdatableEntity, M> ConcurrentHashMapStorage getStorage( + Class modelType, Flag... flags) { EnumSet f = flags == null || flags.length == 0 ? EnumSet.noneOf(Flag.class) : EnumSet.of(flags[0], flags); String name = MODEL_TO_NAME.getOrDefault(modelType, modelType.getSimpleName()); /* From ConcurrentHashMapStorage.computeIfAbsent javadoc: @@ -282,9 +304,9 @@ public class ConcurrentHashMapStorageProviderFactory implements AmphibianProvide * to prepare clientSessionStore outside computeIfAbsent, otherwise deadlock occurs. */ if (modelType == UserSessionModel.class) { - getStorage(MapAuthenticatedClientSessionEntity.class, AuthenticatedClientSessionModel.class); + getStorage(AuthenticatedClientSessionModel.class, flags); } - return (ConcurrentHashMapStorage) storages.computeIfAbsent(name, n -> loadMap(name, valueType, modelType, f)); + return (ConcurrentHashMapStorage) storages.computeIfAbsent(name, n -> loadMap(name, modelType, f)); } private File getFile(String fileName) { diff --git a/model/map/src/main/java/org/keycloak/models/map/storage/chm/MapFieldPredicates.java b/model/map/src/main/java/org/keycloak/models/map/storage/chm/MapFieldPredicates.java index d6c48d8b15..0966d4ada0 100644 --- a/model/map/src/main/java/org/keycloak/models/map/storage/chm/MapFieldPredicates.java +++ b/model/map/src/main/java/org/keycloak/models/map/storage/chm/MapFieldPredicates.java @@ -66,6 +66,7 @@ import java.util.function.Predicate; import java.util.stream.Stream; import org.keycloak.models.map.storage.CriterionNotSupportedException; +import java.util.IdentityHashMap; import static org.keycloak.models.UserSessionModel.CORRESPONDING_SESSION_ID; /** @@ -92,7 +93,7 @@ public class MapFieldPredicates { @SuppressWarnings("unchecked") private static final Map, Map> PREDICATES = new HashMap<>(); - private static final Map, Comparator> COMPARATORS = new HashMap<>(); + private static final Map, Comparator> COMPARATORS = new IdentityHashMap<>(); static { put(REALM_PREDICATES, RealmModel.SearchableFields.NAME, MapRealmEntity::getName); diff --git a/model/map/src/main/java/org/keycloak/models/map/storage/chm/UserSessionConcurrentHashMapStorage.java b/model/map/src/main/java/org/keycloak/models/map/storage/chm/UserSessionConcurrentHashMapStorage.java index 99eeb98731..858cdcbfb4 100644 --- a/model/map/src/main/java/org/keycloak/models/map/storage/chm/UserSessionConcurrentHashMapStorage.java +++ b/model/map/src/main/java/org/keycloak/models/map/storage/chm/UserSessionConcurrentHashMapStorage.java @@ -52,19 +52,19 @@ public class UserSessionConcurrentHashMapStorage extends ConcurrentHashMapSto } @Override - public long delete(K artificialKey, QueryParameters queryParameters) { + public long delete(QueryParameters queryParameters) { ModelCriteriaBuilder mcb = queryParameters.getModelCriteriaBuilder(); Set ids = read(queryParameters).map(AbstractEntity::getId).collect(Collectors.toSet()); ModelCriteriaBuilder csMcb = clientSessionStore.createCriteriaBuilder().compare(AuthenticatedClientSessionModel.SearchableFields.USER_SESSION_ID, Operator.IN, ids); - clientSessionTr.delete(artificialKey, withCriteria(csMcb)); - return super.delete(artificialKey, queryParameters); + clientSessionTr.delete(withCriteria(csMcb)); + return super.delete(queryParameters); } @Override public boolean delete(K key) { ModelCriteriaBuilder csMcb = clientSessionStore.createCriteriaBuilder().compare(AuthenticatedClientSessionModel.SearchableFields.USER_SESSION_ID, Operator.EQ, key); - clientSessionTr.delete(key, withCriteria(csMcb)); + clientSessionTr.delete(withCriteria(csMcb)); return super.delete(key); } diff --git a/model/map/src/main/java/org/keycloak/models/map/user/MapUserEntity.java b/model/map/src/main/java/org/keycloak/models/map/user/MapUserEntity.java index 1c39d216c1..c418969826 100644 --- a/model/map/src/main/java/org/keycloak/models/map/user/MapUserEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/user/MapUserEntity.java @@ -19,6 +19,7 @@ package org.keycloak.models.map.user; import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.map.common.AbstractEntity; +import org.keycloak.models.map.common.UpdatableEntity; import org.keycloak.models.utils.KeycloakModelUtils; import java.util.Collection; @@ -39,7 +40,7 @@ import java.util.stream.Stream; * * @author mhajas */ -public class MapUserEntity implements AbstractEntity { +public class MapUserEntity implements AbstractEntity, UpdatableEntity { private final K id; private final String realmId; diff --git a/model/map/src/main/java/org/keycloak/models/map/user/MapUserProvider.java b/model/map/src/main/java/org/keycloak/models/map/user/MapUserProvider.java index b45f00facc..94e88b06a4 100644 --- a/model/map/src/main/java/org/keycloak/models/map/user/MapUserProvider.java +++ b/model/map/src/main/java/org/keycloak/models/map/user/MapUserProvider.java @@ -69,7 +69,6 @@ import static org.keycloak.models.UserModel.EMAIL_VERIFIED; import static org.keycloak.models.UserModel.FIRST_NAME; import static org.keycloak.models.UserModel.LAST_NAME; import static org.keycloak.models.UserModel.USERNAME; -import static org.keycloak.models.map.common.MapStorageUtils.registerEntityForChanges; import static org.keycloak.models.map.storage.QueryParameters.Order.ASCENDING; import static org.keycloak.models.map.storage.QueryParameters.withCriteria; @@ -89,7 +88,7 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS private Function, UserModel> entityToAdapterFunc(RealmModel realm) { // Clone entity before returning back, to avoid giving away a reference to the live object to the caller - return origEntity -> new MapUserAdapter(session, realm, registerEntityForChanges(tx, origEntity)) { + return origEntity -> new MapUserAdapter(session, realm, origEntity) { @Override public String getId() { return userStore.getKeyConvertor().keyToString(entity.getId()); @@ -127,9 +126,8 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS } } - private MapUserEntity getRegisteredEntityByIdOrThrow(RealmModel realm, String id) { + private MapUserEntity getEntityByIdOrThrow(RealmModel realm, String id) { return getEntityById(realm, id) - .map(e -> registerEntityForChanges(tx, e)) .orElseThrow(this::userDoesntExistException); } @@ -142,10 +140,6 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS return Optional.empty(); } - private Optional> getRegisteredEntityById(RealmModel realm, String id) { - return getEntityById(realm, id).map(e -> registerEntityForChanges(tx, e)); - } - @Override public void addFederatedIdentity(RealmModel realm, UserModel user, FederatedIdentityModel socialLink) { if (user == null || user.getId() == null) { @@ -153,7 +147,7 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS } LOG.tracef("addFederatedIdentity(%s, %s, %s)%s", realm, user.getId(), socialLink.getIdentityProvider(), getShortStackTrace()); - getRegisteredEntityById(realm, user.getId()) + getEntityById(realm, user.getId()) .ifPresent(userEntity -> userEntity.addFederatedIdentity(UserFederatedIdentityEntity.fromModel(socialLink))); } @@ -161,7 +155,7 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS @Override public boolean removeFederatedIdentity(RealmModel realm, UserModel user, String socialProvider) { LOG.tracef("removeFederatedIdentity(%s, %s, %s)%s", realm, user.getId(), socialProvider, getShortStackTrace()); - return getRegisteredEntityById(realm, user.getId()) + return getEntityById(realm, user.getId()) .map(entity -> entity.removeFederatedIdentity(socialProvider)) .orElse(false); } @@ -175,14 +169,13 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS .compare(SearchableFields.IDP_AND_USER, Operator.EQ, socialProvider); tx.read(withCriteria(mcb)) - .map(e -> registerEntityForChanges(tx, e)) .forEach(userEntity -> userEntity.removeFederatedIdentity(socialProvider)); } @Override public void updateFederatedIdentity(RealmModel realm, UserModel federatedUser, FederatedIdentityModel federatedIdentityModel) { LOG.tracef("updateFederatedIdentity(%s, %s, %s)%s", realm, federatedUser.getId(), federatedIdentityModel.getIdentityProvider(), getShortStackTrace()); - getRegisteredEntityById(realm, federatedUser.getId()) + getEntityById(realm, federatedUser.getId()) .ifPresent(entity -> entity.updateFederatedIdentity(UserFederatedIdentityEntity.fromModel(federatedIdentityModel))); } @@ -229,7 +222,7 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS public void addConsent(RealmModel realm, String userId, UserConsentModel consent) { LOG.tracef("addConsent(%s, %s, %s)%s", realm, userId, consent, getShortStackTrace()); - getRegisteredEntityByIdOrThrow(realm, userId) + getEntityByIdOrThrow(realm, userId) .addUserConsent(UserConsentEntity.fromModel(consent)); } @@ -255,7 +248,7 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS public void updateConsent(RealmModel realm, String userId, UserConsentModel consent) { LOG.tracef("updateConsent(%s, %s, %s)%s", realm, userId, consent, getShortStackTrace()); - MapUserEntity user = getRegisteredEntityByIdOrThrow(realm, userId); + MapUserEntity user = getEntityByIdOrThrow(realm, userId); UserConsentEntity userConsentEntity = user.getUserConsent(consent.getClient().getId()); if (userConsentEntity == null) { throw new ModelException("Consent not found for client [" + consent.getClient().getId() + "] and user [" + userId + "]"); @@ -273,7 +266,7 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS @Override public boolean revokeConsentForClient(RealmModel realm, String userId, String clientInternalId) { LOG.tracef("revokeConsentForClient(%s, %s, %s)%s", realm, userId, clientInternalId, getShortStackTrace()); - return getRegisteredEntityById(realm, userId) + return getEntityById(realm, userId) .map(userEntity -> userEntity.removeUserConsent(clientInternalId)) .orElse(false); } @@ -281,7 +274,7 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS @Override public void setNotBeforeForUser(RealmModel realm, UserModel user, int notBefore) { LOG.tracef("setNotBeforeForUser(%s, %s, %d)%s", realm, user.getId(), notBefore, getShortStackTrace()); - getRegisteredEntityByIdOrThrow(realm, user.getId()).setNotBefore(notBefore); + getEntityByIdOrThrow(realm, user.getId()).setNotBefore(notBefore); } @Override @@ -363,7 +356,7 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS ModelCriteriaBuilder mcb = userStore.createCriteriaBuilder() .compare(SearchableFields.REALM_ID, Operator.EQ, realm.getId()); - tx.delete(userStore.getKeyConvertor().yieldNewUniqueKey(), withCriteria(mcb)); + tx.delete(withCriteria(mcb)); } @Override @@ -373,7 +366,7 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS .compare(SearchableFields.REALM_ID, Operator.EQ, realm.getId()) .compare(SearchableFields.FEDERATION_LINK, Operator.EQ, storageProviderId); - tx.delete(userStore.getKeyConvertor().yieldNewUniqueKey(), withCriteria(mcb)); + tx.delete(withCriteria(mcb)); } @Override @@ -384,8 +377,7 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS .compare(SearchableFields.FEDERATION_LINK, Operator.EQ, storageProviderId); try (Stream> s = tx.read(withCriteria(mcb))) { - s.map(e -> registerEntityForChanges(tx, e)) - .forEach(userEntity -> userEntity.setFederationLink(null)); + s.forEach(userEntity -> userEntity.setFederationLink(null)); } } @@ -398,8 +390,7 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS .compare(SearchableFields.ASSIGNED_ROLE, Operator.EQ, roleId); try (Stream> s = tx.read(withCriteria(mcb))) { - s.map(e -> registerEntityForChanges(tx, e)) - .forEach(userEntity -> userEntity.removeRolesMembership(roleId)); + s.forEach(userEntity -> userEntity.removeRolesMembership(roleId)); } } @@ -412,8 +403,7 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS .compare(SearchableFields.ASSIGNED_GROUP, Operator.EQ, groupId); try (Stream> s = tx.read(withCriteria(mcb))) { - s.map(e -> registerEntityForChanges(tx, e)) - .forEach(userEntity -> userEntity.removeGroupsMembership(groupId)); + s.forEach(userEntity -> userEntity.removeGroupsMembership(groupId)); } } @@ -426,8 +416,7 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS .compare(SearchableFields.CONSENT_FOR_CLIENT, Operator.EQ, clientId); try (Stream> s = tx.read(withCriteria(mcb))) { - s.map(e -> registerEntityForChanges(tx, e)) - .forEach(userEntity -> userEntity.removeUserConsent(clientId)); + s.forEach(userEntity -> userEntity.removeUserConsent(clientId)); } } @@ -478,7 +467,6 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS .collect(Collectors.toList()); if (! consentClientIds.isEmpty()) { - userEntity = registerEntityForChanges(tx, userEntity); consentClientIds.forEach(userEntity::removeUserConsent); } }; @@ -492,8 +480,7 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS .compare(SearchableFields.REALM_ID, Operator.EQ, realm.getId()); try (Stream> s = tx.read(withCriteria(mcb))) { - s.map(e -> registerEntityForChanges(tx, e)) - .forEach(entity -> entity.addRolesMembership(roleId)); + s.forEach(entity -> entity.addRolesMembership(roleId)); } } @@ -535,7 +522,7 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS throw new ModelDuplicateException("Multiple users with email '" + email + "' exist in Keycloak."); } - MapUserEntity userEntity = registerEntityForChanges(tx, usersWithEmail.get(0)); + MapUserEntity userEntity = usersWithEmail.get(0); if (!realm.isDuplicateEmailsAllowed()) { if (userEntity.getEmail() != null && !userEntity.getEmail().equals(userEntity.getEmailConstraint())) { @@ -545,21 +532,7 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS } } - return new MapUserAdapter(session, realm, userEntity) { - @Override - public String getId() { - return userStore.getKeyConvertor().keyToString(userEntity.getId()); - } - - @Override - public boolean checkEmailUniqueness(RealmModel realm, String email) { - return getUserByEmail(realm, email) != null; - } - @Override - public boolean checkUsernameUniqueness(RealmModel realm, String username) { - return getUserByUsername(realm, username) != null; - } - }; + return entityToAdapterFunc(realm).apply(userEntity); } @Override @@ -758,7 +731,7 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS @Override public void updateCredential(RealmModel realm, UserModel user, CredentialModel cred) { - getRegisteredEntityById(realm, user.getId()) + getEntityById(realm, user.getId()) .ifPresent(updateCredential(cred)); } @@ -780,7 +753,7 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS LOG.tracef("createCredential(%s, %s, %s)%s", realm, user.getId(), cred.getId(), getShortStackTrace()); UserCredentialEntity credentialEntity = UserCredentialEntity.fromModel(cred); - getRegisteredEntityByIdOrThrow(realm, user.getId()) + getEntityByIdOrThrow(realm, user.getId()) .addCredential(credentialEntity); return UserCredentialEntity.toModel(credentialEntity); @@ -789,7 +762,7 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS @Override public boolean removeStoredCredential(RealmModel realm, UserModel user, String id) { LOG.tracef("removeStoredCredential(%s, %s, %s)%s", realm, user.getId(), id, getShortStackTrace()); - return getRegisteredEntityById(realm, user.getId()) + return getEntityById(realm, user.getId()) .map(mapUserEntity -> mapUserEntity.removeCredential(id)) .orElse(false); } @@ -831,7 +804,7 @@ public class MapUserProvider implements UserProvider.Streams, UserCredentialS public boolean moveCredentialTo(RealmModel realm, UserModel user, String id, String newPreviousCredentialId) { LOG.tracef("moveCredentialTo(%s, %s, %s, %s)%s", realm, user.getId(), id, newPreviousCredentialId, getShortStackTrace()); String userId = user.getId(); - MapUserEntity userEntity = getRegisteredEntityById(realm, userId).orElse(null); + MapUserEntity userEntity = getEntityById(realm, userId).orElse(null); if (userEntity == null) { LOG.warnf("User with id: [%s] not found", userId); return false; diff --git a/model/map/src/main/java/org/keycloak/models/map/user/MapUserProviderFactory.java b/model/map/src/main/java/org/keycloak/models/map/user/MapUserProviderFactory.java index 155ad06950..67b243e304 100644 --- a/model/map/src/main/java/org/keycloak/models/map/user/MapUserProviderFactory.java +++ b/model/map/src/main/java/org/keycloak/models/map/user/MapUserProviderFactory.java @@ -30,7 +30,7 @@ import org.keycloak.models.map.common.AbstractMapProviderFactory; public class MapUserProviderFactory extends AbstractMapProviderFactory, UserModel> implements UserProviderFactory { public MapUserProviderFactory() { - super(MapUserEntity.class, UserModel.class); + super(UserModel.class); } @Override diff --git a/model/map/src/main/java/org/keycloak/models/map/userSession/MapAuthenticatedClientSessionEntity.java b/model/map/src/main/java/org/keycloak/models/map/userSession/MapAuthenticatedClientSessionEntity.java index 41ea5a2d9f..cf20401056 100644 --- a/model/map/src/main/java/org/keycloak/models/map/userSession/MapAuthenticatedClientSessionEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/userSession/MapAuthenticatedClientSessionEntity.java @@ -19,6 +19,7 @@ package org.keycloak.models.map.userSession; import org.keycloak.common.util.Time; import org.keycloak.models.map.common.AbstractEntity; +import org.keycloak.models.map.common.UpdatableEntity; import java.util.Map; import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; @@ -26,7 +27,7 @@ import java.util.concurrent.ConcurrentHashMap; /** * @author Martin Kanis */ -public class MapAuthenticatedClientSessionEntity implements AbstractEntity { +public class MapAuthenticatedClientSessionEntity implements AbstractEntity, UpdatableEntity { private K id; private String userSessionId; diff --git a/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionEntity.java b/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionEntity.java index 0e58d2d45a..854e2cffe7 100644 --- a/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionEntity.java @@ -22,6 +22,7 @@ import org.keycloak.models.UserModel; import org.keycloak.models.UserSessionModel; import org.keycloak.models.map.common.AbstractEntity; +import org.keycloak.models.map.common.UpdatableEntity; import java.util.Map; import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; @@ -29,7 +30,7 @@ import java.util.concurrent.ConcurrentHashMap; /** * @author Martin Kanis */ -public class MapUserSessionEntity implements AbstractEntity { +public class MapUserSessionEntity implements AbstractEntity, UpdatableEntity { private K id; private String realmId; diff --git a/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionProvider.java b/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionProvider.java index f798de0e82..258fa9a22a 100644 --- a/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionProvider.java +++ b/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionProvider.java @@ -46,7 +46,6 @@ import java.util.stream.Stream; import static org.keycloak.common.util.StackUtil.getShortStackTrace; import static org.keycloak.models.UserSessionModel.CORRESPONDING_SESSION_ID; import static org.keycloak.models.UserSessionModel.SessionPersistenceState.TRANSIENT; -import static org.keycloak.models.map.common.MapStorageUtils.registerEntityForChanges; import static org.keycloak.models.map.storage.QueryParameters.withCriteria; import static org.keycloak.models.map.userSession.SessionExpiration.setClientSessionExpiration; import static org.keycloak.models.map.userSession.SessionExpiration.setUserSessionExpiration; @@ -90,8 +89,7 @@ public class MapUserSessionProvider implements UserSessionProvider { userSessionTx.delete(origEntity.getId()); return null; } else { - return new MapUserSessionAdapter(session, realm, - Objects.equals(origEntity.getPersistenceState(), TRANSIENT) ? origEntity : registerEntityForChanges(userSessionTx, origEntity)) { + return new MapUserSessionAdapter(session, realm, origEntity) { @Override public String getId() { return userSessionStore.getKeyConvertor().keyToString(entity.getId()); @@ -123,7 +121,7 @@ public class MapUserSessionProvider implements UserSessionProvider { clientSessionTx.delete(origEntity.getId()); return null; } else { - return new MapAuthenticatedClientSessionAdapter(session, realm, client, userSession, registerEntityForChanges(clientSessionTx, origEntity)) { + return new MapAuthenticatedClientSessionAdapter(session, realm, client, userSession, origEntity) { @Override public String getId() { return clientSessionStore.getKeyConvertor().keyToString(entity.getId()); @@ -382,7 +380,7 @@ public class MapUserSessionProvider implements UserSessionProvider { LOG.tracef("removeUserSession(%s, %s)%s", realm, session, getShortStackTrace()); - userSessionTx.delete(userSessionStore.getKeyConvertor().yieldNewUniqueKey(), withCriteria(mcb)); + userSessionTx.delete(withCriteria(mcb)); } @Override @@ -393,7 +391,7 @@ public class MapUserSessionProvider implements UserSessionProvider { LOG.tracef("removeUserSessions(%s, %s)%s", realm, user, getShortStackTrace()); - userSessionTx.delete(userSessionStore.getKeyConvertor().yieldNewUniqueKey(), withCriteria(mcb)); + userSessionTx.delete(withCriteria(mcb)); } @Override @@ -412,7 +410,7 @@ public class MapUserSessionProvider implements UserSessionProvider { LOG.tracef("removeUserSessions(%s)%s", realm, getShortStackTrace()); - userSessionTx.delete(userSessionStore.getKeyConvertor().yieldNewUniqueKey(), withCriteria(mcb)); + userSessionTx.delete(withCriteria(mcb)); } @Override @@ -469,7 +467,7 @@ public class MapUserSessionProvider implements UserSessionProvider { UK uk = userSessionStore.getKeyConvertor().fromString(userSession.getNote(CORRESPONDING_SESSION_ID)); mcb = realmAndOfflineCriteriaBuilder(realm, true) .compare(UserSessionModel.SearchableFields.ID, ModelCriteriaBuilder.Operator.EQ, uk); - userSessionTx.delete(userSessionStore.getKeyConvertor().yieldNewUniqueKey(), withCriteria(mcb)); + userSessionTx.delete(withCriteria(mcb)); userSession.removeNote(CORRESPONDING_SESSION_ID); } } @@ -637,7 +635,7 @@ public class MapUserSessionProvider implements UserSessionProvider { if (userSessionEntity == null) { MapUserSessionEntity userSession = userSessionTx.read(id); - return userSession != null ? registerEntityForChanges(userSessionTx, userSession) : null; + return userSession; } return userSessionEntity; } diff --git a/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionProviderFactory.java b/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionProviderFactory.java index 88e8037c7a..fdb671d970 100644 --- a/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionProviderFactory.java +++ b/model/map/src/main/java/org/keycloak/models/map/userSession/MapUserSessionProviderFactory.java @@ -83,12 +83,12 @@ public class MapUserSessionProviderFactory implements AmphibianProviderF MapStorageProviderFactory storageProviderFactoryUs = (MapStorageProviderFactory) getComponentFactory(session.getKeycloakSessionFactory(), MapStorageProvider.class, storageConfigScopeUserSessions, MapStorageSpi.NAME); final MapStorageProvider factoryUs = storageProviderFactoryUs.create(session); - MapStorage userSessionStore = factoryUs.getStorage(MapUserSessionEntity.class, UserSessionModel.class); + MapStorage userSessionStore = factoryUs.getStorage(UserSessionModel.class); MapStorageProviderFactory storageProviderFactoryCs = (MapStorageProviderFactory) getComponentFactory(session.getKeycloakSessionFactory(), MapStorageProvider.class, storageConfigScopeClientSessions, MapStorageSpi.NAME); final MapStorageProvider factoryCs = storageProviderFactoryCs.create(session); - MapStorage clientSessionStore = factoryCs.getStorage(MapAuthenticatedClientSessionEntity.class, AuthenticatedClientSessionModel.class); + MapStorage clientSessionStore = factoryCs.getStorage(AuthenticatedClientSessionModel.class); return new MapUserSessionProvider<>(session, userSessionStore, clientSessionStore); } diff --git a/server-spi/src/main/java/org/keycloak/storage/SearchableModelField.java b/server-spi/src/main/java/org/keycloak/storage/SearchableModelField.java index 8923565cb9..1b075e71f4 100644 --- a/server-spi/src/main/java/org/keycloak/storage/SearchableModelField.java +++ b/server-spi/src/main/java/org/keycloak/storage/SearchableModelField.java @@ -16,8 +16,6 @@ */ package org.keycloak.storage; -import java.util.Objects; - /** * * @author hmlnarik diff --git a/testsuite/model/src/test/java/org/keycloak/testsuite/model/MapStorageTest.java b/testsuite/model/src/test/java/org/keycloak/testsuite/model/MapStorageTest.java index 97bc544e04..f19e1c1701 100644 --- a/testsuite/model/src/test/java/org/keycloak/testsuite/model/MapStorageTest.java +++ b/testsuite/model/src/test/java/org/keycloak/testsuite/model/MapStorageTest.java @@ -82,9 +82,9 @@ public class MapStorageTest extends KeycloakModelTest { String component2Id = createMapStorageComponent("component2", "keyType", "string"); Object[] ids = withRealm(realmId, (session, realm) -> { - MapStorage, ClientModel> storageMain = (MapStorage, ClientModel>) session.getProvider(MapStorageProvider.class).getStorage(MapClientEntity.class, ClientModel.class); - MapStorage, ClientModel> storage1 = (MapStorage, ClientModel>) session.getComponentProvider(MapStorageProvider.class, component1Id).getStorage(MapClientEntity.class, ClientModel.class); - MapStorage, ClientModel> storage2 = (MapStorage, ClientModel>) session.getComponentProvider(MapStorageProvider.class, component2Id).getStorage(MapClientEntity.class, ClientModel.class); + MapStorage, ClientModel> storageMain = (MapStorage) session.getProvider(MapStorageProvider.class).getStorage(ClientModel.class); + MapStorage, ClientModel> storage1 = (MapStorage) session.getComponentProvider(MapStorageProvider.class, component1Id).getStorage(ClientModel.class); + MapStorage, ClientModel> storage2 = (MapStorage) session.getComponentProvider(MapStorageProvider.class, component2Id).getStorage(ClientModel.class); // Assert that the map storage can be used both as a standalone store and a component assertThat(storageMain, notNullValue()); @@ -161,11 +161,11 @@ public class MapStorageTest extends KeycloakModelTest { // Check that in the next transaction, the objects are still there withRealm(realmId, (session, realm) -> { @SuppressWarnings("unchecked") - MapStorage, ClientModel> storageMain = (MapStorage, ClientModel>) session.getProvider(MapStorageProvider.class).getStorage(MapClientEntity.class, ClientModel.class); + MapStorage, ClientModel> storageMain = (MapStorage) session.getProvider(MapStorageProvider.class).getStorage(ClientModel.class); @SuppressWarnings("unchecked") - MapStorage, ClientModel> storage1 = (MapStorage, ClientModel>) session.getComponentProvider(MapStorageProvider.class, component1Id).getStorage(MapClientEntity.class, ClientModel.class); + MapStorage, ClientModel> storage1 = (MapStorage) session.getComponentProvider(MapStorageProvider.class, component1Id).getStorage(ClientModel.class); @SuppressWarnings("unchecked") - MapStorage, ClientModel> storage2 = (MapStorage, ClientModel>) session.getComponentProvider(MapStorageProvider.class, component2Id).getStorage(MapClientEntity.class, ClientModel.class); + MapStorage, ClientModel> storage2 = (MapStorage) session.getComponentProvider(MapStorageProvider.class, component2Id).getStorage(ClientModel.class); final StringKeyConvertor kcMain = storageMain.getKeyConvertor(); final StringKeyConvertor kc1 = storage1.getKeyConvertor();