Skip to content

Commit

Permalink
Breaking Change: Removed syntax and added has_presence?/`is_packe…
Browse files Browse the repository at this point in the history
…d?`.

Closes #15313

PiperOrigin-RevId: 596962024
  • Loading branch information
haberman authored and Copybara-Service committed Jan 9, 2024
1 parent e7be866 commit dbd4dce
Show file tree
Hide file tree
Showing 9 changed files with 85 additions and 63 deletions.
61 changes: 37 additions & 24 deletions ruby/ext/google/protobuf_c/defs.c
Expand Up @@ -489,7 +489,7 @@ static VALUE FileDescriptor_alloc(VALUE klass) {
* call-seq:
* FileDescriptor.new => file
*
* Returns a new file descriptor. The syntax must be set before it's passed
* Returns a new file descriptor. May
* to a builder.
*/
static VALUE FileDescriptor_initialize(VALUE _self, VALUE cookie,
Expand Down Expand Up @@ -519,28 +519,6 @@ static VALUE FileDescriptor_name(VALUE _self) {
return name == NULL ? Qnil : rb_str_new2(name);
}

/*
* call-seq:
* FileDescriptor.syntax => syntax
*
* Returns this file descriptors syntax.
*
* Valid syntax versions are:
* :proto2 or :proto3.
*/
static VALUE FileDescriptor_syntax(VALUE _self) {
FileDescriptor* self = ruby_to_FileDescriptor(_self);

switch (upb_FileDef_Syntax(self->filedef)) {
case kUpb_Syntax_Proto3:
return ID2SYM(rb_intern("proto3"));
case kUpb_Syntax_Proto2:
return ID2SYM(rb_intern("proto2"));
default:
return Qnil;
}
}

/*
* call-seq:
* FileDescriptor.options => options
Expand All @@ -564,7 +542,6 @@ static void FileDescriptor_register(VALUE module) {
rb_define_alloc_func(klass, FileDescriptor_alloc);
rb_define_method(klass, "initialize", FileDescriptor_initialize, 3);
rb_define_method(klass, "name", FileDescriptor_name, 0);
rb_define_method(klass, "syntax", FileDescriptor_syntax, 0);
rb_define_method(klass, "options", FileDescriptor_options, 0);
rb_gc_register_address(&cFileDescriptor);
cFileDescriptor = klass;
Expand Down Expand Up @@ -736,6 +713,28 @@ static VALUE FieldDescriptor_default(VALUE _self) {
return Convert_UpbToRuby(default_val, TypeInfo_get(self->fielddef), Qnil);
}

/*
* call-seq:
* FieldDescriptor.has_presence? => bool
*
* Returns whether this field tracks presence.
*/
static VALUE FieldDescriptor_has_presence(VALUE _self) {
FieldDescriptor* self = ruby_to_FieldDescriptor(_self);
return upb_FieldDef_HasPresence(self->fielddef) ? Qtrue : Qfalse;
}

/*
* call-seq:
* FieldDescriptor.is_packed? => bool
*
* Returns whether this is a repeated field that uses packed encoding.
*/
static VALUE FieldDescriptor_is_packed(VALUE _self) {
FieldDescriptor* self = ruby_to_FieldDescriptor(_self);
return upb_FieldDef_IsPacked(self->fielddef) ? Qtrue : Qfalse;
}

/*
* call-seq:
* FieldDescriptor.json_name => json_name
Expand Down Expand Up @@ -943,6 +942,8 @@ static void FieldDescriptor_register(VALUE module) {
rb_define_method(klass, "name", FieldDescriptor_name, 0);
rb_define_method(klass, "type", FieldDescriptor__type, 0);
rb_define_method(klass, "default", FieldDescriptor_default, 0);
rb_define_method(klass, "has_presence?", FieldDescriptor_has_presence, 0);
rb_define_method(klass, "is_packed?", FieldDescriptor_is_packed, 0);
rb_define_method(klass, "json_name", FieldDescriptor_json_name, 0);
rb_define_method(klass, "label", FieldDescriptor_label, 0);
rb_define_method(klass, "number", FieldDescriptor_number, 0);
Expand Down Expand Up @@ -1163,6 +1164,17 @@ static VALUE EnumDescriptor_file_descriptor(VALUE _self) {
upb_EnumDef_File(self->enumdef));
}

/*
* call-seq:
* EnumDescriptor.is_closed? => bool
*
* Returns whether this enum is open or closed.
*/
static VALUE EnumDescriptor_is_closed(VALUE _self) {
EnumDescriptor* self = ruby_to_EnumDescriptor(_self);
return upb_EnumDef_IsClosed(self->enumdef) ? Qtrue : Qfalse;
}

/*
* call-seq:
* EnumDescriptor.name => name
Expand Down Expand Up @@ -1275,6 +1287,7 @@ static void EnumDescriptor_register(VALUE module) {
rb_define_method(klass, "each", EnumDescriptor_each, 0);
rb_define_method(klass, "enummodule", EnumDescriptor_enummodule, 0);
rb_define_method(klass, "file_descriptor", EnumDescriptor_file_descriptor, 0);
rb_define_method(klass, "is_closed?", EnumDescriptor_is_closed, 0);
rb_define_method(klass, "options", EnumDescriptor_options, 0);
rb_include_module(klass, rb_mEnumerable);
rb_gc_register_address(&cEnumDescriptor);
Expand Down
1 change: 0 additions & 1 deletion ruby/lib/google/protobuf/ffi/descriptor.rb
Expand Up @@ -158,7 +158,6 @@ class FFI
attach_function :oneof_count, :upb_MessageDef_OneofCount, [Descriptor], :int
attach_function :message_options, :Descriptor_serialized_options, [Descriptor, :pointer, Internal::Arena], :pointer
attach_function :get_well_known_type, :upb_MessageDef_WellKnownType, [Descriptor], WellKnown
attach_function :message_def_syntax, :upb_MessageDef_Syntax, [Descriptor], Syntax
attach_function :find_msg_def_by_name, :upb_MessageDef_FindByNameWithSize, [Descriptor, :string, :size_t, :FieldDefPointer, :OneofDefPointer], :bool
end
end
Expand Down
9 changes: 9 additions & 0 deletions ruby/lib/google/protobuf/ffi/field_descriptor.rb
Expand Up @@ -156,6 +156,14 @@ def has_presence?
@has_presence ||= Google::Protobuf::FFI.get_has_presence(self)
end

##
# Tests if this is a repeated field that uses packed encoding.
#
# @return [Boolean] True iff this field is packed
def is_packed?
@is_packed ||= Google::Protobuf::FFI.get_is_packed(self)
end

# @param msg [Google::Protobuf::Message]
def clear(msg)
if msg.class.descriptor != Google::Protobuf::FFI.get_containing_message_def(self)
Expand Down Expand Up @@ -304,6 +312,7 @@ class FFI
attach_function :get_default, :upb_FieldDef_Default, [FieldDescriptor], MessageValue.by_value
attach_function :get_subtype_as_enum, :upb_FieldDef_EnumSubDef, [FieldDescriptor], EnumDescriptor
attach_function :get_has_presence, :upb_FieldDef_HasPresence, [FieldDescriptor], :bool
attach_function :get_is_packed, :upb_FieldDef_IsPacked, [FieldDescriptor], :bool
attach_function :is_map, :upb_FieldDef_IsMap, [FieldDescriptor], :bool
attach_function :is_repeated, :upb_FieldDef_IsRepeated, [FieldDescriptor], :bool
attach_function :is_sub_message, :upb_FieldDef_IsSubMessage, [FieldDescriptor], :bool
Expand Down
12 changes: 0 additions & 12 deletions ruby/lib/google/protobuf/ffi/file_descriptor.rb
Expand Up @@ -10,7 +10,6 @@ module Protobuf
class FFI
# FileDescriptor
attach_function :file_def_name, :upb_FileDef_Name, [:FileDef], :string
attach_function :file_def_syntax, :upb_FileDef_Syntax, [:FileDef], Syntax
attach_function :file_def_pool, :upb_FileDef_Pool, [:FileDef], :DefPool
attach_function :file_options, :FileDescriptor_serialized_options, [:FileDef, :pointer, Internal::Arena], :pointer
end
Expand All @@ -31,17 +30,6 @@ def inspect
"#{self.class.name}: #{name}"
end

def syntax
case Google::Protobuf::FFI.file_def_syntax(@file_def)
when :Proto3
:proto3
when :Proto2
:proto2
else
nil
end
end

def name
Google::Protobuf::FFI.file_def_name(@file_def)
end
Expand Down
Expand Up @@ -92,6 +92,28 @@ public IRubyObject getLabel(ThreadContext context) {
return label;
}

/*
* call-seq:
* FieldDescriptor.has_presence? => bool
*
* Returns whether this field tracks presence.
*/
@JRubyMethod(name = "has_presence?")
public IRubyObject hasPresence(ThreadContext context) {
return this.descriptor.hasPresence() ? context.runtime.getTrue() : context.runtime.getFalse();
}

/*
* call-seq:
* FieldDescriptor.is_packed? => bool
*
* Returns whether this is a repeated field that uses packed encoding.
*/
@JRubyMethod(name = "is_packed?")
public IRubyObject isPacked(ThreadContext context) {
return this.descriptor.isPacked() ? context.runtime.getTrue() : context.runtime.getFalse();
}

/*
* call-seq:
* FieldDescriptor.name => name
Expand Down
Expand Up @@ -86,27 +86,6 @@ public IRubyObject getName(ThreadContext context) {
return name == null ? context.nil : context.runtime.newString(name);
}

/*
* call-seq:
* FileDescriptor.syntax => syntax
*
* Returns this file descriptors syntax.
*
* Valid syntax versions are:
* :proto2 or :proto3.
*/
@JRubyMethod(name = "syntax")
public IRubyObject getSyntax(ThreadContext context) {
switch (LegacyFileDescriptor.getSyntax(fileDescriptor)) {
case PROTO2:
return context.runtime.newSymbol("proto2");
case PROTO3:
return context.runtime.newSymbol("proto3");
default:
return context.nil;
}
}

@JRubyMethod
public IRubyObject options(ThreadContext context) {
RubyDescriptorPool pool = (RubyDescriptorPool) RubyDescriptorPool.generatedPool(null, null);
Expand Down
13 changes: 11 additions & 2 deletions ruby/tests/basic.rb
Expand Up @@ -553,12 +553,10 @@ def test_file_descriptor
file_descriptor = TestMessage.descriptor.file_descriptor
refute_nil file_descriptor
assert_equal "basic_test.proto", file_descriptor.name
assert_equal :proto3, file_descriptor.syntax

file_descriptor = TestEnum.descriptor.file_descriptor
refute_nil file_descriptor
assert_equal "basic_test.proto", file_descriptor.name
assert_equal :proto3, file_descriptor.syntax
end

def test_map_freeze
Expand Down Expand Up @@ -639,6 +637,17 @@ def test_map_fields_respond_to? # regression test for issue 9202
end
end

def test_has_presence
assert_true TestMessage.descriptor.lookup("optional_int32").has_presence?
assert_false TestMessage.descriptor.lookup("repeated_int32").has_presence?
assert_false TestSingularFields.descriptor.lookup("singular_int32").has_presence?
end

def test_is_packed
assert_false TestMessage.descriptor.lookup("optional_int32").is_packed?
assert_true TestMessage.descriptor.lookup("repeated_int32").is_packed?
end

def test_file_descriptor_options
file_descriptor = TestMessage.descriptor.file_descriptor

Expand Down
7 changes: 5 additions & 2 deletions ruby/tests/basic_proto2.rb
Expand Up @@ -231,12 +231,10 @@ def test_file_descriptor
file_descriptor = TestMessage.descriptor.file_descriptor
refute_nil file_descriptor
assert_equal "basic_test_proto2.proto", file_descriptor.name
assert_equal :proto2, file_descriptor.syntax

file_descriptor = TestEnum.descriptor.file_descriptor
refute_nil file_descriptor
assert_equal "basic_test_proto2.proto", file_descriptor.name
assert_equal :proto2, file_descriptor.syntax
end

def test_oneof_fields_respond_to? # regression test for issue 9202
Expand All @@ -254,6 +252,11 @@ def test_oneof_fields_respond_to? # regression test for issue 9202
refute msg.has_d?
end

def test_is_packed
assert_false TestMessage.descriptor.lookup("optional_int32").is_packed?
assert_false TestMessage.descriptor.lookup("repeated_int32").is_packed?
end

def test_extension
message = TestExtensions.new
extension = Google::Protobuf::DescriptorPool.generated_pool.lookup 'basic_test_proto2.optional_int32_extension'
Expand Down
2 changes: 1 addition & 1 deletion upb/reflection/field_def.h
Expand Up @@ -47,7 +47,7 @@ uint32_t upb_FieldDef_Index(const upb_FieldDef* f);
bool upb_FieldDef_IsExtension(const upb_FieldDef* f);
UPB_API bool upb_FieldDef_IsMap(const upb_FieldDef* f);
bool upb_FieldDef_IsOptional(const upb_FieldDef* f);
bool upb_FieldDef_IsPacked(const upb_FieldDef* f);
UPB_API bool upb_FieldDef_IsPacked(const upb_FieldDef* f);
bool upb_FieldDef_IsPrimitive(const upb_FieldDef* f);
UPB_API bool upb_FieldDef_IsRepeated(const upb_FieldDef* f);
bool upb_FieldDef_IsRequired(const upb_FieldDef* f);
Expand Down

0 comments on commit dbd4dce

Please sign in to comment.