diff --git a/.gitignore b/.gitignore index 454dd0e0cee39..63c9519e20cd2 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,7 @@ .eslintrc.js .idea .npm +.npmrc .prettierignore .yarnclean diff --git a/package.json b/package.json index ce2a19355177e..1ac4adedd9954 100644 --- a/package.json +++ b/package.json @@ -5,13 +5,12 @@ "private": true, "scripts": { "build": "lerna run build", - "jest": "lerna run test", "lint": "lerna run lint", "lint:fix": "lerna run lint:fix", "prerelease": "yarn run build", - "prepare-release": "git checkout master && git pull --rebase origin master && yarn run test", + "prepare-release": "git checkout master && git pull --rebase origin master && lerna bootstrap && yarn run lint && yarn run test", "release": "yarn run prepare-release && lerna publish && lerna run gh-pages", - "test": "lerna bootstrap && yarn run lint && yarn run jest" + "test": "lerna run test" }, "repository": "https://github.com/apache-superset/superset-ui.git", "keywords": [ diff --git a/packages/superset-ui-core/package.json b/packages/superset-ui-core/package.json index 4f96f1e24da8d..4d03e822c690f 100644 --- a/packages/superset-ui-core/package.json +++ b/packages/superset-ui-core/package.json @@ -40,7 +40,7 @@ }, "homepage": "https://github.com/apache-superset/superset-ui#readme", "devDependencies": { - "@data-ui/build-config": "^0.0.12", + "@data-ui/build-config": "^0.0.14", "fetch-mock": "^6.5.2", "node-fetch": "^2.2.0" }, diff --git a/packages/superset-ui-core/src/SupersetClient.js b/packages/superset-ui-core/src/SupersetClient.js index aad85a1a3ba81..e75982776604d 100644 --- a/packages/superset-ui-core/src/SupersetClient.js +++ b/packages/superset-ui-core/src/SupersetClient.js @@ -75,7 +75,7 @@ class SupersetClient { ); } - get({ host, url, endpoint, mode, credentials, headers, body, timeout, signal }) { + get({ body, credentials, headers, host, endpoint, mode, parseMethod, signal, timeout, url }) { return this.ensureAuth().then(() => callApi({ body, @@ -83,6 +83,7 @@ class SupersetClient { headers: { ...this.headers, ...headers }, method: 'GET', mode: mode || this.mode, + parseMethod, signal, timeout: timeout || this.timeout, url: url || this.getUrl({ endpoint, host: host || this.host }), @@ -91,16 +92,17 @@ class SupersetClient { } post({ + credentials, + headers, host, endpoint, - url, mode, - credentials, - headers, + parseMethod, postPayload, - timeout, signal, stringify, + timeout, + url, }) { return this.ensureAuth().then(() => callApi({ @@ -108,6 +110,7 @@ class SupersetClient { headers: { ...this.headers, ...headers }, method: 'POST', mode: mode || this.mode, + parseMethod, postPayload, signal, stringify, diff --git a/packages/superset-ui-core/src/callApi/callApi.js b/packages/superset-ui-core/src/callApi/callApi.js index ed5dca15ed78f..65efd2ff30221 100644 --- a/packages/superset-ui-core/src/callApi/callApi.js +++ b/packages/superset-ui-core/src/callApi/callApi.js @@ -15,7 +15,6 @@ export default function callApi({ postPayload, stringify = true, redirect = 'follow', // manual, follow, error - timeoutId, signal, // used for aborting }) { let request = { diff --git a/packages/superset-ui-core/src/callApi/callApiAndParseWithTimeout.js b/packages/superset-ui-core/src/callApi/callApiAndParseWithTimeout.js index 8f41a8c06a899..7af0ab88509c4 100644 --- a/packages/superset-ui-core/src/callApi/callApiAndParseWithTimeout.js +++ b/packages/superset-ui-core/src/callApi/callApiAndParseWithTimeout.js @@ -2,7 +2,7 @@ import callApi from './callApi'; import rejectAfterTimeout from './rejectAfterTimeout'; import parseResponse from './parseResponse'; -export default function callApiAndParseWithTimeout({ timeout, ...rest }) { +export default function callApiAndParseWithTimeout({ timeout, parseMethod, ...rest }) { const apiPromise = callApi(rest); const racedPromise = @@ -10,5 +10,5 @@ export default function callApiAndParseWithTimeout({ timeout, ...rest }) { ? Promise.race([rejectAfterTimeout(timeout), apiPromise]) : apiPromise; - return parseResponse(racedPromise); + return parseResponse(racedPromise, parseMethod); } diff --git a/packages/superset-ui-core/src/callApi/parseResponse.js b/packages/superset-ui-core/src/callApi/parseResponse.js index 07b680e4b3f55..1235f651fd602 100644 --- a/packages/superset-ui-core/src/callApi/parseResponse.js +++ b/packages/superset-ui-core/src/callApi/parseResponse.js @@ -1,26 +1,25 @@ -export default function parseResponse(apiPromise) { - return apiPromise.then(apiResponse => - // first try to parse as json, and fall back to text (e.g., in the case of HTML stacktrace) - // cannot fall back to .text() without cloning the response because body is single-use - apiResponse - .clone() - .json() - .catch(() => /* jsonParseError */ apiResponse.text().then(textPayload => ({ textPayload }))) - .then(maybeJson => ({ - json: maybeJson.textPayload ? undefined : maybeJson, - response: apiResponse, - text: maybeJson.textPayload, - })) - .then(({ response, json, text }) => { - if (!response.ok) { - return Promise.reject({ - error: response.error || (json && json.error) || text || 'An error occurred', - status: response.status, - statusText: response.statusText, - }); - } +const PARSERS = { + json: response => response.json().then(json => ({ json, response })), + text: response => response.text().then(text => ({ response, text })), +}; - return typeof text === 'undefined' ? { json, response } : { response, text }; - }), - ); +export default function parseResponse(apiPromise, parseMethod = 'json') { + if (parseMethod === null) return apiPromise; + + const responseParser = PARSERS[parseMethod] || PARSERS.json; + + return apiPromise + .then(response => { + if (!response.ok) { + return Promise.reject({ + error: response.error || 'An error occurred', + response, + status: response.status, + statusText: response.statusText, + }); + } + + return response; + }) + .then(responseParser); } diff --git a/packages/superset-ui-core/test/SupersetClient.test.js b/packages/superset-ui-core/test/SupersetClient.test.js index 5a95df737ebec..bc6bde0807350 100644 --- a/packages/superset-ui-core/test/SupersetClient.test.js +++ b/packages/superset-ui-core/test/SupersetClient.test.js @@ -1,4 +1,3 @@ -/* eslint promise/no-callback-in-promise: 'off' */ import fetchMock from 'fetch-mock'; import PublicAPI, { SupersetClient } from '../src/SupersetClient'; @@ -80,36 +79,30 @@ describe('SupersetClient', () => { describe('CSRF', () => { afterEach(fetchMock.reset); - it('calls superset/csrf_token/ upon initialization', done => { + it('calls superset/csrf_token/ upon initialization', () => { expect.assertions(1); const client = new SupersetClient({}); - client - .init() - .then(() => { - expect(fetchMock.calls(LOGIN_GLOB)).toHaveLength(1); + return client.init().then(() => { + expect(fetchMock.calls(LOGIN_GLOB)).toHaveLength(1); - return done(); - }) - .catch(throwIfCalled); + return Promise.resolve(); + }); }); - it('isAuthenticated() returns true if there is a token and false if not', done => { + it('isAuthenticated() returns true if there is a token and false if not', () => { expect.assertions(2); const client = new SupersetClient({}); expect(client.isAuthenticated()).toBe(false); - client - .init() - .then(() => { - expect(client.isAuthenticated()).toBe(true); + return client.init().then(() => { + expect(client.isAuthenticated()).toBe(true); - return done(); - }) - .catch(throwIfCalled); + return Promise.resolve(); + }); }); - it('init() throws if superset/csrf_token/ returns an error', done => { + it('init() throws if superset/csrf_token/ returns an error', () => { expect.assertions(1); fetchMock.get(LOGIN_GLOB, () => Promise.reject({ status: 403 }), { @@ -118,7 +111,7 @@ describe('SupersetClient', () => { const client = new SupersetClient({}); - client + return client .init() .then(throwIfCalled) .catch(error => { @@ -133,16 +126,17 @@ describe('SupersetClient', () => { }, ); - return done(); + return Promise.resolve(); }); }); - it('init() throws if superset/csrf_token/ does not return a token', done => { + it('init() throws if superset/csrf_token/ does not return a token', () => { expect.assertions(1); fetchMock.get(LOGIN_GLOB, {}, { overwriteRoutes: true }); const client = new SupersetClient({}); - client + + return client .init() .then(throwIfCalled) .catch(error => { @@ -157,49 +151,46 @@ describe('SupersetClient', () => { }, ); - return done(); + return Promise.resolve(); }); }); }); describe('CSRF queuing', () => { - it(`client.ensureAuth() returns a promise that rejects init() has not been called`, done => { + it(`client.ensureAuth() returns a promise that rejects init() has not been called`, () => { expect.assertions(2); const client = new SupersetClient({}); - client + return client .ensureAuth() .then(throwIfCalled) .catch(error => { expect(error).toEqual(expect.objectContaining({ error: expect.any(String) })); expect(client.didAuthSuccessfully).toBe(false); - return done(); + return Promise.resolve(); }); }); - it('client.ensureAuth() returns a promise that resolves if client.init() resolves successfully', done => { + it('client.ensureAuth() returns a promise that resolves if client.init() resolves successfully', () => { expect.assertions(1); const client = new SupersetClient({}); - client - .init() - .then(() => - client - .ensureAuth() - .then(throwIfCalled) - .catch(() => { - expect(client.didAuthSuccessfully).toBe(true); - - return done(); - }), - ) - .catch(throwIfCalled); + return client.init().then(() => + client + .ensureAuth() + .then(throwIfCalled) + .catch(() => { + expect(client.didAuthSuccessfully).toBe(true); + + return Promise.resolve(); + }), + ); }); - it(`client.ensureAuth() returns a promise that rejects if init() is unsuccessful`, done => { + it(`client.ensureAuth() returns a promise that rejects if init() is unsuccessful`, () => { const rejectValue = { status: 403 }; fetchMock.get(LOGIN_GLOB, () => Promise.reject(rejectValue), { overwriteRoutes: true, @@ -209,7 +200,7 @@ describe('SupersetClient', () => { const client = new SupersetClient({}); - client + return client .init() .then(throwIfCalled) .catch(error => { @@ -231,7 +222,7 @@ describe('SupersetClient', () => { }, ); - return done(); + return Promise.resolve(); }); }); }); @@ -243,35 +234,37 @@ describe('SupersetClient', () => { const host = 'HOST'; const mockGetEndpoint = '/get/url'; const mockPostEndpoint = '/post/url'; + const mockTextEndpoint = '/text/endpoint'; const mockGetUrl = `${protocol}://${host}${mockGetEndpoint}`; const mockPostUrl = `${protocol}://${host}${mockPostEndpoint}`; + const mockTextUrl = `${protocol}://${host}${mockTextEndpoint}`; + const mockTextJsonResponse = '{ "value": 9223372036854775807 }'; - fetchMock.get(mockGetUrl, 'Ok'); - fetchMock.post(mockPostUrl, 'Ok'); + fetchMock.get(mockGetUrl, { json: 'payload' }); + fetchMock.post(mockPostUrl, { json: 'payload' }); + fetchMock.get(mockTextUrl, mockTextJsonResponse); + fetchMock.post(mockTextUrl, mockTextJsonResponse); - it('checks for authentication before every get and post request', done => { + it('checks for authentication before every get and post request', () => { expect.assertions(3); const authSpy = jest.spyOn(SupersetClient.prototype, 'ensureAuth'); const client = new SupersetClient({ protocol, host }); - client - .init() - .then(() => - Promise.all([client.get({ url: mockGetUrl }), client.post({ url: mockPostUrl })]) - .then(() => { - expect(fetchMock.calls(mockGetUrl)).toHaveLength(1); - expect(fetchMock.calls(mockPostUrl)).toHaveLength(1); - expect(authSpy).toHaveBeenCalledTimes(2); - authSpy.mockRestore(); - - return done(); - }) - .catch(throwIfCalled), - ) - .catch(throwIfCalled); + return client.init().then(() => + Promise.all([client.get({ url: mockGetUrl }), client.post({ url: mockPostUrl })]).then( + () => { + expect(fetchMock.calls(mockGetUrl)).toHaveLength(1); + expect(fetchMock.calls(mockPostUrl)).toHaveLength(1); + expect(authSpy).toHaveBeenCalledTimes(2); + authSpy.mockRestore(); + + return Promise.resolve(); + }, + ), + ); }); - it('sets protocol, host, headers, mode, and credentials from config', done => { + it('sets protocol, host, headers, mode, and credentials from config', () => { expect.assertions(3); const clientConfig = { host, @@ -282,46 +275,57 @@ describe('SupersetClient', () => { }; const client = new SupersetClient(clientConfig); - client - .init() - .then(() => - client - .get({ url: mockGetUrl }) - .then(() => { - const fetchRequest = fetchMock.calls(mockGetUrl)[0][1]; - expect(fetchRequest.mode).toBe(clientConfig.mode); - expect(fetchRequest.credentials).toBe(clientConfig.credentials); - expect(fetchRequest.headers).toEqual(expect.objectContaining(clientConfig.headers)); - - return done(); - }) - .catch(throwIfCalled), - ) - .catch(throwIfCalled); + + return client.init().then(() => + client.get({ url: mockGetUrl }).then(() => { + const fetchRequest = fetchMock.calls(mockGetUrl)[0][1]; + expect(fetchRequest.mode).toBe(clientConfig.mode); + expect(fetchRequest.credentials).toBe(clientConfig.credentials); + expect(fetchRequest.headers).toEqual(expect.objectContaining(clientConfig.headers)); + + return Promise.resolve(); + }), + ); }); describe('GET', () => { - it('makes a request using url or endpoint', done => { + it('makes a request using url or endpoint', () => { expect.assertions(1); const client = new SupersetClient({ protocol, host }); - client + + return client.init().then(() => + Promise.all([ + client.get({ url: mockGetUrl }), + client.get({ endpoint: mockGetEndpoint }), + ]).then(() => { + expect(fetchMock.calls(mockGetUrl)).toHaveLength(2); + + return Promise.resolve(); + }), + ); + }); + + it('supports parsing a response as text', () => { + expect.assertions(2); + const client = new SupersetClient({ protocol, host }); + + return client .init() .then(() => - Promise.all([ - client.get({ url: mockGetUrl }), - client.get({ endpoint: mockGetEndpoint }), - ]) - .then(() => { - expect(fetchMock.calls(mockGetUrl)).toHaveLength(2); + client + .get({ url: mockTextUrl, parseMethod: 'text' }) + .then(({ text }) => { + expect(fetchMock.calls(mockTextUrl)).toHaveLength(1); + expect(text).toBe(mockTextJsonResponse); - return done(); + return Promise.resolve(); }) .catch(throwIfCalled), ) .catch(throwIfCalled); }); - it('allows overriding host, headers, mode, and credentials per-request', done => { + it('allows overriding host, headers, mode, and credentials per-request', () => { expect.assertions(3); const clientConfig = { host, @@ -339,7 +343,8 @@ describe('SupersetClient', () => { }; const client = new SupersetClient(clientConfig); - client + + return client .init() .then(() => client @@ -352,7 +357,7 @@ describe('SupersetClient', () => { expect.objectContaining(overrideConfig.headers), ); - return done(); + return Promise.resolve(); }) .catch(throwIfCalled), ) @@ -361,27 +366,23 @@ describe('SupersetClient', () => { }); describe('POST', () => { - it('makes a request using url or endpoint', done => { + it('makes a request using url or endpoint', () => { expect.assertions(1); const client = new SupersetClient({ protocol, host }); - client - .init() - .then(() => - Promise.all([ - client.post({ url: mockPostUrl }), - client.post({ endpoint: mockPostEndpoint }), - ]) - .then(() => { - expect(fetchMock.calls(mockPostUrl)).toHaveLength(2); - return done(); - }) - .catch(throwIfCalled), - ) - .catch(throwIfCalled); + return client.init().then(() => + Promise.all([ + client.post({ url: mockPostUrl }), + client.post({ endpoint: mockPostEndpoint }), + ]).then(() => { + expect(fetchMock.calls(mockPostUrl)).toHaveLength(2); + + return Promise.resolve(); + }), + ); }); - it('allows overriding host, headers, mode, and credentials per-request', done => { + it('allows overriding host, headers, mode, and credentials per-request', () => { const clientConfig = { host, protocol, @@ -398,72 +399,68 @@ describe('SupersetClient', () => { }; const client = new SupersetClient(clientConfig); - client - .init() - .then(() => - client - .post({ url: mockPostUrl, ...overrideConfig }) - .then(() => { - const fetchRequest = fetchMock.calls(mockPostUrl)[0][1]; - expect(fetchRequest.mode).toBe(overrideConfig.mode); - expect(fetchRequest.credentials).toBe(overrideConfig.credentials); - expect(fetchRequest.headers).toEqual( - expect.objectContaining(overrideConfig.headers), - ); - return done(); - }) - .catch(throwIfCalled), - ) - .catch(throwIfCalled); + return client.init().then(() => + client.post({ url: mockPostUrl, ...overrideConfig }).then(() => { + const fetchRequest = fetchMock.calls(mockPostUrl)[0][1]; + expect(fetchRequest.mode).toBe(overrideConfig.mode); + expect(fetchRequest.credentials).toBe(overrideConfig.credentials); + expect(fetchRequest.headers).toEqual(expect.objectContaining(overrideConfig.headers)); + + return Promise.resolve(); + }), + ); }); - it('passes postPayload key,values in the body', done => { + it('supports parsing a response as text', () => { + expect.assertions(2); + const client = new SupersetClient({ protocol, host }); + + return client.init().then(() => + client.post({ url: mockTextUrl, parseMethod: 'text' }).then(({ text }) => { + expect(fetchMock.calls(mockTextUrl)).toHaveLength(1); + expect(text).toBe(mockTextJsonResponse); + + return Promise.resolve(); + }), + ); + }); + + it('passes postPayload key,values in the body', () => { expect.assertions(3); const postPayload = { number: 123, array: [1, 2, 3] }; const client = new SupersetClient({ protocol, host }); - client - .init() - .then(() => - client - .post({ url: mockPostUrl, postPayload }) - .then(() => { - const formData = fetchMock.calls(mockPostUrl)[0][1].body; - expect(fetchMock.calls(mockPostUrl)).toHaveLength(1); - Object.keys(postPayload).forEach(key => { - expect(formData.get(key)).toBe(JSON.stringify(postPayload[key])); - }); - return done(); - }) - .catch(throwIfCalled), - ) - .catch(throwIfCalled); + return client.init().then(() => + client.post({ url: mockPostUrl, postPayload }).then(() => { + const formData = fetchMock.calls(mockPostUrl)[0][1].body; + expect(fetchMock.calls(mockPostUrl)).toHaveLength(1); + Object.keys(postPayload).forEach(key => { + expect(formData.get(key)).toBe(JSON.stringify(postPayload[key])); + }); + + return Promise.resolve(); + }), + ); }); - it('respects the stringify parameter for postPayload key,values', done => { + it('respects the stringify parameter for postPayload key,values', () => { expect.assertions(3); const postPayload = { number: 123, array: [1, 2, 3] }; const client = new SupersetClient({ protocol, host }); - client - .init() - .then(() => - client - .post({ url: mockPostUrl, postPayload, stringify: false }) - .then(() => { - const formData = fetchMock.calls(mockPostUrl)[0][1].body; - expect(fetchMock.calls(mockPostUrl)).toHaveLength(1); - Object.keys(postPayload).forEach(key => { - expect(formData.get(key)).toBe(String(postPayload[key])); - }); + return client.init().then(() => + client.post({ url: mockPostUrl, postPayload, stringify: false }).then(() => { + const formData = fetchMock.calls(mockPostUrl)[0][1].body; + expect(fetchMock.calls(mockPostUrl)).toHaveLength(1); + Object.keys(postPayload).forEach(key => { + expect(formData.get(key)).toBe(String(postPayload[key])); + }); - return done(); - }) - .catch(throwIfCalled), - ) - .catch(throwIfCalled); + return Promise.resolve(); + }), + ); }); }); }); diff --git a/packages/superset-ui-core/test/callApi/callApi.test.js b/packages/superset-ui-core/test/callApi/callApi.test.js index d4c670ccccd0b..2a54229af82f8 100644 --- a/packages/superset-ui-core/test/callApi/callApi.test.js +++ b/packages/superset-ui-core/test/callApi/callApi.test.js @@ -27,23 +27,21 @@ describe('callApi()', () => { afterEach(fetchMock.reset); describe('request config', () => { - it('calls the right url with the specified method', done => { + it('calls the right url with the specified method', () => { expect.assertions(2); - Promise.all([ + return Promise.all([ callApi({ url: mockGetUrl, method: 'GET' }), callApi({ url: mockPostUrl, method: 'POST' }), - ]) - .then(() => { - expect(fetchMock.calls(mockGetUrl)).toHaveLength(1); - expect(fetchMock.calls(mockPostUrl)).toHaveLength(1); - - return done(); - }) - .catch(throwIfCalled); + ]).then(() => { + expect(fetchMock.calls(mockGetUrl)).toHaveLength(1); + expect(fetchMock.calls(mockPostUrl)).toHaveLength(1); + + return Promise.resolve(); + }); }); - it('passes along mode, cache, credentials, headers, body, signal, and redirect parameters in the request', done => { + it('passes along mode, cache, credentials, headers, body, signal, and redirect parameters in the request', () => { expect.assertions(8); const mockRequest = { @@ -59,68 +57,62 @@ describe('callApi()', () => { body: 'BODY', }; - callApi(mockRequest) - .then(() => { - const calls = fetchMock.calls(mockGetUrl); - const fetchParams = calls[0][1]; - expect(calls).toHaveLength(1); - expect(fetchParams.mode).toBe(mockRequest.mode); - expect(fetchParams.cache).toBe(mockRequest.cache); - expect(fetchParams.credentials).toBe(mockRequest.credentials); - expect(fetchParams.headers).toEqual(expect.objectContaining(mockRequest.headers)); - expect(fetchParams.redirect).toBe(mockRequest.redirect); - expect(fetchParams.signal).toBe(mockRequest.signal); - expect(fetchParams.body).toBe(mockRequest.body); - - return done(); - }) - .catch(throwIfCalled); + return callApi(mockRequest).then(() => { + const calls = fetchMock.calls(mockGetUrl); + const fetchParams = calls[0][1]; + expect(calls).toHaveLength(1); + expect(fetchParams.mode).toBe(mockRequest.mode); + expect(fetchParams.cache).toBe(mockRequest.cache); + expect(fetchParams.credentials).toBe(mockRequest.credentials); + expect(fetchParams.headers).toEqual(expect.objectContaining(mockRequest.headers)); + expect(fetchParams.redirect).toBe(mockRequest.redirect); + expect(fetchParams.signal).toBe(mockRequest.signal); + expect(fetchParams.body).toBe(mockRequest.body); + + return Promise.resolve(); + }); }); }); describe('POST requests', () => { - it('encodes key,value pairs from postPayload', done => { + it('encodes key,value pairs from postPayload', () => { expect.assertions(3); const postPayload = { key: 'value', anotherKey: 1237 }; - callApi({ url: mockPostUrl, method: 'POST', postPayload }) - .then(() => { - const calls = fetchMock.calls(mockPostUrl); - expect(calls).toHaveLength(1); + return callApi({ url: mockPostUrl, method: 'POST', postPayload }).then(() => { + const calls = fetchMock.calls(mockPostUrl); + expect(calls).toHaveLength(1); - const fetchParams = calls[0][1]; - const { body } = fetchParams; + const fetchParams = calls[0][1]; + const { body } = fetchParams; - Object.keys(postPayload).forEach(key => { - expect(body.get(key)).toBe(JSON.stringify(postPayload[key])); - }); + Object.keys(postPayload).forEach(key => { + expect(body.get(key)).toBe(JSON.stringify(postPayload[key])); + }); - return done(); - }) - .catch(throwIfCalled); + return Promise.resolve(); + }); }); // the reason for this is to omit strings like 'undefined' from making their way to the backend - it('omits key,value pairs from postPayload that have undefined values', done => { + it('omits key,value pairs from postPayload that have undefined values', () => { expect.assertions(3); const postPayload = { key: 'value', noValue: undefined }; - callApi({ url: mockPostUrl, method: 'POST', postPayload }) - .then(() => { - const calls = fetchMock.calls(mockPostUrl); - expect(calls).toHaveLength(1); + return callApi({ url: mockPostUrl, method: 'POST', postPayload }).then(() => { + const calls = fetchMock.calls(mockPostUrl); + expect(calls).toHaveLength(1); - const fetchParams = calls[0][1]; - const { body } = fetchParams; - expect(body.get('key')).toBe(JSON.stringify(postPayload.key)); - expect(body.get('noValue')).toBeNull(); + const fetchParams = calls[0][1]; + const { body } = fetchParams; + expect(body.get('key')).toBe(JSON.stringify(postPayload.key)); + expect(body.get('noValue')).toBeNull(); - return done(); - }) - .catch(throwIfCalled); + return Promise.resolve(); + }); }); - it('respects the stringify flag in POST requests', done => { + it('respects the stringify flag in POST requests', () => { const postPayload = { string: 'value', number: 1237, @@ -132,25 +124,35 @@ describe('callApi()', () => { expect.assertions(1 + 2 * Object.keys(postPayload).length); - Promise.all([ + return Promise.all([ callApi({ url: mockPostUrl, method: 'POST', postPayload }), callApi({ url: mockPostUrl, method: 'POST', postPayload, stringify: false }), - ]) - .then(() => { - const calls = fetchMock.calls(mockPostUrl); - expect(calls).toHaveLength(2); - - const stringified = calls[0][1].body; - const unstringified = calls[1][1].body; - - Object.keys(postPayload).forEach(key => { - expect(stringified.get(key)).toBe(JSON.stringify(postPayload[key])); - expect(unstringified.get(key)).toBe(String(postPayload[key])); - }); - - return done(); - }) - .catch(throwIfCalled); + ]).then(() => { + const calls = fetchMock.calls(mockPostUrl); + expect(calls).toHaveLength(2); + + const stringified = calls[0][1].body; + const unstringified = calls[1][1].body; + + Object.keys(postPayload).forEach(key => { + expect(stringified.get(key)).toBe(JSON.stringify(postPayload[key])); + expect(unstringified.get(key)).toBe(String(postPayload[key])); + }); + + return Promise.resolve(); + }); }); }); + + it('rejects if the request throws', () => { + expect.assertions(3); + + return callApi({ url: mockErrorUrl, method: 'GET' }) + .then(throwIfCalled) + .catch(error => { + expect(fetchMock.calls(mockErrorUrl)).toHaveLength(1); + expect(error.status).toBe(mockErrorPayload.status); + expect(error.statusText).toBe(mockErrorPayload.statusText); + }); + }); }); diff --git a/packages/superset-ui-core/test/callApi/callApiAndParseWithTimeout.test.js b/packages/superset-ui-core/test/callApi/callApiAndParseWithTimeout.test.js index a9006d9b4dc96..8cb21b249bbff 100644 --- a/packages/superset-ui-core/test/callApi/callApiAndParseWithTimeout.test.js +++ b/packages/superset-ui-core/test/callApi/callApiAndParseWithTimeout.test.js @@ -1,4 +1,3 @@ -/* eslint promise/no-callback-in-promise: 'off' */ import fetchMock from 'fetch-mock'; import callApiAndParseWithTimeout from '../../src/callApi/callApiAndParseWithTimeout'; @@ -80,23 +79,23 @@ describe('callApiAndParseWithTimeout()', () => { ); expect(timeoutError.statusText).toBe('timeout'); - return done(); + return done(); // eslint-disable-line promise/no-callback-in-promise }); jest.runOnlyPendingTimers(); }); - it('resolves if the request does not exceed the timeout', done => { + it('resolves if the request does not exceed the timeout', () => { expect.assertions(1); jest.useFakeTimers(); - callApiAndParseWithTimeout({ url: mockGetUrl, method: 'GET', timeout: 100 }) - .then(response => { + return callApiAndParseWithTimeout({ url: mockGetUrl, method: 'GET', timeout: 100 }).then( + response => { expect(response.json).toEqual(expect.objectContaining(mockGetPayload)); - return done(); - }) - .catch(throwIfCalled); + return Promise.resolve(); + }, + ); }); }); }); diff --git a/packages/superset-ui-core/test/callApi/parseResponse.test.js b/packages/superset-ui-core/test/callApi/parseResponse.test.js index de288a28083ca..809e219f68571 100644 --- a/packages/superset-ui-core/test/callApi/parseResponse.test.js +++ b/packages/superset-ui-core/test/callApi/parseResponse.test.js @@ -1,4 +1,3 @@ -/* eslint promise/no-callback-in-promise: 'off' */ import fetchMock from 'fetch-mock'; import callApi from '../../src/callApi/callApi'; import parseResponse from '../../src/callApi/parseResponse'; @@ -33,22 +32,20 @@ describe('parseResponse()', () => { expect(parsedResponsePromise).toEqual(expect.any(Promise)); }); - it('resolves to { json, response } if the request succeeds', done => { + it('resolves to { json, response } if the request succeeds', () => { expect.assertions(3); const apiPromise = callApi({ url: mockGetUrl, method: 'GET' }); - parseResponse(apiPromise) - .then(args => { - expect(fetchMock.calls(mockGetUrl)).toHaveLength(1); - expect(Object.keys(args)).toEqual(expect.arrayContaining(['response', 'json'])); - expect(args.json).toEqual(expect.objectContaining(mockGetPayload)); + return parseResponse(apiPromise).then(args => { + expect(fetchMock.calls(mockGetUrl)).toHaveLength(1); + expect(Object.keys(args)).toEqual(expect.arrayContaining(['response', 'json'])); + expect(args.json).toEqual(expect.objectContaining(mockGetPayload)); - return done(); - }) - .catch(throwIfCalled); + return Promise.resolve(); + }); }); - it('resolves to { text, response } if the request succeeds with text response', done => { + it('throws if `parseMethod=json` and .json() fails', () => { expect.assertions(3); const mockTextUrl = '/mock/text/url'; @@ -57,28 +54,51 @@ describe('parseResponse()', () => { fetchMock.get(mockTextUrl, mockTextResponse); const apiPromise = callApi({ url: mockTextUrl, method: 'GET' }); - parseResponse(apiPromise) - .then(args => { + + return parseResponse(apiPromise, 'json') + .then(throwIfCalled) + .catch(error => { expect(fetchMock.calls(mockTextUrl)).toHaveLength(1); - expect(Object.keys(args)).toEqual(expect.arrayContaining(['response', 'text'])); - expect(args.text).toBe(mockTextResponse); + expect(error.stack).toBeDefined(); + expect(error.message.includes('Unexpected token')).toBe(true); - return done(); - }) - .catch(throwIfCalled); + return Promise.resolve(); + }); }); - it('rejects if the request throws', done => { + it('resolves to { text, response } if the `parseMethod=text`', () => { expect.assertions(3); - callApi({ url: mockErrorUrl, method: 'GET' }) - .then(throwIfCalled) - .catch(error => { - expect(fetchMock.calls(mockErrorUrl)).toHaveLength(1); - expect(error.status).toBe(mockErrorPayload.status); - expect(error.statusText).toBe(mockErrorPayload.statusText); + // test with json + bigint to ensure that it was not first parsed as json + const mockTextParseUrl = '/mock/textparse/url'; + const mockTextJsonResponse = '{ "value": 9223372036854775807 }'; + fetchMock.get(mockTextParseUrl, mockTextJsonResponse); - return done(); - }); + const apiPromise = callApi({ url: mockTextParseUrl, method: 'GET' }); + + return parseResponse(apiPromise, 'text').then(args => { + expect(fetchMock.calls(mockTextParseUrl)).toHaveLength(1); + expect(Object.keys(args)).toEqual(expect.arrayContaining(['response', 'text'])); + expect(args.text).toBe(mockTextJsonResponse); + + return Promise.resolve(); + }); + }); + + it('resolves to the unmodified `Response` object if `parseMethod=null`', () => { + expect.assertions(2); + + const mockNoParseUrl = '/mock/noparse/url'; + const mockResponse = new Response('test response'); + fetchMock.get(mockNoParseUrl, mockResponse); + + const apiPromise = callApi({ url: mockNoParseUrl, method: 'GET' }); + + return parseResponse(apiPromise, null).then(response => { + expect(fetchMock.calls(mockNoParseUrl)).toHaveLength(1); + expect(response.bodyUsed).toBe(false); + + return Promise.resolve(); + }); }); }); diff --git a/packages/superset-ui-core/test/utils/throwIfCalled.js b/packages/superset-ui-core/test/utils/throwIfCalled.js index 41079b95ee99f..e3b4f694c912b 100644 --- a/packages/superset-ui-core/test/utils/throwIfCalled.js +++ b/packages/superset-ui-core/test/utils/throwIfCalled.js @@ -1,3 +1,3 @@ -export default function throwIfCalled() { - throw new Error('Unexpected call to throwIfCalled()'); +export default function throwIfCalled(args) { + throw new Error(`Unexpected call to throwIfCalled(): ${JSON.stringify(args)}`); }