Skip to content

Commit 112684d

Browse files
committed
Update so that null is handled for removing keys.
Add ClearDefaultEventParameters since we can't pass in null.
1 parent 7d7e82a commit 112684d

File tree

5 files changed

+85
-22
lines changed

5 files changed

+85
-22
lines changed

analytics/integration_test/src/integration_test.cc

+38-18
Original file line numberDiff line numberDiff line change
@@ -341,26 +341,32 @@ TEST_F(FirebaseAnalyticsTest, TestSetConsent) {
341341
did_test_setconsent_ = true;
342342
}
343343

344-
// Test that it compiles and runs on all platforms.
345344
TEST_F(FirebaseAnalyticsTest, TestSetDefaultEventParameters) {
346345
LogInfo("Testing SetDefaultEventParameters().");
347346

348-
// Set some default parameters.
347+
// Set some default parameters with various types.
349348
std::map<std::string, firebase::Variant> defaults;
350-
defaults["default_int"] = 123;
351-
defaults["default_string"] = "default_value";
349+
defaults["default_int"] = static_cast<int64_t>(123); // int64_t
350+
defaults["default_double"] = 45.67; // double
351+
defaults["default_bool"] = true; // bool
352+
defaults["default_string"] = "test_string_value"; // const char*
353+
defaults["default_to_clear"] = "will_be_cleared"; // Another string
354+
352355
firebase::analytics::SetDefaultEventParameters(defaults);
356+
LogInfo("Set initial default parameters.");
353357

354358
// Log an event - the defaults should be included automatically by the
355359
// underlying SDK if logging immediately after setting is supported.
356360
// Verification might need manual checking in the Analytics console or
357361
// via platform-specific debug logs if possible.
358-
firebase::analytics::LogEvent("event_with_defaults");
359-
LogInfo("Logged event_with_defaults");
362+
firebase::analytics::LogEvent("event_with_mixed_defaults");
363+
LogInfo("Logged event_with_mixed_defaults");
360364

361-
// Clear one default parameter by setting it to null.
362-
defaults["default_int"] = firebase::Variant::Null();
365+
// Clear one default parameter and update another.
366+
defaults["default_to_clear"] = firebase::Variant::Null(); // Clear this one
367+
defaults["default_int"] = static_cast<int64_t>(999); // Update this one
363368
firebase::analytics::SetDefaultEventParameters(defaults);
369+
LogInfo("Cleared one parameter and updated another.");
364370

365371
// Log another event.
366372
firebase::analytics::LogEvent("event_with_one_default_cleared");
@@ -369,22 +375,36 @@ TEST_F(FirebaseAnalyticsTest, TestSetDefaultEventParameters) {
369375
// Set only one parameter, clearing others implicitly if underlying SDK works
370376
// like that
371377
std::map<std::string, firebase::Variant> single_default;
372-
single_default["only_this"] = 45.6;
378+
single_default["only_this_double"] = 78.9;
373379
firebase::analytics::SetDefaultEventParameters(single_default);
374-
firebase::analytics::LogEvent("event_with_only_one_default");
380+
LogInfo("Set a single different default parameter.");
381+
firebase::analytics::LogEvent(
382+
"event_with_only_one_default"); // Changed log event name slightly
375383
LogInfo("Logged event_with_only_one_default");
376384

377-
// Clear all defaults by passing an empty map.
378-
std::map<std::string, firebase::Variant> empty_defaults;
379-
firebase::analytics::SetDefaultEventParameters(empty_defaults);
380-
381-
// Log an event with no defaults.
382-
firebase::analytics::LogEvent("event_with_no_defaults");
383-
LogInfo("Logged event_with_no_defaults");
384-
385385
// If we reach here without crashing, consider the basic test passed for the
386386
// C++ layer. Deeper verification requires platform tools.
387387
LogInfo("SetDefaultEventParameters() tests completed.");
388388
}
389389

390+
TEST_F(FirebaseAnalyticsTest, TestClearDefaultEventParameters) {
391+
LogInfo("Testing ClearDefaultEventParameters().");
392+
393+
// Set some defaults first.
394+
std::map<std::string, firebase::Variant> defaults;
395+
defaults["temp_default"] = "will_be_cleared";
396+
firebase::analytics::SetDefaultEventParameters(defaults);
397+
398+
// Now clear them all.
399+
firebase::analytics::ClearDefaultEventParameters();
400+
401+
// Log an event - no defaults should be included.
402+
firebase::analytics::LogEvent("event_after_clear_defaults");
403+
LogInfo("Logged event_after_clear_defaults");
404+
405+
// If we reach here without crashing, consider the basic test passed for the
406+
// C++ layer.
407+
LogInfo("ClearDefaultEventParameters() test completed.");
408+
}
409+
390410
} // namespace firebase_testapp_automated

analytics/src/analytics_android.cc

