diff options
Diffstat (limited to 'ruby')
-rw-r--r-- | ruby/Rakefile | 8 | ||||
-rw-r--r-- | ruby/compatibility_tests/v3.0.0/tests/basic.rb | 2 | ||||
-rw-r--r-- | ruby/ext/google/protobuf_c/defs.c | 17 | ||||
-rw-r--r-- | ruby/ext/google/protobuf_c/message.c | 42 | ||||
-rw-r--r-- | ruby/ext/google/protobuf_c/protobuf.c | 2 | ||||
-rw-r--r-- | ruby/ext/google/protobuf_c/protobuf.h | 5 | ||||
-rw-r--r-- | ruby/ext/google/protobuf_c/storage.c | 13 | ||||
-rw-r--r-- | ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java | 57 | ||||
-rw-r--r-- | ruby/tests/basic.rb | 53 | ||||
-rw-r--r-- | ruby/tests/gc_test.rb | 58 | ||||
-rwxr-xr-x | ruby/travis-test.sh | 1 |
11 files changed, 220 insertions, 38 deletions
diff --git a/ruby/Rakefile b/ruby/Rakefile index a329a777..e30a75a3 100644 --- a/ruby/Rakefile +++ b/ruby/Rakefile @@ -99,7 +99,13 @@ Gem::PackageTask.new(spec) do |pkg| end Rake::TestTask.new(:test => :build) do |t| - t.test_files = FileList["tests/*.rb"] + t.test_files = FileList["tests/*.rb"].exclude("tests/gc_test.rb") +end + +# gc_test needs to be split out to ensure the generated file hasn't been +# imported by other tests. +Rake::TestTask.new(:gc_test => :build) do |t| + t.test_files = FileList["tests/gc_test.rb"] end task :build => [:clean, :compile, :genproto] diff --git a/ruby/compatibility_tests/v3.0.0/tests/basic.rb b/ruby/compatibility_tests/v3.0.0/tests/basic.rb index f81e456c..05fe0278 100644 --- a/ruby/compatibility_tests/v3.0.0/tests/basic.rb +++ b/ruby/compatibility_tests/v3.0.0/tests/basic.rb @@ -600,7 +600,7 @@ module BasicTest assert_raise RangeError do m["z"] = :Z end - assert_raise TypeError do + assert_raise RangeError do m["z"] = "z" end end diff --git a/ruby/ext/google/protobuf_c/defs.c b/ruby/ext/google/protobuf_c/defs.c index 845c0225..34d9663a 100644 --- a/ruby/ext/google/protobuf_c/defs.c +++ b/ruby/ext/google/protobuf_c/defs.c @@ -228,7 +228,6 @@ DEFINE_CLASS(Descriptor, "Google::Protobuf::Descriptor"); void Descriptor_mark(void* _self) { Descriptor* self = _self; rb_gc_mark(self->klass); - rb_gc_mark(self->typeclass_references); } void Descriptor_free(void* _self) { @@ -283,7 +282,6 @@ VALUE Descriptor_alloc(VALUE klass) { self->pb_serialize_handlers = NULL; self->json_serialize_handlers = NULL; self->json_serialize_handlers_preserve = NULL; - self->typeclass_references = rb_ary_new(); return ret; } @@ -1635,7 +1633,7 @@ VALUE Builder_alloc(VALUE klass) { Builder* self = ALLOC(Builder); VALUE ret = TypedData_Wrap_Struct( klass, &_Builder_type, self); - self->pending_list = rb_ary_new(); + self->pending_list = Qnil; self->defs = NULL; return ret; } @@ -1645,6 +1643,7 @@ void Builder_register(VALUE module) { rb_define_alloc_func(klass, Builder_alloc); rb_define_method(klass, "add_message", Builder_add_message, 1); rb_define_method(klass, "add_enum", Builder_add_enum, 1); + rb_define_method(klass, "initialize", Builder_initialize, 0); rb_define_method(klass, "finalize_to_pool", Builder_finalize_to_pool, 1); cBuilder = klass; rb_gc_register_address(&cBuilder); @@ -1652,6 +1651,18 @@ void Builder_register(VALUE module) { /* * call-seq: + * Builder.new(d) => builder + * + * Create a new message builder. + */ +VALUE Builder_initialize(VALUE _self) { + DEFINE_SELF(Builder, self, _self); + self->pending_list = rb_ary_new(); + return Qnil; +} + +/* + * call-seq: * Builder.add_message(name, &block) * * Creates a new, empty descriptor with the given name, and invokes the block in diff --git a/ruby/ext/google/protobuf_c/message.c b/ruby/ext/google/protobuf_c/message.c index 29911140..42910bfe 100644 --- a/ruby/ext/google/protobuf_c/message.c +++ b/ruby/ext/google/protobuf_c/message.c @@ -217,20 +217,32 @@ VALUE Message_respond_to_missing(int argc, VALUE* argv, VALUE _self) { return Qtrue; } +VALUE create_submsg_from_hash(const upb_fielddef *f, VALUE hash) { + const upb_def *d = upb_fielddef_subdef(f); + assert(d != NULL); + + VALUE descriptor = get_def_obj(d); + VALUE msgclass = rb_funcall(descriptor, rb_intern("msgclass"), 0, NULL); + + VALUE args[1] = { hash }; + return rb_class_new_instance(1, args, msgclass); +} + int Message_initialize_kwarg(VALUE key, VALUE val, VALUE _self) { MessageHeader* self; - VALUE method_str; - char* name; + char *name; const upb_fielddef* f; TypedData_Get_Struct(_self, MessageHeader, &Message_type, self); - if (!SYMBOL_P(key)) { + if (TYPE(key) == T_STRING) { + name = RSTRING_PTR(key); + } else if (TYPE(key) == T_SYMBOL) { + name = RSTRING_PTR(rb_id2str(SYM2ID(key))); + } else { rb_raise(rb_eArgError, - "Expected symbols as hash keys in initialization map."); + "Expected string or symbols as hash keys when initializing proto from hash."); } - method_str = rb_id2str(SYM2ID(key)); - name = RSTRING_PTR(method_str); f = upb_msgdef_ntofz(self->descriptor->msgdef, name); if (f == NULL) { rb_raise(rb_eArgError, @@ -255,9 +267,18 @@ int Message_initialize_kwarg(VALUE key, VALUE val, VALUE _self) { } ary = layout_get(self->descriptor->layout, Message_data(self), f); for (int i = 0; i < RARRAY_LEN(val); i++) { - RepeatedField_push(ary, rb_ary_entry(val, i)); + VALUE entry = rb_ary_entry(val, i); + if (TYPE(entry) == T_HASH && upb_fielddef_issubmsg(f)) { + entry = create_submsg_from_hash(f, entry); + } + + RepeatedField_push(ary, entry); } } else { + if (TYPE(val) == T_HASH && upb_fielddef_issubmsg(f)) { + val = create_submsg_from_hash(f, val); + } + layout_set(self->descriptor->layout, Message_data(self), f, val); } return 0; @@ -419,6 +440,13 @@ VALUE Message_to_h(VALUE _self) { msg_value = Map_to_h(msg_value); } else if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) { msg_value = RepeatedField_to_ary(msg_value); + + if (upb_fielddef_type(field) == UPB_TYPE_MESSAGE) { + for (int i = 0; i < RARRAY_LEN(msg_value); i++) { + VALUE elem = rb_ary_entry(msg_value, i); + rb_ary_store(msg_value, i, Message_to_h(elem)); + } + } } else if (msg_value != Qnil && upb_fielddef_type(field) == UPB_TYPE_MESSAGE) { msg_value = Message_to_h(msg_value); diff --git a/ruby/ext/google/protobuf_c/protobuf.c b/ruby/ext/google/protobuf_c/protobuf.c index 7cde4aec..c7750c44 100644 --- a/ruby/ext/google/protobuf_c/protobuf.c +++ b/ruby/ext/google/protobuf_c/protobuf.c @@ -110,6 +110,6 @@ void Init_protobuf_c() { kRubyStringASCIIEncoding = rb_usascii_encoding(); kRubyString8bitEncoding = rb_ascii8bit_encoding(); - upb_def_to_ruby_obj_map = rb_hash_new(); rb_gc_register_address(&upb_def_to_ruby_obj_map); + upb_def_to_ruby_obj_map = rb_hash_new(); } diff --git a/ruby/ext/google/protobuf_c/protobuf.h b/ruby/ext/google/protobuf_c/protobuf.h index f4b110fe..21c7f001 100644 --- a/ruby/ext/google/protobuf_c/protobuf.h +++ b/ruby/ext/google/protobuf_c/protobuf.h @@ -116,10 +116,6 @@ struct Descriptor { const upb_handlers* pb_serialize_handlers; const upb_handlers* json_serialize_handlers; const upb_handlers* json_serialize_handlers_preserve; - // Handlers hold type class references for sub-message fields directly in some - // cases. We need to keep these rooted because they might otherwise be - // collected. - VALUE typeclass_references; }; struct FieldDescriptor { @@ -280,6 +276,7 @@ void Builder_free(void* _self); VALUE Builder_alloc(VALUE klass); void Builder_register(VALUE module); Builder* ruby_to_Builder(VALUE value); +VALUE Builder_initialize(VALUE _self); VALUE Builder_add_message(VALUE _self, VALUE name); VALUE Builder_add_enum(VALUE _self, VALUE name); VALUE Builder_finalize_to_pool(VALUE _self, VALUE pool_rb); diff --git a/ruby/ext/google/protobuf_c/storage.c b/ruby/ext/google/protobuf_c/storage.c index 3ff2bda6..24064dfd 100644 --- a/ruby/ext/google/protobuf_c/storage.c +++ b/ruby/ext/google/protobuf_c/storage.c @@ -176,6 +176,15 @@ void native_slot_set_value_and_case(upb_fieldtype_t type, VALUE type_class, break; } case UPB_TYPE_STRING: + if (CLASS_OF(value) == rb_cSymbol) { + value = rb_funcall(value, rb_intern("to_s"), 0, NULL); + } else if (CLASS_OF(value) != rb_cString) { + rb_raise(rb_eTypeError, "Invalid argument for string field."); + } + + DEREF(memory, VALUE) = native_slot_encode_and_freeze_string(type, value); + break; + case UPB_TYPE_BYTES: { if (CLASS_OF(value) != rb_cString) { rb_raise(rb_eTypeError, "Invalid argument for string field."); @@ -197,7 +206,9 @@ void native_slot_set_value_and_case(upb_fieldtype_t type, VALUE type_class, } case UPB_TYPE_ENUM: { int32_t int_val = 0; - if (!is_ruby_num(value) && TYPE(value) != T_SYMBOL) { + if (TYPE(value) == T_STRING) { + value = rb_funcall(value, rb_intern("to_sym"), 0, NULL); + } else if (!is_ruby_num(value) && TYPE(value) != T_SYMBOL) { rb_raise(rb_eTypeError, "Expected number or symbol type for enum field."); } 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 733ccfbc..07558fbc 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java @@ -82,8 +82,8 @@ public class RubyMessage extends RubyObject { hash.visitAll(new RubyHash.Visitor() { @Override public void visit(IRubyObject key, IRubyObject value) { - if (!(key instanceof RubySymbol)) - throw runtime.newTypeError("Expected symbols as hash keys in initialization map."); + if (!(key instanceof RubySymbol) && !(key instanceof RubyString)) + throw runtime.newTypeError("Expected string or symbols as hash keys in initialization map."); final Descriptors.FieldDescriptor fieldDescriptor = findField(context, key); if (Utils.isMapEntry(fieldDescriptor)) { @@ -103,9 +103,15 @@ public class RubyMessage extends RubyObject { if (oneof != null) { oneofCases.put(oneof, fieldDescriptor); } + + if (value instanceof RubyHash && fieldDescriptor.getType() == Descriptors.FieldDescriptor.Type.MESSAGE) { + RubyDescriptor descriptor = (RubyDescriptor) getDescriptorForField(context, fieldDescriptor); + RubyClass typeClass = (RubyClass) descriptor.msgclass(context); + value = (IRubyObject) typeClass.newInstance(context, value, Block.NULL_BLOCK); + } + fields.put(fieldDescriptor, value); } - } }); } @@ -367,7 +373,19 @@ public class RubyMessage extends RubyObject { for (Descriptors.FieldDescriptor fdef : this.descriptor.getFields()) { IRubyObject value = getField(context, fdef); if (!value.isNil()) { - if (value.respondsTo("to_h")) { + if (fdef.isRepeated() && !fdef.isMapField()) { + if (fdef.getType() != Descriptors.FieldDescriptor.Type.MESSAGE) { + value = Helpers.invoke(context, value, "to_a"); + } else { + RubyArray ary = value.convertToArray(); + for (int i = 0; i < ary.size(); i++) { + IRubyObject submsg = Helpers.invoke(context, ary.eltInternal(i), "to_h"); + ary.eltInternalSet(i, submsg); + } + + value = ary.to_ary(); + } + } else if (value.respondsTo("to_h")) { value = Helpers.invoke(context, value, "to_h"); } else if (value.respondsTo("to_a")) { value = Helpers.invoke(context, value, "to_a"); @@ -518,19 +536,12 @@ public class RubyMessage extends RubyObject { val = value.isTrue(); break; case BYTES: + Utils.validateStringEncoding(context, fieldDescriptor.getType(), value); + val = ByteString.copyFrom(((RubyString) value).getBytes()); + break; case STRING: Utils.validateStringEncoding(context, fieldDescriptor.getType(), value); - RubyString str = (RubyString) value; - switch (fieldDescriptor.getType()) { - case BYTES: - val = ByteString.copyFrom(str.getBytes()); - break; - case STRING: - val = str.asJavaString(); - break; - default: - break; - } + val = ((RubyString) value).asJavaString(); break; case MESSAGE: RubyClass typeClass = (RubyClass) ((RubyDescriptor) getDescriptorForField(context, fieldDescriptor)).msgclass(context); @@ -543,7 +554,7 @@ public class RubyMessage extends RubyObject { if (Utils.isRubyNum(value)) { val = enumDescriptor.findValueByNumberCreatingIfUnknown(RubyNumeric.num2int(value)); - } else if (value instanceof RubySymbol) { + } else if (value instanceof RubySymbol || value instanceof RubyString) { val = enumDescriptor.findValueByName(value.asJavaString()); } else { throw runtime.newTypeError("Expected number or symbol type for enum field."); @@ -741,8 +752,20 @@ public class RubyMessage extends RubyObject { Descriptors.FieldDescriptor fieldDescriptor, IRubyObject value) { RubyArray arr = value.convertToArray(); RubyRepeatedField repeatedField = repeatedFieldForFieldDescriptor(context, fieldDescriptor); + + RubyClass typeClass = null; + if (fieldDescriptor.getType() == Descriptors.FieldDescriptor.Type.MESSAGE) { + RubyDescriptor descriptor = (RubyDescriptor) getDescriptorForField(context, fieldDescriptor); + typeClass = (RubyClass) descriptor.msgclass(context); + } + for (int i = 0; i < arr.size(); i++) { - repeatedField.push(context, arr.eltInternal(i)); + IRubyObject row = arr.eltInternal(i); + if (row instanceof RubyHash && typeClass != null) { + row = (IRubyObject) typeClass.newInstance(context, row, Block.NULL_BLOCK); + } + + repeatedField.push(context, row); } return repeatedField; } diff --git a/ruby/tests/basic.rb b/ruby/tests/basic.rb index 94071ca0..ad34d53d 100644 --- a/ruby/tests/basic.rb +++ b/ruby/tests/basic.rb @@ -51,6 +51,17 @@ module BasicTest optional :foo, :int32, 1 end + add_message "TestEmbeddedMessageParent" do + optional :child_msg, :message, 1, "TestEmbeddedMessageChild" + optional :number, :int32, 2 + + repeated :repeated_msg, :message, 3, "TestEmbeddedMessageChild" + repeated :repeated_number, :int32, 4 + end + add_message "TestEmbeddedMessageChild" do + optional :sub_child, :message, 1, "TestMessage" + end + add_message "Recursive1" do optional :foo, :message, 1, "Recursive2" end @@ -113,6 +124,8 @@ module BasicTest Baz = pool.lookup("Baz").msgclass TestMessage = pool.lookup("TestMessage").msgclass TestMessage2 = pool.lookup("TestMessage2").msgclass + TestEmbeddedMessageParent = pool.lookup("TestEmbeddedMessageParent").msgclass + TestEmbeddedMessageChild = pool.lookup("TestEmbeddedMessageChild").msgclass Recursive1 = pool.lookup("Recursive1").msgclass Recursive2 = pool.lookup("Recursive2").msgclass TestEnum = pool.lookup("TestEnum").enummodule @@ -161,12 +174,18 @@ module BasicTest m.optional_double = 0.5 m.optional_string = "hello" assert m.optional_string == "hello" + m.optional_string = :hello + assert m.optional_string == "hello" m.optional_bytes = "world".encode!('ASCII-8BIT') assert m.optional_bytes == "world" m.optional_msg = TestMessage2.new(:foo => 42) assert m.optional_msg == TestMessage2.new(:foo => 42) m.optional_msg = nil assert m.optional_msg == nil + m.optional_enum = :C + assert m.optional_enum == :C + m.optional_enum = 'C' + assert m.optional_enum == :C end def test_ctor_args @@ -183,6 +202,34 @@ module BasicTest assert m.repeated_string[2] == "world" end + def test_ctor_string_symbol_args + m = TestMessage.new(:optional_enum => 'C', :repeated_enum => ['A', 'B']) + assert_equal :C, m.optional_enum + assert_equal [:A, :B], m.repeated_enum + + m = TestMessage.new(:optional_string => :foo, :repeated_string => [:foo, :bar]) + assert_equal 'foo', m.optional_string + assert_equal ['foo', 'bar'], m.repeated_string + end + + def test_embeddedmsg_hash_init + m = TestEmbeddedMessageParent.new(:child_msg => {sub_child: {optional_int32: 1}}, + :number => 2, + :repeated_msg => [{sub_child: {optional_int32: 3}}], + :repeated_number => [10, 20, 30]) + + assert_equal 2, m.number + assert_equal [10, 20, 30], m.repeated_number + + assert_not_nil m.child_msg + assert_not_nil m.child_msg.sub_child + assert_equal m.child_msg.sub_child.optional_int32, 1 + + assert_not_nil m.repeated_msg + assert_equal 1, m.repeated_msg.length + assert_equal 3, m.repeated_msg.first.sub_child.optional_int32 + end + def test_inspect m = TestMessage.new(:optional_int32 => -42, :optional_enum => :A, @@ -614,7 +661,7 @@ module BasicTest assert_raise RangeError do m["z"] = :Z end - assert_raise TypeError do + assert_raise RangeError do m["z"] = "z" end end @@ -927,7 +974,7 @@ module BasicTest end def test_to_h - m = TestMessage.new(:optional_bool => true, :optional_double => -10.100001, :optional_string => 'foo', :repeated_string => ['bar1', 'bar2']) + m = TestMessage.new(:optional_bool => true, :optional_double => -10.100001, :optional_string => 'foo', :repeated_string => ['bar1', 'bar2'], :repeated_msg => [TestMessage2.new(:foo => 100)]) expected_result = { :optional_bool=>true, :optional_bytes=>"", @@ -947,7 +994,7 @@ module BasicTest :repeated_float=>[], :repeated_int32=>[], :repeated_int64=>[], - :repeated_msg=>[], + :repeated_msg=>[{:foo => 100}], :repeated_string=>["bar1", "bar2"], :repeated_uint32=>[], :repeated_uint64=>[] diff --git a/ruby/tests/gc_test.rb b/ruby/tests/gc_test.rb new file mode 100644 index 00000000..f3470cca --- /dev/null +++ b/ruby/tests/gc_test.rb @@ -0,0 +1,58 @@ +#!/usr/bin/ruby +# +# generated_code.rb is in the same directory as this test. +$LOAD_PATH.unshift(File.expand_path(File.dirname(__FILE__))) + +old_gc = GC.stress +GC.stress = 0x01 | 0x04 +require 'generated_code_pb' +GC.stress = old_gc + +require 'test/unit' + +class GCTest < Test::Unit::TestCase + def get_msg + A::B::C::TestMessage.new( + :optional_int32 => 1, + :optional_int64 => 1, + :optional_uint32 => 1, + :optional_uint64 => 1, + :optional_bool => true, + :optional_double => 1.0, + :optional_float => 1.0, + :optional_string => "a", + :optional_bytes => "b", + :optional_enum => A::B::C::TestEnum::A, + :optional_msg => A::B::C::TestMessage.new(), + :repeated_int32 => [1], + :repeated_int64 => [1], + :repeated_uint32 => [1], + :repeated_uint64 => [1], + :repeated_bool => [true], + :repeated_double => [1.0], + :repeated_float => [1.0], + :repeated_string => ["a"], + :repeated_bytes => ["b"], + :repeated_enum => [A::B::C::TestEnum::A], + :repeated_msg => [A::B::C::TestMessage.new()], + :map_int32_string => {1 => "a"}, + :map_int64_string => {1 => "a"}, + :map_uint32_string => {1 => "a"}, + :map_uint64_string => {1 => "a"}, + :map_bool_string => {true => "a"}, + :map_string_string => {"a" => "a"}, + :map_string_msg => {"a" => A::B::C::TestMessage.new()}, + :map_string_int32 => {"a" => 1}, + :map_string_bool => {"a" => true}, + ) + end + def test_generated_msg + old_gc = GC.stress + GC.stress = 0x01 | 0x04 + from = get_msg + data = A::B::C::TestMessage.encode(from) + to = A::B::C::TestMessage.decode(data) + GC.stress = old_gc + puts "passed" + end +end diff --git a/ruby/travis-test.sh b/ruby/travis-test.sh index 52ea81b6..cbe7cd98 100755 --- a/ruby/travis-test.sh +++ b/ruby/travis-test.sh @@ -20,6 +20,7 @@ test_version() { git clean -f && \ gem install bundler && bundle && \ rake test && + rake gc_test && cd ../conformance && make test_ruby && cd ../ruby/compatibility_tests/v3.0.0 && ./test.sh" fi |