Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/414#discussion_r87090436
--- Diff:
test-framework/src/main/java/org/apache/brooklyn/test/framework/TestHttpCallImpl.java
---
@@ -109,6 +130,49 @@ public Integer get() {
}
}
+ private HttpRequestBase createHttpMethod(HttpMethod method, String
url, Map<String, String> headers, String body) throws Exception {
+ switch (method) {
+ case GET:
+ HttpTool.HttpGetBuilder httpGetBuilder = new
HttpTool.HttpGetBuilder(new URI(url));
--- End diff --
I wondered about moving the `HttpMethod` into `HttpTool`. No strong
feelings. If we did try to add a generic builder, then it would allow setting a
body for all methods, and then would have to validate on `build()`. That's not
ideal, but maybe worth it to avoid duplication.
Also, see
https://en.wikipedia.org/wiki/Hypertext_Transfer_Protocol#Request_methods.
There are other methods we're not supporting (e.g. `TRACE`, `OPTIONS`, etc).
Are we right to use an enum with a subset of the methods? I think we are: the
enum lists all the methods that are supported. And there is no need to try to
support the less common methods until there's a real use-case.
Let's leave it as-is, and think about it some more :-)
But if you feel it would be a lot cleaner to have a generic builder, and/or
to move `HttpMethod` into `HttpTool` then go for it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
