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

Handle clear for Java proto3 optionals (synthetic oneofs) using field descriptor instead of clear method.

Browse Source 
This treats clear similarly to has and get to avoids issues from missing clear method for escaped synthetic oneofs (e.g. field _underscore -> oneof X_underscore). Previously, `clear` was using the clear method of the field (which has the same camel-cased name outside of the underscore case).

We also remove synthetic oneof camelCase names from the gencode for FieldAccesorTable since these should not be used / exposed.

Fixes #12880

PiperOrigin-RevId: 539069001
pull/12949/head
Sandy Zhang 2 years ago committed by Copybara-Service
parent 6831d2072e
commit a5349027e3
2 changed files with 79 additions and 46 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. 122
      java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java
  2. 3
      src/google/protobuf/compiler/java/message.cc

122
java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java
Unescape View File

@ -2096,8 +2096,10 @@ public abstract class GeneratedMessageV3 extends AbstractMessage implements Seri
FieldDescriptor field = descriptor.getFields().get(i);
String containingOneofCamelCaseName = null;
if (field.getContainingOneof() != null) {
containingOneofCamelCaseName =
camelCaseNames[fieldsSize + field.getContainingOneof().getIndex()];
int index = fieldsSize + field.getContainingOneof().getIndex();
if (index < camelCaseNames.length) {
containingOneofCamelCaseName = camelCaseNames[index];
}
}
if (field.isRepeated()) {
if (field.getJavaType() == FieldDescriptor.JavaType.MESSAGE) {
@ -2153,12 +2155,16 @@ public abstract class GeneratedMessageV3 extends AbstractMessage implements Seri
}
}
int oneofsSize = oneofs.length;
for (int i = 0; i < oneofsSize; i++) {
oneofs[i] =
new OneofAccessor(
descriptor, i, camelCaseNames[i + fieldsSize], messageClass, builderClass);
for (int i = 0; i < descriptor.getOneofs().size(); i++) {
if (i < descriptor.getRealOneofs().size()) {
oneofs[i] =
new RealOneofAccessor(
descriptor, i, camelCaseNames[i + fieldsSize], messageClass, builderClass);
} else {
oneofs[i] = new SyntheticOneofAccessor(descriptor, i);
}
}
initialized = true;
camelCaseNames = null;
return this;
@ -2230,24 +2236,29 @@ public abstract class GeneratedMessageV3 extends AbstractMessage implements Seri
}
/** OneofAccessor provides access to a single oneof. */
private static class OneofAccessor {
OneofAccessor(
private static interface OneofAccessor {
public boolean has(final GeneratedMessageV3 message);
public boolean has(GeneratedMessageV3.Builder<?> builder);
public FieldDescriptor get(final GeneratedMessageV3 message);
public FieldDescriptor get(GeneratedMessageV3.Builder<?> builder);
public void clear(final Builder<?> builder);
}
/** RealOneofAccessor provides access to a single real oneof. */
private static class RealOneofAccessor implements OneofAccessor {
RealOneofAccessor(
final Descriptor descriptor,
final int oneofIndex,
final String camelCaseName,
final Class<? extends GeneratedMessageV3> messageClass,
final Class<? extends Builder<?>> builderClass) {
this.descriptor = descriptor;
OneofDescriptor oneofDescriptor = descriptor.getOneofs().get(oneofIndex);
if (oneofDescriptor.isSynthetic()) {
caseMethod = null;
caseMethodBuilder = null;
fieldDescriptor = oneofDescriptor.getFields().get(0);
} else {
caseMethod = getMethodOrDie(messageClass, "get" + camelCaseName + "Case");
caseMethodBuilder = getMethodOrDie(builderClass, "get" + camelCaseName + "Case");
fieldDescriptor = null;
}
caseMethod = getMethodOrDie(messageClass, "get" + camelCaseName + "Case");
caseMethodBuilder = getMethodOrDie(builderClass, "get" + camelCaseName + "Case");
clearMethod = getMethodOrDie(builderClass, "clear" + camelCaseName);
}
@ -2255,55 +2266,76 @@ public abstract class GeneratedMessageV3 extends AbstractMessage implements Seri
private final Method caseMethod;
private final Method caseMethodBuilder;
private final Method clearMethod;
private final FieldDescriptor fieldDescriptor;
@Override
public boolean has(final GeneratedMessageV3 message) {
if (fieldDescriptor != null) {
return message.hasField(fieldDescriptor);
} else {
return ((Internal.EnumLite) invokeOrDie(caseMethod, message)).getNumber() != 0;
}
return ((Internal.EnumLite) invokeOrDie(caseMethod, message)).getNumber() != 0;
}
@Override
public boolean has(GeneratedMessageV3.Builder<?> builder) {
if (fieldDescriptor != null) {
return builder.hasField(fieldDescriptor);
} else {
return ((Internal.EnumLite) invokeOrDie(caseMethodBuilder, builder)).getNumber() != 0;
}
return ((Internal.EnumLite) invokeOrDie(caseMethodBuilder, builder)).getNumber() != 0;
}
@Override
public FieldDescriptor get(final GeneratedMessageV3 message) {
if (fieldDescriptor != null) {
return message.hasField(fieldDescriptor) ? fieldDescriptor : null;
} else {
int fieldNumber = ((Internal.EnumLite) invokeOrDie(caseMethod, message)).getNumber();
if (fieldNumber > 0) {
return descriptor.findFieldByNumber(fieldNumber);
}
int fieldNumber = ((Internal.EnumLite) invokeOrDie(caseMethod, message)).getNumber();
if (fieldNumber > 0) {
return descriptor.findFieldByNumber(fieldNumber);
}
return null;
}
@Override
public FieldDescriptor get(GeneratedMessageV3.Builder<?> builder) {
if (fieldDescriptor != null) {
return builder.hasField(fieldDescriptor) ? fieldDescriptor : null;
} else {
int fieldNumber =
((Internal.EnumLite) invokeOrDie(caseMethodBuilder, builder)).getNumber();
if (fieldNumber > 0) {
return descriptor.findFieldByNumber(fieldNumber);
}
int fieldNumber = ((Internal.EnumLite) invokeOrDie(caseMethodBuilder, builder)).getNumber();
if (fieldNumber > 0) {
return descriptor.findFieldByNumber(fieldNumber);
}
return null;
}
@Override
public void clear(final Builder<?> builder) {
// TODO(b/230609037): remove the unused variable
Object unused = invokeOrDie(clearMethod, builder);
}
}
/** SyntheticOneofAccessor provides access to a single synthetic oneof. */
private static class SyntheticOneofAccessor implements OneofAccessor {
SyntheticOneofAccessor(final Descriptor descriptor, final int oneofIndex) {
OneofDescriptor oneofDescriptor = descriptor.getOneofs().get(oneofIndex);
fieldDescriptor = oneofDescriptor.getFields().get(0);
}
private final FieldDescriptor fieldDescriptor;
@Override
public boolean has(final GeneratedMessageV3 message) {
return message.hasField(fieldDescriptor);
}
@Override
public boolean has(GeneratedMessageV3.Builder<?> builder) {
return builder.hasField(fieldDescriptor);
}
@Override
public FieldDescriptor get(final GeneratedMessageV3 message) {
return message.hasField(fieldDescriptor) ? fieldDescriptor : null;
}
public FieldDescriptor get(GeneratedMessageV3.Builder<?> builder) {
return builder.hasField(fieldDescriptor) ? fieldDescriptor : null;
}
@Override
public void clear(final Builder<?> builder) {
builder.clearField(fieldDescriptor);
}
}
// ---------------------------------------------------------------
@SuppressWarnings("SameNameButDifferent")

3
src/google/protobuf/compiler/java/message.cc
Unescape View File

@ -247,7 +247,8 @@ int ImmutableMessageGenerator::GenerateFieldAccessorTableInitializer(
bytecode_estimate += 6;
printer->Print("\"$field_name$\", ", "field_name", info->capitalized_name);
}
// We reproduce synthetic oneofs here since proto reflection needs these.
// TODO(b/286434301): Once cl/534906231 propagates, only consider real oneofs
// since proto reflection does not use this for synthetic oneofs.
for (int i = 0; i < descriptor_->oneof_decl_count(); i++) {
const OneofDescriptor* oneof = descriptor_->oneof_decl(i);
const OneofGeneratorInfo* info = context_->GetOneofGeneratorInfo(oneof);

Write Preview
Loading…
Cancel
Save