From d8000621599608aed75582a2d3e3887616b2374f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 14 Nov 2022 23:25:39 +0100 Subject: [PATCH] Extracted helper fetch function and migrated remoteServers redux action from axios to fetch --- src/api/services/ShlinkApiClient.ts | 30 +++----- src/api/services/provideServices.ts | 2 +- src/common/services/provideServices.ts | 2 + src/servers/reducers/remoteServers.ts | 12 ++-- src/servers/services/provideServices.ts | 2 +- src/utils/helpers/fetch.ts | 10 +++ src/utils/types.ts | 2 +- test/api/services/ShlinkApiClient.test.ts | 6 +- test/servers/reducers/remoteServers.test.ts | 79 ++++++++++----------- 9 files changed, 67 insertions(+), 78 deletions(-) create mode 100644 src/utils/helpers/fetch.ts diff --git a/src/api/services/ShlinkApiClient.ts b/src/api/services/ShlinkApiClient.ts index 5ac02e2d..bcca4505 100644 --- a/src/api/services/ShlinkApiClient.ts +++ b/src/api/services/ShlinkApiClient.ts @@ -119,29 +119,19 @@ export class ShlinkApiClient { const normalizedQuery = stringifyQuery(rejectNilProps(query)); const stringifiedQuery = isEmpty(normalizedQuery) ? '' : `?${normalizedQuery}`; - return this.fetch(`${buildShlinkBaseUrl(this.baseUrl, this.apiVersion)}${url}${stringifiedQuery}`, { + return this.fetch(`${buildShlinkBaseUrl(this.baseUrl, this.apiVersion)}${url}${stringifiedQuery}`, { method, body: body && JSON.stringify(body), headers: { 'X-Api-Key': this.apiKey }, - }) - .then(async (resp) => { - const parsed = await resp.json(); + }).catch((e: unknown) => { + if (!isRegularNotFound(parseApiError(e))) { + throw e; + } - if (!resp.ok) { - throw parsed; // eslint-disable-line @typescript-eslint/no-throw-literal - } - - return parsed as T; // TODO Improve type inference here without explicit casting - }) - .catch((e: unknown) => { - if (!isRegularNotFound(parseApiError(e))) { - throw e; - } - - // If we capture a not found error, let's assume this Shlink version does not support API v3, so we decrease to - // v2 and retry - this.apiVersion = 2; - return this.performRequest(url, method, query, body); - }); + // If we capture a not found error, let's assume this Shlink version does not support API v3, so we decrease to + // v2 and retry + this.apiVersion = 2; + return this.performRequest(url, method, query, body); + }); }; } diff --git a/src/api/services/provideServices.ts b/src/api/services/provideServices.ts index a8d61f13..07697256 100644 --- a/src/api/services/provideServices.ts +++ b/src/api/services/provideServices.ts @@ -2,7 +2,7 @@ import Bottle from 'bottlejs'; import { buildShlinkApiClient } from './ShlinkApiClientBuilder'; const provideServices = (bottle: Bottle) => { - bottle.serviceFactory('buildShlinkApiClient', buildShlinkApiClient, 'fetch'); + bottle.serviceFactory('buildShlinkApiClient', buildShlinkApiClient, 'jsonFetch'); }; export default provideServices; diff --git a/src/common/services/provideServices.ts b/src/common/services/provideServices.ts index 18558991..335a74b1 100644 --- a/src/common/services/provideServices.ts +++ b/src/common/services/provideServices.ts @@ -12,6 +12,7 @@ import { withoutSelectedServer } from '../../servers/helpers/withoutSelectedServ import { sidebarNotPresent, sidebarPresent } from '../reducers/sidebar'; import { ImageDownloader } from './ImageDownloader'; import { ReportExporter } from './ReportExporter'; +import { jsonFetch } from '../../utils/helpers/fetch'; const provideServices = (bottle: Bottle, connect: ConnectDecorator) => { // Services @@ -19,6 +20,7 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator) => { bottle.constant('console', global.console); bottle.constant('axios', axios); bottle.constant('fetch', (global as any).fetch.bind((global as any))); + bottle.serviceFactory('jsonFetch', jsonFetch, 'fetch'); bottle.service('ImageDownloader', ImageDownloader, 'axios', 'window'); bottle.service('ReportExporter', ReportExporter, 'window', 'jsonToCsv'); diff --git a/src/servers/reducers/remoteServers.ts b/src/servers/reducers/remoteServers.ts index 8f5083a8..9baa0f58 100644 --- a/src/servers/reducers/remoteServers.ts +++ b/src/servers/reducers/remoteServers.ts @@ -1,19 +1,15 @@ -import { pipe, prop } from 'ramda'; -import { AxiosInstance } from 'axios'; import pack from '../../../package.json'; import { hasServerData, ServerData } from '../data'; import { createServers } from './servers'; import { createAsyncThunk } from '../../utils/helpers/redux'; +import { Fetch } from '../../utils/types'; -const responseToServersList = pipe( - prop('data'), - (data: any): ServerData[] => (Array.isArray(data) ? data.filter(hasServerData) : []), -); +const responseToServersList = (data: any): ServerData[] => (Array.isArray(data) ? data.filter(hasServerData) : []); -export const fetchServers = ({ get }: AxiosInstance) => createAsyncThunk( +export const fetchServers = (fetch: Fetch) => createAsyncThunk( 'shlink/remoteServers/fetchServers', async (_: void, { dispatch }): Promise => { - const resp = await get(`${pack.homepage}/servers.json`); + const resp = await fetch(`${pack.homepage}/servers.json`); const result = responseToServersList(resp); dispatch(createServers(result)); diff --git a/src/servers/services/provideServices.ts b/src/servers/services/provideServices.ts index 842f931a..73235dc5 100644 --- a/src/servers/services/provideServices.ts +++ b/src/servers/services/provideServices.ts @@ -80,7 +80,7 @@ const provideServices = (bottle: Bottle, connect: ConnectDecorator) => { bottle.serviceFactory('deleteServer', () => deleteServer); bottle.serviceFactory('editServer', () => editServer); bottle.serviceFactory('setAutoConnect', () => setAutoConnect); - bottle.serviceFactory('fetchServers', fetchServers, 'axios'); + bottle.serviceFactory('fetchServers', fetchServers, 'jsonFetch'); bottle.serviceFactory('resetSelectedServer', () => resetSelectedServer); diff --git a/src/utils/helpers/fetch.ts b/src/utils/helpers/fetch.ts new file mode 100644 index 00000000..98f5fdb3 --- /dev/null +++ b/src/utils/helpers/fetch.ts @@ -0,0 +1,10 @@ +export const jsonFetch = (fetch: typeof window.fetch) => (url: string, options?: RequestInit) => fetch(url, options) + .then(async (resp) => { + const parsed = await resp.json(); + + if (!resp.ok) { + throw parsed; // eslint-disable-line @typescript-eslint/no-throw-literal + } + + return parsed as T; + }); diff --git a/src/utils/types.ts b/src/utils/types.ts index 953c1981..09a88ef5 100644 --- a/src/utils/types.ts +++ b/src/utils/types.ts @@ -1,3 +1,3 @@ export type MediaMatcher = (query: string) => MediaQueryList; -export type Fetch = typeof window.fetch; +export type Fetch = (url: string, options?: RequestInit) => Promise; diff --git a/test/api/services/ShlinkApiClient.test.ts b/test/api/services/ShlinkApiClient.test.ts index 28dc7ee7..c65c9e0c 100644 --- a/test/api/services/ShlinkApiClient.test.ts +++ b/test/api/services/ShlinkApiClient.test.ts @@ -6,10 +6,8 @@ import { ShortUrl, ShortUrlsOrder } from '../../../src/short-urls/data'; import { Fetch } from '../../../src/utils/types'; describe('ShlinkApiClient', () => { - const buildFetch = (data: any) => jest.fn().mockResolvedValue({ json: () => Promise.resolve(data), ok: true }); - const buildRejectedFetch = (error: any) => jest.fn().mockResolvedValueOnce( - { json: () => Promise.resolve(error), ok: false }, - ); + const buildFetch = (data: any) => jest.fn().mockResolvedValue(data); + const buildRejectedFetch = (error: any) => jest.fn().mockRejectedValueOnce(error); const buildApiClient = (fetch: Fetch) => new ShlinkApiClient(fetch, '', ''); const shortCodesWithDomainCombinations: [string, OptionalString][] = [ ['abc123', null], diff --git a/test/servers/reducers/remoteServers.test.ts b/test/servers/reducers/remoteServers.test.ts index 41e19238..efc44907 100644 --- a/test/servers/reducers/remoteServers.test.ts +++ b/test/servers/reducers/remoteServers.test.ts @@ -1,5 +1,3 @@ -import { Mock } from 'ts-mockery'; -import { AxiosInstance } from 'axios'; import { fetchServers } from '../../../src/servers/reducers/remoteServers'; import { createServers } from '../../../src/servers/reducers/servers'; @@ -8,27 +6,24 @@ describe('remoteServersReducer', () => { describe('fetchServers', () => { const dispatch = jest.fn(); - const get = jest.fn(); - const axios = Mock.of({ get }); + const fetch = jest.fn(); it.each([ [ - { - data: [ - { - id: '111', - name: 'acel.me from servers.json', - url: 'https://acel.me', - apiKey: '07fb8a96-8059-4094-a24c-80a7d5e7e9b0', - }, - { - id: '222', - name: 'Local from servers.json', - url: 'http://localhost:8000', - apiKey: '7a531c75-134e-4d5c-86e0-a71b7167b57a', - }, - ], - }, + [ + { + id: '111', + name: 'acel.me from servers.json', + url: 'https://acel.me', + apiKey: '07fb8a96-8059-4094-a24c-80a7d5e7e9b0', + }, + { + id: '222', + name: 'Local from servers.json', + url: 'http://localhost:8000', + apiKey: '7a531c75-134e-4d5c-86e0-a71b7167b57a', + }, + ], { 111: { id: '111', @@ -45,26 +40,24 @@ describe('remoteServersReducer', () => { }, ], [ - { - data: [ - { - id: '111', - name: 'acel.me from servers.json', - url: 'https://acel.me', - apiKey: '07fb8a96-8059-4094-a24c-80a7d5e7e9b0', - }, - { - id: '222', - name: 'Invalid', - }, - { - id: '333', - name: 'Local from servers.json', - url: 'http://localhost:8000', - apiKey: '7a531c75-134e-4d5c-86e0-a71b7167b57a', - }, - ], - }, + [ + { + id: '111', + name: 'acel.me from servers.json', + url: 'https://acel.me', + apiKey: '07fb8a96-8059-4094-a24c-80a7d5e7e9b0', + }, + { + id: '222', + name: 'Invalid', + }, + { + id: '333', + name: 'Local from servers.json', + url: 'http://localhost:8000', + apiKey: '7a531c75-134e-4d5c-86e0-a71b7167b57a', + }, + ], { 111: { id: '111', @@ -83,8 +76,8 @@ describe('remoteServersReducer', () => { ['', {}], [{}, {}], ])('tries to fetch servers from remote', async (mockedValue, expectedNewServers) => { - get.mockResolvedValue(mockedValue); - const doFetchServers = fetchServers(axios); + fetch.mockResolvedValue(mockedValue); + const doFetchServers = fetchServers(fetch); await doFetchServers()(dispatch, jest.fn(), {}); @@ -95,7 +88,7 @@ describe('remoteServersReducer', () => { expect(dispatch).toHaveBeenNthCalledWith(3, expect.objectContaining({ type: doFetchServers.fulfilled.toString(), })); - expect(get).toHaveBeenCalledTimes(1); + expect(fetch).toHaveBeenCalledTimes(1); }); }); });