segmentio · f2prateek · Nov 14, 2016 · Nov 14, 2016
diff --git a/analytics-tests/src/test/java/com/segment/analytics/AnalyticsTest.java b/analytics-tests/src/test/java/com/segment/analytics/AnalyticsTest.java
@@ -21,13 +21,10 @@
 import com.segment.analytics.integrations.TrackPayload;
 import com.segment.analytics.internal.Utils.AnalyticsNetworkExecutorService;
 import java.io.IOException;
-import java.net.HttpURLConnection;
-import java.nio.charset.Charset;
 import java.util.Collections;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.atomic.AtomicReference;
-import okio.Buffer;
 import org.assertj.core.data.MapEntry;
 import org.hamcrest.Description;
 import org.hamcrest.TypeSafeMatcher;
@@ -38,8 +35,6 @@
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.Spy;
-import org.mockito.invocation.InvocationOnMock;
-import org.mockito.stubbing.Answer;
 import org.robolectric.RobolectricGradleTestRunner;
 import org.robolectric.RuntimeEnvironment;
 import org.robolectric.Shadows;
@@ -88,6 +83,7 @@ public class AnalyticsTest {
   @Spy ExecutorService analyticsExecutor = new SynchronousExecutor();
   @Mock Client client;
   @Mock Stats stats;
+  @Mock ProjectSettings.Cache projectSettingsCache;
   @Mock Integration integration;
   Integration.Factory factory;
   BooleanPreference optOut;
@@ -119,27 +115,18 @@ public static void grantPermission(final Application app, final String permissio
         return "test";
       }
     };
-
-    when(client.fetchSettings()).thenAnswer(new Answer<Client.Connection>() {
-      @Override public Client.Connection answer(InvocationOnMock invocation) throws Throwable {
-        Buffer buffer = new Buffer();
-        buffer.writeString(SETTINGS, Charset.forName("UTF-8"));
-        return new Client.Connection(mock(HttpURLConnection.class), buffer.inputStream(), null) {
-          @Override public void close() throws IOException {
-            super.close();
-          }
-        };
-      }
-    });
+    when(projectSettingsCache.get()) //
+        .thenReturn(ProjectSettings.create(Cartographer.INSTANCE.fromJson(SETTINGS)));
 
     SharedPreferences sharedPreferences =
         RuntimeEnvironment.application.getSharedPreferences("analytics-test-qaz", MODE_PRIVATE);
     optOut = new BooleanPreference(sharedPreferences, "opt-out-test", false);
 
     analytics = new Analytics(application, networkExecutor, stats, traitsCache, analyticsContext,
         defaultOptions, Logger.with(NONE), "qaz", Collections.singletonList(factory), client,
-        Cartographer.INSTANCE, "foo", DEFAULT_FLUSH_QUEUE_SIZE, DEFAULT_FLUSH_INTERVAL,
-        analyticsExecutor, false, new CountDownLatch(0), false, false, optOut, Crypto.none());
+        Cartographer.INSTANCE, projectSettingsCache, "foo", DEFAULT_FLUSH_QUEUE_SIZE,
+        DEFAULT_FLUSH_INTERVAL, analyticsExecutor, false, new CountDownLatch(0), false, false,
+        optOut, Crypto.none());
 
     // Used by singleton tests.
     grantPermission(RuntimeEnvironment.application, Manifest.permission.INTERNET);
