Replace FileDescriptor.supportsUnknownEnumValues() with FieldDescriptor.legacyEnumFieldTreatedAsClosed() to support legacy enum field behavior.

This documents Java's non-conformant behavior where closedness is determined by the field rather than the enum definition.

PiperOrigin-RevId: 512627297
pull/11951/head
Sandy Zhang 2 years ago committed by Copybara-Service
parent c0bd3dd984
commit 343cef1bd9
  1. 28
      java/core/src/main/java/com/google/protobuf/Descriptors.java
  2. 4
      java/core/src/main/java/com/google/protobuf/GeneratedMessage.java
  3. 4
      java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java
  4. 14
      java/core/src/main/java/com/google/protobuf/MessageReflection.java
  5. 124
      java/core/src/test/java/com/google/protobuf/DescriptorsTest.java

@ -634,10 +634,6 @@ public final class Descriptors {
extensions[i].setProto(proto.getExtension(i)); extensions[i].setProto(proto.getExtension(i));
} }
} }
boolean supportsUnknownEnumValue() {
return getSyntax() == Syntax.PROTO3;
}
} }
// ================================================================= // =================================================================
@ -1328,6 +1324,30 @@ public final class Descriptors {
return enumType; return enumType;
} }
/**
* Determines if the given enum field is treated as closed based on legacy non-conformant
* behavior.
*
* <p>Conformant behavior determines closedness based on the enum and can be queried using
* {@code EnumDescriptor.isClosed()}.
*
* <p>Some runtimes currently have a quirk where non-closed enums are treated as closed when
* used as the type of fields defined in a `syntax = proto2;` file. This quirk is not present in
* all runtimes; as of writing, we know that:
*
* <ul>
* <li>C++, Java, and C++-based Python share this quirk.
* <li>UPB and UPB-based Python do not.
* <li>PHP and Ruby treat all enums as open regardless of declaration.
* </ul>
*
* <p>Care should be taken when using this function to respect the target runtime's enum
* handling quirks.
*/
boolean legacyEnumFieldTreatedAsClosed() {
return getType() == Type.ENUM && getFile().getSyntax() == Syntax.PROTO2;
}
/** /**
* Compare with another {@code FieldDescriptor}. This orders fields in "canonical" order, which * Compare with another {@code FieldDescriptor}. This orders fields in "canonical" order, which
* simply means ascending order by field number. {@code other} must be a field of the same type. * simply means ascending order by field number. {@code other} must be a field of the same type.

@ -2646,7 +2646,7 @@ public abstract class GeneratedMessage extends AbstractMessage implements Serial
valueOfMethod = getMethodOrDie(type, "valueOf", EnumValueDescriptor.class); valueOfMethod = getMethodOrDie(type, "valueOf", EnumValueDescriptor.class);
getValueDescriptorMethod = getMethodOrDie(type, "getValueDescriptor"); getValueDescriptorMethod = getMethodOrDie(type, "getValueDescriptor");
supportUnknownEnumValue = descriptor.getFile().supportsUnknownEnumValue(); supportUnknownEnumValue = !descriptor.legacyEnumFieldTreatedAsClosed();
if (supportUnknownEnumValue) { if (supportUnknownEnumValue) {
getValueMethod = getMethodOrDie(messageClass, "get" + camelCaseName + "Value"); getValueMethod = getMethodOrDie(messageClass, "get" + camelCaseName + "Value");
getValueMethodBuilder = getMethodOrDie(builderClass, "get" + camelCaseName + "Value"); getValueMethodBuilder = getMethodOrDie(builderClass, "get" + camelCaseName + "Value");
@ -2705,7 +2705,7 @@ public abstract class GeneratedMessage extends AbstractMessage implements Serial
valueOfMethod = getMethodOrDie(type, "valueOf", EnumValueDescriptor.class); valueOfMethod = getMethodOrDie(type, "valueOf", EnumValueDescriptor.class);
getValueDescriptorMethod = getMethodOrDie(type, "getValueDescriptor"); getValueDescriptorMethod = getMethodOrDie(type, "getValueDescriptor");
supportUnknownEnumValue = descriptor.getFile().supportsUnknownEnumValue(); supportUnknownEnumValue = !descriptor.legacyEnumFieldTreatedAsClosed();
if (supportUnknownEnumValue) { if (supportUnknownEnumValue) {
getRepeatedValueMethod = getRepeatedValueMethod =
getMethodOrDie(messageClass, "get" + camelCaseName + "Value", int.class); getMethodOrDie(messageClass, "get" + camelCaseName + "Value", int.class);

@ -2889,7 +2889,7 @@ public abstract class GeneratedMessageV3 extends AbstractMessage implements Seri
valueOfMethod = getMethodOrDie(type, "valueOf", EnumValueDescriptor.class); valueOfMethod = getMethodOrDie(type, "valueOf", EnumValueDescriptor.class);
getValueDescriptorMethod = getMethodOrDie(type, "getValueDescriptor"); getValueDescriptorMethod = getMethodOrDie(type, "getValueDescriptor");
supportUnknownEnumValue = descriptor.getFile().supportsUnknownEnumValue(); supportUnknownEnumValue = !descriptor.legacyEnumFieldTreatedAsClosed();
if (supportUnknownEnumValue) { if (supportUnknownEnumValue) {
getValueMethod = getMethodOrDie(messageClass, "get" + camelCaseName + "Value"); getValueMethod = getMethodOrDie(messageClass, "get" + camelCaseName + "Value");
getValueMethodBuilder = getMethodOrDie(builderClass, "get" + camelCaseName + "Value"); getValueMethodBuilder = getMethodOrDie(builderClass, "get" + camelCaseName + "Value");
@ -2950,7 +2950,7 @@ public abstract class GeneratedMessageV3 extends AbstractMessage implements Seri
valueOfMethod = getMethodOrDie(type, "valueOf", EnumValueDescriptor.class); valueOfMethod = getMethodOrDie(type, "valueOf", EnumValueDescriptor.class);
getValueDescriptorMethod = getMethodOrDie(type, "getValueDescriptor"); getValueDescriptorMethod = getMethodOrDie(type, "getValueDescriptor");
supportUnknownEnumValue = descriptor.getFile().supportsUnknownEnumValue(); supportUnknownEnumValue = !descriptor.legacyEnumFieldTreatedAsClosed();
if (supportUnknownEnumValue) { if (supportUnknownEnumValue) {
getRepeatedValueMethod = getRepeatedValueMethod =
getMethodOrDie(messageClass, "get" + camelCaseName + "Value", int.class); getMethodOrDie(messageClass, "get" + camelCaseName + "Value", int.class);

@ -1199,10 +1199,7 @@ class MessageReflection {
if (field.getLiteType() == WireFormat.FieldType.ENUM) { if (field.getLiteType() == WireFormat.FieldType.ENUM) {
while (input.getBytesUntilLimit() > 0) { while (input.getBytesUntilLimit() > 0) {
final int rawValue = input.readEnum(); final int rawValue = input.readEnum();
if (field.getFile().supportsUnknownEnumValue()) { if (field.legacyEnumFieldTreatedAsClosed()) {
target.addRepeatedField(
field, field.getEnumType().findValueByNumberCreatingIfUnknown(rawValue));
} else {
final Object value = field.getEnumType().findValueByNumber(rawValue); final Object value = field.getEnumType().findValueByNumber(rawValue);
// If the number isn't recognized as a valid value for this enum, // If the number isn't recognized as a valid value for this enum,
// add it to the unknown fields. // add it to the unknown fields.
@ -1213,6 +1210,9 @@ class MessageReflection {
} else { } else {
target.addRepeatedField(field, value); target.addRepeatedField(field, value);
} }
} else {
target.addRepeatedField(
field, field.getEnumType().findValueByNumberCreatingIfUnknown(rawValue));
} }
} }
} else { } else {
@ -1239,9 +1239,7 @@ class MessageReflection {
} }
case ENUM: case ENUM:
final int rawValue = input.readEnum(); final int rawValue = input.readEnum();
if (field.getFile().supportsUnknownEnumValue()) { if (field.legacyEnumFieldTreatedAsClosed()) {
value = field.getEnumType().findValueByNumberCreatingIfUnknown(rawValue);
} else {
value = field.getEnumType().findValueByNumber(rawValue); value = field.getEnumType().findValueByNumber(rawValue);
// If the number isn't recognized as a valid value for this enum, // If the number isn't recognized as a valid value for this enum,
// add it to the unknown fields. // add it to the unknown fields.
@ -1251,6 +1249,8 @@ class MessageReflection {
} }
return true; return true;
} }
} else {
value = field.getEnumType().findValueByNumberCreatingIfUnknown(rawValue);
} }
break; break;
default: default:

@ -289,6 +289,130 @@ public class DescriptorsTest {
assertThat(d.findFieldByName("large_uint64").getDefaultValue()).isEqualTo(-1L); assertThat(d.findFieldByName("large_uint64").getDefaultValue()).isEqualTo(-1L);
} }
@Test
public void testFieldDescriptorLegacyEnumFieldTreatedAsClosed() throws Exception {
// Make an open enum definition.
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())
.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 closedEnumFile =
FileDescriptorProto.newBuilder()
.setName("closed_enum_field.proto")
.addDependency("open_enum.proto")
.setSyntax("proto2")
.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)
.build())
.addField(
FieldDescriptorProto.newBuilder()
.setName("closed_enum")
.setNumber(3)
.setType(FieldDescriptorProto.Type.TYPE_ENUM)
.setTypeName("TestEnum")
.setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL)
.build())
.build())
.build();
Descriptor closedMessage =
Descriptors.FileDescriptor.buildFrom(
closedEnumFile, new FileDescriptor[] {openFileDescriptor})
.getMessageTypes()
.get(0);
assertThat(closedMessage.findFieldByName("int_field").legacyEnumFieldTreatedAsClosed())
.isFalse();
assertThat(closedMessage.findFieldByName("closed_enum").legacyEnumFieldTreatedAsClosed())
.isTrue();
assertThat(closedMessage.findFieldByName("open_enum").legacyEnumFieldTreatedAsClosed())
.isTrue();
}
@Test
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 @Test
public void testEnumDescriptor() throws Exception { public void testEnumDescriptor() throws Exception {
EnumDescriptor enumType = ForeignEnum.getDescriptor(); EnumDescriptor enumType = ForeignEnum.getDescriptor();

Loading…
Cancel
Save