keycloak-memoizeit

refactor user cache

3/10/2016 2:57:20 PM

Changes

model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserCache.java 42(+0 -42)

Details

diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/CacheManager.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/CacheManager.java
index ee66761..e41913d 100755
--- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/CacheManager.java
+++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/CacheManager.java
@@ -23,24 +23,26 @@ public abstract class CacheManager {
     protected static final Logger logger = Logger.getLogger(CacheManager.class);
     protected final Cache<String, Long> revisions;
     protected final Cache<String, Revisioned> cache;
+    protected final UpdateCounter counter = new UpdateCounter();
 
     public CacheManager(Cache<String, Revisioned> cache, Cache<String, Long> revisions) {
         this.cache = cache;
         this.revisions = revisions;
+        this.cache.addListener(this);
     }
 
     public Cache<String, Revisioned> getCache() {
         return cache;
     }
 
-    public Cache<String, Long> getRevisions() {
-        return revisions;
+    public long getCurrentCounter() {
+        return counter.current();
     }
 
     public Long getCurrentRevision(String id) {
         Long revision = revisions.get(id);
         if (revision == null) {
-            revision = UpdateCounter.current();
+            revision = counter.current();
         }
         // if you do cache.remove() on node 1 and the entry doesn't exist on node 2, node 2 never receives a invalidation event
         // so, we do this to force this.
@@ -85,7 +87,7 @@ public abstract class CacheManager {
     }
 
     protected void bumpVersion(String id) {
-        long next = UpdateCounter.next();
+        long next = counter.next();
         Object rev = revisions.put(id, next);
     }
 
@@ -97,7 +99,7 @@ public abstract class CacheManager {
             Long rev = revisions.get(id);
             if (rev == null) {
                 if (id.endsWith("realm.clients")) RealmCacheManager.logger.trace("addRevisioned rev == null realm.clients");
-                rev = UpdateCounter.current();
+                rev = counter.current();
                 revisions.put(id, rev);
             }
             revisions.startBatch();
diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedUser.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedUser.java
index c1bf4cd..af30000 100755
--- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedUser.java
+++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedUser.java
@@ -34,8 +34,7 @@ import java.util.Set;
  * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
  * @version $Revision: 1 $
  */
-public class CachedUser implements Serializable {
-    private String id;
+public class CachedUser extends AbstractRevisioned implements InRealm  {
     private String realm;
     private String username;
     private Long createdTimestamp;
@@ -53,8 +52,10 @@ public class CachedUser implements Serializable {
     private Set<String> roleMappings = new HashSet<>();
     private Set<String> groups = new HashSet<>();
 
-    public CachedUser(RealmModel realm, UserModel user) {
-        this.id = user.getId();
+
+
+    public CachedUser(Long revision, RealmModel realm, UserModel user) {
+        super(revision, user.getId());
         this.realm = realm.getId();
         this.username = user.getUsername();
         this.createdTimestamp = user.getCreatedTimestamp();
@@ -80,10 +81,6 @@ public class CachedUser implements Serializable {
         }
     }
 
-    public String getId() {
-        return id;
-    }
-
     public String getRealm() {
         return realm;
     }
diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/UserListQuery.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/UserListQuery.java
new file mode 100755
index 0000000..c19e7aa
--- /dev/null
+++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/UserListQuery.java
@@ -0,0 +1,49 @@
+package org.keycloak.models.cache.infinispan.entities;
+
+import org.keycloak.models.RealmModel;
+
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
+ * @version $Revision: 1 $
+ */
+public class UserListQuery extends AbstractRevisioned implements UserQuery {
+    private final Set<String> users;
+    private final String realm;
+    private final String realmName;
+
+    public UserListQuery(Long revisioned, String id, RealmModel realm, Set<String> users) {
+        super(revisioned, id);
+        this.realm = realm.getId();
+        this.realmName = realm.getName();
+        this.users = users;
+    }
+
+    public UserListQuery(Long revisioned, String id, RealmModel realm, String user) {
+        super(revisioned, id);
+        this.realm = realm.getId();
+        this.realmName = realm.getName();
+        this.users = new HashSet<>();
+        this.users.add(user);
+    }
+
+    @Override
+    public Set<String> getUsers() {
+        return users;
+    }
+
+    @Override
+    public String getRealm() {
+        return realm;
+    }
+
+    @Override
+    public String toString() {
+        return "UserListQuery{" +
+                "id='" + getId() + "'" +
+                "realmName='" + realmName + '\'' +
+                '}';
+    }
+}
diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/UserQuery.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/UserQuery.java
new file mode 100755
index 0000000..5f890a2
--- /dev/null
+++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/UserQuery.java
@@ -0,0 +1,11 @@
+package org.keycloak.models.cache.infinispan.entities;
+
+import java.util.Set;
+
+/**
+ * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
+ * @version $Revision: 1 $
+ */
+public interface UserQuery extends InRealm {
+    Set<String> getUsers();
+}
diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/InfinispanCacheUserProviderFactory.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/InfinispanCacheUserProviderFactory.java
index e8657ff..9e93d55 100755
--- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/InfinispanCacheUserProviderFactory.java
+++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/InfinispanCacheUserProviderFactory.java
@@ -29,6 +29,7 @@ import org.keycloak.models.KeycloakSessionFactory;
 import org.keycloak.models.cache.CacheUserProvider;
 import org.keycloak.models.cache.CacheUserProviderFactory;
 import org.keycloak.models.cache.infinispan.entities.CachedUser;
+import org.keycloak.models.cache.infinispan.entities.Revisioned;
 
 import java.util.concurrent.ConcurrentHashMap;
 
@@ -39,25 +40,23 @@ public class InfinispanCacheUserProviderFactory implements CacheUserProviderFact
 
     private static final Logger log = Logger.getLogger(InfinispanCacheUserProviderFactory.class);
 
-    protected volatile InfinispanUserCache userCache;
+    protected volatile UserCacheManager userCache;
 
-    protected final RealmLookup usernameLookup = new RealmLookup();
 
-    protected final RealmLookup emailLookup = new RealmLookup();
 
     @Override
     public CacheUserProvider create(KeycloakSession session) {
         lazyInit(session);
-        return new DefaultCacheUserProvider(userCache, session);
+        return new UserCacheSession(userCache, session);
     }
 
     private void lazyInit(KeycloakSession session) {
         if (userCache == null) {
             synchronized (this) {
                 if (userCache == null) {
-                    Cache<String, CachedUser> cache = session.getProvider(InfinispanConnectionProvider.class).getCache(InfinispanConnectionProvider.USER_CACHE_NAME);
-                    cache.addListener(new CacheListener());
-                    userCache = new InfinispanUserCache(cache, usernameLookup, emailLookup);
+                    Cache<String, Revisioned> cache = session.getProvider(InfinispanConnectionProvider.class).getCache(InfinispanConnectionProvider.USER_CACHE_NAME);
+                    Cache<String, Long> revisions = session.getProvider(InfinispanConnectionProvider.class).getCache(InfinispanConnectionProvider.VERSION_CACHE_NAME);
+                    userCache = new UserCacheManager(cache, revisions);
                 }
             }
         }
@@ -81,100 +80,5 @@ public class InfinispanCacheUserProviderFactory implements CacheUserProviderFact
         return "default";
     }
 
-    @Listener
-    public class CacheListener {
-
-        @CacheEntryCreated
-        public void userCreated(CacheEntryCreatedEvent<String, CachedUser> event) {
-            if (!event.isPre()) {
-                CachedUser user = event.getValue();
-                if (user != null) {
-                    String realm = user.getRealm();
-
-                    usernameLookup.put(realm, user.getUsername(), user.getId());
-                    if (user.getEmail() != null) {
-                        emailLookup.put(realm, user.getEmail(), user.getId());
-                    }
-
-                    log.tracev("User added realm={0}, id={1}, username={2}", realm, user.getId(), user.getUsername());
-                }
-            }
-        }
-
-        @CacheEntryRemoved
-        public void userRemoved(CacheEntryRemovedEvent<String, CachedUser> event) {
-            if (event.isPre()) {
-                CachedUser user = event.getValue();
-                if (user != null) {
-                    removeUser(user);
-
-                    log.tracev("User invalidated realm={0}, id={1}, username={2}", user.getRealm(), user.getId(), user.getUsername());
-                }
-            }
-        }
-
-        @CacheEntryInvalidated
-        public void userInvalidated(CacheEntryInvalidatedEvent<String, CachedUser> event) {
-            if (event.isPre()) {
-                CachedUser user = event.getValue();
-                if (user != null) {
-                    removeUser(user);
-
-                    log.tracev("User invalidated realm={0}, id={1}, username={2}", user.getRealm(), user.getId(), user.getUsername());
-                }
-            }
-        }
-
-        @CacheEntriesEvicted
-        public void userEvicted(CacheEntriesEvictedEvent<String, CachedUser> event) {
-            for (CachedUser user : event.getEntries().values()) {
-                removeUser(user);
-
-                log.tracev("User evicted realm={0}, id={1}, username={2}", user.getRealm(), user.getId(), user.getUsername());
-            }
-        }
-
-        private void removeUser(CachedUser cachedUser) {
-            String realm = cachedUser.getRealm();
-            usernameLookup.remove(realm, cachedUser.getUsername());
-            if (cachedUser.getEmail() != null) {
-                emailLookup.remove(realm, cachedUser.getEmail());
-            }
-        }
-
-    }
-
-    static class RealmLookup {
-
-        protected final ConcurrentHashMap<String, ConcurrentHashMap<String, String>> lookup = new ConcurrentHashMap<>();
-
-        public void put(String realm, String key, String value) {
-            ConcurrentHashMap<String, String> map = lookup.get(realm);
-            if(map == null) {
-                map = new ConcurrentHashMap<>();
-                ConcurrentHashMap<String, String> p = lookup.putIfAbsent(realm, map);
-                if (p != null) {
-                    map = p;
-                }
-            }
-            map.put(key, value);
-        }
-
-        public String get(String realm, String key) {
-            ConcurrentHashMap<String, String> map = lookup.get(realm);
-            return map != null ? map.get(key) : null;
-        }
-
-        public void remove(String realm, String key) {
-            ConcurrentHashMap<String, String> map = lookup.get(realm);
-            if (map != null) {
-                map.remove(key);
-                if (map.isEmpty()) {
-                    lookup.remove(realm);
-                }
-            }
-        }
-
-    }
 
 }
diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheManager.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheManager.java
index 9923edb..55e3b38 100755
--- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheManager.java
+++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheManager.java
@@ -48,7 +48,6 @@ public class RealmCacheManager extends CacheManager {
 
     public RealmCacheManager(Cache<String, Revisioned> cache, Cache<String, Long> revisions) {
         super(cache, revisions);
-        this.cache.addListener(this);
     }
 
 
diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java
index af8fbb1..4a149ba 100755
--- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java
+++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java
@@ -131,7 +131,7 @@ public class RealmCacheSession implements CacheRealmProvider {
     public RealmCacheSession(RealmCacheManager cache, KeycloakSession session) {
         this.cache = cache;
         this.session = session;
-        this.startupRevision = UpdateCounter.current();
+        this.startupRevision = cache.getCurrentCounter();
         session.getTransaction().enlistPrepare(getPrepareTransaction());
         session.getTransaction().enlistAfterCompletion(getAfterTransaction());
     }
@@ -380,6 +380,14 @@ public class RealmCacheSession implements CacheRealmProvider {
         return getDelegate().removeRealm(id);
     }
 
+    protected void invalidateClient(RealmModel realm, ClientModel client) {
+        invalidations.add(client.getId());
+        invalidations.add(getRealmClientsQueryCacheKey(realm.getId()));
+        invalidations.add(getClientByClientIdCacheKey(client.getClientId(), realm));
+        listInvalidations.add(realm.getId());
+    }
+
+
     @Override
     public ClientModel addClient(RealmModel realm, String clientId) {
         ClientModel client = getDelegate().addClient(realm, clientId);
@@ -395,8 +403,7 @@ public class RealmCacheSession implements CacheRealmProvider {
     private ClientModel addedClient(RealmModel realm, ClientModel client) {
         logger.trace("added Client.....");
         // need to invalidate realm client query cache every time as it may not be loaded on this node, but loaded on another
-        invalidations.add(getRealmClientsQueryCacheKey(realm.getId()));
-        invalidations.add(client.getId());
+        invalidateClient(realm, client);
         cache.clientAdded(realm.getId(), client.getId(), invalidations);
         // this is needed so that a new client that hasn't been committed isn't cached in a query
         listInvalidations.add(realm.getId());
@@ -464,10 +471,7 @@ public class RealmCacheSession implements CacheRealmProvider {
         ClientModel client = getClientById(id, realm);
         if (client == null) return false;
         // need to invalidate realm client query cache every time client list is changed
-        invalidations.add(getRealmClientsQueryCacheKey(realm.getId()));
-        invalidations.add(getClientByClientIdCacheKey(client.getClientId(), realm));
-        listInvalidations.add(realm.getId());
-        registerClientInvalidation(id);
+        invalidateClient(realm, client);
         cache.clientRemoval(realm.getId(), id, invalidations);
         for (RoleModel role : client.getRoles()) {
             cache.roleInvalidation(role.getId(), invalidations);
diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UpdateCounter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UpdateCounter.java
index 8097074..966b1f4 100755
--- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UpdateCounter.java
+++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UpdateCounter.java
@@ -9,13 +9,13 @@ import java.util.concurrent.atomic.AtomicLong;
  */
 public class UpdateCounter {
 
-    private static final AtomicLong counter = new AtomicLong();
+    private final AtomicLong counter = new AtomicLong();
 
-    public static long current() {
+    public long current() {
         return counter.get();
     }
 
-    public static long next() {
+    public long next() {
         return counter.incrementAndGet();
     }
 
diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserAdapter.java
index 6421ca2..6e53626 100755
--- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserAdapter.java
+++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserAdapter.java
@@ -31,11 +31,11 @@ import java.util.*;
 public class UserAdapter implements UserModel {
     protected UserModel updated;
     protected CachedUser cached;
-    protected CacheUserProvider userProviderCache;
+    protected UserCacheSession userProviderCache;
     protected KeycloakSession keycloakSession;
     protected RealmModel realm;
 
-    public UserAdapter(CachedUser cached, CacheUserProvider userProvider, KeycloakSession keycloakSession, RealmModel realm) {
+    public UserAdapter(CachedUser cached, UserCacheSession userProvider, KeycloakSession keycloakSession, RealmModel realm) {
         this.cached = cached;
         this.userProviderCache = userProvider;
         this.keycloakSession = keycloakSession;
@@ -44,7 +44,7 @@ public class UserAdapter implements UserModel {
 
     protected void getDelegateForUpdate() {
         if (updated == null) {
-            userProviderCache.registerUserInvalidation(realm, getId());
+            userProviderCache.registerUserInvalidation(realm, cached);
             updated = userProviderCache.getDelegate().getUserById(getId(), realm);
             if (updated == null) throw new IllegalStateException("Not found in database");
         }
diff --git a/server-spi/src/main/java/org/keycloak/models/cache/CacheUserProvider.java b/server-spi/src/main/java/org/keycloak/models/cache/CacheUserProvider.java
index d9c9e56..63c9706 100755
--- a/server-spi/src/main/java/org/keycloak/models/cache/CacheUserProvider.java
+++ b/server-spi/src/main/java/org/keycloak/models/cache/CacheUserProvider.java
@@ -27,5 +27,4 @@ import org.keycloak.models.UserProvider;
 public interface CacheUserProvider extends UserProvider {
     void clear();
     UserProvider getDelegate();
-    void registerUserInvalidation(RealmModel realm, String id);
 }
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/LDAPGroupMapperTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/LDAPGroupMapperTest.java
old mode 100644
new mode 100755
index 45afc47..43a9634
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/LDAPGroupMapperTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/LDAPGroupMapperTest.java
@@ -190,6 +190,7 @@ public class LDAPGroupMapperTest {
     public void test02_readOnlyGroupMappings() {
         KeycloakSession session = keycloakRule.startSession();
         try {
+            System.out.println("starting test02_readOnlyGroupMappings");
             RealmModel appRealm = session.realms().getRealmByName("test");
 
             UserFederationMapperModel mapperModel = appRealm.getUserFederationMapperByName(ldapModel.getId(), "groupsMapper");
@@ -221,6 +222,7 @@ public class LDAPGroupMapperTest {
             Assert.assertTrue(maryGroups.contains(group12));
 
             // Assert that access through DB will have just DB mapped groups
+            System.out.println("******");
             UserModel maryDB = session.userStorage().getUserByUsername("marykeycloak", appRealm);
             Set<GroupModel> maryDBGroups = maryDB.getGroups();
             Assert.assertFalse(maryDBGroups.contains(group1));
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/OAuthClient.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/OAuthClient.java
index e53ebfb..7ccd8d7 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/OAuthClient.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/OAuthClient.java
@@ -93,7 +93,7 @@ public class OAuthClient {
 
     public AuthorizationCodeResponse doLogin(String username, String password) {
         openLoginForm();
-
+        String src = driver.getPageSource();
         driver.findElement(By.id("username")).sendKeys(username);
         driver.findElement(By.id("password")).sendKeys(password);
         driver.findElement(By.name("login")).click();