Skip to content

Commit

Permalink
chore: Refactor constants
Browse files Browse the repository at this point in the history
- Move constants into ExpoClientValues
- Allow use for Expo internal push hydrant testing with env variable EXPO_BASE_URL
- Fix and refactor unit tests
  • Loading branch information
douglowder committed Sep 5, 2024
1 parent 586c2ad commit 7927286
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 59 deletions.
45 changes: 16 additions & 29 deletions src/ExpoClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,31 +12,18 @@ import promiseLimit from 'promise-limit';
import promiseRetry from 'promise-retry';
import zlib from 'zlib';

import { requestRetryMinTimeout } from './ExpoClientValues';

const BASE_URL = 'https://exp.host';
const BASE_API_URL = `${BASE_URL}/--/api/v2`;

/**
* The max number of push notifications to be sent at once. Since we can't automatically upgrade
* everyone using this library, we should strongly try not to decrease it.
*/
const PUSH_NOTIFICATION_CHUNK_LIMIT = 100;

/**
* The max number of push notification receipts to request at once.
*/
const PUSH_NOTIFICATION_RECEIPT_CHUNK_LIMIT = 300;

/**
* The default max number of concurrent HTTP requests to send at once and spread out the load,
* increasing the reliability of notification delivery.
*/
const DEFAULT_CONCURRENT_REQUEST_LIMIT = 6;
import {
defaultConcurrentRequestLimit,
getReceiptsApiUrl,
pushNotificationChunkLimit,
pushNotificationReceiptChunkLimit,
requestRetryMinTimeout,
sendApiUrl,
} from './ExpoClientValues';

