diff --git a/eppo/build.gradle b/eppo/build.gradle index b123caac..720126b7 100644 --- a/eppo/build.gradle +++ b/eppo/build.gradle @@ -4,7 +4,7 @@ plugins { } group = "cloud.eppo" -version = "1.0.8" +version = "1.0.9" 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 fd632b1a..9b97f072 100644 --- a/eppo/src/androidTest/java/cloud/eppo/android/EppoClientTest.java +++ b/eppo/src/androidTest/java/cloud/eppo/android/EppoClientTest.java @@ -1,15 +1,15 @@ package cloud.eppo.android; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; -import static cloud.eppo.android.ConfigCacheFile.CACHE_FILE_NAME; +import static cloud.eppo.android.ConfigCacheFile.cacheFileName; import static cloud.eppo.android.util.Utils.logTag; +import static cloud.eppo.android.util.Utils.safeCacheKey; -import android.content.Context; -import android.content.SharedPreferences; import android.content.res.AssetManager; import android.util.Log; @@ -52,6 +52,8 @@ 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() @@ -124,29 +126,16 @@ static class AssignmentTestCase { List expectedAssignments; } - private void deleteFileIfExists(String fileName) { - File file = new File(ApplicationProvider.getApplicationContext().getFilesDir(), fileName); - if (file.exists()) { - file.delete(); - } - } - - private void deleteCacheFiles() { - deleteFileIfExists(CACHE_FILE_NAME); - SharedPreferences sharedPreferences = ApplicationProvider.getApplicationContext().getSharedPreferences("eppo", Context.MODE_PRIVATE); - sharedPreferences.edit().clear().commit(); - } - - private void initClient(String host, boolean throwOnCallackError, boolean shouldDeleteCacheFiles, boolean isGracefulMode) { + private void initClient(String host, boolean throwOnCallackError, boolean shouldDeleteCacheFiles, boolean isGracefulMode, String apiKey) { if (shouldDeleteCacheFiles) { - deleteCacheFiles(); + clearCacheFile(apiKey); } CountDownLatch lock = new CountDownLatch(1); new EppoClient.Builder() .application(ApplicationProvider.getApplicationContext()) - .apiKey("mock-api-key") + .apiKey(apiKey) .isGracefulMode(isGracefulMode) .host(host) .callback(new InitializationCallback() { @@ -178,19 +167,27 @@ public void onError(String errorMessage) { } @After - public void teardown() { - deleteCacheFiles(); + public void clearCaches() { + String[] apiKeys = { DUMMY_API_KEY, DUMMY_OTHER_API_KEY }; + for (String apiKey : apiKeys) { + clearCacheFile(apiKey); + } + } + + private void clearCacheFile(String apiKey) { + ConfigCacheFile cacheFile = new ConfigCacheFile(ApplicationProvider.getApplicationContext(), apiKey); + cacheFile.delete(); } @Test public void testAssignments() { - initClient(TEST_HOST, true, true, false); + initClient(TEST_HOST, true, true, false, DUMMY_API_KEY); runTestCases(); } @Test public void testErrorGracefulModeOn() { - initClient(TEST_HOST, false, true, true); + initClient(TEST_HOST, false, true, true, DUMMY_API_KEY); EppoClient realClient = EppoClient.getInstance(); EppoClient spyClient = spy(realClient); @@ -219,7 +216,7 @@ public void testErrorGracefulModeOn() { @Test public void testErrorGracefulModeOff() { - initClient(TEST_HOST, false, true, false); + initClient(TEST_HOST, false, true, false, DUMMY_API_KEY); EppoClient realClient = EppoClient.getInstance(); EppoClient spyClient = spy(realClient); @@ -263,13 +260,13 @@ private void runTestCases() { @Test public void testCachedAssignments() { // First initialize successfully - initClient(TEST_HOST, false, true, false); // ensure cache is populated + initClient(TEST_HOST, true, true, false, DUMMY_API_KEY); // ensure cache is populated // wait for a bit since cache file is loaded asynchronously waitForPopulatedCache(); // Then reinitialize with a bad host so we know it's using the cached RAC built from the first initialization - initClient(INVALID_HOST, false, false, false); // invalid port to force to use cache + initClient(INVALID_HOST, true, false, false, DUMMY_API_KEY); // invalid port to force to use cache runTestCases(); } @@ -382,7 +379,7 @@ public void testInvalidConfigJSON() { httpClientOverrideField.set(null, mockHttpClient); - initClient(TEST_HOST, true, true, false); + initClient(TEST_HOST, true, true, false, DUMMY_API_KEY); } catch (NoSuchFieldException | IllegalAccessException e) { throw new RuntimeException(e); } finally { @@ -401,26 +398,82 @@ public void testInvalidConfigJSON() { } @Test - public void testCachedBadResponseAllowsLaterFetching() { + public void testCachedBadResponseRequiresFetch() { // Populate the cache with a bad response - ConfigCacheFile cacheFile = new ConfigCacheFile(ApplicationProvider.getApplicationContext()); - cacheFile.delete(); - try { - cacheFile.getOutputWriter().write("{}"); - cacheFile.getOutputWriter().close(); - } catch (IOException e) { - throw new RuntimeException(e); - } + ConfigCacheFile cacheFile = new ConfigCacheFile(ApplicationProvider.getApplicationContext(), safeCacheKey(DUMMY_API_KEY)); + cacheFile.setContents("{ invalid }"); - initClient(TEST_HOST, false, false, false); + initClient(TEST_HOST, true, false, false, DUMMY_API_KEY); - String result = EppoClient.getInstance().getStringAssignment("dummy subject", "dummy flag"); - assertNull(result); - // Failure callback will have fired from cache read error, but configuration request will still be fired off on init - // Wait for the configuration request to load the configuration - waitForNonNullAssignment(); - String assignment = EppoClient.getInstance().getStringAssignment("6255e1a7fc33a9c050ce9508", "randomization_algo"); - assertEquals("control", assignment); + String assignment = EppoClient.getInstance().getStringAssignment("6255e1a7d1a3025a26078b95", "randomization_algo"); + assertEquals("green", assignment); + } + + @Test + public void testEmptyFlagsResponseRequiresFetch() { + // Populate the cache with a bad response + ConfigCacheFile cacheFile = new ConfigCacheFile(ApplicationProvider.getApplicationContext(), safeCacheKey(DUMMY_API_KEY)); + cacheFile.setContents("{\"flags\": {}}"); + + initClient(TEST_HOST, true, false, false, DUMMY_API_KEY); + String assignment = EppoClient.getInstance().getStringAssignment("6255e1a7d1a3025a26078b95", "randomization_algo"); + assertEquals("green", assignment); + } + + @Test + public void testDifferentCacheFilesPerKey() { + initClient(TEST_HOST, true, true, false, DUMMY_API_KEY); + // API Key 1 will fetch and then populate its cache with the usual test data + Boolean apiKey1Assignment = EppoClient.getInstance().getBooleanAssignment("subject-2", "experiment_with_boolean_variations"); + assertFalse(apiKey1Assignment); + + // Pre-seed a different flag configuration for the other API Key + ConfigCacheFile cacheFile2 = new ConfigCacheFile(ApplicationProvider.getApplicationContext(), safeCacheKey(DUMMY_OTHER_API_KEY)); + cacheFile2.setContents("{\n" + + " \"flags\": {\n" + + " \"8fc1fb33379d78c8a9edbf43afd6703a\": {\n" + + " \"subjectShards\": 10000,\n" + + " \"enabled\": true,\n" + + " \"rules\": [\n" + + " {\n" + + " \"allocationKey\": \"mock-allocation\",\n" + + " \"conditions\": []\n" + + " }\n" + + " ],\n" + + " \"allocations\": {\n" + + " \"mock-allocation\": {\n" + + " \"percentExposure\": 1,\n" + + " \"statusQuoVariationKey\": null,\n" + + " \"shippedVariationKey\": null,\n" + + " \"holdouts\": [],\n" + + " \"variations\": [\n" + + " {\n" + + " \"name\": \"on\",\n" + + " \"value\": \"true\",\n" + + " \"typedValue\": true,\n" + + " \"shardRange\": {\n" + + " \"start\": 0,\n" + + " \"end\": 10000\n" + + " }\n" + + " }" + + " ]\n" + + " }\n" + + " }\n" + + " }\n" + + " }\n" + + "}"); + + initClient(TEST_HOST, true, false, false, DUMMY_OTHER_API_KEY); + + // Ensure API key 2 uses its cache + Boolean apiKey2Assignment = EppoClient.getInstance().getBooleanAssignment("subject-2", "experiment_with_boolean_variations"); + assertTrue(apiKey2Assignment); + + // Reinitialize API key 1 to be sure it used its cache + initClient(TEST_HOST, true, false, false, DUMMY_API_KEY); + // API Key 1 will fetch and then populate its cache with the usual test data + apiKey1Assignment = EppoClient.getInstance().getBooleanAssignment("subject-2", "experiment_with_boolean_variations"); + assertFalse(apiKey1Assignment); } private void waitForPopulatedCache() { @@ -428,7 +481,7 @@ private void waitForPopulatedCache() { long waitEnd = waitStart + 10 * 1000; // allow up to 10 seconds boolean cachePopulated = false; try { - File file = new File(ApplicationProvider.getApplicationContext().getFilesDir(), CACHE_FILE_NAME); + File file = new File(ApplicationProvider.getApplicationContext().getFilesDir(), cacheFileName(safeCacheKey(DUMMY_API_KEY))); while (!cachePopulated) { if (System.currentTimeMillis() > waitEnd) { throw new InterruptedException("Cache file never populated; assuming configuration error"); diff --git a/eppo/src/main/java/cloud/eppo/android/ConfigCacheFile.java b/eppo/src/main/java/cloud/eppo/android/ConfigCacheFile.java index 7cfc1b0d..5954fb50 100644 --- a/eppo/src/main/java/cloud/eppo/android/ConfigCacheFile.java +++ b/eppo/src/main/java/cloud/eppo/android/ConfigCacheFile.java @@ -2,21 +2,23 @@ import android.app.Application; +import java.io.BufferedReader; +import java.io.BufferedWriter; import java.io.File; -import java.io.FileInputStream; -import java.io.FileOutputStream; +import java.io.FileReader; +import java.io.FileWriter; import java.io.IOException; -import java.io.InputStreamReader; -import java.io.OutputStreamWriter; public class ConfigCacheFile { - static final String CACHE_FILE_NAME = "eppo-sdk-config-v2.json"; - private final File filesDir; private final File cacheFile; - public ConfigCacheFile(Application application) { - filesDir = application.getFilesDir(); - cacheFile = new File(filesDir, CACHE_FILE_NAME); + public ConfigCacheFile(Application application, String fileNameSuffix) { + File filesDir = application.getFilesDir(); + cacheFile = new File(filesDir, cacheFileName(fileNameSuffix)); + } + + public static String cacheFileName(String suffix) { + return "eppo-sdk-config-v3-" + suffix + ".json"; } public boolean exists() { @@ -29,13 +31,31 @@ public void delete() { } } - public OutputStreamWriter getOutputWriter() throws IOException { - FileOutputStream fos = new FileOutputStream(cacheFile); - return new OutputStreamWriter(fos); + /** + * Useful for passing in as a writer for gson serialization + */ + public BufferedWriter getWriter() throws IOException { + return new BufferedWriter(new FileWriter(cacheFile)); } - public InputStreamReader getInputReader() throws IOException { - FileInputStream fis = new FileInputStream(cacheFile); - return new InputStreamReader(fis); + /** + * Useful for passing in as a reader for gson deserialization + */ + public BufferedReader getReader() throws IOException { + return new BufferedReader(new FileReader(cacheFile)); + } + + /** + * Useful for mocking caches in automated tests + */ + public void setContents(String contents) { + delete(); + try { + BufferedWriter writer = getWriter(); + writer.write(contents); + writer.close(); + } catch (IOException ex) { + throw new RuntimeException(ex); + } } } diff --git a/eppo/src/main/java/cloud/eppo/android/ConfigurationRequestor.java b/eppo/src/main/java/cloud/eppo/android/ConfigurationRequestor.java index ebef8547..fad8a69a 100644 --- a/eppo/src/main/java/cloud/eppo/android/ConfigurationRequestor.java +++ b/eppo/src/main/java/cloud/eppo/android/ConfigurationRequestor.java @@ -44,7 +44,8 @@ public void onCacheLoadFail() { @Override public void onSuccess(Reader response) { try { - configurationStore.setFlags(response); + configurationStore.setFlagsFromResponse(response); + Log.d(TAG, "Configuration fetch successful"); } catch (JsonSyntaxException | JsonIOException e) { Log.e(TAG, "Error loading configuration response", e); if (callback != null && !cachedUsed.get()) { @@ -60,7 +61,7 @@ public void onSuccess(Reader response) { @Override public void onFailure(String errorMessage) { - Log.e(TAG, errorMessage); + Log.e(TAG, "Error fetching configuration: " + errorMessage); if (callback != null && !cachedUsed.get()) { 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 d601a120..a205e28d 100644 --- a/eppo/src/main/java/cloud/eppo/android/ConfigurationStore.java +++ b/eppo/src/main/java/cloud/eppo/android/ConfigurationStore.java @@ -3,7 +3,6 @@ import static cloud.eppo.android.util.Utils.logTag; import android.app.Application; -import android.content.SharedPreferences; import android.os.AsyncTask; import android.util.Log; @@ -11,11 +10,9 @@ import com.google.gson.GsonBuilder; import com.google.gson.JsonSyntaxException; -import java.io.InputStreamReader; -import java.io.OutputStreamWriter; +import java.io.BufferedReader; +import java.io.BufferedWriter; import java.io.Reader; -import java.util.HashSet; -import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import cloud.eppo.android.dto.EppoValue; @@ -34,15 +31,12 @@ public class ConfigurationStore { .registerTypeAdapter(EppoValue.class, new EppoValueAdapter()) .serializeNulls() .create(); - private final Set flagConfigsToSaveToPrefs = new HashSet<>(); - private final SharedPreferences sharedPrefs; private final ConfigCacheFile cacheFile; private ConcurrentHashMap flags; - public ConfigurationStore(Application application) { - cacheFile = new ConfigCacheFile(application); - this.sharedPrefs = Utils.getSharedPrefs(application); + public ConfigurationStore(Application application, String cacheFileNameSuffix) { + cacheFile = new ConfigCacheFile(application, cacheFileNameSuffix); } public void loadFromCache(CacheLoadCallback callback) { @@ -55,15 +49,18 @@ public void loadFromCache(CacheLoadCallback callback) { AsyncTask.execute(() -> { Log.d(TAG, "Loading from cache"); try { + RandomizationConfigResponse configResponse; synchronized (cacheFile) { - InputStreamReader reader = cacheFile.getInputReader(); - RandomizationConfigResponse configResponse = gson.fromJson(reader, RandomizationConfigResponse.class); + BufferedReader reader = cacheFile.getReader(); + configResponse = gson.fromJson(reader, RandomizationConfigResponse.class); reader.close(); - if (configResponse == null || configResponse.getFlags() == null) { - throw new JsonSyntaxException("Configuration file missing flags"); - } - flags = configResponse.getFlags(); - updateConfigsInSharedPrefs(); + } + if (configResponse == null || configResponse.getFlags() == null) { + throw new JsonSyntaxException("Configuration file missing flags"); + } + flags = configResponse.getFlags(); + if (flags.isEmpty()) { + throw new IllegalStateException("Cached configuration file has empty flags"); } Log.d(TAG, "Cache loaded successfully"); callback.onCacheLoadSuccess(); @@ -75,55 +72,24 @@ public void loadFromCache(CacheLoadCallback callback) { }); } - public void setFlags(Reader response) { + public void setFlagsFromResponse(Reader response) { RandomizationConfigResponse config = gson.fromJson(response, RandomizationConfigResponse.class); if (config == null || config.getFlags() == null) { Log.w(TAG, "Flags missing in configuration response"); flags = new ConcurrentHashMap<>(); } else { flags = config.getFlags(); - } - - // update any existing flags already in shared prefs - updateConfigsInSharedPrefs(); - - if (!flagConfigsToSaveToPrefs.isEmpty()) { - SharedPreferences.Editor editor = sharedPrefs.edit(); - for (String plaintextFlagKey : flagConfigsToSaveToPrefs) { - FlagConfig flagConfig = getFlag(plaintextFlagKey); - if (flagConfig == null) { - continue; - } - - String hashedFlagKey = Utils.getMD5Hex(plaintextFlagKey); - writeFlagToSharedPrefs(hashedFlagKey, flagConfig, editor); - } - editor.apply(); - flagConfigsToSaveToPrefs.clear(); + Log.d(TAG, "Loaded" + flags.size() + "flags from configuration response"); } writeConfigToFile(config); } - private void updateConfigsInSharedPrefs() { - SharedPreferences.Editor editor = sharedPrefs.edit(); - for (String hashedFlagKey : flags.keySet()) { - if (sharedPrefs.contains(hashedFlagKey)) { - writeFlagToSharedPrefs(hashedFlagKey, flags.get(hashedFlagKey), editor); - } - } - editor.apply(); - } - - private void writeFlagToSharedPrefs(String hashedFlagKey, FlagConfig config, SharedPreferences.Editor editor) { - editor.putString(hashedFlagKey, gson.toJson(config)); - } - private void writeConfigToFile(RandomizationConfigResponse config) { AsyncTask.execute(() -> { try { synchronized (cacheFile) { - OutputStreamWriter writer = cacheFile.getOutputWriter(); + BufferedWriter writer = cacheFile.getWriter(); gson.toJson(config, writer); writer.close(); } @@ -133,24 +99,13 @@ private void writeConfigToFile(RandomizationConfigResponse config) { }); } - private FlagConfig getFlagFromSharedPrefs(String hashedFlagKey) { - try { - return gson.fromJson(sharedPrefs.getString(hashedFlagKey, null), FlagConfig.class); - } catch (Exception e) { - Log.e(TAG, "Unable to load flag from prefs", e); - } - return null; - } - public FlagConfig getFlag(String flagKey) { String hashedFlagKey = Utils.getMD5Hex(flagKey); if (flags == null) { - FlagConfig flagFromSharedPrefs = getFlagFromSharedPrefs(hashedFlagKey); - if (flagFromSharedPrefs != null) { - return flagFromSharedPrefs; - } - flagConfigsToSaveToPrefs.add(flagKey); + Log.w(TAG, "Request for flag " + flagKey + " before flags have been loaded"); return null; + } else if (flags.isEmpty()) { + Log.w(TAG, "Request for flag " + flagKey + " with empty flags"); } return flags.get(hashedFlagKey); } diff --git a/eppo/src/main/java/cloud/eppo/android/EppoClient.java b/eppo/src/main/java/cloud/eppo/android/EppoClient.java index d6ee1e8c..b3d91978 100644 --- a/eppo/src/main/java/cloud/eppo/android/EppoClient.java +++ b/eppo/src/main/java/cloud/eppo/android/EppoClient.java @@ -1,6 +1,7 @@ package cloud.eppo.android; import static cloud.eppo.android.util.Utils.logTag; +import static cloud.eppo.android.util.Utils.safeCacheKey; import static cloud.eppo.android.util.Utils.validateNotEmptyOrNull; import android.app.ActivityManager; @@ -10,7 +11,6 @@ import com.google.gson.JsonElement; -import java.util.Date; import java.util.List; import cloud.eppo.android.dto.Allocation; @@ -42,7 +42,8 @@ public class EppoClient { private EppoClient(Application application, String apiKey, String host, AssignmentLogger assignmentLogger, boolean isGracefulMode) { EppoHttpClient httpClient = httpClientOverride == null ? new EppoHttpClient(host, apiKey) : httpClientOverride; - ConfigurationStore configStore = new ConfigurationStore(application); + String cacheFileNameSuffix = safeCacheKey(apiKey); // Cache at a per-API key level (useful for development) + ConfigurationStore configStore = new ConfigurationStore(application, cacheFileNameSuffix); requestor = new ConfigurationRequestor(configStore, httpClient); this.isGracefulMode = isGracefulMode; this.assignmentLogger = assignmentLogger; diff --git a/eppo/src/main/java/cloud/eppo/android/logging/Assignment.java b/eppo/src/main/java/cloud/eppo/android/logging/Assignment.java index 666f36d5..8ebf5b1d 100644 --- a/eppo/src/main/java/cloud/eppo/android/logging/Assignment.java +++ b/eppo/src/main/java/cloud/eppo/android/logging/Assignment.java @@ -46,6 +46,14 @@ public String getExperiment() { return experiment; } + public String getFeatureFlag() { + return featureFlag; + } + + public String getAllocation() { + return allocation; + } + public String getVariation() { return variation; } diff --git a/eppo/src/main/java/cloud/eppo/android/util/Utils.java b/eppo/src/main/java/cloud/eppo/android/util/Utils.java index 2f5c28d4..1a2e5568 100644 --- a/eppo/src/main/java/cloud/eppo/android/util/Utils.java +++ b/eppo/src/main/java/cloud/eppo/android/util/Utils.java @@ -54,8 +54,14 @@ public static String getISODate(Date date) { return dateFormat.format(date); } - public static SharedPreferences getSharedPrefs(Context context) { - return context.getSharedPreferences("eppo", Context.MODE_PRIVATE); + public static String safeCacheKey(String key) { + // Take the first eight characters to avoid the key being sensitive information + // Remove non-alphanumeric characters so it plays nice with filesystem + return key.substring(0,8).replaceAll("\\W", ""); + } + + public static SharedPreferences getSharedPrefs(Context context, String keySuffix) { + return context.getSharedPreferences("eppo-"+keySuffix, Context.MODE_PRIVATE); } public static String generateExperimentKey(String featureFlagKey, String allocationKey) {