502144 - model associations binding wtf

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Public Pages, defect)

Description

[Jeff Balogh (:jbalogh)]

attachment 386653 [details] [diff] [review] on bug 501187 is triggering a weird model association error where bindOnly fails intermittently.  When you unbindModel, cake stores that "deleted" association in $this->__backAssociation, and resurrects it with __resetAssociations().  bindOnly calls __reset to make sure everything is there before it does its work.  When we hit the error, __backAssociation is empty but Addon->hasAndBelongsToMany only contains the Tag association.  User, Category, and Collection are getting lost somewhere along the way, so we'll never be able to bind those again.

The Tag association is new, so I'm suspecting some webmocha foul play.

            echo '<pre>';print_r($this->Addon->__backAssociation);echo '</pre><hr>';
            echo '<pre>';print_r($this->Addon->hasAndBelongsToMany);echo '</pre><hr>';
            $this->Addon->__resetAssociations();
            echo '<pre>';print_r($this->Addon->hasAndBelongsToMany);echo '</pre><hr>';
            echo '<pre>';print_r($this->Addon->__backAssociation);echo '</pre><hr>';

shows you what's going on, and I put that before bindOnly('User') in addons_controller:display

Comment 1

[Wil Clouser [:clouserw]]

This happens about 50% of the time I think, but I've had it work fine for 10 or
so reloads before it breaks too. :(

Comment 2

[Jeff Balogh (:jbalogh)]

More print debugging (which is completely awesome, btw) shows that we have errors when getAddon *misses* the cache.

Comment 3

[Jeff Balogh (:jbalogh)]

Attachment: resetting model associations the right way

This fixes what appears to me to be a cake bug.  The functions we're looking at are __resetAssociations, bindModel and unbindModel in site/cake/libs/model/model_php5.php.

We'll use $this->hasMany as an example association, but the following applies to all of them.  

When you change unbind something in $this->hasMany, cake stores a copy of $this->hasMany in __backAssociation['hasMany'].  Good idea, now we can undo the unbinding.  But then let's say you call bindModel again, with something in hasMany.  This is a common pattern in amo, unbindFully and then rebind specific parts.

*BUT* when you bindModel, cake stores a copy of $this->hasMany in __backAssociation['hasMany'].  Yes, overwriting the original full hasMany with this pared-down version.  Then we do a query and cake calls __resetAssociations to put everything back, but we only remember the meager version of our associations.  Doh!

This patch stores all the associations at startup and never messes with that copy again.  When we want to reset we look at the pristine copy we stored and reset from there.

Comment 4

[Jeff Balogh (:jbalogh)]

Attachment: v2

Turns out the associations aren't all set up in the beginning, so we need to grab our copy after initialization.

Comment 5

[Wil Clouser [:clouserw]]

Comment on attachment 386687 [details] [diff] [review]
v2

triple secret underscores!

Comment 6

[Jeff Balogh (:jbalogh)]

r29329