Merge pull request protocolbuffers#334 from skippy/allow-msg-to-accep…

…t-nil ruby: allow a message field to be unset
zphilip · May 3, 2015 · 16a283f · 16a283f
2 parents dcf1213 + 6467826
commit 16a283f
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 6 deletions.
diff --git 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/src/main/java/com/google/protobuf/jruby/RubyMessage.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java
@@ -659,22 +659,27 @@ protected IRubyObject setField(ThreadContext context, Descriptors.FieldDescripto
             } 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 =
                                 enumDescriptor.findValueByNumberCreatingIfUnknown(RubyNumeric.num2int(value));
                         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
@@ -154,6 +154,8 @@ def test_setters
       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