Bug 1662868 - Use Glean's String implementation. r=janerik,chutten

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

toolkit/components/glean/api/src/private/string.rs

@@ -2,6 +2,8 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
+use inherent::inherent;
+
 use super::{CommonMetricData, MetricId};
 
 use crate::ipc::need_ipc;
@@ -35,15 +37,12 @@ use crate::ipc::need_ipc;
 /// ```rust,ignore
 /// browser::search_engine.set("websearch");
 /// ```
-#[derive(Debug)]
+#[derive(Clone)]
 pub enum StringMetric {
-    Parent(StringMetricImpl),
+    Parent(glean::private::StringMetric),
     Child(StringMetricIpc),
 }
-
 #[derive(Clone, Debug)]
-pub struct StringMetricImpl(pub(crate) glean_core::metrics::StringMetric);
-#[derive(Debug)]
 pub struct StringMetricIpc;
 
 impl StringMetric {
@@ -52,7 +51,7 @@ impl StringMetric {
         if need_ipc() {
             StringMetric::Child(StringMetricIpc)
         } else {
-            StringMetric::Parent(StringMetricImpl::new(meta))
+            StringMetric::Parent(glean::private::StringMetric::new(meta))
         }
     }
 
@@ -63,68 +62,80 @@ impl StringMetric {
             StringMetric::Child(_) => panic!("Can't get a child metric from a child metric"),
         }
     }
+}
 
-    /// Set to the specified value.
+#[inherent(pub)]
+impl glean_core::traits::String for StringMetric {
+    /// Sets to the specified value.
     ///
-    /// ## Arguments
+    /// # Arguments
     ///
     /// * `value` - The string to set the metric to.
     ///
     /// ## Notes
     ///
     /// Truncates the value if it is longer than `MAX_STRING_LENGTH` bytes and logs an error.
-    /// See [String metric limits](https://mozilla.github.io/glean/book/user/metrics/string.html#limits).
-    pub fn set<S: Into<String>>(&self, value: S) {
+    fn set<S: Into<std::string::String>>(&self, value: S) {
         match self {
-            StringMetric::Parent(p) => p.set(value),
-            // No truncation here. Hrm.
+            StringMetric::Parent(p) => {
+                glean_core::traits::String::set(&*p, value);
+            }
             StringMetric::Child(_) => {
-                log::error!(
-                    "Unable to set string metric {:?} in non-main process. Ignoring.",
-                    self
-                );
+                log::error!("Unable to set string metric in non-main process. Ignoring.");
                 // TODO: Record an error.
             }
         };
     }
 
-    /// **Test-only API.**
+    /// **Exported for test purposes.**
+    ///
+    /// Gets the currently stored value as a string.
     ///
-    /// Get the currently stored value as a string.
     /// This doesn't clear the stored value.
     ///
-    /// ## Arguments
+    /// # Arguments
     ///
-    /// * `storage_name` - the storage name to look into.
+    /// * `ping_name` - represents the optional name of the ping to retrieve the
+    ///   metric for. Defaults to the first value in `send_in_pings`.
+    fn test_get_value<'a, S: Into<Option<&'a str>>>(
+        &self,
+        ping_name: S,
+    ) -> Option<std::string::String> {
+        match self {
+            StringMetric::Parent(p) => p.test_get_value(ping_name),
+            StringMetric::Child(_) => {
+                panic!("Cannot get test value for string metric in non-parent process!")
+            }
+        }
+    }
+
+    /// **Exported for test purposes.**
     ///
-    /// ## Return value
+    /// Gets the number of recorded errors for the given metric and error type.
     ///
-    /// Returns the stored value or `None` if nothing stored.
-    pub fn test_get_value(&self, storage_name: &str) -> Option<String> {
+    /// # Arguments
+    ///
+    /// * `error` - The type of error
+    /// * `ping_name` - represents the optional name of the ping to retrieve the
+    ///   metric for. Defaults to the first value in `send_in_pings`.
+    ///
+    /// # Returns
+    ///
+    /// The number of errors reported.
+    fn test_get_num_recorded_errors<'a, S: Into<Option<&'a str>>>(
+        &self,
+        error: glean::ErrorType,
+        ping_name: S,
+    ) -> i32 {
         match self {
-            StringMetric::Parent(p) => p.test_get_value(storage_name),
+            StringMetric::Parent(p) => p.test_get_num_recorded_errors(error, ping_name),
             StringMetric::Child(_) => panic!(
-                "Cannot get test value for {:?} in non-parent process!",
-                self
+                "Cannot get the number of recorded errors for string metric in non-parent process!"
             ),
         }
     }
 }
 
-impl StringMetricImpl {
-    fn new(meta: CommonMetricData) -> Self {
-        Self(glean_core::metrics::StringMetric::new(meta))
-    }
-
-    fn set<S: Into<String>>(&self, value: S) {
-        crate::with_glean(move |glean| self.0.set(glean, value))
-    }
-
-    fn test_get_value(&self, storage_name: &str) -> Option<String> {
-        crate::with_glean(move |glean| self.0.test_get_value(glean, storage_name))
-    }
-}
-
 #[cfg(test)]
 mod test {
     use crate::{common_test::*, ipc, metrics};