From 9d65f0adc54cb5dccf2ded7a0e0828443a8fde8f Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Wed, 16 Sep 2015 22:12:13 -0500 Subject: [PATCH] Distinguish options ivar from local; Extract latent Adapter::CachedSerializer --- lib/active_model/serializer.rb | 90 ++++++++++--------- lib/active_model/serializer/adapter.rb | 38 +------- .../serializer/adapter/cached_serializer.rb | 45 ++++++++++ .../serializer/adapter/fragment_cache.rb | 12 ++- lib/active_model/serializer/adapter/json.rb | 8 +- .../serializer/adapter/json_api.rb | 20 +++-- .../serializer/array_serializer.rb | 8 +- lib/active_model/serializer/associations.rb | 2 +- lib/active_model/serializer/fieldset.rb | 2 +- test/fixtures/poro.rb | 6 +- 10 files changed, 131 insertions(+), 100 deletions(-) create mode 100644 lib/active_model/serializer/adapter/cached_serializer.rb diff --git a/lib/active_model/serializer.rb b/lib/active_model/serializer.rb index 2a3465cb..6e00c2db 100644 --- a/lib/active_model/serializer.rb +++ b/lib/active_model/serializer.rb @@ -42,16 +42,16 @@ module ActiveModel end def self.inherited(base) - base._attributes = self._attributes.try(:dup) || [] - base._attributes_keys = self._attributes_keys.try(:dup) || {} + base._attributes = _attributes.try(:dup) || [] + base._attributes_keys = _attributes_keys.try(:dup) || {} base._cache_digest = digest_caller_file(caller.first) super end def self.attributes(*attrs) attrs = attrs.first if attrs.first.class == Array - @_attributes.concat attrs - @_attributes.uniq! + _attributes.concat(attrs) + _attributes.uniq! attrs.each do |attr| define_method attr do @@ -62,8 +62,8 @@ module ActiveModel def self.attribute(attr, options = {}) key = options.fetch(:key, attr) - @_attributes_keys[attr] = { key: key } if key != attr - @_attributes << key unless @_attributes.include?(key) + self._attributes_keys[attr] = { key: key } if key != attr + self._attributes << key unless _attributes.include?(key) ActiveModelSerializers.silence_warnings do define_method key do @@ -73,16 +73,16 @@ module ActiveModel end def self.fragmented(serializer) - @_fragmented = serializer + self._fragmented = serializer end # Enables a serializer to be automatically cached def self.cache(options = {}) - @_cache = ActionController::Base.cache_store if Rails.configuration.action_controller.perform_caching - @_cache_key = options.delete(:key) - @_cache_only = options.delete(:only) - @_cache_except = options.delete(:except) - @_cache_options = (options.empty?) ? nil : options + self._cache = ActionController::Base.cache_store if Rails.configuration.action_controller.perform_caching + self._cache_key = options.delete(:key) + self._cache_only = options.delete(:only) + self._cache_except = options.delete(:except) + self._cache_options = (options.empty?) ? nil : options end def self.serializer_for(resource, options = {}) @@ -91,7 +91,7 @@ module ActiveModel elsif resource.respond_to?(:to_ary) config.array_serializer else - options.fetch(:serializer, get_serializer_for(resource.class)) + options.fetch(:serializer) { get_serializer_for(resource.class) } end end @@ -104,17 +104,40 @@ module ActiveModel name.demodulize.underscore.sub(/_serializer$/, '') if name end + def self.serializers_cache + @serializers_cache ||= ThreadSafe::Cache.new + end + + def self.digest_caller_file(caller_line) + serializer_file_path = caller_line[CALLER_FILE] + serializer_file_contents = IO.read(serializer_file_path) + Digest::MD5.hexdigest(serializer_file_contents) + end + + def self.get_serializer_for(klass) + serializers_cache.fetch_or_store(klass) do + serializer_class_name = "#{klass.name}Serializer" + serializer_class = serializer_class_name.safe_constantize + + if serializer_class + serializer_class + elsif klass.superclass + get_serializer_for(klass.superclass) + end + end + end + attr_accessor :object, :root, :meta, :meta_key, :scope def initialize(object, options = {}) - @object = object - @options = options - @root = options[:root] - @meta = options[:meta] - @meta_key = options[:meta_key] - @scope = options[:scope] + self.object = object + self.instance_options = options + self.root = instance_options[:root] + self.meta = instance_options[:meta] + self.meta_key = instance_options[:meta_key] + self.scope = instance_options[:scope] - scope_name = options[:scope_name] + scope_name = instance_options[:scope_name] if scope_name && !respond_to?(scope_name) self.class.class_eval do define_method scope_name, lambda { scope } @@ -123,7 +146,7 @@ module ActiveModel end def json_key - @root || object.class.model_name.to_s.underscore + root || object.class.model_name.to_s.underscore end def attributes(options = {}) @@ -143,29 +166,10 @@ module ActiveModel end end - def self.serializers_cache - @serializers_cache ||= ThreadSafe::Cache.new - end + private # rubocop:disable Lint/UselessAccessModifier - def self.digest_caller_file(caller_line) - serializer_file_path = caller_line[CALLER_FILE] - serializer_file_contents = IO.read(serializer_file_path) - Digest::MD5.hexdigest(serializer_file_contents) - end - - attr_reader :options - - def self.get_serializer_for(klass) - serializers_cache.fetch_or_store(klass) do - serializer_class_name = "#{klass.name}Serializer" - serializer_class = serializer_class_name.safe_constantize - - if serializer_class - serializer_class - elsif klass.superclass - get_serializer_for(klass.superclass) - end - end + ActiveModelSerializers.silence_warnings do + attr_accessor :instance_options end end end diff --git a/lib/active_model/serializer/adapter.rb b/lib/active_model/serializer/adapter.rb index 98987bda..35f35686 100644 --- a/lib/active_model/serializer/adapter.rb +++ b/lib/active_model/serializer/adapter.rb @@ -10,6 +10,7 @@ module ActiveModel autoload :JsonApi autoload :Null autoload :FlattenJson + autoload :CachedSerializer def self.create(resource, options = {}) override = options.delete(:adapter) @@ -80,11 +81,11 @@ module ActiveModel ActiveModel::Serializer::Adapter.register(subclass.to_s.demodulize, subclass) end - attr_reader :serializer + attr_reader :serializer, :instance_options def initialize(serializer, options = {}) @serializer = serializer - @options = options + @instance_options = options end def serializable_hash(options = nil) @@ -101,43 +102,12 @@ module ActiveModel raise NotImplementedError, 'This is an abstract method. Should be implemented at the concrete adapter.' end - private - def cache_check(serializer) - @cached_serializer = serializer - @klass = @cached_serializer.class - if is_cached? - @klass._cache.fetch(cache_key, @klass._cache_options) do - yield - end - elsif is_fragment_cached? - FragmentCache.new(self, @cached_serializer, @options).fetch - else + CachedSerializer.new(serializer).cache_check(self) do yield end end - def is_cached? - @klass._cache && !@klass._cache_only && !@klass._cache_except - end - - def is_fragment_cached? - @klass._cache_only && !@klass._cache_except || !@klass._cache_only && @klass._cache_except - end - - def cache_key - parts = [] - parts << object_cache_key - parts << @klass._cache_digest unless @klass._cache_options && @klass._cache_options[:skip_digest] - parts.join('/') - end - - def object_cache_key - object_time_safe = @cached_serializer.object.updated_at - object_time_safe = object_time_safe.strftime('%Y%m%d%H%M%S%9N') if object_time_safe.respond_to?(:strftime) - (@klass._cache_key) ? "#{@klass._cache_key}/#{@cached_serializer.object.id}-#{object_time_safe}" : @cached_serializer.object.cache_key - end - def meta serializer.meta if serializer.respond_to?(:meta) end diff --git a/lib/active_model/serializer/adapter/cached_serializer.rb b/lib/active_model/serializer/adapter/cached_serializer.rb new file mode 100644 index 00000000..86bf65ac --- /dev/null +++ b/lib/active_model/serializer/adapter/cached_serializer.rb @@ -0,0 +1,45 @@ +module ActiveModel + class Serializer + class Adapter + class CachedSerializer + def initialize(serializer) + @cached_serializer = serializer + @klass = @cached_serializer.class + end + + def cache_check(adapter_instance) + if cached? + @klass._cache.fetch(cache_key, @klass._cache_options) do + yield + end + elsif fragment_cached? + FragmentCache.new(adapter_instance, @cached_serializer, adapter_instance.instance_options).fetch + else + yield + end + end + + def cached? + @klass._cache && !@klass._cache_only && !@klass._cache_except + end + + def fragment_cached? + @klass._cache_only && !@klass._cache_except || !@klass._cache_only && @klass._cache_except + end + + def cache_key + parts = [] + parts << object_cache_key + parts << @klass._cache_digest unless @klass._cache_options && @klass._cache_options[:skip_digest] + parts.join('/') + end + + def object_cache_key + object_time_safe = @cached_serializer.object.updated_at + object_time_safe = object_time_safe.strftime('%Y%m%d%H%M%S%9N') if object_time_safe.respond_to?(:strftime) + (@klass._cache_key) ? "#{@klass._cache_key}/#{@cached_serializer.object.id}-#{object_time_safe}" : @cached_serializer.object.cache_key + end + end + end + end +end diff --git a/lib/active_model/serializer/adapter/fragment_cache.rb b/lib/active_model/serializer/adapter/fragment_cache.rb index 13558c08..8b1380f2 100644 --- a/lib/active_model/serializer/adapter/fragment_cache.rb +++ b/lib/active_model/serializer/adapter/fragment_cache.rb @@ -2,7 +2,7 @@ class ActiveModel::Serializer::Adapter::FragmentCache attr_reader :serializer def initialize(adapter, serializer, options) - @options = options + @instance_options = options @adapter = adapter @serializer = serializer end @@ -16,19 +16,23 @@ class ActiveModel::Serializer::Adapter::FragmentCache cached_serializer = serializers[:cached].constantize.new(serializer.object) non_cached_serializer = serializers[:non_cached].constantize.new(serializer.object) - cached_adapter = @adapter.class.new(cached_serializer, @options) - non_cached_adapter = @adapter.class.new(non_cached_serializer, @options) + cached_adapter = adapter.class.new(cached_serializer, instance_options) + non_cached_adapter = adapter.class.new(non_cached_serializer, instance_options) # Get serializable hash from both cached_hash = cached_adapter.serializable_hash non_cached_hash = non_cached_adapter.serializable_hash # Merge both results - @adapter.fragment_cache(cached_hash, non_cached_hash) + adapter.fragment_cache(cached_hash, non_cached_hash) end private + ActiveModelSerializers.silence_warnings do + attr_reader :instance_options, :adapter + end + def cached_attributes(klass, serializers) attributes = serializer.class._attributes cached_attributes = (klass._cache_only) ? klass._cache_only : attributes.reject { |attr| klass._cache_except.include?(attr) } diff --git a/lib/active_model/serializer/adapter/json.rb b/lib/active_model/serializer/adapter/json.rb index f7b4fb84..aec8fc4e 100644 --- a/lib/active_model/serializer/adapter/json.rb +++ b/lib/active_model/serializer/adapter/json.rb @@ -15,13 +15,13 @@ class ActiveModel::Serializer::Adapter::Json < ActiveModel::Serializer::Adapter serializer.associations.each do |association| serializer = association.serializer - opts = association.options + association_options = association.options if serializer.respond_to?(:each) array_serializer = serializer hash[association.key] = array_serializer.map do |item| cache_check(item) do - item.attributes(opts) + item.attributes(association_options) end end else @@ -30,8 +30,8 @@ class ActiveModel::Serializer::Adapter::Json < ActiveModel::Serializer::Adapter cache_check(serializer) do serializer.attributes(options) end - elsif opts[:virtual_value] - opts[:virtual_value] + elsif association_options[:virtual_value] + association_options[:virtual_value] end end end diff --git a/lib/active_model/serializer/adapter/json_api.rb b/lib/active_model/serializer/adapter/json_api.rb index 1bd9ce52..89d196e8 100644 --- a/lib/active_model/serializer/adapter/json_api.rb +++ b/lib/active_model/serializer/adapter/json_api.rb @@ -5,7 +5,7 @@ class ActiveModel::Serializer::Adapter::JsonApi < ActiveModel::Serializer::Adapt def initialize(serializer, options = {}) super - @included = ActiveModel::Serializer::Utils.include_args_to_hash(@options[:include]) + @included = ActiveModel::Serializer::Utils.include_args_to_hash(instance_options[:include]) fields = options.delete(:fields) if fields @fieldset = ActiveModel::Serializer::Fieldset.new(fields, serializer.json_key) @@ -24,16 +24,20 @@ class ActiveModel::Serializer::Adapter::JsonApi < ActiveModel::Serializer::Adapt end def fragment_cache(cached_hash, non_cached_hash) - root = false if @options.include?(:include) + root = false if instance_options.include?(:include) ActiveModel::Serializer::Adapter::JsonApi::FragmentCache.new.fragment_cache(root, cached_hash, non_cached_hash) end private + ActiveModel.silence_warnings do + attr_reader :included, :fieldset + end + def serializable_hash_for_collection(serializer, options) hash = { data: [] } serializer.each do |s| - result = self.class.new(s, @options.merge(fieldset: @fieldset)).serializable_hash(options) + result = self.class.new(s, instance_options.merge(fieldset: fieldset)).serializable_hash(options) hash[:data] << result[:data] if result[:included] @@ -85,7 +89,7 @@ class ActiveModel::Serializer::Adapter::JsonApi < ActiveModel::Serializer::Adapt end def resource_object_for(serializer, options = {}) - options[:fields] = @fieldset && @fieldset.fields_for(serializer) + options[:fields] = fieldset && fieldset.fields_for(serializer) cache_check(serializer) do result = resource_identifier_for(serializer) @@ -120,12 +124,10 @@ class ActiveModel::Serializer::Adapter::JsonApi < ActiveModel::Serializer::Adapt end def included_for(serializer) - included = @included.flat_map do |inc| + included.flat_map { |inc| association = serializer.associations.find { |assoc| assoc.key == inc.first } _included_for(association.serializer, inc.second) if association - end - - included.uniq + }.uniq end def _included_for(serializer, includes) @@ -134,7 +136,7 @@ class ActiveModel::Serializer::Adapter::JsonApi < ActiveModel::Serializer::Adapt else return [] unless serializer && serializer.object - primary_data = primary_data_for(serializer, @options) + primary_data = primary_data_for(serializer, instance_options) relationships = relationships_for(serializer) primary_data[:relationships] = relationships if relationships.any? diff --git a/lib/active_model/serializer/array_serializer.rb b/lib/active_model/serializer/array_serializer.rb index 54cb17a0..7a5aed20 100644 --- a/lib/active_model/serializer/array_serializer.rb +++ b/lib/active_model/serializer/array_serializer.rb @@ -26,7 +26,7 @@ module ActiveModel end def json_key - key = root || @serializers.first.try(:json_key) || object.try(:name).try(:underscore) + key = root || serializers.first.try(:json_key) || object.try(:name).try(:underscore) key.try(:pluralize) end @@ -35,6 +35,12 @@ module ActiveModel object.respond_to?(:total_pages) && object.respond_to?(:size) end + + private # rubocop:disable Lint/UselessAccessModifier + + ActiveModelSerializers.silence_warnings do + attr_reader :serializers + end end end end diff --git a/lib/active_model/serializer/associations.rb b/lib/active_model/serializer/associations.rb index 1320eae3..c8628435 100644 --- a/lib/active_model/serializer/associations.rb +++ b/lib/active_model/serializer/associations.rb @@ -88,7 +88,7 @@ module ActiveModel Enumerator.new do |y| self.class._reflections.each do |reflection| - y.yield reflection.build_association(self, options) + y.yield reflection.build_association(self, instance_options) end end end diff --git a/lib/active_model/serializer/fieldset.rb b/lib/active_model/serializer/fieldset.rb index 9d1f8ae3..55dad634 100644 --- a/lib/active_model/serializer/fieldset.rb +++ b/lib/active_model/serializer/fieldset.rb @@ -26,7 +26,7 @@ module ActiveModel raw_fields.inject({}) { |h, (k, v)| h[k.to_sym] = v.map(&:to_sym); h } elsif raw_fields.is_a?(Array) if root.nil? - raise ArgumentError, 'The root argument must be specified if the fields argument is an array.' + raise ArgumentError, 'The root argument must be specified if the fields argument is an array.'.freeze end hash = {} hash[root.to_sym] = raw_fields.map(&:to_sym) diff --git a/test/fixtures/poro.rb b/test/fixtures/poro.rb index 72394e66..bfb9c84a 100644 --- a/test/fixtures/poro.rb +++ b/test/fixtures/poro.rb @@ -60,7 +60,7 @@ class ProfileSerializer < ActiveModel::Serializer attributes :name, :description def arguments_passed_in? - options[:my_options] == :accessible + instance_options[:my_options] == :accessible end end @@ -102,7 +102,7 @@ PostSerializer = Class.new(ActiveModel::Serializer) do end def custom_options - options + instance_options end end @@ -123,7 +123,7 @@ CommentSerializer = Class.new(ActiveModel::Serializer) do belongs_to :author def custom_options - options + instance_options end end