Merge pull request #78 from BenWhitehead/cleanup

Add javadoc and cleanup ResponseUtils to indicate thread constraints.
d2iq-archive · May 16, 2017 · ac18b30 · ac18b30
2 parents 1b17fbb + 87994ca
commit ac18b30
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 26 deletions.
diff --git a/mesos-rxjava-client/src/main/java/com/mesosphere/mesos/rx/java/ResponseUtils.java b/mesos-rxjava-client/src/main/java/com/mesosphere/mesos/rx/java/ResponseUtils.java
@@ -29,23 +29,29 @@ final class ResponseUtils {
 
     private ResponseUtils() {}
 
+    /**
+     * Attempts to read the content of an error response as {@code text/plain;charset=utf-8}, otherwise the content
+     * will be ignored and a string detailing the Content-Type that was not processed.
+     * <p>
+     * <b>NOTE:</b>
+     * <i>
+     *     This method MUST be called from the netty-io thread otherwise the content of the response will not be
+     *     available because if will be released automatically as soon as the netty-io thread is left.
+     * </i>
+     * @param resp  The response to attempt to read from
+     * @return An {@link Observable} representing the {@code text/plain;charset=utf-8} response content if it existed
+     *         or an error message indicating the content-type that was not attempted to read.
+     */
     @NotNull
     static Observable<String> attemptToReadErrorResponse(@NotNull final HttpClientResponse<ByteBuf> resp) {
-        return attemptToReadErrorResponse(resp, true);
-    }
-
-    @NotNull
-    static Observable<String> attemptToReadErrorResponse(@NotNull final HttpClientResponse<ByteBuf> resp, final boolean ignoreContentWhenUnreadable) {
         final HttpResponseHeaders headers = resp.getHeaders();
         final String contentType = resp.getHeaders().get(HttpHeaderNames.CONTENT_TYPE);
         if (headers.isContentLengthSet() && headers.getContentLength() > 0 ) {
             if (contentType != null && contentType.startsWith("text/plain")) {
                 return resp.getContent()
                     .map(r -> r.toString(StandardCharsets.UTF_8));
             } else {
-                if (ignoreContentWhenUnreadable) {
-                    resp.ignoreContent();
-                }
+                resp.ignoreContent();
                 final String errMsg = getErrMsg(contentType);
                 return Observable.just(errMsg);
             }

diff --git a/mesos-rxjava-client/src/test/java/com/mesosphere/mesos/rx/java/ResponseUtilsTest.java b/mesos-rxjava-client/src/test/java/com/mesosphere/mesos/rx/java/ResponseUtilsTest.java
@@ -114,24 +114,6 @@ public void attemptToReadErrorResponse_responseContentIgnoredByDefaultWhenNotStr
         }
     }
 
-    @Test
-    public void attemptToReadErrorResponse_responseContentNotIgnoredWhenFalse() throws Exception {
-        final String errMsg = "lies";
-        final byte[] bytes = errMsg.getBytes(StandardCharsets.UTF_8);
-        final HttpClientResponse<ByteBuf> resp = response(Unpooled.copiedBuffer(bytes), (headers) -> {
-            headers.add("Content-Type", "application/json;charset=utf-8");
-            headers.add("Content-Length", bytes.length);
-        });
-
-        final String err = ResponseUtils.attemptToReadErrorResponse(resp, false).toBlocking().first();
-        assertThat(err).isNotEqualTo("lies");
-
-        final String first = resp.getContent()
-            .map(buf -> buf.toString(StandardCharsets.UTF_8))
-            .toBlocking().first();
-        assertThat(first).isEqualTo("lies");
-    }
-
     private static HttpClientResponse<ByteBuf> response(
         @NotNull final ByteBuf content,
         @NotNull final Action1<HttpHeaders> headerTransformer