From e6926cf21723f0e948d7c243328ccf2d8c7e8f3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aslak=20Helles=C3=B8y?= Date: Mon, 3 Feb 2020 16:31:59 +0000 Subject: [PATCH 1/7] Add failing spec for int64 to JSON --- spec/lib/protobuf/field/int64_field_spec.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/spec/lib/protobuf/field/int64_field_spec.rb b/spec/lib/protobuf/field/int64_field_spec.rb index 1bbe7c04..2d1d5311 100644 --- a/spec/lib/protobuf/field/int64_field_spec.rb +++ b/spec/lib/protobuf/field/int64_field_spec.rb @@ -4,4 +4,17 @@ it_behaves_like :packable_field, described_class + let(:message) do + Class.new(::Protobuf::Message) do + optional :int64, :field, 1 + end + end + + # https://developers.google.com/protocol-buffers/docs/proto3#json + describe '.{to_json, from_json}' do + it 'serialises int64 value as string' do + instance = message.new(:field => 10) + expect(instance.to_json).to eq('{"field":"10"}') + end + end end From 7631663ff96b515c99af312e188803eac9947e10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aslak=20Helles=C3=B8y?= Date: Mon, 3 Feb 2020 21:57:24 +0000 Subject: [PATCH 2/7] Implement proper JSON encoding for 64 bit ints --- lib/protobuf/field/int64_field.rb | 3 +++ lib/protobuf/field/sint64_field.rb | 3 +++ lib/protobuf/field/uint64_field.rb | 3 +++ lib/protobuf/message.rb | 6 ++++- spec/lib/protobuf/field/fixed64_field_spec.rb | 23 +++++++++++++++++++ spec/lib/protobuf/field/int64_field_spec.rb | 16 ++++++++++--- .../lib/protobuf/field/sfixed64_field_spec.rb | 23 +++++++++++++++++++ spec/lib/protobuf/field/sint64_field_spec.rb | 23 +++++++++++++++++++ spec/lib/protobuf/field/uint64_field_spec.rb | 23 +++++++++++++++++++ 9 files changed, 119 insertions(+), 4 deletions(-) diff --git a/lib/protobuf/field/int64_field.rb b/lib/protobuf/field/int64_field.rb index 3b338894..915dc617 100644 --- a/lib/protobuf/field/int64_field.rb +++ b/lib/protobuf/field/int64_field.rb @@ -29,6 +29,9 @@ def acceptable?(val) return false end + def json_encode(value) + value == 0 ? nil : value.to_s + end end end end diff --git a/lib/protobuf/field/sint64_field.rb b/lib/protobuf/field/sint64_field.rb index 2aba7dfa..7f8ec1fa 100644 --- a/lib/protobuf/field/sint64_field.rb +++ b/lib/protobuf/field/sint64_field.rb @@ -16,6 +16,9 @@ def self.min INT64_MIN end + def json_encode(value) + value == 0 ? nil : value.to_s + end end end end diff --git a/lib/protobuf/field/uint64_field.rb b/lib/protobuf/field/uint64_field.rb index 8a060f14..2521dcfd 100644 --- a/lib/protobuf/field/uint64_field.rb +++ b/lib/protobuf/field/uint64_field.rb @@ -16,6 +16,9 @@ def self.min 0 end + def json_encode(value) + value == 0 ? nil : value.to_s + end end end end diff --git a/lib/protobuf/message.rb b/lib/protobuf/message.rb index a13c0d19..faa771dd 100644 --- a/lib/protobuf/message.rb +++ b/lib/protobuf/message.rb @@ -156,7 +156,11 @@ def to_json_hash value end - result[field.name] = hashed_value + if hashed_value.nil? + result.delete(field.name) + else + result[field.name] = hashed_value + end end result diff --git a/spec/lib/protobuf/field/fixed64_field_spec.rb b/spec/lib/protobuf/field/fixed64_field_spec.rb index d7feb120..4074c3d1 100644 --- a/spec/lib/protobuf/field/fixed64_field_spec.rb +++ b/spec/lib/protobuf/field/fixed64_field_spec.rb @@ -4,4 +4,27 @@ it_behaves_like :packable_field, described_class + let(:message) do + Class.new(::Protobuf::Message) do + optional :fixed64, :field, 1 + end + end + + # https://developers.google.com/protocol-buffers/docs/proto3#json + describe '.{to_json, from_json}' do + it 'serialises 0 as empty' do + instance = message.new(:field => 0) + expect(instance.to_json).to eq('{}') + end + + it 'serialises max value as string' do + instance = message.new(:field => described_class.max) + expect(instance.to_json).to eq('{"field":"18446744073709551615"}') + end + + it 'serialises min value as string' do + instance = message.new(:field => described_class.min) + expect(instance.to_json).to eq('{}') + end + end end diff --git a/spec/lib/protobuf/field/int64_field_spec.rb b/spec/lib/protobuf/field/int64_field_spec.rb index 2d1d5311..32f64e78 100644 --- a/spec/lib/protobuf/field/int64_field_spec.rb +++ b/spec/lib/protobuf/field/int64_field_spec.rb @@ -12,9 +12,19 @@ # https://developers.google.com/protocol-buffers/docs/proto3#json describe '.{to_json, from_json}' do - it 'serialises int64 value as string' do - instance = message.new(:field => 10) - expect(instance.to_json).to eq('{"field":"10"}') + it 'serialises 0 as empty' do + instance = message.new(:field => 0) + expect(instance.to_json).to eq('{}') + end + + it 'serialises max value as string' do + instance = message.new(:field => described_class.max) + expect(instance.to_json).to eq('{"field":"9223372036854775807"}') + end + + it 'serialises min value as string' do + instance = message.new(:field => described_class.min) + expect(instance.to_json).to eq('{"field":"-9223372036854775808"}') end end end diff --git a/spec/lib/protobuf/field/sfixed64_field_spec.rb b/spec/lib/protobuf/field/sfixed64_field_spec.rb index f380ed5d..8e9ce5c7 100644 --- a/spec/lib/protobuf/field/sfixed64_field_spec.rb +++ b/spec/lib/protobuf/field/sfixed64_field_spec.rb @@ -6,4 +6,27 @@ let(:value) { [-1, 0, 1] } end + let(:message) do + Class.new(::Protobuf::Message) do + optional :sfixed64, :field, 1 + end + end + + # https://developers.google.com/protocol-buffers/docs/proto3#json + describe '.{to_json, from_json}' do + it 'serialises 0 as empty' do + instance = message.new(:field => 0) + expect(instance.to_json).to eq('{}') + end + + it 'serialises max value as string' do + instance = message.new(:field => described_class.max) + expect(instance.to_json).to eq('{"field":"9223372036854775807"}') + end + + it 'serialises min value as string' do + instance = message.new(:field => described_class.min) + expect(instance.to_json).to eq('{"field":"-9223372036854775808"}') + end + end end diff --git a/spec/lib/protobuf/field/sint64_field_spec.rb b/spec/lib/protobuf/field/sint64_field_spec.rb index 84ca7304..7ed2ad99 100644 --- a/spec/lib/protobuf/field/sint64_field_spec.rb +++ b/spec/lib/protobuf/field/sint64_field_spec.rb @@ -6,4 +6,27 @@ let(:value) { [-1, 0, 1] } end + let(:message) do + Class.new(::Protobuf::Message) do + optional :sint64, :field, 1 + end + end + + # https://developers.google.com/protocol-buffers/docs/proto3#json + describe '.{to_json, from_json}' do + it 'serialises 0 as empty' do + instance = message.new(:field => 0) + expect(instance.to_json).to eq('{}') + end + + it 'serialises max value as string' do + instance = message.new(:field => described_class.max) + expect(instance.to_json).to eq('{"field":"9223372036854775807"}') + end + + it 'serialises min value as string' do + instance = message.new(:field => described_class.min) + expect(instance.to_json).to eq('{"field":"-9223372036854775808"}') + end + end end diff --git a/spec/lib/protobuf/field/uint64_field_spec.rb b/spec/lib/protobuf/field/uint64_field_spec.rb index 61b8d1b9..7cfd1dca 100644 --- a/spec/lib/protobuf/field/uint64_field_spec.rb +++ b/spec/lib/protobuf/field/uint64_field_spec.rb @@ -4,4 +4,27 @@ it_behaves_like :packable_field, described_class + let(:message) do + Class.new(::Protobuf::Message) do + optional :uint64, :field, 1 + end + end + + # https://developers.google.com/protocol-buffers/docs/proto3#json + describe '.{to_json, from_json}' do + it 'serialises 0 as empty' do + instance = message.new(:field => 0) + expect(instance.to_json).to eq('{}') + end + + it 'serialises max value as string' do + instance = message.new(:field => described_class.max) + expect(instance.to_json).to eq('{"field":"18446744073709551615"}') + end + + it 'serialises min value as string' do + instance = message.new(:field => described_class.min) + expect(instance.to_json).to eq('{}') + end + end end From 9ef398b9147accb99cba400995fb94fdb10f72d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aslak=20Helles=C3=B8y?= Date: Tue, 4 Feb 2020 09:59:41 +0000 Subject: [PATCH 3/7] Preserve proto2 compatibility for JSON serialisation --- lib/protobuf/field/int64_field.rb | 8 +++++-- lib/protobuf/field/sint64_field.rb | 8 +++++-- lib/protobuf/field/uint64_field.rb | 8 +++++-- lib/protobuf/message.rb | 9 ++++---- spec/lib/protobuf/field/fixed64_field_spec.rb | 23 +++++++++++-------- spec/lib/protobuf/field/int64_field_spec.rb | 23 +++++++++++-------- .../lib/protobuf/field/sfixed64_field_spec.rb | 21 +++++++++-------- spec/lib/protobuf/field/sint64_field_spec.rb | 19 ++++++++------- spec/lib/protobuf/field/uint64_field_spec.rb | 23 +++++++++++-------- 9 files changed, 84 insertions(+), 58 deletions(-) diff --git a/lib/protobuf/field/int64_field.rb b/lib/protobuf/field/int64_field.rb index 915dc617..f2597b25 100644 --- a/lib/protobuf/field/int64_field.rb +++ b/lib/protobuf/field/int64_field.rb @@ -29,8 +29,12 @@ def acceptable?(val) return false end - def json_encode(value) - value == 0 ? nil : value.to_s + def json_encode(value, options = {}) + if options[:proto3] + value == 0 ? nil : value.to_s + else + value + end end end end diff --git a/lib/protobuf/field/sint64_field.rb b/lib/protobuf/field/sint64_field.rb index 7f8ec1fa..0c1c0856 100644 --- a/lib/protobuf/field/sint64_field.rb +++ b/lib/protobuf/field/sint64_field.rb @@ -16,8 +16,12 @@ def self.min INT64_MIN end - def json_encode(value) - value == 0 ? nil : value.to_s + def json_encode(value, options = {}) + if options[:proto3] + value == 0 ? nil : value.to_s + else + value + end end end end diff --git a/lib/protobuf/field/uint64_field.rb b/lib/protobuf/field/uint64_field.rb index 2521dcfd..df041fc5 100644 --- a/lib/protobuf/field/uint64_field.rb +++ b/lib/protobuf/field/uint64_field.rb @@ -16,8 +16,12 @@ def self.min 0 end - def json_encode(value) - value == 0 ? nil : value.to_s + def json_encode(value, options = {}) + if options[:proto3] + value == 0 ? nil : value.to_s + else + value + end end end end diff --git a/lib/protobuf/message.rb b/lib/protobuf/message.rb index faa771dd..a8b4a8fe 100644 --- a/lib/protobuf/message.rb +++ b/lib/protobuf/message.rb @@ -134,29 +134,28 @@ def to_hash_with_string_keys end def to_json(options = {}) - to_json_hash.to_json(options) + to_json_hash(options).to_json(options) end # Return a hash-representation of the given fields for this message type that # is safe to convert to JSON. - def to_json_hash + def to_json_hash(options = {}) result = {} @values.each_key do |field_name| value = self[field_name] field = self.class.get_field(field_name, true) - # 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) value.to_json_hash_value elsif field.respond_to?(:json_encode) - field.json_encode(value) + field.json_encode(value, options) else value end - if hashed_value.nil? + if hashed_value.nil? || (options[:proto3] && value == field.default) result.delete(field.name) else result[field.name] = hashed_value diff --git a/spec/lib/protobuf/field/fixed64_field_spec.rb b/spec/lib/protobuf/field/fixed64_field_spec.rb index 4074c3d1..5543800a 100644 --- a/spec/lib/protobuf/field/fixed64_field_spec.rb +++ b/spec/lib/protobuf/field/fixed64_field_spec.rb @@ -6,25 +6,28 @@ let(:message) do Class.new(::Protobuf::Message) do - optional :fixed64, :field, 1 + optional :fixed64, :some_field, 1 end end # https://developers.google.com/protocol-buffers/docs/proto3#json describe '.{to_json, from_json}' do - it 'serialises 0 as empty' do - instance = message.new(:field => 0) - expect(instance.to_json).to eq('{}') + it 'serialises 0' do + instance = message.new(some_field: 0) + expect(instance.to_json(proto3: true)).to eq('{}') + expect(instance.to_json).to eq('{"some_field":0}') end - it 'serialises max value as string' do - instance = message.new(:field => described_class.max) - expect(instance.to_json).to eq('{"field":"18446744073709551615"}') + it 'serialises max value' do + instance = message.new(some_field: described_class.max) + expect(instance.to_json(proto3: true)).to eq('{"some_field":"18446744073709551615"}') + expect(instance.to_json).to eq('{"some_field":18446744073709551615}') end - it 'serialises min value as string' do - instance = message.new(:field => described_class.min) - expect(instance.to_json).to eq('{}') + it 'serialises min value' do + instance = message.new(some_field: described_class.min) + expect(instance.to_json(proto3: true)).to eq('{}') + expect(instance.to_json).to eq('{"some_field":0}') end end end diff --git a/spec/lib/protobuf/field/int64_field_spec.rb b/spec/lib/protobuf/field/int64_field_spec.rb index 32f64e78..2608ca5c 100644 --- a/spec/lib/protobuf/field/int64_field_spec.rb +++ b/spec/lib/protobuf/field/int64_field_spec.rb @@ -6,25 +6,28 @@ let(:message) do Class.new(::Protobuf::Message) do - optional :int64, :field, 1 + optional :int64, :some_field, 1 end end # https://developers.google.com/protocol-buffers/docs/proto3#json describe '.{to_json, from_json}' do - it 'serialises 0 as empty' do - instance = message.new(:field => 0) - expect(instance.to_json).to eq('{}') + it 'serialises 0' do + instance = message.new(some_field: 0) + expect(instance.to_json(proto3: true)).to eq('{}') + expect(instance.to_json).to eq('{"some_field":0}') end - it 'serialises max value as string' do - instance = message.new(:field => described_class.max) - expect(instance.to_json).to eq('{"field":"9223372036854775807"}') + it 'serialises max value' do + instance = message.new(some_field: described_class.max) + expect(instance.to_json(proto3: true)).to eq('{"some_field":"9223372036854775807"}') + expect(instance.to_json).to eq('{"some_field":9223372036854775807}') end - it 'serialises min value as string' do - instance = message.new(:field => described_class.min) - expect(instance.to_json).to eq('{"field":"-9223372036854775808"}') + it 'serialises min value' do + instance = message.new(some_field: described_class.min) + expect(instance.to_json(proto3: true)).to eq('{"some_field":"-9223372036854775808"}') + expect(instance.to_json).to eq('{"some_field":-9223372036854775808}') end end end diff --git a/spec/lib/protobuf/field/sfixed64_field_spec.rb b/spec/lib/protobuf/field/sfixed64_field_spec.rb index 8e9ce5c7..ed8f5097 100644 --- a/spec/lib/protobuf/field/sfixed64_field_spec.rb +++ b/spec/lib/protobuf/field/sfixed64_field_spec.rb @@ -8,25 +8,28 @@ let(:message) do Class.new(::Protobuf::Message) do - optional :sfixed64, :field, 1 + optional :sfixed64, :some_field, 1 end end # https://developers.google.com/protocol-buffers/docs/proto3#json describe '.{to_json, from_json}' do - it 'serialises 0 as empty' do - instance = message.new(:field => 0) - expect(instance.to_json).to eq('{}') + it 'serialises 0' do + instance = message.new(some_field: 0) + expect(instance.to_json(proto3: true)).to eq('{}') + expect(instance.to_json).to eq('{"some_field":0}') end - it 'serialises max value as string' do - instance = message.new(:field => described_class.max) - expect(instance.to_json).to eq('{"field":"9223372036854775807"}') + it 'serialises max value' do + instance = message.new(some_field: described_class.max) + expect(instance.to_json(proto3: true)).to eq('{"some_field":"9223372036854775807"}') + expect(instance.to_json).to eq('{"some_field":9223372036854775807}') end it 'serialises min value as string' do - instance = message.new(:field => described_class.min) - expect(instance.to_json).to eq('{"field":"-9223372036854775808"}') + instance = message.new(some_field: described_class.min) + expect(instance.to_json(proto3: true)).to eq('{"some_field":"-9223372036854775808"}') + expect(instance.to_json).to eq('{"some_field":-9223372036854775808}') end end end diff --git a/spec/lib/protobuf/field/sint64_field_spec.rb b/spec/lib/protobuf/field/sint64_field_spec.rb index 7ed2ad99..9e705354 100644 --- a/spec/lib/protobuf/field/sint64_field_spec.rb +++ b/spec/lib/protobuf/field/sint64_field_spec.rb @@ -8,25 +8,28 @@ let(:message) do Class.new(::Protobuf::Message) do - optional :sint64, :field, 1 + optional :sint64, :some_field, 1 end end # https://developers.google.com/protocol-buffers/docs/proto3#json describe '.{to_json, from_json}' do - it 'serialises 0 as empty' do - instance = message.new(:field => 0) - expect(instance.to_json).to eq('{}') + it 'serialises 0' do + instance = message.new(some_field: 0) + expect(instance.to_json(proto3: true)).to eq('{}') + expect(instance.to_json).to eq('{"some_field":0}') end it 'serialises max value as string' do - instance = message.new(:field => described_class.max) - expect(instance.to_json).to eq('{"field":"9223372036854775807"}') + instance = message.new(some_field: described_class.max) + expect(instance.to_json(proto3: true)).to eq('{"some_field":"9223372036854775807"}') + expect(instance.to_json).to eq('{"some_field":9223372036854775807}') end it 'serialises min value as string' do - instance = message.new(:field => described_class.min) - expect(instance.to_json).to eq('{"field":"-9223372036854775808"}') + instance = message.new(some_field: described_class.min) + expect(instance.to_json(proto3: true)).to eq('{"some_field":"-9223372036854775808"}') + expect(instance.to_json).to eq('{"some_field":-9223372036854775808}') end end end diff --git a/spec/lib/protobuf/field/uint64_field_spec.rb b/spec/lib/protobuf/field/uint64_field_spec.rb index 7cfd1dca..d1bf07c1 100644 --- a/spec/lib/protobuf/field/uint64_field_spec.rb +++ b/spec/lib/protobuf/field/uint64_field_spec.rb @@ -6,25 +6,28 @@ let(:message) do Class.new(::Protobuf::Message) do - optional :uint64, :field, 1 + optional :uint64, :some_field, 1 end end # https://developers.google.com/protocol-buffers/docs/proto3#json describe '.{to_json, from_json}' do - it 'serialises 0 as empty' do - instance = message.new(:field => 0) - expect(instance.to_json).to eq('{}') + it 'serialises 0' do + instance = message.new(some_field: 0) + expect(instance.to_json(proto3: true)).to eq('{}') + expect(instance.to_json).to eq('{"some_field":0}') end - it 'serialises max value as string' do - instance = message.new(:field => described_class.max) - expect(instance.to_json).to eq('{"field":"18446744073709551615"}') + it 'serialises max value' do + instance = message.new(some_field: described_class.max) + expect(instance.to_json(proto3: true)).to eq('{"some_field":"18446744073709551615"}') + expect(instance.to_json).to eq('{"some_field":18446744073709551615}') end - it 'serialises min value as string' do - instance = message.new(:field => described_class.min) - expect(instance.to_json).to eq('{}') + it 'serialises min value' do + instance = message.new(some_field: described_class.min) + expect(instance.to_json(proto3: true)).to eq('{}') + expect(instance.to_json).to eq('{"some_field":0}') end end end From 11df2d11f54b3895c4541e270c30408fdf3bbd6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aslak=20Helles=C3=B8y?= Date: Tue, 4 Feb 2020 10:03:18 +0000 Subject: [PATCH 4/7] Restore backwards compatibility in Message#to_json_hash --- lib/protobuf/message.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/protobuf/message.rb b/lib/protobuf/message.rb index a8b4a8fe..247f5f0e 100644 --- a/lib/protobuf/message.rb +++ b/lib/protobuf/message.rb @@ -155,7 +155,7 @@ def to_json_hash(options = {}) value end - if hashed_value.nil? || (options[:proto3] && value == field.default) + if options[:proto3] && (hashed_value.nil? || value == field.default) result.delete(field.name) else result[field.name] = hashed_value From b4c4203daa7550eac0ceb6c2cbb275d8b7dab6d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aslak=20Helles=C3=B8y?= Date: Tue, 4 Feb 2020 10:13:32 +0000 Subject: [PATCH 5/7] Space --- lib/protobuf/message.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/protobuf/message.rb b/lib/protobuf/message.rb index ce3919d5..01f00fd8 100644 --- a/lib/protobuf/message.rb +++ b/lib/protobuf/message.rb @@ -147,6 +147,7 @@ def to_json_hash(options = {}) @values.each_key do |field_name| value = self[field_name] field = self.class.get_field(field_name, true) + # 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) From 18d6ffc724d8736a74bf761a038e06c43d412fbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aslak=20Helles=C3=B8y?= Date: Wed, 5 Feb 2020 13:59:23 +0000 Subject: [PATCH 6/7] Add missing options vararg --- lib/protobuf/field/bytes_field.rb | 2 +- lib/protobuf/field/string_field.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/protobuf/field/bytes_field.rb b/lib/protobuf/field/bytes_field.rb index 81a3634d..b8d88078 100644 --- a/lib/protobuf/field/bytes_field.rb +++ b/lib/protobuf/field/bytes_field.rb @@ -59,7 +59,7 @@ def coerce!(value) end end - def json_encode(value) + def json_encode(value, options={}) Base64.strict_encode64(value) end end diff --git a/lib/protobuf/field/string_field.rb b/lib/protobuf/field/string_field.rb index 6c9c278f..551bdd23 100644 --- a/lib/protobuf/field/string_field.rb +++ b/lib/protobuf/field/string_field.rb @@ -43,7 +43,7 @@ def encode(value) "#{::Protobuf::Field::VarintField.encode(value_to_encode.bytesize)}#{value_to_encode}" end - def json_encode(value) + def json_encode(value, options={}) value end end From 7803c6e01c456728955565f81686b9024b8736d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aslak=20Helles=C3=B8y?= Date: Wed, 5 Feb 2020 14:17:14 +0000 Subject: [PATCH 7/7] Compare with class default, not instance default. --- lib/protobuf/message.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/protobuf/message.rb b/lib/protobuf/message.rb index 01f00fd8..87e15566 100644 --- a/lib/protobuf/message.rb +++ b/lib/protobuf/message.rb @@ -158,7 +158,7 @@ def to_json_hash(options = {}) value end - if proto3 && (hashed_value.nil? || value == field.default) + if proto3 && (hashed_value.nil? || value == field.class.default) result.delete(field.name) else key = proto3 ? field.name.to_s.camelize(:lower).to_sym : field.name