-
Notifications
You must be signed in to change notification settings - Fork 2
Client validation #139
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
Client validation #139
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,58 @@ | ||||||||||||||||||||||||||||||
| import { newZodValidator, ZodApiEndpoints } from "../../src"; | ||||||||||||||||||||||||||||||
| import { ValidateError, withValidation } from "../../src/fetch/validation"; | ||||||||||||||||||||||||||||||
| import { z, ZodError } from "zod"; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const GITHUB_API_ORIGIN = "https://api.github.com"; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // See https://docs.github.com/ja/rest/repos/repos?apiVersion=2022-11-28#get-all-repository-topics | ||||||||||||||||||||||||||||||
| const spec = { | ||||||||||||||||||||||||||||||
| "/repos/:owner/:repo/topics": { | ||||||||||||||||||||||||||||||
| get: { | ||||||||||||||||||||||||||||||
| responses: { 200: { body: z.object({ names: z.string().array() }) } }, | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| } satisfies ZodApiEndpoints; | ||||||||||||||||||||||||||||||
| // type Spec = ToApiEndpoints<typeof spec>; | ||||||||||||||||||||||||||||||
| const spec2 = { | ||||||||||||||||||||||||||||||
| "/repos/:owner/:repo/topics": { | ||||||||||||||||||||||||||||||
| get: { | ||||||||||||||||||||||||||||||
| responses: { 200: { body: z.object({ noexist: z.string() }) } }, | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| } satisfies ZodApiEndpoints; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| const main = async () => { | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| // const fetchT = fetch as FetchT<typeof GITHUB_API_ORIGIN, Spec>; | ||||||||||||||||||||||||||||||
| const { req: reqValidator, res: resValidator } = newZodValidator(spec); | ||||||||||||||||||||||||||||||
| const fetchWithV = withValidation(fetch, spec, reqValidator, resValidator); | ||||||||||||||||||||||||||||||
| const response = await fetchWithV( | ||||||||||||||||||||||||||||||
| `${GITHUB_API_ORIGIN}/repos/mpppk/typed-api-spec/topics?page=1`, | ||||||||||||||||||||||||||||||
| { headers: { Accept: "application/vnd.github+json" } }, | ||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||
| if (!response.ok) { | ||||||||||||||||||||||||||||||
| const { message } = await response.json(); | ||||||||||||||||||||||||||||||
| return console.error(message); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| const { names } = await response.json(); | ||||||||||||||||||||||||||||||
| console.log(names); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| // const fetchT = fetch as FetchT<typeof GITHUB_API_ORIGIN, Spec>; | ||||||||||||||||||||||||||||||
| const { req: reqValidator, res: resValidator } = newZodValidator(spec2); | ||||||||||||||||||||||||||||||
| const fetchWithV = withValidation(fetch, spec2, reqValidator, resValidator); | ||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||
| await fetchWithV( | ||||||||||||||||||||||||||||||
| `${GITHUB_API_ORIGIN}/repos/mpppk/typed-api-spec/topics?page=1`, | ||||||||||||||||||||||||||||||
| { headers: { Accept: "application/vnd.github+json" } }, | ||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||
| } catch (e: unknown) { | ||||||||||||||||||||||||||||||
| if (e instanceof ValidateError) { | ||||||||||||||||||||||||||||||
| console.log("error thrown", (e.error as ZodError).format()); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
Comment on lines
+50
to
+54
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance error handling The error handling could be more specific and informative:
} catch (e: unknown) {
if (e instanceof ValidateError) {
console.log("error thrown", (e.error as ZodError).format());
+ } else if (e instanceof TypeError) {
+ console.error("Network error:", e.message);
+ } else {
+ console.error("Unexpected error:", e);
}
}📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| main(); | ||||||||||||||||||||||||||||||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,11 +14,13 @@ | |
| "example:express-zod": "tsx examples/express/zod/express.ts", | ||
| "example:express-zod-fetch": "tsx examples/express/zod/fetch.ts", | ||
| "example:fasitify-zod": "tsx examples/fastify/zod/fastify.ts", | ||
| "example:fasitify-zod-fetch": "tsx examples/fastify/zod/fetch.ts" | ||
| "example:fasitify-zod-fetch": "tsx examples/fastify/zod/fetch.ts", | ||
| "example:withValidation": "tsx examples/simple/withValidation.ts" | ||
| }, | ||
| "author": "mpppk", | ||
| "license": "ISC", | ||
| "devDependencies": { | ||
| "@types/path-to-regexp": "^1.7.0", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Update @types/path-to-regexp to match path-to-regexp version The type definitions package
Since path-to-regexp v8.x.x now includes built-in TypeScript type definitions, the separate @types package is no longer needed. You should remove the 🔗 Analysis chainFix version mismatch between @types/path-to-regexp and path-to-regexp The type definitions version ( Let's verify the latest compatible versions: Consider updating to matching major versions to ensure type safety. Also applies to: 123-123 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check latest versions of path-to-regexp and its type definitions
echo "Latest path-to-regexp version:"
npm view path-to-regexp version
echo "\nLatest @types/path-to-regexp version:"
npm view @types/path-to-regexp version
echo "\nVerifying version compatibility..."
npm view @types/path-to-regexp@* version
Length of output: 380 |
||
| "@types/qs": "^6.9.15", | ||
| "@types/supertest": "^6.0.2", | ||
| "@typescript-eslint/eslint-plugin": "^7.0.0", | ||
|
|
@@ -116,5 +118,8 @@ | |
| }, | ||
| "main": "./dist/index.js", | ||
| "module": "./dist/index.mjs", | ||
| "types": "./dist/index.d.ts" | ||
| "types": "./dist/index.d.ts", | ||
| "dependencies": { | ||
| "path-to-regexp": "^8.2.0" | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,5 @@ | ||||||||||||||||||||||||||||||||
| import { Result } from "../utils"; | ||||||||||||||||||||||||||||||||
| import { AnyApiEndpoint, AnyApiEndpoints, Method } from "./spec"; | ||||||||||||||||||||||||||||||||
| import { AnyApiEndpoint, AnyApiEndpoints, isMethod, Method } from "./spec"; | ||||||||||||||||||||||||||||||||
| import { ParsedQs } from "qs"; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| export type Validators< | ||||||||||||||||||||||||||||||||
|
|
@@ -25,18 +25,61 @@ export type ValidatorsMap = { | |||||||||||||||||||||||||||||||
| [Path in string]: Partial<Record<Method, AnyValidators>>; | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| export const runValidators = (validators: AnyValidators, error: unknown) => { | ||||||||||||||||||||||||||||||||
| const newD = () => Result.data(undefined); | ||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||
| preCheck: error, | ||||||||||||||||||||||||||||||||
| params: validators.params?.() ?? newD(), | ||||||||||||||||||||||||||||||||
| query: validators.query?.() ?? newD(), | ||||||||||||||||||||||||||||||||
| body: validators.body?.() ?? newD(), | ||||||||||||||||||||||||||||||||
| headers: validators.headers?.() ?? newD(), | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| export type ResponseValidators< | ||||||||||||||||||||||||||||||||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||||||||||||||||||||||||||||||||
| BodyValidator extends AnyValidator | undefined, | ||||||||||||||||||||||||||||||||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||||||||||||||||||||||||||||||||
| HeadersValidator extends AnyValidator | undefined, | ||||||||||||||||||||||||||||||||
| > = { | ||||||||||||||||||||||||||||||||
| body: BodyValidator; | ||||||||||||||||||||||||||||||||
| headers: HeadersValidator; | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
| export type AnyResponseValidators = Partial< | ||||||||||||||||||||||||||||||||
| ResponseValidators<AnyValidator, AnyValidator> | ||||||||||||||||||||||||||||||||
| >; | ||||||||||||||||||||||||||||||||
| export const runResponseValidators = (validators: { | ||||||||||||||||||||||||||||||||
| validator: AnyResponseValidators; | ||||||||||||||||||||||||||||||||
| error: unknown; | ||||||||||||||||||||||||||||||||
| }) => { | ||||||||||||||||||||||||||||||||
| const newD = () => Result.data(undefined); | ||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||
| // TODO: スキーマが間違っていても、bodyのvalidatorがなぜか定義されていない | ||||||||||||||||||||||||||||||||
| preCheck: validators.error, | ||||||||||||||||||||||||||||||||
| body: validators.validator.body?.() ?? newD(), | ||||||||||||||||||||||||||||||||
| headers: validators.validator.headers?.() ?? newD(), | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| export type Validator<Data, Error> = () => Result<Data, Error>; | ||||||||||||||||||||||||||||||||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||||||||||||||||||||||||||||||||
| export type AnyValidator = Validator<any, any>; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| export type ValidatorsInput = { | ||||||||||||||||||||||||||||||||
| path: string; | ||||||||||||||||||||||||||||||||
| method: string; | ||||||||||||||||||||||||||||||||
| params?: Record<string, string>; | ||||||||||||||||||||||||||||||||
| params: Record<string, string | string[]>; | ||||||||||||||||||||||||||||||||
| query?: ParsedQs; | ||||||||||||||||||||||||||||||||
| body?: Record<string, string>; | ||||||||||||||||||||||||||||||||
| headers: Record<string, string | string[] | undefined>; | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
| export type ResponseValidatorsInput = { | ||||||||||||||||||||||||||||||||
| path: string; | ||||||||||||||||||||||||||||||||
| method: string; | ||||||||||||||||||||||||||||||||
| statusCode: number; | ||||||||||||||||||||||||||||||||
| body?: unknown; | ||||||||||||||||||||||||||||||||
| headers: Headers; | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| type ValidationError = { | ||||||||||||||||||||||||||||||||
| actual: string; | ||||||||||||||||||||||||||||||||
|
|
@@ -100,3 +143,36 @@ export const getApiSpec = < | |||||||||||||||||||||||||||||||
| const r = validatePathAndMethod(endpoints, maybePath, maybeMethod); | ||||||||||||||||||||||||||||||||
| return Result.map(r, (d) => endpoints[d.path][d.method]); | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| export const preCheck = <E extends AnyApiEndpoints>( | ||||||||||||||||||||||||||||||||
| endpoints: E, | ||||||||||||||||||||||||||||||||
| path: string, | ||||||||||||||||||||||||||||||||
| maybeMethod: string, | ||||||||||||||||||||||||||||||||
| ) => { | ||||||||||||||||||||||||||||||||
| const method = maybeMethod?.toLowerCase(); | ||||||||||||||||||||||||||||||||
| if (!isMethod(method)) { | ||||||||||||||||||||||||||||||||
| return Result.error(newValidatorMethodNotFoundError(method)); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| return getApiSpec(endpoints, path, method); | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
|
Comment on lines
+152
to
+157
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle potential If Consider updating the code to handle const method = maybeMethod?.toLowerCase();
+ if (!method) {
+ return Result.error(newValidatorMethodNotFoundError('undefined'));
+ }
if (!isMethod(method)) {
return Result.error(newValidatorMethodNotFoundError(method));
}📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| export type ValidatorError = | ||||||||||||||||||||||||||||||||
| | ValidatorMethodNotFoundError | ||||||||||||||||||||||||||||||||
| | ValidatorPathNotFoundError; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| export const newValidatorMethodNotFoundError = (method: string) => ({ | ||||||||||||||||||||||||||||||||
| target: "method", | ||||||||||||||||||||||||||||||||
| actual: method, | ||||||||||||||||||||||||||||||||
| message: `method does not exist in endpoint`, | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
| type ValidatorMethodNotFoundError = ReturnType< | ||||||||||||||||||||||||||||||||
| typeof newValidatorMethodNotFoundError | ||||||||||||||||||||||||||||||||
| >; | ||||||||||||||||||||||||||||||||
| export const newValidatorPathNotFoundError = (path: string) => ({ | ||||||||||||||||||||||||||||||||
| target: "path", | ||||||||||||||||||||||||||||||||
| actual: path, | ||||||||||||||||||||||||||||||||
| message: `path does not exist in endpoints`, | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
| type ValidatorPathNotFoundError = ReturnType< | ||||||||||||||||||||||||||||||||
| typeof newValidatorPathNotFoundError | ||||||||||||||||||||||||||||||||
| >; | ||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -92,20 +92,24 @@ export type RouterT< | |
| }; | ||
|
|
||
| export const validatorMiddleware = < | ||
| V extends (input: ValidatorsInput) => AnyValidators, | ||
| V extends (input: ValidatorsInput) => { | ||
| validator: AnyValidators; | ||
| error: unknown; | ||
| }, | ||
|
Comment on lines
+95
to
+98
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle the The Consider handling the |
||
| >( | ||
| validator: V, | ||
| ) => { | ||
| return (_req: Request, res: Response, next: NextFunction) => { | ||
| res.locals.validate = (req: Request) => { | ||
| return validator({ | ||
| const { validator: v2 } = validator({ | ||
| path: req.route?.path?.toString(), | ||
| method: req.method, | ||
| headers: req.headers, | ||
| params: req.params, | ||
| query: req.query, | ||
| body: req.body, | ||
| }); | ||
| return v2; | ||
| }; | ||
| next(); | ||
| }; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid parsing response body twice
The response body is being parsed twice: once for error checking and once for data extraction. This can lead to errors as the body stream can only be consumed once.
📝 Committable suggestion