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

match() throws an error if no matching validator is found. Fixes #64. #67

Closed aautio closed 3 years ago

aautio commented 3 years ago

Hello 👋

Here is a breaking change to match(). I've implemented it as discussed in #64 and #63. Now match() will throw an error when no matching routes are found from OpenAPI schema. The error is an instance of ValidationError without the .data array.

To skip the error you can invoke match({ allowNoMatch: true }).

There was also a bug in the original tests for match. There was no matching path for /match/works-with-url-param in the OpenAPI schema. Thus match() did not invoke any validators and the test yielded a false negative. This was fixed by the first commit in this pull request.

Would this be ok to be published as 0.6.0? Feel free to edit anything as you wish. I'll be happy to help with this one further. Let me know if e.g. changes to ValidationError are not welcome.

coveralls commented 3 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling dffa3cec38d2eb8d2bf379c4e0818162772af97f on aautio:master into 71b4af06b3c96fa5b38d877c3f623986f17b5945 on Hilzu:master.

Hilzu commented 3 years ago

Hi Ari! Thanks for your PR.

To me this looks good. I wouldn't touch ValidationError for this feature. It's only meant for errors thrown when validating the incoming requests. You could replace it with just plain Error or create a new MatchError if you feel like it.

aautio commented 3 years ago

Ok super 👌

I've switched the ValidationError to Error and updated docs & tests appropriately. Changes were pushed a couple of minutes ago.

codeclown commented 3 years ago

@Hilzu Can we help in any way to get this merged? Checks have failed due to:

Bad response: 422 {"message":"service_job_id (723838053) must be unique for Travis Jobs not supplying a Coveralls Repo Token","error":true}

Either COVERALLS_REPO_TOKEN has become a requirement, or this is an issue in Travis. Some confusing discussion in https://github.com/lemurheavy/coveralls-public/issues/1264.

Hilzu commented 3 years ago

Sorry this got stuck due to me looking into the Travis issue. I'm merging this in and replacing Travis with GitHub actions which should work nicer for open-source projects.