TablePanel: Prevents crash when data contains mixed data formats (#20202)

* Fix: Prevents crash when table receives mixed data formats
Fixes #20075

* Tests: Adds tests for data format reducers

* Refactor: Updates after PR comments

* Refactor: Missed a couple of places where filtering was needed

public/app/core/specs/table_model.test.ts

@@ -133,6 +133,18 @@ describe('mergeTables', () => {
     }),
   ];
 
+  const multipleTablesButWithMixedDataFormat = [
+    new TableModel({
+      type: 'table',
+      columns: [{ text: 'Time' }, { text: 'Label Key 1' }, { text: 'Value' }],
+      rows: [[time, 'Label Value 1', 42]],
+    }),
+    ({
+      target: 'series1',
+      datapoints: [[12.12, time], [14.44, time + 1]],
+    } as any) as TableModel,
+  ];
+
   it('should return the single table as is', () => {
     const table = mergeTablesIntoModel(new TableModel(), singleTable);
     expect(table.columns.length).toBe(3);
@@ -196,4 +208,24 @@ describe('mergeTables', () => {
     expect(table.rows[1][4]).toBeUndefined();
     expect(table.rows[1][5]).toBe(7);
   });
+
+  it('should not crash if tables array contain non table data', () => {
+    expect(() => mergeTablesIntoModel(new TableModel(), ...multipleTablesButWithMixedDataFormat)).not.toThrow();
+  });
+
+  it('should should return the single table as is if tables array also contains non table data', () => {
+    const table = mergeTablesIntoModel(new TableModel(), ...multipleTablesButWithMixedDataFormat);
+    expect(table.columns.length).toBe(3);
+    expect(table.columns[0].text).toBe('Time');
+    expect(table.columns[1].text).toBe('Label Key 1');
+    expect(table.columns[2].text).toBe('Value');
+  });
+
+  it('should return 1 row for a single table if tables array also contains non table data', () => {
+    const table = mergeTablesIntoModel(new TableModel(), ...multipleTablesButWithMixedDataFormat);
+    expect(table.rows.length).toBe(1);
+    expect(table.rows[0][0]).toBe(time);
+    expect(table.rows[0][1]).toBe('Label Value 1');
+    expect(table.rows[0][2]).toBe(42);
+  });
 });

public/app/core/table_model.ts

@@ -99,11 +99,14 @@ export function mergeTablesIntoModel(dst?: TableModel, ...tables: TableModel[]):
     return model;
   }
 
+  // Filter out any tables that are not of TableData format
+  const tableDataTables = tables.filter(table => !!table.columns);
+
   // Track column indexes of union: name -> index
   const columnNames: { [key: string]: any } = {};
 
   // Union of all non-value columns
