Don't close generator / parser in marshaller. (#543)

* Don't close generator / parser in marshaller. * Restore protobuf configuration
curioswitch · May 1, 2020 · b91c726 · b91c726
1 parent 911bed4
commit b91c726
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 18 deletions.
diff --git a/common/grpc/protobuf-jackson/build.gradle.kts b/common/grpc/protobuf-jackson/build.gradle.kts
@@ -45,7 +45,8 @@ jmh {
     iterations = 10
     fork = 5
     profilers = listOf()
-    setIncludeTests(true)
+    isIncludeTests = true
+    isZip64 = true
 }
 
 protobuf {

diff --git a/common/grpc/protobuf-jackson/gradle.properties b/common/grpc/protobuf-jackson/gradle.properties
@@ -22,4 +22,4 @@
 # SOFTWARE.
 #
 
-version = 1.0.0
+version = 1.1.0
diff --git a/...rotobuf-jackson/src/main/java/org/curioswitch/common/protobuf/json/MessageMarshaller.java b/...rotobuf-jackson/src/main/java/org/curioswitch/common/protobuf/json/MessageMarshaller.java
@@ -156,8 +156,9 @@ private MessageMarshaller(@Nullable PrettyPrinter prettyPrinter, MarshallerRegis
   public void mergeValue(byte[] json, Message.Builder builder) throws IOException {
     checkNotNull(json, "json");
     checkNotNull(builder, "builder");
-    JsonParser parser = jsonFactory.createParser(json);
-    mergeValue(parser, builder);
+    try (JsonParser parser = jsonFactory.createParser(json)) {
+      mergeValue(parser, builder);
+    }
   }
 
   /**
@@ -169,8 +170,9 @@ public void mergeValue(byte[] json, Message.Builder builder) throws IOException
   public void mergeValue(String json, Message.Builder builder) throws IOException {
     checkNotNull(json, "json");
     checkNotNull(builder, "builder");
-    JsonParser parser = jsonFactory.createParser(json);
-    mergeValue(parser, builder);
+    try (JsonParser parser = jsonFactory.createParser(json)) {
+      mergeValue(parser, builder);
+    }
   }
 
   /**
@@ -183,8 +185,9 @@ public void mergeValue(String json, Message.Builder builder) throws IOException
   public void mergeValue(InputStream json, Message.Builder builder) throws IOException {
     checkNotNull(json, "json");
     checkNotNull(builder, "builder");
-    JsonParser parser = jsonFactory.createParser(json);
-    mergeValue(parser, builder);
+    try (JsonParser parser = jsonFactory.createParser(json)) {
+      mergeValue(parser, builder);
+    }
   }
 
   /**
@@ -204,8 +207,6 @@ public void mergeValue(JsonParser jsonParser, Message.Builder builder) throws IO
       throw e;
     } catch (IOException e) {
       throw new InvalidProtocolBufferException(e);
-    } finally {
-      jsonParser.close();
     }
   }
 
@@ -217,8 +218,9 @@ public void mergeValue(JsonParser jsonParser, Message.Builder builder) throws IO
   public <T extends Message> byte[] writeValueAsBytes(T message) throws IOException {
     checkNotNull(message, "message");
     ByteArrayBuilder builder = new ByteArrayBuilder(jsonFactory._getBufferRecycler());
-    JsonGenerator gen = jsonFactory.createGenerator(builder);
-    writeValue(message, gen);
+    try (JsonGenerator gen = jsonFactory.createGenerator(builder)) {
+      writeValue(message, gen);
+    }
     return builder.toByteArray();
   }
 
@@ -230,8 +232,9 @@ public <T extends Message> byte[] writeValueAsBytes(T message) throws IOExceptio
   public <T extends Message> String writeValueAsString(T message) throws IOException {
     checkNotNull(message, "message");
     SegmentedStringWriter sw = new SegmentedStringWriter(jsonFactory._getBufferRecycler());
-    JsonGenerator gen = jsonFactory.createGenerator(sw);
-    writeValue(message, gen);
+    try (JsonGenerator gen = jsonFactory.createGenerator(sw)) {
+      writeValue(message, gen);
+    }
     return sw.getAndClear();
   }
 
@@ -242,8 +245,9 @@ public <T extends Message> String writeValueAsString(T message) throws IOExcepti
   public <T extends Message> void writeValue(T message, OutputStream out) throws IOException {
     checkNotNull(message, "message");
     checkNotNull(out, "out");
-    JsonGenerator gen = jsonFactory.createGenerator(out);
-    writeValue(message, gen);
+    try (JsonGenerator gen = jsonFactory.createGenerator(out)) {
+      writeValue(message, gen);
+    }
   }
 
   /**
@@ -267,8 +271,6 @@ public <T extends Message> void writeValue(T message, JsonGenerator gen) throws
       throw e;
     } catch (IOException e) {
       throw new InvalidProtocolBufferException(e);
-    } finally {
-      gen.close();
     }
   }
 

diff --git a/...buf-jackson/src/test/java/org/curioswitch/common/protobuf/json/MessageMarshallerTest.java b/...buf-jackson/src/test/java/org/curioswitch/common/protobuf/json/MessageMarshallerTest.java
@@ -28,6 +28,9 @@
 import static org.assertj.core.api.Assertions.entry;
 import static org.junit.Assert.assertEquals;
 
+import com.fasterxml.jackson.core.JsonGenerator;
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.base.Strings;
 import com.google.protobuf.Any;
 import com.google.protobuf.BoolValue;
@@ -933,6 +936,25 @@ public void fieldsOutOfOrder() throws Exception {
     assertMatchesUpstream(TestFieldOrder.newBuilder().setValue1("foo").setValue2("bar").build());
   }
 
+  @Test
+  public void doesNotCloseJsonGenerator() throws Exception {
+    JsonGenerator generator =
+        new ObjectMapper().getFactory().createGenerator(new ByteArrayOutputStream());
+    MessageMarshaller marshaller =
+        MessageMarshaller.builder().register(TestAllTypes.getDefaultInstance()).build();
+    marshaller.writeValue(TestAllTypes.getDefaultInstance(), generator);
+    assertThat(generator.isClosed()).isFalse();
+  }
+
+  @Test
+  public void doesNotCloseJsonParser() throws Exception {
+    JsonParser parser = new ObjectMapper().getFactory().createParser("{}");
+    MessageMarshaller marshaller =
+        MessageMarshaller.builder().register(TestAllTypes.getDefaultInstance()).build();
+    marshaller.mergeValue(parser, TestAllTypes.newBuilder());
+    assertThat(parser.isClosed()).isFalse();
+  }
+
   private static String recursiveJson(int numRecursions) {
     StringBuilder input = new StringBuilder("{\n");
     for (int i = 0; i < numRecursions; i++) {