Skip to content

N
nexpie-grafana-theme
Commit c3a19c6d by Andrej Ocenas Committed by GitHub
Options

Influxdb: Fix issues with request creation and parsing (#21743)

parent 7569a860
Showing with 134 additions and 125 deletions
public/app/core/services/backend_srv.ts
...@@ -454,42 +454,9 @@ export class BackendSrv implements BackendService { ...@@ -454,42 +454,9 @@ export class BackendSrv implements BackendService {
return options; return options;
}; };
private parseUrlFromOptions = (options: BackendSrvRequest): string => {
const cleanParams = omitBy(options.params, v => v === undefined || (v && v.length === 0));
const serializedParams = serializeParams(cleanParams);
return options.params && serializedParams.length ? `${options.url}?${serializedParams}` : options.url;
};
private parseInitFromOptions = (options: BackendSrvRequest): RequestInit => {
const method = options.method;
const headers = {
'Content-Type': 'application/json',
Accept: 'application/json, text/plain, */*',
...options.headers,
};
const body = this.parseBody({ ...options, headers });
return {
method,
headers,
body,
};
};
private parseBody = (options: BackendSrvRequest) => {
if (!options.data || typeof options.data === 'string') {
return options.data;
}
if (options.headers['Content-Type'] === 'application/json') {
return JSON.stringify(options.data);
}
return new URLSearchParams(options.data);
};
private getFromFetchStream = (options: BackendSrvRequest) => { private getFromFetchStream = (options: BackendSrvRequest) => {
const url = this.parseUrlFromOptions(options); const url = parseUrlFromOptions(options);
const init = this.parseInitFromOptions(options); const init = parseInitFromOptions(options);
return this.dependencies.fromFetch(url, init).pipe( return this.dependencies.fromFetch(url, init).pipe(
mergeMap(async response => { mergeMap(async response => {
const { status, statusText, ok, headers, url, type, redirected } = response; const { status, statusText, ok, headers, url, type, redirected } = response;
...@@ -545,3 +512,36 @@ coreModule.factory('backendSrv', () => backendSrv); ...@@ -545,3 +512,36 @@ coreModule.factory('backendSrv', () => backendSrv);
// Used for testing and things that really need BackendSrv // Used for testing and things that really need BackendSrv
export const backendSrv = new BackendSrv(); export const backendSrv = new BackendSrv();
export const getBackendSrv = (): BackendSrv => backendSrv; export const getBackendSrv = (): BackendSrv => backendSrv;
export const parseUrlFromOptions = (options: BackendSrvRequest): string => {
const cleanParams = omitBy(options.params, v => v === undefined || (v && v.length === 0));
const serializedParams = serializeParams(cleanParams);
return options.params && serializedParams.length ? `${options.url}?${serializedParams}` : options.url;
};
export const parseInitFromOptions = (options: BackendSrvRequest): RequestInit => {
const method = options.method;
const headers = {
'Content-Type': 'application/json',
Accept: 'application/json, text/plain, */*',
...options.headers,
};
const body = parseBody({ ...options, headers });
return {
method,
headers,
body,
};
};
const parseBody = (options: BackendSrvRequest) => {
if (!options.data || typeof options.data === 'string') {
return options.data;
}
if (options.headers['Content-Type'] === 'application/json') {
return JSON.stringify(options.data);
}
return new URLSearchParams(options.data);
};
public/app/core/specs/backend_srv.test.ts
import { BackendSrv, getBackendSrv } from '../services/backend_srv'; import { BackendSrv, getBackendSrv, parseInitFromOptions, parseUrlFromOptions } from '../services/backend_srv';
import { Emitter } from '../utils/emitter'; import { Emitter } from '../utils/emitter';
import { ContextSrv, User } from '../services/context_srv'; import { ContextSrv, User } from '../services/context_srv';
import { Observable, of } from 'rxjs'; import { Observable, of } from 'rxjs';
...@@ -47,11 +47,6 @@ const getTestContext = (overides?: object) => { ...@@ -47,11 +47,6 @@ const getTestContext = (overides?: object) => {
const logoutMock = jest.fn(); const logoutMock = jest.fn();
const parseRequestOptionsMock = jest.fn().mockImplementation(options => options); const parseRequestOptionsMock = jest.fn().mockImplementation(options => options);
const parseDataSourceRequestOptionsMock = jest.fn().mockImplementation(options => options); const parseDataSourceRequestOptionsMock = jest.fn().mockImplementation(options => options);
const parseUrlFromOptionsMock = jest.fn().mockImplementation(options => options.url);
const parseInitFromOptionsMock = jest.fn().mockImplementation(options => ({
method: options.method,
url: options.url,
}));
const backendSrv = new BackendSrv({ const backendSrv = new BackendSrv({
fromFetch: fromFetchMock, fromFetch: fromFetchMock,
...@@ -62,19 +57,9 @@ const getTestContext = (overides?: object) => { ...@@ -62,19 +57,9 @@ const getTestContext = (overides?: object) => {
backendSrv['parseRequestOptions'] = parseRequestOptionsMock; backendSrv['parseRequestOptions'] = parseRequestOptionsMock;
backendSrv['parseDataSourceRequestOptions'] = parseDataSourceRequestOptionsMock; backendSrv['parseDataSourceRequestOptions'] = parseDataSourceRequestOptionsMock;
backendSrv['parseUrlFromOptions'] = parseUrlFromOptionsMock;
backendSrv['parseInitFromOptions'] = parseInitFromOptionsMock;
const expectCallChain = (options: any) => { const expectCallChain = (options: any) => {
expect(parseUrlFromOptionsMock).toHaveBeenCalledTimes(1);
expect(parseUrlFromOptionsMock).toHaveBeenCalledWith(options);
expect(parseInitFromOptionsMock).toHaveBeenCalledTimes(1);
expect(parseInitFromOptionsMock).toHaveBeenCalledWith(options);
expect(fromFetchMock).toHaveBeenCalledTimes(1); expect(fromFetchMock).toHaveBeenCalledTimes(1);
expect(fromFetchMock).toHaveBeenCalledWith(options.url, {
method: options.method,
url: options.url,
});
}; };
const expectRequestCallChain = (options: any) => { const expectRequestCallChain = (options: any) => {
...@@ -98,8 +83,6 @@ const getTestContext = (overides?: object) => { ...@@ -98,8 +83,6 @@ const getTestContext = (overides?: object) => {
logoutMock, logoutMock,
parseRequestOptionsMock, parseRequestOptionsMock,
parseDataSourceRequestOptionsMock, parseDataSourceRequestOptionsMock,
parseUrlFromOptionsMock,
parseInitFromOptionsMock,
expectRequestCallChain, expectRequestCallChain,
expectDataSourceRequestCallChain, expectDataSourceRequestCallChain,
}; };
...@@ -150,45 +133,6 @@ describe('backendSrv', () => { ...@@ -150,45 +133,6 @@ describe('backendSrv', () => {
); );
}); });
describe('parseUrlFromOptions', () => {
it.each`
params | url | expected
${undefined} | ${'api/dashboard'} | ${'api/dashboard'}
${{ key: 'value' }} | ${'api/dashboard'} | ${'api/dashboard?key=value'}
${{ key: undefined }} | ${'api/dashboard'} | ${'api/dashboard'}
${{ firstKey: 'first value', secondValue: 'second value' }} | ${'api/dashboard'} | ${'api/dashboard?firstKey=first%20value&secondValue=second%20value'}
${{ firstKey: 'first value', secondValue: undefined }} | ${'api/dashboard'} | ${'api/dashboard?firstKey=first%20value'}
${{ id: [1, 2, 3] }} | ${'api/dashboard'} | ${'api/dashboard?id=1&id=2&id=3'}
${{ id: [] }} | ${'api/dashboard'} | ${'api/dashboard'}
`(
"when called with params: '$params' and url: '$url' then result should be '$expected'",
({ params, url, expected }) => {
expect(getBackendSrv()['parseUrlFromOptions']({ params, url })).toEqual(expected);
}
);
});
describe('parseInitFromOptions', () => {
it.each`
method | headers | data | expected
${undefined} | ${undefined} | ${undefined} | ${{ method: undefined, headers: { 'Content-Type': 'application/json', Accept: 'application/json, text/plain, */*' }, body: undefined }}
${'GET'} | ${undefined} | ${undefined} | ${{ method: 'GET', headers: { 'Content-Type': 'application/json', Accept: 'application/json, text/plain, */*' }, body: undefined }}
${'GET'} | ${{ Auth: 'Some Auth' }} | ${undefined} | ${{ method: 'GET', headers: { 'Content-Type': 'application/json', Accept: 'application/json, text/plain, */*', Auth: 'Some Auth' }, body: undefined }}
${'GET'} | ${{ Auth: 'Some Auth' }} | ${{ data: { test: 'Some data' } }} | ${{ method: 'GET', headers: { 'Content-Type': 'application/json', Accept: 'application/json, text/plain, */*', Auth: 'Some Auth' }, body: '{"data":{"test":"Some data"}}' }}
${'GET'} | ${{ Auth: 'Some Auth' }} | ${'some data'} | ${{ method: 'GET', headers: { 'Content-Type': 'application/json', Accept: 'application/json, text/plain, */*', Auth: 'Some Auth' }, body: 'some data' }}
${'GET'} | ${{ Auth: 'Some Auth' }} | ${'{"data":{"test":"Some data"}}'} | ${{ method: 'GET', headers: { 'Content-Type': 'application/json', Accept: 'application/json, text/plain, */*', Auth: 'Some Auth' }, body: '{"data":{"test":"Some data"}}' }}
${'POST'} | ${{ Auth: 'Some Auth', 'Content-Type': 'application/x-www-form-urlencoded' }} | ${undefined} | ${{ method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded', Accept: 'application/json, text/plain, */*', Auth: 'Some Auth' }, body: undefined }}
${'POST'} | ${{ Auth: 'Some Auth', 'Content-Type': 'application/x-www-form-urlencoded' }} | ${{ data: 'Some data' }} | ${{ method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded', Accept: 'application/json, text/plain, */*', Auth: 'Some Auth' }, body: new URLSearchParams({ data: 'Some data' }) }}
${'POST'} | ${{ Auth: 'Some Auth', 'Content-Type': 'application/x-www-form-urlencoded' }} | ${'some data'} | ${{ method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded', Accept: 'application/json, text/plain, */*', Auth: 'Some Auth' }, body: 'some data' }}
${'POST'} | ${{ Auth: 'Some Auth', 'Content-Type': 'application/x-www-form-urlencoded' }} | ${'{"data":{"test":"Some data"}}'} | ${{ method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded', Accept: 'application/json, text/plain, */*', Auth: 'Some Auth' }, body: '{"data":{"test":"Some data"}}' }}
`(
"when called with method: '$method', headers: '$headers' and data: '$data' then result should be '$expected'",
({ method, headers, data, expected }) => {
expect(getBackendSrv()['parseInitFromOptions']({ method, headers, data, url: '' })).toEqual(expected);
}
);
});
describe('request', () => { describe('request', () => {
describe('when making a successful call and conditions for showSuccessAlert are not favorable', () => { describe('when making a successful call and conditions for showSuccessAlert are not favorable', () => {
it('then it should return correct result and not emit anything', async () => { it('then it should return correct result and not emit anything', async () => {
...@@ -354,7 +298,15 @@ describe('backendSrv', () => { ...@@ -354,7 +298,15 @@ describe('backendSrv', () => {
statusText: 'Ok', statusText: 'Ok',
type: 'basic', type: 'basic',
url, url,
request: { url, method: 'GET' }, request: {
url,
method: 'GET',
body: undefined,
headers: {
'Content-Type': 'application/json',
Accept: 'application/json, text/plain, */*',
},
},
}); });
expect(appEventsMock.emit).not.toHaveBeenCalled(); expect(appEventsMock.emit).not.toHaveBeenCalled();
expectDataSourceRequestCallChain({ url, method: 'GET', silent: true }); expectDataSourceRequestCallChain({ url, method: 'GET', silent: true });
...@@ -366,7 +318,7 @@ describe('backendSrv', () => { ...@@ -366,7 +318,7 @@ describe('backendSrv', () => {
const url = 'http://localhost:3000/api/some-mock'; const url = 'http://localhost:3000/api/some-mock';
const { backendSrv, appEventsMock, expectDataSourceRequestCallChain } = getTestContext({ url }); const { backendSrv, appEventsMock, expectDataSourceRequestCallChain } = getTestContext({ url });
const result = await backendSrv.datasourceRequest({ url, method: 'GET' }); const result = await backendSrv.datasourceRequest({ url, method: 'GET' });
expect(result).toEqual({ const expectedResult = {
data: { test: 'hello world' }, data: { test: 'hello world' },
headers: { headers: {
'Content-Type': 'application/json', 'Content-Type': 'application/json',
...@@ -377,22 +329,20 @@ describe('backendSrv', () => { ...@@ -377,22 +329,20 @@ describe('backendSrv', () => {
statusText: 'Ok', statusText: 'Ok',
type: 'basic', type: 'basic',
url, url,
request: { url, method: 'GET' }, request: {
}); url,
expect(appEventsMock.emit).toHaveBeenCalledTimes(1); method: 'GET',
expect(appEventsMock.emit).toHaveBeenCalledWith(CoreEvents.dsRequestResponse, { body: undefined as any,
data: { test: 'hello world' }, headers: {
headers: { 'Content-Type': 'application/json',
'Content-Type': 'application/json', Accept: 'application/json, text/plain, */*',
},
}, },
ok: true, };
redirected: false,
status: 200, expect(result).toEqual(expectedResult);
statusText: 'Ok', expect(appEventsMock.emit).toHaveBeenCalledTimes(1);
type: 'basic', expect(appEventsMock.emit).toHaveBeenCalledWith(CoreEvents.dsRequestResponse, expectedResult);
url,
request: { url, method: 'GET' },
});
expectDataSourceRequestCallChain({ url, method: 'GET' }); expectDataSourceRequestCallChain({ url, method: 'GET' });
}); });
}); });
...@@ -451,7 +401,15 @@ describe('backendSrv', () => { ...@@ -451,7 +401,15 @@ describe('backendSrv', () => {
statusText: 'Ok', statusText: 'Ok',
type: 'basic', type: 'basic',
url, url,
request: { url, method: 'GET' }, request: {
url,
method: 'GET',
body: undefined,
headers: {
'Content-Type': 'application/json',
Accept: 'application/json, text/plain, */*',
},
},
}); });
const slowResponse = await slowRequest; const slowResponse = await slowRequest;
...@@ -610,3 +568,43 @@ describe('backendSrv', () => { ...@@ -610,3 +568,43 @@ describe('backendSrv', () => {
}); });
}); });
}); });
describe('parseUrlFromOptions', () => {
it.each`
params | url | expected
${undefined} | ${'api/dashboard'} | ${'api/dashboard'}
${{ key: 'value' }} | ${'api/dashboard'} | ${'api/dashboard?key=value'}
${{ key: undefined }} | ${'api/dashboard'} | ${'api/dashboard'}
${{ firstKey: 'first value', secondValue: 'second value' }} | ${'api/dashboard'} | ${'api/dashboard?firstKey=first%20value&secondValue=second%20value'}
${{ firstKey: 'first value', secondValue: undefined }} | ${'api/dashboard'} | ${'api/dashboard?firstKey=first%20value'}
${{ id: [1, 2, 3] }} | ${'api/dashboard'} | ${'api/dashboard?id=1&id=2&id=3'}
${{ id: [] }} | ${'api/dashboard'} | ${'api/dashboard'}
`(
"when called with params: '$params' and url: '$url' then result should be '$expected'",
({ params, url, expected }) => {
expect(parseUrlFromOptions({ params, url })).toEqual(expected);
}
);
});
describe('parseInitFromOptions', () => {
it.each`
method | headers | data | expected
${undefined} | ${undefined} | ${undefined} | ${{ method: undefined, headers: { 'Content-Type': 'application/json', Accept: 'application/json, text/plain, */*' }, body: undefined }}
${'GET'} | ${undefined} | ${undefined} | ${{ method: 'GET', headers: { 'Content-Type': 'application/json', Accept: 'application/json, text/plain, */*' }, body: undefined }}
${'GET'} | ${undefined} | ${null} | ${{ method: 'GET', headers: { 'Content-Type': 'application/json', Accept: 'application/json, text/plain, */*' }, body: null }}
${'GET'} | ${{ Auth: 'Some Auth' }} | ${undefined} | ${{ method: 'GET', headers: { 'Content-Type': 'application/json', Accept: 'application/json, text/plain, */*', Auth: 'Some Auth' }, body: undefined }}
${'GET'} | ${{ Auth: 'Some Auth' }} | ${{ data: { test: 'Some data' } }} | ${{ method: 'GET', headers: { 'Content-Type': 'application/json', Accept: 'application/json, text/plain, */*', Auth: 'Some Auth' }, body: '{"data":{"test":"Some data"}}' }}
${'GET'} | ${{ Auth: 'Some Auth' }} | ${'some data'} | ${{ method: 'GET', headers: { 'Content-Type': 'application/json', Accept: 'application/json, text/plain, */*', Auth: 'Some Auth' }, body: 'some data' }}
${'GET'} | ${{ Auth: 'Some Auth' }} | ${'{"data":{"test":"Some data"}}'} | ${{ method: 'GET', headers: { 'Content-Type': 'application/json', Accept: 'application/json, text/plain, */*', Auth: 'Some Auth' }, body: '{"data":{"test":"Some data"}}' }}
${'POST'} | ${{ Auth: 'Some Auth', 'Content-Type': 'application/x-www-form-urlencoded' }} | ${undefined} | ${{ method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded', Accept: 'application/json, text/plain, */*', Auth: 'Some Auth' }, body: undefined }}
${'POST'} | ${{ Auth: 'Some Auth', 'Content-Type': 'application/x-www-form-urlencoded' }} | ${{ data: 'Some data' }} | ${{ method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded', Accept: 'application/json, text/plain, */*', Auth: 'Some Auth' }, body: new URLSearchParams({ data: 'Some data' }) }}
${'POST'} | ${{ Auth: 'Some Auth', 'Content-Type': 'application/x-www-form-urlencoded' }} | ${'some data'} | ${{ method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded', Accept: 'application/json, text/plain, */*', Auth: 'Some Auth' }, body: 'some data' }}
${'POST'} | ${{ Auth: 'Some Auth', 'Content-Type': 'application/x-www-form-urlencoded' }} | ${'{"data":{"test":"Some data"}}'} | ${{ method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded', Accept: 'application/json, text/plain, */*', Auth: 'Some Auth' }, body: '{"data":{"test":"Some data"}}' }}
`(
"when called with method: '$method', headers: '$headers' and data: '$data' then result should be '$expected'",
({ method, headers, data, expected }) => {
expect(parseInitFromOptions({ method, headers, data, url: '' })).toEqual(expected);
}
);
});
public/app/plugins/datasource/influxdb/components/InfluxLogsQueryField.tsx
...@@ -81,6 +81,7 @@ export class InfluxLogsQueryField extends React.PureComponent<Props, State> { ...@@ -81,6 +81,7 @@ export class InfluxLogsQueryField extends React.PureComponent<Props, State> {
this.setState({ measurements }); this.setState({ measurements });
} catch (error) { } catch (error) {
const message = error && error.message ? error.message : error; const message = error && error.message ? error.message : error;
console.error(error);
this.setState({ error: message }); this.setState({ error: message });
} }
} }
......
public/app/plugins/datasource/influxdb/datasource.ts
...@@ -204,7 +204,9 @@ export default class InfluxDatasource extends DataSourceApi<InfluxQuery, InfluxO ...@@ -204,7 +204,9 @@ export default class InfluxDatasource extends DataSourceApi<InfluxQuery, InfluxO
metricFindQuery(query: string, options?: any) { metricFindQuery(query: string, options?: any) {
const interpolated = this.templateSrv.replace(query, null, 'regex'); const interpolated = this.templateSrv.replace(query, null, 'regex');
return this._seriesQuery(interpolated, options).then(() => this.responseParser.parse(query)); return this._seriesQuery(interpolated, options).then(resp => {
return this.responseParser.parse(query, resp);
});
} }
getTagKeys(options: any = {}) { getTagKeys(options: any = {}) {
...@@ -323,7 +325,7 @@ export default class InfluxDatasource extends DataSourceApi<InfluxQuery, InfluxO ...@@ -323,7 +325,7 @@ export default class InfluxDatasource extends DataSourceApi<InfluxQuery, InfluxO
return result.data; return result.data;
}, },
(err: any) => { (err: any) => {
if (err.status !== 0 || err.status >= 300) { if ((Number.isInteger(err.status) && err.status !== 0) || err.status >= 300) {
if (err.data && err.data.error) { if (err.data && err.data.error) {
throw { throw {
message: 'InfluxDB Error: ' + err.data.error, message: 'InfluxDB Error: ' + err.data.error,
...@@ -337,6 +339,8 @@ export default class InfluxDatasource extends DataSourceApi<InfluxQuery, InfluxO ...@@ -337,6 +339,8 @@ export default class InfluxDatasource extends DataSourceApi<InfluxQuery, InfluxO
config: err.config, config: err.config,
}; };
} }
} else {
throw err;
} }
} }
); );
......
public/app/plugins/datasource/influxdb/specs/datasource.test.ts
...@@ -31,7 +31,7 @@ describe('InfluxDataSource', () => { ...@@ -31,7 +31,7 @@ describe('InfluxDataSource', () => {
to: '2018-01-02T00:00:00Z', to: '2018-01-02T00:00:00Z',
}, },
}; };
let requestQuery: any, requestMethod: any, requestData: any; let requestQuery: any, requestMethod: any, requestData: any, response: any;
beforeEach(async () => { beforeEach(async () => {
datasourceRequestMock.mockImplementation((req: any) => { datasourceRequestMock.mockImplementation((req: any) => {
...@@ -39,21 +39,23 @@ describe('InfluxDataSource', () => { ...@@ -39,21 +39,23 @@ describe('InfluxDataSource', () => {
requestQuery = req.params.q; requestQuery = req.params.q;
requestData = req.data; requestData = req.data;
return Promise.resolve({ return Promise.resolve({
results: [ data: {
{ results: [
series: [ {
{ series: [
name: 'measurement', {
columns: ['max'], name: 'measurement',
values: [[1]], columns: ['name'],
}, values: [['cpu']],
], },
}, ],
], },
],
},
}); });
}); });
await ctx.ds.metricFindQuery(query, queryOptions).then(() => {}); response = await ctx.ds.metricFindQuery(query, queryOptions);
}); });
it('should replace $timefilter', () => { it('should replace $timefilter', () => {
...@@ -67,6 +69,10 @@ describe('InfluxDataSource', () => { ...@@ -67,6 +69,10 @@ describe('InfluxDataSource', () => {
it('should not have any data in request body', () => { it('should not have any data in request body', () => {
expect(requestData).toBeNull(); expect(requestData).toBeNull();
}); });
it('parse response correctly', () => {
expect(response).toEqual([{ text: 'cpu' }]);
});
}); });
describe('InfluxDataSource in POST query mode', () => { describe('InfluxDataSource in POST query mode', () => {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment