From 1b22071b276a2a2b3e65e6db40c6a0aa92d9ebc3 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 24 Feb 2009 11:34:32 -0600 Subject: Ensure ActiveRecord session store's connections are checked in after each request [#1927 state:resolved] --- .../connection_adapters/abstract/connection_pool.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 901b17124c..aac84cc5f4 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -351,5 +351,21 @@ module ActiveRecord retrieve_connection_pool klass.superclass end end + + class ConnectionManagement + def initialize(app) + @app = app + end + + def call(env) + @app.call(env) + ensure + # Don't return connection (and peform implicit rollback) if + # this request is a part of integration test + unless env.key?("rack.test") + ActiveRecord::Base.clear_active_connections! + end + end + end end end -- cgit v1.2.3 From 0dd2f96f5c90f8abacb0fe0757ef7e5db4a4d501 Mon Sep 17 00:00:00 2001 From: Diego Algorta Date: Fri, 13 Feb 2009 03:07:39 -0300 Subject: Fixed bug that makes named_scopes _forgot_ current scope --- activerecord/lib/active_record/named_scope.rb | 13 +++++++++++-- activerecord/test/cases/named_scope_test.rb | 9 +++++++++ activerecord/test/models/post.rb | 6 ++++++ 3 files changed, 26 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 989b2a1ec5..022fdaf9b2 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -100,7 +100,7 @@ module ActiveRecord end class Scope - attr_reader :proxy_scope, :proxy_options + attr_reader :proxy_scope, :proxy_options, :current_scoped_methods_when_defined NON_DELEGATE_METHODS = %w(nil? send object_id class extend find size count sum average maximum minimum paginate first last empty? any? respond_to?).to_set [].methods.each do |m| unless m =~ /^__/ || NON_DELEGATE_METHODS.include?(m.to_s) @@ -113,6 +113,9 @@ module ActiveRecord def initialize(proxy_scope, options, &block) [options[:extend]].flatten.each { |extension| extend extension } if options[:extend] extend Module.new(&block) if block_given? + unless Scope === proxy_scope + @current_scoped_methods_when_defined = proxy_scope.send(:current_scoped_methods) + end @proxy_scope, @proxy_options = proxy_scope, options.except(:extend) end @@ -168,7 +171,13 @@ module ActiveRecord else with_scope :find => proxy_options, :create => proxy_options[:conditions].is_a?(Hash) ? proxy_options[:conditions] : {} do method = :new if method == :build - proxy_scope.send(method, *args, &block) + if current_scoped_methods_when_defined + with_scope current_scoped_methods_when_defined do + proxy_scope.send(method, *args, &block) + end + else + proxy_scope.send(method, *args, &block) + end end end end diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index e1e27fa130..f28285faaf 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -142,6 +142,15 @@ class NamedScopeTest < ActiveRecord::TestCase assert_equal authors(:david).comments & Comment.containing_the_letter_e, authors(:david).comments.containing_the_letter_e end + def test_named_scopes_honor_current_scopes_from_when_defined + assert !Post.ranked_by_comments.limit(5).empty? + assert !authors(:david).posts.ranked_by_comments.limit(5).empty? + assert_not_equal Post.ranked_by_comments.limit(5), authors(:david).posts.ranked_by_comments.limit(5) + assert_not_equal Post.top(5), authors(:david).posts.top(5) + assert_equal authors(:david).posts.ranked_by_comments.limit(5), authors(:david).posts.top(5) + assert_equal Post.ranked_by_comments.limit(5), Post.top(5) + end + def test_active_records_have_scope_named__all__ assert !Topic.find(:all).empty? diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 388fff8fba..374e536a5b 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -1,5 +1,7 @@ class Post < ActiveRecord::Base named_scope :containing_the_letter_a, :conditions => "body LIKE '%a%'" + named_scope :ranked_by_comments, :order => "comments_count DESC" + named_scope :limit, lambda {|limit| {:limit => limit} } named_scope :with_authors_at_address, lambda { |address| { :conditions => [ 'authors.author_address_id = ?', address.id ], :joins => 'JOIN authors ON authors.id = posts.author_id' @@ -68,6 +70,10 @@ class Post < ActiveRecord::Base :before_remove => lambda {|owner, reader| log(:removed, :before, reader.first_name) }, :after_remove => lambda {|owner, reader| log(:removed, :after, reader.first_name) } + def self.top(limit) + ranked_by_comments.limit(limit) + end + def self.reset_log @log = [] end -- cgit v1.2.3 From 1cfc39e7e7ab72db1f797fbf21cdd149edfadd22 Mon Sep 17 00:00:00 2001 From: rick Date: Wed, 25 Feb 2009 09:27:44 -0800 Subject: update the default scope test to take the model's default scope into account --- activerecord/test/cases/method_scoping_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb index 71e2ce8790..c676c1c72b 100644 --- a/activerecord/test/cases/method_scoping_test.rb +++ b/activerecord/test/cases/method_scoping_test.rb @@ -597,7 +597,7 @@ class DefaultScopingTest < ActiveRecord::TestCase end def test_named_scope - expected = Developer.find(:all, :order => 'name DESC').collect { |dev| dev.salary } + expected = Developer.find(:all, :order => 'salary DESC, name DESC').collect { |dev| dev.salary } received = DeveloperOrderedBySalary.by_name.find(:all).collect { |dev| dev.salary } assert_equal expected, received end -- cgit v1.2.3 From 809409d92fa57555fe0b3a148e41d2a47a1551d2 Mon Sep 17 00:00:00 2001 From: Manfred Stienstra Date: Wed, 25 Feb 2009 10:29:22 +0100 Subject: Add test case for ActiveRecord::ConnectionAdapters::ConnectionManagement. - Make sure ConnectionManagement clears connections after each call, except in tests Signed-off-by: Joshua Peek --- activerecord/test/cases/connection_pool_test.rb | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 activerecord/test/cases/connection_pool_test.rb (limited to 'activerecord') diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb new file mode 100644 index 0000000000..cc9b2a45f4 --- /dev/null +++ b/activerecord/test/cases/connection_pool_test.rb @@ -0,0 +1,25 @@ +require "cases/helper" + +class ConnectionManagementTest < ActiveRecord::TestCase + def setup + @env = {} + @app = stub('App') + @management = ActiveRecord::ConnectionAdapters::ConnectionManagement.new(@app) + + @connections_cleared = false + ActiveRecord::Base.stubs(:clear_active_connections!).with { @connections_cleared = true } + end + + test "clears active connections after each call" do + @app.expects(:call).with(@env) + @management.call(@env) + assert @connections_cleared + end + + test "doesn't clear active connections when running in a test case" do + @env['rack.test'] = true + @app.expects(:call).with(@env) + @management.call(@env) + assert !@connections_cleared + end +end \ No newline at end of file -- cgit v1.2.3 From 5cda000bf0f6d85d1a1efedf9fa4d0b6eaf988a1 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Fri, 27 Feb 2009 13:50:24 +0100 Subject: Fixed that autosave should validate associations even if master is invalid [#1930 status:committed] --- activerecord/lib/active_record/associations.rb | 120 +----- .../has_and_belongs_to_many_association.rb | 4 +- .../associations/has_many_association.rb | 4 +- .../associations/has_many_through_association.rb | 4 +- .../lib/active_record/autosave_association.rb | 237 ++++++++--- .../associations/belongs_to_associations_test.rb | 129 ------ .../associations/has_many_associations_test.rb | 129 ------ .../associations/has_one_associations_test.rb | 87 ---- .../test/cases/autosave_association_test.rb | 450 ++++++++++++++++++++- 9 files changed, 629 insertions(+), 535 deletions(-) (limited to 'activerecord') 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 has_one :through queries. Only use it if the name cannot be # inferred from the association. has_one :favorite, :through => :favorites will look for a - # :favorite on Favorite, unless a :source is given. + # :favorite on Favorite, unless a :source is given. # [:source_type] # Specifies type of the source association used by has_one :through 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 :dependent 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, self, 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, self, 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, self, 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 :validate or :autosave 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 :validate or + # :autosave 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, self, 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 + # :autosave 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 :autosave 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 :autosave 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'] -- cgit v1.2.3 From e2b925fa68203607fb4f9a9aa457c0435c969f9a Mon Sep 17 00:00:00 2001 From: Eloy Duran Date: Thu, 26 Feb 2009 11:56:27 +0100 Subject: Don't validate records of an :autosave association if they're marked for destruction. [#2064 status:resolved] Signed-off-by: David Heinemeier Hansson --- .../lib/active_record/autosave_association.rb | 11 +++++--- .../test/cases/autosave_association_test.rb | 30 +++++++++++++++++++--- 2 files changed, 34 insertions(+), 7 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 6e09b13150..1c3d0567c1 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -243,13 +243,16 @@ module ActiveRecord end # Returns whether or not the association is valid and applies any errors to - # the parent, self, if it wasn't. + # the parent, self, if it wasn't. Skips any :autosave + # enabled records if they're marked_for_destruction?. 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) + unless association.marked_for_destruction? + association.errors.each do |attribute, message| + attribute = "#{reflection.name}_#{attribute}" + errors.add(attribute, message) unless errors.on(attribute) + end end else errors.add(reflection.name) diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 6ced84e0b6..b179bd827a 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -13,9 +13,6 @@ 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 assert base.valid_keys_for_has_one_association.include?(:autosave) @@ -449,6 +446,14 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase assert_nil Ship.find_by_id(id) end + def test_should_skip_validation_on_a_child_association_if_marked_for_destruction + @pirate.ship.name = '' + assert !@pirate.valid? + + @pirate.ship.mark_for_destruction + assert_difference('Ship.count', -1) { @pirate.save! } + end + def test_should_rollback_destructions_if_an_exception_occurred_while_saving_a_child # Stub the save method of the @pirate.ship instance to destroy and then raise an exception class << @pirate.ship @@ -478,6 +483,14 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase assert_nil Pirate.find_by_id(id) end + def test_should_skip_validation_on_a_parent_association_if_marked_for_destruction + @ship.pirate.catchphrase = '' + assert !@ship.valid? + + @ship.pirate.mark_for_destruction + assert_difference('Pirate.count', -1) { @ship.save! } + end + def test_should_rollback_destructions_if_an_exception_occurred_while_saving_a_parent # Stub the save method of the @ship.pirate instance to destroy and then raise an exception class << @ship.pirate @@ -511,6 +524,17 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase ids.each { |id| assert_nil klass.find_by_id(id) } end + define_method("test_should_skip_validation_on_the_#{association_name}_association_if_marked_for_destruction") do + 2.times { |i| @pirate.send(association_name).create!(:name => "#{association_name}_#{i}") } + children = @pirate.send(association_name) + + children.each { |child| child.name = '' } + assert !@pirate.valid? + + children.each { |child| child.mark_for_destruction } + assert_difference("#{association_name.classify}.count", -2) { @pirate.save! } + end + define_method("test_should_rollback_destructions_if_an_exception_occurred_while_saving_#{association_name}") do 2.times { |i| @pirate.send(association_name).create!(:name => "#{association_name}_#{i}") } before = @pirate.send(association_name).map { |c| c } -- cgit v1.2.3 From 6de83562f91028629bd24447aa521bc72ef8277a Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Fri, 27 Feb 2009 14:22:39 +0100 Subject: Force all internal calls to Array#to_sentence to use English [#2010 state:resolved] --- activerecord/lib/active_record/associations.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index b6f87a0486..6e88c89737 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -22,7 +22,7 @@ module ActiveRecord through_reflection = reflection.through_reflection source_reflection_names = reflection.source_reflection_names source_associations = reflection.through_reflection.klass.reflect_on_all_associations.collect { |a| a.name.inspect } - super("Could not find the source association(s) #{source_reflection_names.collect(&:inspect).to_sentence :two_words_connector => ' or ', :last_word_connector => ', or '} in model #{through_reflection.klass}. Try 'has_many #{reflection.name.inspect}, :through => #{through_reflection.name.inspect}, :source => '. Is it one of #{source_associations.to_sentence :two_words_connector => ' or ', :last_word_connector => ', or '}?") + super("Could not find the source association(s) #{source_reflection_names.collect(&:inspect).to_sentence(:two_words_connector => ' or ', :last_word_connector => ', or ', :locale => :en)} in model #{through_reflection.klass}. Try 'has_many #{reflection.name.inspect}, :through => #{through_reflection.name.inspect}, :source => '. Is it one of #{source_associations.to_sentence(:two_words_connector => ' or ', :last_word_connector => ', or ', :locale => :en)}?") end end -- cgit v1.2.3 From ff894b55dd787af1d6284567f2a898090aae64ca Mon Sep 17 00:00:00 2001 From: Zack Hobson Date: Fri, 13 Feb 2009 11:23:24 -0800 Subject: Update rdoc: all is not a named_scope [#1959 state:committed] Signed-off-by: David Heinemeier Hansson --- activerecord/lib/active_record/named_scope.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 022fdaf9b2..8a7449f95b 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -1,11 +1,12 @@ module ActiveRecord module NamedScope - # All subclasses of ActiveRecord::Base have two named \scopes: - # * all - which is similar to a find(:all) query, and + # All subclasses of ActiveRecord::Base have one named scope: # * scoped - which allows for the creation of anonymous \scopes, on the fly: Shirt.scoped(:conditions => {:color => 'red'}).scoped(:include => :washing_instructions) # # These anonymous \scopes tend to be useful when procedurally generating complex queries, where passing # intermediate values (scopes) around as first-class objects is convenient. + # + # You can define a scope that applies to all finders using ActiveRecord::Base.default_scope. def self.included(base) base.class_eval do extend ClassMethods -- cgit v1.2.3 From 77b0994c7835610982d708ce7ce5cd95e6e99e5a Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Fri, 27 Feb 2009 14:46:23 +0100 Subject: Prep for RC2 later today --- activerecord/CHANGELOG | 2 +- activerecord/Rakefile | 2 +- activerecord/lib/active_record/version.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index fc6986912c..231402c51c 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,4 +1,4 @@ -*Edge* +*2.3.1 [RC2] (February 27th, 2009)* * Added ActiveRecord::Base.each and ActiveRecord::Base.find_in_batches for batch processing [DHH/Jamis Buck] diff --git a/activerecord/Rakefile b/activerecord/Rakefile index 8b3b97bac4..aec9b3825b 100644 --- a/activerecord/Rakefile +++ b/activerecord/Rakefile @@ -177,7 +177,7 @@ spec = Gem::Specification.new do |s| s.files = s.files + Dir.glob( "#{dir}/**/*" ).delete_if { |item| item.include?( "\.svn" ) } end - s.add_dependency('activesupport', '= 2.3.0' + PKG_BUILD) + s.add_dependency('activesupport', '= 2.3.1' + PKG_BUILD) s.files.delete FIXTURES_ROOT + "/fixture_database.sqlite" s.files.delete FIXTURES_ROOT + "/fixture_database_2.sqlite" diff --git a/activerecord/lib/active_record/version.rb b/activerecord/lib/active_record/version.rb index 6ac4bdc905..809e91897e 100644 --- a/activerecord/lib/active_record/version.rb +++ b/activerecord/lib/active_record/version.rb @@ -2,7 +2,7 @@ module ActiveRecord module VERSION #:nodoc: MAJOR = 2 MINOR = 3 - TINY = 0 + TINY = 1 STRING = [MAJOR, MINOR, TINY].join('.') end -- cgit v1.2.3 From 7058c1366ee33f3095b8737c2b71876702cddea6 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Sat, 28 Feb 2009 10:30:49 +0100 Subject: So it didnt happen yesterday, but very soon! Just need the final details ironed out --- activerecord/CHANGELOG | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 231402c51c..c521a3beff 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,4 +1,4 @@ -*2.3.1 [RC2] (February 27th, 2009)* +*2.3.1 [RC2] (February ?, 2009)* * Added ActiveRecord::Base.each and ActiveRecord::Base.find_in_batches for batch processing [DHH/Jamis Buck] -- cgit v1.2.3 From c653f700d3afb1208b15fb7fec6250bf3a3f0321 Mon Sep 17 00:00:00 2001 From: Mike Gunderloy Date: Sat, 28 Feb 2009 11:01:13 -0600 Subject: Fix that scoped find with :group and :having [#2006 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/base.rb | 2 +- activerecord/test/cases/base_test.rb | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 55ab1facf2..206b4efa38 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1759,7 +1759,7 @@ module ActiveRecord #:nodoc: scope = scope(:find) if :auto == scope if scope && (scoped_group = scope[:group]) sql << " GROUP BY #{scoped_group}" - sql << " HAVING #{scoped_having}" if (scoped_having = scope[:having]) + sql << " HAVING #{scope[:having]}" if scope[:having] end end end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 973bb567bd..eec16c045a 100755 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1755,6 +1755,13 @@ class BasicsTest < ActiveRecord::TestCase end end + def test_scoped_find_with_group_and_having + developers = Developer.with_scope(:find => { :group => 'salary', :having => "SUM(salary) > 10000", :select => "SUM(salary) as salary" }) do + Developer.find(:all) + end + assert_equal 3, developers.size + end + def test_find_last last = Developer.find :last assert_equal last, Developer.find(:first, :order => 'id desc') -- cgit v1.2.3 From 0db6c3f51828e1a37e2c7b9245ffa8c12ac59c83 Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Sat, 28 Feb 2009 19:44:25 +0000 Subject: Merge docrails and update the release notes --- .../lib/active_record/serializers/json_serializer.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/serializers/json_serializer.rb b/activerecord/lib/active_record/serializers/json_serializer.rb index 419b45d475..1fd65ed006 100644 --- a/activerecord/lib/active_record/serializers/json_serializer.rb +++ b/activerecord/lib/active_record/serializers/json_serializer.rb @@ -8,6 +8,25 @@ module ActiveRecord #:nodoc: # Returns a JSON string representing the model. Some configuration is # available through +options+. # + # The option ActiveRecord::Base.include_root_in_json controls the + # top-level behavior of to_json. In a new Rails application, it is set to + # true in initializers/new_rails_defaults.rb. When it is true, + # to_json will emit a single root node named after the object's type. For example: + # + # konata = User.find(1) + # ActiveRecord::Base.include_root_in_json = true + # konata.to_json + # # => { "user": {"id": 1, "name": "Konata Izumi", "age": 16, + # "created_at": "2006/08/01", "awesome": true} } + # + # ActiveRecord::Base.include_root_in_json = false + # konata.to_json + # # => {"id": 1, "name": "Konata Izumi", "age": 16, + # "created_at": "2006/08/01", "awesome": true} + # + # The remainder of the examples in this section assume include_root_in_json is set to + # false. + # # Without any +options+, the returned JSON string will include all # the model's attributes. For example: # -- cgit v1.2.3 From 38136f86dc5504bde94dc7399d4a854023d7481f Mon Sep 17 00:00:00 2001 From: Jason King Date: Thu, 26 Feb 2009 16:51:38 +1100 Subject: DDL transactions and savepoints for sqlite Sqlite has had DDL transactions since 2.0.0[1] and savepoints since 3.6.8[2]. This patch updates the connection_adapters. [1] http://tinyurl.com/sqlite-v2-0-0 [2] http://tinyurl.com/sqlite-v3-6-8 Signed-off-by: Michael Koziarski [#2080 state:committed] --- .../connection_adapters/sqlite_adapter.rb | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index 5390f49f04..8373163e56 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -72,10 +72,29 @@ module ActiveRecord # # * :database - Path to the database file. class SQLiteAdapter < AbstractAdapter + + class Version + include Comparable + def initialize(vs) + @v = vs.split('.').map(&:to_i) + end + def <=>(rhs) + @v <=> rhs.split('.').map(&:to_i) + end + end + def adapter_name #:nodoc: 'SQLite' end + def supports_ddl_transactions? + sqlite_version >= '2.0.0' + end + + def supports_savepoints? + sqlite_version >= '3.6.8' + end + def supports_migrations? #:nodoc: true end @@ -380,7 +399,7 @@ module ActiveRecord end def sqlite_version - @sqlite_version ||= select_value('select sqlite_version(*)') + @sqlite_version ||= SQLiteAdapter::Version.new(select_value('select sqlite_version(*)')) end def default_primary_key_type -- cgit v1.2.3 From 818556ec4f237b19f28fdecdfe6037718cceba37 Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Mon, 2 Mar 2009 19:07:39 +1300 Subject: Revert "DDL transactions and savepoints for sqlite" This reverts commit 38136f86dc5504bde94dc7399d4a854023d7481f. Caused several test failures on the ci box: http://ci.rubyonrails.org/builds/rails/38136f [#2080 state:open] --- .../connection_adapters/sqlite_adapter.rb | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index 8373163e56..5390f49f04 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -72,29 +72,10 @@ module ActiveRecord # # * :database - Path to the database file. class SQLiteAdapter < AbstractAdapter - - class Version - include Comparable - def initialize(vs) - @v = vs.split('.').map(&:to_i) - end - def <=>(rhs) - @v <=> rhs.split('.').map(&:to_i) - end - end - def adapter_name #:nodoc: 'SQLite' end - def supports_ddl_transactions? - sqlite_version >= '2.0.0' - end - - def supports_savepoints? - sqlite_version >= '3.6.8' - end - def supports_migrations? #:nodoc: true end @@ -399,7 +380,7 @@ module ActiveRecord end def sqlite_version - @sqlite_version ||= SQLiteAdapter::Version.new(select_value('select sqlite_version(*)')) + @sqlite_version ||= select_value('select sqlite_version(*)') end def default_primary_key_type -- cgit v1.2.3 From 9b1b88f09cf1498f04e1cd469d0d5ffccf2b93cc Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Tue, 3 Mar 2009 13:07:03 -0600 Subject: Fixed reset_session for ActiveRecord session store [#2108 state:resolved] --- activerecord/lib/active_record/session_store.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/session_store.rb b/activerecord/lib/active_record/session_store.rb index de199d30bf..3cc4640f42 100644 --- a/activerecord/lib/active_record/session_store.rb +++ b/activerecord/lib/active_record/session_store.rb @@ -287,8 +287,7 @@ module ActiveRecord def get_session(env, sid) Base.silence do sid ||= generate_sid - session = @@session_class.find_by_session_id(sid) - session ||= @@session_class.new(:session_id => sid, :data => {}) + session = find_session(sid) env[SESSION_RECORD_KEY] = session [sid, session.data] end @@ -296,7 +295,7 @@ module ActiveRecord def set_session(env, sid, session_data) Base.silence do - record = env[SESSION_RECORD_KEY] + record = env[SESSION_RECORD_KEY] ||= find_session(sid) record.data = session_data return false unless record.save @@ -310,5 +309,10 @@ module ActiveRecord return true end + + def find_session(id) + @@session_class.find_by_session_id(id) || + @@session_class.new(:session_id => id, :data => {}) + end end end -- cgit v1.2.3 From 0cf49381d115d641dee242b6a62e0a5dc91c2ce3 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Wed, 4 Mar 2009 23:38:17 +0100 Subject: removes questionable :message examples (reported in Rails LH #2107) --- activerecord/lib/active_record/validations.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index 8f3c80565e..8862bd05d6 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -802,7 +802,7 @@ module ActiveRecord # Validates whether the value of the specified attribute is available in a particular enumerable object. # # class Person < ActiveRecord::Base - # validates_inclusion_of :gender, :in => %w( m f ), :message => "woah! what are you then!??!!" + # validates_inclusion_of :gender, :in => %w( m f ) # validates_inclusion_of :age, :in => 0..99 # validates_inclusion_of :format, :in => %w( jpg gif png ), :message => "extension {{value}} is not included in the list" # end -- cgit v1.2.3 From dfef3d8b14b5d3dcb61d83c30d5d5b27d33ff530 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Thu, 5 Mar 2009 11:00:04 +0100 Subject: Release RC2 today --- activerecord/CHANGELOG | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index c521a3beff..404481ea38 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,4 +1,4 @@ -*2.3.1 [RC2] (February ?, 2009)* +*2.3.1 [RC2] (March 5, 2009)* * Added ActiveRecord::Base.each and ActiveRecord::Base.find_in_batches for batch processing [DHH/Jamis Buck] -- cgit v1.2.3 From 6a13376525f34a00e013fc3a6022838329dfe856 Mon Sep 17 00:00:00 2001 From: jvoorhis Date: Fri, 6 Mar 2009 16:47:50 +0000 Subject: Methods invoked within named scope Procs should respect the scope stack. [#1267 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/named_scope.rb | 7 ++++++- activerecord/test/cases/named_scope_test.rb | 4 ++++ activerecord/test/models/topic.rb | 2 ++ 3 files changed, 12 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 8a7449f95b..43411dfb55 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -89,7 +89,12 @@ module ActiveRecord when Hash options when Proc - options.call(*args) + case parent_scope + when Scope + with_scope(:find => parent_scope.proxy_options) { options.call(*args) } + else + options.call(*args) + end end, &block) end (class << self; self end).instance_eval do diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index f28285faaf..9f3a3848e2 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -306,6 +306,10 @@ class NamedScopeTest < ActiveRecord::TestCase # Nested hash conditions with different keys assert_equal [posts(:sti_comments)], Post.with_special_comments.with_post(4).all.uniq end + + def test_methods_invoked_within_scopes_should_respect_scope + assert_equal [], Topic.approved.by_rejected_ids.proxy_options[:conditions][:id] + end end class DynamicScopeMatchTest < ActiveRecord::TestCase diff --git a/activerecord/test/models/topic.rb b/activerecord/test/models/topic.rb index 08bb24ed03..2fe846db48 100644 --- a/activerecord/test/models/topic.rb +++ b/activerecord/test/models/topic.rb @@ -33,6 +33,8 @@ class Topic < ActiveRecord::Base end named_scope :named_extension, :extend => NamedExtension named_scope :multiple_extensions, :extend => [MultipleExtensionTwo, MultipleExtensionOne] + + named_scope :by_rejected_ids, lambda {{ :conditions => { :id => all(:conditions => {:approved => false}).map(&:id) } }} has_many :replies, :dependent => :destroy, :foreign_key => "parent_id" serialize :content -- cgit v1.2.3 From c896d56c6e5520346c260477f7d7f8bf951e72cc Mon Sep 17 00:00:00 2001 From: Tom Lea Date: Fri, 6 Mar 2009 18:26:34 +0000 Subject: Ensure self referential HABTM associations raise an exception if association_foreign_key is missing. [#1252 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/associations.rb | 10 ++++++++++ .../associations/has_and_belongs_to_many_associations_test.rb | 8 ++++++++ 2 files changed, 18 insertions(+) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 6e88c89737..80b3e23026 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -51,6 +51,12 @@ module ActiveRecord end end + class HasAndBelongsToManyAssociationForeignKeyNeeded < ActiveRecordError #:nodoc: + def initialize(reflection) + super("Cannot create self referential has_and_belongs_to_many association on '#{reflection.class_name rescue nil}##{reflection.name rescue nil}'. :association_foreign_key cannot be the same as the :foreign_key.") + end + end + class EagerLoadPolymorphicError < ActiveRecordError #:nodoc: def initialize(reflection) super("Can not eagerly load the polymorphic association #{reflection.name.inspect}") @@ -1526,6 +1532,10 @@ module ActiveRecord options[:extend] = create_extension_modules(association_id, extension, options[:extend]) reflection = create_reflection(:has_and_belongs_to_many, association_id, options, self) + + if reflection.association_foreign_key == reflection.primary_key_name + raise HasAndBelongsToManyAssociationForeignKeyNeeded.new(reflection) + end reflection.options[:join_table] ||= join_table_name(undecorated_table_name(self.to_s), undecorated_table_name(reflection.class_name)) diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index 0f43d97f4a..9f5f34306d 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -740,6 +740,14 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal developer, project.developers.find(:first) assert_equal project, developer.projects.find(:first) end + + def test_self_referential_habtm_without_foreign_key_set_should_raise_exception + assert_raise(ActiveRecord::HasAndBelongsToManyAssociationForeignKeyNeeded) { + Member.class_eval do + has_and_belongs_to_many :friends, :class_name => "Member", :join_table => "member_friends" + end + } + end def test_dynamic_find_should_respect_association_include # SQL error in sort clause if :include is not included -- cgit v1.2.3 From 4863634a157a8e4a0033734617253b42325daf34 Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Fri, 6 Mar 2009 18:36:30 +0000 Subject: Ensure supplie :from has precedence over scoped :from [#1370 state:resolved] --- activerecord/lib/active_record/base.rb | 2 +- activerecord/test/cases/finder_test.rb | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 206b4efa38..ea791b5bfe 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1690,7 +1690,7 @@ module ActiveRecord #:nodoc: def construct_finder_sql(options) scope = scope(:find) sql = "SELECT #{options[:select] || (scope && scope[:select]) || default_select(options[:joins] || (scope && scope[:joins]))} " - sql << "FROM #{(scope && scope[:from]) || options[:from] || quoted_table_name} " + sql << "FROM #{options[:from] || (scope && scope[:from]) || quoted_table_name} " add_joins!(sql, options[:joins], scope) add_conditions!(sql, options[:conditions], scope) diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index ee8f4901f9..9d444a92ae 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -1057,6 +1057,14 @@ class FinderTest < ActiveRecord::TestCase assert_equal [0, 1, 1], posts.map(&:author_id).sort end + def test_finder_with_scoped_from + all_topics = Topic.all + + Topic.with_scope(:find => { :from => 'fake_topics' }) do + assert_equal all_topics, Topic.all(:from => 'topics') + end + end + protected def bind(statement, *vars) if vars.first.is_a?(Hash) -- cgit v1.2.3 From 984bc7a614852944808739fae09a654b6e62872e Mon Sep 17 00:00:00 2001 From: Dimitri Krassovski Date: Fri, 6 Mar 2009 19:11:13 +0000 Subject: Ensure replacing has_one associations respects the supplied :dependent option. [#1305 state:resolved] Signed-off-by: Pratik Naik --- .../active_record/associations/has_one_association.rb | 13 +++++++++++-- .../cases/associations/has_one_associations_test.rb | 18 ++++++++++++++++++ activerecord/test/cases/reflection_test.rb | 5 +++-- activerecord/test/models/company.rb | 1 + 4 files changed, 33 insertions(+), 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index 960323004d..b92cbbdeab 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -29,8 +29,17 @@ module ActiveRecord unless @target.nil? || @target == obj if dependent? && !dont_save - @target.destroy unless @target.new_record? - @owner.clear_association_cache + case @reflection.options[:dependent] + when :delete + @target.delete unless @target.new_record? + @owner.clear_association_cache + when :destroy + @target.destroy unless @target.new_record? + @owner.clear_association_cache + when :nullify + @target[@reflection.primary_key_name] = nil + @target.save unless @owner.new_record? || @target.new_record? + end else @target[@reflection.primary_key_name] = nil @target.save unless @owner.new_record? || @target.new_record? diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index e81bba790a..0aafbcee36 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -79,6 +79,24 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert_raises(ActiveRecord::RecordNotFound) { Account.find(old_account_id) } end + def test_nullification_on_association_change + firm = companies(:rails_core) + old_account_id = firm.account.id + firm.account = Account.new + # account is dependent with nullify, therefore its firm_id should be nil + assert_nil Account.find(old_account_id).firm_id + end + + def test_association_changecalls_delete + companies(:first_firm).deletable_account = Account.new + assert_equal [], Account.destroyed_account_ids[companies(:first_firm).id] + end + + def test_association_change_calls_destroy + companies(:first_firm).account = Account.new + assert_equal [companies(:first_firm).id], Account.destroyed_account_ids[companies(:first_firm).id] + end + def test_natural_assignment_to_already_associated_record company = companies(:first_firm) account = accounts(:signals37) diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index 8b1c714ead..34ebd99989 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -4,6 +4,7 @@ require 'models/customer' require 'models/company' require 'models/company_in_module' require 'models/subscriber' +require 'models/pirate' class ReflectionTest < ActiveRecord::TestCase fixtures :topics, :customers, :companies, :subscribers @@ -169,9 +170,9 @@ class ReflectionTest < ActiveRecord::TestCase def test_reflection_of_all_associations # FIXME these assertions bust a lot - assert_equal 26, Firm.reflect_on_all_associations.size + assert_equal 27, Firm.reflect_on_all_associations.size assert_equal 20, Firm.reflect_on_all_associations(:has_many).size - assert_equal 6, Firm.reflect_on_all_associations(:has_one).size + assert_equal 7, Firm.reflect_on_all_associations(:has_one).size assert_equal 0, Firm.reflect_on_all_associations(:belongs_to).size end diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index 3b27a9e272..a47992a8e7 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -69,6 +69,7 @@ class Firm < Company has_one :account_with_select, :foreign_key => "firm_id", :select => "id, firm_id", :class_name=>'Account' has_one :readonly_account, :foreign_key => "firm_id", :class_name => "Account", :readonly => true has_one :account_using_primary_key, :primary_key => "firm_id", :class_name => "Account" + has_one :deletable_account, :foreign_key => "firm_id", :class_name => "Account", :dependent => :delete end class DependentFirm < Company -- cgit v1.2.3 From 3ca5a0f9fd7b7921bca970859da8637011b22dd1 Mon Sep 17 00:00:00 2001 From: Adam Cooper Date: Wed, 31 Dec 2008 01:43:13 -0800 Subject: Ensure belongs_to association with a counter cache in name spaced model works [#1678 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/associations.rb | 4 +--- activerecord/lib/active_record/reflection.rb | 2 +- .../cases/associations/belongs_to_associations_test.rb | 17 +++++++++++++++++ activerecord/test/models/reply.rb | 6 ++++++ activerecord/test/models/topic.rb | 6 ++++++ 5 files changed, 31 insertions(+), 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 80b3e23026..65a1319c7e 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1003,9 +1003,7 @@ module ActiveRecord # Create the callbacks to update counter cache if options[:counter_cache] - cache_column = options[:counter_cache] == true ? - "#{self.to_s.demodulize.underscore.pluralize}_count" : - options[:counter_cache] + cache_column = reflection.counter_cache_column method_name = "belongs_to_counter_cache_after_create_for_#{reflection.name}".to_sym define_method(method_name) do diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index e69bfb1355..2d4c1d5507 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -197,7 +197,7 @@ module ActiveRecord def counter_cache_column if options[:counter_cache] == true - "#{active_record.name.underscore.pluralize}_count" + "#{active_record.name.demodulize.underscore.pluralize}_count" elsif options[:counter_cache] options[:counter_cache] end diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 92ed465ae7..78b90121b4 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -154,6 +154,23 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_equal 0, Topic.find(t2.id).replies.size end + def test_belongs_to_reassign_with_namespaced_models_and_counters + t1 = Web::Topic.create("title" => "t1") + t2 = Web::Topic.create("title" => "t2") + r1 = Web::Reply.new("title" => "r1", "content" => "r1") + r1.topic = t1 + + assert r1.save + assert_equal 1, Web::Topic.find(t1.id).replies.size + assert_equal 0, Web::Topic.find(t2.id).replies.size + + r1.topic = Web::Topic.find(t2.id) + + assert r1.save + assert_equal 0, Web::Topic.find(t1.id).replies.size + assert_equal 1, Web::Topic.find(t2.id).replies.size + end + def test_belongs_to_counter_after_save topic = Topic.create!(:title => "monday night") topic.replies.create!(:title => "re: monday night", :content => "football") diff --git a/activerecord/test/models/reply.rb b/activerecord/test/models/reply.rb index 812bc1f535..1c990acab6 100644 --- a/activerecord/test/models/reply.rb +++ b/activerecord/test/models/reply.rb @@ -37,3 +37,9 @@ end class SillyReply < Reply belongs_to :reply, :foreign_key => "parent_id", :counter_cache => :replies_count end + +module Web + class Reply < Web::Topic + belongs_to :topic, :foreign_key => "parent_id", :counter_cache => true, :class_name => 'Web::Topic' + end +end \ No newline at end of file diff --git a/activerecord/test/models/topic.rb b/activerecord/test/models/topic.rb index 2fe846db48..f1b7bbae3b 100644 --- a/activerecord/test/models/topic.rb +++ b/activerecord/test/models/topic.rb @@ -71,3 +71,9 @@ class Topic < ActiveRecord::Base end end end + +module Web + class Topic < ActiveRecord::Base + has_many :replies, :dependent => :destroy, :foreign_key => "parent_id", :class_name => 'Web::Reply' + end +end \ No newline at end of file -- cgit v1.2.3 From 7fb7b48a1f771e50896a82d749a70922a18afae7 Mon Sep 17 00:00:00 2001 From: Will Bryant Date: Fri, 6 Mar 2009 22:29:36 +0000 Subject: Allow :having conditions to be sanitized like regular :condition. [#2158 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/base.rb | 4 ++-- activerecord/lib/active_record/calculations.rb | 8 +++++--- activerecord/test/cases/calculations_test.rb | 8 ++++++++ activerecord/test/cases/finder_test.rb | 7 +++++++ 4 files changed, 22 insertions(+), 5 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index ea791b5bfe..60a1221e66 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1754,12 +1754,12 @@ module ActiveRecord #:nodoc: def add_group!(sql, group, having, scope = :auto) if group sql << " GROUP BY #{group}" - sql << " HAVING #{having}" if having + sql << " HAVING #{sanitize_sql_for_conditions(having)}" if having else scope = scope(:find) if :auto == scope if scope && (scoped_group = scope[:group]) sql << " GROUP BY #{scoped_group}" - sql << " HAVING #{scope[:having]}" if scope[:having] + sql << " HAVING #{sanitize_sql_for_conditions(scope[:having])}" if scope[:having] end end end diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb index b239c03284..7af97d7296 100644 --- a/activerecord/lib/active_record/calculations.rb +++ b/activerecord/lib/active_record/calculations.rb @@ -214,13 +214,15 @@ module ActiveRecord end if options[:group] && options[:having] + having = sanitize_sql_for_conditions(options[:having]) + # FrontBase requires identifiers in the HAVING clause and chokes on function calls if connection.adapter_name == 'FrontBase' - options[:having].downcase! - options[:having].gsub!(/#{operation}\s*\(\s*#{column_name}\s*\)/, aggregate_alias) + having.downcase! + having.gsub!(/#{operation}\s*\(\s*#{column_name}\s*\)/, aggregate_alias) end - sql << " HAVING #{options[:having]} " + sql << " HAVING #{having} " end sql << " ORDER BY #{options[:order]} " if options[:order] diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 5a4ed429b6..c158706645 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -92,6 +92,14 @@ class CalculationsTest < ActiveRecord::TestCase assert_equal 60, c[2] end + def test_should_group_by_summed_field_having_sanitized_condition + c = Account.sum(:credit_limit, :group => :firm_id, + :having => ['sum(credit_limit) > ?', 50]) + assert_nil c[1] + assert_equal 105, c[6] + assert_equal 60, c[2] + end + def test_should_group_by_summed_association c = Account.sum(:credit_limit, :group => :firm) assert_equal 50, c[companies(:first_firm)] diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 9d444a92ae..375b9a849c 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -191,6 +191,13 @@ class FinderTest < ActiveRecord::TestCase assert developers.all? { |developer| developer.salary > 10000 } end + def test_find_with_group_and_sanitized_having + developers = Developer.find(:all, :group => "salary", :having => ["sum(salary) > ?", 10000], :select => "salary") + assert_equal 3, developers.size + assert_equal 3, developers.map(&:salary).uniq.size + assert developers.all? { |developer| developer.salary > 10000 } + end + def test_find_with_entire_select_statement topics = Topic.find_by_sql "SELECT * FROM topics WHERE author_name = 'Mary'" -- cgit v1.2.3 From dfcf1e4cf6128ae67f9cab291fd6c08081aa5d80 Mon Sep 17 00:00:00 2001 From: Marcello Nuccio Date: Wed, 7 Jan 2009 01:37:56 +0100 Subject: Fixed broken migrations tests: added assert_equal. [#1704 state:resolved] Signed-off-by: Pratik Naik --- activerecord/test/cases/migration_test.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 1c974e4825..710634f08a 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -945,20 +945,20 @@ if ActiveRecord::Base.connection.supports_migrations? def test_finds_migrations migrations = ActiveRecord::Migrator.new(:up, MIGRATIONS_ROOT + "/valid").migrations - [['1', 'people_have_last_names'], - ['2', 'we_need_reminders'], - ['3', 'innocent_jointable']].each_with_index do |pair, i| - migrations[i].version == pair.first - migrations[1].name == pair.last + + [[1, 'PeopleHaveLastNames'], [2, 'WeNeedReminders'], [3, 'InnocentJointable']].each_with_index do |pair, i| + assert_equal migrations[i].version, pair.first + assert_equal migrations[i].name, pair.last end end def test_finds_pending_migrations ActiveRecord::Migrator.up(MIGRATIONS_ROOT + "/interleaved/pass_2", 1) migrations = ActiveRecord::Migrator.new(:up, MIGRATIONS_ROOT + "/interleaved/pass_2").pending_migrations + assert_equal 1, migrations.size - migrations[0].version == '3' - migrations[0].name == 'innocent_jointable' + assert_equal migrations[0].version, 3 + assert_equal migrations[0].name, 'InnocentJointable' end def test_only_loads_pending_migrations -- cgit v1.2.3 From fd3ccbe955ed58298df43f537a01358d15d8d4cf Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Sat, 7 Mar 2009 11:06:39 +0000 Subject: DRY with_kcode in Active Record tests --- activerecord/lib/active_record/test_case.rb | 13 +++++++++++++ activerecord/test/cases/base_test.rb | 14 -------------- activerecord/test/cases/validations_test.rb | 14 -------------- 3 files changed, 13 insertions(+), 28 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/test_case.rb b/activerecord/lib/active_record/test_case.rb index 211dd78874..8c6abaaccb 100644 --- a/activerecord/lib/active_record/test_case.rb +++ b/activerecord/lib/active_record/test_case.rb @@ -49,5 +49,18 @@ module ActiveRecord ActiveRecord::Base.clear_all_connections! ActiveRecord::Base.establish_connection(@connection) end + + def with_kcode(kcode) + if RUBY_VERSION < '1.9' + orig_kcode, $KCODE = $KCODE, kcode + begin + yield + ensure + $KCODE = orig_kcode + end + else + yield + end + end end end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index eec16c045a..eacf4e7699 100755 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -2099,18 +2099,4 @@ class BasicsTest < ActiveRecord::TestCase assert_equal custom_datetime, parrot[attribute] end end - - private - def with_kcode(kcode) - if RUBY_VERSION < '1.9' - orig_kcode, $KCODE = $KCODE, kcode - begin - yield - ensure - $KCODE = orig_kcode - end - else - yield - end - end end diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb index cbb184131f..8ef5dc09c5 100644 --- a/activerecord/test/cases/validations_test.rb +++ b/activerecord/test/cases/validations_test.rb @@ -1442,20 +1442,6 @@ class ValidationsTest < ActiveRecord::TestCase t.author_name = "Hubert J. Farnsworth" assert t.valid?, "A topic with an important title and author should be valid" end - - private - def with_kcode(kcode) - if RUBY_VERSION < '1.9' - orig_kcode, $KCODE = $KCODE, kcode - begin - yield - ensure - $KCODE = orig_kcode - end - else - yield - end - end end -- cgit v1.2.3 From f36513653fceb01691288dfdebfdc56d17dc85c9 Mon Sep 17 00:00:00 2001 From: Adam Milligan Date: Sat, 7 Mar 2009 12:34:56 +0000 Subject: Fix RDoc grammar and ensure hm:t tests can run in isolation. [#1644 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/associations.rb | 2 +- .../lib/active_record/associations/has_many_through_association.rb | 4 ++-- .../test/cases/associations/has_many_through_associations_test.rb | 2 ++ 3 files changed, 5 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 65a1319c7e..301b3a3b58 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -71,7 +71,7 @@ module ActiveRecord # See ActiveRecord::Associations::ClassMethods for documentation. module Associations # :nodoc: - # These classes will be loaded when associatoins are created. + # These classes will be loaded when associations are created. # So there is no need to eager load them. autoload :AssociationCollection, 'active_record/associations/association_collection' autoload :AssociationProxy, 'active_record/associations/association_proxy' 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 d5d188ac2a..23fe76de2e 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -23,8 +23,8 @@ module ActiveRecord end # Returns the size of the collection by executing a SELECT COUNT(*) query if the collection hasn't been loaded and - # calling collection.size if it has. If it's more likely than not that the collection does have a size larger than zero - # and you need to fetch that collection afterwards, it'll take one less SELECT query if you use length. + # calling collection.size if it has. If it's more likely than not that the collection does have a size larger than zero, + # and you need to fetch that collection afterwards, it'll take one fewer SELECT query if you use #length. def size return @owner.send(:read_attribute, cached_counter_attribute_name) if has_cached_counter? return @target.size if loaded? diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 1e5d1a0202..ac00508b5f 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -1,6 +1,8 @@ require "cases/helper" require 'models/post' require 'models/person' +require 'models/reference' +require 'models/job' require 'models/reader' require 'models/comment' require 'models/tag' -- cgit v1.2.3 From ccb0a92fa2ba0bf7be50033090c3a861e6d907be Mon Sep 17 00:00:00 2001 From: Arthur Klepchukov Date: Sat, 7 Mar 2009 13:52:17 +0000 Subject: Ensure validates_uniqueness_of considers limit on the field. [#1042 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/validations.rb | 8 ++++---- activerecord/test/cases/validations_test.rb | 9 +++++++++ activerecord/test/models/event.rb | 3 +++ activerecord/test/schema/schema.rb | 4 ++++ 4 files changed, 20 insertions(+), 4 deletions(-) create mode 100644 activerecord/test/models/event.rb (limited to 'activerecord') diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index 8f3c80565e..4453047daf 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -720,20 +720,20 @@ module ActiveRecord # class (which has a database table to query from). finder_class = class_hierarchy.detect { |klass| !klass.abstract_class? } - is_text_column = finder_class.columns_hash[attr_name.to_s].text? + column = finder_class.columns_hash[attr_name.to_s] if value.nil? comparison_operator = "IS ?" - elsif is_text_column + elsif column.text? comparison_operator = "#{connection.case_sensitive_equality_operator} ?" - value = value.to_s + value = column.limit ? value.to_s[0, column.limit] : value.to_s else comparison_operator = "= ?" end sql_attribute = "#{record.class.quoted_table_name}.#{connection.quote_column_name(attr_name)}" - if value.nil? || (configuration[:case_sensitive] || !is_text_column) + if value.nil? || (configuration[:case_sensitive] || !column.text?) condition_sql = "#{sql_attribute} #{comparison_operator}" condition_params = [value] else diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb index 8ef5dc09c5..f87168860f 100644 --- a/activerecord/test/cases/validations_test.rb +++ b/activerecord/test/cases/validations_test.rb @@ -8,6 +8,7 @@ require 'models/warehouse_thing' require 'models/guid' require 'models/owner' require 'models/pet' +require 'models/event' # The following methods in Topic are used in test_conditional_validation_* class Topic @@ -530,6 +531,14 @@ class ValidationsTest < ActiveRecord::TestCase end end + def test_validate_uniqueness_with_limit + # Event.title is limited to 5 characters + e1 = Event.create(:title => "abcde") + assert e1.valid?, "Could not create an event with a unique, 5 character title" + e2 = Event.create(:title => "abcdefgh") + assert !e2.valid?, "Created an event whose title, with limit taken into account, is not unique" + end + def test_validate_straight_inheritance_uniqueness w1 = IneptWizard.create(:name => "Rincewind", :city => "Ankh-Morpork") assert w1.valid?, "Saving w1" diff --git a/activerecord/test/models/event.rb b/activerecord/test/models/event.rb new file mode 100644 index 0000000000..99fa0feeb7 --- /dev/null +++ b/activerecord/test/models/event.rb @@ -0,0 +1,3 @@ +class Event < ActiveRecord::Base + validates_uniqueness_of :title +end \ No newline at end of file diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 74a893983f..9ebf1d82be 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -155,6 +155,10 @@ ActiveRecord::Schema.define do t.integer :course_id, :null => false end + create_table :events, :force => true do |t| + t.string :title, :limit => 5 + end + create_table :funny_jokes, :force => true do |t| t.string :name end -- cgit v1.2.3 From 6543426c73fa9ccf3649d7cbacbbb0fda9b6a099 Mon Sep 17 00:00:00 2001 From: stopdropandrew Date: Sat, 7 Mar 2009 15:26:56 +0000 Subject: Ensure calculations respect scoped :select [#1334 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/calculations.rb | 18 +++++++++++++----- activerecord/test/cases/calculations_test.rb | 11 +++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb index 7af97d7296..f077818d3b 100644 --- a/activerecord/lib/active_record/calculations.rb +++ b/activerecord/lib/active_record/calculations.rb @@ -141,22 +141,30 @@ module ActiveRecord def construct_count_options_from_args(*args) options = {} column_name = :all - + # We need to handle # count() # count(:column_name=:all) # count(options={}) # count(column_name=:all, options={}) + # selects specified by scopes case args.size + when 0 + column_name = scope(:find)[:select] if scope(:find) when 1 - args[0].is_a?(Hash) ? options = args[0] : column_name = args[0] + if args[0].is_a?(Hash) + column_name = scope(:find)[:select] if scope(:find) + options = args[0] + else + column_name = args[0] + end when 2 column_name, options = args else raise ArgumentError, "Unexpected parameters passed to count(): #{args.inspect}" - end if args.size > 0 - - [column_name, options] + end + + [column_name || :all, options] end def construct_calculation_sql(operation, column_name, options) #:nodoc: diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index c158706645..0257fe57cb 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -264,6 +264,17 @@ class CalculationsTest < ActiveRecord::TestCase assert_equal 4, Account.count(:distinct => true, :include => :firm, :select => :credit_limit) end + def test_should_count_scoped_select + Account.update_all("credit_limit = 50") + assert_equal 1, Account.scoped(:select => "DISTINCT credit_limit").count + end + + def test_should_count_scoped_select_with_options + Account.update_all("credit_limit = 50") + Account.first.update_attribute('credit_limit', 49) + assert_equal 1, Account.scoped(:select => "DISTINCT credit_limit").count(:conditions => [ 'credit_limit >= 50'] ) + end + def test_should_count_manual_select_with_include assert_equal 6, Account.count(:select => "DISTINCT accounts.id", :include => :firm) end -- cgit v1.2.3 From df8669d4b5691646ca8bb4ba01f6e5348ae8dd69 Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Sat, 7 Mar 2009 16:36:40 +0000 Subject: Fix calculation tests to work on sqlite --- activerecord/test/cases/calculations_test.rb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 0257fe57cb..f01ac2ea39 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -265,14 +265,16 @@ class CalculationsTest < ActiveRecord::TestCase end def test_should_count_scoped_select - Account.update_all("credit_limit = 50") - assert_equal 1, Account.scoped(:select => "DISTINCT credit_limit").count + Account.update_all("credit_limit = NULL") + assert_equal 0, Account.scoped(:select => "credit_limit").count end def test_should_count_scoped_select_with_options - Account.update_all("credit_limit = 50") - Account.first.update_attribute('credit_limit', 49) - assert_equal 1, Account.scoped(:select => "DISTINCT credit_limit").count(:conditions => [ 'credit_limit >= 50'] ) + Account.update_all("credit_limit = NULL") + Account.last.update_attribute('credit_limit', 49) + Account.first.update_attribute('credit_limit', 51) + + assert_equal 1, Account.scoped(:select => "credit_limit").count(:conditions => ['credit_limit >= 50']) end def test_should_count_manual_select_with_include -- cgit v1.2.3 From 7a26a67b42bdabd89f3685945009ffe99a6d6162 Mon Sep 17 00:00:00 2001 From: John Aughey Date: Sun, 8 Mar 2009 14:47:32 +0000 Subject: Ensure SQLite adapters stores the config [#1947 state:resolved] [John Aughey, Pratik Naik] Signed-off-by: Pratik Naik --- .../lib/active_record/connection_adapters/sqlite3_adapter.rb | 2 +- .../lib/active_record/connection_adapters/sqlite_adapter.rb | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index cc9c46505f..75420f69aa 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -18,7 +18,7 @@ module ActiveRecord db.busy_timeout(config[:timeout]) unless config[:timeout].nil? - ConnectionAdapters::SQLite3Adapter.new(db, logger) + ConnectionAdapters::SQLite3Adapter.new(db, logger, config) end end diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index 5390f49f04..6077ddcdb6 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -17,9 +17,9 @@ module ActiveRecord # "Downgrade" deprecated sqlite API if SQLite.const_defined?(:Version) - ConnectionAdapters::SQLite2Adapter.new(db, logger) + ConnectionAdapters::SQLite2Adapter.new(db, logger, config) else - ConnectionAdapters::DeprecatedSQLiteAdapter.new(db, logger) + ConnectionAdapters::DeprecatedSQLiteAdapter.new(db, logger, config) end end end @@ -72,6 +72,11 @@ module ActiveRecord # # * :database - Path to the database file. class SQLiteAdapter < AbstractAdapter + def initialize(connection, logger, config) + super(connection, logger) + @config = config + end + def adapter_name #:nodoc: 'SQLite' end -- cgit v1.2.3 From 96eaeee4467a03cba3c4c30aeb6fc6afe545ae1d Mon Sep 17 00:00:00 2001 From: Jeffrey Hardy Date: Fri, 6 Mar 2009 17:43:00 -0500 Subject: Add ActiveRecord::Base#invalid? as the opposite of #valid? [#2159 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/validations.rb | 5 +++++ activerecord/test/cases/validations_test.rb | 13 ++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index 4453047daf..ba60180ae0 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -1040,6 +1040,11 @@ module ActiveRecord errors.empty? end + # Performs the opposite of valid?. Returns true if errors were added, false otherwise. + def invalid? + !valid? + end + # Returns the Errors object that holds all information about attribute error messages. def errors @errors ||= Errors.new(self) diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb index f87168860f..60e75d2f88 100644 --- a/activerecord/test/cases/validations_test.rb +++ b/activerecord/test/cases/validations_test.rb @@ -170,7 +170,7 @@ class ValidationsTest < ActiveRecord::TestCase assert_equal person.first_name, "Mary", "should be ok when no attributes are passed to create!" end end - end + end def test_single_error_per_attr_iteration r = Reply.new @@ -1430,6 +1430,17 @@ class ValidationsTest < ActiveRecord::TestCase assert_equal "can't be blank", t.errors.on("title").first end + def test_invalid_should_be_the_opposite_of_valid + Topic.validates_presence_of :title + + t = Topic.new + assert t.invalid? + assert t.errors.invalid?(:title) + + t.title = 'Things are going to change' + assert !t.invalid? + end + # previous implementation of validates_presence_of eval'd the # string with the wrong binding, this regression test is to # ensure that it works correctly -- cgit v1.2.3 From f3e4d14ab97145fae156364e7e467f2d0313e1b9 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Sun, 8 Mar 2009 16:48:23 +0100 Subject: remove unneeded curlies in AR::Base.update example --- activerecord/lib/active_record/base.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 55ab1facf2..cfe1f8b8ef 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -737,12 +737,12 @@ module ActiveRecord #:nodoc: # ==== Parameters # # * +id+ - This should be the id or an array of ids to be updated. - # * +attributes+ - This should be a Hash of attributes to be set on the object, or an array of Hashes. + # * +attributes+ - This should be a hash of attributes to be set on the object, or an array of hashes. # # ==== Examples # # # Updating one record: - # Person.update(15, { :user_name => 'Samuel', :group => 'expert' }) + # Person.update(15, :user_name => 'Samuel', :group => 'expert') # # # Updating multiple records: # people = { 1 => { "first_name" => "David" }, 2 => { "first_name" => "Jeremy" } } -- cgit v1.2.3 From 1c36172c13fc51b22167f5e8fc41b1da9f8fcb2a Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Sun, 8 Mar 2009 13:11:58 -0700 Subject: Ruby 1.9 compat: rename deprecated assert_raises to assert_raise. [#1617 state:resolved] --- .../associations/belongs_to_associations_test.rb | 4 +- activerecord/test/cases/associations/eager_test.rb | 8 +-- .../has_and_belongs_to_many_associations_test.rb | 2 +- .../associations/has_many_associations_test.rb | 26 ++++----- .../associations/has_one_associations_test.rb | 10 ++-- .../has_one_through_associations_test.rb | 4 +- activerecord/test/cases/attribute_methods_test.rb | 2 +- activerecord/test/cases/base_test.rb | 18 +++--- activerecord/test/cases/calculations_test.rb | 4 +- activerecord/test/cases/callbacks_test.rb | 4 +- activerecord/test/cases/dirty_test.rb | 2 +- activerecord/test/cases/finder_test.rb | 64 +++++++++++----------- activerecord/test/cases/fixtures_test.rb | 4 +- activerecord/test/cases/inheritance_test.rb | 4 +- activerecord/test/cases/locking_test.rb | 10 ++-- activerecord/test/cases/migration_test.rb | 26 ++++----- activerecord/test/cases/transactions_test.rb | 4 +- activerecord/test/cases/validations_test.rb | 12 ++-- 18 files changed, 104 insertions(+), 104 deletions(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 78b90121b4..ff3e54712e 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -318,8 +318,8 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase 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 } + assert_raise(NoMethodError) { companies(:first_firm).private_method } + assert_raise(NoMethodError) { companies(:second_client).firm.private_method } end def test_belongs_to_proxy_should_respond_to_private_methods_via_send diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 14099d4176..40723814c5 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -549,16 +549,16 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_eager_with_invalid_association_reference - assert_raises(ActiveRecord::ConfigurationError, "Association was not found; perhaps you misspelled it? You specified :include => :monkeys") { + assert_raise(ActiveRecord::ConfigurationError, "Association was not found; perhaps you misspelled it? You specified :include => :monkeys") { post = Post.find(6, :include=> :monkeys ) } - assert_raises(ActiveRecord::ConfigurationError, "Association was not found; perhaps you misspelled it? You specified :include => :monkeys") { + assert_raise(ActiveRecord::ConfigurationError, "Association was not found; perhaps you misspelled it? You specified :include => :monkeys") { post = Post.find(6, :include=>[ :monkeys ]) } - assert_raises(ActiveRecord::ConfigurationError, "Association was not found; perhaps you misspelled it? You specified :include => :monkeys") { + assert_raise(ActiveRecord::ConfigurationError, "Association was not found; perhaps you misspelled it? You specified :include => :monkeys") { post = Post.find(6, :include=>[ 'monkeys' ]) } - assert_raises(ActiveRecord::ConfigurationError, "Association was not found; perhaps you misspelled it? You specified :include => :monkeys, :elephants") { + assert_raise(ActiveRecord::ConfigurationError, "Association was not found; perhaps you misspelled it? You specified :include => :monkeys, :elephants") { post = Post.find(6, :include=>[ :monkeys, :elephants ]) } end diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index 9f5f34306d..ca1772d1ca 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -616,7 +616,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase def test_updating_attributes_on_rich_associations david = projects(:action_controller).developers.first david.name = "DHH" - assert_raises(ActiveRecord::ReadOnlyRecord) { david.save! } + assert_raise(ActiveRecord::ReadOnlyRecord) { david.save! } end def test_updating_attributes_on_rich_associations_with_limited_find_from_reflection diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 5efbc5bd0c..3f33baf88c 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -176,7 +176,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_find_ids firm = Firm.find(:first) - assert_raises(ActiveRecord::RecordNotFound) { firm.clients.find } + assert_raise(ActiveRecord::RecordNotFound) { firm.clients.find } client = firm.clients.find(2) assert_kind_of Client, client @@ -190,7 +190,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 2, client_ary.size assert_equal client, client_ary.first - assert_raises(ActiveRecord::RecordNotFound) { firm.clients.find(2, 99) } + assert_raise(ActiveRecord::RecordNotFound) { firm.clients.find(2, 99) } end def test_find_string_ids_when_using_finder_sql @@ -238,7 +238,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_find_in_collection assert_equal Client.find(2).name, companies(:first_firm).clients.find(2).name - assert_raises(ActiveRecord::RecordNotFound) { companies(:first_firm).clients.find(6) } + assert_raise(ActiveRecord::RecordNotFound) { companies(:first_firm).clients.find(6) } end def test_find_grouped @@ -278,36 +278,36 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end def test_create_with_bang_on_has_many_when_parent_is_new_raises - assert_raises(ActiveRecord::RecordNotSaved) do + assert_raise(ActiveRecord::RecordNotSaved) do firm = Firm.new firm.plain_clients.create! :name=>"Whoever" end end def test_regular_create_on_has_many_when_parent_is_new_raises - assert_raises(ActiveRecord::RecordNotSaved) do + assert_raise(ActiveRecord::RecordNotSaved) do firm = Firm.new firm.plain_clients.create :name=>"Whoever" end end def test_create_with_bang_on_has_many_raises_when_record_not_saved - assert_raises(ActiveRecord::RecordInvalid) do + assert_raise(ActiveRecord::RecordInvalid) do firm = Firm.find(:first) firm.plain_clients.create! end end def test_create_with_bang_on_habtm_when_parent_is_new_raises - assert_raises(ActiveRecord::RecordNotSaved) do + assert_raise(ActiveRecord::RecordNotSaved) do Developer.new("name" => "Aredridel").projects.create! end end def test_adding_a_mismatch_class - assert_raises(ActiveRecord::AssociationTypeMismatch) { companies(:first_firm).clients_of_firm << nil } - assert_raises(ActiveRecord::AssociationTypeMismatch) { companies(:first_firm).clients_of_firm << 1 } - assert_raises(ActiveRecord::AssociationTypeMismatch) { companies(:first_firm).clients_of_firm << Topic.find(1) } + assert_raise(ActiveRecord::AssociationTypeMismatch) { companies(:first_firm).clients_of_firm << nil } + assert_raise(ActiveRecord::AssociationTypeMismatch) { companies(:first_firm).clients_of_firm << 1 } + assert_raise(ActiveRecord::AssociationTypeMismatch) { companies(:first_firm).clients_of_firm << Topic.find(1) } end def test_adding_a_collection @@ -602,7 +602,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end def test_invalid_belongs_to_dependent_option_raises_exception - assert_raises ArgumentError do + assert_raise ArgumentError do Author.belongs_to :special_author_address, :dependent => :nullify end end @@ -628,13 +628,13 @@ class HasManyAssociationsTest < ActiveRecord::TestCase def test_deleting_type_mismatch david = Developer.find(1) david.projects.reload - assert_raises(ActiveRecord::AssociationTypeMismatch) { david.projects.delete(1) } + assert_raise(ActiveRecord::AssociationTypeMismatch) { david.projects.delete(1) } end def test_deleting_self_type_mismatch david = Developer.find(1) david.projects.reload - assert_raises(ActiveRecord::AssociationTypeMismatch) { david.projects.delete(Project.find(1).developers) } + assert_raise(ActiveRecord::AssociationTypeMismatch) { david.projects.delete(Project.find(1).developers) } end def test_destroy_all diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index 0aafbcee36..4947f1543c 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -59,8 +59,8 @@ class HasOneAssociationsTest < ActiveRecord::TestCase end def test_type_mismatch - assert_raises(ActiveRecord::AssociationTypeMismatch) { companies(:first_firm).account = 1 } - assert_raises(ActiveRecord::AssociationTypeMismatch) { companies(:first_firm).account = Project.find(1) } + assert_raise(ActiveRecord::AssociationTypeMismatch) { companies(:first_firm).account = 1 } + assert_raise(ActiveRecord::AssociationTypeMismatch) { companies(:first_firm).account = Project.find(1) } end def test_natural_assignment @@ -76,7 +76,7 @@ class HasOneAssociationsTest < ActiveRecord::TestCase companies(:first_firm).save assert_nil companies(:first_firm).account # account is dependent, therefore is destroyed when reference to owner is lost - assert_raises(ActiveRecord::RecordNotFound) { Account.find(old_account_id) } + assert_raise(ActiveRecord::RecordNotFound) { Account.find(old_account_id) } end def test_nullification_on_association_change @@ -281,8 +281,8 @@ class HasOneAssociationsTest < ActiveRecord::TestCase end def test_has_one_proxy_should_not_respond_to_private_methods - assert_raises(NoMethodError) { accounts(:signals37).private_method } - assert_raises(NoMethodError) { companies(:first_firm).account.private_method } + assert_raise(NoMethodError) { accounts(:signals37).private_method } + assert_raise(NoMethodError) { companies(:first_firm).account.private_method } end def test_has_one_proxy_should_respond_to_private_methods_via_send diff --git a/activerecord/test/cases/associations/has_one_through_associations_test.rb b/activerecord/test/cases/associations/has_one_through_associations_test.rb index f65d76e2ce..f96b55513e 100644 --- a/activerecord/test/cases/associations/has_one_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb @@ -115,8 +115,8 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase end def test_has_one_through_proxy_should_not_respond_to_private_methods - assert_raises(NoMethodError) { clubs(:moustache_club).private_method } - assert_raises(NoMethodError) { @member.club.private_method } + assert_raise(NoMethodError) { clubs(:moustache_club).private_method } + assert_raise(NoMethodError) { @member.club.private_method } end def test_has_one_through_proxy_should_respond_to_private_methods_via_send diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 759f9f3872..17ed302465 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -100,7 +100,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase %w(save create_or_update).each do |method| klass = Class.new ActiveRecord::Base klass.class_eval "def #{method}() 'defined #{method}' end" - assert_raises ActiveRecord::DangerousAttributeError do + assert_raise ActiveRecord::DangerousAttributeError do klass.instance_method_already_implemented?(method) end end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index eacf4e7699..748a64af5e 100755 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -424,8 +424,8 @@ class BasicsTest < ActiveRecord::TestCase def test_non_attribute_access_and_assignment topic = Topic.new assert !topic.respond_to?("mumbo") - assert_raises(NoMethodError) { topic.mumbo } - assert_raises(NoMethodError) { topic.mumbo = 5 } + assert_raise(NoMethodError) { topic.mumbo } + assert_raise(NoMethodError) { topic.mumbo = 5 } end def test_preserving_date_objects @@ -490,7 +490,7 @@ class BasicsTest < ActiveRecord::TestCase end def test_record_not_found_exception - assert_raises(ActiveRecord::RecordNotFound) { topicReloaded = Topic.find(99999) } + assert_raise(ActiveRecord::RecordNotFound) { topicReloaded = Topic.find(99999) } end def test_initialize_with_attributes @@ -848,7 +848,7 @@ class BasicsTest < ActiveRecord::TestCase client.delete assert client.frozen? assert_kind_of Firm, client.firm - assert_raises(ActiveSupport::FrozenObjectError) { client.name = "something else" } + assert_raise(ActiveSupport::FrozenObjectError) { client.name = "something else" } end def test_destroy_new_record @@ -862,7 +862,7 @@ class BasicsTest < ActiveRecord::TestCase client.destroy assert client.frozen? assert_kind_of Firm, client.firm - assert_raises(ActiveSupport::FrozenObjectError) { client.name = "something else" } + assert_raise(ActiveSupport::FrozenObjectError) { client.name = "something else" } end def test_update_attribute @@ -910,8 +910,8 @@ class BasicsTest < ActiveRecord::TestCase def test_mass_assignment_should_raise_exception_if_accessible_and_protected_attribute_writers_are_both_used topic = TopicWithProtectedContentAndAccessibleAuthorName.new - assert_raises(RuntimeError) { topic.attributes = { "author_name" => "me" } } - assert_raises(RuntimeError) { topic.attributes = { "content" => "stuff" } } + assert_raise(RuntimeError) { topic.attributes = { "author_name" => "me" } } + assert_raise(RuntimeError) { topic.attributes = { "content" => "stuff" } } end def test_mass_assignment_protection @@ -949,7 +949,7 @@ class BasicsTest < ActiveRecord::TestCase def test_mass_assigning_invalid_attribute firm = Firm.new - assert_raises(ActiveRecord::UnknownAttributeError) do + assert_raise(ActiveRecord::UnknownAttributeError) do firm.attributes = { "id" => 5, "type" => "Client", "i_dont_even_exist" => 20 } end end @@ -1402,7 +1402,7 @@ class BasicsTest < ActiveRecord::TestCase end def test_sql_injection_via_find - assert_raises(ActiveRecord::RecordNotFound, ActiveRecord::StatementInvalid) do + assert_raise(ActiveRecord::RecordNotFound, ActiveRecord::StatementInvalid) do Topic.find("123456 OR id > 0") end end diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index f01ac2ea39..56dcdea110 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -255,8 +255,8 @@ class CalculationsTest < ActiveRecord::TestCase Company.send(:validate_calculation_options, :count, :include => true) end - assert_raises(ArgumentError) { Company.send(:validate_calculation_options, :sum, :foo => :bar) } - assert_raises(ArgumentError) { Company.send(:validate_calculation_options, :count, :foo => :bar) } + assert_raise(ArgumentError) { Company.send(:validate_calculation_options, :sum, :foo => :bar) } + assert_raise(ArgumentError) { Company.send(:validate_calculation_options, :count, :foo => :bar) } end def test_should_count_selected_field_with_include diff --git a/activerecord/test/cases/callbacks_test.rb b/activerecord/test/cases/callbacks_test.rb index 33b1ea034d..95fddaeef6 100644 --- a/activerecord/test/cases/callbacks_test.rb +++ b/activerecord/test/cases/callbacks_test.rb @@ -352,13 +352,13 @@ class CallbacksTest < ActiveRecord::TestCase david = ImmutableDeveloper.find(1) assert david.valid? assert !david.save - assert_raises(ActiveRecord::RecordNotSaved) { david.save! } + assert_raise(ActiveRecord::RecordNotSaved) { david.save! } david = ImmutableDeveloper.find(1) david.salary = 10_000_000 assert !david.valid? assert !david.save - assert_raises(ActiveRecord::RecordInvalid) { david.save! } + assert_raise(ActiveRecord::RecordInvalid) { david.save! } someone = CallbackCancellationDeveloper.find(1) someone.cancel_before_save = true diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index 5f5707b388..ac95bac4ad 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -228,7 +228,7 @@ class DirtyTest < ActiveRecord::TestCase pirate = Pirate.new pirate.parrot_id = 1 - assert_raises(ActiveRecord::RecordInvalid) { pirate.save! } + assert_raise(ActiveRecord::RecordInvalid) { pirate.save! } check_pirate_after_save_failure(pirate) end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 375b9a849c..d8778957c0 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -146,7 +146,7 @@ class FinderTest < ActiveRecord::TestCase end def test_find_by_ids_missing_one - assert_raises(ActiveRecord::RecordNotFound) { Topic.find(1, 2, 45) } + assert_raise(ActiveRecord::RecordNotFound) { Topic.find(1, 2, 45) } end def test_find_all_with_limit @@ -236,7 +236,7 @@ class FinderTest < ActiveRecord::TestCase end def test_unexisting_record_exception_handling - assert_raises(ActiveRecord::RecordNotFound) { + assert_raise(ActiveRecord::RecordNotFound) { Topic.find(1).parent } @@ -245,7 +245,7 @@ class FinderTest < ActiveRecord::TestCase def test_find_only_some_columns topic = Topic.find(1, :select => "author_name") - assert_raises(ActiveRecord::MissingAttributeError) {topic.title} + assert_raise(ActiveRecord::MissingAttributeError) {topic.title} assert_equal "David", topic.author_name assert !topic.attribute_present?("title") #assert !topic.respond_to?("title") @@ -267,22 +267,22 @@ class FinderTest < ActiveRecord::TestCase def test_find_on_array_conditions assert Topic.find(1, :conditions => ["approved = ?", false]) - assert_raises(ActiveRecord::RecordNotFound) { Topic.find(1, :conditions => ["approved = ?", true]) } + assert_raise(ActiveRecord::RecordNotFound) { Topic.find(1, :conditions => ["approved = ?", true]) } end def test_find_on_hash_conditions assert Topic.find(1, :conditions => { :approved => false }) - assert_raises(ActiveRecord::RecordNotFound) { Topic.find(1, :conditions => { :approved => true }) } + assert_raise(ActiveRecord::RecordNotFound) { Topic.find(1, :conditions => { :approved => true }) } end def test_find_on_hash_conditions_with_explicit_table_name assert Topic.find(1, :conditions => { 'topics.approved' => false }) - assert_raises(ActiveRecord::RecordNotFound) { Topic.find(1, :conditions => { 'topics.approved' => true }) } + assert_raise(ActiveRecord::RecordNotFound) { Topic.find(1, :conditions => { 'topics.approved' => true }) } end def test_find_on_hash_conditions_with_hashed_table_name assert Topic.find(1, :conditions => {:topics => { :approved => false }}) - assert_raises(ActiveRecord::RecordNotFound) { Topic.find(1, :conditions => {:topics => { :approved => true }}) } + assert_raise(ActiveRecord::RecordNotFound) { Topic.find(1, :conditions => {:topics => { :approved => true }}) } end def test_find_with_hash_conditions_on_joined_table @@ -300,7 +300,7 @@ class FinderTest < ActiveRecord::TestCase def test_find_on_hash_conditions_with_explicit_table_name_and_aggregate david = customers(:david) assert Customer.find(david.id, :conditions => { 'customers.name' => david.name, :address => david.address }) - assert_raises(ActiveRecord::RecordNotFound) { + assert_raise(ActiveRecord::RecordNotFound) { Customer.find(david.id, :conditions => { 'customers.name' => david.name + "1", :address => david.address }) } end @@ -311,13 +311,13 @@ class FinderTest < ActiveRecord::TestCase def test_find_on_hash_conditions_with_range assert_equal [1,2], Topic.find(:all, :conditions => { :id => 1..2 }).map(&:id).sort - assert_raises(ActiveRecord::RecordNotFound) { Topic.find(1, :conditions => { :id => 2..3 }) } + assert_raise(ActiveRecord::RecordNotFound) { Topic.find(1, :conditions => { :id => 2..3 }) } end def test_find_on_hash_conditions_with_end_exclusive_range assert_equal [1,2,3], Topic.find(:all, :conditions => { :id => 1..3 }).map(&:id).sort assert_equal [1,2], Topic.find(:all, :conditions => { :id => 1...3 }).map(&:id).sort - assert_raises(ActiveRecord::RecordNotFound) { Topic.find(3, :conditions => { :id => 2...3 }) } + assert_raise(ActiveRecord::RecordNotFound) { Topic.find(3, :conditions => { :id => 2...3 }) } end def test_find_on_hash_conditions_with_multiple_ranges @@ -327,9 +327,9 @@ class FinderTest < ActiveRecord::TestCase def test_find_on_multiple_hash_conditions assert Topic.find(1, :conditions => { :author_name => "David", :title => "The First Topic", :replies_count => 1, :approved => false }) - assert_raises(ActiveRecord::RecordNotFound) { Topic.find(1, :conditions => { :author_name => "David", :title => "The First Topic", :replies_count => 1, :approved => true }) } - assert_raises(ActiveRecord::RecordNotFound) { Topic.find(1, :conditions => { :author_name => "David", :title => "HHC", :replies_count => 1, :approved => false }) } - assert_raises(ActiveRecord::RecordNotFound) { Topic.find(1, :conditions => { :author_name => "David", :title => "The First Topic", :replies_count => 1, :approved => true }) } + assert_raise(ActiveRecord::RecordNotFound) { Topic.find(1, :conditions => { :author_name => "David", :title => "The First Topic", :replies_count => 1, :approved => true }) } + assert_raise(ActiveRecord::RecordNotFound) { Topic.find(1, :conditions => { :author_name => "David", :title => "HHC", :replies_count => 1, :approved => false }) } + assert_raise(ActiveRecord::RecordNotFound) { Topic.find(1, :conditions => { :author_name => "David", :title => "The First Topic", :replies_count => 1, :approved => true }) } end def test_condition_interpolation @@ -353,7 +353,7 @@ class FinderTest < ActiveRecord::TestCase end def test_hash_condition_find_malformed - assert_raises(ActiveRecord::StatementInvalid) { + assert_raise(ActiveRecord::StatementInvalid) { Company.find(:first, :conditions => { :id => 2, :dhh => true }) } end @@ -422,10 +422,10 @@ class FinderTest < ActiveRecord::TestCase assert_nil Company.find(:first, :conditions => ["name = ?", "37signals!"]) assert_nil Company.find(:first, :conditions => ["name = ?", "37signals!' OR 1=1"]) assert_kind_of Time, Topic.find(:first, :conditions => ["id = ?", 1]).written_on - assert_raises(ActiveRecord::PreparedStatementInvalid) { + assert_raise(ActiveRecord::PreparedStatementInvalid) { Company.find(:first, :conditions => ["id=? AND name = ?", 2]) } - assert_raises(ActiveRecord::PreparedStatementInvalid) { + assert_raise(ActiveRecord::PreparedStatementInvalid) { Company.find(:first, :conditions => ["id=?", 2, 3, 4]) } end @@ -442,11 +442,11 @@ class FinderTest < ActiveRecord::TestCase def test_bind_arity assert_nothing_raised { bind '' } - assert_raises(ActiveRecord::PreparedStatementInvalid) { bind '', 1 } + assert_raise(ActiveRecord::PreparedStatementInvalid) { bind '', 1 } - assert_raises(ActiveRecord::PreparedStatementInvalid) { bind '?' } + assert_raise(ActiveRecord::PreparedStatementInvalid) { bind '?' } assert_nothing_raised { bind '?', 1 } - assert_raises(ActiveRecord::PreparedStatementInvalid) { bind '?', 1, 1 } + assert_raise(ActiveRecord::PreparedStatementInvalid) { bind '?', 1, 1 } end def test_named_bind_variables @@ -551,7 +551,7 @@ class FinderTest < ActiveRecord::TestCase def test_find_by_one_attribute_bang assert_equal topics(:first), Topic.find_by_title!("The First Topic") - assert_raises(ActiveRecord::RecordNotFound) { Topic.find_by_title!("The First Topic!") } + assert_raise(ActiveRecord::RecordNotFound) { Topic.find_by_title!("The First Topic!") } end def test_find_by_one_attribute_caches_dynamic_finder @@ -632,14 +632,14 @@ class FinderTest < ActiveRecord::TestCase end def test_find_by_one_missing_attribute - assert_raises(NoMethodError) { Topic.find_by_undertitle("The First Topic!") } + assert_raise(NoMethodError) { Topic.find_by_undertitle("The First Topic!") } end def test_find_by_invalid_method_syntax - assert_raises(NoMethodError) { Topic.fail_to_find_by_title("The First Topic") } - assert_raises(NoMethodError) { Topic.find_by_title?("The First Topic") } - assert_raises(NoMethodError) { Topic.fail_to_find_or_create_by_title("Nonexistent Title") } - assert_raises(NoMethodError) { Topic.find_or_create_by_title?("Nonexistent Title") } + assert_raise(NoMethodError) { Topic.fail_to_find_by_title("The First Topic") } + assert_raise(NoMethodError) { Topic.find_by_title?("The First Topic") } + assert_raise(NoMethodError) { Topic.fail_to_find_or_create_by_title("Nonexistent Title") } + assert_raise(NoMethodError) { Topic.find_or_create_by_title?("Nonexistent Title") } end def test_find_by_two_attributes @@ -661,8 +661,8 @@ class FinderTest < ActiveRecord::TestCase end def test_find_last_by_invalid_method_syntax - assert_raises(NoMethodError) { Topic.fail_to_find_last_by_title("The First Topic") } - assert_raises(NoMethodError) { Topic.find_last_by_title?("The First Topic") } + assert_raise(NoMethodError) { Topic.fail_to_find_last_by_title("The First Topic") } + assert_raise(NoMethodError) { Topic.find_last_by_title?("The First Topic") } end def test_find_last_by_one_attribute_with_several_options @@ -670,7 +670,7 @@ class FinderTest < ActiveRecord::TestCase end def test_find_last_by_one_missing_attribute - assert_raises(NoMethodError) { Topic.find_last_by_undertitle("The Last Topic!") } + assert_raise(NoMethodError) { Topic.find_last_by_undertitle("The Last Topic!") } end def test_find_last_by_two_attributes @@ -923,16 +923,16 @@ class FinderTest < ActiveRecord::TestCase end def test_find_with_bad_sql - assert_raises(ActiveRecord::StatementInvalid) { Topic.find_by_sql "select 1 from badtable" } + assert_raise(ActiveRecord::StatementInvalid) { Topic.find_by_sql "select 1 from badtable" } end def test_find_with_invalid_params - assert_raises(ArgumentError) { Topic.find :first, :join => "It should be `joins'" } - assert_raises(ArgumentError) { Topic.find :first, :conditions => '1 = 1', :join => "It should be `joins'" } + assert_raise(ArgumentError) { Topic.find :first, :join => "It should be `joins'" } + assert_raise(ArgumentError) { Topic.find :first, :conditions => '1 = 1', :join => "It should be `joins'" } end def test_dynamic_finder_with_invalid_params - assert_raises(ArgumentError) { Topic.find_by_title 'No Title', :join => "It should be `joins'" } + assert_raise(ArgumentError) { Topic.find_by_title 'No Title', :join => "It should be `joins'" } end def test_find_all_with_limit diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index eb63fd5f34..252bf4ff61 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -151,7 +151,7 @@ class FixturesTest < ActiveRecord::TestCase end def test_dirty_dirty_yaml_file - assert_raises(Fixture::FormatError) do + assert_raise(Fixture::FormatError) do Fixtures.new( Account.connection, "courses", 'Course', FIXTURES_ROOT + "/naked/yml/courses") end end @@ -420,7 +420,7 @@ class InvalidTableNameFixturesTest < ActiveRecord::TestCase self.use_transactional_fixtures = false def test_raises_error - assert_raises FixtureClassNotFound do + assert_raise FixtureClassNotFound do funny_jokes(:a_joke) end end diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index 3f59eb9706..eae5a60829 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -68,7 +68,7 @@ class InheritanceTest < ActiveRecord::TestCase if current_adapter?(:SybaseAdapter) Company.connection.execute "SET IDENTITY_INSERT companies OFF" end - assert_raises(ActiveRecord::SubclassNotFound) { Company.find(100) } + assert_raise(ActiveRecord::SubclassNotFound) { Company.find(100) } end def test_inheritance_find @@ -124,7 +124,7 @@ class InheritanceTest < ActiveRecord::TestCase end def test_finding_incorrect_type_data - assert_raises(ActiveRecord::RecordNotFound) { Firm.find(2) } + assert_raise(ActiveRecord::RecordNotFound) { Firm.find(2) } assert_nothing_raised { Firm.find(1) } end diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index 077cac7747..8d2059d63f 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -35,7 +35,7 @@ class OptimisticLockingTest < ActiveRecord::TestCase assert_equal 0, p2.lock_version p2.first_name = 'sue' - assert_raises(ActiveRecord::StaleObjectError) { p2.save! } + assert_raise(ActiveRecord::StaleObjectError) { p2.save! } end def test_lock_repeating @@ -50,9 +50,9 @@ class OptimisticLockingTest < ActiveRecord::TestCase assert_equal 0, p2.lock_version p2.first_name = 'sue' - assert_raises(ActiveRecord::StaleObjectError) { p2.save! } + assert_raise(ActiveRecord::StaleObjectError) { p2.save! } p2.first_name = 'sue2' - assert_raises(ActiveRecord::StaleObjectError) { p2.save! } + assert_raise(ActiveRecord::StaleObjectError) { p2.save! } end def test_lock_new @@ -71,7 +71,7 @@ class OptimisticLockingTest < ActiveRecord::TestCase assert_equal 0, p2.lock_version p2.first_name = 'sue' - assert_raises(ActiveRecord::StaleObjectError) { p2.save! } + assert_raise(ActiveRecord::StaleObjectError) { p2.save! } end def test_lock_new_with_nil @@ -95,7 +95,7 @@ class OptimisticLockingTest < ActiveRecord::TestCase assert_equal 0, t2.version t2.tps_report_number = 800 - assert_raises(ActiveRecord::StaleObjectError) { t2.save! } + assert_raise(ActiveRecord::StaleObjectError) { t2.save! } end def test_lock_column_is_mass_assignable diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 710634f08a..50d039ec77 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -111,7 +111,7 @@ if ActiveRecord::Base.connection.supports_migrations? end end - assert_raises(ActiveRecord::StatementInvalid) do + assert_raise(ActiveRecord::StatementInvalid) do Person.connection.execute "insert into testings (foo) values (NULL)" end ensure @@ -278,7 +278,7 @@ if ActiveRecord::Base.connection.supports_migrations? end Person.connection.add_column :testings, :bar, :string, :null => false - assert_raises(ActiveRecord::StatementInvalid) do + assert_raise(ActiveRecord::StatementInvalid) do Person.connection.execute "insert into testings (foo, bar) values ('hello', NULL)" end ensure @@ -297,7 +297,7 @@ if ActiveRecord::Base.connection.supports_migrations? Person.connection.enable_identity_insert("testings", false) if current_adapter?(:SybaseAdapter) assert_nothing_raised {Person.connection.add_column :testings, :bar, :string, :null => false, :default => "default" } - assert_raises(ActiveRecord::StatementInvalid) do + assert_raise(ActiveRecord::StatementInvalid) do unless current_adapter?(:OpenBaseAdapter) Person.connection.execute "insert into testings (#{con.quote_column_name('id')}, #{con.quote_column_name('foo')}, #{con.quote_column_name('bar')}) values (2, 'hello', NULL)" else @@ -545,7 +545,7 @@ if ActiveRecord::Base.connection.supports_migrations? else ActiveRecord::ActiveRecordError end - assert_raises(exception) do + assert_raise(exception) do Person.connection.rename_column "hats", "nonexistent", "should_fail" end ensure @@ -795,7 +795,7 @@ if ActiveRecord::Base.connection.supports_migrations? assert_equal "hello world", Reminder.find(:first).content WeNeedReminders.down - assert_raises(ActiveRecord::StatementInvalid) { Reminder.find(:first) } + assert_raise(ActiveRecord::StatementInvalid) { Reminder.find(:first) } end def test_add_table_with_decimals @@ -856,7 +856,7 @@ if ActiveRecord::Base.connection.supports_migrations? end GiveMeBigNumbers.down - assert_raises(ActiveRecord::StatementInvalid) { BigNumber.find(:first) } + assert_raise(ActiveRecord::StatementInvalid) { BigNumber.find(:first) } end def test_migrator @@ -876,7 +876,7 @@ if ActiveRecord::Base.connection.supports_migrations? assert_equal 0, ActiveRecord::Migrator.current_version Person.reset_column_information assert !Person.column_methods_hash.include?(:last_name) - assert_raises(ActiveRecord::StatementInvalid) { Reminder.find(:first) } + assert_raise(ActiveRecord::StatementInvalid) { Reminder.find(:first) } end def test_migrator_one_up @@ -932,7 +932,7 @@ if ActiveRecord::Base.connection.supports_migrations? def test_migrator_one_up_with_exception_and_rollback assert !Person.column_methods_hash.include?(:last_name) - e = assert_raises(StandardError) do + e = assert_raise(StandardError) do ActiveRecord::Migrator.up(MIGRATIONS_ROOT + "/broken", 100) end @@ -1107,7 +1107,7 @@ if ActiveRecord::Base.connection.supports_migrations? assert_equal "hello world", Reminder.find(:first).content WeNeedReminders.down - assert_raises(ActiveRecord::StatementInvalid) { Reminder.find(:first) } + assert_raise(ActiveRecord::StatementInvalid) { Reminder.find(:first) } ensure ActiveRecord::Base.table_name_prefix = '' ActiveRecord::Base.table_name_suffix = '' @@ -1137,13 +1137,13 @@ if ActiveRecord::Base.connection.supports_migrations? end def test_migrator_with_duplicates - assert_raises(ActiveRecord::DuplicateMigrationVersionError) do + assert_raise(ActiveRecord::DuplicateMigrationVersionError) do ActiveRecord::Migrator.migrate(MIGRATIONS_ROOT + "/duplicate", nil) end end def test_migrator_with_duplicate_names - assert_raises(ActiveRecord::DuplicateMigrationNameError, "Multiple migrations have the name Chunky") do + assert_raise(ActiveRecord::DuplicateMigrationNameError, "Multiple migrations have the name Chunky") do ActiveRecord::Migrator.migrate(MIGRATIONS_ROOT + "/duplicate_names", nil) end end @@ -1159,7 +1159,7 @@ if ActiveRecord::Base.connection.supports_migrations? # table name is 29 chars, the standard sequence name will # be 33 chars and fail - assert_raises(ActiveRecord::StatementInvalid) do + assert_raise(ActiveRecord::StatementInvalid) do begin Person.connection.create_table :table_with_name_thats_just_ok do |t| t.column :foo, :string, :null => false @@ -1186,7 +1186,7 @@ if ActiveRecord::Base.connection.supports_migrations? end # confirm the custom sequence got dropped - assert_raises(ActiveRecord::StatementInvalid) do + assert_raise(ActiveRecord::StatementInvalid) do Person.connection.execute("select suitably_short_seq.nextval from dual") end end diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index 40abd935df..f9ae5115fc 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -214,7 +214,7 @@ class TransactionTest < ActiveRecord::TestCase end def test_invalid_keys_for_transaction - assert_raises ArgumentError do + assert_raise ArgumentError do Topic.transaction :nested => true do end end @@ -370,7 +370,7 @@ class TransactionTest < ActiveRecord::TestCase # Test now inside a transaction: add_column should raise a StatementInvalid Topic.transaction do - assert_raises(ActiveRecord::StatementInvalid) { Topic.connection.add_column('topics', 'stuff', :string) } + assert_raise(ActiveRecord::StatementInvalid) { Topic.connection.add_column('topics', 'stuff', :string) } raise ActiveRecord::Rollback end end diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb index 60e75d2f88..c20f5ae63e 100644 --- a/activerecord/test/cases/validations_test.rb +++ b/activerecord/test/cases/validations_test.rb @@ -114,8 +114,8 @@ class ValidationsTest < ActiveRecord::TestCase end def test_invalid_record_exception - assert_raises(ActiveRecord::RecordInvalid) { Reply.create! } - assert_raises(ActiveRecord::RecordInvalid) { Reply.new.save! } + assert_raise(ActiveRecord::RecordInvalid) { Reply.create! } + assert_raise(ActiveRecord::RecordInvalid) { Reply.new.save! } begin r = Reply.new @@ -127,13 +127,13 @@ class ValidationsTest < ActiveRecord::TestCase end def test_exception_on_create_bang_many - assert_raises(ActiveRecord::RecordInvalid) do + assert_raise(ActiveRecord::RecordInvalid) do Reply.create!([ { "title" => "OK" }, { "title" => "Wrong Create" }]) end end def test_exception_on_create_bang_with_block - assert_raises(ActiveRecord::RecordInvalid) do + assert_raise(ActiveRecord::RecordInvalid) do Reply.create!({ "title" => "OK" }) do |r| r.content = nil end @@ -141,7 +141,7 @@ class ValidationsTest < ActiveRecord::TestCase end def test_exception_on_create_bang_many_with_block - assert_raises(ActiveRecord::RecordInvalid) do + assert_raise(ActiveRecord::RecordInvalid) do Reply.create!([{ "title" => "OK" }, { "title" => "Wrong Create" }]) do |r| r.content = nil end @@ -150,7 +150,7 @@ class ValidationsTest < ActiveRecord::TestCase def test_scoped_create_without_attributes Reply.with_scope(:create => {}) do - assert_raises(ActiveRecord::RecordInvalid) { Reply.create! } + assert_raise(ActiveRecord::RecordInvalid) { Reply.create! } end end -- cgit v1.2.3 From 277c799d58be4b3e0e885d7b3fd6d954facc111b Mon Sep 17 00:00:00 2001 From: Jan De Poorter Date: Mon, 9 Mar 2009 13:02:31 +0100 Subject: Fix find_by_last when order is given [#2127 state:committed] Signed-off-by: David Heinemeier Hansson --- activerecord/lib/active_record/base.rb | 2 +- activerecord/test/cases/associations/has_many_associations_test.rb | 4 ++++ activerecord/test/cases/reflection_test.rb | 4 ++-- activerecord/test/models/company.rb | 1 + 4 files changed, 8 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 60a1221e66..d8337602e8 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -2174,7 +2174,7 @@ module ActiveRecord #:nodoc: # Test whether the given method and optional key are scoped. def scoped?(method, key = nil) #:nodoc: if current_scoped_methods && (scope = current_scoped_methods[method]) - !key || scope.has_key?(key) + !key || !scope[key].nil? end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 3f33baf88c..b7fa9d9d7c 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -70,6 +70,10 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 2, companies(:first_firm).limited_clients.find(:all, :limit => nil).size end + def test_dynamic_find_last_without_specified_order + assert_equal companies(:second_client), companies(:first_firm).unsorted_clients.find_last_by_type('Client') + end + def test_dynamic_find_should_respect_association_order assert_equal companies(:second_client), companies(:first_firm).clients_sorted_desc.find(:first, :conditions => "type = 'Client'") assert_equal companies(:second_client), companies(:first_firm).clients_sorted_desc.find_by_type('Client') diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index 34ebd99989..db64bbb806 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -170,8 +170,8 @@ class ReflectionTest < ActiveRecord::TestCase def test_reflection_of_all_associations # FIXME these assertions bust a lot - assert_equal 27, Firm.reflect_on_all_associations.size - assert_equal 20, Firm.reflect_on_all_associations(:has_many).size + assert_equal 28, Firm.reflect_on_all_associations.size + assert_equal 21, Firm.reflect_on_all_associations(:has_many).size assert_equal 7, Firm.reflect_on_all_associations(:has_one).size assert_equal 0, Firm.reflect_on_all_associations(:belongs_to).size end diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index a47992a8e7..02a775f9ef 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -37,6 +37,7 @@ class Firm < Company has_many :clients, :order => "id", :dependent => :destroy, :counter_sql => "SELECT COUNT(*) FROM companies WHERE firm_id = 1 " + "AND (#{QUOTED_TYPE} = 'Client' OR #{QUOTED_TYPE} = 'SpecialClient' OR #{QUOTED_TYPE} = 'VerySpecialClient' )" + has_many :unsorted_clients, :class_name => "Client" has_many :clients_sorted_desc, :class_name => "Client", :order => "id DESC" has_many :clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :order => "id" has_many :unvalidated_clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :validate => false -- cgit v1.2.3 From faf4ba6b79e56dde383a8761fb1f485fcd07c72b Mon Sep 17 00:00:00 2001 From: Rob Anderton Date: Mon, 9 Mar 2009 12:51:55 +0000 Subject: Allow find(:last) :order be a symbol [#2024 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/base.rb | 2 +- activerecord/test/cases/base_test.rb | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index d8337602e8..edc145985d 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1537,7 +1537,7 @@ module ActiveRecord #:nodoc: end def reverse_sql_order(order_query) - reversed_query = order_query.split(/,/).each { |s| + reversed_query = order_query.to_s.split(/,/).each { |s| if s.match(/\s(asc|ASC)$/) s.gsub!(/\s(asc|ASC)$/, ' DESC') elsif s.match(/\s(desc|DESC)$/) diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 748a64af5e..99d77961fc 100755 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1790,6 +1790,11 @@ class BasicsTest < ActiveRecord::TestCase assert_equal last, Developer.find(:all, :order => 'developers.name, developers.salary DESC').last end + def test_find_symbol_ordered_last + last = Developer.find :last, :order => :salary + assert_equal last, Developer.find(:all, :order => :salary).last + end + def test_find_scoped_ordered_last last_developer = Developer.with_scope(:find => { :order => 'developers.salary ASC' }) do Developer.find(:last) -- cgit v1.2.3 From 1e6c50e21bdb8c99116a7dc6921ef3eb4ed9531a Mon Sep 17 00:00:00 2001 From: Jim Remsik and Tim Pope Date: Mon, 9 Mar 2009 13:42:51 +0000 Subject: Ensure has_many :through works with changed primary keys [#736 state:resolved] Signed-off-by: Pratik Naik --- .../active_record/associations/has_many_through_association.rb | 2 +- .../cases/associations/has_many_through_associations_test.rb | 9 ++++++++- activerecord/test/fixtures/toys.yml | 4 ++++ activerecord/test/models/owner.rb | 3 ++- activerecord/test/models/pet.rb | 3 ++- activerecord/test/models/toy.rb | 4 ++++ activerecord/test/schema/schema.rb | 5 +++++ 7 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 activerecord/test/fixtures/toys.yml create mode 100644 activerecord/test/models/toy.rb (limited to 'activerecord') 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 23fe76de2e..1c091e7d5a 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -150,7 +150,7 @@ module ActiveRecord end else reflection_primary_key = @reflection.source_reflection.primary_key_name - source_primary_key = @reflection.klass.primary_key + source_primary_key = @reflection.through_reflection.klass.primary_key if @reflection.source_reflection.options[:as] polymorphic_join = "AND %s.%s = %s" % [ @reflection.quoted_table_name, "#{@reflection.source_reflection.options[:as]}_type", diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index ac00508b5f..c3ad0ee6de 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -8,9 +8,12 @@ require 'models/comment' require 'models/tag' require 'models/tagging' require 'models/author' +require 'models/owner' +require 'models/pet' +require 'models/toy' class HasManyThroughAssociationsTest < ActiveRecord::TestCase - fixtures :posts, :readers, :people, :comments, :authors + fixtures :posts, :readers, :people, :comments, :authors, :owners, :pets, :toys def test_associate_existing assert_queries(2) { posts(:thinking);people(:david) } @@ -251,4 +254,8 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase author.author_favorites.create(:favorite_author_id => 3) assert_equal post.author.author_favorites, post.author_favorites end + + def test_has_many_association_through_a_has_many_association_with_nonstandard_primary_keys + assert_equal 1, owners(:blackbeard).toys.count + end end diff --git a/activerecord/test/fixtures/toys.yml b/activerecord/test/fixtures/toys.yml new file mode 100644 index 0000000000..037e335e0a --- /dev/null +++ b/activerecord/test/fixtures/toys.yml @@ -0,0 +1,4 @@ +bone: + toy_id: 1 + name: Bone + pet_id: 1 diff --git a/activerecord/test/models/owner.rb b/activerecord/test/models/owner.rb index dbaf2ce688..5760b991ec 100644 --- a/activerecord/test/models/owner.rb +++ b/activerecord/test/models/owner.rb @@ -1,4 +1,5 @@ class Owner < ActiveRecord::Base set_primary_key :owner_id has_many :pets -end \ No newline at end of file + has_many :toys, :through => :pets +end diff --git a/activerecord/test/models/pet.rb b/activerecord/test/models/pet.rb index 889ce46f33..dc1a3c5e94 100644 --- a/activerecord/test/models/pet.rb +++ b/activerecord/test/models/pet.rb @@ -1,4 +1,5 @@ class Pet < ActiveRecord::Base set_primary_key :pet_id belongs_to :owner -end \ No newline at end of file + has_many :toys +end diff --git a/activerecord/test/models/toy.rb b/activerecord/test/models/toy.rb new file mode 100644 index 0000000000..79a88db0da --- /dev/null +++ b/activerecord/test/models/toy.rb @@ -0,0 +1,4 @@ +class Toy < ActiveRecord::Base + set_primary_key :toy_id + belongs_to :pet +end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 9ebf1d82be..ea848a2940 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -425,6 +425,11 @@ ActiveRecord::Schema.define do t.column :taggings_count, :integer, :default => 0 end + create_table :toys, :primary_key => :toy_id ,:force => true do |t| + t.string :name + t.integer :pet_id, :integer + end + create_table :treasures, :force => true do |t| t.column :name, :string t.column :looter_id, :integer -- cgit v1.2.3 From 0d922885fb54c19f04680482f024452859218910 Mon Sep 17 00:00:00 2001 From: Curtis Hawthorne Date: Mon, 9 Mar 2009 14:56:09 +0000 Subject: Ensure Model#destroy respects optimistic locking [#1966 state:resolved] Signed-off-by: Pratik Naik --- .../lib/active_record/locking/optimistic.rb | 33 ++++++++++++++++++++++ activerecord/test/cases/locking_test.rb | 18 ++++++++++++ 2 files changed, 51 insertions(+) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index ff9899d032..7fa7e267d8 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -23,6 +23,16 @@ module ActiveRecord # p2.first_name = "should fail" # p2.save # Raises a ActiveRecord::StaleObjectError # + # Optimistic locking will also check for stale data when objects are destroyed. Example: + # + # p1 = Person.find(1) + # p2 = Person.find(1) + # + # p1.first_name = "Michael" + # p1.save + # + # p2.destroy # Raises a ActiveRecord::StaleObjectError + # # You're then responsible for dealing with the conflict by rescuing the exception and either rolling back, merging, # or otherwise apply the business logic needed to resolve the conflict. # @@ -39,6 +49,7 @@ module ActiveRecord base.lock_optimistically = true base.alias_method_chain :update, :lock + base.alias_method_chain :destroy, :lock base.alias_method_chain :attributes_from_column_definition, :lock class << base @@ -98,6 +109,28 @@ module ActiveRecord end end + def destroy_with_lock #:nodoc: + return destroy_without_lock unless locking_enabled? + + unless new_record? + lock_col = self.class.locking_column + previous_value = send(lock_col).to_i + + affected_rows = connection.delete( + "DELETE FROM #{self.class.quoted_table_name} " + + "WHERE #{connection.quote_column_name(self.class.primary_key)} = #{quoted_id} " + + "AND #{self.class.quoted_locking_column} = #{quote_value(previous_value)}", + "#{self.class.name} Destroy" + ) + + unless affected_rows == 1 + raise ActiveRecord::StaleObjectError, "Attempted to delete a stale object" + end + end + + freeze + end + module ClassMethods DEFAULT_LOCKING_COLUMN = 'lock_version' diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index 8d2059d63f..e177235591 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -38,6 +38,24 @@ class OptimisticLockingTest < ActiveRecord::TestCase assert_raise(ActiveRecord::StaleObjectError) { p2.save! } end + def test_lock_destroy + p1 = Person.find(1) + p2 = Person.find(1) + assert_equal 0, p1.lock_version + assert_equal 0, p2.lock_version + + p1.first_name = 'stu' + p1.save! + assert_equal 1, p1.lock_version + assert_equal 0, p2.lock_version + + assert_raises(ActiveRecord::StaleObjectError) { p2.destroy } + + assert p1.destroy + assert_equal true, p1.frozen? + assert_raises(ActiveRecord::RecordNotFound) { Person.find(1) } + end + def test_lock_repeating p1 = Person.find(1) p2 = Person.find(1) -- cgit v1.2.3 From 4b6458bf4cbf14007878b84a3fbc193f3365eebe Mon Sep 17 00:00:00 2001 From: Jeffrey Hardy Date: Mon, 9 Mar 2009 19:45:43 -0400 Subject: Improve documentation for ActiveRecord::Base.update_all. Document Array and Hash syntax for conditions, include usage examples. --- activerecord/lib/active_record/base.rb | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index cfe1f8b8ef..d258461b29 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -811,25 +811,28 @@ module ActiveRecord #:nodoc: # Updates all records with details given if they match a set of conditions supplied, limits and order can # also be supplied. This method constructs a single SQL UPDATE statement and sends it straight to the - # database. It does not instantiate the involved models and it does not trigger Active Record callbacks. + # database. It does not instantiate the involved models and it does not trigger Active Record callbacks + # or validations. # # ==== Parameters # - # * +updates+ - A string of column and value pairs that will be set on any records that match conditions. This creates the SET clause of the generated SQL. - # * +conditions+ - An SQL fragment like "administrator = 1" or [ "user_name = ?", username ]. See conditions in the intro for more info. + # * +updates+ - A string, array, or hash representing the SET part of an SQL statement. + # * +conditions+ - A string, array, or hash representing the WHERE part of an SQL statement. See conditions in the intro. # * +options+ - Additional options are :limit and :order, see the examples for usage. # # ==== Examples # - # # Update all billing objects with the 3 different attributes given - # Billing.update_all( "category = 'authorized', approved = 1, author = 'David'" ) + # # Update all customers with the given attributes + # Customer.update_all :wants_email => true # - # # Update records that match our conditions - # Billing.update_all( "author = 'David'", "title LIKE '%Rails%'" ) + # # Update all books with 'Rails' in their title + # Book.update_all "author = 'David'", "title LIKE '%Rails%'" # - # # Update records that match our conditions but limit it to 5 ordered by date - # Billing.update_all( "author = 'David'", "title LIKE '%Rails%'", - # :order => 'created_at', :limit => 5 ) + # # Update all avatars migrated more than a week ago + # Avatar.update_all ['migrated_at = ?, Time.now.utc], ['migrated_at > ?', 1.week.ago] + # + # # Update all books that match our conditions, but limit it to 5 ordered by date + # Book.update_all "author = 'David'", "title LIKE '%Rails%'", :order => 'created_at', :limit => 5 def update_all(updates, conditions = nil, options = {}) sql = "UPDATE #{quoted_table_name} SET #{sanitize_sql_for_assignment(updates)} " -- cgit v1.2.3 From 8272630ce8af0546e7d1aa9211a9d91b80700cbd Mon Sep 17 00:00:00 2001 From: Bruce Krysiak Date: Tue, 10 Mar 2009 11:17:16 +0000 Subject: Ensure ActiveRecord#to_xml respects :skip_types for included associations [#1632 state:resolved] Signed-off-by: Pratik Naik --- .../lib/active_record/serializers/xml_serializer.rb | 18 ++++++++++++------ activerecord/test/cases/xml_serialization_test.rb | 13 ++++++++++++- 2 files changed, 24 insertions(+), 7 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/serializers/xml_serializer.rb b/activerecord/lib/active_record/serializers/xml_serializer.rb index 4749823b94..fa75874603 100644 --- a/activerecord/lib/active_record/serializers/xml_serializer.rb +++ b/activerecord/lib/active_record/serializers/xml_serializer.rb @@ -231,16 +231,22 @@ module ActiveRecord #:nodoc: def add_associations(association, records, opts) if records.is_a?(Enumerable) tag = reformat_name(association.to_s) + type = options[:skip_types] ? {} : {:type => "array"} + if records.empty? - builder.tag!(tag, :type => :array) + builder.tag!(tag, type) else - builder.tag!(tag, :type => :array) do + builder.tag!(tag, type) do association_name = association.to_s.singularize records.each do |record| - record.to_xml opts.merge( - :root => association_name, - :type => (record.class.to_s.underscore == association_name ? nil : record.class.name) - ) + if options[:skip_types] + record_type = {} + else + record_class = (record.class.to_s.underscore == association_name) ? nil : record.class.name + record_type = {:type => record_class} + end + + record.to_xml opts.merge(:root => association_name).merge(record_type) end end end diff --git a/activerecord/test/cases/xml_serialization_test.rb b/activerecord/test/cases/xml_serialization_test.rb index 39c6ea820d..b49997669e 100644 --- a/activerecord/test/cases/xml_serialization_test.rb +++ b/activerecord/test/cases/xml_serialization_test.rb @@ -38,11 +38,15 @@ class XmlSerializationTest < ActiveRecord::TestCase assert_match %r{ 25).to_xml :skip_types => true + assert %r{25}.match(@xml) + end + def test_should_include_yielded_additions @xml = Contact.new.to_xml do |xml| xml.creator "David" end - assert_match %r{David}, @xml end end @@ -145,6 +149,13 @@ class DatabaseConnectedXmlSerializationTest < ActiveRecord::TestCase assert_match %r{}, xml end + def test_included_associations_should_skip_types + xml = authors(:david).to_xml :include=>:hello_posts, :indent => 0, :skip_types => true + assert_match %r{}, xml + assert_match %r{}, xml + assert_match %r{}, xml + end + def test_methods_are_called_on_object xml = authors(:david).to_xml :methods => :label, :indent => 0 assert_match %r{}, xml -- cgit v1.2.3 From c3aa2bcdcffb42f578b0e89fe08e1c4e234ccf3b Mon Sep 17 00:00:00 2001 From: Manfred Stienstra Date: Tue, 10 Mar 2009 11:49:58 +0000 Subject: Ensure nested with_scope merges conditions inside out [#2193 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/base.rb | 19 ++++++------- activerecord/lib/active_record/named_scope.rb | 4 +-- activerecord/test/cases/method_scoping_test.rb | 38 ++++++++++++++++++++++---- activerecord/test/cases/named_scope_test.rb | 22 +++++++++------ 4 files changed, 57 insertions(+), 26 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index edc145985d..4f39761672 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -416,7 +416,7 @@ module ActiveRecord #:nodoc: end @@subclasses = {} - + ## # :singleton-method: # Contains the database configuration - as is typically stored in config/database.yml - @@ -661,7 +661,6 @@ module ActiveRecord #:nodoc: connection.select_all(sanitize_sql(sql), "#{name} Load").collect! { |record| instantiate(record) } end - # Returns true if a record exists in the table that matches the +id+ or # conditions given, or false otherwise. The argument can take five forms: # @@ -1003,7 +1002,6 @@ module ActiveRecord #:nodoc: update_counters(id, counter_name => -1) end - # Attributes named in this macro are protected from mass-assignment, # such as new(attributes), # update_attributes(attributes), or @@ -1104,7 +1102,6 @@ module ActiveRecord #:nodoc: read_inheritable_attribute(:attr_serialized) or write_inheritable_attribute(:attr_serialized, {}) end - # Guesses the table name (in forced lower-case) based on the name of the class in the inheritance hierarchy descending # directly from ActiveRecord::Base. So if the hierarchy looks like: Reply < Message < ActiveRecord::Base, then Message is used # to guess the table name even when called on Reply. The rules used to do the guess are handled by the Inflector class @@ -1417,7 +1414,6 @@ module ActiveRecord #:nodoc: end end - def quote_value(value, column = nil) #:nodoc: connection.quote(value,column) end @@ -1486,7 +1482,7 @@ module ActiveRecord #:nodoc: elsif match = DynamicScopeMatch.match(method_id) return true if all_attributes_exists?(match.attribute_names) end - + super end @@ -2014,7 +2010,6 @@ module ActiveRecord #:nodoc: end end - # Defines an "attribute" method (like +inheritance_column+ or # +table_name+). A new (class) method will be created with the # given name. If a value is specified, the new method will @@ -2111,7 +2106,7 @@ module ActiveRecord #:nodoc: end # Merge scopings - if action == :merge && current_scoped_methods + if [:merge, :reverse_merge].include?(action) && current_scoped_methods method_scoping = current_scoped_methods.inject(method_scoping) do |hash, (method, params)| case hash[method] when Hash @@ -2133,7 +2128,11 @@ module ActiveRecord #:nodoc: end end else - hash[method] = hash[method].merge(params) + if action == :reverse_merge + hash[method] = hash[method].merge(params) + else + hash[method] = params.merge(hash[method]) + end end else hash[method] = params @@ -2143,7 +2142,6 @@ module ActiveRecord #:nodoc: end self.scoped_methods << method_scoping - begin yield ensure @@ -2749,7 +2747,6 @@ module ActiveRecord #:nodoc: assign_multiparameter_attributes(multi_parameter_attributes) end - # Returns a hash of all the attributes with their names as keys and the values of the attributes as values. def attributes self.attribute_names.inject({}) do |attrs, name| diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 43411dfb55..519941dd94 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -104,7 +104,7 @@ module ActiveRecord end end end - + class Scope attr_reader :proxy_scope, :proxy_options, :current_scoped_methods_when_defined NON_DELEGATE_METHODS = %w(nil? send object_id class extend find size count sum average maximum minimum paginate first last empty? any? respond_to?).to_set @@ -175,7 +175,7 @@ module ActiveRecord if scopes.include?(method) scopes[method].call(self, *args) else - with_scope :find => proxy_options, :create => proxy_options[:conditions].is_a?(Hash) ? proxy_options[:conditions] : {} do + with_scope({:find => proxy_options, :create => proxy_options[:conditions].is_a?(Hash) ? proxy_options[:conditions] : {}}, :reverse_merge) do method = :new if method == :build if current_scoped_methods_when_defined with_scope current_scoped_methods_when_defined do diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb index c676c1c72b..68a7017f86 100644 --- a/activerecord/test/cases/method_scoping_test.rb +++ b/activerecord/test/cases/method_scoping_test.rb @@ -262,6 +262,15 @@ class NestedScopingTest < ActiveRecord::TestCase end end + def test_merge_inner_scope_has_priority + Developer.with_scope(:find => { :limit => 5 }) do + Developer.with_scope(:find => { :limit => 10 }) do + merged_option = Developer.instance_eval('current_scoped_methods')[:find] + assert_equal({ :limit => 10 }, merged_option) + end + end + end + def test_replace_options Developer.with_scope(:find => { :conditions => "name = 'David'" }) do Developer.with_exclusive_scope(:find => { :conditions => "name = 'Jamis'" }) do @@ -400,6 +409,29 @@ class NestedScopingTest < ActiveRecord::TestCase end end + def test_nested_scoped_create + comment = nil + Comment.with_scope(:create => { :post_id => 1}) do + Comment.with_scope(:create => { :post_id => 2}) do + assert_equal({ :post_id => 2 }, Comment.send(:current_scoped_methods)[:create]) + comment = Comment.create :body => "Hey guys, nested scopes are broken. Please fix!" + end + end + assert_equal 2, comment.post_id + end + + def test_nested_exclusive_scope_for_create + comment = nil + Comment.with_scope(:create => { :body => "Hey guys, nested scopes are broken. Please fix!" }) do + Comment.with_exclusive_scope(:create => { :post_id => 1 }) do + assert_equal({ :post_id => 1 }, Comment.send(:current_scoped_methods)[:create]) + comment = Comment.create :body => "Hey guys" + end + end + assert_equal 1, comment.post_id + assert_equal 'Hey guys', comment.body + end + def test_merged_scoped_find_on_blank_conditions [nil, " ", [], {}].each do |blank| Developer.with_scope(:find => {:conditions => blank}) do @@ -523,7 +555,6 @@ class HasManyScopingTest< ActiveRecord::TestCase end end - class HasAndBelongsToManyScopingTest< ActiveRecord::TestCase fixtures :posts, :categories, :categories_posts @@ -549,7 +580,6 @@ class HasAndBelongsToManyScopingTest< ActiveRecord::TestCase end end - class DefaultScopingTest < ActiveRecord::TestCase fixtures :developers @@ -577,7 +607,7 @@ class DefaultScopingTest < ActiveRecord::TestCase # Scopes added on children should append to parent scope expected_klass_scope = [{ :create => {}, :find => { :order => 'salary DESC' }}, { :create => {}, :find => {} }] assert_equal expected_klass_scope, klass.send(:scoped_methods) - + # Parent should still have the original scope assert_equal scope, DeveloperOrderedBySalary.send(:scoped_methods) end @@ -620,7 +650,6 @@ end =begin # We disabled the scoping for has_one and belongs_to as we can't think of a proper use case - class BelongsToScopingTest< ActiveRecord::TestCase fixtures :comments, :posts @@ -640,7 +669,6 @@ class BelongsToScopingTest< ActiveRecord::TestCase end - class HasOneScopingTest< ActiveRecord::TestCase fixtures :comments, :posts diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index 9f3a3848e2..4b2be0987e 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -247,7 +247,7 @@ class NamedScopeTest < ActiveRecord::TestCase topic = Topic.approved.create!({}) assert topic.approved end - + def test_should_build_with_proxy_options_chained topic = Topic.approved.by_lifo.build({}) assert topic.approved @@ -287,15 +287,21 @@ class NamedScopeTest < ActiveRecord::TestCase assert_equal post.comments.size, Post.scoped(:joins => join).scoped(:joins => join, :conditions => "posts.id = #{post.id}").size end - def test_chanining_should_use_latest_conditions_when_creating - post1 = Topic.rejected.approved.new - assert post1.approved? + def test_chaining_should_use_latest_conditions_when_creating + post = Topic.rejected.new + assert !post.approved? + + post = Topic.rejected.approved.new + assert post.approved? - post2 = Topic.approved.rejected.new - assert ! post2.approved? + post = Topic.approved.rejected.new + assert !post.approved? + + post = Topic.approved.rejected.approved.new + assert post.approved? end - def test_chanining_should_use_latest_conditions_when_searching + def test_chaining_should_use_latest_conditions_when_searching # Normal hash conditions assert_equal Topic.all(:conditions => {:approved => true}), Topic.rejected.approved.all assert_equal Topic.all(:conditions => {:approved => false}), Topic.approved.rejected.all @@ -306,7 +312,7 @@ class NamedScopeTest < ActiveRecord::TestCase # Nested hash conditions with different keys assert_equal [posts(:sti_comments)], Post.with_special_comments.with_post(4).all.uniq end - + def test_methods_invoked_within_scopes_should_respect_scope assert_equal [], Topic.approved.by_rejected_ids.proxy_options[:conditions][:id] end -- cgit v1.2.3 From 19ad375e7afa99dc8bc5d859c478bae19c5ddc18 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 10 Mar 2009 23:11:05 -0700 Subject: Don't duplicate :order from scope and options, it makes mysql do extra work --- activerecord/lib/active_record/base.rb | 4 +++- activerecord/test/cases/method_scoping_test.rb | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 4f39761672..62bdf0483a 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1741,7 +1741,9 @@ module ActiveRecord #:nodoc: scoped_order = scope[:order] if scope if order sql << " ORDER BY #{order}" - sql << ", #{scoped_order}" if scoped_order + if scoped_order && scoped_order != order + sql << ", #{scoped_order}" + end else sql << " ORDER BY #{scoped_order}" if scoped_order end diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb index 68a7017f86..3c34cdeade 100644 --- a/activerecord/test/cases/method_scoping_test.rb +++ b/activerecord/test/cases/method_scoping_test.rb @@ -378,8 +378,10 @@ class NestedScopingTest < ActiveRecord::TestCase def test_merged_scoped_find poor_jamis = developers(:poor_jamis) Developer.with_scope(:find => { :conditions => "salary < 100000" }) do - Developer.with_scope(:find => { :offset => 1 }) do - assert_equal(poor_jamis, Developer.find(:first, :order => 'id asc')) + Developer.with_scope(:find => { :offset => 1, :order => 'id asc' }) do + assert_sql /ORDER BY id asc / do + assert_equal(poor_jamis, Developer.find(:first, :order => 'id asc')) + end end end end -- cgit v1.2.3 From 04333482bd665e4d4ced167ff4f566ecfc0cc919 Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Wed, 11 Mar 2009 14:08:40 +0000 Subject: Rename ActiveRecord::Base.each to ActiveRecord::Base.find_each --- activerecord/CHANGELOG | 3 +-- activerecord/lib/active_record/batches.rb | 4 ++-- activerecord/test/cases/batches_test.rb | 6 +++--- 3 files changed, 6 insertions(+), 7 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 404481ea38..6d2f597004 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,10 +1,9 @@ *2.3.1 [RC2] (March 5, 2009)* -* Added ActiveRecord::Base.each and ActiveRecord::Base.find_in_batches for batch processing [DHH/Jamis Buck] +* Added ActiveRecord::Base.find_each and ActiveRecord::Base.find_in_batches for batch processing [DHH/Jamis Buck] * Added that ActiveRecord::Base.exists? can be called with no arguments #1817 [Scott Taylor] - *2.3.0 [RC1] (February 1st, 2009)* * Add Support for updating deeply nested models from a single form. #1202 [Eloy Duran] diff --git a/activerecord/lib/active_record/batches.rb b/activerecord/lib/active_record/batches.rb index 9e9c8fbbf4..4ea932f34d 100644 --- a/activerecord/lib/active_record/batches.rb +++ b/activerecord/lib/active_record/batches.rb @@ -11,14 +11,14 @@ module ActiveRecord # # Example: # - # Person.each(:conditions => "age > 21") do |person| + # Person.find_each(:conditions => "age > 21") do |person| # person.party_all_night! # end # # Note: This method is only intended to use for batch processing of large amounts of records that wouldn't fit in # memory all at once. If you just need to loop over less than 1000 records, it's probably better just to use the # regular find methods. - def each(options = {}) + def find_each(options = {}) find_in_batches(options) do |records| records.each { |record| yield record } end diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb index 108d679108..705da9f88e 100644 --- a/activerecord/test/cases/batches_test.rb +++ b/activerecord/test/cases/batches_test.rb @@ -11,7 +11,7 @@ class EachTest < ActiveRecord::TestCase def test_each_should_excecute_one_query_per_batch assert_queries(Post.count + 1) do - Post.each(:batch_size => 1) do |post| + Post.find_each(:batch_size => 1) do |post| assert_kind_of Post, post end end @@ -19,13 +19,13 @@ class EachTest < ActiveRecord::TestCase def test_each_should_raise_if_the_order_is_set assert_raise(RuntimeError) do - Post.each(:order => "title") { |post| post } + Post.find_each(:order => "title") { |post| post } end end def test_each_should_raise_if_the_limit_is_set assert_raise(RuntimeError) do - Post.each(:limit => 1) { |post| post } + Post.find_each(:limit => 1) { |post| post } end end -- cgit v1.2.3 From 0b6f514947c38bf7af2b7e98905a49a94832e5f6 Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Wed, 11 Mar 2009 14:26:26 +0000 Subject: Add NamedScope#find_each tests [#2201 state:resolved] --- activerecord/test/cases/named_scope_test.rb | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index 4b2be0987e..a441a8f6e3 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -15,7 +15,7 @@ class NamedScopeTest < ActiveRecord::TestCase assert_equal Topic.find(:all), Topic.base assert_equal Topic.find(:all), Topic.base.to_a assert_equal Topic.find(:first), Topic.base.first - assert_equal Topic.find(:all), Topic.base.each { |i| i } + assert_equal Topic.find(:all), Topic.base.map { |i| i } end def test_found_items_are_cached @@ -316,6 +316,20 @@ class NamedScopeTest < ActiveRecord::TestCase def test_methods_invoked_within_scopes_should_respect_scope assert_equal [], Topic.approved.by_rejected_ids.proxy_options[:conditions][:id] end + + def test_named_scopes_batch_finders + assert_equal 3, Topic.approved.count + + assert_queries(4) do + Topic.approved.find_each(:batch_size => 1) {|t| assert t.approved? } + end + + assert_queries(3) do + Topic.approved.find_in_batches(:batch_size => 2) do |group| + group.each {|t| assert t.approved? } + end + end + end end class DynamicScopeMatchTest < ActiveRecord::TestCase -- cgit v1.2.3 From f23adf0ed4bd596c9e7688455489a79f8b35eb3f Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Wed, 11 Mar 2009 14:52:51 +0000 Subject: Add tests for AssociationCollection#find_each and AssociationCollection#find_in_batches --- .../associations/association_collection.rb | 2 +- .../associations/has_many_associations_test.rb | 39 ++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 0fefec1216..f024f99a34 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -60,7 +60,7 @@ module ActiveRecord @reflection.klass.find(*args) end end - + # Fetches the first one using SQL if possible. def first(*args) if fetch_first_or_last_using_find?(args) diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index b7fa9d9d7c..99e4dc7e6e 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -219,6 +219,45 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 1, firm.clients.find(:all, :conditions => "name = 'Summit'").length end + def test_find_each + firm = companies(:first_firm) + + assert ! firm.clients.loaded? + + assert_queries(3) do + firm.clients.find_each(:batch_size => 1) {|c| assert_equal firm.id, c.firm_id } + end + + assert ! firm.clients.loaded? + end + + def test_find_each_with_conditions + firm = companies(:first_firm) + + assert_queries(2) do + firm.clients.find_each(:batch_size => 1, :conditions => {:name => "Microsoft"}) do |c| + assert_equal firm.id, c.firm_id + assert_equal "Microsoft", c.name + end + end + + assert ! firm.clients.loaded? + end + + def test_find_in_batches + firm = companies(:first_firm) + + assert ! firm.clients.loaded? + + assert_queries(2) do + firm.clients.find_in_batches(:batch_size => 2) do |clients| + clients.each {|c| assert_equal firm.id, c.firm_id } + end + end + + assert ! firm.clients.loaded? + end + def test_find_all_sanitized firm = Firm.find(:first) summit = firm.clients.find(:all, :conditions => "name = 'Summit'") -- cgit v1.2.3 From 106976df0911e423042ec4abc165fd561766a047 Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Wed, 11 Mar 2009 15:24:30 +0000 Subject: Ensure ActiveRecord::Base.find_in_batches fires doesnt fire an extra query unless needed --- activerecord/lib/active_record/batches.rb | 5 ++++- activerecord/test/cases/batches_test.rb | 12 ++++++++++++ activerecord/test/cases/named_scope_test.rb | 2 +- 3 files changed, 17 insertions(+), 2 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/batches.rb b/activerecord/lib/active_record/batches.rb index 4ea932f34d..03bd4f9f93 100644 --- a/activerecord/lib/active_record/batches.rb +++ b/activerecord/lib/active_record/batches.rb @@ -49,12 +49,15 @@ module ActiveRecord raise "You can't specify a limit, it's forced to be the batch_size" if options[:limit] start = options.delete(:start).to_i + batch_size = options.delete(:batch_size) || 1000 - with_scope(:find => options.merge(:order => batch_order, :limit => options.delete(:batch_size) || 1000)) do + with_scope(:find => options.merge(:order => batch_order, :limit => batch_size)) do records = find(:all, :conditions => [ "#{table_name}.#{primary_key} >= ?", start ]) while records.any? yield records + + break if records.size < batch_size records = find(:all, :conditions => [ "#{table_name}.#{primary_key} > ?", records.last.id ]) end end diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb index 705da9f88e..5009a90846 100644 --- a/activerecord/test/cases/batches_test.rb +++ b/activerecord/test/cases/batches_test.rb @@ -46,4 +46,16 @@ class EachTest < ActiveRecord::TestCase end end end + + def test_find_in_batches_shouldnt_excute_query_unless_needed + post_count = Post.count + + assert_queries(2) do + Post.find_in_batches(:batch_size => post_count) {|batch| assert_kind_of Array, batch } + end + + assert_queries(1) do + Post.find_in_batches(:batch_size => post_count + 1) {|batch| assert_kind_of Array, batch } + end + end end \ No newline at end of file diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index a441a8f6e3..8045b136c2 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -324,7 +324,7 @@ class NamedScopeTest < ActiveRecord::TestCase Topic.approved.find_each(:batch_size => 1) {|t| assert t.approved? } end - assert_queries(3) do + assert_queries(2) do Topic.approved.find_in_batches(:batch_size => 2) do |group| group.each {|t| assert t.approved? } end -- cgit v1.2.3 From 92dadf6d7927fde1482ba1d96d0916093bfb83ca Mon Sep 17 00:00:00 2001 From: Will Bryant Date: Thu, 12 Mar 2009 02:13:26 +1300 Subject: Fixed autosave checks on objects with hm:t in :include [#2213 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/autosave_association.rb | 2 +- .../cases/associations/belongs_to_associations_test.rb | 16 ++++++++++++++++ .../test/cases/associations/has_one_associations_test.rb | 16 ++++++++++++++++ .../associations/has_one_through_associations_test.rb | 16 ++++++++++++++++ 4 files changed, 49 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 1c3d0567c1..6dcc5005d1 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -310,7 +310,7 @@ module ActiveRecord # 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 (association = association_instance_get(reflection.name)) && !association.target.nil? 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] diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index ff3e54712e..13a78a1890 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -326,4 +326,20 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase companies(:first_firm).send(:private_method) companies(:second_client).firm.send(:private_method) end + + def test_save_of_record_with_loaded_belongs_to + @account = companies(:first_firm).account + + assert_nothing_raised do + Account.find(@account.id).save! + Account.find(@account.id, :include => :firm).save! + end + + @account.firm.delete + + assert_nothing_raised do + Account.find(@account.id).save! + Account.find(@account.id, :include => :firm).save! + end + end end diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index 4947f1543c..1ddb3f49bf 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -290,4 +290,20 @@ class HasOneAssociationsTest < ActiveRecord::TestCase companies(:first_firm).account.send(:private_method) end + def test_save_of_record_with_loaded_has_one + @firm = companies(:first_firm) + assert_not_nil @firm.account + + assert_nothing_raised do + Firm.find(@firm.id).save! + Firm.find(@firm.id, :include => :account).save! + end + + @firm.account.destroy + + assert_nothing_raised do + Firm.find(@firm.id).save! + Firm.find(@firm.id, :include => :account).save! + end + end end diff --git a/activerecord/test/cases/associations/has_one_through_associations_test.rb b/activerecord/test/cases/associations/has_one_through_associations_test.rb index f96b55513e..12c598751b 100644 --- a/activerecord/test/cases/associations/has_one_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb @@ -173,4 +173,20 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase assert_not_nil assert_no_queries { @new_detail.member_type } end + def test_save_of_record_with_loaded_has_one_through + @club = @member.club + assert_not_nil @club.sponsored_member + + assert_nothing_raised do + Club.find(@club.id).save! + Club.find(@club.id, :include => :sponsored_member).save! + end + + @club.sponsor.destroy + + assert_nothing_raised do + Club.find(@club.id).save! + Club.find(@club.id, :include => :sponsored_member).save! + end + end end -- cgit v1.2.3 From db26ace030f6704da6fc80bcc6cd00a2aee664ce Mon Sep 17 00:00:00 2001 From: Murray Steele Date: Thu, 12 Mar 2009 13:49:16 +0000 Subject: Ensure NoMethodError isn't raised when some of the nested eager loaded associations are empty [#1696 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/associations.rb | 13 +++++++--- .../associations/eager_load_nested_include_test.rb | 29 ++++++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 301b3a3b58..6d25b36aea 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1856,9 +1856,10 @@ module ActiveRecord def construct(parent, associations, joins, row) case associations when Symbol, String - while (join = joins.shift).reflection.name.to_s != associations.to_s - raise ConfigurationError, "Not Enough Associations" if joins.empty? - end + join = joins.detect{|j| j.reflection.name.to_s == associations.to_s && j.parent_table_name == parent.class.table_name } + raise(ConfigurationError, "No such association") if join.nil? + + joins.delete(join) construct_association(parent, join, row) when Array associations.each do |association| @@ -1866,7 +1867,11 @@ module ActiveRecord end when Hash associations.keys.sort{|a,b|a.to_s<=>b.to_s}.each do |name| - association = construct_association(parent, joins.shift, row) + join = joins.detect{|j| j.reflection.name.to_s == name.to_s && j.parent_table_name == parent.class.table_name } + raise(ConfigurationError, "No such association") if join.nil? + + association = construct_association(parent, join, row) + joins.delete(join) construct(association, associations[name], joins, row) if association end else diff --git a/activerecord/test/cases/associations/eager_load_nested_include_test.rb b/activerecord/test/cases/associations/eager_load_nested_include_test.rb index 12dec5ccd1..1b2e0fc11e 100644 --- a/activerecord/test/cases/associations/eager_load_nested_include_test.rb +++ b/activerecord/test/cases/associations/eager_load_nested_include_test.rb @@ -1,4 +1,9 @@ require 'cases/helper' +require 'models/author' +require 'models/post' +require 'models/comment' +require 'models/category' +require 'models/categorization' module Remembered def self.included(base) @@ -99,3 +104,27 @@ class EagerLoadPolyAssocsTest < ActiveRecord::TestCase end end end + +class EagerLoadNestedIncludeWithMissingDataTest < ActiveRecord::TestCase + def setup + @davey_mcdave = Author.create(:name => 'Davey McDave') + @first_post = @davey_mcdave.posts.create(:title => 'Davey Speaks', :body => 'Expressive wordage') + @first_comment = @first_post.comments.create(:body => 'Inflamatory doublespeak') + @first_categorization = @davey_mcdave.categorizations.create(:category => Category.first, :post => @first_post) + end + + def teardown + @davey_mcdave.destroy + @first_post.destroy + @first_comment.destroy + @first_categorization.destroy + end + + def test_missing_data_in_a_nested_include_should_not_cause_errors_when_constructing_objects + assert_nothing_raised do + # @davey_mcdave doesn't have any author_favorites + includes = {:posts => :comments, :categorizations => :category, :author_favorites => :favorite_author } + Author.all :include => includes, :conditions => {:authors => {:name => @davey_mcdave.name}}, :order => 'categories.name' + end + end +end \ No newline at end of file -- cgit v1.2.3 From 91b98cf0a5417ce4042a0b3cd1930d5a221b737f Mon Sep 17 00:00:00 2001 From: Elijah Miller Date: Thu, 12 Mar 2009 15:06:19 +0000 Subject: Returning nil from named scope lambda is equivalent to an empty hash [#1773 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/named_scope.rb | 1 + activerecord/test/cases/named_scope_test.rb | 6 ++++++ activerecord/test/models/topic.rb | 4 +++- 3 files changed, 10 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 519941dd94..1f3ef300f2 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -117,6 +117,7 @@ module ActiveRecord delegate :scopes, :with_scope, :to => :proxy_scope def initialize(proxy_scope, options, &block) + options ||= {} [options[:extend]].flatten.each { |extension| extend extension } if options[:extend] extend Module.new(&block) if block_given? unless Scope === proxy_scope diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index 8045b136c2..ae6a54a5bd 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -99,6 +99,12 @@ class NamedScopeTest < ActiveRecord::TestCase assert_equal topics_written_before_the_second, Topic.written_before(topics(:second).written_on) end + def test_procedural_scopes_returning_nil + all_topics = Topic.find(:all) + + assert_equal all_topics, Topic.written_before(nil) + end + def test_scopes_with_joins address = author_addresses(:david_address) posts_with_authors_at_address = Post.find( diff --git a/activerecord/test/models/topic.rb b/activerecord/test/models/topic.rb index f1b7bbae3b..51012d22ed 100644 --- a/activerecord/test/models/topic.rb +++ b/activerecord/test/models/topic.rb @@ -1,7 +1,9 @@ class Topic < ActiveRecord::Base named_scope :base named_scope :written_before, lambda { |time| - { :conditions => ['written_on < ?', time] } + if time + { :conditions => ['written_on < ?', time] } + end } named_scope :approved, :conditions => {:approved => true} named_scope :rejected, :conditions => {:approved => false} -- cgit v1.2.3 From 47bdf3bf40ec17e1f8ca1c0e3d7f697d0c4cd1bf Mon Sep 17 00:00:00 2001 From: Luca Guidi Date: Thu, 12 Mar 2009 15:24:37 +0000 Subject: Ensure AutosaveAssociation runs remove callbacks [#2146 state:resolved] Signed-off-by: Eloy Duran Signed-off-by: Pratik Naik --- .../associations/association_collection.rb | 51 ++++++++++++++-------- .../lib/active_record/autosave_association.rb | 2 +- .../has_and_belongs_to_many_associations_test.rb | 27 ++++++++++++ .../associations/has_many_associations_test.rb | 24 ++++++++++ .../has_many_through_associations_test.rb | 18 ++++++++ .../test/cases/autosave_association_test.rb | 35 +++++++++++++++ activerecord/test/models/pirate.rb | 49 ++++++++++++++++++++- 7 files changed, 185 insertions(+), 21 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index f024f99a34..ad375be184 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -186,7 +186,6 @@ module ActiveRecord end end - # Removes +records+ from this association calling +before_remove+ and # +after_remove+ callbacks. # @@ -195,22 +194,25 @@ module ActiveRecord # are actually removed from the database, that depends precisely on # +delete_records+. They are in any case removed from the collection. def delete(*records) - records = flatten_deeper(records) - records.each { |record| raise_on_type_mismatch(record) } - - transaction do - records.each { |record| callback(:before_remove, record) } - - old_records = records.reject {|r| r.new_record? } + remove_records(records) do |records, old_records| delete_records(old_records) if old_records.any? - - records.each do |record| - @target.delete(record) - callback(:after_remove, record) - end + records.each { |record| @target.delete(record) } end end + # Destroy +records+ and remove from this association calling +before_remove+ + # and +after_remove+ callbacks. + # + # Note this method will always remove records from database ignoring the + # +:dependent+ option. + def destroy(*records) + remove_records(records) do |records, old_records| + old_records.each { |record| record.destroy } + end + + load_target + end + # Removes all records from this association. Returns +self+ so method calls may be chained. def clear return self if length.zero? # forces load_target if it hasn't happened already @@ -223,15 +225,14 @@ module ActiveRecord self end - - def destroy_all - transaction do - each { |record| record.destroy } - end + # Destory all the records from this association + def destroy_all + load_target + destroy(@target) reset_target! end - + def create(attrs = {}) if attrs.is_a?(Array) attrs.collect { |attr| create(attr) } @@ -431,6 +432,18 @@ module ActiveRecord record end + def remove_records(*records) + records = flatten_deeper(records) + records.each { |record| raise_on_type_mismatch(record) } + + transaction do + records.each { |record| callback(:before_remove, record) } + old_records = records.reject { |r| r.new_record? } + yield(records, old_records) + records.each { |record| callback(:after_remove, record) } + end + end + def callback(method, record) callbacks_for(method).each do |callback| ActiveSupport::Callbacks::Callback.new(method, callback, record).call(@owner, record) diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 6dcc5005d1..741aa2acbe 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -283,7 +283,7 @@ module ActiveRecord 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 + association.destroy(record) elsif @new_record_before_save || record.new_record? if autosave association.send(:insert_record, record, false, false) diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index ca1772d1ca..5e8b2cadfc 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -381,6 +381,33 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_date_from_db Date.new(2004, 10, 10), Developer.find(1).projects.first.joined_on.to_date end + def test_destroying + david = Developer.find(1) + active_record = Project.find(1) + david.projects.reload + assert_equal 2, david.projects.size + assert_equal 3, active_record.developers.size + + assert_difference "Project.count", -1 do + david.projects.destroy(active_record) + end + + assert_equal 1, david.reload.projects.size + assert_equal 1, david.projects(true).size + end + + def test_destroying_array + david = Developer.find(1) + david.projects.reload + + assert_difference "Project.count", -Project.count do + david.projects.destroy(Project.find(:all)) + end + + assert_equal 0, david.reload.projects.size + assert_equal 0, david.projects(true).size + end + def test_destroy_all david = Developer.find(1) david.projects.reload diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 99e4dc7e6e..30edf79a26 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -680,6 +680,30 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_raise(ActiveRecord::AssociationTypeMismatch) { david.projects.delete(Project.find(1).developers) } end + def test_destroying + force_signal37_to_load_all_clients_of_firm + + assert_difference "Client.count", -1 do + companies(:first_firm).clients_of_firm.destroy(companies(:first_firm).clients_of_firm.first) + end + + assert_equal 0, companies(:first_firm).reload.clients_of_firm.size + assert_equal 0, companies(:first_firm).clients_of_firm(true).size + end + + def test_destroying_a_collection + force_signal37_to_load_all_clients_of_firm + companies(:first_firm).clients_of_firm.create("name" => "Another Client") + assert_equal 2, companies(:first_firm).clients_of_firm.size + + assert_difference "Client.count", -2 do + companies(:first_firm).clients_of_firm.destroy([companies(:first_firm).clients_of_firm[0], companies(:first_firm).clients_of_firm[1]]) + end + + assert_equal 0, companies(:first_firm).reload.clients_of_firm.size + assert_equal 0, companies(:first_firm).clients_of_firm(true).size + end + def test_destroy_all force_signal37_to_load_all_clients_of_firm assert !companies(:first_firm).clients_of_firm.empty?, "37signals has clients after load" diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index c3ad0ee6de..97efca7891 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -92,6 +92,24 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert posts(:welcome).reload.people(true).empty? end + def test_destroy_association + assert_difference "Person.count", -1 do + posts(:welcome).people.destroy(people(:michael)) + end + + assert posts(:welcome).reload.people.empty? + assert posts(:welcome).people(true).empty? + end + + def test_destroy_all + assert_difference "Person.count", -1 do + posts(:welcome).people.destroy_all + end + + assert posts(:welcome).reload.people.empty? + assert posts(:welcome).people(true).empty? + end + def test_replace_association assert_queries(4){posts(:welcome);people(:david);people(:michael); posts(:welcome).people(true)} diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index b179bd827a..436f50d395 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -556,6 +556,41 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase assert_raise(RuntimeError) { assert !@pirate.save } assert_equal before, @pirate.reload.send(association_name) end + + # Add and remove callbacks tests for association collections. + %w{ method proc }.each do |callback_type| + define_method("test_should_run_add_callback_#{callback_type}s_for_#{association_name}") do + association_name_with_callbacks = "#{association_name}_with_#{callback_type}_callbacks" + + pirate = Pirate.new(:catchphrase => "Arr") + pirate.send(association_name_with_callbacks).build(:name => "Crowe the One-Eyed") + + expected = [ + "before_adding_#{callback_type}_#{association_name.singularize}_", + "after_adding_#{callback_type}_#{association_name.singularize}_" + ] + + assert_equal expected, pirate.ship_log + end + + define_method("test_should_run_remove_callback_#{callback_type}s_for_#{association_name}") do + association_name_with_callbacks = "#{association_name}_with_#{callback_type}_callbacks" + + @pirate.send(association_name_with_callbacks).create!(:name => "Crowe the One-Eyed") + @pirate.send(association_name_with_callbacks).each { |c| c.mark_for_destruction } + child_id = @pirate.send(association_name_with_callbacks).first.id + + @pirate.ship_log.clear + @pirate.save + + expected = [ + "before_removing_#{callback_type}_#{association_name.singularize}_#{child_id}", + "after_removing_#{callback_type}_#{association_name.singularize}_#{child_id}" + ] + + assert_equal expected, @pirate.ship_log + end + end end end diff --git a/activerecord/test/models/pirate.rb b/activerecord/test/models/pirate.rb index 7bc50e0e7b..238917bf30 100644 --- a/activerecord/test/models/pirate.rb +++ b/activerecord/test/models/pirate.rb @@ -1,16 +1,63 @@ class Pirate < ActiveRecord::Base belongs_to :parrot has_and_belongs_to_many :parrots - has_many :treasures, :as => :looter + has_and_belongs_to_many :parrots_with_method_callbacks, :class_name => "Parrot", + :before_add => :log_before_add, + :after_add => :log_after_add, + :before_remove => :log_before_remove, + :after_remove => :log_after_remove + has_and_belongs_to_many :parrots_with_proc_callbacks, :class_name => "Parrot", + :before_add => proc {|p,pa| p.ship_log << "before_adding_proc_parrot_#{pa.id || ''}"}, + :after_add => proc {|p,pa| p.ship_log << "after_adding_proc_parrot_#{pa.id || ''}"}, + :before_remove => proc {|p,pa| p.ship_log << "before_removing_proc_parrot_#{pa.id}"}, + :after_remove => proc {|p,pa| p.ship_log << "after_removing_proc_parrot_#{pa.id}"} + has_many :treasures, :as => :looter has_many :treasure_estimates, :through => :treasures, :source => :price_estimates # These both have :autosave enabled because accepts_nested_attributes_for is used on them. has_one :ship has_many :birds + has_many :birds_with_method_callbacks, :class_name => "Bird", + :before_add => :log_before_add, + :after_add => :log_after_add, + :before_remove => :log_before_remove, + :after_remove => :log_after_remove + has_many :birds_with_proc_callbacks, :class_name => "Bird", + :before_add => proc {|p,b| p.ship_log << "before_adding_proc_bird_#{b.id || ''}"}, + :after_add => proc {|p,b| p.ship_log << "after_adding_proc_bird_#{b.id || ''}"}, + :before_remove => proc {|p,b| p.ship_log << "before_removing_proc_bird_#{b.id}"}, + :after_remove => proc {|p,b| p.ship_log << "after_removing_proc_bird_#{b.id}"} accepts_nested_attributes_for :parrots, :birds, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } + accepts_nested_attributes_for :parrots_with_method_callbacks, :parrots_with_proc_callbacks, + :birds_with_method_callbacks, :birds_with_proc_callbacks, :allow_destroy => true validates_presence_of :catchphrase + + def ship_log + @ship_log ||= [] + end + + private + def log_before_add(record) + log(record, "before_adding_method") + end + + def log_after_add(record) + log(record, "after_adding_method") + end + + def log_before_remove(record) + log(record, "before_removing_method") + end + + def log_after_remove(record) + log(record, "after_removing_method") + end + + def log(record, callback) + ship_log << "#{callback}_#{record.class.name.downcase}_#{record.id || ''}" + end end -- cgit v1.2.3 From 3c64c9a5756193247ac87bb55326b6387dfd3070 Mon Sep 17 00:00:00 2001 From: Chris Kampmeier Date: Thu, 12 Mar 2009 15:41:31 +0000 Subject: Fix spelling of an internal method [#1734 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/base.rb | 6 +++--- activerecord/lib/active_record/validations.rb | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 62bdf0483a..4ef3cc698b 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1344,7 +1344,7 @@ module ActiveRecord #:nodoc: subclasses.each { |klass| klass.reset_inheritable_attributes; klass.reset_column_information } end - def self_and_descendents_from_active_record#nodoc: + def self_and_descendants_from_active_record#nodoc: klass = self classes = [klass] while klass != klass.base_class @@ -1364,7 +1364,7 @@ module ActiveRecord #:nodoc: # module now. # Specify +options+ with additional translating options. def human_attribute_name(attribute_key_name, options = {}) - defaults = self_and_descendents_from_active_record.map do |klass| + defaults = self_and_descendants_from_active_record.map do |klass| :"#{klass.name.underscore}.#{attribute_key_name}" end defaults << options[:default] if options[:default] @@ -1379,7 +1379,7 @@ module ActiveRecord #:nodoc: # Default scope of the translation is activerecord.models # Specify +options+ with additional translating options. def human_name(options = {}) - defaults = self_and_descendents_from_active_record.map do |klass| + defaults = self_and_descendants_from_active_record.map do |klass| :"#{klass.name.underscore}" end defaults << self.name.humanize diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index ba60180ae0..4e084f071c 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -89,7 +89,7 @@ module ActiveRecord message, options[:default] = options[:default], message if options[:default].is_a?(Symbol) - defaults = @base.class.self_and_descendents_from_active_record.map do |klass| + defaults = @base.class.self_and_descendants_from_active_record.map do |klass| [ :"models.#{klass.name.underscore}.attributes.#{attribute}.#{message}", :"models.#{klass.name.underscore}.#{message}" ] end -- cgit v1.2.3 From b304b0be013a05b19708659a350b89fc071e9a7d Mon Sep 17 00:00:00 2001 From: Eloy Duran Date: Thu, 12 Mar 2009 16:37:50 +0100 Subject: Fixed some spelling errors in documentation about AssociationCollection::destroy and AssociationCollection::destroy_all. Also fixed some whitespace. --- .../associations/association_collection.rb | 48 ++++++++++++---------- 1 file changed, 26 insertions(+), 22 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index ad375be184..bcf4a91f0b 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -20,7 +20,7 @@ module ActiveRecord super construct_sql end - + def find(*args) options = args.extract_options! @@ -41,7 +41,7 @@ module ActiveRecord if sanitized_conditions = sanitize_sql(options[:conditions]) conditions << " AND (#{sanitized_conditions})" end - + options[:conditions] = conditions if options[:order] && @reflection.options[:order] @@ -49,12 +49,12 @@ module ActiveRecord elsif @reflection.options[:order] options[:order] = @reflection.options[:order] end - + # Build options specific to association construct_find_options!(options) - + merge_options_from_reflection!(options) - + # Pass through args exactly as we received them. args << options @reflection.klass.find(*args) @@ -142,13 +142,6 @@ module ActiveRecord end end - # Remove all records from this association - def delete_all - load_target - delete(@target) - reset_target! - end - # Calculate sum using SQL, not Enumerable def sum(*args) if block_given? @@ -200,11 +193,11 @@ module ActiveRecord end end - # Destroy +records+ and remove from this association calling +before_remove+ - # and +after_remove+ callbacks. + # Destroy +records+ and remove them from this association calling + # +before_remove+ and +after_remove+ callbacks. # - # Note this method will always remove records from database ignoring the - # +:dependent+ option. + # Note that this method will _always_ remove records from the database + # ignoring the +:dependent+ option. def destroy(*records) remove_records(records) do |records, old_records| old_records.each { |record| record.destroy } @@ -219,14 +212,25 @@ module ActiveRecord if @reflection.options[:dependent] && @reflection.options[:dependent] == :destroy destroy_all - else + else delete_all end self end - # Destory all the records from this association + # Remove all records from this association. + # + # See delete for more info. + def delete_all + load_target + delete(@target) + reset_target! + end + + # Destroy all the records from this association. + # + # See destroy for more info. def destroy_all load_target destroy(@target) @@ -357,7 +361,7 @@ module ActiveRecord loaded if target target end - + def method_missing(method, *args) if @target.respond_to?(method) || (!@reflection.klass.respond_to?(method) && Class.respond_to?(method)) if block_given? @@ -367,7 +371,7 @@ module ActiveRecord end elsif @reflection.klass.scopes.include?(method) @reflection.klass.scopes[method].call(self, *args) - else + else with_scope(construct_scope) do if block_given? @reflection.klass.send(method, *args) { |*block_args| yield(*block_args) } @@ -453,8 +457,8 @@ module ActiveRecord def callbacks_for(callback_name) full_callback_name = "#{callback_name}_for_#{@reflection.name}" @owner.class.read_inheritable_attribute(full_callback_name.to_sym) || [] - end - + end + def ensure_owner_is_not_new if @owner.new_record? raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved" -- cgit v1.2.3 From 8e277f89cdbf7266c700703e437525cc8ea35c1d Mon Sep 17 00:00:00 2001 From: Eloy Duran Date: Thu, 12 Mar 2009 16:46:38 +0100 Subject: Revert "Fixed some spelling errors in documentation about AssociationCollection::destroy and AssociationCollection::destroy_all." This reverts commit b304b0be013a05b19708659a350b89fc071e9a7d. --- .../associations/association_collection.rb | 48 ++++++++++------------ 1 file changed, 22 insertions(+), 26 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index bcf4a91f0b..ad375be184 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -20,7 +20,7 @@ module ActiveRecord super construct_sql end - + def find(*args) options = args.extract_options! @@ -41,7 +41,7 @@ module ActiveRecord if sanitized_conditions = sanitize_sql(options[:conditions]) conditions << " AND (#{sanitized_conditions})" end - + options[:conditions] = conditions if options[:order] && @reflection.options[:order] @@ -49,12 +49,12 @@ module ActiveRecord elsif @reflection.options[:order] options[:order] = @reflection.options[:order] end - + # Build options specific to association construct_find_options!(options) - + merge_options_from_reflection!(options) - + # Pass through args exactly as we received them. args << options @reflection.klass.find(*args) @@ -142,6 +142,13 @@ module ActiveRecord end end + # Remove all records from this association + def delete_all + load_target + delete(@target) + reset_target! + end + # Calculate sum using SQL, not Enumerable def sum(*args) if block_given? @@ -193,11 +200,11 @@ module ActiveRecord end end - # Destroy +records+ and remove them from this association calling - # +before_remove+ and +after_remove+ callbacks. + # Destroy +records+ and remove from this association calling +before_remove+ + # and +after_remove+ callbacks. # - # Note that this method will _always_ remove records from the database - # ignoring the +:dependent+ option. + # Note this method will always remove records from database ignoring the + # +:dependent+ option. def destroy(*records) remove_records(records) do |records, old_records| old_records.each { |record| record.destroy } @@ -212,25 +219,14 @@ module ActiveRecord if @reflection.options[:dependent] && @reflection.options[:dependent] == :destroy destroy_all - else + else delete_all end self end - # Remove all records from this association. - # - # See delete for more info. - def delete_all - load_target - delete(@target) - reset_target! - end - - # Destroy all the records from this association. - # - # See destroy for more info. + # Destory all the records from this association def destroy_all load_target destroy(@target) @@ -361,7 +357,7 @@ module ActiveRecord loaded if target target end - + def method_missing(method, *args) if @target.respond_to?(method) || (!@reflection.klass.respond_to?(method) && Class.respond_to?(method)) if block_given? @@ -371,7 +367,7 @@ module ActiveRecord end elsif @reflection.klass.scopes.include?(method) @reflection.klass.scopes[method].call(self, *args) - else + else with_scope(construct_scope) do if block_given? @reflection.klass.send(method, *args) { |*block_args| yield(*block_args) } @@ -457,8 +453,8 @@ module ActiveRecord def callbacks_for(callback_name) full_callback_name = "#{callback_name}_for_#{@reflection.name}" @owner.class.read_inheritable_attribute(full_callback_name.to_sym) || [] - end - + end + def ensure_owner_is_not_new if @owner.new_record? raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved" -- cgit v1.2.3 From de55c8d7453c5fdf385b5fef9228f3327afddfcf Mon Sep 17 00:00:00 2001 From: Eloy Duran Date: Thu, 12 Mar 2009 16:50:10 +0100 Subject: Fixed some spelling errors in documentation about AssociationCollection::destroy and AssociationCollection::destroy_all. This time without the whitespace fixes and move of the destroy_all method. --- .../active_record/associations/association_collection.rb | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index ad375be184..3aef1b21e9 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -143,6 +143,8 @@ module ActiveRecord end # Remove all records from this association + # + # See delete for more info. def delete_all load_target delete(@target) @@ -200,11 +202,11 @@ module ActiveRecord end end - # Destroy +records+ and remove from this association calling +before_remove+ - # and +after_remove+ callbacks. + # Destroy +records+ and remove them from this association calling + # +before_remove+ and +after_remove+ callbacks. # - # Note this method will always remove records from database ignoring the - # +:dependent+ option. + # Note that this method will _always_ remove records from the database + # ignoring the +:dependent+ option. def destroy(*records) remove_records(records) do |records, old_records| old_records.each { |record| record.destroy } @@ -226,7 +228,9 @@ module ActiveRecord self end - # Destory all the records from this association + # Destory all the records from this association. + # + # See destroy for more info. def destroy_all load_target destroy(@target) -- cgit v1.2.3 From 655f95a8a6b79b629b7464522c9b0ecac7311dae Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Thu, 12 Mar 2009 23:28:33 +0100 Subject: in batches.rb, rewrap rdoc, and document options as symbols --- activerecord/lib/active_record/batches.rb | 38 +++++++++++++++++++------------ 1 file changed, 23 insertions(+), 15 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/batches.rb b/activerecord/lib/active_record/batches.rb index 03bd4f9f93..5a6cecd4ad 100644 --- a/activerecord/lib/active_record/batches.rb +++ b/activerecord/lib/active_record/batches.rb @@ -4,10 +4,12 @@ module ActiveRecord base.extend(ClassMethods) end - # When processing large numbers of records, it's often a good idea to do so in batches to prevent memory ballooning. + # When processing large numbers of records, it's often a good idea to do + # so in batches to prevent memory ballooning. module ClassMethods - # Yields each record that was found by the find +options+. The find is performed by find_in_batches - # with a batch size of 1000 (or as specified by the +batch_size+ option). + # Yields each record that was found by the find +options+. The find is + # performed by find_in_batches with a batch size of 1000 (or as + # specified by the :batch_size option). # # Example: # @@ -15,9 +17,10 @@ module ActiveRecord # person.party_all_night! # end # - # Note: This method is only intended to use for batch processing of large amounts of records that wouldn't fit in - # memory all at once. If you just need to loop over less than 1000 records, it's probably better just to use the - # regular find methods. + # Note: This method is only intended to use for batch processing of + # large amounts of records that wouldn't fit in memory all at once. If + # you just need to loop over less than 1000 records, it's probably + # better just to use the regular find methods. def find_each(options = {}) find_in_batches(options) do |records| records.each { |record| yield record } @@ -26,17 +29,22 @@ module ActiveRecord self end - # Yields each batch of records that was found by the find +options+ as an array. The size of each batch is - # set by the +batch_size+ option; the default is 1000. + # Yields each batch of records that was found by the find +options+ as + # an array. The size of each batch is set by the :batch_size + # option; the default is 1000. # - # You can control the starting point for the batch processing by supplying the +start+ option. This is especially - # useful if you want multiple workers dealing with the same processing queue. You can make worker 1 handle all the - # records between id 0 and 10,000 and worker 2 handle from 10,000 and beyond (by setting the +start+ option on that - # worker). + # You can control the starting point for the batch processing by + # supplying the :start option. This is especially useful if you + # want multiple workers dealing with the same processing queue. You can + # make worker 1 handle all the records between id 0 and 10,000 and + # worker 2 handle from 10,000 and beyond (by setting the :start + # option on that worker). # - # It's not possible to set the order. That is automatically set to ascending on the primary key ("id ASC") - # to make the batch ordering work. This also mean that this method only works with integer-based primary keys. - # You can't set the limit either, that's used to control the the batch sizes. + # It's not possible to set the order. That is automatically set to + # ascending on the primary key ("id ASC") to make the batch ordering + # work. This also mean that this method only works with integer-based + # primary keys. You can't set the limit either, that's used to control + # the the batch sizes. # # Example: # -- cgit v1.2.3 From ac3848201dfd7400708d3ccae0acb9388318fb99 Mon Sep 17 00:00:00 2001 From: Jason King Date: Sat, 14 Mar 2009 12:58:42 +0000 Subject: SQLite adapters now support DDL transactions [#2080 state:resolved] Signed-off-by: Pratik Naik --- .../connection_adapters/sqlite_adapter.rb | 55 +++++++++++++--------- activerecord/test/cases/migration_test.rb | 26 +++++++++- activerecord/test/cases/transactions_test.rb | 15 ++++-- 3 files changed, 68 insertions(+), 28 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index 6077ddcdb6..afd6472db8 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -72,6 +72,18 @@ module ActiveRecord # # * :database - Path to the database file. class SQLiteAdapter < AbstractAdapter + class Version + include Comparable + + def initialize(version_string) + @version = version_string.split('.').map(&:to_i) + end + + def <=>(version_string) + @version <=> version_string.split('.').map(&:to_i) + end + end + def initialize(connection, logger, config) super(connection, logger) @config = config @@ -81,6 +93,10 @@ module ActiveRecord 'SQLite' end + def supports_ddl_transactions? + sqlite_version >= '2.0.0' + end + def supports_migrations? #:nodoc: true end @@ -88,6 +104,10 @@ module ActiveRecord def requires_reloading? true end + + def supports_add_column? + sqlite_version >= '3.1.6' + end def disconnect! super @@ -169,7 +189,6 @@ module ActiveRecord catch_schema_changes { @connection.rollback } end - # SELECT ... FOR UPDATE is redundant since the table is locked. def add_lock!(sql, options) #:nodoc: sql @@ -218,14 +237,20 @@ module ActiveRecord execute "ALTER TABLE #{name} RENAME TO #{new_name}" end + # See: http://www.sqlite.org/lang_altertable.html + # SQLite has an additional restriction on the ALTER TABLE statement + def valid_alter_table_options( type, options) + type.to_sym != :primary_key + end + def add_column(table_name, column_name, type, options = {}) #:nodoc: - if @connection.respond_to?(:transaction_active?) && @connection.transaction_active? - raise StatementInvalid, 'Cannot add columns to a SQLite database while inside a transaction' + if supports_add_column? && valid_alter_table_options( type, options ) + super(table_name, column_name, type, options) + else + alter_table(table_name) do |definition| + definition.column(column_name, type, options) + end end - - super(table_name, column_name, type, options) - # See last paragraph on http://www.sqlite.org/lang_altertable.html - execute "VACUUM" end def remove_column(table_name, *column_names) #:nodoc: @@ -385,7 +410,7 @@ module ActiveRecord end def sqlite_version - @sqlite_version ||= select_value('select sqlite_version(*)') + @sqlite_version ||= SQLiteAdapter::Version.new(select_value('select sqlite_version(*)')) end def default_primary_key_type @@ -398,23 +423,9 @@ module ActiveRecord end class SQLite2Adapter < SQLiteAdapter # :nodoc: - def supports_count_distinct? #:nodoc: - false - end - def rename_table(name, new_name) move_table(name, new_name) end - - def add_column(table_name, column_name, type, options = {}) #:nodoc: - if @connection.respond_to?(:transaction_active?) && @connection.transaction_active? - raise StatementInvalid, 'Cannot add columns to a SQLite database while inside a transaction' - end - - alter_table(table_name) do |definition| - definition.column(column_name, type, options) - end - end end class DeprecatedSQLiteAdapter < SQLite2Adapter # :nodoc: diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 50d039ec77..16861f21b1 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -93,6 +93,30 @@ if ActiveRecord::Base.connection.supports_migrations? end end + def testing_table_with_only_foo_attribute + Person.connection.create_table :testings, :id => false do |t| + t.column :foo, :string + end + + yield Person.connection + ensure + Person.connection.drop_table :testings rescue nil + end + protected :testing_table_with_only_foo_attribute + + def test_create_table_without_id + testing_table_with_only_foo_attribute do |connection| + assert_equal connection.columns(:testings).size, 1 + end + end + + def test_add_column_with_primary_key_attribute + testing_table_with_only_foo_attribute do |connection| + assert_nothing_raised { connection.add_column :testings, :id, :primary_key } + assert_equal connection.columns(:testings).size, 2 + end + end + def test_create_table_adds_id Person.connection.create_table :testings do |t| t.column :foo, :string @@ -928,7 +952,7 @@ if ActiveRecord::Base.connection.supports_migrations? assert_equal(0, ActiveRecord::Migrator.current_version) end - if current_adapter?(:PostgreSQLAdapter) + if ActiveRecord::Base.connection.supports_ddl_transactions? def test_migrator_one_up_with_exception_and_rollback assert !Person.column_methods_hash.include?(:last_name) diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index f9ae5115fc..f6533b5396 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -349,7 +349,7 @@ class TransactionTest < ActiveRecord::TestCase end end - def test_sqlite_add_column_in_transaction_raises_statement_invalid + def test_sqlite_add_column_in_transaction return true unless current_adapter?(:SQLite3Adapter, :SQLiteAdapter) # Test first if column creation/deletion works correctly when no @@ -368,10 +368,15 @@ class TransactionTest < ActiveRecord::TestCase assert !Topic.column_names.include?('stuff') end - # Test now inside a transaction: add_column should raise a StatementInvalid - Topic.transaction do - assert_raise(ActiveRecord::StatementInvalid) { Topic.connection.add_column('topics', 'stuff', :string) } - raise ActiveRecord::Rollback + if Topic.connection.supports_ddl_transactions? + assert_nothing_raised do + Topic.transaction { Topic.connection.add_column('topics', 'stuff', :string) } + end + else + Topic.transaction do + assert_raise(ActiveRecord::StatementInvalid) { Topic.connection.add_column('topics', 'stuff', :string) } + raise ActiveRecord::Rollback + end end end -- cgit v1.2.3 From 73fc42cc0b5e94541480032c2941a50edd4080c2 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Sun, 15 Mar 2009 22:06:50 -0500 Subject: Prepare for final 2.3 release --- activerecord/CHANGELOG | 4 +--- activerecord/Rakefile | 2 +- activerecord/lib/active_record/version.rb | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 6d2f597004..c73ac4649e 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,11 +1,9 @@ -*2.3.1 [RC2] (March 5, 2009)* +*2.3.2 [Final] (March 15, 2009)* * Added ActiveRecord::Base.find_each and ActiveRecord::Base.find_in_batches for batch processing [DHH/Jamis Buck] * Added that ActiveRecord::Base.exists? can be called with no arguments #1817 [Scott Taylor] -*2.3.0 [RC1] (February 1st, 2009)* - * Add Support for updating deeply nested models from a single form. #1202 [Eloy Duran] class Book < ActiveRecord::Base diff --git a/activerecord/Rakefile b/activerecord/Rakefile index aec9b3825b..b50008c971 100644 --- a/activerecord/Rakefile +++ b/activerecord/Rakefile @@ -177,7 +177,7 @@ spec = Gem::Specification.new do |s| s.files = s.files + Dir.glob( "#{dir}/**/*" ).delete_if { |item| item.include?( "\.svn" ) } end - s.add_dependency('activesupport', '= 2.3.1' + PKG_BUILD) + s.add_dependency('activesupport', '= 2.3.2' + PKG_BUILD) s.files.delete FIXTURES_ROOT + "/fixture_database.sqlite" s.files.delete FIXTURES_ROOT + "/fixture_database_2.sqlite" diff --git a/activerecord/lib/active_record/version.rb b/activerecord/lib/active_record/version.rb index 809e91897e..852807b4c5 100644 --- a/activerecord/lib/active_record/version.rb +++ b/activerecord/lib/active_record/version.rb @@ -2,7 +2,7 @@ module ActiveRecord module VERSION #:nodoc: MAJOR = 2 MINOR = 3 - TINY = 1 + TINY = 2 STRING = [MAJOR, MINOR, TINY].join('.') end -- cgit v1.2.3 From 0e6b9695aff6500ad48c4dd9ab61343d7090b030 Mon Sep 17 00:00:00 2001 From: "Hongli Lai (Phusion)" Date: Tue, 24 Mar 2009 17:05:21 +0100 Subject: TRUNCATE is also a MySQL DDL statement, so document this is a possible caveat when using transactions and savepoints. --- activerecord/lib/active_record/transactions.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 0b6e52c79b..b059eb7f6f 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -175,6 +175,8 @@ module ActiveRecord # end # RELEASE savepoint active_record_1 # # ^^^^ BOOM! database error! # end + # + # Note that "TRUNCATE" is also a MySQL DDL statement! module ClassMethods # See ActiveRecord::Transactions::ClassMethods for detailed documentation. def transaction(options = {}, &block) -- cgit v1.2.3