Cherry-pick the fix to protocolbuffers#7480 from protocolbuffers#7485.

haberman · May 12, 2020 · 01e8c0f · 01e8c0f
1 parent c0b1ce0
commit 01e8c0f
Show file tree

Hide file tree

Showing 6 changed files with 132 additions and 4 deletions.
diff --git a/java/core/generate-test-sources-build.xml b/java/core/generate-test-sources-build.xml
@@ -17,6 +17,7 @@
         <arg value="${protobuf.source.dir}/google/protobuf/unittest_no_generic_services.proto"/>
         <arg value="${protobuf.source.dir}/google/protobuf/unittest_optimize_for.proto"/>
         <arg value="${protobuf.source.dir}/google/protobuf/unittest_proto3.proto"/>
+        <arg value="${protobuf.source.dir}/google/protobuf/unittest_proto3_optional.proto"/>
         <arg value="${protobuf.source.dir}/google/protobuf/unittest_well_known_types.proto"/>
         <arg value="${test.proto.dir}/com/google/protobuf/any_test.proto"/>
         <arg value="${test.proto.dir}/com/google/protobuf/cached_field_size_test.proto"/>

diff --git a/java/core/src/test/java/com/google/protobuf/FieldPresenceTest.java b/java/core/src/test/java/com/google/protobuf/FieldPresenceTest.java
@@ -38,6 +38,7 @@
 import com.google.protobuf.FieldPresenceTestProto.TestAllTypes;
 import com.google.protobuf.FieldPresenceTestProto.TestOptionalFieldsOnly;
 import com.google.protobuf.FieldPresenceTestProto.TestRepeatedFieldsOnly;
+import com.google.protobuf.testing.proto.TestProto3Optional;
 import protobuf_unittest.UnittestProto;
 import junit.framework.TestCase;
 
@@ -101,6 +102,113 @@ public void testHasMethod() {
         UnittestProto.TestAllTypes.Builder.class, TestAllTypes.Builder.class, "OneofBytes");
   }
 
