-
Notifications
You must be signed in to change notification settings - Fork 6
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
Hash Serialization #182
Hash Serialization #182
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -77,6 +77,139 @@ def resolve | |
end | ||
end | ||
|
||
# Generates #to_h, #as_json, and #to_json methods | ||
class HashSerializationCompiler | ||
include TypeHelper | ||
|
||
attr_reader :message, :fields, :oneof_selection_fields, :generate_types | ||
|
||
class << self | ||
def result(message:, fields:, oneof_selection_fields:, generate_types:) | ||
new(message:, fields:, oneof_selection_fields:, generate_types:).result | ||
end | ||
end | ||
|
||
def initialize(message:, fields:, oneof_selection_fields:, generate_types:) | ||
@message = message | ||
@fields = fields.sort_by(&:number) # Serialize fields in their proto order | ||
@oneof_selection_fields = oneof_selection_fields | ||
@generate_types = generate_types | ||
end | ||
|
||
def result | ||
<<~RUBY | ||
#{type_signature(returns: "T::Hash[Symbol, T.untyped]")} | ||
def to_h | ||
result = {} | ||
|
||
#{fields.map { |f| to_h_assign_statement_rb(f) }.join("\n")} | ||
|
||
result | ||
end | ||
|
||
#{type_signature(params: { options: "T::Hash[T.untyped, T.untyped]" }, returns: "T::Hash[Symbol, T.untyped]")} | ||
def as_json(options = {}) | ||
result = {} | ||
|
||
#{fields.map { |f| as_json_assign_statement_rb(f) }.join("\n")} | ||
|
||
result | ||
end | ||
|
||
def to_json(options = {}) | ||
require 'json' | ||
JSON.dump(as_json(options)) | ||
end | ||
RUBY | ||
end | ||
|
||
private | ||
|
||
def to_h_assign_statement_rb(field) | ||
key = to_h_hash_key_rb(field) | ||
value = to_h_hash_value_rb(field) | ||
|
||
if field.has_oneof_index? && !field.optional? | ||
oneof_selection_field_name = oneof_selection_fields[field.oneof_index].name.dump | ||
field_name = field.name.dump | ||
|
||
"result[#{key}] = #{value} if send(:#{oneof_selection_field_name}) == :#{field_name}" | ||
else | ||
"result[#{key}] = #{value}" | ||
end | ||
end | ||
|
||
def to_h_hash_key_rb(field) | ||
":#{field.name.dump}" | ||
end | ||
|
||
def to_h_hash_value_rb(field) | ||
return to_h_hash_value_for_map_rb(field) if field.map_field? | ||
|
||
# For primitives or arrays of primitives we can just use the instance variable value | ||
return field.iv_name unless field.type == :TYPE_MESSAGE | ||
|
||
if field.repeated? | ||
# repeated maps aren't possible so we don't have to worry about to_h arity or as_json not being defined | ||
"#{field.iv_name}.map { |v| v.to_h }" | ||
else | ||
"#{field.iv_name}.to_h" | ||
end | ||
end | ||
|
||
def to_h_hash_value_for_map_rb(field) | ||
if field.map_type.value.type == :TYPE_MESSAGE | ||
"#{field.iv_name}.transform_values { |value| value.to_h }" | ||
else | ||
field.iv_name | ||
end | ||
end | ||
|
||
def as_json_assign_statement_rb(field) | ||
key = as_json_hash_key_rb(field) | ||
value = as_json_hash_value_rb(field) | ||
|
||
if field.has_oneof_index? && !field.optional? | ||
oneof_selection_field_name = oneof_selection_fields[field.oneof_index].name.dump | ||
field_name = field.name.dump | ||
|
||
"result[#{key}] = #{value} if send(:#{oneof_selection_field_name}) == :#{field_name}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can see it in the generated code, sometimes there are multiple of these in a row, so we're calling |
||
elsif field.repeated? | ||
"#{value}.tap { |v| result[#{key}] = v if !options[:compact] || v.any? }" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I often write my ruby code this way (and I see an instance of this in the old code), but I wonder if, for performance, the generated code should not have unnecessary blocks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The issue here is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed. I was thinking maybe we could use something like |
||
elsif field.optional? | ||
"result[#{key}] = #{value} if !options[:compact] || has_#{field.name}?" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This interpolation for the predicate name triggers my spidey-sense but this is how we generate the name when defining the predicate so I guess it's fine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the idea of putting |
||
else | ||
"result[#{key}] = #{value}" | ||
end | ||
end | ||
|
||
def as_json_hash_key_rb(field) | ||
field.json_name.dump | ||
end | ||
|
||
def as_json_hash_value_rb(field) | ||
return as_json_hash_value_for_map_rb(field) if field.map_field? | ||
|
||
# For primitives or arrays of primitives we can just use the instance variable value | ||
return field.iv_name unless field.type == :TYPE_MESSAGE | ||
|
||
if field.repeated? | ||
# repeated maps aren't possible so we don't have to worry about to_h arity or as_json not being defined | ||
"#{field.iv_name}.map { |v| v.as_json(options) }" | ||
else | ||
"#{field.iv_name}.nil? ? {} : #{field.iv_name}.as_json(options)" | ||
end | ||
end | ||
|
||
def as_json_hash_value_for_map_rb(field) | ||
if field.map_type.value.type == :TYPE_MESSAGE | ||
"#{field.iv_name}.transform_values { |value| value.as_json(options) }" | ||
else | ||
field.iv_name | ||
end | ||
end | ||
end | ||
|
||
class MessageCompiler | ||
attr_reader :generate_types, :requires | ||
|
||
|
@@ -88,8 +221,13 @@ def result(message, toplevel_enums, generate_types:, requires:, syntax:, options | |
end | ||
end | ||
|
||
attr_reader :message, :fields, :oneof_fields, :syntax | ||
attr_reader :optional_fields, :enum_field_types | ||
attr_reader :message, | ||
:fields, | ||
:oneof_fields, | ||
:syntax, | ||
:optional_fields, | ||
:enum_field_types, | ||
:oneof_selection_fields | ||
|
||
def initialize(message, toplevel_enums, generate_types:, requires:, syntax:, options:) | ||
@message = message | ||
|
@@ -160,32 +298,7 @@ def class_body | |
end | ||
|
||
def conversion | ||
fields = self.fields.reject do |field| | ||
field.has_oneof_index? && !field.optional? | ||
end | ||
|
||
oneofs = @oneof_selection_fields.map do |field| | ||
"send(#{field.name.dump}).tap { |f| result[f.to_sym] = send(f) if f }" | ||
end | ||
|
||
<<~RUBY | ||
#{type_signature(returns: "T::Hash[Symbol, T.untyped]")} | ||
def to_h | ||
result = {} | ||
#{(oneofs + fields.map { |field| convert_field(field) }).join("\n")} | ||
result | ||
end | ||
RUBY | ||
end | ||
|
||
def convert_field(field) | ||
if field.repeated? | ||
"result['#{field.name}'.to_sym] = #{field.iv_name}" | ||
elsif field.type == :TYPE_MESSAGE | ||
"result['#{field.name}'.to_sym] = #{field.iv_name}.to_h" | ||
else | ||
"result['#{field.name}'.to_sym] = #{field.iv_name}" | ||
end | ||
HashSerializationCompiler.result(message:, fields:, oneof_selection_fields:, generate_types:) | ||
end | ||
|
||
def encode | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we use
send
here?I see it in the old code, but we (now) have (public) attribute readers for these. 🤔
Not sure if maybe we didn't at some point.
If there is a reason, we should put a comment above this line explaining it.
If we don't, should we switch to
self.#{...}
(to still avoid any possible name clashes with local vars)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense.
self.#{...}
is probably better.