Skip to content

Commit 1275c73

Browse files
cdcabreracdcabrera
committed
fix: review update
1 parent 47c042f commit 1275c73

2 files changed

Lines changed: 77 additions & 77 deletions

File tree

‎src/__tests__/server.assertions.test.ts‎

Lines changed: 43 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -10,21 +10,18 @@ describe('assertInput', () => {
1010
it.each([
1111
{
1212
description: 'basic string validation',
13-
param: '',
14-
condition: (value: any) => typeof value === 'string' && value.trim().length > 0
13+
condition: ' '.trim().length > 0
1514
},
1615
{
17-
description: 'pattern in string validation',
18-
param: 'patternfly://lorem-ipsum',
19-
condition: (value: any) => new RegExp('patternfly://', 'i').test(value)
16+
description: 'pattern in string validation with callback format',
17+
condition: () => new RegExp('patternfly://', 'i').test('fly://lorem-ipsum')
2018
},
2119
{
2220
description: 'array entry length validation',
23-
param: ['lorem', 'ipsum'],
24-
condition: (value: any) => Array.isArray(value) && value.length > 2
21+
condition: Array.isArray(['lorem']) && ['lorem'].length > 2
2522
}
26-
])('should throw an error for validation, $description', ({ param, condition }) => {
27-
const errorMessage = `Lorem ipsum error message for ${param} validation.`;
23+
])('should throw an error for validation, $description', ({ condition }) => {
24+
const errorMessage = `Lorem ipsum error message for validation.`;
2825

2926
expect(() => assertInput(
3027
condition,
@@ -37,25 +34,25 @@ describe('assertInputString', () => {
3734
it.each([
3835
{
3936
description: 'empty string',
40-
param: ''
37+
input: ''
4138
},
4239
{
4340
description: 'undefined',
44-
param: undefined
41+
input: undefined
4542
},
4643
{
4744
description: 'number',
48-
param: 1
45+
input: 1
4946
},
5047
{
5148
description: 'null',
52-
param: null
49+
input: null
5350
}
54-
])('should throw an error for validation, $description', ({ param }) => {
51+
])('should throw an error for validation, $description', ({ input }) => {
5552
const errorMessage = '"Input" must be a string';
5653

5754
expect(() => assertInputString(
58-
param
55+
input
5956
)).toThrow(errorMessage);
6057
});
6158
});
@@ -64,50 +61,50 @@ describe('assertInputStringLength', () => {
6461
it.each([
6562
{
6663
description: 'empty string',
67-
param: ''
64+
input: ''
6865
},
6966
{
7067
description: 'undefined',
71-
param: undefined
68+
input: undefined
7269
},
7370
{
7471
description: 'number',
75-
param: 1
72+
input: 1
7673
},
7774
{
7875
description: 'null',
79-
param: null
76+
input: null
8077
},
8178
{
8279
description: 'max',
83-
param: 'lorem ipsum',
80+
input: 'lorem ipsum',
8481
options: { max: 5 }
8582
},
8683
{
8784
description: 'min',
88-
param: 'lorem ipsum',
85+
input: 'lorem ipsum',
8986
options: { min: 15 }
9087
},
9188
{
9289
description: 'max and min',
93-
param: 'lorem ipsum',
90+
input: 'lorem ipsum',
9491
options: { min: 1, max: 10 }
9592
},
9693
{
9794
description: 'max and min and display name',
98-
param: 'lorem ipsum',
95+
input: 'lorem ipsum',
9996
options: { min: 1, max: 10, inputDisplayName: 'lorem ipsum' }
10097
},
10198
{
10299
description: 'max and min and description',
103-
param: 'lorem ipsum',
100+
input: 'lorem ipsum',
104101
options: { min: 1, max: 10, message: 'dolor sit amet, consectetur adipiscing elit.' }
105102
}
106-
])('should throw an error for validation, $description', ({ param, options }) => {
103+
])('should throw an error for validation, $description', ({ input, options }) => {
107104
const errorMessage = options?.message || `"${options?.inputDisplayName || 'Input'}" must be a string`;
108105

109106
expect(() => assertInputStringLength(
110-
param,
107+
input,
111108
{ min: 1, max: 100, ...options } as any
112109
)).toThrow(errorMessage);
113110
});
@@ -117,50 +114,50 @@ describe('assertInputStringArrayEntryLength', () => {
117114
it.each([
118115
{
119116
description: 'empty string',
120-
param: ''
117+
input: ''
121118
},
122119
{
123120
description: 'undefined',
124-
param: undefined
121+
input: undefined
125122
},
126123
{
127124
description: 'number',
128-
param: 1
125+
input: 1
129126
},
130127
{
131128
description: 'null',
132-
param: null
129+
input: null
133130
},
134131
{
135132
description: 'max',
136-
param: ['lorem ipsum'],
133+
input: ['lorem ipsum'],
137134
options: { max: 5 }
138135
},
139136
{
140137
description: 'min',
141-
param: ['lorem ipsum'],
138+
input: ['lorem ipsum'],
142139
options: { min: 15 }
143140
},
144141
{
145142
description: 'max and min',
146-
param: ['lorem ipsum'],
143+
input: ['lorem ipsum'],
147144
options: { min: 1, max: 10 }
148145
},
149146
{
150147
description: 'max and min and display name',
151-
param: ['lorem ipsum'],
148+
input: ['lorem ipsum'],
152149
options: { min: 1, max: 10, inputDisplayName: 'lorem ipsum' }
153150
},
154151
{
155152
description: 'max and min and description',
156-
param: ['lorem ipsum'],
153+
input: ['lorem ipsum'],
157154
options: { min: 1, max: 10, message: 'dolor sit amet, consectetur adipiscing elit.' }
158155
}
159-
])('should throw an error for validation, $description', ({ param, options }) => {
156+
])('should throw an error for validation, $description', ({ input, options }) => {
160157
const errorMessage = options?.message || `"${options?.inputDisplayName || 'Input'}" array must contain strings`;
161158

162159
expect(() => assertInputStringArrayEntryLength(
163-
param as any,
160+
input as any,
164161
{ min: 1, max: 100, ...options } as any
165162
)).toThrow(errorMessage);
166163
});
@@ -170,46 +167,46 @@ describe('assertInputStringNumberEnumLike', () => {
170167
it.each([
171168
{
172169
description: 'empty string',
173-
param: '',
170+
input: '',
174171
compare: [2, 3]
175172
},
176173
{
177174
description: 'undefined',
178-
param: undefined,
175+
input: undefined,
179176
compare: [2, 3]
180177
},
181178
{
182179
description: 'null',
183-
param: null,
180+
input: null,
184181
compare: [2, 3]
185182
},
186183
{
187184
description: 'number',
188-
param: 1,
185+
input: 1,
189186
compare: [2, 3]
190187
},
191188
{
192189
description: 'string',
193-
param: 'lorem ipsum',
190+
input: 'lorem ipsum',
194191
compare: ['amet', 'dolor sit']
195192
},
196193
{
197194
description: 'string and display name',
198-
param: 'lorem ipsum',
195+
input: 'lorem ipsum',
199196
compare: ['amet', 'dolor sit'],
200197
options: { inputDisplayName: 'lorem ipsum' }
201198
},
202199
{
203200
description: 'string and description',
204-
param: 'lorem ipsum',
201+
input: 'lorem ipsum',
205202
compare: [1, 2],
206203
options: { message: 'dolor sit amet, consectetur adipiscing elit.' }
207204
}
208-
])('should throw an error for validation, $description', ({ param, compare, options }) => {
205+
])('should throw an error for validation, $description', ({ input, compare, options }) => {
209206
const errorMessage = options?.message || `"${options?.inputDisplayName || 'Input'}" must be one of the following values`;
210207

211208
expect(() => assertInputStringNumberEnumLike(
212-
param as any,
209+
input as any,
213210
compare as any,
214211
{ ...options } as any
215212
)).toThrow(errorMessage);

‎src/server.assertions.ts‎

Lines changed: 34 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@ import { ErrorCode, McpError } from '@modelcontextprotocol/sdk/types.js';
44
/**
55
* MCP assert. Centralizes and throws an error if the validation fails.
66
*
7-
* @param condition - The function or condition to be validated.
8-
* @param message - The error message, or function that returns the error message, to be thrown if the validation fails.
9-
* @param {McpError} code - The error code to be thrown if the validation fails. Defaults to `ErrorCode.InvalidParams`.
7+
* @param condition - Function or condition to be validated.
8+
* @param message - Thrown error message, or function, that returns the error message.
9+
* @param {ErrorCode} [code] - Thrown error code when validation fails. Defaults to `ErrorCode.InvalidParams`.
1010
*
11-
* @throws {McpError} Throws the provided error message if the input validation fails.
11+
* @throws {McpError} Throw the provided error message and code on failure.
1212
*/
1313
const mcpAssert = (condition: unknown, message: string | (() => string), code: ErrorCode = ErrorCode.InvalidParams) => {
1414
try {
@@ -26,11 +26,11 @@ const mcpAssert = (condition: unknown, message: string | (() => string), code: E
2626
*
2727
* @alias mcpAssert
2828
*
29-
* @param condition - The function or condition to be validated.
30-
* @param message - The error message, or function that returns the error message, to be thrown if the validation fails.
31-
* @param {McpError} [code] - The error code to be thrown if the validation fails. Defaults to `ErrorCode.InvalidParams`.
29+
* @param condition - Function or condition to be validated.
30+
* @param message - Thrown error message, or function, that returns the error message.
31+
* @param {ErrorCode} [code] - Thrown error code when validation fails. Defaults to `ErrorCode.InvalidParams`.
3232
*
33-
* @throws {McpError} Throws the provided error message if the input validation fails.
33+
* @throws {McpError} Throw the provided error message and code on failure.
3434
*/
3535
function assertInput(
3636
condition: unknown,
@@ -43,10 +43,10 @@ function assertInput(
4343
/**
4444
* Assert/validate if the input is a string.
4545
*
46-
* @param input - The input value
47-
* @param options - Validation options
48-
* @param options.inputDisplayName - Display name for the input. Used in the default error message. Defaults to 'Input'.
49-
* @param options.message - Custom error message. A default error message with optional `inputDisplayName` is generated if not provided.
46+
* @param input - Input value
47+
* @param [options] - Validation options
48+
* @param [options.inputDisplayName] - Display name for the input. Used in the default error message. Defaults to 'Input'.
49+
* @param [options.message] - Custom error message. A default error message with optional `inputDisplayName` is generated if not provided.
5050
*
5151
* @throws McpError If input is not a string OR is empty
5252
*/
@@ -60,14 +60,14 @@ function assertInputString(
6060
}
6161

6262
/**
63-
* Assert/validate if input is a string and meets length requirements.
63+
* Assert/validate if the input is a string and meets length requirements.
6464
*
65-
* @param input - The input string
65+
* @param input - Input string
6666
* @param options - Validation options
67-
* @param options.max - Maximum length of the string
68-
* @param options.min - Minimum length of the string
69-
* @param options.inputDisplayName - Display name for the input. Used in the default error message. Defaults to 'Input'.
70-
* @param options.message - Error description. A default error message with optional `inputDisplayName` is generated if not provided.
67+
* @param options.max - Maximum length of the string. `Required`
68+
* @param options.min - Minimum length of the string. `Required`
69+
* @param [options.inputDisplayName] - Display name for the input. Used in the default error message. Defaults to 'Input'.
70+
* @param [options.message] - Error description. A default error message with optional `inputDisplayName` is generated if not provided.
7171
*
7272
* @throws McpError If input is not a string OR does not meet length requirements
7373
*/
@@ -85,10 +85,10 @@ function assertInputStringLength(
8585
*
8686
* @param input - Array of strings
8787
* @param options - Validation options
88-
* @param options.max - Maximum length of each string in the array
89-
* @param options.min - Minimum length of each string in the array
90-
* @param options.inputDisplayName - Display name for the input. Used in the default error messages. Defaults to 'Input'.
91-
* @param options.message - Error description. A default error message with optional `inputDisplayName` is generated if not provided.
88+
* @param options.max - Maximum length of each string in the array. `Required`
89+
* @param options.min - Minimum length of each string in the array. `Required`
90+
* @param [options.inputDisplayName] - Display name for the input. Used in the default error messages. Defaults to 'Input'.
91+
* @param [options.message] - Error description. A default error message with optional `inputDisplayName` is generated if not provided.
9292
*
9393
* @throws McpError If input is not an array of strings OR does not meet length requirements
9494
*/
@@ -106,27 +106,30 @@ function assertInputStringArrayEntryLength(
106106
*
107107
* @param input - The input value
108108
* @param values - List of allowed values
109-
* @param options - Validation options
110-
* @param options.inputDisplayName - Display name for the input. Used in the default error messages. Defaults to 'Input'.
111-
* @param options.message - Error description. A default error message with optional `inputDisplayName` is generated if not provided.
109+
* @param [options] - Validation options
110+
* @param [options.inputDisplayName] - Display name for the input. Used in the default error messages. Defaults to 'Input'.
111+
* @param [options.message] - Error description. A default error message with optional `inputDisplayName` is generated if not provided.
112112
*
113113
* @throws McpError If input is not a string or number OR is not one of the allowed values
114114
*/
115115
function assertInputStringNumberEnumLike(
116116
input: unknown,
117117
values: unknown[],
118118
{ inputDisplayName, message }: { inputDisplayName?: string, message?: string } = {}
119-
): asserts input is unknown[] {
120-
const hasArrayWithLength = Array.isArray(values) && values.length;
121-
let updatedDescription = message || `"${inputDisplayName || 'Input'}" must be one of the following values: ${values.join(', ')}`;
122-
let errorCode = ErrorCode.InvalidParams;
119+
): asserts input is string | number {
120+
const hasArrayWithLength = Array.isArray(values) && values.length > 0;
121+
let updatedDescription;
122+
let errorCode;
123123

124-
if (!hasArrayWithLength) {
124+
if (hasArrayWithLength) {
125+
errorCode = ErrorCode.InvalidParams;
126+
updatedDescription = message || `"${inputDisplayName || 'Input'}" must be one of the following values: ${values.join(', ')}`;
127+
} else {
125128
errorCode = ErrorCode.InternalError;
126129
updatedDescription = `Unable to confirm "${inputDisplayName || 'input'}." List of allowed values is empty or undefined.`;
127130
}
128131

129-
const isStringOrNumber = (typeof input === 'string' && typeof input.trim().length) || typeof input === 'number';
132+
const isStringOrNumber = (typeof input === 'string' && input.trim().length > 0) || typeof input === 'number';
130133
const isValid = isStringOrNumber && hasArrayWithLength && values.includes(input);
131134

132135
mcpAssert(isValid, updatedDescription, errorCode);

0 commit comments

Comments
 (0)