Merge branch 'master' of git@github.com:rails/rails

11 files changed, 107 insertions, 19 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 154b282130..f72d8aceaa 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Added block-setting of attributes for Base.create like Base.new already has (Adam Meehan) [#39]
+
 * Fixed that pessimistic locking you reference the quoted table name (Josh Susser) [#67]
 
 * Fixed that change_column should be able to use :null => true on a field that formerly had false [Nate Wiger] [#26]
diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb
index 55c0cdd808..3e7c787dee 100644
--- a/activerecord/lib/active_record/association_preload.rb
+++ b/activerecord/lib/active_record/association_preload.rb
@@ -59,14 +59,14 @@ module ActiveRecord
 
       def set_association_collection_records(id_to_record_map, reflection_name, associated_records, key)
         associated_records.each do |associated_record|
-          mapped_records = id_to_record_map[associated_record[key].to_i]
+          mapped_records = id_to_record_map[associated_record[key].to_s]
           add_preloaded_records_to_collection(mapped_records, reflection_name, associated_record)
         end
       end
 
       def set_association_single_records(id_to_record_map, reflection_name, associated_records, key)
         associated_records.each do |associated_record|
-          mapped_records = id_to_record_map[associated_record[key].to_i]
+          mapped_records = id_to_record_map[associated_record[key].to_s]
           mapped_records.each do |mapped_record|
             mapped_record.send("set_#{reflection_name}_target", associated_record)
           end
@@ -78,7 +78,7 @@ module ActiveRecord
         ids = []
         records.each do |record|
           ids << record.id
-          mapped_records = (id_to_record_map[record.id] ||= [])
+          mapped_records = (id_to_record_map[record.id.to_s] ||= [])
           mapped_records << record
         end
         ids.uniq!
@@ -115,7 +115,7 @@ module ActiveRecord
             source = reflection.source_reflection.name
             through_records.first.class.preload_associations(through_records, source)
             through_records.each do |through_record|
-              add_preloaded_record_to_collection(id_to_record_map[through_record[through_primary_key].to_i],
+              add_preloaded_record_to_collection(id_to_record_map[through_record[through_primary_key].to_s],
                                                  reflection.name, through_record.send(source))
             end
           end
@@ -140,7 +140,7 @@ module ActiveRecord
             source = reflection.source_reflection.name
             through_records.first.class.preload_associations(through_records, source)
             through_records.each do |through_record|
-              add_preloaded_records_to_collection(id_to_record_map[through_record[through_primary_key].to_i],
+              add_preloaded_records_to_collection(id_to_record_map[through_record[through_primary_key].to_s],
                                                  reflection.name, through_record.send(source))
             end
           end
@@ -195,18 +195,22 @@ module ActiveRecord
           records.each do |record|
             if klass = record.send(polymorph_type)
               klass_id = record.send(primary_key_name)
-
-              id_map = klasses_and_ids[klass] ||= {}
-              id_list_for_klass_id = (id_map[klass_id] ||= [])
-              id_list_for_klass_id << record
+              if klass_id
+                id_map = klasses_and_ids[klass] ||= {}
+                id_list_for_klass_id = (id_map[klass_id.to_s] ||= [])
+                id_list_for_klass_id << record
+              end
             end
           end
           klasses_and_ids = klasses_and_ids.to_a
         else
           id_map = {}
           records.each do |record|
-            mapped_records = (id_map[record.send(primary_key_name)] ||= [])
-            mapped_records << record
+            key = record.send(primary_key_name)
+            if key
+              mapped_records = (id_map[key.to_s] ||= [])
+              mapped_records << record
+            end
           end
           klasses_and_ids = [[reflection.klass.name, id_map]]
         end
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 63306644fb..4f4ba83a47 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -603,13 +603,25 @@ module ActiveRecord #:nodoc:
       # ==== Examples
       #   # Create a single new object
       #   User.create(:first_name => 'Jamie')
+      #
       #   # Create an Array of new objects
       #   User.create([{:first_name => 'Jamie'}, {:first_name => 'Jeremy'}])
-      def create(attributes = nil)
+      #
+      #   # Create a single object and pass it into a block to set other attributes.
+      #   User.create(:first_name => 'Jamie') do |u|
+      #     u.is_admin = false
+      #   end
+      #
+      #   # Creating an Array of new objects using a block, where the block is executed for each object:
+      #   User.create([{:first_name => 'Jamie'}, {:first_name => 'Jeremy'}]) do |u|
+      #     u.is_admin = false
+      #   end 
+      def create(attributes = nil, &block)
         if attributes.is_a?(Array)
-          attributes.collect { |attr| create(attr) }
+          attributes.collect { |attr| create(attr, &block) }
         else
           object = new(attributes)
+          yield(object) if block_given?
           object.save
           object
         end
diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb
index f5b2e73da9..7d5fd35dae 100755
--- a/activerecord/lib/active_record/fixtures.rb
+++ b/activerecord/lib/active_record/fixtures.rb
@@ -469,8 +469,8 @@ class Fixtures < (RUBY_VERSION < '1.9' ? YAML::Omap : Hash)
     fixtures.size > 1 ? fixtures : fixtures.first
   end
 
-  def self.cache_fixtures(connection, fixtures)
-    cache_for_connection(connection).update(fixtures.index_by { |f| f.table_name })
+  def self.cache_fixtures(connection, fixtures_map)
+    cache_for_connection(connection).update(fixtures_map)
   end
 
   def self.instantiate_fixtures(object, table_name, fixtures, load_instances = true)
@@ -526,7 +526,7 @@ class Fixtures < (RUBY_VERSION < '1.9' ? YAML::Omap : Hash)
             end
           end
 
-          cache_fixtures(connection, fixtures)
+          cache_fixtures(connection, fixtures_map)
         end
       end
     end
diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb
index 1d12ea8ad7..5ca51c014c 100755
--- a/activerecord/lib/active_record/validations.rb
+++ b/activerecord/lib/active_record/validations.rb
@@ -873,11 +873,12 @@ module ActiveRecord
 
       # Creates an object just like Base.create but calls save! instead of save
       # so an exception is raised if the record is invalid.
-      def create!(attributes = nil)
+      def create!(attributes = nil, &block)
         if attributes.is_a?(Array)
-          attributes.collect { |attr| create!(attr) }
+          attributes.collect { |attr| create!(attr, &block) }
         else
           object = new(attributes)
+          yield(object) if block_given?
           object.save!
           object
         end
diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb
index 1a3017a22c..546ed80894 100644
--- a/activerecord/test/cases/associations/eager_test.rb
+++ b/activerecord/test/cases/associations/eager_test.rb
@@ -11,11 +11,14 @@ require 'models/owner'
 require 'models/pet'
 require 'models/reference'
 require 'models/job'
+require 'models/subscriber'
+require 'models/subscription'
+require 'models/book'
 
 class EagerAssociationTest < ActiveRecord::TestCase
   fixtures :posts, :comments, :authors, :categories, :categories_posts,
             :companies, :accounts, :tags, :taggings, :people, :readers,
-            :owners, :pets, :author_favorites, :jobs, :references
+            :owners, :pets, :author_favorites, :jobs, :references, :subscribers, :subscriptions, :books
 
   def test_loading_with_one_association
     posts = Post.find(:all, :include => :comments)
@@ -220,6 +223,24 @@ class EagerAssociationTest < ActiveRecord::TestCase
     assert_no_queries{ assert_equal jobs(:unicyclist, :magician), michael.jobs.sort_by(&:id) }
   end
 
+  def test_eager_load_has_many_with_string_keys
+    subscriptions = subscriptions(:webster_awdr, :webster_rfr)
+    subscriber =Subscriber.find(subscribers(:second).id, :include => :subscriptions)
+    assert_equal subscriptions, subscriber.subscriptions.sort_by(&:id)
+  end
+  
+  def test_eager_load_has_many_through_with_string_keys
+    books = books(:awdr, :rfr)
+    subscriber = Subscriber.find(subscribers(:second).id, :include => :books)
+    assert_equal books, subscriber.books.sort_by(&:id)
+  end
+  
+  def test_eager_load_belongs_to_with_string_keys
+    subscriber = subscribers(:second)
+    subscription = Subscription.find(subscriptions(:webster_awdr).id, :include => :subscriber)
+    assert_equal subscriber, subscription.subscriber
+  end
+
   def test_eager_association_loading_with_explicit_join
     posts = Post.find(:all, :include => :comments, :joins => "INNER JOIN authors ON posts.author_id = authors.id AND authors.name = 'Mary'", :limit => 1, :order => 'author_id')
     assert_equal 1, posts.length
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index 93719c710a..e07ec50c46 100755
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -251,6 +251,27 @@ class BasicsTest < ActiveRecord::TestCase
     topic = Topic.create("title" => "New Topic")
     topicReloaded = Topic.find(topic.id)
     assert_equal(topic, topicReloaded)
+  end  
+
+  def test_create_through_factory_with_block
+    topic = Topic.create("title" => "New Topic") do |t|
+      t.author_name = "David"
+    end
+    topicReloaded = Topic.find(topic.id)
+    assert_equal("New Topic", topic.title)
+    assert_equal("David", topic.author_name)
+  end
+
+  def test_create_many_through_factory_with_block
+    topics = Topic.create([ { "title" => "first" }, { "title" => "second" }]) do |t|
+      t.author_name = "David"
+    end
+    assert_equal 2, topics.size
+    topic1, topic2 = Topic.find(topics[0].id), Topic.find(topics[1].id)
+    assert_equal "first", topic1.title
+    assert_equal "David", topic1.author_name
+    assert_equal "second", topic2.title
+    assert_equal "David", topic2.author_name
   end
 
   def test_update
diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb
index 182f4f0f0b..2787b9a39d 100755
--- a/activerecord/test/cases/fixtures_test.rb
+++ b/activerecord/test/cases/fixtures_test.rb
@@ -96,6 +96,10 @@ class FixturesTest < ActiveRecord::TestCase
 
       second_row = ActiveRecord::Base.connection.select_one("SELECT * FROM prefix_topics_suffix WHERE author_name = 'Mary'")
       assert_nil(second_row["author_email_address"])
+
+      # This checks for a caching problem which causes a bug in the fixtures 
+      # class-level configuration helper.
+      assert_not_nil topics, "Fixture data inserted, but fixture objects not returned from create"
     ensure
       # Restore prefix/suffix to its previous values
       ActiveRecord::Base.table_name_prefix = old_prefix
diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb
index ca36ad3581..e3ca8660ac 100755
--- a/activerecord/test/cases/validations_test.rb
+++ b/activerecord/test/cases/validations_test.rb
@@ -133,6 +133,22 @@ class ValidationsTest < ActiveRecord::TestCase
       Reply.create!([ { "title" => "OK" }, { "title" => "Wrong Create" }])
     end
   end
+  
+  def test_exception_on_create_bang_with_block
+    assert_raises(ActiveRecord::RecordInvalid) do
+      Reply.create!({ "title" => "OK" }) do |r|
+        r.content = nil
+      end
+    end
+  end
+  
+  def test_exception_on_create_bang_many_with_block
+    assert_raises(ActiveRecord::RecordInvalid) do
+      Reply.create!([{ "title" => "OK" }, { "title" => "Wrong Create" }]) do |r|
+        r.content = nil
+      end
+    end
+  end
 
   def test_scoped_create_without_attributes
     Reply.with_scope(:create => {}) do
diff --git a/activerecord/test/models/subscriber.rb b/activerecord/test/models/subscriber.rb
index 51335a8f20..5b78014e6f 100644
--- a/activerecord/test/models/subscriber.rb
+++ b/activerecord/test/models/subscriber.rb
@@ -1,5 +1,7 @@
 class Subscriber < ActiveRecord::Base
   set_primary_key 'nick'
+  has_many :subscriptions
+  has_many :books, :through => :subscriptions
 end
 
 class SpecialSubscriber < Subscriber
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index e22b78749c..818237f076 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -349,6 +349,11 @@ ActiveRecord::Schema.define do
   end
   add_index :subscribers, :nick, :unique => true
 
+  create_table :subscriptions, :force => true do |t|
+    t.string :subscriber_id
+    t.integer :book_id
+  end
+
   create_table :tasks, :force => true do |t|
     t.datetime :starting
     t.datetime :ending