From 138f2fef8d77911ac7783c4f5599c91b515173e6 Mon Sep 17 00:00:00 2001 From: ismail simsek Date: Wed, 22 Mar 2023 23:51:24 +0100 Subject: [PATCH] [v9.3.x] Expressions: More robust expression check (#65230) * Expressions: More robust expression check (#65006) More robust expression check (cherry picked from commit 1328878aceb37da7051c5685375e2ca32ef0a6b4) * Fix import * Fix tests --- .betterer.results | 20 ++++++++-------- .../src/utils/DataSourceWithBackend.test.ts | 23 +++++++++++++++---- .../src/utils/DataSourceWithBackend.ts | 4 ++-- public/app/features/plugins/datasource_srv.ts | 19 ++++++++++----- .../plugins/tests/datasource_srv.test.ts | 20 ++++++++++++++++ 5 files changed, 63 insertions(+), 23 deletions(-) diff --git a/.betterer.results b/.betterer.results index e46196375b4..d7809d85b8b 100644 --- a/.betterer.results +++ b/.betterer.results @@ -1038,19 +1038,17 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "2"] ], "packages/grafana-runtime/src/utils/DataSourceWithBackend.ts:5381": [ - [0, 0, 0, "Do not use any type assertions.", "0"], + [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, "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.", "2"], + [0, 0, 0, "Do not use any type assertions.", "3"], + [0, 0, 0, "Unexpected any. Specify a different type.", "4"], + [0, 0, 0, "Unexpected any. Specify a different type.", "5"], [0, 0, 0, "Unexpected any. Specify a different type.", "6"], - [0, 0, 0, "Unexpected any. Specify a different type.", "7"], - [0, 0, 0, "Unexpected any. Specify a different type.", "8"], - [0, 0, 0, "Do not use any type assertions.", "9"], - [0, 0, 0, "Do not use any type assertions.", "10"], - [0, 0, 0, "Unexpected any. Specify a different type.", "11"], - [0, 0, 0, "Do not use any type assertions.", "12"] + [0, 0, 0, "Do not use any type assertions.", "7"], + [0, 0, 0, "Do not use any type assertions.", "8"], + [0, 0, 0, "Unexpected any. Specify a different type.", "9"], + [0, 0, 0, "Do not use any type assertions.", "10"] ], "packages/grafana-runtime/src/utils/plugin.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"] diff --git a/packages/grafana-runtime/src/utils/DataSourceWithBackend.test.ts b/packages/grafana-runtime/src/utils/DataSourceWithBackend.test.ts index db7a43baa03..686806d01a3 100644 --- a/packages/grafana-runtime/src/utils/DataSourceWithBackend.test.ts +++ b/packages/grafana-runtime/src/utils/DataSourceWithBackend.test.ts @@ -2,16 +2,21 @@ import { of } from 'rxjs'; import { BackendSrv, BackendSrvRequest, FetchResponse } from 'src/services'; import { - DataSourceJsonData, DataQuery, - DataSourceInstanceSettings, DataQueryRequest, DataQueryResponseData, - MutableDataFrame, + DataSourceInstanceSettings, + DataSourceJsonData, DataSourceRef, + MutableDataFrame, } from '@grafana/data'; -import { DataSourceWithBackend, standardStreamOptionsProvider, toStreamingDataResponse } from './DataSourceWithBackend'; +import { + DataSourceWithBackend, + isExpressionReference, + standardStreamOptionsProvider, + toStreamingDataResponse, +} from './DataSourceWithBackend'; class MyDataSource extends DataSourceWithBackend { constructor(instanceSettings: DataSourceInstanceSettings) { @@ -182,6 +187,16 @@ describe('DataSourceWithBackend', () => { obs = toStreamingDataResponse(rsp, request, standardStreamOptionsProvider); expect(obs).toBeDefined(); }); + + describe('isExpressionReference', () => { + test('check all possible expression references', () => { + expect(isExpressionReference('__expr__')).toBeTruthy(); // New UID + expect(isExpressionReference('-100')).toBeTruthy(); // Legacy UID + expect(isExpressionReference('Expression')).toBeTruthy(); // Name + expect(isExpressionReference({ type: '__expr__' })).toBeTruthy(); + expect(isExpressionReference({ type: '-100' })).toBeTruthy(); + }); + }); }); function createMockDatasource() { diff --git a/packages/grafana-runtime/src/utils/DataSourceWithBackend.ts b/packages/grafana-runtime/src/utils/DataSourceWithBackend.ts index 1dd5cee01b6..a3cbfaaedbc 100644 --- a/packages/grafana-runtime/src/utils/DataSourceWithBackend.ts +++ b/packages/grafana-runtime/src/utils/DataSourceWithBackend.ts @@ -46,8 +46,8 @@ export function isExpressionReference(ref?: DataSourceRef | string | null): bool if (!ref) { return false; } - const v = (ref as any).type ?? ref; - return v === ExpressionDatasourceRef.type || v === '-100'; // -100 was a legacy accident that should be removed + const v = typeof ref === 'string' ? ref : ref.type; + return v === ExpressionDatasourceRef.type || v === ExpressionDatasourceRef.name || v === '-100'; // -100 was a legacy accident that should be removed } export class HealthCheckError extends Error { diff --git a/public/app/features/plugins/datasource_srv.ts b/public/app/features/plugins/datasource_srv.ts index 6483056f97b..fd02192626f 100644 --- a/public/app/features/plugins/datasource_srv.ts +++ b/public/app/features/plugins/datasource_srv.ts @@ -7,15 +7,15 @@ import { ScopedVars, } from '@grafana/data'; import { - GetDataSourceListFilters, DataSourceSrv as DataSourceService, - getDataSourceSrv as getDataSourceService, - TemplateSrv, - getTemplateSrv, - getLegacyAngularInjector, getBackendSrv, + GetDataSourceListFilters, + getDataSourceSrv as getDataSourceService, + getLegacyAngularInjector, + getTemplateSrv, + TemplateSrv, } from '@grafana/runtime'; -import { ExpressionDatasourceRef } from '@grafana/runtime/src/utils/DataSourceWithBackend'; +import { ExpressionDatasourceRef, isExpressionReference } from '@grafana/runtime/src/utils/DataSourceWithBackend'; import appEvents from 'app/core/app_events'; import config from 'app/core/config'; import { @@ -80,6 +80,13 @@ export class DatasourceSrv implements DataSourceService { return this.settingsMapByUid[this.defaultName] ?? this.settingsMapByName[this.defaultName]; } + // Expressions has a new UID as __expr__ See: https://github.com/grafana/grafana/pull/62510/ + // But we still have dashboards/panels with old expression UID (-100) + // To support both UIDs until we migrate them all to new one, this check is necessary + if (isExpressionReference(nameOrUid)) { + return expressionDatasource.instanceSettings; + } + // Complex logic to support template variable data source names // For this we just pick the current or first data source in the variable if (nameOrUid[0] === '$') { diff --git a/public/app/features/plugins/tests/datasource_srv.test.ts b/public/app/features/plugins/tests/datasource_srv.test.ts index 14517b7d3e8..d19635ba595 100644 --- a/public/app/features/plugins/tests/datasource_srv.test.ts +++ b/public/app/features/plugins/tests/datasource_srv.test.ts @@ -5,6 +5,7 @@ import { DataSourcePluginMeta, ScopedVar, } from '@grafana/data'; +import { ExpressionDatasourceRef } from '@grafana/runtime/src/utils/DataSourceWithBackend'; import { DatasourceSrv } from 'app/features/plugins/datasource_srv'; // Datasource variable $datasource with current value 'BBB' @@ -199,6 +200,25 @@ describe('datasource_srv', () => { } `); }); + + it('should return expression settings with either expression UIDs', () => { + const exprWithOldUID = dataSourceSrv.getInstanceSettings('-100'); + expect(exprWithOldUID?.name).toBe('Expression'); + expect(exprWithOldUID?.uid).toBe(ExpressionDatasourceRef.uid); + expect(exprWithOldUID?.type).toBe(ExpressionDatasourceRef.type); + + const exprWithNewUID = dataSourceSrv.getInstanceSettings('__expr__'); + expect(exprWithNewUID?.name).toBe('Expression'); + expect(exprWithNewUID?.uid).toBe(ExpressionDatasourceRef.uid); + expect(exprWithNewUID?.type).toBe(ExpressionDatasourceRef.type); + }); + + it('should return expression settings with expression name', () => { + const exprWithName = dataSourceSrv.getInstanceSettings('Expression'); + expect(exprWithName?.name).toBe(ExpressionDatasourceRef.name); + expect(exprWithName?.uid).toBe(ExpressionDatasourceRef.uid); + expect(exprWithName?.type).toBe(ExpressionDatasourceRef.type); + }); }); describe('when getting external metric sources', () => {