Skip to content

Commit

Permalink
Datasource: Fix - apply default query also to queries in new panels (#…
Browse files Browse the repository at this point in the history
…59625)

* apply default query also to new panels

* add comment

* add tests

(cherry picked from commit 32309a8)
  • Loading branch information
sunker committed Dec 5, 2022
1 parent b9b6caf commit b47c255
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 6 deletions.
23 changes: 22 additions & 1 deletion public/app/core/utils/query.test.ts
@@ -1,6 +1,10 @@
import { DataQuery } from '@grafana/data';

import { getNextRefIdChar } from './query';
import { getNextRefIdChar, queryIsEmpty } from './query';

export interface TestQuery extends DataQuery {
name?: string;
}

function dataQueryHelper(ids: string[]): DataQuery[] {
return ids.map((letter) => {
Expand Down Expand Up @@ -29,3 +33,20 @@ describe('Get next refId char', () => {
expect(getNextRefIdChar(singleExtendedDataQuery)).toEqual('AA');
});
});

describe('queryIsEmpty', () => {
it('should return true if query only includes props that are defined in the DataQuery interface', () => {
const testQuery: DataQuery = { refId: 'A' };
expect(queryIsEmpty(testQuery)).toEqual(true);
});

it('should return true if query only includes props that are defined in the DataQuery interface and a label prop', () => {
const testQuery: DataQuery & { label: string } = { refId: 'A', label: '' };
expect(queryIsEmpty(testQuery)).toEqual(true);
});

it('should return false if query only includes props that are not defined in the DataQuery interface', () => {
const testQuery: TestQuery = { refId: 'A', name: 'test' };
expect(queryIsEmpty(testQuery)).toEqual(false);
});
});
17 changes: 17 additions & 0 deletions public/app/core/utils/query.ts
Expand Up @@ -9,6 +9,23 @@ export const getNextRefIdChar = (queries: DataQuery[]): string => {
}
};

// This function checks if the query has defined properties beyond those defined in the DataQuery interface.
export function queryIsEmpty(query: DataQuery): boolean {
const dataQueryProps = ['refId', 'hide', 'key', 'queryType', 'datasource'];

for (const key in query) {
// label is not a DataQuery prop, but it's defined in the query when called from the QueryRunner.
if (key === 'label') {
continue;
}
if (!dataQueryProps.includes(key)) {
return false;
}
}

return true;
}

export function addQuery(queries: DataQuery[], query?: Partial<DataQuery>, datasource?: DataSourceRef): DataQuery[] {
const q = query || {};
q.refId = getNextRefIdChar(queries);
Expand Down
8 changes: 6 additions & 2 deletions public/app/features/query/components/QueryGroup.tsx
Expand Up @@ -17,7 +17,7 @@ import { Button, CustomScrollbar, HorizontalGroup, InlineFormLabel, Modal, style
import { PluginHelp } from 'app/core/components/PluginHelp/PluginHelp';
import config from 'app/core/config';
import { backendSrv } from 'app/core/services/backend_srv';
import { addQuery } from 'app/core/utils/query';
import { addQuery, queryIsEmpty } from 'app/core/utils/query';
import { dataSource as expressionDatasource } from 'app/features/expressions/ExpressionDatasource';
import { DashboardQueryEditor, isSharedDashboardQuery } from 'app/plugins/datasource/dashboard';
import { QueryGroupOptions } from 'app/types';
Expand Down Expand Up @@ -82,7 +82,11 @@ export class QueryGroup extends PureComponent<Props, State> {
const dsSettings = this.dataSourceSrv.getInstanceSettings(options.dataSource);
const defaultDataSource = await this.dataSourceSrv.get();
const datasource = ds.getRef();
const queries = options.queries.map((q) => (q.datasource ? q : { ...q, datasource }));
const queries = options.queries.map((q) => ({
...(queryIsEmpty(q) && ds?.getDefaultQuery?.(CoreApp.PanelEditor)),
datasource,
...q,
}));
this.setState({ queries, dataSource: ds, dsSettings, defaultDataSource });
} catch (error) {
console.log('failed to load data source', error);
Expand Down
7 changes: 7 additions & 0 deletions public/app/features/query/state/runRequest.ts
Expand Up @@ -6,6 +6,7 @@ import { catchError, map, mapTo, share, takeUntil, tap } from 'rxjs/operators';
// Utils & Services
// Types
import {
CoreApp,
DataFrame,
DataQueryError,
DataQueryRequest,
Expand All @@ -23,6 +24,7 @@ import {
import { toDataQueryError } from '@grafana/runtime';
import { isExpressionReference } from '@grafana/runtime/src/utils/DataSourceWithBackend';
import { backendSrv } from 'app/core/services/backend_srv';
import { queryIsEmpty } from 'app/core/utils/query';
import { dataSource as expressionDatasource } from 'app/features/expressions/ExpressionDatasource';
import { ExpressionQuery } from 'app/features/expressions/types';

Expand Down Expand Up @@ -174,6 +176,11 @@ export function callQueryMethod(
request: DataQueryRequest,
queryFunction?: typeof datasource.query
) {
// If the datasource has defined a default query, make sure it's applied if the query is empty
request.targets = request.targets.map((t) =>
queryIsEmpty(t) ? { ...datasource?.getDefaultQuery?.(CoreApp.PanelEditor), ...t } : t
);

// If its a public datasource, just return the result. Expressions will be handled on the backend.
if (datasource.type === 'public-ds') {
return from(datasource.query(request));
Expand Down
51 changes: 51 additions & 0 deletions public/app/features/query/state/updateQueries.test.ts
Expand Up @@ -5,6 +5,7 @@ import {
DataSourceWithQueryImportSupport,
} from '@grafana/data';
import { ExpressionDatasourceRef } from '@grafana/runtime/src/utils/DataSourceWithBackend';
import { TestQuery } from 'app/core/utils/query.test';

import { updateQueries } from './updateQueries';

Expand Down Expand Up @@ -41,6 +42,9 @@ const newUidSameTypeDS = {
} as DataSourceApi;

describe('updateQueries', () => {
afterEach(() => {
jest.clearAllMocks();
});
it('Should update all queries except expression query when changing data source with same type', async () => {
const updated = await updateQueries(
newUidSameTypeDS,
Expand Down Expand Up @@ -111,6 +115,53 @@ describe('updateQueries', () => {
expect(updated[0].datasource).toEqual({ type: 'new-type', uid: 'new-uid' });
});

it('Should clear queries and get default query from ds when changing type', async () => {
newUidDS.getDefaultQuery = jest.fn().mockReturnValue({ test: 'default-query1' } as Partial<TestQuery>);
const updated = await updateQueries(
newUidDS,
'new-uid',
[
{
refId: 'A',
datasource: {
uid: 'old-uid',
type: 'old-type',
},
},
{
refId: 'B',
datasource: {
uid: 'old-uid',
type: 'old-type',
},
},
],
oldUidDS
);

expect(newUidDS.getDefaultQuery).toHaveBeenCalled();
expect(updated as TestQuery[]).toEqual([
{
datasource: { type: 'new-type', uid: 'new-uid' },
refId: 'A',
test: 'default-query1',
},
]);
});

it('Should return default query from ds when changing type and no new queries exist', async () => {
newUidDS.getDefaultQuery = jest.fn().mockReturnValue({ test: 'default-query2' } as Partial<TestQuery>);
const updated = await updateQueries(newUidDS, 'new-uid', [], oldUidDS);
expect(newUidDS.getDefaultQuery).toHaveBeenCalled();
expect(updated as TestQuery[]).toEqual([
{
datasource: { type: 'new-type', uid: 'new-uid' },
refId: 'A',
test: 'default-query2',
},
]);
});

it('Should preserve query data source when changing to mixed', async () => {
const updated = await updateQueries(
mixedDS,
Expand Down
7 changes: 4 additions & 3 deletions public/app/features/query/state/updateQueries.ts
@@ -1,4 +1,4 @@
import { DataQuery, DataSourceApi, hasQueryExportSupport, hasQueryImportSupport } from '@grafana/data';
import { CoreApp, DataQuery, DataSourceApi, hasQueryExportSupport, hasQueryImportSupport } from '@grafana/data';
import { isExpressionReference } from '@grafana/runtime/src/utils/DataSourceWithBackend';

export async function updateQueries(
Expand All @@ -9,6 +9,7 @@ export async function updateQueries(
): Promise<DataQuery[]> {
let nextQueries = queries;
const datasource = { type: nextDS.type, uid: nextDSUidOrVariableExpression };
const DEFAULT_QUERY = { ...nextDS?.getDefaultQuery?.(CoreApp.PanelEditor), datasource, refId: 'A' };

// we are changing data source type
if (currentDS?.meta.id !== nextDS.meta.id) {
Expand All @@ -27,12 +28,12 @@ export async function updateQueries(
}
// Otherwise clear queries
else {
return [{ refId: 'A', datasource }];
return [DEFAULT_QUERY];
}
}

if (nextQueries.length === 0) {
return [{ refId: 'A', datasource }];
return [DEFAULT_QUERY];
}

// Set data source on all queries except expression queries
Expand Down

0 comments on commit b47c255

Please sign in to comment.