Fail validation of counter and quantity if value is not int

Author: brizental

glean/src/core/metrics/types/counter.ts

@@ -5,7 +5,7 @@
 import type { CommonMetricData } from "../index.js";
 import type { JSONValue } from "../../utils.js";
 import { MetricType } from "../index.js";
-import { isUndefined, isNumber } from "../../utils.js";
+import { isUndefined, isInteger } from "../../utils.js";
 import { Context } from "../../context.js";
 import { Metric } from "../metric.js";
 import { ErrorType, recordError, testGetNumRecordedErrors } from "../../error_recording.js";
@@ -16,7 +16,7 @@ export class CounterMetric extends Metric<number, number> {
   }
 
   validate(v: unknown): v is number {
-    if (!isNumber(v)) {
+    if (!isInteger(v)) {
       return false;
     }
 

glean/src/core/metrics/types/quantity.ts

@@ -4,7 +4,7 @@
 
 import type { CommonMetricData } from "../index.js";
 import { MetricType } from "../index.js";
-import { isNumber } from "../../utils.js";
+import { isInteger } from "../../utils.js";
 import { Context } from "../../context.js";
 import { Metric } from "../metric.js";
 import { ErrorType, recordError, testGetNumRecordedErrors } from "../../error_recording.js";
@@ -15,7 +15,7 @@ export class QuantityMetric extends Metric<number, number> {
   }
 
   validate(v: unknown): v is number {
-    if (!isNumber(v)) {
+    if (!isInteger(v)) {
       return false;
     }
 

glean/src/core/utils.ts

@@ -101,6 +101,21 @@ export function isNumber(v: unknown): v is number {
   return ((typeof v === "number" || (typeof v === "object" && v !== null && v.constructor === Number)) && !isNaN(v));
 }
 
+/**
+ * Checks whether or not `v` is an integer.
+ *
+ * @param v The value to verify.
+ *
+ * @returns A special Typescript value (which compiles down to a boolean)
+ *          stating whether `v` is a number.
+ */
+export function isInteger(v: unknown): v is number {
+  // Decided against using `Number.isInteger` here,
+  // because that fails if you pass it a number created through `new Number(...)`.
+  return isNumber(v) && v % 1 === 0;
+}
+
+
 /**
  * Generates a pipeline-friendly string
  * that replaces non alphanumeric characters with dashes.

glean/tests/core/utils.spec.ts

@@ -79,6 +79,20 @@ describe("utils", function() {
     assert.strictEqual(utils.isNumber(new Number(10)), true);
   });
 
+  it("isInteger validates correctly", function() {
+    // Invalid values
+    assert.strictEqual(utils.isInteger(undefined), false);
+    assert.strictEqual(utils.isInteger("10"), false);
+    assert.strictEqual(utils.isInteger({}), false);
+    assert.strictEqual(utils.isInteger(NaN), false);
+    assert.strictEqual(utils.isInteger(0.1), false);
+
+    // Valid values
+    assert.strictEqual(utils.isInteger(10), true);
+    assert.strictEqual(utils.isInteger(-10), true);
+    assert.strictEqual(utils.isInteger(new Number(10)), true);
+  });
+
   it("sanitizeApplicationId works correctly", function() {
     assert.strictEqual(utils.sanitizeApplicationId("org.mozilla.test-app"), "org-mozilla-test-app");
     assert.strictEqual(utils.sanitizeApplicationId("org.mozilla..test---app"), "org-mozilla-test-app");