NR-364665: setUserId tweaks

Agent/Analytics/NRMASAM.mm

@@ -89,9 +89,18 @@ -  (BOOL) setAttribute:(NSString*)name value:(id)value {
 
 -  (BOOL) checkAttribute:(NSString*)name value:(id)value {
     BOOL validAttribute = [attributeValidator nameValidator:name];
-    BOOL validValue = [attributeValidator valueValidator:value];
 
-    if (name == kNRMA_Attrib_userId && !value ) {
+    // We allow userId to be nil, so skip value validation in that case to prevent errant log.
+    BOOL validValue = NO;
+    if ([name isEqualToString: kNRMA_Attrib_userId] && !value ) {
+        validValue = YES;
+    }
+    else {
+        validValue = [attributeValidator valueValidator:value];
+    }
+
+
+    if ([name isEqualToString: kNRMA_Attrib_userId] && !value ) {
         NRLOG_AGENT_VERBOSE(@"Successfully set userId to nil");
         return YES;
     }

Agent/Public/NewRelic.m

@@ -631,21 +631,22 @@ + (BOOL) incrementAttribute:(NSString*)name
 
 + (BOOL) setUserId:(NSString* _Nullable)userId {
     NSString *previousUserId = [[NewRelicAgentInternal sharedInstance] getUserId];
+
     BOOL newSession = false;
-    // If the client passes a new userId that is non NULL.
-    if (userId != NULL) {
-        // A new userId has been set where the previously set one (during this app session (since app launch)) was not NULL or the previous set one was NULL, we start a new session.
+
+    // If userId has been set before and is different from the current userId, then we need to start a new session.
+    if ([previousUserId isEqualToString: userId] == NO) {
+        // end session and harvest.
         newSession = true;
     }
-    // If the client passes a new NULL userId.
-    else {
-        if (previousUserId != NULL) {
-            // end session and harvest.
-            newSession = true;
-        }
         // Do nothing if passed userId is null and saved userId (for this app session (since app launch)) is null.
+    if (userId == nil) {
+        // end session and harvest.
+        newSession = true;
     }
 
+    NRLOG_AGENT_VERBOSE(@"setUserId: %@ and previousUserId: %@ and will start newSession=%d", userId, previousUserId, newSession);
+
     // Update in memory userId.
     [NewRelicAgentInternal sharedInstance].userId = userId;
 

Tests/Unit-Tests/NewRelicAgentTests/API-Tests/NRMAAPIHelperTests.m

@@ -34,8 +34,9 @@ - (void) testSetUserId {
 
         XCTAssert([analytics setUserId:@"AUniqueId1"], @"Good input produced incorrect result");
 
-        XCTAssertFalse([analytics setUserId:nil], @"bad input produced a incorrect result");
-
+        // Setting userId to nil is allowed.
+        XCTAssertTrue([analytics setUserId:nil], @"bad input produced a incorrect result");
+
         XCTAssertFalse([analytics setUserId:@""], @"bad input produced a incorrect result");
 }
 @end