From a4e843bf71454a8328534c410989a1734c44fcbc Mon Sep 17 00:00:00 2001 From: Vignesh Raja Date: Tue, 17 Jan 2017 20:52:36 -0800 Subject: [PATCH] Change key to ID in all variable names within UserProfile (#76) --- .../user_profile/AndroidUserProfileTest.java | 96 ++++++++-------- .../WriteThroughCacheFactoryTest.java | 48 ++++---- .../user_profile/AndroidUserProfile.java | 104 +++++++++--------- 3 files changed, 124 insertions(+), 124 deletions(-) diff --git a/user-profile/src/androidTest/java/com/optimizely/ab/android/user_profile/AndroidUserProfileTest.java b/user-profile/src/androidTest/java/com/optimizely/ab/android/user_profile/AndroidUserProfileTest.java index db4cef3e6..c8c1a19d2 100644 --- a/user-profile/src/androidTest/java/com/optimizely/ab/android/user_profile/AndroidUserProfileTest.java +++ b/user-profile/src/androidTest/java/com/optimizely/ab/android/user_profile/AndroidUserProfileTest.java @@ -80,131 +80,131 @@ public void teardown() { @Test public void saveActivation() { String userId = "user1"; - String expKey = "exp1"; - String varKey = "var1"; - assertTrue(androidUserProfile.save(userId, expKey, varKey)); + String expId = "1"; + String varId = "2"; + assertTrue(androidUserProfile.save(userId, expId, varId)); try { executor.awaitTermination(5, TimeUnit.SECONDS); } catch (InterruptedException e) { fail("Time out"); } - assertEquals(varKey, androidUserProfile.lookup(userId, expKey)); + assertEquals(varId, androidUserProfile.lookup(userId, expId)); } @Test public void saveActivationNullUserId() { - assertFalse(androidUserProfile.save(null, "exp1", "var1")); - verify(logger).error("Received null userId, unable to save activation"); + assertFalse(androidUserProfile.save(null, "1", "2")); + verify(logger).error("Received null userId, unable to save activation."); } @Test - public void saveActivationNullExperimentKey() { - assertFalse(androidUserProfile.save("foo", null, "var1")); - verify(logger).error("Received null experiment key, unable to save activation"); + public void saveActivationNullExperimentId() { + assertFalse(androidUserProfile.save("foo", null, "1")); + verify(logger).error("Received null experiment ID, unable to save activation."); } @Test - public void saveActivationNullVariationKey() { - assertFalse(androidUserProfile.save("foo", "exp1", null)); - verify(logger).error("Received null variation key, unable to save activation"); + public void saveActivationNullVariationId() { + assertFalse(androidUserProfile.save("foo", "1", null)); + verify(logger).error("Received null variation ID, unable to save activation."); } @Test public void saveActivationEmptyUserId() { - assertFalse(androidUserProfile.save("", "exp1", "var1")); - verify(logger).error("Received empty user id, unable to save activation"); + assertFalse(androidUserProfile.save("", "1", "2")); + verify(logger).error("Received empty user ID, unable to save activation."); } @Test - public void saveActivationEmptyExperimentKey() { - assertFalse(androidUserProfile.save("foo", "", "var1")); - verify(logger).error("Received empty experiment key, unable to save activation"); + public void saveActivationEmptyExperimentId() { + assertFalse(androidUserProfile.save("foo", "", "1")); + verify(logger).error("Received empty experiment ID, unable to save activation."); } @Test - public void saveActivationEmptyVariationKey() { - assertFalse(androidUserProfile.save("foo", "exp1", "")); - verify(logger).error("Received empty variation key, unable to save activation"); + public void saveActivationEmptyVariationId() { + assertFalse(androidUserProfile.save("foo", "1", "")); + verify(logger).error("Received empty variation ID, unable to save activation."); } @Test public void lookupActivationNullUserId() { - assertNull(androidUserProfile.lookup(null, "exp1")); - verify(logger).error("Received null user id, unable to lookup activation"); + assertNull(androidUserProfile.lookup(null, "1")); + verify(logger).error("Received null user ID, unable to lookup activation."); } @Test - public void lookupActivationNullExperimentKey() { - assertNull(androidUserProfile.lookup("foo", null)); - verify(logger).error("Received null experiment key, unable to lookup activation"); + public void lookupActivationNullExperimentId() { + assertNull(androidUserProfile.lookup("1", null)); + verify(logger).error("Received null experiment ID, unable to lookup activation."); } @Test public void lookupActivationEmptyUserId() { - assertNull(androidUserProfile.lookup("", "exp1")); - verify(logger).error("Received empty user id, unable to lookup activation"); + assertNull(androidUserProfile.lookup("", "1")); + verify(logger).error("Received empty user ID, unable to lookup activation."); } @Test - public void lookupActivationEmptyExperimentKey() { + public void lookupActivationEmptyExperimentId() { assertNull(androidUserProfile.lookup("foo", "")); - verify(logger).error("Received empty experiment key, unable to lookup activation"); + verify(logger).error("Received empty experiment ID, unable to lookup activation."); } @Test public void removeExistingActivation() { - androidUserProfile.save("user1", "exp1", "var1"); - assertTrue(androidUserProfile.remove("user1", "exp1")); - assertNull(androidUserProfile.lookup("user1", "exp1")); + androidUserProfile.save("user1", "1", "2"); + assertTrue(androidUserProfile.remove("user1", "1")); + assertNull(androidUserProfile.lookup("user1", "1")); } @Test public void removeNonExistingActivation() { - assertFalse(androidUserProfile.remove("user1", "exp1")); + assertFalse(androidUserProfile.remove("user1", "1")); } @Test public void removeActivationNullUserId() { - assertFalse(androidUserProfile.remove(null, "exp1")); - verify(logger).error("Received null user id, unable to remove activation"); + assertFalse(androidUserProfile.remove(null, "1")); + verify(logger).error("Received null user ID, unable to remove activation."); } @Test - public void removeActivationNullExperimentKey() { + public void removeActivationNullExperimentId() { assertFalse(androidUserProfile.remove("foo", null)); - verify(logger).error("Received null experiment key, unable to remove activation"); + verify(logger).error("Received null experiment ID, unable to remove activation."); } @Test public void removeActivationEmptyUserId() { - assertFalse(androidUserProfile.remove("", "exp1")); - verify(logger).error("Received empty user id, unable to remove activation"); + assertFalse(androidUserProfile.remove("", "1")); + verify(logger).error("Received empty user ID, unable to remove activation."); } @Test - public void removeActivationEmptyExperimentKey() { + public void removeActivationEmptyExperimentId() { assertFalse(androidUserProfile.remove("foo", "")); - verify(logger).error("Received empty experiment key, unable to remove activation"); + verify(logger).error("Received empty experiment ID, unable to remove activation."); } @Test public void startHandlesJSONException() throws IOException { assertTrue(cache.save(diskUserProfileCache.getFileName(), "{")); androidUserProfile.start(); - verify(logger).error(eq("Unable to parse user profile cache"), any(JSONException.class)); + verify(logger).error(eq("Unable to parse user profile cache."), any(JSONException.class)); } @Test public void start() throws JSONException { androidUserProfile.start(); - androidUserProfile.save("user1", "exp1", "var1"); - androidUserProfile.save("user1", "exp2", "var2"); + androidUserProfile.save("user1", "1", "3"); + androidUserProfile.save("user1", "2", "4"); - Map expKeyToVarKeyMap = new HashMap<>(); - expKeyToVarKeyMap.put("exp1", "var1"); - expKeyToVarKeyMap.put("exp2", "var2"); + Map expIdToVarIdMap = new HashMap<>(); + expIdToVarIdMap.put("1", "3"); + expIdToVarIdMap.put("2", "4"); Map> profileMap = new HashMap<>(); - profileMap.put("user1", expKeyToVarKeyMap); + profileMap.put("user1", expIdToVarIdMap); assertEquals(profileMap, memoryUserProfileCache); } diff --git a/user-profile/src/androidTest/java/com/optimizely/ab/android/user_profile/WriteThroughCacheFactoryTest.java b/user-profile/src/androidTest/java/com/optimizely/ab/android/user_profile/WriteThroughCacheFactoryTest.java index 3bd762242..8c717050d 100644 --- a/user-profile/src/androidTest/java/com/optimizely/ab/android/user_profile/WriteThroughCacheFactoryTest.java +++ b/user-profile/src/androidTest/java/com/optimizely/ab/android/user_profile/WriteThroughCacheFactoryTest.java @@ -69,25 +69,25 @@ public void startWriteCacheTask() throws JSONException { new AndroidUserProfile.WriteThroughCacheTaskFactory(diskUserProfileCache, new HashMap>(), executor, logger); - writeThroughCacheTaskFactory.startWriteCacheTask("user1", "exp1", "var1"); + writeThroughCacheTaskFactory.startWriteCacheTask("user1", "1", "2"); try { executor.awaitTermination(5, TimeUnit.SECONDS); } catch (InterruptedException e) { fail("Timed out"); } - verify(logger).info("Updated in memory user profile"); + verify(logger).info("Updated in memory user profile."); assertTrue(writeThroughCacheTaskFactory.getMemoryUserProfileCache().containsKey("user1")); Map activation = writeThroughCacheTaskFactory.getMemoryUserProfileCache().get("user1"); - assertTrue(activation.containsKey("exp1")); - assertEquals("var1", activation.get("exp1")); + assertTrue(activation.containsKey("1")); + assertEquals("2", activation.get("1")); final JSONObject json = diskUserProfileCache.load(); assertTrue(json.has("user1")); final JSONObject user1 = json.getJSONObject("user1"); - assertTrue(user1.has("exp1")); - assertEquals("var1", user1.getString("exp1")); - verify(logger).info("Persisted user in variation {} for experiment {}.", "var1", "exp1"); + assertTrue(user1.has("1")); + assertEquals("2", user1.getString("1")); + verify(logger).info("Persisted user in variation {} for experiment {}.", "2", "1"); cache.delete(diskUserProfileCache.getFileName()); } @@ -105,19 +105,19 @@ public void startWriteCacheTaskFail() throws JSONException { when(cache.save(diskUserProfileCache.getFileName(), json.toString())).thenReturn(false); - writeThroughCacheTaskFactory.startWriteCacheTask("user1", "exp1", "var1"); + writeThroughCacheTaskFactory.startWriteCacheTask("user1", "1", "2"); try { executor.awaitTermination(5, TimeUnit.SECONDS); } catch (InterruptedException e) { fail("Timed out"); } - verify(logger).info("Updated in memory user profile"); + verify(logger).info("Updated in memory user profile."); assertTrue(writeThroughCacheTaskFactory.getMemoryUserProfileCache().containsKey("user1")); Map activation = writeThroughCacheTaskFactory.getMemoryUserProfileCache().get("user1"); - assertFalse(activation.containsKey("exp1")); + assertFalse(activation.containsKey("1")); - verify(logger).error("Failed to persist user in variation {} for experiment {}.", "var1", "exp1"); + verify(logger).error("Failed to persist user in variation {} for experiment {}.", "2", "1"); } @RequiresApi(api = Build.VERSION_CODES.HONEYCOMB) @@ -129,26 +129,26 @@ public void startRemoveCacheTask() throws JSONException, IOException { new AndroidUserProfile.WriteThroughCacheTaskFactory(diskUserProfileCache, new HashMap>(), executor, logger); - diskUserProfileCache.save("user1", "exp1", "var1"); + diskUserProfileCache.save("user1", "1", "2"); Map activation = new HashMap<>(); - activation.put("exp1", "var1"); + activation.put("1", "2"); writeThroughCacheTaskFactory.getMemoryUserProfileCache().put("user1", activation); - writeThroughCacheTaskFactory.startRemoveCacheTask("user1", "exp1", "var1"); + writeThroughCacheTaskFactory.startRemoveCacheTask("user1", "1", "2"); try { executor.awaitTermination(5, TimeUnit.SECONDS); } catch (InterruptedException e) { fail("Timed out"); } - verify(logger).info("Removed experimentKey: {} variationKey: {} record for user: {} from memory", "exp1", "var1", "user1"); + verify(logger).info("Removed experimentId: {} variationId: {} record for user: {} from memory.", "1", "2", "user1"); JSONObject json = diskUserProfileCache.load(); assertTrue(json.has("user1")); json = json.getJSONObject("user1"); - assertFalse(json.has("exp1")); - assertFalse(writeThroughCacheTaskFactory.getMemoryUserProfileCache().get("user1").containsKey("exp1")); - verify(logger).info("Removed experimentKey: {} variationKey: {} record for user: {} from disk", "exp1", "var1", "user1"); + assertFalse(json.has("1")); + assertFalse(writeThroughCacheTaskFactory.getMemoryUserProfileCache().get("user1").containsKey("1")); + verify(logger).info("Removed experimentId: {} variationId: {} record for user: {} from disk.", "1", "2", "user1"); } @RequiresApi(api = Build.VERSION_CODES.HONEYCOMB) @@ -163,25 +163,25 @@ public void startRemoveCacheTaskFail() throws JSONException, IOException { when(cache.save(diskUserProfileCache.getFileName(), getJsonObject().toString())).thenReturn(false); Map activation = new HashMap<>(); - activation.put("exp1", "var1"); + activation.put("1", "2"); writeThroughCacheTaskFactory.getMemoryUserProfileCache().put("user1", activation); - writeThroughCacheTaskFactory.startRemoveCacheTask("user1", "exp1", "var1"); + writeThroughCacheTaskFactory.startRemoveCacheTask("user1", "1", "2"); try { executor.awaitTermination(5, TimeUnit.SECONDS); } catch (InterruptedException e) { fail("Timed out"); } - verify(logger).info("Removed experimentKey: {} variationKey: {} record for user: {} from memory", "exp1", "var1", "user1"); - verify(logger).error("Restored experimentKey: {} variationKey: {} record for user: {} to memory", "exp1", "var1","user1"); - assertTrue(writeThroughCacheTaskFactory.getMemoryUserProfileCache().get("user1").containsKey("exp1")); + verify(logger).info("Removed experimentId: {} variationId: {} record for user: {} from memory.", "1", "2", "user1"); + verify(logger).error("Restored experimentId: {} variationId: {} record for user: {} to memory.", "1", "2","user1"); + assertTrue(writeThroughCacheTaskFactory.getMemoryUserProfileCache().get("user1").containsKey("1")); } @NonNull private JSONObject getJsonObject() throws JSONException { JSONObject json = new JSONObject(); JSONObject user1 = new JSONObject(); - user1.put("exp1", "var1"); + user1.put("1", "2"); json.put("user1", user1); return json; } diff --git a/user-profile/src/main/java/com/optimizely/ab/android/user_profile/AndroidUserProfile.java b/user-profile/src/main/java/com/optimizely/ab/android/user_profile/AndroidUserProfile.java index 2b9872923..a4b2448f3 100644 --- a/user-profile/src/main/java/com/optimizely/ab/android/user_profile/AndroidUserProfile.java +++ b/user-profile/src/main/java/com/optimizely/ab/android/user_profile/AndroidUserProfile.java @@ -53,8 +53,8 @@ public class AndroidUserProfile implements UserProfile { @NonNull private final WriteThroughCacheTaskFactory writeThroughCacheTaskFactory; AndroidUserProfile(@NonNull UserProfileCache diskUserProfileCache, - @NonNull WriteThroughCacheTaskFactory writeThroughCacheTaskFactory, - @NonNull Logger logger) { + @NonNull WriteThroughCacheTaskFactory writeThroughCacheTaskFactory, + @NonNull Logger logger) { this.diskUserProfileCache = diskUserProfileCache; this.writeThroughCacheTaskFactory = writeThroughCacheTaskFactory; this.logger = logger; @@ -105,7 +105,7 @@ public void start() { } } } catch (JSONException e) { - logger.error("Unable to parse user profile cache", e); + logger.error("Unable to parse user profile cache.", e); } } @@ -114,28 +114,28 @@ public void start() { */ @RequiresApi(api = Build.VERSION_CODES.HONEYCOMB) @Override - public boolean save(final String userId, final String experimentKey, final String variationKey) { + public boolean save(final String userId, final String experimentId, final String variationId) { if (userId == null) { - logger.error("Received null userId, unable to save activation"); + logger.error("Received null userId, unable to save activation."); return false; - } else if (experimentKey == null) { - logger.error("Received null experiment key, unable to save activation"); + } else if (experimentId == null) { + logger.error("Received null experiment ID, unable to save activation."); return false; - } else if (variationKey == null) { - logger.error("Received null variation key, unable to save activation"); + } else if (variationId == null) { + logger.error("Received null variation ID, unable to save activation."); return false; } else if (userId.isEmpty()) { - logger.error("Received empty user id, unable to save activation"); + logger.error("Received empty user ID, unable to save activation."); return false; - } else if (experimentKey.isEmpty()) { - logger.error("Received empty experiment key, unable to save activation"); + } else if (experimentId.isEmpty()) { + logger.error("Received empty experiment ID, unable to save activation."); return false; - } else if (variationKey.isEmpty()) { - logger.error("Received empty variation key, unable to save activation"); + } else if (variationId.isEmpty()) { + logger.error("Received empty variation ID, unable to save activation."); return false; } - writeThroughCacheTaskFactory.startWriteCacheTask(userId, experimentKey, variationKey); + writeThroughCacheTaskFactory.startWriteCacheTask(userId, experimentId, variationId); return true; } @@ -145,28 +145,28 @@ public boolean save(final String userId, final String experimentKey, final Strin */ @Override @Nullable - public String lookup(String userId, String experimentKey) { + public String lookup(String userId, String experimentId) { if (userId == null) { - logger.error("Received null user id, unable to lookup activation"); + logger.error("Received null user ID, unable to lookup activation."); return null; - } else if (experimentKey == null) { - logger.error("Received null experiment key, unable to lookup activation"); + } else if (experimentId == null) { + logger.error("Received null experiment ID, unable to lookup activation."); return null; } else if (userId.isEmpty()) { - logger.error("Received empty user id, unable to lookup activation"); + logger.error("Received empty user ID, unable to lookup activation."); return null; - } else if (experimentKey.isEmpty()) { - logger.error("Received empty experiment key, unable to lookup activation"); + } else if (experimentId.isEmpty()) { + logger.error("Received empty experiment ID, unable to lookup activation."); return null; } Map expIdToVarIdMap = writeThroughCacheTaskFactory.getMemoryUserProfileCache().get(userId); - String variationKey = null; + String variationId = null; if (expIdToVarIdMap != null) { - variationKey = expIdToVarIdMap.get(experimentKey); + variationId = expIdToVarIdMap.get(experimentId); } - return variationKey; + return variationId; } /** @@ -174,27 +174,27 @@ public String lookup(String userId, String experimentKey) { */ @RequiresApi(api = Build.VERSION_CODES.HONEYCOMB) @Override - public boolean remove(final String userId, final String experimentKey) { + public boolean remove(final String userId, final String experimentId) { if (userId == null) { - logger.error("Received null user id, unable to remove activation"); + logger.error("Received null user ID, unable to remove activation."); return false; - } else if (experimentKey == null) { - logger.error("Received null experiment key, unable to remove activation"); + } else if (experimentId == null) { + logger.error("Received null experiment ID, unable to remove activation."); return false; } else if (userId.isEmpty()) { - logger.error("Received empty user id, unable to remove activation"); + logger.error("Received empty user ID, unable to remove activation."); return false; - } else if (experimentKey.isEmpty()) { - logger.error("Received empty experiment key, unable to remove activation"); + } else if (experimentId.isEmpty()) { + logger.error("Received empty experiment ID, unable to remove activation."); return false; } - Map expKeyToVarKeyMap = writeThroughCacheTaskFactory.getMemoryUserProfileCache().get(userId); - if (expKeyToVarKeyMap == null) { + Map expIdToVarIdMap = writeThroughCacheTaskFactory.getMemoryUserProfileCache().get(userId); + if (expIdToVarIdMap == null) { return false; } - if (expKeyToVarKeyMap.containsKey(experimentKey)) { // Don't do anything if the mapping doesn't exist - writeThroughCacheTaskFactory.startRemoveCacheTask(userId, experimentKey, expKeyToVarKeyMap.get(experimentKey)); + if (expIdToVarIdMap.containsKey(experimentId)) { // Don't do anything if the mapping doesn't exist + writeThroughCacheTaskFactory.startRemoveCacheTask(userId, experimentId, expIdToVarIdMap.get(experimentId)); } return true; @@ -227,11 +227,11 @@ Map> getMemoryUserProfileCache() { } @RequiresApi(api = Build.VERSION_CODES.HONEYCOMB) - void startWriteCacheTask(final String userId, final String experimentKey, final String variationKey) { + void startWriteCacheTask(final String userId, final String experimentId, final String variationId) { AsyncTask task = new AsyncTask() { @Override protected Boolean doInBackground(Void[] params) { - return diskUserProfileCache.save(userId, experimentKey, variationKey); + return diskUserProfileCache.save(userId, experimentId, variationId); } @Override @@ -240,20 +240,20 @@ protected void onPreExecute() { if (expIdToVarIdMap == null) { expIdToVarIdMap = new ConcurrentHashMap<>(); } - expIdToVarIdMap.put(experimentKey, variationKey); + expIdToVarIdMap.put(experimentId, variationId); memoryUserProfileCache.put(userId, expIdToVarIdMap); - logger.info("Updated in memory user profile"); + logger.info("Updated in memory user profile."); } @Override protected void onPostExecute(Boolean success) { if (success) { - logger.info("Persisted user in variation {} for experiment {}.", variationKey, experimentKey); + logger.info("Persisted user in variation {} for experiment {}.", variationId, experimentId); } else { // Remove the activation from the cache since saving failed - memoryUserProfileCache.get(userId).remove(experimentKey); - logger.error("Failed to persist user in variation {} for experiment {}.", variationKey, experimentKey); + memoryUserProfileCache.get(userId).remove(experimentId); + logger.error("Failed to persist user in variation {} for experiment {}.", variationId, experimentId); } } }; @@ -261,25 +261,25 @@ protected void onPostExecute(Boolean success) { } @RequiresApi(api = Build.VERSION_CODES.HONEYCOMB) - void startRemoveCacheTask(final String userId, final String experimentKey, final String variationKey) { + void startRemoveCacheTask(final String userId, final String experimentId, final String variationId) { AsyncTask> task = new AsyncTask>() { @Override protected void onPreExecute() { Map expIdToVarIdMap = memoryUserProfileCache.get(userId); if (expIdToVarIdMap != null) { - expIdToVarIdMap.remove(experimentKey); - logger.info("Removed experimentKey: {} variationKey: {} record for user: {} from memory", experimentKey, variationKey, userId); + expIdToVarIdMap.remove(experimentId); + logger.info("Removed experimentId: {} variationId: {} record for user: {} from memory.", experimentId, variationId, userId); } } @Override protected Pair doInBackground(String... params) { - boolean success = diskUserProfileCache.remove(userId, experimentKey); + boolean success = diskUserProfileCache.remove(userId, experimentId); if (success) { return new Pair<>(params[0], true); } else { - // This is the variationKey + // This is the variationId return new Pair<>(params[0], false); } } @@ -289,15 +289,15 @@ protected void onPostExecute(Pair result) { // Put the mapping back in the write through cache if removing failed if (!result.second) { Map expIdToVarIdMap = new ConcurrentHashMap<>(); - expIdToVarIdMap.put(experimentKey, result.first); + expIdToVarIdMap.put(experimentId, result.first); memoryUserProfileCache.put(userId, expIdToVarIdMap); - logger.error("Restored experimentKey: {} variationKey: {} record for user: {} to memory", experimentKey, result.first, userId); + logger.error("Restored experimentId: {} variationId: {} record for user: {} to memory.", experimentId, result.first, userId); } else { - logger.info("Removed experimentKey: {} variationKey: {} record for user: {} from disk", experimentKey, result.first, userId); + logger.info("Removed experimentId: {} variationId: {} record for user: {} from disk.", experimentId, result.first, userId); } } }; - task.executeOnExecutor(executor, variationKey); + task.executeOnExecutor(executor, variationId); } }