From 3f2b28975c08fe366c0036b539de85b8d62fdba1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20H=C3=A4ggmark?= Date: Wed, 9 Dec 2020 11:18:58 +0100 Subject: [PATCH] Variables: Fixes upgrade of legacy Prometheus queries (#29704) * Variables: Fixes upgrade of legacy queries * Chore: changes after PR comments --- .../datasource/DataSourceVariableEditor.tsx | 4 +- .../variables/datasource/actions.test.ts | 175 +++++++++--------- .../features/variables/datasource/actions.ts | 27 ++- .../app/features/variables/state/actions.ts | 50 ++--- .../state/onTimeRangeUpdated.test.ts | 7 +- 5 files changed, 146 insertions(+), 117 deletions(-) diff --git a/public/app/features/variables/datasource/DataSourceVariableEditor.tsx b/public/app/features/variables/datasource/DataSourceVariableEditor.tsx index 5a58f2d02b6..ce287290770 100644 --- a/public/app/features/variables/datasource/DataSourceVariableEditor.tsx +++ b/public/app/features/variables/datasource/DataSourceVariableEditor.tsx @@ -30,8 +30,8 @@ interface DispatchProps { type Props = OwnProps & ConnectedProps & DispatchProps; export class DataSourceVariableEditorUnConnected extends PureComponent { - async componentDidMount() { - await this.props.initDataSourceVariableEditor(); + componentDidMount() { + this.props.initDataSourceVariableEditor(); } onRegExChange = (event: ChangeEvent) => { diff --git a/public/app/features/variables/datasource/actions.test.ts b/public/app/features/variables/datasource/actions.test.ts index 93e3a6cfdfa..466e2ad5eb0 100644 --- a/public/app/features/variables/datasource/actions.test.ts +++ b/public/app/features/variables/datasource/actions.test.ts @@ -9,39 +9,47 @@ import { initDataSourceVariableEditor, updateDataSourceVariableOptions, } from './actions'; -import { DataSourcePluginMeta, DataSourceSelectItem } from '@grafana/data'; +import { DataSourceInstanceSettings, DataSourceJsonData, DataSourcePluginMeta } from '@grafana/data'; import { getMockPlugin } from '../../plugins/__mocks__/pluginMocks'; import { createDataSourceOptions } from './reducer'; -import { setCurrentVariableValue, addVariable } from '../state/sharedReducer'; +import { addVariable, setCurrentVariableValue } from '../state/sharedReducer'; import { changeVariableEditorExtended } from '../editor/reducer'; import { datasourceBuilder } from '../shared/testing/builders'; +interface Args { + sources?: DataSourceInstanceSettings[]; + query?: string; + regex?: string; +} + +function getTestContext({ sources = [], query, regex }: Args = {}) { + const getListMock = jest.fn().mockReturnValue(sources); + const getDatasourceSrvMock = jest.fn().mockReturnValue({ getList: getListMock }); + const dependencies: DataSourceVariableActionDependencies = { getDatasourceSrv: getDatasourceSrvMock }; + const datasource = datasourceBuilder() + .withId('0') + .withQuery(query) + .withRegEx(regex) + .build(); + + return { getListMock, getDatasourceSrvMock, dependencies, datasource }; +} + describe('data source actions', () => { variableAdapters.setInit(() => [createDataSourceVariableAdapter()]); describe('when updateDataSourceVariableOptions is dispatched', () => { describe('and there is no regex', () => { it('then the correct actions are dispatched', async () => { - const sources: DataSourceSelectItem[] = [ - { - name: 'first-name', - value: 'first-value', - meta: getMockPlugin({ name: 'mock-data-name', id: 'mock-data-id' }), - }, - { - name: 'second-name', - value: 'second-value', - meta: getMockPlugin({ name: 'mock-data-name', id: 'mock-data-id' }), - }, + const meta = getMockPlugin({ name: 'mock-data-name', id: 'mock-data-id' }); + const sources: DataSourceInstanceSettings[] = [ + getDataSourceInstanceSetting('first-name', meta), + getDataSourceInstanceSetting('second-name', meta), ]; - - const getMetricSourcesMock = jest.fn().mockResolvedValue(sources); - const getDatasourceSrvMock = jest.fn().mockReturnValue({ getMetricSources: getMetricSourcesMock }); - const dependencies: DataSourceVariableActionDependencies = { getDatasourceSrv: getDatasourceSrvMock }; - const datasource = datasourceBuilder() - .withId('0') - .withQuery('mock-data-id') - .build(); + const { datasource, dependencies, getListMock, getDatasourceSrvMock } = getTestContext({ + sources, + query: 'mock-data-id', + }); const tester = await reduxTester<{ templating: TemplatingState }>() .givenRootReducer(getRootReducer()) @@ -55,7 +63,16 @@ describe('data source actions', () => { await tester.thenDispatchedActionsShouldEqual( createDataSourceOptions( - toVariablePayload({ type: 'datasource', id: '0' }, { sources, regex: (undefined as unknown) as RegExp }) + toVariablePayload( + { type: 'datasource', id: '0' }, + { + sources: [ + { name: 'first-name', value: 'first-name', meta }, + { name: 'second-name', value: 'second-name', meta }, + ], + regex: (undefined as unknown) as RegExp, + } + ) ), setCurrentVariableValue( toVariablePayload( @@ -65,35 +82,26 @@ describe('data source actions', () => { ) ); - expect(getMetricSourcesMock).toHaveBeenCalledTimes(1); - expect(getMetricSourcesMock).toHaveBeenCalledWith({ skipVariables: true }); + expect(getListMock).toHaveBeenCalledTimes(1); + expect(getListMock).toHaveBeenCalledWith({ metrics: true, variables: false }); expect(getDatasourceSrvMock).toHaveBeenCalledTimes(1); }); }); describe('and there is a regex', () => { it('then the correct actions are dispatched', async () => { - const sources: DataSourceSelectItem[] = [ - { - name: 'first-name', - value: 'first-value', - meta: getMockPlugin({ name: 'mock-data-name', id: 'mock-data-id' }), - }, - { - name: 'second-name', - value: 'second-value', - meta: getMockPlugin({ name: 'mock-data-name', id: 'mock-data-id' }), - }, + const meta = getMockPlugin({ name: 'mock-data-name', id: 'mock-data-id' }); + const sources: DataSourceInstanceSettings[] = [ + getDataSourceInstanceSetting('first-name', meta), + getDataSourceInstanceSetting('second-name', meta), ]; - const getMetricSourcesMock = jest.fn().mockResolvedValue(sources); - const getDatasourceSrvMock = jest.fn().mockReturnValue({ getMetricSources: getMetricSourcesMock }); - const dependencies: DataSourceVariableActionDependencies = { getDatasourceSrv: getDatasourceSrvMock }; - const datasource = datasourceBuilder() - .withId('0') - .withQuery('mock-data-id') - .withRegEx('/.*(second-name).*/') - .build(); + const { datasource, dependencies, getListMock, getDatasourceSrvMock } = getTestContext({ + sources, + query: 'mock-data-id', + regex: '/.*(second-name).*/', + }); + const tester = await reduxTester<{ templating: TemplatingState }>() .givenRootReducer(getRootReducer()) .whenActionIsDispatched( @@ -106,7 +114,16 @@ describe('data source actions', () => { await tester.thenDispatchedActionsShouldEqual( createDataSourceOptions( - toVariablePayload({ type: 'datasource', id: '0' }, { sources, regex: /.*(second-name).*/ }) + toVariablePayload( + { type: 'datasource', id: '0' }, + { + sources: [ + { name: 'first-name', value: 'first-name', meta }, + { name: 'second-name', value: 'second-name', meta }, + ], + regex: /.*(second-name).*/, + } + ) ), setCurrentVariableValue( toVariablePayload( @@ -116,8 +133,8 @@ describe('data source actions', () => { ) ); - expect(getMetricSourcesMock).toHaveBeenCalledTimes(1); - expect(getMetricSourcesMock).toHaveBeenCalledWith({ skipVariables: true }); + expect(getListMock).toHaveBeenCalledTimes(1); + expect(getListMock).toHaveBeenCalledWith({ metrics: true, variables: false }); expect(getDatasourceSrvMock).toHaveBeenCalledTimes(1); }); }); @@ -125,49 +142,41 @@ describe('data source actions', () => { describe('when initDataSourceVariableEditor is dispatched', () => { it('then the correct actions are dispatched', async () => { - const sources: DataSourceSelectItem[] = [ - { - name: 'first-name', - value: 'first-value', - meta: getMockPlugin({ name: 'mock-data-name', id: 'mock-data-id' }), - }, - { - name: 'second-name', - value: 'second-value', - meta: getMockPlugin({ name: 'mock-data-name', id: 'mock-data-id' }), - }, - { - name: 'mixed-name', - value: 'mixed-value', - meta: getMockPlugin(({ - name: 'mixed-data-name', - id: 'mixed-data-id', - mixed: true, - } as unknown) as DataSourcePluginMeta), - }, + const meta = getMockPlugin({ name: 'mock-data-name', id: 'mock-data-id' }); + const sources: DataSourceInstanceSettings[] = [ + getDataSourceInstanceSetting('first-name', meta), + getDataSourceInstanceSetting('second-name', meta), ]; - const getMetricSourcesMock = jest.fn().mockResolvedValue(sources); - const getDatasourceSrvMock = jest.fn().mockReturnValue({ getMetricSources: getMetricSourcesMock }); - const dependencies: DataSourceVariableActionDependencies = { getDatasourceSrv: getDatasourceSrvMock }; + const { dependencies, getListMock, getDatasourceSrvMock } = getTestContext({ sources }); - const tester = await reduxTester<{ templating: TemplatingState }>() + await reduxTester<{ templating: TemplatingState }>() .givenRootReducer(getRootReducer()) - .whenAsyncActionIsDispatched(initDataSourceVariableEditor(dependencies)); + .whenActionIsDispatched(initDataSourceVariableEditor(dependencies)) + .thenDispatchedActionsShouldEqual( + changeVariableEditorExtended({ + propName: 'dataSourceTypes', + propValue: [ + { text: '', value: '' }, + { text: 'mock-data-name', value: 'mock-data-id' }, + ], + }) + ); - await tester.thenDispatchedActionsShouldEqual( - changeVariableEditorExtended({ - propName: 'dataSourceTypes', - propValue: [ - { text: '', value: '' }, - { text: 'mock-data-name', value: 'mock-data-id' }, - ], - }) - ); - - expect(getMetricSourcesMock).toHaveBeenCalledTimes(1); - expect(getMetricSourcesMock).toHaveBeenCalledWith(); + expect(getListMock).toHaveBeenCalledTimes(1); + expect(getListMock).toHaveBeenCalledWith({ metrics: true, variables: true }); expect(getDatasourceSrvMock).toHaveBeenCalledTimes(1); }); }); }); + +function getDataSourceInstanceSetting(name: string, meta: DataSourcePluginMeta): DataSourceInstanceSettings { + return { + id: 1, + uid: '', + type: '', + name, + meta, + jsonData: ({} as unknown) as DataSourceJsonData, + }; +} diff --git a/public/app/features/variables/datasource/actions.ts b/public/app/features/variables/datasource/actions.ts index 3c2e21151a4..1eadaffd53a 100644 --- a/public/app/features/variables/datasource/actions.ts +++ b/public/app/features/variables/datasource/actions.ts @@ -2,7 +2,7 @@ import { toVariablePayload, VariableIdentifier } from '../state/types'; import { ThunkResult } from '../../../types'; import { createDataSourceOptions } from './reducer'; import { validateVariableSelectionState } from '../state/actions'; -import { DataSourceSelectItem, stringToJsRegex } from '@grafana/data'; +import { DataSourceInstanceSettings, stringToJsRegex } from '@grafana/data'; import { getDatasourceSrv } from '../../plugins/datasource_srv'; import { getVariable } from '../state/selectors'; import { DataSourceVariableModel } from '../types'; @@ -18,7 +18,10 @@ export const updateDataSourceVariableOptions = ( identifier: VariableIdentifier, dependencies: DataSourceVariableActionDependencies = { getDatasourceSrv: getDatasourceSrv } ): ThunkResult => async (dispatch, getState) => { - const sources = await dependencies.getDatasourceSrv().getMetricSources({ skipVariables: true }); + const sources = dependencies + .getDatasourceSrv() + .getList({ metrics: true, variables: false }) + .map(toDataSourceSelectItem); const variableInState = getVariable(identifier.id, getState()); let regex; @@ -27,16 +30,18 @@ export const updateDataSourceVariableOptions = ( regex = stringToJsRegex(regex); } - await dispatch(createDataSourceOptions(toVariablePayload(identifier, { sources, regex }))); + dispatch(createDataSourceOptions(toVariablePayload(identifier, { sources, regex }))); await dispatch(validateVariableSelectionState(identifier)); }; export const initDataSourceVariableEditor = ( dependencies: DataSourceVariableActionDependencies = { getDatasourceSrv: getDatasourceSrv } -): ThunkResult => async dispatch => { - const dataSources: DataSourceSelectItem[] = await dependencies.getDatasourceSrv().getMetricSources(); - const filtered = dataSources.filter(ds => !ds.meta.mixed && ds.value !== null); - const dataSourceTypes = _(filtered) +): ThunkResult => dispatch => { + const dataSources = dependencies + .getDatasourceSrv() + .getList({ metrics: true, variables: true }) + .map(toDataSourceSelectItem); + const dataSourceTypes = _(dataSources) .uniqBy('meta.id') .map((ds: any) => { return { text: ds.meta.name, value: ds.meta.id }; @@ -52,3 +57,11 @@ export const initDataSourceVariableEditor = ( }) ); }; + +function toDataSourceSelectItem(setting: DataSourceInstanceSettings) { + return { + name: setting.name, + value: setting.name, + meta: setting.meta, + }; +} diff --git a/public/app/features/variables/state/actions.ts b/public/app/features/variables/state/actions.ts index f25d935f825..25e78d7f274 100644 --- a/public/app/features/variables/state/actions.ts +++ b/public/app/features/variables/state/actions.ts @@ -103,7 +103,6 @@ export const initDashboardTemplating = (list: VariableModel[]): ThunkResult> => { - return dispatch => { + return async dispatch => { dispatch(setCurrentVariableValue(toVariablePayload(identifier, { option: current }))); - return dispatch(variableUpdated(identifier, emitChanges)); + return await dispatch(variableUpdated(identifier, emitChanges)); }; }; @@ -459,13 +458,14 @@ export const variableUpdated = ( identifier: VariableIdentifier, emitChangeEvents: boolean ): ThunkResult> => { - return (dispatch, getState) => { + return async (dispatch, getState) => { const variableInState = getVariable(identifier.id, getState()); // if we're initializing variables ignore cascading update because we are in a boot up scenario if (getState().templating.transaction.status === TransactionStatus.Fetching) { if (getVariableRefresh(variableInState) === VariableRefresh.never) { // for variable types with updates that go the setValueFromUrl path in the update let's make sure their state is set to Done. + await dispatch(upgradeLegacyQueries(toVariableIdentifier(variableInState))); dispatch(completeVariableLoading(identifier)); } return Promise.resolve(); @@ -639,6 +639,7 @@ export const updateOptions = (identifier: VariableIdentifier, rethrow = false): const variableInState = getVariable(identifier.id, getState()); try { dispatch(variableStateFetching(toVariablePayload(variableInState))); + await dispatch(upgradeLegacyQueries(toVariableIdentifier(variableInState))); await variableAdapters.get(variableInState.type).updateOptions(variableInState); dispatch(completeVariableLoading(identifier)); } catch (error) { @@ -684,26 +685,31 @@ export function upgradeLegacyQueries( return; } - const datasource = await getDatasourceSrvFunc().get(variable.datasource ?? ''); + try { + const datasource = await getDatasourceSrvFunc().get(variable.datasource ?? ''); - if (hasLegacyVariableSupport(datasource)) { - return; + if (hasLegacyVariableSupport(datasource)) { + return; + } + + if (!hasStandardVariableSupport(datasource)) { + return; + } + + if (isDataQueryType(variable.query)) { + return; + } + + const query = { + refId: `${datasource.name}-${identifier.id}-Variable-Query`, + query: variable.query, + }; + + dispatch(changeVariableProp(toVariablePayload(identifier, { propName: 'query', propValue: query }))); + } catch (err) { + dispatch(notifyApp(createVariableErrorNotification('Failed to upgrade legacy queries', err))); + console.error(err); } - - if (!hasStandardVariableSupport(datasource)) { - return; - } - - if (isDataQueryType(variable.query)) { - return; - } - - const query = { - refId: `${datasource.name}-${identifier.id}-Variable-Query`, - query: variable.query, - }; - - dispatch(changeVariableProp(toVariablePayload(identifier, { propName: 'query', propValue: query }))); }; } diff --git a/public/app/features/variables/state/onTimeRangeUpdated.test.ts b/public/app/features/variables/state/onTimeRangeUpdated.test.ts index 1e74481d461..2098166a375 100644 --- a/public/app/features/variables/state/onTimeRangeUpdated.test.ts +++ b/public/app/features/variables/state/onTimeRangeUpdated.test.ts @@ -133,10 +133,11 @@ describe('when onTimeRangeUpdated is dispatched', () => { startRefreshMock, } = getTestContext(); - const tester = await reduxTester<{ templating: TemplatingState }>({ preloadedState }) + const base = await reduxTester<{ templating: TemplatingState }>({ preloadedState }) .givenRootReducer(getRootReducer()) - .whenActionIsDispatched(setOptionAsCurrent(toVariableIdentifier(interval), interval.options[0], false)) - .whenAsyncActionIsDispatched(onTimeRangeUpdated(range, dependencies), true); + .whenAsyncActionIsDispatched(setOptionAsCurrent(toVariableIdentifier(interval), interval.options[0], false)); + + const tester = await base.whenAsyncActionIsDispatched(onTimeRangeUpdated(range, dependencies), true); tester.thenDispatchedActionsShouldEqual( variableStateFetching(toVariablePayload({ type: 'interval', id: 'interval-0' })),