diff --git a/eppo/build.gradle b/eppo/build.gradle index c7c773af..c7696958 100644 --- a/eppo/build.gradle +++ b/eppo/build.gradle @@ -4,7 +4,7 @@ plugins { } group = "cloud.eppo" -version = "1.0.10" +version = "1.0.11" android { compileSdk 33 diff --git a/eppo/src/androidTest/java/cloud/eppo/android/EppoClientTest.java b/eppo/src/androidTest/java/cloud/eppo/android/EppoClientTest.java index 0ef83694..08ef9649 100644 --- a/eppo/src/androidTest/java/cloud/eppo/android/EppoClientTest.java +++ b/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,7 +50,7 @@ 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); @@ -59,7 +58,7 @@ public class EppoClientTest { 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)); diff --git a/eppo/src/main/java/cloud/eppo/android/ConfigurationRequestor.java b/eppo/src/main/java/cloud/eppo/android/ConfigurationRequestor.java index b5088bc8..0320eff7 100644 --- a/eppo/src/main/java/cloud/eppo/android/ConfigurationRequestor.java +++ b/eppo/src/main/java/cloud/eppo/android/ConfigurationRequestor.java @@ -7,16 +7,16 @@ 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; @@ -24,10 +24,17 @@ public ConfigurationRequestor(ConfigurationStore configurationStore, EppoHttpCli } 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 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); } diff --git a/eppo/src/main/java/cloud/eppo/android/ConfigurationStore.java b/eppo/src/main/java/cloud/eppo/android/ConfigurationStore.java index 995fe456..0a21b54f 100644 --- a/eppo/src/main/java/cloud/eppo/android/ConfigurationStore.java +++ b/eppo/src/main/java/cloud/eppo/android/ConfigurationStore.java @@ -13,15 +13,14 @@ 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 { @@ -29,13 +28,13 @@ 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 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); } diff --git a/eppo/src/main/java/cloud/eppo/android/EppoHttpClient.java b/eppo/src/main/java/cloud/eppo/android/EppoHttpClient.java index ebe12f5d..cb2742b3 100644 --- a/eppo/src/main/java/cloud/eppo/android/EppoHttpClient.java +++ b/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); } diff --git a/eppo/src/main/java/cloud/eppo/android/dto/deserializers/EppoValueAdapter.java b/eppo/src/main/java/cloud/eppo/android/dto/adapters/EppoValueAdapter.java similarity index 94% rename from eppo/src/main/java/cloud/eppo/android/dto/deserializers/EppoValueAdapter.java rename to eppo/src/main/java/cloud/eppo/android/dto/adapters/EppoValueAdapter.java index 2630a6fa..f03948ff 100644 --- a/eppo/src/main/java/cloud/eppo/android/dto/deserializers/EppoValueAdapter.java +++ b/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) { } diff --git a/eppo/src/main/java/cloud/eppo/android/dto/deserializers/RandomizationConfigResponseDeserializer.java b/eppo/src/main/java/cloud/eppo/android/dto/adapters/RandomizationConfigResponseAdapter.java similarity index 97% rename from eppo/src/main/java/cloud/eppo/android/dto/deserializers/RandomizationConfigResponseDeserializer.java rename to eppo/src/main/java/cloud/eppo/android/dto/adapters/RandomizationConfigResponseAdapter.java index 28f0085e..ba65e554 100644 --- a/eppo/src/main/java/cloud/eppo/android/dto/deserializers/RandomizationConfigResponseDeserializer.java +++ b/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 { - private static final String TAG = logTag(RandomizationConfigResponseDeserializer.class); +public class RandomizationConfigResponseAdapter implements JsonDeserializer { + private static final String TAG = logTag(RandomizationConfigResponseAdapter.class); private final EppoValueAdapter eppoValueAdapter = new EppoValueAdapter(); diff --git a/eppo/src/test/java/cloud/eppo/android/EppoValueDeserializerTest.java b/eppo/src/test/java/cloud/eppo/android/EppoValueAdapterTest.java similarity index 70% rename from eppo/src/test/java/cloud/eppo/android/EppoValueDeserializerTest.java rename to eppo/src/test/java/cloud/eppo/android/EppoValueAdapterTest.java index f464e710..b09ee747 100644 --- a/eppo/src/test/java/cloud/eppo/android/EppoValueDeserializerTest.java +++ b/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()); diff --git a/eppo/src/test/java/cloud/eppo/android/RandomizationConfigResponseDeserializerTest.java b/eppo/src/test/java/cloud/eppo/android/RandomizationConfigResponseAdapterTest.java similarity index 96% rename from eppo/src/test/java/cloud/eppo/android/RandomizationConfigResponseDeserializerTest.java rename to eppo/src/test/java/cloud/eppo/android/RandomizationConfigResponseAdapterTest.java index abb0a218..1cdf6f1b 100644 --- a/eppo/src/test/java/cloud/eppo/android/RandomizationConfigResponseDeserializerTest.java +++ b/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();