Merge pull request #2394 from cwelton/formatting

oneOf fix for JsonFormat includingDefaultValueFields

2 files changed, 30 insertions, 6 deletions

diff --git a/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java b/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java
index 7f6c8aea..ac712c94 100644
--- a/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java
+++ b/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java
@@ -49,6 +49,7 @@ import com.google.protobuf.Descriptors.EnumDescriptor;
 import com.google.protobuf.Descriptors.EnumValueDescriptor;
 import com.google.protobuf.Descriptors.FieldDescriptor;
 import com.google.protobuf.Descriptors.FileDescriptor;
+import com.google.protobuf.Descriptors.OneofDescriptor;
 import com.google.protobuf.DoubleValue;
 import com.google.protobuf.Duration;
 import com.google.protobuf.DynamicMessage;
@@ -782,12 +783,18 @@ public class JsonFormat {
       if (includingDefaultValueFields) {
         fieldsToPrint = new TreeMap<FieldDescriptor, Object>();
         for (FieldDescriptor field : message.getDescriptorForType().getFields()) {
-          if (field.isOptional()
-              && field.getJavaType() == FieldDescriptor.JavaType.MESSAGE
-              && !message.hasField(field)) {
-            // Always skip empty optional message fields. If not we will recurse indefinitely if
-            // a message has itself as a sub-field.
-            continue;
+          if (field.isOptional()) {
+            if (field.getJavaType() == FieldDescriptor.JavaType.MESSAGE
+                && !message.hasField(field)){
+              // Always skip empty optional message fields. If not we will recurse indefinitely if
+              // a message has itself as a sub-field.
+              continue;
+            }
+            OneofDescriptor oneof = field.getContainingOneof();
+            if (oneof != null && !message.hasField(field)) {
+                // Skip all oneof fields except the one that is actually set
+              continue;
+            }
           }
           fieldsToPrint.put(field, message.getField(field));
         }
diff --git a/java/util/src/test/java/com/google/protobuf/util/JsonFormatTest.java b/java/util/src/test/java/com/google/protobuf/util/JsonFormatTest.java
index 164ee54b..dafd9bb5 100644
--- a/java/util/src/test/java/com/google/protobuf/util/JsonFormatTest.java
+++ b/java/util/src/test/java/com/google/protobuf/util/JsonFormatTest.java
@@ -1267,6 +1267,23 @@ public class JsonFormatTest extends TestCase {
             + "  }\n"
             + "}",
         JsonFormat.printer().includingDefaultValueFields().print(mapMessage));
+
+    TestOneof oneofMessage = TestOneof.getDefaultInstance();
+    assertEquals("{\n}", JsonFormat.printer().print(oneofMessage));
+    assertEquals("{\n}", JsonFormat.printer().includingDefaultValueFields().print(oneofMessage));
+
+    oneofMessage = TestOneof.newBuilder().setOneofInt32(42).build();
+    assertEquals("{\n  \"oneofInt32\": 42\n}",
+        JsonFormat.printer().print(oneofMessage));
+    assertEquals("{\n  \"oneofInt32\": 42\n}",
+        JsonFormat.printer().includingDefaultValueFields().print(oneofMessage));
+
+    TestOneof.Builder oneofBuilder = TestOneof.newBuilder();
+    mergeFromJson("{\n" + "  \"oneofNullValue\": null \n" + "}", oneofBuilder);
+    oneofMessage = oneofBuilder.build();
+    assertEquals("{\n  \"oneofNullValue\": null\n}", JsonFormat.printer().print(oneofMessage));
+    assertEquals("{\n  \"oneofNullValue\": null\n}",
+        JsonFormat.printer().includingDefaultValueFields().print(oneofMessage));
   }
 
   public void testPreservingProtoFieldNames() throws Exception {