From 761cfa08e6cc60039a0bf490d75078d6738724ae Mon Sep 17 00:00:00 2001 From: Adam Greene Date: Fri, 1 May 2015 08:48:56 -0700 Subject: build cleanups * update docs to simplify build steps * Gemfile.lock seemed to have an older version specified * do not check in the pkg dir --- ruby/.gitignore | 1 + ruby/Gemfile.lock | 2 +- ruby/README.md | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) (limited to 'ruby') diff --git a/ruby/.gitignore b/ruby/.gitignore index 80c978f2..93e24502 100644 --- a/ruby/.gitignore +++ b/ruby/.gitignore @@ -4,3 +4,4 @@ tags lib/google/protobuf_java.jar protobuf-jruby.iml target/ +pkg/ diff --git a/ruby/Gemfile.lock b/ruby/Gemfile.lock index 89deb47d..6f349276 100644 --- a/ruby/Gemfile.lock +++ b/ruby/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - google-protobuf (3.0.0.alpha.2) + google-protobuf (3.0.0.alpha.3.1.pre) GEM remote: https://rubygems.org/ diff --git a/ruby/README.md b/ruby/README.md index d2fa76ab..9ae3ac36 100644 --- a/ruby/README.md +++ b/ruby/README.md @@ -76,7 +76,7 @@ Then install the required Ruby gems: Then build the Gem: $ rake gem - $ gem install pkg/protobuf-$VERSION.gem + $ gem install `ls pkg/google-protobuf-*.gem` To run the specs: -- cgit v1.2.3 From c70b6058eaae4fa5b1af577c548e6809a53dfd98 Mon Sep 17 00:00:00 2001 From: Adam Greene Date: Fri, 1 May 2015 08:54:18 -0700 Subject: add size alias for length starting to make `RepeatedField` quack like an array additional changes: * make sure gemspec gets all ruby code files * add homepage in gem spec removes one of the warnings, and the gem spec authors are pushing everyone to include a homepage in the gem * remove excess whitespace in test suite to bring formatting inline with the rest of the file --- ruby/.gitignore | 1 + ruby/google-protobuf.gemspec | 3 ++- ruby/lib/google/protobuf.rb | 2 ++ ruby/lib/google/protobuf/repeated_field.rb | 40 ++++++++++++++++++++++++++++++ ruby/tests/basic.rb | 14 +++++++++-- 5 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 ruby/lib/google/protobuf/repeated_field.rb (limited to 'ruby') diff --git a/ruby/.gitignore b/ruby/.gitignore index 93e24502..bd8745dd 100644 --- a/ruby/.gitignore +++ b/ruby/.gitignore @@ -5,3 +5,4 @@ lib/google/protobuf_java.jar protobuf-jruby.iml target/ pkg/ +tmp/ diff --git a/ruby/google-protobuf.gemspec b/ruby/google-protobuf.gemspec index abbbde35..28cdebf5 100644 --- a/ruby/google-protobuf.gemspec +++ b/ruby/google-protobuf.gemspec @@ -4,10 +4,11 @@ Gem::Specification.new do |s| s.licenses = ["BSD"] s.summary = "Protocol Buffers" s.description = "Protocol Buffers are Google's data interchange format." + s.homepage = "https://developers.google.com/protocol-buffers" s.authors = ["Protobuf Authors"] s.email = "protobuf@googlegroups.com" s.require_paths = ["lib"] - s.files = ["lib/google/protobuf.rb"] + s.files = `git ls-files -z`.split("\x0").find_all{|f| f =~ /lib\/.+\.rb/} unless RUBY_PLATFORM == "java" s.files += `git ls-files "*.c" "*.h" extconf.rb Makefile`.split s.extensions= ["ext/google/protobuf_c/extconf.rb"] diff --git a/ruby/lib/google/protobuf.rb b/ruby/lib/google/protobuf.rb index 75869dd8..72797245 100644 --- a/ruby/lib/google/protobuf.rb +++ b/ruby/lib/google/protobuf.rb @@ -34,3 +34,5 @@ if RUBY_PLATFORM == "java" else require 'google/protobuf_c' end + +require 'google/protobuf/repeated_field' diff --git a/ruby/lib/google/protobuf/repeated_field.rb b/ruby/lib/google/protobuf/repeated_field.rb new file mode 100644 index 00000000..5b934e56 --- /dev/null +++ b/ruby/lib/google/protobuf/repeated_field.rb @@ -0,0 +1,40 @@ +# Protocol Buffers - Google's data interchange format +# Copyright 2008 Google Inc. All rights reserved. +# https://developers.google.com/protocol-buffers/ +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +# add syntatic sugar on top of the core library +module Google + module Protobuf + class RepeatedField + + alias_method :size, :length + + end + end +end diff --git a/ruby/tests/basic.rb b/ruby/tests/basic.rb index 1c3fb62c..307374e3 100644 --- a/ruby/tests/basic.rb +++ b/ruby/tests/basic.rb @@ -377,6 +377,18 @@ module BasicTest end end + def test_rptfield_array_ducktyping + l = Google::Protobuf::RepeatedField.new(:int32) + length_methods = %w(count length size) + length_methods.each do |lm| + assert l.send(lm) == 0 + end + l.push 4 + length_methods.each do |lm| + assert l.send(lm) == 1 + end + end + def test_map_basic # allowed key types: # :int32, :int64, :uint32, :uint64, :bool, :string, :bytes. @@ -827,7 +839,6 @@ module BasicTest assert m['a.b'] == 4 end - def test_int_ranges m = TestMessage.new @@ -933,7 +944,6 @@ module BasicTest assert_raise RangeError do m.optional_uint64 = 1.5 end - end def test_stress_test -- cgit v1.2.3 From d55733c76ee1db702529f38f602548ffe48a4ab1 Mon Sep 17 00:00:00 2001 From: Adam Greene Date: Fri, 1 May 2015 11:54:29 -0700 Subject: return nil if array index indicie is out of bounds ruby arrays don't throw an exception; they return nil. Lets do the same! this fix also includes the ability to use negative array indicies --- ruby/README.md | 5 ++- ruby/Rakefile | 3 ++ ruby/ext/google/protobuf_c/repeated_field.c | 20 +++++++--- ruby/pom.xml | 2 +- .../com/google/protobuf/jruby/RubyMessage.java | 14 +++---- .../google/protobuf/jruby/RubyRepeatedField.java | 24 +++++++++--- ruby/tests/basic.rb | 43 ++++++++++++++++++++-- 7 files changed, 85 insertions(+), 26 deletions(-) (limited to 'ruby') diff --git a/ruby/README.md b/ruby/README.md index 9ae3ac36..16474322 100644 --- a/ruby/README.md +++ b/ruby/README.md @@ -63,7 +63,7 @@ To build this Ruby extension, you will need: To Build the JRuby extension, you will need: * Maven -* The latest version of the protobuf java library +* The latest version of the protobuf java library (see ../java/README.md) * Install JRuby via rbenv or RVM First switch to the desired platform with rbenv or RVM. @@ -75,7 +75,8 @@ Then install the required Ruby gems: Then build the Gem: - $ rake gem + $ rake + $ rake clobber_package gem $ gem install `ls pkg/google-protobuf-*.gem` To run the specs: diff --git a/ruby/Rakefile b/ruby/Rakefile index 7c1d8495..c25103d8 100644 --- a/ruby/Rakefile +++ b/ruby/Rakefile @@ -6,6 +6,9 @@ require "rake/testtask" spec = Gem::Specification.load("google-protobuf.gemspec") if RUBY_PLATFORM == "java" + if `which mvn` == '' + raise ArgumentError, "maven needs to be installed" + end task :clean do system("mvn clean") end diff --git a/ruby/ext/google/protobuf_c/repeated_field.c b/ruby/ext/google/protobuf_c/repeated_field.c index 8cf2e29b..5148ee87 100644 --- a/ruby/ext/google/protobuf_c/repeated_field.c +++ b/ruby/ext/google/protobuf_c/repeated_field.c @@ -47,6 +47,15 @@ RepeatedField* ruby_to_RepeatedField(VALUE _self) { return self; } +static int index_position(VALUE _index, RepeatedField* repeated_field) { + int index = NUM2INT(_index); + if (index < 0 && repeated_field->size > 0) { + index = repeated_field->size + index; + } + return index; +} + + /* * call-seq: * RepeatedField.each(&block) @@ -74,8 +83,7 @@ VALUE RepeatedField_each(VALUE _self) { * call-seq: * RepeatedField.[](index) => value * - * Accesses the element at the given index. Throws an exception on out-of-bounds - * errors. + * Accesses the element at the given index. Returns nil on out-of-bounds */ VALUE RepeatedField_index(VALUE _self, VALUE _index) { RepeatedField* self = ruby_to_RepeatedField(_self); @@ -83,9 +91,9 @@ VALUE RepeatedField_index(VALUE _self, VALUE _index) { upb_fieldtype_t field_type = self->field_type; VALUE field_type_class = self->field_type_class; - int index = NUM2INT(_index); + int index = index_position(_index, self); if (index < 0 || index >= self->size) { - rb_raise(rb_eRangeError, "Index out of range"); + return Qnil; } void* memory = (void *) (((uint8_t *)self->elements) + index * element_size); @@ -105,9 +113,9 @@ VALUE RepeatedField_index_set(VALUE _self, VALUE _index, VALUE val) { VALUE field_type_class = self->field_type_class; int element_size = native_slot_size(field_type); - int index = NUM2INT(_index); + int index = index_position(_index, self); if (index < 0 || index >= (INT_MAX - 1)) { - rb_raise(rb_eRangeError, "Index out of range"); + return Qnil; } if (index >= self->size) { RepeatedField_reserve(self, index + 1); diff --git a/ruby/pom.xml b/ruby/pom.xml index 1630fe84..01f0e16b 100644 --- a/ruby/pom.xml +++ b/ruby/pom.xml @@ -78,7 +78,7 @@ com.google.protobuf protobuf-java - 3.0.0-pre + 3.0.0-alpha-3-pre 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 04bc0b76..20e825e2 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java @@ -246,16 +246,15 @@ public class RubyMessage extends RubyObject { public IRubyObject dup(ThreadContext context) { RubyMessage dup = (RubyMessage) metaClass.newInstance(context, Block.NULL_BLOCK); IRubyObject value; - for (Descriptors.FieldDescriptor fieldDescriptor : builder.getAllFields().keySet()) { + for (Descriptors.FieldDescriptor fieldDescriptor : this.descriptor.getFields()) { if (fieldDescriptor.isRepeated()) { - dup.repeatedFields.put(fieldDescriptor, getRepeatedField(context, fieldDescriptor)); - } else if (builder.hasField(fieldDescriptor)) { - dup.fields.put(fieldDescriptor, wrapField(context, fieldDescriptor, builder.getField(fieldDescriptor))); + dup.addRepeatedField(fieldDescriptor, this.getRepeatedField(context, fieldDescriptor)); + } else if (fields.containsKey(fieldDescriptor)) { + dup.fields.put(fieldDescriptor, fields.get(fieldDescriptor)); + } else if (this.builder.hasField(fieldDescriptor)) { + dup.fields.put(fieldDescriptor, wrapField(context, fieldDescriptor, this.builder.getField(fieldDescriptor))); } } - for (Descriptors.FieldDescriptor fieldDescriptor : fields.keySet()) { - dup.fields.put(fieldDescriptor, fields.get(fieldDescriptor)); - } for (Descriptors.FieldDescriptor fieldDescriptor : maps.keySet()) { dup.maps.put(fieldDescriptor, maps.get(fieldDescriptor)); } @@ -411,6 +410,7 @@ public class RubyMessage extends RubyObject { for (int i = 0; i < count; i++) { ret.push(context, wrapField(context, fieldDescriptor, this.builder.getRepeatedField(fieldDescriptor, i))); } + addRepeatedField(fieldDescriptor, ret); return ret; } diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyRepeatedField.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyRepeatedField.java index 9788317a..84bf8956 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyRepeatedField.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyRepeatedField.java @@ -108,8 +108,9 @@ public class RubyRepeatedField extends RubyObject { */ @JRubyMethod(name = "[]=") public IRubyObject indexSet(ThreadContext context, IRubyObject index, IRubyObject value) { + int arrIndex = normalizeArrayIndex(index); Utils.checkType(context, fieldType, value, (RubyModule) typeClass); - this.storage.set(RubyNumeric.num2int(index), value); + this.storage.set(arrIndex, value); return context.runtime.getNil(); } @@ -117,12 +118,15 @@ public class RubyRepeatedField extends RubyObject { * call-seq: * RepeatedField.[](index) => value * - * Accesses the element at the given index. Throws an exception on out-of-bounds - * errors. + * Accesses the element at the given index. Returns nil on out-of-bounds */ @JRubyMethod(name = "[]") public IRubyObject index(ThreadContext context, IRubyObject index) { - return this.storage.eltInternal(RubyNumeric.num2int(index)); + int arrIndex = normalizeArrayIndex(index); + if (arrIndex < 0 || arrIndex >= this.storage.size()) { + return context.runtime.getNil(); + } + return this.storage.eltInternal(arrIndex); } /* @@ -134,8 +138,7 @@ public class RubyRepeatedField extends RubyObject { @JRubyMethod(rest = true) public IRubyObject insert(ThreadContext context, IRubyObject[] args) { for (int i = 0; i < args.length; i++) { - Utils.checkType(context, fieldType, args[i], (RubyModule) typeClass); - this.storage.add(args[i]); + push(context, args[i]); } return context.runtime.getNil(); } @@ -385,6 +388,15 @@ public class RubyRepeatedField extends RubyObject { } } + private int normalizeArrayIndex(IRubyObject index) { + int arrIndex = RubyNumeric.num2int(index); + int arrSize = this.storage.size(); + if (arrIndex < 0 && arrSize > 0) { + arrIndex = arrSize + arrIndex; + } + return arrIndex; + } + private RubyArray storage; private Descriptors.FieldDescriptor.Type fieldType; private IRubyObject typeClass; diff --git a/ruby/tests/basic.rb b/ruby/tests/basic.rb index 307374e3..141ce7c5 100644 --- a/ruby/tests/basic.rb +++ b/ruby/tests/basic.rb @@ -314,6 +314,17 @@ module BasicTest assert l4 == [0, 0, 0, 0, 0, 42, 100, 101, 102] end + def test_parent_rptfield + #make sure we set the RepeatedField and can add to it + m = TestMessage.new + assert m.repeated_string == [] + m.repeated_string << 'ok' + m.repeated_string.push('ok2') + assert m.repeated_string == ['ok', 'ok2'] + m.repeated_string += ['ok3'] + assert m.repeated_string == ['ok', 'ok2', 'ok3'] + end + def test_rptfield_msg l = Google::Protobuf::RepeatedField.new(:message, TestMessage) l.push TestMessage.new @@ -383,10 +394,31 @@ module BasicTest length_methods.each do |lm| assert l.send(lm) == 0 end + # out of bounds returns a nil + assert l[0] == nil + assert l[1] == nil + assert l[-1] == nil l.push 4 length_methods.each do |lm| - assert l.send(lm) == 1 + assert l.send(lm) == 1 + end + assert l[0] == 4 + assert l[1] == nil + assert l[-1] == 4 + assert l[-2] == nil + + l.push 2 + length_methods.each do |lm| + assert l.send(lm) == 2 end + assert l[0] == 4 + assert l[1] == 2 + assert l[2] == nil + assert l[-1] == 2 + assert l[-2] == 4 + assert l[-3] == nil + + #adding out of scope will backfill with empty objects end def test_map_basic @@ -724,9 +756,12 @@ module BasicTest m = TestMessage.new m.optional_string = "hello" m.optional_int32 = 42 - m.repeated_msg.push TestMessage2.new(:foo => 100) - m.repeated_msg.push TestMessage2.new(:foo => 200) - + tm1 = TestMessage2.new(:foo => 100) + tm2 = TestMessage2.new(:foo => 200) + m.repeated_msg.push tm1 + assert m.repeated_msg[-1] == tm1 + m.repeated_msg.push tm2 + assert m.repeated_msg[-1] == tm2 m2 = m.dup assert m == m2 m.optional_int32 += 1 -- cgit v1.2.3 From 64678265c5ae28998d031900c2de52419a8ed7e4 Mon Sep 17 00:00:00 2001 From: Adam Greene Date: Sat, 2 May 2015 13:48:23 -0700 Subject: allow a message field to be unset --- ruby/ext/google/protobuf_c/storage.c | 4 +++- ruby/pom.xml | 2 +- .../main/java/com/google/protobuf/jruby/RubyMessage.java | 15 ++++++++++----- ruby/tests/basic.rb | 2 ++ 4 files changed, 16 insertions(+), 7 deletions(-) (limited to 'ruby') diff --git a/ruby/ext/google/protobuf_c/storage.c b/ruby/ext/google/protobuf_c/storage.c index 5b1549d2..2ad8bd74 100644 --- a/ruby/ext/google/protobuf_c/storage.c +++ b/ruby/ext/google/protobuf_c/storage.c @@ -155,7 +155,9 @@ void native_slot_set_value_and_case(upb_fieldtype_t type, VALUE type_class, break; } case UPB_TYPE_MESSAGE: { - if (CLASS_OF(value) != type_class) { + if (CLASS_OF(value) == CLASS_OF(Qnil)) { + value = Qnil; + } else if (CLASS_OF(value) != type_class) { rb_raise(rb_eTypeError, "Invalid type %s to assign to submessage field.", rb_class2name(CLASS_OF(value))); diff --git a/ruby/pom.xml b/ruby/pom.xml index 1630fe84..01f0e16b 100644 --- a/ruby/pom.xml +++ b/ruby/pom.xml @@ -78,7 +78,7 @@ com.google.protobuf protobuf-java - 3.0.0-pre + 3.0.0-alpha-3-pre 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 04bc0b76..9ddadfcd 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java @@ -659,14 +659,14 @@ public class RubyMessage extends RubyObject { } else { Descriptors.FieldDescriptor.Type fieldType = fieldDescriptor.getType(); IRubyObject typeClass = context.runtime.getObject(); + boolean addValue = true; if (fieldType == Descriptors.FieldDescriptor.Type.MESSAGE) { typeClass = ((RubyDescriptor) getDescriptorForField(context, fieldDescriptor)).msgclass(context); + if (value.isNil()){ + addValue = false; + } } else if (fieldType == Descriptors.FieldDescriptor.Type.ENUM) { typeClass = ((RubyEnumDescriptor) getDescriptorForField(context, fieldDescriptor)).enummodule(context); - } - Utils.checkType(context, fieldType, value, (RubyModule) typeClass); - // Convert integer enum to symbol - if (fieldType == Descriptors.FieldDescriptor.Type.ENUM) { Descriptors.EnumDescriptor enumDescriptor = fieldDescriptor.getEnumType(); if (Utils.isRubyNum(value)) { Descriptors.EnumValueDescriptor val = @@ -674,7 +674,12 @@ public class RubyMessage extends RubyObject { if (val.getIndex() != -1) value = context.runtime.newSymbol(val.getName()); } } - this.fields.put(fieldDescriptor, value); + if (addValue) { + Utils.checkType(context, fieldType, value, (RubyModule) typeClass); + this.fields.put(fieldDescriptor, value); + } else { + this.fields.remove(fieldDescriptor); + } } } return context.runtime.getNil(); diff --git a/ruby/tests/basic.rb b/ruby/tests/basic.rb index 307374e3..0eb5cefc 100644 --- a/ruby/tests/basic.rb +++ b/ruby/tests/basic.rb @@ -154,6 +154,8 @@ module BasicTest 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 end def test_ctor_args -- cgit v1.2.3