From 8352e716ddac2779b9e38fb766cbcfcd67ee90d3 Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Thu, 27 Apr 2023 12:38:19 -0700 Subject: [PATCH] ArrayDataFrame: Convert to a simple utility function rather than dynamically loaded values (#67427) --- .betterer.results | 35 ++-- .../src/dataframe/ArrayDataFrame.test.ts | 59 +++---- .../src/dataframe/ArrayDataFrame.ts | 163 ++++++------------ .../src/dataframe/processDataFrame.test.ts | 3 +- .../src/dataframe/processDataFrame.ts | 14 +- packages/grafana-data/src/types/data.ts | 6 + .../src/vector/FunctionalVector.ts | 6 +- .../dashboard/utils/loadSnapshotData.ts | 4 +- .../datasource/azuremonitor/variables.test.ts | 32 ++-- 9 files changed, 128 insertions(+), 194 deletions(-) diff --git a/.betterer.results b/.betterer.results index 883cfa6551b..3e165bedf0d 100644 --- a/.betterer.results +++ b/.betterer.results @@ -7,12 +7,8 @@ exports[`better eslint`] = { value: `{ "packages/grafana-data/src/dataframe/ArrayDataFrame.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], - [0, 0, 0, "Unexpected any. Specify a different type.", "1"], - [0, 0, 0, "Unexpected any. Specify a different type.", "2"], - [0, 0, 0, "Unexpected any. Specify a different type.", "3"], - [0, 0, 0, "Unexpected any. Specify a different type.", "4"], - [0, 0, 0, "Do not use any type assertions.", "5"], - [0, 0, 0, "Unexpected any. Specify a different type.", "6"] + [0, 0, 0, "Do not use any type assertions.", "1"], + [0, 0, 0, "Unexpected any. Specify a different type.", "2"] ], "packages/grafana-data/src/dataframe/CircularDataFrame.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"] @@ -73,29 +69,24 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "1"], [0, 0, 0, "Do not use any type assertions.", "2"], [0, 0, 0, "Do not use any type assertions.", "3"], - [0, 0, 0, "Unexpected any. Specify a different type.", "4"], + [0, 0, 0, "Do not use any type assertions.", "4"], [0, 0, 0, "Do not use any type assertions.", "5"], [0, 0, 0, "Do not use any type assertions.", "6"], - [0, 0, 0, "Do not use any type assertions.", "7"], - [0, 0, 0, "Unexpected any. Specify a different type.", "8"], + [0, 0, 0, "Unexpected any. Specify a different type.", "7"], + [0, 0, 0, "Do not use any type assertions.", "8"], [0, 0, 0, "Do not use any type assertions.", "9"], [0, 0, 0, "Do not use any type assertions.", "10"], - [0, 0, 0, "Do not use any type assertions.", "11"], - [0, 0, 0, "Unexpected any. Specify a different type.", "12"], - [0, 0, 0, "Do not use any type assertions.", "13"], - [0, 0, 0, "Unexpected any. Specify a different type.", "14"], - [0, 0, 0, "Do not use any type assertions.", "15"], - [0, 0, 0, "Unexpected any. Specify a different type.", "16"], + [0, 0, 0, "Unexpected any. Specify a different type.", "11"], + [0, 0, 0, "Do not use any type assertions.", "12"], + [0, 0, 0, "Unexpected any. Specify a different type.", "13"], + [0, 0, 0, "Do not use any type assertions.", "14"], + [0, 0, 0, "Unexpected any. Specify a different type.", "15"], + [0, 0, 0, "Do not use any type assertions.", "16"], [0, 0, 0, "Do not use any type assertions.", "17"], [0, 0, 0, "Do not use any type assertions.", "18"], - [0, 0, 0, "Unexpected any. Specify a different type.", "19"], + [0, 0, 0, "Do not use any type assertions.", "19"], [0, 0, 0, "Do not use any type assertions.", "20"], - [0, 0, 0, "Unexpected any. Specify a different type.", "21"], - [0, 0, 0, "Do not use any type assertions.", "22"], - [0, 0, 0, "Do not use any type assertions.", "23"], - [0, 0, 0, "Do not use any type assertions.", "24"], - [0, 0, 0, "Unexpected any. Specify a different type.", "25"], - [0, 0, 0, "Unexpected any. Specify a different type.", "26"] + [0, 0, 0, "Do not use any type assertions.", "21"] ], "packages/grafana-data/src/datetime/datemath.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], diff --git a/packages/grafana-data/src/dataframe/ArrayDataFrame.test.ts b/packages/grafana-data/src/dataframe/ArrayDataFrame.test.ts index e9ae03e3dd8..75222ede2d0 100644 --- a/packages/grafana-data/src/dataframe/ArrayDataFrame.test.ts +++ b/packages/grafana-data/src/dataframe/ArrayDataFrame.test.ts @@ -1,6 +1,6 @@ -import { FieldType, DataFrame } from '../types'; +import { DataFrame } from '../types'; -import { ArrayDataFrame } from './ArrayDataFrame'; +import { ArrayDataFrame, arrayToDataFrame } from './ArrayDataFrame'; import { toDataFrameDTO } from './processDataFrame'; describe('Array DataFrame', () => { @@ -15,30 +15,9 @@ describe('Array DataFrame', () => { const frame = new ArrayDataFrame(input); frame.name = 'Hello'; frame.refId = 'Z'; - frame.setFieldType('phantom', FieldType.string, (v) => '🦥'); const field = frame.fields.find((f) => f.name === 'value'); field!.config.unit = 'kwh'; - test('Should support functional methods', () => { - const expectedNames = input.map((row) => row.name); - - // Check map - expect(frame.map((row) => row.name)).toEqual(expectedNames); - expect(frame[0].name).toEqual(input[0].name); - - let names: string[] = []; - for (const row of frame) { - names.push(row.name); - } - expect(names).toEqual(expectedNames); - - names = []; - frame.forEach((row) => { - names.push(row.name); - }); - expect(names).toEqual(expectedNames); - }); - test('Should convert an array of objects to a dataframe', () => { expect(toDataFrameDTO(frame)).toMatchInlineSnapshot(` { @@ -84,19 +63,6 @@ describe('Array DataFrame', () => { 1100, ], }, - { - "config": {}, - "labels": undefined, - "name": "phantom", - "type": "string", - "values": [ - "🦥", - "🦥", - "🦥", - "🦥", - "🦥", - ], - }, ], "meta": undefined, "name": "Hello", @@ -114,4 +80,25 @@ describe('Array DataFrame', () => { expect(copy.length).toEqual(frame.length); expect(copy.length).toEqual(input.length); }); + + test('Handles any array input', () => { + const f = arrayToDataFrame([1, 2, 3]); + expect(f).toMatchInlineSnapshot(` + { + "fields": [ + { + "config": {}, + "name": "Value", + "type": "number", + "values": [ + 1, + 2, + 3, + ], + }, + ], + "length": 3, + } + `); + }); }); diff --git a/packages/grafana-data/src/dataframe/ArrayDataFrame.ts b/packages/grafana-data/src/dataframe/ArrayDataFrame.ts index fcf665a6282..ae8b821175e 100644 --- a/packages/grafana-data/src/dataframe/ArrayDataFrame.ts +++ b/packages/grafana-data/src/dataframe/ArrayDataFrame.ts @@ -1,118 +1,69 @@ -import { makeArrayIndexableVector, QueryResultMeta } from '../types'; -import { Field, FieldType, DataFrame } from '../types/dataFrame'; -import { FunctionalVector } from '../vector/FunctionalVector'; -import { vectorToArray } from '../vector/vectorToArray'; +import { QueryResultMeta } from '../types'; +import { Field, FieldType, DataFrame, TIME_SERIES_VALUE_FIELD_NAME } from '../types/dataFrame'; -import { guessFieldTypeFromNameAndValue, toDataFrameDTO } from './processDataFrame'; - -/** @public */ -export type ValueConverter = (val: unknown) => T; - -const NOOP: ValueConverter = (v) => v; - -class ArrayPropertyVector extends FunctionalVector { - converter = NOOP; - - constructor(private source: any[], private prop: string) { - super(); - return makeArrayIndexableVector(this); - } - - get length(): number { - return this.source.length; - } - - get(index: number): T { - return this.converter(this.source[index][this.prop]); - } - - toArray(): T[] { - return vectorToArray(this); - } - - toJSON(): T[] { - return vectorToArray(this); - } -} +import { guessFieldTypeForField } from './processDataFrame'; /** * The ArrayDataFrame takes an array of objects and presents it as a DataFrame * - * @alpha + * @deprecated use arrayToDataFrame */ -export class ArrayDataFrame extends FunctionalVector implements DataFrame { +export class ArrayDataFrame implements DataFrame { + fields: Field[] = []; + length = 0; name?: string; refId?: string; meta?: QueryResultMeta; - fields: Field[] = []; - length = 0; - - constructor(private source: T[], names?: string[]) { - super(); - - this.length = source.length; - const first: any = source.length ? source[0] : {}; - if (names) { - this.fields = names.map((name) => { - return { - name, - type: guessFieldTypeFromNameAndValue(name, first[name]), - config: {}, - values: new ArrayPropertyVector(source, name), - }; - }); - } else { - this.setFieldsFromObject(first); - } - return makeArrayIndexableVector(this); - } - - /** - * Add a field for each property in the object. This will guess the type - */ - setFieldsFromObject(obj: Record) { - this.fields = Object.keys(obj).map((name) => { - return { - name, - type: guessFieldTypeFromNameAndValue(name, obj[name]), - config: {}, - values: new ArrayPropertyVector(this.source, name), - }; - }); - } - - /** - * Configure how the object property is passed to the data frame - */ - setFieldType(name: string, type: FieldType, converter?: ValueConverter): Field { - let field = this.fields.find((f) => f.name === name); - if (field) { - field.type = type; - } else { - field = { - name, - type, - config: {}, - values: new ArrayPropertyVector(this.source, name), - }; - this.fields.push(field); - } - (field.values as any).converter = converter ?? NOOP; - return field; - } - - /** - * Get an object with a property for each field in the DataFrame - */ - get(idx: number): T { - return this.source[idx]; - } - - /** - * The simplified JSON values used in JSON.stringify() - */ - toJSON() { - return toDataFrameDTO(this); + constructor(source: T[], names?: string[]) { + return arrayToDataFrame(source, names) as ArrayDataFrame; // returns a standard DataFrame } } + +/** + * arrayToDataFrame will convert any array into a DataFrame + * + * @public + */ +export function arrayToDataFrame(source: any[], names?: string[]): DataFrame { + const df: DataFrame = { + fields: [], + length: source.length, + }; + if (!source?.length) { + return df; + } + + if (names) { + for (const name of names) { + df.fields.push( + makeFieldFromValues( + name, + source.map((v) => v[name]) + ) + ); + } + return df; + } + + const first = source.find((v) => v != null); // first not null|undefined + if (first != null) { + if (typeof first === 'object') { + df.fields = Object.keys(first).map((name) => { + return makeFieldFromValues( + name, + source.map((v) => v[name]) + ); + }); + } else { + df.fields.push(makeFieldFromValues(TIME_SERIES_VALUE_FIELD_NAME, source)); + } + } + return df; +} + +function makeFieldFromValues(name: string, values: unknown[]): Field { + const f = { name, config: {}, values, type: FieldType.other }; + f.type = guessFieldTypeForField(f) ?? FieldType.other; + return f; +} diff --git a/packages/grafana-data/src/dataframe/processDataFrame.test.ts b/packages/grafana-data/src/dataframe/processDataFrame.test.ts index 9357245127f..a6906452f7c 100644 --- a/packages/grafana-data/src/dataframe/processDataFrame.test.ts +++ b/packages/grafana-data/src/dataframe/processDataFrame.test.ts @@ -83,10 +83,9 @@ describe('toDataFrame', () => { { a: 1, b: 2 }, { a: 3, b: 4 }, ]; - const array = new ArrayDataFrame(orig); + const array = new ArrayDataFrame(orig); // will return a simple DataFrame const frame = toDataFrame(array); expect(frame).toEqual(array); - expect(frame instanceof ArrayDataFrame).toEqual(true); expect(frame.length).toEqual(orig.length); expect(frame.fields.map((f) => f.name)).toEqual(['a', 'b']); }); diff --git a/packages/grafana-data/src/dataframe/processDataFrame.ts b/packages/grafana-data/src/dataframe/processDataFrame.ts index 5fad1214d9e..25047ee86db 100644 --- a/packages/grafana-data/src/dataframe/processDataFrame.ts +++ b/packages/grafana-data/src/dataframe/processDataFrame.ts @@ -25,7 +25,7 @@ import { GraphSeriesValue, } from '../types/index'; -import { ArrayDataFrame } from './ArrayDataFrame'; +import { arrayToDataFrame } from './ArrayDataFrame'; import { dataFrameFromJSON } from './DataFrameJSON'; import { MutableDataFrame } from './MutableDataFrame'; @@ -36,7 +36,7 @@ function convertTableToDataFrame(table: TableData): DataFrame { return { name: text?.length ? text : c, // rename 'text' to the 'name' field config: (disp || {}) as FieldConfig, - values: [] as any[], + values: [] as unknown[], type: type && Object.values(FieldType).includes(type as FieldType) ? (type as FieldType) : FieldType.other, }; }); @@ -339,7 +339,7 @@ export function toDataFrame(data: any): DataFrame { } if (Array.isArray(data)) { - return new ArrayDataFrame(data); + return arrayToDataFrame(data); } console.warn('Can not convert', data); @@ -350,7 +350,7 @@ export const toLegacyResponseData = (frame: DataFrame): TimeSeries | TableData = const { fields } = frame; const rowCount = frame.length; - const rows: any[][] = []; + const rows: unknown[][] = []; if (fields.length === 2) { const { timeField, timeIndex } = getTimeField(frame); @@ -379,7 +379,7 @@ export const toLegacyResponseData = (frame: DataFrame): TimeSeries | TableData = } for (let i = 0; i < rowCount; i++) { - const row: any[] = []; + const row: unknown[] = []; for (let j = 0; j < fields.length; j++) { row.push(fields[j].values[i]); } @@ -460,8 +460,8 @@ export function reverseDataFrame(data: DataFrame): DataFrame { /** * Wrapper to get an array from each field value */ -export function getDataFrameRow(data: DataFrame, row: number): any[] { - const values: any[] = []; +export function getDataFrameRow(data: DataFrame, row: number): unknown[] { + const values: unknown[] = []; for (const field of data.fields) { values.push(field.values[row]); } diff --git a/packages/grafana-data/src/types/data.ts b/packages/grafana-data/src/types/data.ts index 4a24a248e45..067d4af8f92 100644 --- a/packages/grafana-data/src/types/data.ts +++ b/packages/grafana-data/src/types/data.ts @@ -146,6 +146,8 @@ export interface QueryResultBase { export interface Labels { [key: string]: string; } + +/** @deprecated this is a very old (pre Grafana 7 + DataFrame) representation for tabular data */ export interface Column { text: string; // For a Column, the 'text' is the field name filterable?: boolean; @@ -153,6 +155,7 @@ export interface Column { custom?: Record; } +/** @deprecated this is a very old (pre Grafana 7 + DataFrame) representation for tabular data */ export interface TableData extends QueryResultBase { name?: string; columns: Column[]; @@ -160,10 +163,13 @@ export interface TableData extends QueryResultBase { type?: string; } +/** @deprecated this is a very old (pre Grafana 7 + DataFrame) representation for tabular data */ export type TimeSeriesValue = number | null; +/** @deprecated this is a very old (pre Grafana 7 + DataFrame) representation for tabular data */ export type TimeSeriesPoints = TimeSeriesValue[][]; +/** @deprecated this is a very old (pre Grafana 7 + DataFrame) representation for tabular data */ export interface TimeSeries extends QueryResultBase { target: string; /** diff --git a/packages/grafana-data/src/vector/FunctionalVector.ts b/packages/grafana-data/src/vector/FunctionalVector.ts index 7ee154e60f5..62782fe718f 100644 --- a/packages/grafana-data/src/vector/FunctionalVector.ts +++ b/packages/grafana-data/src/vector/FunctionalVector.ts @@ -85,9 +85,6 @@ export abstract class FunctionalVector implements Vector { shift(): T | undefined { throw new Error('Method not implemented.'); } - slice(start?: number | undefined, end?: number | undefined): T[] { - throw new Error('Method not implemented.'); - } sort(compareFn?: ((a: T, b: T) => number) | undefined): this { throw new Error('Method not implemented.'); } @@ -121,6 +118,9 @@ export abstract class FunctionalVector implements Vector { // Delegated Array function -- these will not be efficient :grimmice: //-------------------------------------------------------------------------------- + slice(start?: number | undefined, end?: number | undefined): T[] { + return this.toArray().slice(start, end); + } indexOf(searchElement: T, fromIndex?: number | undefined): number { return this.toArray().indexOf(searchElement, fromIndex); } diff --git a/public/app/features/dashboard/utils/loadSnapshotData.ts b/public/app/features/dashboard/utils/loadSnapshotData.ts index e6ab56369e1..b0068246693 100644 --- a/public/app/features/dashboard/utils/loadSnapshotData.ts +++ b/public/app/features/dashboard/utils/loadSnapshotData.ts @@ -1,6 +1,6 @@ import { applyFieldOverrides, - ArrayDataFrame, + arrayToDataFrame, getDefaultTimeRange, getProcessedDataFrames, LoadingState, @@ -19,7 +19,7 @@ export function loadSnapshotData(panel: PanelModel, dashboard: DashboardModel): const worker = new SnapshotWorker(); const options = { dashboard, range: getDefaultTimeRange() }; const annotationEvents = worker.canWork(options) ? worker.getAnnotationsInSnapshot(dashboard, panel.id) : []; - const annotations = [new ArrayDataFrame(annotationEvents)]; + const annotations = [arrayToDataFrame(annotationEvents)]; const timeData = applyPanelTimeOverrides(panel, getTimeSrv().timeRange()); return { diff --git a/public/app/plugins/datasource/azuremonitor/variables.test.ts b/public/app/plugins/datasource/azuremonitor/variables.test.ts index 244e282bbc3..53aba537f5d 100644 --- a/public/app/plugins/datasource/azuremonitor/variables.test.ts +++ b/public/app/plugins/datasource/azuremonitor/variables.test.ts @@ -38,7 +38,7 @@ describe('VariableSupport', () => { ], } as DataQueryRequest; const result = await lastValueFrom(variableSupport.query(mockRequest)); - expect(result.data[0].source).toEqual(fakeSubscriptions); + expect(result.data[0].fields[0].values).toEqual(fakeSubscriptions); }); it('can fetch resourceGroups with a subscriptionId arg', async () => { @@ -62,7 +62,7 @@ describe('VariableSupport', () => { ], } as DataQueryRequest; const result = await lastValueFrom(variableSupport.query(mockRequest)); - expect(result.data[0].source).toEqual(expectedResults); + expect(result.data[0].fields[0].values).toEqual(expectedResults); }); it('can fetch metricNamespaces with a subscriptionId', async () => { @@ -87,7 +87,7 @@ describe('VariableSupport', () => { ], } as DataQueryRequest; const result = await lastValueFrom(variableSupport.query(mockRequest)); - expect(result.data[0].source).toEqual(expectedResults); + expect(result.data[0].fields[0].values).toEqual(expectedResults); }); it('can fetch resourceNames with a subscriptionId', async () => { @@ -113,7 +113,7 @@ describe('VariableSupport', () => { ], } as DataQueryRequest; const result = await lastValueFrom(variableSupport.query(mockRequest)); - expect(result.data[0].source).toEqual(expectedResults); + expect(result.data[0].fields[0].values).toEqual(expectedResults); }); it('can fetch a metricNamespace with a subscriptionId', async () => { @@ -140,7 +140,7 @@ describe('VariableSupport', () => { ], } as DataQueryRequest; const result = await lastValueFrom(variableSupport.query(mockRequest)); - expect(result.data[0].source).toEqual(expectedResults); + expect(result.data[0].fields[0].values).toEqual(expectedResults); }); it('can fetch metricNames with a subscriptionId', async () => { @@ -167,7 +167,7 @@ describe('VariableSupport', () => { ], } as DataQueryRequest; const result = await lastValueFrom(variableSupport.query(mockRequest)); - expect(result.data[0].source).toEqual(expectedResults); + expect(result.data[0].fields[0].values).toEqual(expectedResults); }); it('can fetch workspaces with a subscriptionId', async () => { @@ -191,7 +191,7 @@ describe('VariableSupport', () => { ], } as DataQueryRequest; const result = await lastValueFrom(variableSupport.query(mockRequest)); - expect(result.data[0].source).toEqual(expectedResults); + expect(result.data[0].fields[0].values).toEqual(expectedResults); }); it('can handle legacy string queries with a default subscription', async () => { @@ -213,7 +213,7 @@ describe('VariableSupport', () => { targets: ['Namespaces(resourceGroup)' as unknown as AzureMonitorQuery], } as DataQueryRequest; const result = await lastValueFrom(variableSupport.query(mockRequest)); - expect(result.data[0].source).toEqual(expectedResults); + expect(result.data[0].fields[0].values).toEqual(expectedResults); }); it('can handle legacy string queries', async () => { @@ -232,7 +232,7 @@ describe('VariableSupport', () => { targets: ['Namespaces(subscriptionId, resourceGroup)' as unknown as AzureMonitorQuery], } as DataQueryRequest; const result = await lastValueFrom(variableSupport.query(mockRequest)); - expect(result.data[0].source).toEqual(expectedResults); + expect(result.data[0].fields[0].values).toEqual(expectedResults); }); it('returns an empty array for unknown queries', async () => { @@ -301,7 +301,7 @@ describe('VariableSupport', () => { ], } as DataQueryRequest; const result = await lastValueFrom(variableSupport.query(mockRequest)); - expect(result.data[0].source).toEqual(expectedResults); + expect(result.data[0].fields[0].values).toEqual(expectedResults); }); it('passes on the query error for a log query', async () => { @@ -374,7 +374,7 @@ describe('VariableSupport', () => { ], } as DataQueryRequest; const result = await lastValueFrom(variableSupport.query(mockRequest)); - expect(result.data[0].source).toEqual(fakeSubscriptions); + expect(result.data[0].fields[0].values).toEqual(fakeSubscriptions); }); it('can fetch resourceGroups', async () => { @@ -394,7 +394,7 @@ describe('VariableSupport', () => { ], } as DataQueryRequest; const result = await lastValueFrom(variableSupport.query(mockRequest)); - expect(result.data[0].source).toEqual(expectedResults); + expect(result.data[0].fields[0].values).toEqual(expectedResults); }); it('returns no data if calling resourceGroups but the subscription is a template variable with no value', async () => { @@ -429,7 +429,7 @@ describe('VariableSupport', () => { ], } as DataQueryRequest; const result = await lastValueFrom(variableSupport.query(mockRequest)); - expect(result.data[0].source).toEqual(expectedResults); + expect(result.data[0].fields[0].values).toEqual(expectedResults); }); it('returns no data if calling namespaces but the subscription is a template variable with no value', async () => { @@ -464,7 +464,7 @@ describe('VariableSupport', () => { ], } as DataQueryRequest; const result = await lastValueFrom(variableSupport.query(mockRequest)); - expect(result.data[0].source).toEqual(expectedResults); + expect(result.data[0].fields[0].values).toEqual(expectedResults); }); it('returns no data if calling resourceNames but the subscription is a template variable with no value', async () => { @@ -502,7 +502,7 @@ describe('VariableSupport', () => { ], } as DataQueryRequest; const result = await lastValueFrom(variableSupport.query(mockRequest)); - expect(result.data[0].source).toEqual(expectedResults); + expect(result.data[0].fields[0].values).toEqual(expectedResults); }); it('returns no data if calling metric names but the subscription is a template variable with no value', async () => { @@ -540,7 +540,7 @@ describe('VariableSupport', () => { ], } as DataQueryRequest; const result = await lastValueFrom(variableSupport.query(mockRequest)); - expect(result.data[0].source).toEqual(expectedResults); + expect(result.data[0].fields[0].values).toEqual(expectedResults); }); }); });