Chiebot-Mirror
/
protobuf
mirror of https://github.com/protocolbuffers/protobuf.git
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

Fix features inheritance of oneof fields and extensions and fix/move unit tests to actually run.

Browse Source 
JUnit4 does not support nested tests so these weren't running. Fixes setup problems and test logic. Oneof fields now inherit from their oneof, and top-level extensions inherit from top-level file when parent descriptor is null.

PiperOrigin-RevId: 609840087
pull/15928/head
Sandy Zhang 1 year ago committed by Copybara-Service
parent baaf402b29
commit eb10ebd169
2 changed files with 1117 additions and 1074 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 27
      java/core/src/main/java/com/google/protobuf/Descriptors.java
  2. 144
      java/core/src/test/java/com/google/protobuf/DescriptorsTest.java

27
java/core/src/main/java/com/google/protobuf/Descriptors.java
Unescape View File

@ -75,7 +75,13 @@ public final class Descriptors {
@SuppressWarnings("NonFinalStaticField")
private static volatile FeatureSetDefaults javaEditionDefaults = null;
private static FeatureSet getEditionDefaults(Edition edition) {
/** Sets the default feature mappings used during the build. Exposed for tests. */
static void setTestJavaEditionDefaults(FeatureSetDefaults defaults) {
javaEditionDefaults = defaults;
}
/** Gets the default feature mappings used during the build. */
static FeatureSetDefaults getJavaEditionDefaults() {
// Force explicit initialization before synchronized block which can trigger initialization in
// `JavaFeaturesProto.registerAllExtensions()` and `FeatureSetdefaults.parseFrom()` calls.
// Otherwise, this can result in deadlock if another threads holds the static init block's
@ -88,18 +94,22 @@ public final class Descriptors {
try {
ExtensionRegistry registry = ExtensionRegistry.newInstance();
registry.add(JavaFeaturesProto.java);
javaEditionDefaults =
setTestJavaEditionDefaults(
FeatureSetDefaults.parseFrom(
JavaEditionDefaults.PROTOBUF_INTERNAL_JAVA_EDITION_DEFAULTS.getBytes(
Internal.ISO_8859_1),
registry);
registry));
} catch (Exception e) {
throw new AssertionError(e);
}
}
}
}
return javaEditionDefaults;
}
static FeatureSet getEditionDefaults(Edition edition) {
FeatureSetDefaults javaEditionDefaults = getJavaEditionDefaults();
if (edition.getNumber() < javaEditionDefaults.getMinimumEdition().getNumber()) {
throw new IllegalArgumentException(
"Edition "
@ -1116,6 +1126,11 @@ public final class Descriptors {
enumType.resolveAllFeatures();
}
// Oneofs must be resolved before any children oneof fields.
for (OneofDescriptor oneof : oneofs) {
oneof.resolveAllFeatures();
}
for (FieldDescriptor field : fields) {
field.resolveAllFeatures();
}
@ -1123,10 +1138,6 @@ public final class Descriptors {
for (FieldDescriptor extension : extensions) {
extension.resolveAllFeatures();
}
for (OneofDescriptor oneof : oneofs) {
oneof.resolveAllFeatures();
}
}
/** Look up and cross-link all field types, etc. */
@ -1703,6 +1714,7 @@ public final class Descriptors {
extensionScope = parent;
} else {
extensionScope = null;
this.parent = file;
}
if (proto.hasOneofIndex()) {
@ -1726,6 +1738,7 @@ public final class Descriptors {
}
containingOneof = parent.getOneofs().get(proto.getOneofIndex());
containingOneof.fieldCount++;
this.parent = containingOneof;
} else {
containingOneof = null;
}

144
java/core/src/test/java/com/google/protobuf/DescriptorsTest.java
Unescape View File

@ -12,9 +12,12 @@ import static com.google.common.truth.Truth.assertWithMessage;
import static org.junit.Assert.assertThrows;
import com.google.protobuf.DescriptorProtos.DescriptorProto;
import com.google.protobuf.DescriptorProtos.DescriptorProto.ExtensionRange;
import com.google.protobuf.DescriptorProtos.Edition;
import com.google.protobuf.DescriptorProtos.EnumDescriptorProto;
import com.google.protobuf.DescriptorProtos.EnumValueDescriptorProto;
import com.google.protobuf.DescriptorProtos.FeatureSetDefaults;
import com.google.protobuf.DescriptorProtos.FeatureSetDefaults.FeatureSetEditionDefault;
import com.google.protobuf.DescriptorProtos.FieldDescriptorProto;
import com.google.protobuf.DescriptorProtos.FileDescriptorProto;
import com.google.protobuf.DescriptorProtos.FileOptions;
@ -55,15 +58,17 @@ import java.util.Collections;
import java.util.List;
import org.junit.Before;
import org.junit.Test;
import org.junit.experimental.runners.Enclosed;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import protobuf_unittest.NestedExtension;
import protobuf_unittest.NonNestedExtension;
/** Unit test for {@link Descriptors}. */
@RunWith(JUnit4.class)
@RunWith(Enclosed.class)
public class DescriptorsTest {
public static class GeneralDescriptorsTest {
// Regression test for bug where referencing a FieldDescriptor.Type value
// before a FieldDescriptorProto.Type value would yield a
// ExceptionInInitializerError.
@ -72,7 +77,8 @@ public class DescriptorsTest {
@Test
public void testFieldTypeEnumMapping() throws Exception {
assertThat(FieldDescriptor.Type.values()).hasLength(FieldDescriptorProto.Type.values().length);
assertThat(FieldDescriptor.Type.values())
.hasLength(FieldDescriptorProto.Type.values().length);
for (FieldDescriptor.Type type : FieldDescriptor.Type.values()) {
FieldDescriptorProto.Type protoType = type.toProto();
assertThat(protoType.name()).isEqualTo("TYPE_" + type.name());
@ -157,7 +163,8 @@ public class DescriptorsTest {
IllegalArgumentException exception =
assertThrows(
IllegalArgumentException.class,
() -> Descriptors.FileDescriptor.buildFrom(protoMissingEdition, new FileDescriptor[0]));
() ->
Descriptors.FileDescriptor.buildFrom(protoMissingEdition, new FileDescriptor[0]));
assertThat(exception)
.hasMessageThat()
.contains("Edition EDITION_UNKNOWN is lower than the minimum supported edition");
@ -301,8 +308,10 @@ public class DescriptorsTest {
@Test
public void testFieldDescriptorLabel() throws Exception {
FieldDescriptor requiredField = TestRequired.getDescriptor().findFieldByName("a");
FieldDescriptor optionalField = TestAllTypes.getDescriptor().findFieldByName("optional_int32");
FieldDescriptor repeatedField = TestAllTypes.getDescriptor().findFieldByName("repeated_int32");
FieldDescriptor optionalField =
TestAllTypes.getDescriptor().findFieldByName("optional_int32");
FieldDescriptor repeatedField =
TestAllTypes.getDescriptor().findFieldByName("repeated_int32");
assertThat(requiredField.isRequired()).isTrue();
assertThat(requiredField.isRepeated()).isFalse();
@ -315,8 +324,10 @@ public class DescriptorsTest {
@Test
public void testFieldDescriptorJsonName() throws Exception {
FieldDescriptor requiredField = TestRequired.getDescriptor().findFieldByName("a");
FieldDescriptor optionalField = TestAllTypes.getDescriptor().findFieldByName("optional_int32");
FieldDescriptor repeatedField = TestAllTypes.getDescriptor().findFieldByName("repeated_int32");
FieldDescriptor optionalField =
TestAllTypes.getDescriptor().findFieldByName("optional_int32");
FieldDescriptor repeatedField =
TestAllTypes.getDescriptor().findFieldByName("repeated_int32");
assertThat(requiredField.getJsonName()).isEqualTo("a");
assertThat(optionalField.getJsonName()).isEqualTo("optionalInt32");
assertThat(repeatedField.getJsonName()).isEqualTo("repeatedInt32");
@ -459,8 +470,10 @@ public class DescriptorsTest {
Descriptor openMessage = openEnumFileDescriptor.getMessageTypes().get(0);
EnumDescriptor openEnum = openEnumFileDescriptor.findEnumTypeByName("TestEnumOpen");
assertThat(openEnum.isClosed()).isFalse();
assertThat(openMessage.findFieldByName("int_field").legacyEnumFieldTreatedAsClosed()).isFalse();
assertThat(openMessage.findFieldByName("open_enum").legacyEnumFieldTreatedAsClosed()).isFalse();
assertThat(openMessage.findFieldByName("int_field").legacyEnumFieldTreatedAsClosed())
.isFalse();
assertThat(openMessage.findFieldByName("open_enum").legacyEnumFieldTreatedAsClosed())
.isFalse();
}
@Test
@ -473,7 +486,8 @@ public class DescriptorsTest {
assertThat(enumType.getFile()).isEqualTo(UnittestProto.getDescriptor());
assertThat(enumType.isClosed()).isTrue();
assertThat(enumType.getContainingType()).isNull();
assertThat(enumType.getOptions()).isEqualTo(DescriptorProtos.EnumOptions.getDefaultInstance());
assertThat(enumType.getOptions())
.isEqualTo(DescriptorProtos.EnumOptions.getDefaultInstance());
assertThat(nestedType.getName()).isEqualTo("NestedEnum");
assertThat(nestedType.getFullName()).isEqualTo("protobuf_unittest.TestAllTypes.NestedEnum");
@ -555,7 +569,8 @@ public class DescriptorsTest {
assertThat(enumType.getOptions().getExtension(UnittestCustomOptions.enumOpt1))
.isEqualTo(Integer.valueOf(-789));
ServiceDescriptor service = UnittestCustomOptions.TestServiceWithCustomOptions.getDescriptor();
ServiceDescriptor service =
UnittestCustomOptions.TestServiceWithCustomOptions.getDescriptor();
assertThat(service.getOptions().hasExtension(UnittestCustomOptions.serviceOpt1)).isTrue();
assertThat(service.getOptions().getExtension(UnittestCustomOptions.serviceOpt1))
@ -672,8 +687,8 @@ public class DescriptorsTest {
}
/**
* Tests the translate/crosslink for an example where a message field's name and type name are the
* same.
* Tests the translate/crosslink for an example where a message field's name and type name are
* the same.
*/
@Test
public void testDescriptorComplexCrosslink() throws Exception {
@ -739,7 +754,8 @@ public class DescriptorsTest {
.addPublicDependency(0)
.addPublicDependency(1)
.build();
FileDescriptor fooFile = Descriptors.FileDescriptor.buildFrom(fooProto, new FileDescriptor[0]);
FileDescriptor fooFile =
Descriptors.FileDescriptor.buildFrom(fooProto, new FileDescriptor[0]);
FileDescriptor barFile =
Descriptors.FileDescriptor.buildFrom(barProto, new FileDescriptor[] {fooFile});
@ -759,7 +775,8 @@ public class DescriptorsTest {
.addDependency("foo.proto")
.addPublicDependency(1) // Error, should be 0.
.build();
FileDescriptor fooFile = Descriptors.FileDescriptor.buildFrom(fooProto, new FileDescriptor[0]);
FileDescriptor fooFile =
Descriptors.FileDescriptor.buildFrom(fooProto, new FileDescriptor[0]);
try {
Descriptors.FileDescriptor.buildFrom(barProto, new FileDescriptor[] {fooFile});
assertWithMessage("DescriptorValidationException expected").fail();
@ -839,7 +856,8 @@ public class DescriptorsTest {
.setName("bar")
.setNumber(1)))
.build();
FileDescriptor barFile = Descriptors.FileDescriptor.buildFrom(barProto, new FileDescriptor[0]);
FileDescriptor barFile =
Descriptors.FileDescriptor.buildFrom(barProto, new FileDescriptor[0]);
FileDescriptor forwardFile =
Descriptors.FileDescriptor.buildFrom(forwardProto, new FileDescriptor[] {barFile});
@ -880,7 +898,8 @@ public class DescriptorsTest {
.setName("bar")
.setNumber(1)))
.build();
FileDescriptor barFile = Descriptors.FileDescriptor.buildFrom(barProto, new FileDescriptor[0]);
FileDescriptor barFile =
Descriptors.FileDescriptor.buildFrom(barProto, new FileDescriptor[0]);
FileDescriptor forwardFile =
Descriptors.FileDescriptor.buildFrom(forwardProto, new FileDescriptor[] {barFile});
FileDescriptor unused =
@ -915,7 +934,8 @@ public class DescriptorsTest {
.setNumber(1)))
.build();
// translate and crosslink
FileDescriptor fooFile = Descriptors.FileDescriptor.buildFrom(fooProto, new FileDescriptor[0]);
FileDescriptor fooFile =
Descriptors.FileDescriptor.buildFrom(fooProto, new FileDescriptor[0]);
FileDescriptor barFile =
Descriptors.FileDescriptor.buildFrom(barProto, new FileDescriptor[] {fooFile});
// verify resulting descriptors
@ -933,7 +953,8 @@ public class DescriptorsTest {
assertThat(field.getType()).isSameInstanceAs(FieldDescriptor.Type.ENUM);
assertThat(field.getEnumType().getName().equals("MyEnum")).isTrue();
assertThat(field.getEnumType().getFile().getName().equals("bar.proto")).isTrue();
assertThat(field.getEnumType().getFile().getPackage().equals("a.b.c.d.bar.shared")).isTrue();
assertThat(field.getEnumType().getFile().getPackage().equals("a.b.c.d.bar.shared"))
.isTrue();
}
}
@ -1025,7 +1046,9 @@ public class DescriptorsTest {
.setNumber(1)
.setLabel(FieldDescriptorProto.Label.LABEL_REPEATED)
.setOptions(
DescriptorProtos.FieldOptions.newBuilder().setPacked(true).build())
DescriptorProtos.FieldOptions.newBuilder()
.setPacked(true)
.build())
.build())
.build())
.build();
@ -1178,8 +1201,8 @@ public class DescriptorsTest {
assertThat(features.getExtension(JavaFeaturesProto.java).getUtf8Validation())
.isEqualTo(JavaFeaturesProto.JavaFeatures.Utf8Validation.DEFAULT);
}
}
@RunWith(JUnit4.class)
public static class FeatureInheritanceTest {
FileDescriptorProto.Builder fileProto;
FieldDescriptorProto.Builder topExtensionProto;
@ -1197,6 +1220,12 @@ public class DescriptorsTest {
@Before
public void setUp() {
FeatureSetDefaults.Builder defaults = Descriptors.getJavaEditionDefaults().toBuilder();
for (FeatureSetEditionDefault.Builder editionDefaults : defaults.getDefaultsBuilderList()) {
setTestFeature(editionDefaults.getFeaturesBuilder(), 1);
}
Descriptors.setTestJavaEditionDefaults(defaults.build());
this.fileProto =
DescriptorProtos.FileDescriptorProto.newBuilder()
.setName("some/filename/some.proto")
@ -1205,59 +1234,61 @@ public class DescriptorsTest {
.setSyntax("editions");
this.topExtensionProto =
FieldDescriptorProto.newBuilder()
this.fileProto
.addExtensionBuilder()
.setName("top_extension")
.setNumber(10)
.setType(FieldDescriptorProto.Type.TYPE_INT32)
.setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL)
.setExtendee(".protobuf_unittest.TopMessage");
this.fileProto.addExtension(topExtensionProto);
this.topEnumProto = EnumDescriptorProto.newBuilder().setName("TopEnum");
this.enumValueProto = EnumValueDescriptorProto.newBuilder().setName("TOP_VALUE").setNumber(0);
this.topEnumProto.addValue(enumValueProto);
this.fileProto.addEnumType(topEnumProto);
this.topEnumProto = this.fileProto.addEnumTypeBuilder().setName("TopEnum");
this.enumValueProto = this.topEnumProto.addValueBuilder().setName("TOP_VALUE").setNumber(0);
this.topMessageProto =
this.fileProto
.addMessageTypeBuilder()
.setName("TopMessage")
.addExtensionRange(ExtensionRange.newBuilder().setStart(10).setEnd(20).build());
this.topMessageProto = DescriptorProto.newBuilder().setName("TopMessage");
this.fieldProto =
FieldDescriptorProto.newBuilder()
this.topMessageProto
.addFieldBuilder()
.setName("field")
.setNumber(1)
.setType(FieldDescriptorProto.Type.TYPE_INT32)
.setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL);
this.topMessageProto.addField(fieldProto);
this.nestedExtensionProto =
FieldDescriptorProto.newBuilder()
this.topMessageProto
.addExtensionBuilder()
.setName("nested_extension")
.setNumber(11)
.setType(FieldDescriptorProto.Type.TYPE_INT32)
.setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL)
.setExtendee(".protobuf_unittest.TopMessage");
this.topMessageProto.addExtension(nestedExtensionProto);
this.nestedMessageProto = DescriptorProto.newBuilder().setName("NestedMessage");
this.topMessageProto.addNestedType(nestedMessageProto);
this.nestedEnumProto = EnumDescriptorProto.newBuilder().setName("NestedEnum");
this.nestedEnumProto.addValue(
EnumValueDescriptorProto.newBuilder().setName("NESTED_VALUE").setNumber(0));
this.topMessageProto.addEnumType(nestedEnumProto);
this.topMessageProto.addOneofDecl(OneofDescriptorProto.newBuilder().setName("Oneof"));
this.topMessageProto.addField(
FieldDescriptorProto.newBuilder()
this.nestedMessageProto =
this.topMessageProto.addNestedTypeBuilder().setName("NestedMessage");
this.nestedEnumProto =
this.topMessageProto
.addEnumTypeBuilder()
.setName("NestedEnum")
.addValue(EnumValueDescriptorProto.newBuilder().setName("NESTED_VALUE").setNumber(0));
this.oneofProto = this.topMessageProto.addOneofDeclBuilder().setName("Oneof");
this.oneofFieldProto =
this.topMessageProto
.addFieldBuilder()
.setName("oneof_field")
.setNumber(2)
.setType(FieldDescriptorProto.Type.TYPE_INT32)
.setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL)
.setOneofIndex(0));
this.fileProto.addMessageType(topMessageProto);
this.serviceProto = ServiceDescriptorProto.newBuilder().setName("TestService");
.setOneofIndex(0);
this.serviceProto = this.fileProto.addServiceBuilder().setName("TestService");
this.methodProto =
MethodDescriptorProto.newBuilder()
this.serviceProto
.addMethodBuilder()
.setName("CallMethod")
.setInputType(".protobuf_unittest.TopMessage")
.setOutputType(".protobuf_unittest.TopMessage");
this.serviceProto.addMethod(methodProto);
this.fileProto.addService(serviceProto);
}
void setTestFeature(DescriptorProtos.FeatureSet.Builder features, int value) {
@ -1308,7 +1339,6 @@ public class DescriptorsTest {
public void testFileEnumInherit() throws Exception {
setTestFeature(fileProto.getOptionsBuilder().getFeaturesBuilder(), 3);
FileDescriptor descriptor = buildFrom(fileProto.build());
assertThat(getTestFeature(descriptor.getEnumTypes().get(0).features)).isEqualTo(3);
}
@ -1360,7 +1390,7 @@ public class DescriptorsTest {
@Test
public void testMessageFieldOverride() throws Exception {
setTestFeature(fileProto.getOptionsBuilder().getFeaturesBuilder(), 3);
setTestFeature(topMessageProto.getOptionsBuilder().getFeaturesBuilder(), 3);
setTestFeature(fieldProto.getOptionsBuilder().getFeaturesBuilder(), 5);
FileDescriptor descriptor = buildFrom(fileProto.build());
assertThat(getTestFeature(descriptor.getMessageTypes().get(0).getFields().get(0).features))
@ -1377,7 +1407,7 @@ public class DescriptorsTest {
@Test
public void testMessageEnumOverride() throws Exception {
setTestFeature(fileProto.getOptionsBuilder().getFeaturesBuilder(), 3);
setTestFeature(topMessageProto.getOptionsBuilder().getFeaturesBuilder(), 3);
setTestFeature(nestedEnumProto.getOptionsBuilder().getFeaturesBuilder(), 5);
FileDescriptor descriptor = buildFrom(fileProto.build());
assertThat(getTestFeature(descriptor.getMessageTypes().get(0).getEnumTypes().get(0).features))
@ -1441,7 +1471,7 @@ public class DescriptorsTest {
@Test
public void testOneofFieldInherit() throws Exception {
setTestFeature(topMessageProto.getOptionsBuilder().getFeaturesBuilder(), 3);
setTestFeature(oneofProto.getOptionsBuilder().getFeaturesBuilder(), 3);
FileDescriptor descriptor = buildFrom(fileProto.build());
assertThat(
getTestFeature(
@ -1458,8 +1488,8 @@ public class DescriptorsTest {
@Test
public void testOneofFieldOverride() throws Exception {
setTestFeature(topMessageProto.getOptionsBuilder().getFeaturesBuilder(), 3);
setTestFeature(oneofProto.getOptionsBuilder().getFeaturesBuilder(), 5);
setTestFeature(oneofProto.getOptionsBuilder().getFeaturesBuilder(), 3);
setTestFeature(oneofFieldProto.getOptionsBuilder().getFeaturesBuilder(), 5);
FileDescriptor descriptor = buildFrom(fileProto.build());
assertThat(
getTestFeature(
@ -1476,7 +1506,7 @@ public class DescriptorsTest {
@Test
public void testEnumValueInherit() throws Exception {
setTestFeature(topMessageProto.getOptionsBuilder().getFeaturesBuilder(), 3);
setTestFeature(topEnumProto.getOptionsBuilder().getFeaturesBuilder(), 3);
FileDescriptor descriptor = buildFrom(fileProto.build());
assertThat(getTestFeature(descriptor.getEnumTypes().get(0).getValues().get(0).features))
.isEqualTo(3);
@ -1484,7 +1514,7 @@ public class DescriptorsTest {
@Test
public void testEnumValueOverride() throws Exception {
setTestFeature(topMessageProto.getOptionsBuilder().getFeaturesBuilder(), 3);
setTestFeature(topEnumProto.getOptionsBuilder().getFeaturesBuilder(), 3);
setTestFeature(enumValueProto.getOptionsBuilder().getFeaturesBuilder(), 5);
FileDescriptor descriptor = buildFrom(fileProto.build());
assertThat(getTestFeature(descriptor.getEnumTypes().get(0).getValues().get(0).features))

Write Preview
Loading…
Cancel
Save