Skip to content

Commit

Permalink
Embedding the request options normalization
Browse files Browse the repository at this point in the history
  • Loading branch information
jdalrymple committed Aug 7, 2023
1 parent 17f6a01 commit e5a8c63
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 43 deletions.
4 changes: 2 additions & 2 deletions packages/requester-utils/src/BaseResource.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
18 changes: 11 additions & 7 deletions packages/requester-utils/src/RequesterUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -82,23 +82,23 @@ 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>) {
return typeof object === 'object' && object.constructor.name === 'FormData';
}

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;
Expand Down Expand Up @@ -144,15 +144,19 @@ 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) => {
const requester: RequesterType = {} as RequesterType;

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);
};
Expand Down
11 changes: 2 additions & 9 deletions packages/requester-utils/test/unit/BaseResource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' }),
);
});
});
25 changes: 17 additions & 8 deletions packages/requester-utils/test/unit/RequesterUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});

Expand All @@ -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);
Expand All @@ -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();
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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, {});
}
});
Expand Down Expand Up @@ -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' }),
);
});
});

Expand Down
17 changes: 5 additions & 12 deletions packages/rest/src/Requester.ts
Original file line number Diff line number Diff line change
@@ -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') &&
Expand Down
10 changes: 5 additions & 5 deletions packages/rest/test/unit/Requester.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand Down

0 comments on commit e5a8c63

Please sign in to comment.