Removed the AR::Recursion module--it broke more code than it fixed

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@1470 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

5 files changed, 38 insertions, 78 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 4f7f69220e..e07265b3ac 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -21,8 +21,6 @@
 
 * Corrected typo in find SQL for has_and_belongs_to_many.  #1312 [ben@bensinclair.com]
 
-* Added ActiveRecord::Recursion for guarding against recursive saves
-
 * Fixed sanitized conditions for has_many finder method.  #1281 [jackc@hylesanderson.com, pragdave, Tobias Luetke]
 
 * Comprehensive PostgreSQL schema support.  Use the optional schema_search_path directive in database.yml to give a comma-separated list of schemas to search for your tables.  This allows you, for example, to have tables in a shared schema without having to use a custom table name.  See http://www.postgresql.org/docs/8.0/interactive/ddl-schemas.html to learn more.  #827 [dave@cherryville.org]
diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb
index 168719060d..9d821d17e9 100755
--- a/activerecord/lib/active_record.rb
+++ b/activerecord/lib/active_record.rb
@@ -46,7 +46,6 @@ require 'active_record/acts/tree'
 require 'active_record/acts/nested_set'
 require 'active_record/locking'
 require 'active_record/migration'
-require 'active_record/recursion'
 
 ActiveRecord::Base.class_eval do
   include ActiveRecord::Validations
@@ -60,7 +59,6 @@ ActiveRecord::Base.class_eval do
   include ActiveRecord::Acts::Tree
   include ActiveRecord::Acts::List
   include ActiveRecord::Acts::NestedSet
-  include ActiveRecord::Recursion # must go last!
 end
 
 require 'active_record/connection_adapters/mysql_adapter'
diff --git a/activerecord/lib/active_record/recursion.rb b/activerecord/lib/active_record/recursion.rb
deleted file mode 100644
index 36589238e7..0000000000
--- a/activerecord/lib/active_record/recursion.rb
+++ /dev/null
@@ -1,60 +0,0 @@
-module ActiveRecord
-  # Wraps a guard around #save to make sure that recursive calls don't actually
-  # invoke save multiple times. Recursive calls to save can occur quite
-  # easily, and unintentionally. Consider the following case:
-  #
-  #   class Project < ActiveRecord::Base
-  #     has_and_belongs_to_many :people
-  #     after_create :grant_access_to_admins
-  #
-  #     def grant_access_to_admins
-  #       Person.admins.each do |admin|
-  #         admin.projects.push_with_attributes(self, "access_level" => 42)
-  #       end
-  #     end
-  #   end
-  #
-  #   class Person < ActiveRecord::Base
-  #     has_and_belongs_to_many :projects
-  #     ...
-  #   end
-  #
-  #   teddy = Person.find_by_name("teddy")
-  #   project = Project.new :name => "sumo wrestling"
-  #   project.people << teddy
-  #   project.save!
-  #
-  # The #push_with_attributes causes +self+ (the project) to be saved again,
-  # even though we're already in the midst of doing a save. This results in
-  # "teddy" _not_ being added to the project's people list, because the
-  # recursive call resets the new-record status and thus ignores any
-  # non-new records in the collection.
-  #
-  # Thus, the need for a recursive guard on save.
-  module Recursion 
-    def self.append_features(base) # :nodoc:
-      super
-
-      base.class_eval do
-        alias_method :save_without_recursive_guard, :save
-        alias_method :save, :save_with_recursive_guard
-      end
-    end    
-
-    # Wrap the save call with a sentinel that prevents saves from occuring if
-    # a save is already in progress.
-    def save_with_recursive_guard(*args)
-      critical = Thread.critical
-      Thread.critical = true
-      old_save_state = @currently_saving_record
-      return true if @currently_saving_record
-      @currently_saving_record = true
-      Thread.critical = critical
-
-      save_without_recursive_guard(*args)
-    ensure
-      Thread.critical = critical
-      @currently_saving_record = old_save_state
-    end
-  end
-end
diff --git a/activerecord/test/associations_test.rb b/activerecord/test/associations_test.rb
index 214b582f56..6fb8757153 100755
--- a/activerecord/test/associations_test.rb
+++ b/activerecord/test/associations_test.rb
@@ -774,6 +774,32 @@ class BelongsToAssociationsTest < Test::Unit::TestCase
 end
 
 
+class ProjectWithAfterCreateHook < ActiveRecord::Base
+  set_table_name 'projects'
+  has_and_belongs_to_many :developers,
+    :class_name => "DeveloperForProjectWithAfterCreateHook",
+    :join_table => "developers_projects",
+    :foreign_key => "project_id",
+    :association_foreign_key => "developer_id"
+
+  after_create :add_david
+
+  def add_david
+    david = DeveloperForProjectWithAfterCreateHook.find_by_name('David')
+    david.projects << self
+  end
+end
+
+class DeveloperForProjectWithAfterCreateHook < ActiveRecord::Base
+  set_table_name 'developers'
+  has_and_belongs_to_many :projects,
+    :class_name => "ProjectWithAfterCreateHook",
+    :join_table => "developers_projects",
+    :association_foreign_key => "project_id",
+    :foreign_key => "developer_id"
+end
+
+
 class HasAndBelongsToManyAssociationsTest < Test::Unit::TestCase
   fixtures :accounts, :companies, :developers, :projects, :developers_projects
   
@@ -1018,6 +1044,18 @@ class HasAndBelongsToManyAssociationsTest < Test::Unit::TestCase
     assert_equal developers(:david), active_record.developers.find(developers(:david).id), "Ruby find"
   end
 
+  def test_new_with_values_in_collection
+    jamis = DeveloperForProjectWithAfterCreateHook.find_by_name('Jamis')
+    david = DeveloperForProjectWithAfterCreateHook.find_by_name('David')
+    project = ProjectWithAfterCreateHook.new(:name => "Cooking with Bertie")
+    project.developers << jamis
+    project.save!
+    project.reload
+
+    assert project.developers.include?(jamis)
+    assert project.developers.include?(david)
+  end
+
   def xtest_find_in_association_with_options
     developers = projects(:active_record).developers.find(:all)
     assert_equal 2, developers.size
diff --git a/activerecord/test/callbacks_test.rb b/activerecord/test/callbacks_test.rb
index 2e3bf54a86..d915b3a875 100644
--- a/activerecord/test/callbacks_test.rb
+++ b/activerecord/test/callbacks_test.rb
@@ -304,18 +304,4 @@ class CallbacksTest < Test::Unit::TestCase
       [ :before_validation, :returning_false  ]
     ], david.history
   end
-
-  def test_save_not_called_recursively
-    david = RecursiveCallbackDeveloper.find(1)
-    david.save
-    assert_equal 1, david.on_before_save_called
-    assert_equal 1, david.on_after_save_called
-  end
-
-  def test_save_bang_not_called_recursively
-    david = RecursiveCallbackDeveloper.find(1)
-    david.save!
-    assert_equal 1, david.on_before_save_called
-    assert_equal 1, david.on_after_save_called
-  end
 end