-
Notifications
You must be signed in to change notification settings - Fork 1.4k
configurable serializer_lookup option #1757
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
Changes from all commits
c9f9db0
4d4f71e
eb7282d
7d587ef
cdfc0db
4222fb6
7e7074c
bdb71bc
9240392
999c858
c1a35fe
9aeace8
9202deb
1c318dc
143c004
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,6 +60,56 @@ 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`. | ||
|
|
||
| Note that: | ||
| - `resource_class` is the class of the resource being rendered | ||
| - by default `serializer_class` is `ActiveModel::Serializer` | ||
| - for association lookup it's the "parent" serializer | ||
| - `namespace` correspond to either the controller namespace or the [optionally] specified [namespace render option](./rendering.md#namespace) | ||
|
|
||
| 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( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's an opportunity to add a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe in another pr?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @richmolj this looks nice - It's already chached, see line 86
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 }
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DEFAULT is a frozen array, and we want mutatability?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
|
||
| 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 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. None of these actually use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
| 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 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| require_relative './benchmarking_support' | ||
| require_relative './app' | ||
|
|
||
| time = 10 | ||
| disable_gc = true | ||
| ActiveModelSerializers.config.key_transform = :unaltered | ||
|
|
||
| module AmsBench | ||
| module Api | ||
| module V1 | ||
| class PrimaryResourceSerializer < ActiveModel::Serializer | ||
| attributes :title, :body | ||
|
|
||
| has_many :has_many_relationships | ||
| end | ||
|
|
||
| class HasManyRelationshipSerializer < ActiveModel::Serializer | ||
| attribute :body | ||
| end | ||
| end | ||
| end | ||
| class PrimaryResourceSerializer < ActiveModel::Serializer | ||
| attributes :title, :body | ||
|
|
||
| has_many :has_many_relationships | ||
|
|
||
| class HasManyRelationshipSerializer < ActiveModel::Serializer | ||
| attribute :body | ||
| end | ||
| end | ||
| end | ||
|
|
||
| resource = PrimaryResource.new( | ||
| id: 1, | ||
| title: 'title', | ||
| body: 'body', | ||
| has_many_relationships: [ | ||
| HasManyRelationship.new(id: 1, body: 'body1'), | ||
| HasManyRelationship.new(id: 2, body: 'body1') | ||
| ] | ||
| ) | ||
|
|
||
| serialization = lambda do | ||
| ActiveModelSerializers::SerializableResource.new(resource, serializer: AmsBench::PrimaryResourceSerializer).as_json | ||
| ActiveModelSerializers::SerializableResource.new(resource, namespace: AmsBench::Api::V1).as_json | ||
| ActiveModelSerializers::SerializableResource.new(resource).as_json | ||
| end | ||
|
|
||
| def clear_cache | ||
| AmsBench::PrimaryResourceSerializer.serializers_cache.clear | ||
| AmsBench::Api::V1::PrimaryResourceSerializer.serializers_cache.clear | ||
| ActiveModel::Serializer.serializers_cache.clear | ||
| end | ||
|
|
||
| configurable = lambda do | ||
| clear_cache | ||
| Benchmark.ams('Configurable Lookup Chain', time: time, disable_gc: disable_gc, &serialization) | ||
| end | ||
|
|
||
| old = lambda do | ||
| clear_cache | ||
| module ActiveModel | ||
| class Serializer | ||
| 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 | ||
| end | ||
| end | ||
| end | ||
|
|
||
| Benchmark.ams('Old Lookup Chain (v0.10)', time: time, disable_gc: disable_gc, &serialization) | ||
| end | ||
|
|
||
| configurable.call | ||
| old.call |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,14 @@ def with_namespace_separator(separator) | |
| ActiveModelSerializers.config.jsonapi_namespace_separator = original_separator | ||
| end | ||
|
|
||
| def with_prepended_lookup(lookup_proc) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
|
||
There was a problem hiding this comment.
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:
serializer_classisActiveModel::Serializernamespacecorrespond to either the controller namespace or the one specified withnamespaceoption?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks 💯