This project is archived and is in readonly mode.
caching bugs in ActiveRecord#create and associations
Reported by nickretallack | September 24th, 2008 @ 11:54 PM | in 3.x
When find_or_create
creates a record, it does not
clear the cache like create
does, causing some
interesting anomalies.
Here is a quick example: http://pastie.org/278991
Enclosed is a rails app with a single test that confirms the problem.
Comments and changes to this ticket
-
nickretallack December 4th, 2008 @ 10:26 PM
- Tag changed from database-caching, find_or_create to create, database-caching, find_or_create
- Title changed from find_or_create causes caching anomalies to caching bugs in ActiveRecord#create and associations
Looks like this isn't just a problem with find_or_create. I can cause a similar problem with regular create. It only takes a little bit of indirection on the very same association to screw things up.
Patch added contains tests that verify the problem
-
RainChen September 10th, 2009 @ 10:00 AM
- Tag changed from create, database-caching, find_or_create to counter_cache, create, database-caching, find_or_create
I still have this issue today (Rails version 2.3.2).
The test code will explain what's wrong.
class Page < ActiveRecord::Base acts_as_tree :counter_cache => :children_count end
Test:
require File.dirname(FILE) + '/../test_helper'
class PageTest < ActiveSupport::TestCase context "pages" do
should "have a counter cache" do Page.root.children_count.should == 0 Page.root.children.create(:title => 'about') Page.root.children.count.should == 1 Page.root.children_count.should == 1 Page.root.children.find_or_create_by_title('contact') Page.root.children.count.should == 2 Page.root.children_count.should == 2 # false end
end end
-
Neeraj Singh June 28th, 2010 @ 06:33 PM
- Importance changed from to
@nickretallack Do you want to rebase your test against rails3 edge?
-
Neeraj Singh June 28th, 2010 @ 10:29 PM
- State changed from new to open
Just recapping.
class Car has_many :brakes end c = Car.first c.brakes # => 0 . so that all brakes are loaded c.brakes.find_or_create(:name => 'super_brake') # new brake is created c.brakes.length #=> 0 c.brakes.count #=> 1
when c.brakes.find_or_create is invoked then method_missing is called and method_missing simply does
@reflection.klass.send(method, *args) { |*block_args| yield(*block_args) }
In contrast when car.brakes.create(:name => 'super_brake') is called then add_record_to_target_with_callbacks is called
def add_record_to_target_with_callbacks(record) callback(:before_add, record) yield(record) if block_given? @target ||= [] unless loaded? @target << record unless @reflection.options[:uniq] && @target.include?(record) callback(:after_add, record) set_inverse_instance(record, @owner) record end
So not only cache is not getting refreshed but inverse_instance will also not be set.
-
Aaron Patterson June 29th, 2010 @ 11:43 PM
- Tag changed from counter_cache, create, database-caching, find_or_create to counter_cache, create, database-caching, find_or_create, patch
Added a patch with fixes and tests.
-
Repository June 30th, 2010 @ 12:55 AM
- State changed from open to resolved
(from [fad166c15277c72b370c90e890d509d0f6c9af63]) AssociationCollection#create_by*, find_or_create_by* work properly now. [#1108 state:resolved]
Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/fad166c15277c72b370c90e890d509... -
Repository June 30th, 2010 @ 12:55 AM
(from [3f563f169612ec340816f0a549fe9db7ff95c238]) AssociationCollection#create_by*, find_or_create_by* work properly now. [#1108 state:resolved]
Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/3f563f169612ec340816f0a549fe9d... -
Repository July 14th, 2010 @ 06:33 PM
(from [d5921cdb7a00b393d0ba5f2f795da60a33b11cf1]) Remove unintentional API changes. [#1108] http://github.com/rails/rails/commit/d5921cdb7a00b393d0ba5f2f795da6...
-
toby cabot September 28th, 2010 @ 02:47 PM
Commit fad166c15277c72b370c90e890d509d0f6c9af63 (i.e. 2.3.9) broke find_or_create_by_x with additional parameters for the create, when called through an association. Since http://dev.rubyonrails.org/ticket/7368 you've been able to pass additional parameters that are ignored by the find but used by the create. fad166c broke that behavior.
I'll attach a test case that demonstrates the problem, but in a nutshell:
# 2.3.8: success # 2.3.9: ArgumentError: Unknown key(s): type, body # ./test/cases/../../../activesupport/lib/active_support/core_ext/hash/keys.rb:47:in `assert_valid_keys' post.comments.find_or_create_by_body(:body => 'other test comment body', :type => 'test')
-
toby cabot September 28th, 2010 @ 09:17 PM
fad166c15277c72b370c90e890d509d0f6c9af63 also broke find_or_create_by_x with a block provided. It used to yield to the block but it doesn't anymore. I'll attach another test case that works with 2.3.8 but fails with 2.3.9. For example:
comment = post.comments.find_or_create_by_body('other test comment body') { |comment| comment.type = 'test' }
-
toby cabot September 29th, 2010 @ 09:31 PM
Here's a 2-part patch that fixes both problems. The gist of the first part of the patch is to filter parameters provided to a find_or_create_by_x method call so that we only pass the ones to find() that find() cares about. The second part adds code to yield to a provided block.
-
toby cabot September 30th, 2010 @ 01:32 PM
Looks like I can't re-open this bug so I've opened #5737 to track the resolution of these bugs.
-
David Trasbo October 10th, 2010 @ 04:54 PM
- State changed from resolved to open
-
Aaron Patterson October 10th, 2010 @ 11:56 PM
- Assigned user set to Aaron Patterson
-
Aaron Patterson October 21st, 2010 @ 01:25 AM
- State changed from open to resolved
I've applied the patch to the 2-3 branch. Thanks!
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Attachments
Referenced by
- 1108 caching bugs in ActiveRecord#create and associations (from [fad166c15277c72b370c90e890d509d0f6c9af63]) Associa...
- 1108 caching bugs in ActiveRecord#create and associations (from [3f563f169612ec340816f0a549fe9db7ff95c238]) Associa...
- 1108 caching bugs in ActiveRecord#create and associations (from [d5921cdb7a00b393d0ba5f2f795da60a33b11cf1]) Remove ...
- 6147 find_or_create via has_many fails for hash parameters I tried to use your test Brandon, but I was unable to pro...
- 5737 find_or_create_by_x with create params via association broke in 2.3.9 Sorry about opening this bug - I'd prefer to re-open #110...
- 5737 find_or_create_by_x with create params via association broke in 2.3.9 The fix to #1108 added two new bugs.
- 5737 find_or_create_by_x with create params via association broke in 2.3.9 For more info please see my comments to #1108.
- 5737 find_or_create_by_x with create params via association broke in 2.3.9 I've reopened #1108. Please continue the discussion there.
- 5737 find_or_create_by_x with create params via association broke in 2.3.9 Aaron Patterson committed the fix to #1108 so this ticket...