From faf31e8883cff1b71ffbd65c7a3bc638ec7605a9 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Thu, 29 May 2025 20:18:20 +0000 Subject: [PATCH 01/10] Add SetDefaultEventParameters and ClearDefaultEventParameters to Analytics This change introduces two new C++ Analytics SDK functions: - `SetDefaultEventParameters(const std::map& params)`: Allows setting default parameters that will be included in all subsequent `LogEvent` calls. If a `Variant::Null()` is provided as a value for a key, that specific default parameter will be cleared/removed. - `ClearDefaultEventParameters()`: Clears all currently set default event parameters. Platform implementations: - iOS: Uses `[FIRAnalytics setDefaultEventParameters:]`. `Variant::Null()` maps to `[NSNull null]` for clearing individual parameters. Calling with `nil` clears all. - Android: Uses `FirebaseAnalytics.setDefaultEventParameters(Bundle)`. `Variant::Null()` results in `Bundle.putString(key, null)`. Calling with a `null` Bundle clears all. - Stub: Implemented as no-ops. Unit tests and integration tests have been updated to cover these new functionalities, including the null-value handling for clearing specific parameters and the overall clearing of all parameters. --- .../integration_test/src/integration_test.cc | 57 +++++++++++++ analytics/src/analytics_android.cc | 68 +++++++++++++++ analytics/src/analytics_ios.mm | 36 ++++++++ analytics/src/analytics_stub.cc | 11 +++ analytics/src/include/firebase/analytics.h | 17 ++++ analytics/src_ios/fake/FIRAnalytics.h | 2 + analytics/src_ios/fake/FIRAnalytics.mm | 13 +++ .../firebase/analytics/FirebaseAnalytics.java | 26 ++++++ analytics/tests/analytics_test.cc | 84 +++++++++++++++++++ 9 files changed, 314 insertions(+) diff --git a/analytics/integration_test/src/integration_test.cc b/analytics/integration_test/src/integration_test.cc index bedccee516..401d0e5203 100644 --- a/analytics/integration_test/src/integration_test.cc +++ b/analytics/integration_test/src/integration_test.cc @@ -341,4 +341,61 @@ TEST_F(FirebaseAnalyticsTest, TestSetConsent) { did_test_setconsent_ = true; } +TEST_F(FirebaseAnalyticsTest, TestDefaultEventParametersUsage) { + LogInfo("Testing SetDefaultEventParameters with initial values, then updating with Null."); + + std::map initial_defaults; + initial_defaults["initial_key"] = "initial_value"; + initial_defaults["key_to_be_nulled"] = "text_before_null"; + initial_defaults["numeric_default"] = 12345LL; + + LogInfo("Setting initial default event parameters."); + firebase::analytics::SetDefaultEventParameters(initial_defaults); + // Log an event that would pick up these defaults. + firebase::analytics::LogEvent("event_with_initial_defaults"); + LogInfo("Logged event_with_initial_defaults."); + ProcessEvents(500); // Short pause for event logging, if it matters for backend. + + std::map updated_defaults; + updated_defaults["key_to_be_nulled"] = firebase::analytics::Variant::Null(); + updated_defaults["another_key"] = "another_value"; + // "initial_key" should persist if not overwritten. + // "numeric_default" should persist. + + LogInfo("Updating default event parameters, setting key_to_be_nulled to Null."); + firebase::analytics::SetDefaultEventParameters(updated_defaults); + // Log an event that would pick up the updated defaults. + firebase::analytics::LogEvent("event_after_nulling_and_adding"); + LogInfo("Logged event_after_nulling_and_adding."); + ProcessEvents(500); + + // For this C++ SDK integration test, we primarily ensure API calls complete. + // Actual parameter presence on logged events would be verified by backend/native tests. + LogInfo("TestDefaultEventParametersUsage completed. Calls were made successfully."); +} + +TEST_F(FirebaseAnalyticsTest, TestClearDefaultEventParametersFunctionality) { + LogInfo("Testing ClearDefaultEventParameters."); + + std::map defaults_to_clear; + defaults_to_clear["default_one"] = "will_be_cleared"; + defaults_to_clear["default_two"] = 9876LL; + + LogInfo("Setting default parameters before clearing."); + firebase::analytics::SetDefaultEventParameters(defaults_to_clear); + // Log an event that would pick up these defaults. + firebase::analytics::LogEvent("event_before_global_clear"); + LogInfo("Logged event_before_global_clear."); + ProcessEvents(500); + + LogInfo("Calling ClearDefaultEventParameters."); + firebase::analytics::ClearDefaultEventParameters(); + // Log an event that should not have the previous defaults. + firebase::analytics::LogEvent("event_after_global_clear"); + LogInfo("Logged event_after_global_clear."); + ProcessEvents(500); + + LogInfo("TestClearDefaultEventParametersFunctionality completed. Call was made successfully."); +} + } // namespace firebase_testapp_automated diff --git a/analytics/src/analytics_android.cc b/analytics/src/analytics_android.cc index d1279c6ba7..a21b967fd3 100644 --- a/analytics/src/analytics_android.cc +++ b/analytics/src/analytics_android.cc @@ -58,6 +58,8 @@ static const ::firebase::App* g_app = nullptr; "()Lcom/google/android/gms/tasks/Task;"), \ X(GetSessionId, "getSessionId", \ "()Lcom/google/android/gms/tasks/Task;"), \ + X(SetDefaultEventParameters, "setDefaultEventParameters", \ + "(Landroid/os/Bundle;)V", util::kMethodTypeInstance), \ X(GetInstance, "getInstance", "(Landroid/content/Context;)" \ "Lcom/google/firebase/analytics/FirebaseAnalytics;", \ firebase::util::kMethodTypeStatic) @@ -609,6 +611,72 @@ void ResetAnalyticsData() { util::CheckAndClearJniExceptions(env); } +void SetDefaultEventParameters( + const std::map& default_parameters) { + FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized()); + JNIEnv* env = g_app->GetJNIEnv(); + if (!env) return; + + jobject bundle = + env->NewObject(util::bundle::GetClass(), + util::bundle::GetMethodId(util::bundle::kConstructor)); + if (util::CheckAndClearJniExceptions(env) || !bundle) { + LogError("Failed to create Bundle for SetDefaultEventParameters."); + if (bundle) env->DeleteLocalRef(bundle); + return; + } + + for (const auto& pair : default_parameters) { + jstring key_jstring = env->NewStringUTF(pair.first.c_str()); + if (util::CheckAndClearJniExceptions(env) || !key_jstring) { + LogError("Failed to create jstring for key: %s", pair.first.c_str()); + if (key_jstring) env->DeleteLocalRef(key_jstring); + continue; + } + + if (pair.second.is_null()) { + // Equivalent to Bundle.putString(String key, String value) with value as null. + env->CallVoidMethod(bundle, util::bundle::GetMethodId(util::bundle::kPutString), + key_jstring, nullptr); + if (util::CheckAndClearJniExceptions(env)) { + LogError("Failed to put null string for key: %s", pair.first.c_str()); + } + } else { + if (!AddVariantToBundle(env, bundle, pair.first.c_str(), pair.second)) { + // AddVariantToBundle already logs errors for unsupported types. + } + } + env->DeleteLocalRef(key_jstring); + } + + env->CallVoidMethod(g_analytics_class_instance, + analytics::GetMethodId(analytics::kSetDefaultEventParameters), + bundle); + if (util::CheckAndClearJniExceptions(env)) { + LogError("Failed to call setDefaultEventParameters on Java instance."); + } + + env->DeleteLocalRef(bundle); +} + +void ClearDefaultEventParameters() { + FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized()); + JNIEnv* env = g_app->GetJNIEnv(); + if (!env) return; + + // Calling with nullptr bundle should clear the parameters. + env->CallVoidMethod(g_analytics_class_instance, + analytics::GetMethodId(analytics::kSetDefaultEventParameters), + nullptr); + if (util::CheckAndClearJniExceptions(env)) { + // This might happen if the method isn't available on older SDKs, + // or if some other JNI error occurs. + LogError("Failed to call setDefaultEventParameters(null) on Java instance. " + "This may indicate the method is not available on this Android SDK version " + "or another JNI error occurred."); + } +} + Future GetAnalyticsInstanceId() { FIREBASE_ASSERT_RETURN(GetAnalyticsInstanceIdLastResult(), internal::IsInitialized()); diff --git a/analytics/src/analytics_ios.mm b/analytics/src/analytics_ios.mm index d56fd61356..34db04ba69 100644 --- a/analytics/src/analytics_ios.mm +++ b/analytics/src/analytics_ios.mm @@ -373,6 +373,42 @@ void SetSessionTimeoutDuration(int64_t milliseconds) { setSessionTimeoutInterval:static_cast(milliseconds) / kMillisecondsPerSecond]; } +void SetDefaultEventParameters( + const std::map& default_parameters) { + FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized()); + NSMutableDictionary* ns_default_parameters = + [[NSMutableDictionary alloc] initWithCapacity:default_parameters.size()]; + for (const auto& pair : default_parameters) { + NSString* key = SafeString(pair.first.c_str()); + const Variant& value = pair.second; + + if (value.is_null()) { + [ns_default_parameters setObject:[NSNull null] forKey:key]; + } else if (value.is_int64()) { + [ns_default_parameters setObject:[NSNumber numberWithLongLong:value.int64_value()] forKey:key]; + } else if (value.is_double()) { + [ns_default_parameters setObject:[NSNumber numberWithDouble:value.double_value()] forKey:key]; + } else if (value.is_string()) { + [ns_default_parameters setObject:SafeString(value.string_value()) forKey:key]; + } else if (value.is_bool()) { + [ns_default_parameters setObject:[NSNumber numberWithBool:value.bool_value()] forKey:key]; + } else { + // Log an error for unsupported types. + // Note: FIRAnalytics.setDefaultEventParameters only supports NSNumber, NSString, NSNull. + // It does not support nested collections (NSArray, NSDictionary) unlike LogEvent. + LogError("SetDefaultEventParameters: Unsupported Variant type (%s) for key %s. " + "Only Int64, Double, String, Bool, and Null are supported for default event parameters.", + Variant::TypeName(value.type()), pair.first.c_str()); + } + } + [FIRAnalytics setDefaultEventParameters:ns_default_parameters]; +} + +void ClearDefaultEventParameters() { + FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized()); + [FIRAnalytics setDefaultEventParameters:nil]; +} + void ResetAnalyticsData() { MutexLock lock(g_mutex); FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized()); diff --git a/analytics/src/analytics_stub.cc b/analytics/src/analytics_stub.cc index 37c4b3520b..4ae7410bbc 100644 --- a/analytics/src/analytics_stub.cc +++ b/analytics/src/analytics_stub.cc @@ -142,6 +142,17 @@ void SetSessionTimeoutDuration(int64_t /*milliseconds*/) { FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized()); } +void SetDefaultEventParameters( + const std::map& /*default_parameters*/) { + FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized()); + // This is a stub implementation. No operation needed. +} + +void ClearDefaultEventParameters() { + FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized()); + // This is a stub implementation. No operation needed. +} + void ResetAnalyticsData() { FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized()); g_fake_instance_id++; diff --git a/analytics/src/include/firebase/analytics.h b/analytics/src/include/firebase/analytics.h index 746df99894..f9bdc64eb0 100644 --- a/analytics/src/include/firebase/analytics.h +++ b/analytics/src/include/firebase/analytics.h @@ -558,6 +558,23 @@ void SetSessionTimeoutDuration(int64_t milliseconds); /// instance id. void ResetAnalyticsData(); +/// @brief Sets the default event parameters. +/// +/// These parameters will be automatically logged with all calls to `LogEvent`. +/// Default parameters are overridden by parameters supplied to the `LogEvent` +/// method. +/// +/// When a value in the `default_parameters` map is +/// `firebase::Variant::Null()`, it signifies that the default parameter for +/// that specific key should be cleared. +/// +/// @param[in] default_parameters A map of parameter names to Variant values. +void SetDefaultEventParameters( + const std::map& default_parameters); + +/// @brief Clears all default event parameters. +void ClearDefaultEventParameters(); + /// Get the instance ID from the analytics service. /// /// @note This is *not* the same ID as the ID returned by diff --git a/analytics/src_ios/fake/FIRAnalytics.h b/analytics/src_ios/fake/FIRAnalytics.h index 50b1b7ca36..90ffb32fb7 100644 --- a/analytics/src_ios/fake/FIRAnalytics.h +++ b/analytics/src_ios/fake/FIRAnalytics.h @@ -37,4 +37,6 @@ + (void)resetAnalyticsData; ++ (void)setDefaultEventParameters:(nullable NSDictionary *)parameters; + @end diff --git a/analytics/src_ios/fake/FIRAnalytics.mm b/analytics/src_ios/fake/FIRAnalytics.mm index 11048e3dd0..64bd1c343d 100644 --- a/analytics/src_ios/fake/FIRAnalytics.mm +++ b/analytics/src_ios/fake/FIRAnalytics.mm @@ -21,6 +21,9 @@ @implementation FIRAnalytics + (NSString *)stringForValue:(id)value { + if (value == [NSNull null]) { + return @""; + } return [NSString stringWithFormat:@"%@", value]; } @@ -94,4 +97,14 @@ + (void)resetAnalyticsData { FakeReporter->AddReport("+[FIRAnalytics resetAnalyticsData]", {}); } ++ (void)setDefaultEventParameters:(nullable NSDictionary *)parameters { + if (parameters == nil) { + FakeReporter->AddReport("+[FIRAnalytics setDefaultEventParameters:]", {"nil"}); + } else { + NSString *parameterString = [self stringForParameters:parameters]; + FakeReporter->AddReport("+[FIRAnalytics setDefaultEventParameters:]", + { [parameterString UTF8String] }); + } +} + @end diff --git a/analytics/src_java/fake/com/google/firebase/analytics/FirebaseAnalytics.java b/analytics/src_java/fake/com/google/firebase/analytics/FirebaseAnalytics.java index 8be63c473d..208a85612a 100644 --- a/analytics/src_java/fake/com/google/firebase/analytics/FirebaseAnalytics.java +++ b/analytics/src_java/fake/com/google/firebase/analytics/FirebaseAnalytics.java @@ -79,4 +79,30 @@ public void setSessionTimeoutDuration(long milliseconds) { FakeReporter.addReport( "FirebaseAnalytics.setSessionTimeoutDuration", Long.toString(milliseconds)); } + + public void setDefaultEventParameters(Bundle bundle) { + if (bundle == null) { + FakeReporter.addReport("FirebaseAnalytics.setDefaultEventParameters", "null"); + } else { + StringBuilder paramsString = new StringBuilder(); + // Sort keys for predictable ordering. + for (String key : new TreeSet<>(bundle.keySet())) { + paramsString.append(key); + paramsString.append("="); + Object value = bundle.get(key); // Get as Object first + if (value == null) { + // This case handles when bundle.putString(key, null) was called. + paramsString.append(""); + } else { + paramsString.append(value.toString()); + } + paramsString.append(","); + } + // Remove trailing comma if paramsString is not empty + if (paramsString.length() > 0) { + paramsString.setLength(paramsString.length() - 1); + } + FakeReporter.addReport("FirebaseAnalytics.setDefaultEventParameters", paramsString.toString()); + } + } } diff --git a/analytics/tests/analytics_test.cc b/analytics/tests/analytics_test.cc index 8235c69e25..e6180fbd23 100644 --- a/analytics/tests/analytics_test.cc +++ b/analytics/tests/analytics_test.cc @@ -296,5 +296,89 @@ TEST_F(AnalyticsTest, TestGetAnalyticsInstanceId) { EXPECT_EQ(std::string("FakeAnalyticsInstanceId0"), *result.result()); } +TEST_F(AnalyticsTest, TestSetDefaultEventParameters) { + // Android part is not tested here as it's about iOS fakes. + // This test focuses on the iOS fake reporting. + // Android: AddExpectationAndroid("FirebaseAnalytics.setDefaultEventParameters", {"my_param_bool=true,my_param_double=3.14,my_param_int=123,my_param_string=hello"}); + AddExpectationApple("+[FIRAnalytics setDefaultEventParameters:]", + {"my_param_bool=1,my_param_double=3.14,my_param_int=123,my_param_string=hello"}); + + std::map default_params; + default_params["my_param_string"] = "hello"; + default_params["my_param_double"] = 3.14; + default_params["my_param_int"] = 123; + default_params["my_param_bool"] = true; // Note: [NSNumber numberWithBool:YES] stringifies to 1 + + SetDefaultEventParameters(default_params); +} + +TEST_F(AnalyticsTest, TestSetDefaultEventParametersWithNull) { + // Android: AddExpectationAndroid("FirebaseAnalytics.setDefaultEventParameters", {"key_to_clear=null,other_key=value"}); + AddExpectationApple("+[FIRAnalytics setDefaultEventParameters:]", + {"key_to_clear=,other_key=value"}); + + std::map default_params; + default_params["key_to_clear"] = Variant::Null(); + default_params["other_key"] = "value"; + + SetDefaultEventParameters(default_params); +} + +TEST_F(AnalyticsTest, TestSetDefaultEventParametersEmpty) { + // Android: AddExpectationAndroid("FirebaseAnalytics.setDefaultEventParameters", {""}); // Or however an empty bundle is represented + AddExpectationApple("+[FIRAnalytics setDefaultEventParameters:]", {""}); + + std::map default_params; + SetDefaultEventParameters(default_params); +} + +TEST_F(AnalyticsTest, TestClearDefaultEventParameters) { + // Android: AddExpectationAndroid("FirebaseAnalytics.setDefaultEventParameters", {"null"}); // Passing null bundle + AddExpectationApple("+[FIRAnalytics setDefaultEventParameters:]", {"nil"}); + + ClearDefaultEventParameters(); +} + +TEST_F(AnalyticsTest, TestSetDefaultEventParametersAndroid) { + AddExpectationAndroid("FirebaseAnalytics.setDefaultEventParameters", + {"my_bool=true,my_double=3.14,my_int=123,my_string=hello"}); + + std::map default_params; + default_params["my_string"] = "hello"; + default_params["my_double"] = 3.14; + default_params["my_int"] = 123; + default_params["my_bool"] = true; + + SetDefaultEventParameters(default_params); + WaitForMainThreadTask(); +} + +TEST_F(AnalyticsTest, TestSetDefaultEventParametersWithNullValueAndroid) { + AddExpectationAndroid("FirebaseAnalytics.setDefaultEventParameters", + {"key_to_clear=,other_key=value"}); + + std::map default_params; + default_params["key_to_clear"] = Variant::Null(); + default_params["other_key"] = "value"; + + SetDefaultEventParameters(default_params); + WaitForMainThreadTask(); +} + +TEST_F(AnalyticsTest, TestSetDefaultEventParametersEmptyAndroid) { + AddExpectationAndroid("FirebaseAnalytics.setDefaultEventParameters", {""}); + + std::map default_params; + SetDefaultEventParameters(default_params); + WaitForMainThreadTask(); +} + +TEST_F(AnalyticsTest, TestClearDefaultEventParametersAndroid) { + AddExpectationAndroid("FirebaseAnalytics.setDefaultEventParameters", {"null"}); + + ClearDefaultEventParameters(); + WaitForMainThreadTask(); +} + } // namespace analytics } // namespace firebase From 8dad0370aa3f4bbe39d0a71bf10667b05722e418 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 30 May 2025 02:00:04 +0000 Subject: [PATCH 02/10] Add SetDefaultEventParameters and ClearDefaultEventParameters to Analytics This change introduces two new C++ Analytics SDK functions: - `SetDefaultEventParameters(const std::map& params)`: Allows setting default parameters that will be included in all subsequent `LogEvent` calls. If a `firebase::Variant::Null()` is provided as a value for a key, that specific default parameter will be cleared/removed. - `ClearDefaultEventParameters()`: Clears all currently set default event parameters. Platform implementations: - iOS: Uses `[FIRAnalytics setDefaultEventParameters:]`. `firebase::Variant::Null()` maps to `[NSNull null]` for clearing individual parameters. Calling with `nil` clears all. - Android: Uses `FirebaseAnalytics.setDefaultEventParameters(Bundle)`. `firebase::Variant::Null()` results in `Bundle.putString(key, null)`. Calling with a `null` Bundle clears all. - Stub: Implemented as no-ops. Unit tests and integration tests have been updated to cover these new functionalities, including the null-value handling for clearing specific parameters and the overall clearing of all parameters. --- .../integration_test/src/integration_test.cc | 8 +++---- analytics/src/analytics_android.cc | 18 +++++++------- analytics/src/analytics_ios.mm | 24 +++++++++---------- analytics/src/include/firebase/analytics.h | 6 ++--- 4 files changed, 28 insertions(+), 28 deletions(-) diff --git a/analytics/integration_test/src/integration_test.cc b/analytics/integration_test/src/integration_test.cc index 401d0e5203..92d4dcab1e 100644 --- a/analytics/integration_test/src/integration_test.cc +++ b/analytics/integration_test/src/integration_test.cc @@ -344,7 +344,7 @@ TEST_F(FirebaseAnalyticsTest, TestSetConsent) { TEST_F(FirebaseAnalyticsTest, TestDefaultEventParametersUsage) { LogInfo("Testing SetDefaultEventParameters with initial values, then updating with Null."); - std::map initial_defaults; + std::map initial_defaults; initial_defaults["initial_key"] = "initial_value"; initial_defaults["key_to_be_nulled"] = "text_before_null"; initial_defaults["numeric_default"] = 12345LL; @@ -356,8 +356,8 @@ TEST_F(FirebaseAnalyticsTest, TestDefaultEventParametersUsage) { LogInfo("Logged event_with_initial_defaults."); ProcessEvents(500); // Short pause for event logging, if it matters for backend. - std::map updated_defaults; - updated_defaults["key_to_be_nulled"] = firebase::analytics::Variant::Null(); + std::map updated_defaults; + updated_defaults["key_to_be_nulled"] = firebase::Variant::Null(); updated_defaults["another_key"] = "another_value"; // "initial_key" should persist if not overwritten. // "numeric_default" should persist. @@ -377,7 +377,7 @@ TEST_F(FirebaseAnalyticsTest, TestDefaultEventParametersUsage) { TEST_F(FirebaseAnalyticsTest, TestClearDefaultEventParametersFunctionality) { LogInfo("Testing ClearDefaultEventParameters."); - std::map defaults_to_clear; + std::map defaults_to_clear; defaults_to_clear["default_one"] = "will_be_cleared"; defaults_to_clear["default_two"] = 9876LL; diff --git a/analytics/src/analytics_android.cc b/analytics/src/analytics_android.cc index a21b967fd3..a0e3b48ed3 100644 --- a/analytics/src/analytics_android.cc +++ b/analytics/src/analytics_android.cc @@ -380,17 +380,17 @@ void AddBundleToBundle(JNIEnv* env, jobject bundle, const char* key, } // Declared here so that it can be used, defined below. -jobject MapToBundle(JNIEnv* env, const std::map& map); +jobject MapToBundle(JNIEnv* env, const std::map& map); // Converts the given vector into a Java ArrayList. It is up to the // caller to delete the local reference when done. jobject VectorOfMapsToArrayList(JNIEnv* env, - const std::vector& vector) { + const std::vector& vector) { jobject arraylist = env->NewObject( util::array_list::GetClass(), util::array_list::GetMethodId(util::array_list::kConstructor)); - for (const Variant& element : vector) { + for (const firebase::Variant& element : vector) { if (element.is_map()) { jobject bundle = MapToBundle(env, element.map()); env->CallBooleanMethod( @@ -400,7 +400,7 @@ jobject VectorOfMapsToArrayList(JNIEnv* env, env->DeleteLocalRef(bundle); } else { LogError("VectorOfMapsToArrayList: Unsupported type (%s) within vector.", - Variant::TypeName(element.type())); + firebase::Variant::TypeName(element.type())); } } return arraylist; @@ -408,7 +408,7 @@ jobject VectorOfMapsToArrayList(JNIEnv* env, // Converts and adds the Variant to the given Bundle. bool AddVariantToBundle(JNIEnv* env, jobject bundle, const char* key, - const Variant& value) { + const firebase::Variant& value) { if (value.is_int64()) { AddToBundle(env, bundle, key, value.int64_value()); } else if (value.is_double()) { @@ -440,7 +440,7 @@ bool AddVariantToBundle(JNIEnv* env, jobject bundle, const char* key, // Converts the given map into a Java Bundle. It is up to the caller // to delete the local reference when done. -jobject MapToBundle(JNIEnv* env, const std::map& map) { +jobject MapToBundle(JNIEnv* env, const std::map& map) { jobject bundle = env->NewObject(util::bundle::GetClass(), util::bundle::GetMethodId(util::bundle::kConstructor)); @@ -452,7 +452,7 @@ jobject MapToBundle(JNIEnv* env, const std::map& map) { if (!AddVariantToBundle(env, bundle, pair.first.string_value(), pair.second)) { LogError("MapToBundle: Unsupported type (%s) within map with key %s.", - Variant::TypeName(pair.second.type()), + firebase::Variant::TypeName(pair.second.type()), pair.first.string_value()); } util::CheckAndClearJniExceptions(env); @@ -514,7 +514,7 @@ void LogEvent(const char* name, const Parameter* parameters, LogError( "LogEvent(%s): %s is not a valid parameter value type. " "No event was logged.", - parameter.name, Variant::TypeName(parameter.value.type())); + parameter.name, firebase::Variant::TypeName(parameter.value.type())); } } }); @@ -612,7 +612,7 @@ void ResetAnalyticsData() { } void SetDefaultEventParameters( - const std::map& default_parameters) { + const std::map& default_parameters) { FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized()); JNIEnv* env = g_app->GetJNIEnv(); if (!env) return; diff --git a/analytics/src/analytics_ios.mm b/analytics/src/analytics_ios.mm index 34db04ba69..af4cb6f7f1 100644 --- a/analytics/src/analytics_ios.mm +++ b/analytics/src/analytics_ios.mm @@ -232,25 +232,25 @@ void LogEvent(const char* name) { } // Declared here so that it can be used, defined below. -NSDictionary* MapToDictionary(const std::map& map); +NSDictionary* MapToDictionary(const std::map& map); // Converts the given vector of Maps into an ObjC NSArray of ObjC NSDictionaries. -NSArray* VectorOfMapsToArray(const std::vector& vector) { +NSArray* VectorOfMapsToArray(const std::vector& vector) { NSMutableArray* array = [NSMutableArray arrayWithCapacity:vector.size()]; - for (const Variant& element : vector) { + for (const firebase::Variant& element : vector) { if (element.is_map()) { NSDictionary* dict = MapToDictionary(element.map()); [array addObject:dict]; } else { LogError("VectorOfMapsToArray: Unsupported type (%s) within vector.", - Variant::TypeName(element.type())); + firebase::Variant::TypeName(element.type())); } } return array; } // Converts and adds the Variant to the given Dictionary. -bool AddVariantToDictionary(NSMutableDictionary* dict, NSString* key, const Variant& value) { +bool AddVariantToDictionary(NSMutableDictionary* dict, NSString* key, const firebase::Variant& value) { if (value.is_int64()) { [dict setObject:[NSNumber numberWithLongLong:value.int64_value()] forKey:key]; } else if (value.is_double()) { @@ -277,7 +277,7 @@ bool AddVariantToDictionary(NSMutableDictionary* dict, NSString* key, const Vari } // Converts the given map into an ObjC dictionary of ObjC objects. -NSDictionary* MapToDictionary(const std::map& map) { +NSDictionary* MapToDictionary(const std::map& map) { NSMutableDictionary* dict = [NSMutableDictionary dictionaryWithCapacity:map.size()]; for (const auto& pair : map) { // Only add elements that use a string key @@ -285,10 +285,10 @@ bool AddVariantToDictionary(NSMutableDictionary* dict, NSString* key, const Vari continue; } NSString* key = SafeString(pair.first.string_value()); - const Variant& value = pair.second; + const firebase::Variant& value = pair.second; if (!AddVariantToDictionary(dict, key, value)) { LogError("MapToDictionary: Unsupported type (%s) within map with key %s.", - Variant::TypeName(value.type()), key); + firebase::Variant::TypeName(value.type()), key); } } return dict; @@ -306,7 +306,7 @@ void LogEvent(const char* name, const Parameter* parameters, size_t number_of_pa // A Variant type that couldn't be handled was passed in. LogError("LogEvent(%s): %s is not a valid parameter value type. " "No event was logged.", - parameter.name, Variant::TypeName(parameter.value.type())); + parameter.name, firebase::Variant::TypeName(parameter.value.type())); } } [FIRAnalytics logEventWithName:@(name) parameters:parameters_dict]; @@ -374,13 +374,13 @@ void SetSessionTimeoutDuration(int64_t milliseconds) { } void SetDefaultEventParameters( - const std::map& default_parameters) { + const std::map& default_parameters) { FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized()); NSMutableDictionary* ns_default_parameters = [[NSMutableDictionary alloc] initWithCapacity:default_parameters.size()]; for (const auto& pair : default_parameters) { NSString* key = SafeString(pair.first.c_str()); - const Variant& value = pair.second; + const firebase::Variant& value = pair.second; if (value.is_null()) { [ns_default_parameters setObject:[NSNull null] forKey:key]; @@ -398,7 +398,7 @@ void SetDefaultEventParameters( // It does not support nested collections (NSArray, NSDictionary) unlike LogEvent. LogError("SetDefaultEventParameters: Unsupported Variant type (%s) for key %s. " "Only Int64, Double, String, Bool, and Null are supported for default event parameters.", - Variant::TypeName(value.type()), pair.first.c_str()); + firebase::Variant::TypeName(value.type()), pair.first.c_str()); } } [FIRAnalytics setDefaultEventParameters:ns_default_parameters]; diff --git a/analytics/src/include/firebase/analytics.h b/analytics/src/include/firebase/analytics.h index f9bdc64eb0..eeafcbade8 100644 --- a/analytics/src/include/firebase/analytics.h +++ b/analytics/src/include/firebase/analytics.h @@ -142,7 +142,7 @@ struct Parameter { /// @param parameter_name Name of the parameter (see Parameter::name). /// @param parameter_value Value for the parameter. Variants can /// hold numbers and strings. - Parameter(const char* parameter_name, Variant parameter_value) + Parameter(const char* parameter_name, firebase::Variant parameter_value) : name(parameter_name) { value = parameter_value; } @@ -245,7 +245,7 @@ struct Parameter { /// /// See firebase::Variant for usage information. /// @note String values can be up to 100 characters long. - Variant value; + firebase::Variant value; #endif // SWIG }; @@ -570,7 +570,7 @@ void ResetAnalyticsData(); /// /// @param[in] default_parameters A map of parameter names to Variant values. void SetDefaultEventParameters( - const std::map& default_parameters); + const std::map& default_parameters); /// @brief Clears all default event parameters. void ClearDefaultEventParameters(); From 032dc376314910d20a5c34c8d3729a44f4bb0c66 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 30 May 2025 17:05:00 +0000 Subject: [PATCH 03/10] Add SetDefaultEventParameters and ClearDefaultEventParameters to Analytics This change introduces two new C++ Analytics SDK functions: - `SetDefaultEventParameters(const std::map& params)`: Allows setting default parameters that will be included in all subsequent `LogEvent` calls. If a `firebase::Variant::Null()` is provided as a value for a key, that specific default parameter will be cleared/removed. - `ClearDefaultEventParameters()`: Clears all currently set default event parameters. Platform implementations: - iOS: Uses `[FIRAnalytics setDefaultEventParameters:]`. `firebase::Variant::Null()` maps to `[NSNull null]` for clearing individual parameters. Calling with `nil` clears all. - Android: Uses `FirebaseAnalytics.setDefaultEventParameters(Bundle)`. `firebase::Variant::Null()` results in `Bundle.putString(key, null)`. Calling with a `null` Bundle clears all. - Stub: Implemented as no-ops. Unit tests and integration tests have been updated to cover these new functionalities, including the null-value handling for clearing specific parameters and the overall clearing of all parameters. Integer literals in integration tests use standard int types. --- analytics/integration_test/src/integration_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/analytics/integration_test/src/integration_test.cc b/analytics/integration_test/src/integration_test.cc index 92d4dcab1e..d16ff57f30 100644 --- a/analytics/integration_test/src/integration_test.cc +++ b/analytics/integration_test/src/integration_test.cc @@ -347,7 +347,7 @@ TEST_F(FirebaseAnalyticsTest, TestDefaultEventParametersUsage) { std::map initial_defaults; initial_defaults["initial_key"] = "initial_value"; initial_defaults["key_to_be_nulled"] = "text_before_null"; - initial_defaults["numeric_default"] = 12345LL; + initial_defaults["numeric_default"] = 12345; LogInfo("Setting initial default event parameters."); firebase::analytics::SetDefaultEventParameters(initial_defaults); @@ -379,7 +379,7 @@ TEST_F(FirebaseAnalyticsTest, TestClearDefaultEventParametersFunctionality) { std::map defaults_to_clear; defaults_to_clear["default_one"] = "will_be_cleared"; - defaults_to_clear["default_two"] = 9876LL; + defaults_to_clear["default_two"] = 9876; LogInfo("Setting default parameters before clearing."); firebase::analytics::SetDefaultEventParameters(defaults_to_clear); From b6da98c6b06868584803f9ccc0103036294a2701 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 30 May 2025 18:48:46 +0000 Subject: [PATCH 04/10] Add SetDefaultEventParameters and ClearDefaultEventParameters to Analytics This change introduces two new C++ Analytics SDK functions: - `SetDefaultEventParameters(const std::map& params)`: Allows setting default parameters (string, int64, double, bool, null) that will be included in all subsequent `LogEvent` calls. If a `firebase::Variant::Null()` is provided for a key, that specific default parameter will be cleared. Aggregate types (maps, vectors) are not supported and will be skipped with an error logged. - `ClearDefaultEventParameters()`: Clears all currently set default event parameters. Platform implementations: - iOS: Uses `[FIRAnalytics setDefaultEventParameters:]`. `firebase::Variant::Null()` maps to `[NSNull null]`. Unsupported types are skipped. - Android: Uses `FirebaseAnalytics.setDefaultEventParameters(Bundle)`. `firebase::Variant::Null()` results in `Bundle.putString(key, null)`. Unsupported types are skipped. `AddVariantToBundle` is used for efficiency with scalar types. - Stub: Implemented as no-ops. Unit tests and integration tests have been reviewed and updated to cover these functionalities and type constraints. --- analytics/src/analytics_android.cc | 49 +++++++++++++++++++----------- analytics/src/analytics_ios.mm | 11 +++---- 2 files changed, 36 insertions(+), 24 deletions(-) diff --git a/analytics/src/analytics_android.cc b/analytics/src/analytics_android.cc index a0e3b48ed3..0afc3543d6 100644 --- a/analytics/src/analytics_android.cc +++ b/analytics/src/analytics_android.cc @@ -627,26 +627,39 @@ void SetDefaultEventParameters( } for (const auto& pair : default_parameters) { - jstring key_jstring = env->NewStringUTF(pair.first.c_str()); - if (util::CheckAndClearJniExceptions(env) || !key_jstring) { - LogError("Failed to create jstring for key: %s", pair.first.c_str()); - if (key_jstring) env->DeleteLocalRef(key_jstring); - continue; - } - - if (pair.second.is_null()) { - // Equivalent to Bundle.putString(String key, String value) with value as null. - env->CallVoidMethod(bundle, util::bundle::GetMethodId(util::bundle::kPutString), - key_jstring, nullptr); - if (util::CheckAndClearJniExceptions(env)) { - LogError("Failed to put null string for key: %s", pair.first.c_str()); - } + const firebase::Variant& value = pair.second; + const char* key_cstr = pair.first.c_str(); + + if (value.is_null()) { + jstring key_jstring = env->NewStringUTF(key_cstr); + if (util::CheckAndClearJniExceptions(env) || !key_jstring) { + LogError("SetDefaultEventParameters: Failed to create jstring for null value key: %s", key_cstr); + if (key_jstring) env->DeleteLocalRef(key_jstring); + continue; + } + env->CallVoidMethod(bundle, util::bundle::GetMethodId(util::bundle::kPutString), + key_jstring, nullptr); + if (util::CheckAndClearJniExceptions(env)) { + LogError("SetDefaultEventParameters: Failed to put null string for key: %s", key_cstr); + } + env->DeleteLocalRef(key_jstring); + } else if (value.is_string() || value.is_int64() || value.is_double() || value.is_bool()) { + // AddVariantToBundle handles these types and their JNI conversions. + // It also logs if an individual AddToBundle within it fails or if a type is unsupported by it. + if (!AddVariantToBundle(env, bundle, key_cstr, value)) { + // This specific log gives context that the failure happened during SetDefaultEventParameters + // for a type that was expected to be supported by AddVariantToBundle. + LogError("SetDefaultEventParameters: Failed to add parameter for key '%s' with supported type '%s'. This might indicate a JNI issue during conversion.", + key_cstr, firebase::Variant::TypeName(value.type())); + } + } else if (value.is_vector() || value.is_map()) { + LogError("SetDefaultEventParameters: Value for key '%s' has type '%s' which is not supported for default event parameters. Only string, int64, double, bool, and null are supported. Skipping.", + key_cstr, firebase::Variant::TypeName(value.type())); } else { - if (!AddVariantToBundle(env, bundle, pair.first.c_str(), pair.second)) { - // AddVariantToBundle already logs errors for unsupported types. - } + // This case handles other fundamental Variant types that are not scalars and not vector/map. + LogError("SetDefaultEventParameters: Value for key '%s' has an unexpected and unsupported type '%s'. Skipping.", + key_cstr, firebase::Variant::TypeName(value.type())); } - env->DeleteLocalRef(key_jstring); } env->CallVoidMethod(g_analytics_class_instance, diff --git a/analytics/src/analytics_ios.mm b/analytics/src/analytics_ios.mm index af4cb6f7f1..1a0e386da5 100644 --- a/analytics/src/analytics_ios.mm +++ b/analytics/src/analytics_ios.mm @@ -392,13 +392,12 @@ void SetDefaultEventParameters( [ns_default_parameters setObject:SafeString(value.string_value()) forKey:key]; } else if (value.is_bool()) { [ns_default_parameters setObject:[NSNumber numberWithBool:value.bool_value()] forKey:key]; + } else if (value.is_vector() || value.is_map()) { + LogError("SetDefaultEventParameters: Value for key '%s' has type '%s' which is not supported for default event parameters. Only string, int64, double, bool, and null are supported. Skipping.", + pair.first.c_str(), firebase::Variant::TypeName(value.type())); } else { - // Log an error for unsupported types. - // Note: FIRAnalytics.setDefaultEventParameters only supports NSNumber, NSString, NSNull. - // It does not support nested collections (NSArray, NSDictionary) unlike LogEvent. - LogError("SetDefaultEventParameters: Unsupported Variant type (%s) for key %s. " - "Only Int64, Double, String, Bool, and Null are supported for default event parameters.", - firebase::Variant::TypeName(value.type()), pair.first.c_str()); + LogError("SetDefaultEventParameters: Value for key '%s' has an unexpected type '%s' which is not supported. Skipping.", + pair.first.c_str(), firebase::Variant::TypeName(value.type())); } } [FIRAnalytics setDefaultEventParameters:ns_default_parameters]; From 242777dac300d3241076b8758951b1b57e2c2d8f Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 2 Jun 2025 19:23:52 +0000 Subject: [PATCH 05/10] Here's a summary of the changes I've made to add `SetDefaultEventParameters` and `ClearDefaultEventParameters` to Analytics: This change introduces two new C++ Analytics SDK functions: - `SetDefaultEventParameters(const std::map& params)`: This allows you to set default parameters (string, int64, double, bool, null) that will be included in all subsequent `LogEvent` calls. If a `firebase::Variant::Null()` is provided for a key, that specific default parameter will be cleared. Aggregate types (maps, vectors) are not supported and will be skipped with an error logged. - `ClearDefaultEventParameters()`: This clears all currently set default event parameters. Here's how it's handled on different platforms: - iOS: I'm using `[FIRAnalytics setDefaultEventParameters:]`. `firebase::Variant::Null()` maps to `[NSNull null]`. Unsupported types are skipped. - Android: I'm using `FirebaseAnalytics.setDefaultEventParameters(Bundle)`. `firebase::Variant::Null()` results in `Bundle.putString(key, null)`. Unsupported types are skipped. I'm using `AddVariantToBundle` for efficiency with scalar types. - Stub: These are implemented as no-ops. I've reviewed and updated the unit tests and integration tests to cover these functionalities and type constraints. I also applied code formatting using the project's script and updated and shortened the release notes in `release_build_files/readme.md`. --- .../integration_test/src/integration_test.cc | 21 ++-- analytics/src/analytics_android.cc | 99 ++++++++++++------- analytics/src/analytics_ios.mm | 20 ++-- .../firebase/analytics/FirebaseAnalytics.java | 39 ++++---- analytics/tests/analytics_test.cc | 30 ++++-- release_build_files/readme.md | 2 + 6 files changed, 131 insertions(+), 80 deletions(-) diff --git a/analytics/integration_test/src/integration_test.cc b/analytics/integration_test/src/integration_test.cc index d16ff57f30..fd39f6c615 100644 --- a/analytics/integration_test/src/integration_test.cc +++ b/analytics/integration_test/src/integration_test.cc @@ -342,7 +342,9 @@ TEST_F(FirebaseAnalyticsTest, TestSetConsent) { } TEST_F(FirebaseAnalyticsTest, TestDefaultEventParametersUsage) { - LogInfo("Testing SetDefaultEventParameters with initial values, then updating with Null."); + LogInfo( + "Testing SetDefaultEventParameters with initial values, then updating " + "with Null."); std::map initial_defaults; initial_defaults["initial_key"] = "initial_value"; @@ -354,7 +356,8 @@ TEST_F(FirebaseAnalyticsTest, TestDefaultEventParametersUsage) { // Log an event that would pick up these defaults. firebase::analytics::LogEvent("event_with_initial_defaults"); LogInfo("Logged event_with_initial_defaults."); - ProcessEvents(500); // Short pause for event logging, if it matters for backend. + ProcessEvents( + 500); // Short pause for event logging, if it matters for backend. std::map updated_defaults; updated_defaults["key_to_be_nulled"] = firebase::Variant::Null(); @@ -362,7 +365,8 @@ TEST_F(FirebaseAnalyticsTest, TestDefaultEventParametersUsage) { // "initial_key" should persist if not overwritten. // "numeric_default" should persist. - LogInfo("Updating default event parameters, setting key_to_be_nulled to Null."); + LogInfo( + "Updating default event parameters, setting key_to_be_nulled to Null."); firebase::analytics::SetDefaultEventParameters(updated_defaults); // Log an event that would pick up the updated defaults. firebase::analytics::LogEvent("event_after_nulling_and_adding"); @@ -370,8 +374,11 @@ TEST_F(FirebaseAnalyticsTest, TestDefaultEventParametersUsage) { ProcessEvents(500); // For this C++ SDK integration test, we primarily ensure API calls complete. - // Actual parameter presence on logged events would be verified by backend/native tests. - LogInfo("TestDefaultEventParametersUsage completed. Calls were made successfully."); + // Actual parameter presence on logged events would be verified by + // backend/native tests. + LogInfo( + "TestDefaultEventParametersUsage completed. Calls were made " + "successfully."); } TEST_F(FirebaseAnalyticsTest, TestClearDefaultEventParametersFunctionality) { @@ -395,7 +402,9 @@ TEST_F(FirebaseAnalyticsTest, TestClearDefaultEventParametersFunctionality) { LogInfo("Logged event_after_global_clear."); ProcessEvents(500); - LogInfo("TestClearDefaultEventParametersFunctionality completed. Call was made successfully."); + LogInfo( + "TestClearDefaultEventParametersFunctionality completed. Call was made " + "successfully."); } } // namespace firebase_testapp_automated diff --git a/analytics/src/analytics_android.cc b/analytics/src/analytics_android.cc index 0afc3543d6..eba8260256 100644 --- a/analytics/src/analytics_android.cc +++ b/analytics/src/analytics_android.cc @@ -380,7 +380,8 @@ void AddBundleToBundle(JNIEnv* env, jobject bundle, const char* key, } // Declared here so that it can be used, defined below. -jobject MapToBundle(JNIEnv* env, const std::map& map); +jobject MapToBundle(JNIEnv* env, + const std::map& map); // Converts the given vector into a Java ArrayList. It is up to the // caller to delete the local reference when done. @@ -440,7 +441,8 @@ bool AddVariantToBundle(JNIEnv* env, jobject bundle, const char* key, // Converts the given map into a Java Bundle. It is up to the caller // to delete the local reference when done. -jobject MapToBundle(JNIEnv* env, const std::map& map) { +jobject MapToBundle(JNIEnv* env, + const std::map& map) { jobject bundle = env->NewObject(util::bundle::GetClass(), util::bundle::GetMethodId(util::bundle::kConstructor)); @@ -514,7 +516,8 @@ void LogEvent(const char* name, const Parameter* parameters, LogError( "LogEvent(%s): %s is not a valid parameter value type. " "No event was logged.", - parameter.name, firebase::Variant::TypeName(parameter.value.type())); + parameter.name, + firebase::Variant::TypeName(parameter.value.type())); } } }); @@ -631,40 +634,58 @@ void SetDefaultEventParameters( const char* key_cstr = pair.first.c_str(); if (value.is_null()) { - jstring key_jstring = env->NewStringUTF(key_cstr); - if (util::CheckAndClearJniExceptions(env) || !key_jstring) { - LogError("SetDefaultEventParameters: Failed to create jstring for null value key: %s", key_cstr); - if (key_jstring) env->DeleteLocalRef(key_jstring); - continue; - } - env->CallVoidMethod(bundle, util::bundle::GetMethodId(util::bundle::kPutString), - key_jstring, nullptr); - if (util::CheckAndClearJniExceptions(env)) { - LogError("SetDefaultEventParameters: Failed to put null string for key: %s", key_cstr); - } - env->DeleteLocalRef(key_jstring); - } else if (value.is_string() || value.is_int64() || value.is_double() || value.is_bool()) { - // AddVariantToBundle handles these types and their JNI conversions. - // It also logs if an individual AddToBundle within it fails or if a type is unsupported by it. - if (!AddVariantToBundle(env, bundle, key_cstr, value)) { - // This specific log gives context that the failure happened during SetDefaultEventParameters - // for a type that was expected to be supported by AddVariantToBundle. - LogError("SetDefaultEventParameters: Failed to add parameter for key '%s' with supported type '%s'. This might indicate a JNI issue during conversion.", - key_cstr, firebase::Variant::TypeName(value.type())); - } + jstring key_jstring = env->NewStringUTF(key_cstr); + if (util::CheckAndClearJniExceptions(env) || !key_jstring) { + LogError( + "SetDefaultEventParameters: Failed to create jstring for null " + "value key: %s", + key_cstr); + if (key_jstring) env->DeleteLocalRef(key_jstring); + continue; + } + env->CallVoidMethod(bundle, + util::bundle::GetMethodId(util::bundle::kPutString), + key_jstring, nullptr); + if (util::CheckAndClearJniExceptions(env)) { + LogError( + "SetDefaultEventParameters: Failed to put null string for key: %s", + key_cstr); + } + env->DeleteLocalRef(key_jstring); + } else if (value.is_string() || value.is_int64() || value.is_double() || + value.is_bool()) { + // AddVariantToBundle handles these types and their JNI conversions. + // It also logs if an individual AddToBundle within it fails or if a type + // is unsupported by it. + if (!AddVariantToBundle(env, bundle, key_cstr, value)) { + // This specific log gives context that the failure happened during + // SetDefaultEventParameters for a type that was expected to be + // supported by AddVariantToBundle. + LogError( + "SetDefaultEventParameters: Failed to add parameter for key '%s' " + "with supported type '%s'. This might indicate a JNI issue during " + "conversion.", + key_cstr, firebase::Variant::TypeName(value.type())); + } } else if (value.is_vector() || value.is_map()) { - LogError("SetDefaultEventParameters: Value for key '%s' has type '%s' which is not supported for default event parameters. Only string, int64, double, bool, and null are supported. Skipping.", - key_cstr, firebase::Variant::TypeName(value.type())); + LogError( + "SetDefaultEventParameters: Value for key '%s' has type '%s' which " + "is not supported for default event parameters. Only string, int64, " + "double, bool, and null are supported. Skipping.", + key_cstr, firebase::Variant::TypeName(value.type())); } else { - // This case handles other fundamental Variant types that are not scalars and not vector/map. - LogError("SetDefaultEventParameters: Value for key '%s' has an unexpected and unsupported type '%s'. Skipping.", - key_cstr, firebase::Variant::TypeName(value.type())); + // This case handles other fundamental Variant types that are not scalars + // and not vector/map. + LogError( + "SetDefaultEventParameters: Value for key '%s' has an unexpected and " + "unsupported type '%s'. Skipping.", + key_cstr, firebase::Variant::TypeName(value.type())); } } - env->CallVoidMethod(g_analytics_class_instance, - analytics::GetMethodId(analytics::kSetDefaultEventParameters), - bundle); + env->CallVoidMethod( + g_analytics_class_instance, + analytics::GetMethodId(analytics::kSetDefaultEventParameters), bundle); if (util::CheckAndClearJniExceptions(env)) { LogError("Failed to call setDefaultEventParameters on Java instance."); } @@ -678,15 +699,17 @@ void ClearDefaultEventParameters() { if (!env) return; // Calling with nullptr bundle should clear the parameters. - env->CallVoidMethod(g_analytics_class_instance, - analytics::GetMethodId(analytics::kSetDefaultEventParameters), - nullptr); + env->CallVoidMethod( + g_analytics_class_instance, + analytics::GetMethodId(analytics::kSetDefaultEventParameters), nullptr); if (util::CheckAndClearJniExceptions(env)) { // This might happen if the method isn't available on older SDKs, // or if some other JNI error occurs. - LogError("Failed to call setDefaultEventParameters(null) on Java instance. " - "This may indicate the method is not available on this Android SDK version " - "or another JNI error occurred."); + LogError( + "Failed to call setDefaultEventParameters(null) on Java instance. " + "This may indicate the method is not available on this Android SDK " + "version " + "or another JNI error occurred."); } } diff --git a/analytics/src/analytics_ios.mm b/analytics/src/analytics_ios.mm index 1a0e386da5..656ed7b541 100644 --- a/analytics/src/analytics_ios.mm +++ b/analytics/src/analytics_ios.mm @@ -250,7 +250,8 @@ void LogEvent(const char* name) { } // Converts and adds the Variant to the given Dictionary. -bool AddVariantToDictionary(NSMutableDictionary* dict, NSString* key, const firebase::Variant& value) { +bool AddVariantToDictionary(NSMutableDictionary* dict, NSString* key, + const firebase::Variant& value) { if (value.is_int64()) { [dict setObject:[NSNumber numberWithLongLong:value.int64_value()] forKey:key]; } else if (value.is_double()) { @@ -373,8 +374,7 @@ void SetSessionTimeoutDuration(int64_t milliseconds) { setSessionTimeoutInterval:static_cast(milliseconds) / kMillisecondsPerSecond]; } -void SetDefaultEventParameters( - const std::map& default_parameters) { +void SetDefaultEventParameters(const std::map& default_parameters) { FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized()); NSMutableDictionary* ns_default_parameters = [[NSMutableDictionary alloc] initWithCapacity:default_parameters.size()]; @@ -385,7 +385,8 @@ void SetDefaultEventParameters( if (value.is_null()) { [ns_default_parameters setObject:[NSNull null] forKey:key]; } else if (value.is_int64()) { - [ns_default_parameters setObject:[NSNumber numberWithLongLong:value.int64_value()] forKey:key]; + [ns_default_parameters setObject:[NSNumber numberWithLongLong:value.int64_value()] + forKey:key]; } else if (value.is_double()) { [ns_default_parameters setObject:[NSNumber numberWithDouble:value.double_value()] forKey:key]; } else if (value.is_string()) { @@ -393,11 +394,14 @@ void SetDefaultEventParameters( } else if (value.is_bool()) { [ns_default_parameters setObject:[NSNumber numberWithBool:value.bool_value()] forKey:key]; } else if (value.is_vector() || value.is_map()) { - LogError("SetDefaultEventParameters: Value for key '%s' has type '%s' which is not supported for default event parameters. Only string, int64, double, bool, and null are supported. Skipping.", - pair.first.c_str(), firebase::Variant::TypeName(value.type())); + LogError("SetDefaultEventParameters: Value for key '%s' has type '%s' which is not supported " + "for default event parameters. Only string, int64, double, bool, and null are " + "supported. Skipping.", + pair.first.c_str(), firebase::Variant::TypeName(value.type())); } else { - LogError("SetDefaultEventParameters: Value for key '%s' has an unexpected type '%s' which is not supported. Skipping.", - pair.first.c_str(), firebase::Variant::TypeName(value.type())); + LogError("SetDefaultEventParameters: Value for key '%s' has an unexpected type '%s' which is " + "not supported. Skipping.", + pair.first.c_str(), firebase::Variant::TypeName(value.type())); } } [FIRAnalytics setDefaultEventParameters:ns_default_parameters]; diff --git a/analytics/src_java/fake/com/google/firebase/analytics/FirebaseAnalytics.java b/analytics/src_java/fake/com/google/firebase/analytics/FirebaseAnalytics.java index 208a85612a..cf70459b6a 100644 --- a/analytics/src_java/fake/com/google/firebase/analytics/FirebaseAnalytics.java +++ b/analytics/src_java/fake/com/google/firebase/analytics/FirebaseAnalytics.java @@ -82,27 +82,28 @@ public void setSessionTimeoutDuration(long milliseconds) { public void setDefaultEventParameters(Bundle bundle) { if (bundle == null) { - FakeReporter.addReport("FirebaseAnalytics.setDefaultEventParameters", "null"); + FakeReporter.addReport("FirebaseAnalytics.setDefaultEventParameters", "null"); } else { - StringBuilder paramsString = new StringBuilder(); - // Sort keys for predictable ordering. - for (String key : new TreeSet<>(bundle.keySet())) { - paramsString.append(key); - paramsString.append("="); - Object value = bundle.get(key); // Get as Object first - if (value == null) { - // This case handles when bundle.putString(key, null) was called. - paramsString.append(""); - } else { - paramsString.append(value.toString()); - } - paramsString.append(","); + StringBuilder paramsString = new StringBuilder(); + // Sort keys for predictable ordering. + for (String key : new TreeSet<>(bundle.keySet())) { + paramsString.append(key); + paramsString.append("="); + Object value = bundle.get(key); // Get as Object first + if (value == null) { + // This case handles when bundle.putString(key, null) was called. + paramsString.append(""); + } else { + paramsString.append(value.toString()); } - // Remove trailing comma if paramsString is not empty - if (paramsString.length() > 0) { - paramsString.setLength(paramsString.length() - 1); - } - FakeReporter.addReport("FirebaseAnalytics.setDefaultEventParameters", paramsString.toString()); + paramsString.append(","); + } + // Remove trailing comma if paramsString is not empty + if (paramsString.length() > 0) { + paramsString.setLength(paramsString.length() - 1); + } + FakeReporter.addReport( + "FirebaseAnalytics.setDefaultEventParameters", paramsString.toString()); } } } diff --git a/analytics/tests/analytics_test.cc b/analytics/tests/analytics_test.cc index e6180fbd23..32852667f0 100644 --- a/analytics/tests/analytics_test.cc +++ b/analytics/tests/analytics_test.cc @@ -299,21 +299,27 @@ TEST_F(AnalyticsTest, TestGetAnalyticsInstanceId) { TEST_F(AnalyticsTest, TestSetDefaultEventParameters) { // Android part is not tested here as it's about iOS fakes. // This test focuses on the iOS fake reporting. - // Android: AddExpectationAndroid("FirebaseAnalytics.setDefaultEventParameters", {"my_param_bool=true,my_param_double=3.14,my_param_int=123,my_param_string=hello"}); + // Android: + // AddExpectationAndroid("FirebaseAnalytics.setDefaultEventParameters", + // {"my_param_bool=true,my_param_double=3.14,my_param_int=123,my_param_string=hello"}); AddExpectationApple("+[FIRAnalytics setDefaultEventParameters:]", - {"my_param_bool=1,my_param_double=3.14,my_param_int=123,my_param_string=hello"}); + {"my_param_bool=1,my_param_double=3.14,my_param_int=123," + "my_param_string=hello"}); std::map default_params; default_params["my_param_string"] = "hello"; default_params["my_param_double"] = 3.14; default_params["my_param_int"] = 123; - default_params["my_param_bool"] = true; // Note: [NSNumber numberWithBool:YES] stringifies to 1 + default_params["my_param_bool"] = + true; // Note: [NSNumber numberWithBool:YES] stringifies to 1 SetDefaultEventParameters(default_params); } TEST_F(AnalyticsTest, TestSetDefaultEventParametersWithNull) { - // Android: AddExpectationAndroid("FirebaseAnalytics.setDefaultEventParameters", {"key_to_clear=null,other_key=value"}); + // Android: + // AddExpectationAndroid("FirebaseAnalytics.setDefaultEventParameters", + // {"key_to_clear=null,other_key=value"}); AddExpectationApple("+[FIRAnalytics setDefaultEventParameters:]", {"key_to_clear=,other_key=value"}); @@ -325,7 +331,9 @@ TEST_F(AnalyticsTest, TestSetDefaultEventParametersWithNull) { } TEST_F(AnalyticsTest, TestSetDefaultEventParametersEmpty) { - // Android: AddExpectationAndroid("FirebaseAnalytics.setDefaultEventParameters", {""}); // Or however an empty bundle is represented + // Android: + // AddExpectationAndroid("FirebaseAnalytics.setDefaultEventParameters", {""}); + // // Or however an empty bundle is represented AddExpectationApple("+[FIRAnalytics setDefaultEventParameters:]", {""}); std::map default_params; @@ -333,15 +341,18 @@ TEST_F(AnalyticsTest, TestSetDefaultEventParametersEmpty) { } TEST_F(AnalyticsTest, TestClearDefaultEventParameters) { - // Android: AddExpectationAndroid("FirebaseAnalytics.setDefaultEventParameters", {"null"}); // Passing null bundle + // Android: + // AddExpectationAndroid("FirebaseAnalytics.setDefaultEventParameters", + // {"null"}); // Passing null bundle AddExpectationApple("+[FIRAnalytics setDefaultEventParameters:]", {"nil"}); ClearDefaultEventParameters(); } TEST_F(AnalyticsTest, TestSetDefaultEventParametersAndroid) { - AddExpectationAndroid("FirebaseAnalytics.setDefaultEventParameters", - {"my_bool=true,my_double=3.14,my_int=123,my_string=hello"}); + AddExpectationAndroid( + "FirebaseAnalytics.setDefaultEventParameters", + {"my_bool=true,my_double=3.14,my_int=123,my_string=hello"}); std::map default_params; default_params["my_string"] = "hello"; @@ -374,7 +385,8 @@ TEST_F(AnalyticsTest, TestSetDefaultEventParametersEmptyAndroid) { } TEST_F(AnalyticsTest, TestClearDefaultEventParametersAndroid) { - AddExpectationAndroid("FirebaseAnalytics.setDefaultEventParameters", {"null"}); + AddExpectationAndroid("FirebaseAnalytics.setDefaultEventParameters", + {"null"}); ClearDefaultEventParameters(); WaitForMainThreadTask(); diff --git a/release_build_files/readme.md b/release_build_files/readme.md index 1d52fae82f..6db531c4ca 100644 --- a/release_build_files/readme.md +++ b/release_build_files/readme.md @@ -666,6 +666,8 @@ code. be removed soon. - Messaging (Android): Fix issue with the Subscribe Future not completing when a cached token is available. + - Analytics (C++): Added `SetDefaultEventParameters` and + `ClearDefaultEventParameters` to the Analytics SDK. ### 12.7.0 - Changes From f743e52975299aa186a767c3dbf08043dc36c358 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 2 Jun 2025 22:06:00 +0000 Subject: [PATCH 06/10] Here's a summary of the changes I've made to add `SetDefaultEventParameters` and `ClearDefaultEventParameters` to Analytics: This change introduces two new C++ Analytics SDK functions: - `SetDefaultEventParameters(const std::map& params)`: This allows you to set default parameters (string, int64, double, bool, null) that will be included in all subsequent `LogEvent` calls. If a `firebase::Variant::Null()` is provided for a key, that specific default parameter will be cleared. Aggregate types (maps, vectors) are not supported and will be skipped with an error logged. - `ClearDefaultEventParameters()`: This clears all currently set default event parameters. Here's how it's handled on different platforms: - iOS: I'm using `[FIRAnalytics setDefaultEventParameters:]`. `firebase::Variant::Null()` maps to `[NSNull null]`. Unsupported types are skipped. - Android: I'm using `FirebaseAnalytics.setDefaultEventParameters(Bundle)`. `firebase::Variant::Null()` results in `Bundle.putString(key, null)`. Unsupported types are skipped. I'm using `AddVariantToBundle` for efficiency with scalar types. - Stub: These are implemented as no-ops. I've reviewed and updated the unit tests and integration tests to cover these functionalities and type constraints. I also applied code formatting using the project's script and updated and shortened the release notes in `release_build_files/readme.md`. --- release_build_files/readme.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/release_build_files/readme.md b/release_build_files/readme.md index 6db531c4ca..41998c1e76 100644 --- a/release_build_files/readme.md +++ b/release_build_files/readme.md @@ -666,8 +666,8 @@ code. be removed soon. - Messaging (Android): Fix issue with the Subscribe Future not completing when a cached token is available. - - Analytics (C++): Added `SetDefaultEventParameters` and - `ClearDefaultEventParameters` to the Analytics SDK. + - Analytics: Added `SetDefaultEventParameters` and + `ClearDefaultEventParameters`. ### 12.7.0 - Changes From 8ddab84c5f077e10990add948a97554da715f307 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Wed, 4 Jun 2025 18:12:20 +0000 Subject: [PATCH 07/10] Here's what I've done to add `SetDefaultEventParameters` and `ClearDefaultEventParameters` to Analytics. This change introduces two new C++ Analytics SDK functions: - `SetDefaultEventParameters(const std::map& params)`: This allows you to set default parameters (string, int64, double, bool, null) that will be included in all subsequent `LogEvent` calls. If a `firebase::Variant::Null()` is provided for a key, that specific default parameter will be cleared. Aggregate types (maps, vectors) are not supported and will be skipped with an error logged. - `ClearDefaultEventParameters()`: This clears all currently set default event parameters. Platform implementations: - iOS: Uses `[FIRAnalytics setDefaultEventParameters:]`. `firebase::Variant::Null()` maps to `[NSNull null]`. Unsupported types are skipped. - Android: Uses `FirebaseAnalytics.setDefaultEventParameters(Bundle)`. `firebase::Variant::Null()` results in `Bundle.putString(key, null)`. Unsupported types are skipped. `AddVariantToBundle` is used for efficiency with scalar types. - Stub: Implemented as no-ops. I've reviewed and updated the unit tests and integration tests to cover these functionalities and type constraints. Namespace usage for `Variant` has been refined: - Public API (`analytics.h`) and integration tests use `firebase::Variant`. - Internal SDK implementation code uses unqualified `Variant` where appropriate, relying on the enclosing `firebase` namespace. I've also applied code formatting using the project's script and updated and shortened the release notes in `release_build_files/readme.md`. --- analytics/src/analytics_android.cc | 24 +++++++++++------------- analytics/src/analytics_ios.mm | 23 +++++++++++------------ analytics/src/analytics_stub.cc | 2 +- 3 files changed, 23 insertions(+), 26 deletions(-) diff --git a/analytics/src/analytics_android.cc b/analytics/src/analytics_android.cc index eba8260256..a2825a2b76 100644 --- a/analytics/src/analytics_android.cc +++ b/analytics/src/analytics_android.cc @@ -380,18 +380,17 @@ void AddBundleToBundle(JNIEnv* env, jobject bundle, const char* key, } // Declared here so that it can be used, defined below. -jobject MapToBundle(JNIEnv* env, - const std::map& map); +jobject MapToBundle(JNIEnv* env, const std::map& map); // Converts the given vector into a Java ArrayList. It is up to the // caller to delete the local reference when done. jobject VectorOfMapsToArrayList(JNIEnv* env, - const std::vector& vector) { + const std::vector& vector) { jobject arraylist = env->NewObject( util::array_list::GetClass(), util::array_list::GetMethodId(util::array_list::kConstructor)); - for (const firebase::Variant& element : vector) { + for (const Variant& element : vector) { if (element.is_map()) { jobject bundle = MapToBundle(env, element.map()); env->CallBooleanMethod( @@ -401,7 +400,7 @@ jobject VectorOfMapsToArrayList(JNIEnv* env, env->DeleteLocalRef(bundle); } else { LogError("VectorOfMapsToArrayList: Unsupported type (%s) within vector.", - firebase::Variant::TypeName(element.type())); + Variant::TypeName(element.type())); } } return arraylist; @@ -409,7 +408,7 @@ jobject VectorOfMapsToArrayList(JNIEnv* env, // Converts and adds the Variant to the given Bundle. bool AddVariantToBundle(JNIEnv* env, jobject bundle, const char* key, - const firebase::Variant& value) { + const Variant& value) { if (value.is_int64()) { AddToBundle(env, bundle, key, value.int64_value()); } else if (value.is_double()) { @@ -441,8 +440,7 @@ bool AddVariantToBundle(JNIEnv* env, jobject bundle, const char* key, // Converts the given map into a Java Bundle. It is up to the caller // to delete the local reference when done. -jobject MapToBundle(JNIEnv* env, - const std::map& map) { +jobject MapToBundle(JNIEnv* env, const std::map& map) { jobject bundle = env->NewObject(util::bundle::GetClass(), util::bundle::GetMethodId(util::bundle::kConstructor)); @@ -454,7 +452,7 @@ jobject MapToBundle(JNIEnv* env, if (!AddVariantToBundle(env, bundle, pair.first.string_value(), pair.second)) { LogError("MapToBundle: Unsupported type (%s) within map with key %s.", - firebase::Variant::TypeName(pair.second.type()), + Variant::TypeName(pair.second.type()), pair.first.string_value()); } util::CheckAndClearJniExceptions(env); @@ -630,7 +628,7 @@ void SetDefaultEventParameters( } for (const auto& pair : default_parameters) { - const firebase::Variant& value = pair.second; + const Variant& value = pair.second; const char* key_cstr = pair.first.c_str(); if (value.is_null()) { @@ -665,21 +663,21 @@ void SetDefaultEventParameters( "SetDefaultEventParameters: Failed to add parameter for key '%s' " "with supported type '%s'. This might indicate a JNI issue during " "conversion.", - key_cstr, firebase::Variant::TypeName(value.type())); + key_cstr, Variant::TypeName(value.type())); } } else if (value.is_vector() || value.is_map()) { LogError( "SetDefaultEventParameters: Value for key '%s' has type '%s' which " "is not supported for default event parameters. Only string, int64, " "double, bool, and null are supported. Skipping.", - key_cstr, firebase::Variant::TypeName(value.type())); + key_cstr, Variant::TypeName(value.type())); } else { // This case handles other fundamental Variant types that are not scalars // and not vector/map. LogError( "SetDefaultEventParameters: Value for key '%s' has an unexpected and " "unsupported type '%s'. Skipping.", - key_cstr, firebase::Variant::TypeName(value.type())); + key_cstr, Variant::TypeName(value.type())); } } diff --git a/analytics/src/analytics_ios.mm b/analytics/src/analytics_ios.mm index 656ed7b541..debb6de282 100644 --- a/analytics/src/analytics_ios.mm +++ b/analytics/src/analytics_ios.mm @@ -232,26 +232,25 @@ void LogEvent(const char* name) { } // Declared here so that it can be used, defined below. -NSDictionary* MapToDictionary(const std::map& map); +NSDictionary* MapToDictionary(const std::map& map); // Converts the given vector of Maps into an ObjC NSArray of ObjC NSDictionaries. -NSArray* VectorOfMapsToArray(const std::vector& vector) { +NSArray* VectorOfMapsToArray(const std::vector& vector) { NSMutableArray* array = [NSMutableArray arrayWithCapacity:vector.size()]; - for (const firebase::Variant& element : vector) { + for (const Variant& element : vector) { if (element.is_map()) { NSDictionary* dict = MapToDictionary(element.map()); [array addObject:dict]; } else { LogError("VectorOfMapsToArray: Unsupported type (%s) within vector.", - firebase::Variant::TypeName(element.type())); + Variant::TypeName(element.type())); } } return array; } // Converts and adds the Variant to the given Dictionary. -bool AddVariantToDictionary(NSMutableDictionary* dict, NSString* key, - const firebase::Variant& value) { +bool AddVariantToDictionary(NSMutableDictionary* dict, NSString* key, const Variant& value) { if (value.is_int64()) { [dict setObject:[NSNumber numberWithLongLong:value.int64_value()] forKey:key]; } else if (value.is_double()) { @@ -278,7 +277,7 @@ bool AddVariantToDictionary(NSMutableDictionary* dict, NSString* key, } // Converts the given map into an ObjC dictionary of ObjC objects. -NSDictionary* MapToDictionary(const std::map& map) { +NSDictionary* MapToDictionary(const std::map& map) { NSMutableDictionary* dict = [NSMutableDictionary dictionaryWithCapacity:map.size()]; for (const auto& pair : map) { // Only add elements that use a string key @@ -286,10 +285,10 @@ bool AddVariantToDictionary(NSMutableDictionary* dict, NSString* key, continue; } NSString* key = SafeString(pair.first.string_value()); - const firebase::Variant& value = pair.second; + const Variant& value = pair.second; if (!AddVariantToDictionary(dict, key, value)) { LogError("MapToDictionary: Unsupported type (%s) within map with key %s.", - firebase::Variant::TypeName(value.type()), key); + Variant::TypeName(value.type()), key); } } return dict; @@ -380,7 +379,7 @@ void SetDefaultEventParameters(const std::map& d [[NSMutableDictionary alloc] initWithCapacity:default_parameters.size()]; for (const auto& pair : default_parameters) { NSString* key = SafeString(pair.first.c_str()); - const firebase::Variant& value = pair.second; + const Variant& value = pair.second; if (value.is_null()) { [ns_default_parameters setObject:[NSNull null] forKey:key]; @@ -397,11 +396,11 @@ void SetDefaultEventParameters(const std::map& d LogError("SetDefaultEventParameters: Value for key '%s' has type '%s' which is not supported " "for default event parameters. Only string, int64, double, bool, and null are " "supported. Skipping.", - pair.first.c_str(), firebase::Variant::TypeName(value.type())); + pair.first.c_str(), Variant::TypeName(value.type())); } else { LogError("SetDefaultEventParameters: Value for key '%s' has an unexpected type '%s' which is " "not supported. Skipping.", - pair.first.c_str(), firebase::Variant::TypeName(value.type())); + pair.first.c_str(), Variant::TypeName(value.type())); } } [FIRAnalytics setDefaultEventParameters:ns_default_parameters]; diff --git a/analytics/src/analytics_stub.cc b/analytics/src/analytics_stub.cc index 4ae7410bbc..4e86fb6258 100644 --- a/analytics/src/analytics_stub.cc +++ b/analytics/src/analytics_stub.cc @@ -143,7 +143,7 @@ void SetSessionTimeoutDuration(int64_t /*milliseconds*/) { } void SetDefaultEventParameters( - const std::map& /*default_parameters*/) { + const std::map& /*default_parameters*/) { FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized()); // This is a stub implementation. No operation needed. } From 573b647d6217ee0706642c27db6f7fed3dc7b44b Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sat, 7 Jun 2025 04:42:45 +0000 Subject: [PATCH 08/10] I've added `SetDefaultEventParameters` and `ClearDefaultEventParameters` to Analytics for you. This change introduces two new C++ Analytics SDK functions: - `SetDefaultEventParameters(const std::map& params)`: This allows you to set default parameters (string, int64, double, bool, null) that will be included in all subsequent `LogEvent` calls. If a `Variant::Null()` is provided for a key, that specific default parameter will be cleared. Aggregate types (maps, vectors) are not supported and will be skipped with an error logged. - `ClearDefaultEventParameters()`: This clears all currently set default event parameters. Here's how it's implemented on different platforms: - iOS: Uses `[FIRAnalytics setDefaultEventParameters:]`. `Variant::Null()` maps to `[NSNull null]`. Unsupported types are skipped. Passing an empty dictionary (if all input params are invalid) is a no-op. - Android: Uses `FirebaseAnalytics.setDefaultEventParameters(Bundle)`. `Variant::Null()` results in `Bundle.putString(key, null)`. Unsupported types are skipped. `AddVariantToBundle` is used for efficiency with scalar types. - Stub: Implemented as no-ops. I've also reviewed and updated the unit tests and integration tests to cover these functionalities and type constraints. Namespace usage for `Variant` has been refined: - Public API (`analytics.h`) uses unqualified `Variant` as it's within the `firebase` namespace. - SDK implementation file signatures match `analytics.h` (unqualified `Variant`). - Internal SDK code uses unqualified `Variant`. - Integration tests use `firebase::Variant` as they are outside the `firebase` namespace. I've applied code formatting using the project's script and updated and shortened the release notes in `release_build_files/readme.md`. --- analytics/src/analytics_ios.mm | 2 +- analytics/src/analytics_stub.cc | 2 +- analytics/src/include/firebase/analytics.h | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/analytics/src/analytics_ios.mm b/analytics/src/analytics_ios.mm index debb6de282..9d1982b17e 100644 --- a/analytics/src/analytics_ios.mm +++ b/analytics/src/analytics_ios.mm @@ -373,7 +373,7 @@ void SetSessionTimeoutDuration(int64_t milliseconds) { setSessionTimeoutInterval:static_cast(milliseconds) / kMillisecondsPerSecond]; } -void SetDefaultEventParameters(const std::map& default_parameters) { +void SetDefaultEventParameters(const std::map& default_parameters) { FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized()); NSMutableDictionary* ns_default_parameters = [[NSMutableDictionary alloc] initWithCapacity:default_parameters.size()]; diff --git a/analytics/src/analytics_stub.cc b/analytics/src/analytics_stub.cc index 4e86fb6258..4ae7410bbc 100644 --- a/analytics/src/analytics_stub.cc +++ b/analytics/src/analytics_stub.cc @@ -143,7 +143,7 @@ void SetSessionTimeoutDuration(int64_t /*milliseconds*/) { } void SetDefaultEventParameters( - const std::map& /*default_parameters*/) { + const std::map& /*default_parameters*/) { FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized()); // This is a stub implementation. No operation needed. } diff --git a/analytics/src/include/firebase/analytics.h b/analytics/src/include/firebase/analytics.h index eeafcbade8..f9bdc64eb0 100644 --- a/analytics/src/include/firebase/analytics.h +++ b/analytics/src/include/firebase/analytics.h @@ -142,7 +142,7 @@ struct Parameter { /// @param parameter_name Name of the parameter (see Parameter::name). /// @param parameter_value Value for the parameter. Variants can /// hold numbers and strings. - Parameter(const char* parameter_name, firebase::Variant parameter_value) + Parameter(const char* parameter_name, Variant parameter_value) : name(parameter_name) { value = parameter_value; } @@ -245,7 +245,7 @@ struct Parameter { /// /// See firebase::Variant for usage information. /// @note String values can be up to 100 characters long. - firebase::Variant value; + Variant value; #endif // SWIG }; @@ -570,7 +570,7 @@ void ResetAnalyticsData(); /// /// @param[in] default_parameters A map of parameter names to Variant values. void SetDefaultEventParameters( - const std::map& default_parameters); + const std::map& default_parameters); /// @brief Clears all default event parameters. void ClearDefaultEventParameters(); From b2b225dea021c390717b731192a3e3a0cccff53a Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sat, 7 Jun 2025 06:20:33 +0000 Subject: [PATCH 09/10] Add SetDefaultEventParameters and ClearDefaultEventParameters to Analytics This change introduces two new C++ Analytics SDK functions: - `SetDefaultEventParameters(const std::map& params)`: Allows setting default parameters (string, int64, double, bool, null) that will be included in all subsequent `LogEvent` calls. If a `Variant::Null()` is provided for a key, that specific default parameter will be cleared. Aggregate types (maps, vectors) are not supported and will be skipped with an error logged. Passing only invalid parameters resulting in an empty parameter set is a no-op on iOS/Android. - `ClearDefaultEventParameters()`: Clears all currently set default event parameters. Platform implementations: - iOS: Uses `[FIRAnalytics setDefaultEventParameters:]`. `Variant::Null()` maps to `[NSNull null]`. Unsupported types are skipped. - Android: Uses `FirebaseAnalytics.setDefaultEventParameters(Bundle)`. `Variant::Null()` results in `Bundle.putString(key, null)`. Unsupported types are skipped. `AddVariantToBundle` is used for efficiency with scalar types. - Stub: Implemented as no-ops. Unit tests and integration tests have been reviewed and updated. Integration test names have been clarified. Namespace usage for `Variant` has been refined: - Public API (`analytics.h`) uses unqualified `Variant` as it's within the `firebase` namespace. - SDK implementation file signatures match `analytics.h` (unqualified `Variant`). - Internal SDK code uses unqualified `Variant`. - Integration tests use `firebase::Variant` as they are outside the `firebase` namespace. Code formatting applied using the project's script. Release notes in `release_build_files/readme.md` updated and shortened. --- analytics/integration_test/src/integration_test.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/analytics/integration_test/src/integration_test.cc b/analytics/integration_test/src/integration_test.cc index fd39f6c615..b2ad112333 100644 --- a/analytics/integration_test/src/integration_test.cc +++ b/analytics/integration_test/src/integration_test.cc @@ -341,7 +341,7 @@ TEST_F(FirebaseAnalyticsTest, TestSetConsent) { did_test_setconsent_ = true; } -TEST_F(FirebaseAnalyticsTest, TestDefaultEventParametersUsage) { +TEST_F(FirebaseAnalyticsTest, TestSetDefaultEventParameters) { LogInfo( "Testing SetDefaultEventParameters with initial values, then updating " "with Null."); @@ -377,11 +377,11 @@ TEST_F(FirebaseAnalyticsTest, TestDefaultEventParametersUsage) { // Actual parameter presence on logged events would be verified by // backend/native tests. LogInfo( - "TestDefaultEventParametersUsage completed. Calls were made " + "TestSetDefaultEventParameters completed. Calls were made " "successfully."); } -TEST_F(FirebaseAnalyticsTest, TestClearDefaultEventParametersFunctionality) { +TEST_F(FirebaseAnalyticsTest, TestClearDefaultEventParameters) { LogInfo("Testing ClearDefaultEventParameters."); std::map defaults_to_clear; @@ -403,7 +403,7 @@ TEST_F(FirebaseAnalyticsTest, TestClearDefaultEventParametersFunctionality) { ProcessEvents(500); LogInfo( - "TestClearDefaultEventParametersFunctionality completed. Call was made " + "TestClearDefaultEventParameters completed. Call was made " "successfully."); } From 9681c183e61179bb6ad5adb1c53ea554795bc589 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Thu, 26 Jun 2025 15:55:59 -0700 Subject: [PATCH 10/10] =?UTF-8?q?Fix:=20Prevent=20SetDefaultEventParameter?= =?UTF-8?q?s=20from=20clearing=20params=20with=20all-=E2=80=A6=20(#1747)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix: Prevent SetDefaultEventParameters from clearing params with all-invalid input Addresses code review feedback regarding the behavior of SetDefaultEventParameters when all provided parameters are of invalid types. Previously, if all parameters in the C++ map were invalid, an empty NSDictionary/Bundle would be passed to the native iOS/Android setDefaultEventParameters method. According to documentation, passing nil/null (for iOS/Android respectively) clears all default parameters. While passing an empty dictionary/bundle is a valid operation, it could lead to unintentionally clearing parameters if that was the state before, or it might be a no-op if parameters were already set. This change modifies the iOS and Android implementations to explicitly check if the processed native parameter collection (NSDictionary for iOS, Bundle for Android) is empty after filtering for valid types. If it is empty, the call to the native SDK's setDefaultEventParameters method is skipped. This ensures that providing a C++ map containing exclusively invalid parameters (or an empty map) to SetDefaultEventParameters will be a true no-op from the C++ SDK's perspective, preventing any accidental modification or clearing of existing default parameters on the native side. * Fix: Prevent SetDefaultEventParameters from clearing params with all-invalid input Addresses code review feedback regarding the behavior of SetDefaultEventParameters when all provided parameters are of invalid types. Previously, if all parameters in the C++ map were invalid, an empty NSDictionary/Bundle would be passed to the native iOS/Android setDefaultEventParameters method. According to documentation, passing nil/null (for iOS/Android respectively) clears all default parameters. While passing an empty dictionary/bundle is a valid operation, it could lead to unintentionally clearing parameters if that was the state before, or it might be a no-op if parameters were already set. This change modifies the iOS and Android implementations to explicitly check if the processed native parameter collection (NSDictionary for iOS, Bundle for Android) is effectively empty after filtering for valid types. If it is, the call to the native SDK's setDefaultEventParameters method is skipped. For iOS, this is done by checking `[ns_default_parameters count] == 0`. For Android, this is done by tracking if any parameter was successfully added to the Bundle using a boolean flag, as a direct `bundle.isEmpty()` check before the native call could miss JNI exceptions during bundle population. This ensures that providing a C++ map containing exclusively invalid parameters (or an empty map) to SetDefaultEventParameters will be a true no-op from the C++ SDK's perspective, preventing any accidental modification or clearing of existing default parameters on the native side. --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> --- analytics/src/analytics_android.cc | 15 ++++++++++++++- analytics/src/analytics_ios.mm | 9 +++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/analytics/src/analytics_android.cc b/analytics/src/analytics_android.cc index a2825a2b76..8d3289e4db 100644 --- a/analytics/src/analytics_android.cc +++ b/analytics/src/analytics_android.cc @@ -627,6 +627,7 @@ void SetDefaultEventParameters( return; } + bool any_parameter_added = false; for (const auto& pair : default_parameters) { const Variant& value = pair.second; const char* key_cstr = pair.first.c_str(); @@ -648,6 +649,8 @@ void SetDefaultEventParameters( LogError( "SetDefaultEventParameters: Failed to put null string for key: %s", key_cstr); + } else { + any_parameter_added = true; } env->DeleteLocalRef(key_jstring); } else if (value.is_string() || value.is_int64() || value.is_double() || @@ -655,7 +658,9 @@ void SetDefaultEventParameters( // AddVariantToBundle handles these types and their JNI conversions. // It also logs if an individual AddToBundle within it fails or if a type // is unsupported by it. - if (!AddVariantToBundle(env, bundle, key_cstr, value)) { + if (AddVariantToBundle(env, bundle, key_cstr, value)) { + any_parameter_added = true; + } else { // This specific log gives context that the failure happened during // SetDefaultEventParameters for a type that was expected to be // supported by AddVariantToBundle. @@ -681,6 +686,14 @@ void SetDefaultEventParameters( } } + if (!any_parameter_added) { + LogDebug( + "SetDefaultEventParameters: No valid parameters were processed, " + "skipping native call to avoid clearing existing parameters."); + env->DeleteLocalRef(bundle); + return; + } + env->CallVoidMethod( g_analytics_class_instance, analytics::GetMethodId(analytics::kSetDefaultEventParameters), bundle); diff --git a/analytics/src/analytics_ios.mm b/analytics/src/analytics_ios.mm index 9d1982b17e..df413b2359 100644 --- a/analytics/src/analytics_ios.mm +++ b/analytics/src/analytics_ios.mm @@ -403,6 +403,15 @@ void SetDefaultEventParameters(const std::map& default_par pair.first.c_str(), Variant::TypeName(value.type())); } } + // If ns_default_parameters is empty at this point, it means all input + // parameters were invalid or the input map itself was empty. + // In this case, we should not call the native SDK, as passing an empty + // NSDictionary might clear existing parameters, which is not the intent + // if all inputs were simply invalid. + if ([ns_default_parameters count] == 0) { + LogDebug("SetDefaultEventParameters: No valid parameters to set, skipping native call."); + return; + } [FIRAnalytics setDefaultEventParameters:ns_default_parameters]; }