Use completion handler when updating experiments after activation (#6711)

* Use completion handler when updating experiments after activation

* Fix Travis

* Add changelog

* Fix whitespace

* Nonnull initializers

Author: karenyz

FirebaseRemoteConfig/CHANGELOG.md

@@ -6,6 +6,7 @@
 - [fixed] Completion handler for `fetchAndActivateWithCompletionHandler` is now run on the main thread. (#5897)
 - [fixed] Fixed database creation on tvOS. (#6612)
 - [changed] Updated public API documentation to no longer reference removed APIs. (#6641)
+- [fixed] Updated `activateWithCompletion:` to use completion handler for experiment updates. (#3687)
 
 # v4.9.1
 - [fixed] Fix an `attempt to insert nil object` crash in `fetchWithExpirationDuration:`. (#6522)

FirebaseRemoteConfig/Sources/FIRRemoteConfig.m

@@ -284,22 +284,19 @@ - (void)activateWithCompletion:(FIRRemoteConfigActivateChangeCompletion)completi
     [strongSelf->_configContent copyFromDictionary:self->_configContent.fetchedConfig
                                           toSource:RCNDBSourceActive
                                       forNamespace:self->_FIRNamespace];
-    [strongSelf updateExperiments];
     strongSelf->_settings.lastApplyTimeInterval = [[NSDate date] timeIntervalSince1970];
     FIRLogDebug(kFIRLoggerRemoteConfig, @"I-RCN000069", @"Config activated.");
-    if (completion) {
-      dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
-        completion(YES, nil);
-      });
-    }
+    [strongSelf->_configExperiment updateExperimentsWithHandler:^(NSError *_Nullable error) {
+      if (completion) {
+        dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+          completion(YES, nil);
+        });
+      }
+    }];
   };
   dispatch_async(_queue, applyBlock);
 }
 
-- (void)updateExperiments {
-  [self->_configExperiment updateExperiments];
-}
-
 #pragma mark - helpers
 - (NSString *)fullyQualifiedNamespace:(NSString *)namespace {
   // If this is already a fully qualified namespace, return.

FirebaseRemoteConfig/Sources/RCNConfigExperiment.h

@@ -23,15 +23,16 @@
 @interface RCNConfigExperiment : NSObject
 
 /// Designated initializer;
-- (instancetype)initWithDBManager:(RCNConfigDBManager *)DBManager
-             experimentController:(FIRExperimentController *)controller NS_DESIGNATED_INITIALIZER;
+- (nonnull instancetype)initWithDBManager:(RCNConfigDBManager *_Nullable)DBManager
+                     experimentController:(FIRExperimentController *_Nullable)controller
+    NS_DESIGNATED_INITIALIZER;
 
 /// Use `initWithDBManager:` instead.
-- (instancetype)init NS_UNAVAILABLE;
+- (nonnull instancetype)init NS_UNAVAILABLE;
 
 /// Update/Persist experiment information from config fetch response.
-- (void)updateExperimentsWithResponse:(NSArray<NSDictionary<NSString *, id> *> *)response;
+- (void)updateExperimentsWithResponse:(NSArray<NSDictionary<NSString *, id> *> *_Nullable)response;
 
 /// Update experiments to Firebase Analytics when `activateWithCompletion:` happens.
-- (void)updateExperiments;
+- (void)updateExperimentsWithHandler:(nullable void (^)(NSError *_Nullable error))handler;
 @end

FirebaseRemoteConfig/Sources/RCNConfigExperiment.m

@@ -111,7 +111,7 @@ - (void)updateExperimentsWithResponse:(NSArray<NSDictionary<NSString *, id> *> *
   }
 }
 
