From b2447d3956924e5be999cab389ee9af4e6dc06cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Thu, 4 Nov 2021 21:37:03 +0100 Subject: [PATCH] DataSourceRef: Fixes migrations for mixed data source panels & queries and adds unit tests for data source ref migration (#41245) * DataSourceRef: Fixes migrations for mixed data source panels & queries and adds unit tests for data source ref migration * Fixing tests and migration logic a bit more * use helper functions * simplify migration logic * Fixing test Co-authored-by: Ryan McKinley --- packages/grafana-data/src/types/query.ts | 2 +- packages/grafana-data/src/utils/datasource.ts | 7 +- .../DashExportModal/DashboardExporter.test.ts | 6 +- .../dashboard/state/DashboardMigrator.test.ts | 76 +++++++++++++++++++ .../dashboard/state/DashboardMigrator.ts | 52 ++++++------- 5 files changed, 106 insertions(+), 37 deletions(-) diff --git a/packages/grafana-data/src/types/query.ts b/packages/grafana-data/src/types/query.ts index 69d7d59b801..1bb09d226d0 100644 --- a/packages/grafana-data/src/types/query.ts +++ b/packages/grafana-data/src/types/query.ts @@ -50,5 +50,5 @@ export interface DataQuery { * For mixed data sources the selected datasource is on the query level. * For non mixed scenarios this is undefined. */ - datasource?: DataSourceRef; + datasource?: DataSourceRef | null; } diff --git a/packages/grafana-data/src/utils/datasource.ts b/packages/grafana-data/src/utils/datasource.ts index 50543b1df32..31055d595d6 100644 --- a/packages/grafana-data/src/utils/datasource.ts +++ b/packages/grafana-data/src/utils/datasource.ts @@ -17,7 +17,12 @@ export function getDataSourceRef(ds: DataSourceInstanceSettings): DataSourceRef return { uid: ds.uid, type: ds.type }; } -function isDataSourceRef(ref: DataSourceRef | string | null): ref is DataSourceRef { +/** + * Returns true if the argument is a DataSourceRef + * + * @public + */ +export function isDataSourceRef(ref: DataSourceRef | string | null): ref is DataSourceRef { return typeof ref === 'object' && (typeof ref?.uid === 'string' || typeof ref?.uid === 'undefined'); } diff --git a/public/app/features/dashboard/components/DashExportModal/DashboardExporter.test.ts b/public/app/features/dashboard/components/DashExportModal/DashboardExporter.test.ts index b1f5307bec8..6602ebb6cbd 100644 --- a/public/app/features/dashboard/components/DashExportModal/DashboardExporter.test.ts +++ b/public/app/features/dashboard/components/DashExportModal/DashboardExporter.test.ts @@ -267,12 +267,8 @@ describe('given dashboard with repeated panels', () => { expect(element.kind).toBe(LibraryElementKind.Panel); expect(element.model).toEqual({ id: 17, - datasource: '${DS_OTHER2}', + datasource: { type: 'other2', uid: '$ds' }, type: 'graph', - fieldConfig: { - defaults: {}, - overrides: [], - }, }); }); diff --git a/public/app/features/dashboard/state/DashboardMigrator.test.ts b/public/app/features/dashboard/state/DashboardMigrator.test.ts index 5846b83e847..d4e34e7fcbf 100644 --- a/public/app/features/dashboard/state/DashboardMigrator.test.ts +++ b/public/app/features/dashboard/state/DashboardMigrator.test.ts @@ -7,9 +7,26 @@ import { DataLinkBuiltInVars, MappingType } from '@grafana/data'; import { VariableHide } from '../../variables/types'; import { config } from 'app/core/config'; import { getPanelPlugin } from 'app/features/plugins/__mocks__/pluginMocks'; +import { setDataSourceSrv } from '@grafana/runtime'; +import { mockDataSource, MockDataSourceSrv } from 'app/features/alerting/unified/mocks'; +import { MIXED_DATASOURCE_NAME } from 'app/plugins/datasource/mixed/MixedDataSource'; jest.mock('app/core/services/context_srv', () => ({})); +const dataSources = { + prom: mockDataSource({ + name: 'prom', + type: 'prometheus', + }), + [MIXED_DATASOURCE_NAME]: mockDataSource({ + name: MIXED_DATASOURCE_NAME, + type: 'mixed', + uid: MIXED_DATASOURCE_NAME, + }), +}; + +setDataSourceSrv(new MockDataSourceSrv(dataSources)); + describe('DashboardModel', () => { describe('when creating dashboard with old schema', () => { let model: any; @@ -1749,6 +1766,65 @@ describe('DashboardModel', () => { }); }); }); + + describe('when migrating datasource to refs', () => { + let model: DashboardModel; + + beforeEach(() => { + model = new DashboardModel({ + templating: { + list: [ + { + type: 'query', + name: 'var', + options: [{ text: 'A', value: 'A' }], + refresh: 0, + datasource: 'prom', + }, + ], + }, + panels: [ + { + id: 1, + datasource: 'prom', + }, + { + id: 2, + datasource: null, + }, + { + id: 3, + datasource: MIXED_DATASOURCE_NAME, + targets: [ + { + datasource: 'prom', + }, + ], + }, + ], + }); + }); + + it('should update variable datasource props to refs', () => { + expect(model.templating.list[0].datasource).toEqual({ type: 'prometheus', uid: 'mock-ds-2' }); + }); + + it('should update panel datasource props to refs for named data source', () => { + expect(model.panels[0].datasource).toEqual({ type: 'prometheus', uid: 'mock-ds-2' }); + }); + + it('should update panel datasource props to refs for default data source', () => { + expect(model.panels[1].datasource).toEqual(null); + }); + + it('should update panel datasource props to refs for mixed data source', () => { + expect(model.panels[2].datasource).toEqual({ type: 'mixed', uid: MIXED_DATASOURCE_NAME }); + }); + + it('should update target datasource props to refs', () => { + expect(model.panels[2].targets[0].datasource).toEqual({ type: 'prometheus', uid: 'mock-ds-2' }); + }); + }); }); function createRow(options: any, panelDescriptions: any[]) { diff --git a/public/app/features/dashboard/state/DashboardMigrator.ts b/public/app/features/dashboard/state/DashboardMigrator.ts index 718124d6b8b..216eb4115a6 100644 --- a/public/app/features/dashboard/state/DashboardMigrator.ts +++ b/public/app/features/dashboard/state/DashboardMigrator.ts @@ -23,6 +23,8 @@ import { DataTransformerConfig, AnnotationQuery, DataQuery, + getDataSourceRef, + isDataSourceRef, } from '@grafana/data'; // Constants import { @@ -40,7 +42,6 @@ import { config } from 'app/core/config'; import { plugin as statPanelPlugin } from 'app/plugins/panel/stat/module'; import { plugin as gaugePanelPlugin } from 'app/plugins/panel/gauge/module'; import { getStandardFieldConfigs, getStandardOptionEditors } from '@grafana/ui'; -import { MIXED_DATASOURCE_NAME } from 'app/plugins/datasource/mixed/MixedDataSource'; import { getDataSourceSrv } from '@grafana/runtime'; import { labelsToFieldsTransformer } from '../../../../../packages/grafana-data/src/transformations/transformers/labelsToFields'; import { mergeTransformer } from '../../../../../packages/grafana-data/src/transformations/transformers/merge'; @@ -704,34 +705,21 @@ export class DashboardMigrator { if (variable.type !== 'query') { continue; } - let name = (variable as any).datasource as string; - if (name) { - variable.datasource = migrateDatasourceNameToRef(name); - } + variable.datasource = migrateDatasourceNameToRef(variable.datasource); } // Mutate panel models for (const panel of this.dashboard.panels) { - let name = (panel as any).datasource as string; - if (!name) { - panel.datasource = null; // use default - } else if (name === MIXED_DATASOURCE_NAME) { - panel.datasource = { type: MIXED_DATASOURCE_NAME }; - for (const target of panel.targets) { - name = (target as any).datasource as string; - panel.datasource = migrateDatasourceNameToRef(name); - } - continue; // do not cleanup targets - } else { - panel.datasource = migrateDatasourceNameToRef(name); + panel.datasource = migrateDatasourceNameToRef(panel.datasource); + + if (!panel.targets) { + continue; } - // cleanup query datasource references - if (!panel.targets) { - panel.targets = []; - } else { - for (const target of panel.targets) { - delete target.datasource; + for (const target of panel.targets) { + const targetRef = migrateDatasourceNameToRef(target.datasource); + if (targetRef != null) { + target.datasource = targetRef; } } } @@ -1051,17 +1039,21 @@ function migrateSinglestat(panel: PanelModel) { return panel; } -export function migrateDatasourceNameToRef(name: string): DataSourceRef | null { - if (!name || name === 'default') { +export function migrateDatasourceNameToRef(nameOrRef?: string | DataSourceRef | null): DataSourceRef | null { + if (nameOrRef == null || nameOrRef === 'default') { return null; } - const ds = getDataSourceSrv().getInstanceSettings(name); - if (!ds) { - return { uid: name }; // not found + if (isDataSourceRef(nameOrRef)) { + return nameOrRef; } - return { type: ds.meta.id, uid: ds.uid }; + const ds = getDataSourceSrv().getInstanceSettings(nameOrRef); + if (!ds) { + return { uid: nameOrRef as string }; // not found + } + + return getDataSourceRef(ds); } // mutates transformations appending a new transformer after the existing one @@ -1085,7 +1077,7 @@ function upgradeValueMappingsForPanel(panel: PanelModel) { return panel; } - if (fieldConfig.defaults) { + if (fieldConfig.defaults && fieldConfig.defaults.mappings) { fieldConfig.defaults.mappings = upgradeValueMappings( fieldConfig.defaults.mappings, fieldConfig.defaults.thresholds