feat: Identity API response caching#513
feat: Identity API response caching#513Mansi-mParticle wants to merge 12 commits intodevelopmentfrom
Conversation
android-core/src/androidTest/kotlin/com.mparticle/identity/IdentityApiTest.kt
Outdated
Show resolved
Hide resolved
android-core/src/main/java/com/mparticle/identity/IdentityApi.java
Outdated
Show resolved
Hide resolved
android-core/src/main/java/com/mparticle/identity/IdentityApiRequest.java
Outdated
Show resolved
Hide resolved
android-core/src/main/java/com/mparticle/identity/MParticleIdentityClientImpl.java
Outdated
Show resolved
Hide resolved
fa9b79b to
e5c30d2
Compare
| import java.security.MessageDigest; | ||
| import java.security.NoSuchAlgorithmException; |
There was a problem hiding this comment.
| import java.security.MessageDigest; | |
| import java.security.NoSuchAlgorithmException; |
Looks like these imports aren't required
| } | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
Also if we removed this extra line change, the whole file will be removed from the PR as it's not actually being changed
| connection = makeUrlRequest(Endpoint.IDENTITY, connection, jsonObject.toString(), false); | ||
| int responseCode = connection.getResponseCode(); | ||
| try { | ||
| maxAgeTime = Long.valueOf(connection.getHeaderField(IDENTITY_HEADER_TIMEOUT)); |
There was a problem hiding this comment.
| maxAgeTime = Long.valueOf(connection.getHeaderField(IDENTITY_HEADER_TIMEOUT)); | |
| maxAgeTime = Long.valueOf(connection.getHeaderField(IDENTITY_HEADER_TIMEOUT)); |
Nitpick, but extra space here
| try { | ||
| maxAgeTime = Long.valueOf(connection.getHeaderField(IDENTITY_HEADER_TIMEOUT)); | ||
| maxAgeTimeForIdentityCache = maxAgeTime; | ||
| }catch (Exception e){ |
There was a problem hiding this comment.
| }catch (Exception e){ | |
| } catch (Exception e) { |
Nitpick, but missing spaces here
| maxAgeTime = Long.valueOf(connection.getHeaderField(IDENTITY_HEADER_TIMEOUT)); | ||
| maxAgeTimeForIdentityCache = maxAgeTime; | ||
| }catch (Exception e){ | ||
|
|
There was a problem hiding this comment.
Should we at least print something to console if the header field is missing? It may help with debugging.
| private Long maxAgeTimeForIdentityCache = 0L; | ||
| private Long maxAgeTime = 86400L; | ||
| Long identityCacheTime = 0L; | ||
| HashMap<String, IdentityHttpResponse> identityCacheArray = new HashMap<>(); |
There was a problem hiding this comment.
identityCacheArray should be called identityCacheMap
| private static synchronized JSONArray getIdentityCache() { | ||
| String json = sPreferences.getString(Constants.PrefKeys.IDENTITY_API_REQUEST, null); | ||
| if (json != null) { | ||
| try { | ||
| JSONArray jsonArray = new JSONArray(json); | ||
| return jsonArray; | ||
| } catch (JSONException e) { | ||
| Logger.error("Failed to fetch identity cache from storage : " + e.getMessage()); | ||
| } | ||
| } | ||
| return new JSONArray(); | ||
| } | ||
|
|
||
| public synchronized HashMap<String, IdentityHttpResponse> fetchIdentityCache() { | ||
| try { | ||
| JSONArray jsonArray = getIdentityCache(); | ||
| HashMap<String, IdentityHttpResponse> identityCache = new HashMap<>(); | ||
| for (int i = 0; i < jsonArray.length(); i++) { | ||
| JSONObject jsonObject = jsonArray.getJSONObject(i); | ||
| String key = jsonObject.keys().next(); | ||
| JSONObject identityJson = jsonObject.getJSONObject(key); | ||
| IdentityHttpResponse response = IdentityHttpResponse.fromJson(identityJson); | ||
|
|
||
| identityCache.put(key, response); | ||
| } | ||
| return identityCache; | ||
| } catch (Exception e) { | ||
| Logger.error("Error while fetching identity cache: " + e.getMessage()); | ||
| } | ||
|
|
||
| return new HashMap<String, IdentityHttpResponse>(); | ||
| } | ||
| public synchronized void saveIdentityCache(String key, IdentityHttpResponse identityHttpResponse) throws JSONException { | ||
| JSONArray jsonArray = new JSONArray(); | ||
| JSONArray identityCacheExist = getIdentityCache(); | ||
| try { | ||
|
|
||
| if (identityCacheExist != null && identityCacheExist.length() > 0) { | ||
| for (int i = 0; i < identityCacheExist.length(); i++) { | ||
| jsonArray.put(identityCacheExist.get(i)); | ||
| } | ||
| } | ||
| } catch (Exception e) { | ||
| Logger.error("Error while storing identity cache: " + e.getMessage()); | ||
|
|
||
| } | ||
|
|
||
| JSONObject jsonObject = new JSONObject(); | ||
| jsonObject.put(key, identityHttpResponse.toJson()); | ||
| jsonArray.put(jsonObject); | ||
|
|
||
| sPreferences.edit().putString(Constants.PrefKeys.IDENTITY_API_REQUEST, jsonArray.toString()).apply(); | ||
| } |
There was a problem hiding this comment.
| private static synchronized JSONArray getIdentityCache() { | |
| String json = sPreferences.getString(Constants.PrefKeys.IDENTITY_API_REQUEST, null); | |
| if (json != null) { | |
| try { | |
| JSONArray jsonArray = new JSONArray(json); | |
| return jsonArray; | |
| } catch (JSONException e) { | |
| Logger.error("Failed to fetch identity cache from storage : " + e.getMessage()); | |
| } | |
| } | |
| return new JSONArray(); | |
| } | |
| public synchronized HashMap<String, IdentityHttpResponse> fetchIdentityCache() { | |
| try { | |
| JSONArray jsonArray = getIdentityCache(); | |
| HashMap<String, IdentityHttpResponse> identityCache = new HashMap<>(); | |
| for (int i = 0; i < jsonArray.length(); i++) { | |
| JSONObject jsonObject = jsonArray.getJSONObject(i); | |
| String key = jsonObject.keys().next(); | |
| JSONObject identityJson = jsonObject.getJSONObject(key); | |
| IdentityHttpResponse response = IdentityHttpResponse.fromJson(identityJson); | |
| identityCache.put(key, response); | |
| } | |
| return identityCache; | |
| } catch (Exception e) { | |
| Logger.error("Error while fetching identity cache: " + e.getMessage()); | |
| } | |
| return new HashMap<String, IdentityHttpResponse>(); | |
| } | |
| public synchronized void saveIdentityCache(String key, IdentityHttpResponse identityHttpResponse) throws JSONException { | |
| JSONArray jsonArray = new JSONArray(); | |
| JSONArray identityCacheExist = getIdentityCache(); | |
| try { | |
| if (identityCacheExist != null && identityCacheExist.length() > 0) { | |
| for (int i = 0; i < identityCacheExist.length(); i++) { | |
| jsonArray.put(identityCacheExist.get(i)); | |
| } | |
| } | |
| } catch (Exception e) { | |
| Logger.error("Error while storing identity cache: " + e.getMessage()); | |
| } | |
| JSONObject jsonObject = new JSONObject(); | |
| jsonObject.put(key, identityHttpResponse.toJson()); | |
| jsonArray.put(jsonObject); | |
| sPreferences.edit().putString(Constants.PrefKeys.IDENTITY_API_REQUEST, jsonArray.toString()).apply(); | |
| } | |
| private static synchronized JSONObject getIdentityCache() { | |
| String json = sPreferences.getString(Constants.PrefKeys.IDENTITY_API_REQUEST, null); | |
| if (json != null) { | |
| try { | |
| return new JSONObject(json); | |
| } catch (JSONException e) { | |
| Logger.error("Failed to fetch identity cache from storage : " + e.getMessage()); | |
| } | |
| } | |
| return new JSONObject(); | |
| } | |
| public synchronized HashMap<String, IdentityHttpResponse> fetchIdentityCache() { | |
| try { | |
| JSONObject jsonObject = getIdentityCache(); | |
| HashMap<String, IdentityHttpResponse> identityCache = new HashMap<>(); | |
| for (Iterator<String> it = jsonObject.keys(); it.hasNext();) { | |
| String key = it.next(); | |
| JSONObject json = jsonObject.getJSONObject(key); | |
| IdentityHttpResponse response = IdentityHttpResponse.fromJson(json); | |
| identityCache.put(key, response); | |
| } | |
| return identityCache; | |
| } catch (Exception e) { | |
| Logger.error("Error while fetching identity cache: " + e.getMessage()); | |
| } | |
| return new HashMap<String, IdentityHttpResponse>(); | |
| } | |
| public synchronized void addToIdentityCache(String key, IdentityHttpResponse identityHttpResponse) throws JSONException { | |
| JSONObject identityCache = getIdentityCache(); | |
| try { | |
| identityCache.put(key, identityHttpResponse.toJson()); | |
| } catch (Exception e) { | |
| Logger.error("Error while adding to identity cache: " + e.getMessage()); | |
| } | |
| sPreferences.edit().putString(Constants.PrefKeys.IDENTITY_API_REQUEST, identityCache.toString()).apply(); | |
| } | |
| public synchronized void removeFromIdentityCache(String key) throws JSONException { | |
| JSONObject identityCache = getIdentityCache(); | |
| try { | |
| identityCache.remove(key); | |
| } catch (Exception e) { | |
| Logger.error("Error while removing from identity cache: " + e.getMessage()); | |
| } | |
| sPreferences.edit().putString(Constants.PrefKeys.IDENTITY_API_REQUEST, identityCache.toString()).apply(); | |
| } | |
| public synchronized void pruneIdentityCache() { | |
| try { | |
| JSONObject oldIdentityCache = getIdentityCache(); | |
| JSONObject newIdentityCache = new JSONObject(); | |
| for (Iterator<String> it = oldIdentityCache.keys(); it.hasNext(); ) { | |
| String key = it.next(); | |
| JSONObject json = oldIdentityCache.getJSONObject(key); | |
| IdentityHttpResponse response = IdentityHttpResponse.fromJson(json); | |
| if (response.getCacheExpirationMillis() > System.currentTimeMillis()) { | |
| newIdentityCache.put(key, response); | |
| } | |
| } | |
| sPreferences.edit().putString(Constants.PrefKeys.IDENTITY_API_REQUEST, newIdentityCache.toString()).apply(); | |
| } catch (Exception e) { | |
| Logger.error("Error while pruning identity cache: " + e.getMessage()); | |
| clearIdentityCatch(); | |
| } | |
| } |
This is the idea I was talking about on the Zoom call. Since a JSONObject is essentially just a map, there's no need to place the JSONObjects inside a JSONArray. Also the key lookups will be much faster this way since there's no need to scan the whole array.
Note I haven't tested this code other than confirming the compiler doesn't complain.
Also I added a prune method that needs to be called at some point, maybe when we do other clean up tasks? On iOS we call our prune method when we enter the background and do database and other cleanup.
| public void saveIdentityCacheTime(long time) { | ||
| sPreferences.edit().putLong(Constants.PrefKeys.IDENTITY_API_CACHE_TIME, time).apply(); | ||
| } | ||
|
|
||
| public void saveIdentityMaxAge(long time) { | ||
| sPreferences.edit().putLong(Constants.PrefKeys.IDENTITY_MAX_AGE, time).apply(); | ||
| } | ||
|
|
||
| public synchronized Long getIdentityCacheTime() { | ||
| return sPreferences.getLong(Constants.PrefKeys.IDENTITY_API_CACHE_TIME, 0); | ||
| } | ||
|
|
||
| public Long getIdentityMaxAge() { | ||
| return sPreferences.getLong(Constants.PrefKeys.IDENTITY_MAX_AGE, 0); | ||
| } | ||
|
|
||
| public void clearIdentityCatch() { | ||
| sPreferences.edit() | ||
| .remove(Constants.PrefKeys.IDENTITY_API_REQUEST).apply(); | ||
| sPreferences.edit() | ||
| .remove(Constants.PrefKeys.IDENTITY_API_CACHE_TIME).apply(); | ||
| sPreferences.edit() | ||
| .remove(Constants.PrefKeys.IDENTITY_MAX_AGE).apply(); | ||
| } |
There was a problem hiding this comment.
| public void saveIdentityCacheTime(long time) { | |
| sPreferences.edit().putLong(Constants.PrefKeys.IDENTITY_API_CACHE_TIME, time).apply(); | |
| } | |
| public void saveIdentityMaxAge(long time) { | |
| sPreferences.edit().putLong(Constants.PrefKeys.IDENTITY_MAX_AGE, time).apply(); | |
| } | |
| public synchronized Long getIdentityCacheTime() { | |
| return sPreferences.getLong(Constants.PrefKeys.IDENTITY_API_CACHE_TIME, 0); | |
| } | |
| public Long getIdentityMaxAge() { | |
| return sPreferences.getLong(Constants.PrefKeys.IDENTITY_MAX_AGE, 0); | |
| } | |
| public void clearIdentityCatch() { | |
| sPreferences.edit() | |
| .remove(Constants.PrefKeys.IDENTITY_API_REQUEST).apply(); | |
| sPreferences.edit() | |
| .remove(Constants.PrefKeys.IDENTITY_API_CACHE_TIME).apply(); | |
| sPreferences.edit() | |
| .remove(Constants.PrefKeys.IDENTITY_MAX_AGE).apply(); | |
| } | |
| public void clearIdentityCatch() { | |
| sPreferences.edit() | |
| .remove(Constants.PrefKeys.IDENTITY_API_REQUEST).apply(); | |
| } |
Then there's no need to store the max age or cache time since expiration timestamps are included with the cached responses.
| } | ||
| return builder.toString(); | ||
| } | ||
|
|
There was a problem hiding this comment.
| public long getCacheExpirationMillis() { | |
| return cacheExpirationMillis; | |
| } | |
| public void setCacheExpirationMillis(long expirationMillis) { | |
| cacheExpirationMillis = expirationMillis; | |
| } |
If we add a cache expiration property to this class, we can use that to determine cache validity without needing a wrapper object.
| private IdentityHttpResponse checkIfExists(IdentityApiRequest request, String callType) { | ||
| if (mConfigManager.isIdentityCacheFlagEnabled()) { | ||
| try { | ||
| String key = request.objectToHash() + callType; | ||
| if (identityCacheTime <= 0L) { | ||
| identityCacheTime = mConfigManager.getIdentityCacheTime(); | ||
| } | ||
| if (maxAgeTimeForIdentityCache <= 0L) { | ||
| maxAgeTimeForIdentityCache = mConfigManager.getIdentityMaxAge(); | ||
| } | ||
| if (identityCacheArray.isEmpty()) { | ||
| identityCacheArray = mConfigManager.fetchIdentityCache(); | ||
| } | ||
| if ((((System.currentTimeMillis() - identityCacheTime) / 1000) <= maxAgeTimeForIdentityCache) && identityCacheArray.containsKey(key)) { | ||
| return identityCacheArray.get(key); | ||
| } else { | ||
| return null; | ||
|
|
||
| } | ||
| } catch (Exception e) { | ||
| Logger.error("Exception " + e); | ||
|
|
||
| } | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
| private IdentityHttpResponse checkIfExists(IdentityApiRequest request, String callType) { | |
| if (mConfigManager.isIdentityCacheFlagEnabled()) { | |
| try { | |
| String key = request.objectToHash() + callType; | |
| if (identityCacheTime <= 0L) { | |
| identityCacheTime = mConfigManager.getIdentityCacheTime(); | |
| } | |
| if (maxAgeTimeForIdentityCache <= 0L) { | |
| maxAgeTimeForIdentityCache = mConfigManager.getIdentityMaxAge(); | |
| } | |
| if (identityCacheArray.isEmpty()) { | |
| identityCacheArray = mConfigManager.fetchIdentityCache(); | |
| } | |
| if ((((System.currentTimeMillis() - identityCacheTime) / 1000) <= maxAgeTimeForIdentityCache) && identityCacheArray.containsKey(key)) { | |
| return identityCacheArray.get(key); | |
| } else { | |
| return null; | |
| } | |
| } catch (Exception e) { | |
| Logger.error("Exception " + e); | |
| } | |
| } | |
| return null; | |
| } | |
| private IdentityHttpResponse checkIfExists(IdentityApiRequest request, String callType) { | |
| if (!mConfigManager.isIdentityCacheFlagEnabled()) { | |
| return null; | |
| } | |
| try { | |
| String key = identityCacheKey(request, callType); | |
| if (key == null) { | |
| return null; | |
| } | |
| if (identityCacheMap.isEmpty()) { | |
| identityCacheMap = mConfigManager.fetchIdentityCache(); | |
| } | |
| IdentityHttpResponse cachedResponse = identityCacheMap.get(key); | |
| if (cachedResponse != null) { | |
| if (cachedResponse.getCacheExpirationMillis() > System.currentTimeMillis()) { | |
| return cachedResponse; | |
| } else { | |
| // Expired, so remove from cache | |
| mConfigManager.removeFromIdentityCache(key); | |
| } | |
| } | |
| } catch (Exception e) { | |
| Logger.error("Exception " + e); | |
| } | |
| return null; | |
| } |
Using the suggestion I made in the ConfigManager class, this method would look something like the above.
73f2388 to
5910509
Compare
Instructions
developmentSummary
Testing Plan
1)Login with same user on same session and on cold launch.
2)Login with two different user on same session and on cold launch.
3)Call Identity with same details as login details.
4)Call Identity with same user details on same session and on cold launch.
Reference Issue (For mParticle employees only. Ignore if you are an outside contributor)