Open aperezg opened 4 years ago
@aperezg I like this idea, and I think I could implement swagger -> various.imp.json.
I would like to volunteer to contribute here 🚀.
However, I have a few questions that I think we should elaborate before I start to work on this.
swagger3-gen
?Since we match in gorilla/mux
order, my assumption is that the generated imposter files would then be manually reviewed / edited following the generation step? Such that the maintainer of the generated files would add e.g. regexen to narrow down the each case so that there'd be some HTTP 400
and HTTP 404
matches, too.
The generator would still be doing most of the work, but the user would then need to do the most difficult work. The user would have the necessary context to make those decisions, so they should still find adding the necessary regexen to be fairly simple. Does that sound right?
I think it would be very difficult to automatically map e.g. parameter having format of int64
-> HTTP 400
response handler (would need to be present in the spec, and the generator would need to be able to figure this out). Is it safe for the generator to guess this by adding certain predefined regexen based on type that the user can then refine?
Assuming a manual post-editing step seems safest in any case. Does this mean that we need to modify the format of .imp
files to have e.g. THIS FILE WAS AUTOMATICALLY GENERATED
and also THIS FILE MUST BE EDITED PRIOR TO BEING USED BY KILLGRAVE
metadata? Similar to reviewing a git merge conflict, the user would then need to open each such .imp
file and remove the automated metadata to tell Killgrave that they've reviewed things.
.imp
formats too?Once solved for one format, I think this should be straightfoward to port to the others. Worth doing as a separate issue for visibility?
Alternatively, would it make more sense to output JSON schema rather than just simple JSON imposters?
This would solve several of the problems above, but again would only make sense if the API also supported JSON schema (else we might map responses incorrectly in a similar way to that described above).
Again, assuming that a manual post-editing step will be done might make sense (in order to control the size of this issue).
Also, in the case of the example above, I think the generator would generate three and not four imposters, because only HTTP 200
, HTTP 400
and HTTP 404
are expressly defined.
My understanding is that the default handler in the swagger will be used by generated clients if none of the other codes match, e.g. if HTTP 500
.
If swagger3-gen
should indeed generate four imp files, then what status code should be used for the default
case?
One that doesn't match HTTP 200
, HTTP 400
, HTTP 404
(i.e. the others specified)? Chosen at random, or e.g. HTTP 500
? What if HTTP 500
is one of the ones specified in the spec? Again, we could take the --default-imposter-response-code-value
to be used as a flag, and have a sensible default value for the flag e.g. HTTP 500
. We might also error if this value matches one of the expressly-defined response values in any of the routes in the swagger file.
I also noticed a couple of minor outdated links in the TOC on the readme (and, I think, within the README to other locations in the README). I can do these as part of this PR or I can raise a separate issue if preferred.
Hello @stoovon first of all, thanks for you interest on the project :) I will try to answer your questions:
Are the files going to have a further editing step after swagger3-gen?
Yes, I understand that all the generations on Killgrave even that the proxy record
feature on which I am working, it will require manual work for part of the user to finish the job, if not as you said could be a nightmare to try to cover all the cases.
Only generate JSON or generate the other .imp formats too?
For now, we support .imp.json
and .imp.yml
in the near future we don't see support for more extensions
Use JSON schema (possibly a flag)
I don't understand well this point, the idea is try to generate files almost final, where we could liberate the user the tedious part of create all the imposters
of their APIs
Default response case Also, in the case of the example above, I think the generator would generate three and not four imposters, because only HTTP 200 , HTTP 400 and HTTP 404 are expressly defined.
Yes I understand that define the 500 error could be a little tricky, so we can start doing as you said and we will see
So, I hope have answered all your questions, and I want to see your contribution, remember use the v0.5.0 branch as base :)
Awesome, thanks a lot for this. I know code > talk but I wanted to set a few boundaries before cutting in. I should have time to look into this on Friday evening and over the weekend.
@aperezg thanks again for the responses. Here's my first cut of the PR!
https://github.com/friendsofgo/killgrave/pull/116
I think this should address much of the above, there are going to be a couple of edge cases that this does not address but I'd be happy to resolve either in this PR or, if they're slightly out of scope as I think, I think this PR sets a good foundation to add them in future.
Context
A lot of API that run on the world using Swagger to create a specification to document it. Is for that reason that we want to get the possibility to read the swagger files to create the imposters.
Proposed implementation
The idea is create a new flag called
swagger-gen
with the address of the swagger responses schema, we need to process those files to create all the necessary imposters on the directory that the user selected with the flagimposters
.We need create as many imposters for each endpoints as responses have, for example:
In this case we will need to create for the endpoint
/user/{userId}
with for imposters, one for200
response, another for400
and so on.We need be compatible with OpenAPI 3 Specification and Open API 2 Specification we could do that letting to the user decide wich version want to use with two differents flags to select the path,
swagger-gen
andswagger3-gen