Consolidated different create and create! versions to call through to the base class with scope. This fixes inconsistencies, especially related to protected attribtues.

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

activerecord/CHANGELOG

@@ -1,5 +1,7 @@
 *SVN*
 
+* Consolidated different create and create! versions to call through to the base class with scope. This fixes inconsistencies, especially related to protected attribtues. Closes #5847 [Alexander Dymo, Tobias Luetke]
+
 * find supports :lock with :include. Check whether your database allows SELECT ... FOR UPDATE with outer joins before using.  #6764 [vitaly, Jeremy Kemper]
 
 * Add AssociationCollection#create! to be consistent with AssociationCollection#create when dealing with a foreign key that is a protected attribute [Cody Fauser]

activerecord/lib/active_record/associations/association_collection.rb

@@ -85,12 +85,18 @@ module ActiveRecord
         reset_target!
       end
       
-      def create(attributes = {})
-        build_and_save_with(attributes, :save)        
+      def create(attrs = {})
+        record = @reflection.klass.with_scope(construct_scope) { @reflection.klass.create(attrs) }                
+        @target ||= [] unless loaded?
+        @target << record 
+        record
       end
 
-      def create!(attributes = {})
-        build_and_save_with(attributes, :save!)
+      def create!(attrs = {})
+        record = @reflection.klass.with_scope(construct_scope) { @reflection.klass.create!(attrs) }                
+        @target ||= [] unless loaded?
+        @target << record 
+        record
       end
 
       # Returns the size of the collection by executing a SELECT COUNT(*) query if the collection hasn't been loaded and
@@ -202,17 +208,6 @@ module ActiveRecord
           full_callback_name = "#{callback_name}_for_#{@reflection.name}"
           @owner.class.read_inheritable_attribute(full_callback_name.to_sym) || []
         end        
-        
-        def build_and_save_with(attributes, method)
-          # Can't use Base.create since the foreign key may be a protected attribute.
-          if attributes.is_a?(Array)
-            attributes.collect { |attr| create(attr) }
-          else
-            record = build(attributes)
-            record.send(method) unless @owner.new_record?
-            record
-          end          
-        end
     end
   end
 end

activerecord/lib/active_record/associations/has_one_association.rb

@@ -6,9 +6,29 @@ module ActiveRecord
         construct_sql
       end
 
-      def create(attributes = {}, replace_existing = true)
-        record = build(attributes, replace_existing)
-        record.save
+      def create(attrs = {}, replace_existing = true)
+        record = @reflection.klass.with_scope(construct_scope) { @reflection.klass.create(attrs) }                
+
+        if replace_existing
+          replace(record, true) 
+        else
+          record[@reflection.primary_key_name] = @owner.id unless @owner.new_record?
+          self.target = record
+        end
+
+        record
+      end
+
+      def create!(attrs = {}, replace_existing = true)
+        record = @reflection.klass.with_scope(construct_scope) { @reflection.klass.create!(attrs) }                
+
+        if replace_existing
+          replace(record, true) 
+        else
+          record[@reflection.primary_key_name] = @owner.id unless @owner.new_record?
+          self.target = record
+        end
+
         record
       end
 
@@ -75,6 +95,13 @@ module ActiveRecord
           end
           @finder_sql << " AND (#{conditions})" if conditions
         end
+        
+        def construct_scope
+          create_scoping = {}
+          set_belongs_to_association_for(create_scoping)
+          { :create => create_scoping }
+        end
+        
     end
   end
 end

activerecord/lib/active_record/validations.rb

@@ -717,12 +717,10 @@ module ActiveRecord
       # so an exception is raised if the record is invalid.
       def create!(attributes = nil)
         if attributes.is_a?(Array)
-          attributes.collect { |attr| create!(attr) }
+          attributes.collect { |attr| create(attr) }
         else
-          attributes ||= {}
-          attributes.reverse_merge!(scope(:create)) if scoped?(:create)
-
           object = new(attributes)
+          scope(:create).each { |att,value| object.send("#{att}=", value) } if scoped?(:create)
           object.save!
           object
         end

activerecord/test/validations_test.rb

@@ -1,6 +1,7 @@
 require 'abstract_unit'
 require 'fixtures/topic'
 require 'fixtures/reply'
+require 'fixtures/person'
 require 'fixtures/developer'
 
 # The following methods in Topic are used in test_conditional_validation_*
@@ -14,6 +15,12 @@ class Topic
   end
 end
 
+class ProtectedPerson < ActiveRecord::Base
+  set_table_name 'people'
+  attr_accessor :addon
+  attr_protected :first_name
+end
+
 class ValidationsTest < Test::Unit::TestCase
   fixtures :topics, :developers
 
@@ -94,6 +101,24 @@ class ValidationsTest < Test::Unit::TestCase
     end
   end
 
+  def test_create_with_exceptions_using_scope_for_protected_attributes
+    assert_nothing_raised do
+      ProtectedPerson.with_scope( :create => { :first_name => "Mary" } ) do
+        person = ProtectedPerson.create! :addon => "Addon"
+        assert_equal person.first_name, "Mary", "scope should ignore attr_protected"
+      end
+    end
+  end
+
+  def test_create_with_exceptions_using_scope_and_empty_attributes
+    assert_nothing_raised do
+      ProtectedPerson.with_scope( :create => { :first_name => "Mary" } ) do        
+        person = ProtectedPerson.create!
+        assert_equal person.first_name, "Mary", "should be ok when no attributes are passed to create!"
+      end
+    end
+ end
+
   def test_single_error_per_attr_iteration
     r = Reply.new
     r.save