From ba8c619c2e8ca9f7726b0bb879b0ede0ef9582a2 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Thu, 2 Jul 2020 17:46:54 -0700 Subject: [PATCH] feat(connection): optimize typing and API for SupersetClient (#635) --- .../test/components/MockChartPlugins.tsx | 2 +- .../src/SupersetClient.ts | 24 +- .../src/SupersetClientClass.ts | 100 ++-- .../src/callApi/callApi.ts | 59 ++- .../src/callApi/callApiAndParseWithTimeout.ts | 11 +- .../src/callApi/parseResponse.ts | 51 +- .../src/callApi/rejectAfterTimeout.ts | 4 +- .../superset-ui-connection/src/types.ts | 34 +- .../test/SupersetClient.test.ts | 43 +- .../test/SupersetClientClass.test.ts | 441 ++++++++---------- .../test/callApi/callApi.test.ts | 415 ++++++++-------- .../callApiAndParseWithTimeout.test.ts | 74 +-- .../test/callApi/parseResponse.test.ts | 132 +++--- .../test/callApi/rejectAfterTimeout.test.ts | 45 +- .../test/fixtures/constants.ts | 18 + .../test/utils/throwIfCalled.ts | 3 - .../shared/components/ErrorMessage.tsx | 8 +- .../ChartDataProviderStories.tsx | 18 +- .../ConnectionStories.tsx | 6 +- 19 files changed, 744 insertions(+), 744 deletions(-) delete mode 100644 superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/utils/throwIfCalled.ts diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/test/components/MockChartPlugins.tsx b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/test/components/MockChartPlugins.tsx index 657ebb40cc..53e014ddf9 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/test/components/MockChartPlugins.tsx +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart/test/components/MockChartPlugins.tsx @@ -1,6 +1,6 @@ /* eslint-disable max-classes-per-file */ import React from 'react'; -import { QueryFormData } from '@superset-ui/query/src'; +import { QueryFormData } from '@superset-ui/query'; import { ChartMetadata, ChartPlugin } from '../../src'; const DIMENSION_STYLE = { diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/SupersetClient.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/SupersetClient.ts index a94e6342e1..91e09779c1 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/SupersetClient.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/SupersetClient.ts @@ -1,5 +1,5 @@ import SupersetClientClass from './SupersetClientClass'; -import { ClientConfig, RequestConfig, SupersetClientInterface } from './types'; +import { SupersetClientInterface } from './types'; let singletonClient: SupersetClientClass | undefined; @@ -7,28 +7,26 @@ function getInstance(): SupersetClientClass { if (!singletonClient) { throw new Error('You must call SupersetClient.configure(...) before calling other methods'); } - return singletonClient; } const SupersetClient: SupersetClientInterface = { - configure: (config?: ClientConfig): SupersetClientClass => { + configure: config => { singletonClient = new SupersetClientClass(config); - return singletonClient; }, - delete: (request: RequestConfig) => getInstance().delete(request), - get: (request: RequestConfig) => getInstance().get(request), - getInstance, - init: (force?: boolean) => getInstance().init(force), - isAuthenticated: () => getInstance().isAuthenticated(), - post: (request: RequestConfig) => getInstance().post(request), - put: (request: RequestConfig) => getInstance().put(request), - reAuthenticate: () => getInstance().init(/* force = */ true), - request: (request: RequestConfig) => getInstance().request(request), reset: () => { singletonClient = undefined; }, + getInstance, + delete: request => getInstance().delete(request), + get: request => getInstance().get(request), + init: force => getInstance().init(force), + isAuthenticated: () => getInstance().isAuthenticated(), + post: request => getInstance().post(request), + put: request => getInstance().put(request), + reAuthenticate: () => getInstance().reAuthenticate(), + request: request => getInstance().request(request), }; export default SupersetClient; diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/SupersetClientClass.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/SupersetClientClass.ts index 08e4645b97..03dd74bf0c 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/SupersetClientClass.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/SupersetClientClass.ts @@ -1,4 +1,4 @@ -import callApi from './callApi'; +import callApiAndParseWithTimeout from './callApi/callApiAndParseWithTimeout'; import { ClientConfig, ClientTimeout, @@ -11,7 +11,7 @@ import { Mode, Protocol, RequestConfig, - SupersetClientResponse, + ParseMethod, } from './types'; import { DEFAULT_FETCH_RETRY_OPTIONS } from './constants'; @@ -20,6 +20,7 @@ export default class SupersetClientClass { csrfToken?: CsrfToken; csrfPromise?: CsrfPromise; fetchRetryOptions?: FetchRetryOptions; + baseUrl: string; protocol: Protocol; host: Host; headers: Headers; @@ -27,8 +28,9 @@ export default class SupersetClientClass { timeout: ClientTimeout; constructor({ - protocol = 'http:', - host = 'localhost', + baseUrl = 'http://localhost', + host, + protocol, headers = {}, fetchRetryOptions = {}, mode = 'same-origin', @@ -36,52 +38,57 @@ export default class SupersetClientClass { credentials = undefined, csrfToken = undefined, }: ClientConfig = {}) { + const url = new URL( + host || protocol ? `${protocol || 'https:'}//${host || 'localhost'}` : baseUrl, + ); + this.baseUrl = url.href.replace(/\/+$/, ''); // always strip trailing slash + this.host = url.host; + this.protocol = url.protocol as Protocol; this.headers = { ...headers }; - this.host = host; this.mode = mode; this.timeout = timeout; - this.protocol = protocol; this.credentials = credentials; this.csrfToken = csrfToken; - this.csrfPromise = undefined; this.fetchRetryOptions = { ...DEFAULT_FETCH_RETRY_OPTIONS, ...fetchRetryOptions }; - if (typeof this.csrfToken === 'string') { this.headers = { ...this.headers, 'X-CSRFToken': this.csrfToken }; this.csrfPromise = Promise.resolve(this.csrfToken); } } - init(force: boolean = false): CsrfPromise { + async init(force: boolean = false): CsrfPromise { if (this.isAuthenticated() && !force) { return this.csrfPromise as CsrfPromise; } - return this.getCSRFToken(); } + async reAuthenticate() { + return this.init(true); + } + isAuthenticated(): boolean { // if CSRF protection is disabled in the Superset app, the token may be an empty string return this.csrfToken !== null && this.csrfToken !== undefined; } - async get(requestConfig: RequestConfig): Promise { + async get(requestConfig: RequestConfig & { parseMethod?: T }) { return this.request({ ...requestConfig, method: 'GET' }); } - async delete(requestConfig: RequestConfig): Promise { + async delete(requestConfig: RequestConfig & { parseMethod?: T }) { return this.request({ ...requestConfig, method: 'DELETE' }); } - async put(requestConfig: RequestConfig): Promise { + async put(requestConfig: RequestConfig & { parseMethod?: T }) { return this.request({ ...requestConfig, method: 'PUT' }); } - async post(requestConfig: RequestConfig): Promise { + async post(requestConfig: RequestConfig & { parseMethod?: T }) { return this.request({ ...requestConfig, method: 'POST' }); } - async request({ + async request({ body, credentials, endpoint, @@ -97,44 +104,41 @@ export default class SupersetClientClass { stringify, timeout, url, - }: RequestConfig): Promise { - return this.ensureAuth().then(() => - callApi({ - body, - credentials: credentials ?? this.credentials, - fetchRetryOptions, - headers: { ...this.headers, ...headers }, - method, - mode: mode ?? this.mode, - parseMethod, - postPayload, - jsonPayload, - signal, - stringify, - timeout: timeout ?? this.timeout, - url: this.getUrl({ endpoint, host, url }), - }), - ); + }: RequestConfig & { parseMethod?: T }) { + await this.ensureAuth(); + return callApiAndParseWithTimeout({ + body, + credentials: credentials ?? this.credentials, + fetchRetryOptions, + headers: { ...this.headers, ...headers }, + method, + mode: mode ?? this.mode, + parseMethod, + postPayload, + jsonPayload, + signal, + stringify, + timeout: timeout ?? this.timeout, + url: this.getUrl({ endpoint, host, url }), + }); } - ensureAuth(): CsrfPromise { + async ensureAuth(): CsrfPromise { return ( this.csrfPromise ?? Promise.reject({ - error: `SupersetClient has no CSRF token, ensure it is initialized or - try logging into the Superset instance at ${this.getUrl({ - endpoint: '/login', - })}`, + error: `SupersetClient has not been provided a CSRF token, ensure it is + initialized with \`client.getCSRFToken()\` or try logging in at + ${this.getUrl({ endpoint: '/login' })}`, }) ); } - async getCSRFToken(): CsrfPromise { + async getCSRFToken() { this.csrfToken = undefined; - // If we can request this resource successfully, it means that the user has // authenticated. If not we throw an error prompting to authenticate. - this.csrfPromise = callApi({ + this.csrfPromise = callApiAndParseWithTimeout({ credentials: this.credentials, headers: { ...this.headers, @@ -143,19 +147,19 @@ export default class SupersetClientClass { mode: this.mode, timeout: this.timeout, url: this.getUrl({ endpoint: 'superset/csrf_token/' }), - }).then(response => { - if (typeof response.json === 'object') { - this.csrfToken = response.json.csrf_token as string; + parseMethod: 'json', + }).then(({ json }) => { + if (typeof json === 'object') { + this.csrfToken = json.csrf_token as string; if (typeof this.csrfToken === 'string') { this.headers = { ...this.headers, 'X-CSRFToken': this.csrfToken }; } } - if (!this.isAuthenticated()) { - return Promise.reject({ error: 'Failed to fetch CSRF token' }); + if (this.isAuthenticated()) { + return this.csrfToken; } - return this.csrfToken; + return Promise.reject({ error: 'Failed to fetch CSRF token' }); }); - return this.csrfPromise; } diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/callApi/callApi.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/callApi/callApi.ts index 5d0c805f51..6801bc17b2 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/callApi/callApi.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/callApi/callApi.ts @@ -10,7 +10,7 @@ import { CACHE_AVAILABLE, CACHE_KEY, HTTP_STATUS_NOT_MODIFIED, HTTP_STATUS_OK } * @param {Payload} jsonPayload json payload to post, will automatically add Content-Type header * @param {string} stringify whether to stringify field values when post as formData */ -export default function callApi({ +export default async function callApi({ body, cache = 'default', credentials = 'same-origin', @@ -40,39 +40,32 @@ export default function callApi({ if ( method === 'GET' && + cache !== 'no-store' && + cache !== 'reload' && CACHE_AVAILABLE && (self.location && self.location.protocol) === 'https:' ) { - return caches.open(CACHE_KEY).then(supersetCache => - supersetCache - .match(url) - .then(cachedResponse => { - if (cachedResponse) { - // if we have a cached response, send its ETag in the - // `If-None-Match` header in a conditional request - const etag = cachedResponse.headers.get('Etag') as string; - request.headers = { ...request.headers, 'If-None-Match': etag }; - } - - return fetchWithRetry(url, request); - }) - .then(response => { - if (response.status === HTTP_STATUS_NOT_MODIFIED) { - return supersetCache.match(url).then(cachedResponse => { - if (cachedResponse) { - return cachedResponse.clone(); - } - throw new Error('Received 304 but no content is cached!'); - }); - } - if (response.status === HTTP_STATUS_OK && response.headers.get('Etag')) { - supersetCache.delete(url); - supersetCache.put(url, response.clone()); - } - - return response; - }), - ); + const supersetCache = await caches.open(CACHE_KEY); + const cachedResponse = await supersetCache.match(url); + if (cachedResponse) { + // if we have a cached response, send its ETag in the + // `If-None-Match` header in a conditional request + const etag = cachedResponse.headers.get('Etag') as string; + request.headers = { ...request.headers, 'If-None-Match': etag }; + } + const response = await fetchWithRetry(url, request); + if (response.status === HTTP_STATUS_NOT_MODIFIED) { + const cachedFullResponse = await supersetCache.match(url); + if (cachedFullResponse) { + return cachedFullResponse.clone(); + } + throw new Error('Received 304 but no content is cached!'); + } + if (response.status === HTTP_STATUS_OK && response.headers.get('Etag')) { + supersetCache.delete(url); + supersetCache.put(url, response.clone()); + } + return response; } if (method === 'POST' || method === 'PATCH' || method === 'PUT') { @@ -80,13 +73,13 @@ export default function callApi({ try { return JSON.parse(payloadString) as JsonObject; } catch (error) { - throw new Error(`Invalid postPayload:\n\n${payloadString}`); + throw new Error(`Invalid payload:\n\n${payloadString}`); } }; - // override request body with post payload const payload: JsonObject | undefined = typeof postPayload === 'string' ? tryParsePayload(postPayload) : postPayload; + if (typeof payload === 'object') { // using FormData has the effect that Content-Type header is set to `multipart/form-data`, // not e.g., 'application/x-www-form-urlencoded' diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/callApi/callApiAndParseWithTimeout.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/callApi/callApiAndParseWithTimeout.ts index f359d7d30e..76a161a0f6 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/callApi/callApiAndParseWithTimeout.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/callApi/callApiAndParseWithTimeout.ts @@ -1,20 +1,17 @@ import callApi from './callApi'; import rejectAfterTimeout from './rejectAfterTimeout'; import parseResponse from './parseResponse'; -import { CallApi, ClientTimeout, SupersetClientResponse, ParseMethod } from '../types'; +import { CallApi, ClientTimeout, ParseMethod } from '../types'; -export default function callApiAndParseWithTimeout({ +export default async function callApiAndParseWithTimeout({ timeout, parseMethod, ...rest -}: { timeout?: ClientTimeout; parseMethod?: ParseMethod } & CallApi): Promise< - SupersetClientResponse -> { +}: { timeout?: ClientTimeout; parseMethod?: T } & CallApi) { const apiPromise = callApi(rest); - const racedPromise = typeof timeout === 'number' && timeout > 0 - ? Promise.race([rejectAfterTimeout(timeout), apiPromise]) + ? Promise.race([apiPromise, rejectAfterTimeout(timeout)]) : apiPromise; return parseResponse(racedPromise, parseMethod); diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/callApi/parseResponse.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/callApi/parseResponse.ts index 3ac3c53b5a..ba509cb6eb 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/callApi/parseResponse.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/callApi/parseResponse.ts @@ -1,26 +1,43 @@ -import { ParseMethod, SupersetClientResponse } from '../types'; +import { ParseMethod, TextResponse, JsonResponse } from '../types'; -function rejectIfNotOkay(response: Response): Promise { - if (!response.ok) return Promise.reject(response); - - return Promise.resolve(response); -} - -export default function parseResponse( +export default async function parseResponse( apiPromise: Promise, - parseMethod: ParseMethod = 'json', -): Promise { - const checkedPromise = apiPromise.then(rejectIfNotOkay); + parseMethod?: T, +) { + type ReturnType = T extends 'raw' | null + ? Response + : T extends 'json' | undefined + ? JsonResponse + : T extends 'text' + ? TextResponse + : never; + const response = await apiPromise; + // reject failed HTTP requests with the raw response + if (!response.ok) { + // eslint-disable-next-line @typescript-eslint/no-throw-literal + throw response; + } - if (parseMethod === null) { - return apiPromise.then(rejectIfNotOkay); + if (parseMethod === null || parseMethod === 'raw') { + return response as ReturnType; } if (parseMethod === 'text') { - return checkedPromise.then(response => response.text().then(text => ({ response, text }))); + const text = await response.text(); + const result: TextResponse = { + response, + text, + }; + return result as ReturnType; } - if (parseMethod === 'json') { - return checkedPromise.then(response => response.json().then(json => ({ json, response }))); + // by default treat this as json + if (parseMethod === undefined || parseMethod === 'json') { + const json = await response.json(); + const result: JsonResponse = { + json, + response, + }; + return result as ReturnType; } - throw new Error(`Expected parseResponse=null|json|text, got '${parseMethod}'.`); + throw new Error(`Expected parseResponse=json|text|raw|null, got '${parseMethod}'.`); } diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/callApi/rejectAfterTimeout.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/callApi/rejectAfterTimeout.ts index 83f7a5db12..6b01ecfa60 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/callApi/rejectAfterTimeout.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/callApi/rejectAfterTimeout.ts @@ -1,6 +1,6 @@ // returns a Promise that rejects after the specified timeout -export default function rejectAfterTimeout(timeout: number): Promise { - return new Promise((resolve, reject) => { +export default function rejectAfterTimeout(timeout: number) { + return new Promise((resolve, reject) => { setTimeout( () => reject({ diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/types.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/types.ts index 57af04daa8..40f3caaed3 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/types.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/src/types.ts @@ -44,7 +44,7 @@ export type Method = RequestInit['method']; export type Mode = RequestInit['mode']; export type Redirect = RequestInit['redirect']; export type ClientTimeout = number | undefined; -export type ParseMethod = 'json' | 'text' | null; +export type ParseMethod = 'json' | 'text' | 'raw' | null | undefined; export type Signal = RequestInit['signal']; export type Stringify = boolean; export type Url = string; @@ -57,7 +57,6 @@ export interface RequestBase { host?: Host; mode?: Mode; method?: Method; - parseMethod?: ParseMethod; postPayload?: Payload; jsonPayload?: Payload; signal?: Signal; @@ -84,10 +83,14 @@ export interface RequestWithUrl extends RequestBase { // this make sure at least one of `url` or `endpoint` is set export type RequestConfig = RequestWithEndpoint | RequestWithUrl; -export interface JsonTextResponse { - json?: JsonObject; +export interface JsonResponse { response: Response; - text?: string; + json: JsonObject; +} + +export interface TextResponse { + response: Response; + text: string; } export type CsrfToken = string; @@ -95,28 +98,25 @@ export type CsrfPromise = Promise; export type Protocol = 'http:' | 'https:'; export interface ClientConfig { + baseUrl?: string; + host?: Host; + protocol?: Protocol; credentials?: Credentials; csrfToken?: CsrfToken; fetchRetryOptions?: FetchRetryOptions; headers?: Headers; - host?: Host; - protocol?: Protocol; mode?: Mode; timeout?: ClientTimeout; } -export interface SupersetClientInterface { +export interface SupersetClientInterface + extends Pick< + SupersetClientClass, + 'delete' | 'get' | 'post' | 'put' | 'request' | 'init' | 'isAuthenticated' | 'reAuthenticate' + > { configure: (config?: ClientConfig) => SupersetClientClass; - delete: (request: RequestConfig) => Promise; - get: (request: RequestConfig) => Promise; getInstance: (maybeClient?: SupersetClientClass) => SupersetClientClass; - init: (force?: boolean) => Promise; - isAuthenticated: () => boolean; - post: (request: RequestConfig) => Promise; - put: (request: RequestConfig) => Promise; - reAuthenticate: () => Promise; - request: (request: RequestConfig) => Promise; reset: () => void; } -export type SupersetClientResponse = Response | JsonTextResponse; +export type SupersetClientResponse = Response | JsonResponse | TextResponse; diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/SupersetClient.test.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/SupersetClient.test.ts index 7275b112b8..6d49295767 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/SupersetClient.test.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/SupersetClient.test.ts @@ -1,3 +1,21 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ import fetchMock from 'fetch-mock'; import { SupersetClient, SupersetClientClass } from '../src'; @@ -30,12 +48,12 @@ describe('SupersetClient', () => { expect(SupersetClient.isAuthenticated).toThrow(); expect(SupersetClient.reAuthenticate).toThrow(); expect(SupersetClient.request).toThrow(); - expect(SupersetClient.configure).not.toThrow(); }); // this also tests that the ^above doesn't throw if configure is called appropriately - it('calls appropriate SupersetClient methods when configured', () => { + it('calls appropriate SupersetClient methods when configured', async () => { + expect.assertions(10); const mockGetUrl = '/mock/get/url'; const mockPostUrl = '/mock/post/url'; const mockRequestUrl = '/mock/request/url'; @@ -43,8 +61,13 @@ describe('SupersetClient', () => { const mockDeleteUrl = '/mock/delete/url'; const mockGetPayload = { get: 'payload' }; const mockPostPayload = { post: 'payload' }; + const mockDeletePayload = { delete: 'ok' }; + const mockPutPayload = { put: 'ok' }; fetchMock.get(mockGetUrl, mockGetPayload); fetchMock.post(mockPostUrl, mockPostPayload); + fetchMock.delete(mockDeleteUrl, mockDeletePayload); + fetchMock.put(mockPutUrl, mockPutPayload); + fetchMock.get(mockRequestUrl, mockGetPayload); const initSpy = jest.spyOn(SupersetClientClass.prototype, 'init'); const getSpy = jest.spyOn(SupersetClientClass.prototype, 'get'); @@ -56,19 +79,19 @@ describe('SupersetClient', () => { const requestSpy = jest.spyOn(SupersetClientClass.prototype, 'request'); SupersetClient.configure({}); - SupersetClient.init(); + await SupersetClient.init(); expect(initSpy).toHaveBeenCalledTimes(1); - expect(authenticatedSpy).toHaveBeenCalledTimes(1); + expect(authenticatedSpy).toHaveBeenCalledTimes(2); expect(csrfSpy).toHaveBeenCalledTimes(1); - SupersetClient.get({ url: mockGetUrl }); - SupersetClient.post({ url: mockPostUrl }); - SupersetClient.delete({ url: mockDeleteUrl }); - SupersetClient.put({ url: mockPutUrl }); - SupersetClient.request({ url: mockRequestUrl }); + await SupersetClient.get({ url: mockGetUrl }); + await SupersetClient.post({ url: mockPostUrl }); + await SupersetClient.delete({ url: mockDeleteUrl }); + await SupersetClient.put({ url: mockPutUrl }); + await SupersetClient.request({ url: mockRequestUrl }); SupersetClient.isAuthenticated(); - SupersetClient.reAuthenticate(); + await SupersetClient.reAuthenticate(); expect(initSpy).toHaveBeenCalledTimes(2); expect(deleteSpy).toHaveBeenCalledTimes(1); diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/SupersetClientClass.test.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/SupersetClientClass.test.ts index f12c7869c3..41f5a0114a 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/SupersetClientClass.test.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/SupersetClientClass.test.ts @@ -1,6 +1,23 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ import fetchMock from 'fetch-mock'; import { SupersetClientClass, ClientConfig } from '../src'; -import throwIfCalled from './utils/throwIfCalled'; import { LOGIN_GLOB } from './fixtures/constants'; describe('SupersetClientClass', () => { @@ -15,6 +32,11 @@ describe('SupersetClientClass', () => { expect(client).toBeInstanceOf(SupersetClientClass); }); + it('fallback protocol to https when setting only host', () => { + const client = new SupersetClientClass({ host: 'TEST-HOST' }); + expect(client.baseUrl).toEqual('https://test-host'); + }); + describe('.getUrl()', () => { let client = new SupersetClientClass(); @@ -36,12 +58,12 @@ describe('SupersetClientClass', () => { }); it('constructs a valid url from config.host + endpoint if host is omitted', () => { - expect(client.getUrl({ endpoint: '/test' })).toBe('https://CONFIG_HOST/test'); + expect(client.getUrl({ endpoint: '/test' })).toBe('https://config_host/test'); }); - it('does not throw if url, endpoint, and host are', () => { + it('does not throw if url, endpoint, and host are all empty', () => { client = new SupersetClientClass({ protocol: 'https:', host: '' }); - expect(client.getUrl()).toBe('https:///'); + expect(client.getUrl()).toBe('https://localhost/'); }); }); @@ -52,194 +74,151 @@ describe('SupersetClientClass', () => { fetchMock.get(LOGIN_GLOB, { csrf_token: 1234 }, { overwriteRoutes: true }); }); - it('calls superset/csrf_token/ when init() is called if no CSRF token is passed', () => { + it('calls superset/csrf_token/ when init() is called if no CSRF token is passed', async () => { expect.assertions(1); - - return new SupersetClientClass().init().then(() => { - expect(fetchMock.calls(LOGIN_GLOB)).toHaveLength(1); - - return true; - }); + await new SupersetClientClass().init(); + expect(fetchMock.calls(LOGIN_GLOB)).toHaveLength(1); }); - it('does NOT call superset/csrf_token/ when init() is called if a CSRF token is passed', () => { + it('does NOT call superset/csrf_token/ when init() is called if a CSRF token is passed', async () => { expect.assertions(1); - - return new SupersetClientClass({ csrfToken: 'abc' }).init().then(() => { - expect(fetchMock.calls(LOGIN_GLOB)).toHaveLength(0); - - return true; - }); + await new SupersetClientClass({ csrfToken: 'abc' }).init(); + expect(fetchMock.calls(LOGIN_GLOB)).toHaveLength(0); }); - it('calls superset/csrf_token/ when init(force=true) is called even if a CSRF token is passed', () => { + it('calls superset/csrf_token/ when init(force=true) is called even if a CSRF token is passed', async () => { expect.assertions(4); const initialToken = 'initial_token'; const client = new SupersetClientClass({ csrfToken: initialToken }); - return client.init().then(() => { - expect(fetchMock.calls(LOGIN_GLOB)).toHaveLength(0); - expect(client.csrfToken).toBe(initialToken); + await client.init(); + expect(fetchMock.calls(LOGIN_GLOB)).toHaveLength(0); + expect(client.csrfToken).toBe(initialToken); - return client.init(true).then(() => { - expect(fetchMock.calls(LOGIN_GLOB)).toHaveLength(1); - expect(client.csrfToken).not.toBe(initialToken); - - return true; - }); - }); + await client.init(true); + expect(fetchMock.calls(LOGIN_GLOB)).toHaveLength(1); + expect(client.csrfToken).not.toBe(initialToken); }); - it('throws if superset/csrf_token/ returns an error', () => { + it('throws if superset/csrf_token/ returns an error', async () => { expect.assertions(1); - - fetchMock.get(LOGIN_GLOB, () => Promise.reject({ status: 403 }), { + const rejectError = { status: 403 }; + fetchMock.get(LOGIN_GLOB, () => Promise.reject(rejectError), { overwriteRoutes: true, }); - - return new SupersetClientClass({}) - .init() - .then(throwIfCalled) - .catch((error: { status: number }) => { - expect(error.status).toBe(403); - - return true; - }); + try { + await new SupersetClientClass({}).init(); + } catch (error) { + expect(error as typeof rejectError).toEqual(rejectError); + } }); - it('throws if superset/csrf_token/ does not return a token', () => { + const invalidCsrfTokenError = { error: 'Failed to fetch CSRF token' }; + + it('throws if superset/csrf_token/ does not return a token', async () => { expect.assertions(1); fetchMock.get(LOGIN_GLOB, {}, { overwriteRoutes: true }); - - return new SupersetClientClass({}) - .init() - .then(throwIfCalled) - .catch((error: unknown) => { - expect(error).toBeDefined(); - - return true; - }); + try { + await new SupersetClientClass({}).init(); + } catch (error) { + expect(error as typeof invalidCsrfTokenError).toEqual(invalidCsrfTokenError); + } }); - it('does not set csrfToken if response is not json', () => { + it('does not set csrfToken if response is not json', async () => { + expect.assertions(1); fetchMock.get(LOGIN_GLOB, '123', { overwriteRoutes: true, }); - - return new SupersetClientClass({}) - .init() - .then(throwIfCalled) - .catch((error: unknown) => { - expect(error).toBeDefined(); - - return true; - }); + try { + await new SupersetClientClass({}).init(); + } catch (error) { + expect(error as typeof invalidCsrfTokenError).toEqual(invalidCsrfTokenError); + } }); }); describe('.isAuthenticated()', () => { afterEach(fetchMock.reset); - it('returns true if there is a token and false if not', () => { + it('returns true if there is a token and false if not', async () => { expect.assertions(2); const client = new SupersetClientClass({}); expect(client.isAuthenticated()).toBe(false); - - return client.init().then(() => { - expect(client.isAuthenticated()).toBe(true); - - return true; - }); + await client.init(); + expect(client.isAuthenticated()).toBe(true); }); it('returns true if a token is passed at configuration', () => { expect.assertions(2); const clientWithoutToken = new SupersetClientClass({ csrfToken: undefined }); const clientWithToken = new SupersetClientClass({ csrfToken: 'token' }); - expect(clientWithoutToken.isAuthenticated()).toBe(false); expect(clientWithToken.isAuthenticated()).toBe(true); }); }); describe('.ensureAuth()', () => { - it(`returns a promise that rejects if .init() has not been called`, () => { + it(`returns a promise that rejects if .init() has not been called`, async () => { expect.assertions(2); const client = new SupersetClientClass({}); - - return client - .ensureAuth() - .then(throwIfCalled) - .catch((error: { error: string }) => { - expect(error).toEqual( - expect.objectContaining({ error: expect.any(String) }) as typeof error, - ); - expect(client.isAuthenticated()).toBe(false); - - return true; - }); + try { + await client.ensureAuth(); + } catch (error) { + expect(error).toEqual({ error: expect.any(String) }); + } + expect(client.isAuthenticated()).toBe(false); }); - it('returns a promise that resolves if .init() resolves successfully', () => { + it('returns a promise that resolves if .init() resolves successfully', async () => { expect.assertions(1); const client = new SupersetClientClass({}); + await client.init(); + await client.ensureAuth(); - return client.init().then(() => - client - .ensureAuth() - .then(throwIfCalled) - .catch(() => { - expect(client.isAuthenticated()).toBe(true); - - return true; - }), - ); + expect(client.isAuthenticated()).toBe(true); }); - it(`returns a promise that rejects if .init() is unsuccessful`, () => { + it(`returns a promise that rejects if .init() is unsuccessful`, async () => { + expect.assertions(4); + const rejectValue = { status: 403 }; fetchMock.get(LOGIN_GLOB, () => Promise.reject(rejectValue), { overwriteRoutes: true, }); - expect.assertions(3); - const client = new SupersetClientClass({}); - return client - .init() - .then(throwIfCalled) - .catch((error: unknown) => { - expect(error).toEqual(expect.objectContaining(rejectValue) as unknown); + try { + await client.init(); + } catch (error) { + expect(error).toEqual(expect.objectContaining(rejectValue)); + expect(client.isAuthenticated()).toBe(false); + try { + await client.ensureAuth(); + } catch (error2) { + expect(error2).toEqual(expect.objectContaining(rejectValue)); + expect(client.isAuthenticated()).toBe(false); + } + } - return client - .ensureAuth() - .then(throwIfCalled) - .catch((error2: unknown) => { - expect(error2).toEqual(expect.objectContaining(rejectValue) as unknown); - expect(client.isAuthenticated()).toBe(false); - - // reset - fetchMock.get( - LOGIN_GLOB, - { csrf_token: 1234 }, - { - overwriteRoutes: true, - }, - ); - - return true; - }); - }); + // reset + fetchMock.get( + LOGIN_GLOB, + { csrf_token: 1234 }, + { + overwriteRoutes: true, + }, + ); }); }); describe('requests', () => { afterEach(fetchMock.reset); const protocol = 'https:'; - const host = 'HOST'; + const host = 'host'; const mockGetEndpoint = '/get/url'; const mockRequestEndpoint = '/request/url'; const mockPostEndpoint = '/post/url'; @@ -263,34 +242,32 @@ describe('SupersetClientClass', () => { fetchMock.get(mockTextUrl, mockTextJsonResponse); fetchMock.post(mockTextUrl, mockTextJsonResponse); - it('checks for authentication before every get and post request', () => { + it('checks for authentication before every get and post request', async () => { expect.assertions(6); + const authSpy = jest.spyOn(SupersetClientClass.prototype, 'ensureAuth'); const client = new SupersetClientClass({ protocol, host }); - return client.init().then(() => - Promise.all([ - client.get({ url: mockGetUrl }), - client.post({ url: mockPostUrl }), - client.put({ url: mockPutUrl }), - client.delete({ url: mockDeleteUrl }), - client.request({ url: mockRequestUrl, method: 'DELETE' }), - ]).then(() => { - expect(fetchMock.calls(mockGetUrl)).toHaveLength(1); - expect(fetchMock.calls(mockPostUrl)).toHaveLength(1); - expect(fetchMock.calls(mockDeleteUrl)).toHaveLength(1); - expect(fetchMock.calls(mockPutUrl)).toHaveLength(1); - expect(fetchMock.calls(mockRequestUrl)).toHaveLength(1); - expect(authSpy).toHaveBeenCalledTimes(5); - authSpy.mockRestore(); + await client.init(); + await client.get({ url: mockGetUrl }); + await client.post({ url: mockPostUrl }); + await client.put({ url: mockPutUrl }); + await client.delete({ url: mockDeleteUrl }); + await client.request({ url: mockRequestUrl, method: 'DELETE' }); - return true; - }), - ); + expect(fetchMock.calls(mockGetUrl)).toHaveLength(1); + expect(fetchMock.calls(mockPostUrl)).toHaveLength(1); + expect(fetchMock.calls(mockDeleteUrl)).toHaveLength(1); + expect(fetchMock.calls(mockPutUrl)).toHaveLength(1); + expect(fetchMock.calls(mockRequestUrl)).toHaveLength(1); + + expect(authSpy).toHaveBeenCalledTimes(5); + authSpy.mockRestore(); }); - it('sets protocol, host, headers, mode, and credentials from config', () => { + it('sets protocol, host, headers, mode, and credentials from config', async () => { expect.assertions(3); + const clientConfig: ClientConfig = { host, protocol, @@ -300,60 +277,43 @@ describe('SupersetClientClass', () => { }; const client = new SupersetClientClass(clientConfig); + await client.init(); + await client.get({ url: mockGetUrl }); - 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) as typeof fetchRequest.headers, - ); - - return true; - }), + 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) as typeof fetchRequest.headers, ); }); describe('.get()', () => { - it('makes a request using url or endpoint', () => { - expect.assertions(1); + it('makes a request using url or endpoint', async () => { + expect.assertions(2); + const client = new SupersetClientClass({ protocol, host }); + await client.init(); - return client.init().then(() => - Promise.all([ - client.get({ url: mockGetUrl }), - client.get({ endpoint: mockGetEndpoint }), - ]).then(() => { - expect(fetchMock.calls(mockGetUrl)).toHaveLength(2); + await client.get({ url: mockGetUrl }); + expect(fetchMock.calls(mockGetUrl)).toHaveLength(1); - return true; - }), - ); + await client.get({ endpoint: mockGetEndpoint }); + expect(fetchMock.calls(mockGetUrl)).toHaveLength(2); }); - it('supports parsing a response as text', () => { + it('supports parsing a response as text', async () => { expect.assertions(2); const client = new SupersetClientClass({ protocol, host }); - - return client - .init() - .then(() => - client - .get({ url: mockTextUrl, parseMethod: 'text' }) - .then(({ text }) => { - expect(fetchMock.calls(mockTextUrl)).toHaveLength(1); - expect(text).toBe(mockTextJsonResponse); - - return true; - }) - .catch(throwIfCalled), - ) - .catch(throwIfCalled); + await client.init(); + const { text } = await client.get({ url: mockTextUrl, parseMethod: 'text' }); + expect(fetchMock.calls(mockTextUrl)).toHaveLength(1); + expect(text).toBe(mockTextJsonResponse); }); - it('allows overriding host, headers, mode, and credentials per-request', () => { + it('allows overriding host, headers, mode, and credentials per-request', async () => { expect.assertions(3); + const clientConfig: ClientConfig = { host, protocol, @@ -361,7 +321,6 @@ describe('SupersetClientClass', () => { credentials: 'include', headers: { my: 'header' }, }; - const overrideConfig: ClientConfig = { host: 'override_host', mode: 'no-cors', @@ -370,46 +329,34 @@ describe('SupersetClientClass', () => { }; const client = new SupersetClientClass(clientConfig); + await client.init(); + await client.get({ url: mockGetUrl, ...overrideConfig }); - return client - .init() - .then(() => - client - .get({ url: mockGetUrl, ...overrideConfig }) - .then(() => { - const fetchRequest = fetchMock.calls(mockGetUrl)[0][1]; - expect(fetchRequest.mode).toBe(overrideConfig.mode); - expect(fetchRequest.credentials).toBe(overrideConfig.credentials); - expect(fetchRequest.headers).toEqual( - expect.objectContaining(overrideConfig.headers) as typeof fetchRequest.headers, - ); - - return true; - }) - .catch(throwIfCalled), - ) - .catch(throwIfCalled); + const fetchRequest = fetchMock.calls(mockGetUrl)[0][1]; + expect(fetchRequest.mode).toBe(overrideConfig.mode); + expect(fetchRequest.credentials).toBe(overrideConfig.credentials); + expect(fetchRequest.headers).toEqual( + expect.objectContaining(overrideConfig.headers) as typeof fetchRequest.headers, + ); }); }); describe('.post()', () => { - it('makes a request using url or endpoint', () => { - expect.assertions(1); + it('makes a request using url or endpoint', async () => { + expect.assertions(2); + const client = new SupersetClientClass({ protocol, host }); + await client.init(); - return client.init().then(() => - Promise.all([ - client.post({ url: mockPostUrl }), - client.post({ endpoint: mockPostEndpoint }), - ]).then(() => { - expect(fetchMock.calls(mockPostUrl)).toHaveLength(2); + await client.post({ url: mockPostUrl }); + expect(fetchMock.calls(mockPostUrl)).toHaveLength(1); - return true; - }), - ); + await client.post({ endpoint: mockPostEndpoint }); + expect(fetchMock.calls(mockPostUrl)).toHaveLength(2); }); - it('allows overriding host, headers, mode, and credentials per-request', () => { + it('allows overriding host, headers, mode, and credentials per-request', async () => { + expect.assertions(3); const clientConfig: ClientConfig = { host, protocol, @@ -417,7 +364,6 @@ describe('SupersetClientClass', () => { credentials: 'include', headers: { my: 'header' }, }; - const overrideConfig: ClientConfig = { host: 'override_host', mode: 'no-cors', @@ -426,70 +372,57 @@ describe('SupersetClientClass', () => { }; const client = new SupersetClientClass(clientConfig); + await client.init(); + await client.post({ url: mockPostUrl, ...overrideConfig }); - 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) as typeof fetchRequest.headers, - ); + const fetchRequest = fetchMock.calls(mockPostUrl)[0][1]; - return true; - }), + expect(fetchRequest.mode).toBe(overrideConfig.mode); + expect(fetchRequest.credentials).toBe(overrideConfig.credentials); + expect(fetchRequest.headers).toEqual( + expect.objectContaining(overrideConfig.headers) as typeof fetchRequest.headers, ); }); - it('supports parsing a response as text', () => { + it('supports parsing a response as text', async () => { expect.assertions(2); const client = new SupersetClientClass({ 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 true; - }), - ); + await client.init(); + const { text } = await client.post({ url: mockTextUrl, parseMethod: 'text' }); + expect(fetchMock.calls(mockTextUrl)).toHaveLength(1); + expect(text).toBe(mockTextJsonResponse); }); - it('passes postPayload key,values in the body', () => { + it('passes postPayload key,values in the body', async () => { expect.assertions(3); const postPayload = { number: 123, array: [1, 2, 3] }; const client = new SupersetClientClass({ protocol, host }); + await client.init(); + await client.post({ url: mockPostUrl, postPayload }); - return client.init().then(() => - client.post({ url: mockPostUrl, postPayload }).then(() => { - const formData = fetchMock.calls(mockPostUrl)[0][1].body as FormData; - expect(fetchMock.calls(mockPostUrl)).toHaveLength(1); - Object.entries(postPayload).forEach(([key, value]) => { - expect(formData.get(key)).toBe(JSON.stringify(value)); - }); + const formData = fetchMock.calls(mockPostUrl)[0][1].body as FormData; - return true; - }), - ); + expect(fetchMock.calls(mockPostUrl)).toHaveLength(1); + Object.entries(postPayload).forEach(([key, value]) => { + expect(formData.get(key)).toBe(JSON.stringify(value)); + }); }); - it('respects the stringify parameter for postPayload key,values', () => { + it('respects the stringify parameter for postPayload key,values', async () => { expect.assertions(3); + const postPayload = { number: 123, array: [1, 2, 3] }; const client = new SupersetClientClass({ protocol, host }); + await client.init(); + await client.post({ url: mockPostUrl, postPayload, stringify: false }); - return client.init().then(() => - client.post({ url: mockPostUrl, postPayload, stringify: false }).then(() => { - const formData = fetchMock.calls(mockPostUrl)[0][1].body as FormData; - expect(fetchMock.calls(mockPostUrl)).toHaveLength(1); - Object.entries(postPayload).forEach(([key, value]) => { - expect(formData.get(key)).toBe(String(value)); - }); + const formData = fetchMock.calls(mockPostUrl)[0][1].body as FormData; - return true; - }), - ); + expect(fetchMock.calls(mockPostUrl)).toHaveLength(1); + Object.entries(postPayload).forEach(([key, value]) => { + expect(formData.get(key)).toBe(String(value)); + }); }); }); }); diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/callApi/callApi.test.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/callApi/callApi.test.ts index 85d60aabf2..ff2e104510 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/callApi/callApi.test.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/callApi/callApi.test.ts @@ -1,10 +1,26 @@ -/* eslint promise/no-callback-in-promise: 'off' */ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ import fetchMock from 'fetch-mock'; import callApi from '../../src/callApi/callApi'; import * as constants from '../../src/constants'; import { LOGIN_GLOB } from '../fixtures/constants'; -import throwIfCalled from '../utils/throwIfCalled'; import { CallApi, JsonObject } from '../../src/types'; import { DEFAULT_FETCH_RETRY_OPTIONS } from '../../src/constants'; @@ -47,27 +63,22 @@ describe('callApi()', () => { afterEach(fetchMock.reset); describe('request config', () => { - it('calls the right url with the specified method', () => { + it('calls the right url with the specified method', async () => { expect.assertions(4); - - return Promise.all([ + await Promise.all([ callApi({ url: mockGetUrl, method: 'GET' }), callApi({ url: mockPostUrl, method: 'POST' }), callApi({ url: mockPutUrl, method: 'PUT' }), callApi({ url: mockPatchUrl, method: 'PATCH' }), - ]).then(() => { - expect(fetchMock.calls(mockGetUrl)).toHaveLength(1); - expect(fetchMock.calls(mockPostUrl)).toHaveLength(1); - expect(fetchMock.calls(mockPutUrl)).toHaveLength(1); - expect(fetchMock.calls(mockPatchUrl)).toHaveLength(1); - - return true; - }); + ]); + expect(fetchMock.calls(mockGetUrl)).toHaveLength(1); + expect(fetchMock.calls(mockPostUrl)).toHaveLength(1); + expect(fetchMock.calls(mockPutUrl)).toHaveLength(1); + expect(fetchMock.calls(mockPatchUrl)).toHaveLength(1); }); - it('passes along mode, cache, credentials, headers, body, signal, and redirect parameters in the request', () => { + it('passes along mode, cache, credentials, headers, body, signal, and redirect parameters in the request', async () => { expect.assertions(8); - const mockRequest: CallApi = { url: mockGetUrl, mode: 'cors', @@ -81,64 +92,55 @@ describe('callApi()', () => { body: 'BODY', }; - 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) as typeof fetchParams.headers, - ); - expect(fetchParams.redirect).toBe(mockRequest.redirect); - expect(fetchParams.signal).toBe(mockRequest.signal); - expect(fetchParams.body).toBe(mockRequest.body); - - return true; - }); + await callApi(mockRequest); + 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) as typeof fetchParams.headers, + ); + expect(fetchParams.redirect).toBe(mockRequest.redirect); + expect(fetchParams.signal).toBe(mockRequest.signal); + expect(fetchParams.body).toBe(mockRequest.body); }); }); describe('POST requests', () => { - it('encodes key,value pairs from postPayload', () => { + it('encodes key,value pairs from postPayload', async () => { expect.assertions(3); const postPayload = { key: 'value', anotherKey: 1237 }; - return callApi({ url: mockPostUrl, method: 'POST', postPayload }).then(() => { - const calls = fetchMock.calls(mockPostUrl); - expect(calls).toHaveLength(1); + await callApi({ url: mockPostUrl, method: 'POST', postPayload }); + const calls = fetchMock.calls(mockPostUrl); + expect(calls).toHaveLength(1); - const fetchParams = calls[0][1]; - const body = fetchParams.body as FormData; + const fetchParams = calls[0][1]; + const body = fetchParams.body as FormData; - Object.entries(postPayload).forEach(([key, value]) => { - expect(body.get(key)).toBe(JSON.stringify(value)); - }); - - return true; + Object.entries(postPayload).forEach(([key, value]) => { + expect(body.get(key)).toBe(JSON.stringify(value)); }); }); // 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 (POST)', () => { + it('omits key,value pairs from postPayload that have undefined values (POST)', async () => { expect.assertions(3); const postPayload = { key: 'value', noValue: undefined }; - return callApi({ url: mockPostUrl, method: 'POST', postPayload }).then(() => { - const calls = fetchMock.calls(mockPostUrl); - expect(calls).toHaveLength(1); + await callApi({ url: mockPostUrl, method: 'POST', postPayload }); + const calls = fetchMock.calls(mockPostUrl); + expect(calls).toHaveLength(1); - const fetchParams = calls[0][1]; - const body = fetchParams.body as FormData; - expect(body.get('key')).toBe(JSON.stringify(postPayload.key)); - expect(body.get('noValue')).toBeNull(); - - return true; - }); + const fetchParams = calls[0][1]; + const body = fetchParams.body as FormData; + expect(body.get('key')).toBe(JSON.stringify(postPayload.key)); + expect(body.get('noValue')).toBeNull(); }); - it('respects the stringify flag in POST requests', () => { + it('respects the stringify flag in POST requests', async () => { const postPayload = { string: 'value', number: 1237, @@ -150,68 +152,59 @@ describe('callApi()', () => { expect.assertions(1 + 3 * Object.keys(postPayload).length); - return Promise.all([ + await Promise.all([ callApi({ url: mockPostUrl, method: 'POST', postPayload }), callApi({ url: mockPostUrl, method: 'POST', postPayload, stringify: false }), callApi({ url: mockPostUrl, method: 'POST', jsonPayload: postPayload }), - ]).then(() => { - const calls = fetchMock.calls(mockPostUrl); - expect(calls).toHaveLength(3); + ]); + const calls = fetchMock.calls(mockPostUrl); + expect(calls).toHaveLength(3); - const stringified = calls[0][1].body as FormData; - const unstringified = calls[1][1].body as FormData; - const jsonRequestBody = JSON.parse(calls[2][1].body as string) as JsonObject; + const stringified = calls[0][1].body as FormData; + const unstringified = calls[1][1].body as FormData; + const jsonRequestBody = JSON.parse(calls[2][1].body as string) as JsonObject; - Object.entries(postPayload).forEach(([key, value]) => { - expect(stringified.get(key)).toBe(JSON.stringify(value)); - expect(unstringified.get(key)).toBe(String(value)); - expect(jsonRequestBody[key]).toEqual(value); - }); - - return true; + Object.entries(postPayload).forEach(([key, value]) => { + expect(stringified.get(key)).toBe(JSON.stringify(value)); + expect(unstringified.get(key)).toBe(String(value)); + expect(jsonRequestBody[key]).toEqual(value); }); }); }); describe('PUT requests', () => { - it('encodes key,value pairs from postPayload', () => { + it('encodes key,value pairs from postPayload', async () => { expect.assertions(3); const postPayload = { key: 'value', anotherKey: 1237 }; - return callApi({ url: mockPutUrl, method: 'PUT', postPayload }).then(() => { - const calls = fetchMock.calls(mockPutUrl); - expect(calls).toHaveLength(1); + await callApi({ url: mockPutUrl, method: 'PUT', postPayload }); + const calls = fetchMock.calls(mockPutUrl); + expect(calls).toHaveLength(1); - const fetchParams = calls[0][1]; - const body = fetchParams.body as FormData; + const fetchParams = calls[0][1]; + const body = fetchParams.body as FormData; - Object.entries(postPayload).forEach(([key, value]) => { - expect(body.get(key)).toBe(JSON.stringify(value)); - }); - - return true; + Object.entries(postPayload).forEach(([key, value]) => { + expect(body.get(key)).toBe(JSON.stringify(value)); }); }); // 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 (PUT)', () => { + it('omits key,value pairs from postPayload that have undefined values (PUT)', async () => { expect.assertions(3); const postPayload = { key: 'value', noValue: undefined }; - return callApi({ url: mockPutUrl, method: 'PUT', postPayload }).then(() => { - const calls = fetchMock.calls(mockPutUrl); - expect(calls).toHaveLength(1); + await callApi({ url: mockPutUrl, method: 'PUT', postPayload }); + const calls = fetchMock.calls(mockPutUrl); + expect(calls).toHaveLength(1); - const fetchParams = calls[0][1]; - const body = fetchParams.body as FormData; - expect(body.get('key')).toBe(JSON.stringify(postPayload.key)); - expect(body.get('noValue')).toBeNull(); - - return true; - }); + const fetchParams = calls[0][1]; + const body = fetchParams.body as FormData; + expect(body.get('key')).toBe(JSON.stringify(postPayload.key)); + expect(body.get('noValue')).toBeNull(); }); - it('respects the stringify flag in PUT requests', () => { + it('respects the stringify flag in PUT requests', async () => { const postPayload = { string: 'value', number: 1237, @@ -223,65 +216,56 @@ describe('callApi()', () => { expect.assertions(1 + 2 * Object.keys(postPayload).length); - return Promise.all([ + await Promise.all([ callApi({ url: mockPutUrl, method: 'PUT', postPayload }), callApi({ url: mockPutUrl, method: 'PUT', postPayload, stringify: false }), - ]).then(() => { - const calls = fetchMock.calls(mockPutUrl); - expect(calls).toHaveLength(2); + ]); + const calls = fetchMock.calls(mockPutUrl); + expect(calls).toHaveLength(2); - const stringified = calls[0][1].body as FormData; - const unstringified = calls[1][1].body as FormData; + const stringified = calls[0][1].body as FormData; + const unstringified = calls[1][1].body as FormData; - Object.entries(postPayload).forEach(([key, value]) => { - expect(stringified.get(key)).toBe(JSON.stringify(value)); - expect(unstringified.get(key)).toBe(String(value)); - }); - - return true; + Object.entries(postPayload).forEach(([key, value]) => { + expect(stringified.get(key)).toBe(JSON.stringify(value)); + expect(unstringified.get(key)).toBe(String(value)); }); }); }); describe('PATCH requests', () => { - it('encodes key,value pairs from postPayload', () => { + it('encodes key,value pairs from postPayload', async () => { expect.assertions(3); const postPayload = { key: 'value', anotherKey: 1237 }; - return callApi({ url: mockPatchUrl, method: 'PATCH', postPayload }).then(() => { - const calls = fetchMock.calls(mockPatchUrl); - expect(calls).toHaveLength(1); + await callApi({ url: mockPatchUrl, method: 'PATCH', postPayload }); + const calls = fetchMock.calls(mockPatchUrl); + expect(calls).toHaveLength(1); - const fetchParams = calls[0][1]; - const body = fetchParams.body as FormData; + const fetchParams = calls[0][1]; + const body = fetchParams.body as FormData; - Object.entries(postPayload).forEach(([key, value]) => { - expect(body.get(key)).toBe(JSON.stringify(value)); - }); - - return true; + Object.entries(postPayload).forEach(([key, value]) => { + expect(body.get(key)).toBe(JSON.stringify(value)); }); }); // 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 (PATCH)', () => { + it('omits key,value pairs from postPayload that have undefined values (PATCH)', async () => { expect.assertions(3); const postPayload = { key: 'value', noValue: undefined }; - return callApi({ url: mockPatchUrl, method: 'PATCH', postPayload }).then(() => { - const calls = fetchMock.calls(mockPatchUrl); - expect(calls).toHaveLength(1); + await callApi({ url: mockPatchUrl, method: 'PATCH', postPayload }); + const calls = fetchMock.calls(mockPatchUrl); + expect(calls).toHaveLength(1); - const fetchParams = calls[0][1]; - const body = fetchParams.body as FormData; - expect(body.get('key')).toBe(JSON.stringify(postPayload.key)); - expect(body.get('noValue')).toBeNull(); - - return true; - }); + const fetchParams = calls[0][1]; + const body = fetchParams.body as FormData; + expect(body.get('key')).toBe(JSON.stringify(postPayload.key)); + expect(body.get('noValue')).toBeNull(); }); - it('respects the stringify flag in PATCH requests', () => { + it('respects the stringify flag in PATCH requests', async () => { const postPayload = { string: 'value', number: 1237, @@ -293,22 +277,19 @@ describe('callApi()', () => { expect.assertions(1 + 2 * Object.keys(postPayload).length); - return Promise.all([ + await Promise.all([ callApi({ url: mockPatchUrl, method: 'PATCH', postPayload }), callApi({ url: mockPatchUrl, method: 'PATCH', postPayload, stringify: false }), - ]).then(() => { - const calls = fetchMock.calls(mockPatchUrl); - expect(calls).toHaveLength(2); + ]); + const calls = fetchMock.calls(mockPatchUrl); + expect(calls).toHaveLength(2); - const stringified = calls[0][1].body as FormData; - const unstringified = calls[1][1].body as FormData; + const stringified = calls[0][1].body as FormData; + const unstringified = calls[1][1].body as FormData; - Object.entries(postPayload).forEach(([key, value]) => { - expect(stringified.get(key)).toBe(JSON.stringify(value)); - expect(unstringified.get(key)).toBe(String(value)); - }); - - return true; + Object.entries(postPayload).forEach(([key, value]) => { + expect(stringified.get(key)).toBe(JSON.stringify(value)); + expect(unstringified.get(key)).toBe(String(value)); }); }); }); @@ -326,42 +307,34 @@ describe('callApi()', () => { beforeEach(() => { self.location.protocol = 'https:'; - return caches.delete(constants.CACHE_KEY); }); - it('caches requests with ETags', () => - callApi({ url: mockCacheUrl, method: 'GET' }).then(() => { - const calls = fetchMock.calls(mockCacheUrl); - expect(calls).toHaveLength(1); + it('caches requests with ETags', async () => { + expect.assertions(2); + await callApi({ url: mockCacheUrl, method: 'GET' }); + const calls = fetchMock.calls(mockCacheUrl); + expect(calls).toHaveLength(1); + const supersetCache = await caches.open(constants.CACHE_KEY); + const cachedResponse = await supersetCache.match(mockCacheUrl); + expect(cachedResponse).toBeDefined(); + }); - return caches.open(constants.CACHE_KEY).then(supersetCache => - supersetCache.match(mockCacheUrl).then(cachedResponse => { - expect(cachedResponse).toBeDefined(); - - return true; - }), - ); - })); - - it('will not use cache when running off an insecure connection', () => { + it('will not use cache when running off an insecure connection', async () => { + expect.assertions(2); self.location.protocol = 'http:'; - return callApi({ url: mockCacheUrl, method: 'GET' }).then(() => { - const calls = fetchMock.calls(mockCacheUrl); - expect(calls).toHaveLength(1); + await callApi({ url: mockCacheUrl, method: 'GET' }); + const calls = fetchMock.calls(mockCacheUrl); + expect(calls).toHaveLength(1); - return caches.open(constants.CACHE_KEY).then(supersetCache => - supersetCache.match(mockCacheUrl).then(cachedResponse => { - expect(cachedResponse).toBeUndefined(); - - return true; - }), - ); - }); + const supersetCache = await caches.open(constants.CACHE_KEY); + const cachedResponse = await supersetCache.match(mockCacheUrl); + expect(cachedResponse).toBeUndefined(); }); it('works when the Cache API is disabled', async () => { + expect.assertions(5); // eslint-disable-next-line no-import-assign Object.defineProperty(constants, 'CACHE_AVAILABLE', { value: false }); @@ -383,26 +356,25 @@ describe('callApi()', () => { Object.defineProperty(constants, 'CACHE_AVAILABLE', { value: true }); }); - it('sends known ETags in the If-None-Match header', () => + it('sends known ETags in the If-None-Match header', async () => { + expect.assertions(3); // first call sets the cache - callApi({ url: mockCacheUrl, method: 'GET' }).then(() => { - const calls = fetchMock.calls(mockCacheUrl); - expect(calls).toHaveLength(1); + await callApi({ url: mockCacheUrl, method: 'GET' }); + const calls = fetchMock.calls(mockCacheUrl); + expect(calls).toHaveLength(1); - // second call sends the Etag in the If-None-Match header - return callApi({ url: mockCacheUrl, method: 'GET' }).then(() => { - const fetchParams = calls[1][1]; - const headers = { 'If-None-Match': 'etag' }; - expect(calls).toHaveLength(2); - expect(fetchParams.headers).toEqual( - expect.objectContaining(headers) as typeof fetchParams.headers, - ); - - return true; - }); - })); + // second call sends the Etag in the If-None-Match header + await callApi({ url: mockCacheUrl, method: 'GET' }); + const fetchParams = calls[1][1]; + const headers = { 'If-None-Match': 'etag' }; + expect(calls).toHaveLength(2); + expect(fetchParams.headers).toEqual( + expect.objectContaining(headers) as typeof fetchParams.headers, + ); + }); it('reuses cached responses on 304 status', async () => { + expect.assertions(3); // first call sets the cache await callApi({ url: mockCacheUrl, method: 'GET' }); const calls = fetchMock.calls(mockCacheUrl); @@ -417,23 +389,28 @@ describe('callApi()', () => { expect(secondBody).toEqual('BODY'); }); - it('throws error when cache fails on 304', () => { + it('throws error when cache fails on 304', async () => { + expect.assertions(2); + // this should never happen, since a 304 is only returned if we have // the cached response and sent the If-None-Match header const mockUncachedUrl = '/mock/uncached/url'; const mockCachedPayload = { status: 304 }; fetchMock.get(mockUncachedUrl, mockCachedPayload); - return callApi({ url: mockUncachedUrl, method: 'GET' }).catch( - (error: { message: string }) => { - const calls = fetchMock.calls(mockUncachedUrl); - expect(calls).toHaveLength(1); - expect(error.message).toEqual('Received 304 but no content is cached!'); - }, - ); + try { + await callApi({ url: mockUncachedUrl, method: 'GET' }); + } catch (error) { + const calls = fetchMock.calls(mockUncachedUrl); + expect(calls).toHaveLength(1); + expect((error as { message: string }).message).toEqual( + 'Received 304 but no content is cached!', + ); + } }); it('returns original response if no Etag', async () => { + expect.assertions(3); const url = mockGetUrl; const response = await callApi({ url, method: 'GET' }); const calls = fetchMock.calls(url); @@ -444,6 +421,7 @@ describe('callApi()', () => { }); it('returns original response if status not 304 or 200', async () => { + expect.assertions(2); const url = mockNotFound; const response = await callApi({ url, method: 'GET' }); const calls = fetchMock.calls(url); @@ -452,39 +430,40 @@ describe('callApi()', () => { }); }); - it('rejects after retrying thrice if the request throws', () => { + it('rejects after retrying thrice if the request throws', async () => { expect.assertions(3); - - return callApi({ - fetchRetryOptions: DEFAULT_FETCH_RETRY_OPTIONS, - url: mockErrorUrl, - method: 'GET', - }) - .then(throwIfCalled) - .catch((error: { status: number; statusText: string }) => { - expect(fetchMock.calls(mockErrorUrl)).toHaveLength(4); - expect(error.status).toBe(mockErrorPayload.status); - expect(error.statusText).toBe(mockErrorPayload.statusText); + try { + await callApi({ + fetchRetryOptions: DEFAULT_FETCH_RETRY_OPTIONS, + url: mockErrorUrl, + method: 'GET', }); + } catch (error) { + const err = error as { status: number; statusText: string }; + expect(fetchMock.calls(mockErrorUrl)).toHaveLength(4); + expect(err.status).toBe(mockErrorPayload.status); + expect(err.statusText).toBe(mockErrorPayload.statusText); + } }); - it('rejects without retries if the config is set to 0 retries', () => { + it('rejects without retries if the config is set to 0 retries', async () => { expect.assertions(3); - - return callApi({ - fetchRetryOptions: { retries: 0 }, - url: mockErrorUrl, - method: 'GET', - }) - .then(throwIfCalled) - .catch((error: { status: number; statusText: string }) => { - expect(fetchMock.calls(mockErrorUrl)).toHaveLength(1); - expect(error.status).toBe(mockErrorPayload.status); - expect(error.statusText).toBe(mockErrorPayload.statusText); + try { + await callApi({ + fetchRetryOptions: { retries: 0 }, + url: mockErrorUrl, + method: 'GET', }); + } catch (error) { + const err = error as { status: number; statusText: string }; + expect(fetchMock.calls(mockErrorUrl)).toHaveLength(1); + expect(err.status).toBe(mockErrorPayload.status); + expect(err.statusText).toBe(mockErrorPayload.statusText); + } }); it('rejects after retrying thrice if the request returns a 503', async () => { + expect.assertions(2); const url = mock503; const response = await callApi({ fetchRetryOptions: DEFAULT_FETCH_RETRY_OPTIONS, @@ -496,13 +475,17 @@ describe('callApi()', () => { expect(response.status).toEqual(503); }); - it('invalid json for postPayload should thrown error', () => { - expect(() => { - callApi({ + it('invalid json for postPayload should thrown error', async () => { + expect.assertions(2); + try { + await callApi({ url: mockPostUrl, method: 'POST', postPayload: 'haha', }); - }).toThrow('Invalid postPayload:\n\nhaha'); + } catch (error) { + expect(error).toBeInstanceOf(Error); + expect(error.message).toEqual('Invalid payload:\n\nhaha'); + } }); }); diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/callApi/callApiAndParseWithTimeout.test.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/callApi/callApiAndParseWithTimeout.test.ts index 12a31f2449..0fd2d13e86 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/callApi/callApiAndParseWithTimeout.test.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/callApi/callApiAndParseWithTimeout.test.ts @@ -1,3 +1,21 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ import fetchMock from 'fetch-mock'; import callApiAndParseWithTimeout from '../../src/callApi/callApiAndParseWithTimeout'; @@ -8,7 +26,6 @@ import * as parseResponse from '../../src/callApi/parseResponse'; import * as rejectAfterTimeout from '../../src/callApi/rejectAfterTimeout'; import { LOGIN_GLOB } from '../fixtures/constants'; -import throwIfCalled from '../utils/throwIfCalled'; describe('callApiAndParseWithTimeout()', () => { beforeAll(() => { @@ -64,40 +81,39 @@ describe('callApiAndParseWithTimeout()', () => { rejectionSpy.mockClear(); }); - it('rejects if the request exceeds the timeout', () => { - return new Promise(done => { - expect.assertions(3); - jest.useFakeTimers(); + it('rejects if the request exceeds the timeout', async () => { + expect.assertions(2); + jest.useFakeTimers(); - const mockTimeoutUrl = '/mock/timeout/url'; - const unresolvingPromise = new Promise(() => {}); - fetchMock.get(mockTimeoutUrl, () => unresolvingPromise); - - callApiAndParseWithTimeout({ url: mockTimeoutUrl, method: 'GET', timeout: 1 }) - .then(throwIfCalled) - .catch((error: { error: string; statusText: string }) => { - expect(fetchMock.calls(mockTimeoutUrl)).toHaveLength(1); - expect(Object.keys(error)).toEqual(['error', 'statusText']); - expect(error.statusText).toBe('timeout'); - - return done(); // eslint-disable-line promise/no-callback-in-promise - }); + const mockTimeoutUrl = '/mock/timeout/url'; + const unresolvingPromise = new Promise(() => {}); + fetchMock.get(mockTimeoutUrl, () => unresolvingPromise); + try { + const promise = callApiAndParseWithTimeout({ + url: mockTimeoutUrl, + method: 'GET', + timeout: 1, + }); jest.advanceTimersByTime(2); - }); + await promise; + } catch (error) { + expect(fetchMock.calls(mockTimeoutUrl)).toHaveLength(1); + expect(error).toEqual({ + error: 'Request timed out', + statusText: 'timeout', + }); + } }); - it('resolves if the request does not exceed the timeout', () => { + it('resolves if the request does not exceed the timeout', async () => { expect.assertions(1); - - return callApiAndParseWithTimeout({ url: mockGetUrl, method: 'GET', timeout: 100 }).then( - response => { - // eslint-disable-next-line @typescript-eslint/no-unsafe-call - expect(response.json).toEqual(expect.objectContaining(mockGetPayload)); - - return true; - }, - ); + const { json } = await callApiAndParseWithTimeout({ + url: mockGetUrl, + method: 'GET', + timeout: 100, + }); + expect(json).toEqual(mockGetPayload); }); }); }); diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/callApi/parseResponse.test.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/callApi/parseResponse.test.ts index 105b64c43b..b0a568c9ee 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/callApi/parseResponse.test.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/callApi/parseResponse.test.ts @@ -1,10 +1,26 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ import fetchMock from 'fetch-mock'; import callApi from '../../src/callApi/callApi'; import parseResponse from '../../src/callApi/parseResponse'; import { LOGIN_GLOB } from '../fixtures/constants'; -import throwIfCalled from '../utils/throwIfCalled'; -import { SupersetClientResponse } from '../../src'; describe('parseResponse()', () => { beforeAll(() => { @@ -35,22 +51,17 @@ describe('parseResponse()', () => { expect(parsedResponsePromise).toBeInstanceOf(Promise); }); - it('resolves to { json, response } if the request succeeds', () => { + it('resolves to { json, response } if the request succeeds', async () => { expect.assertions(4); - const apiPromise = callApi({ url: mockGetUrl, method: 'GET' }); - - return parseResponse(apiPromise).then(args => { - expect(fetchMock.calls(mockGetUrl)).toHaveLength(1); - const keys = Object.keys(args); - expect(keys).toContain('response'); - expect(keys).toContain('json'); - expect(args.json).toEqual(expect.objectContaining(mockGetPayload) as typeof args.json); - - return true; - }); + const args = await parseResponse(callApi({ url: mockGetUrl, method: 'GET' })); + expect(fetchMock.calls(mockGetUrl)).toHaveLength(1); + const keys = Object.keys(args); + expect(keys).toContain('response'); + expect(keys).toContain('json'); + expect(args.json).toEqual(expect.objectContaining(mockGetPayload) as typeof args.json); }); - it('throws if `parseMethod=json` and .json() fails', () => { + it('throws if `parseMethod=json` and .json() fails', async () => { expect.assertions(3); const mockTextUrl = '/mock/text/url'; @@ -58,20 +69,17 @@ describe('parseResponse()', () => { 'I could be a stack trace or something'; fetchMock.get(mockTextUrl, mockTextResponse); - const apiPromise = callApi({ url: mockTextUrl, method: 'GET' }); - - return parseResponse(apiPromise, 'json') - .then(throwIfCalled) - .catch((error: { stack: unknown; message: string }) => { - expect(fetchMock.calls(mockTextUrl)).toHaveLength(1); - expect(error.stack).toBeDefined(); - expect(error.message).toContain('Unexpected token'); - - return true; - }); + try { + await parseResponse(callApi({ url: mockTextUrl, method: 'GET' })); + } catch (error) { + const err = error as Error; + expect(fetchMock.calls(mockTextUrl)).toHaveLength(1); + expect(err.stack).toBeDefined(); + expect(err.message).toContain('Unexpected token'); + } }); - it('resolves to { text, response } if the `parseMethod=text`', () => { + it('resolves to { text, response } if the `parseMethod=text`', async () => { expect.assertions(4); // test with json + bigint to ensure that it was not first parsed as json @@ -79,53 +87,49 @@ describe('parseResponse()', () => { const mockTextJsonResponse = '{ "value": 9223372036854775807 }'; fetchMock.get(mockTextParseUrl, mockTextJsonResponse); - const apiPromise = callApi({ url: mockTextParseUrl, method: 'GET' }); - - return parseResponse(apiPromise, 'text').then(args => { - expect(fetchMock.calls(mockTextParseUrl)).toHaveLength(1); - const keys = Object.keys(args); - expect(keys).toContain('response'); - expect(keys).toContain('text'); - expect(args.text).toBe(mockTextJsonResponse); - - return true; - }); + const args = await parseResponse(callApi({ url: mockTextParseUrl, method: 'GET' }), 'text'); + expect(fetchMock.calls(mockTextParseUrl)).toHaveLength(1); + const keys = Object.keys(args); + expect(keys).toContain('response'); + expect(keys).toContain('text'); + expect(args.text).toBe(mockTextJsonResponse); }); - it('throws if parseMethod is not null|json|text', () => { - const apiPromise = callApi({ url: mockNoParseUrl, method: 'GET' }); - - // @ts-ignore - 'something-else' is *intentionally* an invalid type - expect(() => parseResponse(apiPromise, 'something-else')).toThrow(); + it('throws if parseMethod is not null|json|text', async () => { + expect.assertions(1); + try { + await parseResponse( + callApi({ url: mockNoParseUrl, method: 'GET' }), + 'something-else' as never, + ); + } catch (error) { + expect(error.message).toEqual(expect.stringContaining('Expected parseResponse=json')); + } }); - it('resolves to the unmodified `Response` object if `parseMethod=null`', () => { - expect.assertions(2); - - const apiPromise = callApi({ url: mockNoParseUrl, method: 'GET' }); - - return parseResponse(apiPromise, null).then((clientResponse: SupersetClientResponse) => { - const response = clientResponse as Response; - expect(fetchMock.calls(mockNoParseUrl)).toHaveLength(1); - expect(response.bodyUsed).toBe(false); - - return true; - }); + it('resolves to unmodified `Response` object if `parseMethod=null|raw`', async () => { + expect.assertions(3); + const responseNull = await parseResponse(callApi({ url: mockNoParseUrl, method: 'GET' }), null); + const responseRaw = await parseResponse(callApi({ url: mockNoParseUrl, method: 'GET' }), 'raw'); + expect(fetchMock.calls(mockNoParseUrl)).toHaveLength(2); + expect(responseNull.bodyUsed).toBe(false); + expect(responseRaw.bodyUsed).toBe(false); }); - it('rejects if request.ok=false', () => { + it('rejects if request.ok=false', async () => { + expect.assertions(3); const mockNotOkayUrl = '/mock/notokay/url'; fetchMock.get(mockNotOkayUrl, 404); // 404s result in not response.ok=false - expect.assertions(3); const apiPromise = callApi({ url: mockNotOkayUrl, method: 'GET' }); - return parseResponse(apiPromise) - .then(throwIfCalled) - .catch((error: { ok: boolean; status: number }) => { - expect(fetchMock.calls(mockNotOkayUrl)).toHaveLength(1); - expect(error.ok).toBe(false); - expect(error.status).toBe(404); - }); + try { + await parseResponse(apiPromise); + } catch (error) { + const err = error as { ok: boolean; status: number }; + expect(fetchMock.calls(mockNotOkayUrl)).toHaveLength(1); + expect(err.ok).toBe(false); + expect(err.status).toBe(404); + } }); }); diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/callApi/rejectAfterTimeout.test.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/callApi/rejectAfterTimeout.test.ts index b7aedb023b..9b08430e74 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/callApi/rejectAfterTimeout.test.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/callApi/rejectAfterTimeout.test.ts @@ -1,23 +1,34 @@ -/* eslint promise/no-callback-in-promise: 'off' */ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ import rejectAfterTimeout from '../../src/callApi/rejectAfterTimeout'; -import throwIfCalled from '../utils/throwIfCalled'; describe('rejectAfterTimeout()', () => { - it('returns a promise that rejects after the specified timeout', () => { - return new Promise(done => { - expect.assertions(1); - jest.useFakeTimers(); - - rejectAfterTimeout(10) - .then(throwIfCalled) - .catch((error: Error) => { - expect(error).toBeDefined(); - - return done(); - }); - + it('returns a promise that rejects after the specified timeout', async () => { + expect.assertions(1); + jest.useFakeTimers(); + try { + const promise = rejectAfterTimeout(10); jest.advanceTimersByTime(11); - jest.useRealTimers(); - }); + await promise; + } catch (error) { + expect(error).toBeDefined(); + } + jest.useRealTimers(); }); }); diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/fixtures/constants.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/fixtures/constants.ts index 4457bc0115..30faacda85 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/fixtures/constants.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/fixtures/constants.ts @@ -1 +1,19 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ export const LOGIN_GLOB = 'glob:*superset/csrf_token/*'; // eslint-disable-line import/prefer-default-export diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/utils/throwIfCalled.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/utils/throwIfCalled.ts deleted file mode 100644 index c0b11ee8f0..0000000000 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-connection/test/utils/throwIfCalled.ts +++ /dev/null @@ -1,3 +0,0 @@ -export default function throwIfCalled(args: unknown) { - throw new Error(`Unexpected call to throwIfCalled(): ${JSON.stringify(args)}`); -} diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-demo/storybook/shared/components/ErrorMessage.tsx b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-demo/storybook/shared/components/ErrorMessage.tsx index 78b38d7fa6..8b41aebb64 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-demo/storybook/shared/components/ErrorMessage.tsx +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-demo/storybook/shared/components/ErrorMessage.tsx @@ -5,12 +5,14 @@ export type Props = { }; export default function ErrorMessage({ error }: Props) { + // eslint-disable-next-line no-console + console.error(error); return ( -
+ 
       {error.stack || error.message}
       {!error.message &&
         !error.stack &&
-        (typeof error === 'object' ? JSON.stringify(error) : String(error))}
-
+ (typeof error === 'object' ? JSON.stringify(error, null, 2) : String(error))} + ); } diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-demo/storybook/stories/superset-ui-chart/ChartDataProviderStories.tsx b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-demo/storybook/stories/superset-ui-chart/ChartDataProviderStories.tsx index c02c94f948..a680b3eff4 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-demo/storybook/stories/superset-ui-chart/ChartDataProviderStories.tsx +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-demo/storybook/stories/superset-ui-chart/ChartDataProviderStories.tsx @@ -1,5 +1,5 @@ import React from 'react'; -import { text, select } from '@storybook/addon-knobs'; +import { text, select, withKnobs } from '@storybook/addon-knobs'; import { SuperChart, ChartDataProvider } from '@superset-ui/chart'; import { SupersetClient } from '@superset-ui/connection'; @@ -26,13 +26,12 @@ const WORD_CLOUD_LEGACY = wordCloudFormData.viz_type; const WORD_CLOUD = 'new_word_cloud'; new LegacyBigNumberPlugin().configure({ key: BIG_NUMBER }).register(); -// @ts-ignore +// eslint-disable-next-line new LegacySankeyPlugin().configure({ key: SANKEY }).register(); -// @ts-ignore +// eslint-disable-next-line new LegacySunburstPlugin().configure({ key: SUNBURST }).register(); -// @ts-ignore +// eslint-disable-next-line new LegacyWordCloudPlugin().configure({ key: WORD_CLOUD_LEGACY }).register(); -// @ts-ignore new WordCloudChartPlugin().configure({ key: WORD_CLOUD }).register(); const VIS_TYPES = [BIG_NUMBER, SANKEY, SUNBURST, WORD_CLOUD, WORD_CLOUD_LEGACY]; @@ -46,14 +45,19 @@ const FORM_DATA_LOOKUP = { export default { title: 'Core Packages|@superset-ui/chart', + decorators: [ + withKnobs({ + escapeHTML: false, + }), + ], }; export const dataProvider = () => { - const host = text('Set Superset App host for CORS request', 'localhost:9000'); + const host = text('Set Superset App host for CORS request', 'localhost:8088'); const visType = select('Chart Plugin Type', VIS_TYPES, VIS_TYPES[0]); - const formData = text('Override formData', JSON.stringify(FORM_DATA_LOOKUP[visType])); const width = text('Vis width', '500'); const height = text('Vis height', '300'); + const formData = text('Override formData', JSON.stringify(FORM_DATA_LOOKUP[visType])); return (
diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-demo/storybook/stories/superset-ui-connection/ConnectionStories.tsx b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-demo/storybook/stories/superset-ui-connection/ConnectionStories.tsx index 0155dce88c..d9b76f613e 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-demo/storybook/stories/superset-ui-connection/ConnectionStories.tsx +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-demo/storybook/stories/superset-ui-connection/ConnectionStories.tsx @@ -1,6 +1,6 @@ import React from 'react'; import { select, text, withKnobs } from '@storybook/addon-knobs'; -import { sankeyFormData } from '@superset-ui/chart/test/fixtures/formData'; +import { bigNumberFormData } from '@superset-ui/chart/test/fixtures/formData'; import VerifyCORS, { Props as VerifyCORSProps } from '../../shared/components/VerifyCORS'; import Expandable from '../../shared/components/Expandable'; @@ -21,14 +21,14 @@ export default { }; export const configureCORS = () => { - const host = text('Superset App host for CORS request', 'localhost:9000'); + const host = text('Superset App host for CORS request', 'localhost:8088'); const selectEndpoint = select('Endpoint', ENDPOINTS, ''); const customEndpoint = text('Custom Endpoint (override above)', ''); const endpoint = customEndpoint || selectEndpoint; const method = endpoint ? select('Request method', REQUEST_METHODS, 'POST') : undefined; const postPayload = endpoint && method === 'POST' - ? text('POST payload', JSON.stringify({ form_data: sankeyFormData }, null, 2)) + ? text('POST payload', JSON.stringify({ form_data: bigNumberFormData })) : undefined; return (