Bug 1679379 - Implement the boolean metric type

.eslintrc

@@ -20,7 +20,8 @@
     "no-debugger":  ["error"],
     "jsdoc/no-types": "off",
     "jsdoc/require-param-type": "off",
-    "jsdoc/require-returns-type": "off"
+    "jsdoc/require-returns-type": "off",
+    "jsdoc/check-param-names": ["error"]
   },
   "extends": [
     "plugin:jsdoc/recommended"

package.json

@@ -4,8 +4,8 @@
   "description": "An implementation of the Glean SDK, a modern cross-platform telemetry client, for Javascript environments.",
   "main": "./dist/glean.js",
   "scripts": {
-    "test": "npm run build:test-webext && ts-mocha tests/**/*.spec.ts --paths -p ./tsconfig.json  --timeout 0",
-    "test:debug": "ts-mocha tests/**/*.spec.ts --paths -p ./tsconfig.json --inspect-brk",
+    "test": "npm run build:test-webext && ts-mocha \"tests/**/*.spec.ts\" --paths -p ./tsconfig.json  --recursive --timeout 0",
+    "test:debug": "ts-mocha \"tests/**/*.spec.ts\" --paths -p ./tsconfig.json --recursive --inspect-brk",
     "lint": "eslint . --ext .ts,.js,.json",
     "fix": "eslint . --ext .ts,.js,.json --fix",
     "build:webext": "webpack --config webpack.config.webext.js --mode production",

src/database.ts

@@ -5,11 +5,12 @@
 import { StorageValue, Store } from "storage";
 import PersistentStore from "storage/persistent";
 import Metric, { Lifetime } from "metrics";
-import { isObject, isString, isUndefined } from "utils";
+import { MetricPayload, isMetricPayload } from "metrics/payload";
+import { isObject, isUndefined } from "utils";
 
 export interface PingPayload {
   [aMetricType: string]: {
-    [aMetricIdentifier: string]: string
+    [aMetricIdentifier: string]: MetricPayload
   }
 }
 
@@ -20,14 +21,14 @@ export interface PingPayload {
  *
  * @returns Whether or not `v` is a valid PingPayload.
  */
-export function isValidPingPayload(v: StorageValue): v is PingPayload {
+export function isValidPingPayload(v: unknown): v is PingPayload {
   if (isObject(v)) {
     // The root keys should all be metric types.
     for (const metricType in v) {
       const metrics = v[metricType];
       if (isObject(metrics)) {
         for (const metricIdentifier in metrics) {
-          if (!isString(metrics[metricIdentifier])) {
+          if (!isMetricPayload(metricType, metrics[metricIdentifier])) {
             return false;
           }
         }
@@ -98,16 +99,8 @@ class Database {
    * @param metric The metric to record to.
    * @param value The value we want to record to the given metric.
    */
-  async record(metric: Metric, value: string): Promise<void> {
-    if (metric.disabled) {
-      return;
-    }
-
-    const store = this._chooseStore(metric.lifetime);
-    const storageKey = metric.identifier;
-    for (const ping of metric.sendInPings) {
-      await store.update([ping, metric.type, storageKey], () => value);
-    }
+  async record(metric: Metric, value: MetricPayload): Promise<void> {
+    await this.transform(metric, () => value);
   }
 
   /**
@@ -117,17 +110,17 @@ class Database {
    * @param metric The metric to record to.
    * @param transformFn The transformation function to apply to the currently persisted value.
    */
-  async transform(metric: Metric, transformFn: (v?: string) => string): Promise<void> {
+  async transform(metric: Metric, transformFn: (v?: MetricPayload) => MetricPayload): Promise<void> {
     if (metric.disabled) {
       return;
     }
 
     const store = this._chooseStore(metric.lifetime);
     const storageKey = metric.identifier;
     for (const ping of metric.sendInPings) {
-      const finalTransformFn = (v: StorageValue): string => {
-        if (isObject(v)) {
-          throw new Error(`Unexpected value found for metric ${metric}: ${JSON.stringify(v)}.`);
+      const finalTransformFn = (v: StorageValue): Exclude<StorageValue, undefined> => {
+        if (!isUndefined(v) && !isMetricPayload(metric.type, v)) {
+          throw new Error(`Unexpected value found for metric ${metric.identifier}: ${JSON.stringify(v)}.`);
         }
         return transformFn(v);
       };
@@ -139,17 +132,29 @@ class Database {
    * Gets the persisted payload of a given metric in a given ping.
    *
    * @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 metric An object containing the information about the metric to retrieve.
    *
-   * @returns The string encoded payload persisted for the given metric,
+   * @returns The payload persisted for the given metric,
    *          `undefined` in case the metric has not been recorded yet.
    */
-  async getMetric(ping: string, metric: Metric): Promise<string | undefined> {
+  async getMetric<T>(
+    ping: string,
+    validateFn: (v: unknown) => v is T,
+    metric: Metric
+  ): Promise<T | undefined> {
     const store = this._chooseStore(metric.lifetime);
     const storageKey = metric.identifier;
     const value = await store.get([ping, metric.type, storageKey]);
-    if (isObject(value)) {
-      console.error(`Unexpected value found for metric ${metric}: ${JSON.stringify(value)}. Clearing.`);
+    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;
     } else {

src/glean.ts

@@ -0,0 +1,52 @@
+/* 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 Database from "database";
+import { isUndefined } from "utils";
+
+class Glean {
+  // The Glean singleton.
+  private static _instance?: Glean;
+
+  // The metrics database.
+  private _db: Database;
+  // Whether or not to record metrics.
+  private _uploadEnabled: boolean;
+
+  private constructor() {
+    if (!isUndefined(Glean._instance)) {
+      throw new Error(
+        `Tried to instantiate Glean through \`new\`.
+        Use Glean.instance instead to access the Glean singleton.`);
+    }
+
+    this._db = new Database();
+    // Temporarily setting this to true always, until Bug 1677444 is resolved.
+    this._uploadEnabled = true;
+  }
+
+  private static get instance(): Glean {
+    if (!Glean._instance) {
+      Glean._instance = new Glean();
+    }
+
+    return Glean._instance;
+  }
+
+
+  static get db(): Database {
+    return Glean.instance._db;
+  }
+
+  // TODO: Make the following functions `private` once Bug 1682769 is resolved.
+  static get uploadEnabled(): boolean {
+    return Glean.instance._uploadEnabled;
+  }
+
+  static set uploadEnabled(value: boolean) {
+    Glean.instance._uploadEnabled = value;
+  }
+}
+
+export default Glean;

src/index.ts

@@ -2,16 +2,6 @@
  * 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/. */
 
-// Importing this here just so the size increase will show on the PR comments,
-// once everything is implemented we remove it.
-import StorageWeak from "storage/weak";
-import StoragePersistent from "storage/persistent";
-// If we leave the above imports unused they will not be added to the final webpack bundle.
-console.log(
-  StorageWeak,
-  StoragePersistent
-);
-
 export = {
   /**
    * Initializes Glean.

src/metrics/boolean.ts

@@ -0,0 +1,58 @@
+/* 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 { 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.
+ */
+export function isBooleanMetricPayload(v: unknown): v is BooleanMetricPayload {
+  return isBoolean(v);
+}
+
+class BooleanMetric extends Metric {
+  constructor(meta: CommonMetricData) {
+    super("boolean", meta);
+  }
+
+  /**
+   * Sets to the specified boolean value.
+   *
+   * @param value the value to set.
+   */
+  async set(value: BooleanMetricPayload): Promise<void> {
+    if (!this.shouldRecord()) {
+      return;
+    }
+
+    await Glean.db.record(this, value);
+  }
+
+  /**
+   * **Test-only API (exported for FFI purposes).**
+   *
+   * Gets the currently stored value as a boolean.
+   *
+   * 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<BooleanMetricPayload | undefined> {
+    return Glean.db.getMetric(ping, isBooleanMetricPayload, this);
+  }
+}
+
+export default BooleanMetric;

src/metrics/index.ts

@@ -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 http://mozilla.org/MPL/2.0/. */
 
+import Glean from "glean";
+
 /**
  * An enum representing the possible metric lifetimes.
  */
@@ -17,7 +19,7 @@ export const enum Lifetime {
 /**
  * The common set of data shared across all different metric types.
  */
-interface CommonMetricData {
+export interface CommonMetricData {
   // The metric's name.
   readonly name: string,
   // The metric's category.
@@ -65,6 +67,15 @@ class Metric implements CommonMetricData {
       return this.name;
     }
   }
+
+  /**
+   * Verify if whether or not this metric instance should be recorded to a given Glean instance.
+   *
+   * @returns Whether or not this metric instance should be recorded.
+   */
+  shouldRecord(): boolean {
+    return (Glean.uploadEnabled && !this.disabled);
+  }
 }
 
 export default Metric;

src/metrics/payload.ts

@@ -0,0 +1,27 @@
+/* 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 { BooleanMetricPayload, isBooleanMetricPayload } from "metrics/boolean";
+import { isString } from "utils";
+
+/**
+ * Validates that a given value is the correct type of payload for a metric of a given type.
+ *
+ * @param type The type of the metric to validate
+ * @param v The value to verify
+ *
+ * @returns Whether or not `v` is of the correct type.
+ */
+export function isMetricPayload(type: string, v: unknown): v is MetricPayload {
+  switch (type) {
+  case "boolean":
+    return isBooleanMetricPayload(v);
+  default:
+    return isString(v);
+  }
+}
+
+// Leaving the `string` as a valid metric payload here so that tests keep working for now.
+export type MetricPayload = BooleanMetricPayload | string;
+

src/storage/index.ts

@@ -2,7 +2,7 @@
  * 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 { isString, isUndefined, isObject } from "utils";
+import { isString, isUndefined, isObject, isBoolean } from "utils";
 
 /**
  * The storage index in the ordered list of keys to navigate on the store
@@ -28,7 +28,7 @@ export type StorageIndex = string[];
 /**
  * The possible values to be retrievd from storage.
  */
-export type StorageValue = undefined | string | StorageObject;
+export type StorageValue = undefined | string | boolean | StorageObject;
 export interface StorageObject {
   [key: string]: StorageValue;
 }
@@ -42,10 +42,10 @@ export interface StorageObject {
  *          stating wether `v` is a valid StorageValue.
  */
 export function isStorageValue(v: unknown): v is StorageValue {
-  if (isUndefined(v) || isString(v)) {
+  if (isUndefined(v) || isString(v) || isBoolean(v)) {
     return true;
   }
-  
+
   if (isObject(v)) {
     if (Object.keys(v).length === 0) {
       return true;

src/storage/persistent/webext.ts

@@ -4,7 +4,7 @@
 
 import { Store, StorageIndex, StorageValue, StorageObject, isStorageValue } from "storage";
 import { updateNestedObject, getValueFromNestedObject, deleteKeyFromNestedObject } from "storage/utils";
-import { isString, isUndefined } from "utils";
+import { isObject } from "utils";
 
 type WebExtStoreQuery = { [x: string]: { [x: string]: null; } | null; };
 
@@ -75,7 +75,7 @@ class WebExtStore implements Store {
     }
 
     if (isStorageValue(response)) {
-      if (!isUndefined(response) && !isString(response)) {
+      if (isObject(response)) {
         return getValueFromNestedObject(response, [ this.rootKey, ...index ]);
       } else {
         return response;

src/utils.ts

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