-
Notifications
You must be signed in to change notification settings - Fork 17
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
WIP: Support field renaming #19
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 |
---|---|---|
|
@@ -4,3 +4,5 @@ inherit_gem: | |
AllCops: | ||
TargetRubyVersion: 2.2 | ||
|
||
Style/MultilineBlockChain: | ||
Enabled: false |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
ruby-2.3.1 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,8 @@ def self.first_union_schema(field_type) | |
|
||
module ClassMethods | ||
def add_avro_fields | ||
validate_schemas! | ||
|
||
if key_avro_schema | ||
check_for_field_conflicts! | ||
define_avro_attributes(key_avro_schema) | ||
|
@@ -51,14 +53,11 @@ def schema_fields_differ?(name) | |
end | ||
|
||
def define_avro_attributes(schema) | ||
if schema.type_sym != :record | ||
raise "Unsupported schema type '#{schema.type_sym}', only 'record' schemas are supported." | ||
end | ||
|
||
schema.fields.each do |field| | ||
field_class = avro_field_class(field.type) | ||
field_class = avro_field_class(field: field) | ||
field_name = avro_field_name(field.name) | ||
|
||
attribute(field.name, | ||
attribute(field_name, | ||
field_class, | ||
avro_field_options(field)) | ||
|
||
|
@@ -67,6 +66,40 @@ def define_avro_attributes(schema) | |
end | ||
end | ||
|
||
def validate_schemas! | ||
[key_avro_schema, avro_schema].compact | ||
.each { |schema| validate_schema!(schema) } | ||
end | ||
|
||
def validate_schema!(schema) | ||
if schema.type_sym != :record | ||
raise "Unsupported schema type '#{schema.type_sym}', only 'record' schemas are supported." | ||
end | ||
|
||
invalid_fields = schema.fields.select do |field| | ||
instance_methods.include?(field.name.to_sym) && no_valid_alias?(field.name) | ||
end | ||
if invalid_fields.any? | ||
raise "Disallowed field names: #{invalid_fields.map(&:name).map(&:inspect).join(', ')}.\n"\ | ||
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. Nice! This will avoid some really hard to find bugs. |
||
'Consider using the `aliases` option when defining the model to specify an alternate name.' | ||
end | ||
end | ||
|
||
def raw_field_names | ||
@raw_field_names ||= | ||
[key_avro_schema, avro_schema].compact.flat_map(&:fields).map(&:name).to_set | ||
end | ||
|
||
# TODO: should this check for aliases that conflict with | ||
# existing fields in general, or duplicate aliases? | ||
# This is only checking the aliases for invalid fields. | ||
def no_valid_alias?(name) | ||
alias_name = aliases[name] | ||
alias_name.nil? || | ||
(raw_field_names.include?(alias_name) && | ||
raise("alias `#{alias_name}` for field `#{name}` conflicts with an existing field.")) | ||
end | ||
|
||
def add_validation(field) | ||
case field.type.type_sym | ||
when :enum | ||
|
@@ -96,7 +129,13 @@ def required?(field) | |
!optional?(field) | ||
end | ||
|
||
def avro_field_class(field_type) | ||
def avro_field_name(field_name) | ||
aliases[field_name] || field_name | ||
end | ||
|
||
def avro_field_class(field_type: nil, field: nil, name: nil) | ||
field_type ||= field.type | ||
field_name = name || field.name | ||
custom_type = Avromatic.type_registry.fetch(field_type) | ||
return custom_type.value_class if custom_type.value_class | ||
|
||
|
@@ -114,24 +153,36 @@ def avro_field_class(field_type) | |
when :null | ||
NilClass | ||
when :array | ||
Array[avro_field_class(field_type.items)] | ||
Array[avro_field_class(field_type: field_type.items, name: field_name)] | ||
when :map | ||
Hash[String => avro_field_class(field_type.values)] | ||
Hash[String => avro_field_class(field_type: field_type.values, name: field_name)] | ||
when :union | ||
union_field_class(field_type) | ||
union_field_class(field_type, field_name) | ||
when :record | ||
# TODO: This should add the generated model to a module. | ||
# A hash of generated models should be kept by name for reuse. | ||
record_aliases = propagated_aliases(field_name) | ||
Class.new do | ||
include Avromatic::Model.build(schema: field_type) | ||
include Avromatic::Model.build(schema: field_type, | ||
aliases: record_aliases) | ||
end | ||
else | ||
raise "Unsupported type #{field_type}" | ||
end | ||
end | ||
|
||
def union_field_class(field_type) | ||
avro_field_class(Avromatic::Model::Attributes.first_union_schema(field_type)) | ||
def propagated_aliases(prefix) | ||
field_name_prefix = "#{prefix}." | ||
aliases.select do |key, _value| | ||
key.start_with?(field_name_prefix) | ||
end.each_with_object(Hash.new) do |(key, value), result| | ||
result[key.slice(field_name_prefix.length, key.length)] = value | ||
end | ||
end | ||
|
||
def union_field_class(field_type, field_name) | ||
avro_field_class(field_type: Avromatic::Model::Attributes.first_union_schema(field_type), | ||
name: field_name) | ||
end | ||
|
||
def avro_field_options(field) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,14 +8,19 @@ module Configurable | |
|
||
module ClassMethods | ||
attr_accessor :config | ||
delegate :avro_schema, :value_avro_schema, :key_avro_schema, to: :config | ||
delegate :avro_schema, :value_avro_schema, :key_avro_schema, | ||
:aliases, :inverse_aliases, to: :config | ||
|
||
def inherited(subclass) | ||
subclass.config = config | ||
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. Does this need to be an 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 think it is I guess by definition that's a class variable, so we could switch to that for the config on a generated model class. |
||
end | ||
|
||
def value_avro_field_names | ||
@value_avro_field_names ||= value_avro_schema.fields.map(&:name).map(&:to_sym).freeze | ||
@value_avro_field_names ||= extract_field_names(value_avro_schema) | ||
end | ||
|
||
def key_avro_field_names | ||
@key_avro_field_names ||= key_avro_schema.fields.map(&:name).map(&:to_sym).freeze | ||
@key_avro_field_names ||= extract_field_names(key_avro_schema) | ||
end | ||
|
||
def value_avro_fields_by_name | ||
|
@@ -26,8 +31,18 @@ def key_avro_fields_by_name | |
@key_avro_fields_by_name ||= mapped_by_name(key_avro_schema) | ||
end | ||
|
||
def inverse_aliases | ||
@inverse_aliases ||= aliases.select { |k, _| k.index('.').nil? }.invert | ||
end | ||
|
||
private | ||
|
||
def extract_field_names(schema) | ||
schema.fields.map do |field| | ||
(aliases[field.name] || field.name).to_sym | ||
end.freeze | ||
end | ||
|
||
def mapped_by_name(schema) | ||
schema.fields.each_with_object(Hash.new) do |field, result| | ||
result[field.name.to_sym] = field | ||
|
@@ -37,6 +52,7 @@ def mapped_by_name(schema) | |
|
||
delegate :avro_schema, :value_avro_schema, :key_avro_schema, | ||
:value_avro_field_names, :key_avro_field_names, | ||
:inverse_aliases, | ||
to: :class | ||
end | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
module Avromatic | ||
VERSION = '0.7.1'.freeze | ||
VERSION = '0.8.0'.freeze | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
namespace :test | ||
|
||
# record type contains fields that are either reserved words | ||
# for Virtus or methods that are used in the model implementation. | ||
record :reserved do | ||
required :attributes, :array, items: :string | ||
required :avro_message_value, :string | ||
required :hash, :map, values: :string | ||
required :okay, :string, default: '' | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
{ | ||
"type": "record", | ||
"name": "reserved", | ||
"namespace": "test", | ||
"fields": [ | ||
{ | ||
"name": "attributes", | ||
"type": { | ||
"type": "array", | ||
"items": "string" | ||
} | ||
}, | ||
{ | ||
"name": "avro_message_value", | ||
"type": "string" | ||
}, | ||
{ | ||
"name": "hash", | ||
"type": { | ||
"type": "map", | ||
"values": "string" | ||
} | ||
}, | ||
{ | ||
"name": "okay", | ||
"type": "string", | ||
"default": "" | ||
} | ||
] | ||
} |
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.
I agree
aliases
is a bit misleading.renames
ormappings
work for me.