+26-3
Original file line numberDiff line numberDiff line change
@@ -530,12 +530,22 @@ jobject StringVariantMapToBundle(JNIEnv* env,
530530
// Bundle keys must be strings.
531531
const char* key = pair.first.c_str();
532532
const Variant& value = pair.second;
533-
// AddVariantToBundle handles all Variant types, including null.
534-
if (!AddVariantToBundle(env, bundle, key, value)) {
533+
// A null variant clears the default parameter for that key.
534+
// The Android SDK uses Bundle.putString(key, null) for this.
535+
if (value.is_null()) {
536+
jstring key_string = env->NewStringUTF(key);
537+
// Call Bundle.putString(key, null)
538+
env->CallVoidMethod(bundle,
539+
util::bundle::GetMethodId(util::bundle::kPutString),
540+
key_string, nullptr);
541+
util::CheckAndClearJniExceptions(env);
542+
env->DeleteLocalRef(key_string);
543+
} else if (!AddVariantToBundle(env, bundle, key, value)) {
535544
LogError("SetDefaultEventParameters: Unsupported type (%s) for key %s.",
536545
Variant::TypeName(value.type()), key);
537546
}
538-
// CheckAndClearJniExceptions is called within AddVariantToBundle.
547+
// CheckAndClearJniExceptions is called within AddVariantToBundle or above
548+
// for the null case.
539549
}
540550
return bundle;
541551
}
@@ -555,6 +565,19 @@ void SetDefaultEventParameters(
555565
env->DeleteLocalRef(bundle);
556566
}
557567

568+
void ClearDefaultEventParameters() {
569+
FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized());
570+
JNIEnv* env = g_app->GetJNIEnv();
571+
572+
// Call FirebaseAnalytics.setDefaultEventParameters(null)
573+
env->CallVoidMethod(
574+
g_analytics_class_instance,
575+
analytics::GetMethodId(analytics::kSetDefaultEventParameters),
576+
nullptr); // Pass null Bundle to clear all parameters
577+
578+
util::CheckAndClearJniExceptions(env);
579+
}
580+
558581
/// Initiates on-device conversion measurement given a user email address on iOS
559582
/// (no-op on Android). On iOS, requires dependency
560583
/// GoogleAppMeasurementOnDeviceConversion to be linked in, otherwise it is a

analytics/src/analytics_ios.mm

+11
Original file line numberDiff line numberDiff line change
@@ -322,10 +322,15 @@ void SetDefaultEventParameters(const std::map<std::string, Variant>& default_par
322322
NSString* key = firebase::util::StringToNSString(pair.first);
323323
// A null Variant indicates the default parameter should be cleared.
324324
// In ObjC, setting a key to [NSNull null] in the dictionary achieves this.
325+
// A null Variant indicates the default parameter for that key should be
326+
// cleared. In ObjC, setting a key to [NSNull null] in the dictionary
327+
// achieves this.
325328
id value = pair.second.is_null() ? [NSNull null] : firebase::util::VariantToId(pair.second);
326329
if (value) {
327330
[parameters_dict setObject:value forKey:key];
328331
} else {
332+
// VariantToId could return nil if the variant type is unsupported.
333+
// Log an error but continue, as NSNull case is handled above.
329334
LogError("SetDefaultEventParameters: Failed to convert value for key %s.",
330335
pair.first.c_str());
331336
}
@@ -334,6 +339,12 @@ void SetDefaultEventParameters(const std::map<std::string, Variant>& default_par
334339
[FIRAnalytics setDefaultEventParameters:parameters_dict];
335340
}
336341

342+
void ClearDefaultEventParameters() {
343+
FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized());
344+
// Passing nil to the underlying SDK method clears all parameters.
345+
[FIRAnalytics setDefaultEventParameters:nil];
346+
}
347+
337348
/// Initiates on-device conversion measurement given a user email address on iOS (no-op on
338349
/// Android). On iOS, requires dependency GoogleAppMeasurementOnDeviceConversion to be linked
339350
/// in, otherwise it is a no-op.

analytics/src/analytics_stub.cc

+5
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,11 @@ void SetDefaultEventParameters(
103103
// No-op on stub.
104104
}
105105

106+
void ClearDefaultEventParameters() {
107+
FIREBASE_ASSERT_RETURN_VOID(internal::IsInitialized());
108+
// No-op on stub.
109+
}
110+
106111
/// Initiates on-device conversion measurement given a user email address on iOS
107112
/// (no-op on Android). On iOS, requires dependency
108113
/// GoogleAppMeasurementOnDeviceConversion to be linked in, otherwise it is a

analytics/src/include/firebase/analytics.h

+5-1
Original file line numberDiff line numberDiff line change
@@ -483,10 +483,14 @@ void LogEvent(const char* name, const Parameter* parameters,
483483
/// The same limitations apply to these parameters as are documented for
484484
/// `LogEvent`.
485485
/// @param[in] default_parameters Map of default parameter names and values.
486-
/// Passing a null value for a parameter name clears the default parameter.
486+
/// Passing a null `Variant` value for a parameter name clears the default
487+
/// parameter for that key.
487488
void SetDefaultEventParameters(
488489
const std::map<std::string, Variant>& default_parameters);
489490

491+
/// @brief Clears all default event parameters.
492+
void ClearDefaultEventParameters();
493+
490494
/// Initiates on-device conversion measurement given a user email address on iOS
491495
/// and tvOS (no-op on Android). On iOS and tvOS, this method requires the
492496
/// dependency GoogleAppMeasurementOnDeviceConversion to be linked in,

0 commit comments

Comments
 (0)