Expressions: More robust expression check (#65006)

More robust expression check
This commit is contained in:
ismail simsek 2023-03-22 14:02:20 +01:00 committed by GitHub
parent aa857e2a4f
commit 1328878ace
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 53 additions and 11 deletions

View File

@ -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<DataQuery, DataSourceJsonData> {
constructor(instanceSettings: DataSourceInstanceSettings<DataSourceJsonData>) {
@ -239,6 +244,16 @@ describe('DataSourceWithBackend', () => {
url: '/api/datasources/uid/abc/health',
});
});
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() {

View File

@ -47,7 +47,7 @@ export function isExpressionReference(ref?: DataSourceRef | string | null): bool
return false;
}
const v = typeof ref === 'string' ? ref : ref.type;
return v === ExpressionDatasourceRef.type || v === '-100'; // -100 was a legacy accident that should be removed
return v === ExpressionDatasourceRef.type || v === ExpressionDatasourceRef.name || v === '-100'; // -100 was a legacy accident that should be removed
}
export class HealthCheckError extends Error {

View File

@ -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] === '$') {

View File

@ -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', () => {