From 36a1f7418ee8f2916a5aa0fec35bc5982c61533d Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Tue, 1 Apr 2025 19:44:28 -0700 Subject: [PATCH 01/11] Try out get-cmake action. --- .github/workflows/checks.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index c1ee5375f0..e211ecba6f 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -51,6 +51,9 @@ jobs: # This check succeeds if Doxygen documentation generates without errors. runs-on: ubuntu-22.04 steps: + - uses: lukka/get-cmake@latest + with: + cmakeVersion: "~3.31.0" # <--= optional, use most recent 3.31.x version - uses: actions/checkout@v3 with: submodules: false From 27a1c793bf0305208766ce805fcb45a545e3fcf7 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Tue, 1 Apr 2025 19:55:37 -0700 Subject: [PATCH 02/11] Fix indent. --- .github/workflows/checks.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index e211ecba6f..7500f989be 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -53,7 +53,7 @@ jobs: steps: - uses: lukka/get-cmake@latest with: - cmakeVersion: "~3.31.0" # <--= optional, use most recent 3.31.x version + cmakeVersion: "~3.31.0" - uses: actions/checkout@v3 with: submodules: false From bdd0886ca6e2df93cb2dd834249f39a0a2b5fa54 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Tue, 1 Apr 2025 20:00:57 -0700 Subject: [PATCH 03/11] Install cmake 3.31 for other jobs. --- .github/workflows/android.yml | 3 +++ .github/workflows/cpp-packaging.yml | 12 ++++++++++++ .github/workflows/desktop.yml | 6 ++++++ .github/workflows/integration_tests.yml | 12 ++++++++++++ .github/workflows/ios.yml | 3 +++ 5 files changed, 36 insertions(+) diff --git a/.github/workflows/android.yml b/.github/workflows/android.yml index f1834b3d1c..cdd35932d9 100644 --- a/.github/workflows/android.yml +++ b/.github/workflows/android.yml @@ -51,6 +51,9 @@ jobs: architecture: ${{ fromJson(needs.prepare_matrix.outputs.matrix_architecture) }} python_version: ${{ fromJson(needs.prepare_matrix.outputs.matrix_python_version) }} steps: + - uses: lukka/get-cmake@latest + with: + cmakeVersion: "~3.31.0" - name: setup Xcode version (macos) if: runner.os == 'macOS' run: sudo xcode-select -s /Applications/Xcode_${{ env.xcodeVersion }}.app/Contents/Developer diff --git a/.github/workflows/cpp-packaging.yml b/.github/workflows/cpp-packaging.yml index 706a30515a..525062c493 100644 --- a/.github/workflows/cpp-packaging.yml +++ b/.github/workflows/cpp-packaging.yml @@ -90,6 +90,9 @@ jobs: # Binutils 2.35.1 released Sep 19, 2020 binutils_version: "2.35.1" steps: + - uses: lukka/get-cmake@latest + with: + cmakeVersion: "~3.31.0" - name: setup Xcode version (macos) if: runner.os == 'macOS' run: sudo xcode-select -s /Applications/Xcode_${{ env.xcodeVersion }}.app/Contents/Developer @@ -188,6 +191,9 @@ jobs: runs-on: macos-13 if: ${{ github.event.inputs.downloadPublicVersion == '' && github.event.inputs.downloadPreviousRun == '' }} steps: + - uses: lukka/get-cmake@latest + with: + cmakeVersion: "~3.31.0" - name: Store git credentials for all git commands # Forces all git commands to use authenticated https, to prevent throttling. shell: bash @@ -248,6 +254,9 @@ jobs: strategy: fail-fast: false steps: + - uses: lukka/get-cmake@latest + with: + cmakeVersion: "~3.31.0" - name: Force Java 11 shell: bash run: echo "JAVA_HOME=${JAVA_HOME_11_X64}" >> $GITHUB_ENV @@ -352,6 +361,9 @@ jobs: architecture: "arm64" steps: + - uses: lukka/get-cmake@latest + with: + cmakeVersion: "~3.31.0" - name: Store git credentials for all git commands # Forces all git commands to use authenticated https, to prevent throttling. shell: bash diff --git a/.github/workflows/desktop.yml b/.github/workflows/desktop.yml index fa1b792cad..36754a1361 100644 --- a/.github/workflows/desktop.yml +++ b/.github/workflows/desktop.yml @@ -95,6 +95,9 @@ jobs: - xcode_version: "11.7" architecture: "arm64" steps: + - uses: lukka/get-cmake@latest + with: + cmakeVersion: "~3.31.0" - name: Store git credentials for all git commands # Forces all git commands to use authenticated https, to prevent throttling. shell: bash @@ -296,6 +299,9 @@ jobs: strategy: fail-fast: false steps: + - uses: lukka/get-cmake@latest + with: + cmakeVersion: "~3.31.0" - uses: actions/checkout@v3 with: ref: ${{needs.check_and_prepare.outputs.github_ref}} diff --git a/.github/workflows/integration_tests.yml b/.github/workflows/integration_tests.yml index ac88fb96dd..ad3a9e6a05 100644 --- a/.github/workflows/integration_tests.yml +++ b/.github/workflows/integration_tests.yml @@ -286,6 +286,9 @@ jobs: ssl_variant: openssl arch: arm64 steps: + - uses: lukka/get-cmake@latest + with: + cmakeVersion: "~3.31.0" - uses: actions/checkout@v3 with: ref: ${{needs.check_and_prepare.outputs.github_ref}} @@ -424,6 +427,9 @@ jobs: matrix: os: ${{ fromJson(needs.check_and_prepare.outputs.matrix_os) }} steps: + - uses: lukka/get-cmake@latest + with: + cmakeVersion: "~3.31.0" - uses: actions/checkout@v3 with: ref: ${{needs.check_and_prepare.outputs.github_ref}} @@ -535,6 +541,9 @@ jobs: matrix: os: [macos-13] steps: + - uses: lukka/get-cmake@latest + with: + cmakeVersion: "~3.31.0" - uses: actions/checkout@v3 with: ref: ${{needs.check_and_prepare.outputs.github_ref}} @@ -640,6 +649,9 @@ jobs: matrix: os: [macos-13] steps: + - uses: lukka/get-cmake@latest + with: + cmakeVersion: "~3.31.0" - uses: actions/checkout@v3 with: ref: ${{needs.check_and_prepare.outputs.github_ref}} diff --git a/.github/workflows/ios.yml b/.github/workflows/ios.yml index 45dbe79caf..942fdf6090 100644 --- a/.github/workflows/ios.yml +++ b/.github/workflows/ios.yml @@ -44,6 +44,9 @@ jobs: os: [ 'macos-13' ] xcode_version: ${{ fromJson(needs.prepare_matrix.outputs.matrix_xcode_version) }} steps: + - uses: lukka/get-cmake@latest + with: + cmakeVersion: "~3.31.0" - name: Store git credentials for all git commands # Forces all git commands to use authenticated https, to prevent throttling. shell: bash From 831854e95af141bb0877657c2d3e15e2136065e9 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Wed, 2 Apr 2025 12:55:35 -0700 Subject: [PATCH 04/11] Add setDefaultEventParameters. --- analytics/src/analytics_android.cc | 40 ++++++++++++++++++++++ analytics/src/analytics_ios.mm | 22 ++++++++++++ analytics/src/analytics_stub.cc | 6 ++++ analytics/src/include/firebase/analytics.h | 11 ++++++ 4 files changed, 79 insertions(+) diff --git a/analytics/src/analytics_android.cc b/analytics/src/analytics_android.cc index d1279c6ba7..239c03957a 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"), \ X(GetInstance, "getInstance", "(Landroid/content/Context;)" \ "Lcom/google/firebase/analytics/FirebaseAnalytics;", \ firebase::util::kMethodTypeStatic) @@ -518,6 +520,44 @@ void LogEvent(const char* name, const Parameter* parameters, }); } +// Convert std::map to Bundle. +jobject StringVariantMapToBundle(JNIEnv* env, + const std::map& map) { + jobject bundle = + env->NewObject(util::bundle::GetClass(), + util::bundle::GetMethodId(util::bundle::kConstructor)); + for (const auto& pair : map) { + // Bundle keys must be strings. + const char* key = pair.first.c_str(); + const Variant& value = pair.second; + // A null variant clears the default parameter. The Android SDK uses a null + // value in the Bundle for this. + if (value.is_null()) { + // Add null string to clear the parameter. + AddToBundle(env, bundle, key, static_cast(nullptr)); + } else if (!AddVariantToBundle(env, bundle, key, value)) { + LogError("SetDefaultEventParameters: Unsupported type (%s) for key %s.", + Variant::TypeName(value.type()), key); + } + } + return bundle; +} + +void SetDefaultEventParameters( + const std::map& default_parameters) { + FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized()); + JNIEnv* env = g_app->GetJNIEnv(); + + jobject bundle = StringVariantMapToBundle(env, default_parameters); + + env->CallVoidMethod( + g_analytics_class_instance, + analytics::GetMethodId(analytics::kSetDefaultEventParameters), bundle); + + util::CheckAndClearJniExceptions(env); + env->DeleteLocalRef(bundle); +} + /// Initiates on-device conversion measurement given a user email address on iOS /// (no-op on Android). On iOS, requires dependency /// GoogleAppMeasurementOnDeviceConversion to be linked in, otherwise it is a diff --git a/analytics/src/analytics_ios.mm b/analytics/src/analytics_ios.mm index d56fd61356..ca5ac517b2 100644 --- a/analytics/src/analytics_ios.mm +++ b/analytics/src/analytics_ios.mm @@ -312,6 +312,28 @@ void LogEvent(const char* name, const Parameter* parameters, size_t number_of_pa [FIRAnalytics logEventWithName:@(name) parameters:parameters_dict]; } +void SetDefaultEventParameters(const std::map& default_parameters) { + FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized()); + // Convert the std::map to NSDictionary* + // The keys must be strings for FIRAnalytics. + NSMutableDictionary* parameters_dict = + [NSMutableDictionary dictionaryWithCapacity:default_parameters.size()]; + for (const auto& pair : default_parameters) { + NSString* key = firebase::util::StringToNSString(pair.first); + // A null Variant indicates the default parameter should be cleared. + // In ObjC, setting a key to [NSNull null] in the dictionary achieves this. + id value = pair.second.is_null() ? [NSNull null] : firebase::util::VariantToId(pair.second); + if (value) { + [parameters_dict setObject:value forKey:key]; + } else { + LogError("SetDefaultEventParameters: Failed to convert value for key %s.", + pair.first.c_str()); + } + } + + [FIRAnalytics setDefaultEventParameters:parameters_dict]; +} + /// Initiates on-device conversion measurement given a user email address on iOS (no-op on /// Android). On iOS, requires dependency GoogleAppMeasurementOnDeviceConversion to be linked /// in, otherwise it is a no-op. diff --git a/analytics/src/analytics_stub.cc b/analytics/src/analytics_stub.cc index 37c4b3520b..d529d9f685 100644 --- a/analytics/src/analytics_stub.cc +++ b/analytics/src/analytics_stub.cc @@ -97,6 +97,12 @@ void LogEvent(const char* /*name*/, const Parameter* /*parameters*/, FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized()); } +void SetDefaultEventParameters( + const std::map& default_parameters) { + FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized()); + // No-op on stub. +} + /// Initiates on-device conversion measurement given a user email address on iOS /// (no-op on Android). On iOS, requires dependency /// GoogleAppMeasurementOnDeviceConversion to be linked in, otherwise it is a diff --git a/analytics/src/include/firebase/analytics.h b/analytics/src/include/firebase/analytics.h index 746df99894..46fbc62427 100644 --- a/analytics/src/include/firebase/analytics.h +++ b/analytics/src/include/firebase/analytics.h @@ -476,6 +476,17 @@ void LogEvent(const char* name); void LogEvent(const char* name, const Parameter* parameters, size_t number_of_parameters); +/// @brief Sets default event parameters for this app. +/// +/// This specifies parameters to be included with every subsequent call to +/// `LogEvent`, in addition to the parameters passed to `LogEvent`. +/// The same limitations apply to these parameters as are documented for +/// `LogEvent`. +/// @param[in] default_parameters Map of default parameter names and values. +/// Passing a null value for a parameter name clears the default parameter. +void SetDefaultEventParameters( + const std::map& default_parameters); + /// Initiates on-device conversion measurement given a user email address on iOS /// and tvOS (no-op on Android). On iOS and tvOS, this method requires the /// dependency GoogleAppMeasurementOnDeviceConversion to be linked in, From ea5dfe1e1d116656b6111506134a6f374dab8754 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Wed, 2 Apr 2025 12:57:31 -0700 Subject: [PATCH 05/11] Add integration test --- .../integration_test/src/integration_test.cc | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/analytics/integration_test/src/integration_test.cc b/analytics/integration_test/src/integration_test.cc index bedccee516..3ab8584bd8 100644 --- a/analytics/integration_test/src/integration_test.cc +++ b/analytics/integration_test/src/integration_test.cc @@ -341,4 +341,50 @@ TEST_F(FirebaseAnalyticsTest, TestSetConsent) { did_test_setconsent_ = true; } +// Test that it compiles and runs on all platforms. +TEST_F(FirebaseAnalyticsTest, TestSetDefaultEventParameters) { + LogDebug("Testing SetDefaultEventParameters()."); + + // Set some default parameters. + std::map defaults; + defaults["default_int"] = 123; + defaults["default_string"] = "default_value"; + firebase::analytics::SetDefaultEventParameters(defaults); + + // Log an event - the defaults should be included automatically by the + // underlying SDK if logging immediately after setting is supported. + // Verification might need manual checking in the Analytics console or + // via platform-specific debug logs if possible. + firebase::analytics::LogEvent("event_with_defaults"); + LogDebug("Logged event_with_defaults"); + + // Clear one default parameter by setting it to null. + defaults["default_int"] = Variant::Null(); + firebase::analytics::SetDefaultEventParameters(defaults); + + // Log another event. + firebase::analytics::LogEvent("event_with_one_default_cleared"); + LogDebug("Logged event_with_one_default_cleared"); + + // Set only one parameter, clearing others implicitly if underlying SDK works + // like that + std::map single_default; + single_default["only_this"] = 45.6; + firebase::analytics::SetDefaultEventParameters(single_default); + firebase::analytics::LogEvent("event_with_only_one_default"); + LogDebug("Logged event_with_only_one_default"); + + // Clear all defaults by passing an empty map. + std::map empty_defaults; + firebase::analytics::SetDefaultEventParameters(empty_defaults); + + // Log an event with no defaults. + firebase::analytics::LogEvent("event_with_no_defaults"); + LogDebug("Logged event_with_no_defaults"); + + // If we reach here without crashing, consider the basic test passed for the + // C++ layer. Deeper verification requires platform tools. + LogDebug("SetDefaultEventParameters() tests completed."); +} + } // namespace firebase_testapp_automated From 38bba040450d2bbf0591edd44237f69712b793a8 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Wed, 2 Apr 2025 13:58:38 -0700 Subject: [PATCH 06/11] Fix test error. --- .../integration_test/src/integration_test.cc | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/analytics/integration_test/src/integration_test.cc b/analytics/integration_test/src/integration_test.cc index 3ab8584bd8..4c3236de00 100644 --- a/analytics/integration_test/src/integration_test.cc +++ b/analytics/integration_test/src/integration_test.cc @@ -343,10 +343,10 @@ TEST_F(FirebaseAnalyticsTest, TestSetConsent) { // Test that it compiles and runs on all platforms. TEST_F(FirebaseAnalyticsTest, TestSetDefaultEventParameters) { - LogDebug("Testing SetDefaultEventParameters()."); + LogInfo("Testing SetDefaultEventParameters()."); // Set some default parameters. - std::map defaults; + std::map defaults; defaults["default_int"] = 123; defaults["default_string"] = "default_value"; firebase::analytics::SetDefaultEventParameters(defaults); @@ -356,35 +356,38 @@ TEST_F(FirebaseAnalyticsTest, TestSetDefaultEventParameters) { // Verification might need manual checking in the Analytics console or // via platform-specific debug logs if possible. firebase::analytics::LogEvent("event_with_defaults"); - LogDebug("Logged event_with_defaults"); + LogInfo("Logged event_with_defaults"); // Clear one default parameter by setting it to null. - defaults["default_int"] = Variant::Null(); + defaults["default_int"] = firebase::Variant::Null(); firebase::analytics::SetDefaultEventParameters(defaults); // Log another event. firebase::analytics::LogEvent("event_with_one_default_cleared"); - LogDebug("Logged event_with_one_default_cleared"); + LogInfo("Logged event_with_one_default_cleared"); // Set only one parameter, clearing others implicitly if underlying SDK works // like that - std::map single_default; + std::map single_default; single_default["only_this"] = 45.6; firebase::analytics::SetDefaultEventParameters(single_default); firebase::analytics::LogEvent("event_with_only_one_default"); - LogDebug("Logged event_with_only_one_default"); + LogInfo("Logged event_with_only_one_default"); // Clear all defaults by passing an empty map. - std::map empty_defaults; + std::map empty_defaults; firebase::analytics::SetDefaultEventParameters(empty_defaults); // Log an event with no defaults. firebase::analytics::LogEvent("event_with_no_defaults"); - LogDebug("Logged event_with_no_defaults"); + LogInfo("Logged event_with_no_defaults"); // If we reach here without crashing, consider the basic test passed for the // C++ layer. Deeper verification requires platform tools. - LogDebug("SetDefaultEventParameters() tests completed."); + LogInfo("SetDefaultEventParameters() tests completed."); } + + + } // namespace firebase_testapp_automated From b33d6c9a0d9c8f5304015fd49e5500f5ac59daec Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Wed, 2 Apr 2025 15:47:36 -0700 Subject: [PATCH 07/11] Remove extra space. --- analytics/integration_test/src/integration_test.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/analytics/integration_test/src/integration_test.cc b/analytics/integration_test/src/integration_test.cc index 4c3236de00..5480f93d05 100644 --- a/analytics/integration_test/src/integration_test.cc +++ b/analytics/integration_test/src/integration_test.cc @@ -387,7 +387,4 @@ TEST_F(FirebaseAnalyticsTest, TestSetDefaultEventParameters) { LogInfo("SetDefaultEventParameters() tests completed."); } - - - } // namespace firebase_testapp_automated From 240fcf7790e77a528ac04870b66eafb24ee7e326 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Wed, 2 Apr 2025 15:53:29 -0700 Subject: [PATCH 08/11] Add release note. --- release_build_files/readme.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/release_build_files/readme.md b/release_build_files/readme.md index ae6d754b2c..cadceeb572 100644 --- a/release_build_files/readme.md +++ b/release_build_files/readme.md @@ -631,6 +631,11 @@ workflow use only during the development of your app, not for publicly shipping code. ## Release Notes +### Upcoming Release +- Changes + - Analytics: Add SetDefaultEventParams to set default parameters for + all LogEvent calls. + ### 12.7.0 - Changes - General (iOS): Update to Firebase Cocoapods version 11.10.0. From 7d7e82a0834d48caeec61c0f14a814a503681214 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Wed, 2 Apr 2025 16:19:20 -0700 Subject: [PATCH 09/11] Update function with bug fix. --- analytics/src/analytics_android.cc | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/analytics/src/analytics_android.cc b/analytics/src/analytics_android.cc index 239c03957a..1607930020 100644 --- a/analytics/src/analytics_android.cc +++ b/analytics/src/analytics_android.cc @@ -530,15 +530,12 @@ jobject StringVariantMapToBundle(JNIEnv* env, // Bundle keys must be strings. const char* key = pair.first.c_str(); const Variant& value = pair.second; - // A null variant clears the default parameter. The Android SDK uses a null - // value in the Bundle for this. - if (value.is_null()) { - // Add null string to clear the parameter. - AddToBundle(env, bundle, key, static_cast(nullptr)); - } else if (!AddVariantToBundle(env, bundle, key, value)) { + // AddVariantToBundle handles all Variant types, including null. + if (!AddVariantToBundle(env, bundle, key, value)) { LogError("SetDefaultEventParameters: Unsupported type (%s) for key %s.", Variant::TypeName(value.type()), key); } + // CheckAndClearJniExceptions is called within AddVariantToBundle. } return bundle; } From 112684d322de4164f146915dade3fd8490a45fff Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Thu, 3 Apr 2025 13:39:33 -0700 Subject: [PATCH 10/11] Update so that null is handled for removing keys. Add ClearDefaultEventParameters since we can't pass in null. --- .../integration_test/src/integration_test.cc | 56 +++++++++++++------ analytics/src/analytics_android.cc | 29 +++++++++- analytics/src/analytics_ios.mm | 11 ++++ analytics/src/analytics_stub.cc | 5 ++ analytics/src/include/firebase/analytics.h | 6 +- 5 files changed, 85 insertions(+), 22 deletions(-) diff --git a/analytics/integration_test/src/integration_test.cc b/analytics/integration_test/src/integration_test.cc index 5480f93d05..29dfe4f928 100644 --- a/analytics/integration_test/src/integration_test.cc +++ b/analytics/integration_test/src/integration_test.cc @@ -341,26 +341,32 @@ TEST_F(FirebaseAnalyticsTest, TestSetConsent) { did_test_setconsent_ = true; } -// Test that it compiles and runs on all platforms. TEST_F(FirebaseAnalyticsTest, TestSetDefaultEventParameters) { LogInfo("Testing SetDefaultEventParameters()."); - // Set some default parameters. + // Set some default parameters with various types. std::map defaults; - defaults["default_int"] = 123; - defaults["default_string"] = "default_value"; + defaults["default_int"] = static_cast(123); // int64_t + defaults["default_double"] = 45.67; // double + defaults["default_bool"] = true; // bool + defaults["default_string"] = "test_string_value"; // const char* + defaults["default_to_clear"] = "will_be_cleared"; // Another string + firebase::analytics::SetDefaultEventParameters(defaults); + LogInfo("Set initial default parameters."); // Log an event - the defaults should be included automatically by the // underlying SDK if logging immediately after setting is supported. // Verification might need manual checking in the Analytics console or // via platform-specific debug logs if possible. - firebase::analytics::LogEvent("event_with_defaults"); - LogInfo("Logged event_with_defaults"); + firebase::analytics::LogEvent("event_with_mixed_defaults"); + LogInfo("Logged event_with_mixed_defaults"); - // Clear one default parameter by setting it to null. - defaults["default_int"] = firebase::Variant::Null(); + // Clear one default parameter and update another. + defaults["default_to_clear"] = firebase::Variant::Null(); // Clear this one + defaults["default_int"] = static_cast(999); // Update this one firebase::analytics::SetDefaultEventParameters(defaults); + LogInfo("Cleared one parameter and updated another."); // Log another event. firebase::analytics::LogEvent("event_with_one_default_cleared"); @@ -369,22 +375,36 @@ TEST_F(FirebaseAnalyticsTest, TestSetDefaultEventParameters) { // Set only one parameter, clearing others implicitly if underlying SDK works // like that std::map single_default; - single_default["only_this"] = 45.6; + single_default["only_this_double"] = 78.9; firebase::analytics::SetDefaultEventParameters(single_default); - firebase::analytics::LogEvent("event_with_only_one_default"); + LogInfo("Set a single different default parameter."); + firebase::analytics::LogEvent( + "event_with_only_one_default"); // Changed log event name slightly LogInfo("Logged event_with_only_one_default"); - // Clear all defaults by passing an empty map. - std::map empty_defaults; - firebase::analytics::SetDefaultEventParameters(empty_defaults); - - // Log an event with no defaults. - firebase::analytics::LogEvent("event_with_no_defaults"); - LogInfo("Logged event_with_no_defaults"); - // If we reach here without crashing, consider the basic test passed for the // C++ layer. Deeper verification requires platform tools. LogInfo("SetDefaultEventParameters() tests completed."); } +TEST_F(FirebaseAnalyticsTest, TestClearDefaultEventParameters) { + LogInfo("Testing ClearDefaultEventParameters()."); + + // Set some defaults first. + std::map defaults; + defaults["temp_default"] = "will_be_cleared"; + firebase::analytics::SetDefaultEventParameters(defaults); + + // Now clear them all. + firebase::analytics::ClearDefaultEventParameters(); + + // Log an event - no defaults should be included. + firebase::analytics::LogEvent("event_after_clear_defaults"); + LogInfo("Logged event_after_clear_defaults"); + + // If we reach here without crashing, consider the basic test passed for the + // C++ layer. + LogInfo("ClearDefaultEventParameters() test completed."); +} + } // namespace firebase_testapp_automated diff --git a/analytics/src/analytics_android.cc b/analytics/src/analytics_android.cc index 1607930020..e0fe25b009 100644 --- a/analytics/src/analytics_android.cc +++ b/analytics/src/analytics_android.cc @@ -530,12 +530,22 @@ jobject StringVariantMapToBundle(JNIEnv* env, // Bundle keys must be strings. const char* key = pair.first.c_str(); const Variant& value = pair.second; - // AddVariantToBundle handles all Variant types, including null. - if (!AddVariantToBundle(env, bundle, key, value)) { + // A null variant clears the default parameter for that key. + // The Android SDK uses Bundle.putString(key, null) for this. + if (value.is_null()) { + jstring key_string = env->NewStringUTF(key); + // Call Bundle.putString(key, null) + env->CallVoidMethod(bundle, + util::bundle::GetMethodId(util::bundle::kPutString), + key_string, nullptr); + util::CheckAndClearJniExceptions(env); + env->DeleteLocalRef(key_string); + } else if (!AddVariantToBundle(env, bundle, key, value)) { LogError("SetDefaultEventParameters: Unsupported type (%s) for key %s.", Variant::TypeName(value.type()), key); } - // CheckAndClearJniExceptions is called within AddVariantToBundle. + // CheckAndClearJniExceptions is called within AddVariantToBundle or above + // for the null case. } return bundle; } @@ -555,6 +565,19 @@ void SetDefaultEventParameters( env->DeleteLocalRef(bundle); } +void ClearDefaultEventParameters() { + FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized()); + JNIEnv* env = g_app->GetJNIEnv(); + + // Call FirebaseAnalytics.setDefaultEventParameters(null) + env->CallVoidMethod( + g_analytics_class_instance, + analytics::GetMethodId(analytics::kSetDefaultEventParameters), + nullptr); // Pass null Bundle to clear all parameters + + util::CheckAndClearJniExceptions(env); +} + /// Initiates on-device conversion measurement given a user email address on iOS /// (no-op on Android). On iOS, requires dependency /// GoogleAppMeasurementOnDeviceConversion to be linked in, otherwise it is a diff --git a/analytics/src/analytics_ios.mm b/analytics/src/analytics_ios.mm index ca5ac517b2..35622971e2 100644 --- a/analytics/src/analytics_ios.mm +++ b/analytics/src/analytics_ios.mm @@ -322,10 +322,15 @@ void SetDefaultEventParameters(const std::map& default_par NSString* key = firebase::util::StringToNSString(pair.first); // A null Variant indicates the default parameter should be cleared. // In ObjC, setting a key to [NSNull null] in the dictionary achieves this. + // A null Variant indicates the default parameter for that key should be + // cleared. In ObjC, setting a key to [NSNull null] in the dictionary + // achieves this. id value = pair.second.is_null() ? [NSNull null] : firebase::util::VariantToId(pair.second); if (value) { [parameters_dict setObject:value forKey:key]; } else { + // VariantToId could return nil if the variant type is unsupported. + // Log an error but continue, as NSNull case is handled above. LogError("SetDefaultEventParameters: Failed to convert value for key %s.", pair.first.c_str()); } @@ -334,6 +339,12 @@ void SetDefaultEventParameters(const std::map& default_par [FIRAnalytics setDefaultEventParameters:parameters_dict]; } +void ClearDefaultEventParameters() { + FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized()); + // Passing nil to the underlying SDK method clears all parameters. + [FIRAnalytics setDefaultEventParameters:nil]; +} + /// Initiates on-device conversion measurement given a user email address on iOS (no-op on /// Android). On iOS, requires dependency GoogleAppMeasurementOnDeviceConversion to be linked /// in, otherwise it is a no-op. diff --git a/analytics/src/analytics_stub.cc b/analytics/src/analytics_stub.cc index d529d9f685..9d1d5ba9a9 100644 --- a/analytics/src/analytics_stub.cc +++ b/analytics/src/analytics_stub.cc @@ -103,6 +103,11 @@ void SetDefaultEventParameters( // No-op on stub. } +void ClearDefaultEventParameters() { + FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized()); + // No-op on stub. +} + /// Initiates on-device conversion measurement given a user email address on iOS /// (no-op on Android). On iOS, requires dependency /// GoogleAppMeasurementOnDeviceConversion to be linked in, otherwise it is a diff --git a/analytics/src/include/firebase/analytics.h b/analytics/src/include/firebase/analytics.h index 46fbc62427..78cf72c1fd 100644 --- a/analytics/src/include/firebase/analytics.h +++ b/analytics/src/include/firebase/analytics.h @@ -483,10 +483,14 @@ void LogEvent(const char* name, const Parameter* parameters, /// The same limitations apply to these parameters as are documented for /// `LogEvent`. /// @param[in] default_parameters Map of default parameter names and values. -/// Passing a null value for a parameter name clears the default parameter. +/// Passing a null `Variant` value for a parameter name clears the default +/// parameter for that key. void SetDefaultEventParameters( const std::map& default_parameters); +/// @brief Clears all default event parameters. +void ClearDefaultEventParameters(); + /// Initiates on-device conversion measurement given a user email address on iOS /// and tvOS (no-op on Android). On iOS and tvOS, this method requires the /// dependency GoogleAppMeasurementOnDeviceConversion to be linked in, From e26f7b005cd9e243d8ca790f5afc4ed9ae1f3285 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Thu, 3 Apr 2025 13:57:57 -0700 Subject: [PATCH 11/11] Remove extraneous comment. --- analytics/src/analytics_ios.mm | 2 -- 1 file changed, 2 deletions(-) diff --git a/analytics/src/analytics_ios.mm b/analytics/src/analytics_ios.mm index 35622971e2..934eff7ab3 100644 --- a/analytics/src/analytics_ios.mm +++ b/analytics/src/analytics_ios.mm @@ -320,8 +320,6 @@ void SetDefaultEventParameters(const std::map& default_par [NSMutableDictionary dictionaryWithCapacity:default_parameters.size()]; for (const auto& pair : default_parameters) { NSString* key = firebase::util::StringToNSString(pair.first); - // A null Variant indicates the default parameter should be cleared. - // In ObjC, setting a key to [NSNull null] in the dictionary achieves this. // A null Variant indicates the default parameter for that key should be // cleared. In ObjC, setting a key to [NSNull null] in the dictionary // achieves this.