[Remote Config] Add Custom Signals (#1899)

* [Remote Config] Add Custom Signals

* Real desktop support

* Formatting

* Update remote_config.h

* Update readme.md

* Update rest_test.cc

* Address feedback

* Update readme.md

* More fixes

* Pass config by reference

* Update readme.md

* Change rest call to be string to string

* Fix windows issues

* Formatting fix

* Update rest_test.cc

* Update remote_config_desktop.cc

Author: a-maurice

release_build_files/readme.md

@@ -619,6 +619,7 @@ code.
     - Auth (Desktop): Fixed log spam and high CPU utilization when offline by moving `USE_AUTH_EMULATOR` environment variable check to initialization and eliminating per-request logging (#1629).
     - Messaging: Added new Registration methods using Installation Ids.
       Deprecated old Token based methods.
+    - Remote Config: Add support for setting Custom Signals.
 
 ### 13.11.0
 - Changes

remote_config/integration_test/src/integration_test.cc

@@ -273,9 +273,8 @@ TEST_F(FirebaseRemoteConfigTest, TestAddOnConfigUpdateListener) {
   // Check if the config has default values. If not, we have cached data
   // from a previous test run, and auto-fetch will not happen.
   EXPECT_TRUE(WaitForCompletion(SetDefaults(rc_), "SetDefaults"));
-  bool validated_defaults = true;
   firebase::remote_config::ValueInfo value_info;
-  bool bool_value = rc_->GetBoolean("TestBoolean", &value_info);
+  rc_->GetBoolean("TestBoolean", &value_info);
   bool has_cached_data =
       value_info.source != firebase::remote_config::kValueSourceDefaultValue;
 
@@ -507,4 +506,17 @@ TEST_F(FirebaseRemoteConfigTest, TestFetchSecondsParameter) {
 
   FLAKY_TEST_SECTION_END();
 }
+
+TEST_F(FirebaseRemoteConfigTest, TestSetCustomSignals) {
+  ASSERT_NE(rc_, nullptr);
+
+  std::map<std::string, firebase::Variant> custom_signals = {
+      {"test_string", firebase::Variant("alpha")},
+      {"test_int", firebase::Variant(42)},
+      {"test_double", firebase::Variant(3.14)}};
+
+  EXPECT_TRUE(WaitForCompletion(rc_->SetCustomSignals(custom_signals),
+                                "SetCustomSignals"));
+  EXPECT_EQ(rc_->SetCustomSignalsLastResult().error(), 0);
+}
 }  // namespace firebase_testapp_automated

remote_config/src/android/remote_config_android.cc

@@ -60,6 +60,9 @@ DEFINE_FIREBASE_VERSION_STRING(FirebaseRemoteConfig);
   X(SetConfigSettingsAsync, "setConfigSettingsAsync",                      \
     "(Lcom/google/firebase/remoteconfig/FirebaseRemoteConfigSettings;)"    \
     "Lcom/google/android/gms/tasks/Task;"),                                \
+  X(SetCustomSignalsAsync, "setCustomSignals",                             \
+    "(Lcom/google/firebase/remoteconfig/CustomSignals;)"                   \
+    "Lcom/google/android/gms/tasks/Task;"),                                \
   X(GetLong, "getLong", "(Ljava/lang/String;)J"),                          \
   X(GetString, "getString", "(Ljava/lang/String;)Ljava/lang/String;"),     \
   X(GetBoolean, "getBoolean", "(Ljava/lang/String;)Z"),                    \
@@ -150,6 +153,30 @@ METHOD_LOOKUP_DEFINITION(
     "com/google/firebase/remoteconfig/FirebaseRemoteConfigSettings$Builder",
     REMOTE_CONFIG_SETTINGS_BUILDER_METHODS)
 
+// Methods of CustomSignals.Builder
+// clang-format off
+#define CUSTOM_SIGNALS_BUILDER_METHODS(X)                                 \
+  X(Constructor, "<init>", "()V"),                                        \
+  X(PutString, "put",                                                     \
+    "(Ljava/lang/String;Ljava/lang/String;)"                              \
+    "Lcom/google/firebase/remoteconfig/CustomSignals$Builder;"),          \
+  X(PutLong, "put",                                                       \
+    "(Ljava/lang/String;J)"                                               \
+    "Lcom/google/firebase/remoteconfig/CustomSignals$Builder;"),          \
+  X(PutDouble, "put",                                                     \
+    "(Ljava/lang/String;D)"                                               \
+    "Lcom/google/firebase/remoteconfig/CustomSignals$Builder;"),          \
+  X(Build, "build",                                                       \
+    "()Lcom/google/firebase/remoteconfig/CustomSignals;")
+// clang-format on
+METHOD_LOOKUP_DECLARATION(custom_signals_builder,
+                          CUSTOM_SIGNALS_BUILDER_METHODS)
+METHOD_LOOKUP_DEFINITION(
+    custom_signals_builder,
+    PROGUARD_KEEP_CLASS
+    "com/google/firebase/remoteconfig/CustomSignals$Builder",
+    CUSTOM_SIGNALS_BUILDER_METHODS)
+
 // Methods of FirebaseRemoteConfigFetchThrottledException.
 // clang-format off
 #define REMOTE_CONFIG_THROTTLED_EXCEPTION_METHODS(X)     \
@@ -248,6 +275,7 @@ static bool CacheJNIMethodIds(
           config_info::CacheMethodIds(env, activity) &&
           config_settings::CacheMethodIds(env, activity) &&
           config_settings_builder::CacheMethodIds(env, activity) &&
+          custom_signals_builder::CacheMethodIds(env, activity) &&
           throttled_exception::CacheMethodIds(env, activity) &&
           config_update::CacheMethodIds(env, activity) &&
           config_update_listener_registration::CacheMethodIds(env, activity));
@@ -260,6 +288,7 @@ static void ReleaseClasses(JNIEnv* env) {
   config_info::ReleaseClass(env);
   config_settings::ReleaseClass(env);
   config_settings_builder::ReleaseClass(env);
+  custom_signals_builder::ReleaseClass(env);
   throttled_exception::ReleaseClass(env);
   config_update::ReleaseClass(env);
   config_update_listener_registration::ReleaseClass(env);
@@ -361,6 +390,88 @@ static jobject ConfigKeyValueVariantArrayToHashMap(
   return hash_map;
 }
 
+// Convert a std::map<std::string, Variant> into a Java CustomSignals object
+// using CustomSignals.Builder to populate String, Long, and Double signal
+// entries, or null values to clear signals.
+static jobject CustomSignalsFromMap(JNIEnv* env,
+                                    const std::map<std::string, Variant>& map) {
+  jobject builder = env->NewObject(custom_signals_builder::GetClass(),
+                                   custom_signals_builder::GetMethodId(
+                                       custom_signals_builder::kConstructor));
+  if (util::CheckAndClearJniExceptions(env) || !builder) return nullptr;
+
+  for (const auto& kv : map) {
+    jstring key = env->NewStringUTF(kv.first.c_str());
+    if (util::CheckAndClearJniExceptions(env) || !key) {
+      env->DeleteLocalRef(builder);
+      return nullptr;
+    }
+    jobject result_builder = nullptr;
+    if (kv.second.is_null()) {
+      result_builder =
+          env->CallObjectMethod(builder,
+                                custom_signals_builder::GetMethodId(
+                                    custom_signals_builder::kPutString),
+                                key, nullptr);
+    } else if (kv.second.is_string()) {
+      jstring str_val = env->NewStringUTF(kv.second.string_value());
+      if (util::CheckAndClearJniExceptions(env) || !str_val) {
+        env->DeleteLocalRef(key);
+        env->DeleteLocalRef(builder);
+        return nullptr;
+      }
+      result_builder =
+          env->CallObjectMethod(builder,
+                                custom_signals_builder::GetMethodId(
+                                    custom_signals_builder::kPutString),
+                                key, str_val);
+      env->DeleteLocalRef(str_val);
+    } else if (kv.second.is_int64()) {
+      result_builder = env->CallObjectMethod(
+          builder,
+          custom_signals_builder::GetMethodId(custom_signals_builder::kPutLong),
+          key, kv.second.int64_value());
+    } else if (kv.second.is_double()) {
+      result_builder =
+          env->CallObjectMethod(builder,
+                                custom_signals_builder::GetMethodId(
+                                    custom_signals_builder::kPutDouble),
+                                key, kv.second.double_value());
+    } else {
+      LogError(
+          "Remote Config: Invalid Variant type for SetCustomSignals() key %s.",
+          kv.first.c_str());
+      env->DeleteLocalRef(key);
+      env->DeleteLocalRef(builder);
+      return nullptr;
+    }
+
+    if (util::CheckAndClearJniExceptions(env) || !result_builder) {
+      if (result_builder) {
+        env->DeleteLocalRef(result_builder);
+      }
+      env->DeleteLocalRef(key);
+      env->DeleteLocalRef(builder);
+      return nullptr;
+    }
+
+    env->DeleteLocalRef(key);
+    env->DeleteLocalRef(result_builder);
+  }
+
+  jobject custom_signals = env->CallObjectMethod(
+      builder,
+      custom_signals_builder::GetMethodId(custom_signals_builder::kBuild));
+  if (util::CheckAndClearJniExceptions(env) || !custom_signals) {
+    if (custom_signals) {
+      env->DeleteLocalRef(custom_signals);
+    }
+    custom_signals = nullptr;
+  }
+  env->DeleteLocalRef(builder);
+  return custom_signals;
+}
+
 // Check pending exceptions following a key fetch and log an error if a
 // failure occurred.  If an error occurs this method returns true, false
 // otherwise.
@@ -959,6 +1070,40 @@ Future<void> RemoteConfigInternal::SetConfigSettingsLastResult() {
       future_impl_.LastResult(kRemoteConfigFnSetConfigSettings));
 }
 
+Future<void> RemoteConfigInternal::SetCustomSignals(
+    const std::map<std::string, Variant>& custom_signals) {
+  const auto handle =
+      future_impl_.SafeAlloc<void>(kRemoteConfigFnSetCustomSignals);
+  JNIEnv* env = app_.GetJNIEnv();
+  jobject j_custom_signals = CustomSignalsFromMap(env, custom_signals);
+  if (!j_custom_signals) {
+    future_impl_.Complete(handle, kFutureStatusFailure,
+                          "SetCustomSignals native function fails");
+    return MakeFuture<void>(&future_impl_, handle);
+  }
+  jobject task = env->CallObjectMethod(
+      internal_obj_, config::GetMethodId(config::kSetCustomSignalsAsync),
+      j_custom_signals);
+  if (util::CheckAndClearJniExceptions(env)) {
+    task = nullptr;
+    future_impl_.Complete(handle, kFutureStatusFailure,
+                          "SetCustomSignals native function fails");
+  } else {
+    auto data_handle = new RCDataHandle<void>(&future_impl_, handle, this);
+    util::RegisterCallbackOnTask(env, task, CompleteVoidCallback,
+                                 reinterpret_cast<void*>(data_handle),
+                                 jni_task_id_.c_str());
+  }
+  env->DeleteLocalRef(task);
+  env->DeleteLocalRef(j_custom_signals);
+  return MakeFuture<void>(&future_impl_, handle);
+}
+
+Future<void> RemoteConfigInternal::SetCustomSignalsLastResult() {
+  return static_cast<const Future<void>&>(
+      future_impl_.LastResult(kRemoteConfigFnSetCustomSignals));
+}
+
 ConfigSettings RemoteConfigInternal::GetConfigSettings() {
   ConfigSettings settings;
   JNIEnv* env = app_.GetJNIEnv();

remote_config/src/android/remote_config_android.h

@@ -66,6 +66,9 @@ class RemoteConfigInternal {
   Future<void> SetDefaultsLastResult();
   Future<void> SetConfigSettings(ConfigSettings settings);
   Future<void> SetConfigSettingsLastResult();
+  Future<void> SetCustomSignals(
+      const std::map<std::string, Variant>& custom_signals);
+  Future<void> SetCustomSignalsLastResult();
   ConfigSettings GetConfigSettings();
   bool GetBoolean(const char* key, ValueInfo* info);
 

remote_config/src/common.h

@@ -28,6 +28,7 @@ enum RemoteConfigFn {
   kRemoteConfigFnFetchAndActivate,
   kRemoteConfigFnSetDefaults,
   kRemoteConfigFnSetConfigSettings,
+  kRemoteConfigFnSetCustomSignals,
   kRemoteConfigFnCount
 };
 

remote_config/src/desktop/metadata.cc

@@ -20,6 +20,7 @@
 #include <limits>
 #include <map>
 #include <string>
+#include <vector>
 
 #include "flatbuffers/flexbuffers.h"
 #include "remote_config/src/include/firebase/remote_config.h"
@@ -53,6 +54,20 @@ std::string RemoteConfigMetadata::Serialize() const {
         fbb.String(std::to_string(setting.first).c_str(), setting.second);
       }
     });
+
+    fbb.Map("custom_signals", [&]() {
+      for (const auto& signal : custom_signals_) {
+        const std::string& key = signal.first;
+        const Variant& val = signal.second;
+        if (val.is_string()) {
+          fbb.String(key.c_str(), val.string_value());
+        } else if (val.is_int64()) {
+          fbb.Int(key.c_str(), val.int64_value());
+        } else if (val.is_double()) {
+          fbb.Double(key.c_str(), val.double_value());
+        }
+      }
+    });
   });
   fbb.Finish();
   const std::vector<uint8_t>& buffer = fbb.GetBuffer();
