Fix duplicate install issue

Summary:
There exists a potential issue that duplicate install can be sent because `applicationDidBecomeActive` may be triggered multiple times within a very short time (e.g. App Launch and ATT prompt can trigger this function) and thus cause  `publichInstall` is triggered multiple times.

In order to fix this issue, this diff reuses the existing key `lastAttributionPingString` and `lastInstallResponseKey`. The flow will be
1. PublishInstall is triggered
2. Check if `lastAttributionPingString` exists and will send install event and set the key if the key doesn't exist. We do nothing if the key exists.
3. If the install event is sent successfully, we will set the key `lastInstallResponseKey`. If not, we'll remove the key `lastAttributionPingString`.
4. When app terminates, we'll check if `lastInstallResponseKey` exists to check if the install event is sent successfully. If the key exists, we'll do nothing. If the key doesn't exist, it means the install event is not sent successfully in the app launch and we'll remove the key `lastAttributionPingString`.

Reviewed By: jjiang10

Differential Revision: D53170017

fbshipit-source-id: 340e21188d884088215a65d06fffc402065a5102

FBSDKCoreKit/FBSDKCoreKit/AppEvents/FBSDKAppEvents.m

@@ -171,13 +171,13 @@ - (void)startObservingApplicationLifecycleNotifications
 {
   [NSNotificationCenter.defaultCenter
    addObserver:self
-   selector:@selector(applicationMovingFromActiveStateOrTerminating)
+   selector:@selector(applicationMovingFromActiveState)
    name:UIApplicationWillResignActiveNotification
    object:NULL];
 
   [NSNotificationCenter.defaultCenter
    addObserver:self
-   selector:@selector(applicationMovingFromActiveStateOrTerminating)
+   selector:@selector(applicationTerminating)
    name:UIApplicationWillTerminateNotification
    object:NULL];
 
@@ -850,6 +850,7 @@ - (void)publishInstall
   if ([self.primaryDataStore fb_objectForKey:lastAttributionPingString]) {
     return;
   }
+  [self.primaryDataStore fb_setObject:[NSDate date] forKey:lastAttributionPingString];
   [self fetchServerConfiguration:^{
     if ([self.appEventsUtility shouldDropAppEvents] || [self.gateKeeperManager boolForKey:FBSDKGateKeeperAppEventsKillSwitch defaultValue:NO]) {
       return;
@@ -869,9 +870,10 @@ - (void)publishInstall
     __block id<FBSDKDataPersisting> weakStore = self.primaryDataStore;
     [request startWithCompletion:^(id<FBSDKGraphRequestConnecting> connection, id result, NSError *error) {
       if (!error) {
-        [weakStore fb_setObject:[NSDate date] forKey:lastAttributionPingString];
         NSString *lastInstallResponseKey = [NSString stringWithFormat:@"com.facebook.sdk:lastInstallResponse%@", appID];
         [weakStore fb_setObject:result forKey:lastInstallResponseKey];
+      } else {
+        [weakStore fb_removeObjectForKey:lastAttributionPingString];
       }
     }];
   }];
@@ -1423,7 +1425,7 @@ - (void)applicationDidBecomeActive
   [self.timeSpentRecorder restore:NO];
 }
 
-- (void)applicationMovingFromActiveStateOrTerminating
+- (void)applicationMovingFromActiveState
 {
   // When moving from active state, we don't have time to wait for the result of a flush, so
   // just persist events to storage, and we'll process them at the next activation.
@@ -1438,6 +1440,19 @@ - (void)applicationMovingFromActiveStateOrTerminating
   [self.timeSpentRecorder suspend];
 }
 
+- (void)applicationTerminating
+{
+  NSString *appID = [self appID];
+  if (appID) {
+    NSString *lastAttributionPingString = [NSString stringWithFormat:@"com.facebook.sdk:lastAttributionPing%@", appID];
+    NSString *lastInstallResponseKey = [NSString stringWithFormat:@"com.facebook.sdk:lastInstallResponse%@", appID];
+    if (nil == [self.primaryDataStore fb_objectForKey:lastInstallResponseKey]) {
+      [self.primaryDataStore fb_removeObjectForKey:lastAttributionPingString];
+    }
+  }
+  [self applicationMovingFromActiveState];
+}
+
 #pragma mark - Configuration Validation
 
 - (void)validateConfiguration

FBSDKCoreKit/FBSDKCoreKitTests/Interfaces/FBSDKAppEvents+Testing.h

@@ -50,7 +50,8 @@ NS_ASSUME_NONNULL_BEGIN
   isImplicitlyLogged:(BOOL)isImplicitlyLogged
          accessToken:(nullable FBSDKAccessToken *)accessToken;
 - (void)applicationDidBecomeActive;
-- (void)applicationMovingFromActiveStateOrTerminating;
+- (void)applicationMovingFromActiveState;
+- (void)applicationTerminating;
 
 @end
 

FBSDKCoreKit/FBSDKCoreKitTests/Internal/AppEvents/AppEventsTests.swift

@@ -647,11 +647,37 @@ final class AppEventsTests: XCTestCase {
     )
   }
 
