Fixes concurrency issue

Author: Josh Deffibaugh

android-sdk/src/androidTest/java/com/optimizely/ab/android/sdk/OptimizelyManagerTest.java

@@ -19,27 +19,22 @@
 import android.content.Intent;
 import android.os.Build;
 import android.support.annotation.RequiresApi;
-import android.support.test.InstrumentationRegistry;
 import android.support.test.espresso.core.deps.guava.util.concurrent.ListeningExecutorService;
 import android.support.test.espresso.core.deps.guava.util.concurrent.MoreExecutors;
 import android.support.test.runner.AndroidJUnit4;
 
-import com.optimizely.ab.android.shared.Cache;
 import com.optimizely.ab.android.shared.ServiceScheduler;
 import com.optimizely.ab.android.user_experiment_record.AndroidUserExperimentRecord;
 
-import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
-import org.mockito.Mock;
 import org.slf4j.Logger;
 
 import java.util.concurrent.TimeUnit;
 
 import static junit.framework.Assert.assertEquals;
-import static junit.framework.Assert.assertFalse;
 import static junit.framework.Assert.assertNotNull;
 import static junit.framework.Assert.assertNull;
 import static junit.framework.Assert.assertTrue;

android-sdk/src/main/java/com/optimizely/ab/android/sdk/OptimizelyManager.java

@@ -123,6 +123,7 @@ void setOptimizelyStartListener(@Nullable OptimizelyStartListener optimizelyStar
      * <p>
      * Instantiates and returns an {@link OptimizelyClient} instance. Will also cache the instance
      * for future lookups via getClient
+     *
      * @param context  any {@link Context} instance
      * @param datafile the datafile
      * @return an {@link OptimizelyClient} instance
@@ -162,7 +163,7 @@ public OptimizelyClient initialize(@NonNull Context context, @NonNull String dat
      * Instantiates and returns an {@link OptimizelyClient} instance. Will also cache the instance
      * for future lookups via getClient. The datafile should be stored in res/raw.
      *
-     * @param context    any {@link Context} instance
+     * @param context     any {@link Context} instance
      * @param dataFileRes the R id that the data file is located under.
      * @return an {@link OptimizelyClient} instance
      */
@@ -184,14 +185,15 @@ public OptimizelyClient initialize(@NonNull Context context, @RawRes int dataFil
      * <p>
      * Instantiates and returns an {@link OptimizelyClient} instance using the datafile cached on disk
      * if not available then it will return a dummy instance.
+     *
      * @param context any {@link Context} instance
      * @return an {@link OptimizelyClient} instance
      */
     public OptimizelyClient initialize(@NonNull Context context) {
         DataFileCache dataFileCache = new DataFileCache(
-            projectId,
-            new Cache(context, LoggerFactory.getLogger(Cache.class)),
-            LoggerFactory.getLogger(DataFileCache.class)
+                projectId,
+                new Cache(context, LoggerFactory.getLogger(Cache.class)),
+                LoggerFactory.getLogger(DataFileCache.class)
         );
 
         JSONObject datafile = dataFileCache.load();
@@ -304,6 +306,7 @@ private String loadRawResource(Context context, @RawRes int rawRes) throws IOExc
 
     /**
      * Check if the datafile is cached on the disk
+     *
      * @param context any {@link Context} instance
      * @return True if the datafile is cached on the disk
      */

build.gradle

@@ -52,7 +52,7 @@ ext {
     min_sdk_version = 10
     target_sdk_version = 24
 
-    java_core_ver = "1.0.2"
+    java_core_ver = "1.0.3-SNAPSHOT"
     android_logger_ver = "1.3.1"
     support_annotations_ver = "24.2.1"
     junit_ver = "4.12"

test-app/build.gradle

@@ -32,7 +32,6 @@ dependencies {
 //    compile 'com.optimizely.ab:android-sdk:0.2.0'
     compile 'com.android.support:appcompat-v7:24.2.1'
     compile 'com.android.support:design:24.2.1'
-    compile 'com.fasterxml.jackson.core:jackson-annotations:2.7.1'
 
     testCompile "junit:junit:$junit_ver"
     testCompile "org.mockito:mockito-core:$mockito_ver"

user-experiment-record/src/main/java/com/optimizely/ab/android/user_experiment_record/AndroidUserExperimentRecord.java

@@ -35,6 +35,7 @@
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.Executor;
 import java.util.concurrent.Executors;
 
@@ -68,7 +69,7 @@ public class AndroidUserExperimentRecord implements UserExperimentRecord {
      * @return the instance as {@link UserExperimentRecord}
      */
     public static UserExperimentRecord newInstance(@NonNull String projectId, @NonNull Context context) {
-        HashMap<String, Map<String, String>> memoryUserExperimentRecordCache = new HashMap<>();
+        Map<String, Map<String, String>> memoryUserExperimentRecordCache = new ConcurrentHashMap<>();
         UserExperimentRecordCache userExperimentRecordCache =
                 new UserExperimentRecordCache(projectId,
                         new Cache(context,
@@ -98,7 +99,7 @@ public void start() {
                     String varId = expIdToVarIdJson.getString(expId);
                     Map<String, String> expIdToVarIdMap = writeThroughCacheTaskFactory.getMemoryUserExperimentRecordCache().get(userId);
                     if (expIdToVarIdMap == null) {
-                        expIdToVarIdMap = new HashMap<>();
+                        expIdToVarIdMap = new ConcurrentHashMap<>();
                     }
                     expIdToVarIdMap.put(expId, varId);
                     writeThroughCacheTaskFactory.getMemoryUserExperimentRecordCache().put(userId, expIdToVarIdMap);
@@ -238,7 +239,7 @@ protected Boolean doInBackground(Void[] params) {
                 protected void onPreExecute() {
                     Map<String, String> expIdToVarIdMap = memoryUserExperimentRecordCache.get(userId);
                     if (expIdToVarIdMap == null) {
-                        expIdToVarIdMap = new HashMap<>();
+                        expIdToVarIdMap = new ConcurrentHashMap<>();
                     }
                     expIdToVarIdMap.put(experimentKey, variationKey);
                     memoryUserExperimentRecordCache.put(userId, expIdToVarIdMap);
@@ -288,7 +289,7 @@ protected Pair<String, Boolean> doInBackground(String... params) {
                 protected void onPostExecute(Pair<String, Boolean> result) {
                     // Put the mapping back in the write through cache if removing failed
                     if (!result.second) {
-                        Map<String, String> expIdToVarIdMap = new HashMap<>();
+                        Map<String, String> expIdToVarIdMap = new ConcurrentHashMap<>();
                         expIdToVarIdMap.put(experimentKey, result.first);
                         memoryUserExperimentRecordCache.put(userId, expIdToVarIdMap);
                         logger.error("Restored experimentKey: {} variationKey: {} record for user: {} to memory", experimentKey, result.first, userId);