keycloak-uncached

Details

diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/StreamCacheRealmProvider.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/StreamCacheRealmProvider.java
index c10bba4..b33a0e8 100755
--- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/StreamCacheRealmProvider.java
+++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/StreamCacheRealmProvider.java
@@ -66,8 +66,6 @@ import java.util.Set;
  * it is added to the cache.  So, we keep the version number around for this.
  * - In a transaction, objects are registered to be invalidated.  If an object is marked for invalidation within a transaction
  * a cached object should never be returned.  An DB adapter should always be returned.
- * - At prepare phase of the transaction, a local lock on the revision cache will be obtained for each object marked for invalidation
- * we sort the list of these keys to order local acquisition and avoid deadlocks.
  * - After DB commits, the objects marked for invalidation are invalidated, or rather removed from the cache.  At this time
  * the revision cache entry for this object has its version number bumped.
  * - Whenever an object is marked for invalidation, the cache is also searched for any objects that are related to this object
@@ -88,15 +86,20 @@ import java.util.Set;
  * - There is a Infinispan @Listener registered.  If an invalidation event happens, this is treated like
  * the object was removed from the database and will perform evictions based on that assumption.
  * - Eviction events will also cascade other evictions, but not assume this is a db removal.
+ * - With an invalidation cache, if you remove an entry on node 1 and this entry does not exist on node 2, node 2 will not receive a @Listener invalidation event.
+ * so, hat we have to put a marker entry in the invalidation cache before we read from the DB, so if the DB changes in between reading and adding a cache entry, the cache will be notified and bump
+ * the version information.
+ *
+ * DBs with Repeatable Read:
+ * - DBs like MySQL are Repeatable Read by default.  So, if you query a Client for instance, it will always return the same result in the same transaction even if the DB
+ * was updated in between these queries.  This makes it possible to store stale cache entries.  To avoid this problem, this class stores the current local version counter
+ * at the beginningof the transaction.  Whenever an entry is added to the cache, the current coutner is compared against the counter at the beginning of the tx.  If the current
+ * is greater, then don't cache.
  *
  * Groups and Roles:
  * - roles are tricky because of composites.  Composite lists are cached too.  So, when a role is removed
  * we also iterate and invalidate any role or group that contains that role being removed.
  *
- * - Clustering gotchyas. With an invalidation cache, if you remove an entry on node 1 and this entry does not exist on node 2, node 2 will not receive a @Listener invalidation event.
- * so, hat we have to put a marker entry in the invalidation cache before we read from the DB, so if the DB changes in between reading and adding a cache entry, the cache will be notified and bump
- * the version information.
- *
  * - any relationship should be resolved from session.realms().  For example if JPA.getClientByClientId() is invoked,
  *  JPA should find the id of the client and then call session.realms().getClientById().  THis is to ensure that the cached
  *  object is invoked and all proper invalidation are being invoked.
