Skip to content

Commit

Permalink
Reparse unknown features using extension registry containing Java fea…
Browse files Browse the repository at this point in the history 
…tures.

This should not be needed for generated code, but may be needed for user calls to the public buildFrom method. FileDescriptorProto should really be parsed with java features in the extension registry (like other extensions), but we can handle this in Java runtime to ease editions adoption.

PiperOrigin-RevId: 638715579
  • Loading branch information
@zhangskz @copybara-github
zhangskz authored and copybara-github committed May 30, 2024
1 parent 023e69d commit 68d9087
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 1 deletion.
15 changes: 15 additions & 0 deletions java/core/src/main/java/com/google/protobuf/Descriptors.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2782,6 +2782,21 @@ private GenericDescriptor() {}
public abstract FileDescriptor getFile();

void resolveFeatures(FeatureSet unresolvedFeatures) throws DescriptorValidationException {
// Unknown java features may be passed by users via public buildFrom but should not occur from
// generated code.
if (!unresolvedFeatures.getUnknownFields().isEmpty()
&& unresolvedFeatures.getUnknownFields().hasField(JavaFeaturesProto.java_.getNumber())) {
ExtensionRegistry registry = ExtensionRegistry.newInstance();
registry.add(JavaFeaturesProto.java_);
ByteString bytes = unresolvedFeatures.toByteString();
try {
unresolvedFeatures = FeatureSet.parseFrom(bytes, registry);
} catch (InvalidProtocolBufferException e) {
throw new DescriptorValidationException(
this, "Failed to parse features with Java feature extension registry.", e);
}
}

if (this.parent != null
&& unresolvedFeatures.equals(FeatureSet.getDefaultInstance())
&& !hasInferredLegacyProtoFeatures()) {
Expand Down
107 changes: 106 additions & 1 deletion java/core/src/test/java/com/google/protobuf/DescriptorsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ public void testFieldDescriptorDefault() throws Exception {
}

@Test
public void testFieldDescriptorLegacyEnumFieldTreatedAsClosed() throws Exception {
public void testProto2FieldDescriptorLegacyEnumFieldTreatedAsClosed() throws Exception {
// Make an open enum definition.
FileDescriptorProto openEnumFile =
FileDescriptorProto.newBuilder()
Expand Down Expand Up @@ -430,6 +430,111 @@ public void testFieldDescriptorLegacyEnumFieldTreatedAsClosed() throws Exception
.isTrue();
}

@Test
public void testEditionFieldDescriptorLegacyEnumFieldTreatedAsClosed() throws Exception {
// Make an open enum definition.
FileDescriptorProto openEnumFile =
FileDescriptorProto.newBuilder()
.setName("open_enum.proto")
.setSyntax("editions")
.setEdition(Edition.EDITION_2023)
.addEnumType(
EnumDescriptorProto.newBuilder()
.setName("TestEnumOpen")
.addValue(
EnumValueDescriptorProto.newBuilder()
.setName("TestEnumOpen_VALUE0")
.setNumber(0)
.build())
.build())
.build();
FileDescriptor openFileDescriptor =
Descriptors.FileDescriptor.buildFrom(openEnumFile, new FileDescriptor[0]);
EnumDescriptor openEnum = openFileDescriptor.getEnumTypes().get(0);
assertThat(openEnum.isClosed()).isFalse();

// Create a message that treats enum fields as closed.
FileDescriptorProto editionsClosedEnumFile =
FileDescriptorProto.newBuilder()
.setName("editions_closed_enum_field.proto")
.addDependency("open_enum.proto")
.setSyntax("editions")
.setEdition(Edition.EDITION_2023)
.setOptions(
FileOptions.newBuilder()
.setFeatures(
DescriptorProtos.FeatureSet.newBuilder()
.setEnumType(DescriptorProtos.FeatureSet.EnumType.CLOSED)
.build())
.build())
.addEnumType(
EnumDescriptorProto.newBuilder()
.setName("TestEnum")
.addValue(
EnumValueDescriptorProto.newBuilder()
.setName("TestEnum_VALUE0")
.setNumber(0)
.build())
.build())
.addMessageType(
DescriptorProto.newBuilder()
.setName("TestClosedEnumField")
.addField(
FieldDescriptorProto.newBuilder()
.setName("int_field")
.setNumber(1)
.setType(FieldDescriptorProto.Type.TYPE_INT32)
.setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL)
.build())
.addField(
FieldDescriptorProto.newBuilder()
.setName("open_enum")
.setNumber(2)
.setType(FieldDescriptorProto.Type.TYPE_ENUM)
.setTypeName("TestEnumOpen")
.setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL)
.setOptions(
DescriptorProtos.FieldOptions.newBuilder()
.setFeatures(
DescriptorProtos.FeatureSet.newBuilder()
.setExtension(
JavaFeaturesProto.java_,
JavaFeaturesProto.JavaFeatures.newBuilder()
.setLegacyClosedEnum(true)
.build())
.build())
.build())
.build())
.addField(
FieldDescriptorProto.newBuilder()
.setName("closed_enum")
.setNumber(3)
.setType(FieldDescriptorProto.Type.TYPE_ENUM)
.setTypeName("TestEnum")
.setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL)
.build())
.build())
.build();
// Ensure Java features are in unknown fields.
editionsClosedEnumFile =
FileDescriptorProto.parseFrom(
editionsClosedEnumFile.toByteString(), ExtensionRegistry.getEmptyRegistry());
Descriptor editionsClosedMessage =
Descriptors.FileDescriptor.buildFrom(
editionsClosedEnumFile, new FileDescriptor[] {openFileDescriptor})
.getMessageTypes()
.get(0);
assertThat(
editionsClosedMessage.findFieldByName("int_field").legacyEnumFieldTreatedAsClosed())
.isFalse();
assertThat(
editionsClosedMessage.findFieldByName("closed_enum").legacyEnumFieldTreatedAsClosed())
.isTrue();
assertThat(
editionsClosedMessage.findFieldByName("open_enum").legacyEnumFieldTreatedAsClosed())
.isTrue();
}

@Test
public void testFieldDescriptorLegacyEnumFieldTreatedAsOpen() throws Exception {
// Make an open enum definition and message that treats enum fields as open.
Expand Down

0 comments on commit 68d9087

Please sign in to comment.