Skip to content

Commit

Permalink
Ensure inheritance hooks run
Browse files Browse the repository at this point in the history
I was seeing transient failures where adapters may not be registered.

e.g. https://travis-ci.org/rails-api/active_model_serializers/builds/77735382

Since we're using the Adapter, JsonApi, and Json classes
as namespaces, some of the conventions we use for modules don't apply.
Basically, we don't want to define the class anywhere besides itself.
Otherwise, the inherited hooks may not run, and some adapters may not
be registered.

For example:

If we have a class Api `class Api; end`
And Api is also used as a namespace for `Api::Product`
And the classes are defined in different files.

In one file:

```ruby
class Api
  autoload :Product
  def self.inherited(subclass)
    puts
    p [:inherited, subclass.name]
    puts
  end
end
```

And in another:

```ruby
class Api
  class Product < Api
    def sell_sell_sell!
      # TODO: sell
    end
  end
end
```

If we load the Api class file first, the inherited hook will be defined on the class
so that when we load the Api::Product class, we'll see the output:

```plain
[ :inherited, Api::Product]
```

However, if we load the Api::Product class first, since it defines the `Api` class
and then inherited from it, the Api file was never loaded, the hook never defined,
and thus never run.

By defining the class as `class Api::Product < Api` We ensure the the Api class
MUST be defined, and thus, the hook will be defined and run and so sunshine and unicorns.

Appendix:

The below would work, but triggers a circular reference warning.
It's also not recommended to mix require with autoload.

```ruby
require 'api'
class Api
  class Product < Api
    def sell_sell_sell!
      # TODO: sell
    end
  end
end
```

This failure scenario was introduced by removing the circular reference warnings in
#1067
  • Loading branch information
bf4 committed Aug 28, 2015
1 parent 18e5812 commit cf6a074
Show file tree
Hide file tree
Showing 10 changed files with 332 additions and 389 deletions.
8 changes: 4 additions & 4 deletions lib/active_model/serializer/adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ class Adapter
UnknownAdapterError = Class.new(ArgumentError)
ADAPTER_MAP = {}
extend ActiveSupport::Autoload
require 'active_model/serializer/adapter/json'
require 'active_model/serializer/adapter/json_api'
autoload :FlattenJson
autoload :Null
autoload :FragmentCache
autoload :Json
autoload :JsonApi
autoload :Null
autoload :FlattenJson

def self.create(resource, options = {})
override = options.delete(:adapter)
Expand Down
24 changes: 9 additions & 15 deletions lib/active_model/serializer/adapter/flatten_json.rb
Original file line number Diff line number Diff line change
@@ -1,19 +1,13 @@
module ActiveModel
class Serializer
class Adapter
class FlattenJson < Json
def serializable_hash(options = {})
super
@result
end
class ActiveModel::Serializer::Adapter::FlattenJson < ActiveModel::Serializer::Adapter::Json
def serializable_hash(options = {})
super
@result
end

private
private

# no-op: FlattenJson adapter does not include meta data, because it does not support root.
def include_meta(json)
json
end
end
end
# no-op: FlattenJson adapter does not include meta data, because it does not support root.
def include_meta(json)
json
end
end
152 changes: 73 additions & 79 deletions lib/active_model/serializer/adapter/fragment_cache.rb
Original file line number Diff line number Diff line change
@@ -1,82 +1,76 @@
module ActiveModel
class Serializer
class Adapter
class FragmentCache

attr_reader :serializer

def initialize(adapter, serializer, options)
@options = options
@adapter = adapter
@serializer = serializer
end

def fetch
klass = serializer.class
# It will split the serializer into two, one that will be cached and other wont
serializers = fragment_serializer(serializer.object.class.name, klass)

# Instanciate both serializers
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)

# 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)
end

private

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) }
non_cached_attributes = attributes - cached_attributes

cached_attributes.each do |attribute|
options = serializer.class._attributes_keys[attribute]
options ||= {}
# Add cached attributes to cached Serializer
serializers[:cached].constantize.attribute(attribute, options)
end

non_cached_attributes.each do |attribute|
options = serializer.class._attributes_keys[attribute]
options ||= {}
# Add non-cached attributes to non-cached Serializer
serializers[:non_cached].constantize.attribute(attribute, options)
end
end

def fragment_serializer(name, klass)
cached = "#{to_valid_const_name(name)}CachedSerializer"
non_cached = "#{to_valid_const_name(name)}NonCachedSerializer"

Object.const_set cached, Class.new(ActiveModel::Serializer) unless Object.const_defined?(cached)
Object.const_set non_cached, Class.new(ActiveModel::Serializer) unless Object.const_defined?(non_cached)

klass._cache_options ||= {}
klass._cache_options[:key] = klass._cache_key if klass._cache_key

cached.constantize.cache(klass._cache_options)

