From a45934a762d8726d1cd3ea9c2c0e34e09dd5b849 Mon Sep 17 00:00:00 2001 From: Alexander Schwartz Date: Wed, 15 Nov 2023 15:40:28 +0100 Subject: [PATCH] Disable cache store and load only if a remote store is used Closes #10803 Closes #24766 Signed-off-by: Alexander Schwartz Co-authored-by: daviddelannoy <16318239+daviddelannoy@users.noreply.github.com> --- .../sessions/infinispan/CacheDecorators.java | 24 +++++++++++++++---- .../InfinispanUserLoginFailureProvider.java | 2 +- .../InfinispanUserSessionProvider.java | 12 +++++----- .../InfinispanChangelogBasedTransaction.java | 13 +++++----- 4 files changed, 33 insertions(+), 18 deletions(-) diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/CacheDecorators.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/CacheDecorators.java index 23e5fb1b0a..ca88a6807e 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/CacheDecorators.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/CacheDecorators.java @@ -21,6 +21,8 @@ import org.infinispan.AdvancedCache; import org.infinispan.Cache; import org.infinispan.context.Flag; +import static org.keycloak.connections.infinispan.InfinispanUtil.getRemoteStores; + /** * @author Marek Posolda */ @@ -40,8 +42,15 @@ public class CacheDecorators { * @param cache * @return Cache with the flags applied. */ - public static AdvancedCache skipCacheLoaders(Cache cache) { - return cache.getAdvancedCache().withFlags(Flag.SKIP_CACHE_LOAD, Flag.SKIP_CACHE_STORE); + public static AdvancedCache skipCacheLoadersIfRemoteStoreIsEnabled(Cache cache) { + if (!getRemoteStores(cache).isEmpty()) { + // Disabling of the cache load and cache store is only needed when a remote store is used and handled separately. + return cache.getAdvancedCache().withFlags(Flag.SKIP_CACHE_LOAD, Flag.SKIP_CACHE_STORE); + } else { + // If there is no remote store, use write through for all stores of the cache. + // Mixing remote and non-remote caches is not supported. + return cache.getAdvancedCache().withFlags(Flag.SKIP_CACHE_LOAD); + } } /** @@ -49,8 +58,15 @@ public class CacheDecorators { * @param cache * @return Cache with the flags applied. */ - public static AdvancedCache skipCacheStore(Cache cache) { - return cache.getAdvancedCache().withFlags(Flag.SKIP_CACHE_STORE); + public static AdvancedCache skipCacheStoreIfRemoteCacheIsEnabled(Cache cache) { + if (!getRemoteStores(cache).isEmpty()) { + // Disabling of the cache load and cache store is only needed when a remote store is used and handled separately. + return cache.getAdvancedCache().withFlags(Flag.SKIP_CACHE_STORE); + } else { + // If there is no remote store, use write through for all stores of the cache. + // Mixing remote and non-remote caches is not supported. + return cache.getAdvancedCache(); + } } } diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserLoginFailureProvider.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserLoginFailureProvider.java index ce0b065f9f..78fcffc4b4 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserLoginFailureProvider.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserLoginFailureProvider.java @@ -118,7 +118,7 @@ public class InfinispanUserLoginFailureProvider implements UserLoginFailureProvi Cache> localCache = CacheDecorators.localCache(loginFailureCache); - Cache> localCacheStoreIgnore = CacheDecorators.skipCacheLoaders(localCache); + Cache> localCacheStoreIgnore = CacheDecorators.skipCacheLoadersIfRemoteStoreIsEnabled(localCache); localCacheStoreIgnore .entrySet() diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java index ae544b2398..9bb122b682 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java @@ -394,7 +394,7 @@ public class InfinispanUserSessionProvider implements UserSessionProvider { } Cache> cache = getCache(offline); - cache = CacheDecorators.skipCacheLoaders(cache); + cache = CacheDecorators.skipCacheLoadersIfRemoteStoreIsEnabled(cache); // return a stream that 'wraps' the infinispan cache stream so that the cache stream's elements are read one by one // and then mapped locally to avoid serialization issues when trying to manipulate the cache stream directly. @@ -564,7 +564,7 @@ public class InfinispanUserSessionProvider implements UserSessionProvider { } Cache> cache = getCache(offline); - cache = CacheDecorators.skipCacheLoaders(cache); + cache = CacheDecorators.skipCacheLoadersIfRemoteStoreIsEnabled(cache); return cache.entrySet().stream() .filter(UserSessionPredicate.create(realm.getId())) .map(Mappers.authClientSessionSetMapper()) @@ -603,7 +603,7 @@ public class InfinispanUserSessionProvider implements UserSessionProvider { protected void removeUserSessions(RealmModel realm, UserModel user, boolean offline) { Cache> cache = getCache(offline); - cache = CacheDecorators.skipCacheLoaders(cache); + cache = CacheDecorators.skipCacheLoadersIfRemoteStoreIsEnabled(cache); Iterator itr = cache.entrySet().stream().filter(UserSessionPredicate.create(realm.getId()).user(user.getId())).map(Mappers.userSessionEntity()).iterator(); @@ -647,7 +647,7 @@ public class InfinispanUserSessionProvider implements UserSessionProvider { Cache> clientSessionCache = getClientSessionCache(offline); Cache> localClientSessionCache = CacheDecorators.localCache(clientSessionCache); - Cache> localCacheStoreIgnore = CacheDecorators.skipCacheLoaders(localCache); + Cache> localCacheStoreIgnore = CacheDecorators.skipCacheLoadersIfRemoteStoreIsEnabled(localCache); final AtomicInteger userSessionsSize = new AtomicInteger(); @@ -906,7 +906,7 @@ public class InfinispanUserSessionProvider implements UserSessionProvider { .collect(Collectors.toMap(sessionEntityWrapper -> sessionEntityWrapper.getEntity().getId(), Function.identity())); // Directly put all entities to the infinispan cache - Cache> cache = CacheDecorators.skipCacheLoaders(getCache(offline)); + Cache> cache = CacheDecorators.skipCacheLoadersIfRemoteStoreIsEnabled(getCache(offline)); boolean importWithExpiration = sessionsById.size() == 1; if (importWithExpiration) { @@ -951,7 +951,7 @@ public class InfinispanUserSessionProvider implements UserSessionProvider { // Import client sessions Cache> clientSessCache = - CacheDecorators.skipCacheLoaders(offline ? offlineClientSessionCache : clientSessionCache); + CacheDecorators.skipCacheLoadersIfRemoteStoreIsEnabled(offline ? offlineClientSessionCache : clientSessionCache); if (importWithExpiration) { importSessionsWithExpiration(clientSessionsById, clientSessCache, diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/changes/InfinispanChangelogBasedTransaction.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/changes/InfinispanChangelogBasedTransaction.java index b3b027fd2f..2df4073a2f 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/changes/InfinispanChangelogBasedTransaction.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/changes/InfinispanChangelogBasedTransaction.java @@ -24,7 +24,6 @@ import java.util.concurrent.TimeUnit; import org.infinispan.Cache; import org.infinispan.context.Flag; import org.jboss.logging.Logger; -import org.keycloak.models.ClientModel; import org.keycloak.models.AbstractKeycloakTransaction; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; @@ -189,19 +188,19 @@ public class InfinispanChangelogBasedTransaction ext switch (operation) { case REMOVE: // Just remove it - CacheDecorators.skipCacheStore(cache) - .getAdvancedCache().withFlags(Flag.IGNORE_RETURN_VALUES) + CacheDecorators.skipCacheStoreIfRemoteCacheIsEnabled(cache) + .withFlags(Flag.IGNORE_RETURN_VALUES) .remove(key); break; case ADD: - CacheDecorators.skipCacheStore(cache) - .getAdvancedCache().withFlags(Flag.IGNORE_RETURN_VALUES) + CacheDecorators.skipCacheStoreIfRemoteCacheIsEnabled(cache) + .withFlags(Flag.IGNORE_RETURN_VALUES) .put(key, sessionWrapper, task.getLifespanMs(), TimeUnit.MILLISECONDS, task.getMaxIdleTimeMs(), TimeUnit.MILLISECONDS); logger.tracef("Added entity '%s' to the cache '%s' . Lifespan: %d ms, MaxIdle: %d ms", key, cache.getName(), task.getLifespanMs(), task.getMaxIdleTimeMs()); break; case ADD_IF_ABSENT: - SessionEntityWrapper existing = CacheDecorators.skipCacheStore(cache).putIfAbsent(key, sessionWrapper, task.getLifespanMs(), TimeUnit.MILLISECONDS, task.getMaxIdleTimeMs(), TimeUnit.MILLISECONDS); + SessionEntityWrapper existing = CacheDecorators.skipCacheStoreIfRemoteCacheIsEnabled(cache).putIfAbsent(key, sessionWrapper, task.getLifespanMs(), TimeUnit.MILLISECONDS, task.getMaxIdleTimeMs(), TimeUnit.MILLISECONDS); if (existing != null) { logger.debugf("Existing entity in cache for key: %s . Will update it", key); @@ -234,7 +233,7 @@ public class InfinispanChangelogBasedTransaction ext SessionEntityWrapper newVersionEntity = generateNewVersionAndWrapEntity(session, oldVersionEntity.getLocalMetadata()); // Atomic cluster-aware replace - replaced = CacheDecorators.skipCacheStore(cache).replace(key, oldVersionEntity, newVersionEntity, lifespanMs, TimeUnit.MILLISECONDS, maxIdleTimeMs, TimeUnit.MILLISECONDS); + replaced = CacheDecorators.skipCacheStoreIfRemoteCacheIsEnabled(cache).replace(key, oldVersionEntity, newVersionEntity, lifespanMs, TimeUnit.MILLISECONDS, maxIdleTimeMs, TimeUnit.MILLISECONDS); // Replace fail. Need to load latest entity from cache, apply updates again and try to replace in cache again if (!replaced) {