From 13311b9fc8d2b1defa4ac1325ad05561bc483f57 Mon Sep 17 00:00:00 2001
From: Scott Bell <scott@traclabs.com>
Date: Mon, 23 Oct 2023 18:22:13 +0200
Subject: [PATCH] Prevent infinite loop when updating a table row in place
 (#7154)

* bump index on update row in place

* add test

* Removing problematic test

* spelling
---
 .../collections/TableRowCollection.js         | 35 ++++++++-------
 src/plugins/telemetryTable/pluginSpec.js      | 44 ++++++++++++++++---
 2 files changed, 56 insertions(+), 23 deletions(-)

diff --git a/src/plugins/telemetryTable/collections/TableRowCollection.js b/src/plugins/telemetryTable/collections/TableRowCollection.js
index a803bb1f1e..55732167df 100644
--- a/src/plugins/telemetryTable/collections/TableRowCollection.js
+++ b/src/plugins/telemetryTable/collections/TableRowCollection.js
@@ -153,40 +153,41 @@ define(['lodash', 'EventEmitter'], function (_, EventEmitter) {
       });
     }
 
-    mergeSortedRows(rows) {
+    mergeSortedRows(incomingRows) {
       const mergedRows = [];
-      let i = 0;
-      let j = 0;
+      let existingRowIndex = 0;
+      let incomingRowIndex = 0;
 
-      while (i < this.rows.length && j < rows.length) {
-        const existingRow = this.rows[i];
-        const incomingRow = rows[j];
+      while (existingRowIndex < this.rows.length && incomingRowIndex < incomingRows.length) {
+        const existingRow = this.rows[existingRowIndex];
+        const incomingRow = incomingRows[incomingRowIndex];
 
-        const index = this.getInPlaceUpdateIndex(incomingRow);
-        if (index > -1) {
-          this.updateRowInPlace(incomingRow, index);
+        const inPlaceIndex = this.getInPlaceUpdateIndex(incomingRow);
+        if (inPlaceIndex > -1) {
+          this.updateRowInPlace(incomingRow, inPlaceIndex);
+          incomingRowIndex++;
         } else {
           if (this.firstRowInSortOrder(existingRow, incomingRow) === existingRow) {
             mergedRows.push(existingRow);
-            i++;
+            existingRowIndex++;
           } else {
             mergedRows.push(incomingRow);
-            j++;
+            incomingRowIndex++;
           }
         }
       }
 
       // tail of existing rows is all that is left to merge
-      if (i < this.rows.length) {
-        for (i; i < this.rows.length; i++) {
-          mergedRows.push(this.rows[i]);
+      if (existingRowIndex < this.rows.length) {
+        for (existingRowIndex; existingRowIndex < this.rows.length; existingRowIndex++) {
+          mergedRows.push(this.rows[existingRowIndex]);
         }
       }
 
       // tail of incoming rows is all that is left to merge
-      if (j < rows.length) {
-        for (j; j < rows.length; j++) {
-          mergedRows.push(rows[j]);
+      if (incomingRowIndex < incomingRows.length) {
+        for (incomingRowIndex; incomingRowIndex < incomingRows.length; incomingRowIndex++) {
+          mergedRows.push(incomingRows[incomingRowIndex]);
         }
       }
 
diff --git a/src/plugins/telemetryTable/pluginSpec.js b/src/plugins/telemetryTable/pluginSpec.js
index eb15767e0b..7a11825868 100644
--- a/src/plugins/telemetryTable/pluginSpec.js
+++ b/src/plugins/telemetryTable/pluginSpec.js
@@ -49,7 +49,7 @@ describe('the plugin', () => {
   let tablePlugin;
   let element;
   let child;
-  let historicalProvider;
+  let historicalTelemetryProvider;
   let originalRouterPath;
   let unlistenConfigMutation;
 
@@ -61,12 +61,12 @@ describe('the plugin', () => {
     tablePlugin = new TablePlugin();
     openmct.install(tablePlugin);
 
-    historicalProvider = {
+    historicalTelemetryProvider = {
       request: () => {
         return Promise.resolve([]);
       }
     };
-    spyOn(openmct.telemetry, 'findRequestProvider').and.returnValue(historicalProvider);
+    spyOn(openmct.telemetry, 'findRequestProvider').and.returnValue(historicalTelemetryProvider);
 
     element = document.createElement('div');
     child = document.createElement('div');
@@ -137,11 +137,12 @@ describe('the plugin', () => {
     let tableView;
     let tableInstance;
     let mockClock;
+    let telemetryCallback;
 
     beforeEach(async () => {
       openmct.time.timeSystem('utc', {
         start: 0,
-        end: 4
+        end: 10
       });
 
       mockClock = jasmine.createSpyObj('clock', ['on', 'off', 'currentValue']);
@@ -151,7 +152,7 @@ describe('the plugin', () => {
       openmct.time.addClock(mockClock);
       openmct.time.clock('mockClock', {
         start: 0,
-        end: 4
+        end: 10
       });
 
       testTelemetryObject = {
@@ -214,7 +215,21 @@ describe('the plugin', () => {
         }
       ];
 
-      historicalProvider.request = () => Promise.resolve(testTelemetry);
+      historicalTelemetryProvider.request = () => {
+        return Promise.resolve(testTelemetry);
+      };
+
+      const realtimeTelemetryProvider = {
+        supportsSubscribe: () => true,
+        subscribe: (domainObject, passedCallback) => {
+          telemetryCallback = passedCallback;
+          return Promise.resolve(() => {});
+        }
+      };
+
+      spyOn(openmct.telemetry, 'findSubscriptionProvider').and.returnValue(
+        realtimeTelemetryProvider
+      );
 
       openmct.router.path = [testTelemetryObject];
 
@@ -256,6 +271,23 @@ describe('the plugin', () => {
       expect(rows.length).toBe(3);
     });
 
+    it('Adds a row in place when updating with existing telemetry', async () => {
+      let rows = element.querySelectorAll('table.c-telemetry-table__body tr');
+      await nextTick();
+      expect(rows.length).toBe(3);
+      // fire some telemetry
+      const newTelemetry = {
+        utc: 2,
+        'some-key': 'some-value 2',
+        'some-other-key': 'spacecraft'
+      };
+      spyOn(tableInstance.tableRows, 'getInPlaceUpdateIndex').and.returnValue(1);
+      spyOn(tableInstance.tableRows, 'updateRowInPlace').and.callThrough();
+      telemetryCallback(newTelemetry);
+
+      expect(tableInstance.tableRows.updateRowInPlace.calls.count()).toBeGreaterThan(0);
+    });
+
     it('Renders a column for every item in telemetry metadata', () => {
       let headers = element.querySelectorAll('span.c-telemetry-table__headers__label');
       expect(headers.length).toBe(4);