Save cache file as received rather than serializing it again (#51)

* write cache file as we get it

* update comment

* bump version number

eppo/build.gradle

@@ -4,7 +4,7 @@ plugins {
 }
 
 group = "cloud.eppo"
-version = "1.0.10"
+version = "1.0.11"
 
 android {
     compileSdk 33

eppo/src/androidTest/java/cloud/eppo/android/EppoClientTest.java

@@ -38,7 +38,6 @@
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
-import java.io.StringReader;
 import java.lang.reflect.Type;
 import java.lang.reflect.Field;
 import java.util.List;
@@ -51,15 +50,15 @@
 import cloud.eppo.android.dto.FlagConfig;
 import cloud.eppo.android.dto.RandomizationConfigResponse;
 import cloud.eppo.android.dto.SubjectAttributes;
-import cloud.eppo.android.dto.deserializers.EppoValueAdapter;
+import cloud.eppo.android.dto.adapters.EppoValueAdapter;
 
 public class EppoClientTest {
     private static final String TAG = logTag(EppoClient.class);
     private static final String DUMMY_API_KEY = "mock-api-key";
     private static final String DUMMY_OTHER_API_KEY = "another-mock-api-key";
     private static final String TEST_HOST = "https://us-central1-eppo-qa.cloudfunctions.net/serveGitHubRacTestFile";
     private static final String INVALID_HOST = "https://thisisabaddomainforthistest.com";
-    private Gson gson = new GsonBuilder()
+    private final Gson gson = new GsonBuilder()
             .registerTypeAdapter(EppoValue.class, new EppoValueAdapter())
             .registerTypeAdapter(AssignmentValueType.class, new AssignmentValueTypeAdapter(AssignmentValueType.STRING))
             .create();
@@ -266,7 +265,7 @@ public void testCachedAssignments() {
         // First initialize successfully
         initClient(TEST_HOST, true, true, false, DUMMY_API_KEY); // ensure cache is populated
 
-        // wait for a bit since cache file is loaded asynchronously
+        // wait for a bit since cache file is written asynchronously
         waitForPopulatedCache();
 
         // Then reinitialize with a bad host so we know it's using the cached RAC built from the first initialization
@@ -371,7 +370,7 @@ public void testInvalidConfigJSON() {
 
         doAnswer(invocation -> {
             RequestCallback callback = invocation.getArgument(1);
-            callback.onSuccess(new StringReader("{}"));
+            callback.onSuccess("{}");
             return null; // doAnswer doesn't require a return value
         }).when(mockHttpClient).get(anyString(), any(RequestCallback.class));
 

eppo/src/main/java/cloud/eppo/android/ConfigurationRequestor.java

@@ -7,27 +7,34 @@
 import com.google.gson.JsonIOException;
 import com.google.gson.JsonSyntaxException;
 
-import java.io.Reader;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
 
 import cloud.eppo.android.dto.FlagConfig;
 
 public class ConfigurationRequestor {
     private static final String TAG = logTag(ConfigurationRequestor.class);
 
-    private EppoHttpClient client;
-    private ConfigurationStore configurationStore;
+    private final EppoHttpClient client;
+    private final ConfigurationStore configurationStore;
 
     public ConfigurationRequestor(ConfigurationStore configurationStore, EppoHttpClient client) {
         this.configurationStore = configurationStore;
         this.client = client;
     }
 
     public void load(InitializationCallback callback) {
+        // We have two at-bats to load the configuration: loading from cache and fetching
+        // The below variables help them keep track of each other's progress
+        AtomicBoolean cacheLoadInProgress = new AtomicBoolean(true);
+        AtomicReference<String> fetchErrorMessage = new AtomicReference<>(null);
+        // We only want to fire the callback off once; so track whether or not we have yet
         AtomicBoolean callbackCalled = new AtomicBoolean(false);
         configurationStore.loadFromCache(new CacheLoadCallback() {
             @Override
             public void onCacheLoadSuccess() {
+                cacheLoadInProgress.set(false);
+                // If cache loaded successfully, fire off success callback if not yet done so by fetching
                 if (callback != null && callbackCalled.compareAndSet(false, true)) {
                     Log.d(TAG, "Initialized from cache");
                     callback.onCompleted();
@@ -36,27 +43,39 @@ public void onCacheLoadSuccess() {
 
             @Override
             public void onCacheLoadFail() {
-                // no-op; fall-back to Fetch
+                cacheLoadInProgress.set(false);
+                Log.d(TAG, "Did not initialize from cache");
+                // If cache loading failed, and fetching failed, fire off the failure callback if not yet done so
+                // Otherwise, if fetching has not failed yet, defer to it for firing off callbacks
+                if (callback != null && fetchErrorMessage.get() != null && callbackCalled.compareAndSet(false, true)) {
+                    Log.e(TAG, "Failed to initialize from fetching or by cache");
+                    callback.onError("Cache and fetch failed "+fetchErrorMessage.get());
+                }
             }
         });
 
         Log.d(TAG, "Fetching configuration");
         client.get("/api/randomized_assignment/v3/config", new RequestCallback() {
             @Override
-            public void onSuccess(Reader response) {
+            public void onSuccess(String responseBody) {
                 try {
                     Log.d(TAG, "Processing fetch response");
-                    configurationStore.setFlagsFromResponse(response);
+                    configurationStore.setFlagsFromJsonString(responseBody);
                     Log.d(TAG, "Configuration fetch successful");
+                    configurationStore.asyncWriteToCache(responseBody);
                 } catch (JsonSyntaxException | JsonIOException e) {
+                    fetchErrorMessage.set(e.getMessage());
                     Log.e(TAG, "Error loading configuration response", e);
-                    if (callback != null && callbackCalled.compareAndSet(false, true)) {
-                        Log.d(TAG, "Initialization failure due to fetch response");
-                        callback.onError("Unable to request configuration");
+                    // If fetching failed, and cache loading failed, fire off the failure callback if not yet done so
+                    // Otherwise, if cache has not finished yet, defer to it for firing off callbacks
+                    if (callback != null && !cacheLoadInProgress.get() && callbackCalled.compareAndSet(false, true)) {
+                        Log.d(TAG, "Failed to initialize from cache or by fetching");
+                        callback.onError("Cache and fetch failed "+e.getMessage());
                     }
                     return;
                 }
 
+                // If fetching succeeded, fire off success callback if not yet done so from cache loading
                 if (callback != null && callbackCalled.compareAndSet(false, true)) {
                     Log.d(TAG, "Initialized from fetch");
                     callback.onCompleted();
@@ -65,8 +84,11 @@ public void onSuccess(Reader response) {
 
             @Override
             public void onFailure(String errorMessage) {
+                fetchErrorMessage.set(errorMessage);
                 Log.e(TAG, "Error fetching configuration: " + errorMessage);
-                if (callback != null && callbackCalled.compareAndSet(false, true)) {
+                // If fetching failed, and cache loading failed, fire off the failure callback if not yet done so
+                // Otherwise, if cache has not finished yet, defer to it for firing off callbacks
+                if (callback != null && !cacheLoadInProgress.get() && callbackCalled.compareAndSet(false, true)) {
                     Log.d(TAG, "Initialization failure due to fetch error");
                     callback.onError(errorMessage);
                 }

eppo/src/main/java/cloud/eppo/android/ConfigurationStore.java

@@ -13,29 +13,28 @@
 import java.io.BufferedReader;
 import java.io.BufferedWriter;
 import java.io.IOException;
-import java.io.Reader;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 import cloud.eppo.android.dto.EppoValue;
 import cloud.eppo.android.dto.FlagConfig;
 import cloud.eppo.android.dto.RandomizationConfigResponse;
-import cloud.eppo.android.dto.deserializers.EppoValueAdapter;
-import cloud.eppo.android.dto.deserializers.RandomizationConfigResponseDeserializer;
+import cloud.eppo.android.dto.adapters.EppoValueAdapter;
+import cloud.eppo.android.dto.adapters.RandomizationConfigResponseAdapter;
 import cloud.eppo.android.util.Utils;
 
 public class ConfigurationStore {
 
     private static final String TAG = logTag(ConfigurationStore.class);
 
     private final Gson gson = new GsonBuilder()
-            .registerTypeAdapter(RandomizationConfigResponse.class, new RandomizationConfigResponseDeserializer())
+            .registerTypeAdapter(RandomizationConfigResponse.class, new RandomizationConfigResponseAdapter())
             .registerTypeAdapter(EppoValue.class, new EppoValueAdapter())
             .serializeNulls()
             .create();
     private final ConfigCacheFile cacheFile;
 
-    private AtomicBoolean loadedFromFetchResponse = new AtomicBoolean(false);
+    private final AtomicBoolean loadedFromFetchResponse = new AtomicBoolean(false);
     private ConcurrentHashMap<String, FlagConfig> flags;
 
     public ConfigurationStore(Application application, String cacheFileNameSuffix) {
@@ -86,8 +85,8 @@ protected RandomizationConfigResponse readCacheFile() throws IOException {
         return configResponse;
     }
 
-    public void setFlagsFromResponse(Reader response) {
-        RandomizationConfigResponse config = gson.fromJson(response, RandomizationConfigResponse.class);
+    public void setFlagsFromJsonString(String jsonString) {
+        RandomizationConfigResponse config = gson.fromJson(jsonString, RandomizationConfigResponse.class);
         if (config == null || config.getFlags() == null) {
             Log.w(TAG, "Flags missing in configuration response");
             flags = new ConcurrentHashMap<>();
@@ -96,18 +95,17 @@ public void setFlagsFromResponse(Reader response) {
             flags = config.getFlags();
             Log.d(TAG, "Loaded " + flags.size() + " flags from configuration response");
         }
-
-        writeConfigToFile(config);
     }
 
-    private void writeConfigToFile(RandomizationConfigResponse config) {
+    public void asyncWriteToCache(String jsonString) {
         AsyncTask.execute(() -> {
             try {
                 synchronized (cacheFile) {
                     BufferedWriter writer = cacheFile.getWriter();
-                    gson.toJson(config, writer);
+                    writer.write(jsonString);
                     writer.close();
                 }
+                Log.d(TAG, "Updated cache file");
             } catch (Exception e) {
                 Log.e(TAG, "Unable to cache config to file", e);
             }

eppo/src/main/java/cloud/eppo/android/EppoHttpClient.java

@@ -4,21 +4,13 @@
 
 import android.util.Log;
 
-import org.jetbrains.annotations.NotNull;
-
 import java.io.IOException;
-import java.io.Reader;
 import java.net.HttpURLConnection;
-import java.net.InetAddress;
-import java.net.UnknownHostException;
-import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.List;
 import java.util.concurrent.TimeUnit;
 
 import okhttp3.Call;
 import okhttp3.Callback;
-import okhttp3.Dns;
 import okhttp3.HttpUrl;
 import okhttp3.OkHttpClient;
 import okhttp3.Request;
@@ -60,7 +52,11 @@ public void get(String path, RequestCallback callback) {
             public void onResponse(Call call, Response response) {
                 if (response.isSuccessful()) {
                     Log.d(TAG, "Fetch successful");
-                    callback.onSuccess(response.body().charStream());
+                    try {
+                        callback.onSuccess(response.body().string());
+                    } catch (IOException ex) {
+                        callback.onFailure("Failed to read response from URL "+httpUrl);
+                    }
                 } else {
                     switch (response.code()) {
                         case HttpURLConnection.HTTP_FORBIDDEN:
@@ -86,6 +82,6 @@ public void onFailure(Call call, IOException e) {
 }
 
 interface RequestCallback {
-    void onSuccess(Reader response);
+    void onSuccess(String responseBody);
     void onFailure(String errorMessage);
 }

eppo/src/main/java/cloud/eppo/android/dto/deserializers/EppoValueAdapter.java → eppo/src/main/java/cloud/eppo/android/dto/adapters/EppoValueAdapter.java

@@ -1,4 +1,4 @@
-package cloud.eppo.android.dto.deserializers;
+package cloud.eppo.android.dto.adapters;
 
 import static cloud.eppo.android.util.Utils.logTag;
 
@@ -45,9 +45,6 @@ public EppoValue deserialize(JsonElement json, Type typeOfT, JsonDeserialization
 
             try {
                 String stringValue = json.getAsString();
-                if (stringValue == "null") {
-                    return EppoValue.valueOf();
-                }
                 return EppoValue.valueOf(stringValue);
             } catch (Exception ignored) {
             }

eppo/src/main/java/cloud/eppo/android/dto/deserializers/RandomizationConfigResponseDeserializer.java → eppo/src/main/java/cloud/eppo/android/dto/adapters/RandomizationConfigResponseAdapter.java

@@ -1,4 +1,4 @@
-package cloud.eppo.android.dto.deserializers;
+package cloud.eppo.android.dto.adapters;
 
 import static cloud.eppo.android.util.Utils.logTag;
 
@@ -32,8 +32,8 @@
  *  unreliable when ProGuard minification is in-use and not configured to protect
  *  JSON-deserialization-related classes and annotations.
  */
-public class RandomizationConfigResponseDeserializer implements JsonDeserializer<RandomizationConfigResponse> {
-    private static final String TAG = logTag(RandomizationConfigResponseDeserializer.class);
+public class RandomizationConfigResponseAdapter implements JsonDeserializer<RandomizationConfigResponse> {
+    private static final String TAG = logTag(RandomizationConfigResponseAdapter.class);
 
     private final EppoValueAdapter eppoValueAdapter = new EppoValueAdapter();
 

eppo/src/test/java/cloud/eppo/android/EppoValueDeserializerTest.java → eppo/src/test/java/cloud/eppo/android/EppoValueAdapterTest.java

@@ -9,41 +9,41 @@
 import org.junit.Test;
 
 import cloud.eppo.android.dto.EppoValue;
-import cloud.eppo.android.dto.deserializers.EppoValueAdapter;
+import cloud.eppo.android.dto.adapters.EppoValueAdapter;
 
-public class EppoValueDeserializerTest {
-    private EppoValueAdapter adapter = new EppoValueAdapter();
+public class EppoValueAdapterTest {
+    private final EppoValueAdapter adapter = new EppoValueAdapter();
 
     @Test
-    public void testDeserializingDouble() throws Exception {
+    public void testDeserializingDouble() {
         JsonElement object = JsonParser.parseString("1");
         EppoValue value = adapter.deserialize(object, EppoValue.class, null);
         assertEquals(value.doubleValue(), 1, 0.001);
     }
 
     @Test
-    public void testDeserializingBoolean() throws Exception {
+    public void testDeserializingBoolean() {
         JsonElement object = JsonParser.parseString("true");
         EppoValue value = adapter.deserialize(object, EppoValue.class, null);
-        assertEquals(value.boolValue(), true);
+        assertTrue(value.boolValue());
     }
 
     @Test
-    public void testDeserializingString() throws Exception {
+    public void testDeserializingString() {
         JsonElement object = JsonParser.parseString("\"true\"");
         EppoValue value = adapter.deserialize(object, EppoValue.class, null);
         assertEquals(value.stringValue(), "true");
     }
 
     @Test
-    public void testDeserializingArray() throws Exception {
+    public void testDeserializingArray() {
         JsonElement object = JsonParser.parseString("[\"value1\", \"value2\"]");
         EppoValue value = adapter.deserialize(object, EppoValue.class, null);
         assertTrue(value.arrayValue().contains("value1"));
     }
 
     @Test
-    public void testDeserializingNull() throws Exception {
+    public void testDeserializingNull() {
         JsonElement object = JsonParser.parseString("null");
         EppoValue value = adapter.deserialize(object, EppoValue.class, null);
         assertTrue(value.isNull());

eppo/src/test/java/cloud/eppo/android/RandomizationConfigResponseDeserializerTest.java → eppo/src/test/java/cloud/eppo/android/RandomizationConfigResponseAdapterTest.java

@@ -25,13 +25,13 @@
 import cloud.eppo.android.dto.TargetingCondition;
 import cloud.eppo.android.dto.TargetingRule;
 import cloud.eppo.android.dto.Variation;
-import cloud.eppo.android.dto.deserializers.EppoValueAdapter;
-import cloud.eppo.android.dto.deserializers.RandomizationConfigResponseDeserializer;
+import cloud.eppo.android.dto.adapters.EppoValueAdapter;
+import cloud.eppo.android.dto.adapters.RandomizationConfigResponseAdapter;
 
-public class RandomizationConfigResponseDeserializerTest {
+public class RandomizationConfigResponseAdapterTest {
 
     private final Gson gson = new GsonBuilder()
-            .registerTypeAdapter(RandomizationConfigResponse.class, new RandomizationConfigResponseDeserializer())
+            .registerTypeAdapter(RandomizationConfigResponse.class, new RandomizationConfigResponseAdapter())
             .registerTypeAdapter(EppoValue.class, new EppoValueAdapter())
             .serializeNulls()
             .create();