Merge "Update MessageNano#toString() to return mostly valid TextFormat."

3 files changed, 84 insertions, 40 deletions

diff --git a/java/src/main/java/com/google/protobuf/nano/MessageNano.java b/java/src/main/java/com/google/protobuf/nano/MessageNano.java
index 7eff153..0cf8416 100644
--- a/java/src/main/java/com/google/protobuf/nano/MessageNano.java
+++ b/java/src/main/java/com/google/protobuf/nano/MessageNano.java
@@ -139,7 +139,10 @@ public abstract class MessageNano {
     }
 
     /**
-     * Intended for debugging purposes only. It does not use ASCII protobuf formatting.
+     * Returns a string that is (mostly) compatible with ProtoBuffer's TextFormat. Note that groups
+     * (which are deprecated) are not serialized with the correct field name.
+     *
+     * <p>This is implemented using reflection, so it is not especially fast.
      */
     @Override
     public String toString() {
diff --git a/java/src/main/java/com/google/protobuf/nano/MessageNanoPrinter.java b/java/src/main/java/com/google/protobuf/nano/MessageNanoPrinter.java
index 3a5ee7c..d135a51 100644
--- a/java/src/main/java/com/google/protobuf/nano/MessageNanoPrinter.java
+++ b/java/src/main/java/com/google/protobuf/nano/MessageNanoPrinter.java
@@ -47,20 +47,22 @@ public final class MessageNanoPrinter {
     private static final int MAX_STRING_LEN = 200;
 
     /**
-     * Returns an text representation of a MessageNano suitable for debugging.
+     * Returns an text representation of a MessageNano suitable for debugging. The returned string
+     * is mostly compatible with Protocol Buffer's TextFormat (as provided by non-nano protocol
+     * buffers) -- groups (which are deprecated) are output with an underscore name (e.g. foo_bar
+     * instead of FooBar) and will thus not parse.
      *
      * <p>Employs Java reflection on the given object and recursively prints primitive fields,
      * groups, and messages.</p>
      */
     public static <T extends MessageNano> String print(T message) {
         if (message == null) {
-            return "null";
+            return "";
         }
 
         StringBuffer buf = new StringBuffer();
         try {
-            print(message.getClass().getSimpleName(), message.getClass(), message,
-                    new StringBuffer(), buf);
+            print(null, message.getClass(), message, new StringBuffer(), buf);
         } catch (IllegalAccessException e) {
             return "Error printing proto: " + e.getMessage();
         }
@@ -70,21 +72,30 @@ public final class MessageNanoPrinter {
     /**
      * Function that will print the given message/class into the StringBuffer.
      * Meant to be called recursively.
+     *
+     * @param identifier the identifier to use, or {@code null} if this is the root message to
+     *        print.
+     * @param clazz the class of {@code message}.
+     * @param message the value to print. May in fact be a primitive value or byte array and not a
+     *        message.
+     * @param indentBuf the indentation each line should begin with.
+     * @param buf the output buffer.
      */
     private static void print(String identifier, Class<?> clazz, Object message,
             StringBuffer indentBuf, StringBuffer buf) throws IllegalAccessException {
-        if (MessageNano.class.isAssignableFrom(clazz)) {
-            // Nano proto message
-            buf.append(indentBuf).append(identifier);
-
-            // If null, just print it and return
-            if (message == null) {
-                buf.append(": ").append(message).append("\n");
-                return;
+        if (message == null) {
+            // This can happen if...
+            //   - we're about to print a message, String, or byte[], but it not present;
+            //   - we're about to print a primitive, but "reftype" optional style is enabled, and
+            //     the field is unset.
+            // In both cases the appropriate behavior is to output nothing.
+        } else if (MessageNano.class.isAssignableFrom(clazz)) {  // Nano proto message
+            int origIndentBufLength = indentBuf.length();
+            if (identifier != null) {
+                buf.append(indentBuf).append(deCamelCaseify(identifier)).append(" <\n");
+                indentBuf.append(INDENT);
             }
 
-            indentBuf.append(INDENT);
-            buf.append(" <\n");
             for (Field field : clazz.getFields()) {
                 // Proto fields are public, non-static variables that do not begin or end with '_'
                 int modifiers = field.getModifiers();
@@ -115,15 +126,19 @@ public final class MessageNanoPrinter {
                     print(fieldName, fieldType, value, indentBuf, buf);
                 }
             }
-            indentBuf.delete(indentBuf.length() - INDENT.length(), indentBuf.length());
-            buf.append(indentBuf).append(">\n");
+            if (identifier != null) {
+                indentBuf.setLength(origIndentBufLength);
+                buf.append(indentBuf).append(">\n");
+            }
         } else {
-            // Primitive value
+            // Non-null primitive value
             identifier = deCamelCaseify(identifier);
             buf.append(indentBuf).append(identifier).append(": ");
             if (message instanceof String) {
                 String stringMessage = sanitizeString((String) message);
                 buf.append("\"").append(stringMessage).append("\"");
+            } else if (message instanceof byte[]) {
+                appendQuotedBytes((byte[]) message, buf);
             } else {
                 buf.append(message);
             }
@@ -176,4 +191,27 @@ public final class MessageNanoPrinter {
         }
         return b.toString();
     }
+
+    /**
+     * Appends a quoted byte array to the provided {@code StringBuffer}.
+     */
+    private static void appendQuotedBytes(byte[] bytes, StringBuffer builder) {
+        if (bytes == null) {
+            builder.append("\"\"");
+            return;
+        }
+
+        builder.append('"');
+        for (int i = 0; i < bytes.length; ++i) {
+            int ch = bytes[i];
+            if (ch == '\\' || ch == '"') {
+                builder.append('\\').append((char) ch);
+            } else if (ch >= 32 && ch < 127) {
+                builder.append((char) ch);
+            } else {
+                builder.append(String.format("\\%03o", ch));
+            }
+        }
+        builder.append('"');
+    }
 }
diff --git a/java/src/test/java/com/google/protobuf/NanoTest.java b/java/src/test/java/com/google/protobuf/NanoTest.java
index 68d2c45..724e741 100644
--- a/java/src/test/java/com/google/protobuf/NanoTest.java
+++ b/java/src/test/java/com/google/protobuf/NanoTest.java
@@ -2490,14 +2490,14 @@ public class NanoTest extends TestCase {
     msg.optionalInt32 = 14;
     msg.optionalFloat = 42.3f;
     msg.optionalString = "String \"with' both quotes";
-    msg.optionalBytes = new byte[5];
+    msg.optionalBytes = new byte[] {'"', '\0', 1, 8};
     msg.optionalGroup = new TestAllTypesNano.OptionalGroup();
     msg.optionalGroup.a = 15;
     msg.repeatedInt64 = new long[2];
     msg.repeatedInt64[0] = 1L;
     msg.repeatedInt64[1] = -1L;
     msg.repeatedBytes = new byte[2][];
-    msg.repeatedBytes[1] = new byte[5];
+    msg.repeatedBytes[1] = new byte[] {'h', 'e', 'l', 'l', 'o'};
     msg.repeatedGroup = new TestAllTypesNano.RepeatedGroup[2];
     msg.repeatedGroup[0] = new TestAllTypesNano.RepeatedGroup();
     msg.repeatedGroup[0].a = -27;
@@ -2514,28 +2514,31 @@ public class NanoTest extends TestCase {
     msg.repeatedNestedEnum = new int[2];
     msg.repeatedNestedEnum[0] = TestAllTypesNano.BAR;
     msg.repeatedNestedEnum[1] = TestAllTypesNano.FOO;
+    msg.repeatedStringPiece = new String[] {null, "world"};
 
     String protoPrint = msg.toString();
-    assertTrue(protoPrint.contains("TestAllTypesNano <"));
-    assertTrue(protoPrint.contains("  optional_int32: 14"));
-    assertTrue(protoPrint.contains("  optional_float: 42.3"));
-    assertTrue(protoPrint.contains("  optional_double: 0.0"));
-    assertTrue(protoPrint.contains("  optional_string: \"String \\u0022with\\u0027 both quotes\""));
-    assertTrue(protoPrint.contains("  optional_bytes: [B@"));
-    assertTrue(protoPrint.contains("  optionalGroup <\n    a: 15\n  >"));
-
-    assertTrue(protoPrint.contains("  repeated_int64: 1"));
-    assertTrue(protoPrint.contains("  repeated_int64: -1"));
-    assertTrue(protoPrint.contains("  repeated_bytes: null\n  repeated_bytes: [B@"));
-    assertTrue(protoPrint.contains("  repeatedGroup <\n    a: -27\n  >\n"
-            + "  repeatedGroup <\n    a: -72\n  >"));
-    assertTrue(protoPrint.contains("  optionalNestedMessage <\n    bb: 7\n  >"));
-    assertTrue(protoPrint.contains("  repeatedNestedMessage <\n    bb: 77\n  >\n"
-            + "  repeatedNestedMessage <\n    bb: 88\n  >"));
-    assertTrue(protoPrint.contains("  optional_nested_enum: 3"));
-    assertTrue(protoPrint.contains("  repeated_nested_enum: 2\n  repeated_nested_enum: 1"));
-    assertTrue(protoPrint.contains("  default_int32: 41"));
-    assertTrue(protoPrint.contains("  default_string: \"hello\""));
+    assertTrue(protoPrint.contains("optional_int32: 14"));
+    assertTrue(protoPrint.contains("optional_float: 42.3"));
+    assertTrue(protoPrint.contains("optional_double: 0.0"));
+    assertTrue(protoPrint.contains("optional_string: \"String \\u0022with\\u0027 both quotes\""));
+    assertTrue(protoPrint.contains("optional_bytes: \"\\\"\\000\\001\\010\""));
+    assertTrue(protoPrint.contains("optional_group <\n  a: 15\n>"));
+
+    assertTrue(protoPrint.contains("repeated_int64: 1"));
+    assertTrue(protoPrint.contains("repeated_int64: -1"));
+    assertFalse(protoPrint.contains("repeated_bytes: \"\"")); // null should be dropped
+    assertTrue(protoPrint.contains("repeated_bytes: \"hello\""));
+    assertTrue(protoPrint.contains("repeated_group <\n  a: -27\n>\n"
+            + "repeated_group <\n  a: -72\n>"));
+    assertTrue(protoPrint.contains("optional_nested_message <\n  bb: 7\n>"));
+    assertTrue(protoPrint.contains("repeated_nested_message <\n  bb: 77\n>\n"
+            + "repeated_nested_message <\n  bb: 88\n>"));
+    assertTrue(protoPrint.contains("optional_nested_enum: 3"));
+    assertTrue(protoPrint.contains("repeated_nested_enum: 2\nrepeated_nested_enum: 1"));
+    assertTrue(protoPrint.contains("default_int32: 41"));
+    assertTrue(protoPrint.contains("default_string: \"hello\""));
+    assertFalse(protoPrint.contains("repeated_string_piece: \"\""));  // null should be dropped
+    assertTrue(protoPrint.contains("repeated_string_piece: \"world\""));
   }
 
   public void testExtensions() throws Exception {