From 14694fdf04c4d00a351809e7cf7f67a9022bfe62 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= <ivarconr@gmail.com>
Date: Fri, 22 Apr 2022 13:52:27 +0200
Subject: [PATCH] fix: allow missing instanceId in client metrics (#1522)

* fix: allow missing instanceId in client metrics

* fix: remove ts-ignore
---
 src/lib/routes/client-api/register.test.ts    | 16 +++-
 .../services/client-metrics/schema.test.ts    | 95 +++++++++++++++++++
 src/lib/services/client-metrics/schema.ts     |  4 +-
 3 files changed, 112 insertions(+), 3 deletions(-)
 create mode 100644 src/lib/services/client-metrics/schema.test.ts

diff --git a/src/lib/routes/client-api/register.test.ts b/src/lib/routes/client-api/register.test.ts
index 49e55da352..3cf08b7029 100644
--- a/src/lib/routes/client-api/register.test.ts
+++ b/src/lib/routes/client-api/register.test.ts
@@ -84,7 +84,7 @@ test('should require strategies field', () => {
         .expect(400);
 });
 
-test('should allow an empty instanceId field', () => {
+test('should allow an no instanceId field', () => {
     expect.assertions(0);
     return request
         .post('/api/client/register')
@@ -96,3 +96,17 @@ test('should allow an empty instanceId field', () => {
         })
         .expect(202);
 });
+
+test('should allow an empty instanceId field', () => {
+    expect.assertions(0);
+    return request
+        .post('/api/client/register')
+        .send({
+            appName: 'demo',
+            instanceId: '',
+            strategies: ['default'],
+            started: Date.now(),
+            interval: 10,
+        })
+        .expect(202);
+});
diff --git a/src/lib/services/client-metrics/schema.test.ts b/src/lib/services/client-metrics/schema.test.ts
new file mode 100644
index 0000000000..1a97efc95d
--- /dev/null
+++ b/src/lib/services/client-metrics/schema.test.ts
@@ -0,0 +1,95 @@
+import { clientRegisterSchema, clientMetricsSchema } from './schema';
+
+test('clientRegisterSchema should allow empty ("") instanceId', () => {
+    const { value } = clientRegisterSchema.validate({
+        appName: 'test',
+        instanceId: '',
+        strategies: ['default'],
+        started: Date.now(),
+        interval: 100,
+    });
+    //@ts-ignore
+    expect(value.instanceId).toBe('default');
+});
+
+test('clientRegisterSchema should allow undefined instanceId', () => {
+    const { value } = clientRegisterSchema.validate({
+        appName: 'test',
+        strategies: ['default'],
+        started: Date.now(),
+        interval: 100,
+    });
+
+    expect(value.instanceId).toBe('default');
+});
+
+test('clientRegisterSchema should allow null instanceId', () => {
+    const { value } = clientRegisterSchema.validate({
+        appName: 'test',
+        instanceId: null,
+        strategies: ['default'],
+        started: Date.now(),
+        interval: 100,
+    });
+    expect(value.instanceId).toBe('default');
+});
+
+test('clientRegisterSchema should use instanceId', () => {
+    const { value } = clientRegisterSchema.validate({
+        appName: 'test',
+        instanceId: 'some',
+        strategies: ['default'],
+        started: Date.now(),
+        interval: 100,
+    });
+    expect(value.instanceId).toBe('some');
+});
+
+test('clientMetricsSchema should allow null instanceId', () => {
+    const { value } = clientMetricsSchema.validate({
+        appName: 'test',
+        instanceId: null,
+        bucket: {
+            started: Date.now(),
+            stopped: Date.now(),
+        },
+    });
+    expect(value.instanceId).toBe('default');
+});
+
+test('clientMetricsSchema should allow empty ("") instanceId', () => {
+    const { value } = clientMetricsSchema.validate({
+        appName: 'test',
+        instanceId: '',
+        bucket: {
+            started: Date.now(),
+            stopped: Date.now(),
+        },
+    });
+    expect(value.instanceId).toBe('default');
+});
+
+test('clientMetricsSchema should allow undefined instanceId', () => {
+    const { value } = clientMetricsSchema.validate({
+        appName: 'test',
+        bucket: {
+            started: Date.now(),
+            stopped: Date.now(),
+        },
+    });
+
+    expect(value.instanceId).toBe('default');
+});
+
+test('clientMetricsSchema should use instanceId', () => {
+    const { value } = clientMetricsSchema.validate({
+        appName: 'test',
+        instanceId: 'some',
+        bucket: {
+            started: Date.now(),
+            stopped: Date.now(),
+        },
+    });
+
+    expect(value.instanceId).toBe('some');
+});
diff --git a/src/lib/services/client-metrics/schema.ts b/src/lib/services/client-metrics/schema.ts
index 2458c734b4..edf8fce2e7 100644
--- a/src/lib/services/client-metrics/schema.ts
+++ b/src/lib/services/client-metrics/schema.ts
@@ -15,7 +15,7 @@ export const clientMetricsSchema = joi
     .keys({
         environment: joi.string().optional(),
         appName: joi.string().required(),
-        instanceId: joi.string().default('default'),
+        instanceId: joi.string().empty(['', null]).default('default'),
         bucket: joi
             .object()
             .required()
@@ -48,7 +48,7 @@ export const clientRegisterSchema = joi
     .options({ stripUnknown: true })
     .keys({
         appName: joi.string().required(),
-        instanceId: joi.string().default('default'),
+        instanceId: joi.string().empty(['', null]).default('default'),
         sdkVersion: joi.string().optional(),
         strategies: joi
             .array()