Skip to content

Commit af99c0d

Browse files
committed
Ensure inheritance hooks run
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 Style note: To make diffs on the adapters smalleer and easier to read, I've maintained the same identention that was in the original file. I've decided to prefer ease of reading the diff over style, esp. since we may later return to the preferred class declaration style. with '#' will be ignored, and an empty message aborts the commit.
1 parent d9e76c2 commit af99c0d

File tree

15 files changed

+73
-95
lines changed

15 files changed

+73
-95
lines changed

.rubocop.yml

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,39 @@ AllCops:
88
DisplayCopNames: true
99
DisplayStyleGuide: true
1010

11+
Style/IndentationConsistency:
12+
Exclude:
13+
- lib/active_model/serializer/adapter/flatten_json.rb
14+
- lib/active_model/serializer/adapter/fragment_cache.rb
15+
- lib/active_model/serializer/adapter/json.rb
16+
- lib/active_model/serializer/adapter/json/fragment_cache.rb
17+
- lib/active_model/serializer/adapter/json_api.rb
18+
- lib/active_model/serializer/adapter/json_api/fragment_cache.rb
19+
- lib/active_model/serializer/adapter/json_api/pagination_links.rb
20+
- lib/active_model/serializer/adapter/null.rb
21+
22+
Style/IndentationWidth:
23+
Exclude:
24+
- lib/active_model/serializer/adapter/flatten_json.rb
25+
- lib/active_model/serializer/adapter/fragment_cache.rb
26+
- lib/active_model/serializer/adapter/json.rb
27+
- lib/active_model/serializer/adapter/json/fragment_cache.rb
28+
- lib/active_model/serializer/adapter/json_api.rb
29+
- lib/active_model/serializer/adapter/json_api/fragment_cache.rb
30+
- lib/active_model/serializer/adapter/json_api/pagination_links.rb
31+
- lib/active_model/serializer/adapter/null.rb
32+
33+
Style/AccessModifierIndentation:
34+
Exclude:
35+
- lib/active_model/serializer/adapter/flatten_json.rb
36+
- lib/active_model/serializer/adapter/fragment_cache.rb
37+
- lib/active_model/serializer/adapter/json.rb
38+
- lib/active_model/serializer/adapter/json/fragment_cache.rb
39+
- lib/active_model/serializer/adapter/json_api.rb
40+
- lib/active_model/serializer/adapter/json_api/fragment_cache.rb
41+
- lib/active_model/serializer/adapter/json_api/pagination_links.rb
42+
- lib/active_model/serializer/adapter/null.rb
43+
1144
Lint/NestedMethodDefinition:
1245
Enabled: false
1346
Exclude:
@@ -16,9 +49,6 @@ Lint/NestedMethodDefinition:
1649
Style/StringLiterals:
1750
EnforcedStyle: single_quotes
1851

19-
Style/SpecialGlobalVars:
20-
Enabled: false
21-
2252
Metrics/AbcSize:
2353
Max: 35 # TODO: Lower to 15
2454

lib/active_model/serializable_resource.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ def serializer?
7676
private
7777

7878
ActiveModelSerializers.silence_warnings do
79-
attr_reader :resource, :adapter_opts, :serializer_opts
79+
attr_reader :resource, :adapter_opts, :serializer_opts
8080
end
8181
end
8282
end

lib/active_model/serializer/adapter.rb

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@ class Adapter
55
ADAPTER_MAP = {}
66
private_constant :ADAPTER_MAP if defined?(private_constant)
77
extend ActiveSupport::Autoload
8-
require 'active_model/serializer/adapter/json'
9-
require 'active_model/serializer/adapter/json_api'
10-
autoload :FlattenJson
11-
autoload :Null
128
autoload :FragmentCache
9+
autoload :Json
10+
autoload :JsonApi
11+
autoload :Null
12+
autoload :FlattenJson
1313

1414
def self.create(resource, options = {})
1515
override = options.delete(:adapter)
@@ -60,14 +60,14 @@ def get(adapter)
6060
register(adapter_name, adapter_class)
6161
adapter_class
6262
}
63-
rescue ArgumentError
63+
rescue ArgumentError => e
6464
failure_message =
6565
"Unknown adapter: #{adapter.inspect}. Valid adapters are: #{adapters}"
66-
raise UnknownAdapterError, failure_message, $!.backtrace
67-
rescue NameError
66+
raise UnknownAdapterError, failure_message, e.backtrace
67+
rescue NameError => e
6868
failure_message =
69-
"NameError: #{$!.message}. Unknown adapter: #{adapter.inspect}. Valid adapters are: #{adapters}"
70-
raise UnknownAdapterError, failure_message, $!.backtrace
69+
"NameError: #{e.message}. Unknown adapter: #{adapter.inspect}. Valid adapters are: #{adapters}"
70+
raise UnknownAdapterError, failure_message, e.backtrace
7171
end
7272

7373
# @api private
Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
1-
module ActiveModel
2-
class Serializer
3-
class Adapter
4-
class FlattenJson < Json
1+
class ActiveModel::Serializer::Adapter::FlattenJson < ActiveModel::Serializer::Adapter::Json
52
def serializable_hash(options = {})
63
super
74
@result
@@ -13,7 +10,4 @@ def serializable_hash(options = {})
1310
def include_meta(json)
1411
json
1512
end
16-
end
17-
end
18-
end
1913
end

lib/active_model/serializer/adapter/fragment_cache.rb

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
1-
module ActiveModel
2-
class Serializer
3-
class Adapter
4-
class FragmentCache
1+
class ActiveModel::Serializer::Adapter::FragmentCache
52
attr_reader :serializer
63