@@ -124,14 +127,20 @@ public class StreamCacheRealmProvider implements CacheRealmProvider {
     protected Set<String> invalidations = new HashSet<>();
 
     protected boolean clearAll;
+    protected final long startupRevision;
 
     public StreamCacheRealmProvider(StreamRealmCache cache, KeycloakSession session) {
         this.cache = cache;
         this.session = session;
+        this.startupRevision = UpdateCounter.current();
         session.getTransaction().enlistPrepare(getPrepareTransaction());
         session.getTransaction().enlistAfterCompletion(getAfterTransaction());
     }
 
+    public long getStartupRevision() {
+        return startupRevision;
+    }
+
     @Override
     public void clear() {
         cache.clear();
@@ -305,7 +314,7 @@ public class StreamCacheRealmProvider implements CacheRealmProvider {
             if (model == null) return null;
             if (invalidations.contains(id)) return model;
             cached = new CachedRealm(loaded, model);
-            cache.addRevisioned(cached, session);
+            cache.addRevisioned(cached, startupRevision);
         } else if (invalidations.contains(id)) {
             return getDelegate().getRealm(id);
         } else if (managedRealms.containsKey(id)) {
@@ -329,7 +338,7 @@ public class StreamCacheRealmProvider implements CacheRealmProvider {
             if (model == null) return null;
             if (invalidations.contains(model.getId())) return model;
             query = new RealmListQuery(loaded, cacheKey, model.getId());
-            cache.addRevisioned(query, session);
+            cache.addRevisioned(query, startupRevision);
             return model;
         } else if (invalidations.contains(cacheKey)) {
             return getDelegate().getRealmByName(name);
@@ -435,7 +444,7 @@ public class StreamCacheRealmProvider implements CacheRealmProvider {
             for (ClientModel client : model) ids.add(client.getId());
             query = new ClientListQuery(loaded, cacheKey, realm, ids);
             logger.tracev("adding realm clients cache miss: realm {0} key {1}", realm.getName(), cacheKey);
-            cache.addRevisioned(query, session);
+            cache.addRevisioned(query, startupRevision);
             return model;
         }
         List<ClientModel> list = new LinkedList<>();
@@ -508,7 +517,7 @@ public class StreamCacheRealmProvider implements CacheRealmProvider {
             for (RoleModel role : model) ids.add(role.getId());
             query = new RoleListQuery(loaded, cacheKey, realm, ids);
             logger.tracev("adding realm roles cache miss: realm {0} key {1}", realm.getName(), cacheKey);
-            cache.addRevisioned(query, session);
+            cache.addRevisioned(query, startupRevision);
             return model;
         }
         Set<RoleModel> list = new HashSet<>();
@@ -544,7 +553,7 @@ public class StreamCacheRealmProvider implements CacheRealmProvider {
             for (RoleModel role : model) ids.add(role.getId());
             query = new RoleListQuery(loaded, cacheKey, realm, ids, client.getClientId());
             logger.tracev("adding client roles cache miss: client {0} key {1}", client.getClientId(), cacheKey);
-            cache.addRevisioned(query, session);
+            cache.addRevisioned(query, startupRevision);
             return model;
         }
         Set<RoleModel> list = new HashSet<>();
@@ -593,7 +602,7 @@ public class StreamCacheRealmProvider implements CacheRealmProvider {
             if (model == null) return null;
             query = new RoleListQuery(loaded, cacheKey, realm, model.getId());
             logger.tracev("adding realm role cache miss: client {0} key {1}", realm.getName(), cacheKey);
-            cache.addRevisioned(query, session);
+            cache.addRevisioned(query, startupRevision);
             return model;
         }
         RoleModel role = getRoleById(query.getRoles().iterator().next(), realm);
@@ -623,7 +632,7 @@ public class StreamCacheRealmProvider implements CacheRealmProvider {
             if (model == null) return null;
             query = new RoleListQuery(loaded, cacheKey, realm, model.getId(), client.getClientId());
             logger.tracev("adding client role cache miss: client {0} key {1}", client.getClientId(), cacheKey);
-            cache.addRevisioned(query, session);
+            cache.addRevisioned(query, startupRevision);
             return model;
         }
         RoleModel role = getRoleById(query.getRoles().iterator().next(), realm);
@@ -660,7 +669,7 @@ public class StreamCacheRealmProvider implements CacheRealmProvider {
             } else {
                 cached = new CachedRealmRole(loaded, model, realm);
             }
-            cache.addRevisioned(cached, session);
+            cache.addRevisioned(cached, startupRevision);
 
         } else if (invalidations.contains(id)) {
             return getDelegate().getRoleById(id, realm);
@@ -685,7 +694,7 @@ public class StreamCacheRealmProvider implements CacheRealmProvider {
             if (model == null) return null;
             if (invalidations.contains(id)) return model;
             cached = new CachedGroup(loaded, realm, model);
-            cache.addRevisioned(cached, session);
+            cache.addRevisioned(cached, startupRevision);
 
         } else if (invalidations.contains(id)) {
             return getDelegate().getGroupById(id, realm);
@@ -725,7 +734,7 @@ public class StreamCacheRealmProvider implements CacheRealmProvider {
             for (GroupModel client : model) ids.add(client.getId());
             query = new GroupListQuery(loaded, cacheKey, realm, ids);
             logger.tracev("adding realm getGroups cache miss: realm {0} key {1}", realm.getName(), cacheKey);
-            cache.addRevisioned(query, session);
+            cache.addRevisioned(query, startupRevision);
             return model;
         }
         List<GroupModel> list = new LinkedList<>();
@@ -761,7 +770,7 @@ public class StreamCacheRealmProvider implements CacheRealmProvider {
             for (GroupModel client : model) ids.add(client.getId());
             query = new GroupListQuery(loaded, cacheKey, realm, ids);
             logger.tracev("adding realm getTopLevelGroups cache miss: realm {0} key {1}", realm.getName(), cacheKey);
-            cache.addRevisioned(query, session);
+            cache.addRevisioned(query, startupRevision);
             return model;
         }
         List<GroupModel> list = new LinkedList<>();
@@ -837,7 +846,7 @@ public class StreamCacheRealmProvider implements CacheRealmProvider {
             if (invalidations.contains(id)) return model;
             cached = new CachedClient(loaded, realm, model);
             logger.tracev("adding client by id cache miss: {0}", cached.getClientId());
-            cache.addRevisioned(cached, session);
+            cache.addRevisioned(cached, startupRevision);
         } else if (invalidations.contains(id)) {
             return getDelegate().getClientById(id, realm);
         } else if (managedApplications.containsKey(id)) {
@@ -866,7 +875,7 @@ public class StreamCacheRealmProvider implements CacheRealmProvider {
             id = model.getId();
             query = new ClientListQuery(loaded, cacheKey, realm, id);
             logger.tracev("adding client by name cache miss: {0}", clientId);
-            cache.addRevisioned(query, session);
+            cache.addRevisioned(query, startupRevision);
         } else if (invalidations.contains(cacheKey)) {
             return getDelegate().getClientByClientId(clientId, realm);
         } else {
@@ -895,7 +904,7 @@ public class StreamCacheRealmProvider implements CacheRealmProvider {
             if (model == null) return null;
             if (invalidations.contains(id)) return model;
             cached = new CachedClientTemplate(loaded, realm, model);
-            cache.addRevisioned(cached, session);
+            cache.addRevisioned(cached, startupRevision);
         } else if (invalidations.contains(id)) {
             return getDelegate().getClientTemplateById(id, realm);
         } else if (managedClientTemplates.containsKey(id)) {
diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/StreamRealmCache.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/StreamRealmCache.java
index 4815fb0..119db6f 100755
--- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/StreamRealmCache.java
+++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/StreamRealmCache.java
@@ -39,7 +39,6 @@ import org.keycloak.models.cache.infinispan.stream.HasRolePredicate;
 import org.keycloak.models.cache.infinispan.stream.InClientPredicate;
 import org.keycloak.models.cache.infinispan.stream.InRealmPredicate;
 import org.keycloak.models.cache.infinispan.stream.RealmQueryPredicate;
-import org.keycloak.models.utils.UpdateCounter;
 
 import java.util.HashSet;
 import java.util.Iterator;
@@ -124,7 +123,7 @@ public class StreamRealmCache {
         Object rev = revisions.put(id, next);
     }
 
-    public void addRevisioned(Revisioned object, KeycloakSession session) {
+    public void addRevisioned(Revisioned object, long startupRevision) {
         //startRevisionBatch();
         String id = object.getId();
         try {
@@ -145,9 +144,9 @@ public class StreamRealmCache {
                 if (id.endsWith("realm.clients")) logger.trace("addRevisioned rev2 == null realm.clients");
                 return;
             }
-            if (rev > session.getTransaction().getStartupRevision()) { // revision is ahead transaction start. Other transaction updated in the meantime. Don't cache
+            if (rev > startupRevision) { // revision is ahead transaction start. Other transaction updated in the meantime. Don't cache
                 if (logger.isTraceEnabled()) {
-                    logger.tracev("Skipped cache. Current revision {0}, Transaction start revision {1}", object.getRevision(), session.getTransaction().getStartupRevision());
+                    logger.tracev("Skipped cache. Current revision {0}, Transaction start revision {1}", object.getRevision(), startupRevision);
                 }
                 return;
             }
diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java
index 2040b22..8ed10be 100755
--- a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java
+++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java
@@ -55,15 +55,6 @@ public class JpaRealmProvider implements RealmProvider {
     private final KeycloakSession session;
     protected EntityManager em;
 
-    // we have a local map of adapter classes for two reasons
-    // 1. So we don't have to allocate one again
-    // 2. So that we can do em.refresh() on the entity to make sure the state
-    // is up to date.  If em.find is called a second time without a session, the same
-    // entity instance is returned.  With the caching layer above it, it will cache stale
-    // entries.
-    protected Map<String, Object> adapters = new HashMap<>();
-
-
     public JpaRealmProvider(KeycloakSession session, EntityManager em) {
         this.session = session;
         this.em = em;
@@ -93,20 +84,14 @@ public class JpaRealmProvider implements RealmProvider {
                 return adapter;
             }
         });
-        adapters.put(id, adapter);
         return adapter;
     }
 
     @Override
     public RealmModel getRealm(String id) {
-        RealmAdapter adapter = (RealmAdapter)findJpaModel(id);
-        if (adapter != null) {
-            return adapter;
-        }
         RealmEntity realm = em.find(RealmEntity.class, id);
         if (realm == null) return null;
-        adapter = new RealmAdapter(session, em, realm);
-        adapters.put(id, adapter);
+        RealmAdapter adapter = new RealmAdapter(session, em, realm);
         return adapter;
     }
 
@@ -144,7 +129,6 @@ public class JpaRealmProvider implements RealmProvider {
         em.refresh(realm);
         RealmAdapter adapter = new RealmAdapter(session, em, realm);
         session.users().preRemove(adapter);
-        adapters.remove(id);
         int num = em.createNamedQuery("deleteGroupRoleMappingsByRealm")
                 .setParameter("realm", realm).executeUpdate();
         num = em.createNamedQuery("deleteGroupAttributesByRealm")
@@ -194,7 +178,6 @@ public class JpaRealmProvider implements RealmProvider {
         em.persist(entity);
         em.flush();
         RoleAdapter adapter = new RoleAdapter(session, realm, em, entity);
-        adapters.put(id, adapter);
         return adapter;
 
     }
@@ -225,7 +208,6 @@ public class JpaRealmProvider implements RealmProvider {
         em.persist(roleEntity);
         em.flush();
         RoleAdapter adapter = new RoleAdapter(session, realm, em, roleEntity);
-        adapters.put(id, adapter);
         return adapter;
     }
 
@@ -274,7 +256,6 @@ public class JpaRealmProvider implements RealmProvider {
         if (container.getDefaultRoles().contains(role.getName())) {
             container.removeDefaultRoles(role.getName());
         }
-        adapters.remove(role.getId());
         RoleEntity roleEntity = em.getReference(RoleEntity.class, role.getId());
         String compositeRoleTable = JpaUtils.getTableNameForNativeQuery("COMPOSITE_ROLE", em);
         em.createNativeQuery("delete from " + compositeRoleTable + " where CHILD_ROLE = :role").setParameter("role", roleEntity).executeUpdate();
@@ -290,28 +271,19 @@ public class JpaRealmProvider implements RealmProvider {
 
     @Override
     public RoleModel getRoleById(String id, RealmModel realm) {
-        RoleAdapter adapter = (RoleAdapter)findJpaModel(id);
-        if (adapter != null) {
-            if (!realm.getId().equals(adapter.getEntity().getRealmId())) return null;
-            return adapter;
-        }
         RoleEntity entity = em.find(RoleEntity.class, id);
         if (entity == null) return null;
         if (!realm.getId().equals(entity.getRealmId())) return null;
-        adapter = new RoleAdapter(session, realm, em, entity);
-        adapters.put(id, adapter);
+        RoleAdapter adapter = new RoleAdapter(session, realm, em, entity);
         return adapter;
     }
 
     @Override
     public GroupModel getGroupById(String id, RealmModel realm) {
-        GroupAdapter adapter = (GroupAdapter)findJpaModel(id);
-        if (adapter != null) return adapter;
         GroupEntity groupEntity = em.find(GroupEntity.class, id);
         if (groupEntity == null) return null;
         if (!groupEntity.getRealm().getId().equals(realm.getId())) return null;
-        adapter =  new GroupAdapter(realm, em, groupEntity);
-        adapters.put(id, adapter);
+        GroupAdapter adapter =  new GroupAdapter(realm, em, groupEntity);
         return adapter;
     }
 
@@ -360,7 +332,6 @@ public class JpaRealmProvider implements RealmProvider {
         }
 
         session.users().preRemove(realm, group);
-        adapters.remove(group.getId());
 
         realm.removeDefaultGroup(group);
         for (GroupModel subGroup : group.getSubGroups()) {
@@ -397,7 +368,6 @@ public class JpaRealmProvider implements RealmProvider {
         em.persist(groupEntity);
 
         GroupAdapter adapter = new GroupAdapter(realm, em, groupEntity);
-        adapters.put(id, adapter);
         return adapter;
     }
 
@@ -428,7 +398,6 @@ public class JpaRealmProvider implements RealmProvider {
         em.persist(entity);
         em.flush();
         final ClientModel resource = new ClientAdapter(realm, em, session, entity);
-        adapters.put(id, resource);
 
         em.flush();
         session.getKeycloakSessionFactory().publish(new RealmModel.ClientCreationEvent() {
@@ -455,37 +424,12 @@ public class JpaRealmProvider implements RealmProvider {
 
     }
 
-    protected JpaModel findJpaModel(String id) {
-        // we have a local map of adapter classes for two reasons
-        // 1. So we don't have to allocate one again
-        // 2. So that we can do em.refresh() on the entity to make sure the state
-        // is up to date.  If em.find is called a second time without a session, the same
-        // entity instance is returned as its already in the first level cache.  With the caching layer above it, it will cache stale
-        // entries.
-        JpaModel client = (JpaModel)adapters.get(id);
-        if (client != null) {
-            if (em.contains(client.getEntity())) {
-                em.flush(); // have to flush as refresh blows away updates
-                em.refresh(client.getEntity());
-                return client;
-            }
-        }
-        return null;
-
-    }
-
     @Override
     public ClientModel getClientById(String id, RealmModel realm) {
-        ClientAdapter client = (ClientAdapter)findJpaModel(id);
-        if (client != null) {
-            if (!realm.getId().equals(client.getRealm().getId())) return null;
-            return client;
-        }
         ClientEntity app = em.find(ClientEntity.class, id);
         // Check if application belongs to this realm
         if (app == null || !realm.getId().equals(app.getRealm().getId())) return null;
-        client = new ClientAdapter(realm, em, session, app);
-        adapters.put(id, client);
+        ClientAdapter client = new ClientAdapter(realm, em, session, app);
         return client;
 
     }
@@ -523,23 +467,16 @@ public class JpaRealmProvider implements RealmProvider {
             logger.errorv("Unable to delete client entity: {0} from realm {1}", client.getClientId(), realm.getName());
             throw e;
         }
-        adapters.remove(id);
         return true;
     }
 
     @Override
     public ClientTemplateModel getClientTemplateById(String id, RealmModel realm) {
-        ClientTemplateAdapter adapter = (ClientTemplateAdapter)findJpaModel(id);
-        if (adapter != null) {
-            if (!realm.getId().equals(adapter.getRealm().getId())) return null;
-            return adapter;
-        }
         ClientTemplateEntity app = em.find(ClientTemplateEntity.class, id);
 
         // Check if application belongs to this realm
         if (app == null || !realm.getId().equals(app.getRealm().getId())) return null;
-        adapter = new ClientTemplateAdapter(realm, em, session, app);
-        adapters.put(id, adapter);
+        ClientTemplateAdapter adapter = new ClientTemplateAdapter(realm, em, session, app);
         return adapter;
     }
 }
diff --git a/server-spi/src/main/java/org/keycloak/models/KeycloakTransactionManager.java b/server-spi/src/main/java/org/keycloak/models/KeycloakTransactionManager.java
index 456de8a..0e2dcbe 100755
--- a/server-spi/src/main/java/org/keycloak/models/KeycloakTransactionManager.java
+++ b/server-spi/src/main/java/org/keycloak/models/KeycloakTransactionManager.java
@@ -23,8 +23,6 @@ package org.keycloak.models;
  */
 public interface KeycloakTransactionManager extends KeycloakTransaction {
 
-    long getStartupRevision();
-
     void enlist(KeycloakTransaction transaction);
     void enlistAfterCompletion(KeycloakTransaction transaction);
 
diff --git a/services/src/main/java/org/keycloak/services/DefaultKeycloakTransactionManager.java b/services/src/main/java/org/keycloak/services/DefaultKeycloakTransactionManager.java
index 0a208d6..53f92f4 100755
--- a/services/src/main/java/org/keycloak/services/DefaultKeycloakTransactionManager.java
+++ b/services/src/main/java/org/keycloak/services/DefaultKeycloakTransactionManager.java
@@ -18,8 +18,6 @@ package org.keycloak.services;
 
 import org.keycloak.models.KeycloakTransaction;
 import org.keycloak.models.KeycloakTransactionManager;
-import org.keycloak.models.utils.UpdateCounter;
-import org.keycloak.services.ServicesLogger;
 
 import java.util.LinkedList;
 import java.util.List;
@@ -36,12 +34,6 @@ public class DefaultKeycloakTransactionManager implements KeycloakTransactionMan
     private List<KeycloakTransaction> afterCompletion = new LinkedList<KeycloakTransaction>();
     private boolean active;
     private boolean rollback;
-    private long startupRevision;
-
-    @Override
-    public long getStartupRevision() {
-        return startupRevision;
-    }
 
     @Override
     public void enlist(KeycloakTransaction transaction) {
@@ -76,8 +68,6 @@ public class DefaultKeycloakTransactionManager implements KeycloakTransactionMan
              throw new IllegalStateException("Transaction already active");
         }
 
-        startupRevision = UpdateCounter.current();
-
         for (KeycloakTransaction tx : transactions) {
             tx.begin();
         }