diff --git a/ruby/ext/google/protobuf_c/message.c b/ruby/ext/google/protobuf_c/message.c index d48c8c025f..3b12c93f3d 100644 --- a/ruby/ext/google/protobuf_c/message.c +++ b/ruby/ext/google/protobuf_c/message.c @@ -766,58 +766,34 @@ static VALUE Message_CreateHash(const upb_Message* msg, if (!msg) return Qnil; VALUE hash = rb_hash_new(); - int n = upb_MessageDef_FieldCount(m); - bool is_proto2; - - // We currently have a few behaviors that are specific to proto2. - // This is unfortunate, we should key behaviors off field attributes (like - // whether a field has presence), not proto2 vs. proto3. We should see if we - // can change this without breaking users. - is_proto2 = upb_MessageDef_Syntax(m) == kUpb_Syntax_Proto2; - - for (int i = 0; i < n; i++) { - const upb_FieldDef* field = upb_MessageDef_Field(m, i); - TypeInfo type_info = TypeInfo_get(field); - upb_MessageValue msgval; - VALUE msg_value; - VALUE msg_key; - - if (!is_proto2 && upb_FieldDef_IsSubMessage(field) && - !upb_FieldDef_IsRepeated(field) && - !upb_Message_HasFieldByDef(msg, field)) { - // TODO: Legacy behavior, remove when we fix the is_proto2 differences. - msg_key = ID2SYM(rb_intern(upb_FieldDef_Name(field))); - rb_hash_aset(hash, msg_key, Qnil); - continue; - } + size_t iter = kUpb_Message_Begin; + const upb_DefPool* pool = upb_FileDef_Pool(upb_MessageDef_File(m)); + const upb_FieldDef* field; + upb_MessageValue val; - // Do not include fields that are not present (oneof or optional fields). - if (is_proto2 && upb_FieldDef_HasPresence(field) && - !upb_Message_HasFieldByDef(msg, field)) { + while (upb_Message_Next(msg, m, pool, &field, &val, &iter)) { + if (upb_FieldDef_IsExtension(field)) { + // TODO: allow extensions once we have decided what naming scheme the + // symbol should use. eg. :"[pkg.ext]" continue; } - msg_key = ID2SYM(rb_intern(upb_FieldDef_Name(field))); - msgval = upb_Message_GetFieldByDef(msg, field); - - // Proto2 omits empty map/repeated filds also. + TypeInfo type_info = TypeInfo_get(field); + VALUE msg_value; if (upb_FieldDef_IsMap(field)) { const upb_MessageDef* entry_m = upb_FieldDef_MessageSubDef(field); const upb_FieldDef* key_f = upb_MessageDef_FindFieldByNumber(entry_m, 1); const upb_FieldDef* val_f = upb_MessageDef_FindFieldByNumber(entry_m, 2); upb_CType key_type = upb_FieldDef_CType(key_f); - msg_value = Map_CreateHash(msgval.map_val, key_type, TypeInfo_get(val_f)); + msg_value = Map_CreateHash(val.map_val, key_type, TypeInfo_get(val_f)); } else if (upb_FieldDef_IsRepeated(field)) { - if (is_proto2 && - (!msgval.array_val || upb_Array_Size(msgval.array_val) == 0)) { - continue; - } - msg_value = RepeatedField_CreateArray(msgval.array_val, type_info); + msg_value = RepeatedField_CreateArray(val.array_val, type_info); } else { - msg_value = Scalar_CreateHash(msgval, type_info); + msg_value = Scalar_CreateHash(val, type_info); } + VALUE msg_key = ID2SYM(rb_intern(upb_FieldDef_Name(field))); rb_hash_aset(hash, msg_key, msg_value); } diff --git a/ruby/lib/google/protobuf/ffi/ffi.rb b/ruby/lib/google/protobuf/ffi/ffi.rb index 77ee42ead2..1858c91a7d 100644 --- a/ruby/lib/google/protobuf/ffi/ffi.rb +++ b/ruby/lib/google/protobuf/ffi/ffi.rb @@ -190,6 +190,8 @@ module Google :str_val, StringView end + Upb_Message_Begin = -1 + class MutableMessageValue < ::FFI::Union layout :map, :Map, :msg, :Message, diff --git a/ruby/lib/google/protobuf/ffi/internal/convert.rb b/ruby/lib/google/protobuf/ffi/internal/convert.rb index 2b44315634..b1eb0005e2 100644 --- a/ruby/lib/google/protobuf/ffi/internal/convert.rb +++ b/ruby/lib/google/protobuf/ffi/internal/convert.rb @@ -190,39 +190,23 @@ module Google def to_h_internal(msg, message_descriptor) return nil if msg.nil? or msg.null? hash = {} - is_proto2 = Google::Protobuf::FFI.message_def_syntax(message_descriptor) == :Proto2 - message_descriptor.each do |field_descriptor| - # TODO: Legacy behavior, remove when we fix the is_proto2 differences. - if !is_proto2 and - field_descriptor.sub_message? and - !field_descriptor.repeated? and - !Google::Protobuf::FFI.get_message_has(msg, field_descriptor) - hash[field_descriptor.name.to_sym] = nil - next - end - - # Do not include fields that are not present (oneof or optional fields). - if is_proto2 and field_descriptor.has_presence? and !Google::Protobuf::FFI.get_message_has(msg, field_descriptor) - next - end + iter = ::FFI::MemoryPointer.new(:size_t, 1) + iter.write(:size_t, Google::Protobuf::FFI::Upb_Message_Begin) + message_value = Google::Protobuf::FFI::MessageValue.new + field_def_ptr = ::FFI::MemoryPointer.new :pointer - message_value = Google::Protobuf::FFI.get_message_value msg, field_descriptor + while Google::Protobuf::FFI::message_next(msg, message_descriptor, nil, field_def_ptr, message_value, iter) do + field_descriptor = FieldDescriptor.from_native field_def_ptr.get_pointer(0) - # Proto2 omits empty map/repeated fields also. if field_descriptor.map? hash_entry = map_create_hash(message_value[:map_val], field_descriptor) elsif field_descriptor.repeated? - array = message_value[:array_val] - if is_proto2 and (array.null? || Google::Protobuf::FFI.array_size(array).zero?) - next - end - hash_entry = repeated_field_create_array(array, field_descriptor, field_descriptor.type) + hash_entry = repeated_field_create_array(message_value[:array_val], field_descriptor, field_descriptor.type) else hash_entry = scalar_create_hash(message_value, field_descriptor.type, field_descriptor: field_descriptor) end hash[field_descriptor.name.to_sym] = hash_entry - end hash diff --git a/ruby/lib/google/protobuf/ffi/message.rb b/ruby/lib/google/protobuf/ffi/message.rb index 39eb403853..5d9a365927 100644 --- a/ruby/lib/google/protobuf/ffi/message.rb +++ b/ruby/lib/google/protobuf/ffi/message.rb @@ -23,6 +23,7 @@ module Google attach_function :get_mutable_message, :upb_Message_Mutable, [:Message, FieldDescriptor, Internal::Arena], MutableMessageValue.by_value attach_function :get_message_which_oneof, :upb_Message_WhichOneof, [:Message, OneofDescriptor], FieldDescriptor attach_function :message_discard_unknown, :upb_Message_DiscardUnknown, [:Message, Descriptor, :int], :bool + attach_function :message_next, :upb_Message_Next, [:Message, Descriptor, :DefPool, :FieldDefPointer, MessageValue.by_ref, :pointer], :bool # MessageValue attach_function :message_value_equal, :shared_Msgval_IsEqual, [MessageValue.by_value, MessageValue.by_value, CType, Descriptor], :bool attach_function :message_value_hash, :shared_Msgval_GetHash, [MessageValue.by_value, CType, Descriptor, :uint64_t], :uint64_t 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 ec57a4bff2..2ac9fc912e 100644 --- a/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java +++ b/ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java @@ -787,33 +787,29 @@ public class RubyMessage extends RubyObject { public IRubyObject toHash(ThreadContext context) { Ruby runtime = context.runtime; RubyHash ret = RubyHash.newHash(runtime); - for (FieldDescriptor fdef : this.descriptor.getFields()) { + build(context, 0, SINK_MAXIMUM_NESTING); // Sync Ruby data to the Builder object. + for (Map.Entry field : builder.getAllFields().entrySet()) { + FieldDescriptor fdef = field.getKey(); IRubyObject value = getFieldInternal(context, fdef, proto3); - if (!value.isNil()) { - if (fdef.isRepeated() && !fdef.isMapField()) { - if (!proto3 && ((RubyRepeatedField) value).size() == 0) - continue; // Don't output empty repeated fields for proto2 - if (fdef.getType() != FieldDescriptor.Type.MESSAGE) { - value = Helpers.invoke(context, value, "to_a"); - } else { - RubyArray ary = value.convertToArray(); - for (int i = 0; i < ary.size(); i++) { - IRubyObject submsg = Helpers.invoke(context, ary.eltInternal(i), "to_h"); - ary.eltInternalSet(i, submsg); - } - - value = ary.to_ary(); - } - } else if (value.respondsTo("to_h")) { - value = Helpers.invoke(context, value, "to_h"); - } else if (value.respondsTo("to_a")) { + if (fdef.isRepeated() && !fdef.isMapField()) { + if (fdef.getType() != FieldDescriptor.Type.MESSAGE) { value = Helpers.invoke(context, value, "to_a"); + } else { + RubyArray ary = value.convertToArray(); + for (int i = 0; i < ary.size(); i++) { + IRubyObject submsg = Helpers.invoke(context, ary.eltInternal(i), "to_h"); + ary.eltInternalSet(i, submsg); + } + + value = ary.to_ary(); } + } else 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"); } - if (proto3 || !value.isNil()) { - ret.fastASet(runtime.newSymbol(fdef.getName()), value); - } + ret.fastASet(runtime.newSymbol(fdef.getName()), value); } return ret; } diff --git a/ruby/tests/basic.rb b/ruby/tests/basic.rb index 4dc45e748c..b71fdac4ea 100755 --- a/ruby/tests/basic.rb +++ b/ruby/tests/basic.rb @@ -469,32 +469,19 @@ module BasicTest #end def test_to_h - m = TestMessage.new(:optional_bool => true, :optional_double => -10.100001, :optional_string => 'foo', :repeated_string => ['bar1', 'bar2'], :repeated_msg => [TestMessage2.new(:foo => 100)]) + m = TestMessage.new( + :optional_bool => true, + :optional_double => -10.100001, + :optional_string => 'foo', + :repeated_string => ['bar1', 'bar2'], + :repeated_msg => [TestMessage2.new(:foo => 100)] + ) 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_msg2=>nil, - :optional_proto2_submessage=>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=>[{:foo => 100}], :repeated_string=>["bar1", "bar2"], - :repeated_uint32=>[], - :repeated_uint64=>[] + :repeated_msg=>[{:foo => 100}], } assert_equal expected_result, m.to_h diff --git a/upb/reflection/message.h b/upb/reflection/message.h index d31f6b9b26..b3b52d7ace 100644 --- a/upb/reflection/message.h +++ b/upb/reflection/message.h @@ -71,9 +71,10 @@ UPB_API bool upb_Message_SetFieldByDef(upb_Message* msg, const upb_FieldDef* f, #define kUpb_Message_Begin -1 -bool upb_Message_Next(const upb_Message* msg, const upb_MessageDef* m, - const upb_DefPool* ext_pool, const upb_FieldDef** f, - upb_MessageValue* val, size_t* iter); +UPB_API bool upb_Message_Next(const upb_Message* msg, const upb_MessageDef* m, + const upb_DefPool* ext_pool, + const upb_FieldDef** f, upb_MessageValue* val, + size_t* iter); // Clears all unknown field data from this message and all submessages. UPB_API bool upb_Message_DiscardUnknown(upb_Message* msg,