@@ -289,7 +276,7 @@ public static void grantPermission(final Application app, final String permissio
   }
 
   @Test public void emptyTrackingPlan() throws IOException {
-    analytics.projectSettings = new ProjectSettings(Cartographer.INSTANCE.fromJson("{\n"
+    analytics.projectSettings = ProjectSettings.create(Cartographer.INSTANCE.fromJson("{\n"
         + "  \"integrations\": {\n"
         + "    \"test\": {\n"
         + "      \"foo\": \"bar\"\n"
@@ -309,7 +296,7 @@ public static void grantPermission(final Application app, final String permissio
   }
 
   @Test public void emptyEventPlan() throws IOException {
-    analytics.projectSettings = new ProjectSettings(Cartographer.INSTANCE.fromJson("{\n"
+    analytics.projectSettings = ProjectSettings.create(Cartographer.INSTANCE.fromJson("{\n"
         + "  \"integrations\": {\n"
         + "    \"test\": {\n"
         + "      \"foo\": \"bar\"\n"
@@ -331,7 +318,7 @@ public static void grantPermission(final Application app, final String permissio
   }
 
   @Test public void trackingPlanDisablesEvent() throws IOException {
-    analytics.projectSettings = new ProjectSettings(Cartographer.INSTANCE.fromJson("{\n"
+    analytics.projectSettings = ProjectSettings.create(Cartographer.INSTANCE.fromJson("{\n"
         + "  \"integrations\": {\n"
         + "    \"test\": {\n"
         + "      \"foo\": \"bar\"\n"
@@ -351,7 +338,7 @@ public static void grantPermission(final Application app, final String permissio
   }
 
   @Test public void trackingPlanDisablesEventForSingleIntegration() throws IOException {
-    analytics.projectSettings = new ProjectSettings(Cartographer.INSTANCE.fromJson("{\n"
+    analytics.projectSettings = ProjectSettings.create(Cartographer.INSTANCE.fromJson("{\n"
         + "  \"integrations\": {\n"
         + "    \"test\": {\n"
         + "      \"foo\": \"bar\"\n"
@@ -374,7 +361,7 @@ public static void grantPermission(final Application app, final String permissio
   }
 
   @Test public void trackingPlanDisabledEventCannotBeOverriddenByOptions() throws IOException {
-    analytics.projectSettings = new ProjectSettings(Cartographer.INSTANCE.fromJson("{\n"
+    analytics.projectSettings = ProjectSettings.create(Cartographer.INSTANCE.fromJson("{\n"
         + "  \"integrations\": {\n"
         + "    \"test\": {\n"
         + "      \"foo\": \"bar\"\n"
@@ -395,7 +382,7 @@ public static void grantPermission(final Application app, final String permissio
 
   @Test public void trackingPlanDisabledEventForIntegrationOverriddenByOptions()
       throws IOException {
-    analytics.projectSettings = new ProjectSettings(Cartographer.INSTANCE.fromJson("{\n"
+    analytics.projectSettings = ProjectSettings.create(Cartographer.INSTANCE.fromJson("{\n"
         + "  \"integrations\": {\n"
         + "    \"test\": {\n"
         + "      \"foo\": \"bar\"\n"
@@ -609,8 +596,9 @@ protected boolean matchesSafely(Application.ActivityLifecycleCallbacks item) {
 
     analytics = new Analytics(application, networkExecutor, stats, traitsCache, analyticsContext,
         defaultOptions, Logger.with(NONE), "qaz", Collections.singletonList(factory), client,
-        Cartographer.INSTANCE, "foo", DEFAULT_FLUSH_QUEUE_SIZE, DEFAULT_FLUSH_INTERVAL,
-        analyticsExecutor, true, new CountDownLatch(0), false, false, optOut, Crypto.none());
+        Cartographer.INSTANCE, projectSettingsCache, "foo", DEFAULT_FLUSH_QUEUE_SIZE,
+        DEFAULT_FLUSH_INTERVAL, analyticsExecutor, true, new CountDownLatch(0), false, false,
+        optOut, Crypto.none());
 
     callback.get().onActivityCreated(null, null);
 
@@ -667,8 +655,9 @@ protected boolean matchesSafely(Application.ActivityLifecycleCallbacks item) {
 
     analytics = new Analytics(application, networkExecutor, stats, traitsCache, analyticsContext,
         defaultOptions, Logger.with(NONE), "qaz", Collections.singletonList(factory), client,
-        Cartographer.INSTANCE, "foo", DEFAULT_FLUSH_QUEUE_SIZE, DEFAULT_FLUSH_INTERVAL,
-        analyticsExecutor, true, new CountDownLatch(0), false, false, optOut, Crypto.none());
+        Cartographer.INSTANCE, projectSettingsCache, "foo", DEFAULT_FLUSH_QUEUE_SIZE,
+        DEFAULT_FLUSH_INTERVAL, analyticsExecutor, true, new CountDownLatch(0), false, false,
+        optOut, Crypto.none());
 
     callback.get().onActivityCreated(null, null);
 
@@ -707,8 +696,9 @@ protected boolean matchesSafely(Application.ActivityLifecycleCallbacks item) {
 
     analytics = new Analytics(application, networkExecutor, stats, traitsCache, analyticsContext,
         defaultOptions, Logger.with(NONE), "qaz", Collections.singletonList(factory), client,
-        Cartographer.INSTANCE, "foo", DEFAULT_FLUSH_QUEUE_SIZE, DEFAULT_FLUSH_INTERVAL,
-        analyticsExecutor, false, new CountDownLatch(0), true, false, optOut, Crypto.none());
+        Cartographer.INSTANCE, projectSettingsCache, "foo", DEFAULT_FLUSH_QUEUE_SIZE,
+        DEFAULT_FLUSH_INTERVAL, analyticsExecutor, false, new CountDownLatch(0), true, false,
+        optOut, Crypto.none());
 
     Activity activity = mock(Activity.class);
     PackageManager packageManager = mock(PackageManager.class);
@@ -746,8 +736,9 @@ protected boolean matchesSafely(Application.ActivityLifecycleCallbacks item) {
 
     analytics = new Analytics(application, networkExecutor, stats, traitsCache, analyticsContext,
         defaultOptions, Logger.with(NONE), "qaz", Collections.singletonList(factory), client,
-        Cartographer.INSTANCE, "foo", DEFAULT_FLUSH_QUEUE_SIZE, DEFAULT_FLUSH_INTERVAL,
-        analyticsExecutor, false, new CountDownLatch(0), false, false, optOut, Crypto.none());
+        Cartographer.INSTANCE, projectSettingsCache, "foo", DEFAULT_FLUSH_QUEUE_SIZE,
+        DEFAULT_FLUSH_INTERVAL, analyticsExecutor, false, new CountDownLatch(0), false, false,
+        optOut, Crypto.none());
 
     Activity activity = mock(Activity.class);
     Bundle bundle = new Bundle();

diff --git a/analytics-tests/src/test/java/com/segment/analytics/ProjectSettingsTest.java b/analytics-tests/src/test/java/com/segment/analytics/ProjectSettingsTest.java
@@ -45,9 +45,9 @@ public class ProjectSettingsTest {
         + "  }\n"
         + "}";
     ProjectSettings projectSettings =
-        new ProjectSettings(cartographer.fromJson(projectSettingsJson));
+        ProjectSettings.create(cartographer.fromJson(projectSettingsJson));
 
-    assertThat(projectSettings).hasSize(4).containsKey("Segment.io");
+    assertThat(projectSettings).hasSize(5).containsKey("timestamp").containsKey("Segment.io");
 
     try {
       projectSettings.put("foo", "bar");

diff --git a/analytics/src/main/java/com/segment/analytics/Analytics.java b/analytics/src/main/java/com/segment/analytics/Analytics.java
@@ -33,7 +33,6 @@
 import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
-import android.net.http.HttpResponseCache;
 import android.os.Bundle;
 import android.os.Handler;
 import android.os.Looper;
@@ -50,7 +49,6 @@
 import com.segment.analytics.internal.Utils;
 import com.segment.analytics.internal.Utils.AnalyticsNetworkExecutorService;
 import java.io.BufferedWriter;
-import java.io.File;
 import java.io.IOException;
 import java.io.OutputStreamWriter;
 import java.io.Writer;
@@ -119,6 +117,7 @@ public class Analytics {
   final String tag;
   final Client client;
   final Cartographer cartographer;
+  private final ProjectSettings.Cache projectSettingsCache;
   final Crypto crypto;
   ProjectSettings projectSettings; // todo: make final (non-final for testing).
   @Private final String writeKey;
@@ -193,10 +192,11 @@ public static void setSingletonInstance(Analytics analytics) {
   Analytics(Application application, ExecutorService networkExecutor, Stats stats,
       Traits.Cache traitsCache, AnalyticsContext analyticsContext, Options defaultOptions,
       final Logger logger, String tag, final List<Integration.Factory> factories, Client client,
-      Cartographer cartographer, String writeKey, int flushQueueSize, long flushIntervalInMillis,
-      final ExecutorService analyticsExecutor, final boolean shouldTrackApplicationLifecycleEvents,
-      CountDownLatch advertisingIdLatch, final boolean shouldRecordScreenViews,
-      final boolean trackAttributionInformation, BooleanPreference optOut, Crypto crypto) {
+      Cartographer cartographer, ProjectSettings.Cache projectSettingsCache, String writeKey,
+      int flushQueueSize, long flushIntervalInMillis, final ExecutorService analyticsExecutor,
+      final boolean shouldTrackApplicationLifecycleEvents, CountDownLatch advertisingIdLatch,
+      final boolean shouldRecordScreenViews, final boolean trackAttributionInformation,
+      BooleanPreference optOut, Crypto crypto) {
     this.application = application;
     this.networkExecutor = networkExecutor;
     this.stats = stats;
@@ -207,6 +207,7 @@ public static void setSingletonInstance(Analytics analytics) {
     this.tag = tag;
     this.client = client;
     this.cartographer = cartographer;
+    this.projectSettingsCache = projectSettingsCache;
     this.writeKey = writeKey;
     this.flushQueueSize = flushQueueSize;
     this.flushIntervalInMillis = flushIntervalInMillis;
@@ -220,7 +221,7 @@ public static void setSingletonInstance(Analytics analytics) {
 
     analyticsExecutor.submit(new Runnable() {
       @Override public void run() {
-        projectSettings = downloadSettings();
+        projectSettings = getSettings();
         if (isNullOrEmpty(projectSettings)) {
           // Backup mode — Enable just the Segment integration.
           // {
@@ -230,7 +231,7 @@ public static void setSingletonInstance(Analytics analytics) {
           //     }
           //   }
           // }
-          projectSettings = new ProjectSettings(new ValueMap() //
+          projectSettings = ProjectSettings.create(new ValueMap() //
               .putValue("integrations", new ValueMap().putValue("Segment.io",
                   new ValueMap().putValue("apiKey", Analytics.this.writeKey))));
         }
@@ -1197,6 +1198,9 @@ public Analytics build() {
       final Cartographer cartographer = Cartographer.INSTANCE;
       final Client client = new Client(writeKey, connectionFactory);
 
+      ProjectSettings.Cache projectSettingsCache =
+          new ProjectSettings.Cache(application, cartographer, tag);
+
       BooleanPreference optOut =
           new BooleanPreference(getSegmentSharedPreferences(application, tag),
               OPT_OUT_PREFERENCE_KEY, false);
@@ -1218,51 +1222,65 @@ public Analytics build() {
       factories.addAll(this.factories);
 
       return new Analytics(application, networkExecutor, stats, traitsCache, analyticsContext,
-          defaultOptions, logger, tag, factories, client, cartographer, writeKey, flushQueueSize,
-          flushIntervalInMillis, Executors.newSingleThreadExecutor(),
+          defaultOptions, logger, tag, factories, client, cartographer, projectSettingsCache,
+          writeKey, flushQueueSize, flushIntervalInMillis, Executors.newSingleThreadExecutor(),
           trackApplicationLifecycleEvents, advertisingIdLatch, recordScreenViews,
           trackAttributionInformation, optOut, crypto);
     }
   }
 
   // Handler Logic.
+  private static final long SETTINGS_REFRESH_INTERVAL = 1000 * 60 * 60 * 24; // 24 hours
+  private static final long SETTINGS_RETRY_INTERVAL = 1000 * 60; // 1 minute
 
   private ProjectSettings downloadSettings() {
-    HttpResponseCache cache = HttpResponseCache.getInstalled();
-    if (cache == null) {
-      try {
-        File httpCacheDir = new File(application.getCacheDir(), "segment-" + tag);
-        long httpCacheSize = 10 * 1024 * 1024; // 10 MiB
-        cache = HttpResponseCache.install(httpCacheDir, httpCacheSize);
-      } catch (IOException e) {
-        logger.error(e, "HTTP response cache installation failed");
-      }
-    }
     try {
-      return networkExecutor.submit(new Callable<ProjectSettings>() {
+      ProjectSettings projectSettings = networkExecutor.submit(new Callable<ProjectSettings>() {
         @Override public ProjectSettings call() throws Exception {
           Client.Connection connection = null;
           try {
             connection = client.fetchSettings();
             Map<String, Object> map = cartographer.fromJson(buffer(connection.is));
-            return new ProjectSettings(map);
+            return ProjectSettings.create(map);
           } finally {
             closeQuietly(connection);
           }
         }
       }).get();
+      projectSettingsCache.set(projectSettings);
+      return projectSettings;
     } catch (InterruptedException e) {
       logger.error(e, "Thread interrupted while fetching settings.");
     } catch (ExecutionException e) {
-      logger.error(e, "Unable to fetch settings.");
-    } finally {
-      if (cache != null) {
-        cache.flush();
-      }
+      logger.error(e, "Unable to fetch settings. Retrying in %s ms.", SETTINGS_RETRY_INTERVAL);
     }
     return null;
   }
 
+  /**
+   * Retrieve settings from the cache or the network:
+   * 1. If the cache is empty, fetch new settings.
+   * 2. If the cache is not stale, use it.
+   * 2. If the cache is stale, try to get new settings.
+   */
+  @Private ProjectSettings getSettings() {
+    ProjectSettings cachedSettings = projectSettingsCache.get();
+    if (isNullOrEmpty(cachedSettings)) {
+      return downloadSettings();
+    }
+
+    long expirationTime = cachedSettings.timestamp() + SETTINGS_REFRESH_INTERVAL;
+    if (expirationTime > System.currentTimeMillis()) {
+      return cachedSettings;
+    }
+
+    ProjectSettings downloadedSettings = downloadSettings();
+    if (isNullOrEmpty(downloadedSettings)) {
+      return cachedSettings;
+    }
+    return downloadedSettings;
+  }
+
   void performInitializeIntegrations(ProjectSettings projectSettings) {
     ValueMap integrationSettings = projectSettings.integrations();
     integrations = new LinkedHashMap<>(factories.size());

diff --git a/analytics/src/main/java/com/segment/analytics/ProjectSettings.java b/analytics/src/main/java/com/segment/analytics/ProjectSettings.java
@@ -24,19 +24,32 @@
 
 package com.segment.analytics;
 
+import android.content.Context;
+import com.segment.analytics.internal.Private;
 import java.util.Map;
 
 import static java.util.Collections.unmodifiableMap;
 
 class ProjectSettings extends ValueMap {
+
+  private static final String TIMESTAMP_KEY = "timestamp";
   private static final String PLAN_KEY = "plan";
   private static final String INTEGRATIONS_KEY = "integrations";
   private static final String TRACKING_PLAN_KEY = "track";
 
-  ProjectSettings(Map<String, Object> map) {
+  static ProjectSettings create(Map<String, Object> map) {
+    map.put(TIMESTAMP_KEY, System.currentTimeMillis());
+    return new ProjectSettings(map);
+  }
+
+  @Private ProjectSettings(Map<String, Object> map) {
     super(unmodifiableMap(map));
   }
 
+  long timestamp() {
+    return getLong(TIMESTAMP_KEY, 0L);
+  }
+
   ValueMap plan() {
     return getValueMap(PLAN_KEY);
   }
@@ -52,4 +65,20 @@ ValueMap trackingPlan() {
   ValueMap integrations() {
     return getValueMap(INTEGRATIONS_KEY);
   }
+
+  static class Cache extends ValueMap.Cache<ProjectSettings> {
+
+    // todo: remove. This is legacy behaviour from before we started namespacing the entire shared
+    // preferences object and were namespacing keys instead.
+    private static final String PROJECT_SETTINGS_CACHE_KEY_PREFIX = "project-settings-plan-";
+
+    Cache(Context context, Cartographer cartographer, String tag) {
+      super(context, cartographer, PROJECT_SETTINGS_CACHE_KEY_PREFIX + tag, tag,
+          ProjectSettings.class);
+    }
+
+    @Override public ProjectSettings create(Map<String, Object> map) {
+      return new ProjectSettings(map);
+    }
+  }
 }