From 545527e8cfedc43dc6b862af23691affcb1285f7 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Wed, 18 May 2016 10:58:02 -0700 Subject: Ruby oneofs: return default instead of nil for unset fields. --- ruby/ext/google/protobuf_c/storage.c | 24 ++++++++++++++++++++++- ruby/tests/basic.rb | 38 ++++++++++++++++++------------------ 2 files changed, 42 insertions(+), 20 deletions(-) diff --git a/ruby/ext/google/protobuf_c/storage.c b/ruby/ext/google/protobuf_c/storage.c index b1f65f41..5dae5aec 100644 --- a/ruby/ext/google/protobuf_c/storage.c +++ b/ruby/ext/google/protobuf_c/storage.c @@ -57,6 +57,28 @@ size_t native_slot_size(upb_fieldtype_t type) { } } +VALUE value_from_default(const upb_fielddef *field) { + switch (upb_fielddef_type(field)) { + case UPB_TYPE_FLOAT: return DBL2NUM(upb_fielddef_defaultfloat(field)); + case UPB_TYPE_DOUBLE: return DBL2NUM(upb_fielddef_defaultdouble(field)); + case UPB_TYPE_BOOL: + return upb_fielddef_defaultbool(field) ? Qtrue : Qfalse; + case UPB_TYPE_MESSAGE: return Qnil; + case UPB_TYPE_ENUM: return INT2NUM(upb_fielddef_defaultint32(field)); + case UPB_TYPE_INT32: return INT2NUM(upb_fielddef_defaultint32(field)); + case UPB_TYPE_INT64: return LL2NUM(upb_fielddef_defaultint64(field));; + case UPB_TYPE_UINT32: return UINT2NUM(upb_fielddef_defaultuint32(field)); + case UPB_TYPE_UINT64: return ULL2NUM(upb_fielddef_defaultuint64(field)); + case UPB_TYPE_STRING: + case UPB_TYPE_BYTES: { + size_t size; + const char *str = upb_fielddef_defaultstr(field, &size); + return rb_str_new(str, size); + } + default: return Qnil; + } +} + static bool is_ruby_num(VALUE value) { return (TYPE(value) == T_FLOAT || TYPE(value) == T_FIXNUM || @@ -537,7 +559,7 @@ VALUE layout_get(MessageLayout* layout, if (upb_fielddef_containingoneof(field)) { if (*oneof_case != upb_fielddef_number(field)) { - return Qnil; + return value_from_default(field); } return native_slot_get(upb_fielddef_type(field), field_type_class(field), diff --git a/ruby/tests/basic.rb b/ruby/tests/basic.rb index 77c186ef..db6a3c75 100644 --- a/ruby/tests/basic.rb +++ b/ruby/tests/basic.rb @@ -703,36 +703,36 @@ module BasicTest def test_oneof d = OneofMessage.new - assert d.a == nil - assert d.b == nil + assert d.a == "" + assert d.b == 0 assert d.c == nil - assert d.d == nil + assert d.d == 0 assert d.my_oneof == nil d.a = "hi" assert d.a == "hi" - assert d.b == nil + assert d.b == 0 assert d.c == nil - assert d.d == nil + assert d.d == 0 assert d.my_oneof == :a d.b = 42 - assert d.a == nil + assert d.a == "" assert d.b == 42 assert d.c == nil - assert d.d == nil + assert d.d == 0 assert d.my_oneof == :b d.c = TestMessage2.new(:foo => 100) - assert d.a == nil - assert d.b == nil + assert d.a == "" + assert d.b == 0 assert d.c.foo == 100 - assert d.d == nil + assert d.d == 0 assert d.my_oneof == :c d.d = :C - assert d.a == nil - assert d.b == nil + assert d.a == "" + assert d.b == 0 assert d.c == nil assert d.d == :C assert d.my_oneof == :d @@ -748,23 +748,23 @@ module BasicTest d3 = OneofMessage.decode( encoded_field_c + encoded_field_a + encoded_field_d) - assert d3.a == nil - assert d3.b == nil + assert d3.a == "" + assert d3.b == 0 assert d3.c == nil assert d3.d == :B d4 = OneofMessage.decode( encoded_field_c + encoded_field_a + encoded_field_d + encoded_field_c) - assert d4.a == nil - assert d4.b == nil + assert d4.a == "" + assert d4.b == 0 assert d4.c.foo == 1 - assert d4.d == nil + assert d4.d == 0 d5 = OneofMessage.new(:a => "hello") - assert d5.a != nil + assert d5.a == "hello" d5.a = nil - assert d5.a == nil + assert d5.a == "" assert OneofMessage.encode(d5) == '' assert d5.my_oneof == nil end -- cgit v1.2.3 From 2d514ce2d8b1f5fb00c6031a1a8e4dab968e4927 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Wed, 18 May 2016 15:39:29 -0700 Subject: Fixed oneof behavior for enums and fixed JRuby. --- ruby/ext/google/protobuf_c/storage.c | 13 +++++++++++-- .../main/java/com/google/protobuf/jruby/RubyMessage.java | 12 ++++++++---- ruby/tests/basic.rb | 10 +++++----- 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/ruby/ext/google/protobuf_c/storage.c b/ruby/ext/google/protobuf_c/storage.c index 5dae5aec..1c839781 100644 --- a/ruby/ext/google/protobuf_c/storage.c +++ b/ruby/ext/google/protobuf_c/storage.c @@ -57,14 +57,23 @@ size_t native_slot_size(upb_fieldtype_t type) { } } -VALUE value_from_default(const upb_fielddef *field) { +static VALUE value_from_default(const upb_fielddef *field) { switch (upb_fielddef_type(field)) { case UPB_TYPE_FLOAT: return DBL2NUM(upb_fielddef_defaultfloat(field)); case UPB_TYPE_DOUBLE: return DBL2NUM(upb_fielddef_defaultdouble(field)); case UPB_TYPE_BOOL: return upb_fielddef_defaultbool(field) ? Qtrue : Qfalse; case UPB_TYPE_MESSAGE: return Qnil; - case UPB_TYPE_ENUM: return INT2NUM(upb_fielddef_defaultint32(field)); + case UPB_TYPE_ENUM: { + const upb_enumdef *enumdef = upb_fielddef_enumsubdef(field); + int32_t num = upb_fielddef_defaultint32(field); + const char *label = upb_enumdef_iton(enumdef, num); + if (label) { + return ID2SYM(rb_intern(label)); + } else { + return INT2NUM(num); + } + } case UPB_TYPE_INT32: return INT2NUM(upb_fielddef_defaultint32(field)); case UPB_TYPE_INT64: return LL2NUM(upb_fielddef_defaultint64(field));; case UPB_TYPE_UINT32: return UINT2NUM(upb_fielddef_defaultuint32(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 39213c4d..12893f73 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java @@ -592,13 +592,17 @@ public class RubyMessage extends RubyObject { protected IRubyObject getField(ThreadContext context, Descriptors.FieldDescriptor fieldDescriptor) { Descriptors.OneofDescriptor oneofDescriptor = fieldDescriptor.getContainingOneof(); if (oneofDescriptor != null) { - if (oneofCases.containsKey(oneofDescriptor)) { - if (oneofCases.get(oneofDescriptor) != fieldDescriptor) - return context.runtime.getNil(); + if (oneofCases.get(oneofDescriptor) == fieldDescriptor) { return fields.get(fieldDescriptor); } else { Descriptors.FieldDescriptor oneofCase = builder.getOneofFieldDescriptor(oneofDescriptor); - if (oneofCase != fieldDescriptor) return context.runtime.getNil(); + if (oneofCase != fieldDescriptor) { + if (fieldDescriptor.getType() == Descriptors.FieldDescriptor.Type.MESSAGE) { + return context.runtime.getNil(); + } else { + return wrapField(context, fieldDescriptor, fieldDescriptor.getDefaultValue()); + } + } IRubyObject value = wrapField(context, oneofCase, builder.getField(oneofCase)); fields.put(fieldDescriptor, value); return value; diff --git a/ruby/tests/basic.rb b/ruby/tests/basic.rb index db6a3c75..fee07e33 100644 --- a/ruby/tests/basic.rb +++ b/ruby/tests/basic.rb @@ -706,28 +706,28 @@ module BasicTest assert d.a == "" assert d.b == 0 assert d.c == nil - assert d.d == 0 + assert d.d == :Default assert d.my_oneof == nil d.a = "hi" assert d.a == "hi" assert d.b == 0 assert d.c == nil - assert d.d == 0 + assert d.d == :Default assert d.my_oneof == :a d.b = 42 assert d.a == "" assert d.b == 42 assert d.c == nil - assert d.d == 0 + assert d.d == :Default assert d.my_oneof == :b d.c = TestMessage2.new(:foo => 100) assert d.a == "" assert d.b == 0 assert d.c.foo == 100 - assert d.d == 0 + assert d.d == :Default assert d.my_oneof == :c d.d = :C @@ -759,7 +759,7 @@ module BasicTest assert d4.a == "" assert d4.b == 0 assert d4.c.foo == 1 - assert d4.d == 0 + assert d4.d == :Default d5 = OneofMessage.new(:a => "hello") assert d5.a == "hello" -- cgit v1.2.3