-  const columnsUnion = tables.slice().reduce(
+  const columnsUnion = tableDataTables.slice().reduce(
     (acc, series) => {
       series.columns.forEach(col => {
         const { text } = col;
@@ -120,10 +123,10 @@ export function mergeTablesIntoModel(dst?: TableModel, ...tables: TableModel[]):
   // Map old column index to union index per series, e.g.,
   // given columnNames {A: 0, B: 1} and
   // data [{columns: [{ text: 'A' }]}, {columns: [{ text: 'B' }]}] => [[0], [1]]
-  const columnIndexMapper = tables.map(series => series.columns.map(col => columnNames[col.text]));
+  const columnIndexMapper = tableDataTables.map(series => series.columns.map(col => columnNames[col.text]));
 
   // Flatten rows of all series and adjust new column indexes
-  const flattenedRows = tables.reduce(
+  const flattenedRows = tableDataTables.reduce(
     (acc, series, seriesIndex) => {
       const mapper = columnIndexMapper[seriesIndex];
       series.rows.forEach(row => {

public/app/plugins/panel/table/specs/transformers.test.ts

@@ -1,4 +1,4 @@
-import { transformers, transformDataToTable } from '../transformers';
+import { tableDataFormatFilterer, timeSeriesFormatFilterer, transformDataToTable, transformers } from '../transformers';
 
 describe('when transforming time series table', () => {
   let table: any;
@@ -300,4 +300,224 @@ describe('when transforming time series table', () => {
       });
     });
   });
+
+  describe('given time series and table data', () => {
+    const time = new Date().getTime();
+    const data = [
+      {
+        target: 'series1',
+        datapoints: [[12.12, time], [14.44, time + 1]],
+      },
+      {
+        columns: [
+          {
+            type: 'time',
+            text: 'Time',
+          },
+          {
+            text: 'mean',
+          },
+        ],
+        type: 'table',
+        rows: [[time, 13.13], [time + 1, 26.26]],
+      },
+    ];
+
+    describe('timeseries_to_rows', () => {
+      const panel = {
+        transform: 'timeseries_to_rows',
+        sort: { col: 0, desc: true },
+      };
+
+      beforeEach(() => {
+        table = transformDataToTable(data, panel);
+      });
+
+      it('should return 2 rows', () => {
+        expect(table.rows.length).toBe(2);
+        expect(table.rows[0][1]).toBe('series1');
+        expect(table.rows[1][1]).toBe('series1');
+        expect(table.rows[0][2]).toBe(12.12);
+      });
+
+      it('should return 3 columns', () => {
+        expect(table.columns.length).toBe(3);
+        expect(table.columns[0].text).toBe('Time');
+        expect(table.columns[1].text).toBe('Metric');
+        expect(table.columns[2].text).toBe('Value');
+      });
+    });
+
+    describe('timeseries_to_columns', () => {
+      const panel = {
+        transform: 'timeseries_to_columns',
+      };
+
+      beforeEach(() => {
+        table = transformDataToTable(data, panel);
+      });
+
+      it('should return 2 columns', () => {
+        expect(table.columns.length).toBe(2);
+        expect(table.columns[0].text).toBe('Time');
+        expect(table.columns[1].text).toBe('series1');
+      });
+
+      it('should return 2 rows', () => {
+        expect(table.rows.length).toBe(2);
+        expect(table.rows[0][1]).toBe(12.12);
+      });
+
+      it('should be undefined when no value for timestamp', () => {
+        expect(table.rows[1][2]).toBe(undefined);
+      });
+    });
+
+    describe('timeseries_aggregations', () => {
+      const panel = {
+        transform: 'timeseries_aggregations',
+        sort: { col: 0, desc: true },
+        columns: [{ text: 'Max', value: 'max' }, { text: 'Min', value: 'min' }],
+      };
+
+      beforeEach(() => {
+        table = transformDataToTable(data, panel);
+      });
+
+      it('should return 1 row', () => {
+        expect(table.rows.length).toBe(1);
+        expect(table.rows[0][0]).toBe('series1');
+        expect(table.rows[0][1]).toBe(14.44);
+        expect(table.rows[0][2]).toBe(12.12);
+      });
+
+      it('should return 2 columns', () => {
+        expect(table.columns.length).toBe(3);
+        expect(table.columns[0].text).toBe('Metric');
+        expect(table.columns[1].text).toBe('Max');
+        expect(table.columns[2].text).toBe('Min');
+      });
+    });
+  });
+});
+
+describe('timeSeriesFormatFilterer', () => {
+  describe('when called with an object that contains datapoints property', () => {
+    it('then it should return same object in array', () => {
+      const data: any = { datapoints: [] };
+
+      const result = timeSeriesFormatFilterer(data);
+
+      expect(result).toEqual([data]);
+    });
+  });
+
+  describe('when called with an object that does not contain datapoints property', () => {
+    it('then it should return empty array', () => {
+      const data: any = { prop: [] };
+
+      const result = timeSeriesFormatFilterer(data);
+
+      expect(result).toEqual([]);
+    });
+  });
+
+  describe('when called with an array of series with both timeseries and table data', () => {
+    it('then it should return an array with timeseries', () => {
+      const time = new Date().getTime();
+      const data: any[] = [
+        {
+          target: 'series1',
+          datapoints: [[12.12, time], [14.44, time + 1]],
+        },
+        {
+          columns: [
+            {
+              type: 'time',
+              text: 'Time',
+            },
+            {
+              text: 'mean',
+            },
+          ],
+          type: 'table',
+          rows: [[time, 13.13], [time + 1, 26.26]],
+        },
+      ];
+
+      const result = timeSeriesFormatFilterer(data);
+
+      expect(result).toEqual([
+        {
+          target: 'series1',
+          datapoints: [[12.12, time], [14.44, time + 1]],
+        },
+      ]);
+    });
+  });
+});
+
+describe('tableDataFormatFilterer', () => {
+  describe('when called with an object that contains columns property', () => {
+    it('then it should return same object in array', () => {
+      const data: any = { columns: [] };
+
+      const result = tableDataFormatFilterer(data);
+
+      expect(result).toEqual([data]);
+    });
+  });
+
+  describe('when called with an object that does not contain columns property', () => {
+    it('then it should return empty array', () => {
+      const data: any = { prop: [] };
+
+      const result = tableDataFormatFilterer(data);
+
+      expect(result).toEqual([]);
+    });
+  });
+
+  describe('when called with an array of series with both timeseries and table data', () => {
+    it('then it should return an array with table data', () => {
+      const time = new Date().getTime();
+      const data: any[] = [
+        {
+          target: 'series1',
+          datapoints: [[12.12, time], [14.44, time + 1]],
+        },
+        {
+          columns: [
+            {
+              type: 'time',
+              text: 'Time',
+            },
+            {
+              text: 'mean',
+            },
+          ],
+          type: 'table',
+          rows: [[time, 13.13], [time + 1, 26.26]],
+        },
+      ];
+
+      const result = tableDataFormatFilterer(data);
+
+      expect(result).toEqual([
+        {
+          columns: [
+            {
+              type: 'time',
+              text: 'Time',
+            },
+            {
+              text: 'mean',
+            },
+          ],
+          type: 'table',
+          rows: [[time, 13.13], [time + 1, 26.26]],
+        },
+      ]);
+    });
+  });
 });

public/app/plugins/panel/table/transformers.ts

@@ -6,6 +6,33 @@ import { TableTransform } from './types';
 import { Column, TableData } from '@grafana/data';
 
 const transformers: { [key: string]: TableTransform } = {};
+export const timeSeriesFormatFilterer = (data: any): any[] => {
+  if (!Array.isArray(data)) {
+    return data.datapoints ? [data] : [];
+  }
+
+  return data.reduce((acc, series) => {
+    if (!series.datapoints) {
+      return acc;
+    }
+
+    return acc.concat(series);
+  }, []);
+};
+
+export const tableDataFormatFilterer = (data: any): any[] => {
+  if (!Array.isArray(data)) {
+    return data.columns ? [data] : [];
+  }
+
+  return data.reduce((acc, series) => {
+    if (!series.columns) {
+      return acc;
+    }
+
+    return acc.concat(series);
+  }, []);
+};
 
 transformers['timeseries_to_rows'] = {
   description: 'Time series to rows',
@@ -14,9 +41,10 @@ transformers['timeseries_to_rows'] = {
   },
   transform: (data, panel, model) => {
     model.columns = [{ text: 'Time', type: 'date' }, { text: 'Metric' }, { text: 'Value' }];
+    const filteredData = timeSeriesFormatFilterer(data);
 
-    for (let i = 0; i < data.length; i++) {
+    for (let i = 0; i < filteredData.length; i++) {
-      const series = data[i];
+      const series = filteredData[i];
       for (let y = 0; y < series.datapoints.length; y++) {
         const dp = series.datapoints[y];
         model.rows.push([dp[1], series.target, dp[0]]);
@@ -35,9 +63,10 @@ transformers['timeseries_to_columns'] = {
 
     // group by time
     const points: any = {};
+    const filteredData = timeSeriesFormatFilterer(data);
 
-    for (let i = 0; i < data.length; i++) {
+    for (let i = 0; i < filteredData.length; i++) {
-      const series = data[i];
+      const series = filteredData[i];
       model.columns.push({ text: series.target });
 
       for (let y = 0; y < series.datapoints.length; y++) {
@@ -57,7 +86,7 @@ transformers['timeseries_to_columns'] = {
       const point = points[time];
       const values = [point.time];
 
-      for (let i = 0; i < data.length; i++) {
+      for (let i = 0; i < filteredData.length; i++) {
         const value = point[i];
         values.push(value);
       }
@@ -87,10 +116,12 @@ transformers['timeseries_aggregations'] = {
       model.columns.push({ text: panel.columns[i].text });
     }
 
-    for (i = 0; i < data.length; i++) {
+    const filteredData = timeSeriesFormatFilterer(data);
+
+    for (i = 0; i < filteredData.length; i++) {
       const series = new TimeSeries({
-        datapoints: data[i].datapoints,
+        datapoints: filteredData[i].datapoints,
-        alias: data[i].target,
+        alias: filteredData[i].target,
       });
 
       series.getFlotPairs('connected');
@@ -139,11 +170,13 @@ transformers['table'] = {
       return [...data[0].columns];
     }
 
+    const filteredData = tableDataFormatFilterer(data);
+
     // Track column indexes: name -> index
     const columnNames: any = {};
 
     // Union of all columns
-    const columns = data.reduce((acc: Column[], series: TableData) => {
+    const columns = filteredData.reduce((acc: Column[], series: TableData) => {
       series.columns.forEach(col => {
         const { text } = col;
         if (columnNames[text] === undefined) {
@@ -160,7 +193,8 @@ transformers['table'] = {
     if (!data || data.length === 0) {
       return;
     }
-    const noTableIndex = _.findIndex(data, d => 'columns' in d && 'rows' in d);
+    const filteredData = tableDataFormatFilterer(data);
+    const noTableIndex = _.findIndex(filteredData, d => 'columns' in d && 'rows' in d);
     if (noTableIndex < 0) {
       throw {
         message: `Result of query #${String.fromCharCode(
@@ -169,7 +203,7 @@ transformers['table'] = {
       };
     }
 
-    mergeTablesIntoModel(model, ...data);
+    mergeTablesIntoModel(model, ...filteredData);
   },
 };