-
Notifications
You must be signed in to change notification settings - Fork 100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for templating in response bodies #177
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Adam Ludes <[email protected]>
Signed-off-by: Adam Ludes <[email protected]>
Signed-off-by: Adam Ludes <[email protected]>
Signed-off-by: Adam Ludes <[email protected]>
a22df5e
to
ba9cd81
Compare
Signed-off-by: Adam Ludes <[email protected]>
ba9cd81
to
cd95984
Compare
README.md
Outdated
|
||
Templates can also include data from the request body, allowing you to create dynamic responses based on the request data. Currently only JSON bodies are supported. The request also needs to have the correct content type set (Content-Type: application/json). | ||
|
||
This example also showcases the functions `timeNow`, `timeUTC`, `timeAdd`, `timeFormat`, `jsonMarshal` and `stringsJoin` that are available for use in templates. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd love to have a two separated examples, please 🙏🏻 So, we can see how to use data from a JSON request in one, and how to use the functions in another one.
You already set another section for the functions, so we could perhaps duplicate the example, and simplify the one for using data from JSON requests to not use any function. Wdyt?
internal/server/http/handler.go
Outdated
tmpl, err := template.New("body"). | ||
Funcs(template.FuncMap{ | ||
"stringsJoin": strings.Join, | ||
"jsonMarshal": func(v interface{}) (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer if we can have these functions declared somewhere else, with their own tests (at least the happy path), and just referenced here. Like with strings.Join
, but for the custom ones.
We could have a package like: internal/template
or internal/templating
specifically for that. Even, defining the apply function and the types there, so we can have proper tests for them as well, and we only use that functionality from this internal/server/http
package.
internal/server/http/handler.go
Outdated
return body, nil | ||
} | ||
|
||
func extractPathParams(i Imposter, r *http.Request) map[string]string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have a couple of tests for this function, please?
return params | ||
} | ||
|
||
func extractQueryParams(r *http.Request) map[string][]string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also for this one, please 🙏🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @deerbone,
Thanks for contributing with such an amazing feature. I'm really excited with the idea of having this feature in the next release of Killgrave 🙌🏻
Please, try to address the minor feedback I left, re-organize a bit the new code, add some unit tests (I'm not concerned about the correctness of your implementation, but about it's maintainability, like having a clear and easy way to make sure we don't break anything in the future), and I'll be delighted to press the merge button! 🚀
Co-authored-by: Joan López de la Franca Beltran <[email protected]>
Signed-off-by: Adam Ludes <[email protected]>
Co-authored-by: Joan López de la Franca Beltran <[email protected]>
Co-authored-by: Joan López de la Franca Beltran <[email protected]>
Signed-off-by: Adam Ludes <[email protected]>
I feel like this code is now ready for round two. Re-requesting review. |
@joanlopez just bumping this in case you missed it. No hard feelings if you don't have the time. |
I have added support for using
text/template
engine in the response body.This change allows users to declare dynamic responses using templating from path parameters, query parameters and in a limited way the request body.
If a template response uses the request body, the request currently needs to be a JSON and contain the
"Content-Type": "application/json"
header.