Skip to content

Commit

Permalink
Merge pull request #17161 from protocolbuffers/backport-java
Browse files Browse the repository at this point in the history 
Backport handling of unknown and custom java features to 27.x
  • Loading branch information
@zhangskz
zhangskz authored Jun 17, 2024
2 parents 86768b3 + c7a006a commit 6c6f514
Show file tree
Hide file tree
Showing 2 changed files with 196 additions and 23 deletions.
28 changes: 28 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 @@ -2792,6 +2792,34 @@ void resolveFeatures(FeatureSet unresolvedFeatures) throws DescriptorValidationE
validateFeatures();
return;
}

// Java features from a custom pool (i.e. buildFrom) may end up in unknown fields or
// use a different descriptor from the generated pool used by the Java runtime.
boolean hasPossibleCustomJavaFeature = false;
for (FieldDescriptor f : unresolvedFeatures.getExtensionFields().keySet()) {
if (f.getNumber() == JavaFeaturesProto.java_.getNumber()
&& f != JavaFeaturesProto.java_.getDescriptor()) {
hasPossibleCustomJavaFeature = true;
continue;
}
}
boolean hasPossibleUnknownJavaFeature =
!unresolvedFeatures.getUnknownFields().asMap().isEmpty()
&& unresolvedFeatures
.getUnknownFields()
.hasField(JavaFeaturesProto.java_.getNumber());
if (hasPossibleCustomJavaFeature || hasPossibleUnknownJavaFeature) {
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);
}
}

FeatureSet.Builder features;
if (this.parent == null) {
Edition edition = getFile().getEdition();
Expand Down
191 changes: 168 additions & 23 deletions 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,12 +352,61 @@ public void testFieldDescriptorDefault() throws Exception {
}

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

FileDescriptorProto openEnumFile =
FileDescriptorProto.newBuilder()
.setName("open_enum.proto")
.setSyntax("proto3")
.addEnumType(
EnumDescriptorProto.newBuilder()
.setName("TestEnumOpen")
.addValue(
EnumValueDescriptorProto.newBuilder()
.setName("TestEnumOpen_VALUE0")
.setNumber(0)
.build())
.build())
.addMessageType(
DescriptorProto.newBuilder()
.setName("TestOpenEnumField")
.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)
.build())
.build())
.build();
FileDescriptor openEnumFileDescriptor =
Descriptors.FileDescriptor.buildFrom(openEnumFile, new FileDescriptor[0]);
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();
}

@Test
public void testEditionFieldDescriptorLegacyEnumFieldTreatedAsClosedUnknown() 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")
Expand All @@ -374,11 +423,19 @@ public void testFieldDescriptorLegacyEnumFieldTreatedAsClosed() throws Exception
assertThat(openEnum.isClosed()).isFalse();

// Create a message that treats enum fields as closed.
FileDescriptorProto closedEnumFile =
FileDescriptorProto editionsClosedEnumFile =
FileDescriptorProto.newBuilder()
.setName("closed_enum_field.proto")
.setName("editions_closed_enum_field.proto")
.addDependency("open_enum.proto")
.setSyntax("proto2")
.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")
Expand All @@ -405,6 +462,17 @@ public void testFieldDescriptorLegacyEnumFieldTreatedAsClosed() throws Exception
.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()
Expand All @@ -416,27 +484,40 @@ public void testFieldDescriptorLegacyEnumFieldTreatedAsClosed() throws Exception
.build())
.build())
.build();
Descriptor closedMessage =
// Ensure Java features are in unknown fields.
editionsClosedEnumFile =
FileDescriptorProto.parseFrom(
editionsClosedEnumFile.toByteString(), ExtensionRegistry.getEmptyRegistry());
Descriptor editionsClosedMessage =
Descriptors.FileDescriptor.buildFrom(
closedEnumFile, new FileDescriptor[] {openFileDescriptor})
editionsClosedEnumFile, new FileDescriptor[] {openFileDescriptor})
.getMessageTypes()
.get(0);
assertThat(closedMessage.findFieldByName("int_field").legacyEnumFieldTreatedAsClosed())
assertThat(
editionsClosedMessage.findFieldByName("int_field").legacyEnumFieldTreatedAsClosed())
.isFalse();

assertThat(closedMessage.findFieldByName("closed_enum").legacyEnumFieldTreatedAsClosed())
assertThat(
editionsClosedMessage.findFieldByName("closed_enum").legacyEnumFieldTreatedAsClosed())
.isTrue();
assertThat(closedMessage.findFieldByName("open_enum").legacyEnumFieldTreatedAsClosed())
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.
public void testEditionFieldDescriptorLegacyEnumFieldTreatedAsClosedCustomPool()
throws Exception {

FileDescriptor javaFeaturesDescriptor =
Descriptors.FileDescriptor.buildFrom(
JavaFeaturesProto.getDescriptor().toProto(),
new FileDescriptor[] {DescriptorProtos.getDescriptor()});
// Make an open enum definition.
FileDescriptorProto openEnumFile =
FileDescriptorProto.newBuilder()
.setName("open_enum.proto")
.setSyntax("proto3")
.setSyntax("editions")
.setEdition(Edition.EDITION_2023)
.addEnumType(
EnumDescriptorProto.newBuilder()
.setName("TestEnumOpen")
Expand All @@ -446,9 +527,38 @@ public void testFieldDescriptorLegacyEnumFieldTreatedAsOpen() throws Exception {
.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("TestOpenEnumField")
.setName("TestClosedEnumField")
.addField(
FieldDescriptorProto.newBuilder()
.setName("int_field")
Expand All @@ -463,18 +573,53 @@ public void testFieldDescriptorLegacyEnumFieldTreatedAsOpen() throws Exception {
.setType(FieldDescriptorProto.Type.TYPE_ENUM)
.setTypeName("TestEnumOpen")
.setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL)
.setOptions(
DescriptorProtos.FieldOptions.newBuilder()
.setFeatures(
DescriptorProtos.FeatureSet.newBuilder()
.setExtension(
// Extension cannot be directly set using custom
// descriptor, so set using generated for now.
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();
FileDescriptor openEnumFileDescriptor =
Descriptors.FileDescriptor.buildFrom(openEnumFile, new FileDescriptor[0]);
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())
// Reparse using custom java features descriptor.
ExtensionRegistry registry = ExtensionRegistry.newInstance();
registry.add(
javaFeaturesDescriptor.getExtensions().get(0),
DynamicMessage.getDefaultInstance(
javaFeaturesDescriptor.getExtensions().get(0).getMessageType()));
editionsClosedEnumFile =
FileDescriptorProto.parseFrom(editionsClosedEnumFile.toByteString(), registry);
Descriptor editionsClosedMessage =
Descriptors.FileDescriptor.buildFrom(
editionsClosedEnumFile,
new FileDescriptor[] {openFileDescriptor, javaFeaturesDescriptor})
.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
Expand Down

0 comments on commit 6c6f514

Please sign in to comment.