From 21b507473b8ec892c1c5867121a375bd49fc1fef Mon Sep 17 00:00:00 2001 From: Chris Williams Date: Wed, 12 Sep 2018 18:07:35 -0700 Subject: [PATCH 1/9] [core] expose 'parseMethod' in SupersetClient and callApi for text responses --- packages/superset-ui-core/package.json | 2 +- .../superset-ui-core/src/SupersetClient.js | 13 +++--- .../superset-ui-core/src/callApi/callApi.js | 1 - .../src/callApi/callApiAndParseWithTimeout.js | 4 +- .../src/callApi/parseResponse.js | 33 ++++++++------ .../test/SupersetClient.test.js | 43 +++++++++++++++++++ .../test/callApi/parseResponse.test.js | 20 +++++++++ 7 files changed, 95 insertions(+), 21 deletions(-) diff --git a/packages/superset-ui-core/package.json b/packages/superset-ui-core/package.json index 4f96f1e24d..079de03d1a 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.13", "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 aad85a1a3b..e759827766 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 ed5dca15ed..65efd2ff30 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 8f41a8c06a..7af0ab8850 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 07b680e4b3..cdd44c5c9f 100644 --- a/packages/superset-ui-core/src/callApi/parseResponse.js +++ b/packages/superset-ui-core/src/callApi/parseResponse.js @@ -1,17 +1,26 @@ -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 }))) +export default function parseResponse(apiPromise, parseMethod = 'json') { + return apiPromise.then(response => { + let parsedPromise; + if (parseMethod === 'json') { + // 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 + parsedPromise = response + .clone() + .json() + .catch(() => /* jsonParseError */ response.text().then(textPayload => ({ textPayload }))); + } else if (parseMethod === 'text') { + parsedPromise = response.text().then(textPayload => ({ textPayload })); + } else { + throw Error(`Unrecognized parseMethod '${parseMethod}', expected 'json' or 'text'`); + } + + return parsedPromise .then(maybeJson => ({ json: maybeJson.textPayload ? undefined : maybeJson, - response: apiResponse, text: maybeJson.textPayload, })) - .then(({ response, json, text }) => { + .then(({ json, text }) => { + // HTTP 404 or 500 are not rejected, ok is just set to false if (!response.ok) { return Promise.reject({ error: response.error || (json && json.error) || text || 'An error occurred', @@ -21,6 +30,6 @@ export default function parseResponse(apiPromise) { } return typeof text === 'undefined' ? { json, response } : { response, text }; - }), - ); + }); + }); } diff --git a/packages/superset-ui-core/test/SupersetClient.test.js b/packages/superset-ui-core/test/SupersetClient.test.js index 5a95df737e..6dcc685f3c 100644 --- a/packages/superset-ui-core/test/SupersetClient.test.js +++ b/packages/superset-ui-core/test/SupersetClient.test.js @@ -243,11 +243,16 @@ 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(mockTextUrl, mockTextJsonResponse); + fetchMock.post(mockTextUrl, mockTextJsonResponse); it('checks for authentication before every get and post request', done => { expect.assertions(3); @@ -321,6 +326,25 @@ describe('SupersetClient', () => { .catch(throwIfCalled); }); + it('supports parsing a response as text', done => { + expect.assertions(2); + const client = new SupersetClient({ protocol, host }); + client + .init() + .then(() => + client + .get({ url: mockTextUrl, parseMethod: 'text' }) + .then(({ text }) => { + expect(fetchMock.calls(mockTextUrl)).toHaveLength(1); + expect(text).toBe(mockTextJsonResponse); + + return done(); + }) + .catch(throwIfCalled), + ) + .catch(throwIfCalled); + }); + it('allows overriding host, headers, mode, and credentials per-request', done => { expect.assertions(3); const clientConfig = { @@ -418,6 +442,25 @@ describe('SupersetClient', () => { .catch(throwIfCalled); }); + it('supports parsing a response as text', done => { + expect.assertions(2); + const client = new SupersetClient({ protocol, host }); + client + .init() + .then(() => + client + .post({ url: mockTextUrl, parseMethod: 'text' }) + .then(({ text }) => { + expect(fetchMock.calls(mockTextUrl)).toHaveLength(1); + expect(text).toBe(mockTextJsonResponse); + + return done(); + }) + .catch(throwIfCalled), + ) + .catch(throwIfCalled); + }); + it('passes postPayload key,values in the body', done => { expect.assertions(3); diff --git a/packages/superset-ui-core/test/callApi/parseResponse.test.js b/packages/superset-ui-core/test/callApi/parseResponse.test.js index de288a2808..f0dc9c2e00 100644 --- a/packages/superset-ui-core/test/callApi/parseResponse.test.js +++ b/packages/superset-ui-core/test/callApi/parseResponse.test.js @@ -68,6 +68,26 @@ describe('parseResponse()', () => { .catch(throwIfCalled); }); + it('resolves to { text, response } if the parseMethod is set to `text`', done => { + expect.assertions(3); + + // test with json + bigint to ensure that it was not first parsed as json + const mockTextUrl = '/mock/textparse/url'; + const mockTextJsonResponse = '{ "value": 9223372036854775807 }'; + fetchMock.get(mockTextUrl, mockTextJsonResponse); + + const apiPromise = callApi({ url: mockTextUrl, method: 'GET', parseMethod: 'text' }); + parseResponse(apiPromise) + .then(args => { + expect(fetchMock.calls(mockTextUrl)).toHaveLength(1); + expect(Object.keys(args)).toEqual(expect.arrayContaining(['response', 'text'])); + expect(args.text).toBe(mockTextJsonResponse); + + return done(); + }) + .catch(throwIfCalled); + }); + it('rejects if the request throws', done => { expect.assertions(3); From 397240e3460302ceb2b2116a7dae84c3c9a64017 Mon Sep 17 00:00:00 2001 From: Chris Williams Date: Thu, 13 Sep 2018 12:11:58 -0700 Subject: [PATCH 2/9] [parseResponse][test] fix parseMethod test --- .../test/callApi/parseResponse.test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/superset-ui-core/test/callApi/parseResponse.test.js b/packages/superset-ui-core/test/callApi/parseResponse.test.js index f0dc9c2e00..6243719547 100644 --- a/packages/superset-ui-core/test/callApi/parseResponse.test.js +++ b/packages/superset-ui-core/test/callApi/parseResponse.test.js @@ -72,14 +72,14 @@ describe('parseResponse()', () => { expect.assertions(3); // test with json + bigint to ensure that it was not first parsed as json - const mockTextUrl = '/mock/textparse/url'; + const mockTextParseUrl = '/mock/textparse/url'; const mockTextJsonResponse = '{ "value": 9223372036854775807 }'; - fetchMock.get(mockTextUrl, mockTextJsonResponse); + fetchMock.get(mockTextParseUrl, mockTextJsonResponse); - const apiPromise = callApi({ url: mockTextUrl, method: 'GET', parseMethod: 'text' }); - parseResponse(apiPromise) + const apiPromise = callApi({ url: mockTextParseUrl, method: 'GET' }); + parseResponse(apiPromise, 'text') .then(args => { - expect(fetchMock.calls(mockTextUrl)).toHaveLength(1); + expect(fetchMock.calls(mockTextParseUrl)).toHaveLength(1); expect(Object.keys(args)).toEqual(expect.arrayContaining(['response', 'text'])); expect(args.text).toBe(mockTextJsonResponse); From 560f30b5e86da30603714df63334cbf9a3b9f75f Mon Sep 17 00:00:00 2001 From: Chris Williams Date: Thu, 13 Sep 2018 12:29:25 -0700 Subject: [PATCH 3/9] [monorepo] gitignore .npmrc --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 454dd0e0ce..63c9519e20 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,7 @@ .eslintrc.js .idea .npm +.npmrc .prettierignore .yarnclean From b01e770bb38a30bf175286f13f17184a05c09e2d Mon Sep 17 00:00:00 2001 From: Chris Williams Date: Thu, 13 Sep 2018 12:29:53 -0700 Subject: [PATCH 4/9] [core][deps] @data-ui/build-config => ^0.0.14 --- packages/superset-ui-core/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/superset-ui-core/package.json b/packages/superset-ui-core/package.json index 079de03d1a..4d03e822c6 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.13", + "@data-ui/build-config": "^0.0.14", "fetch-mock": "^6.5.2", "node-fetch": "^2.2.0" }, From e138fc254c231e1b7dbc78858025431c547737a6 Mon Sep 17 00:00:00 2001 From: Chris Williams Date: Thu, 13 Sep 2018 12:30:14 -0700 Subject: [PATCH 5/9] [monorepo] tweak npm scripts --- package.json | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index ce2a193551..1ac4adedd9 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": [ From a19871ca4f1c68dcc6d3753468e1d117b2d6be64 Mon Sep 17 00:00:00 2001 From: Chris Williams Date: Thu, 13 Sep 2018 12:47:41 -0700 Subject: [PATCH 6/9] [core] clean up parseResponse --- .../src/callApi/parseResponse.js | 54 +++++++++---------- 1 file changed, 24 insertions(+), 30 deletions(-) diff --git a/packages/superset-ui-core/src/callApi/parseResponse.js b/packages/superset-ui-core/src/callApi/parseResponse.js index cdd44c5c9f..f5778d87b3 100644 --- a/packages/superset-ui-core/src/callApi/parseResponse.js +++ b/packages/superset-ui-core/src/callApi/parseResponse.js @@ -1,35 +1,29 @@ -export default function parseResponse(apiPromise, parseMethod = 'json') { - return apiPromise.then(response => { - let parsedPromise; - if (parseMethod === 'json') { - // 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 - parsedPromise = response - .clone() - .json() - .catch(() => /* jsonParseError */ response.text().then(textPayload => ({ textPayload }))); - } else if (parseMethod === 'text') { - parsedPromise = response.text().then(textPayload => ({ textPayload })); - } else { - throw Error(`Unrecognized parseMethod '${parseMethod}', expected 'json' or 'text'`); - } +const PARSERS = { + json: response => + // 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 + response + .clone() + .json() + .then(json => ({ json, response })) + .catch(() => /* jsonParseError */ response.text().then(text => ({ response, text }))), + + text: response => response.text().then(text => ({ response, text })), +}; - return parsedPromise - .then(maybeJson => ({ - json: maybeJson.textPayload ? undefined : maybeJson, - text: maybeJson.textPayload, - })) - .then(({ json, text }) => { - // HTTP 404 or 500 are not rejected, ok is just set to false - if (!response.ok) { - return Promise.reject({ - error: response.error || (json && json.error) || text || 'An error occurred', - status: response.status, - statusText: response.statusText, - }); - } +export default function parseResponse(apiPromise, parseMethod = 'json') { + const responseParser = PARSERS[parseMethod] || PARSERS.json; - return typeof text === 'undefined' ? { json, response } : { response, text }; + return apiPromise.then(responseParser).then(({ json, text, response }) => { + // HTTP 404 or 500 are not rejected, ok is just set to false + if (!response.ok) { + return Promise.reject({ + error: response.error || (json && json.error) || text || 'An error occurred', + status: response.status, + statusText: response.statusText, }); + } + + return typeof text === 'undefined' ? { json, response } : { response, text }; }); } From c8c35584d32186a3e4c5b84289d5f6d4c021be49 Mon Sep 17 00:00:00 2001 From: Chris Williams Date: Thu, 13 Sep 2018 12:50:56 -0700 Subject: [PATCH 7/9] [core] clean up parseResponse more --- packages/superset-ui-core/src/callApi/parseResponse.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/superset-ui-core/src/callApi/parseResponse.js b/packages/superset-ui-core/src/callApi/parseResponse.js index f5778d87b3..714ea5b4c8 100644 --- a/packages/superset-ui-core/src/callApi/parseResponse.js +++ b/packages/superset-ui-core/src/callApi/parseResponse.js @@ -14,7 +14,7 @@ const PARSERS = { export default function parseResponse(apiPromise, parseMethod = 'json') { const responseParser = PARSERS[parseMethod] || PARSERS.json; - return apiPromise.then(responseParser).then(({ json, text, response }) => { + return apiPromise.then(responseParser).then(({ json, response, text }) => { // HTTP 404 or 500 are not rejected, ok is just set to false if (!response.ok) { return Promise.reject({ @@ -24,6 +24,6 @@ export default function parseResponse(apiPromise, parseMethod = 'json') { }); } - return typeof text === 'undefined' ? { json, response } : { response, text }; + return { json, response, text }; }); } From fc7e7db316cfbf35047d3d9e131e15b883abc359 Mon Sep 17 00:00:00 2001 From: Chris Williams Date: Mon, 17 Sep 2018 18:16:58 -0700 Subject: [PATCH 8/9] [core] don't fallback to text parsing if response is supposed to be json, allow for null responseParser --- .../src/callApi/parseResponse.js | 36 +++++++++---------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/packages/superset-ui-core/src/callApi/parseResponse.js b/packages/superset-ui-core/src/callApi/parseResponse.js index 714ea5b4c8..1235f651fd 100644 --- a/packages/superset-ui-core/src/callApi/parseResponse.js +++ b/packages/superset-ui-core/src/callApi/parseResponse.js @@ -1,29 +1,25 @@ const PARSERS = { - json: response => - // 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 - response - .clone() - .json() - .then(json => ({ json, response })) - .catch(() => /* jsonParseError */ response.text().then(text => ({ response, text }))), - + json: response => response.json().then(json => ({ json, response })), text: response => response.text().then(text => ({ response, text })), }; export default function parseResponse(apiPromise, parseMethod = 'json') { + if (parseMethod === null) return apiPromise; + const responseParser = PARSERS[parseMethod] || PARSERS.json; - return apiPromise.then(responseParser).then(({ json, response, text }) => { - // HTTP 404 or 500 are not rejected, ok is just set to false - if (!response.ok) { - return Promise.reject({ - error: response.error || (json && json.error) || text || 'An error occurred', - status: response.status, - statusText: response.statusText, - }); - } + return apiPromise + .then(response => { + if (!response.ok) { + return Promise.reject({ + error: response.error || 'An error occurred', + response, + status: response.status, + statusText: response.statusText, + }); + } - return { json, response, text }; - }); + return response; + }) + .then(responseParser); } From 85a1bdd347e4e694a6acea7833938fa2d90d4326 Mon Sep 17 00:00:00 2001 From: Chris Williams Date: Mon, 17 Sep 2018 18:17:37 -0700 Subject: [PATCH 9/9] [core][tests] fix borken tests, return promises instead of calling done() --- .../test/SupersetClient.test.js | 314 ++++++++---------- .../test/callApi/callApi.test.js | 142 ++++---- .../callApiAndParseWithTimeout.test.js | 15 +- .../test/callApi/parseResponse.test.js | 76 ++--- .../test/utils/throwIfCalled.js | 4 +- 5 files changed, 253 insertions(+), 298 deletions(-) diff --git a/packages/superset-ui-core/test/SupersetClient.test.js b/packages/superset-ui-core/test/SupersetClient.test.js index 6dcc685f3c..bc6bde0807 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(); }); }); }); @@ -249,34 +240,31 @@ describe('SupersetClient', () => { 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, @@ -287,49 +275,41 @@ 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 - .init() - .then(() => - Promise.all([ - client.get({ url: mockGetUrl }), - client.get({ endpoint: mockGetEndpoint }), - ]) - .then(() => { - expect(fetchMock.calls(mockGetUrl)).toHaveLength(2); - return done(); - }) - .catch(throwIfCalled), - ) - .catch(throwIfCalled); + 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', done => { + it('supports parsing a response as text', () => { expect.assertions(2); const client = new SupersetClient({ protocol, host }); - client + + return client .init() .then(() => client @@ -338,14 +318,14 @@ describe('SupersetClient', () => { 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, @@ -363,7 +343,8 @@ describe('SupersetClient', () => { }; const client = new SupersetClient(clientConfig); - client + + return client .init() .then(() => client @@ -376,7 +357,7 @@ describe('SupersetClient', () => { expect.objectContaining(overrideConfig.headers), ); - return done(); + return Promise.resolve(); }) .catch(throwIfCalled), ) @@ -385,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, @@ -422,91 +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('supports parsing a response as text', done => { + it('supports parsing a response as text', () => { expect.assertions(2); const client = new SupersetClient({ protocol, host }); - client - .init() - .then(() => - client - .post({ url: mockTextUrl, parseMethod: 'text' }) - .then(({ text }) => { - expect(fetchMock.calls(mockTextUrl)).toHaveLength(1); - expect(text).toBe(mockTextJsonResponse); - return done(); - }) - .catch(throwIfCalled), - ) - .catch(throwIfCalled); + 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', done => { + 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 d4c670cccc..2a54229af8 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 a9006d9b4d..8cb21b249b 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 6243719547..809e219f68 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,18 +54,19 @@ 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('resolves to { text, response } if the parseMethod is set to `text`', done => { + it('resolves to { text, response } if the `parseMethod=text`', () => { expect.assertions(3); // test with json + bigint to ensure that it was not first parsed as json @@ -77,28 +75,30 @@ describe('parseResponse()', () => { fetchMock.get(mockTextParseUrl, mockTextJsonResponse); const apiPromise = callApi({ url: mockTextParseUrl, method: 'GET' }); - 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 done(); - }) - .catch(throwIfCalled); + + 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('rejects if the request throws', done => { - expect.assertions(3); + it('resolves to the unmodified `Response` object if `parseMethod=null`', () => { + expect.assertions(2); - 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); + const mockNoParseUrl = '/mock/noparse/url'; + const mockResponse = new Response('test response'); + fetchMock.get(mockNoParseUrl, mockResponse); - return done(); - }); + 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 41079b95ee..e3b4f694c9 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)}`); }