fix missing validators for referencing parameters

krakenjs / swaggerize-routes

Swagger document driven route builder.

Other

58 stars 57 forks source link

fix missing validators for referencing parameters #65

Open gotchazipc opened 8 years ago

gotchazipc commented 8 years ago

fix to solve #40

moved merge logic into validators, removing temporal map in builder
added testcase and fixed some tests for the added path in pets.json

If some parameters has only $ref without name or in, builder overwrites the parameters in raw object with same key, NaN (undefined + undefined). So, merging should be done after resolving references.

subeeshcbabu-zz commented 8 years ago

Like the idea of using the resolved $ref. However the operation level parameters should be overriding the path level (applicable for all the operations) parameters. The concat here - https://github.com/krakenjs/swaggerize-routes/pull/65/files#diff-2d057fabb5adb84befefd4c50a8edc6bL64 - simply adds the duplicates as well, I think.

gotchazipc commented 8 years ago

Actually, the validators for operation level parameters, made in validators.js, overwrites the validators for path level parameters in validators#makeAll(). Because, input parameters are just built with concat (def.parameters, operation.parameters) and they are checked with forEach().

Since new logic does not use 'name' nor 'in' before calling makeAll(), eleminating 'to-be-overrided' parameters (not to build unused validators) seems to be a liitle bit hard. Is there any property that can be used to check?

subeeshcbabu-zz commented 7 years ago

How about passing both def.parameters and operation.parameters to makeAll , instead of concat? Then we will be able to call make for all the parameters and construct the final validator array/list by using the unique key combination of name and in

subeeshcbabu-zz commented 7 years ago

@gotchazipc - How about passing both def.parameters and operation.parameters to makeAll , instead of concat?