From 1f6f866a36b933242d941b10e42b59616f70df0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Farkas?= Date: Mon, 18 Sep 2023 10:39:05 +0200 Subject: [PATCH] sql: do not use the getTimeSrv call (#74800) * sql: eliminate the getTimeSrv call * adjusted tests --- .../plugins/sql/datasource/SqlDatasource.ts | 22 +++++++++++++------ .../datasource/mssql/datasource.test.ts | 12 +++++----- .../datasource/mysql/specs/datasource.test.ts | 16 ++++++++------ .../datasource/postgres/datasource.test.ts | 16 ++++++++------ 4 files changed, 40 insertions(+), 26 deletions(-) diff --git a/public/app/features/plugins/sql/datasource/SqlDatasource.ts b/public/app/features/plugins/sql/datasource/SqlDatasource.ts index 37a53bd855a..a1e71965f34 100644 --- a/public/app/features/plugins/sql/datasource/SqlDatasource.ts +++ b/public/app/features/plugins/sql/datasource/SqlDatasource.ts @@ -2,6 +2,7 @@ import { lastValueFrom, Observable, throwError } from 'rxjs'; import { map } from 'rxjs/operators'; import { + getDefaultTimeRange, DataFrame, DataFrameView, DataQuery, @@ -15,6 +16,7 @@ import { getSearchFilterScopedVar, LegacyMetricFindQueryOptions, VariableWithMultiSupport, + TimeRange, } from '@grafana/data'; import { EditorMode } from '@grafana/experimental'; import { @@ -27,7 +29,6 @@ import { TemplateSrv, reportInteraction, } from '@grafana/runtime'; -import { getTimeSrv } from 'app/features/dashboard/services/TimeSrv'; import { ResponseParser } from '../ResponseParser'; import { SqlQueryEditor } from '../components/QueryEditor'; @@ -182,6 +183,12 @@ export abstract class SqlDatasource extends DataSourceWithBackend { + const range = options?.range; + if (range == null) { + // i cannot create a scenario where this happens, we handle it just to be sure. + return []; + } + let refId = 'tempvar'; if (options && options.variable && options.variable.name) { refId = options.variable.name; @@ -201,17 +208,18 @@ export abstract class SqlDatasource extends DataSourceWithBackend(query: string, options?: RunSQLOptions) { - const frame = await this.runMetaQuery({ rawSql: query, format: QueryFormat.Table, refId: options?.refId }, options); + const range = getDefaultTimeRange(); + const frame = await this.runMetaQuery({ rawSql: query, format: QueryFormat.Table, refId: options?.refId }, range); return new DataFrameView(frame); } - private runMetaQuery(request: Partial, options?: LegacyMetricFindQueryOptions): Promise { - const range = getTimeSrv().timeRange(); + private runMetaQuery(request: Partial, range: TimeRange): Promise { const refId = request.refId || 'meta'; const queries: DataQuery[] = [{ ...request, datasource: request.datasource || this.getRef(), refId }]; @@ -222,8 +230,8 @@ export abstract class SqlDatasource extends DataSourceWithBackend; describe('MSSQLDatasource', () => { + const defaultRange = getDefaultTimeRange(); // it does not matter what value this has const fetchMock = jest.spyOn(backendSrv, 'fetch'); const ctx = { @@ -69,7 +71,7 @@ describe('MSSQLDatasource', () => { beforeEach(() => { fetchMock.mockImplementation(() => of(createFetchResponse(response))); - return ctx.ds.metricFindQuery(query).then((data: MetricFindValue[]) => { + return ctx.ds.metricFindQuery(query, { range: defaultRange }).then((data: MetricFindValue[]) => { results = data; }); }); @@ -97,7 +99,7 @@ describe('MSSQLDatasource', () => { it('should return an empty array when metricFindQuery is called', async () => { const query = 'select * from atable'; - const results = await ctx.ds.metricFindQuery(query); + const results = await ctx.ds.metricFindQuery(query, { range: defaultRange }); expect(results.length).toBe(0); }); @@ -231,7 +233,7 @@ describe('MSSQLDatasource', () => { beforeEach(() => { fetchMock.mockImplementation(() => of(createFetchResponse(response))); - return ctx.ds.metricFindQuery(query).then((data) => { + return ctx.ds.metricFindQuery(query, { range: defaultRange }).then((data) => { results = data; }); }); @@ -272,7 +274,7 @@ describe('MSSQLDatasource', () => { beforeEach(() => { fetchMock.mockImplementation(() => of(createFetchResponse(response))); - return ctx.ds.metricFindQuery(query).then((data) => { + return ctx.ds.metricFindQuery(query, { range: defaultRange }).then((data) => { results = data; }); }); @@ -311,7 +313,7 @@ describe('MSSQLDatasource', () => { beforeEach(() => { fetchMock.mockImplementation(() => of(createFetchResponse(response))); - return ctx.ds.metricFindQuery(query).then((data) => { + return ctx.ds.metricFindQuery(query, { range: defaultRange }).then((data) => { results = data; }); }); diff --git a/public/app/plugins/datasource/mysql/specs/datasource.test.ts b/public/app/plugins/datasource/mysql/specs/datasource.test.ts index 2149011f16e..9218b07e01c 100644 --- a/public/app/plugins/datasource/mysql/specs/datasource.test.ts +++ b/public/app/plugins/datasource/mysql/specs/datasource.test.ts @@ -2,6 +2,7 @@ import { of } from 'rxjs'; import { dataFrameToJSON, + getDefaultTimeRange, DataQueryRequest, DataSourceInstanceSettings, dateTime, @@ -18,6 +19,7 @@ import { MySqlDatasource } from '../MySqlDatasource'; import { MySQLOptions } from '../types'; describe('MySQLDatasource', () => { + const defaultRange = getDefaultTimeRange(); // it does not matter what value this has const setupTestContext = (response: unknown) => { jest.clearAllMocks(); setBackendSrv(backendSrv); @@ -82,7 +84,7 @@ describe('MySQLDatasource', () => { it('should return an empty array when metricFindQuery is called', async () => { const query = 'select * from atable'; - const results = await ds.metricFindQuery(query); + const results = await ds.metricFindQuery(query, { range: defaultRange }); expect(results.length).toBe(0); }); @@ -216,7 +218,7 @@ describe('MySQLDatasource', () => { it('should return list of all string field values', async () => { const { ds } = setupTestContext(response); - const results = await ds.metricFindQuery(query, {}); + const results = await ds.metricFindQuery(query, { range: defaultRange }); expect(results.length).toBe(6); expect(results[0].text).toBe('aTitle'); @@ -249,7 +251,7 @@ describe('MySQLDatasource', () => { it('should return list of all column values', async () => { const { ds, fetchMock } = setupTestContext(response); - const results = await ds.metricFindQuery(query, { searchFilter: 'aTit' }); + const results = await ds.metricFindQuery(query, { range: defaultRange, searchFilter: 'aTit' }); expect(fetchMock).toBeCalledTimes(1); expect(fetchMock.mock.calls[0][0].data.queries[0].rawSql).toBe( @@ -284,7 +286,7 @@ describe('MySQLDatasource', () => { it('should return list of all column values', async () => { const { ds, fetchMock } = setupTestContext(response); - const results = await ds.metricFindQuery(query, {}); + const results = await ds.metricFindQuery(query, { range: defaultRange }); expect(fetchMock).toBeCalledTimes(1); expect(fetchMock.mock.calls[0][0].data.queries[0].rawSql).toBe("select title from atable where title LIKE '%'"); @@ -317,7 +319,7 @@ describe('MySQLDatasource', () => { it('should return list of as text, value', async () => { const { ds } = setupTestContext(response); - const results = await ds.metricFindQuery(query, {}); + const results = await ds.metricFindQuery(query, { range: defaultRange }); expect(results.length).toBe(3); expect(results[0].text).toBe('aTitle'); @@ -352,7 +354,7 @@ describe('MySQLDatasource', () => { it('should return list of all field values as text', async () => { const { ds } = setupTestContext(response); - const results = await ds.metricFindQuery(query, {}); + const results = await ds.metricFindQuery(query, { range: defaultRange }); expect(results).toEqual([ { text: 1 }, @@ -390,7 +392,7 @@ describe('MySQLDatasource', () => { it('should return list of unique keys', async () => { const { ds } = setupTestContext(response); - const results = await ds.metricFindQuery(query, {}); + const results = await ds.metricFindQuery(query, { range: defaultRange }); expect(results.length).toBe(1); expect(results[0].text).toBe('aTitle'); diff --git a/public/app/plugins/datasource/postgres/datasource.test.ts b/public/app/plugins/datasource/postgres/datasource.test.ts index 5129098192e..d95594be110 100644 --- a/public/app/plugins/datasource/postgres/datasource.test.ts +++ b/public/app/plugins/datasource/postgres/datasource.test.ts @@ -2,6 +2,7 @@ import { Observable, of } from 'rxjs'; import { TestScheduler } from 'rxjs/testing'; import { + getDefaultTimeRange, dataFrameToJSON, DataQueryRequest, DataQueryResponse, @@ -37,6 +38,7 @@ jest.mock('@grafana/runtime/src/services', () => ({ })); describe('PostgreSQLDatasource', () => { + const defaultRange = getDefaultTimeRange(); // it does not matter what value this has const fetchMock = jest.spyOn(backendSrv, 'fetch'); const setupTestContext = (data: unknown, mock?: Observable>) => { jest.clearAllMocks(); @@ -311,7 +313,7 @@ describe('PostgreSQLDatasource', () => { it('should return an empty array when metricFindQuery is called', async () => { const query = 'select * from atable'; - const results = await ds.metricFindQuery(query); + const results = await ds.metricFindQuery(query, { range: defaultRange }); expect(results.length).toBe(0); }); @@ -474,7 +476,7 @@ describe('PostgreSQLDatasource', () => { }; const { ds } = setupTestContext(response); - const results = await ds.metricFindQuery(query, {}); + const results = await ds.metricFindQuery(query, { range: defaultRange }); expect(results.length).toBe(6); expect(results[0].text).toBe('aTitle'); @@ -507,7 +509,7 @@ describe('PostgreSQLDatasource', () => { }; const { ds } = setupTestContext(response); - const results = await ds.metricFindQuery(query, { searchFilter: 'aTit' }); + const results = await ds.metricFindQuery(query, { range: defaultRange, searchFilter: 'aTit' }); expect(fetchMock).toBeCalledTimes(1); expect(fetchMock.mock.calls[0][0].data.queries[0].rawSql).toBe( @@ -549,7 +551,7 @@ describe('PostgreSQLDatasource', () => { }; const { ds } = setupTestContext(response); - const results = await ds.metricFindQuery(query, {}); + const results = await ds.metricFindQuery(query, { range: defaultRange }); expect(fetchMock).toBeCalledTimes(1); expect(fetchMock.mock.calls[0][0].data.queries[0].rawSql).toBe("select title from atable where title LIKE '%'"); @@ -588,7 +590,7 @@ describe('PostgreSQLDatasource', () => { }, }; const { ds } = setupTestContext(response); - const results = await ds.metricFindQuery(query, {}); + const results = await ds.metricFindQuery(query, { range: defaultRange }); expect(results).toEqual([ { text: 'aTitle', value: 'value1' }, @@ -622,7 +624,7 @@ describe('PostgreSQLDatasource', () => { }, }; const { ds } = setupTestContext(response); - const results = await ds.metricFindQuery(query, {}); + const results = await ds.metricFindQuery(query, { range: defaultRange }); expect(results).toEqual([ { text: 1 }, @@ -659,7 +661,7 @@ describe('PostgreSQLDatasource', () => { }, }; const { ds } = setupTestContext(response); - const results = await ds.metricFindQuery(query, {}); + const results = await ds.metricFindQuery(query, { range: defaultRange }); expect(results).toEqual([{ text: 'aTitle', value: 'same' }]); });