+  func testApplicationMovingFromActiveStateSuspendsTimeSpentRecording() {
+    appEvents.applicationMovingFromActiveState()
+    XCTAssertTrue(
+      timeSpentRecorder.suspendWasCalled,
+      "When application moves from active state, the time spent recording should be suspended."
+    )
+  }
+
   func testApplicationTerminatingSuspendsTimeSpentRecording() {
-    appEvents.applicationMovingFromActiveStateOrTerminating()
+    primaryDataStore.set(Date(), forKey: "com.facebook.sdk:lastAttributionPingmockAppID")
+    primaryDataStore.set(Date(), forKey: "com.facebook.sdk:lastInstallResponsemockAppID")
+    appEvents.applicationTerminating()
     XCTAssertTrue(
       timeSpentRecorder.suspendWasCalled,
-      "When application terminates or moves from active state, the time spent recording should be suspended."
+      "When application terminates, the time spent recording should be suspended."
+    )
+    XCTAssertFalse(
+      primaryDataStore.capturedRemoveObjectKeys.contains("com.facebook.sdk:lastAttributionPing123")
+    )
+  }
+
+  func testApplicationTerminatingWithoutInstallResponse() {
+    settings.appID = "123"
+    primaryDataStore.set(Date(), forKey: "com.facebook.sdk:lastAttributionPingmockAppID")
+    appEvents.applicationTerminating()
+    XCTAssertTrue(
+      timeSpentRecorder.suspendWasCalled,
+      "When application terminates, the time spent recording should be suspended."
+    )
+    XCTAssertTrue(
+      primaryDataStore.capturedRemoveObjectKeys.contains("com.facebook.sdk:lastAttributionPingmockAppID")
     )
   }
 
@@ -671,7 +697,7 @@ final class AppEventsTests: XCTestCase {
       isImplicitlyLogged: false,
       accessToken: SampleAccessTokens.validToken
     )
-    appEvents.applicationMovingFromActiveStateOrTerminating()
+    appEvents.applicationMovingFromActiveState()
 
     XCTAssertTrue(
       !appEventsStateStore.capturedPersistedState.isEmpty,

TestTools/TestTools/UserDefaultsSpy.swift

@@ -12,6 +12,7 @@ import Foundation
 public final class UserDefaultsSpy: UserDefaults {
   public var capturedObjectRetrievalKeys = [String]()
   public var capturedSetObjectKeys = [String]()
+  public var capturedRemoveObjectKeys = [String]()
   public var capturedObjectRetrievalKey: String?
   public var capturedSetObjectKey: String?
   public var capturedValues = [String: Any]()
@@ -33,4 +34,8 @@ public final class UserDefaultsSpy: UserDefaults {
     capturedSetObjectKeys.append(defaultName)
     capturedSetObjectKey = defaultName
   }
+
+  public override func removeObject(forKey defaultName: String) {
+    capturedRemoveObjectKeys.append(defaultName)
+  }
 }