From 519284a55961fca339d69d516c31bd62082c177b Mon Sep 17 00:00:00 2001 From: Sandy Zhang Date: Fri, 12 Jan 2024 06:25:02 -0800 Subject: [PATCH] Remove LegacyDescriptorsUtil.java and JRuby's use of legacy descriptor APIs This is not a breaking change since this has not been released yet. PiperOrigin-RevId: 597824548 --- .../protobuf/LegacyDescriptorsUtil.java | 72 ------------------- .../protobuf/jruby/RubyEnumDescriptor.java | 4 +- .../protobuf/jruby/RubyFieldDescriptor.java | 9 --- .../protobuf/jruby/RubyFileDescriptor.java | 2 - .../google/protobuf/jruby/RubyMessage.java | 31 ++++---- 5 files changed, 14 insertions(+), 104 deletions(-) delete mode 100644 java/core/src/main/java/com/google/protobuf/LegacyDescriptorsUtil.java diff --git a/java/core/src/main/java/com/google/protobuf/LegacyDescriptorsUtil.java b/java/core/src/main/java/com/google/protobuf/LegacyDescriptorsUtil.java deleted file mode 100644 index 7b2351caf8..0000000000 --- a/java/core/src/main/java/com/google/protobuf/LegacyDescriptorsUtil.java +++ /dev/null @@ -1,72 +0,0 @@ -// Protocol Buffers - Google's data interchange format -// Copyright 2008 Google Inc. All rights reserved. -// -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file or at -// https://developers.google.com/open-source/licenses/bsd - -package com.google.protobuf; - -import com.google.protobuf.Descriptors.FieldDescriptor; -import com.google.protobuf.Descriptors.FileDescriptor; -import com.google.protobuf.Descriptors.OneofDescriptor; - -/** - * This file is meant to be a temporary housing for legacy descriptor APIs we want to deprecate and - * remove. This will help prevent backslide by allowing us to control visibility. - */ -public final class LegacyDescriptorsUtil { - - /** Wraps FileDescriptor */ - public static final class LegacyFileDescriptor { - - /** The syntax of the .proto file. */ - public static enum Syntax { - UNKNOWN("unknown"), - PROTO2("proto2"), - PROTO3("proto3"); - - Syntax(String name) { - this.name = name; - } - - final String name; - } - - public static Syntax getSyntax(FileDescriptor descriptor) { - switch (descriptor.getSyntax()) { - case UNKNOWN: - return Syntax.UNKNOWN; - case PROTO2: - return Syntax.PROTO2; - case PROTO3: - return Syntax.PROTO3; - } - throw new IllegalArgumentException("Unexpected syntax"); - } - - private LegacyFileDescriptor() {} - } - - /** Wraps FieldDescriptor */ - public static final class LegacyFieldDescriptor { - - public static boolean hasOptionalKeyword(FieldDescriptor descriptor) { - return descriptor.hasOptionalKeyword(); - } - - private LegacyFieldDescriptor() {} - } - - /** Wraps OneofDescriptor */ - public static final class LegacyOneofDescriptor { - - public static boolean isSynthetic(OneofDescriptor descriptor) { - return descriptor.isSynthetic(); - } - - private LegacyOneofDescriptor() {} - } - - private LegacyDescriptorsUtil() {} -} diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyEnumDescriptor.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyEnumDescriptor.java index 7e8247c014..3ef946f12d 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyEnumDescriptor.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyEnumDescriptor.java @@ -36,7 +36,6 @@ import com.google.protobuf.CodedInputStream; import com.google.protobuf.DescriptorProtos.EnumDescriptorProto; import com.google.protobuf.Descriptors.EnumDescriptor; import com.google.protobuf.Descriptors.EnumValueDescriptor; -import com.google.protobuf.LegacyDescriptorsUtil.LegacyFileDescriptor; import org.jruby.Ruby; import org.jruby.RubyClass; import org.jruby.RubyModule; @@ -176,8 +175,7 @@ public class RubyEnumDescriptor extends RubyObject { Ruby runtime = context.runtime; RubyModule enumModule = RubyModule.newModule(runtime); - boolean defaultValueRequiredButNotFound = - LegacyFileDescriptor.getSyntax(descriptor.getFile()) == LegacyFileDescriptor.Syntax.PROTO3; + boolean defaultValueRequiredButNotFound = !descriptor.isClosed(); for (EnumValueDescriptor value : descriptor.getValues()) { String name = fixEnumConstantName(value.getName()); // Make sure it's a valid constant name before trying to create it diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyFieldDescriptor.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyFieldDescriptor.java index 8e6427ee54..6f52e10067 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyFieldDescriptor.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyFieldDescriptor.java @@ -34,7 +34,6 @@ package com.google.protobuf.jruby; import com.google.protobuf.CodedInputStream; import com.google.protobuf.Descriptors.FieldDescriptor; -import com.google.protobuf.LegacyDescriptorsUtil.LegacyFileDescriptor; import org.jruby.*; import org.jruby.anno.JRubyClass; import org.jruby.anno.JRubyMethod; @@ -276,14 +275,6 @@ public class RubyFieldDescriptor extends RubyObject { protected void setDescriptor( ThreadContext context, FieldDescriptor descriptor, RubyDescriptorPool pool) { - if (descriptor.isRequired() - && LegacyFileDescriptor.getSyntax(descriptor.getFile()) - == LegacyFileDescriptor.Syntax.PROTO3) { - throw Utils.createTypeError( - context, - descriptor.getName() - + " is labeled required but required fields are unsupported in proto3"); - } this.descriptor = descriptor; this.name = context.runtime.newString(descriptor.getName()); this.pool = pool; diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyFileDescriptor.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyFileDescriptor.java index ff3c6d9c9f..f0147d686c 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyFileDescriptor.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyFileDescriptor.java @@ -35,8 +35,6 @@ package com.google.protobuf.jruby; import com.google.protobuf.CodedInputStream; import com.google.protobuf.Descriptors.FileDescriptor; import com.google.protobuf.Descriptors.GenericDescriptor; -import com.google.protobuf.LegacyDescriptorsUtil.LegacyFileDescriptor; -import com.google.protobuf.LegacyDescriptorsUtil.LegacyFileDescriptor.Syntax.*; import org.jruby.*; import org.jruby.anno.JRubyClass; import org.jruby.anno.JRubyMethod; diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java index 25d169b759..f8c3950237 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java @@ -41,8 +41,6 @@ import com.google.protobuf.Descriptors.FieldDescriptor; import com.google.protobuf.Descriptors.OneofDescriptor; import com.google.protobuf.DynamicMessage; import com.google.protobuf.InvalidProtocolBufferException; -import com.google.protobuf.LegacyDescriptorsUtil.LegacyFileDescriptor; -import com.google.protobuf.LegacyDescriptorsUtil.LegacyOneofDescriptor; import com.google.protobuf.Message; import com.google.protobuf.UnknownFieldSet; import com.google.protobuf.util.JsonFormat; @@ -74,8 +72,6 @@ public class RubyMessage extends RubyObject { this.builder = DynamicMessage.newBuilder(descriptor); this.fields = new HashMap(); this.oneofCases = new HashMap(); - this.proto3 = - LegacyFileDescriptor.getSyntax(descriptor.getFile()) == LegacyFileDescriptor.Syntax.PROTO3; } /* @@ -644,15 +640,15 @@ public class RubyMessage extends RubyObject { e.getMessage()); } - if (!ret.proto3) { - // Need to reset unknown values in repeated enum fields - ret.builder - .getUnknownFields() - .asMap() - .forEach( - (i, values) -> { - FieldDescriptor fd = ret.builder.getDescriptorForType().findFieldByNumber(i); - if (fd != null && fd.isRepeated() && fd.getType() == FieldDescriptor.Type.ENUM) { + ret.builder + .getUnknownFields() + .asMap() + .forEach( + (i, values) -> { + FieldDescriptor fd = ret.builder.getDescriptorForType().findFieldByNumber(i); + if (fd != null && fd.isRepeated() && fd.getType() == FieldDescriptor.Type.ENUM) { + // Need to reset unknown values in repeated enum fields + if (fd.legacyEnumFieldTreatedAsClosed()) { EnumDescriptor ed = fd.getEnumType(); values .getVarintList() @@ -662,8 +658,8 @@ public class RubyMessage extends RubyObject { fd, ed.findValueByNumberCreatingIfUnknown(value.intValue())); }); } - }); - } + } + }); if (freeze) { ret.freeze(context); } @@ -791,7 +787,7 @@ public class RubyMessage extends RubyObject { build(context, 0, SINK_MAXIMUM_NESTING); // Sync Ruby data to the Builder object. for (Map.Entry field : builder.getAllFields().entrySet()) { FieldDescriptor fdef = field.getKey(); - IRubyObject value = getFieldInternal(context, fdef, proto3); + IRubyObject value = getFieldInternal(context, fdef, !fdef.hasPresence()); if (fdef.isRepeated() && !fdef.isMapField()) { if (fdef.getType() != FieldDescriptor.Type.MESSAGE) { @@ -1342,7 +1338,7 @@ public class RubyMessage extends RubyObject { // Keep track of what Oneofs are set if (value.isNil()) { oneofCases.remove(oneofDescriptor); - if (!LegacyOneofDescriptor.isSynthetic(oneofDescriptor)) { + if (fieldDescriptor.getRealContainingOneof() != null) { addValue = false; } } else { @@ -1506,5 +1502,4 @@ public class RubyMessage extends RubyObject { private RubyClass cRepeatedField; private RubyClass cMap; private boolean ignoreUnknownFieldsOnInit = false; - private boolean proto3; }