+  public void testHasMethodForProto3Optional() throws Exception {
+    assertFalse(TestProto3Optional.getDefaultInstance().hasOptionalInt32());
+    assertFalse(TestProto3Optional.getDefaultInstance().hasOptionalInt64());
+    assertFalse(TestProto3Optional.getDefaultInstance().hasOptionalUint32());
+    assertFalse(TestProto3Optional.getDefaultInstance().hasOptionalUint64());
+    assertFalse(TestProto3Optional.getDefaultInstance().hasOptionalSint32());
+    assertFalse(TestProto3Optional.getDefaultInstance().hasOptionalSint64());
+    assertFalse(TestProto3Optional.getDefaultInstance().hasOptionalFixed32());
+    assertFalse(TestProto3Optional.getDefaultInstance().hasOptionalFixed64());
+    assertFalse(TestProto3Optional.getDefaultInstance().hasOptionalFloat());
+    assertFalse(TestProto3Optional.getDefaultInstance().hasOptionalDouble());
+    assertFalse(TestProto3Optional.getDefaultInstance().hasOptionalBool());
+    assertFalse(TestProto3Optional.getDefaultInstance().hasOptionalString());
+    assertFalse(TestProto3Optional.getDefaultInstance().hasOptionalBytes());
+
+    TestProto3Optional.Builder builder = TestProto3Optional.newBuilder().setOptionalInt32(0);
+    assertTrue(builder.hasOptionalInt32());
+    assertTrue(builder.build().hasOptionalInt32());
+
+    TestProto3Optional.Builder otherBuilder = TestProto3Optional.newBuilder().setOptionalInt32(1);
+    otherBuilder.mergeFrom(builder.build());
+    assertTrue(otherBuilder.hasOptionalInt32());
+    assertEquals(0, otherBuilder.getOptionalInt32());
+
+    TestProto3Optional.Builder builder3 =
+        TestProto3Optional.newBuilder().setOptionalNestedEnumValue(5);
+    assertTrue(builder3.hasOptionalNestedEnum());
+
+    TestProto3Optional.Builder builder4 =
+        TestProto3Optional.newBuilder().setOptionalNestedEnum(TestProto3Optional.NestedEnum.FOO);
+    assertTrue(builder4.hasOptionalNestedEnum());
+
+    TestProto3Optional proto = TestProto3Optional.parseFrom(builder.build().toByteArray());
+    assertTrue(proto.hasOptionalInt32());
+    assertTrue(proto.toBuilder().hasOptionalInt32());
+  }
+
+  private static void assertProto3OptionalReflection(String name) throws Exception {
+    FieldDescriptor fieldDescriptor = TestProto3Optional.getDescriptor().findFieldByName(name);
+    OneofDescriptor oneofDescriptor = fieldDescriptor.getContainingOneof();
+    assertNotNull(fieldDescriptor.getContainingOneof());
+    assertTrue(fieldDescriptor.hasOptionalKeyword());
+    assertTrue(fieldDescriptor.hasPresence());
+
+    assertFalse(TestProto3Optional.getDefaultInstance().hasOneof(oneofDescriptor));
+    assertNull(TestProto3Optional.getDefaultInstance().getOneofFieldDescriptor(oneofDescriptor));
+
+    TestProto3Optional.Builder builder = TestProto3Optional.newBuilder();
+    builder.setField(fieldDescriptor, fieldDescriptor.getDefaultValue());
+    assertTrue(builder.hasField(fieldDescriptor));
+    assertEquals(fieldDescriptor.getDefaultValue(), builder.getField(fieldDescriptor));
+    assertTrue(builder.build().hasField(fieldDescriptor));
+    assertEquals(fieldDescriptor.getDefaultValue(), builder.build().getField(fieldDescriptor));
+    assertTrue(builder.hasOneof(oneofDescriptor));
+    assertEquals(fieldDescriptor, builder.getOneofFieldDescriptor(oneofDescriptor));
+    assertTrue(builder.build().hasOneof(oneofDescriptor));
+    assertEquals(fieldDescriptor, builder.build().getOneofFieldDescriptor(oneofDescriptor));
+
+    TestProto3Optional.Builder otherBuilder = TestProto3Optional.newBuilder();
+    otherBuilder.mergeFrom(builder.build());
+    assertTrue(otherBuilder.hasField(fieldDescriptor));
+    assertEquals(fieldDescriptor.getDefaultValue(), otherBuilder.getField(fieldDescriptor));
+
+    TestProto3Optional proto = TestProto3Optional.parseFrom(builder.build().toByteArray());
+    assertTrue(proto.hasField(fieldDescriptor));
+    assertTrue(proto.toBuilder().hasField(fieldDescriptor));
+
+    DynamicMessage.Builder dynamicBuilder =
+        DynamicMessage.newBuilder(TestProto3Optional.getDescriptor());
+    dynamicBuilder.setField(fieldDescriptor, fieldDescriptor.getDefaultValue());
+    assertTrue(dynamicBuilder.hasField(fieldDescriptor));
+    assertEquals(fieldDescriptor.getDefaultValue(), dynamicBuilder.getField(fieldDescriptor));
+    assertTrue(dynamicBuilder.build().hasField(fieldDescriptor));
+    assertEquals(
+        fieldDescriptor.getDefaultValue(), dynamicBuilder.build().getField(fieldDescriptor));
+    assertTrue(dynamicBuilder.hasOneof(oneofDescriptor));
+    assertEquals(fieldDescriptor, dynamicBuilder.getOneofFieldDescriptor(oneofDescriptor));
+    assertTrue(dynamicBuilder.build().hasOneof(oneofDescriptor));
+    assertEquals(fieldDescriptor, dynamicBuilder.build().getOneofFieldDescriptor(oneofDescriptor));
+
+    DynamicMessage.Builder otherDynamicBuilder =
+        DynamicMessage.newBuilder(TestProto3Optional.getDescriptor());
+    otherDynamicBuilder.mergeFrom(dynamicBuilder.build());
+    assertTrue(otherDynamicBuilder.hasField(fieldDescriptor));
+    assertEquals(fieldDescriptor.getDefaultValue(), otherDynamicBuilder.getField(fieldDescriptor));
+
+    DynamicMessage dynamicProto =
+        DynamicMessage.parseFrom(TestProto3Optional.getDescriptor(), builder.build().toByteArray());
+    assertTrue(dynamicProto.hasField(fieldDescriptor));
+    assertTrue(dynamicProto.toBuilder().hasField(fieldDescriptor));
+  }
+
+  public void testProto3Optional_reflection() throws Exception {
+    assertProto3OptionalReflection("optional_int32");
+    assertProto3OptionalReflection("optional_int64");
+    assertProto3OptionalReflection("optional_uint32");
+    assertProto3OptionalReflection("optional_uint64");
+    assertProto3OptionalReflection("optional_sint32");
+    assertProto3OptionalReflection("optional_sint64");
+    assertProto3OptionalReflection("optional_fixed32");
+    assertProto3OptionalReflection("optional_fixed64");
+    assertProto3OptionalReflection("optional_float");
+    assertProto3OptionalReflection("optional_double");
+    assertProto3OptionalReflection("optional_bool");
+    assertProto3OptionalReflection("optional_string");
+    assertProto3OptionalReflection("optional_bytes");
+  }
 
   public void testOneofEquals() throws Exception {
     TestAllTypes.Builder builder = TestAllTypes.newBuilder();

diff --git a/java/core/src/test/java/com/google/protobuf/TextFormatTest.java b/java/core/src/test/java/com/google/protobuf/TextFormatTest.java
@@ -40,6 +40,8 @@
 import com.google.protobuf.Descriptors.FieldDescriptor;
 import com.google.protobuf.Descriptors.FileDescriptor;
 import com.google.protobuf.TextFormat.Parser.SingularOverwritePolicy;
+import com.google.protobuf.testing.proto.TestProto3Optional;
+import com.google.protobuf.testing.proto.TestProto3Optional.NestedEnum;
 import any_test.AnyTestProto.TestAny;
 import map_test.MapTestProto.TestMap;
 import protobuf_unittest.UnittestMset.TestMessageSetExtension1;
@@ -319,6 +321,24 @@ public void testPrintExotic() throws Exception {
     assertEquals(canonicalExoticText, message.toString());
   }
 
+  public void testRoundtripProto3Optional() throws Exception {
+    Message message =
+        TestProto3Optional.newBuilder()
+            .setOptionalInt32(1)
+            .setOptionalInt64(2)
+            .setOptionalNestedEnum(NestedEnum.BAZ)
+            .build();
+    TestProto3Optional.Builder message2 = TestProto3Optional.newBuilder();
+    TextFormat.merge(message.toString(), message2);
+
+    assertTrue(message2.hasOptionalInt32());
+    assertTrue(message2.hasOptionalInt64());
+    assertTrue(message2.hasOptionalNestedEnum());
+    assertEquals(1, message2.getOptionalInt32());
+    assertEquals(2, message2.getOptionalInt64());
+    assertEquals(NestedEnum.BAZ, message2.getOptionalNestedEnum());
+  }
+
   public void testPrintMessageSet() throws Exception {
     TestMessageSet messageSet =
         TestMessageSet.newBuilder()

diff --git a/java/lite/generate-test-sources-build.xml b/java/lite/generate-test-sources-build.xml
@@ -15,6 +15,7 @@
         <arg value="${protobuf.source.dir}/google/protobuf/unittest_no_generic_services.proto"/>
         <arg value="${protobuf.source.dir}/google/protobuf/unittest_optimize_for.proto"/>
         <arg value="${protobuf.source.dir}/google/protobuf/unittest_proto3.proto"/>
+        <arg value="${protobuf.source.dir}/google/protobuf/unittest_proto3_optional.proto"/>
         <arg value="${protobuf.source.dir}/google/protobuf/unittest_well_known_types.proto"/>
         <arg value="${protobuf.basedir}/java/core/${test.proto.dir}/com/google/protobuf/any_test.proto"/>
         <arg value="${protobuf.basedir}/java/core/${test.proto.dir}/com/google/protobuf/cached_field_size_test.proto"/>

diff --git a/python/google/protobuf/internal/test_proto3_optional.proto b/python/google/protobuf/internal/test_proto3_optional.proto
@@ -30,10 +30,7 @@
 
 syntax = "proto3";
 
-package protobuf_unittest;
-
-option java_multiple_files = true;
-option java_package = "com.google.protobuf.testing.proto";
+package google.protobuf.python.internal;
 
 message TestProto3Optional {
   message NestedMessage {

diff --git a/src/google/protobuf/compiler/java/java_enum_field.cc b/src/google/protobuf/compiler/java/java_enum_field.cc
@@ -225,6 +225,7 @@ void ImmutableEnumFieldGenerator::GenerateBuilderMembers(
     printer->Print(variables_,
                    "$deprecation$public Builder "
                    "${$set$capitalized_name$Value$}$(int value) {\n"
+                   "  $set_has_field_bit_builder$\n"
                    "  $name$_ = value;\n"
                    "  $on_changed$\n"
                    "  return this;\n"