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

WIP: Support field renaming #19

Closed
wants to merge 1 commit into from
Closed
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
2 changes: 2 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,5 @@ inherit_gem:
AllCops:
TargetRubyVersion: 2.2

Style/MultilineBlockChain:
Enabled: false
1 change: 1 addition & 0 deletions .ruby-version
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ruby-2.3.1
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# avromatic changelog

## v0.8.0
- Allow schema fields to be aliased if they conflict with an internal
method defined on the model.

## v0.7.1
- Raise a more descriptive error when attempting to generate a model for a
non-record Avro type.
Expand Down
24 changes: 24 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,30 @@ inbound or outbound value is nil.
If a custom type is registered for a record-type field, then any `to_avro`
method/Proc should return a Hash with string keys for encoding using Avro.

#### Model Attribute Aliasing

Note: this is different than the aliasing of fields and records within an Avro
schema, which is not currently supported by the ruby `avro` gem.

If a field in an Avro schema conflicts with the name of an internal method for a
model, then field can be aliased by specifying the `aliases` option when
defining the model.

For example, `attributes` is a reserved name for Virtus (used internally by
Avromatic). If a schema contains an attributes field, then it can be aliased
to a different name on the model:

```ruby
Avromatic::Model.model(
schema_name: 'reserved_names',
aliases: { attributes: :my_attributes }
Copy link
Member

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 or mappings work for me.

)
```

A value for `attributes` will be assigned to `my_attributes` when creating an
instance of the model, and the `my_attributes` value will be written to the
`attributes` field when serializing to Avro.

### Encoding and Decoding

`Avromatic` provides two different interfaces for encoding the key (optional)
Expand Down
77 changes: 64 additions & 13 deletions lib/avromatic/model/attributes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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))

Expand All @@ -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"\
Copy link
Member

Choose a reason for hiding this comment

The 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
Expand Down Expand Up @@ -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

Expand All @@ -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)
Expand Down
22 changes: 19 additions & 3 deletions lib/avromatic/model/configurable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be an extended hook?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it is inherited because the generated model class owns the configuration. If that model is subclassed then we want the subclass to refer back to the same configuration.

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
Expand All @@ -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
Expand All @@ -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
Expand Down
9 changes: 8 additions & 1 deletion lib/avromatic/model/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module Model
# This class holds configuration for a model built from Avro schema(s).
class Configuration

attr_reader :avro_schema, :key_avro_schema
attr_reader :avro_schema, :key_avro_schema, :aliases
delegate :schema_store, to: Avromatic

# Either schema(_name) or value_schema(_name), but not both, must be
Expand All @@ -21,6 +21,7 @@ def initialize(**options)
@avro_schema = find_avro_schema(**options)
raise ArgumentError.new('value_schema(_name) or schema(_name) must be specified') unless avro_schema
@key_avro_schema = find_schema_by_option(:key_schema, **options)
@aliases = normalize_aliases(options[:aliases])
end

alias_method :value_avro_schema, :avro_schema
Expand All @@ -41,6 +42,12 @@ def find_schema_by_option(option_name, **options)
(options[schema_name_option] && schema_store.find(options[schema_name_option]))

end

def normalize_aliases(aliases)
(aliases || {}).each_with_object(Hash.new) do |(key, value), result|
result[key.to_s] = value.to_s
end.reject { |key, value| key == value }
end
end
end
end
37 changes: 26 additions & 11 deletions lib/avromatic/model/raw_serialization.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'avromatic/model/passthrough_serializer'
require 'active_support/core_ext/hash/indifferent_access'

module Avromatic
module Model
Expand Down Expand Up @@ -44,17 +45,19 @@ def hash_of_models?(key)

def avro_hash(fields)
attributes.slice(*fields).each_with_object(Hash.new) do |(key, value), result|
result[key.to_s] = if value.is_a?(Avromatic::Model::Attributes)
value.value_attributes_for_avro
elsif array_of_models?(key)
value.map(&:value_attributes_for_avro)
elsif hash_of_models?(key)
value.each_with_object({}) do |(k, v), hash|
hash[k] = v.value_attributes_for_avro
end
else
avro_serializer[key].call(value)
end
key_str = key.to_s
name = inverse_aliases[key_str] || key_str
result[name] = if value.is_a?(Avromatic::Model::Attributes)
value.value_attributes_for_avro
elsif array_of_models?(key)
value.map(&:value_attributes_for_avro)
elsif hash_of_models?(key)
value.each_with_object({}) do |(k, v), hash|
hash[k] = v.value_attributes_for_avro
end
else
avro_serializer[key].call(value)
end
end
end

Expand All @@ -80,8 +83,20 @@ def avro_raw_decode(key: nil, value:, key_schema: nil, value_schema: nil)
new(value_attributes.merge!(key_attributes || {}))
end

def new(attributes = {})
super(alias_attribute_hash(attributes))
end

private

def alias_attribute_hash(model_attributes)
model_attributes.with_indifferent_access.tap do |hash|
(hash.keys & aliases.keys).each do |key|
hash[aliases[key]] = hash.delete(key)
end
end
end

def decode_avro_datum(data, schema = nil, key_or_value = :value)
stream = StringIO.new(data)
decoder = Avro::IO::BinaryDecoder.new(stream)
Expand Down
2 changes: 1 addition & 1 deletion lib/avromatic/version.rb
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
10 changes: 10 additions & 0 deletions spec/avro/dsl/test/reserved.rb
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
30 changes: 30 additions & 0 deletions spec/avro/schema/test/reserved.avsc
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": ""
}
]
}
Loading