rails: :dependent => :destroy may delete the wrong objects.
Whenever you define two classes like in the following example …
class User < ActiveRecord::Base
has_many :things, :dependent => :destroy
end
class Thing < ActiveRecord::Base
belongs_to :user
end
… there is a possibility to get the wrong objects destroyed if you do something like this …
# We need two different users.
User.all.size > 1 # => true
user_a = User.first
user_b = User.last
# Both users should have some things.
not (user_a.things.empty? or user_b.things.empty?) # => true
# Now for the actual problem:
problematic_thing = user_a.things.last
problematic_thing.update_attributes!(:user => user_b)
user_b.things.reload.include?(problematic_thing) # => true
user_a.destroy
# The problematic_thing will be destroyed.
user_b.things.reload.include?(problematic_thing) # => false
So by destroying user_a we destroyed one of user_b’s things. That doesn’t seem fair.
One possible solution to this problem would be to add an additional reload to activerecord/lib/active_record/transactions.rb:236 in 'destroy'. However I’m not sure this is the most elegant way to do this.
If I find the time, I’ll write a test and a patch for the fix. I’d create a pull request of it.
About this issue
- Original URL
- State: closed
- Created 13 years ago
- Comments: 16 (9 by maintainers)
Hello guys! I just spent 5-6 hours battling weird issues and realized it was this one… Please don’t close it - right now I’m making my code very un-dry just to work around it… and I shudder at the thought of how many weird bugs I have in my codebase that I don’t know of…
I have no idea how this works under the hood, but seeing that it’s some association cache or model cache that causes the problem – it should be fixable by introducing another flag in the cache.
Here are my suggestions for fixes: When problematic_thing gets it’s association updated, maybe you could either: a) flag the cache for problematic_thing to with a “reload yourself if you wanna :dependent => :destroy me!”-flag … then if any other record tries to destroy it, it will reload itself when needed b) flag the old association as needing reload before deletion (just in the cache), so it no longer
Or: If you can’t hook something up to the association change in a good way, perhaps we can use, or introduce, timestamps for the cache. I.e, when destroying user_a, user_a is in the cache. Most probable, problematic_thing will also be in the cache in cases where this is a problem. If the problematic_thing cache is younger than user_a cache, it is very probable that it can be a problem to :dependent_destroy user a. (because the most probable thing you’ve done to problematic_thing is to change it’s association away from user_a).