Support absent protobuf arrays in ProtobufJsonFormat

Protobuf does not support concept of absent arrays (repeated field).
It does not distinguish empty arrays from absent arrays.
ProtobufJsonFormat introduces a naming convention to mark an
empty array field as absent. In this case the field is not output
to JSON.

sonar-core/src/main/java/org/sonar/core/util/ProtobufJsonFormat.java

@@ -19,15 +19,54 @@
  */
 package org.sonar.core.util;
 
+import com.google.common.base.Preconditions;
+import com.google.common.collect.BiMap;
+import com.google.common.collect.HashBiMap;
 import com.google.protobuf.Descriptors;
 import com.google.protobuf.Message;
+import java.io.StringWriter;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import org.sonar.api.utils.text.JsonWriter;
 
 /**
  * Converts a Protocol Buffers message to JSON. Unknown fields, binary fields, (deprecated) groups
- * and maps are not supported.
+ * and maps are not supported. Absent fields are ignored, so it's possible to distinguish
+ * null strings (field is absent) and empty strings (field is present with value {@code ""}).
+ * <p/>
+ * <h2>Empty Arrays</h2>
+ * Protobuf does not make the difference between absent arrays and empty arrays (size is zero).
+ * The consequence is that arrays are always output in JSON. Empty arrays are converted to {@code []}.
+ * <p/>
+ * A workaround is implemented in {@link ProtobufJsonFormat} to not generate absent arrays into JSON document.
+ * A boolean field is used to declare if the related repeated field (the array) is present or not. The
+ * name of the boolean field must be the array field name suffixed with "PresentIfEmpty". This field is "for internal
+ * use" and is not generated into JSON document. It is ignored when the array is not empty.
+ *
+ * For example:
+ * <pre>
+ *   // proto specification
+ *   message Response {
+ *     optional bool issuesPresentIfEmpty = 1;
+ *     repeated Issue issues = 2;
+ *   }
+ * </pre>
+ * <pre>
+ *   // Java usage
+ *
+ *   Response.newBuilder().build();
+ *   // output: {}
+ *
+ *   Response.newBuilder().setIssuesPresentIfEmpty(true).build();
+ *   // output: {"issues": []}
+ *
+ *   // no need to set the flag to true when the array is not empty
+ *   Response.newBuilder().setIssues(atLeastOneIssues).build();
+ *   // output: {"issues": [{...}, {...}]}
+ * </pre>
  */
 public class ProtobufJsonFormat {
 
@@ -35,33 +74,183 @@ public class ProtobufJsonFormat {
     // only statics
   }
 
+  private static abstract class MessageField {
+    protected final Descriptors.FieldDescriptor descriptor;
+
+    public MessageField(Descriptors.FieldDescriptor descriptor) {
+      this.descriptor = descriptor;
+    }
+
+    public String getName() {
+      return descriptor.getName();
+    }
+
+    public Descriptors.FieldDescriptor.JavaType getJavaType() {
+      return descriptor.getJavaType();
+    }
+
+    public abstract boolean isRepeated();
+
+    public abstract boolean hasValue(Message message);
+
+    public abstract Object getValue(Message message);
+  }
+
+  private static class MessageNonRepeatedField extends MessageField {
+    public MessageNonRepeatedField(Descriptors.FieldDescriptor descriptor) {
+      super(descriptor);
+      Preconditions.checkArgument(!descriptor.isRepeated());
+    }
+
+    @Override
+    public boolean isRepeated() {
+      return false;
+    }
+
+    @Override
+    public boolean hasValue(Message message) {
+      return message.hasField(descriptor);
+    }
+
+    @Override
+    public Object getValue(Message message) {
+      return message.getField(descriptor);
+    }
+  }
+
+  private static class MessageRepeatedField extends MessageField {
+    public MessageRepeatedField(Descriptors.FieldDescriptor descriptor) {
+      super(descriptor);
+      Preconditions.checkArgument(descriptor.isRepeated());
+    }
+
+    @Override
+    public boolean isRepeated() {
+      return true;
+    }
+
+    @Override
+    public boolean hasValue(Message message) {
+      return true;
+    }
+
+    @Override
+    public Object getValue(Message message) {
+      return message.getField(descriptor);
+    }
+  }
+
+  private static class MessageNullableRepeatedField extends MessageField {
+    private final Descriptors.FieldDescriptor booleanDesc;
+
+    public MessageNullableRepeatedField(Descriptors.FieldDescriptor booleanDesc, Descriptors.FieldDescriptor arrayDescriptor) {
+      super(arrayDescriptor);
+      Preconditions.checkArgument(arrayDescriptor.isRepeated());
+      Preconditions.checkArgument(booleanDesc.getJavaType() == Descriptors.FieldDescriptor.JavaType.BOOLEAN);
+      this.booleanDesc = booleanDesc;
+    }
+
+    @Override
+    public boolean isRepeated() {
+      return true;
+    }
+
+    @Override
+    public boolean hasValue(Message message) {
+      if (((Collection) message.getField(descriptor)).isEmpty()) {
+        return message.hasField(booleanDesc) && (boolean) message.getField(booleanDesc);
+      }
+      return true;
+    }
+
+    @Override
+    public Object getValue(Message message) {
+      return message.getField(descriptor);
+    }
+  }
+
+  static class MessageJsonDescriptor {
+    private static final Map<Class<? extends Message>, MessageJsonDescriptor> BY_CLASS = new HashMap<>();
+    private final MessageField[] fields;
+
+    private MessageJsonDescriptor(MessageField[] fields) {
+      this.fields = fields;
+    }
+
+    MessageField[] getFields() {
+      return fields;
+    }
+
+    static MessageJsonDescriptor of(Message message) {
+      MessageJsonDescriptor desc = BY_CLASS.get(message.getClass());
+      if (desc == null) {
+        desc = introspect(message);
+        BY_CLASS.put(message.getClass(), desc);
+      }
+      return desc;
+    }
+
+    private static MessageJsonDescriptor introspect(Message message) {
+      List<MessageField> fields = new ArrayList<>();
+      BiMap<Descriptors.FieldDescriptor, Descriptors.FieldDescriptor> repeatedToBoolean = HashBiMap.create();
+      for (Descriptors.FieldDescriptor desc : message.getDescriptorForType().getFields()) {
+        if (desc.isRepeated()) {
+          String booleanName = desc.getName() + "PresentIfEmpty";
+          Descriptors.FieldDescriptor booleanDesc = message.getDescriptorForType().findFieldByName(booleanName);
+          if (booleanDesc != null && booleanDesc.getJavaType() == Descriptors.FieldDescriptor.JavaType.BOOLEAN) {
+            repeatedToBoolean.put(desc, booleanDesc);
+          }
+        }
+      }
+      for (Descriptors.FieldDescriptor descriptor : message.getDescriptorForType().getFields()) {
+        if (descriptor.isRepeated()) {
+          Descriptors.FieldDescriptor booleanDesc = repeatedToBoolean.get(descriptor);
+          if (booleanDesc == null) {
+            fields.add(new MessageRepeatedField(descriptor));
+          } else {
+            fields.add(new MessageNullableRepeatedField(booleanDesc, descriptor));
+          }
+        } else if (!repeatedToBoolean.containsValue(descriptor)) {
+          fields.add(new MessageNonRepeatedField(descriptor));
+        }
+      }
+      return new MessageJsonDescriptor(fields.toArray(new MessageField[fields.size()]));
+    }
+  }
+
   public static void write(Message message, JsonWriter writer) {
+    writer.setSerializeNulls(false).setSerializeEmptys(true);
     writer.beginObject();
     writeMessage(message, writer);
     writer.endObject();
   }
 
-  private static void writeMessage(Message message, JsonWriter writer) {
-    for (Map.Entry<Descriptors.FieldDescriptor, Object> entry : message.getAllFields().entrySet()) {
-      writeField(entry.getKey(), entry.getValue(), writer);
-    }
+  public static String toJson(Message message) {
+    StringWriter json = new StringWriter();
+    JsonWriter jsonWriter = JsonWriter.of(json);
+    write(message, jsonWriter);
+    return json.toString();
   }
 
-  private static void writeField(Descriptors.FieldDescriptor field, Object value, JsonWriter writer) {
-    writer.name(field.getName());
-    if (field.isRepeated()) {
-      // Repeated field. Print each element.
-      writer.beginArray();
-      for (Object o : (Collection) value) {
-        writeFieldValue(field, o, writer);
+  private static void writeMessage(Message message, JsonWriter writer) {
+    MessageJsonDescriptor fields = MessageJsonDescriptor.of(message);
+    for (MessageField field : fields.getFields()) {
+      if (field.hasValue(message)) {
+        writer.name(field.getName());
+        if (field.isRepeated()) {
+          writer.beginArray();
+          for (Object o : (Collection) field.getValue(message)) {
+            writeFieldValue(field, o, writer);
+          }
+          writer.endArray();
+        } else {
+          writeFieldValue(field, field.getValue(message), writer);
+        }
       }
-      writer.endArray();
-    } else {
-      writeFieldValue(field, value, writer);
     }
   }
 
-  private static void writeFieldValue(Descriptors.FieldDescriptor field, Object value, JsonWriter writer) {
+  private static void writeFieldValue(MessageField field, Object value, JsonWriter writer) {
     switch (field.getJavaType()) {
       case INT:
         writer.value((Integer) value);
@@ -78,8 +267,6 @@ public class ProtobufJsonFormat {
       case STRING:
         writer.value((String) value);
         break;
-      case BYTE_STRING:
-        throw new IllegalStateException(String.format("JSON format does not support the binary field '%s'", field.getName()));
       case ENUM:
         writer.value(((Descriptors.EnumValueDescriptor) value).getName());
         break;
@@ -89,6 +276,7 @@ public class ProtobufJsonFormat {
         writer.endObject();
         break;
       default:
+        throw new IllegalStateException(String.format("JSON format does not support type '%s' of field '%s'", field.getJavaType(), field.getName()));
     }
   }
 }

sonar-core/src/test/java/org/sonar/core/util/ProtobufJsonFormatTest.java

@@ -25,9 +25,12 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.sonar.api.utils.text.JsonWriter;
+import org.sonar.core.test.Test.JsonArrayTest;
+import org.sonar.core.test.Test.JsonTest;
 
 import static java.util.Arrays.asList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.sonar.core.util.ProtobufJsonFormat.toJson;
 
 public class ProtobufJsonFormatTest {
 
@@ -35,49 +38,100 @@ public class ProtobufJsonFormatTest {
   public ExpectedException expectedException = ExpectedException.none();
 
   @Test
-  public void convert_protobuf_to_json() throws Exception {
-    org.sonar.core.test.Test.Fake protobuf = org.sonar.core.test.Test.Fake.newBuilder()
-      .setAString("foo")
-      .setAnInt(10)
-      .setALong(100L)
-      .setABoolean(true)
-      .setADouble(3.14)
-      .setAnEnum(org.sonar.core.test.Test.FakeEnum.GREEN)
+  public void convert_protobuf_to_json() {
+    JsonTest protobuf = JsonTest.newBuilder()
+      .setStringField("foo")
+      .setIntField(10)
+      .setLongField(100L)
+      .setDoubleField(3.14)
+      .setBooleanField(true)
+      .setEnumField(org.sonar.core.test.Test.FakeEnum.GREEN)
       .addAllAnArray(asList("one", "two", "three"))
-      .setANestedMessage(org.sonar.core.test.Test.NestedFake.newBuilder().setLabel("bar").build())
+      .setNested(org.sonar.core.test.Test.NestedJsonTest.newBuilder().setLabel("bar").build())
       .build();
 
-    StringWriter json = new StringWriter();
-    JsonWriter jsonWriter = JsonWriter.of(json);
-    ProtobufJsonFormat.write(protobuf, jsonWriter);
-
-    assertThat(json.toString())
-      .isEqualTo("{\"aString\":\"foo\",\"anInt\":10,\"aLong\":100,\"aDouble\":3.14,\"aBoolean\":true,\"anEnum\":\"GREEN\",\"anArray\":[\"one\",\"two\",\"three\"],\"aNestedMessage\":{\"label\":\"bar\"}}");
+    assertThat(toJson(protobuf))
+      .isEqualTo(
+        "{\"stringField\":\"foo\",\"intField\":10,\"longField\":100,\"doubleField\":3.14,\"booleanField\":true,\"enumField\":\"GREEN\",\"nested\":{\"label\":\"bar\"},\"anArray\":[\"one\",\"two\",\"three\"]}");
   }
 
   @Test
-  public void protobuf_bytes_field_can_not_be_converted_to_json() throws Exception {
+  public void protobuf_bytes_field_can_not_be_converted_to_json() {
     expectedException.expect(RuntimeException.class);
-    expectedException.expectMessage("JSON format does not support the binary field 'someBytes'");
+    expectedException.expectMessage("JSON format does not support type 'BYTE_STRING' of field 'bytesField'");
 
-    org.sonar.core.test.Test.Fake protobuf = org.sonar.core.test.Test.Fake.newBuilder()
-      .setSomeBytes(ByteString.copyFrom(new byte[]{2, 4}))
+    JsonTest protobuf = JsonTest.newBuilder()
+      .setBytesField(ByteString.copyFrom(new byte[]{2, 4}))
       .build();
 
     ProtobufJsonFormat.write(protobuf, JsonWriter.of(new StringWriter()));
   }
 
   @Test
-  public void protobuf_empty_strings_are_not_output() throws Exception {
-    org.sonar.core.test.Test.Fake protobuf = org.sonar.core.test.Test.Fake.newBuilder().build();
+  public void protobuf_absent_fields_are_not_output() {
+    JsonTest msg = JsonTest.newBuilder().build();
+
+    // fields are absent
+    assertThat(msg.hasStringField()).isFalse();
+    assertThat(msg.hasIntField()).isFalse();
+
+    // the repeated field "anArray" is always present. This is the standard behavior of protobuf. It
+    // does not make the difference between null and empty arrays.
+    assertThat(toJson(msg)).isEqualTo("{\"anArray\":[]}");
+  }
+
+  @Test
+  public void protobuf_present_and_empty_string_field_is_output() {
+    JsonTest msg = JsonTest.newBuilder().setStringField("").build();
+
+    // field is present
+    assertThat(msg.hasStringField()).isTrue();
+    assertThat(msg.getStringField()).isEqualTo("");
+
+    assertThat(toJson(msg)).contains("\"stringField\":\"\"");
+  }
+
+
+  @Test
+  public void protobuf_empty_array_marked_as_present_is_output() {
+    JsonArrayTest msg = JsonArrayTest.newBuilder()
+      .setANullableArrayPresentIfEmpty(true)
+      .build();
+
+    // repeated field "aNullableArray" is marked as present through the boolean field "aNullableArrayPresentIfEmpty"
+    assertThat(msg.hasANullableArrayPresentIfEmpty()).isTrue();
+    assertThat(msg.getANullableArrayPresentIfEmpty()).isTrue();
+
+    // JSON contains the repeated field, but not the boolean marker field
+    assertThat(toJson(msg)).isEqualTo("{\"aNullableArray\":[]}");
+  }
+
+  @Test
+  public void protobuf_empty_array_marked_as_absent_is_not_output() {
+    JsonArrayTest msg = JsonArrayTest.newBuilder()
+      .setANullableArrayPresentIfEmpty(false)
+      .build();
+
+    // repeated field "aNullableArray" is marked as absent through the boolean field "aNullableArrayPresentIfEmpty"
+    assertThat(msg.hasANullableArrayPresentIfEmpty()).isTrue();
+    assertThat(msg.getANullableArrayPresentIfEmpty()).isFalse();
+
+    // JSON does not contain the array nor the boolean marker
+    assertThat(toJson(msg)).isEqualTo("{}");
+  }
+
+  @Test
+  public void protobuf_non_empty_array_is_output_even_if_not_marked_as_present() {
+    JsonArrayTest msg = JsonArrayTest.newBuilder()
+      .addANullableArray("foo")
+      .build();
 
-    // field is not set but value is "", not null
-    assertThat(protobuf.hasAString()).isFalse();
-    assertThat(protobuf.getAString()).isEqualTo("");
+    // repeated field "aNullableArray" is present, but the boolean marker "aNullableArrayPresentIfEmpty"
+    // is not set.
+    assertThat(msg.hasANullableArrayPresentIfEmpty()).isFalse();
+    assertThat(msg.getANullableArrayPresentIfEmpty()).isFalse();
 
-    StringWriter json = new StringWriter();
-    JsonWriter jsonWriter = JsonWriter.of(json);
-    ProtobufJsonFormat.write(protobuf, jsonWriter);
-    assertThat(json.toString()).isEqualTo("{}");
+    // JSON contains the array but not the boolean marker
+    assertThat(toJson(msg)).isEqualTo("{\"aNullableArray\":[\"foo\"]}");
   }
 }

sonar-core/src/test/java/org/sonar/core/util/ProtobufTest.java

@@ -84,17 +84,17 @@ public class ProtobufTest {
   public void write_and_read_streams() throws Exception {
     File file = temp.newFile();
 
-    Fake item1 = Fake.newBuilder().setAString("one").setAnInt(1).build();
-    Fake item2 = Fake.newBuilder().setAString("two").build();
+    Fake item1 = Fake.newBuilder().setLabel("one").setLine(1).build();
+    Fake item2 = Fake.newBuilder().setLabel("two").build();
     Protobuf.writeStream(asList(item1, item2), file, false);
 
     CloseableIterator<Fake> it = Protobuf.readStream(file, Fake.PARSER);
     Fake read = it.next();
-    assertThat(read.getAString()).isEqualTo("one");
-    assertThat(read.getAnInt()).isEqualTo(1);
+    assertThat(read.getLabel()).isEqualTo("one");
+    assertThat(read.getLine()).isEqualTo(1);
     read = it.next();
-    assertThat(read.getAString()).isEqualTo("two");
-    assertThat(read.hasAnInt()).isFalse();
+    assertThat(read.getLabel()).isEqualTo("two");
+    assertThat(read.hasLine()).isFalse();
     assertThat(it.hasNext()).isFalse();
   }
 

sonar-core/src/test/protobuf/test.proto

@@ -24,15 +24,8 @@ option java_package = "org.sonar.core.test";
 option optimize_for = SPEED;
 
 message Fake {
-    optional string aString = 1;
-    optional int32 anInt = 2;
-    optional int64 aLong = 3;
-    optional double aDouble = 4;
-    optional bool aBoolean = 5;
-    optional FakeEnum anEnum = 6;
-    optional bytes someBytes = 7;
-    repeated string anArray = 8;
-    optional NestedFake aNestedMessage = 9;
+  optional string label = 1;
+  optional int32 line = 2;
 }
 
 enum FakeEnum {
@@ -41,6 +34,25 @@ enum FakeEnum {
   GREEN = 2;
 }
 
-message NestedFake {
+message JsonTest {
+  optional string stringField = 1;
+  optional int32 intField = 2;
+  optional int64 longField = 3;
+  optional double doubleField = 4;
+  optional bool booleanField = 5;
+  optional FakeEnum enumField = 6;
+  optional bytes bytesField = 7;
+  optional NestedJsonTest nested = 8;
+  repeated string anArray = 9;
+}
+
+message JsonArrayTest {
+// naming convention. A boolean field is used
+  // to know if the array field is present.
+  optional bool aNullableArrayPresentIfEmpty = 1;
+  repeated string aNullableArray = 2;
+}
+
+message NestedJsonTest {
   optional string label = 1;
 }