From e5a8c63f52c26de4e15e930242dfcefa4b017427 Mon Sep 17 00:00:00 2001 From: Justin Dalrymple Date: Mon, 7 Aug 2023 16:08:56 -0400 Subject: [PATCH] Embedding the request options normalization --- packages/requester-utils/src/BaseResource.ts | 4 +-- .../requester-utils/src/RequesterUtils.ts | 18 +++++++------ .../requester-utils/test/unit/BaseResource.ts | 11 ++------ .../test/unit/RequesterUtils.ts | 25 +++++++++++++------ packages/rest/src/Requester.ts | 17 ++++--------- packages/rest/test/unit/Requester.ts | 10 ++++---- 6 files changed, 42 insertions(+), 43 deletions(-) diff --git a/packages/requester-utils/src/BaseResource.ts b/packages/requester-utils/src/BaseResource.ts index 1b99d5a9a..ed81b8f9c 100644 --- a/packages/requester-utils/src/BaseResource.ts +++ b/packages/requester-utils/src/BaseResource.ts @@ -1,8 +1,8 @@ -import { DefaultResourceOptions, RequesterType } from './RequesterUtils'; +import { RequesterType, ResourceOptions } from './RequesterUtils'; export interface RootResourceOptions { // TODO: Not actually optional - Need to fix wrapper typing in requestUtils.ts: - requesterFn?: (resourceOptions: DefaultResourceOptions) => RequesterType; + requesterFn?: (resourceOptions: ResourceOptions) => RequesterType; host?: string; prefixUrl?: string; rejectUnauthorized?: boolean; diff --git a/packages/requester-utils/src/RequesterUtils.ts b/packages/requester-utils/src/RequesterUtils.ts index 7bf0480bd..fcbfd37c3 100644 --- a/packages/requester-utils/src/RequesterUtils.ts +++ b/packages/requester-utils/src/RequesterUtils.ts @@ -23,7 +23,7 @@ export interface Constructable { new (...args: any[]): T; } -export type DefaultResourceOptions = { +export type ResourceOptions = { headers: { [header: string]: string }; authHeaders: { [authHeader: string]: () => Promise }; url: string; @@ -82,8 +82,8 @@ export function formatQuery(params: Record = {}): string { } export type OptionsHandlerFn = ( - serviceOptions: DefaultResourceOptions, - requestOptions: DefaultRequestOptions, + serviceOptions: ResourceOptions, + requestOptions: RequestOptions, ) => Promise; function isFormData(object: FormData | Record) { @@ -91,14 +91,14 @@ function isFormData(object: FormData | Record) { } export async function defaultOptionsHandler( - resourceOptions: DefaultResourceOptions, + resourceOptions: ResourceOptions, { body, searchParams, sudo, signal, asStream = false, - method = 'get', + method = 'GET', }: DefaultRequestOptions = {}, ): Promise { const { headers: preconfiguredHeaders, authHeaders, url } = resourceOptions; @@ -144,7 +144,7 @@ export type RequestHandlerFn = export function createRequesterFn( optionsHandler: OptionsHandlerFn, requestHandler: RequestHandlerFn, -): (serviceOptions: DefaultResourceOptions) => RequesterType { +): (serviceOptions: ResourceOptions) => RequesterType { const methods = ['get', 'post', 'put', 'patch', 'delete']; return (serviceOptions) => { @@ -152,7 +152,11 @@ export function createRequesterFn( methods.forEach((m) => { requester[m] = async (endpoint: string, options: Record) => { - const requestOptions = await optionsHandler(serviceOptions, { ...options, method: m }); + const defaultRequestOptions = await defaultOptionsHandler(serviceOptions, { + ...options, + method: m.toUpperCase(), + }); + const requestOptions = await optionsHandler(serviceOptions, defaultRequestOptions); return requestHandler(endpoint, requestOptions); }; diff --git a/packages/requester-utils/test/unit/BaseResource.ts b/packages/requester-utils/test/unit/BaseResource.ts index dc411e7a0..b39fa364a 100644 --- a/packages/requester-utils/test/unit/BaseResource.ts +++ b/packages/requester-utils/test/unit/BaseResource.ts @@ -201,22 +201,15 @@ describe('Creation of BaseResource instance', () => { it('should set the internal requester based on the required requesterFn parameter', async () => { const requestHandler = jest.fn(); const optionsHandler = jest.fn(); + const requesterFn = createRequesterFn(optionsHandler, requestHandler); const serviceA = new BaseResource({ token: '123', requesterFn, prefixUrl: 'test' }); - const serviceB = new BaseResource({ token: '123', requesterFn, prefixUrl: 'test2' }); await serviceA.requester.get('test'); expect(optionsHandler).toHaveBeenCalledWith( expect.objectContaining({ url: 'https://gitlab.com/api/v4/test' }), - { method: 'get' }, - ); - - await serviceB.requester.get('test'); - - expect(optionsHandler).toHaveBeenCalledWith( - expect.objectContaining({ url: 'https://gitlab.com/api/v4/test2' }), - { method: 'get' }, + expect.objectContaining({ method: 'GET' }), ); }); }); diff --git a/packages/requester-utils/test/unit/RequesterUtils.ts b/packages/requester-utils/test/unit/RequesterUtils.ts index b9d9cd898..0148f0092 100644 --- a/packages/requester-utils/test/unit/RequesterUtils.ts +++ b/packages/requester-utils/test/unit/RequesterUtils.ts @@ -21,13 +21,13 @@ describe('defaultOptionsHandler', () => { it('should not use default request options if not passed', async () => { const options = await defaultOptionsHandler(serviceOptions); - expect(options.method).toBe('get'); + expect(options.method).toBe('GET'); }); it('should stringify body if it isnt of type FormData', async () => { const testBody = { test: 6 }; const { body, headers } = await defaultOptionsHandler(serviceOptions, { - method: 'post', + method: 'POST', body: testBody, }); @@ -39,7 +39,7 @@ describe('defaultOptionsHandler', () => { const testBody: globalThis.FormData = new FormData() as unknown as globalThis.FormData; const { body } = await defaultOptionsHandler(serviceOptions, { body: testBody, - method: 'post', + method: 'POST', }); expect(body).toBeInstanceOf(FormData); @@ -48,7 +48,7 @@ describe('defaultOptionsHandler', () => { it('should not assign the sudo property if omitted', async () => { const { headers } = await defaultOptionsHandler(serviceOptions, { sudo: undefined, - method: 'get', + method: 'GET', }); expect(headers.sudo).toBeUndefined(); @@ -95,7 +95,7 @@ describe('defaultOptionsHandler', () => { it('should append dynamic authentication token headers', async () => { const { headers } = await defaultOptionsHandler(serviceOptions, { sudo: undefined, - method: 'get', + method: 'GET', }); expect(headers.token).toBe('1234'); @@ -137,7 +137,10 @@ describe('createInstance', () => { // eslint-disable-next-line await requester[m](testEndpoint, {}); - expect(optionsHandler).toHaveBeenCalledWith(serviceOptions, { method: m }); + expect(optionsHandler).toHaveBeenCalledWith( + serviceOptions, + expect.objectContaining({ method: m.toUpperCase() }), + ); expect(requestHandler).toHaveBeenCalledWith(testEndpoint, {}); } }); @@ -166,11 +169,17 @@ describe('createInstance', () => { await requesterA.get('test'); - expect(optionsHandler).toHaveBeenCalledWith(serviceOptions1, { method: 'get' }); + expect(optionsHandler).toHaveBeenCalledWith( + serviceOptions1, + expect.objectContaining({ method: 'GET' }), + ); await requesterB.get('test'); - expect(optionsHandler).toHaveBeenCalledWith(serviceOptions2, { method: 'get' }); + expect(optionsHandler).toHaveBeenCalledWith( + serviceOptions2, + expect.objectContaining({ method: 'GET' }), + ); }); }); diff --git a/packages/rest/src/Requester.ts b/packages/rest/src/Requester.ts index 0278badc9..8c05c63e5 100644 --- a/packages/rest/src/Requester.ts +++ b/packages/rest/src/Requester.ts @@ -1,22 +1,15 @@ import type { - DefaultRequestOptions, - DefaultResourceOptions, RequestOptions, + ResourceOptions, ResponseBodyTypes, } from '@gitbeaker/requester-utils'; -import { - defaultOptionsHandler as baseOptionsHandler, - createRequesterFn, -} from '@gitbeaker/requester-utils'; +import { createRequesterFn } from '@gitbeaker/requester-utils'; export async function defaultOptionsHandler( - resourceOptions: DefaultResourceOptions, - requestOptions: DefaultRequestOptions = {}, + resourceOptions: ResourceOptions, + requestOptions: RequestOptions, ): Promise { - const options: RequestOptions & { agent?: unknown } = await baseOptionsHandler( - resourceOptions, - requestOptions, - ); + const options: RequestOptions & { agent?: unknown } = { ...requestOptions }; if ( resourceOptions.url.includes('https') && diff --git a/packages/rest/test/unit/Requester.ts b/packages/rest/test/unit/Requester.ts index 0aed1e7b9..20a972cb2 100644 --- a/packages/rest/test/unit/Requester.ts +++ b/packages/rest/test/unit/Requester.ts @@ -293,7 +293,7 @@ describe('defaultRequest', () => { it('should not assign the agent property if given https url and not rejectUnauthorized', async () => { const { agent } = await defaultOptionsHandler( { ...service, url: 'https://test.com' }, - { method: 'post' }, + { method: 'POST' }, ); expect(agent).toBeUndefined(); @@ -302,7 +302,7 @@ describe('defaultRequest', () => { it('should not assign the agent property if given http url and rejectUnauthorized', async () => { const { agent } = await defaultOptionsHandler( { ...service, url: 'http://test.com' }, - { method: 'post' }, + { method: 'POST' }, ); expect(agent).toBeUndefined(); @@ -311,21 +311,21 @@ describe('defaultRequest', () => { it('should assign the agent property if given https url and rejectUnauthorized is false', async () => { const { agent: agent1 } = await defaultOptionsHandler( { ...service, url: 'https://test.com', rejectUnauthorized: false }, - { method: 'post' }, + { method: 'POST' }, ); expect(agent1).toBeDefined(); const { agent: agent2 } = await defaultOptionsHandler( { ...service, url: 'https://test.com', rejectUnauthorized: true }, - { method: 'post' }, + { method: 'POST' }, ); expect(agent2).toBeUndefined(); const { agent: agent3 } = await defaultOptionsHandler( { ...service, url: 'https://test.com' }, - { method: 'post' }, + { method: 'POST' }, ); expect(agent3).toBeUndefined();