Merge pull request #2955 from dmitry/master

Polymorphic with optimistic lock and counter cache should be destroyed

5 files changed, 19 insertions, 9 deletions

diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb
index 1759a41d93..4183c222de 100644
--- a/activerecord/lib/active_record/associations/builder/belongs_to.rb
+++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb
@@ -33,8 +33,10 @@ module ActiveRecord::Associations::Builder
 
         method_name = "belongs_to_counter_cache_before_destroy_for_#{name}"
         mixin.redefine_method(method_name) do
-          record = send(name)
-          record.class.decrement_counter(cache_column, record.id) unless record.nil?
+          unless marked_for_destruction?
+            record = send(name)
+            record.class.decrement_counter(cache_column, record.id) unless record.nil?
+          end
         end
         model.before_destroy(method_name)
 
diff --git a/activerecord/lib/active_record/associations/builder/has_many.rb b/activerecord/lib/active_record/associations/builder/has_many.rb
index 9c24f40690..fc6799fb15 100644
--- a/activerecord/lib/active_record/associations/builder/has_many.rb
+++ b/activerecord/lib/active_record/associations/builder/has_many.rb
@@ -31,12 +31,7 @@ module ActiveRecord::Associations::Builder
         mixin.redefine_method(dependency_method_name) do
           send(name).each do |o|
             # No point in executing the counter update since we're going to destroy the parent anyway
-            counter_method = ('belongs_to_counter_cache_before_destroy_for_' + self.class.name.downcase).to_sym
-            if o.respond_to?(counter_method)
-              class << o
-                self
-              end.send(:define_method, counter_method, Proc.new {})
-            end
+            o.mark_for_destruction
           end
 
           send(name).delete_all
diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb
index f7ee83998d..65cd9f9755 100644
--- a/activerecord/test/cases/locking_test.rb
+++ b/activerecord/test/cases/locking_test.rb
@@ -6,6 +6,9 @@ require 'models/reader'
 require 'models/legacy_thing'
 require 'models/reference'
 require 'models/string_key_object'
+require 'models/car'
+require 'models/engine'
+require 'models/wheel'
 
 class LockWithoutDefault < ActiveRecord::Base; end
 
@@ -224,6 +227,15 @@ class OptimisticLockingTest < ActiveRecord::TestCase
       assert_equal lock_version, p1.lock_version
     end
   end
+
+  def test_polymorphic_destroy_with_dependencies_and_lock_version
+    car = Car.create!
+    car.wheels << Wheel.create!
+    assert_equal 1, car.wheels.count
+    assert car.destroy
+    assert_equal 0, car.wheels.count
+    assert car.destroyed?
+  end
 end
 
 class OptimisticLockingWithSchemaChangeTest < ActiveRecord::TestCase
diff --git a/activerecord/test/models/car.rb b/activerecord/test/models/car.rb
index b9c2e8ec9a..6ff1329d8e 100644
--- a/activerecord/test/models/car.rb
+++ b/activerecord/test/models/car.rb
@@ -9,7 +9,7 @@ class Car < ActiveRecord::Base
 
   has_many :tyres
   has_many :engines, :dependent => :destroy
-  has_many :wheels, :as => :wheelable
+  has_many :wheels, :as => :wheelable, :dependent => :destroy
 
   scope :incl_tyres, includes(:tyres)
   scope :incl_engines, includes(:engines)
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index b8f34bb739..11378c12e5 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -107,6 +107,7 @@ ActiveRecord::Schema.define do
     t.string  :name
     t.integer :engines_count
     t.integer :wheels_count
+    t.column :lock_version, :integer, :null => false, :default => 0
   end
 
   create_table :categories, :force => true do |t|