@@ -98,6 +113,22 @@ void RemoteConfigMetadata::Deserialize(const std::string& buffer) {
     settings_[static_cast<ConfigSetting>(int_key)] =
         settings.Values()[i].AsString().c_str();
   }
+
+  custom_signals_.clear();
+  flexbuffers::Map custom_signals = struct_map["custom_signals"].AsMap();
+  for (size_t i = 0, n = custom_signals.size(); i < n; ++i) {
+    const char* key_str = custom_signals.Keys()[i].AsKey();
+    if (!key_str) continue;
+    flexbuffers::Reference val_ref = custom_signals.Values()[i];
+    if (val_ref.IsString()) {
+      custom_signals_[key_str] =
+          Variant(std::string(val_ref.AsString().c_str()));
+    } else if (val_ref.IsInt()) {
+      custom_signals_[key_str] = Variant(val_ref.AsInt64());
+    } else if (val_ref.IsFloat()) {
+      custom_signals_[key_str] = Variant(val_ref.AsDouble());
+    }
+  }
 }
 
 void RemoteConfigMetadata::AddSetting(const ConfigSetting& setting,
@@ -116,6 +147,7 @@ std::string RemoteConfigMetadata::GetSetting(
 bool RemoteConfigMetadata::operator==(const RemoteConfigMetadata& right) const {
   return digest_by_namespace_ == right.digest_by_namespace_ &&
          settings_ == right.settings_ &&
+         custom_signals_ == right.custom_signals_ &&
          info_.fetch_time == right.info_.fetch_time &&
          info_.last_fetch_status == right.info_.last_fetch_status &&
          info_.last_fetch_failure_reason ==

remote_config/src/desktop/metadata.h

@@ -18,6 +18,7 @@
 #include <map>
 #include <string>
 
+#include "firebase/variant.h"
 #include "flatbuffers/flexbuffers.h"
 #include "remote_config/src/include/firebase/remote_config.h"
 
@@ -27,6 +28,7 @@ namespace internal {
 
 typedef std::map<std::string, std::string> MetaDigestMap;
 typedef std::map<ConfigSetting, std::string> MetaSettingsMap;
+typedef std::map<std::string, Variant> MetaCustomSignalsMap;
 
 // Contains different data about Remote Config Client.
 //
@@ -38,6 +40,7 @@ typedef std::map<ConfigSetting, std::string> MetaSettingsMap;
 //  * settings map: corresponds to a single supported setting, "developer mode"
 //  * digest map: Server computed digest (hash) of the config entries, stored
 //                per config namespace.
+//  * custom_signals map: Custom signals dictionary set by the developer.
 class RemoteConfigMetadata {
  public:
   RemoteConfigMetadata();
@@ -62,6 +65,12 @@ class RemoteConfigMetadata {
   // Return setting value by setting. Return "0" if value does not given.
   std::string GetSetting(const ConfigSetting& setting) const;
 
+  // Returns a map of custom signals.
+  const MetaCustomSignalsMap& custom_signals() const { return custom_signals_; }
+  void set_custom_signals(const MetaCustomSignalsMap& custom_signals) {
+    custom_signals_ = custom_signals;
+  }
+
   bool operator==(const RemoteConfigMetadata& right) const;
 
  private:
@@ -80,6 +89,9 @@ class RemoteConfigMetadata {
   // For now it's only one key: kConfigSettingDeveloperMode. Set "1" to enable
   // and "0" to disable.
   MetaSettingsMap settings_;
+
+  // Custom signals.
+  MetaCustomSignalsMap custom_signals_;
 };
 
 // Helper to deserialize elements of a Flexbuffer Map to map or unordered map,