-- (void)updateExperiments {
+- (void)updateExperimentsWithHandler:(void (^)(NSError *_Nullable))handler {
   FIRLifecycleEvents *lifecycleEvent = [[FIRLifecycleEvents alloc] init];
 
   // Get the last experiment start time prior to the latest payload.
@@ -126,7 +126,7 @@ - (void)updateExperiments {
                                   policy:ABTExperimentPayloadExperimentOverflowPolicyDiscardOldest
                            lastStartTime:lastStartTime
                                 payloads:_experimentPayloads
-                       completionHandler:nil];
+                       completionHandler:handler];
 }
 
 - (void)updateExperimentStartTime {

FirebaseRemoteConfig/Tests/Unit/RCNConfigExperimentTest.m

@@ -227,8 +227,11 @@ - (void)testUpdateExperiments {
 
   experiment.experimentPayloads = [@[ payloadData ] mutableCopy];
 
-  [experiment updateExperiments];
-  XCTAssertEqualObjects(experiment.experimentMetadata[@"last_experiment_start_time"], @(12345678));
+  [experiment updateExperimentsWithHandler:^(NSError *_Nullable error) {
+    XCTAssertNil(error);
+    XCTAssertEqualObjects(experiment.experimentMetadata[@"last_experiment_start_time"],
+                          @(12345678));
+  }];
 }
 
 #pragma mark Helpers.

FirebaseRemoteConfig/Tests/Unit/RCNRemoteConfigTest.m

@@ -23,6 +23,7 @@
 #import "FirebaseRemoteConfig/Sources/Public/FirebaseRemoteConfig/FIRRemoteConfig.h"
 #import "FirebaseRemoteConfig/Sources/RCNConfigConstants.h"
 #import "FirebaseRemoteConfig/Sources/RCNConfigDBManager.h"
+#import "FirebaseRemoteConfig/Sources/RCNConfigExperiment.h"
 #import "FirebaseRemoteConfig/Sources/RCNUserDefaultsManager.h"
 
 #import "FirebaseRemoteConfig/Tests/Unit/RCNTestUtilities.h"
@@ -99,6 +100,7 @@ @interface RCNRemoteConfigTest : XCTestCase {
   NSString *_userDefaultsSuiteName;
   NSString *_DBPath;
   id _DBManagerMock;
+  id _experimentMock;
   id _userDefaultsMock;
 }
 @end
@@ -123,6 +125,10 @@ - (void)setUp {
   OCMStub([_userDefaultsMock sharedUserDefaultsForBundleIdentifier:[OCMArg any]])
       .andReturn(_userDefaults);
 
+  _experimentMock = OCMClassMock([RCNConfigExperiment class]);
+  OCMStub([_experimentMock
+      updateExperimentsWithHandler:([OCMArg invokeBlockWithArgs:[NSNull null], nil])]);
+
   RCNConfigContent *configContent = [[RCNConfigContent alloc] initWithDBManager:_DBManager];
   _configInstances = [[NSMutableArray alloc] initWithCapacity:3];
   _entries = [[NSMutableArray alloc] initWithCapacity:3];
@@ -172,7 +178,6 @@ - (void)setUp {
                                                       DBManager:_DBManager
                                                   configContent:configContent
                                                       analytics:nil]);
-
     _configInstances[i] = config;
     RCNConfigSettings *settings =
         [[RCNConfigSettings alloc] initWithDatabaseManager:_DBManager
@@ -186,7 +191,7 @@ - (void)setUp {
                                                                    DBManager:_DBManager
                                                                     settings:settings
                                                                    analytics:nil
-                                                                  experiment:nil
+                                                                  experiment:_experimentMock
                                                                        queue:queue
                                                                    namespace:fullyQualifiedNamespace
                                                                      options:currentOptions]);
@@ -221,7 +226,7 @@ __unsafe_unretained void (^handler)(FIRRemoteConfigFetchStatus status,
     [_configInstances[i] updateWithNewInstancesForConfigFetch:_configFetch[i]
                                                 configContent:configContent
                                                configSettings:settings
-                                             configExperiment:nil];
+                                             configExperiment:_experimentMock];
   }
 }