export class Expo {
static pushNotificationChunkSizeLimit = PUSH_NOTIFICATION_CHUNK_LIMIT;
static pushNotificationReceiptChunkSizeLimit = PUSH_NOTIFICATION_RECEIPT_CHUNK_LIMIT;
static pushNotificationChunkSizeLimit = pushNotificationChunkLimit;
static pushNotificationReceiptChunkSizeLimit = pushNotificationReceiptChunkLimit;

private httpAgent: Agent | undefined;
private limitConcurrentRequests: <T>(thunk: () => Promise<T>) => Promise<T>;
Expand All @@ -48,7 +35,7 @@ export class Expo {
this.limitConcurrentRequests = promiseLimit(
options.maxConcurrentRequests != null
? options.maxConcurrentRequests
: DEFAULT_CONCURRENT_REQUEST_LIMIT,
: defaultConcurrentRequestLimit,
);
this.accessToken = options.accessToken;
this.useFcmV1 = options.useFcmV1;
Expand Down Expand Up @@ -78,7 +65,7 @@ export class Expo {
* sized chunks.
*/
async sendPushNotificationsAsync(messages: ExpoPushMessage[]): Promise<ExpoPushTicket[]> {
const url = new URL(`${BASE_API_URL}/push/send`);
const url = new URL(sendApiUrl);
if (typeof this.useFcmV1 === 'boolean') {
url.searchParams.append('useFcmV1', String(this.useFcmV1));
}
Expand Down Expand Up @@ -126,7 +113,7 @@ export class Expo {
async getPushNotificationReceiptsAsync(
receiptIds: ExpoPushReceiptId[],
): Promise<{ [id: string]: ExpoPushReceipt }> {
const data = await this.requestAsync(`${BASE_API_URL}/push/getReceipts`, {
const data = await this.requestAsync(getReceiptsApiUrl, {
httpMethod: 'post',
body: { ids: receiptIds },
shouldCompress(body) {
Expand Down Expand Up @@ -156,7 +143,7 @@ export class Expo {
for (const recipient of message.to) {
partialTo.push(recipient);
chunkMessagesCount++;
if (chunkMessagesCount >= PUSH_NOTIFICATION_CHUNK_LIMIT) {
if (chunkMessagesCount >= pushNotificationChunkLimit) {
// Cap this chunk here if it already exceeds PUSH_NOTIFICATION_CHUNK_LIMIT.
// Then create a new chunk to continue on the remaining recipients for this message.
chunk.push({ ...message, to: partialTo });
Expand All @@ -175,7 +162,7 @@ export class Expo {
chunkMessagesCount++;
}

if (chunkMessagesCount >= PUSH_NOTIFICATION_CHUNK_LIMIT) {
if (chunkMessagesCount >= pushNotificationChunkLimit) {
// Cap this chunk if it exceeds PUSH_NOTIFICATION_CHUNK_LIMIT.
// Then create a new chunk to continue on the remaining messages.
chunks.push(chunk);
Expand All @@ -192,7 +179,7 @@ export class Expo {
}

chunkPushNotificationReceiptIds(receiptIds: ExpoPushReceiptId[]): ExpoPushReceiptId[][] {
return this.chunkItems(receiptIds, PUSH_NOTIFICATION_RECEIPT_CHUNK_LIMIT);
return this.chunkItems(receiptIds, pushNotificationReceiptChunkLimit);
}

private chunkItems<T>(items: T[], chunkSize: number): T[][] {
Expand Down
32 changes: 32 additions & 0 deletions src/ExpoClientValues.ts
Original file line number Diff line number Diff line change
@@ -1 +1,33 @@
/**
* The URLs for the Expo push service endpoints.
*
* The EXPO_BASE_URL environment variable is only for internal Expo use
* when testing the push service locally.
*/
const baseUrl = process.env.EXPO_BASE_URL ?? 'https://exp.host';

export const sendApiUrl = `${baseUrl}/--/api/v2/push/send`;

export const getReceiptsApiUrl = `${baseUrl}/--/api/v2/push/getReceipts`;

/**
* The max number of push notifications to be sent at once. Since we can't automatically upgrade
* everyone using this library, we should strongly try not to decrease it.
*/
export const pushNotificationChunkLimit = 100;

/**
* The max number of push notification receipts to request at once.
*/
export const pushNotificationReceiptChunkLimit = 300;

/**
* The default max number of concurrent HTTP requests to send at once and spread out the load,
* increasing the reliability of notification delivery.
*/
export const defaultConcurrentRequestLimit = 6;

/**
* Minimum timeout in ms for request retries.
*/
export const requestRetryMinTimeout = 1000;
58 changes: 28 additions & 30 deletions src/__tests__/ExpoClient-test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
import fetch from 'node-fetch';

import ExpoClient, { ExpoPushMessage } from '../ExpoClient';
import { getReceiptsApiUrl, sendApiUrl } from '../ExpoClientValues';

jest.mock('../ExpoClientValues', () => ({
requestRetryMinTimeout: 1,
pushNotificationChunkLimit: 100,
sendApiUrl: 'http://localhost:3000/--/api/v2/push/send',
getReceiptsApiUrl: 'http://localhost:3000/--/api/v2/push/getReceipts',
pushNotificationReceiptChunkLimit: 300,
defaultConcurrentRequestLimit: 6,
}));

afterEach(() => {
Expand All @@ -16,13 +22,13 @@ describe('sending push notification messages', () => {
{ status: 'ok', id: 'XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX' },
{ status: 'ok', id: 'YYYYYYYY-YYYY-YYYY-YYYY-YYYYYYYYYYYY' },
];
(fetch as any).mock('https://exp.host/--/api/v2/push/send', { data: mockTickets });
(fetch as any).mock(sendApiUrl, { data: mockTickets });

const client = new ExpoClient();
const tickets = await client.sendPushNotificationsAsync([{ to: 'a' }, { to: 'b' }]);
expect(tickets).toEqual(mockTickets);

const [, options] = (fetch as any).lastCall('https://exp.host/--/api/v2/push/send');
const [, options] = (fetch as any).lastCall(sendApiUrl);
expect(options.headers.get('accept')).toContain('application/json');
expect(options.headers.get('accept-encoding')).toContain('gzip');
expect(options.headers.get('content-type')).toContain('application/json');
Expand All @@ -35,13 +41,13 @@ describe('sending push notification messages', () => {
{ status: 'ok', id: 'XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX' },
{ status: 'ok', id: 'YYYYYYYY-YYYY-YYYY-YYYY-YYYYYYYYYYYY' },
];
(fetch as any).mock('https://exp.host/--/api/v2/push/send', { data: mockTickets });
(fetch as any).mock(sendApiUrl, { data: mockTickets });

const client = new ExpoClient({ accessToken: 'foobar' });
const tickets = await client.sendPushNotificationsAsync([{ to: 'a' }, { to: 'b' }]);
expect(tickets).toEqual(mockTickets);

const [, options] = (fetch as any).lastCall('https://exp.host/--/api/v2/push/send');
const [, options] = (fetch as any).lastCall(sendApiUrl);
expect(options.headers.get('accept')).toContain('application/json');
expect(options.headers.get('accept-encoding')).toContain('gzip');
expect(options.headers.get('content-type')).toContain('application/json');
Expand All @@ -57,29 +63,25 @@ describe('sending push notification messages', () => {
test('sends requests to the Expo API server without the useFcmV1 parameter', async () => {
const client = new ExpoClient();
await client.sendPushNotificationsAsync([{ to: 'a' }]);
expect((fetch as any).called('https://exp.host/--/api/v2/push/send')).toBe(true);
expect((fetch as any).called(sendApiUrl)).toBe(true);
});

test('sends requests to the Expo API server with useFcmV1=true', async () => {
const client = new ExpoClient({ useFcmV1: true });
await client.sendPushNotificationsAsync([{ to: 'a' }]);
expect((fetch as any).called('https://exp.host/--/api/v2/push/send?useFcmV1=true')).toBe(
true,
);
expect((fetch as any).called(`${sendApiUrl}?useFcmV1=true`)).toBe(true);
});

test('sends requests to the Expo API server with useFcmV1=false', async () => {
const client = new ExpoClient({ useFcmV1: false });
await client.sendPushNotificationsAsync([{ to: 'a' }]);
expect((fetch as any).called('https://exp.host/--/api/v2/push/send?useFcmV1=false')).toBe(
true,
);
expect((fetch as any).called(`${sendApiUrl}?useFcmV1=false`)).toBe(true);
});
});

test('compresses request bodies over 1 KiB', async () => {
const mockTickets = [{ status: 'ok', id: 'XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX' }];
(fetch as any).mock('https://exp.host/--/api/v2/push/send', { data: mockTickets });
(fetch as any).mock(sendApiUrl, { data: mockTickets });

const client = new ExpoClient();

Expand All @@ -89,7 +91,7 @@ describe('sending push notification messages', () => {
expect(tickets).toEqual(mockTickets);

// Ensure the request body was compressed
const [, options] = (fetch as any).lastCall('https://exp.host/--/api/v2/push/send');
const [, options] = (fetch as any).lastCall(sendApiUrl);
expect(options.body.length).toBeLessThan(JSON.stringify(messages).length);
expect(options.headers.get('content-encoding')).toContain('gzip');
});
Expand All @@ -99,7 +101,7 @@ describe('sending push notification messages', () => {
{ status: 'ok', id: 'XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX' },
{ status: 'ok', id: 'YYYYYYYY-YYYY-YYYY-YYYY-YYYYYYYYYYYY' },
];
(fetch as any).mock('https://exp.host/--/api/v2/push/send', { data: mockTickets });
(fetch as any).mock(sendApiUrl, { data: mockTickets });

const client = new ExpoClient();
await expect(client.sendPushNotificationsAsync([{ to: 'a' }])).rejects.toThrow(
Expand All @@ -112,7 +114,7 @@ describe('sending push notification messages', () => {
});

test('handles 200 HTTP responses with well-formed API errors', async () => {
(fetch as any).mock('https://exp.host/--/api/v2/push/send', {
(fetch as any).mock(sendApiUrl, {
status: 200,
errors: [{ code: 'TEST_API_ERROR', message: `This is a test error` }],
});
Expand All @@ -124,7 +126,7 @@ describe('sending push notification messages', () => {
});

test('handles 200 HTTP responses with malformed JSON', async () => {
(fetch as any).mock('https://exp.host/--/api/v2/push/send', {
(fetch as any).mock(sendApiUrl, {
status: 200,
body: '<!DOCTYPE html><body>Not JSON</body>',
});
Expand All @@ -136,7 +138,7 @@ describe('sending push notification messages', () => {
});

test('handles non-200 HTTP responses with well-formed API errors', async () => {
(fetch as any).mock('https://exp.host/--/api/v2/push/send', {
(fetch as any).mock(sendApiUrl, {
status: 400,
body: {
errors: [{ code: 'TEST_API_ERROR', message: `This is a test error` }],
Expand All @@ -150,7 +152,7 @@ describe('sending push notification messages', () => {
});

test('handles non-200 HTTP responses with arbitrary JSON', async () => {
(fetch as any).mock('https://exp.host/--/api/v2/push/send', {
(fetch as any).mock(sendApiUrl, {
status: 400,
body: { clowntown: true },
});
Expand All @@ -162,7 +164,7 @@ describe('sending push notification messages', () => {
});

test('handles non-200 HTTP responses with arbitrary text', async () => {
(fetch as any).mock('https://exp.host/--/api/v2/push/send', {
(fetch as any).mock(sendApiUrl, {
status: 400,
body: '<!DOCTYPE html><body>Not JSON</body>',
});
Expand All @@ -174,7 +176,7 @@ describe('sending push notification messages', () => {
});

test('handles well-formed API responses with multiple errors and extra details', async () => {
(fetch as any).mock('https://exp.host/--/api/v2/push/send', {
(fetch as any).mock(sendApiUrl, {
status: 400,
body: {
errors: [
Expand Down Expand Up @@ -207,7 +209,7 @@ describe('sending push notification messages', () => {

test('handles 429 Too Many Requests by applying exponential backoff', async () => {
(fetch as any).mock(
'https://exp.host/--/api/v2/push/send',
sendApiUrl,
{
status: 429,
body: {
Expand All @@ -234,7 +236,7 @@ describe('sending push notification messages', () => {
];
(fetch as any)
.mock(
'https://exp.host/--/api/v2/push/send',
sendApiUrl,
{
status: 429,
body: {
Expand All @@ -243,11 +245,7 @@ describe('sending push notification messages', () => {
},
{ repeat: 2 },
)
.mock(
'https://exp.host/--/api/v2/push/send',
{ data: mockTickets },
{ overwriteRoutes: false },
);
.mock(sendApiUrl, { data: mockTickets }, { overwriteRoutes: false });

const client = new ExpoClient();
await expect(client.sendPushNotificationsAsync([{ to: 'a' }, { to: 'b' }])).resolves.toEqual(
Expand All @@ -264,7 +262,7 @@ describe('retrieving push notification receipts', () => {
'XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX': { status: 'ok' },
'YYYYYYYY-YYYY-YYYY-YYYY-YYYYYYYYYYYY': { status: 'ok' },
};
(fetch as any).mock('https://exp.host/--/api/v2/push/getReceipts', { data: mockReceipts });
(fetch as any).mock(getReceiptsApiUrl, { data: mockReceipts });

const client = new ExpoClient();
const receipts = await client.getPushNotificationReceiptsAsync([
Expand All @@ -273,15 +271,15 @@ describe('retrieving push notification receipts', () => {
]);
expect(receipts).toEqual(mockReceipts);

const [, options] = (fetch as any).lastCall('https://exp.host/--/api/v2/push/getReceipts');
const [, options] = (fetch as any).lastCall(getReceiptsApiUrl);
expect(options.headers.get('accept')).toContain('application/json');
expect(options.headers.get('accept-encoding')).toContain('gzip');
expect(options.headers.get('content-type')).toContain('application/json');
});

test('throws an error if the response is not a map', async () => {
const mockReceipts = [{ status: 'ok' }];
(fetch as any).mock('https://exp.host/--/api/v2/push/getReceipts', { data: mockReceipts });
(fetch as any).mock(getReceiptsApiUrl, { data: mockReceipts });

const client = new ExpoClient();
const rejection = expect(
Expand Down

0 comments on commit 7927286

Please sign in to comment.