Fixes to JsonFormatter

- Handle oneof properly
- Omit unknown enum values

1 files changed, 66 insertions, 9 deletions

diff --git a/csharp/src/ProtocolBuffers/JsonFormatter.cs b/csharp/src/ProtocolBuffers/JsonFormatter.cs
index a6aa552f..3e2244d6 100644
--- a/csharp/src/ProtocolBuffers/JsonFormatter.cs
+++ b/csharp/src/ProtocolBuffers/JsonFormatter.cs
@@ -136,13 +136,28 @@ namespace Google.Protobuf
             builder.Append("{ ");
             var fields = message.Fields;
             bool first = true;
+            // First non-oneof fields
             foreach (var accessor in fields.Accessors)
             {
+                var descriptor = accessor.Descriptor;
+                // Oneofs are written later
+                if (descriptor.ContainingOneof != null)
+                {
+                    continue;
+                }
+                // Omit default values unless we're asked to format them
                 object value = accessor.GetValue(message);
                 if (!settings.FormatDefaultValues && IsDefaultValue(accessor, value))
                 {
                     continue;
                 }
+                // Omit awkward (single) values such as unknown enum values
+                if (!descriptor.IsRepeated && !descriptor.IsMap && !CanWriteSingleValue(accessor.Descriptor, value))
+                {
+                    continue;
+                }
+
+                // Okay, all tests complete: let's write the field value...
                 if (!first)
                 {
                     builder.Append(", ");
@@ -152,6 +167,32 @@ namespace Google.Protobuf
                 WriteValue(builder, accessor, value);
                 first = false;
             }
+
+            // Now oneofs
+            foreach (var accessor in fields.Oneofs)
+            {
+                var fieldDescriptor = accessor.GetCaseFieldDescriptor(message);
+                if (fieldDescriptor == null)
+                {
+                    continue;
+                }
+                var fieldAccessor = fields[fieldDescriptor];
+                object value = fieldAccessor.GetValue(message);
+                // Omit awkward (single) values such as unknown enum values
+                if (!fieldDescriptor.IsRepeated && !fieldDescriptor.IsMap && !CanWriteSingleValue(fieldDescriptor, value))
+                {
+                    continue;
+                }
+
+                if (!first)
+                {
+                    builder.Append(", ");
+                }
+                WriteString(builder, ToCamelCase(fieldDescriptor.Name));
+                builder.Append(": ");
+                WriteValue(builder, fieldAccessor, value);
+                first = false;
+            }
             builder.Append(first ? "}" : " }");
         }
 
@@ -303,15 +344,8 @@ namespace Google.Protobuf
                     }
                 case FieldType.Enum:
                     EnumValueDescriptor enumValue = descriptor.EnumType.FindValueByNumber((int) value);
-                    if (enumValue != null)
-                    {
-                        WriteString(builder, enumValue.Name);
-                    }
-                    else
-                    {
-                        // ??? Need more documentation
-                        builder.Append(((int) value).ToString("d", CultureInfo.InvariantCulture));
-                    }
+                    // We will already have validated that this is a known value.
+                    WriteString(builder, enumValue.Name);
                     break;
                 case FieldType.Fixed64:
                 case FieldType.UInt64:
@@ -354,6 +388,10 @@ namespace Google.Protobuf
             bool first = true;
             foreach (var value in list)
             {
+                if (!CanWriteSingleValue(accessor.Descriptor, value))
+                {
+                    continue;
+                }
                 if (!first)
                 {
                     builder.Append(", ");
@@ -373,6 +411,10 @@ namespace Google.Protobuf
             // This will box each pair. Could use IDictionaryEnumerator, but that's ugly in terms of disposal.
             foreach (DictionaryEntry pair in dictionary)
             {
+                if (!CanWriteSingleValue(valueType, pair.Value))
+                {
+                    continue;
+                }
                 if (!first)
                 {
                     builder.Append(", ");
@@ -410,6 +452,21 @@ namespace Google.Protobuf
         }
 
         /// <summary>
+        /// Returns whether or not a singular value can be represented in JSON.
+        /// Currently only relevant for enums, where unknown values can't be represented.
+        /// For repeated/map fields, this always returns true.
+        /// </summary>
+        private bool CanWriteSingleValue(FieldDescriptor descriptor, object value)
+        {
+            if (descriptor.FieldType == FieldType.Enum)
+            {
+                EnumValueDescriptor enumValue = descriptor.EnumType.FindValueByNumber((int) value);
+                return enumValue != null;
+            }
+            return true;
+        }
+
+        /// <summary>
         /// Writes a string (including leading and trailing double quotes) to a builder, escaping as required.
         /// </summary>
         /// <remarks>