search

Hilzu / express-openapi-validate

Express middleware to validate requests based on an OpenAPI 3 document
Apache License 2.0
75 stars 12 forks source link

Implements #10 - validator.find() #37

Closed codeclown closed 4 years ago

codeclown commented 5 years ago

Addresses issue https://github.com/Hilzu/express-openapi-validate/issues/10.

This is from an older branch I made when I needed this functionality in a project.

Let me know if there's something to adjust (namings, tests, something else) and I'll happily revisit this.

Hilzu commented 5 years ago

Just came back from holiday and will look at this PR this week. Before that can you rebase this on master, fix the tests and add tests to keep code coverage at 100%? The travis build and coveralls should be working now.

backbone87 commented 5 years ago

path-to-regexp uses express style path, while OAS uses another syntax for path parameter, this implementation does not work with OAS path's like /user/{id}

codeclown commented 4 years ago

Finally taking a stab at resolving this PR.

@backbone87 Thanks for pointing that out!

I checked the V3 spec about paths and it seems like OpenAPI supports path params in format {name} and nothing else (also not allowing slashes), so I've implemented a simple transform from OAS to Express path before passing onto path-to-regexp (could've removed the dependency altogether and implemented the regex logic in this project, but I felt it would've been unnecessary).

I have a problem making the OpenApiValidator.test.ts test case pass because TypeScript will not accept the second parameter. Types come from here. Talking about this line in the test:

match(req, { ...baseRes }, () => {});

I've tried null (obvious fail) and new express.Response (not so obvious fail).

And the build is failing due to something unrelated:

[error] "2019-10-16T10:11:50.900Z"  'error from lcovParse: ' 'Failed to parse string'
[error] "2019-10-16T10:11:50.903Z"  'input: ' ''
[error] "2019-10-16T10:11:50.903Z"  'error from convertLcovToCoveralls'
coveralls commented 4 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling b1c5a458385332f93763b8c316d6cd54dcd2ee08 on codeclown:match into c647e29ad92402bd3f4b0b1e0a09baadab3093e5 on Hilzu:master.

coveralls commented 4 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling a3d07cdffea1e546117bcee79dd1a7bb8a7d3cd0 on codeclown:match into c647e29ad92402bd3f4b0b1e0a09baadab3093e5 on Hilzu:master.

codeclown commented 4 years ago

@Hilzu Tests are green.

Hilzu commented 4 years ago

This looks good. Thank you for your contribution and thanks for waiting this long to get the change merged.

codeclown commented 4 years ago

@Hilzu Thanks for merging. Added some docs as well in separate PR: https://github.com/Hilzu/express-openapi-validate/pull/37