From 68d908768fc19a14ea2db39a24a8c04a23e29541 Mon Sep 17 00:00:00 2001 From: Sandy Zhang Date: Thu, 30 May 2024 11:08:35 -0700 Subject: [PATCH] Reparse unknown features using extension registry containing Java features. 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 --- .../java/com/google/protobuf/Descriptors.java | 15 +++ .../com/google/protobuf/DescriptorsTest.java | 107 +++++++++++++++++- 2 files changed, 121 insertions(+), 1 deletion(-) diff --git a/java/core/src/main/java/com/google/protobuf/Descriptors.java b/java/core/src/main/java/com/google/protobuf/Descriptors.java index 1a81fe6f5058..1c5a75c3e806 100644 --- a/java/core/src/main/java/com/google/protobuf/Descriptors.java +++ b/java/core/src/main/java/com/google/protobuf/Descriptors.java @@ -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()) { diff --git a/java/core/src/test/java/com/google/protobuf/DescriptorsTest.java b/java/core/src/test/java/com/google/protobuf/DescriptorsTest.java index 2ebdf71e75de..508344d3bfc1 100644 --- a/java/core/src/test/java/com/google/protobuf/DescriptorsTest.java +++ b/java/core/src/test/java/com/google/protobuf/DescriptorsTest.java @@ -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() @@ -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.