From c7689ddf69d596df9567ac717bf9cc8b85c93deb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= <ivarconr@gmail.com>
Date: Wed, 6 Oct 2021 14:50:43 +0200
Subject: [PATCH] fix: dedupe duplicates on batchInsert

---
 src/lib/db/client-metrics-store-v2.ts         | 30 ++++++++++++-------
 .../types/stores/client-metrics-store-v2.ts   |  2 +-
 .../client-metrics-store-v2.e2e.test.ts       | 22 ++++++++++++++
 3 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/src/lib/db/client-metrics-store-v2.ts b/src/lib/db/client-metrics-store-v2.ts
index c5e294f8e5..6318448760 100644
--- a/src/lib/db/client-metrics-store-v2.ts
+++ b/src/lib/db/client-metrics-store-v2.ts
@@ -79,21 +79,29 @@ export class ClientMetricsStoreV2 implements IClientMetricsStoreV2 {
         // Nothing to do!
     }
 
+    // this function will collapse metrics before sending it to the database.
     async batchInsertMetrics(metrics: IClientMetricsEnv[]): Promise<void> {
         const rows = metrics.map(toRow);
 
-        // Consider rewriting to SQL batch!
-        for (const row of rows) {
-            const insert = this.db<ClientMetricsEnvTable>(TABLE)
-                .insert(row)
-                .toQuery();
+        const batch = rows.reduce((prev, curr) => {
+            // eslint-disable-next-line prettier/prettier
+            const key = `${curr.feature_name}_${curr.app_name}_${curr.environment}_${curr.timestamp.getTime()}`;
+            if (prev[key]) {
+                prev[key].yes += curr.yes;
+                prev[key].no += curr.no;
+            } else {
+                prev[key] = curr;
+            }
+            return prev;
+        }, {});
 
-            const query = `${insert.toString()} ON CONFLICT (feature_name, app_name, environment, timestamp) 
-                DO UPDATE SET  
-                "yes" = "client_metrics_env"."yes" + ?, 
-                "no" = "client_metrics_env"."no" + ?`;
-            await this.db.raw(query, [row.yes, row.no]);
-        }
+        // Consider rewriting to SQL batch!
+        const insert = this.db<ClientMetricsEnvTable>(TABLE)
+            .insert(Object.values(batch))
+            .toQuery();
+
+        const query = `${insert.toString()} ON CONFLICT (feature_name, app_name, environment, timestamp) DO UPDATE SET "yes" = "client_metrics_env"."yes" + EXCLUDED.yes, "no" = "client_metrics_env"."no" + EXCLUDED.no`;
+        await this.db.raw(query);
     }
 
     async getMetricsForFeatureToggle(
diff --git a/src/lib/types/stores/client-metrics-store-v2.ts b/src/lib/types/stores/client-metrics-store-v2.ts
index 642ba63396..18ecba95f7 100644
--- a/src/lib/types/stores/client-metrics-store-v2.ts
+++ b/src/lib/types/stores/client-metrics-store-v2.ts
@@ -4,10 +4,10 @@ export interface IClientMetricsEnvKey {
     featureName: string;
     appName: string;
     environment: string;
+    timestamp: Date;
 }
 
 export interface IClientMetricsEnv extends IClientMetricsEnvKey {
-    timestamp: Date;
     yes: number;
     no: number;
 }
diff --git a/src/test/e2e/stores/client-metrics-store-v2.e2e.test.ts b/src/test/e2e/stores/client-metrics-store-v2.e2e.test.ts
index b9d8c6d9b8..746ebbed02 100644
--- a/src/test/e2e/stores/client-metrics-store-v2.e2e.test.ts
+++ b/src/test/e2e/stores/client-metrics-store-v2.e2e.test.ts
@@ -192,3 +192,25 @@ test('Should get toggle metrics', async () => {
     expect(savedMetrics[0].yes).toBe(4950);
     expect(savedMetrics[0].no).toBe(5050);
 });
+
+test('Should insert 1500 feature toggle metrics', async () => {
+    const metrics: IClientMetricsEnv[] = [];
+
+    const date = new Date();
+
+    for (let i = 0; i < 1500; i++) {
+        metrics.push({
+            featureName: `demo-${i}`,
+            appName: `web`,
+            environment: 'dev',
+            timestamp: date,
+            yes: 2,
+            no: 2,
+        });
+    }
+
+    await clientMetricsStore.batchInsertMetrics(metrics);
+    const savedMetrics = await clientMetricsStore.getAll();
+
+    expect(savedMetrics).toHaveLength(1500);
+});