Bug 1679378 - Implement the string metric type (#13)

Author: Beatriz Rizental

src/database.ts

@@ -129,14 +129,28 @@ class Database {
   }
 
   /**
-   * Gets the persisted payload of a given metric in a given ping.
+   * Gets  and validates the persisted payload of a given metric in a given ping.
+   *
+   * If the persisted value is invalid for the metric we are attempting to retrieve,
+   * the persisted value is deleted and `undefined is returned.
+   *
+   * This behaviour is not consistent with what the Glean SDK does, but this is on purpose.
+   * On the Glean SDK we panic when we can't serialize the persisted value,
+   * that is because this is an extremely unlikely situation for that environment.
+   *
+   * Since Glean.js will run on the browser, it is easy for a consumers / developers
+   * to mess with the storage which makes this sort of errors plausible.
+   * That is why we choose to not panic and simply delete the corrupted data here.
+   *
+   * Note: This is not a strong guard against consumers / developers messing with the storage on their own.
+   * Currently Glean.js does not include mechanisms to reliably prevent that.
    *
    * @param ping The ping from which we want to retrieve the given metric.
-   * @param validateFn A validation function to verify if persisted payload is of the correct type.
+   * @param validateFn A validation function to verify if persisted payload is in the correct format.
    * @param metric An object containing the information about the metric to retrieve.
    *
    * @returns The payload persisted for the given metric,
-   *          `undefined` in case the metric has not been recorded yet.
+   *          `undefined` in case the metric has not been recorded yet or the found valus in invalid.
    */
   async getMetric<T>(
     ping: string,
@@ -147,13 +161,6 @@ class Database {
     const storageKey = metric.identifier;
     const value = await store.get([ping, metric.type, storageKey]);
     if (!isUndefined(value) && !validateFn(value)) {
-      // The following behaviour is not consistent with what the Glean SDK does, but this is on purpose.
-      // On the Glean SDK we panic when we can't serialize the given,
-      // that is because this is a extremely unlikely situation for that environment.
-      //
-      // Since Glean.js will run on the browser, it is easy for a user to mess with the persisted data
-      // which makes this sort of errors plausible. That is why we choose to not panic and
-      // simply delete the corrupted data here.
       console.error(`Unexpected value found for metric ${metric.identifier}: ${JSON.stringify(value)}. Clearing.`);
       await store.delete([ping, metric.type, storageKey]);
       return;

src/glean.ts

@@ -47,6 +47,20 @@ class Glean {
   static set uploadEnabled(value: boolean) {
     Glean.instance._uploadEnabled = value;
   }
+
+  /**
+   * **Test-only API**
+   *
+   * Resets the Glean singleton to its initial state.
+   *
+   * TODO: Only allow this function to be called on test mode (depends on Bug 1682771).
+   */
+  static async resetGlean(): Promise<void> {
+    // Reset upload enabled state, not to inerfere with other tests.
+    Glean.uploadEnabled = true;
+    // Clear the database.
+    await Glean.db.clearAll();
+  }
 }
 
 export default Glean;

src/metrics/boolean.ts

@@ -7,13 +7,14 @@ import Glean from "glean";
 import { isBoolean } from "utils";
 
 export type BooleanMetricPayload = boolean;
+
 /**
  * Checks whether or not `v` is a valid boolean metric payload.
  *
  * @param v The value to verify.
  *
  * @returns A special Typescript value (which compiles down to a boolean)
- *          stating wether `v` is a valid boolean metric payload.
+ *          stating whether `v` is a valid boolean metric payload.
  */
 export function isBooleanMetricPayload(v: unknown): v is BooleanMetricPayload {
   return isBoolean(v);
@@ -38,7 +39,7 @@ class BooleanMetric extends Metric {
   }
 
   /**
-   * **Test-only API (exported for FFI purposes).**
+   * **Test-only API**
    *
    * Gets the currently stored value as a boolean.
    *

src/metrics/payload.ts

@@ -3,7 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 import { BooleanMetricPayload, isBooleanMetricPayload } from "metrics/boolean";
-import { isString } from "utils";
+import { StringMetricPayload, isStringMetricPayload } from "metrics/string";
 
 /**
  * Validates that a given value is the correct type of payload for a metric of a given type.
@@ -17,11 +17,15 @@ export function isMetricPayload(type: string, v: unknown): v is MetricPayload {
   switch (type) {
   case "boolean":
     return isBooleanMetricPayload(v);
+  case "string":
+    return isStringMetricPayload(v);
   default:
-    return isString(v);
+    return false;
   }
 }
 
 // Leaving the `string` as a valid metric payload here so that tests keep working for now.
-export type MetricPayload = BooleanMetricPayload | string;
+export type MetricPayload =
+  BooleanMetricPayload |
+  StringMetricPayload;
 

src/metrics/string.ts

@@ -0,0 +1,84 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+import Metric, { CommonMetricData } from "metrics";
+import Glean from "glean";
+import { isString } from "utils";
+
+export const MAX_LENGTH_VALUE = 100;
+
+export type StringMetricPayload = string;
+
+/**
+ * Checks whether or not `v` is a valid string metric payload.
+ *
+ * # Note
+ *
+ * Not only will this verify if `v` is a string,
+ * it will also check if its length is less than `MAX_LENGTH_VALUE`.
+ *
+ * @param v The value to verify.
+ *
+ * @returns A special Typescript value (which compiles down to a boolean)
+ *          stating wether `v` is a valid string metric payload.
+ */
+export function isStringMetricPayload(v: unknown): v is StringMetricPayload {
+  if (!isString(v)) {
+    return false;
+  }
+
+  if (v.length > MAX_LENGTH_VALUE) {
+    return false;
+  }
+
+  return true;
+}
+
+class StringMetric extends Metric {
+  constructor(meta: CommonMetricData) {
+    super("string", meta);
+  }
+
+  /**
+   * Sets to the specified string value.
+   *
+   * # Note
+   *
+   * Truncates the value if it is longer than `MAX_STRING_LENGTH` bytes
+   * and logs an error.
+   *
+   * @param value the value to set.
+   */
+  async set(value: string): Promise<void> {
+    if (!this.shouldRecord()) {
+      return;
+    }
+
+    if (value.length > MAX_LENGTH_VALUE) {
+      // TODO: record error once Bug 1682574 is resolved.
+      console.warn(`String ${value} is longer than ${MAX_LENGTH_VALUE} chars. Truncating.`);
+    }
+
+    await Glean.db.record(this, value.substring(0, MAX_LENGTH_VALUE));
+  }
+
+  /**
+   * **Test-only API**
+   *
+   * Gets the currently stored value as a string.
+   *
+   * This doesn't clear the stored value.
+   *
+   * TODO: Only allow this function to be called on test mode (depends on Bug 1682771).
+   *
+   * @param ping the ping from which we want to retrieve this metrics value from.
+   *
+   * @returns The value found in storage or `undefined` if nothing was found.
+   */
+  async testGetValue(ping: string): Promise<StringMetricPayload | undefined> {
+    return Glean.db.getMetric(ping, isStringMetricPayload, this);
+  }
+}
+
+export default StringMetric;

src/storage/index.ts

@@ -39,7 +39,7 @@ export interface StorageObject {
  * @param v The value to verify
  *
  * @returns A special Typescript value (which compiles down to a boolean)
- *          stating wether `v` is a valid StorageValue.
+ *          stating whether `v` is a valid StorageValue.
  */
 export function isStorageValue(v: unknown): v is StorageValue {
   if (isUndefined(v) || isString(v) || isBoolean(v)) {

src/utils.ts

@@ -8,7 +8,7 @@
  * @param v The value to verify.
  *
  * @returns A special Typescript value (which compiles down to a boolean)
- *          stating wether `v` is a valid data object.
+ *          stating whether `v` is a valid data object.
  */
 export function isObject(v: unknown): v is Record<string | number | symbol, unknown> {
   return (typeof v === "object" && v !== null  && v.constructor === Object);
@@ -20,7 +20,7 @@ export function isObject(v: unknown): v is Record<string | number | symbol, unkn
  * @param v The value to verify.
  *
  * @returns A special Typescript value (which compiles down to a boolean)
- *          stating wether `v` is undefined.
+ *          stating whether `v` is undefined.
  */
 export function isUndefined(v: unknown): v is undefined {
   return typeof v === "undefined";
@@ -32,7 +32,7 @@ export function isUndefined(v: unknown): v is undefined {
  * @param v The value to verify.
  *
  * @returns A special Typescript value (which compiles down to a boolean)
- *          stating wether `v` is a string.
+ *          stating whether `v` is a string.
  */
 export function isString(v: unknown): v is string {
   return (typeof v === "string" || (typeof v === "object" && v !== null  && v.constructor === String));
@@ -44,7 +44,7 @@ export function isString(v: unknown): v is string {
  * @param v The value to verify.
  *
  * @returns A special Typescript value (which compiles down to a boolean)
- *          stating wether `v` is a boolean.
+ *          stating whether `v` is a boolean.
  */
 export function isBoolean(v: unknown): v is string {
   return (typeof v === "boolean" || (typeof v === "object" && v !== null && v.constructor === Boolean));