cached.constantize.fragmented(serializer)
non_cached.constantize.fragmented(serializer)

serializers = {cached: cached, non_cached: non_cached}
cached_attributes(klass, serializers)
serializers
end

def to_valid_const_name(name)
name.gsub('::', '_')
end
end
class ActiveModel::Serializer::Adapter::FragmentCache

attr_reader :serializer

def initialize(adapter, serializer, options)
@options = options
@adapter = adapter
@serializer = serializer
end

def fetch
klass = serializer.class
# It will split the serializer into two, one that will be cached and other wont
serializers = fragment_serializer(serializer.object.class.name, klass)

# Instanciate both serializers
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)

# 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)
end

private

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) }
non_cached_attributes = attributes - cached_attributes

cached_attributes.each do |attribute|
options = serializer.class._attributes_keys[attribute]
options ||= {}
# Add cached attributes to cached Serializer
serializers[:cached].constantize.attribute(attribute, options)
end

non_cached_attributes.each do |attribute|
options = serializer.class._attributes_keys[attribute]
options ||= {}
# Add non-cached attributes to non-cached Serializer
serializers[:non_cached].constantize.attribute(attribute, options)
end
end

def fragment_serializer(name, klass)
cached = "#{to_valid_const_name(name)}CachedSerializer"
non_cached = "#{to_valid_const_name(name)}NonCachedSerializer"

Object.const_set cached, Class.new(ActiveModel::Serializer) unless Object.const_defined?(cached)
Object.const_set non_cached, Class.new(ActiveModel::Serializer) unless Object.const_defined?(non_cached)

klass._cache_options ||= {}
klass._cache_options[:key] = klass._cache_key if klass._cache_key

cached.constantize.cache(klass._cache_options)

cached.constantize.fragmented(serializer)
non_cached.constantize.fragmented(serializer)

serializers = {cached: cached, non_cached: non_cached}
cached_attributes(klass, serializers)
serializers
end

def to_valid_const_name(name)
name.gsub('::', '_')
end
end
78 changes: 36 additions & 42 deletions lib/active_model/serializer/adapter/json.rb
Original file line number Diff line number Diff line change
@@ -1,53 +1,47 @@
require 'active_model/serializer/adapter/json/fragment_cache'
class ActiveModel::Serializer::Adapter::Json < ActiveModel::Serializer::Adapter
extend ActiveSupport::Autoload
autoload :FragmentCache

module ActiveModel
class Serializer
class Adapter
class Json < Adapter
def serializable_hash(options = nil)
options ||= {}
if serializer.respond_to?(:each)
@result = serializer.map { |s| FlattenJson.new(s).serializable_hash(options) }
else
@hash = {}
def serializable_hash(options = nil)
options ||= {}
if serializer.respond_to?(:each)
@result = serializer.map { |s| FlattenJson.new(s).serializable_hash(options) }
else
@hash = {}

@core = cache_check(serializer) do
serializer.attributes(options)
end
@core = cache_check(serializer) do
serializer.attributes(options)
end

serializer.associations.each do |association|
serializer = association.serializer
opts = association.options
serializer.associations.each do |association|
serializer = association.serializer
opts = 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)
end
end
else
@hash[association.key] =
if serializer && serializer.object
cache_check(serializer) do
serializer.attributes(options)
end
elsif opts[:virtual_value]
opts[:virtual_value]
end
end
if serializer.respond_to?(:each)
array_serializer = serializer
@hash[association.key] = array_serializer.map do |item|
cache_check(item) do
item.attributes(opts)
end
@result = @core.merge @hash
end

{ root => @result }
end

def fragment_cache(cached_hash, non_cached_hash)
Json::FragmentCache.new().fragment_cache(cached_hash, non_cached_hash)
else
@hash[association.key] =
if serializer && serializer.object
cache_check(serializer) do
serializer.attributes(options)
end
elsif opts[:virtual_value]
opts[:virtual_value]
end
end

end
@result = @core.merge @hash
end

{ root => @result }
end

def fragment_cache(cached_hash, non_cached_hash)
ActiveModel::Serializer::Adapter::Json::FragmentCache.new().fragment_cache(cached_hash, non_cached_hash)
end
end
16 changes: 3 additions & 13 deletions lib/active_model/serializer/adapter/json/fragment_cache.rb
Original file line number Diff line number Diff line change
@@ -1,16 +1,6 @@
require 'active_model/serializer/adapter/fragment_cache'
module ActiveModel
class Serializer
class Adapter
class Json < Adapter
class FragmentCache
class ActiveModel::Serializer::Adapter::Json::FragmentCache

def fragment_cache(cached_hash, non_cached_hash)
non_cached_hash.merge cached_hash
end

end
end
end
def fragment_cache(cached_hash, non_cached_hash)
non_cached_hash.merge cached_hash
end
end
Loading

0 comments on commit cf6a074

Please sign in to comment.