Do not activate if fetch returns NO_CHANGE

FirebaseRemoteConfig/Sources/FIRRemoteConfig.m

@@ -291,10 +291,10 @@ - (void)activateWithCompletionHandler:(FIRRemoteConfigActivateCompletion)complet
       FIRLogError(kFIRLoggerRemoteConfig, @"I-RCN000068", @"Internal error activating config.");
       return;
     }
-    // If Fetched Config is no fresher than Active Config.
-    if (strongSelf->_settings.lastFetchTimeInterval == 0 ||
-        strongSelf->_settings.lastFetchTimeInterval <=
-            strongSelf->_settings.lastApplyTimeInterval) {
+    // Check if the last fetched config has already been activated. Fetches with no data change are
+    // ignored.
+    if (strongSelf->_settings.lastETagUpdateTime == 0 ||
+        strongSelf->_settings.lastETagUpdateTime <= strongSelf->_settings.lastApplyTimeInterval) {
       FIRLogWarning(kFIRLoggerRemoteConfig, @"I-RCN000069",
                     @"Most recently fetched config is already activated.");
       NSError *error = [NSError

FirebaseRemoteConfig/Sources/Private/RCNConfigSettings.h

@@ -76,6 +76,8 @@
 @property(nonatomic, readwrite, assign) NSTimeInterval lastSetDefaultsTimeInterval;
 /// The latest eTag value stored from the last successful response.
 @property(nonatomic, readwrite, assign) NSString *lastETag;
+/// The timestamp of the last eTag update.
+@property(nonatomic, readwrite, assign) NSTimeInterval lastETagUpdateTime;
 
 #pragma mark Throttling properties
 

FirebaseRemoteConfig/Sources/RCNConfigSettings.m

@@ -67,6 +67,8 @@ @interface RCNConfigSettings () {
   NSString *_googleAppID;
   /// The user defaults manager scoped to this RC instance of FIRApp and namespace.
   RCNUserDefaultsManager *_userDefaultsManager;
+  /// The timestamp of last eTag update.
+  NSTimeInterval _lastETagUpdateTime;
 }
 @end
 
@@ -111,13 +113,22 @@ - (NSString *)lastETag {
 }
 
 - (void)setLastETag:(NSString *)lastETag {
+  [self setLastETagUpdateTime:[[NSDate date] timeIntervalSince1970]];
   [_userDefaultsManager setLastETag:lastETag];
 }
 
+- (void)setLastETagUpdateTime:(NSTimeInterval)lastETagUpdateTime {
+  [_userDefaultsManager setLastETagUpdateTime:lastETagUpdateTime];
+}
+
 - (NSTimeInterval)lastFetchTimeInterval {
   return _userDefaultsManager.lastFetchTime;
 }
 
+- (NSTimeInterval)lastETagUpdateTime {
+  return _userDefaultsManager.lastETagUpdateTime;
+}
+
 // TODO: Update logic for app extensions as required.
 - (void)updateLastFetchTimeInterval:(NSTimeInterval)lastFetchTimeInterval {
   _userDefaultsManager.lastFetchTime = lastFetchTimeInterval;

FirebaseRemoteConfig/Sources/RCNFetch.m

@@ -476,8 +476,11 @@ - (void)fetchWithUserProperties:(NSDictionary *)userProperties
                     @"Empty response with no fetched config.");
       }
 
-      // We had a successful fetch. Update the current eTag in settings.
-      self->_settings.lastETag = ((NSHTTPURLResponse *)response).allHeaderFields[kETagHeaderName];
+      // We had a successful fetch. Update the current eTag in settings if different.
+      NSString *latestETag = ((NSHTTPURLResponse *)response).allHeaderFields[kETagHeaderName];
+      if (!self->_settings.lastETag || !([self->_settings.lastETag isEqualToString:latestETag])) {
+        self->_settings.lastETag = latestETag;
+      }
 
       [self->_settings updateMetadataWithFetchSuccessStatus:YES];
       return [strongSelf reportCompletionOnHandler:completionHandler

FirebaseRemoteConfig/Sources/RCNUserDefaultsManager.h

@@ -22,6 +22,8 @@ NS_ASSUME_NONNULL_BEGIN
 
 /// The last eTag received from the backend.
 @property(nonatomic, assign) NSString *lastETag;
+/// The time of the last eTag update.
+@property(nonatomic, assign) NSTimeInterval lastETagUpdateTime;
 /// The time of the last successful fetch.
 @property(nonatomic, assign) NSTimeInterval lastFetchTime;
 /// The time of the last successful fetch.

FirebaseRemoteConfig/Sources/RCNUserDefaultsManager.m

@@ -21,6 +21,7 @@
 static NSString *const kRCNGroupPrefix = @"group";
 static NSString *const kRCNGroupSuffix = @"firebase";
 static NSString *const kRCNUserDefaultsKeyNamelastETag = @"lastETag";
+static NSString *const kRCNUserDefaultsKeyNamelastETagUpdateTime = @"lastETagUpdateTime";
 static NSString *const kRCNUserDefaultsKeyNameLastSuccessfulFetchTime = @"lastSuccessfulFetchTime";
 static NSString *const kRCNUserDefaultsKeyNamelastFetchStatus = @"lastFetchStatus";
 static NSString *const kRCNUserDefaultsKeyNameIsClientThrottled =
@@ -105,6 +106,19 @@ - (void)setLastETag:(NSString *)lastETag {
   }
 }
 
+- (NSTimeInterval)lastETagUpdateTime {
+  NSNumber *lastETagUpdateTime =
+      [[self instanceUserDefaults] objectForKey:kRCNUserDefaultsKeyNamelastETagUpdateTime];
+  return lastETagUpdateTime.doubleValue;
+}
+
+- (void)setLastETagUpdateTime:(NSTimeInterval)lastETagUpdateTime {
+  if (lastETagUpdateTime) {
+    [self setInstanceUserDefaultsValue:@(lastETagUpdateTime)
+                                forKey:kRCNUserDefaultsKeyNamelastETagUpdateTime];
+  }
+}
+
 - (NSTimeInterval)lastFetchTime {
   NSNumber *lastFetchTime =
       [[self instanceUserDefaults] objectForKey:kRCNUserDefaultsKeyNameLastSuccessfulFetchTime];

FirebaseRemoteConfig/Tests/Unit/RCNRemoteConfigTest.m

@@ -560,6 +560,132 @@ - (void)testFetchConfigsFailed {
                                }];
 }
 
+// Activate should return false if a fetch response returns 200 with NO_CHANGE as the response body.
+- (void)testActivateOnFetchNoChangeStatus {
+  // Override the setup values to return back an error status.
+  RCNConfigContent *configContent = [[RCNConfigContent alloc] initWithDBManager:_DBManager];
+  // Populate the default, second app, second namespace instances.
+  for (int i = 0; i < RCNTestRCNumTotalInstances; i++) {
+    NSString *currentAppName = nil;
+    FIROptions *currentOptions = nil;
+    NSString *currentNamespace = nil;
+    switch (i) {
+      case RCNTestRCInstanceSecondNamespace:
+        currentAppName = RCNTestsDefaultFIRAppName;
+        currentOptions = [self firstAppOptions];
+        currentNamespace = RCNTestsPerfNamespace;
+        break;
+      case RCNTestRCInstanceSecondApp:
+        currentAppName = RCNTestsSecondFIRAppName;
+        currentOptions = [self secondAppOptions];
+        currentNamespace = FIRNamespaceGoogleMobilePlatform;
+        break;
+      case RCNTestRCInstanceDefault:
+      default:
+        currentAppName = RCNTestsDefaultFIRAppName;
+        currentOptions = [self firstAppOptions];
+        currentNamespace = RCNTestsFIRNamespace;
+        break;
+    }
+    NSString *fullyQualifiedNamespace =
+        [NSString stringWithFormat:@"%@:%@", currentNamespace, currentAppName];
+    RCNUserDefaultsManager *userDefaultsManager =
+        [[RCNUserDefaultsManager alloc] initWithAppName:currentAppName
+                                               bundleID:[NSBundle mainBundle].bundleIdentifier
+                                              namespace:fullyQualifiedNamespace];
+    userDefaultsManager.lastFetchTime = 10;
+
+    FIRRemoteConfig *config =
+        OCMPartialMock([[FIRRemoteConfig alloc] initWithAppName:currentAppName
+                                                     FIROptions:currentOptions
+                                                      namespace:currentNamespace
+                                                      DBManager:_DBManager
+                                                  configContent:configContent
+                                                      analytics:nil]);
+
+    _configInstances[i] = config;
+    RCNConfigSettings *settings =
+        [[RCNConfigSettings alloc] initWithDatabaseManager:_DBManager
+                                                 namespace:fullyQualifiedNamespace
+                                           firebaseAppName:currentAppName
+                                               googleAppID:currentOptions.googleAppID];
+    // Start the test with the assumption that we have some data that was fetched and activated.
+    settings.lastETag = @"etag1";
+    settings.lastETagUpdateTime = 100;
+    settings.lastApplyTimeInterval = 101;
+
+    dispatch_queue_t queue =
+        dispatch_queue_create([[NSString stringWithFormat:@"testNoStatusFetchQueue: %d", i]
+                                  cStringUsingEncoding:NSUTF8StringEncoding],
+                              DISPATCH_QUEUE_SERIAL);
+    _configFetch[i] = OCMPartialMock([[RCNConfigFetch alloc] initWithContent:configContent
+                                                                   DBManager:_DBManager
+                                                                    settings:settings
+                                                                   analytics:nil
+                                                                  experiment:nil
+                                                                       queue:queue
+                                                                   namespace:fullyQualifiedNamespace
+                                                                     options:currentOptions]);
+
+    OCMStub([_configFetch[i] fetchAllConfigsWithExpirationDuration:43200
+                                                 completionHandler:OCMOCK_ANY])
+        .andDo(^(NSInvocation *invocation) {
+          void (^handler)(FIRRemoteConfigFetchStatus status, NSError *_Nullable error) = nil;
+
+          [invocation getArgument:&handler atIndex:3];
+          [_configFetch[i] fetchWithUserProperties:[[NSDictionary alloc] init]
+                                 completionHandler:handler];
+        });
+
+    _response[i] = @{@"state" : @"NO_CHANGE"};
+
+    _responseData[i] = [NSJSONSerialization dataWithJSONObject:_response[i] options:0 error:nil];
+
+    _URLResponse[i] =
+        [[NSHTTPURLResponse alloc] initWithURL:[NSURL URLWithString:@"https://firebase.com"]
+                                    statusCode:200
+                                   HTTPVersion:nil
+                                  headerFields:@{@"etag" : @"etag1"}];
+
+    id completionBlock =
+        [OCMArg invokeBlockWithArgs:_responseData[i], _URLResponse[i], [NSNull null], nil];
+
+    OCMExpect([_configFetch[i] URLSessionDataTaskWithContent:[OCMArg any]
+                                           completionHandler:completionBlock])
+        .andReturn(nil);
+    [_configInstances[i] updateWithNewInstancesForConfigFetch:_configFetch[i]
+                                                configContent:configContent
+                                               configSettings:settings
+                                             configExperiment:nil];
+  }
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+  // Make the fetch calls for all instances.
+  NSMutableArray<XCTestExpectation *> *expectations =
+      [[NSMutableArray alloc] initWithCapacity:RCNTestRCNumTotalInstances];
+
+  for (int i = 0; i < RCNTestRCNumTotalInstances; i++) {
+    expectations[i] = [self
+        expectationWithDescription:
+            [NSString stringWithFormat:@"Test enumerating configs successfully - instance %d", i]];
+    XCTAssertEqual(_configInstances[i].lastFetchStatus, FIRRemoteConfigFetchStatusNoFetchYet);
+
+    // Make sure activate returns false in fetch completion.
+    FIRRemoteConfigFetchCompletion fetchCompletion =
+        ^void(FIRRemoteConfigFetchStatus status, NSError *error) {
+          XCTAssertEqual(_configInstances[i].lastFetchStatus, FIRRemoteConfigFetchStatusSuccess);
+          XCTAssertFalse([_configInstances[i] activateFetched]);
+          XCTAssertNil(error);
+          [expectations[i] fulfill];
+        };
+    [_configInstances[i] fetchWithExpirationDuration:43200 completionHandler:fetchCompletion];
+  }
+  [self waitForExpectationsWithTimeout:_expectationTimeout
+                               handler:^(NSError *error) {
+                                 XCTAssertNil(error);
+                               }];
+}
+
 - (void)testConfigValueForKey {
   NSMutableArray<XCTestExpectation *> *expectations =
       [[NSMutableArray alloc] initWithCapacity:RCNTestRCNumTotalInstances];

FirebaseRemoteConfig/Tests/Unit/RCNUserDefaultsManagerTests.m

@@ -58,6 +58,18 @@ - (void)testUserDefaultsLastFetchTimeWriteAndRead {
   XCTAssertEqual([manager lastFetchTime], RCNUserDefaultsSampleTimeStamp - 1000);
 }
 
+- (void)testUserDefaultsLastETagUpdateTimeWriteAndRead {
+  RCNUserDefaultsManager* manager =
+      [[RCNUserDefaultsManager alloc] initWithAppName:@"TESTING"
+                                             bundleID:[NSBundle mainBundle].bundleIdentifier
+                                            namespace:@"testNamespace1"];
+  [manager setLastETagUpdateTime:RCNUserDefaultsSampleTimeStamp];
+  XCTAssertEqual([manager lastETagUpdateTime], RCNUserDefaultsSampleTimeStamp);
+
+  [manager setLastETagUpdateTime:RCNUserDefaultsSampleTimeStamp - 1000];
+  XCTAssertEqual([manager lastETagUpdateTime], RCNUserDefaultsSampleTimeStamp - 1000);
+}
+
 - (void)testUserDefaultsLastFetchStatusWriteAndRead {
   RCNUserDefaultsManager* manager =
       [[RCNUserDefaultsManager alloc] initWithAppName:@"TESTING"