From ee0283cb57ba8fdd3f1d93233972628e4e335c80 Mon Sep 17 00:00:00 2001 From: Lucas Hosseini Date: Fri, 11 Dec 2015 00:29:38 +0100 Subject: [PATCH 1/6] Simplify attributes handling. --- lib/active_model/serializer/attributes.rb | 66 ++++++++--------------- 1 file changed, 22 insertions(+), 44 deletions(-) diff --git a/lib/active_model/serializer/attributes.rb b/lib/active_model/serializer/attributes.rb index 81f6e49a..676e02e0 100644 --- a/lib/active_model/serializer/attributes.rb +++ b/lib/active_model/serializer/attributes.rb @@ -1,55 +1,23 @@ module ActiveModel class Serializer module Attributes - # @api private - class Attribute - delegate :call, to: :reader - - attr_reader :name, :reader - - def initialize(name) - @name = name - @reader = :no_reader - end - - def self.build(name, block) - if block - AttributeBlock.new(name, block) - else - AttributeReader.new(name) - end - end - end - # @api private - class AttributeReader < Attribute - def initialize(name) - super(name) - @reader = ->(instance) { instance.read_attribute_for_serialization(name) } - end - end - # @api private - class AttributeBlock < Attribute - def initialize(name, block) - super(name) - @reader = ->(instance) { instance.instance_eval(&block) } - end - end - extend ActiveSupport::Concern included do with_options instance_writer: false, instance_reader: false do |serializer| serializer.class_attribute :_attribute_mappings # @api private : maps attribute key names to names to names of implementing methods, @see #attribute self._attribute_mappings ||= {} + serializer.class_attribute :_attribute_keys # @api private : maps attribute names to keys, @see #attribute + self._attribute_keys ||= {} end # Return the +attributes+ of +object+ as presented # by the serializer. def attributes(requested_attrs = nil, reload = false) @attributes = nil if reload - @attributes ||= self.class._attribute_mappings.each_with_object({}) do |(key, attribute_mapping), hash| + @attributes ||= self.class._attribute_keys.each_with_object({}) do |(name, key), hash| next unless requested_attrs.nil? || requested_attrs.include?(key) - hash[key] = attribute_mapping.call(self) + hash[key] = self.class._attribute_mappings[name].call(self) end end end @@ -58,6 +26,7 @@ module ActiveModel def inherited(base) super base._attribute_mappings = _attribute_mappings.dup + base._attribute_keys = _attribute_keys.dup end # @example @@ -84,15 +53,24 @@ module ActiveModel # object.edits.last(5) # end def attribute(attr, options = {}, &block) - key = options.fetch(:key, attr) - _attribute_mappings[key] = Attribute.build(attr, block) + _attribute_keys[attr] = options.fetch(:key, attr) + _attribute_mappings[attr] = _attribute_mapping(attr, block) end # @api private - # names of attribute methods + def _attribute_mapping(name, block) + if block + ->(instance) { instance.instance_eval(&block) } + else + ->(instance) { instance.read_attribute_for_serialization(name) } + end + end + + # @api private + # keys of attributes # @see Serializer::attribute def _attributes - _attribute_mappings.keys + _attribute_keys.values end # @api private @@ -100,10 +78,10 @@ module ActiveModel # @see Serializer::attribute # @see Adapter::FragmentCache#fragment_serializer def _attributes_keys - _attribute_mappings - .each_with_object({}) do |(key, attribute_mapping), hash| - next if key == attribute_mapping.name - hash[attribute_mapping.name] = { key: key } + _attribute_keys + .each_with_object({}) do |(name, key), hash| + next if key == name + hash[name] = { key: key } end end end From 1d4b27f60f97537c63ebd82c0aa34bb9fe69fcad Mon Sep 17 00:00:00 2001 From: Lucas Hosseini Date: Sun, 20 Dec 2015 15:56:32 +0100 Subject: [PATCH 2/6] Improve attribute value computation. --- lib/active_model/serializer/attributes.rb | 28 +++++++++++------------ 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/active_model/serializer/attributes.rb b/lib/active_model/serializer/attributes.rb index 676e02e0..35263374 100644 --- a/lib/active_model/serializer/attributes.rb +++ b/lib/active_model/serializer/attributes.rb @@ -5,8 +5,8 @@ module ActiveModel included do with_options instance_writer: false, instance_reader: false do |serializer| - serializer.class_attribute :_attribute_mappings # @api private : maps attribute key names to names to names of implementing methods, @see #attribute - self._attribute_mappings ||= {} + serializer.class_attribute :_attribute_procs # @api private : maps attribute key names to names to names of implementing methods, @see #attribute + self._attribute_procs ||= {} serializer.class_attribute :_attribute_keys # @api private : maps attribute names to keys, @see #attribute self._attribute_keys ||= {} end @@ -17,7 +17,16 @@ module ActiveModel @attributes = nil if reload @attributes ||= self.class._attribute_keys.each_with_object({}) do |(name, key), hash| next unless requested_attrs.nil? || requested_attrs.include?(key) - hash[key] = self.class._attribute_mappings[name].call(self) + hash[key] = _attribute_value(name) + end + end + + # @api private + def _attribute_value(name) + if self.class._attribute_procs[name] + instance_eval(&self.class._attribute_procs[name]) + else + read_attribute_for_serialization(name) end end end @@ -25,7 +34,7 @@ module ActiveModel module ClassMethods def inherited(base) super - base._attribute_mappings = _attribute_mappings.dup + base._attribute_procs = _attribute_procs.dup base._attribute_keys = _attribute_keys.dup end @@ -54,16 +63,7 @@ module ActiveModel # end def attribute(attr, options = {}, &block) _attribute_keys[attr] = options.fetch(:key, attr) - _attribute_mappings[attr] = _attribute_mapping(attr, block) - end - - # @api private - def _attribute_mapping(name, block) - if block - ->(instance) { instance.instance_eval(&block) } - else - ->(instance) { instance.read_attribute_for_serialization(name) } - end + _attribute_procs[attr] = block end # @api private From 7d24cbfd3d3de1c7232142c40e2ae58acbaf687b Mon Sep 17 00:00:00 2001 From: Lucas Hosseini Date: Sun, 27 Dec 2015 22:57:52 +0100 Subject: [PATCH 3/6] Extract latent Attribute object. --- lib/active_model/serializer/attribute.rb | 13 ++++++++ lib/active_model/serializer/attributes.rb | 40 +++++++++-------------- 2 files changed, 28 insertions(+), 25 deletions(-) create mode 100644 lib/active_model/serializer/attribute.rb diff --git a/lib/active_model/serializer/attribute.rb b/lib/active_model/serializer/attribute.rb new file mode 100644 index 00000000..0ab0a37b --- /dev/null +++ b/lib/active_model/serializer/attribute.rb @@ -0,0 +1,13 @@ +module ActiveModel + class Serializer + Attribute = Struct.new(:name, :key, :block) do + def value(serializer) + if block + serializer.instance_eval(&block) + else + serializer.read_attribute_for_serialization(name) + end + end + end + end +end diff --git a/lib/active_model/serializer/attributes.rb b/lib/active_model/serializer/attributes.rb index 35263374..c7018439 100644 --- a/lib/active_model/serializer/attributes.rb +++ b/lib/active_model/serializer/attributes.rb @@ -5,28 +5,19 @@ module ActiveModel included do with_options instance_writer: false, instance_reader: false do |serializer| - serializer.class_attribute :_attribute_procs # @api private : maps attribute key names to names to names of implementing methods, @see #attribute - self._attribute_procs ||= {} - serializer.class_attribute :_attribute_keys # @api private : maps attribute names to keys, @see #attribute - self._attribute_keys ||= {} + serializer.class_attribute :_attributes_data # @api private + self._attributes_data ||= {} end + autoload :Attribute + # Return the +attributes+ of +object+ as presented # by the serializer. def attributes(requested_attrs = nil, reload = false) @attributes = nil if reload - @attributes ||= self.class._attribute_keys.each_with_object({}) do |(name, key), hash| - next unless requested_attrs.nil? || requested_attrs.include?(key) - hash[key] = _attribute_value(name) - end - end - - # @api private - def _attribute_value(name) - if self.class._attribute_procs[name] - instance_eval(&self.class._attribute_procs[name]) - else - read_attribute_for_serialization(name) + @attributes ||= self.class._attributes_data.values.each_with_object({}) do |attr, hash| + next unless requested_attrs.nil? || requested_attrs.include?(attr.key) + hash[attr.key] = attr.value(self) end end end @@ -34,8 +25,7 @@ module ActiveModel module ClassMethods def inherited(base) super - base._attribute_procs = _attribute_procs.dup - base._attribute_keys = _attribute_keys.dup + base._attributes_data = _attributes_data.dup end # @example @@ -62,15 +52,15 @@ module ActiveModel # object.edits.last(5) # end def attribute(attr, options = {}, &block) - _attribute_keys[attr] = options.fetch(:key, attr) - _attribute_procs[attr] = block + key = options.fetch(:key, attr) + _attributes_data[attr] = Attribute.new(attr, key, block) end # @api private # keys of attributes # @see Serializer::attribute def _attributes - _attribute_keys.values + _attributes_data.values.map(&:key) end # @api private @@ -78,10 +68,10 @@ module ActiveModel # @see Serializer::attribute # @see Adapter::FragmentCache#fragment_serializer def _attributes_keys - _attribute_keys - .each_with_object({}) do |(name, key), hash| - next if key == name - hash[name] = { key: key } + _attributes_data.values + .each_with_object({}) do |attr, hash| + next if attr.key == attr.name + hash[attr.name] = { key: attr.key } end end end From a586a45863c953c89f931a292923521c19bc2315 Mon Sep 17 00:00:00 2001 From: Lucas Hosseini Date: Tue, 29 Dec 2015 22:45:41 +0100 Subject: [PATCH 4/6] Remove `key` from `Attribute` class. --- lib/active_model/serializer/attribute.rb | 2 +- lib/active_model/serializer/attributes.rb | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/active_model/serializer/attribute.rb b/lib/active_model/serializer/attribute.rb index 0ab0a37b..5c9893ca 100644 --- a/lib/active_model/serializer/attribute.rb +++ b/lib/active_model/serializer/attribute.rb @@ -1,6 +1,6 @@ module ActiveModel class Serializer - Attribute = Struct.new(:name, :key, :block) do + Attribute = Struct.new(:name, :block) do def value(serializer) if block serializer.instance_eval(&block) diff --git a/lib/active_model/serializer/attributes.rb b/lib/active_model/serializer/attributes.rb index c7018439..8109d2e7 100644 --- a/lib/active_model/serializer/attributes.rb +++ b/lib/active_model/serializer/attributes.rb @@ -15,9 +15,9 @@ module ActiveModel # by the serializer. def attributes(requested_attrs = nil, reload = false) @attributes = nil if reload - @attributes ||= self.class._attributes_data.values.each_with_object({}) do |attr, hash| - next unless requested_attrs.nil? || requested_attrs.include?(attr.key) - hash[attr.key] = attr.value(self) + @attributes ||= self.class._attributes_data.each_with_object({}) do |(key, attr), hash| + next unless requested_attrs.nil? || requested_attrs.include?(key) + hash[key] = attr.value(self) end end end @@ -53,14 +53,14 @@ module ActiveModel # end def attribute(attr, options = {}, &block) key = options.fetch(:key, attr) - _attributes_data[attr] = Attribute.new(attr, key, block) + _attributes_data[key] = Attribute.new(attr, block) end # @api private # keys of attributes # @see Serializer::attribute def _attributes - _attributes_data.values.map(&:key) + _attributes_data.keys end # @api private @@ -68,10 +68,10 @@ module ActiveModel # @see Serializer::attribute # @see Adapter::FragmentCache#fragment_serializer def _attributes_keys - _attributes_data.values - .each_with_object({}) do |attr, hash| - next if attr.key == attr.name - hash[attr.name] = { key: attr.key } + _attributes_data + .each_with_object({}) do |(key, attr), hash| + next if key == attr.name + hash[attr.name] = { key: key } end end end From 77095f2a847d8545be7361c0b14282e16035bf2a Mon Sep 17 00:00:00 2001 From: Lucas Hosseini Date: Wed, 30 Dec 2015 17:44:19 +0100 Subject: [PATCH 5/6] Add ActiveSupport::Autoload extension to Attribute. --- lib/active_model/serializer/attributes.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/active_model/serializer/attributes.rb b/lib/active_model/serializer/attributes.rb index 8109d2e7..f57ab205 100644 --- a/lib/active_model/serializer/attributes.rb +++ b/lib/active_model/serializer/attributes.rb @@ -9,6 +9,7 @@ module ActiveModel self._attributes_data ||= {} end + extend ActiveSupport::Autoload autoload :Attribute # Return the +attributes+ of +object+ as presented From ccb05f11ef4a3f3bd4a373330d89d74cbcf8be80 Mon Sep 17 00:00:00 2001 From: Lucas Hosseini Date: Wed, 30 Dec 2015 17:46:29 +0100 Subject: [PATCH 6/6] Add changelog. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dd7a819..113c1ec1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ Fixes: - [#1358](https://github.com/rails-api/active_model_serializers/pull/1358) Handle serializer file paths with spaces (@rwstauner, @bf4) Misc: +- [#1370](https://github.com/rails-api/active_model_serializers/pull/1370) Simplify attributes handling via a mixin (@beauby) - [#1233](https://github.com/rails-api/active_model_serializers/pull/1233) Top-level meta and meta_key options no longer handled at serializer level (@beauby) - [#1232](https://github.com/rails-api/active_model_serializers/pull/1232) fields option no longer handled at serializer level (@beauby) - [#1178](https://github.com/rails-api/active_model_serializers/pull/1178) env CAPTURE_STDERR=false lets devs see hard failures (@bf4)