Skip to content

Commit 16f8ef0

Browse files
TangoYankeepratishta
TangoYankee
authored and
pratishta
committed
Update invalid request parameter exemption
Add zod error details to messages of invalid request parameters
1 parent 6f29702 commit 16f8ef0

8 files changed

Lines changed: 113 additions & 179 deletions

File tree

‎src/exception/invalid-request-parameter.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
export class InvalidRequestParameterException extends Error {
22
constructor(message: string) {
3-
super(`Invalid data type or format for request parameter: ${message}`);
3+
super(`Invalid request parameter: ${message}`);
44
this.name = "InvalidRequestParameterException";
55
}
66
}

‎src/pipes/zod-transform-pipe.ts‎

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
import { PipeTransform } from "@nestjs/common";
22
import { InvalidRequestParameterException } from "src/exception";
3-
import { ZodRawShape, ZodObject, ZodOptional, ZodArray, ZodBoolean } from "zod";
3+
import {
4+
ZodRawShape,
5+
ZodObject,
6+
ZodOptional,
7+
ZodArray,
8+
ZodBoolean,
9+
ZodError,
10+
} from "zod";
411
export class ZodTransformPipe<T extends ZodRawShape> implements PipeTransform {
512
constructor(private schema: ZodObject<T> | ZodOptional<ZodObject<T>>) {}
613

@@ -18,8 +25,11 @@ export class ZodTransformPipe<T extends ZodRawShape> implements PipeTransform {
1825
> = {};
1926

2027
Object.entries(params).forEach(([param, value]) => {
21-
const schema = schemaProperties[param];
22-
const property = schema instanceof ZodOptional ? schema.unwrap() : schema;
28+
const parameterSchema = schemaProperties[param];
29+
const property =
30+
parameterSchema instanceof ZodOptional
31+
? parameterSchema.unwrap()
32+
: parameterSchema;
2333

2434
if (property instanceof ZodArray) {
2535
decodedParams[param] = Array.isArray(value) ? value : value.split(",");
@@ -35,18 +45,35 @@ export class ZodTransformPipe<T extends ZodRawShape> implements PipeTransform {
3545
decodedParams[param] = false;
3646
return;
3747
}
38-
throw new InvalidRequestParameterException("invalid value for boolean schema property");
48+
throw new InvalidRequestParameterException(
49+
"invalid value for boolean schema property",
50+
);
3951
}
4052

4153
decodedParams[param] = value;
4254
});
4355

4456
try {
4557
const parsedParams = this.schema.parse(decodedParams);
58+
// It is possible for Zod to return `undefined` when optional parameters are provided `undefined` values
59+
// Though, this should not be possible within this transform pipe because all values are passed through urls, which are strings
60+
// However, the type definition doesn't know that the values come from a url.
61+
// We account for undefined and satisify the type defintion by throwing an error, even though we should never encounter `undefined`
4662
if (parsedParams === undefined) throw new Error();
4763
return parsedParams;
48-
} catch (error) {
49-
throw new InvalidRequestParameterException("no params");
64+
} catch (e) {
65+
if (e instanceof ZodError) {
66+
const errorMessages: Array<string> = [];
67+
const { errors } = e;
68+
errors.forEach((error) => {
69+
const parameter = error.path[0]; // first position of array holds the parameter name
70+
const { message } = error;
71+
errorMessages.push(`${parameter}: ${message}`);
72+
});
73+
throw new InvalidRequestParameterException(errorMessages.join("; "));
74+
}
75+
76+
throw new InvalidRequestParameterException("unable to parse parameters");
5077
}
5178
}
5279
}

‎test/borough/borough.e2e-spec.ts‎

