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

configurable serializer_lookup option #1757

Merged
merged 15 commits into from
Nov 16, 2016
Merged
Show file tree
Hide file tree
Changes from 7 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
44 changes: 44 additions & 0 deletions docs/general/configuration_options.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,50 @@ application, setting `config.key_transform` to `:unaltered` will provide a perfo
What relationships to serialize by default. Default: `'*'`, which includes one level of related
objects. See [includes](adapters.md#included) for more info.


##### serializer_lookup_chain

Configures how serializers are searched for. By default, the lookup chain is

```ruby
ActiveModelSerializers::LookupChain::DEFAULT
```

which is shorthand for

```ruby
[
ActiveModelSerializers::LookupChain::BY_PARENT_SERIALIZER,
ActiveModelSerializers::LookupChain::BY_NAMESPACE,
ActiveModelSerializers::LookupChain::BY_RESOURCE_NAMESPACE,
ActiveModelSerializers::LookupChain::BY_RESOURCE
]
```

Each of the array entries represent a proc. A serializer lookup proc will be yielded 3 arguments. `resource_class`, `serializer_class`, and `namespace`.
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we explain that:

  • by default serializer_class is ActiveModel::Serializer
  • for association lookup it's the "parent" serializer
  • namespace correspond to either the controller namespace or the one specified with namespace option

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

couldn't hurt. I'll add that.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks 💯


An example config could be:

```ruby
ActiveModelSerializers.config.serializer_lookup_chain = [
lambda do |resource_class, serializer_class, namespace|
"API::#{namespace}::#{resource_class}"
end
]
```

If you simply want to add to the existing lookup_chain. Use `unshift`.

```ruby
ActiveModelSerializers.config.serializer_lookup_chain.unshift(
Copy link
Member

Choose a reason for hiding this comment

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

That's an opportunity to add a ActiveModelSerializers.add_lookup(&block) I guess.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe in another pr?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah sure. Just thinking out loud 😉

lambda do |resource_class, serializer_class, namespace|
# ...
end
)
```

See [lookup_chain.rb](https://github.com/rails-api/active_model_serializers/blob/master/lib/active_model_serializers/lookup_chain.rb) for further explanations and examples.

## JSON API

##### jsonapi_resource_type
Expand Down
15 changes: 4 additions & 11 deletions lib/active_model/serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,17 +60,10 @@ class << self

# @api private
def self.serializer_lookup_chain_for(klass, namespace = nil)
chain = []

resource_class_name = klass.name.demodulize
resource_namespace = klass.name.deconstantize
serializer_class_name = "#{resource_class_name}Serializer"

chain.push("#{namespace}::#{serializer_class_name}") if namespace
chain.push("#{name}::#{serializer_class_name}") if self != ActiveModel::Serializer
chain.push("#{resource_namespace}::#{serializer_class_name}")

chain
lookups = ActiveModelSerializers.config.serializer_lookup_chain
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@richmolj this looks nice - It's already chached, see line 86

Copy link
Member

Choose a reason for hiding this comment

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

I think it would be better for the lookup_chain to be a class_attribute or some such on the serializer

class_attribute(:lookup_chain) {  ActiveModelSerializers.config.serializer_lookup_chain }

Copy link
Member

Choose a reason for hiding this comment

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

makes it easier to configure in place, and no need for changing global state

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that'd probably allow the lookup tests to not need to be isolated.

Array[*lookups].flat_map do |lookup|
lookup.call(klass, self, namespace)
end.compact
end

# Used to cache serializer name => serializer class
Expand Down
20 changes: 20 additions & 0 deletions lib/active_model/serializer/concerns/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,26 @@ def config.array_serializer
config.jsonapi_include_toplevel_object = false
config.include_data_default = true

# For configuring how serializers are found.
# This should be an array of procs.
#
# The priority of the output is that the first item
# in the evaluated result array will take precedence
# over other possible serializer paths.
#
# i.e.: First match wins.
#
# @example output
# => [
# "CustomNamespace::ResourceSerializer",
# "ParentSerializer::ResourceSerializer",
# "ResourceNamespace::ResourceSerializer" ,
# "ResourceSerializer"]
#
# If CustomNamespace::ResourceSerializer exists, it will be used
# for serialization
config.serializer_lookup_chain = ActiveModelSerializers::LookupChain::DEFAULT.dup
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the .dup?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DEFAULT is a frozen array, and we want mutatability?

Copy link
Contributor

Choose a reason for hiding this comment

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

DEFAULT should be frozen, but are there real use cases for injecting stuff in the lookup chain rather than replacing it altogether?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not entirely sure on that one.

Do we get a performance boost if serializer_lookup_chain is frozen? I wouldn't think it matters, since config is a singletonish thing


config.schema_path = 'test/support/schemas'
end
end
Expand Down
1 change: 1 addition & 0 deletions lib/active_model_serializers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ module ActiveModelSerializers
autoload :Adapter
autoload :JsonPointer
autoload :Deprecate
autoload :LookupChain

class << self; attr_accessor :logger; end
self.logger = ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new(STDOUT))
Expand Down
80 changes: 80 additions & 0 deletions lib/active_model_serializers/lookup_chain.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
module ActiveModelSerializers
module LookupChain
# Standard appending of Serializer to the resource name.
#
# Example:
# Author => AuthorSerializer
BY_RESOURCE = lambda do |resource_class, _serializer_class, _namespace|
serializer_from(resource_class)
end
Copy link
Member

Choose a reason for hiding this comment

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

@NullVoxPopuli don't kill me, but can we put the new lookup chain in a separate PR and just add the namespace code in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lol. It'll have to be tonight, so I can dedicate more mental capacity to splitting this apart. :-)


