Commit @4a6b42fa8e69bd2404faaa09e30100ea3a6119a9

doortts 2016-04-18

cache: Fix userMap cache bugs

- Remove userMap cache from CacheStore
- Change sessionMap cache to be timid

It fix user related cache bugs.
eg. https://github.com/yona-projects/yona/issues/39

@4a6b42fa8e69bd2404faaa09e30100ea3a6119a9

f974fa1

4a6b42f

app/controllers/UserApp.java

--- app/controllers/UserApp.java

+++ app/controllers/UserApp.java


             return invalidSession();
         }
         User user = null;
-        if (userKey != null){
-            user = CacheStore.sessionMap.get(userKey);
+        if (userKey != null && CacheStore.sessionMap.get(userKey) != null){
+            user = User.find.byId(CacheStore.sessionMap.get(userKey));
         }
         if (user == null) {
             return invalidSession();

     public static void addUserInfoToSession(User user) {
         String key = new Sha256Hash(new Date().toString(), ByteSource.Util.bytes(user.passwordSalt), 1024)
                 .toBase64();
-        CacheStore.sessionMap.put(key, user);
+        CacheStore.sessionMap.put(key, user.id);
         session(SESSION_USERID, String.valueOf(user.id));
         session(SESSION_LOGINID, user.loginId);
         session(SESSION_USERNAME, user.name);

f974fa1

4a6b42f

app/models/AbstractPosting.java

--- app/models/AbstractPosting.java

+++ app/models/AbstractPosting.java


 
         actualWatchers.add(getAuthor());
         for (Comment c : getComments()) {
-            User user = User.findByIdUsingCache(c.authorId);
+            User user = User.find.byId(c.id);
             if (user != null) {
                 actualWatchers.add(user);
             }

f974fa1

4a6b42f

app/models/NotificationEvent.java

--- app/models/NotificationEvent.java

+++ app/models/NotificationEvent.java


         }
 
         if (issue.assignee != null) {
-            notiEvent.newValue = User.findByIdUsingCache(issue.assignee.user.id).loginId;
+            notiEvent.newValue = User.find.byId(issue.assignee.user.id).loginId;
         }
         notiEvent.title = formatReplyTitle(issue);
         notiEvent.receivers = receivers;

f974fa1

4a6b42f

app/models/User.java

--- app/models/User.java

+++ app/models/User.java


         return user.id;
     }
 
-
-    /**
-     * find a user by id with cache
-     *
-     * If there is no user correspond to login id string,
-     * then null
-     *
-     * @param id
-     * @return User or null
-     */
-    public static User findByIdUsingCache(Long id) {
-        User cached = CacheStore.userMap.get(Long.toString(id)); // special char + added to loginId key
-        if(cached != null) {
-            return cached;
-        }
-        User user = find.byId(id);
-        if(user != null){
-            CacheStore.userMap.putIfAbsent(Long.toString(id), user);
-        }
-        return user;
-    }
-
     /**
      * find a user by login id string
      *

      * @return User or {@link #anonymous}
      */
     public static User findByLoginId(String loginId) {
-        User cached = CacheStore.userMap.get("+" + loginId); // special char + added to loginId key
-        if(cached != null) {
-            return cached;
-        }
         User user = find.where().eq("loginId", loginId).findUnique();
         if (user == null) {
             return anonymous;
         }
         else {
-            CacheStore.userMap.putIfAbsent("+" + loginId, user);
             return user;
         }
     }

f974fa1

4a6b42f

app/models/resource/Resource.java

--- app/models/resource/Resource.java

+++ app/models/resource/Resource.java


                 resource = Posting.finder.byId(longId).asResource();
                 break;
             case USER:
-                resource = User.findByIdUsingCache(longId).asResource();
+                resource = User.find.byId(longId).asResource();
                 break;
             case PROJECT:
                 resource = Project.find.byId(longId).asResource();

             case PULL_REQUEST:
                 return PullRequest.finder.byId(longId).asResource();
             case USER_AVATAR:
-                return User.findByIdUsingCache(longId).avatarAsResource();
+                return User.find.byId(longId).avatarAsResource();
             case REVIEW_COMMENT:
                 return ReviewComment.find.byId(longId).asResource();
             case ORGANIZATION:

f974fa1

4a6b42f

app/utils/CacheStore.java

--- app/utils/CacheStore.java

+++ app/utils/CacheStore.java


 
 import com.google.common.cache.Cache;
 import com.google.common.cache.CacheBuilder;
-import models.User;
 
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;

  * CacheStore
  */
 public class CacheStore {
-    public static Map<String, User> sessionMap = new ConcurrentHashMap<>();
-    public static Map<String, User> userMap = new ConcurrentHashMap<>();
+    public static Map<String, Long> sessionMap = new ConcurrentHashMap<>();
     public static Map<String, Long> projectMap = new ConcurrentHashMap<>();
     public static final int MAXIMUM_CACHED_MARKDOWN_ENTRY = 10000;
 

Add a comment

Open 0
Closed 0

List

...	...	@@ -374,8 +374,8 @@
374	374	return invalidSession();
375	375	}
376	376	User user = null;
377		- if (userKey != null){
378		- user = CacheStore.sessionMap.get(userKey);
	377	+ if (userKey != null && CacheStore.sessionMap.get(userKey) != null){
	378	+ user = User.find.byId(CacheStore.sessionMap.get(userKey));
379	379	}
380	380	if (user == null) {
381	381	return invalidSession();
...	...	@@ -878,7 +878,7 @@
878	878	public static void addUserInfoToSession(User user) {
879	879	String key = new Sha256Hash(new Date().toString(), ByteSource.Util.bytes(user.passwordSalt), 1024)
880	880	.toBase64();
881		- CacheStore.sessionMap.put(key, user);
	881	+ CacheStore.sessionMap.put(key, user.id);
882	882	session(SESSION_USERID, String.valueOf(user.id));
883	883	session(SESSION_LOGINID, user.loginId);
884	884	session(SESSION_USERNAME, user.name);

...	...	@@ -248,7 +248,7 @@
248	248
249	249	actualWatchers.add(getAuthor());
250	250	for (Comment c : getComments()) {
251		- User user = User.findByIdUsingCache(c.authorId);
	251	+ User user = User.find.byId(c.id);
252	252	if (user != null) {
253	253	actualWatchers.add(user);
254	254	}

...	...	@@ -689,7 +689,7 @@
689	689	}
690	690
691	691	if (issue.assignee != null) {
692		- notiEvent.newValue = User.findByIdUsingCache(issue.assignee.user.id).loginId;
	692	+ notiEvent.newValue = User.find.byId(issue.assignee.user.id).loginId;
693	693	}
694	694	notiEvent.title = formatReplyTitle(issue);
695	695	notiEvent.receivers = receivers;

...	...	@@ -255,28 +255,6 @@
255	255	return user.id;
256	256	}
257	257
258		-
259		- /**
260		- * find a user by id with cache
261		- *
262		- * If there is no user correspond to login id string,
263		- * then null
264		- *
265		- * @param id
266		- * @return User or null
267		- */
268		- public static User findByIdUsingCache(Long id) {
269		- User cached = CacheStore.userMap.get(Long.toString(id)); // special char + added to loginId key
270		- if(cached != null) {
271		- return cached;
272		- }
273		- User user = find.byId(id);
274		- if(user != null){
275		- CacheStore.userMap.putIfAbsent(Long.toString(id), user);
276		- }
277		- return user;
278		- }
279		-
280	258	/**
281	259	* find a user by login id string
282	260	*
...	...	@@ -287,16 +265,11 @@
287	265	* @return User or {@link #anonymous}
288	266	*/
289	267	public static User findByLoginId(String loginId) {
290		- User cached = CacheStore.userMap.get("+" + loginId); // special char + added to loginId key
291		- if(cached != null) {
292		- return cached;
293		- }
294	268	User user = find.where().eq("loginId", loginId).findUnique();
295	269	if (user == null) {
296	270	return anonymous;
297	271	}
298	272	else {
299		- CacheStore.userMap.putIfAbsent("+" + loginId, user);
300	273	return user;
301	274	}
302	275	}

...	...	@@ -133,7 +133,7 @@
133	133	resource = Posting.finder.byId(longId).asResource();
134	134	break;
135	135	case USER:
136		- resource = User.findByIdUsingCache(longId).asResource();
	136	+ resource = User.find.byId(longId).asResource();
137	137	break;
138	138	case PROJECT:
139	139	resource = Project.find.byId(longId).asResource();
...	...	@@ -150,7 +150,7 @@
150	150	case PULL_REQUEST:
151	151	return PullRequest.finder.byId(longId).asResource();
152	152	case USER_AVATAR:
153		- return User.findByIdUsingCache(longId).avatarAsResource();
	153	+ return User.find.byId(longId).avatarAsResource();
154	154	case REVIEW_COMMENT:
155	155	return ReviewComment.find.byId(longId).asResource();
156	156	case ORGANIZATION:

...	...	@@ -2,7 +2,6 @@
2	2
3	3	import com.google.common.cache.Cache;
4	4	import com.google.common.cache.CacheBuilder;
5		-import models.User;
6	5
7	6	import java.util.Map;
8	7	import java.util.concurrent.ConcurrentHashMap;
...	...	@@ -11,8 +10,7 @@
11	10	* CacheStore
12	11	*/
13	12	public class CacheStore {
14		- public static Map<String, User> sessionMap = new ConcurrentHashMap<>();
15		- public static Map<String, User> userMap = new ConcurrentHashMap<>();
	13	+ public static Map<String, Long> sessionMap = new ConcurrentHashMap<>();
16	14	public static Map<String, Long> projectMap = new ConcurrentHashMap<>();
17	15	public static final int MAXIMUM_CACHED_MARKDOWN_ENTRY = 10000;
18	16

Delete comment