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

already loading field on define and should not need to calcualte the … #295

Merged
merged 4 commits into from
Feb 6, 2016
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
4 changes: 2 additions & 2 deletions lib/protobuf/decoder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ def self.read_field(stream)
bytes = case wire_type
when ::Protobuf::WireType::VARINT
Varint.decode(stream)
when ::Protobuf::WireType::FIXED64
read_fixed64(stream)
when ::Protobuf::WireType::LENGTH_DELIMITED
read_length_delimited(stream)
when ::Protobuf::WireType::FIXED64
read_fixed64(stream)
when ::Protobuf::WireType::FIXED32
read_fixed32(stream)
when ::Protobuf::WireType::START_GROUP
Expand Down
11 changes: 3 additions & 8 deletions lib/protobuf/encoder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,15 @@ def self.encode(message, stream)
message.each_field_for_serialization do |field, value|
if field.repeated?
if field.packed?
key = (field.tag << 3) | ::Protobuf::WireType::LENGTH_DELIMITED
packed_value = value.map { |val| field.encode(val) }.join
stream << ::Protobuf::Field::VarintField.encode(key)
stream << ::Protobuf::Field::VarintField.encode(packed_value.size)
stream << packed_value
stream << "#{field.tag_encoded}#{::Protobuf::Field::VarintField.encode(packed_value.size)}#{packed_value}"
else
value.each do |val|
key = (field.tag << 3) | field.wire_type
stream << "#{::Protobuf::Field::VarintField.encode(key)}#{field.encode(val)}"
stream << "#{field.tag_encoded}#{field.encode(val)}"
end
end
else
key = (field.tag << 3) | field.wire_type
stream << "#{::Protobuf::Field::VarintField.encode(key)}#{field.encode(value)}"
stream << "#{field.tag_encoded}#{field.encode(value)}"
end
end

Expand Down
12 changes: 12 additions & 0 deletions lib/protobuf/field/base_field.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ def initialize(message_class, rule, type_class, name, tag, options)

validate_packed_field if packed?
define_accessor
tag_encoded
end

##
Expand Down Expand Up @@ -148,6 +149,17 @@ def setter
@setter ||= "#{name}="
end

def tag_encoded
@tag_encoded ||= begin
case
when repeated? && packed?
::Protobuf::Field::VarintField.encode((tag << 3) | ::Protobuf::WireType::LENGTH_DELIMITED)
else
::Protobuf::Field::VarintField.encode((tag << 3) | wire_type)
end
end
end

# FIXME: add packed, deprecated, extension options to to_s output
def to_s
"#{rule} #{type_class} #{name} = #{tag} #{default ? "[default=#{default.inspect}]" : ''}"
Expand Down
24 changes: 9 additions & 15 deletions lib/protobuf/field/bytes_field.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,21 +58,15 @@ def define_setter
message_class.class_eval do
define_method(method_name) do |val|
@encode = nil
begin
case val
when String, Symbol
@values[field.name] = "#{val}"
when NilClass
@values.delete(field.name)
when ::Protobuf::Message
@values[field.name] = val.dup
else
fail TypeError, "Unacceptable value #{val} for field #{field.name} of type #{field.type_class}"
end
rescue NoMethodError => ex
logger.error { ex.message }
logger.error { ex.backtrace.join("\n") }
raise TypeError, "Got NoMethodError attempting to set #{val} for field #{field.name} of type #{field.type_class}: #{ex.message}"
case val
when String, Symbol
@values[field.name] = "#{val}"
when NilClass
@values.delete(field.name)
when ::Protobuf::Message
@values[field.name] = val.dup
else
fail TypeError, "Unacceptable value #{val} for field #{field.name} of type #{field.type_class}"
end
end
end
Copy link
Member

Choose a reason for hiding this comment

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

Interesting that we had no test coverage for this behavior. In either case we are raising an exception and we can let the caller decider if they want to log and convert the error type themselves 👍

Expand Down
4 changes: 2 additions & 2 deletions lib/protobuf/lifecycle.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
module Protobuf
class Lifecycle
class << self
def register(event_name, &blk)
def register(event_name)
fail "Lifecycle register must have a block" unless block_given?
event_name = normalized_event_name(event_name)

::ActiveSupport::Notifications.subscribe(event_name) do |_name, _start, _finish, _id, args|
blk.call(*args)
yield(*args)
end
end
alias_method :on, :register
Expand Down
4 changes: 2 additions & 2 deletions lib/protobuf/message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,10 @@ def respond_to_has_and_present?(key)
def to_hash
result = {}

@values.keys.each do |field_name|
@values.each_key do |field_name|
value = __send__(field_name)
hashed_value = value.respond_to?(:to_hash_value) ? value.to_hash_value : value
result.merge!(field_name => hashed_value)
result[field_name] = hashed_value
end

result
Expand Down