diff options
18 files changed, 66 insertions, 162 deletions
diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 9f47e7e631..495f0cde59 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -233,10 +233,10 @@ module ActiveRecord def stale_state end - def build_record(attributes, options) - reflection.build_association(attributes, options) do |record| + def build_record(attributes) + reflection.build_association(attributes) do |record| attributes = create_scope.except(*(record.changed - [reflection.foreign_key])) - record.assign_attributes(attributes, :without_protection => true) + record.assign_attributes(attributes) end end end diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 424c1a5b79..fe3e5b00f7 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -95,22 +95,22 @@ module ActiveRecord first_or_last(:last, *args) end - def build(attributes = {}, options = {}, &block) + def build(attributes = {}, &block) if attributes.is_a?(Array) - attributes.collect { |attr| build(attr, options, &block) } + attributes.collect { |attr| build(attr, &block) } else - add_to_target(build_record(attributes, options)) do |record| + add_to_target(build_record(attributes)) do |record| yield(record) if block_given? end end end - def create(attributes = {}, options = {}, &block) - create_record(attributes, options, &block) + def create(attributes = {}, &block) + create_record(attributes, &block) end - def create!(attributes = {}, options = {}, &block) - create_record(attributes, options, true, &block) + def create!(attributes = {}, &block) + create_record(attributes, true, &block) end # Add +records+ to this association. Returns +self+ so method calls may @@ -425,16 +425,16 @@ module ActiveRecord persisted + memory end - def create_record(attributes, options, raise = false, &block) + def create_record(attributes, raise = false, &block) unless owner.persisted? raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved" end if attributes.is_a?(Array) - attributes.collect { |attr| create_record(attr, options, raise, &block) } + attributes.collect { |attr| create_record(attr, raise, &block) } else transaction do - add_to_target(build_record(attributes, options)) do |record| + add_to_target(build_record(attributes)) do |record| yield(record) if block_given? insert_record(record, true, raise) end diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index a45ec33546..c113957faa 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -222,8 +222,8 @@ module ActiveRecord # # person.pets.size # => 5 # size of the collection # person.pets.count # => 0 # count from database - def build(attributes = {}, options = {}, &block) - @association.build(attributes, options, &block) + def build(attributes = {}, &block) + @association.build(attributes, &block) end ## @@ -253,8 +253,8 @@ module ActiveRecord # # #<Pet id: 2, name: "Spook", person_id: 1>, # # #<Pet id: 3, name: "Choo-Choo", person_id: 1> # # ] - def create(attributes = {}, options = {}, &block) - @association.create(attributes, options, &block) + def create(attributes = {}, &block) + @association.create(attributes, &block) end ## @@ -270,8 +270,8 @@ module ActiveRecord # # person.pets.create!(name: nil) # # => ActiveRecord::RecordInvalid: Validation failed: Name can't be blank - def create!(attributes = {}, options = {}, &block) - @association.create!(attributes, options, &block) + def create!(attributes = {}, &block) + @association.create!(attributes, &block) end ## diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index 88ff11f953..4fec5539f2 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -82,10 +82,10 @@ module ActiveRecord @through_records.delete(record.object_id) end - def build_record(attributes, options = {}) + def build_record(attributes) ensure_not_nested - record = super(attributes, options) + record = super(attributes) inverse = source_reflection.inverse_of if inverse diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb index b84cb4922d..32f4557c28 100644 --- a/activerecord/lib/active_record/associations/singular_association.rb +++ b/activerecord/lib/active_record/associations/singular_association.rb @@ -17,16 +17,16 @@ module ActiveRecord replace(record) end - def create(attributes = {}, options = {}, &block) - create_record(attributes, options, &block) + def create(attributes = {}, &block) + create_record(attributes, &block) end - def create!(attributes = {}, options = {}, &block) - create_record(attributes, options, true, &block) + def create!(attributes = {}, &block) + create_record(attributes, true, &block) end - def build(attributes = {}, options = {}) - record = build_record(attributes, options) + def build(attributes = {}) + record = build_record(attributes) yield(record) if block_given? set_new_record(record) record @@ -51,8 +51,8 @@ module ActiveRecord replace(record) end - def create_record(attributes, options, raise_error = false) - record = build_record(attributes, options) + def create_record(attributes, raise_error = false) + record = build_record(attributes) yield(record) if block_given? saved = record.save set_new_record(record) diff --git a/activerecord/lib/active_record/attribute_assignment.rb b/activerecord/lib/active_record/attribute_assignment.rb index f11ca941c4..ccc61ec5d3 100644 --- a/activerecord/lib/active_record/attribute_assignment.rb +++ b/activerecord/lib/active_record/attribute_assignment.rb @@ -19,21 +19,14 @@ module ActiveRecord # Allows you to set all the attributes by passing in a hash of attributes with # keys matching the attribute names (which again matches the column names) - # - # To bypass forbidden attributes protection you can use the without_protection: true - # option. - def assign_attributes(new_attributes, options = {}) + def assign_attributes(new_attributes) return if new_attributes.blank? attributes = new_attributes.stringify_keys multi_parameter_attributes = [] nested_parameter_attributes = [] - previous_options = @mass_assignment_options - @mass_assignment_options = options - unless options[:without_protection] - attributes = sanitize_for_mass_assignment(attributes) - end + attributes = sanitize_for_mass_assignment(attributes) attributes.each do |k, v| if k.include?("(") @@ -47,14 +40,6 @@ module ActiveRecord assign_nested_parameter_attributes(nested_parameter_attributes) unless nested_parameter_attributes.empty? assign_multiparameter_attributes(multi_parameter_attributes) unless multi_parameter_attributes.empty? - ensure - @mass_assignment_options = previous_options - end - - protected - - def mass_assignment_options - @mass_assignment_options ||= {} end private diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 43ffca0227..8f798830d4 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -172,7 +172,7 @@ module ActiveRecord # # # Instantiates a single new object bypassing mass-assignment security # User.new({ :first_name => 'Jamie', :is_admin => true }, :without_protection => true) - def initialize(attributes = nil, options = {}) + def initialize(attributes = nil) defaults = self.class.column_defaults.dup defaults.each { |k, v| defaults[k] = v.dup if v.duplicable? } @@ -183,7 +183,7 @@ module ActiveRecord ensure_proper_type populate_with_current_scope_attributes - assign_attributes(attributes, options) if attributes + assign_attributes(attributes) if attributes yield self if block_given? run_callbacks :initialize unless _initialize_callbacks.empty? @@ -386,7 +386,7 @@ module ActiveRecord @destroyed = false @marked_for_destruction = false @new_record = true - @mass_assignment_options = nil + @txn = nil @_start_transaction_state = {} end end diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 6d535e4ffa..a33285b16f 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -282,7 +282,7 @@ module ActiveRecord remove_method(:#{association_name}_attributes=) end def #{association_name}_attributes=(attributes) - assign_nested_attributes_for_#{type}_association(:#{association_name}, attributes, mass_assignment_options) + assign_nested_attributes_for_#{type}_association(:#{association_name}, attributes) end eoruby else @@ -326,15 +326,15 @@ module ActiveRecord if (options[:update_only] || !attributes['id'].blank?) && (record = send(association_name)) && (options[:update_only] || record.id.to_s == attributes['id'].to_s) - assign_to_or_mark_for_destruction(record, attributes, options[:allow_destroy], assignment_opts) unless call_reject_if(association_name, attributes) + assign_to_or_mark_for_destruction(record, attributes, options[:allow_destroy]) unless call_reject_if(association_name, attributes) - elsif attributes['id'].present? && !assignment_opts[:without_protection] + elsif attributes['id'].present? raise_nested_attributes_record_not_found(association_name, attributes['id']) elsif !reject_new_record?(association_name, attributes) method = "build_#{association_name}" if respond_to?(method) - send(method, attributes.except(*unassignable_keys(assignment_opts)), assignment_opts) + send(method, attributes.except(*UNASSIGNABLE_KEYS)) else raise ArgumentError, "Cannot build association `#{association_name}'. Are you trying to build a polymorphic one-to-one association?" end @@ -368,7 +368,7 @@ module ActiveRecord # { :name => 'John' }, # { :id => '2', :_destroy => true } # ]) - def assign_nested_attributes_for_collection_association(association_name, attributes_collection, assignment_opts = {}) + def assign_nested_attributes_for_collection_association(association_name, attributes_collection) options = self.nested_attributes_options[association_name] unless attributes_collection.is_a?(Hash) || attributes_collection.is_a?(Array) @@ -413,7 +413,7 @@ module ActiveRecord if attributes['id'].blank? unless reject_new_record?(association_name, attributes) - association.build(attributes.except(*unassignable_keys(assignment_opts)), assignment_opts) + association.build(attributes.except(*UNASSIGNABLE_KEYS)) end elsif existing_record = existing_records.detect { |record| record.id.to_s == attributes['id'].to_s } unless association.loaded? || call_reject_if(association_name, attributes) @@ -429,10 +429,8 @@ module ActiveRecord end if !call_reject_if(association_name, attributes) - assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy], assignment_opts) + assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy]) end - elsif assignment_opts[:without_protection] - association.build(attributes.except(*unassignable_keys(assignment_opts)), assignment_opts) else raise_nested_attributes_record_not_found(association_name, attributes['id']) end @@ -441,8 +439,8 @@ module ActiveRecord # Updates a record with the +attributes+ or marks it for destruction if # +allow_destroy+ is +true+ and has_destroy_flag? returns +true+. - def assign_to_or_mark_for_destruction(record, attributes, allow_destroy, assignment_opts) - record.assign_attributes(attributes.except(*unassignable_keys(assignment_opts)), assignment_opts) + def assign_to_or_mark_for_destruction(record, attributes, allow_destroy) + record.assign_attributes(attributes.except(*UNASSIGNABLE_KEYS)) record.mark_for_destruction if has_destroy_flag?(attributes) && allow_destroy end @@ -473,7 +471,7 @@ module ActiveRecord end def unassignable_keys(assignment_opts) - assignment_opts[:without_protection] ? UNASSIGNABLE_KEYS - %w[id] : UNASSIGNABLE_KEYS + UNASSIGNABLE_KEYS end end end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index b58b8881ac..6f38262b86 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -35,11 +35,11 @@ module ActiveRecord # User.create([{ :first_name => 'Jamie' }, { :first_name => 'Jeremy' }]) do |u| # u.is_admin = false # end - def create(attributes = nil, options = {}, &block) + def create(attributes = nil, &block) if attributes.is_a?(Array) - attributes.collect { |attr| create(attr, options, &block) } + attributes.collect { |attr| create(attr, &block) } else - object = new(attributes, options, &block) + object = new(attributes, &block) object.save object end @@ -188,22 +188,22 @@ module ActiveRecord # If no +:as+ option is supplied then the +:default+ role will be used. # If you want to bypass the forbidden attributes protection then you can do so using # the +:without_protection+ option. - def update_attributes(attributes, options = {}) + def update_attributes(attributes) # The following transaction covers any possible database side-effects of the # attributes assignment. For example, setting the IDs of a child collection. with_transaction_returning_status do - assign_attributes(attributes, options) + assign_attributes(attributes) save end end # Updates its receiver just like +update_attributes+ but calls <tt>save!</tt> instead # of +save+, so an exception is raised if the record is invalid. - def update_attributes!(attributes, options = {}) + def update_attributes!(attributes) # The following transaction covers any possible database side-effects of the # attributes assignment. For example, setting the IDs of a child collection. with_transaction_returning_status do - assign_attributes(attributes, options) + assign_attributes(attributes) save! end end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index d9c65fa170..f322b96f79 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -181,8 +181,8 @@ module ActiveRecord # Returns a new, unsaved instance of the associated class. +options+ will # be passed to the class's constructor. - def build_association(*options, &block) - klass.new(*options, &block) + def build_association(attributes, &block) + klass.new(attributes, &block) end def table_name diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 2d0457636e..ed80422336 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -151,22 +151,22 @@ module ActiveRecord # user.last_name = "O'Hara" # end # # => <User id: 2, first_name: 'Scarlett', last_name: 'Johansson'> - def first_or_create(attributes = nil, options = {}, &block) - first || create(attributes, options, &block) + def first_or_create(attributes = nil, &block) + first || create(attributes, &block) end # Like <tt>first_or_create</tt> but calls <tt>create!</tt> so an exception is raised if the created record is invalid. # # Expects arguments in the same format as <tt>Base.create!</tt>. - def first_or_create!(attributes = nil, options = {}, &block) - first || create!(attributes, options, &block) + def first_or_create!(attributes = nil, &block) + first || create!(attributes, &block) end # Like <tt>first_or_create</tt> but calls <tt>new</tt> instead of <tt>create</tt>. # # Expects arguments in the same format as <tt>Base.new</tt>. - def first_or_initialize(attributes = nil, options = {}, &block) - first || new(attributes, options, &block) + def first_or_initialize(attributes = nil, &block) + first || new(attributes, &block) end # Runs EXPLAIN on the query or queries triggered by this relation and diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index cef2bbd563..ed561bfb3c 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -32,11 +32,11 @@ module ActiveRecord module ClassMethods # Creates an object just like Base.create but calls <tt>save!</tt> instead of +save+ # so an exception is raised if the record is invalid. - def create!(attributes = nil, options = {}, &block) + def create!(attributes = nil, &block) if attributes.is_a?(Array) - attributes.collect { |attr| create!(attr, options, &block) } + attributes.collect { |attr| create!(attr, &block) } else - object = new(attributes, options) + object = new(attributes) yield(object) if block_given? object.save! object diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 83128628ef..66b5ea939e 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1558,19 +1558,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal "RED!", car.bulbs.to_a.first.color end - def test_new_is_called_with_attributes_and_options - car = Car.create(:name => 'honda') - - bulb = car.bulbs.build - assert_equal Bulb, bulb.class - - bulb = car.bulbs.build(:bulb_type => :custom) - assert_equal Bulb, bulb.class - - bulb = car.bulbs.build({ :bulb_type => :custom }, :as => :admin) - assert_equal CustomBulb, bulb.class - end - def test_abstract_class_with_polymorphic_has_many post = SubStiPost.create! :title => "fooo", :body => "baa" tagging = Tagging.create! :taggable => post diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index a7ec0f2709..2d3cb654df 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -446,22 +446,6 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert_equal pirate.id, ship.pirate_id end - def test_new_is_called_with_attributes_and_options - car = Car.create(:name => 'honda') - - bulb = car.build_bulb - assert_equal Bulb, bulb.class - - bulb = car.build_bulb - assert_equal Bulb, bulb.class - - bulb = car.build_bulb(:bulb_type => :custom) - assert_equal Bulb, bulb.class - - bulb = car.build_bulb({ :bulb_type => :custom }, :as => :admin) - assert_equal CustomBulb, bulb.class - end - def test_build_with_block car = Car.create(:name => 'honda') diff --git a/activerecord/test/cases/dup_test.rb b/activerecord/test/cases/dup_test.rb index 9705a11387..71b2b16608 100644 --- a/activerecord/test/cases/dup_test.rb +++ b/activerecord/test/cases/dup_test.rb @@ -49,7 +49,7 @@ module ActiveRecord dbtopic = Topic.first topic = Topic.new - topic.attributes = dbtopic.attributes + topic.attributes = dbtopic.attributes.except("id") #duped has no timestamp values duped = dbtopic.dup diff --git a/activerecord/test/cases/forbidden_attributes_protection_test.rb b/activerecord/test/cases/forbidden_attributes_protection_test.rb index 4706eaec2b..9a2172f41e 100644 --- a/activerecord/test/cases/forbidden_attributes_protection_test.rb +++ b/activerecord/test/cases/forbidden_attributes_protection_test.rb @@ -46,14 +46,4 @@ class ForbiddenAttributesProtectionTest < ActiveRecord::TestCase assert_equal 'Guille', person.first_name assert_equal 'm', person.gender end - - def test_protected_attributes_cannot_be_used_for_mass_assignment - params = ProtectedParams.new(id: 1, first_name: 'Guille', gender: 'm') - params.permit! - person = Person.new(params) - - assert_equal 'Guille', person.first_name - assert_equal 'm', person.gender - assert_not_equal 1, person.id - end end diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index 4ffa4836e0..b5f32a57b2 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -608,26 +608,6 @@ class PersistencesTest < ActiveRecord::TestCase assert_equal "The First Topic", topic.title end - def test_update_attributes_as_admin - person = TightPerson.create({ "first_name" => 'Joshua' }) - person.update_attributes({ "first_name" => 'Josh', "gender" => 'm', "comments" => 'from NZ' }, :as => :admin) - person.reload - - assert_equal 'Josh', person.first_name - assert_equal 'm', person.gender - assert_equal 'from NZ', person.comments - end - - def test_update_attributes_without_protection - person = TightPerson.create({ "first_name" => 'Joshua' }) - person.update_attributes({ "first_name" => 'Josh', "gender" => 'm', "comments" => 'from NZ' }, :without_protection => true) - person.reload - - assert_equal 'Josh', person.first_name - assert_equal 'm', person.gender - assert_equal 'from NZ', person.comments - end - def test_update_attributes! Reply.validates_presence_of(:title) reply = Reply.find(2) @@ -649,26 +629,6 @@ class PersistencesTest < ActiveRecord::TestCase Reply.reset_callbacks(:validate) end - def test_update_attributes_with_bang_as_admin - person = TightPerson.create({ "first_name" => 'Joshua' }) - person.update_attributes!({ "first_name" => 'Josh', "gender" => 'm', "comments" => 'from NZ' }, :as => :admin) - person.reload - - assert_equal 'Josh', person.first_name - assert_equal 'm', person.gender - assert_equal 'from NZ', person.comments - end - - def test_update_attributestes_with_bang_without_protection - person = TightPerson.create({ "first_name" => 'Joshua' }) - person.update_attributes!({ "first_name" => 'Josh', "gender" => 'm', "comments" => 'from NZ' }, :without_protection => true) - person.reload - - assert_equal 'Josh', person.first_name - assert_equal 'm', person.gender - assert_equal 'from NZ', person.comments - end - def test_destroyed_returns_boolean developer = Developer.first assert_equal false, developer.destroyed? diff --git a/activerecord/test/models/bulb.rb b/activerecord/test/models/bulb.rb index 967f733fe3..e4c0278c0d 100644 --- a/activerecord/test/models/bulb.rb +++ b/activerecord/test/models/bulb.rb @@ -18,12 +18,12 @@ class Bulb < ActiveRecord::Base self[:color] = color.upcase + "!" end - def self.new(attributes = {}, options = {}, &block) + def self.new(attributes = {}, &block) bulb_type = (attributes || {}).delete(:bulb_type) - if options && options[:as] == :admin && bulb_type.present? + if bulb_type.present? bulb_class = "#{bulb_type.to_s.camelize}Bulb".constantize - bulb_class.new(attributes, options, &block) + bulb_class.new(attributes, &block) else super end |