Decorated associations don't decorate dynamic scopes #389

Closed
tovodeverett opened this Issue Dec 14, 2012 · 2 comments

Comments

Projects
None yet
3 participants
Contributor

tovodeverett commented Dec 14, 2012

It looks like this got hinted at in #235.

Here's another interesting symptom of the same underlying issue. If you dynamically scope a decorated association, you don't get decorated objects. Associations accept methods like limit, order, and where. These behave intelligently (i.e. they only fetch what they need to from the database). Calling these works with a decorated association because they are sent to the association via method_missing, but the results aren't decorated. In a nutshell, if the method is an Array method, it goes straight to the association and doesn't get decorated. If it is an Array method, then the collection gets enumerated (completely) and then the method gets sent to the Array.

The only real solutions I see to this are:

  • Tons of user education and warning
  • Giving up on decorated associations
  • Sending everything through dynamic dispatch and inspecting the returned results for things to decorate, then maintaining a decorated items cache and dynamically decorating them.

Some more notes - you can't inspect an association and expect to learn anything useful. ActiveRecord::Associations::CollectionProxy is seriously twisted - it redirects everything it doesn't handle itself to the underlying Array, and so methods like method and methods get sent to the Array. But it does delegate the following to scoped: group, order, limit, joins, where, preload, eager_load, includes, from, lock, readonly, having, pluck.

Just some more food for thought.

Contributor

tovodeverett commented Dec 14, 2012

The more I think about this, the more I think we have two main options.

The first option is to stop doing method missing on decorated associations. The first time someone accesses the decorated association, it populates the cache array and then everything is delegated to that Array. If the array doesn't handle it, neither do we. This would prevent calling things like reload, build, limit, etc., but since those things don't really work properly anyway, this just prevents users from attempting things that will bite them. If they want to call those, they need to do decorator.source.association.whatever to make it explicit that things won't be decorated and won't affect the decorated association.

The second option is dynamic decoration. While this gives up the benefits of caching, it also delays incurring the cost of decorating until it is unavoidable. In this approach, everything would be dynamically dispatched to the underlying association and the return value would be decorated. If the return value is an Array, we would decorate it with its own CollectionDecorator. This is necessary to handle things like decorator.decorated_association.where(foo: 'bar').limit(3), but it also has the benefit of further delaying decoration. If the return value isn't an Array, we would test to see if it responds to decorate and then decorate it. We'd have to think about how to handle :with, but I have some ideas there. The other thing we would have to do is to proxy blocks. Any block passed in the arguments to the dynamically dispatched method would need to be wrapped in a decorator lambda that would automagically decorate arguments passed to the block. I'm still debating whether it needs to decorate return values from blocks, or whether we can simply rely on the dynamic dispatch method to handle the decoration of the returned value.

The advantages to the first option:

  • Simpler to implement
  • Easier to test
  • Less likely to get it wrong or screw up a corner case
  • Better performance for usage models that access the entire collection

The advantage to the second option:

  • Users retain access to all the power of ActiveRecord associations/collections
Owner

steveklabnik commented Dec 17, 2012

I vote #1.

@haines haines closed this in 4960d49 Dec 27, 2012

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