Fix error in default RC Fetch timeout, and add test for Fetch(0). (#851)

* Add test for Fetch(0) to TestFetchInterval.

* Change Fetch() to use correct default 12 hours, instead of 1200 hours.

* Add release note for fix.

Author: jonsimantov

release_build_files/readme.md

@@ -571,6 +571,8 @@ code.
 -   Changes
     -   General (iOS): Fixed additional issues on iOS 15 caused by early
         initialization of Firebase iOS SDK.
+    -   Remote Config: Fixed default `Fetch()` timeout being 1000 times too
+        high.
 
 ### 8.9.0
 -   Changes

remote_config/integration_test/src/integration_test.cc

@@ -366,4 +366,44 @@ TEST_F(FirebaseRemoteConfigTest, TestFetchInterval) {
   EXPECT_NE(current_fetch_time, rc_->GetInfo().fetch_time);
 }
 
+TEST_F(FirebaseRemoteConfigTest, TestFetchSecondsParameter) {
+  ASSERT_NE(rc_, nullptr);
+
+  EXPECT_TRUE(
+      WaitForCompletion(RunWithRetry([&]() { return rc_->FetchAndActivate(); }),
+                        "FetchAndActivate"));
+  uint64_t current_fetch_time = rc_->GetInfo().fetch_time;
+  // Making sure the config settings's fetch interval is 12 hours
+  EXPECT_TRUE(WaitForCompletion(SetDefaultConfigSettings(rc_),
+                                "SetDefaultConfigSettings"));
+  // Test Fetch() without specifying an interval; it should not fetch.
+  EXPECT_TRUE(WaitForCompletion(RunWithRetry([&]() { return rc_->Fetch(); }),
+                                "Fetch() [should not fetch]"));
+  EXPECT_EQ(current_fetch_time, rc_->GetInfo().fetch_time);
+
+  FLAKY_TEST_SECTION_BEGIN();
+
+  // Call Fetch(0), forcing a fetch.
+  EXPECT_TRUE(WaitForCompletion(RunWithRetry([&]() { return rc_->Fetch(0); }),
+                                "Fetch(0) [should fetch]"));
+  EXPECT_NE(current_fetch_time, rc_->GetInfo().fetch_time);
+
+  current_fetch_time = rc_->GetInfo().fetch_time;
+  // Call Fetch(30), which shouldn't fetch yet.
+  EXPECT_TRUE(WaitForCompletion(RunWithRetry([&]() { return rc_->Fetch(30); }),
+                                "Fetch(30) [should not fetch]"));
+  EXPECT_EQ(current_fetch_time, rc_->GetInfo().fetch_time);
+
+  LogDebug("Pausing 45 seconds before re-running Fetch");
+  for (int i = 0; i < 45; i++) {
+    ProcessEvents(1000);
+  }
+  // After waiting 45 seconds, Fetch(30) should now fetch.
+  EXPECT_TRUE(WaitForCompletion(RunWithRetry([&]() { return rc_->Fetch(30); }),
+                                "Fetch(30) [should fetch]"));
+  EXPECT_NE(current_fetch_time, rc_->GetInfo().fetch_time);
+
+  FLAKY_TEST_SECTION_END();
+}
+
 }  // namespace firebase_testapp_automated

remote_config/src/remote_config.cc

@@ -19,6 +19,7 @@
 #include "app/src/cleanup_notifier.h"
 #include "app/src/include/firebase/internal/mutex.h"
 #include "app/src/semaphore.h"
+#include "app/src/time.h"
 #include "include/firebase/remote_config.h"
 #include "remote_config/src/common.h"
 
@@ -132,7 +133,8 @@ Future<bool> RemoteConfig::FetchAndActivateLastResult() {
 }
 
 Future<void> RemoteConfig::Fetch() {
-  return Fetch(GetConfigSettings().minimum_fetch_interval_in_milliseconds);
+  return Fetch(GetConfigSettings().minimum_fetch_interval_in_milliseconds /
+               firebase::internal::kMillisecondsPerSecond);
 }
 
 Future<void> RemoteConfig::Fetch(uint64_t cache_expiration_in_seconds) {