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

Support for endpoint rate limits #3426

Merged
merged 17 commits into from
Oct 7, 2023
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
2 changes: 1 addition & 1 deletion packages/core/src/resources/ApplicationPlanLimits.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export class ApplicationPlanLimits<C extends boolean = false> extends BaseResour
} = options;

return RequestHelper.put<ApplicationPlanLimitSchema>()(this, 'application/plan_limits', {
...opts,
searchParams: {
planName,
ciPipelineSize,
Expand All @@ -81,7 +82,6 @@ export class ApplicationPlanLimits<C extends boolean = false> extends BaseResour
terraformModuleMaxFileSize,
storageSizeLimit,
},
opts,
});
}
}
36 changes: 36 additions & 0 deletions packages/core/test/unit/resources/ApplicationPlanLimits.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { RequestHelper } from '../../../src/infrastructure';
import { ApplicationPlanLimits } from '../../../src';

jest.mock(
'../../../src/infrastructure/RequestHelper',
() => require('../../__mocks__/RequestHelper').default,
);

let service: ApplicationPlanLimits;

beforeEach(() => {
service = new ApplicationPlanLimits({
requesterFn: jest.fn(),
token: 'abcdefg',
});
});

describe('ApplicationPlanLimits.show', () => {
it('should request GET /application/plan_limits', async () => {
await service.show();

expect(RequestHelper.get()).toHaveBeenCalledWith(service, 'application/plan_limits', undefined);
});
});

describe('ApplicationPlanLimits.edit', () => {
it('should request PUT /application/plan_limits with a terms property', async () => {
await service.edit('Plan name');

expect(RequestHelper.put()).toHaveBeenCalledWith(service, 'application/plan_limits', {
searchParams: {
planName: 'Plan name',
},
});
});
});
24 changes: 24 additions & 0 deletions packages/core/test/unit/resources/ApplicationStatistics.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { RequestHelper } from '../../../src/infrastructure';
import { ApplicationStatistics } from '../../../src';

jest.mock(
'../../../src/infrastructure/RequestHelper',
() => require('../../__mocks__/RequestHelper').default,
);

let service: ApplicationStatistics;

beforeEach(() => {
service = new ApplicationStatistics({
requesterFn: jest.fn(),
token: 'abcdefg',
});
});

describe('ApplicationStatistics.show', () => {
it('should request GET /application/statistics', async () => {
await service.show();

expect(RequestHelper.get()).toHaveBeenCalledWith(service, 'application/statistics', undefined);
});
});
44 changes: 44 additions & 0 deletions packages/core/test/unit/resources/Applications.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { RequestHelper } from '../../../src/infrastructure';
import { Applications } from '../../../src';

jest.mock(
'../../../src/infrastructure/RequestHelper',
() => require('../../__mocks__/RequestHelper').default,
);

let service: Applications;

beforeEach(() => {
service = new Applications({
requesterFn: jest.fn(),
token: 'abcdefg',
});
});

describe('Applications.all', () => {
it('should request GET /applications without options', async () => {
await service.all();

expect(RequestHelper.get()).toHaveBeenCalledWith(service, 'applications', undefined);
});
});

describe('Applications.show', () => {
it('should request GET /applications', async () => {
await service.create('application', 'url', 'scope1');

expect(RequestHelper.get()).toHaveBeenCalledWith(service, 'applications', {
name: 'application',
redirectUri: 'url',
scopes: 'scope1',
});
});
});

describe('Applications.remove', () => {
it('should request GET /applications/:id', async () => {
await service.remove(12);

expect(RequestHelper.get()).toHaveBeenCalledWith(service, 'applications/12', undefined);
});
});
2 changes: 2 additions & 0 deletions packages/requester-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@
"release": "auto shipit"
},
"dependencies": {
"async-sema": "^3.1.1",
"micromatch": "^4.0.5",
"qs": "^6.11.2",
"xcase": "^2.0.1"
},
Expand Down
48 changes: 46 additions & 2 deletions packages/requester-utils/src/BaseResource.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { RequesterType, ResourceOptions } from './RequesterUtils';
import { RateLimitOptions, RequesterType, ResourceOptions } from './RequesterUtils';

export interface RootResourceOptions<C> {
// TODO: Not actually optional - Need to fix wrapper typing in requestUtils.ts:
Expand All @@ -11,6 +11,7 @@ export interface RootResourceOptions<C> {
sudo?: string | number;
profileToken?: string;
profileMode?: 'execution' | 'memory';
rateLimits?: RateLimitOptions;
}

export type GitlabToken = string | (() => Promise<string>);
Expand All @@ -36,6 +37,48 @@ function getDynamicToken(tokenArgument: (() => Promise<string>) | string): Promi
return tokenArgument instanceof Function ? tokenArgument() : Promise.resolve(tokenArgument);
}

// Default rate limits per minute
const DEFAULT_RATE_LIMITS = Object.freeze({
// Default rate limit
'**': 3000,

// Import/Export
'projects/import': 6,
'projects/*/export': 6,
'projects/*/download': 1,
'groups/import': 6,
'groups/*/export': 6,
'groups/*/download': 1,

// Note creation
'projects/*/issues/*/notes': {
method: 'post',
limit: 300,
},
'projects/*/snippets/*/notes': {
method: 'post',
limit: 300,
},
'projects/*/merge_requests/*/notes': {
method: 'post',
limit: 300,
},
'groups/*/epics/*/notes': {
method: 'post',
limit: 300,
},

// Repositories - get file archive
'projects/*/repository/archive*': 5,

// Project Jobs
'projects/*/jobs': 600,

// Member deletion
'projects/*/members': 60,
'groups/*/members': 60,
});

export class BaseResource<C extends boolean = false> {
public readonly url: string;

Expand All @@ -61,6 +104,7 @@ export class BaseResource<C extends boolean = false> {
prefixUrl = '',
rejectUnauthorized = true,
queryTimeout = 300000,
rateLimits = DEFAULT_RATE_LIMITS,
...tokens
}: BaseResourceOptions<C>) {
if (!requesterFn) throw new ReferenceError('requesterFn must be passed');
Expand Down Expand Up @@ -97,6 +141,6 @@ export class BaseResource<C extends boolean = false> {
if (sudo) this.headers.Sudo = `${sudo}`;

// Set requester instance using this information
this.requester = requesterFn({ ...this });
this.requester = requesterFn({ ...this, rateLimits });
}
}
59 changes: 50 additions & 9 deletions packages/requester-utils/src/RequesterUtils.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
import { stringify } from 'qs';
import { decamelizeKeys } from 'xcase';
import { RateLimit } from 'async-sema';
import micromatch from 'micromatch';

// Types
export type RateLimiters = Record<
string,
ReturnType<typeof RateLimit> | { method: string; limit: ReturnType<typeof RateLimit> }
>;
export type RateLimitOptions = Record<string, number | { method: string; limit: number }>;

export type ResponseBodyTypes =
| Record<string, unknown>
| Record<string, unknown>[]
Expand All @@ -28,6 +36,7 @@ export type ResourceOptions = {
authHeaders: { [authHeader: string]: () => Promise<string> };
url: string;
rejectUnauthorized: boolean;
rateLimits?: RateLimitOptions;
};

export type DefaultRequestOptions = {
Expand All @@ -48,6 +57,7 @@ export type RequestOptions = {
body?: string | FormData;
asStream?: boolean;
signal?: AbortSignal;
rateLimiters?: Record<string, ReturnType<typeof RateLimit>>;
};

export interface RequesterType {
Expand All @@ -73,6 +83,11 @@ export interface RequesterType {
): Promise<FormattedResponse<T>>;
}

export type RequestHandlerFn<T extends ResponseBodyTypes = ResponseBodyTypes> = (
endpoint: string,
options?: Record<string, unknown>,
) => Promise<FormattedResponse<T>>;

// Utility methods
export function formatQuery(params: Record<string, unknown> = {}): string {
const decamelized = decamelizeKeys(params);
Expand Down Expand Up @@ -137,10 +152,20 @@ export async function defaultOptionsHandler(
return Promise.resolve(defaultOptions);
}

export type RequestHandlerFn<T extends ResponseBodyTypes = ResponseBodyTypes> = (
endpoint: string,
options?: Record<string, unknown>,
) => Promise<FormattedResponse<T>>;
export function createRateLimiters(rateLimitOptions: RateLimitOptions = {}) {
const rateLimiters: RateLimiters = {};

Object.entries(rateLimitOptions).forEach(([key, config]) => {
if (typeof config === 'number') rateLimiters[key] = RateLimit(config, { timeUnit: 60000 });
else
rateLimiters[key] = {
method: config.method.toUpperCase(),
limit: RateLimit(config.limit, { timeUnit: 60000 }),
};
});

return rateLimiters;
}

export function createRequesterFn(
optionsHandler: OptionsHandlerFn,
Expand All @@ -150,6 +175,7 @@ export function createRequesterFn(

return (serviceOptions) => {
const requester: RequesterType = {} as RequesterType;
const rateLimiters = createRateLimiters(serviceOptions.rateLimits);

methods.forEach((m) => {
requester[m] = async (endpoint: string, options: Record<string, unknown>) => {
Expand All @@ -159,18 +185,15 @@ export function createRequesterFn(
});
const requestOptions = await optionsHandler(serviceOptions, defaultRequestOptions);

return requestHandler(endpoint, requestOptions);
return requestHandler(endpoint, { ...requestOptions, rateLimiters });
};
});

return requester;
};
}

function extendClass<T extends Constructable>(
Base: T,
customConfig: Record<string, unknown> = {},
): T {
function extendClass<T extends Constructable>(Base: T, customConfig: Record<string, unknown>): T {
return class extends Base {
constructor(...options: any[]) {
// eslint-disable-line
Expand All @@ -195,3 +218,21 @@ export function presetResourceArguments<T extends Record<string, Constructable>>

return updated as T;
}

export function getMatchingRateLimiter(
endpoint: string,
rateLimiters: RateLimiters = {},
method: string = 'GET',
): () => Promise<void> {
const sortedEndpoints = Object.keys(rateLimiters).sort().reverse();
const match = sortedEndpoints.find((ep) => micromatch.isMatch(endpoint, ep));
const rateLimitConfig = match && rateLimiters[match];

if (rateLimitConfig && typeof rateLimitConfig !== 'object') {
return rateLimitConfig;
}
if (rateLimitConfig && rateLimitConfig.method.toUpperCase() === method.toUpperCase()) {
return rateLimitConfig.limit;
}
return RateLimit(3000, { timeUnit: 60000 });
}
14 changes: 14 additions & 0 deletions packages/requester-utils/test/unit/BaseResource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,14 @@ describe('Creation of BaseResource instance', () => {
await expect(service.authHeaders['job-token']()).resolves.toBe('1234');
});

it('should throw an error if a token, jobToken or oauthToken is not passed', () => {
expect(() => {
// eslint-disable-next-line
// @ts-ignore
new BaseResource({ requesterFn: jest.fn() }); // eslint-disable-line
}).toThrow();
});

it('should set the X-Profile-Token header if the profileToken option is given', () => {
const service = new BaseResource({
token: '123',
Expand Down Expand Up @@ -196,6 +204,12 @@ describe('Creation of BaseResource instance', () => {
// @ts-ignore
new BaseResource(); // eslint-disable-line
}).toThrow();

expect(() => {
// eslint-disable-next-line
// @ts-ignore
new BaseResource({}); // eslint-disable-line
}).toThrow();
});

it('should set the internal requester based on the required requesterFn parameter', async () => {
Expand Down
Loading