diff options
9 files changed, 629 insertions, 535 deletions
diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index e2dc883b1b..b6f87a0486 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -786,11 +786,7 @@ module ActiveRecord # 'ORDER BY p.first_name' def has_many(association_id, options = {}, &extension) reflection = create_has_many_reflection(association_id, options, &extension) - configure_dependency_for_has_many(reflection) - - add_multiple_associated_validation_callbacks(reflection.name) unless options[:validate] == false - add_multiple_associated_save_callbacks(reflection.name) add_association_callbacks(reflection.name, reflection.options) if options[:through] @@ -872,10 +868,10 @@ module ActiveRecord # [:source] # Specifies the source association name used by <tt>has_one :through</tt> queries. Only use it if the name cannot be # inferred from the association. <tt>has_one :favorite, :through => :favorites</tt> will look for a - # <tt>:favorite</tt> on Favorite, unless a <tt>:source</tt> is given. + # <tt>:favorite</tt> on Favorite, unless a <tt>:source</tt> is given. # [:source_type] # Specifies type of the source association used by <tt>has_one :through</tt> queries where the source - # association is a polymorphic +belongs_to+. + # association is a polymorphic +belongs_to+. # [:readonly] # If true, the associated object is readonly through the association. # [:validate] @@ -898,22 +894,9 @@ module ActiveRecord association_accessor_methods(reflection, ActiveRecord::Associations::HasOneThroughAssociation) else reflection = create_has_one_reflection(association_id, options) - - method_name = "has_one_after_save_for_#{reflection.name}".to_sym - define_method(method_name) do - association = association_instance_get(reflection.name) - if association && (new_record? || association.new_record? || association[reflection.primary_key_name] != id) - association[reflection.primary_key_name] = id - association.save(true) - end - end - after_save method_name - - add_single_associated_validation_callbacks(reflection.name) if options[:validate] == true association_accessor_methods(reflection, HasOneAssociation) association_constructor_method(:build, reflection, HasOneAssociation) association_constructor_method(:create, reflection, HasOneAssociation) - configure_dependency_for_has_one(reflection) end end @@ -1006,40 +989,10 @@ module ActiveRecord if reflection.options[:polymorphic] association_accessor_methods(reflection, BelongsToPolymorphicAssociation) - - method_name = "polymorphic_belongs_to_before_save_for_#{reflection.name}".to_sym - define_method(method_name) do - association = association_instance_get(reflection.name) - if association && association.target - if association.new_record? - association.save(true) - end - - if association.updated? - self[reflection.primary_key_name] = association.id - self[reflection.options[:foreign_type]] = association.class.base_class.name.to_s - end - end - end - before_save method_name else association_accessor_methods(reflection, BelongsToAssociation) association_constructor_method(:build, reflection, BelongsToAssociation) association_constructor_method(:create, reflection, BelongsToAssociation) - - method_name = "belongs_to_before_save_for_#{reflection.name}".to_sym - define_method(method_name) do - if association = association_instance_get(reflection.name) - if association.new_record? - association.save(true) - end - - if association.updated? - self[reflection.primary_key_name] = association.id - end - end - end - before_save method_name end # Create the callbacks to update counter cache @@ -1067,8 +1020,6 @@ module ActiveRecord ) end - add_single_associated_validation_callbacks(reflection.name) if options[:validate] == true - configure_dependency_for_belongs_to(reflection) end @@ -1234,9 +1185,6 @@ module ActiveRecord # 'DELETE FROM developers_projects WHERE active=1 AND developer_id = #{id} AND project_id = #{record.id}' def has_and_belongs_to_many(association_id, options = {}, &extension) reflection = create_has_and_belongs_to_many_reflection(association_id, options, &extension) - - add_multiple_associated_validation_callbacks(reflection.name) unless options[:validate] == false - add_multiple_associated_save_callbacks(reflection.name) collection_accessor_methods(reflection, HasAndBelongsToManyAssociation) # Don't use a before_destroy callback since users' before_destroy @@ -1358,70 +1306,6 @@ module ActiveRecord end end - def add_single_associated_validation_callbacks(association_name) - method_name = "validate_associated_records_for_#{association_name}".to_sym - define_method(method_name) do - if association = association_instance_get(association_name) - errors.add association_name unless association.target.nil? || association.valid? - end - end - - validate method_name - end - - def add_multiple_associated_validation_callbacks(association_name) - method_name = "validate_associated_records_for_#{association_name}".to_sym - define_method(method_name) do - association = association_instance_get(association_name) - - if association - if new_record? - association - elsif association.loaded? - association.select { |record| record.new_record? } - else - association.target.select { |record| record.new_record? } - end.each do |record| - errors.add association_name unless record.valid? - end - end - end - - validate method_name - end - - def add_multiple_associated_save_callbacks(association_name) - method_name = "before_save_associated_records_for_#{association_name}".to_sym - define_method(method_name) do - @new_record_before_save = new_record? - true - end - before_save method_name - - method_name = "after_create_or_update_associated_records_for_#{association_name}".to_sym - define_method(method_name) do - association = association_instance_get(association_name) - - records_to_save = if @new_record_before_save - association - elsif association && association.loaded? - association.select { |record| record.new_record? } - elsif association && !association.loaded? - association.target.select { |record| record.new_record? } - else - [] - end - records_to_save.each { |record| association.send(:insert_record, record) } unless records_to_save.blank? - - # reconstruct the SQL queries now that we know the owner's id - association.send(:construct_sql) if association.respond_to?(:construct_sql) - end - - # Doesn't use after_save as that would save associations added in after_create/after_update twice - after_create method_name - after_update method_name - end - def association_constructor_method(constructor, reflection, association_proxy_class) define_method("#{constructor}_#{reflection.name}") do |*params| attributees = params.first unless params.empty? diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index a5cc3bf091..af9ce3dfb2 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -28,12 +28,12 @@ module ActiveRecord load_target.size end - def insert_record(record, force=true) + def insert_record(record, force = true, validate = true) if record.new_record? if force record.save! else - return false unless record.save + return false unless record.save(validate) end end diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 3348079e9d..a2cbabfe0c 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -56,9 +56,9 @@ module ActiveRecord "#{@reflection.name}_count" end - def insert_record(record) + def insert_record(record, force = false, validate = true) set_belongs_to_association_for(record) - record.save + force ? record.save! : record.save(validate) end # Deletes the records according to the <tt>:dependent</tt> option. 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 2eeeb28d1f..d5d188ac2a 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -47,12 +47,12 @@ module ActiveRecord options[:include] = @reflection.source_reflection.options[:include] if options[:include].nil? end - def insert_record(record, force=true) + def insert_record(record, force = true, validate = true) if record.new_record? if force record.save! else - return false unless record.save + return false unless record.save(validate) end end through_reflection = @reflection.through_reflection diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 680b41518c..6e09b13150 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -125,79 +125,63 @@ module ActiveRecord # post.author.name = '' # post.save(false) # => true module AutosaveAssociation + ASSOCIATION_TYPES = %w{ has_one belongs_to has_many has_and_belongs_to_many } + def self.included(base) base.class_eval do + base.extend(ClassMethods) alias_method_chain :reload, :autosave_associations - alias_method_chain :save, :autosave_associations - alias_method_chain :save!, :autosave_associations - alias_method_chain :valid?, :autosave_associations - %w{ has_one belongs_to has_many has_and_belongs_to_many }.each do |type| + ASSOCIATION_TYPES.each do |type| base.send("valid_keys_for_#{type}_association") << :autosave end end end - # Saves the parent, <tt>self</tt>, and any loaded autosave associations. - # In addition, it destroys all children that were marked for destruction - # with mark_for_destruction. - # - # This all happens inside a transaction, _if_ the Transactions module is included into - # ActiveRecord::Base after the AutosaveAssociation module, which it does by default. - def save_with_autosave_associations(perform_validation = true) - returning(save_without_autosave_associations(perform_validation)) do |valid| - if valid - self.class.reflect_on_all_autosave_associations.each do |reflection| - if (association = association_instance_get(reflection.name)) && association.loaded? - if association.is_a?(Array) - association.proxy_target.each do |child| - child.marked_for_destruction? ? child.destroy : child.save(perform_validation) - end - else - association.marked_for_destruction? ? association.destroy : association.save(perform_validation) - end - end + module ClassMethods + private + + # def belongs_to(name, options = {}) + # super + # add_autosave_association_callbacks(reflect_on_association(name)) + # end + ASSOCIATION_TYPES.each do |type| + module_eval %{ + def #{type}(name, options = {}) + super + add_autosave_association_callbacks(reflect_on_association(name)) end - end + } end - end - # Attempts to save the record just like save_with_autosave_associations but - # will raise a RecordInvalid exception instead of returning false if the - # record is not valid. - def save_with_autosave_associations! - if valid_with_autosave_associations? - save_with_autosave_associations(false) || raise(RecordNotSaved) - else - raise RecordInvalid.new(self) - end - end + # Adds a validate and save callback for the association as specified by + # the +reflection+. + def add_autosave_association_callbacks(reflection) + save_method = "autosave_associated_records_for_#{reflection.name}" + validation_method = "validate_associated_records_for_#{reflection.name}" + validate validation_method - # Returns whether or not the parent, <tt>self</tt>, and any loaded autosave associations are valid. - def valid_with_autosave_associations? - if valid_without_autosave_associations? - self.class.reflect_on_all_autosave_associations.all? do |reflection| - if (association = association_instance_get(reflection.name)) && association.loaded? - if association.is_a?(Array) - association.proxy_target.all? { |child| autosave_association_valid?(reflection, child) } - else - autosave_association_valid?(reflection, association) - end - else - true # association not loaded yet, so it should be valid + case reflection.macro + when :has_many, :has_and_belongs_to_many + before_save :before_save_collection_association + + define_method(save_method) { save_collection_association(reflection) } + # Doesn't use after_save as that would save associations added in after_create/after_update twice + after_create save_method + after_update save_method + + define_method(validation_method) { validate_collection_association(reflection) } + else + case reflection.macro + when :has_one + define_method(save_method) { save_has_one_association(reflection) } + after_save save_method + when :belongs_to + define_method(save_method) { save_belongs_to_association(reflection) } + before_save save_method end + define_method(validation_method) { validate_single_association(reflection) } end - else - false # self was not valid - end - end - - # Returns whether or not the association is valid and applies any errors to the parent, <tt>self</tt>, if it wasn't. - def autosave_association_valid?(reflection, association) - returning(association.valid?) do |valid| - association.errors.each do |attribute, message| - errors.add "#{reflection.name}_#{attribute}", message - end unless valid end end @@ -221,5 +205,142 @@ module ActiveRecord def marked_for_destruction? @marked_for_destruction end + + private + + # Returns the record for an association collection that should be validated + # or saved. If +autosave+ is +false+ only new records will be returned, + # unless the parent is/was a new record itself. + def associated_records_to_validate_or_save(association, new_record, autosave) + if new_record + association + elsif association.loaded? + autosave ? association : association.select { |record| record.new_record? } + else + autosave ? association.target : association.target.select { |record| record.new_record? } + end + end + + # Validate the association if <tt>:validate</tt> or <tt>:autosave</tt> is + # turned on for the association specified by +reflection+. + def validate_single_association(reflection) + if reflection.options[:validate] == true || reflection.options[:autosave] == true + if (association = association_instance_get(reflection.name)) && !association.target.nil? + association_valid?(reflection, association) + end + end + end + + # Validate the associated records if <tt>:validate</tt> or + # <tt>:autosave</tt> is turned on for the association specified by + # +reflection+. + def validate_collection_association(reflection) + if reflection.options[:validate] != false && association = association_instance_get(reflection.name) + if records = associated_records_to_validate_or_save(association, new_record?, reflection.options[:autosave]) + records.each { |record| association_valid?(reflection, record) } + end + end + end + + # Returns whether or not the association is valid and applies any errors to + # the parent, <tt>self</tt>, if it wasn't. + def association_valid?(reflection, association) + unless valid = association.valid? + if reflection.options[:autosave] + association.errors.each do |attribute, message| + attribute = "#{reflection.name}_#{attribute}" + errors.add(attribute, message) unless errors.on(attribute) + end + else + errors.add(reflection.name) + end + end + valid + end + + # Is used as a before_save callback to check while saving a collection + # association whether or not the parent was a new record before saving. + def before_save_collection_association + @new_record_before_save = new_record? + true + end + + # Saves any new associated records, or all loaded autosave associations if + # <tt>:autosave</tt> is enabled on the association. + # + # In addition, it destroys all children that were marked for destruction + # with mark_for_destruction. + # + # This all happens inside a transaction, _if_ the Transactions module is included into + # ActiveRecord::Base after the AutosaveAssociation module, which it does by default. + def save_collection_association(reflection) + if association = association_instance_get(reflection.name) + autosave = reflection.options[:autosave] + + if records = associated_records_to_validate_or_save(association, @new_record_before_save, autosave) + records.each do |record| + if autosave && record.marked_for_destruction? + record.destroy + elsif @new_record_before_save || record.new_record? + if autosave + association.send(:insert_record, record, false, false) + else + association.send(:insert_record, record) + end + elsif autosave + record.save(false) + end + end + end + + # reconstruct the SQL queries now that we know the owner's id + association.send(:construct_sql) if association.respond_to?(:construct_sql) + end + end + + # Saves the associated record if it's new or <tt>:autosave</tt> is enabled + # on the association. + # + # In addition, it will destroy the association if it was marked for + # destruction with mark_for_destruction. + # + # This all happens inside a transaction, _if_ the Transactions module is included into + # ActiveRecord::Base after the AutosaveAssociation module, which it does by default. + def save_has_one_association(reflection) + if association = association_instance_get(reflection.name) + if reflection.options[:autosave] && association.marked_for_destruction? + association.destroy + elsif new_record? || association.new_record? || association[reflection.primary_key_name] != id || reflection.options[:autosave] + association[reflection.primary_key_name] = id + association.save(false) + end + end + end + + # Saves the associated record if it's new or <tt>:autosave</tt> is enabled + # on the association. + # + # In addition, it will destroy the association if it was marked for + # destruction with mark_for_destruction. + # + # This all happens inside a transaction, _if_ the Transactions module is included into + # ActiveRecord::Base after the AutosaveAssociation module, which it does by default. + def save_belongs_to_association(reflection) + if association = association_instance_get(reflection.name) + if reflection.options[:autosave] && association.marked_for_destruction? + association.destroy + else + association.save(false) if association.new_record? || reflection.options[:autosave] + + if association.updated? + self[reflection.primary_key_name] = association.id + # TODO: Removing this code doesn't seem to matter… + if reflection.options[:polymorphic] + self[reflection.options[:foreign_type]] = association.class.base_class.name.to_s + end + end + end + end + end end end
\ No newline at end of file diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 40a8503980..92ed465ae7 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -190,19 +190,6 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_equal 1, Topic.find(topic.id).send(:read_attribute, "replies_count") end - def test_assignment_before_parent_saved - client = Client.find(:first) - apple = Firm.new("name" => "Apple") - client.firm = apple - assert_equal apple, client.firm - assert apple.new_record? - assert client.save - assert apple.save - assert !apple.new_record? - assert_equal apple, client.firm - assert_equal apple, client.firm(true) - end - def test_assignment_before_child_saved final_cut = Client.new("name" => "Final Cut") firm = Firm.find(1) @@ -215,19 +202,6 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_equal firm, final_cut.firm(true) end - def test_assignment_before_either_saved - final_cut = Client.new("name" => "Final Cut") - apple = Firm.new("name" => "Apple") - final_cut.firm = apple - assert final_cut.new_record? - assert apple.new_record? - assert final_cut.save - assert !final_cut.new_record? - assert !apple.new_record? - assert_equal apple, final_cut.firm - assert_equal apple, final_cut.firm(true) - end - def test_new_record_with_foreign_key_but_no_object c = Client.new("firm_id" => 1) assert_equal Firm.find(:first), c.firm_with_basic_id @@ -274,90 +248,6 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_equal 17, reply.replies.size end - def test_store_two_association_with_one_save - num_orders = Order.count - num_customers = Customer.count - order = Order.new - - customer1 = order.billing = Customer.new - customer2 = order.shipping = Customer.new - assert order.save - assert_equal customer1, order.billing - assert_equal customer2, order.shipping - - order.reload - - assert_equal customer1, order.billing - assert_equal customer2, order.shipping - - assert_equal num_orders +1, Order.count - assert_equal num_customers +2, Customer.count - end - - - def test_store_association_in_two_relations_with_one_save - num_orders = Order.count - num_customers = Customer.count - order = Order.new - - customer = order.billing = order.shipping = Customer.new - assert order.save - assert_equal customer, order.billing - assert_equal customer, order.shipping - - order.reload - - assert_equal customer, order.billing - assert_equal customer, order.shipping - - assert_equal num_orders +1, Order.count - assert_equal num_customers +1, Customer.count - end - - def test_store_association_in_two_relations_with_one_save_in_existing_object - num_orders = Order.count - num_customers = Customer.count - order = Order.create - - customer = order.billing = order.shipping = Customer.new - assert order.save - assert_equal customer, order.billing - assert_equal customer, order.shipping - - order.reload - - assert_equal customer, order.billing - assert_equal customer, order.shipping - - assert_equal num_orders +1, Order.count - assert_equal num_customers +1, Customer.count - end - - def test_store_association_in_two_relations_with_one_save_in_existing_object_with_values - num_orders = Order.count - num_customers = Customer.count - order = Order.create - - customer = order.billing = order.shipping = Customer.new - assert order.save - assert_equal customer, order.billing - assert_equal customer, order.shipping - - order.reload - - customer = order.billing = order.shipping = Customer.new - - assert order.save - order.reload - - assert_equal customer, order.billing - assert_equal customer, order.shipping - - assert_equal num_orders +1, Order.count - assert_equal num_customers +2, Customer.count - end - - def test_association_assignment_sticks post = Post.find(:first) @@ -410,25 +300,6 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_equal nil, sponsor.sponsorable_id end - def test_save_fails_for_invalid_belongs_to - assert log = AuditLog.create(:developer_id=>0,:message=>"") - - log.developer = Developer.new - assert !log.developer.valid? - assert !log.valid? - assert !log.save - assert_equal "is invalid", log.errors.on("developer") - end - - def test_save_succeeds_for_invalid_belongs_to_with_validate_false - assert log = AuditLog.create(:developer_id=>0,:message=>"") - - log.unvalidated_developer = Developer.new - assert !log.unvalidated_developer.valid? - assert log.valid? - assert log.save - end - def test_belongs_to_proxy_should_not_respond_to_private_methods assert_raises(NoMethodError) { companies(:first_firm).private_method } assert_raises(NoMethodError) { companies(:second_client).firm.private_method } diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index a2525f1d70..5efbc5bd0c 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -317,81 +317,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 3, companies(:first_firm).clients_of_firm(true).size end - def test_adding_before_save - no_of_firms = Firm.count - no_of_clients = Client.count - - new_firm = Firm.new("name" => "A New Firm, Inc") - c = Client.new("name" => "Apple") - - new_firm.clients_of_firm.push Client.new("name" => "Natural Company") - assert_equal 1, new_firm.clients_of_firm.size - new_firm.clients_of_firm << c - assert_equal 2, new_firm.clients_of_firm.size - - assert_equal no_of_firms, Firm.count # Firm was not saved to database. - assert_equal no_of_clients, Client.count # Clients were not saved to database. - assert new_firm.save - assert !new_firm.new_record? - assert !c.new_record? - assert_equal new_firm, c.firm - assert_equal no_of_firms+1, Firm.count # Firm was saved to database. - assert_equal no_of_clients+2, Client.count # Clients were saved to database. - - assert_equal 2, new_firm.clients_of_firm.size - assert_equal 2, new_firm.clients_of_firm(true).size - end - - def test_invalid_adding - firm = Firm.find(1) - assert !(firm.clients_of_firm << c = Client.new) - assert c.new_record? - assert !firm.valid? - assert !firm.save - assert c.new_record? - end - - def test_invalid_adding_before_save - no_of_firms = Firm.count - no_of_clients = Client.count - new_firm = Firm.new("name" => "A New Firm, Inc") - new_firm.clients_of_firm.concat([c = Client.new, Client.new("name" => "Apple")]) - assert c.new_record? - assert !c.valid? - assert !new_firm.valid? - assert !new_firm.save - assert c.new_record? - assert new_firm.new_record? - end - - def test_invalid_adding_with_validate_false - firm = Firm.find(:first) - client = Client.new - firm.unvalidated_clients_of_firm << client - - assert firm.valid? - assert !client.valid? - assert firm.save - assert client.new_record? - end - - def test_valid_adding_with_validate_false - no_of_clients = Client.count - - firm = Firm.find(:first) - client = Client.new("name" => "Apple") - - assert firm.valid? - assert client.valid? - assert client.new_record? - - firm.unvalidated_clients_of_firm << client - - assert firm.save - assert !client.new_record? - assert_equal no_of_clients+1, Client.count - end - def test_build company = companies(:first_firm) new_client = assert_no_queries { company.clients_of_firm.build("name" => "Another Client") } @@ -400,10 +325,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal "Another Client", new_client.name assert new_client.new_record? assert_equal new_client, company.clients_of_firm.last - company.name += '-changed' - assert_queries(2) { assert company.save } - assert !new_client.new_record? - assert_equal 2, company.clients_of_firm(true).size end def test_collection_size_after_building @@ -428,11 +349,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_build_many company = companies(:first_firm) new_clients = assert_no_queries { company.clients_of_firm.build([{"name" => "Another Client"}, {"name" => "Another Client II"}]) } - assert_equal 2, new_clients.size - company.name += '-changed' - assert_queries(3) { assert company.save } - assert_equal 3, company.clients_of_firm(true).size end def test_build_followed_by_save_does_not_load_target @@ -463,10 +380,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal "Another Client", new_client.name assert new_client.new_record? assert_equal new_client, company.clients_of_firm.last - company.name += '-changed' - assert_queries(2) { assert company.save } - assert !new_client.new_record? - assert_equal 2, company.clients_of_firm(true).size end def test_build_many_via_block @@ -480,10 +393,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 2, new_clients.size assert_equal "changed", new_clients.first.name assert_equal "changed", new_clients.last.name - - company.name += '-changed' - assert_queries(3) { assert company.save } - assert_equal 3, company.clients_of_firm(true).size end def test_create_without_loading_association @@ -501,16 +410,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 2, first_firm.clients_of_firm.size end - def test_invalid_build - new_client = companies(:first_firm).clients_of_firm.build - assert new_client.new_record? - assert !new_client.valid? - assert_equal new_client, companies(:first_firm).clients_of_firm.last - assert !companies(:first_firm).save - assert new_client.new_record? - assert_equal 1, companies(:first_firm).clients_of_firm(true).size - end - def test_create force_signal37_to_load_all_clients_of_firm new_client = companies(:first_firm).clients_of_firm.create("name" => "Another Client") @@ -843,15 +742,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert !firm.clients.include?(:first_client) end - def test_replace_on_new_object - firm = Firm.new("name" => "New Firm") - firm.clients = [companies(:second_client), Client.new("name" => "New Client")] - assert firm.save - firm.reload - assert_equal 2, firm.clients.length - assert firm.clients.include?(Client.find_by_name("New Client")) - end - def test_get_ids assert_equal [companies(:first_client).id, companies(:second_client).id], companies(:first_firm).client_ids end @@ -879,15 +769,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert company.clients_using_sql.loaded? end - def test_assign_ids - firm = Firm.new("name" => "Apple") - firm.client_ids = [companies(:first_client).id, companies(:second_client).id] - firm.save - firm.reload - assert_equal 2, firm.clients.length - assert firm.clients.include?(companies(:second_client)) - end - def test_assign_ids_ignoring_blanks firm = Firm.create!(:name => 'Apple') firm.client_ids = [companies(:first_client).id, nil, companies(:second_client).id, ''] @@ -910,16 +791,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase ].each {|block| assert_raise(ActiveRecord::HasManyThroughCantAssociateThroughHasManyReflection, &block) } end - - def test_assign_ids_for_through_a_belongs_to - post = Post.new(:title => "Assigning IDs works!", :body => "You heared it here first, folks!") - post.person_ids = [people(:david).id, people(:michael).id] - post.save - post.reload - assert_equal 2, post.people.length - assert post.people.include?(people(:david)) - end - def test_dynamic_find_should_respect_association_order_for_through assert_equal Comment.find(10), authors(:david).comments_desc.find(:first, :conditions => "comments.type = 'SpecialComment'") assert_equal Comment.find(10), authors(:david).comments_desc.find_by_type('SpecialComment') diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index 14032a67c0..e81bba790a 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -193,28 +193,6 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert_equal account, firm.account end - def test_build_before_child_saved - firm = Firm.find(1) - - account = firm.account.build("credit_limit" => 1000) - assert_equal account, firm.account - assert account.new_record? - assert firm.save - assert_equal account, firm.account - assert !account.new_record? - end - - def test_build_before_either_saved - firm = Firm.new("name" => "GlobalMegaCorp") - - firm.account = account = Account.new("credit_limit" => 1000) - assert_equal account, firm.account - assert account.new_record? - assert firm.save - assert_equal account, firm.account - assert !account.new_record? - end - def test_failing_build_association firm = Firm.new("name" => "GlobalMegaCorp") firm.save @@ -253,16 +231,6 @@ class HasOneAssociationsTest < ActiveRecord::TestCase firm.destroy end - def test_assignment_before_parent_saved - firm = Firm.new("name" => "GlobalMegaCorp") - firm.account = a = Account.find(1) - assert firm.new_record? - assert_equal a, firm.account - assert firm.save - assert_equal a, firm.account - assert_equal a, firm.account(true) - end - def test_finding_with_interpolated_condition firm = Firm.find(:first) superior = firm.clients.create(:name => 'SuperiorCo') @@ -279,61 +247,6 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert_equal a, firm.account assert_equal a, firm.account(true) end - - def test_save_fails_for_invalid_has_one - firm = Firm.find(:first) - assert firm.valid? - - firm.account = Account.new - - assert !firm.account.valid? - assert !firm.valid? - assert !firm.save - assert_equal "is invalid", firm.errors.on("account") - end - - - def test_save_succeeds_for_invalid_has_one_with_validate_false - firm = Firm.find(:first) - assert firm.valid? - - firm.unvalidated_account = Account.new - - assert !firm.unvalidated_account.valid? - assert firm.valid? - assert firm.save - end - - def test_assignment_before_either_saved - firm = Firm.new("name" => "GlobalMegaCorp") - firm.account = a = Account.new("credit_limit" => 1000) - assert firm.new_record? - assert a.new_record? - assert_equal a, firm.account - assert firm.save - assert !firm.new_record? - assert !a.new_record? - assert_equal a, firm.account - assert_equal a, firm.account(true) - end - - def test_not_resaved_when_unchanged - firm = Firm.find(:first, :include => :account) - firm.name += '-changed' - assert_queries(1) { firm.save! } - - firm = Firm.find(:first) - firm.account = Account.find(:first) - assert_queries(Firm.partial_updates? ? 0 : 1) { firm.save! } - - firm = Firm.find(:first).clone - firm.account = Account.find(:first) - assert_queries(2) { firm.save! } - - firm = Firm.find(:first).clone - firm.account = Account.find(:first).clone - assert_queries(2) { firm.save! } - end def test_save_still_works_after_accessing_nil_has_one jp = Company.new :name => 'Jaded Pixel' diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 381249c0c2..6ced84e0b6 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -1,10 +1,20 @@ -require "cases/helper" -require "models/pirate" -require "models/ship" -require "models/ship_part" -require "models/bird" -require "models/parrot" -require "models/treasure" +require 'cases/helper' +require 'models/bird' +require 'models/company' +require 'models/customer' +require 'models/developer' +require 'models/order' +require 'models/parrot' +require 'models/person' +require 'models/pirate' +require 'models/post' +require 'models/reader' +require 'models/ship' +require 'models/ship_part' +require 'models/treasure' + +# TODO: +# - add test case for new parent and children with invalid data and saving with validate = false class TestAutosaveAssociationsInGeneral < ActiveRecord::TestCase def test_autosave_should_be_a_valid_option_for_has_one @@ -30,6 +40,383 @@ class TestAutosaveAssociationsInGeneral < ActiveRecord::TestCase end end +class TestDefaultAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCase + def test_save_fails_for_invalid_has_one + firm = Firm.find(:first) + assert firm.valid? + + firm.account = Account.new + + assert !firm.account.valid? + assert !firm.valid? + assert !firm.save + assert_equal "is invalid", firm.errors.on("account") + end + + def test_save_succeeds_for_invalid_has_one_with_validate_false + firm = Firm.find(:first) + assert firm.valid? + + firm.unvalidated_account = Account.new + + assert !firm.unvalidated_account.valid? + assert firm.valid? + assert firm.save + end + + def test_build_before_child_saved + firm = Firm.find(1) + + account = firm.account.build("credit_limit" => 1000) + assert_equal account, firm.account + assert account.new_record? + assert firm.save + assert_equal account, firm.account + assert !account.new_record? + end + + def test_build_before_either_saved + firm = Firm.new("name" => "GlobalMegaCorp") + + firm.account = account = Account.new("credit_limit" => 1000) + assert_equal account, firm.account + assert account.new_record? + assert firm.save + assert_equal account, firm.account + assert !account.new_record? + end + + def test_assignment_before_parent_saved + firm = Firm.new("name" => "GlobalMegaCorp") + firm.account = a = Account.find(1) + assert firm.new_record? + assert_equal a, firm.account + assert firm.save + assert_equal a, firm.account + assert_equal a, firm.account(true) + end + + def test_assignment_before_either_saved + firm = Firm.new("name" => "GlobalMegaCorp") + firm.account = a = Account.new("credit_limit" => 1000) + assert firm.new_record? + assert a.new_record? + assert_equal a, firm.account + assert firm.save + assert !firm.new_record? + assert !a.new_record? + assert_equal a, firm.account + assert_equal a, firm.account(true) + end + + def test_not_resaved_when_unchanged + firm = Firm.find(:first, :include => :account) + firm.name += '-changed' + assert_queries(1) { firm.save! } + + firm = Firm.find(:first) + firm.account = Account.find(:first) + assert_queries(Firm.partial_updates? ? 0 : 1) { firm.save! } + + firm = Firm.find(:first).clone + firm.account = Account.find(:first) + assert_queries(2) { firm.save! } + + firm = Firm.find(:first).clone + firm.account = Account.find(:first).clone + assert_queries(2) { firm.save! } + end +end + +class TestDefaultAutosaveAssociationOnABelongsToAssociation < ActiveRecord::TestCase + def test_save_fails_for_invalid_belongs_to + assert log = AuditLog.create(:developer_id => 0, :message => "") + + log.developer = Developer.new + assert !log.developer.valid? + assert !log.valid? + assert !log.save + assert_equal "is invalid", log.errors.on("developer") + end + + def test_save_succeeds_for_invalid_belongs_to_with_validate_false + assert log = AuditLog.create(:developer_id => 0, :message=> "") + + log.unvalidated_developer = Developer.new + assert !log.unvalidated_developer.valid? + assert log.valid? + assert log.save + end + + def test_assignment_before_parent_saved + client = Client.find(:first) + apple = Firm.new("name" => "Apple") + client.firm = apple + assert_equal apple, client.firm + assert apple.new_record? + assert client.save + assert apple.save + assert !apple.new_record? + assert_equal apple, client.firm + assert_equal apple, client.firm(true) + end + + def test_assignment_before_either_saved + final_cut = Client.new("name" => "Final Cut") + apple = Firm.new("name" => "Apple") + final_cut.firm = apple + assert final_cut.new_record? + assert apple.new_record? + assert final_cut.save + assert !final_cut.new_record? + assert !apple.new_record? + assert_equal apple, final_cut.firm + assert_equal apple, final_cut.firm(true) + end + + def test_store_two_association_with_one_save + num_orders = Order.count + num_customers = Customer.count + order = Order.new + + customer1 = order.billing = Customer.new + customer2 = order.shipping = Customer.new + assert order.save + assert_equal customer1, order.billing + assert_equal customer2, order.shipping + + order.reload + + assert_equal customer1, order.billing + assert_equal customer2, order.shipping + + assert_equal num_orders +1, Order.count + assert_equal num_customers +2, Customer.count + end + + def test_store_association_in_two_relations_with_one_save + num_orders = Order.count + num_customers = Customer.count + order = Order.new + + customer = order.billing = order.shipping = Customer.new + assert order.save + assert_equal customer, order.billing + assert_equal customer, order.shipping + + order.reload + + assert_equal customer, order.billing + assert_equal customer, order.shipping + + assert_equal num_orders +1, Order.count + assert_equal num_customers +1, Customer.count + end + + def test_store_association_in_two_relations_with_one_save_in_existing_object + num_orders = Order.count + num_customers = Customer.count + order = Order.create + + customer = order.billing = order.shipping = Customer.new + assert order.save + assert_equal customer, order.billing + assert_equal customer, order.shipping + + order.reload + + assert_equal customer, order.billing + assert_equal customer, order.shipping + + assert_equal num_orders +1, Order.count + assert_equal num_customers +1, Customer.count + end + + def test_store_association_in_two_relations_with_one_save_in_existing_object_with_values + num_orders = Order.count + num_customers = Customer.count + order = Order.create + + customer = order.billing = order.shipping = Customer.new + assert order.save + assert_equal customer, order.billing + assert_equal customer, order.shipping + + order.reload + + customer = order.billing = order.shipping = Customer.new + + assert order.save + order.reload + + assert_equal customer, order.billing + assert_equal customer, order.shipping + + assert_equal num_orders +1, Order.count + assert_equal num_customers +2, Customer.count + end +end + +class TestDefaultAutosaveAssociationOnAHasManyAssociation < ActiveRecord::TestCase + fixtures :companies, :people + + def test_invalid_adding + firm = Firm.find(1) + assert !(firm.clients_of_firm << c = Client.new) + assert c.new_record? + assert !firm.valid? + assert !firm.save + assert c.new_record? + end + + def test_invalid_adding_before_save + no_of_firms = Firm.count + no_of_clients = Client.count + new_firm = Firm.new("name" => "A New Firm, Inc") + new_firm.clients_of_firm.concat([c = Client.new, Client.new("name" => "Apple")]) + assert c.new_record? + assert !c.valid? + assert !new_firm.valid? + assert !new_firm.save + assert c.new_record? + assert new_firm.new_record? + end + + def test_invalid_adding_with_validate_false + firm = Firm.find(:first) + client = Client.new + firm.unvalidated_clients_of_firm << client + + assert firm.valid? + assert !client.valid? + assert firm.save + assert client.new_record? + end + + def test_valid_adding_with_validate_false + no_of_clients = Client.count + + firm = Firm.find(:first) + client = Client.new("name" => "Apple") + + assert firm.valid? + assert client.valid? + assert client.new_record? + + firm.unvalidated_clients_of_firm << client + + assert firm.save + assert !client.new_record? + assert_equal no_of_clients+1, Client.count + end + + def test_invalid_build + new_client = companies(:first_firm).clients_of_firm.build + assert new_client.new_record? + assert !new_client.valid? + assert_equal new_client, companies(:first_firm).clients_of_firm.last + assert !companies(:first_firm).save + assert new_client.new_record? + assert_equal 1, companies(:first_firm).clients_of_firm(true).size + end + + def test_adding_before_save + no_of_firms = Firm.count + no_of_clients = Client.count + + new_firm = Firm.new("name" => "A New Firm, Inc") + c = Client.new("name" => "Apple") + + new_firm.clients_of_firm.push Client.new("name" => "Natural Company") + assert_equal 1, new_firm.clients_of_firm.size + new_firm.clients_of_firm << c + assert_equal 2, new_firm.clients_of_firm.size + + assert_equal no_of_firms, Firm.count # Firm was not saved to database. + assert_equal no_of_clients, Client.count # Clients were not saved to database. + assert new_firm.save + assert !new_firm.new_record? + assert !c.new_record? + assert_equal new_firm, c.firm + assert_equal no_of_firms+1, Firm.count # Firm was saved to database. + assert_equal no_of_clients+2, Client.count # Clients were saved to database. + + assert_equal 2, new_firm.clients_of_firm.size + assert_equal 2, new_firm.clients_of_firm(true).size + end + + def test_assign_ids + firm = Firm.new("name" => "Apple") + firm.client_ids = [companies(:first_client).id, companies(:second_client).id] + firm.save + firm.reload + assert_equal 2, firm.clients.length + assert firm.clients.include?(companies(:second_client)) + end + + def test_assign_ids_for_through_a_belongs_to + post = Post.new(:title => "Assigning IDs works!", :body => "You heared it here first, folks!") + post.person_ids = [people(:david).id, people(:michael).id] + post.save + post.reload + assert_equal 2, post.people.length + assert post.people.include?(people(:david)) + end + + def test_build_before_save + company = companies(:first_firm) + new_client = assert_no_queries { company.clients_of_firm.build("name" => "Another Client") } + assert !company.clients_of_firm.loaded? + + company.name += '-changed' + assert_queries(2) { assert company.save } + assert !new_client.new_record? + assert_equal 2, company.clients_of_firm(true).size + end + + def test_build_many_before_save + company = companies(:first_firm) + new_clients = assert_no_queries { company.clients_of_firm.build([{"name" => "Another Client"}, {"name" => "Another Client II"}]) } + + company.name += '-changed' + assert_queries(3) { assert company.save } + assert_equal 3, company.clients_of_firm(true).size + end + + def test_build_via_block_before_save + company = companies(:first_firm) + new_client = assert_no_queries { company.clients_of_firm.build {|client| client.name = "Another Client" } } + assert !company.clients_of_firm.loaded? + + company.name += '-changed' + assert_queries(2) { assert company.save } + assert !new_client.new_record? + assert_equal 2, company.clients_of_firm(true).size + end + + def test_build_many_via_block_before_save + company = companies(:first_firm) + new_clients = assert_no_queries do + company.clients_of_firm.build([{"name" => "Another Client"}, {"name" => "Another Client II"}]) do |client| + client.name = "changed" + end + end + + company.name += '-changed' + assert_queries(3) { assert company.save } + assert_equal 3, company.clients_of_firm(true).size + end + + def test_replace_on_new_object + firm = Firm.new("name" => "New Firm") + firm.clients = [companies(:second_client), Client.new("name" => "New Client")] + assert firm.save + firm.reload + assert_equal 2, firm.clients.length + assert firm.clients.include?(Client.find_by_name("New Client")) + end +end + class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase self.use_transactional_fixtures = false @@ -181,6 +568,14 @@ class TestAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCase assert !@pirate.errors.on(:ship_name).blank? end + def test_should_merge_errors_on_the_associated_models_onto_the_parent_even_if_it_is_not_valid + @pirate.ship.name = nil + @pirate.catchphrase = nil + assert !@pirate.valid? + assert !@pirate.errors.on(:ship_name).blank? + assert !@pirate.errors.on(:catchphrase).blank? + end + def test_should_still_allow_to_bypass_validations_on_the_associated_model @pirate.catchphrase = '' @pirate.ship.name = '' @@ -263,6 +658,14 @@ class TestAutosaveAssociationOnABelongsToAssociation < ActiveRecord::TestCase assert !@ship.errors.on(:pirate_catchphrase).blank? end + def test_should_merge_errors_on_the_associated_model_onto_the_parent_even_if_it_is_not_valid + @ship.name = nil + @ship.pirate.catchphrase = nil + assert !@ship.valid? + assert !@ship.errors.on(:name).blank? + assert !@ship.errors.on(:pirate_catchphrase).blank? + end + def test_should_still_allow_to_bypass_validations_on_the_associated_model @ship.pirate.catchphrase = '' @ship.name = '' @@ -326,7 +729,24 @@ module AutosaveAssociationOnACollectionAssociationTests assert @pirate.errors.on(@association_name).blank? end - def test_should_still_allow_to_bypass_validations_on_the_associated_models + def test_should_not_use_default_invalid_error_on_associated_models + @pirate.send(@association_name).build(:name => '') + + assert !@pirate.valid? + assert_equal "can't be blank", @pirate.errors.on("#{@association_name}_name") + assert @pirate.errors.on(@association_name).blank? + end + + def test_should_merge_errors_on_the_associated_models_onto_the_parent_even_if_it_is_not_valid + @pirate.send(@association_name).each { |child| child.name = '' } + @pirate.catchphrase = nil + + assert !@pirate.valid? + assert_equal "can't be blank", @pirate.errors.on("#{@association_name}_name") + assert !@pirate.errors.on(:catchphrase).blank? + end + + def test_should_allow_to_bypass_validations_on_the_associated_models_on_update @pirate.catchphrase = '' @pirate.send(@association_name).each { |child| child.name = '' } @@ -338,6 +758,20 @@ module AutosaveAssociationOnACollectionAssociationTests ] end + def test_should_validation_the_associated_models_on_create + assert_no_difference("#{ @association_name == :birds ? 'Bird' : 'Parrot' }.count") do + 2.times { @pirate.send(@association_name).build } + @pirate.save(true) + end + end + + def test_should_allow_to_bypass_validations_on_the_associated_models_on_create + assert_difference("#{ @association_name == :birds ? 'Bird' : 'Parrot' }.count", +2) do + 2.times { @pirate.send(@association_name).build } + @pirate.save(false) + end + end + def test_should_rollback_any_changes_if_an_exception_occurred_while_saving before = [@pirate.catchphrase, *@pirate.send(@association_name).map(&:name)] new_names = ['Grace OMalley', 'Privateers Greed'] |