You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ygy <gi...@git.apache.org> on 2016/11/16 14:18:55 UTC
[GitHub] brooklyn-server pull request #441: Fixing HttpResponse headers value
GitHub user ygy opened a pull request:
https://github.com/apache/brooklyn-server/pull/441
Fixing HttpResponse headers value
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ygy/brooklyn-server fix/http-response
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/441.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #441
----
commit 5649c2d3cb92210e9e566413ddfa3cafbf1d7790
Author: Yavor Yanchev <ya...@yanchev.com>
Date: 2016-11-16T09:36:36Z
Fixing HttpResponse headers value
commit a8cef3f6047a8cd177ce220330494a42cdb99ed5
Author: Yavor Yanchev <ya...@yanchev.com>
Date: 2016-11-16T14:14:25Z
Adding test for HttpResponse headers value
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #441: Fixing HttpResponse headers value
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/441
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #441: Fixing HttpResponse headers value
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/441#discussion_r88297251
--- Diff: core/src/test/java/org/apache/brooklyn/util/executor/HttpExecutorImplTest.java ---
@@ -71,6 +75,36 @@ public void afterMethod() throws Exception {
}
@Test
+ public void testHttpResponse() throws Exception {
+ Multimap<String, String> responseHeaders = ArrayListMultimap.<String, String>create();
+ responseHeaders.put(HTTP_RESPONSE_CUSTOM_HEADER_KEY, HTTP_RESPONSE_CUSTOM_HEADER_VALUE);
+ HttpResponse httpResponse = new HttpResponse.Builder()
+ .headers(responseHeaders)
+ .header(HTTP_RESPONSE_HEADER_KEY, HTTP_RESPONSE_HEADER_VALUE)
+ .build();
+
+ MockResponse serverResponse = new MockResponse()
+ .setResponseCode(200)
+ .setBody(HTTP_BODY);
+
+ for (Map.Entry<String, String> entry : httpResponse.headers().entries()) {
+ serverResponse.addHeader(entry.getKey(), entry.getValue());
+ }
+ server.enqueue(serverResponse);
+ HttpExecutor executor = factory.getHttpExecutor(getProps());
+ HttpRequest executorRequest = new HttpRequest.Builder()
+ .method("GET")
+ .uri(baseUrl.toURI())
+ .build();
+ HttpResponse executorResponse = executor.execute(executorRequest);
+ assertTrue(executorResponse.headers().containsKey(HTTP_RESPONSE_CUSTOM_HEADER_KEY));
+ assertTrue(Iterables.getOnlyElement(executorResponse.headers().get(HTTP_RESPONSE_HEADER_KEY)).equals(HTTP_RESPONSE_HEADER_VALUE));
+
+ assertTrue(executorResponse.headers().containsKey(HTTP_RESPONSE_HEADER_KEY));
+ assertTrue(Iterables.getOnlyElement(executorResponse.headers().get(HTTP_RESPONSE_CUSTOM_HEADER_KEY)).equals(HTTP_RESPONSE_CUSTOM_HEADER_VALUE));
+ }
--- End diff --
The essence of the test is line `90`. You can cut it down to verifying that `httpResponse.headers()` contains the expected values. Everything else just gets that value through a number of hoops before you check it at the end.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #441: Fixing HttpResponse headers value
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the issue:
https://github.com/apache/brooklyn-server/pull/441
Could simplify the test, otherwise good to go.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server issue #441: Fixing HttpResponse headers value
Posted by ygy <gi...@git.apache.org>.
Github user ygy commented on the issue:
https://github.com/apache/brooklyn-server/pull/441
@aledsage @neykov The test has been simplified
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-server pull request #441: Fixing HttpResponse headers value
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/441#discussion_r88349432
--- Diff: core/src/test/java/org/apache/brooklyn/util/executor/HttpExecutorImplTest.java ---
@@ -71,6 +75,36 @@ public void afterMethod() throws Exception {
}
@Test
+ public void testHttpResponse() throws Exception {
+ Multimap<String, String> responseHeaders = ArrayListMultimap.<String, String>create();
+ responseHeaders.put(HTTP_RESPONSE_CUSTOM_HEADER_KEY, HTTP_RESPONSE_CUSTOM_HEADER_VALUE);
+ HttpResponse httpResponse = new HttpResponse.Builder()
+ .headers(responseHeaders)
+ .header(HTTP_RESPONSE_HEADER_KEY, HTTP_RESPONSE_HEADER_VALUE)
+ .build();
+
+ MockResponse serverResponse = new MockResponse()
+ .setResponseCode(200)
+ .setBody(HTTP_BODY);
+
+ for (Map.Entry<String, String> entry : httpResponse.headers().entries()) {
+ serverResponse.addHeader(entry.getKey(), entry.getValue());
+ }
+ server.enqueue(serverResponse);
+ HttpExecutor executor = factory.getHttpExecutor(getProps());
+ HttpRequest executorRequest = new HttpRequest.Builder()
+ .method("GET")
+ .uri(baseUrl.toURI())
+ .build();
+ HttpResponse executorResponse = executor.execute(executorRequest);
+ assertTrue(executorResponse.headers().containsKey(HTTP_RESPONSE_CUSTOM_HEADER_KEY));
+ assertTrue(Iterables.getOnlyElement(executorResponse.headers().get(HTTP_RESPONSE_HEADER_KEY)).equals(HTTP_RESPONSE_HEADER_VALUE));
+
+ assertTrue(executorResponse.headers().containsKey(HTTP_RESPONSE_HEADER_KEY));
+ assertTrue(Iterables.getOnlyElement(executorResponse.headers().get(HTTP_RESPONSE_CUSTOM_HEADER_KEY)).equals(HTTP_RESPONSE_CUSTOM_HEADER_VALUE));
+ }
--- End diff --
Agreed. I'd probably simplify it to something like:
```
HttpResponse executorResponse = executor.execute(executorRequest);
Multimap<String, String> actualHeaders = executorResponse.headers();
assertEquals(Iterables.getOnlyElement(actualHeaders.get(HTTP_RESPONSE_CUSTOM_HEADER_KEY)), HTTP_RESPONSE_CUSTOM_HEADER_VALUE);
assertEquals(Iterables.getOnlyElement(actualHeaders.get(HTTP_RESPONSE_HEADER_KEY)), HTTP_RESPONSE_HEADER_VALUE);
```
That probably good enough for an error message if it fails (the line number is probably enough here, as you've managed to make it a nice self-contained test with use of `MockResponse`).
Do you want to look at simplifying that, and then good to merge?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---