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
diff --git a/ruby/README.md b/ruby/README.md
index 9ae3ac3..1647432 100644
--- a/ruby/README.md
+++ b/ruby/README.md
@@ -63,7 +63,7 @@
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 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 7c1d849..c25103d 100644
--- a/ruby/Rakefile
+++ b/ruby/Rakefile
@@ -6,6 +6,9 @@
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 8cf2e29..5148ee8 100644
--- a/ruby/ext/google/protobuf_c/repeated_field.c
+++ b/ruby/ext/google/protobuf_c/repeated_field.c
@@ -47,6 +47,15 @@
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 @@
* 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 @@
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 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 1630fe8..01f0e16 100644
--- a/ruby/pom.xml
+++ b/ruby/pom.xml
@@ -78,7 +78,7 @@
<dependency>
<groupId>com.google.protobuf</groupId>
<artifactId>protobuf-java</artifactId>
- <version>3.0.0-pre</version>
+ <version>3.0.0-alpha-3-pre</version>
</dependency>
</dependencies>
</project>
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 04bc0b7..20e825e 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 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 @@
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 9788317..84bf895 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 @@
*/
@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 @@
* 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 @@
@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 @@
}
}
+ 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 307374e..141ce7c 100644
--- a/ruby/tests/basic.rb
+++ b/ruby/tests/basic.rb
@@ -314,6 +314,17 @@
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 @@
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 @@
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