74
def initialize(adapter, serializer, options)
@@ -75,7 +72,4 @@ def fragment_serializer(name, klass)
7572
def to_valid_const_name(name)
7673
name.gsub('::', '_')
7774
end
78-
end
79-
end
80-
end
8175
end

lib/active_model/serializer/adapter/json.rb

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
1-
require 'active_model/serializer/adapter/json/fragment_cache'
1+
class ActiveModel::Serializer::Adapter::Json < ActiveModel::Serializer::Adapter
2+
extend ActiveSupport::Autoload
3+
autoload :FragmentCache
24

3-
module ActiveModel
4-
class Serializer
5-
class Adapter
6-
class Json < Adapter
75
def serializable_hash(options = nil)
86
options ||= {}
97
if serializer.respond_to?(:each)
@@ -44,9 +42,6 @@ def serializable_hash(options = nil)
4442
end
4543

4644
def fragment_cache(cached_hash, non_cached_hash)
47-
Json::FragmentCache.new().fragment_cache(cached_hash, non_cached_hash)
45+
ActiveModel::Serializer::Adapter::Json::FragmentCache.new().fragment_cache(cached_hash, non_cached_hash)
4846
end
49-
end
50-
end
51-
end
5247
end
Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,5 @@
1-
require 'active_model/serializer/adapter/fragment_cache'
2-
module ActiveModel
3-
class Serializer
4-
class Adapter
5-
class Json < Adapter
6-
class FragmentCache
1+
class ActiveModel::Serializer::Adapter::Json::FragmentCache
72
def fragment_cache(cached_hash, non_cached_hash)
83
non_cached_hash.merge cached_hash
94
end
10-
end
11-
end
12-
end
13-
end
145
end

lib/active_model/serializer/adapter/json_api.rb

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
1-
require 'active_model/serializer/adapter/json_api/fragment_cache'
2-
require 'active_model/serializer/adapter/json_api/pagination_links'
1+
class ActiveModel::Serializer::Adapter::JsonApi < ActiveModel::Serializer::Adapter
2+
extend ActiveSupport::Autoload
3+
autoload :PaginationLinks
4+
autoload :FragmentCache
35

4-
module ActiveModel
5-
class Serializer
6-
class Adapter
7-
class JsonApi < Adapter
86
def initialize(serializer, options = {})
97
super
108
@hash = { data: [] }
@@ -49,7 +47,7 @@ def serializable_hash(options = nil)
4947

5048
def fragment_cache(cached_hash, non_cached_hash)
5149
root = false if @options.include?(:include)
52-
JsonApi::FragmentCache.new.fragment_cache(root, cached_hash, non_cached_hash)
50+
ActiveModel::Serializer::Adapter::JsonApi::FragmentCache.new().fragment_cache(root, cached_hash, non_cached_hash)
5351
end
5452

5553
private
@@ -62,6 +60,12 @@ def resource_identifier_type_for(serializer)
6260
end
6361
end
6462

63+
def add_relationships(resource, name, serializers)
64+
resource[:relationships] ||= {}
65+
resource[:relationships][name] ||= { data: [] }
66+
resource[:relationships][name][:data] += serializers.map { |serializer| { type: serializer.json_api_type, id: serializer.id.to_s } }
67+
end
68+
6569
def resource_identifier_id_for(serializer)
6670
if serializer.respond_to?(:id)
6771
serializer.id
@@ -161,8 +165,8 @@ def add_links(options)
161165
@hash[:links] = add_pagination_links(links, collection, options) if paginated?(collection)
162166
end
163167

164-
def add_pagination_links(links, collection, options)
165-
pagination_links = JsonApi::PaginationLinks.new(collection, options[:context]).serializable_hash(options)
168+
def add_pagination_links(links, resources, options)
169+
pagination_links = ActiveModel::Serializer::Adapter::JsonApi::PaginationLinks.new(resources, options[:context]).serializable_hash(options)
166170
links.update(pagination_links)
167171
end
168172

@@ -171,7 +175,4 @@ def paginated?(collection)
171175
collection.respond_to?(:total_pages) &&
172176
collection.respond_to?(:size)
173177
end
174-
end
175-
end
176-
end
177178
end
Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,4 @@
1-
require 'active_model/serializer/adapter/fragment_cache'
2-
module ActiveModel
3-
class Serializer
4-
class Adapter
5-
class JsonApi < Adapter
6-
class FragmentCache
1+
class ActiveModel::Serializer::Adapter::JsonApi::FragmentCache
72
def fragment_cache(root, cached_hash, non_cached_hash)
83
hash = {}
94
core_cached = cached_hash.first
@@ -15,8 +10,4 @@ def fragment_cache(root, cached_hash, non_cached_hash)
1510

1611
hash.deep_merge no_root_non_cache.deep_merge no_root_cache
1712
end
18-
end
19-
end
20-
end
21-
end
2213
end

lib/active_model/serializer/adapter/json_api/pagination_links.rb

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
1-
module ActiveModel
2-
class Serializer
3-
class Adapter
4-
class JsonApi < Adapter
5-
class PaginationLinks
1+
class ActiveModel::Serializer::Adapter::JsonApi::PaginationLinks
62
FIRST_PAGE = 1
73

84
attr_reader :collection, :context
@@ -51,8 +47,4 @@ def original_url
5147
def query_parameters
5248
@query_parameters ||= context.query_parameters
5349
end
54-
end
55-
end
56-
end
57-
end
5850
end

0 commit comments

Comments
 (0)