feat: Identity API response caching #513

Mansi-mParticle · 2024-10-08T02:16:12Z

Instructions

PR target branch should be against development
PR title name should follow this format: https://github.com/mParticle/mparticle-workflows/blob/main/.github/workflows/pr-title-check.yml
PR branch prefix should follow this format: https://github.com/mParticle/mparticle-workflows/blob/main/.github/workflows/pr-branch-check-name.yml

Summary

Adds caching of identify and login calls and Clears that cache on modify and logout calls.

Testing Plan

Was this tested locally? If not, explain why.
Tested with sample application. And currently Executed below Test cases:
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)

Closes https://go.mparticle.com/work/SQDSDKS-6673

einsteinx2

Other than the commented out asserts, the added tests (SQDSDKS-6674) looks good to me

android-core/src/androidTest/kotlin/com.mparticle/identity/IdentityApiTest.kt

android-core/src/main/java/com/mparticle/identity/IdentityApi.java

android-core/src/main/java/com/mparticle/identity/IdentityApiRequest.java

android-core/src/main/java/com/mparticle/identity/MParticleIdentityClientImpl.java

…673_identity_caching_2

einsteinx2 · 2024-12-06T19:22:01Z

android-core/src/main/java/com/mparticle/identity/IdentityApi.java

+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;


Suggested change

import java.security.MessageDigest;

import java.security.NoSuchAlgorithmException;

Looks like these imports aren't required

einsteinx2 · 2024-12-06T19:22:42Z

android-core/src/main/java/com/mparticle/identity/IdentityApi.java

@@ -348,6 +350,7 @@ private void reset() {
        }
    }

+


Suggested change

Also if we removed this extra line change, the whole file will be removed from the PR as it's not actually being changed

einsteinx2 · 2024-12-06T19:25:48Z

android-core/src/main/java/com/mparticle/identity/MParticleIdentityClientImpl.java

        Logger.verbose("Identity login request: " + jsonObject.toString());
        MPConnection connection = getPostConnection(LOGIN_PATH, jsonObject.toString());
        String url = connection.getURL().toString();
        InternalListenerManager.getListener().onNetworkRequestStarted(SdkListener.Endpoint.IDENTITY_LOGIN, url, jsonObject, request);
        connection = makeUrlRequest(Endpoint.IDENTITY, connection, jsonObject.toString(), false);
        int responseCode = connection.getResponseCode();
+        try {
+            maxAgeTime  = Long.valueOf(connection.getHeaderField(IDENTITY_HEADER_TIMEOUT));


Suggested change

maxAgeTime = Long.valueOf(connection.getHeaderField(IDENTITY_HEADER_TIMEOUT));

maxAgeTime = Long.valueOf(connection.getHeaderField(IDENTITY_HEADER_TIMEOUT));

Nitpick, but extra space here

einsteinx2 · 2024-12-06T19:26:02Z

android-core/src/main/java/com/mparticle/identity/MParticleIdentityClientImpl.java

+        try {
+            maxAgeTime  = Long.valueOf(connection.getHeaderField(IDENTITY_HEADER_TIMEOUT));
+            maxAgeTimeForIdentityCache = maxAgeTime;
+        }catch (Exception e){


Suggested change

}catch (Exception e){

} catch (Exception e) {

Nitpick, but missing spaces here

einsteinx2 · 2024-12-06T19:26:41Z

android-core/src/main/java/com/mparticle/identity/MParticleIdentityClientImpl.java

+            maxAgeTime  = Long.valueOf(connection.getHeaderField(IDENTITY_HEADER_TIMEOUT));
+            maxAgeTimeForIdentityCache = maxAgeTime;
+        }catch (Exception e){
+


Should we at least print something to console if the header field is missing? It may help with debugging.

einsteinx2 · 2024-12-06T19:33:31Z

android-core/src/main/java/com/mparticle/identity/MParticleIdentityClientImpl.java

+    private Long maxAgeTimeForIdentityCache = 0L;
+    private Long maxAgeTime = 86400L;
+     Long identityCacheTime = 0L;
+    HashMap<String, IdentityHttpResponse> identityCacheArray = new HashMap<>();


identityCacheArray should be called identityCacheMap

einsteinx2 · 2024-12-06T20:25:09Z

android-core/src/main/java/com/mparticle/internal/ConfigManager.java

+    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();
+    }


Suggested change

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.

einsteinx2 · 2024-12-06T20:26:19Z

android-core/src/main/java/com/mparticle/internal/ConfigManager.java

+    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();
+    }


