Rails Ruby on Rails → 3.x

Comments and changes to this ticket

• You flagged this item as spam.
  

  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

  • 0001-tests-for-query-cache-bugs.patch 1.9 KB

• You flagged this item as spam.
  

  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
  
  
  
  
  

• You flagged this item as spam.
  

  nickretallack April 30th, 2010 @ 09:06 AM

  This bug still exists. Will it ever be fixed?

• 

  Jeremy Kemper May 4th, 2010 @ 06:48 PM

  • Milestone changed from “2.x” to “3.x”

  [bulk edit]

• 

  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.

  • your-patch-file.diff 3.9 KB
• 

  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...

• You flagged this item as spam.
  

  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')
  

  • 1108.diff 1.3 KB

• You flagged this item as spam.
  

  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' }
  

  • 1108-2.diff 1.1 KB

• You flagged this item as spam.
  

  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.

  • 1108-fix.diff 7 KB

• You flagged this item as spam.
  

  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.

• 

  Ryan Bigg October 9th, 2010 @ 10:11 PM

  • Tag cleared.

  Automatic cleanup of spam.

• 

  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”
• 

  Ryan Bigg October 19th, 2010 @ 08:24 AM

  Automatic cleanup of spam.

• 

  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!

• 

  Ryan Bigg October 21st, 2010 @ 03:37 AM

  Automatic cleanup of spam.

• You flagged this item as spam.
  

  toby cabot October 21st, 2010 @ 12:34 PM

  Thanks Aaron!

• You flagged this item as spam.
  

  bingbing March 26th, 2011 @ 02:14 AM

  louis vuitton watches