rabl-rails assumes any object that respond to `each` is a collection

[jrunning]

I'd like to use rabl-rails to render an object, but since the object responds to each, RablRails::Renderers::Base tries to render it as a collection. Here's code that reproduces the problem:

# app/views/controllers/config_controller.rb
class ConfigController < ApplicationController
  respond_to :json

  def show
    struct = Struct.new(:foo)
    @config = struct.new("bar")
  end
end

# app/views/config/show.json.rabl
object :@config
attribute :foo

I would expect this to render {"foo":"bar"} since @config.foo returns "bar". However, because @config responds to each, Renderers::Base#render calls render_collection on it instead of render_resource, yielding an error:

undefined method `foo' for "bar":String
  (gem) rabl-rails-0.3.3/lib/rabl-rails/renderers/base.rb:68:in `block in render_resource'
  (gem) rabl-rails-0.3.3/lib/rabl-rails/renderers/base.rb:64:in `each'
  (gem) rabl-rails-0.3.3/lib/rabl-rails/renderers/base.rb:64:in `inject'
  (gem) rabl-rails-0.3.3/lib/rabl-rails/renderers/base.rb:64:in `render_resource'
  (gem) rabl-rails-0.3.3/lib/rabl-rails/renderers/base.rb:116:in `block in render_collection'
  (gem) rabl-rails-0.3.3/lib/rabl-rails/renderers/base.rb:116:in `each'
  (gem) rabl-rails-0.3.3/lib/rabl-rails/renderers/base.rb:116:in `map'
  (gem) rabl-rails-0.3.3/lib/rabl-rails/renderers/base.rb:116:in `render_collection'
  (gem) rabl-rails-0.3.3/lib/rabl-rails/renderers/base.rb:32:in `block in render'
  (gem) rabl-rails-0.3.3/lib/rabl-rails/renderers/base.rb:55:in `render_with_cache'
  (gem) rabl-rails-0.3.3/lib/rabl-rails/renderers/base.rb:31:in `render'
  ...

(You can see the complete backtrace here: https://gist.github.com/jrunning/9279963)

This might seem like a contrived example, but I ran into it in a real scenario (I was trying to render attributes of an ActiveSupport::OrderedOptions rather than a struct). The same issue would manifest if someone defined each on an ActiveRecord model.

I'm not sure what the best solution is. Perhaps object could take an option that would explicitly tell it not to render the object as a collection?

For now my only option seems to be falling back to the regular rabl gem, which lacks some of the Railsy niceties of rabl-rails.

[ccocchi]

FIxed in 781ec8d96cc1ccd8af71da4e3bd6fbf34dcaca80.

[jrunning]

The referenced commit doesn't actually fix the issue, unfortunately. The fix is specific to Structs but the issue is not. As I wrote above, I discovered it when trying to render an attribute of an ActiveSupport::OrderedOptions. The issue still manifests with any object that responds to each and isn't a collection or a Struct, including if someone were to define an ActiveRecord model that responds to each (which I have seen in the wild).

[ccocchi]

What about this https://github.com/ccocchi/rabl-rails/commit/b2eb5beb40f69f776b687dc84224792a300e233f ? You can now define your own objects that respond_to each but should be treated as objects.

[jrunning]

Thanks for taking the time to reopen this, @ccocchi. I think this solution is better, with one reservation. I wonder if specifying class names instead of actual classes (i.e. ["Struct", "NotACollection"] instead of [Struct, NotACollection]) would be better, especially when it comes to autoloading. For example, if you have a Rails model named MyModel, if you do this in your configuration:

RablRails.configure do |config|
  config.non_collection_classes << MyModel
end

...Rails will autoload MyModel as soon as that block is called.

If instead you could do this:

RablRails.configure do |config|
  config.non_collection_classes << "MyModel"
end

...and then in lib/rabl-rails/helpers.rb:

resource && resource.respond_to?(:each) &&
  RablRails.configuration.non_collection_classes.none? { |k| klass <= k.constantize }

...then MyModel will only be loaded when absolutely necessary.

While a typical Rails app will load all of its models eventually, often Rake tasks or workers only need one or two of the app's models, and I think those tend to benefit from optimizations like this.

[ccocchi]

Using strings results in a big performance drawback, see this https://gist.github.com/ccocchi/b74045f5e3c15db44dce

Calculating -------------------------------------
               klass     68989 i/100ms
               const     20790 i/100ms
             strings     20112 i/100ms
-------------------------------------------------
               klass  1685134.9 (±5.6%) i/s -    8416658 in   5.009936s
               const   318492.8 (±5.5%) i/s -    1600830 in   5.042154s
             strings   269006.3 (±7.8%) i/s -    1347504 in   5.040542s

I guess having one or two models autoloaded at boot time is a lesser matter. This method is at least called once per object rendered so kind of a hot path. Workers and Rails would eventually load all models so we should be good. Maybe some rake tasks will load more models with this but I think rendering views in a task is a marginal case.

I will test this in a live application to see performance results and I'll release a new version soon.

[bsedin]

Got problem with non_collection_classes in development environment. When i edit my class, rails removes class constant and initializes new fresh one, which not equal one i've put in initializer to RablRails.configuration.non_collection_classes

[ccocchi]

Ok ok, let's go with strings :disappointed: This commit 3a05ec4e81a87d9996aa163a108b45783d7ea7cd should fix your reload problem

[jrunning]

If performance is an issue for some people, we could have our cake and eat it too by using the string technique in development and the class technique in production. Or you could hook into Rails' reloader to make sure rabl-rails' configuration is updated when classes are reloaded. But that still leaves the autoloading issue. I suppose the third way would be to allow either classes or strings, which would let users choose between the autoloading penalty and the constantize penatly.

[ccocchi]

From v0.4.1, classes that should not be treated as collection are configurable.

[pkuykendall]

I found good results with changing the collection? helper to include a class check for Enumerable

resource && resource.respond_to?(:each) && resource.kind_of?(Enumerable) &&
        klass.ancestors.none? { |a| RablRails.configuration.non_collection_classes.include? a.name }

What are your thoughts, @ccocchi?

[myxrome]

it is not compatible with sequel-rails. the code partial "path/to/partial", object: object will fail on https://github.com/ccocchi/rabl-rails/blob/master/lib/rabl-rails/visitors/to_hash.rb#L127 because of sequel model does respond to each method. and it does not respond to map method. could you please fix it.

[ccocchi]

Hello,

I don't understand what your problem is. If a sequel model (I'm assuming it is an equivalent of ActiveRecord::Base) does not respond to the each method, it won't be treated as a collection but as an object, which is the expected behavior.

Where am I mistaken?