Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http-client: add body form urlencoded option #1110

Merged
merged 3 commits into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 151 additions & 0 deletions packages/integration-sdk-http-client/src/__tests__/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { BaseAPIClient, defaultErrorHandler } from '../client';
import { sleep } from '@lifeomic/attempt';
import FormData from 'form-data';

jest.mock('node-fetch');
import fetch from 'node-fetch';

const { Response } = jest.requireActual('node-fetch');

jest.mock('@lifeomic/attempt', () => {
Expand Down Expand Up @@ -179,6 +181,155 @@ describe('APIClient', () => {
body: `{"test":"test"}`,
});
});

describe('fmtBody', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

test('should format body as JSON string when bodyType is json', async () => {
const mockResponse = {} as Response;
(fetch as unknown as jest.Mock).mockResolvedValue(mockResponse);

const client = new MockAPIClient({
baseUrl: 'https://api.example.com',
logger: mockLogger,
});

const body = { key1: 'value1', key2: 'value2' };
await (client as any).request('/test', {
bodyType: 'json',
body,
});

expect(fetch).toHaveBeenCalledWith(
expect.any(String),
expect.objectContaining({
body: JSON.stringify(body),
headers: expect.objectContaining({
'Content-Type': 'application/json',
}),
}),
);
});

test('should format body as plain text when bodyType is text', async () => {
const mockResponse = {} as Response;
(fetch as unknown as jest.Mock).mockResolvedValue(mockResponse);

const client = new MockAPIClient({
baseUrl: 'https://api.example.com',
logger: mockLogger,
});

const body = 'plain text body';
await (client as any).request('/test', {
bodyType: 'text',
body,
});

expect(fetch).toHaveBeenCalledWith(
expect.any(String),
expect.objectContaining({
body,
headers: expect.objectContaining({
'Content-Type': 'text/plain',
}),
}),
);
});

test('should format body as FormData when bodyType is form', async () => {
const mockResponse = {} as Response;
(fetch as unknown as jest.Mock).mockResolvedValue(mockResponse);

const client = new MockAPIClient({
baseUrl: 'https://api.example.com',
logger: mockLogger,
});

const body = { key1: 'value1', key2: 'value2' };
const appendSpy = jest.spyOn(FormData.prototype, 'append');
const getHeadersSpy = jest.spyOn(FormData.prototype, 'getHeaders');
getHeadersSpy.mockReturnValue({
'content-type': 'multipart/form-data',
});

await (client as any).request('/test', {
bodyType: 'form',
body,
});

expect(fetch).toHaveBeenCalledWith(
expect.any(String),
expect.objectContaining({
headers: expect.objectContaining({
'content-type': 'multipart/form-data',
}),
}),
);

expect(appendSpy).toHaveBeenCalledWith('key1', 'value1');
expect(appendSpy).toHaveBeenCalledWith('key2', 'value2');
});

test('should throw an error when bodyType is form and a non-string value is provided', async () => {
const mockResponse = {} as Response;
(fetch as unknown as jest.Mock).mockResolvedValue(mockResponse);

const client = new MockAPIClient({
baseUrl: 'https://api.example.com',
logger: mockLogger,
});

const body = { key1: 'value1', key2: 123 }; // Invalid value
await expect(
(client as any).request('/test', { bodyType: 'form', body }),
).rejects.toThrow('Form data values must be strings');
});

test('should format body as URLSearchParams when bodyType is urlencoded', async () => {
const mockResponse = {} as Response;
(fetch as unknown as jest.Mock).mockResolvedValue(mockResponse);

const client = new MockAPIClient({
baseUrl: 'https://api.example.com',
logger: mockLogger,
});

const body = { key1: 'value1', key2: 'value2' };
await (client as any).request('/test', {
bodyType: 'urlencoded',
body,
});

expect(fetch).toHaveBeenCalledWith(
expect.any(String),
expect.objectContaining({
body: expect.any(URLSearchParams),
headers: expect.objectContaining({
'Content-Type': 'application/x-www-form-urlencoded',
}),
}),
);

const urlSearchParams = (fetch as unknown as jest.Mock).mock.calls[0][1]
.body as URLSearchParams;
expect(urlSearchParams.get('key1')).toBe('value1');
expect(urlSearchParams.get('key2')).toBe('value2');
});

test('should throw an error when bodyType is urlencoded and a non-string value is provided', async () => {
const mockResponse = {} as Response;
(fetch as unknown as jest.Mock).mockResolvedValue(mockResponse);

const client = new MockAPIClient({
baseUrl: 'https://api.example.com',
logger: mockLogger,
});

const body = { key1: 'value1', key2: 123 }; // Invalid value
await expect(
(client as any).request('/test', { bodyType: 'urlencoded', body }),
).rejects.toThrow('Form values must be strings');
});
});
});

describe('withRateLimiting', () => {
Expand Down
16 changes: 15 additions & 1 deletion packages/integration-sdk-http-client/src/client.ts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unit tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added!

Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ export abstract class BaseAPIClient {
url = this.withBaseUrl(endpoint);
}

let fmtBody: string | FormData | undefined;
let fmtBody: string | FormData | URLSearchParams | undefined;
if (body) {
if (bodyType === 'form') {
fmtBody = new FormData();
Expand All @@ -243,6 +243,17 @@ export abstract class BaseAPIClient {
});
} else if (bodyType === 'text' && typeof body === 'string') {
fmtBody = body;
} else if (bodyType === 'urlencoded') {
fmtBody = new URLSearchParams();
Object.entries(body).forEach(([key, value]) => {
if (typeof value !== 'string') {
throw new IntegrationError({
VDubber marked this conversation as resolved.
Show resolved Hide resolved
code: 'INVALID_FORM_DATA',
message: 'Form values must be strings',
});
}
(fmtBody as URLSearchParams).append(key, value);
});
} else {
fmtBody = JSON.stringify(body);
}
Expand All @@ -253,6 +264,9 @@ export abstract class BaseAPIClient {
...(bodyType === 'json' && { 'Content-Type': 'application/json' }),
...(bodyType === 'form' && (fmtBody as FormData).getHeaders()),
...(bodyType === 'text' && { 'Content-Type': 'text/plain' }),
...(bodyType === 'urlencoded' && {
'Content-Type': 'application/x-www-form-urlencoded',
}),
Accept: 'application/json',
...(authorize && this.authorizationHeaders),
...headers,
Expand Down
2 changes: 1 addition & 1 deletion packages/integration-sdk-http-client/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export type OptionalPromise<T> = T | Promise<T>;
export interface RequestOptions {
method?: 'GET' | 'POST';
body?: Record<string, unknown> | string;
bodyType?: 'json' | 'form' | 'text';
bodyType?: 'json' | 'form' | 'text' | 'urlencoded';
headers?: Record<string, string>;
authorize?: boolean;
bucketTokens?: number;
Expand Down