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

Add option to select adapter by the request header #1082

Closed
wants to merge 3 commits into from

Conversation

akaKuruma
Copy link

It resolves #1062

akaKuruma and others added 3 commits August 25, 2015 04:32
Set adapter only if there isn't set yet
By default, AMS responds to media type application/json and
application/vnd.api+json using the FlattenJson and JsonApi adapters
respectively. If you want to add a different media type, override the
defaults or even create your own, you can do it in your initializer.
@bacarini
Copy link
Contributor

@joaomdmoura, I've worked on #1062.
We also added some explanation about this feature into README.

We will add more tests if you decide to move on.

@akaKuruma akaKuruma changed the title [WIP]Add option to select adapter by the request header Add option to select adapter by the request header Aug 26, 2015
if ActiveModel::Serializer.enabled_adapters_by_media_type && !options.fetch(:adapter, nil)
adapter = ActiveModel::Serializer::Adapter.by_request(request)
options[:adapter] ||= adapter if adapter
end
Copy link
Member

Choose a reason for hiding this comment

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

Would be really great to merge #1017 first to take advantage of the register/get interface cc @joaomdmoura :)

Copy link
Member

Choose a reason for hiding this comment

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

I'll check #1017

@joaomdmoura
Copy link
Member

I made some comments, you might also need to rebase it. I'll check #1017 in order to see if we can get it merged and you can take advantage of it on your PR as well.
It's a nice work, thank you! Keep it up! 😄

@@ -7,6 +7,8 @@ module Configuration
included do |base|
base.config.array_serializer = ActiveModel::Serializer::ArraySerializer
base.config.adapter = :flatten_json
base.config.enabled_adapters_by_media_type = true
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be false by default, as the defined serializers may not play well with all possible adapters.

@bf4
Copy link
Member

bf4 commented Dec 23, 2015

@akaKuruma are you still interested in this PR? A lot has changed in master, but, I think it actually makes this PR simpler. Let me know if I can help.

@akaKuruma
Copy link
Author

@bf4 yes, I'll check what we need to change, thanks

@bf4
Copy link
Member

bf4 commented Dec 23, 2015

Yay!

B mobile phone

On Dec 23, 2015, at 5:32 AM, Tiago Freire [email protected] wrote:

@bf4 yes, I'll check what we need to change, thanks


Reply to this email directly or view it on GitHub.

@landlessness
Copy link

+1 on this feature.

I'm working on writing an AMS adapter for the Siren hypermedia type. And then I plan to create a second adapter for a flavor of Siren for connected devices called Zetta. I'd like to be able to swap among the adapters for ease of dev and test and to provide API clients more flexibility in production. Right now my controllers are not very DRY. My world looks like this today and will look much better after this PR.

Thanks for the great work.

Photocells Controller

# app/controllers/photocells_controller.rb
def show
  respond_to do |format|
    format.html # index.html.erb
    format.json { render json: @photocell, adapter: :json}
    format.json_api { render json: @photocell, adapter: :json_api}
    format.siren { render json: @photocell, adapter: :siren}
    format.zetta { render json: @photocell, adapter: :zetta}
  end
end

Mime Type Initializer

# config/initializers/mime_types.rb
Mime::Type.register 'application/vnd.api+json', :json_api
Mime::Type.register 'application/vnd.siren+json', :siren
Mime::Type.register 'application/vnd.siren.zetta+json', :zetta

@bf4
Copy link
Member

bf4 commented Jan 6, 2016

I wanted to reference related work I'm proposing to Rails rails/rails#21496 (comment)

Impl on master here in ams should use serialization_context

@remear
Copy link
Member

remear commented Mar 17, 2016

I think we're still open to this feature but this is out of date. If anyone wants to open a new PR for this, the implementation should use SerializationContext.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Creating adapters by media-types.
7 participants