search

krakenjs / swaggerize-express

Design-driven apis with swagger 2.0 and express.
Other
355 stars 81 forks source link

Tweaks the expressroutes validateInput behavior to correctly process … #69

Closed ghost closed 9 years ago

ghost commented 9 years ago

…values in the query string.

Closes krakenjs/swaggerize-express/issues/64

tlivings commented 9 years ago

Building failing :(

Also at this point, not much need for isPath, isForm, etc... The purpose of these was when multiple parameter.in types were being lumped together into a single type. At this point they are entirely distinct.

ghost commented 9 years ago

My bad, forgot to run tests locally first. I've tweaked the test to reflect the value coercion on req.query that's now happening in expressroutes.

I'm not quite following re. the isForm, isPath piece - were you thinking of just swapping in another switch to pass through the coerced values from the validator there?

ghost commented 9 years ago

See above for what I'd suggest re dropping isPath etc. Basically moves to an accessor pattern, drops the switch logic into an object lookup and breaks up the longer function into smaller fn calls.

LMK if it seems reasonable, also happy to tweak to fit another preferred pattern.

ghost commented 9 years ago

hmm, will look at the travis errors. tests seem happy locally

ghost commented 9 years ago

couple of lint issues, now fixed

tlivings commented 9 years ago

I like this, but can you remove lodash usage? It was an unused dependency left over that needed to be removed.

tlivings commented 9 years ago

Also, sorry this is taking so long! I appreciate the patience.

ghost commented 9 years ago

no problem. I've pushed a version w/out lodash now - I left the dep in the package.json file for now though, lmk if you'd like me to remove.

If this seems good I'll squash up the commits and get it ready for merge.

tlivings commented 9 years ago

On the face of it, seems ok (with exception of code style comment). Will get another from team to review as well!

grawk commented 9 years ago

LGTM. Code consistency is important but I would argue for functions assigned to variables.. Maybe merge this and we can argue about that later.. :+1: