Merge pull request #338 from skippy/encode-decode-helpers

ruby: Encode decode cleanup and behavior normalization

11 files changed, 182 insertions, 114 deletions

diff --git a/ruby/ext/google/protobuf_c/encode_decode.c b/ruby/ext/google/protobuf_c/encode_decode.c
index 5730504d..f9a046cb 100644
--- a/ruby/ext/google/protobuf_c/encode_decode.c
+++ b/ruby/ext/google/protobuf_c/encode_decode.c
@@ -1118,50 +1118,3 @@ VALUE Message_encode_json(VALUE klass, VALUE msg_rb) {
   return ret;
 }
 
-/*
- * call-seq:
- *     Google::Protobuf.encode(msg) => bytes
- *
- * Encodes the given message object to protocol buffers wire format. This is an
- * alternative to the #encode method on msg's class.
- */
-VALUE Google_Protobuf_encode(VALUE self, VALUE msg_rb) {
-  VALUE klass = CLASS_OF(msg_rb);
-  return Message_encode(klass, msg_rb);
-}
-
-/*
- * call-seq:
- *     Google::Protobuf.encode_json(msg) => json_string
- *
- * Encodes the given message object to its JSON representation. This is an
- * alternative to the #encode_json method on msg's class.
- */
-VALUE Google_Protobuf_encode_json(VALUE self, VALUE msg_rb) {
-  VALUE klass = CLASS_OF(msg_rb);
-  return Message_encode_json(klass, msg_rb);
-}
-
-/*
- * call-seq:
- *     Google::Protobuf.decode(class, bytes) => msg
- *
- * Decodes the given bytes as protocol buffers wire format under the
- * interpretation given by the given class's message definition. This is an
- * alternative to the #decode method on the given class.
- */
-VALUE Google_Protobuf_decode(VALUE self, VALUE klass, VALUE msg_rb) {
-  return Message_decode(klass, msg_rb);
-}
-
-/*
- * call-seq:
- *     Google::Protobuf.decode_json(class, json_string) => msg
- *
- * Decodes the given JSON string under the interpretation given by the given
- * class's message definition. This is an alternative to the #decode_json method
- * on the given class.
- */
-VALUE Google_Protobuf_decode_json(VALUE self, VALUE klass, VALUE msg_rb) {
-  return Message_decode_json(klass, msg_rb);
-}
diff --git a/ruby/ext/google/protobuf_c/message.c b/ruby/ext/google/protobuf_c/message.c
index 7e58a617..6e850427 100644
--- a/ruby/ext/google/protobuf_c/message.c
+++ b/ruby/ext/google/protobuf_c/message.c
@@ -329,6 +329,30 @@ VALUE Message_inspect(VALUE _self) {
   return str;
 }
 