Lines changed: 12 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,7 @@ import {
1212
findCommunityDistrictGeoJsonByBoroughIdCommunityDistrictIdQueryResponseSchema,
1313
findCommunityDistrictsByBoroughIdQueryResponseSchema,
1414
} from "src/gen";
15-
import {
16-
DataRetrievalException,
17-
InvalidRequestParameterException,
18-
} from "src/exception";
15+
import { DataRetrievalException } from "src/exception";
1916
import { HttpName } from "src/filter";
2017
import { AgencyRepositoryMock } from "test/agency/agency.repository.mock";
2118
import { AgencyBudgetRepositoryMock } from "test/agency-budget/agency-budget.repository.mock";
@@ -143,7 +140,7 @@ describe("Borough e2e", () => {
143140
});
144141
});
145142

146-
describe("findCityCouncilDistrictGeoJsonByCityCouncilDistrictId", () => {
143+
describe("findCommunityDistrictGeoJsonByBoroughIdCommunityDistrictIdMocks", () => {
147144
it("should 200 and return documented schema when finding by valid id", async () => {
148145
const mock =
149146
boroughRepositoryMock
@@ -165,9 +162,7 @@ describe("Borough e2e", () => {
165162
const response = await request(app.getHttpServer())
166163
.get(`/boroughs/1/community-districts/${longId}/geojson`)
167164
.expect(400);
168-
expect(response.body.message).toBe(
169-
new InvalidRequestParameterException("invalid parameters").message,
170-
);
165+
expect(response.body.message).toMatch(/communityDistrictId: Invalid/);
171166
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
172167
});
173168

@@ -176,9 +171,7 @@ describe("Borough e2e", () => {
176171
const response = await request(app.getHttpServer())
177172
.get(`/boroughs/1/community-districts/${letterId}/geojson`)
178173
.expect(400);
179-
expect(response.body.message).toBe(
180-
new InvalidRequestParameterException("invalid parameters").message,
181-
);
174+
expect(response.body.message).toMatch(/communityDistrictId: Invalid/);
182175
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
183176
});
184177

@@ -269,9 +262,7 @@ describe("Borough e2e", () => {
269262
`/boroughs/${missingId}/community-districts/${communityDistrict.id}/capital-projects`,
270263
)
271264
.expect(400);
272-
expect(response.body.message).toBe(
273-
new InvalidRequestParameterException("invalid parameters").message,
274-
);
265+
expect(response.body.message).toMatch(/could not check/);
275266
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
276267
});
277268

@@ -282,9 +273,8 @@ describe("Borough e2e", () => {
282273
`/boroughs/${communityDistrict.boroughId}/community-districts/${missingId}/capital-projects`,
283274
)
284275
.expect(400);
285-
expect(response.body.message).toBe(
286-
new InvalidRequestParameterException("invalid parameters").message,
287-
);
276+
expect(response.body.message).toMatch(/could not check/);
277+
288278
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
289279
});
290280

@@ -398,9 +388,7 @@ describe("Borough e2e", () => {
398388
)
399389
.expect(400);
400390

401-
expect(response.body.message).toBe(
402-
new InvalidRequestParameterException("invalid parameters").message,
403-
);
391+
expect(response.body.message).toMatch(/boroughId: Invalid/);
404392

405393
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
406394
});
@@ -417,9 +405,7 @@ describe("Borough e2e", () => {
417405
)
418406
.expect(400);
419407

420-
expect(response.body.message).toBe(
421-
new InvalidRequestParameterException("invalid parameters").message,
422-
);
408+
expect(response.body.message).toMatch(/communityDistrictId: Invalid/);
423409

424410
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
425411
});
@@ -434,10 +420,9 @@ describe("Borough e2e", () => {
434420
)
435421
.expect(400);
436422

437-
expect(response.body.message).toBe(
438-
new InvalidRequestParameterException("invalid parameters").message,
439-
);
440-
423+
expect(response.body.message).toMatch(/z: Expected number/);
424+
expect(response.body.message).toMatch(/x: Expected number/);
425+
expect(response.body.message).toMatch(/y: Expected number/);
441426
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
442427
});
443428

‎test/capital-project/capital-project.e2e-spec.ts‎

Lines changed: 20 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,7 @@ import { AgencyRepositoryMock } from "test/agency/agency.repository.mock";
1212
import { AgencyBudgetRepositoryMock } from "test/agency-budget/agency-budget.repository.mock";
1313
import * as request from "supertest";
1414
import { HttpName } from "src/filter";
15-
import {
16-
DataRetrievalException,
17-
InvalidRequestParameterException,
18-
} from "src/exception";
15+
import { DataRetrievalException } from "src/exception";
1916
import {
2017
findCapitalCommitmentsByManagingCodeCapitalProjectIdQueryResponseSchema,
2118
findCapitalProjectByManagingCodeCapitalProjectIdQueryResponseSchema,
@@ -122,49 +119,39 @@ describe("Capital Projects", () => {
122119
`/capital-projects?agencyBudget=${agencyBudgetCode}`,
123120
);
124121

125-
expect(response.body.message).toBe(
126-
new InvalidRequestParameterException("invalid parameters").message,
127-
);
122+
expect(response.body.message).toMatch(/could not check/);
128123
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
129124
});
130125

131126
it("should 400 when finding by an invalid limit", async () => {
132127
const response = await request(app.getHttpServer()).get(
133128
"/capital-projects?limit=b4d",
134129
);
135-
expect(response.body.message).toBe(
136-
new InvalidRequestParameterException("invalid parameters").message,
137-
);
130+
expect(response.body.message).toMatch(/limit: Expected number/);
138131
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
139132
});
140133

141134
it("should 400 when finding by a 'too-high' limit", async () => {
142135
const response = await request(app.getHttpServer()).get(
143136
"/capital-projects?limit=101",
144137
);
145-
expect(response.body.message).toBe(
146-
new InvalidRequestParameterException("invalid parameters").message,
147-
);
138+
expect(response.body.message).toMatch(/limit: Number must be less/);
148139
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
149140
});
150141

151142
it("should 400 when finding by a 'too-low' limit", async () => {
152143
const response = await request(app.getHttpServer()).get(
153144
"/capital-projects?limit=0",
154145
);
155-
expect(response.body.message).toBe(
156-
new InvalidRequestParameterException("invalid parameters").message,
157-
);
146+
expect(response.body.message).toMatch(/limit: Number must be greater/);
158147
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
159148
});
160149

161150
it("should 400 when finding by invalid offset", async () => {
162151
const response = await request(app.getHttpServer()).get(
163152
"/capital-projects?offset=b4d",
164153
);
165-
expect(response.body.message).toBe(
166-
new InvalidRequestParameterException("invalid parameters").message,
167-
);
154+
expect(response.body.message).toMatch(/offset: Expected number/);
168155
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
169156
});
170157

@@ -192,9 +179,7 @@ describe("Capital Projects", () => {
192179
const response = await request(app.getHttpServer()).get(
193180
`/capital-projects?cityCouncilDistrictId=${id}`,
194181
);
195-
expect(response.body.message).toBe(
196-
new InvalidRequestParameterException("invalid parameters").message,
197-
);
182+
expect(response.body.message).toMatch(/cityCouncilDistrictId: Invalid/);
198183
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
199184
});
200185

@@ -222,9 +207,7 @@ describe("Capital Projects", () => {
222207
const response = await request(app.getHttpServer()).get(
223208
`/capital-projects?communityDistrictId=${id}`,
224209
);
225-
expect(response.body.message).toBe(
226-
new InvalidRequestParameterException("invalid parameters").message,
227-
);
210+
expect(response.body.message).toMatch(/communityDistrictId: Invalid/);
228211
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
229212
});
230213

@@ -253,9 +236,7 @@ describe("Capital Projects", () => {
253236
const response = await request(app.getHttpServer()).get(
254237
`/capital-projects?managingAgency=${managingAgency}`,
255238
);
256-
expect(response.body.message).toBe(
257-
new InvalidRequestParameterException("invalid parameters").message,
258-
);
239+
expect(response.body.message).toMatch(/could not check/);
259240
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
260241
});
261242

@@ -283,9 +264,7 @@ describe("Capital Projects", () => {
283264
const response = await request(app.getHttpServer()).get(
284265
`/capital-projects?commitmentsTotalMin=${commitmentsTotalMin}`,
285266
);
286-
expect(response.body.message).toBe(
287-
new InvalidRequestParameterException("invalid parameters").message,
288-
);
267+
expect(response.body.message).toMatch(/commitmentsTotalMin: Invalid/);
289268
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
290269
});
291270

@@ -313,9 +292,7 @@ describe("Capital Projects", () => {
313292
const response = await request(app.getHttpServer()).get(
314293
`/capital-projects?commitmentsTotalMax=${commitmentsTotalMax}`,
315294
);
316-
expect(response.body.message).toBe(
317-
new InvalidRequestParameterException("invalid parameters").message,
318-
);
295+
expect(response.body.message).toMatch(/commitmentsTotalMax: Invalid/);
319296
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
320297
});
321298

@@ -325,9 +302,7 @@ describe("Capital Projects", () => {
325302
const response = await request(app.getHttpServer()).get(
326303
`/capital-projects?commitmentsTotalMin=${commitmentsTotalMin}&commitmentsTotalMax=${commitmentsTotalMax}`,
327304
);
328-
expect(response.body.message).toBe(
329-
new InvalidRequestParameterException("invalid parameters").message,
330-
);
305+
expect(response.body.message).toMatch(/min amount should be/);
331306
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
332307
});
333308

@@ -407,18 +382,16 @@ describe("Capital Projects", () => {
407382
const response = await request(app.getHttpServer()).get(
408383
`/capital-projects?isMapped=123`,
409384
);
410-
expect(response.body.message).toBe(
411-
new InvalidRequestParameterException("invalid parameters").message,
412-
);
385+
expect(response.body.message).toMatch(/invalid value for boolean/);
413386
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
414387
});
415388

416389
it("should 400 when when both a city council district id and isMapped are provided", async () => {
417390
const response = await request(app.getHttpServer()).get(
418391
`/capital-projects?cityCouncilDistrictId=50&isMapped=true`,
419392
);
420-
expect(response.body.message).toBe(
421-
new InvalidRequestParameterException("invalid parameters").message,
393+
expect(response.body.message).toMatch(
394+
/cannot have isMapped filter in conjunction/,
422395
);
423396
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
424397
});
@@ -427,8 +400,8 @@ describe("Capital Projects", () => {
427400
const response = await request(app.getHttpServer()).get(
428401
`/capital-projects?communityDistrictId=101&isMapped=true`,
429402
);
430-
expect(response.body.message).toBe(
431-
new InvalidRequestParameterException("invalid parameters").message,
403+
expect(response.body.message).toMatch(
404+
/cannot have isMapped filter in conjunction/,
432405
);
433406
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
434407
});
@@ -534,9 +507,6 @@ describe("Capital Projects", () => {
534507
)
535508
.expect(400);
536509

537-
expect(response.body.message).toBe(
538-
new InvalidRequestParameterException("invalid parameters").message,
539-
);
540510
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
541511
});
542512

@@ -609,9 +579,9 @@ describe("Capital Projects", () => {
609579
.get(`/capital-projects/${z}/${x}/${y}.pbf`)
610580
.expect(400);
611581

612-
expect(response.body.message).toBe(
613-
new InvalidRequestParameterException("invalid parameters").message,
614-
);
582+
expect(response.body.message).toMatch(/z: Expected number/);
583+
expect(response.body.message).toMatch(/x: Expected number/);
584+
expect(response.body.message).toMatch(/y: Expected number/);
615585

616586
expect(response.body.error).toBe(HttpName.BAD_REQUEST);
617587
});

0 commit comments

Comments
 (0)