Skip to content

Commit d0fd723

Browse files
CyntiBintiCyntiBinti
committed
test: unit tests added for full coverage
1 parent 1b6ba8e commit d0fd723

File tree

2 files changed

+157
-77
lines changed

2 files changed

+157
-77
lines changed

‎packages/middleware-allow-request-methods/lib/index.js

Lines changed: 29 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
const { logRecoverableError } = require('@dotcom-reliability-kit/log-error');
1+
const { UserInputError } = require('@dotcom-reliability-kit/errors');
22

33
/**
44
* @typedef {object} RequestMethodOptions
@@ -8,65 +8,48 @@ const { logRecoverableError } = require('@dotcom-reliability-kit/log-error');
88
*/
99

1010
/**
11-
* @typedef {import('express').ErrorRequestHandler} ExpressErrorHandler
11+
* @typedef {import('express').Response} ExpressResponse
1212
*/
1313

1414
/**
1515
* Create a middleware function to return 405 (rather than 404) for disallowed request methods.
1616
*
1717
* @param {RequestMethodOptions} [options]
18-
* @returns {ExpressErrorHandler}
18+
* @returns {import('express').RequestHandler} - Returns an Express middleware function.
1919
*/
2020
function allowRequestMethods(
2121
options = { allowedMethods: [], message: 'Method Not Allowed' }
2222
) {
23+
// Check if allowed methods have been specified and are valid
24+
const allowedMethodsSpecified = options?.allowedMethods;
25+
if (
26+
!allowedMethodsSpecified ||
27+
allowedMethodsSpecified.length === 0 ||
28+
allowedMethodsSpecified.every((method) => typeof method !== 'string')
29+
) {
30+
throw new TypeError(
31+
'The `allowedMethods` option must be an array of strings'
32+
);
33+
}
34+
2335
const normalisedAllowedRequestMethods = normaliseAllowedRequestMethods(
24-
options.allowedMethods || []
36+
allowedMethodsSpecified
2537
);
2638

27-
return function allowRequestMethodsMiddleware(
28-
error,
29-
request,
30-
response,
31-
next
32-
) {
39+
return function allowRequestMethodsMiddleware(request, response, next) {
3340
// If headers are already sent, pass the error to the default Express error handler
34-
if (response.headersSent) {
35-
return next(error);
41+
if (!response.headersSent) {
42+
response.header('Allow', normalisedAllowedRequestMethods.join(', '));
3643
}
3744

38-
try {
39-
// If the allowed methods array is empty, you can either allow all methods or reject everything
40-
if (normalisedAllowedRequestMethods.length === 0) {
41-
// TODO: Option 1: Allow all methods (no restriction) i.e. request proceeds as normal
42-
return next();
43-
44-
// TODO: or Option 2: Reject all methods (405 for every request) i.e. block all requests when no methods are explicitly stated
45-
// response.header('Allow', normalisedAllowedRequestMethods.join(', '));
46-
// response.status(405).send(options.message);
47-
// return next(new MethodNotAllowedError(options.message));
48-
}
49-
50-
// If the incoming request method is not in the allowed methods array, then send a 405 error
51-
if (
52-
!normalisedAllowedRequestMethods.includes(request.method.toUpperCase())
53-
) {
54-
response.header('Allow', normalisedAllowedRequestMethods.join(', '));
55-
response.status(405).send(options.message);
56-
return next(new MethodNotAllowedError(options.message));
57-
} else {
58-
// Else if it is, then pass the request to the next() middleware
59-
next();
60-
}
61-
} catch (/** @type {any} */ error) {
62-
if (options.logger) {
63-
logRecoverableError({
64-
error,
65-
logger: options.logger,
66-
request
67-
});
68-
}
69-
next(error);
45+
// If the incoming request method is not in the allowed methods array, then send a 405 error
46+
if (
47+
!normalisedAllowedRequestMethods.includes(request.method.toUpperCase())
48+
) {
49+
return next(new UserInputError({ statusCode: 405 }));
50+
} else {
51+
// Else if it is, then pass the request to the next() middleware
52+
next();
7053
}
7154
};
7255
}
@@ -78,35 +61,7 @@ function allowRequestMethods(
7861
* @returns {string[]} - Returns an array of capitalised HTTP methods.
7962
*/
8063
function normaliseAllowedRequestMethods(methods) {
81-
if (!Array.isArray(methods) || methods.length === 0) {
82-
return [];
83-
}
84-
return methods
85-
.filter((method) => typeof method === 'string')
86-
.map((method) => method.toUpperCase());
87-
}
88-
89-
/**
90-
* Error class for 405 Method Not Allowed errors.
91-
*
92-
* @augments Error
93-
* @property {string} name
94-
* @property {number} status
95-
* @property {number} statusCode
96-
*/
97-
class MethodNotAllowedError extends Error {
98-
/**
99-
* @override
100-
* @type {string}
101-
*/
102-
name = 'MethodNotAllowedError';
103-
104-
/** @type {number} */
105-
status = 405;
106-
107-
/** @type {number} */
108-
statusCode = 405;
64+
return methods.map((method) => method.toUpperCase());
10965
}
11066

111-
module.exports = allowRequestMethods;
112-
module.exports.default = module.exports;
67+
exports.allowRequestMethods = allowRequestMethods;

‎packages/middleware-allow-request-methods/test/unit/lib/index.spec.js

Lines changed: 128 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,130 @@
1-
describe('@dotcom-reliability-kit/middleware-allow-request-methods', () => {
2-
it('has some tests', () => {
3-
throw new Error('Please write some tests');
1+
const { allowRequestMethods } = require('../../../lib/index');
2+
const { UserInputError } = require('@dotcom-reliability-kit/errors');
3+
4+
// Mock Express request and response objects
5+
let mockRequest;
6+
let mockResponse;
7+
let mockNext;
8+
9+
describe('allowRequestMethods', () => {
10+
beforeEach(() => {
11+
// Reset all mocks before each test
12+
mockRequest = {
13+
method: 'GET'
14+
};
15+
mockResponse = {
16+
headersSent: false,
17+
header: jest.fn()
18+
};
19+
mockNext = jest.fn();
20+
});
21+
22+
afterEach(() => {
23+
// Clear all mocks after each test
24+
jest.clearAllMocks();
25+
});
26+
27+
describe('initialisation and validation', () => {
28+
it('throws TypeError when no allowedMethods are provided', () => {
29+
expect(() => {
30+
allowRequestMethods();
31+
}).toThrow(TypeError);
32+
33+
expect(() => {
34+
allowRequestMethods({});
35+
}).toThrow('The `allowedMethods` option must be an array of strings');
36+
});
37+
38+
it('throws TypeError when allowedMethods is an empty array', () => {
39+
expect(() => {
40+
allowRequestMethods({ allowedMethods: [] });
41+
}).toThrow(TypeError);
42+
});
43+
44+
it('throws TypeError when allowedMethods contains non-string values', () => {
45+
expect(() => {
46+
allowRequestMethods({ allowedMethods: [123, true] });
47+
}).toThrow(TypeError);
48+
});
49+
50+
it('creates middleware function when valid allowedMethods are provided', () => {
51+
const middleware = allowRequestMethods({
52+
allowedMethods: ['GET', 'POST']
53+
});
54+
expect(typeof middleware).toBe('function');
55+
});
56+
});
57+
58+
describe('middleware behavior', () => {
59+
it('sets Allow header with normalised methods', () => {
60+
const middleware = allowRequestMethods({
61+
allowedMethods: ['get', 'post']
62+
});
63+
64+
middleware(mockRequest, mockResponse, mockNext);
65+
66+
expect(mockResponse.header).toHaveBeenCalledWith('Allow', 'GET, POST');
67+
});
68+
69+
it('skips setting header if headers are already sent', () => {
70+
mockResponse.headersSent = true;
71+
const middleware = allowRequestMethods({
72+
allowedMethods: ['GET', 'POST']
73+
});
74+
75+
middleware(mockRequest, mockResponse, mockNext);
76+
77+
expect(mockResponse.header).not.toHaveBeenCalled();
78+
});
79+
80+
it('calls next() with 405 error for disallowed method', () => {
81+
mockRequest.method = 'DELETE';
82+
const middleware = allowRequestMethods({
83+
allowedMethods: ['GET', 'POST']
84+
});
85+
86+
middleware(mockRequest, mockResponse, mockNext);
87+
88+
expect(mockNext).toHaveBeenCalledWith(expect.any(UserInputError));
89+
const error = mockNext.mock.calls[0][0];
90+
expect(error.statusCode).toBe(405);
91+
});
92+
93+
it('calls next() without error for allowed method', () => {
94+
mockRequest.method = 'GET';
95+
const middleware = allowRequestMethods({
96+
allowedMethods: ['GET', 'POST']
97+
});
98+
99+
middleware(mockRequest, mockResponse, mockNext);
100+
101+
expect(mockNext).toHaveBeenCalledWith();
102+
});
103+
104+
it('handles case-insensitive method matching', () => {
105+
mockRequest.method = 'get';
106+
const middleware = allowRequestMethods({
107+
allowedMethods: ['GET', 'POST']
108+
});
109+
110+
middleware(mockRequest, mockResponse, mockNext);
111+
112+
expect(mockNext).toHaveBeenCalledWith();
113+
});
114+
});
115+
116+
describe('normaliseAllowedRequestMethods', () => {
117+
it('normalises methods to uppercase', () => {
118+
const middleware = allowRequestMethods({
119+
allowedMethods: ['get', 'Post', 'DELETE']
120+
});
121+
122+
middleware(mockRequest, mockResponse, mockNext);
123+
124+
expect(mockResponse.header).toHaveBeenCalledWith(
125+
'Allow',
126+
'GET, POST, DELETE'
127+
);
128+
});
4129
});
5130
});

0 commit comments

Comments
 (0)