Fixing merge issues

protocolbuffers · Sep 28, 2022 · ebaa566 · ebaa566
1 parent fe442a6
commit ebaa566
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 40 deletions.
diff --git a/java/core/src/main/java/com/google/protobuf/DynamicMessage.java b/java/core/src/main/java/com/google/protobuf/DynamicMessage.java
@@ -418,10 +418,11 @@ public DynamicMessage buildPartial() {
         }
       }
 
+      fields.makeImmutable();
       DynamicMessage result =
           new DynamicMessage(
               type,
-              fields.buildPartial(),
+              fields,
               java.util.Arrays.copyOf(oneofCases, oneofCases.length),
               unknownFields);
       return result;

diff --git a/java/core/src/main/java/com/google/protobuf/MessageReflection.java b/java/core/src/main/java/com/google/protobuf/MessageReflection.java
@@ -378,6 +378,7 @@ MergeTarget newEmptyTargetForField(
   static class BuilderAdapter implements MergeTarget {
 
     private final Message.Builder builder;
+    private boolean hasNestedBuilders = true;
 
     @Override
     public Descriptors.Descriptor getDescriptorForType() {
@@ -393,6 +394,17 @@ public Object getField(Descriptors.FieldDescriptor field) {
       return builder.getField(field);
     }
 
+    private Message.Builder getFieldBuilder(Descriptors.FieldDescriptor field) {
+      if (hasNestedBuilders) {
+        try {
+          return builder.getFieldBuilder(field);
+        } catch (UnsupportedOperationException e) {
+          hasNestedBuilders = false;
+        }
+      }
+      return null;
+    }
+
     @Override
     public boolean hasField(Descriptors.FieldDescriptor field) {
       return builder.hasField(field);
@@ -536,11 +548,19 @@ public void mergeGroup(
         Message defaultInstance)
         throws IOException {
       if (!field.isRepeated()) {
+        Message.Builder subBuilder;
         if (hasField(field)) {
-          input.readGroup(field.getNumber(), builder.getFieldBuilder(field), extensionRegistry);
-          return;
+          subBuilder = getFieldBuilder(field);
+          if (subBuilder != null) {
+            input.readGroup(field.getNumber(), subBuilder, extensionRegistry);
+            return;
+          } else {
+            subBuilder = newMessageFieldInstance(field, defaultInstance);
+            subBuilder.mergeFrom((Message) getField(field));
+          }
+        } else {
+          subBuilder = newMessageFieldInstance(field, defaultInstance);
         }
-        Message.Builder subBuilder = newMessageFieldInstance(field, defaultInstance);
         input.readGroup(field.getNumber(), subBuilder, extensionRegistry);
         Object unused = setField(field, subBuilder.buildPartial());
       } else {
@@ -558,11 +578,19 @@ public void mergeMessage(
         Message defaultInstance)
         throws IOException {
       if (!field.isRepeated()) {
+        Message.Builder subBuilder;
         if (hasField(field)) {
-          input.readMessage(builder.getFieldBuilder(field), extensionRegistry);
-          return;
+          subBuilder = getFieldBuilder(field);
+          if (subBuilder != null) {
+            input.readMessage(subBuilder, extensionRegistry);
+            return;
+          } else {
+            subBuilder = newMessageFieldInstance(field, defaultInstance);
+            subBuilder.mergeFrom((Message) getField(field));
+          }
+        } else {
+          subBuilder = newMessageFieldInstance(field, defaultInstance);
         }
-        Message.Builder subBuilder = newMessageFieldInstance(field, defaultInstance);
         input.readMessage(subBuilder, extensionRegistry);
         Object unused = setField(field, subBuilder.buildPartial());
       } else {
@@ -865,29 +893,25 @@ public boolean hasField(Descriptors.FieldDescriptor field) {
     }
 
     @Override
-    @CanIgnoreReturnValue
     public MergeTarget setField(Descriptors.FieldDescriptor field, Object value) {
       extensions.setField(field, value);
       return this;
     }
 
     @Override
-    @CanIgnoreReturnValue
     public MergeTarget clearField(Descriptors.FieldDescriptor field) {
       extensions.clearField(field);
       return this;
     }
 
     @Override
-    @CanIgnoreReturnValue
     public MergeTarget setRepeatedField(
         Descriptors.FieldDescriptor field, int index, Object value) {
       extensions.setRepeatedField(field, index, value);
       return this;
     }
 
     @Override
-    @CanIgnoreReturnValue
     public MergeTarget addRepeatedField(Descriptors.FieldDescriptor field, Object value) {
       extensions.addRepeatedField(field, value);
       return this;
@@ -899,7 +923,6 @@ public boolean hasOneof(Descriptors.OneofDescriptor oneof) {
     }
 
     @Override
-    @CanIgnoreReturnValue
     public MergeTarget clearOneof(Descriptors.OneofDescriptor oneof) {
       // Nothing to clear.
       return this;

diff --git a/java/core/src/main/java/com/google/protobuf/SchemaUtil.java b/java/core/src/main/java/com/google/protobuf/SchemaUtil.java
@@ -904,7 +904,6 @@ static <T, UT, UB> void mergeUnknownFields(
   }
 
   /** Filters unrecognized enum values in a list. */
-  @CanIgnoreReturnValue
   static <UT, UB> UB filterUnknownEnumList(
       Object containerMessage,
       int number,
@@ -950,7 +949,6 @@ static <UT, UB> UB filterUnknownEnumList(
   }
 
   /** Filters unrecognized enum values in a list. */
-  @CanIgnoreReturnValue
   static <UT, UB> UB filterUnknownEnumList(
       Object containerMessage,
       int number,

diff --git a/java/core/src/main/java/com/google/protobuf/UnknownFieldSetLite.java b/java/core/src/main/java/com/google/protobuf/UnknownFieldSetLite.java
@@ -498,7 +498,6 @@ private UnknownFieldSetLite mergeFrom(final CodedInputStream input) throws IOExc
     return this;
   }
 
-  @CanIgnoreReturnValue
   UnknownFieldSetLite mergeFrom(UnknownFieldSetLite other) {
     if (other.equals(getDefaultInstance())) {
       return this;

diff --git a/java/lite/src/test/java/com/google/protobuf/LiteTest.java b/java/lite/src/test/java/com/google/protobuf/LiteTest.java
@@ -50,15 +50,6 @@
 import com.google.protobuf.UnittestLite.TestAllTypesLiteOrBuilder;
 import com.google.protobuf.UnittestLite.TestHugeFieldNumbersLite;
 import com.google.protobuf.UnittestLite.TestNestedExtensionLite;
-import map_lite_test.MapTestProto.TestMap;
-import map_lite_test.MapTestProto.TestMap.MessageValue;
-import protobuf_unittest.NestedExtensionLite;
-import protobuf_unittest.NonNestedExtensionLite;
-import protobuf_unittest.lite_equals_and_hash.LiteEqualsAndHash.Bar;
-import protobuf_unittest.lite_equals_and_hash.LiteEqualsAndHash.BarPrime;
-import protobuf_unittest.lite_equals_and_hash.LiteEqualsAndHash.Foo;
-import protobuf_unittest.lite_equals_and_hash.LiteEqualsAndHash.TestOneofEquals;
-import protobuf_unittest.lite_equals_and_hash.LiteEqualsAndHash.TestRecursiveOneof;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
@@ -71,6 +62,15 @@
 import java.util.Iterator;
 import java.util.List;
 import junit.framework.TestCase;
+import map_lite_test.MapTestProto.TestMap;
+import map_lite_test.MapTestProto.TestMap.MessageValue;
+import protobuf_unittest.NestedExtensionLite;
+import protobuf_unittest.NonNestedExtensionLite;
+import protobuf_unittest.lite_equals_and_hash.LiteEqualsAndHash.Bar;
+import protobuf_unittest.lite_equals_and_hash.LiteEqualsAndHash.BarPrime;
+import protobuf_unittest.lite_equals_and_hash.LiteEqualsAndHash.Foo;
+import protobuf_unittest.lite_equals_and_hash.LiteEqualsAndHash.TestOneofEquals;
+import protobuf_unittest.lite_equals_and_hash.LiteEqualsAndHash.TestRecursiveOneof;
 
 /**
  * Test lite runtime.
@@ -180,28 +180,27 @@ public void testAddAll() {
   }
 
   public void testMemoization() throws Exception {
-    GeneratedMessageLite<?, ?> message = TestUtilLite.getAllLiteExtensionsSet();
-
-    // This built message should not be mutable
-    assertThat(message.isMutable()).isFalse();
+    TestAllExtensionsLite message = TestUtilLite.getAllLiteExtensionsSet();
 
     // Test serialized size is memoized
-    assertThat(message.getMemoizedSerializedSize())
-        .isEqualTo(GeneratedMessageLite.UNINITIALIZED_SERIALIZED_SIZE);
+    assertEquals(
+      GeneratedMessageLite.UNINITIALIZED_SERIALIZED_SIZE,
+      message.getMemoizedSerializedSize());
     int size = message.getSerializedSize();
-    assertThat(size).isGreaterThan(0);
-    assertThat(message.getMemoizedSerializedSize()).isEqualTo(size);
+    assertTrue(size > 0);
+    assertEquals(size, message.getMemoizedSerializedSize());
     message.clearMemoizedSerializedSize();
-    assertThat(message.getMemoizedSerializedSize())
-        .isEqualTo(GeneratedMessageLite.UNINITIALIZED_SERIALIZED_SIZE);
+    assertEquals(
+      GeneratedMessageLite.UNINITIALIZED_SERIALIZED_SIZE,
+      message.getMemoizedSerializedSize());
 
     // Test hashCode is memoized
-    assertThat(message.hashCodeIsNotMemoized()).isTrue();
+    assertTrue(message.hashCodeIsNotMemoized());
     int hashCode = message.hashCode();
-    assertThat(message.hashCodeIsNotMemoized()).isFalse();
-    assertThat(message.getMemoizedHashCode()).isEqualTo(hashCode);
+    assertFalse(message.hashCodeIsNotMemoized());
+    assertEquals(hashCode, message.getMemoizedHashCode());
     message.clearMemoizedHashCode();
-    assertThat(message.hashCodeIsNotMemoized()).isTrue();
+    assertTrue(message.hashCodeIsNotMemoized());
 
     // Test isInitialized is memoized
     Field memo = message.getClass().getDeclaredField("memoizedIsInitialized");

diff --git a/src/google/protobuf/compiler/java/java_message_builder.cc b/src/google/protobuf/compiler/java/java_message_builder.cc
@@ -673,7 +673,7 @@ void MessageBuilderGenerator::GenerateBuilderFieldParsingCase(
     io::Printer* printer, const FieldDescriptor* field) {
   uint32_t tag = WireFormatLite::MakeTag(
       field->number(), WireFormat::WireTypeForFieldType(field->type()));
-  std::string tagString = absl::StrCat(static_cast<int32_t>(tag));
+  std::string tagString = StrCat(static_cast<int32_t>(tag));
   printer->Print("case $tag$: {\n", "tag", tagString);
   printer->Indent();
 
@@ -692,7 +692,7 @@ void MessageBuilderGenerator::GenerateBuilderPackedFieldParsingCase(
   // packed version of this field regardless of field->options().packed().
   uint32_t tag = WireFormatLite::MakeTag(
       field->number(), WireFormatLite::WIRETYPE_LENGTH_DELIMITED);
-  std::string tagString = absl::StrCat(static_cast<int32_t>(tag));
+  std::string tagString = StrCat(static_cast<int32_t>(tag));
   printer->Print("case $tag$: {\n", "tag", tagString);
   printer->Indent();