Skip to content

Conversation

benhamill
Copy link

If I register a new MIME type with Rails (say, application/hal+json) and I beat on my serializers so that they'll return valid HAL JSON... is there a way to make ActiveModel::Serializers report that new MIME type in the Content-Type header?

At first, I was hoping that something like this in the controller would work:

render hal: @posts, serializer: PostsSerializer

But then Rails::API goes looking for a template, and ignores my serializer option.

If this isn't something that's currently possible, is this something folks would be interested? If people are, I could see about taking a crack.

@steveklabnik
Copy link
Contributor

Right, we override render :json specifically.

I'd like a PR for this. We will need to support application/vnd.api+json, after all...

@benhamill
Copy link
Author

Would this be the spot where that override happens? I don't know a lot about how this part of Rails all fits together, but I'll see about going on a fishing expedition.

@benhamill
Copy link
Author

OK. I am confused. That method seems to be the one that does the intercepting... but I have no idea how. Looking around in the Rails code, I couldn't figure out if that method was defined... maybe dynamically? Or if AMS was getting Rails to call it in some way... Confused.

@benhamill
Copy link
Author

Sorry if I'm spamming inboxes, here. I'll try to stop commenting so often and say more at once.

Here's what I've figured out and I'm writing it down before I take a break:

  • Rails's render ends up calling _render_option_whatever when you tell it render :whatever.
  • Those _render_option_* methods get dynamically defined here in ActionController::Renderers.add.
  • I'm not quite sure of the mechanism, but after reading the comments before AC::Renderers.add, I think that when you define a new mime type with Mime::Type.register, that ends up adding the new renderer for it (possibly here?).

What I'm concerned about is that Rails will load all it's shit, and AM::Serializers will get loaded as part of this, triggering this line, which currently overrides the default json renderer... and then the application's initializers will get loaded that define custom mime types.

This means that if we, say, just added hal support by overriding _render_option_hal like we do with json, it would get overridden by the dynamically defined _render_option_hal method when the mime type is registered.

This implies to me that we might need to either require that users register custom mime types that they want AM::Serializers to intercept with the gem after they've declared the mime type, or that we'll have to shoehorn our way into Mime::Type.register and add some option or something for people to be able to say 'and serialize this, please'.

Neither makes me super happy. I'm going to continue to experiment within the test suite, but would love some guidance.

@benhamill
Copy link
Author

Yeah, I'm clearly missing something. Even with some serious hackery, and getting a _render_option_hal into scope during my test action, it doesn't get called when I call render :hal. Halp?

@benhamill
Copy link
Author

I got some stuff sorted out. Check this compare out: https://github.com/benhamill/active_model_serializers/compare/playing_with_mime_types

I'm not sure how to leverage this understanding... Maybe we have a ActiveModel::Serializers.register_mime_type method that takes a Symbol or a Mime::Type and adds the appropriate renderer as in the top of serialization_test.rb there? Not sure. Any thoughts?

(Side note: If my HAL-based PR gets merged, I think it would make sense to go ahead and register that mime type and add the renderer for it, but that's probably another deal).

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) when pulling c0d04b6 on benhamill:playing_with_mime_types into 919bb38 on rails-api:master.

@benhamill
Copy link
Author

I decided to code up the first idea I had just to see what it would look like and I decided it didn't seem bad, so I wrote it up. Let me know if I should do something else in terms of style or documentation or... whatever. You know... like how Pull Requests normally work. 😓

@ebastien
Copy link

On my side I am changing the Content-Type thanks to Rails built-in content negotiation:

  • config/initializers/mime_types.rb
Mime::Type.register "application/vnd.api+json", :jsonapi
  • app/controllers/my_models_controller.rb
respond_to do |format|
  format.jsonapi { render json: @my_model }
end

I am not sure to understand why we need additional support from AMS ?

@benhamill
Copy link
Author

By default, Rails::API removes the middlewares that give you respond_to and respond_with. And I was hoping to use render jsonapi: @my_model, so that the key you pass to render reflects the specific content type, rather than the general, uh... super-content type or whatever.

Does that make sense?

@ebastien
Copy link

That makes sense, but still, is AMS meant to serialize anything else than JSON ?
If the super-content-type is always JSON, writing something like:

ActiveModel::Serializer.register_mime_type(Mime::XML)

does not make any sense and has to be caught.
I am using Rails::API with AMS and I am including ActionController::MimeResponds in my ApplicationController to benefit from the Rails helpers. I am not saying that we should not seek better support from AMS itself, but the Rails support for custom Content-Type is already pretty good and usable with Rails::API + AMS.

@benhamill
Copy link
Author

Maybe that's an argument for AMS not to remove MimeResponds, then... Either way, I'd like an "official" way to do this. @steveklabnik opinions?

@steveklabnik
Copy link
Contributor

I want this functionality, but we should build it into 0.10. Let's leave the existing behavior as-is for now.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants