diff --git a/packages/remote-config/e2e/config.e2e.js b/packages/remote-config/e2e/config.e2e.js index 311a6984ee..b28613e51a 100644 --- a/packages/remote-config/e2e/config.e2e.js +++ b/packages/remote-config/e2e/config.e2e.js @@ -59,6 +59,82 @@ describe('remoteConfig()', function () { const { getRemoteConfig, fetchAndActivate } = remoteConfigModular; (await fetchAndActivate(getRemoteConfig())).should.be.a.Boolean(); }); + + // Regression test for https://github.com/invertase/react-native-firebase/issues/7779 + // On iOS, fetchAndActivate() used to always resolve `true` whenever the underlying fetch + // succeeded, even when the fetched values were identical to what was already active + // (i.e. nothing new was actually activated). This must be consistent with fetch() + activate(), + // and with the boolean returned by activate() itself, on both platforms. + it('returns false when there is nothing new to activate', async function () { + const { getRemoteConfig, fetchAndActivate } = remoteConfigModular; + const remoteConfig = getRemoteConfig(); + remoteConfig.settings = { minimumFetchIntervalMillis: 0 }; + + // Make sure the current remote values are fetched + activated first. + await fetchAndActivate(remoteConfig); + + // Calling again immediately with no server-side template changes must not report + // that anything was activated, matching fetch() + activate() and matching Android. + const activatedAgain = await fetchAndActivate(remoteConfig); + activatedAgain.should.equal(false); + }); + + it('is consistent with fetch() followed by activate()', async function () { + const { getRemoteConfig, fetchConfig, fetchAndActivate, activate } = remoteConfigModular; + const remoteConfig = getRemoteConfig(); + remoteConfig.settings = { minimumFetchIntervalMillis: 0 }; + + // Get into a known "fully activated, no pending changes" state. + await fetchAndActivate(remoteConfig); + + // With no new remote values, fetch() + activate() should report nothing changed... + await fetchConfig(remoteConfig); + const activatedViaSeparateCalls = await activate(remoteConfig); + activatedViaSeparateCalls.should.equal(false); + + // ...and fetchAndActivate() must agree. + const activatedViaFetchAndActivate = await fetchAndActivate(remoteConfig); + activatedViaFetchAndActivate.should.equal(activatedViaSeparateCalls); + }); + + it('returns true only once new remote values have actually been fetched and activated', async function () { + const { getRemoteConfig, fetchAndActivate, getValue } = remoteConfigModular; + const remoteConfig = getRemoteConfig(); + remoteConfig.settings = { minimumFetchIntervalMillis: 0 }; + const paramName = 'fetchAndActivateTest' + Date.now(); + const paramValue = 'value' + Date.now(); + + // Get into a known "fully activated, no pending changes" state first. + await fetchAndActivate(remoteConfig); + (await fetchAndActivate(remoteConfig)).should.equal(false); + + try { + const response = await FirebaseHelpers.updateRemoteConfigTemplate({ + operations: { add: [{ name: paramName, value: paramValue }] }, + }); + should(response.result !== undefined).equal(true, 'response result not defined'); + + // A newly published template can take a little while to propagate to the fetch + // backend, so poll fetchAndActivate() until it reports the new value was activated, + // rather than asserting on the very next call. + let activated = false; + const deadline = Date.now() + 60000; + while (Date.now() < deadline) { + activated = await fetchAndActivate(remoteConfig); + if (activated && getValue(remoteConfig, paramName).asString() === paramValue) { + break; + } + await Utils.sleep(2000); + } + + activated.should.equal(true); + getValue(remoteConfig, paramName).asString().should.equal(paramValue); + } finally { + await FirebaseHelpers.updateRemoteConfigTemplate({ + operations: { delete: [paramName] }, + }); + } + }); }); describe('activate()', function () { diff --git a/packages/remote-config/ios/RNFBConfig/RNFBConfigModule.mm b/packages/remote-config/ios/RNFBConfig/RNFBConfigModule.mm index dfbaffec67..2c49a98ed1 100644 --- a/packages/remote-config/ios/RNFBConfig/RNFBConfigModule.mm +++ b/packages/remote-config/ios/RNFBConfig/RNFBConfigModule.mm @@ -191,27 +191,57 @@ - (void)fetch:(NSString *)appName - (void)fetchAndActivate:(NSString *)appName resolve:(RCTPromiseResolveBlock)resolve reject:(RCTPromiseRejectBlock)reject { + // NOTE: We deliberately do not use `-[FIRRemoteConfig fetchAndActivateWithCompletionHandler:]` + // here. Its `FIRRemoteConfigFetchAndActivateStatus` result only reflects whether the fetch + // succeeded (`SuccessFetchedFromRemote`) vs fell back to previously fetched data + // (`SuccessUsingPreFetchedData`) - it does NOT reflect whether activation actually changed any + // config values, so it reports `SuccessFetchedFromRemote` (=> we'd resolve `true`) even when the + // fetched values are identical to what's already active. See + // https://github.com/invertase/react-native-firebase/issues/7779 + // + // Instead we perform the fetch + activate ourselves and resolve with the `changed` BOOL from + // `activateWithCompletion:`, which matches the semantics of our own `activate()` method as well + // as the Android implementation, both of which resolve `true` only when activation changed the + // in-use config values. FIRApp *firebaseApp = firebaseAppForName(appName); - FIRRemoteConfigFetchAndActivateCompletion completionHandler = - ^(FIRRemoteConfigFetchAndActivateStatus status, NSError *__nullable error) { - if (error) { - if (error.userInfo && error.userInfo[@"ActivationFailureReason"] != nil && - [error.userInfo[@"ActivationFailureReason"] containsString:@"already activated"]) { - resolve([self resultWithConstants:@([RCTConvert BOOL:@(YES)]) firebaseApp:firebaseApp]); - } else { - [RNFBSharedUtils rejectPromiseWithNSError:reject error:error]; + __weak RNFBConfigModule *weakSelf = self; + FIRRemoteConfigFetchCompletion fetchCompletion = ^(FIRRemoteConfigFetchStatus status, + NSError *__nullable error) { + RNFBConfigModule *strongSelf = weakSelf; + if (!strongSelf) { + return; + } + + if (error) { + [RNFBSharedUtils + rejectPromiseWithUserInfo:reject + userInfo:[@{ + @"code" : convertFIRRemoteConfigFetchStatusToNSString(status), + @"message" : + convertFIRRemoteConfigFetchStatusToNSStringDescription(status) + } mutableCopy]]; + return; + } + + [[FIRRemoteConfig remoteConfigWithApp:firebaseApp] + activateWithCompletion:^(BOOL changed, NSError *_Nullable activateError) { + if (!activateError) { + resolve([strongSelf resultWithConstants:@([RCTConvert BOOL:@(changed)]) + firebaseApp:firebaseApp]); + return; } - } else { - if (status == FIRRemoteConfigFetchAndActivateStatusSuccessFetchedFromRemote) { - resolve([self resultWithConstants:@([RCTConvert BOOL:@(YES)]) firebaseApp:firebaseApp]); + if (activateError.userInfo && activateError.userInfo[@"ActivationFailureReason"] != nil && + [activateError.userInfo[@"ActivationFailureReason"] + containsString:@"already activated"]) { + resolve([strongSelf resultWithConstants:@([RCTConvert BOOL:@(NO)]) + firebaseApp:firebaseApp]); return; } - resolve([self resultWithConstants:@([RCTConvert BOOL:@(NO)]) firebaseApp:firebaseApp]); - } - }; + [RNFBSharedUtils rejectPromiseWithNSError:reject error:activateError]; + }]; + }; - [[FIRRemoteConfig remoteConfigWithApp:firebaseApp] - fetchAndActivateWithCompletionHandler:completionHandler]; + [[FIRRemoteConfig remoteConfigWithApp:firebaseApp] fetchWithCompletionHandler:fetchCompletion]; } - (void)activate:(NSString *)appName