Merge pull request #473 from firebase/feature/rc_rest_update

[RemoteConfig] Fix the fetch interval bug

Author: cynthiajoan

remote_config/integration_test/src/integration_test.cc

@@ -170,21 +170,32 @@ static const firebase::remote_config::ConfigKeyValueVariant kServerValue[] = {
     {"TestDefaultOnly", firebase::Variant::FromMutableString(
                             "Default value that won't be overridden")}};
 
-static Future<void> SetDefaultsV2(RemoteConfig* rc) {
+static Future<void> SetDefaults(RemoteConfig* rc) {
   size_t default_count = FIREBASE_ARRAYSIZE(defaults);
   return rc->SetDefaults(defaults, default_count);
 }
 
+static Future<void> SetDefaultConfigSettings(RemoteConfig* rc) {
+  firebase::remote_config::ConfigSettings defaultConfigSettings;
+  return rc->SetConfigSettings(defaultConfigSettings);
+}
+
+static Future<void> SetZeroIntervalConfigSettings(RemoteConfig* rc) {
+  firebase::remote_config::ConfigSettings zeroIntervalConfigSettings;
+  zeroIntervalConfigSettings.minimum_fetch_interval_in_milliseconds = 0;
+  return rc->SetConfigSettings(zeroIntervalConfigSettings);
+}
+
 // Test cases below.
 
 TEST_F(FirebaseRemoteConfigTest, TestInitializeAndTerminate) {
   // Already tested via SetUp() and TearDown().
 }
 
-TEST_F(FirebaseRemoteConfigTest, TestSetDefaultsV2) {
+TEST_F(FirebaseRemoteConfigTest, TestSetDefault) {
   ASSERT_NE(rc_, nullptr);
 
-  EXPECT_TRUE(WaitForCompletion(SetDefaultsV2(rc_), "SetDefaultsV2"));
+  EXPECT_TRUE(WaitForCompletion(SetDefaults(rc_), "SetDefaults"));
 
   bool validated_defaults = true;
   firebase::remote_config::ValueInfo value_info;
@@ -243,10 +254,10 @@ TEST_F(FirebaseRemoteConfigTest, TestSetDefaultsV2) {
   }
 }
 
-TEST_F(FirebaseRemoteConfigTest, TestGetKeysV2) {
+TEST_F(FirebaseRemoteConfigTest, TestGetKeys) {
   ASSERT_NE(rc_, nullptr);
 
-  EXPECT_TRUE(WaitForCompletion(SetDefaultsV2(rc_), "SetDefaultsV2"));
+  EXPECT_TRUE(WaitForCompletion(SetDefaults(rc_), "SetDefaults"));
 
   std::vector<std::string> keys = rc_->GetKeys();
   EXPECT_THAT(
@@ -265,7 +276,7 @@ TEST_F(FirebaseRemoteConfigTest, TestGetKeysV2) {
 TEST_F(FirebaseRemoteConfigTest, TestGetAll) {
   ASSERT_NE(rc_, nullptr);
 
-  EXPECT_TRUE(WaitForCompletion(SetDefaultsV2(rc_), "SetDefaultsV2"));
+  EXPECT_TRUE(WaitForCompletion(SetDefaults(rc_), "SetDefaults"));
   EXPECT_TRUE(WaitForCompletion(
       RunWithRetry([](RemoteConfig* rc) { return rc->Fetch(); }, rc_),
       "Fetch"));
@@ -288,10 +299,10 @@ TEST_F(FirebaseRemoteConfigTest, TestGetAll) {
    TestBoolean  true
    TestString   This is a string
  */
-TEST_F(FirebaseRemoteConfigTest, TestFetchV2) {
+TEST_F(FirebaseRemoteConfigTest, TestFetch) {
   ASSERT_NE(rc_, nullptr);
 
-  EXPECT_TRUE(WaitForCompletion(SetDefaultsV2(rc_), "SetDefaultsV2"));
+  EXPECT_TRUE(WaitForCompletion(SetDefaults(rc_), "SetDefaults"));
   EXPECT_TRUE(WaitForCompletion(
       RunWithRetry([](RemoteConfig* rc) { return rc->Fetch(); }, rc_),
       "Fetch"));
@@ -334,4 +345,31 @@ TEST_F(FirebaseRemoteConfigTest, TestFetchV2) {
                                         sizeof(kExpectedBlobServerValue)));
 }
 
+TEST_F(FirebaseRemoteConfigTest, TestFetchInterval) {
+  ASSERT_NE(rc_, nullptr);
+  EXPECT_TRUE(WaitForCompletion(
+      RunWithRetry([](RemoteConfig* rc) { return rc->Fetch(); }, rc_),
+      "Fetch"));
+  EXPECT_TRUE(WaitForCompletion(rc_->Activate(), "Activate"));
+  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"));
+  // Second fetch, should respect fetch interval and don't change data.
+  EXPECT_TRUE(WaitForCompletion(
+      RunWithRetry([](RemoteConfig* rc) { return rc->Fetch(); }, rc_),
+      "Fetch"));
+  EXPECT_EQ(current_fetch_time, rc_->GetInfo().fetch_time);
+  // Update fetch interval to 0
+  EXPECT_TRUE(WaitForCompletion(SetZeroIntervalConfigSettings(rc_),
+                                "SetZeroIntervalConfigSettings"));
+  LogDebug("Current Fetch Interval: %lld",
+           rc_->GetConfigSettings().minimum_fetch_interval_in_milliseconds);
+  // Third fetch, this should operate the real fetch and update the fetch time.
+  EXPECT_TRUE(WaitForCompletion(
+      RunWithRetry([](RemoteConfig* rc) { return rc->Fetch(); }, rc_),
+      "Fetch"));
+  EXPECT_NE(current_fetch_time, rc_->GetInfo().fetch_time);
+}
+
 }  // namespace firebase_testapp_automated

remote_config/src/desktop/remote_config_desktop.cc

@@ -139,7 +139,8 @@ Future<bool> RemoteConfigInternal::FetchAndActivate() {
   const auto future_handle =
       future_impl_.SafeAlloc<bool>(kRemoteConfigFnFetchAndActivate);
 
-  cache_expiration_in_seconds_ = kDefaultCacheExpiration;
+  cache_expiration_in_seconds_ =
+      config_settings_.minimum_fetch_interval_in_milliseconds;
 
   uint64_t milliseconds_since_epoch =
       std::chrono::duration_cast<std::chrono::milliseconds>(

remote_config/src/include/firebase/remote_config.h

@@ -511,8 +511,6 @@ class RemoteConfig {
   // Clean up RemoteConfig instance.
   void DeleteInternal();
 
-  uint64_t GetConfigFetchInterval();
-
   /// The Firebase App this remote config is connected to.
   App* app_;
 

remote_config/src/remote_config.cc

@@ -131,7 +131,9 @@ Future<bool> RemoteConfig::FetchAndActivateLastResult() {
   return internal_->FetchAndActivateLastResult();
 }
 
-Future<void> RemoteConfig::Fetch() { return Fetch(GetConfigFetchInterval()); }
+Future<void> RemoteConfig::Fetch() {
+  return Fetch(GetConfigSettings().minimum_fetch_interval_in_milliseconds);
+}
 
 Future<void> RemoteConfig::Fetch(uint64_t cache_expiration_in_seconds) {
   return internal_->Fetch(cache_expiration_in_seconds);
@@ -229,14 +231,5 @@ std::map<std::string, Variant> RemoteConfig::GetAll() {
 // TODO(b/147143718): Change to a more descriptive name.
 const ConfigInfo RemoteConfig::GetInfo() { return internal_->GetInfo(); }
 
-uint64_t RemoteConfig::GetConfigFetchInterval() {
-  uint64_t cache_time =
-      GetConfigSettings().minimum_fetch_interval_in_milliseconds;
-  if (cache_time == 0) {
-    cache_time = kDefaultCacheExpiration;
-  }
-  return cache_time;
-}
-
 }  // namespace remote_config
 }  // namespace firebase