From 7ead826c93cc16d59309557150e242f2e6edbf3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aslak=20Helles=C3=B8y?= Date: Mon, 6 Jan 2020 22:30:40 +0000 Subject: [PATCH 1/6] Add Message.from_json --- lib/protobuf/field/bytes_field.rb | 10 +++++++++- lib/protobuf/field/field_array.rb | 2 ++ lib/protobuf/message.rb | 16 ++++++++++++++++ spec/lib/protobuf/message_spec.rb | 20 ++++++++++++++++++++ spec/support/protos/resource.pb.rb | 2 +- spec/support/protos/resource.proto | 1 + 6 files changed, 49 insertions(+), 2 deletions(-) diff --git a/lib/protobuf/field/bytes_field.rb b/lib/protobuf/field/bytes_field.rb index 81a3634d..5bf3fdc4 100644 --- a/lib/protobuf/field/bytes_field.rb +++ b/lib/protobuf/field/bytes_field.rb @@ -48,7 +48,15 @@ def wire_type def coerce!(value) case value - when String, Symbol + when String + if value.encoding == Encoding::ASCII_8BIT + # This is a "binary" string + value + else + # Assume the value is Base64 encoded (from JSON) + Base64.decode64(value) + end + when Symbol value.to_s when NilClass nil diff --git a/lib/protobuf/field/field_array.rb b/lib/protobuf/field/field_array.rb index e4f2eb1a..eac47d62 100644 --- a/lib/protobuf/field/field_array.rb +++ b/lib/protobuf/field/field_array.rb @@ -81,6 +81,8 @@ def normalize(value) if field.is_a?(::Protobuf::Field::EnumField) field.type_class.fetch(value) + elsif field.is_a?(::Protobuf::Field::BytesField) + field.coerce!(value) elsif field.is_a?(::Protobuf::Field::MessageField) && value.is_a?(field.type_class) value elsif field.is_a?(::Protobuf::Field::MessageField) && value.respond_to?(:to_hash) diff --git a/lib/protobuf/message.rb b/lib/protobuf/message.rb index a13c0d19..13b5ff48 100644 --- a/lib/protobuf/message.rb +++ b/lib/protobuf/message.rb @@ -21,6 +21,22 @@ def self.to_json name end + def self.from_json(json) + fields = normalize_json(JSON.parse(json)) + self.new(fields) + end + + def self.normalize_json(ob) + case ob + when Array + ob.map {|value| normalize_json(value) } + when Hash + Hash[*ob.flat_map {|key, value| [key.underscore, normalize_json(value)] }] + else + ob + end + end + ## # Constructor # diff --git a/spec/lib/protobuf/message_spec.rb b/spec/lib/protobuf/message_spec.rb index 96668b67..4394eac3 100644 --- a/spec/lib/protobuf/message_spec.rb +++ b/spec/lib/protobuf/message_spec.rb @@ -439,6 +439,26 @@ end end + describe '.from_json' do + it 'decodes optional bytes field with base64' do + expected_single_bytes = "\x06\x8D1HP\x17:b".unpack('C*') + single_bytes = ::Test::ResourceFindRequest + .from_json('{"singleBytes":"Bo0xSFAXOmI="}') + .single_bytes.unpack('C*') + + expect(single_bytes).to(eq(expected_single_bytes)) + end + + it 'decodes repeated bytes field with base64' do + expected_widget_bytes = ["\x06\x8D1HP\x17:b"].map {|s| s.unpack('C*')} + widget_bytes = ::Test::ResourceFindRequest + .from_json('{"widgetBytes":["Bo0xSFAXOmI="]}') + .widget_bytes.map {|s| s.unpack('C*')} + + expect(widget_bytes).to(eq(expected_widget_bytes)) + end + end + describe '.to_json' do it 'returns the class name of the message for use in json encoding' do expect do diff --git a/spec/support/protos/resource.pb.rb b/spec/support/protos/resource.pb.rb index f81ef52f..e765b1ce 100644 --- a/spec/support/protos/resource.pb.rb +++ b/spec/support/protos/resource.pb.rb @@ -72,6 +72,7 @@ class ResourceFindRequest optional :bool, :active, 2 repeated :string, :widgets, 3 repeated :bytes, :widget_bytes, 4 + optional :bytes, :single_bytes, 5 end class ResourceSleepRequest @@ -169,4 +170,3 @@ class ResourceService < ::Protobuf::Rpc::Service end end - diff --git a/spec/support/protos/resource.proto b/spec/support/protos/resource.proto index 70b338b3..a5573e24 100644 --- a/spec/support/protos/resource.proto +++ b/spec/support/protos/resource.proto @@ -47,6 +47,7 @@ message ResourceFindRequest { optional bool active = 2; repeated string widgets = 3; repeated bytes widget_bytes = 4; + optional bytes single_bytes = 5; } message ResourceSleepRequest { From af04e05f5e19e55ddf010e9fcb239e6cbaef0f2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aslak=20Helles=C3=B8y?= Date: Tue, 7 Jan 2020 16:03:20 +0000 Subject: [PATCH 2/6] Serialise enum fields to JSON as name instead of integer value --- lib/protobuf/field/enum_field.rb | 5 +++++ lib/protobuf/message.rb | 2 +- spec/lib/protobuf/field/enum_field_spec.rb | 18 ++++++++++++++++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/lib/protobuf/field/enum_field.rb b/lib/protobuf/field/enum_field.rb index 6993faff..59a4b721 100644 --- a/lib/protobuf/field/enum_field.rb +++ b/lib/protobuf/field/enum_field.rb @@ -37,6 +37,11 @@ def coerce!(value) type_class.fetch(value) || fail(TypeError, "Invalid Enum value: #{value.inspect} for #{name}") end + def json_encode(value) + enum = type_class.enums.find {|e| e.to_i == value} + enum.to_s(:name) + end + private ## diff --git a/lib/protobuf/message.rb b/lib/protobuf/message.rb index 13b5ff48..c255a5f4 100644 --- a/lib/protobuf/message.rb +++ b/lib/protobuf/message.rb @@ -164,7 +164,7 @@ def to_json_hash # NB: to_json_hash_value should come before json_encode so as to handle # repeated fields without extra logic. - hashed_value = if value.respond_to?(:to_json_hash_value) + hashed_value = if value.respond_to?(:to_json_hash_value) && !field.is_a?(::Protobuf::Field::EnumField) value.to_json_hash_value elsif field.respond_to?(:json_encode) field.json_encode(value) diff --git a/spec/lib/protobuf/field/enum_field_spec.rb b/spec/lib/protobuf/field/enum_field_spec.rb index cd72760d..c2e04eed 100644 --- a/spec/lib/protobuf/field/enum_field_spec.rb +++ b/spec/lib/protobuf/field/enum_field_spec.rb @@ -23,4 +23,22 @@ expect(message.decode(instance.encode).enum).to eq(-33) end end + + # https://developers.google.com/protocol-buffers/docs/proto3#json + describe '.{to_json, from_json}' do + it 'serialises enum value as string' do + instance = message.new(:enum => :POSITIVE) + expect(instance.to_json).to eq('{"enum":"POSITIVE"}') + end + + it 'deserialises enum value as integer' do + instance = message.from_json('{"enum":22}') + expect(instance.enum).to eq(22) + end + + it 'deserialises enum value as string' do + instance = message.from_json('{"enum":"NEGATIVE"}') + expect(instance.enum).to eq(-33) + end + end end From a26a89ab9695715efe206378cf80dc1c01f84fff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aslak=20Helles=C3=B8y?= Date: Tue, 7 Jan 2020 16:30:10 +0000 Subject: [PATCH 3/6] Force binary encoding to avoid Base64 decoding --- spec/lib/protobuf/message_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/lib/protobuf/message_spec.rb b/spec/lib/protobuf/message_spec.rb index 0aeff18d..8e7dcb07 100644 --- a/spec/lib/protobuf/message_spec.rb +++ b/spec/lib/protobuf/message_spec.rb @@ -429,7 +429,7 @@ specify { expect(subject.to_json).to eq '{"name":"Test Name","active":false}' } context 'for byte fields' do - let(:bytes) { "\x06\x8D1HP\x17:b" } + let(:bytes) { "\x06\x8D1HP\x17:b".force_encoding(Encoding::ASCII_8BIT) } subject do ::Test::ResourceFindRequest.new(:widget_bytes => [bytes]) @@ -439,7 +439,7 @@ end context 'using lower camel case field names' do - let(:bytes) { "\x06\x8D1HP\x17:b" } + let(:bytes) { "\x06\x8D1HP\x17:b".force_encoding(Encoding::ASCII_8BIT) } subject do ::Test::ResourceFindRequest.new(:widget_bytes => [bytes]) From f6f2b33e82cc6774f418f927a34bdda6ae6f0b84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aslak=20Helles=C3=B8y?= Date: Tue, 7 Jan 2020 16:35:35 +0000 Subject: [PATCH 4/6] Force binary encoding to avoid Base64 decoding --- lib/protobuf/field/bytes_field.rb | 3 +++ spec/encoding/all_types_spec.rb | 6 +++--- spec/encoding/extreme_values_spec.rb | Bin 1358 -> 1432 bytes 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/protobuf/field/bytes_field.rb b/lib/protobuf/field/bytes_field.rb index 5bf3fdc4..bce24c80 100644 --- a/lib/protobuf/field/bytes_field.rb +++ b/lib/protobuf/field/bytes_field.rb @@ -54,6 +54,9 @@ def coerce!(value) value else # Assume the value is Base64 encoded (from JSON) + # Ideally we'd do the Base64 decoding while processing the JSON, + # but this is tricky to do since we don't know the protobuf field + # types when we do that. Base64.decode64(value) end when Symbol diff --git a/spec/encoding/all_types_spec.rb b/spec/encoding/all_types_spec.rb index fbd38b46..04ddb866 100644 --- a/spec/encoding/all_types_spec.rb +++ b/spec/encoding/all_types_spec.rb @@ -18,7 +18,7 @@ :optional_double => 112, :optional_bool => true, :optional_string => "115", - :optional_bytes => "116", + :optional_bytes => "116".force_encoding(Encoding::ASCII_8BIT), :optional_nested_message => Protobuf_unittest::TestAllTypes::NestedMessage.new(:bb => 118), :optional_foreign_message => Protobuf_unittest::ForeignMessage.new(:c => 119), :optional_import_message => Protobuf_unittest_import::ImportMessage.new(:d => 120), @@ -43,7 +43,7 @@ :repeated_double => [212, 312], :repeated_bool => [true, false], :repeated_string => ["215", "315"], - :repeated_bytes => ["216", "316"], + :repeated_bytes => ["216".force_encoding(Encoding::ASCII_8BIT), "316".force_encoding(Encoding::ASCII_8BIT)], :repeated_nested_message => [ ::Protobuf_unittest::TestAllTypes::NestedMessage.new(:bb => 218), ::Protobuf_unittest::TestAllTypes::NestedMessage.new(:bb => 318), @@ -88,7 +88,7 @@ :default_double => 412, :default_bool => false, :default_string => "415", - :default_bytes => "416", + :default_bytes => "416".force_encoding(Encoding::ASCII_8BIT), :default_nested_enum => ::Protobuf_unittest::TestAllTypes::NestedEnum::FOO, :default_foreign_enum => ::Protobuf_unittest::ForeignEnum::FOREIGN_FOO, :default_import_enum => ::Protobuf_unittest_import::ImportEnum::IMPORT_FOO, diff --git a/spec/encoding/extreme_values_spec.rb b/spec/encoding/extreme_values_spec.rb index 477e695aa37676462eb7cbcd5cc355e2f7fc387d..7f3d516f75c874a337b4906e197e33c9fca22cc1 100644 GIT binary patch delta 93 zcmX@dHG_KtE2FAjT7FS-YJ6&5a(+r?Ub==WlwoD%80_rn8E@g_8KSwFmC=QnNTr@E Gtc(D$yB*#D delta 17 YcmbQieU57bE8}J@Mi=JIlUZ090WkgqrvLx| From 0ad65270dd5d834fc92534ddfcf0a25fa5750259 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aslak=20Helles=C3=B8y?= Date: Tue, 7 Jan 2020 23:08:53 +0000 Subject: [PATCH 5/6] Fix rubocop warnings --- lib/protobuf/field/enum_field.rb | 2 +- lib/protobuf/message.rb | 6 +++--- spec/lib/protobuf/message_spec.rb | 10 +++++----- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/protobuf/field/enum_field.rb b/lib/protobuf/field/enum_field.rb index 59a4b721..df5448b8 100644 --- a/lib/protobuf/field/enum_field.rb +++ b/lib/protobuf/field/enum_field.rb @@ -38,7 +38,7 @@ def coerce!(value) end def json_encode(value) - enum = type_class.enums.find {|e| e.to_i == value} + enum = type_class.enums.find { |e| e.to_i == value } enum.to_s(:name) end diff --git a/lib/protobuf/message.rb b/lib/protobuf/message.rb index efa14f7d..d2b3f862 100644 --- a/lib/protobuf/message.rb +++ b/lib/protobuf/message.rb @@ -23,15 +23,15 @@ def self.to_json def self.from_json(json) fields = normalize_json(JSON.parse(json)) - self.new(fields) + new(fields) end def self.normalize_json(ob) case ob when Array - ob.map {|value| normalize_json(value) } + ob.map { |value| normalize_json(value) } when Hash - Hash[*ob.flat_map {|key, value| [key.underscore, normalize_json(value)] }] + Hash[*ob.flat_map { |key, value| [key.underscore, normalize_json(value)] }] else ob end diff --git a/spec/lib/protobuf/message_spec.rb b/spec/lib/protobuf/message_spec.rb index 8e7dcb07..40b68a6d 100644 --- a/spec/lib/protobuf/message_spec.rb +++ b/spec/lib/protobuf/message_spec.rb @@ -453,17 +453,17 @@ it 'decodes optional bytes field with base64' do expected_single_bytes = "\x06\x8D1HP\x17:b".unpack('C*') single_bytes = ::Test::ResourceFindRequest - .from_json('{"singleBytes":"Bo0xSFAXOmI="}') - .single_bytes.unpack('C*') + .from_json('{"singleBytes":"Bo0xSFAXOmI="}') + .single_bytes.unpack('C*') expect(single_bytes).to(eq(expected_single_bytes)) end it 'decodes repeated bytes field with base64' do - expected_widget_bytes = ["\x06\x8D1HP\x17:b"].map {|s| s.unpack('C*')} + expected_widget_bytes = ["\x06\x8D1HP\x17:b"].map { |s| s.unpack('C*') } widget_bytes = ::Test::ResourceFindRequest - .from_json('{"widgetBytes":["Bo0xSFAXOmI="]}') - .widget_bytes.map {|s| s.unpack('C*')} + .from_json('{"widgetBytes":["Bo0xSFAXOmI="]}') + .widget_bytes.map { |s| s.unpack('C*') } expect(widget_bytes).to(eq(expected_widget_bytes)) end From 02fb92964af28bd1b0ff44d60d519789ff4720dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aslak=20Helles=C3=B8y?= Date: Wed, 5 Feb 2020 15:12:59 +0000 Subject: [PATCH 6/6] Add optional options field to enum_field.rb. See #415. --- lib/protobuf/field/enum_field.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/protobuf/field/enum_field.rb b/lib/protobuf/field/enum_field.rb index df5448b8..12867adf 100644 --- a/lib/protobuf/field/enum_field.rb +++ b/lib/protobuf/field/enum_field.rb @@ -37,7 +37,7 @@ def coerce!(value) type_class.fetch(value) || fail(TypeError, "Invalid Enum value: #{value.inspect} for #{name}") end - def json_encode(value) + def json_encode(value, options={}) enum = type_class.enums.find { |e| e.to_i == value } enum.to_s(:name) end