From 72a8f20dcec49e0e7f92af7391392c4ec020a3ae Mon Sep 17 00:00:00 2001 From: Guillaume NICOLAS Date: Mon, 9 Oct 2023 16:38:05 +0200 Subject: [PATCH] feat(TDP-12106): rework http interceptors to access request object and return promise (#4899) --- .changeset/brave-birds-report.md | 5 ++ packages/http/README.md | 37 +++++++++++++++ packages/http/src/config.test.ts | 68 +++++++++++++++++++++++++++ packages/http/src/config.ts | 19 +++++--- packages/http/src/http.common.test.ts | 60 ++++++++++++++++++++++- packages/http/src/http.common.ts | 33 +++++++------ packages/http/src/http.types.ts | 5 ++ 7 files changed, 205 insertions(+), 22 deletions(-) create mode 100644 .changeset/brave-birds-report.md diff --git a/.changeset/brave-birds-report.md b/.changeset/brave-birds-report.md new file mode 100644 index 00000000000..72b59e4b0ff --- /dev/null +++ b/.changeset/brave-birds-report.md @@ -0,0 +1,5 @@ +--- +'@talend/http': minor +--- + +feat(TDP-12106): improve interceptors to return a promise, have access to request and a business context from caller diff --git a/packages/http/README.md b/packages/http/README.md index 409eb12fca7..7866d78eb04 100644 --- a/packages/http/README.md +++ b/packages/http/README.md @@ -47,3 +47,40 @@ async function test() { const response = await http.get('/api/v1/my-resource'); } ``` + +## Interceptors + +You can add global response interceptors to catch or modify responses before resolve. + +```es6 +import { addHttpResponseInterceptor, http, HTTP_METHODS } from '@talend/http'; +import type { TalendRequest } from '@talend/http'; + +addHttpResponseInterceptor('my-interceptor', async (response: Response, request: TalendRequest) => { + if (request.method === HTTP_METHODS.GET) { + // your custom logic here + } + + return response; +}); +``` + +You can add multiple interceptors. Each will be called in the order of registration and will receive the same request parameter, but response parameter will be the one returned by previous interceptor. If interceptor returns void, then it'll return received response. + +Once your interceptor is not needed anymore, you can unregister it with `removeHttpResponseInterceptor` function of `@talend/http` package. + +You can identify some requests in interceptor by using `context` property in fetch function config: + +```es6 +import { addHttpResponseInterceptor, http, HTTP_METHODS } from '@talend/http'; + +http.get('/api/v1/data', { context: { intercept: true } }); + +addHttpResponseInterceptor('my-interceptor', async (response: Response, request: TalendRequest) => { + const { context } = request; + if (request.method === HTTP_METHODS.GET && context.intercept) { + // your custom logic here + } + return response; +}); +``` diff --git a/packages/http/src/config.test.ts b/packages/http/src/config.test.ts index 4f2513d6a70..f9d83f0a698 100644 --- a/packages/http/src/config.test.ts +++ b/packages/http/src/config.test.ts @@ -6,7 +6,10 @@ import { removeHttpResponseInterceptor, setDefaultConfig, setDefaultLanguage, + applyInterceptors, } from './config'; +import { HTTP_METHODS, HTTP_STATUS } from './http.constants'; +import { TalendRequest } from './http.types'; describe('Configuration service', () => { describe('setDefaultLanguage', () => { @@ -93,5 +96,70 @@ describe('Configuration service', () => { ); expect(HTTP_RESPONSE_INTERCEPTORS).toEqual({ myInterceptor2: interceptor2 }); }); + it('should apply all interceptors', async () => { + const request: TalendRequest = { + url: '/api/v1/data', + method: HTTP_METHODS.GET, + }; + const response = { + ok: true, + status: HTTP_STATUS.OK, + body: [1, 2, 3], + } as unknown as Response; + + const interceptor1 = jest + .fn() + .mockImplementation((resp, _) => Promise.resolve({ ...resp, body: [...resp.body, 4] })); + addHttpResponseInterceptor('interceptor-1', interceptor1); + + const interceptor2 = jest.fn().mockImplementation((resp, req) => + Promise.resolve({ + ...resp, + body: { interceptor: `interceptor2-${req.method}`, original: resp.body }, + }), + ); + addHttpResponseInterceptor('interceptor-2', interceptor2); + + const interceptedResponse = await applyInterceptors(request, response); + + expect(interceptor1).toHaveBeenCalledWith(response, request); + expect(interceptor2).toHaveBeenLastCalledWith( + expect.objectContaining({ body: [1, 2, 3, 4] }), + request, + ); + expect(interceptedResponse).toEqual({ + ...response, + body: { interceptor: 'interceptor2-GET', original: [1, 2, 3, 4] }, + }); + }); + it('should return response if no interceptors', () => { + const request: TalendRequest = { + url: '/api/v1/data', + method: HTTP_METHODS.GET, + }; + const response = { + ok: true, + status: HTTP_STATUS.OK, + body: [1, 2, 3], + } as unknown as Response; + + expect(applyInterceptors(request, response)).resolves.toEqual(response); + }); + it('should return response if interceptor returns void', async () => { + const request: TalendRequest = { + url: '/api/v1/data', + method: HTTP_METHODS.GET, + }; + const response = { + ok: true, + status: HTTP_STATUS.OK, + body: [1, 2, 3], + } as unknown as Response; + const interceptor = jest.fn().mockImplementation(() => {}); + addHttpResponseInterceptor('interceptor', interceptor); + const gotResponse = await applyInterceptors(request, response); + expect(gotResponse).toEqual(response); + expect(interceptor).toHaveBeenCalledWith(response, request); + }); }); }); diff --git a/packages/http/src/config.ts b/packages/http/src/config.ts index bd12d85b015..dbccd101cc8 100644 --- a/packages/http/src/config.ts +++ b/packages/http/src/config.ts @@ -1,4 +1,4 @@ -import { TalendRequestInit } from './http.types'; +import { TalendRequest, TalendRequestInit } from './http.types'; /** * Storage point for the doc setup using `setDefaultConfig` @@ -7,12 +7,11 @@ export const HTTP: { defaultConfig?: TalendRequestInit | null } = { defaultConfig: null, }; -export const HTTP_RESPONSE_INTERCEPTORS: Record void> = {}; +export type Interceptor = (response: Response, request: TalendRequest) => Promise | void; -export function addHttpResponseInterceptor( - name: string, - interceptor: (response: Response) => void, -) { +export const HTTP_RESPONSE_INTERCEPTORS: Record = {}; + +export function addHttpResponseInterceptor(name: string, interceptor: Interceptor) { if (HTTP_RESPONSE_INTERCEPTORS[name]) { throw new Error(`Interceptor ${name} already exists`); } @@ -26,6 +25,14 @@ export function removeHttpResponseInterceptor(name: string) { delete HTTP_RESPONSE_INTERCEPTORS[name]; } +export function applyInterceptors(request: TalendRequest, response: Response): Promise { + return Object.values(HTTP_RESPONSE_INTERCEPTORS).reduce( + (promise, interceptor) => + promise.then(resp => interceptor(resp, request) || Promise.resolve(response)), + Promise.resolve(response), + ); +} + /** * setDefaultHeader - define a default config to use with the saga http * this default config is stored in this module for the whole application diff --git a/packages/http/src/http.common.test.ts b/packages/http/src/http.common.test.ts index 2905f1d31bb..f4baf5071c6 100644 --- a/packages/http/src/http.common.test.ts +++ b/packages/http/src/http.common.test.ts @@ -1,7 +1,13 @@ import fetchMock from 'fetch-mock'; import { Response, Headers } from 'node-fetch'; -import { HTTP, getDefaultConfig, setDefaultConfig } from './config'; +import { + HTTP, + getDefaultConfig, + setDefaultConfig, + HTTP_RESPONSE_INTERCEPTORS, + addHttpResponseInterceptor, +} from './config'; import { httpFetch, handleBody, encodePayload, handleHttpResponse } from './http.common'; import { HTTP_METHODS, HTTP_STATUS } from './http.constants'; import { TalendHttpError } from './http.types'; @@ -48,7 +54,10 @@ describe('handleBody', () => { const blob = jest.fn(() => Promise.resolve()); - await handleBody({ blob, headers } as any); + await handleBody({ + headers, + clone: jest.fn().mockReturnValue({ blob }), + } as any); expect(blob).toHaveBeenCalled(); }); @@ -71,6 +80,12 @@ describe('handleBody', () => { expect(result.data).toBe(''); }); + it("should manage response's body and return a clone with unused body", async () => { + const result = await handleBody(new Response('ok') as any); + expect(result.data).toBe('ok'); + expect(result.response.bodyUsed).toBe(false); + }); + describe('#handleHttpResponse', () => { it('should handle the response with 2xx code', async () => { const headers = new Headers(); @@ -328,3 +343,44 @@ describe('#httpFetch', () => { expect(mockCalls[0][1]?.headers).toEqual({ Accept: 'application/json' }); }); }); + +describe('#httpFetch with interceptors', () => { + beforeEach(() => { + for (const key in HTTP_RESPONSE_INTERCEPTORS) { + if (HTTP_RESPONSE_INTERCEPTORS.hasOwnProperty(key)) { + delete HTTP_RESPONSE_INTERCEPTORS[key]; + } + } + }); + + afterEach(() => { + fetchMock.restore(); + }); + + it('should call interceptor', async () => { + const interceptor = jest.fn().mockImplementation((res, _) => res); + addHttpResponseInterceptor('interceptor', interceptor); + + const url = '/foo'; + fetchMock.mock(url, { body: defaultBody, status: 200 }); + + await httpFetch(url, {}, HTTP_METHODS.GET, {}); + expect(interceptor).toHaveBeenCalled(); + }); + + it('should have access to context in interceptor', async () => { + const interceptor = jest.fn().mockImplementation((res, _) => res); + addHttpResponseInterceptor('interceptor', interceptor); + + const url = '/foo'; + const context = { async: true }; + const response = { body: defaultBody, status: 200 }; + fetchMock.mock(url, response); + + await httpFetch(url, { context }, HTTP_METHODS.GET, {}); + expect(interceptor).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ url, context, method: HTTP_METHODS.GET }), + ); + }); +}); diff --git a/packages/http/src/http.common.ts b/packages/http/src/http.common.ts index 9ebc4802295..ee6b27e2216 100644 --- a/packages/http/src/http.common.ts +++ b/packages/http/src/http.common.ts @@ -1,4 +1,4 @@ -import { HTTP, HTTP_RESPONSE_INTERCEPTORS } from './config'; +import { applyInterceptors, HTTP } from './config'; import { mergeCSRFToken } from './csrfHandling'; import { HTTP_STATUS, testHTTPCode } from './http.constants'; import { TalendHttpResponse, TalendRequestInit } from './http.types'; @@ -40,17 +40,18 @@ export function encodePayload(headers: HeadersInit, payload: any) { * @return {Promise} A promise that resolves with the result of parsing the body */ export async function handleBody(response: Response) { + const clonedResponse = response.clone(); const { headers } = response; const contentType = headers.get('Content-Type'); if (contentType && contentType.includes('application/json')) { - return response.json().then(data => ({ data, response })); + return clonedResponse.json().then(data => ({ data, response })); } if (contentType && contentType.includes('application/zip')) { - return response.blob().then(data => ({ data, response })); + return clonedResponse.blob().then(data => ({ data, response })); } - return response.text().then(data => ({ data, response })); + return clonedResponse.text().then(data => ({ data, response })); } /** @@ -118,17 +119,21 @@ export async function httpFetch( }, }; - const response = await fetch( - url, - handleCSRFToken({ - ...params, - body: encodePayload(params.headers || {}, payload), - }), - ); - - Object.values(HTTP_RESPONSE_INTERCEPTORS).forEach(interceptor => { - interceptor(response); + const { context, ...init } = handleCSRFToken({ + ...params, + body: encodePayload(params.headers || {}, payload), }); + const response = await fetch(url, init).then(resp => + applyInterceptors( + { + url, + ...init, + context, + }, + resp, + ), + ); + return handleHttpResponse(response); } diff --git a/packages/http/src/http.types.ts b/packages/http/src/http.types.ts index 71294103aba..8c756657030 100644 --- a/packages/http/src/http.types.ts +++ b/packages/http/src/http.types.ts @@ -10,8 +10,13 @@ export type TalendRequestInitSecurity = { export interface TalendRequestInit extends RequestInit { security?: TalendRequestInitSecurity; + context?: Record; } +export type TalendRequest = { + url: string; +} & TalendRequestInit; + export interface TalendHttpError extends Error { response: Response; data: T;