Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ruby: Encode decode cleanup and behavior normalization #338

Merged
merged 1 commit into from May 15, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
47 changes: 0 additions & 47 deletions ruby/ext/google/protobuf_c/encode_decode.c
Expand Up @@ -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);
}
31 changes: 31 additions & 0 deletions ruby/ext/google/protobuf_c/message.c
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#to_hash is a common alias

rb_define_method(klass, "inspect", Message_inspect, 0);
rb_define_method(klass, "[]", Message_index, 1);
rb_define_method(klass, "[]=", Message_index_set, 2);
Expand All @@ -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;
}

Expand Down
7 changes: 0 additions & 7 deletions ruby/ext/google/protobuf_c/protobuf.c
Expand Up @@ -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);

Expand Down
6 changes: 1 addition & 5 deletions ruby/ext/google/protobuf_c/protobuf.h
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
25 changes: 25 additions & 0 deletions ruby/lib/google/protobuf.rb
Expand Up @@ -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'
Expand All @@ -36,3 +39,25 @@
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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these methods existed in both jruby and mri, but I think it is cleaner to just define them once

53 changes: 53 additions & 0 deletions 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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I'll clean this up if the PR direction is worth going in)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the performance delta, I'd rather keep JSON in C, though I wish it were faster to have it at this level, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. FYI: the JSON gem is using the c-ext when running under MRI and it is still quite a bit slower than the upd library. kind of amazing actually


def to_json
self.class.encode_json(self)
end

def to_proto
self.class.encode(self)
end

end
end
end
Expand Up @@ -248,6 +248,8 @@ public IRubyObject allocate(Ruby runtime, RubyClass klazz) {
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;
Expand Down
2 changes: 1 addition & 1 deletion ruby/src/main/java/com/google/protobuf/jruby/RubyMap.java
Expand Up @@ -338,7 +338,7 @@ public IRubyObject dup(ThreadContext context) {
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());
}
Expand Down
12 changes: 8 additions & 4 deletions ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java
Expand Up @@ -338,16 +338,20 @@ public static IRubyObject decodeJson(ThreadContext context, IRubyObject recv, IR
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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE this is a breaking change. It brings it inline with the initializers, and it is quite a bit faster, but it is a breaking change at that...

}
return ret;
}
Expand Down
50 changes: 0 additions & 50 deletions ruby/src/main/java/com/google/protobuf/jruby/RubyProtobuf.java
Expand Up @@ -48,56 +48,6 @@ public static void createProtobuf(Ruby runtime) {
mProtobuf.defineAnnotatedMethods(RubyProtobuf.class);
}

/*
* 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
Expand Down