Bug 1682960 - Make the ping name optional in FOG JS and C++ test APIs r=janerik

Differential Revision: https://phabricator.services.mozilla.com/D99945

toolkit/components/glean/api/src/ffi/macros.rs

@@ -31,9 +31,12 @@ macro_rules! metric_get {
 macro_rules! test_has {
     ($map:ident, $id:ident, $storage:ident) => {{
         let metric = metric_get!($map, $id);
-        let storage = $storage.to_utf8();
-        let storage = Some(&storage[..]);
-        metric.test_get_value(storage).is_some()
+        let storage = if $storage.is_empty() {
+            None
+        } else {
+            Some($storage.to_utf8())
+        };
+        metric.test_get_value(storage.as_deref()).is_some()
     }};
 }
 
@@ -48,8 +51,11 @@ macro_rules! test_has {
 macro_rules! test_get {
     ($map:ident, $id:ident, $storage:ident) => {{
         let metric = metric_get!($map, $id);
-        let storage = $storage.to_utf8();
-        let storage = Some(&storage[..]);
-        metric.test_get_value(storage).unwrap()
+        let storage = if $storage.is_empty() {
+            None
+        } else {
+            Some($storage.to_utf8())
+        };
+        metric.test_get_value(storage.as_deref()).unwrap()
     }};
 }

toolkit/components/glean/bindings/private/Boolean.h

@@ -38,13 +38,16 @@ class BooleanMetric {
    * This doesn't clear the stored value.
    * Parent process only. Panics in child processes.
    *
+   * @param aPingName The (optional) name of the ping to retrieve the metric
+   *        for. Defaults to the first value in `send_in_pings`.
+   *
    * @return value of the stored metric.
    */
-  Maybe<bool> TestGetValue(const nsACString& aStorageName) const {
-    if (!fog_boolean_test_has_value(mId, &aStorageName)) {
+  Maybe<bool> TestGetValue(const nsACString& aPingName = nsCString()) const {
+    if (!fog_boolean_test_has_value(mId, &aPingName)) {
       return Nothing();
     }
-    return Some(fog_boolean_test_get_value(mId, &aStorageName));
+    return Some(fog_boolean_test_get_value(mId, &aPingName));
   }
 
  private:

toolkit/components/glean/bindings/private/Counter.h

@@ -38,13 +38,16 @@ class CounterMetric {
    * This doesn't clear the stored value.
    * Parent process only. Panics in child processes.
    *
+   * @param aPingName The (optional) name of the ping to retrieve the metric
+   *        for. Defaults to the first value in `send_in_pings`.
+   *
    * @return value of the stored metric, or Nothing() if there is no value.
    */
-  Maybe<int32_t> TestGetValue(const nsACString& aStorageName) const {
-    if (!fog_counter_test_has_value(mId, &aStorageName)) {
+  Maybe<int32_t> TestGetValue(const nsACString& aPingName = nsCString()) const {
+    if (!fog_counter_test_has_value(mId, &aPingName)) {
       return Nothing();
     }
-    return Some(fog_counter_test_get_value(mId, &aStorageName));
+    return Some(fog_counter_test_get_value(mId, &aPingName));
   }
 
  private:

toolkit/components/glean/bindings/private/Datetime.h

@@ -53,14 +53,18 @@ class DatetimeMetric {
    * This doesn't clear the stored value.
    * Parent process only. Panics in child processes.
    *
+   * @param aPingName The (optional) name of the ping to retrieve the metric
+   *        for. Defaults to the first value in `send_in_pings`.
+   *
    * @return value of the stored metric, or Nothing() if there is no value.
    */
-  Maybe<nsCString> TestGetValue(const nsACString& aStorageName) const {
-    if (!fog_datetime_test_has_value(mId, &aStorageName)) {
+  Maybe<nsCString> TestGetValue(
+      const nsACString& aPingName = nsCString()) const {
+    if (!fog_datetime_test_has_value(mId, &aPingName)) {
       return Nothing();
     }
     nsCString ret;
-    fog_datetime_test_get_value(mId, &aStorageName, &ret);
+    fog_datetime_test_get_value(mId, &aPingName, &ret);
     return Some(ret);
   }
 

toolkit/components/glean/bindings/private/Event.h

@@ -73,11 +73,14 @@ class EventMetric {
    * This doesn't clear the stored value.
    * Parent process only. Panics in child processes.
    *
+   * @param aPingName The (optional) name of the ping to retrieve the metric
+   *        for. Defaults to the first value in `send_in_pings`.
+   *
    * @return value of the stored metric, or Nothing() if there is no value.
    */
   Maybe<nsTArray<RecordedEvent>> TestGetValue(
-      const nsACString& aStorageName) const {
-    if (!fog_event_test_has_value(mId, &aStorageName)) {
+      const nsACString& aPingName = nsCString()) const {
+    if (!fog_event_test_has_value(mId, &aPingName)) {
       return Nothing();
     }
 

toolkit/components/glean/bindings/private/MemoryDistribution.h

@@ -51,9 +51,13 @@ class MemoryDistributionMetric {
    * This doesn't clear the stored value.
    * Parent process only. Panics in child processes.
    *
+   * @param aPingName The (optional) name of the ping to retrieve the metric
+   *        for. Defaults to the first value in `send_in_pings`.
+   *
    * @return value of the stored metric, or Nothing() if there is no value.
    */
-  Maybe<DistributionData> TestGetValue(const nsACString& aPingName) const {
+  Maybe<DistributionData> TestGetValue(
+      const nsACString& aPingName = nsCString()) const {
     if (!fog_memory_distribution_test_has_value(mId, &aPingName)) {
       return Nothing();
     }

toolkit/components/glean/bindings/private/Ping.h

@@ -40,7 +40,7 @@ class Ping {
    * @param aReason - Optional. The reason the ping is being submitted.
    *                  Must match one of the configured `reason_codes`.
    */
-  void Submit(const nsACString& aReason) const {
+  void Submit(const nsACString& aReason = nsCString()) const {
     fog_submit_ping_by_id(mId, &aReason);
   }
 

toolkit/components/glean/bindings/private/String.h

@@ -42,14 +42,18 @@ class StringMetric {
    * This doesn't clear the stored value.
    * Parent process only. Panics in child processes.
    *
+   * @param aPingName The (optional) name of the ping to retrieve the metric
+   *        for. Defaults to the first value in `send_in_pings`.
+   *
    * @return value of the stored metric, or Nothing() if there is no value.
    */
-  Maybe<nsCString> TestGetValue(const nsACString& aStorageName) const {
-    if (!fog_string_test_has_value(mId, &aStorageName)) {
+  Maybe<nsCString> TestGetValue(
+      const nsACString& aPingName = nsCString()) const {
+    if (!fog_string_test_has_value(mId, &aPingName)) {
       return Nothing();
     }
     nsCString ret;
-    fog_string_test_get_value(mId, &aStorageName, &ret);
+    fog_string_test_get_value(mId, &aPingName, &ret);
     return Some(ret);
   }
 

toolkit/components/glean/bindings/private/Timespan.h

@@ -50,13 +50,16 @@ class TimespanMetric {
    * This doesn't clear the stored value.
    * Parent process only. Panics in child processes.
    *
+   * @param aPingName The (optional) name of the ping to retrieve the metric
+   *        for. Defaults to the first value in `send_in_pings`.
+   *
    * @return value of the stored metric, or Nothing() if there is no value.
    */
-  Maybe<int64_t> TestGetValue(const nsACString& aStorageName) const {
-    if (!fog_timespan_test_has_value(mId, &aStorageName)) {
+  Maybe<int64_t> TestGetValue(const nsACString& aPingName = nsCString()) const {
+    if (!fog_timespan_test_has_value(mId, &aPingName)) {
       return Nothing();
     }
-    return Some(fog_timespan_test_get_value(mId, &aStorageName));
+    return Some(fog_timespan_test_get_value(mId, &aPingName));
   }
 
  private:

toolkit/components/glean/bindings/private/Uuid.h

@@ -43,16 +43,19 @@ class UuidMetric {
    *
    * This doesn't clear the stored value.
    * Parent process only. Panics in child processes.
-   * Panics if there is no value to get.
+   *
+   * @param aPingName The (optional) name of the ping to retrieve the metric
+   *        for. Defaults to the first value in `send_in_pings`.
    *
    * @return value of the stored metric, or Nothing() if there is no value.
    */
-  Maybe<nsCString> TestGetValue(const nsACString& aStorageName) const {
-    if (!fog_uuid_test_has_value(mId, &aStorageName)) {
+  Maybe<nsCString> TestGetValue(
+      const nsACString& aPingName = nsCString()) const {
+    if (!fog_uuid_test_has_value(mId, &aPingName)) {
       return Nothing();
     }
     nsCString ret;
-    fog_uuid_test_get_value(mId, &aStorageName, &ret);
+    fog_uuid_test_get_value(mId, &aPingName, &ret);
     return Some(ret);
   }
 

toolkit/components/glean/gtest/TestFog.cpp

@@ -63,6 +63,8 @@ TEST(FOG, TestCppCounterWorks)
   ASSERT_EQ(
       42,
       mozilla::glean::test_only::bad_code.TestGetValue("test-ping"_ns).value());
+  // And test that the ping name's optional, while you're at it:
+  ASSERT_EQ(42, test_only::bad_code.TestGetValue().value());
 }
 
 TEST(FOG, TestCppStringWorks)

toolkit/components/glean/xpcom/nsIGleanMetrics.idl

@@ -27,9 +27,12 @@ interface nsIGleanBoolean : nsISupports
    * This doesn't clear the stored value.
    * Parent process only. Panics in child processes.
    *
+   * @param aPingName The (optional) name of the ping to retrieve the metric
+   *        for. Defaults to the first value in `send_in_pings`.
+   *
    * @return value of the stored metric, or undefined if there is no value.
    */
-  jsval testGetValue(in AUTF8String aStorageName);
+  jsval testGetValue([optional] in AUTF8String aPingName);
 };
 
 [scriptable, uuid(aa15fd20-1e8a-11eb-9bec-0800200c9a66)]
@@ -55,10 +58,13 @@ interface nsIGleanDatetime : nsISupports
    * This doesn't clear the stored value.
    * Parent process only. Panics in child processes.
    *
+   * @param aPingName The (optional) name of the ping to retrieve the metric
+   *        for. Defaults to the first value in `send_in_pings`.
+   *
    * @return value of the stored metric, or undefined if there is no value.
    */
   [implicit_jscontext]
-  jsval testGetValue(in AUTF8String aStorageName);
+  jsval testGetValue([optional] in AUTF8String aPingName);
 };
 
 [scriptable, uuid(05b89d2a-d57c-11ea-82da-3f63399a6f5a)]
@@ -83,9 +89,12 @@ interface nsIGleanCounter : nsISupports
    * This doesn't clear the stored value.
    * Parent process only. Panics in child processes.
    *
+   * @param aPingName The (optional) name of the ping to retrieve the metric
+   *        for. Defaults to the first value in `send_in_pings`.
+   *
    * @return value of the stored metric, or undefined if there is no value.
    */
-  jsval testGetValue(in AUTF8String aStorageName);
+  jsval testGetValue([optional] in AUTF8String aPingName);
 };
 
 [scriptable, uuid(eea5ed46-16ba-46cd-bb1f-504581987fe1)]
@@ -114,10 +123,13 @@ interface nsIGleanMemoryDistribution : nsISupports
    * This doesn't clear the stored value.
    * Parent process only. Panics in child processes.
    *
+   * @param aPingName The (optional) name of the ping to retrieve the metric
+   *        for. Defaults to the first value in `send_in_pings`.
+   *
    * @return value of the stored metric, or Nothing() if there is no value.
    */
   [implicit_jscontext]
-	jsval testGetValue(in ACString aStorageName);
+	jsval testGetValue([optional] in ACString aPingName);
 };
 
 [scriptable, uuid(5223a48b-687d-47ff-a629-fd4a72d1ecfa)]
@@ -167,10 +179,13 @@ interface nsIGleanString : nsISupports
    * This doesn't clear the stored value.
    * Parent process only. Panics in child processes.
    *
+   * @param aPingName The (optional) name of the ping to retrieve the metric
+   *        for. Defaults to the first value in `send_in_pings`.
+   *
    * @return value of the stored metric, or undefined if there is no value.
    */
   [implicit_jscontext]
-  jsval testGetValue(in AUTF8String aStorageName);
+  jsval testGetValue([optional] in AUTF8String aPingName);
 };
 
 [scriptable, uuid(2586530c-030f-11eb-93cb-cbf30d25225a)]
@@ -207,9 +222,12 @@ interface nsIGleanTimespan : nsISupports
    * This doesn't clear the stored value.
    * Parent process only. Panics in child processes.
    *
+   * @param aPingName The (optional) name of the ping to retrieve the metric
+   *        for. Defaults to the first value in `send_in_pings`.
+   *
    * @return value of the stored metric, or undefined if there is no value.
    */
-  jsval testGetValue(in AUTF8String aStorageName);
+  jsval testGetValue([optional] in AUTF8String aPingName);
 };
 
 [scriptable, uuid(395700e7-06f6-46be-adcc-ea58977fda6d)]
@@ -239,10 +257,13 @@ interface nsIGleanUuid : nsISupports
    * This doesn't clear the stored value.
    * Parent process only. Panics in child processes.
    *
+   * @param aPingName The (optional) name of the ping to retrieve the metric
+   *        for. Defaults to the first value in `send_in_pings`.
+   *
    * @return value of the stored metric, or undefined if there is no value.
    */
   [implicit_jscontext]
-  jsval testGetValue(in AUTF8String aStorageName);
+  jsval testGetValue([optional] in AUTF8String aPingName);
 };
 
 [scriptable, uuid(1b01424a-1f55-11eb-92a5-0754f6c3f240)]
@@ -268,8 +289,11 @@ interface nsIGleanEvent : nsISupports
    * This doesn't clear the stored value.
    * Parent process only. Panics in child processes.
    *
+   * @param aPingName The (optional) name of the ping to retrieve the metric
+   *        for. Defaults to the first value in `send_in_pings`.
+   *
    * @return value of the stored metric.
    */
   [implicit_jscontext]
-  jsval testGetValue(in AUTF8String aStorageName);
+  jsval testGetValue([optional] in AUTF8String aPingName);
 };

toolkit/components/glean/xpcshell/test_Glean.js

@@ -135,6 +135,8 @@ add_task({ skip_if: () => true }, function test_fog_datetime_works() {
 add_task(function test_fog_boolean_works() {
   Glean.test_only.can_we_flag_it.set(false);
   Assert.equal(false, Glean.test_only.can_we_flag_it.testGetValue("test-ping"));
+  // While you're here, might as well test that the ping name's optional.
+  Assert.equal(false, Glean.test_only.can_we_flag_it.testGetValue());
 });
 
 add_task(async function test_fog_event_works() {