Fix potential data race condition

BranchMetrics · Feb 15, 2024 · 9ff026e · 9ff026e
1 parent 346d209
commit 9ff026e
Show file tree

Hide file tree

Showing 9 changed files with 123 additions and 185 deletions.
diff --git a/Branch-TestBed/Branch-SDK-Tests/BNCRequestFactoryTests.m b/Branch-TestBed/Branch-SDK-Tests/BNCRequestFactoryTests.m
@@ -25,7 +25,7 @@ - (void)tearDown {
 
 - (void)testInitWithBranchKeyNil {
     BNCRequestFactory *factory = [[BNCRequestFactory alloc] initWithBranchKey:nil];
-    NSDictionary *json = [factory dataForInstall];
+    NSDictionary *json = [factory dataForInstallWithURLString:@"https://branch.io"];
     XCTAssertNotNil(json);
 
     // key is omitted when nil
@@ -34,7 +34,7 @@ - (void)testInitWithBranchKeyNil {
 
 - (void)testInitWithBranchKeyEmpty {
     BNCRequestFactory *factory = [[BNCRequestFactory alloc] initWithBranchKey:@""];
-    NSDictionary *json = [factory dataForInstall];
+    NSDictionary *json = [factory dataForInstallWithURLString:@"https://branch.io"];
     XCTAssertNotNil(json);
 
     // empty string is allowed
@@ -43,14 +43,14 @@ - (void)testInitWithBranchKeyEmpty {
 
 - (void)testInitWithBranchKey {
     BNCRequestFactory *factory = [[BNCRequestFactory alloc] initWithBranchKey:@"key_abcd"];
-    NSDictionary *json = [factory dataForInstall];
+    NSDictionary *json = [factory dataForInstallWithURLString:@"https://branch.io"];
     XCTAssertNotNil(json);
     XCTAssertTrue([@"key_abcd" isEqualToString:[json objectForKey:@"branch_key"]]);
 }
 
 - (void)testDataForInstall {
     BNCRequestFactory *factory = [[BNCRequestFactory alloc] initWithBranchKey:@"key_abcd"];
-    NSDictionary *json = [factory dataForInstall];
+    NSDictionary *json = [factory dataForInstallWithURLString:@"https://branch.io"];
     XCTAssertNotNil(json);
 
     XCTAssertTrue([@"key_abcd" isEqualToString:[json objectForKey:@"branch_key"]]);
@@ -64,7 +64,7 @@ - (void)testDataForInstall {
 
 - (void)testDataForOpen {
     BNCRequestFactory *factory = [[BNCRequestFactory alloc] initWithBranchKey:@"key_abcd"];
-    NSDictionary *json = [factory dataForOpen];
+    NSDictionary *json = [factory dataForOpenWithURLString:@"https://branch.io"];
     XCTAssertNotNil(json);
 
     XCTAssertTrue([@"key_abcd" isEqualToString:[json objectForKey:@"branch_key"]]);
@@ -182,13 +182,13 @@ - (void)testDataForEventNil {
 
 - (void)testDataForShortURL {
     BNCRequestFactory *factory = [[BNCRequestFactory alloc] initWithBranchKey:@"key_abcd"];
-    NSDictionary *json = [factory dataForInstall];
+    NSDictionary *json = [factory dataForShortURLWithLinkDataDictionary:@{}.mutableCopy isSpotlightRequest:NO];
     XCTAssertNotNil(json);
 }
 
 - (void)testDataForLATD {
     BNCRequestFactory *factory = [[BNCRequestFactory alloc] initWithBranchKey:@"key_abcd"];
-    NSDictionary *json = [factory dataForInstall];
+    NSDictionary *json = [factory dataForLATDWithDataDictionary:@{}.mutableCopy];
     XCTAssertNotNil(json);
 }
 

diff --git a/Sources/BranchSDK/BNCRequestFactory.m b/Sources/BranchSDK/BNCRequestFactory.m
@@ -76,7 +76,7 @@ - (BOOL)isTrackingDisabled {
     return Branch.trackingDisabled;
 }
 
-- (NSDictionary *)dataForInstall {
+- (NSDictionary *)dataForInstallWithURLString:(NSString *)urlString {
     NSMutableDictionary *json = [NSMutableDictionary new];
 
     // All requests
@@ -100,6 +100,10 @@ - (NSDictionary *)dataForInstall {
     [self addAppleReceiptSourceToJSON:json];
     [self addTimestampsToJSON:json];
 
+    if (urlString) {
+        [self safeSetValue:urlString forKey:BRANCH_REQUEST_KEY_UNIVERSAL_LINK_URL onDict:json];
+    }
+
     [self addAppleAttributionTokenToJSON:json];
 
     // Install Only
@@ -116,7 +120,7 @@ - (NSDictionary *)dataForInstall {
     return json;
 }
 
-- (NSDictionary *)dataForOpen {
+- (NSDictionary *)dataForOpenWithURLString:(NSString *)urlString {
     NSMutableDictionary *json = [NSMutableDictionary new];
 
     // All requests
@@ -143,6 +147,10 @@ - (NSDictionary *)dataForOpen {
     [self addAppleReceiptSourceToJSON:json];
     [self addTimestampsToJSON:json];
 
+    if (urlString) {
+        [self safeSetValue:urlString forKey:BRANCH_REQUEST_KEY_UNIVERSAL_LINK_URL onDict:json];
+    }
+
     // Usually sent with install, but retry on open if it didn't get sent
     [self addAppleAttributionTokenToJSON:json];
 
@@ -279,7 +287,6 @@ - (void)addPreferenceHelperDataToJSON:(NSMutableDictionary *)json {
 
     [self safeSetValue:self.preferenceHelper.linkClickIdentifier forKey:BRANCH_REQUEST_KEY_LINK_IDENTIFIER onDict:json];
     [self safeSetValue:self.preferenceHelper.spotlightIdentifier forKey:BRANCH_REQUEST_KEY_SPOTLIGHT_IDENTIFIER onDict:json];
-    [self safeSetValue:self.preferenceHelper.universalLinkUrl forKey:BRANCH_REQUEST_KEY_UNIVERSAL_LINK_URL onDict:json];
     [self safeSetValue:self.preferenceHelper.initialReferrer forKey:BRANCH_REQUEST_KEY_INITIAL_REFERRER onDict:json];
 
     // This was only on opens before, cause it can't exist on install.

diff --git a/Sources/BranchSDK/BNCServerRequestQueue.m b/Sources/BranchSDK/BNCServerRequestQueue.m
@@ -161,56 +161,17 @@ - (BOOL)containsInstallOrOpen {
     }
 }
 
-- (BOOL)removeInstallOrOpen {
+- (BranchOpenRequest *)findExistingInstallOrOpen {
     @synchronized (self) {
         for (NSUInteger i = 0; i < self.queue.count; i++) {
             BNCServerRequest *request = [self.queue objectAtIndex:i];
-            // Install extends open, so only need to check open.
-            if ([request isKindOfClass:[BranchOpenRequest class]]) {
-                [[BranchLogger shared] logDebug:@"Removing open request."];
-                ((BranchOpenRequest *)request).callback = nil;
-                [self remove:request];
-                return YES;
-            }
-        }
-        return NO;
-    }
-}
-
-- (BranchOpenRequest *)moveInstallOrOpenToFront:(NSInteger)networkCount {
-    @synchronized (self) {
-
-        BOOL requestAlreadyInProgress = networkCount > 0;
 
-        BNCServerRequest *openOrInstallRequest;
-        for (NSUInteger i = 0; i < self.queue.count; i++) {
-            BNCServerRequest *req = [self.queue objectAtIndex:i];
-            if ([req isKindOfClass:[BranchOpenRequest class]]) {
-
-                // Already in front, nothing to do
-                if (i == 0 || (i == 1 && requestAlreadyInProgress)) {
-                    return (BranchOpenRequest *)req;
-                }
-
-                // Otherwise, pull this request out and stop early
-                openOrInstallRequest = [self removeAt:i];
-                break;
+            // Install subclasses open, so only need to check open
+            if ([request isKindOfClass:[BranchOpenRequest class]]) {
+                return (BranchOpenRequest *)request;
             }
         }
-
-        if (!openOrInstallRequest) {
-            [[BranchLogger shared] logError:@"No install or open request in queue while trying to move it to the front." error:nil];
-            return nil;
-        }
-
-        if (!requestAlreadyInProgress || !self.queue.count) {
-            [self insert:openOrInstallRequest at:0];
-        }
-        else {
-            [self insert:openOrInstallRequest at:1];
-        }
-
-        return (BranchOpenRequest *)openOrInstallRequest;
+        return nil;
     }
 }