Skip to content

Add Message.from_json #411

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

Open
wants to merge 7 commits into
base: 3-10-stable
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion lib/protobuf/field/bytes_field.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,18 @@ def wire_type

def coerce!(value)
case value
when String, Symbol
when String
if value.encoding == Encoding::ASCII_8BIT
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to make this configurable at the from_json method? You can specify if your uses Base64 encoded binary blobs? Maybe true by default but allow the caller to turn it off? That way we don't have to guess.

# This is a "binary" string
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
value.to_s
when NilClass
nil
Expand Down
5 changes: 5 additions & 0 deletions lib/protobuf/field/enum_field.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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, options={})
enum = type_class.enums.find { |e| e.to_i == value }
enum.to_s(:name)
end

private

##
Expand Down
2 changes: 2 additions & 0 deletions lib/protobuf/field/field_array.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
18 changes: 17 additions & 1 deletion lib/protobuf/message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,22 @@ def self.to_json
name
end

def self.from_json(json)
fields = normalize_json(JSON.parse(json))
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)] }]
Copy link
Member

Choose a reason for hiding this comment

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

The underscore thing seems like we'd want to make that optional?

else
ob
end
end

##
# Constructor
#
Expand Down Expand Up @@ -150,7 +166,7 @@ def to_json_hash(options = {})

# 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(options)
elsif field.respond_to?(:json_encode)
field.json_encode(value)
Expand Down
6 changes: 3 additions & 3 deletions spec/encoding/all_types_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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),
Expand Down Expand Up @@ -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,
Expand Down
Binary file modified spec/encoding/extreme_values_spec.rb
Binary file not shown.
18 changes: 18 additions & 0 deletions spec/lib/protobuf/field/enum_field_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
24 changes: 22 additions & 2 deletions spec/lib/protobuf/message_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand All @@ -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])
Expand All @@ -449,6 +449,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
Expand Down
2 changes: 1 addition & 1 deletion spec/support/protos/resource.pb.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -169,4 +170,3 @@ class ResourceService < ::Protobuf::Rpc::Service
end

end

1 change: 1 addition & 0 deletions spec/support/protos/resource.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down