From 1f55003db24d0db40109ed6fcb7b4990b1a8fa6d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?G=C3=A1bor=20Farkas?= <gabor.farkas@gmail.com>
Date: Thu, 13 Jul 2023 08:01:55 +0200
Subject: [PATCH] logs: make sure log-row-react-keys are always unique (#71279)

---
 .../app/features/logs/UniqueKeyMaker.test.ts  | 41 +++++++++++++++++++
 public/app/features/logs/UniqueKeyMaker.ts    | 25 +++++++++++
 .../app/features/logs/components/LogRows.tsx  |  7 +++-
 3 files changed, 71 insertions(+), 2 deletions(-)
 create mode 100644 public/app/features/logs/UniqueKeyMaker.test.ts
 create mode 100644 public/app/features/logs/UniqueKeyMaker.ts

diff --git a/public/app/features/logs/UniqueKeyMaker.test.ts b/public/app/features/logs/UniqueKeyMaker.test.ts
new file mode 100644
index 00000000000..2d5478c1ca3
--- /dev/null
+++ b/public/app/features/logs/UniqueKeyMaker.test.ts
@@ -0,0 +1,41 @@
+import { UniqueKeyMaker } from './UniqueKeyMaker';
+
+describe('UniqueKeyMaker', () => {
+  const expectKeys = (testData: Array<[string, string]>) => {
+    const k = new UniqueKeyMaker();
+    testData.forEach(([input, output]) => {
+      expect(k.getKey(input)).toBe(output);
+    });
+
+    // we also make a check that all the output-values are unique
+    const outputs = testData.map(([i, o]) => o);
+    const uniqueOutputLength = new Set(outputs).size;
+    expect(uniqueOutputLength).toBe(outputs.length);
+  };
+
+  it('should handle already unique keys', () => {
+    expectKeys([
+      ['one', 'k_one'],
+      ['two', 'k_two'],
+      ['three', 'k_three'],
+    ]);
+  });
+
+  it('should handle duplicate keys', () => {
+    expectKeys([
+      ['one', 'k_one'],
+      ['one', 'i_2'],
+      ['one', 'i_3'],
+    ]);
+  });
+
+  it('should handle a mix of unique and duplicate keys', () => {
+    expectKeys([
+      ['one', 'k_one'],
+      ['two', 'k_two'],
+      ['one', 'i_3'],
+      ['two', 'i_4'],
+      ['three', 'k_three'],
+    ]);
+  });
+});
diff --git a/public/app/features/logs/UniqueKeyMaker.ts b/public/app/features/logs/UniqueKeyMaker.ts
new file mode 100644
index 00000000000..0c274d847a3
--- /dev/null
+++ b/public/app/features/logs/UniqueKeyMaker.ts
@@ -0,0 +1,25 @@
+// this class generates react-keys that are guaranteed to be unique.
+// it will try to use the provided `maybeId`, but if that's a duplicate,
+// it will use an index-based key.
+// NOTE: it will always add a prefix to the string, this is necessary
+// to avoid the problem if your proposed key would conflict with
+// the index-based names
+export class UniqueKeyMaker {
+  seen: Set<string>;
+  count: number;
+  constructor() {
+    this.seen = new Set();
+    this.count = 0;
+  }
+
+  getKey(proposedKey: string) {
+    this.count += 1;
+    const maybeKey = `k_${proposedKey}`;
+    if (this.seen.has(maybeKey)) {
+      return `i_${this.count}`;
+    } else {
+      this.seen.add(maybeKey);
+      return maybeKey;
+    }
+  }
+}
diff --git a/public/app/features/logs/components/LogRows.tsx b/public/app/features/logs/components/LogRows.tsx
index 064eb4e9821..c1794c95caa 100644
--- a/public/app/features/logs/components/LogRows.tsx
+++ b/public/app/features/logs/components/LogRows.tsx
@@ -13,6 +13,7 @@ import {
 } from '@grafana/data';
 import { withTheme2, Themeable2 } from '@grafana/ui';
 
+import { UniqueKeyMaker } from '../UniqueKeyMaker';
 import { sortLogRows } from '../utils';
 
 //Components
@@ -122,13 +123,15 @@ class UnThemedLogRows extends PureComponent<Props, State> {
     // React profiler becomes unusable if we pass all rows to all rows and their labels, using getter instead
     const getRows = this.makeGetRows(orderedRows);
 
+    const keyMaker = new UniqueKeyMaker();
+
     return (
       <table className={styles.logsRowsTable}>
         <tbody>
           {hasData &&
             firstRows.map((row) => (
               <LogRow
-                key={row.uid}
+                key={keyMaker.getKey(row.uid)}
                 getRows={getRows}
                 row={row}
                 showDuplicates={showDuplicates}
@@ -148,7 +151,7 @@ class UnThemedLogRows extends PureComponent<Props, State> {
             renderAll &&
             lastRows.map((row) => (
               <LogRow
-                key={row.uid}
+                key={keyMaker.getKey(row.uid)}
                 getRows={getRows}
                 row={row}
                 showDuplicates={showDuplicates}