Smaller diff

palantir · Mar 27, 2020 · 55aaab6 · 55aaab6
1 parent 4a5c246
commit 55aaab6
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 52 deletions.
diff --git a/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDe.java b/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ConjureBodySerDe.java
@@ -198,7 +198,7 @@ private static final class EncodingDeserializerRegistry<T> implements Deserializ
 
         EncodingDeserializerRegistry(List<Encoding> encodings, ErrorDecoder errorDecoder, TypeMarker<T> token) {
             this.encodings = encodings.stream()
-                    .map(encoding -> EncodingDeserializerContainer.of(encoding, token))
+                    .map(encoding -> new EncodingDeserializerContainer<>(encoding, token))
                     .collect(ImmutableList.toImmutableList());
             this.errorDecoder = errorDecoder;
             this.token = token;
@@ -279,13 +279,9 @@ private static final class EncodingDeserializerContainer<T> {
         private final Encoding encoding;
         private final Encoding.Deserializer<T> deserializer;
 
-        EncodingDeserializerContainer(Encoding encoding, Encoding.Deserializer<T> deserializer) {
+        EncodingDeserializerContainer(Encoding encoding, TypeMarker<T> token) {
             this.encoding = encoding;
-            this.deserializer = deserializer;
-        }
-
-        static <T> EncodingDeserializerContainer<T> of(Encoding encoding, TypeMarker<T> token) {
-            return new EncodingDeserializerContainer<>(encoding, encoding.deserializer(token));
+            this.deserializer = encoding.deserializer(token);
         }
 
         @Override

diff --git a/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ErrorDecoder.java b/dialogue-serde/src/main/java/com/palantir/conjure/java/dialogue/serde/ErrorDecoder.java
@@ -30,8 +30,6 @@
 import java.nio.charset.StandardCharsets;
 import java.util.Optional;
 import javax.ws.rs.core.HttpHeaders;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 /**
  * Extracts and returns a {@link RemoteException} from an {@link Response}.
@@ -42,7 +40,6 @@
 enum ErrorDecoder {
     INSTANCE;
 
-    private static final Logger log = LoggerFactory.getLogger(ErrorDecoder.class);
     private static final ObjectMapper MAPPER = ObjectMappers.newClientObjectMapper();
 
     boolean isError(Response response) {
@@ -53,40 +50,26 @@ RemoteException decode(Response response) {
         // TODO(rfink): What about HTTP/101 switching protocols?
         // TODO(rfink): What about HEAD requests?
 
-        InputStream inputStream = response.body();
+        String body;
         try {
-            String body;
-            try {
-                body = toString(inputStream);
-            } catch (NullPointerException | IOException e) {
-                UnknownRemoteException exception = new UnknownRemoteException(response.code(), "<unparseable>");
-                exception.initCause(e);
-                throw exception;
-            }
-
-            Optional<String> contentType = response.getFirstHeader(HttpHeaders.CONTENT_TYPE);
-            if (contentType.isPresent() && Encodings.matchesContentType("application/json", contentType.get())) {
-                try {
-                    SerializableError serializableError = MAPPER.readValue(body, SerializableError.class);
-                    return new RemoteException(serializableError, response.code());
-                } catch (Exception e) {
-                    throw new UnknownRemoteException(response.code(), body);
-                }
-            }
+            body = toString(response.body());
+        } catch (NullPointerException | IOException e) {
+            UnknownRemoteException exception = new UnknownRemoteException(response.code(), "<unparseable>");
+            exception.initCause(e);
+            throw exception;
+        }
 
-            throw new UnknownRemoteException(response.code(), body);
-        } finally {
-            try {
-                inputStream.close();
-            } catch (RuntimeException | IOException e) {
-                log.warn("Failed to close InputStream", e);
-            }
+        Optional<String> contentType = response.getFirstHeader(HttpHeaders.CONTENT_TYPE);
+        if (contentType.isPresent() && Encodings.matchesContentType("application/json", contentType.get())) {
             try {
-                response.close();
-            } catch (RuntimeException e) {
-                log.warn("Failed to close Response", e);
+                SerializableError serializableError = MAPPER.readValue(body, SerializableError.class);
+                return new RemoteException(serializableError, response.code());
+            } catch (Exception e) {
+                throw new UnknownRemoteException(response.code(), body);
             }
         }
+
+        throw new UnknownRemoteException(response.code(), body);
     }
 
     private static String toString(InputStream body) throws IOException {

diff --git a/dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/ErrorDecoderTest.java b/dialogue-serde/src/test/java/com/palantir/conjure/java/dialogue/serde/ErrorDecoderTest.java
@@ -33,7 +33,6 @@
 import com.palantir.conjure.java.api.errors.UnknownRemoteException;
 import com.palantir.conjure.java.serialization.ObjectMappers;
 import com.palantir.dialogue.Response;
-import com.palantir.dialogue.TestResponse;
 import com.palantir.logsafe.Preconditions;
 import com.palantir.logsafe.SafeArg;
 import java.io.ByteArrayInputStream;
@@ -147,18 +146,6 @@ public void handlesJsonWithEncoding() {
         assertThat(exception.getError().errorName()).isEqualTo(ErrorType.FAILED_PRECONDITION.name());
     }
 
-    @Test
-    public void closes_response_and_inputstream() {
-        TestResponse testResponse = new TestResponse().contentType("application/json");
-        assertThatThrownBy(() -> decoder.decode(testResponse)).isInstanceOf(UnknownRemoteException.class);
-        assertThat(testResponse.body().isClosed())
-                .describedAs("Expected inputstream to be closed")
-                .isTrue();
-        assertThat(testResponse.isClosed())
-                .describedAs("Body should probably be closed too")
-                .isTrue();
-    }
-
     private static RemoteException encodeAndDecode(Exception exception) {
         Preconditions.checkArgument(!(exception instanceof ServiceException), "Use SerializableError#forException");
         Object error = SerializableError.builder()