Suggested change

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.

einsteinx2 · 2024-12-06T20:29:21Z

android-core/src/main/java/com/mparticle/identity/IdentityHttpResponse.java

@@ -135,4 +135,31 @@ public String toString() {
        }
        return builder.toString();
    }
+


Suggested change

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.

einsteinx2 · 2024-12-11T15:12:27Z

android-core/src/main/java/com/mparticle/identity/MParticleIdentityClientImpl.java

+    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;
+    }


Suggested change

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.

Mansi-mParticle added 2 commits October 7, 2024 22:13

feat: Identity API response caching

b3b63f7

Add Remaining unit test cases

529c725

Mansi-mParticle requested review from rmi22186 and einsteinx2 October 8, 2024 16:58

einsteinx2 requested changes Oct 16, 2024

View reviewed changes

android-core/src/androidTest/kotlin/com.mparticle/identity/IdentityApiTest.kt Outdated Show resolved Hide resolved

Mansi-mParticle and others added 6 commits October 16, 2024 11:55

Removed other testcases

983175f

refactor: Migrate Internal OS Helper class to kotlin (#515)

efad64c

refactor: Migrate Internal user storage class to kotlin (#516)

a8efd13

refactor: Migrate Internal CoreCallback class to kotlin (#517)

b6ab438

refactor: Migrate Internal Application context class to kotlin (#511)

268d850

fix: Testcase related to code refactor (#519)

669ae88

einsteinx2 requested changes Nov 13, 2024

View reviewed changes

Mansi-mParticle and others added 3 commits November 13, 2024 19:31

fix: previous Google ad Id from being null on cold launch (#518)

fa9b79b

Merge remote-tracking branch 'origin/development' into feat/SQDSDKS-6…

4eb7566

…673_identity_caching_2

Address review comments

2e832ca

einsteinx2 force-pushed the development branch from fa9b79b to e5c30d2 Compare November 15, 2024 19:27

Address review comments and change algorithm logic

9d432bd

Mansi-mParticle requested a review from einsteinx2 November 26, 2024 22:05

einsteinx2 requested changes Dec 11, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Identity API response caching #513

feat: Identity API response caching #513

Mansi-mParticle commented Oct 8, 2024

einsteinx2 left a comment •

edited

Loading

einsteinx2 Dec 6, 2024

einsteinx2 Dec 6, 2024

einsteinx2 Dec 6, 2024

einsteinx2 Dec 6, 2024

einsteinx2 Dec 6, 2024

einsteinx2 Dec 6, 2024

einsteinx2 Dec 6, 2024

einsteinx2 Dec 6, 2024

einsteinx2 Dec 6, 2024

einsteinx2 Dec 11, 2024

		import java.security.MessageDigest;
		import java.security.NoSuchAlgorithmException;

	maxAgeTime = Long.valueOf(connection.getHeaderField(IDENTITY_HEADER_TIMEOUT));
	maxAgeTime = Long.valueOf(connection.getHeaderField(IDENTITY_HEADER_TIMEOUT));

+    public long getCacheExpirationMillis() {
+        return cacheExpirationMillis;
+    }
+    public void setCacheExpirationMillis(long expirationMillis) {
+        cacheExpirationMillis = expirationMillis;
+    }

feat: Identity API response caching #513

Are you sure you want to change the base?

feat: Identity API response caching #513

Conversation

Mansi-mParticle commented Oct 8, 2024

Instructions

Summary

Testing Plan

Reference Issue (For mParticle employees only. Ignore if you are an outside contributor)

einsteinx2 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

einsteinx2 left a comment •

edited

Loading