+
+VALUE Message_to_h(VALUE _self) {
+  MessageHeader* self;
+  TypedData_Get_Struct(_self, MessageHeader, &Message_type, self);
+
+  VALUE hash = rb_hash_new();
+
+  upb_msg_field_iter it;
+  for (upb_msg_field_begin(&it, self->descriptor->msgdef);
+       !upb_msg_field_done(&it);
+       upb_msg_field_next(&it)) {
+    const upb_fielddef* field = upb_msg_iter_field(&it);
+    VALUE msg_value = layout_get(self->descriptor->layout, Message_data(self), field);
+    VALUE msg_key   = ID2SYM(rb_intern(upb_fielddef_name(field)));
+    if (upb_fielddef_label(field) == UPB_LABEL_REPEATED) {
+      msg_value = RepeatedField_to_ary(msg_value);
+    }
+    rb_hash_aset(hash, msg_key, msg_value);
+  }
+  return hash;
+}
+
+
+
 /*
  * call-seq:
  *     Message.[](index) => value
@@ -399,6 +423,10 @@ VALUE build_class_from_descriptor(Descriptor* desc) {
       rb_cObject);
   rb_iv_set(klass, kDescriptorInstanceVar, get_def_obj(desc->msgdef));
   rb_define_alloc_func(klass, Message_alloc);
+  rb_require("google/protobuf/message_exts");
+  rb_include_module(klass, rb_eval_string("Google::Protobuf::MessageExts"));
+  rb_extend_object(klass, rb_eval_string("Google::Protobuf::MessageExts::ClassMethods"));
+
   rb_define_method(klass, "method_missing",
                    Message_method_missing, -1);
   rb_define_method(klass, "initialize", Message_initialize, -1);
@@ -407,6 +435,8 @@ VALUE build_class_from_descriptor(Descriptor* desc) {
   rb_define_method(klass, "clone", Message_dup, 0);
   rb_define_method(klass, "==", Message_eq, 1);
   rb_define_method(klass, "hash", Message_hash, 0);
+  rb_define_method(klass, "to_h", Message_to_h, 0);
+  rb_define_method(klass, "to_hash", Message_to_h, 0);
   rb_define_method(klass, "inspect", Message_inspect, 0);
   rb_define_method(klass, "[]", Message_index, 1);
   rb_define_method(klass, "[]=", Message_index_set, 2);
@@ -415,6 +445,7 @@ VALUE build_class_from_descriptor(Descriptor* desc) {
   rb_define_singleton_method(klass, "decode_json", Message_decode_json, 1);
   rb_define_singleton_method(klass, "encode_json", Message_encode_json, 1);
   rb_define_singleton_method(klass, "descriptor", Message_descriptor, 0);
+
   return klass;
 }
 
diff --git a/ruby/ext/google/protobuf_c/protobuf.c b/ruby/ext/google/protobuf_c/protobuf.c
index d2d35033..8ab518a5 100644
--- a/ruby/ext/google/protobuf_c/protobuf.c
+++ b/ruby/ext/google/protobuf_c/protobuf.c
@@ -86,13 +86,6 @@ void Init_protobuf_c() {
   RepeatedField_register(protobuf);
   Map_register(protobuf);
 
-  rb_define_singleton_method(protobuf, "encode", Google_Protobuf_encode, 1);
-  rb_define_singleton_method(protobuf, "decode", Google_Protobuf_decode, 2);
-  rb_define_singleton_method(protobuf, "encode_json",
-                             Google_Protobuf_encode_json, 1);
-  rb_define_singleton_method(protobuf, "decode_json",
-                             Google_Protobuf_decode_json, 2);
-
   rb_define_singleton_method(protobuf, "deep_copy",
                              Google_Protobuf_deep_copy, 1);
 
diff --git a/ruby/ext/google/protobuf_c/protobuf.h b/ruby/ext/google/protobuf_c/protobuf.h
index 985b7f3d..f9a0f958 100644
--- a/ruby/ext/google/protobuf_c/protobuf.h
+++ b/ruby/ext/google/protobuf_c/protobuf.h
@@ -374,6 +374,7 @@ VALUE RepeatedField_clear(VALUE _self);
 VALUE RepeatedField_length(VALUE _self);
 VALUE RepeatedField_dup(VALUE _self);
 VALUE RepeatedField_deep_copy(VALUE _self);
+VALUE RepeatedField_to_ary(VALUE _self);
 VALUE RepeatedField_eq(VALUE _self, VALUE _other);
 VALUE RepeatedField_hash(VALUE _self);
 VALUE RepeatedField_inspect(VALUE _self);
@@ -497,11 +498,6 @@ VALUE Message_encode(VALUE klass, VALUE msg_rb);
 VALUE Message_decode_json(VALUE klass, VALUE data);
 VALUE Message_encode_json(VALUE klass, VALUE msg_rb);
 
-VALUE Google_Protobuf_encode(VALUE self, VALUE msg_rb);
-VALUE Google_Protobuf_decode(VALUE self, VALUE klass, VALUE msg_rb);
-VALUE Google_Protobuf_encode_json(VALUE self, VALUE msg_rb);
-VALUE Google_Protobuf_decode_json(VALUE self, VALUE klass, VALUE msg_rb);
-
 VALUE Google_Protobuf_deep_copy(VALUE self, VALUE obj);
 
 VALUE build_module_from_enumdesc(EnumDescriptor* enumdef);
diff --git a/ruby/lib/google/protobuf.rb b/ruby/lib/google/protobuf.rb
index 72797245..99b17929 100644
--- a/ruby/lib/google/protobuf.rb
+++ b/ruby/lib/google/protobuf.rb
@@ -28,6 +28,9 @@
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+# require mixins before we hook them into the java & c code
+require 'google/protobuf/message_exts'
+
 if RUBY_PLATFORM == "java"
   require 'json'
   require 'google/protobuf_java'
@@ -36,3 +39,25 @@ else
 end
 
 require 'google/protobuf/repeated_field'
+
+module Google
+  module Protobuf
+
+    def self.encode(msg)
+      msg.to_proto
+    end
+
+    def self.encode_json(msg)
+      msg.to_json
+    end
+
+    def self.decode(klass, proto)
+      klass.decode(proto)
+    end
+
+    def self.decode_json(klass, json)
+      klass.decode_json(json)
+    end
+
+  end
+end
diff --git a/ruby/lib/google/protobuf/message_exts.rb b/ruby/lib/google/protobuf/message_exts.rb
new file mode 100644
index 00000000..e10266ba
--- /dev/null
+++ b/ruby/lib/google/protobuf/message_exts.rb
@@ -0,0 +1,53 @@
+# 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.
+
+module Google
+  module Protobuf
+    module MessageExts
+
+      #this is only called in jruby; mri loades the ClassMethods differently
+      def self.included(klass)
+        klass.extend(ClassMethods)
+      end
+
+      module ClassMethods
+      end
+
+      def to_json
+        self.class.encode_json(self)
+      end
+
+      def to_proto
+        self.class.encode(self)
+      end
+
+    end
+  end
+end
diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyDescriptor.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyDescriptor.java
index 51c50be8..dd9179b0 100644
--- a/ruby/src/main/java/com/google/protobuf/jruby/RubyDescriptor.java
+++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyDescriptor.java
@@ -248,6 +248,8 @@ public class RubyDescriptor extends RubyObject {
         klass.setAllocator(allocator);
         klass.makeMetaClass(runtime.getObject().getMetaClass());
         klass.inherit(runtime.getObject());
+        RubyModule messageExts = runtime.getClassFromPath("Google::Protobuf::MessageExts");
+        klass.include(new IRubyObject[] {messageExts});
         klass.instance_variable_set(runtime.newString(Utils.DESCRIPTOR_INSTANCE_VAR), this);
         klass.defineAnnotatedMethods(RubyMessage.class);
         return klass;
diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyMap.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyMap.java
index b25dc6e1..2d4c03b5 100644
--- a/ruby/src/main/java/com/google/protobuf/jruby/RubyMap.java
+++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyMap.java
@@ -338,7 +338,7 @@ public class RubyMap extends RubyObject {
         return newMap;
     }
 
-    @JRubyMethod(name = "to_h")
+    @JRubyMethod(name = {"to_h", "to_hash"})
     public RubyHash toHash(ThreadContext context) {
         return RubyHash.newHash(context.runtime, table, context.runtime.getNil());
     }
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 c7fd7aa7..547ab22c 100644
--- a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java
+++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java
@@ -338,16 +338,20 @@ public class RubyMessage extends RubyObject {
         return ret;
     }
 
-    @JRubyMethod(name = "to_h")
+    @JRubyMethod(name = {"to_h", "to_hash"})
     public IRubyObject toHash(ThreadContext context) {
         Ruby runtime = context.runtime;
         RubyHash ret = RubyHash.newHash(runtime);
         for (Descriptors.FieldDescriptor fdef : this.descriptor.getFields()) {
             IRubyObject value = getField(context, fdef);
-            if (value.respondsTo("to_h")) {
-                value = Helpers.invoke(context, value, "to_h");
+            if (!value.isNil()) {
+                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");
+                }
             }
-            ret.fastASet(runtime.newString(fdef.getName()), value);
+            ret.fastASet(runtime.newSymbol(fdef.getName()), value);
         }
         return ret;
     }
diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyProtobuf.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyProtobuf.java
index cb3fcd48..2cf210d2 100644
--- a/ruby/src/main/java/com/google/protobuf/jruby/RubyProtobuf.java
+++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyProtobuf.java
@@ -50,56 +50,6 @@ public class RubyProtobuf {
 
     /*
      * call-seq:
-     *     Google::Protobuf.encode(msg) => bytes
-     *
-     * Encodes the given message object to protocol buffers wire format. This is an
-     * alternative to the #encode method on msg's class.
-     */
-    @JRubyMethod(meta = true)
-    public static IRubyObject encode(ThreadContext context, IRubyObject self, IRubyObject message) {
-        return RubyMessage.encode(context, message.getMetaClass(), message);
-    }
-
-    /*
-     * call-seq:
-     *     Google::Protobuf.decode(class, bytes) => msg
-     *
-     * Decodes the given bytes as protocol buffers wire format under the
-     * interpretation given by the given class's message definition. This is an
-     * alternative to the #decode method on the given class.
-     */
-    @JRubyMethod(meta = true)
-    public static IRubyObject decode(ThreadContext context, IRubyObject self, IRubyObject klazz, IRubyObject message) {
-        return RubyMessage.decode(context, klazz, message);
-    }
-
-    /*
-     * call-seq:
-     *     Google::Protobuf.encode_json(msg) => json_string
-     *
-     * Encodes the given message object to its JSON representation. This is an
-     * alternative to the #encode_json method on msg's class.
-     */
-    @JRubyMethod(name = "encode_json", meta = true)
-    public static IRubyObject encodeJson(ThreadContext context, IRubyObject self, IRubyObject message) {
-        return RubyMessage.encodeJson(context, message.getMetaClass(), message);
-    }
-
-    /*
-     * call-seq:
-     *     Google::Protobuf.decode_json(class, json_string) => msg
-     *
-     * Decodes the given JSON string under the interpretation given by the given
-     * class's message definition. This is an alternative to the #decode_json method
-     * on the given class.
-     */
-    @JRubyMethod(name = "decode_json", meta = true)
-    public static IRubyObject decodeJson(ThreadContext context, IRubyObject self, IRubyObject klazz, IRubyObject message) {
-        return RubyMessage.decodeJson(context, klazz, message);
-    }
-
-    /*
-     * call-seq:
      *     Google::Protobuf.deep_copy(obj) => copy_of_obj
      *
      * Performs a deep copy of either a RepeatedField instance or a message object,
diff --git a/ruby/tests/basic.rb b/ruby/tests/basic.rb
index b6e26108..40c20078 100644
--- a/ruby/tests/basic.rb
+++ b/ruby/tests/basic.rb
@@ -822,6 +822,67 @@ module BasicTest
       assert m == m2
     end
 
+    def test_encode_decode_helpers
+      m = TestMessage.new(:optional_string => 'foo', :repeated_string => ['bar1', 'bar2'])
+      json = m.to_json
+      m2 = TestMessage.decode_json(json)
+      assert m2.optional_string == 'foo'
+      assert m2.repeated_string == ['bar1', 'bar2']
+
+      proto = m.to_proto
+      m2 = TestMessage.decode(proto)
+      assert m2.optional_string == 'foo'
+      assert m2.repeated_string == ['bar1', 'bar2']
+    end
+
+    def test_protobuf_encode_decode_helpers
+      m = TestMessage.new(:optional_string => 'foo', :repeated_string => ['bar1', 'bar2'])
+      encoded_msg = Google::Protobuf.encode(m)
+      assert_equal m.to_proto, encoded_msg
+
+      decoded_msg = Google::Protobuf.decode(TestMessage, encoded_msg)
+      assert_equal TestMessage.decode(m.to_proto), decoded_msg
+    end
+
+    def test_protobuf_encode_decode_json_helpers
+      m = TestMessage.new(:optional_string => 'foo', :repeated_string => ['bar1', 'bar2'])
+      encoded_msg = Google::Protobuf.encode_json(m)
+      assert_equal m.to_json, encoded_msg
+
+      decoded_msg = Google::Protobuf.decode_json(TestMessage, encoded_msg)
+      assert_equal TestMessage.decode_json(m.to_json), decoded_msg
+    end
+
+    def test_to_h
+      m = TestMessage.new(:optional_bool => true, :optional_double => -10.100001, :optional_string => 'foo', :repeated_string => ['bar1', 'bar2'])
+      expected_result = {
+        :optional_bool=>true,
+        :optional_bytes=>"",
+        :optional_double=>-10.100001,
+        :optional_enum=>:Default,
+        :optional_float=>0.0,
+        :optional_int32=>0,
+        :optional_int64=>0,
+        :optional_msg=>nil,
+        :optional_string=>"foo",
+        :optional_uint32=>0,
+        :optional_uint64=>0,
+        :repeated_bool=>[],
+        :repeated_bytes=>[],
+        :repeated_double=>[],
+        :repeated_enum=>[],
+        :repeated_float=>[],
+        :repeated_int32=>[],
+        :repeated_int64=>[],
+        :repeated_msg=>[],
+        :repeated_string=>["bar1", "bar2"],
+        :repeated_uint32=>[],
+        :repeated_uint64=>[]
+      }
+      assert_equal expected_result, m.to_h
+    end
+
+
     def test_def_errors
       s = Google::Protobuf::DescriptorPool.new
       assert_raise TypeError do