Merge pull request #5 from AdevintaSpain/fix-disabled-diffing

Fix crash when trait diffing is disabled

src/androidTest/java/com/segment/analytics/android/integrations/appboy/AppboyAndroidTest.java

@@ -33,8 +33,8 @@ public void testIdentifyCallsChangeUser() {
     Traits traits = createTraits(testUserId);
     IdentifyPayload identifyPayload = new IdentifyPayloadBuilder().traits(traits).build();
     Logger logger = Logger.with(Analytics.LogLevel.DEBUG);
-    AppboyIntegration integration = new AppboyIntegration(Appboy.getInstance(getContext()), "foo", logger, true,
-        new PreferencesTraitsCache(getContext()), new DefaultUserIdMapper());
+    AppboyIntegration integration = new AppboyIntegration(getContext(), Appboy.getInstance(getContext()), "foo", logger, true,
+        true, null);
     integration.identify(identifyPayload);
 
     assertEquals(testUserId, Appboy.getInstance(getContext()).getCurrentUser().getUserId());

src/androidTest/java/com/segment/analytics/android/integrations/appboy/AppboyIntegrationOptionsAndroidTest.java

@@ -6,10 +6,8 @@
 import com.appboy.configuration.AppboyConfig;
 import com.segment.analytics.Analytics;
 import com.segment.analytics.Traits;
-import com.segment.analytics.integrations.IdentifyPayload;
 import com.segment.analytics.integrations.Logger;
 import com.segment.analytics.test.IdentifyPayloadBuilder;
-import org.junit.After;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -20,6 +18,7 @@
 import org.mockito.MockitoAnnotations;
 
 import static android.support.test.InstrumentationRegistry.getContext;
+import static android.support.test.InstrumentationRegistry.getTargetContext;
 import static com.segment.analytics.Utils.createTraits;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
@@ -42,7 +41,6 @@ public class AppboyIntegrationOptionsAndroidTest {
   @Mock AppboyUser appboyUser;
 
   private AppboyIntegration appboyIntegration;
-  private PreferencesTraitsCache traitsCache;
 
   @BeforeClass
   public static void beforeClass() {
@@ -56,22 +54,16 @@ public void setUp() throws Exception {
 
     when(appboy.getCurrentUser()).thenReturn(appboyUser);
 
-    Logger logger = Logger.with(Analytics.LogLevel.DEBUG);
-
-    traitsCache = new PreferencesTraitsCache(getContext());
-
-    appboyIntegration = new AppboyIntegration(
-        appboy, "foo", logger, true,
-        traitsCache, new ReplaceUserIdMapper());
-  }
-
-  @After
-  public void tearDown() throws Exception {
-    traitsCache.clear();
+    new PreferencesTraitsCache(getTargetContext()).clear();
   }
 
   @Test
   public void testUserIdMapperTransformsAppboyUserId() {
+    givenIntegrationWithOptions(AppboyIntegrationOptions.builder()
+        .enableTraitDiffing(true)
+        .userIdMapper(new ReplaceUserIdMapper())
+        .build()
+    );
     Traits traits = createTraits(USER_ID);
 
     callIdentifyWithTraits(traits);
@@ -81,6 +73,10 @@ public void testUserIdMapperTransformsAppboyUserId() {
 
   @Test
   public void testShouldNotTriggerAppboyUpdateIfTraitDoesntChange() {
+    givenIntegrationWithOptions(AppboyIntegrationOptions.builder()
+        .enableTraitDiffing(true)
+        .build()
+    );
     Traits traits = createTraits(USER_ID);
 
     Traits.Address address = new Traits.Address();
@@ -103,6 +99,31 @@ public void testShouldNotTriggerAppboyUpdateIfTraitDoesntChange() {
 
   @Test
   public void testShouldTriggerUpdateIfTraitChanges() {
+    givenIntegrationWithOptions(AppboyIntegrationOptions.builder()
+        .enableTraitDiffing(true)
+        .build()
+    );
+    Traits traits = createTraits(USER_ID);
+    traits.putEmail(TRAIT_EMAIL);
+    callIdentifyWithTraits(traits);
+    callIdentifyWithTraits(traits);
+
+    Traits traitsUpdate = createTraits(USER_ID);
+    traitsUpdate.putEmail(TRAIT_EMAIL_UPDATED);
+    callIdentifyWithTraits(traitsUpdate);
+    callIdentifyWithTraits(traitsUpdate);
+
+    InOrder inOrder = Mockito.inOrder(appboyUser);
+    inOrder.verify(appboyUser, times(1)).setEmail(TRAIT_EMAIL);
+    inOrder.verify(appboyUser, times(1)).setEmail(TRAIT_EMAIL_UPDATED);
+  }
+
+  @Test
+  public void testShouldNotTriggerUpdateIfTraitDiffingDisabled() {
+    givenIntegrationWithOptions(AppboyIntegrationOptions.builder()
+        .enableTraitDiffing(false)
+        .build()
+    );
     Traits traits = createTraits(USER_ID);
     traits.putEmail(TRAIT_EMAIL);
     callIdentifyWithTraits(traits);
@@ -120,6 +141,11 @@ public void testShouldTriggerUpdateIfTraitChanges() {
 
   @Test
   public void testAvoidTriggeringRepeatedUserIdUpdates() {
+    givenIntegrationWithOptions(AppboyIntegrationOptions.builder()
+        .enableTraitDiffing(true)
+        .userIdMapper(new ReplaceUserIdMapper())
+        .build()
+    );
     Traits traits = createTraits(USER_ID);
     traits.putEmail(TRAIT_EMAIL);
 
@@ -132,6 +158,10 @@ public void testAvoidTriggeringRepeatedUserIdUpdates() {
 
   @Test
   public void clearCacheIfUserIdChanges() {
+    givenIntegrationWithOptions(AppboyIntegrationOptions.builder()
+        .enableTraitDiffing(true)
+        .build()
+    );
     Traits traits = createTraits(USER_ID);
     traits.putEmail(TRAIT_EMAIL);
 
@@ -146,6 +176,10 @@ public void clearCacheIfUserIdChanges() {
 
   @Test
   public void clearCacheOnReset() {
+    givenIntegrationWithOptions(AppboyIntegrationOptions.builder()
+        .enableTraitDiffing(true)
+        .build()
+    );
     Traits traits = createTraits(USER_ID);
     traits.putEmail(TRAIT_EMAIL);
 
@@ -158,10 +192,18 @@ public void clearCacheOnReset() {
     verify(appboyUser, times(2)).setEmail(TRAIT_EMAIL);
   }
 
-  public void callIdentifyWithTraits(Traits traits) {
-    IdentifyPayload identifyPayload = new IdentifyPayloadBuilder().traits(traits).build();
+  private void givenIntegrationWithOptions(AppboyIntegrationOptions options) {
+    appboyIntegration = new AppboyIntegration(getTargetContext(), appboy,
+        "foo",
+        Logger.with(Analytics.LogLevel.DEBUG),
+        true,
+        options.isTraitDiffingEnabled(),
+        options.getUserIdMapper()
+    );
+  }
 
-    appboyIntegration.identify(identifyPayload);
+  private void callIdentifyWithTraits(Traits traits) {
+    appboyIntegration.identify(new IdentifyPayloadBuilder().traits(traits).build());
   }
 
   class ReplaceUserIdMapper implements UserIdMapper {

src/main/java/com/segment/analytics/android/integrations/appboy/AppboyIntegration.java

@@ -1,7 +1,9 @@
 package com.segment.analytics.android.integrations.appboy;
 
 import android.app.Activity;
-
+import android.content.Context;
+import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
 import com.appboy.Appboy;
 import com.appboy.configuration.AppboyConfig;
 import com.appboy.enums.Gender;
@@ -19,18 +21,16 @@
 import com.segment.analytics.integrations.Integration;
 import com.segment.analytics.integrations.Logger;
 import com.segment.analytics.integrations.TrackPayload;
-
-import java.util.Map;
-import org.json.JSONObject;
-
 import java.math.BigDecimal;
 import java.util.Arrays;
 import java.util.Calendar;
 import java.util.Date;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 import java.util.Set;
+import org.json.JSONObject;
 
 public class AppboyIntegration extends Integration<Appboy> {
   private static final String APPBOY_KEY = "Appboy";
@@ -73,20 +73,11 @@ public Integration<?> create(ValueMap settings, Analytics analytics) {
           builder.setCustomEndpoint(customEndpoint);
         }
 
-        UserIdMapper userIdMapper = options.getUserIdMapper();
-        if (userIdMapper == null) {
-          userIdMapper = new DefaultUserIdMapper();
-        }
-
-        TraitsCache traitsCache = null;
-        if (options.isTraitDiffingEnabled()) {
-          traitsCache = new PreferencesTraitsCache(analytics.getApplication());
-        }
-
         Appboy.configure(analytics.getApplication().getApplicationContext(), builder.build());
         Appboy appboy = Appboy.getInstance(analytics.getApplication());
         logger.verbose("Configured Appboy+Segment integration and initialized Appboy.");
-        return new AppboyIntegration(appboy, apiKey, logger, inAppMessageRegistrationEnabled, traitsCache, userIdMapper);
+        return new AppboyIntegration(analytics.getApplication(), appboy, apiKey, logger, inAppMessageRegistrationEnabled,
+            options.isTraitDiffingEnabled(), options.getUserIdMapper());
       }
 
       @Override
@@ -100,19 +91,21 @@ public String key() {
   private final String mToken;
   private final Logger mLogger;
   private final boolean mAutomaticInAppMessageRegistrationEnabled;
+  @NonNull
   private final UserIdMapper mUserIdMapper;
+  @Nullable
   private final TraitsCache mTraitsCache;
 
-  public AppboyIntegration(Appboy appboy, String token, Logger logger,
-                           boolean automaticInAppMessageRegistrationEnabled,
-                           TraitsCache traitsCache,
-                           UserIdMapper userIdMapper) {
+  public AppboyIntegration(Context context, Appboy appboy, String token, Logger logger,
+      boolean automaticInAppMessageRegistrationEnabled,
+      boolean enableTraitDiffing,
+      @Nullable UserIdMapper userIdMapper) {
     mAppboy = appboy;
     mToken = token;
     mLogger = logger;
     mAutomaticInAppMessageRegistrationEnabled = automaticInAppMessageRegistrationEnabled;
-    mUserIdMapper = userIdMapper;
-    mTraitsCache = traitsCache;
+    mUserIdMapper = userIdMapper != null ? userIdMapper : new DefaultUserIdMapper();
+    mTraitsCache = enableTraitDiffing ? new PreferencesTraitsCache(context) : null;
   }
 
   public String getToken() {
@@ -129,25 +122,17 @@ public void identify(IdentifyPayload identify) {
     super.identify(identify);
 
     String userId = identify.userId();
-    if (!StringUtils.isNullOrBlank(userId)) {
-
-      String cachedUserId = mTraitsCache.load().userId();
-
-      if (!userId.equals(cachedUserId)) {
-        mAppboy.changeUser(mUserIdMapper.transformUserId(userId));
+    String cachedUserId = mTraitsCache != null ? mTraitsCache.load().userId() : null;
+    if (!StringUtils.isNullOrBlank(userId) && !userId.equals(cachedUserId)) {
+      mAppboy.changeUser(mUserIdMapper.transformUserId(userId));
 
-        if (mTraitsCache != null) {
-          mTraitsCache.clear();
-        }
+      if (mTraitsCache != null) {
+        mTraitsCache.clear();
       }
     }
 
     Traits traits = identify.traits();
 
-    if (traits == null) {
-      return;
-    }
-
     if (mTraitsCache != null) {
       Traits lastEmittedTraits = mTraitsCache.load();
 

src/test/java/com/segment/analytics/android/integrations/appboy/AppboyTest.java

@@ -74,7 +74,7 @@ public void setUp() {
     when(mAnalytics.logger("Appboy")).thenReturn(mLogger);
     when(mAnalytics.getApplication()).thenReturn(mContext);
     mIntegration = new AppboyIntegration(
-        mAppboy, "foo", mLogger, true, new InMemoryTraitsCache(), new DefaultUserIdMapper());
+        mContext, mAppboy, "foo", mLogger, true, true, null);
   }
 
   @Test