Embedding the request options normalization (#3374)

packages/requester-utils/src/BaseResource.ts

@@ -1,8 +1,8 @@
-import { DefaultResourceOptions, RequesterType } from './RequesterUtils';
+import { RequesterType, ResourceOptions } from './RequesterUtils';
 
 export interface RootResourceOptions<C> {
   // 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;

packages/requester-utils/src/RequesterUtils.ts

@@ -23,7 +23,7 @@ export interface Constructable<T = any> {
   new (...args: any[]): T;
 }
 
-export type DefaultResourceOptions = {
+export type ResourceOptions = {
   headers: { [header: string]: string };
   authHeaders: { [authHeader: string]: () => Promise<string> };
   url: string;
@@ -40,11 +40,11 @@ export type DefaultRequestOptions = {
 };
 
 export type RequestOptions = {
-  headers: Record<string, string>;
+  headers?: Record<string, string>;
   timeout?: number;
-  method: string;
+  method?: string;
   searchParams?: string;
-  prefixUrl: string;
+  prefixUrl?: string;
   body?: string | FormData;
   asStream?: boolean;
   signal?: AbortSignal;
@@ -82,8 +82,8 @@ export function formatQuery(params: Record<string, unknown> = {}): string {
 }
 
 export type OptionsHandlerFn = (
-  serviceOptions: DefaultResourceOptions,
-  requestOptions: DefaultRequestOptions,
+  serviceOptions: ResourceOptions,
+  requestOptions: RequestOptions,
 ) => Promise<RequestOptions>;
 
 function isFormData(object: FormData | Record<string, unknown>) {
@@ -91,26 +91,27 @@ function isFormData(object: FormData | Record<string, unknown>) {
 }
 
 export async function defaultOptionsHandler(
-  resourceOptions: DefaultResourceOptions,
+  resourceOptions: ResourceOptions,
   {
     body,
     searchParams,
     sudo,
     signal,
     asStream = false,
-    method = 'get',
+    method = 'GET',
   }: DefaultRequestOptions = {},
 ): Promise<RequestOptions> {
   const { headers: preconfiguredHeaders, authHeaders, url } = resourceOptions;
   const headers = { ...preconfiguredHeaders };
   const defaultOptions: RequestOptions = {
-    headers,
     method,
     asStream,
     signal,
     prefixUrl: url,
   };
 
+  defaultOptions.headers = headers;
+
   if (sudo) defaultOptions.headers.sudo = `${sudo}`;
 
   // FIXME: Not the best comparison, but...it will have to do for now.
@@ -144,7 +145,7 @@ export type RequestHandlerFn<T extends ResponseBodyTypes = ResponseBodyTypes> =
 export function createRequesterFn(
   optionsHandler: OptionsHandlerFn,
   requestHandler: RequestHandlerFn,
-): (serviceOptions: DefaultResourceOptions) => RequesterType {
+): (serviceOptions: ResourceOptions) => RequesterType {
   const methods = ['get', 'post', 'put', 'patch', 'delete'];
 
   return (serviceOptions) => {
@@ -152,7 +153,11 @@ export function createRequesterFn(
 
     methods.forEach((m) => {
       requester[m] = async (endpoint: string, options: Record<string, unknown>) => {
-        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);
       };

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' }),
     );
   });
 });

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,10 +48,10 @@ 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();
+    expect(headers?.sudo).toBeUndefined();
   });
 
   it('should assign the sudo property if passed', async () => {
@@ -59,7 +59,7 @@ describe('defaultOptionsHandler', () => {
       sudo: 'testsudo',
     });
 
-    expect(headers.sudo).toBe('testsudo');
+    expect(headers?.sudo).toBe('testsudo');
   });
 
   it('should assign the prefixUrl property if passed', async () => {
@@ -95,10 +95,10 @@ 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');
+    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' }),
+    );
   });
 });
 

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<RequestOptions & { agent?: unknown }> {
-  const options: RequestOptions & { agent?: unknown } = await baseOptionsHandler(
-    resourceOptions,
-    requestOptions,
-  );
+  const options: RequestOptions & { agent?: unknown } = { ...requestOptions };
 
   if (
     resourceOptions.url.includes('https') &&

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();