Payload Info & Traits Fixes (#912)

* Moved some fields from SEGContext to more appropriate SEGPayload. * Fix trait storage/init issue. * Fixed traits usage w/ tests. Co-authored-by: Brandon Sneed <[email protected]>
segmentio · Jun 25, 2020 · 61f487d · 61f487d
1 parent 60e6abb
commit 61f487d
Show file tree

Hide file tree

Showing 12 changed files with 108 additions and 103 deletions.
diff --git a/Analytics/Classes/SEGAnalytics.m b/Analytics/Classes/SEGAnalytics.m
@@ -224,11 +224,19 @@ - (void)identify:(NSString *)userId traits:(NSDictionary *)traits options:(NSDic
     }
     // configure traits to match what is seen on android.
     NSMutableDictionary *newTraits = [traits mutableCopy];
+    // if no traits were passed in, need to create.
+    if (newTraits == nil) {
+        newTraits = [[NSMutableDictionary alloc] init];
+    }
     newTraits[@"anonymousId"] = anonId;
     if (userId != nil) {
         newTraits[@"userId"] = userId;
         [SEGState sharedInstance].userInfo.userId = userId;
     }
+    // merge w/ existing traits and set them.
+    NSDictionary *existingTraits = [SEGState sharedInstance].userInfo.traits;
+    [newTraits addEntriesFromDictionary:existingTraits];
+    [SEGState sharedInstance].userInfo.traits = newTraits;
 
     [self run:SEGEventTypeIdentify payload:
                                        [[SEGIdentifyPayload alloc] initWithUserId:userId
@@ -481,8 +489,12 @@ - (void)run:(SEGEventType)eventType payload:(SEGPayload *)payload
         ctx.eventType = eventType;
         ctx.payload = payload;
         ctx.payload.messageId = GenerateUUIDString();
-        ctx.anonymousId = [SEGState sharedInstance].userInfo.anonymousId;
-        ctx.userId = [SEGState sharedInstance].userInfo.userId;
+        if (ctx.payload.userId == nil) {
+            ctx.payload.userId = [SEGState sharedInstance].userInfo.userId;
+        }
+        if (ctx.payload.anonymousId == nil) {
+            ctx.payload.anonymousId = [SEGState sharedInstance].userInfo.anonymousId;
+        }
     }];
 
     // Could probably do more things with callback later, but we don't use it yet.

diff --git a/Analytics/Classes/SEGContext.h b/Analytics/Classes/SEGContext.h
@@ -59,8 +59,6 @@ NS_SWIFT_NAME(Context)
 @property (nonatomic, readonly, nonnull) SEGAnalytics *_analytics;
 @property (nonatomic, readonly) SEGEventType eventType;
 
-@property (nonatomic, readonly, nullable) NSString *userId;
-@property (nonatomic, readonly, nullable) NSString *anonymousId;
 @property (nonatomic, readonly, nullable) NSError *error;
 @property (nonatomic, readonly, nullable) SEGPayload *payload;
 @property (nonatomic, readonly) BOOL debug;
@@ -74,8 +72,6 @@ NS_SWIFT_NAME(Context)
 @protocol SEGMutableContext <NSObject>
 
 @property (nonatomic) SEGEventType eventType;
-@property (nonatomic, nullable) NSString *userId;
-@property (nonatomic, nullable) NSString *anonymousId;
 @property (nonatomic, nullable) SEGPayload *payload;
 @property (nonatomic, nullable) NSError *error;
 @property (nonatomic) BOOL debug;

diff --git a/Analytics/Classes/SEGContext.m b/Analytics/Classes/SEGContext.m
@@ -73,8 +73,6 @@ - (id)copyWithZone:(NSZone *)zone
 {
     SEGContext *ctx = [[SEGContext allocWithZone:zone] initWithAnalytics:self._analytics];
     ctx.eventType = self.eventType;
-    ctx.userId = self.userId;
-    ctx.anonymousId = self.anonymousId;
     ctx.payload = self.payload;
     ctx.error = self.error;
     ctx.debug = self.debug;

diff --git a/Analytics/Classes/SEGIdentifyPayload.h b/Analytics/Classes/SEGIdentifyPayload.h
@@ -6,10 +6,6 @@ NS_ASSUME_NONNULL_BEGIN
 NS_SWIFT_NAME(IdentifyPayload)
 @interface SEGIdentifyPayload : SEGPayload
 
-@property (nonatomic, readonly, nullable) NSString *userId;
-
-@property (nonatomic, readonly, nullable) NSString *anonymousId;
-
 @property (nonatomic, readonly, nullable) JSON_DICT traits;
 
 - (instancetype)initWithUserId:(NSString *)userId

diff --git a/Analytics/Classes/SEGIdentifyPayload.m b/Analytics/Classes/SEGIdentifyPayload.m
@@ -1,9 +1,5 @@
 #import "SEGIdentifyPayload.h"
 
-@interface SEGIdentifyPayload ()
-@property (nonatomic, readwrite, nullable) NSString *anonymousId;
-@end
-
 @implementation SEGIdentifyPayload
 
 - (instancetype)initWithUserId:(NSString *)userId
@@ -13,9 +9,9 @@ - (instancetype)initWithUserId:(NSString *)userId
                   integrations:(NSDictionary *)integrations
 {
     if (self = [super initWithContext:context integrations:integrations]) {
-        _userId = [userId copy];
-        _anonymousId = [anonymousId copy];
         _traits = [traits copy];
+        self.anonymousId = [anonymousId copy];
+        self.userId = [userId copy];
     }
     return self;
 }

diff --git a/Analytics/Classes/SEGPayload.h b/Analytics/Classes/SEGPayload.h
@@ -10,6 +10,8 @@ NS_SWIFT_NAME(Payload)
 @property (nonatomic, readonly) JSON_DICT integrations;
 @property (nonatomic, strong) NSString *timestamp;
 @property (nonatomic, strong) NSString *messageId;
+@property (nonatomic, strong) NSString *anonymousId;
+@property (nonatomic, strong) NSString *userId;
 
 - (instancetype)initWithContext:(JSON_DICT)context integrations:(JSON_DICT)integrations;
 

diff --git a/Analytics/Classes/SEGPayload.m b/Analytics/Classes/SEGPayload.m
@@ -3,6 +3,9 @@
 
 @implementation SEGPayload
 
+@synthesize userId = _userId;
+@synthesize anonymousId = _anonymousId;
+
 - (instancetype)initWithContext:(NSDictionary *)context integrations:(NSDictionary *)integrations
 {
     if (self = [super init]) {
@@ -15,6 +18,9 @@ - (instancetype)initWithContext:(NSDictionary *)context integrations:(NSDictiona
 
         _context = [combinedContext copy];
         _integrations = [integrations copy];
+        _messageId = nil;
+        _userId = nil;
+        _anonymousId = nil;
     }
     return self;
 }
@@ -23,20 +29,16 @@ - (instancetype)initWithContext:(NSDictionary *)context integrations:(NSDictiona
 
 
 @implementation SEGApplicationLifecyclePayload
-
 @end
 
 
 @implementation SEGRemoteNotificationPayload
-
 @end
 
 
 @implementation SEGContinueUserActivityPayload
-
 @end
 
 
 @implementation SEGOpenURLPayload
-
 @end
diff --git a/Analytics/Classes/SEGSegmentIntegration.m b/Analytics/Classes/SEGSegmentIntegration.m
@@ -36,7 +36,7 @@ @interface SEGSegmentIntegration ()
 @property (nonatomic, strong) NSTimer *flushTimer;
 @property (nonatomic, strong) dispatch_queue_t serialQueue;
 @property (nonatomic, strong) dispatch_queue_t backgroundTaskQueue;
-@property (nonatomic, strong) NSMutableDictionary *traits;
+@property (nonatomic, strong) NSDictionary *traits;
 @property (nonatomic, assign) SEGAnalytics *analytics;
 @property (nonatomic, assign) SEGAnalyticsConfiguration *configuration;
 @property (atomic, copy) NSDictionary *referrer;
@@ -68,6 +68,9 @@ - (id)initWithAnalytics:(SEGAnalytics *)analytics httpClient:(SEGHTTPClient *)ht
         self.serialQueue = seg_dispatch_queue_create_specific("io.segment.analytics.segmentio", DISPATCH_QUEUE_SERIAL);
         self.backgroundTaskQueue = seg_dispatch_queue_create_specific("io.segment.analytics.backgroundTask", DISPATCH_QUEUE_SERIAL);
         self.flushTaskID = UIBackgroundTaskInvalid;
+
+        // load traits from disk.
+        [self loadTraits];
 
         [self dispatchBackground:^{
             // Check for previous queue data in NSUserDefaults and remove if present.
@@ -97,63 +100,6 @@ - (id)initWithAnalytics:(SEGAnalytics *)analytics httpClient:(SEGHTTPClient *)ht
     return self;
 }
 
-/*
- * There is an iOS bug that causes instances of the CTTelephonyNetworkInfo class to
- * sometimes get notifications after they have been deallocated.
- * Instead of instantiating, using, and releasing instances you * must instead retain
- * and never release them to work around the bug.
- *
- * Ref: http://stackoverflow.com/questions/14238586/coretelephony-crash
- */
-
-#if TARGET_OS_IOS
-static CTTelephonyNetworkInfo *_telephonyNetworkInfo;
-#endif
-
-- (NSDictionary *)liveContext
-{
-    NSMutableDictionary *context = [[NSMutableDictionary alloc] init];
-    context[@"locale"] = [NSString stringWithFormat:
-                                       @"%@-%@",
-                                       [NSLocale.currentLocale objectForKey:NSLocaleLanguageCode],
-                                       [NSLocale.currentLocale objectForKey:NSLocaleCountryCode]];
-
-    context[@"timezone"] = [[NSTimeZone localTimeZone] name];
-
-    context[@"network"] = ({
-        NSMutableDictionary *network = [[NSMutableDictionary alloc] init];
-
-        if (self.reachability.isReachable) {
-            network[@"wifi"] = @(self.reachability.isReachableViaWiFi);
-            network[@"cellular"] = @(self.reachability.isReachableViaWWAN);
-        }
-
-#if TARGET_OS_IOS
-        static dispatch_once_t networkInfoOnceToken;
-        dispatch_once(&networkInfoOnceToken, ^{
-            _telephonyNetworkInfo = [[CTTelephonyNetworkInfo alloc] init];
-        });
-
-        CTCarrier *carrier = [_telephonyNetworkInfo subscriberCellularProvider];
-        if (carrier.carrierName.length)
-            network[@"carrier"] = carrier.carrierName;
-#endif
-
-        network;
-    });
-
-    context[@"traits"] = ({
-        NSMutableDictionary *traits = [[NSMutableDictionary alloc] initWithDictionary:[self traits]];
-        traits;
-    });
-
-    if (self.referrer) {
-        context[@"referrer"] = [self.referrer copy];
-    }
-
-    return [context copy];
-}
-
 - (void)dispatchBackground:(void (^)(void))block
 {
     seg_dispatch_specific_async(_serialQueue, block);
@@ -217,10 +163,19 @@ - (void)saveUserId:(NSString *)userId
     }];
 }
 
-- (void)addTraits:(NSDictionary *)traits
+- (NSDictionary *)traits
+{
+    return [SEGState sharedInstance].userInfo.traits;
+}
+
+- (void)setTraits:(NSDictionary *)traits
+{
+    [self saveTraits:traits];
+}
+
+- (void)saveTraits:(NSDictionary *)traits
 {
     [self dispatchBackground:^{
-        [self.traits addEntriesFromDictionary:traits];
         [SEGState sharedInstance].userInfo.traits = traits;
 #if TARGET_OS_TV
         [self.userDefaultsStorage setDictionary:[self.traits copy] forKey:SEGTraitsKey];
@@ -236,7 +191,7 @@ - (void)identify:(SEGIdentifyPayload *)payload
 {
     [self dispatchBackground:^{
         [self saveUserId:payload.userId];
-        [self addTraits:payload.traits];
+        [self saveTraits:payload.traits];
     }];
 
     NSMutableDictionary *dictionary = [NSMutableDictionary dictionary];
@@ -474,17 +429,17 @@ - (NSMutableArray *)queue
     return _queue;
 }
 
-- (NSMutableDictionary *)traits
+- (void)loadTraits
 {
-    if (!_traits) {
+    if (![SEGState sharedInstance].userInfo.traits) {
+        NSDictionary *traits = nil;
 #if TARGET_OS_TV
-        _traits = [[self.userDefaultsStorage dictionaryForKey:SEGTraitsKey] ?: @{} mutableCopy];
+        traits = [[self.userDefaultsStorage dictionaryForKey:SEGTraitsKey] ?: @{} mutableCopy];
 #else
-        _traits = [[self.fileStorage dictionaryForKey:kSEGTraitsFilename] ?: @{} mutableCopy];
+        traits = [[self.fileStorage dictionaryForKey:kSEGTraitsFilename] ?: @{} mutableCopy];
 #endif
+        [SEGState sharedInstance].userInfo.traits = traits;
     }
-    [SEGState sharedInstance].userInfo.traits = _traits;
-    return _traits;
 }
 
 - (NSUInteger)maxBatchSize

diff --git a/Analytics/Internal/SEGIntegrationsManager.m b/Analytics/Internal/SEGIntegrationsManager.m
@@ -191,12 +191,13 @@ - (void)identify:(SEGIdentifyPayload *)payload
     NSCAssert2(payload.userId.length > 0 || payload.traits.count > 0, @"either userId (%@) or traits (%@) must be provided.", payload.userId, payload.traits);
 
     NSString *anonymousId = payload.anonymousId;
-    if (anonymousId) {
+    NSString *existingAnonymousId = self.cachedAnonymousId;
+
+    if (anonymousId == nil) {
+        payload.anonymousId = anonymousId;
+    } else if (![anonymousId isEqualToString:existingAnonymousId]) {
         [self saveAnonymousId:anonymousId];
-    } else {
-        anonymousId = self.cachedAnonymousId;
     }
-    payload.anonymousId = anonymousId;
 
     [self callIntegrationsWithSelector:NSSelectorFromString(@"identify:")
                              arguments:@[ payload ]

diff --git a/AnalyticsTests/AnalyticsTests.swift b/AnalyticsTests/AnalyticsTests.swift
@@ -88,6 +88,50 @@ class AnalyticsTests: XCTestCase {
         XCTAssertEqual(analytics2.test_integrationsManager()?.test_segmentIntegration()?.test_userId(), "testUserId1")
     }
 
+    func testPersistsTraits() {
+        analytics.identify("testUserId1", traits: ["trait1": "someTrait"])
+
+        let analytics2 = Analytics(configuration: config)
+        analytics2.test_integrationsManager()?.test_setCachedSettings(settings: cachedSettings)
+
+        XCTAssertEqual(analytics.test_integrationsManager()?.test_segmentIntegration()?.test_userId(), "testUserId1")
+        XCTAssertEqual(analytics2.test_integrationsManager()?.test_segmentIntegration()?.test_userId(), "testUserId1")
+
+        var traits = analytics.test_integrationsManager()?.test_segmentIntegration()?.test_traits()
+        var storedTraits = analytics2.test_integrationsManager()?.test_segmentIntegration()?.test_traits()
+
+        if let trait1 = traits?["trait1"] as? String {
+            XCTAssertEqual(trait1, "someTrait")
+        } else {
+            XCTAssert(false, "Traits are nil!")
+        }
+
+        if let storedTrait1 = storedTraits?["trait1"] as? String {
+            XCTAssertEqual(storedTrait1, "someTrait")
+        } else {
+            XCTAssert(false, "Traits were not stored!")
+        }
+
+        analytics.identify("testUserId1", traits: ["trait2": "someOtherTrait"])
+
+        traits = analytics.test_integrationsManager()?.test_segmentIntegration()?.test_traits()
+        storedTraits = analytics2.test_integrationsManager()?.test_segmentIntegration()?.test_traits()
+
+        if let trait1 = traits?["trait2"] as? String {
+            XCTAssertEqual(trait1, "someOtherTrait")
+        } else {
+            XCTAssert(false, "Traits are nil!")
+        }
+
+        if let storedTrait1 = storedTraits?["trait2"] as? String {
+            XCTAssertEqual(storedTrait1, "someOtherTrait")
+        } else {
+            XCTAssert(false, "Traits were not stored!")
+        }
+
+
+    }
+
     func testClearsUserData() {
         analytics.identify("testUserId1", traits: [ "Test trait key" : "Test trait value"])
         analytics.reset()