should ActiveRecord::AssociationProxy#reload allow options?

Hi All,

Recently, in the AR AsscociationProxy class, my colleague and I came across a case that seems to be a violation of the principle of least surprise.

The problem shows itself when trying to take out an elevated lock on an ActiveRecord#Base object: 'ar_obj.reload(:lock => true)'. In general, this works fine. However, if the ar_obj object was not created directly, but instead came from an association proxy off some other AR#base object, then we have a problem. Since AssociationProxy defines its own (no-arg) version of #reload we hit a "wrong number of arguments error" because the object we're calling the method on is of type AssociationProxy, not AR:Base. Thank the stars that the AP#reload method doesn't take an options hash, if that were the case it could be possible to go forward thinking the object was having its lock elevated as expected. With the problem understood it was easy enough to work around by checking if the object had a #target method and using that target object directly if it did.

So, long story short, the implementation of AssociationProxy exposes itself when using the #reload(:lock => true) technique to elevate a lock. My question to the core is whether or not this is an acceptable side effect of the implementation? In particular, I am curious if using this #reload method of lock escalation is a supported mode of operation, or is there some other means to the same end that is better supported by ActiveRecord? For instance, I suppose I could do this (hopefully the formatting on this doesn't get bojacked): <pre> def do_stuff_with_lock(ar_or_ap_object)   transaction do     ar_or_ap_object.class.find(ar_or_ap_object.id, :lock => true)     # do stuff with ar_or_ap_object     ar_or_ap_object.save   end end </pre>

Cheers, Andrew

So, long story short, the implementation of AssociationProxy exposes itself when using the #reload(:lock => true) technique to elevate a lock. My question to the core is whether or not this is an acceptable side effect of the implementation? In particular, I am curious if using this #reload method of lock escalation is a supported mode of operation, or is there some other means to the same end that is better supported by ActiveRecord? For instance, I suppose I could do this (hopefully the formatting on this doesn't get bojacked):

Nice find, that's definitely not an acceptable side-effect of the implementation. A patch to address this would be great.

In the meantime you can use @record.whatever.lock! which will behave as expected.

Sweet, Koz, thanks a lot.

Looking at the AssociationProxy implementation I don't quite know how I'd go about refactoring that #reload method to work like AR::Base#reload. To be honest, I hadn't noticed the lock! method before and now that you've pointed it out I'll probably just stick to it. lock! is more intention revealing anyways so I'm happy to use it in favor of reload and ignore the reload problem for the time being :slight_smile:

Cheers, Andrew