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

Introduces per-request adapter switching based on the request mime. #2122

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ Breaking changes:

Features:

- [#2122](https://github.com/rails-api/active_model_serializers/pull/2122) Introduces per-request adapter switching based on the request mime. (@fletcher91)

Fixes:

Misc:
Expand Down
14 changes: 14 additions & 0 deletions docs/general/adapters.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,20 @@ The `Attributes` adapter does not include a root key. It is just the serialized

Use either the `JSON` or `JSON API` adapters if you want the response document to have a root key.

To automatically switch between adapters based on the request, set the adapter to `:mime`

```ruby
# In configuration
ActiveModelSerializers.config.adapter = :mime

# In controller action
respond_to do |format|
format.json { render json: Model.first } # Uses the JSON adapter
format.json_api { render json: Model.first } # Users the JSON:API adapter
Copy link
Member

Choose a reason for hiding this comment

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

👍

should be jsonapi not json_api, and render jsonapi: model rather than render json: model.

But really, this is just using the content type negotiation that respond_to already does. This would work without any changes to the PR, assuming mime -> adapter is 1:1

Copy link
Author

Choose a reason for hiding this comment

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

Would you prefer just a correction:

To automatically switch between adapters based on the request, set the adapter to :mime:

  # In configuration
  ActiveModelSerializers.config.adapter = :mime
  
  # In controller action
  respond_to do |format|
    format.json { render json: Model.first }     # Uses the JSON adapter
    format.jsonapi { render jsonapi: Model.first } # Users the JSON:API adapter
    format.bug { render json: Model.first } # Will raise `UnknownAdapterError`
  end

or to just leave it implicit:

To automatically switch between adapters based on the request, set the adapter to :mime;

ActiveModelSerializers.config.adapter = :mime

Copy link
Member

@bf4 bf4 May 4, 2017

Choose a reason for hiding this comment

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

Really, if you want to render different mime types, each mime type should be registered to its own formatter. That way, you can do the below, which is the Rails-way IMHO and requires no changes to the codebase.

 # In configuration
ActiveModelSerializers.config.adapter = :json
  
# In controller action
respond_to do |format|
    format.json { render json: model, serializer: ModelSerializer }     # Uses the JSON adapter
    format.jsonapi { render jsonapi: model, serializer: ModelSerializer } # Users the JSON:API adapter
    format.bug { render json: model, serializer: ModelSerializer } # Will raise something about bug not being a known mime type
  end

The respond_to block does the content type negotiation for you and usually we use a renderer format with the same name as the negotiated / acceptable mime types.

I think I'm making different assumptions than you, so please continue to discuss.

format.bug { render json: Model.first } # Will raise `UnknownAdapterError`
end
```

## Built in Adapters

### Attributes - Default
Expand Down
5 changes: 5 additions & 0 deletions lib/active_model_serializers/adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ def configured_adapter

def create(resource, options = {})
override = options.delete(:adapter)
context = options[:serialization_context]
format = context && context.format
if format && (override || ActiveModelSerializers.config.adapter) == :mime
override = lookup(format)
end
klass = override ? adapter_class(override) : configured_adapter
klass.new(resource, options)
end
Expand Down
4 changes: 3 additions & 1 deletion lib/active_model_serializers/serialization_context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,17 @@ def default_url_options
end
end

attr_reader :request_url, :query_parameters, :key_transform
attr_reader :request_url, :query_parameters, :key_transform, :format

def initialize(*args)
options = args.extract_options!
if args.size == 1
request = args.pop
options[:format] = request.format.to_sym
options[:request_url] = request.original_url[/\A[^?]+/]
options[:query_parameters] = request.query_parameters
end
@format = options.delete(:format)
@request_url = options.delete(:request_url)
@query_parameters = options.delete(:query_parameters)
@url_helpers = options.delete(:url_helpers) || self.class.url_helpers
Expand Down
1 change: 1 addition & 0 deletions test/adapter/json/transform_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ def mock_request(key_transform = nil)
context = Minitest::Mock.new
context.expect(:request_url, URI)
context.expect(:query_parameters, {})
context.expect(:format, Mime::Type.new(:json))
options = {}
options[:key_transform] = key_transform if key_transform
options[:serialization_context] = context
Expand Down
1 change: 1 addition & 0 deletions test/adapter/json_api/pagination_links_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ def mock_request(query_parameters = {}, original_url = URI)
context.expect(:request_url, original_url)
context.expect(:query_parameters, query_parameters)
context.expect(:key_transform, nil)
context.expect(:format, Mime::Type.new(:json_api))
end

def load_adapter(paginated_collection, mock_request = nil)
Expand Down
10 changes: 10 additions & 0 deletions test/adapter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,16 @@ def test_create_adapter_with_override
assert_equal ActiveModelSerializers::Adapter::JsonApi, adapter.class
end

def test_create_adapter_with_mime
json_context = ActiveModelSerializers::SerializationContext.new(format: :json)
adapter = ActiveModelSerializers::Adapter.create(@serializer, adapter: :mime, serialization_context: json_context)
assert_equal ActiveModelSerializers::Adapter::Json, adapter.class

json_api_context = ActiveModelSerializers::SerializationContext.new(format: :json_api)
adapter = ActiveModelSerializers::Adapter.create(@serializer, adapter: :mime, serialization_context: json_api_context)
assert_equal ActiveModelSerializers::Adapter::JsonApi, adapter.class
end

def test_inflected_adapter_class_for_known_adapter
ActiveSupport::Inflector.inflections(:en) { |inflect| inflect.acronym 'API' }
klass = ActiveModelSerializers::Adapter.adapter_class(:json_api)
Expand Down