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

Adds support to specify integer and bool proto fields as string. #253

Merged
merged 1 commit into from
Apr 14, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion lib/protobuf/field/bool_field.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,17 @@ def self.default
# #

def acceptable?(val)
[true, false].include?(val)
[true, false].include?(val) || %w(true false).include?(val)
end

def coerce!(val)
if val == 'true'
true
elsif val == 'false'
false
else
val
end
end

def decode(value)
Expand Down
8 changes: 7 additions & 1 deletion lib/protobuf/field/varint_field.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,17 @@ def self.encode(value)
#

def acceptable?(val)
(val > self.class.min || val < self.class.max)
int_val = coerce!(val)
int_val >= self.class.min && int_val <= self.class.max
rescue
false
end

def coerce!(val)
return val.to_i if val.is_a?(Numeric)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this required? (the .to_i and is_a?(Numeric) check)

we have been looking at standardizing serialization with this branch
https://github.com/localshred/protobuf/compare/bdewitt/fix_coercion

Copy link
Author

Choose a reason for hiding this comment

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

It is required to convert a float to an integer, unless that should be disallowed.

Also, Integer('0x10') produces 16. IMO, it should return an error and hence requires Integer(val, 10) which throws an error if val is of type Numeric.

Integer(val, 10)
end

def decode(value)
value
end
Expand Down
51 changes: 51 additions & 0 deletions spec/lib/protobuf/field/bool_field_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
require 'spec_helper'

RSpec.describe Protobuf::Field::Int32Field do

class SomeBoolMessage < ::Protobuf::Message
optional :bool, :some_bool, 1
end

let(:instance) { SomeBoolMessage.new }

describe '#define_setter' do
subject { instance.some_bool = value; instance.some_bool }

[true, false].each do |val|
context "when set with #{val}" do
let(:value) { val }

it 'is readable as a bool' do
expect(subject).to eq(val)
end
end
end

[['true', true], ['false', false]].each do |val, expected|
context "when set with a string of #{val}" do
let(:value) { val }

it 'is readable as a bool' do
expect(subject).to eq(expected)
end
end
end

context 'when set with a non-bool string' do
let(:value) { "aaaa" }

it 'throws an error' do
expect { subject }.to raise_error(TypeError)
end
end

context 'when set with something that is not a bool' do
let(:value) { [1, 2, 3] }

it 'throws an error' do
expect { subject }.to raise_error(TypeError)
end
end
end

end
82 changes: 82 additions & 0 deletions spec/lib/protobuf/field/int32_field_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,86 @@

it_behaves_like :packable_field, described_class

class SomeInt32Message < ::Protobuf::Message
optional :int32, :some_int, 1
end

let(:instance) { SomeInt32Message.new }

describe '#define_setter' do
subject { instance.some_int = value; instance.some_int }

context 'when set with an int' do
let(:value) { 100 }

it 'is readable as an int' do
expect(subject).to eq(100)
end
end

context 'when set with a float' do
let(:value) { 100.1 }

it 'is readable as an int' do
expect(subject).to eq(100)
end
end

context 'when set with a string of an int' do
let(:value) { "101" }

it 'is readable as an int' do
expect(subject).to eq(101)
end
end

context 'when set with a negative representation of an int as string' do
let(:value) { "-101" }

it 'is readable as a negative int' do
expect(subject).to eq(-101)
end
end

context 'when set with a non-numeric string' do
let(:value) { "aaaa" }

it 'throws an error' do
expect { subject }.to raise_error(TypeError)
end
end

context 'when set with a string of an int in hex format' do
let(:value) { "0x101" }

it 'throws an error' do
expect { subject }.to raise_error(TypeError)
end
end

context 'when set with a string of an int larger than int32 max' do
let(:value) { (described_class.max + 1).to_s }

it 'throws an error' do
expect { subject }.to raise_error(TypeError)
end
end

context 'when set with something that is not an int' do
let(:value) { [1, 2, 3] }

it 'throws an error' do
expect { subject }.to raise_error(TypeError)
end
end

context 'when set with a timestamp' do
let(:value) { Time.now }

it 'throws an error' do
expect { subject }.to raise_error(TypeError)
end
end
end

end