# Uses the namespace of the resource to find the serializer
#
# Example:
# British::Author => British::AuthorSerializer
BY_RESOURCE_NAMESPACE = lambda do |resource_class, _serializer_class, _namespace|
resource_namespace = namespace_for(resource_class)
serializer_name = serializer_from(resource_class)

"#{resource_namespace}::#{serializer_name}"
end

# Uses the controller namespace of the resource to find the serializer
#
# Example:
# Api::V3::AuthorsController => Api::V3::AuthorSerializer
BY_NAMESPACE = lambda do |resource_class, _serializer_class, namespace|
resource_name = resource_class_name(resource_class)
namespace ? "#{namespace}::#{resource_name}Serializer" : nil
end

# Allows for serializers to be defined in parent serializers
# - useful if a relationship only needs a different set of attributes
# than if it were rendered independently.
#
# Example:
# class BlogSerializer < ActiveModel::Serializer
# class AuthorSerialier < ActiveModel::Serializer
# ...
# end
#
# belongs_to :author
# ...
# end
#
# The belongs_to relationship would be rendered with
# BlogSerializer::AuthorSerialier
BY_PARENT_SERIALIZER = lambda do |resource_class, serializer_class, _namespace|
return if serializer_class == ActiveModel::Serializer

serializer_name = serializer_from(resource_class)
"#{serializer_class}::#{serializer_name}"
end
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, these are all the lookup strategies that AMS 0.10 has been using -- just re-implemented as procs. This was a suggestion by @richmolj, I think. In case someone wanted to customize the lookup order, they could.


DEFAULT = [
BY_PARENT_SERIALIZER,
BY_NAMESPACE,
BY_RESOURCE_NAMESPACE,
BY_RESOURCE
].freeze

module_function

def namespace_for(klass)
klass.name.deconstantize
end

def resource_class_name(klass)
klass.name.demodulize
end

def serializer_from_resource_name(name)
"#{name}Serializer"
end

def serializer_from(klass)
name = resource_class_name(klass)
serializer_from_resource_name(name)
end
end
end
49 changes: 49 additions & 0 deletions test/action_controller/lookup_proc_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
require 'test_helper'

module ActionController
module Serialization
class LookupProcTest < ActionController::TestCase
module Api
module V3
class PostCustomSerializer < ActiveModel::Serializer
attributes :title, :body

belongs_to :author
end

class AuthorCustomSerializer < ActiveModel::Serializer
attributes :name
end

class LookupProcTestController < ActionController::Base
def implicit_namespaced_serializer
author = Author.new(name: 'Bob')
post = Post.new(title: 'New Post', body: 'Body', author: author)

render json: post
end
end
end
end

tests Api::V3::LookupProcTestController

test 'implicitly uses namespaced serializer' do
controller_namespace = lambda do |resource_class, _parent_serializer_class, namespace|
"#{namespace}::#{resource_class}CustomSerializer" if namespace
end

with_prepended_lookup(controller_namespace) do
get :implicit_namespaced_serializer

assert_serializer Api::V3::PostCustomSerializer

expected = { 'title' => 'New Post', 'body' => 'Body', 'author' => { 'name' => 'Bob' } }
actual = JSON.parse(@response.body)

assert_equal expected, actual
end
end
end
end
end
8 changes: 8 additions & 0 deletions test/support/serialization_testing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ def with_namespace_separator(separator)
ActiveModelSerializers.config.jsonapi_namespace_separator = original_separator
end

def with_prepended_lookup(lookup_proc)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this only works when running isolated tests.

original_lookup = ActiveModelSerializers.config.serializer_lookup_cahin
ActiveModelSerializers.config.serializer_lookup_chain.unshift lookup_proc
yield
ensure
ActiveModelSerializers.config.serializer_lookup_cahin = original_lookup
end

# Aliased as :with_configured_adapter to clarify that
# this method tests the configured adapter.
# When not testing configuration, it may be preferable
Expand Down