Merge pull request square#542 from square/jwilson_0118_pack

Fix a critical bug where we were encoding empty packed values.

wire-runtime/src/main/java/com/squareup/wire/ProtoAdapter.java

@@ -420,6 +420,10 @@ private ProtoAdapter<List<E>> createPacked() {
         return size;
       }
 
+      @Override public int encodedSizeWithTag(int tag, List<E> value) {
+        return value.isEmpty() ? 0 : super.encodedSizeWithTag(tag, value);
+      }
+
       @Override public void encode(ProtoWriter writer, List<E> value) throws IOException {
         for (int i = 0, count = value.size(); i < count; i++) {
           ProtoAdapter.this.encode(writer, value.get(i));

wire-runtime/src/test/java/com/squareup/wire/ProtoAdapterTest.java

@@ -16,10 +16,14 @@
 package com.squareup.wire;
 
 import com.squareup.wire.protos.person.Person;
+import java.io.IOException;
 import okio.ByteString;
 import org.junit.Test;
+import squareup.protos.packed_encoding.EmbeddedMessage;
+import squareup.protos.packed_encoding.OuterMessage;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 
 public final class ProtoAdapterTest {
@@ -65,4 +69,17 @@ public final class ProtoAdapterTest {
     assertThat(adapter.asRepeated()).isSameAs(adapter.asRepeated());
     assertThat(adapter.asPacked()).isSameAs(adapter.asPacked());
   }
+
+  /** https://github.com/square/wire/issues/541 */
+  @Test public void embeddedEmptyPackedMessage() throws IOException {
+    OuterMessage outerMessage = new OuterMessage.Builder()
+        .outer_number_before(2)
+        .embedded_message(new EmbeddedMessage.Builder()
+            .inner_number_after(1)
+            .build())
+        .build();
+    OuterMessage outerMessagesAfterSerialisation = OuterMessage.ADAPTER.decode(
+        OuterMessage.ADAPTER.encode(outerMessage));
+    assertEquals(outerMessagesAfterSerialisation, outerMessage);
+  }
 }

wire-runtime/src/test/proto-java/squareup/protos/packed_encoding/EmbeddedMessage.java

@@ -0,0 +1,161 @@
+// Code generated by Wire protocol buffer compiler, do not edit.
+// Source file: packed_encoding.proto at 18:1
+package squareup.protos.packed_encoding;
+
+import com.squareup.wire.FieldEncoding;
+import com.squareup.wire.Message;
+import com.squareup.wire.ProtoAdapter;
+import com.squareup.wire.ProtoReader;
+import com.squareup.wire.ProtoWriter;
+import com.squareup.wire.WireField;
+import com.squareup.wire.internal.Internal;
+import java.io.IOException;
+import java.lang.Integer;
+import java.lang.Object;
+import java.lang.Override;
+import java.lang.String;
+import java.lang.StringBuilder;
+import java.util.List;
+import okio.ByteString;
+
+public final class EmbeddedMessage extends Message<EmbeddedMessage, EmbeddedMessage.Builder> {
+  public static final ProtoAdapter<EmbeddedMessage> ADAPTER = new ProtoAdapter_EmbeddedMessage();
+
+  private static final long serialVersionUID = 0L;
+
+  public static final Integer DEFAULT_INNER_NUMBER_AFTER = 0;
+
+  @WireField(
+      tag = 1,
+      adapter = "com.squareup.wire.ProtoAdapter#INT32",
+      label = WireField.Label.PACKED
+  )
+  public final List<Integer> inner_repeated_number;
+
+  @WireField(
+      tag = 2,
+      adapter = "com.squareup.wire.ProtoAdapter#INT32"
+  )
+  public final Integer inner_number_after;
+
+  public EmbeddedMessage(List<Integer> inner_repeated_number, Integer inner_number_after) {
+    this(inner_repeated_number, inner_number_after, ByteString.EMPTY);
+  }
+
+  public EmbeddedMessage(List<Integer> inner_repeated_number, Integer inner_number_after, ByteString unknownFields) {
+    super(ADAPTER, unknownFields);
+    this.inner_repeated_number = Internal.immutableCopyOf("inner_repeated_number", inner_repeated_number);
+    this.inner_number_after = inner_number_after;
+  }
+
+  @Override
+  public Builder newBuilder() {
+    Builder builder = new Builder();
+    builder.inner_repeated_number = Internal.copyOf("inner_repeated_number", inner_repeated_number);
+    builder.inner_number_after = inner_number_after;
+    builder.addUnknownFields(unknownFields());
+    return builder;
+  }
+
+  @Override
+  public boolean equals(Object other) {
+    if (other == this) return true;
+    if (!(other instanceof EmbeddedMessage)) return false;
+    EmbeddedMessage o = (EmbeddedMessage) other;
+    return Internal.equals(unknownFields(), o.unknownFields())
+        && Internal.equals(inner_repeated_number, o.inner_repeated_number)
+        && Internal.equals(inner_number_after, o.inner_number_after);
+  }
+
+  @Override
+  public int hashCode() {
+    int result = super.hashCode;
+    if (result == 0) {
+      result = unknownFields().hashCode();
+      result = result * 37 + (inner_repeated_number != null ? inner_repeated_number.hashCode() : 1);
+      result = result * 37 + (inner_number_after != null ? inner_number_after.hashCode() : 0);
+      super.hashCode = result;
+    }
+    return result;
+  }
+
+  @Override
+  public String toString() {
+    StringBuilder builder = new StringBuilder();
+    if (inner_repeated_number != null) builder.append(", inner_repeated_number=").append(inner_repeated_number);
+    if (inner_number_after != null) builder.append(", inner_number_after=").append(inner_number_after);
+    return builder.replace(0, 2, "EmbeddedMessage{").append('}').toString();
+  }
+
+  public static final class Builder extends Message.Builder<EmbeddedMessage, Builder> {
+    public List<Integer> inner_repeated_number;
+
+    public Integer inner_number_after;
+
+    public Builder() {
+      inner_repeated_number = Internal.newMutableList();
+    }
+
+    public Builder inner_repeated_number(List<Integer> inner_repeated_number) {
+      Internal.checkElementsNotNull(inner_repeated_number);
+      this.inner_repeated_number = inner_repeated_number;
+      return this;
+    }
+
+    public Builder inner_number_after(Integer inner_number_after) {
+      this.inner_number_after = inner_number_after;
+      return this;
+    }
+
+    @Override
+    public EmbeddedMessage build() {
+      return new EmbeddedMessage(inner_repeated_number, inner_number_after, buildUnknownFields());
+    }
+  }
+
+  private static final class ProtoAdapter_EmbeddedMessage extends ProtoAdapter<EmbeddedMessage> {
+    ProtoAdapter_EmbeddedMessage() {
+      super(FieldEncoding.LENGTH_DELIMITED, EmbeddedMessage.class);
+    }
+
+    @Override
+    public int encodedSize(EmbeddedMessage value) {
+      return ProtoAdapter.INT32.asPacked().encodedSizeWithTag(1, value.inner_repeated_number)
+          + (value.inner_number_after != null ? ProtoAdapter.INT32.encodedSizeWithTag(2, value.inner_number_after) : 0)
+          + value.unknownFields().size();
+    }
+
+    @Override
+    public void encode(ProtoWriter writer, EmbeddedMessage value) throws IOException {
+      if (value.inner_repeated_number != null) ProtoAdapter.INT32.asPacked().encodeWithTag(writer, 1, value.inner_repeated_number);
+      if (value.inner_number_after != null) ProtoAdapter.INT32.encodeWithTag(writer, 2, value.inner_number_after);
+      writer.writeBytes(value.unknownFields());
+    }
+
+    @Override
+    public EmbeddedMessage decode(ProtoReader reader) throws IOException {
+      Builder builder = new Builder();
+      long token = reader.beginMessage();
+      for (int tag; (tag = reader.nextTag()) != -1;) {
+        switch (tag) {
+          case 1: builder.inner_repeated_number.add(ProtoAdapter.INT32.decode(reader)); break;
+          case 2: builder.inner_number_after(ProtoAdapter.INT32.decode(reader)); break;
+          default: {
+            FieldEncoding fieldEncoding = reader.peekFieldEncoding();
+            Object value = fieldEncoding.rawProtoAdapter().decode(reader);
+            builder.addUnknownField(tag, fieldEncoding, value);
+          }
+        }
+      }
+      reader.endMessage(token);
+      return builder.build();
+    }
+
+    @Override
+    public EmbeddedMessage redact(EmbeddedMessage value) {
+      Builder builder = value.newBuilder();
+      builder.clearUnknownFields();
+      return builder.build();
+    }
+  }
+}

wire-runtime/src/test/proto-java/squareup/protos/packed_encoding/OuterMessage.java

@@ -0,0 +1,158 @@
+// Code generated by Wire protocol buffer compiler, do not edit.
+// Source file: packed_encoding.proto at 23:1
+package squareup.protos.packed_encoding;
+
+import com.squareup.wire.FieldEncoding;
+import com.squareup.wire.Message;
+import com.squareup.wire.ProtoAdapter;
+import com.squareup.wire.ProtoReader;
+import com.squareup.wire.ProtoWriter;
+import com.squareup.wire.WireField;
+import com.squareup.wire.internal.Internal;
+import java.io.IOException;
+import java.lang.Integer;
+import java.lang.Object;
+import java.lang.Override;
+import java.lang.String;
+import java.lang.StringBuilder;
+import okio.ByteString;
+
+public final class OuterMessage extends Message<OuterMessage, OuterMessage.Builder> {
+  public static final ProtoAdapter<OuterMessage> ADAPTER = new ProtoAdapter_OuterMessage();
+
+  private static final long serialVersionUID = 0L;
+
+  public static final Integer DEFAULT_OUTER_NUMBER_BEFORE = 0;
+
+  @WireField(
+      tag = 1,
+      adapter = "com.squareup.wire.ProtoAdapter#INT32"
+  )
+  public final Integer outer_number_before;
+
+  @WireField(
+      tag = 2,
+      adapter = "squareup.protos.packed_encoding.EmbeddedMessage#ADAPTER"
+  )
+  public final EmbeddedMessage embedded_message;
+
+  public OuterMessage(Integer outer_number_before, EmbeddedMessage embedded_message) {
+    this(outer_number_before, embedded_message, ByteString.EMPTY);
+  }
+
+  public OuterMessage(Integer outer_number_before, EmbeddedMessage embedded_message, ByteString unknownFields) {
+    super(ADAPTER, unknownFields);
+    this.outer_number_before = outer_number_before;
+    this.embedded_message = embedded_message;
+  }
+
+  @Override
+  public Builder newBuilder() {
+    Builder builder = new Builder();
+    builder.outer_number_before = outer_number_before;
+    builder.embedded_message = embedded_message;
+    builder.addUnknownFields(unknownFields());
+    return builder;
+  }
+
+  @Override
+  public boolean equals(Object other) {
+    if (other == this) return true;
+    if (!(other instanceof OuterMessage)) return false;
+    OuterMessage o = (OuterMessage) other;
+    return Internal.equals(unknownFields(), o.unknownFields())
+        && Internal.equals(outer_number_before, o.outer_number_before)
+        && Internal.equals(embedded_message, o.embedded_message);
+  }
+
+  @Override
+  public int hashCode() {
+    int result = super.hashCode;
+    if (result == 0) {
+      result = unknownFields().hashCode();
+      result = result * 37 + (outer_number_before != null ? outer_number_before.hashCode() : 0);
+      result = result * 37 + (embedded_message != null ? embedded_message.hashCode() : 0);
+      super.hashCode = result;
+    }
+    return result;
+  }
+
+  @Override
+  public String toString() {
+    StringBuilder builder = new StringBuilder();
+    if (outer_number_before != null) builder.append(", outer_number_before=").append(outer_number_before);
+    if (embedded_message != null) builder.append(", embedded_message=").append(embedded_message);
+    return builder.replace(0, 2, "OuterMessage{").append('}').toString();
+  }
+
+  public static final class Builder extends Message.Builder<OuterMessage, Builder> {
+    public Integer outer_number_before;
+
+    public EmbeddedMessage embedded_message;
+
+    public Builder() {
+    }
+
+    public Builder outer_number_before(Integer outer_number_before) {
+      this.outer_number_before = outer_number_before;
+      return this;
+    }
+
+    public Builder embedded_message(EmbeddedMessage embedded_message) {
+      this.embedded_message = embedded_message;
+      return this;
+    }
+
+    @Override
+    public OuterMessage build() {
+      return new OuterMessage(outer_number_before, embedded_message, buildUnknownFields());
+    }
+  }
+
+  private static final class ProtoAdapter_OuterMessage extends ProtoAdapter<OuterMessage> {
+    ProtoAdapter_OuterMessage() {
+      super(FieldEncoding.LENGTH_DELIMITED, OuterMessage.class);
+    }
+
+    @Override
+    public int encodedSize(OuterMessage value) {
+      return (value.outer_number_before != null ? ProtoAdapter.INT32.encodedSizeWithTag(1, value.outer_number_before) : 0)
+          + (value.embedded_message != null ? EmbeddedMessage.ADAPTER.encodedSizeWithTag(2, value.embedded_message) : 0)
+          + value.unknownFields().size();
+    }
+
+    @Override
+    public void encode(ProtoWriter writer, OuterMessage value) throws IOException {
+      if (value.outer_number_before != null) ProtoAdapter.INT32.encodeWithTag(writer, 1, value.outer_number_before);
+      if (value.embedded_message != null) EmbeddedMessage.ADAPTER.encodeWithTag(writer, 2, value.embedded_message);
+      writer.writeBytes(value.unknownFields());
+    }
+
+    @Override
+    public OuterMessage decode(ProtoReader reader) throws IOException {
+      Builder builder = new Builder();
+      long token = reader.beginMessage();
+      for (int tag; (tag = reader.nextTag()) != -1;) {
+        switch (tag) {
+          case 1: builder.outer_number_before(ProtoAdapter.INT32.decode(reader)); break;
+          case 2: builder.embedded_message(EmbeddedMessage.ADAPTER.decode(reader)); break;
+          default: {
+            FieldEncoding fieldEncoding = reader.peekFieldEncoding();
+            Object value = fieldEncoding.rawProtoAdapter().decode(reader);
+            builder.addUnknownField(tag, fieldEncoding, value);
+          }
+        }
+      }
+      reader.endMessage(token);
+      return builder.build();
+    }
+
+    @Override
+    public OuterMessage redact(OuterMessage value) {
+      Builder builder = value.newBuilder();
+      if (builder.embedded_message != null) builder.embedded_message = EmbeddedMessage.ADAPTER.redact(builder.embedded_message);
+      builder.clearUnknownFields();
+      return builder.build();
+    }
+  }
+}

wire-runtime/src/test/proto/packed_encoding.proto

@@ -0,0 +1,26 @@
+/*
+ * Copyright 2016 Square Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package squareup.protos.packed_encoding;
+
+message EmbeddedMessage {
+  repeated int32 inner_repeated_number = 1 [packed=true];
+  optional int32 inner_number_after = 2;
+}
+
+message OuterMessage {
+  optional int32 outer_number_before = 1;
+  optional EmbeddedMessage embedded_message = 2;
+}