Allow users to globally opt out of automatic lookup

[trek]

Adds an option allowing existing applications to opt out of automatic Serialization lookup.

Two use cases where I've already needed this:

• Existing applications where another serialization system is stomping on the .*Serializer suffix and you need to migrate over to AMS piecemeal
• Existing applications that have existing public API where they can't change JSON responses and can't change the existing response structure. E.g. if you a Tweet model and want to add a TweetSerializer for controllers under the v2 namespace but not change existing responses for v1 that are created with a bare render json: a_tweet

If this looks good, I'll get a test in.

Closes #1293

[NullVoxPopuli]

@trek for the second example, you could specify a serializer under v1? Maybe I miss the point.

also, for the first point, I think I would be more in favor of configuring teh suffix, rather than opt out of the lookup. The lookup is very powerful. :-)

[bf4]

@trek (hi!)

actually, you can pass adapter: false in your render (or options[:adapter] = false if you're outside a controller) and it won't look up a serializer or use an adapter.

ref:

• active_model_serializers/lib/active_model/serializable_resource.rb

  Lines 54 to 58 in f3403c3

  	# True when no explicit adapter given, or explicit appear is truthy (non-nil)
  	# False when explicit adapter is falsy (nil or false)
  	def use_adapter?
  	!(adapter_opts.key?(:adapter) && !adapter_opts[:adapter])
  	end

• active_model_serializers/lib/action_controller/serialization.rb

  Lines 23 to 40 in f3403c3

  	if !use_adapter?
  	warn 'ActionController::Serialization#use_adapter? has been removed. '\
  	"Please pass 'adapter: false' or see ActiveSupport::SerializableResource.new"
  	options[:adapter] = false
  	end
  	serializable_resource = ActiveModel::SerializableResource.new(resource, options)
  	if serializable_resource.serializer?
  	serializable_resource.serialization_scope ||= serialization_scope
  	serializable_resource.serialization_scope_name = _serialization_scope
  	begin
  	serializable_resource.adapter
  	rescue ActiveModel::Serializer::CollectionSerializer::NoSerializerError
  	resource
  	end
  	else
  	resource
  	end
  	end

That should work for you, and would still be useful to include in this PR, to call that out :)

It should also be mentioned in https://github.com/rails-api/active_model_serializers/blob/master/docs/ARCHITECTURE.md, I think

[bf4]

If that's not sufficient, that's the area of the public API that should be affected.. would love to see a code example of what you're trying to do

[NullVoxPopuli]

👍

[bf4]

So, #1294 was merged, does that address this issue or should we revert it and continue here?

[NullVoxPopuli]

It's unclear if that PR addresses the issue here.
My feeling is that it may be a usage issue (the reason for this PR), but we need more information.

[trek]

@bf4 (hey!)

#1294 addresses 1 of the 2 problem cases. I think the root problem is that automatic lookup is too aggressive of a strategy for larger apps that want to introduce AMS only for new versions of an existing API. It'd be nice – and frankly safer – to opt out globally and opt-in locally in these cases.

Another riff on this PR is to make that an explicit flag on render calls:

render json: photos, use_serializer: true

But I already personally prefer supplying the specific serializer to the render call. Yes, it's repetitive, but removes the "where the heck is this object turning into this JSON?!?" problem.

[bf4]

Right now AMS automatically includes the ActionController::Serialization mixin. What if you could opt out of that and mix it in to controllers as desired. Would that work? (As opposed to passing in adapter: false in most of your controllers)

[NullVoxPopuli]

@bf4 are you think that could be an initialization configuration option? something like:

include_action_controller_serialization = true (by default)

?

[bf4]

@trek re:

render json: photos, use_serializer: true

In rails/rails#21496 I have proposed removing use_renderers since it appears never to have been used or tested. Since your suggestion here has a similar name, I was wondering if it might be related to the use_renderers method in the Rails Renderer, or just coincidental

[trek]

for the second example, you could specify a serializer under v1? Maybe I miss the point.

also, for the first point, I think I would be more in favor of configuring teh suffix, rather than opt out of the lookup. The lookup is very powerful. :-)

v1 in this example (actually several versions in reality) does not use AMS at all nor do we want to through and ensure every every controller specifically opts out. The automatic lookup being always on is very nice for greenfield projects or projects that can go back and update all their calls to render, but makes it painful to adopt AMS into an existing project only for the next API version, leaving the response bodies and code that generates them untouched.

[NullVoxPopuli]

I wonder if it would be beneficial to override the lookup hierarchy?

like, if you have 4 versions of an api, maybe you want to fallback to previous versions, so you don't need to specify all the serializers fro each version?

[bf4]

I suppose you could also make an argument to bring back use_adapter?

[bf4]

(or add ActiveModel::Serializer.config.automatic_lookup as config.implicit_serializers = false which would used probably in the serializable resource

[bf4]

What do you think of setting config.implicit_serializers = false ?

[NullVoxPopuli]

I could get behind that, as long as it's documented an well

On Tue, Nov 10, 2015, 4:55 AM Benjamin Fleischer notifications@github.com
wrote:

What do you think of setting config.implicit_serializers = false ?

—
Reply to this email directly or view it on GitHub
#1295 (comment)
.

[trek]

Sorry, I don't quite understand the feedback. Are we just bikeshedding the config name or am I missing something?

[bf4]

@trek I don't think so.

I did bring up different names, but implementation-wise, I think the switch should be somewhere besides ActionController::Serialization#get_serializer.

I'm thinking there are a few good places to change, not exclusive

• Turn off implicit lookup, so that resources are passed through unchanged when no serializer is specified. Something like ActiveModel::Serializer.config.implicit_serializers = false (affects: call to ActiveModel::Serializer.serializer_for in SerializableResource)
• Pass through resource unchanged by setting default adapter to config.adapter = false. Using AMS will then require an explicit adapter option to render in any controller. (impact: SerializableResource#use_adapter?
• Prevent automatic mixing in of ActionController::Serialization into ActionController::Base with some config, maybe config.action_controller.include = false (impact: the railtie, 2)

Thoughts?

[bf4]

Interestingly, this has come up before: #592 (and #611 )

[trek]

Closing in favor of #1353