Make clearing of HABTM join table contents happen in an after_destory callback.

The old method of redefining destroy meant that clearing the HABTM join table would happen as long as the call to destroy succeeded.  Which meant if there was a before_destroy that stopped the instance being destroyed using normal means (returning false, raising ActiveRecord::Rollback) rather than exceptional means the join table would be cleared even though the instance wasn't destroyed.  Doing it in an after_destroy hook avoids this and has the advantage of happening inside the DB transaction too.

1 files changed, 12 insertions, 12 deletions

diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb
index e40b32826a..4b48757da7 100644
--- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb
+++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb
@@ -7,24 +7,24 @@ module ActiveRecord::Associations::Builder
     def build
       reflection = super
       check_validity(reflection)
-      redefine_destroy
+      define_after_destroy_method
       reflection
     end
 
     private
 
-      def redefine_destroy
-        # Don't use a before_destroy callback since users' before_destroy
-        # callbacks will be executed after the association is wiped out.
+      def define_after_destroy_method
         name = self.name
-        model.send(:include, Module.new {
-          class_eval <<-RUBY, __FILE__, __LINE__ + 1
-            def destroy          # def destroy
-              super              #   super
-              #{name}.clear      #   posts.clear
-            end                  # end
-          RUBY
-        })
+        model.send(:class_eval, <<-eoruby, __FILE__, __LINE__ + 1)
+          def #{after_destroy_method_name}
+            association(#{name.to_sym.inspect}).delete_all
+          end
+        eoruby
+        model.after_destroy after_destroy_method_name
+      end
+
+      def after_destroy_method_name
+        "has_and_belongs_to_many_after_destroy_for_#{name}"
       end
 
       # TODO: These checks should probably be moved into the Reflection, and we should not be