search

krakenjs / swaggerize-routes

Swagger document driven route builder.
Other
58 stars 57 forks source link

FIX Issue#76 Adding minLength attribute in validator's template #79

Closed anchnk closed 7 years ago

anchnk commented 7 years ago

Adding minLength attribute in the validator's template so that swagger minLength attributes aren't ignored and overrided by enjoi which sets minLength to 0.

Adding it in the template allows to reject shema that have parameters that doesn't satisfies size constraint

anchnk commented 7 years ago

PR #79

This PR aim to resolve issue #76.

Use Case

For instance a ressource exposed by your API can have an Header (which is not requires) that does have a minLength attribues in it's swagger definition. That means if the parameter (i.e header in that case) is present it must satisfy this constraint of minLength. So if your header myHeader does have minLength set to 3 myHeader's length (if present) should be at least equals to 3.

Why this PR ?

It's actually causing a bug in an application which is in the testing environment. We are using a custom module which depends on another module that depends on swaggerize-routes. And our application isn't sending an error when the header is present with no value when it does have a minLength attributes in the swagger definition.

CI

Build is failing as latest commit on master is also making the build fail (tests are ok but linter throws error because of cyclomatic complexity in lib/buildroutes.js. However tests are working it's not related to this PR.

Any review or input is welcome.

As it's blocking our application to move forward this PR is quite important to be released asap. We are willing to help.

Regards.

tlivings commented 7 years ago

I'm fine with adding the missing attributes from parameter object but we should probably add all of them - not just minLength.

Do you want to do this and add the appropriate tests? Or would you prefer one of us fixes this?

anchnk commented 7 years ago

Thank you for your reply Trevor.

First, I think master branch build should be fixed. Can someone on your side fix the linting issue ? As it's part of the existing codebase.

When you speak about missing attributes from parameter object, do you refer to these ones ?

I think it depends on your bandwidth.

Fixing this will require at least 2 weeks (considering we are having an peak activity atm) will it be faster if you take this responsibility ?

If somebody can just give the appropriate schema and hints about what exactly should be implemented I am willing to start working on it next week. If you want to collaborate on this as well it's ok. Whatever suits you best and serve the project for the best.

Cheers

anchnk commented 7 years ago

I am ok closing this PR and use PR #80 instead as it provide a more complete answer to the original problem. However I think the linting error should be fixed on master before we try to merge incoming PRs as Travis Build will probably fail.