diff options
Diffstat (limited to 'activerecord')
23 files changed, 145 insertions, 28 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 597b876f22..9b936863ba 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,4 +1,8 @@ -*SVN* +*2.1.0 RC1 (May 11th, 2008)* + +* Ensure hm:t preloading honours reflection options. Resolves #137. [Frederick Cheung] + +* Added protection against duplicate migration names (Aslak Hellesøy) [#112] * Base#instantiate_time_object: eliminate check for Time.zone, since we can assume this is set if time_zone_aware_attributes is set to true [Geoff Buesing] diff --git a/activerecord/MIT-LICENSE b/activerecord/MIT-LICENSE index 5fee6e106d..93be57f683 100644 --- a/activerecord/MIT-LICENSE +++ b/activerecord/MIT-LICENSE @@ -1,4 +1,4 @@ -Copyright (c) 2004-2007 David Heinemeier Hansson +Copyright (c) 2004-2008 David Heinemeier Hansson Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the diff --git a/activerecord/Rakefile b/activerecord/Rakefile index d6033a9b85..043ab6d551 100755 --- a/activerecord/Rakefile +++ b/activerecord/Rakefile @@ -171,7 +171,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.0.2' + PKG_BUILD) + s.add_dependency('activesupport', '= 2.0.991' + 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.rb b/activerecord/lib/active_record.rb index 8b274120df..d4f7170305 100755 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -1,5 +1,5 @@ #-- -# Copyright (c) 2004-2007 David Heinemeier Hansson +# Copyright (c) 2004-2008 David Heinemeier Hansson # # Permission is hereby granted, free of charge, to any person obtaining # a copy of this software and associated documentation files (the diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index da4ebdef51..a3d1f12b03 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -31,12 +31,12 @@ module ActiveRecord private def preload_one_association(records, association, preload_options={}) - reflection = reflections[association] - raise ConfigurationError, "Association named '#{ association }' was not found; perhaps you misspelled it?" unless reflection - - # Not all records have the same class, so group then preload. - records.group_by(&:class).each do |klass, records| - reflection = klass.reflections[association] + class_to_reflection = {} + # Not all records have the same class, so group then preload + # group on the reflection itself so that if various subclass share the same association then we do not split them + # unncessarily + records.group_by {|record| class_to_reflection[record.class] ||= record.class.reflections[association]}.each do |reflection, records| + raise ConfigurationError, "Association named '#{ association }' was not found; perhaps you misspelled it?" unless reflection send("preload_#{reflection.macro}_association", records, reflection, preload_options) end end @@ -143,7 +143,8 @@ module ActiveRecord through_primary_key = through_reflection.primary_key_name unless through_records.empty? source = reflection.source_reflection.name - through_records.first.class.preload_associations(through_records, source) + #add conditions from reflection! + through_records.first.class.preload_associations(through_records, source, reflection.options) through_records.each do |through_record| add_preloaded_records_to_collection(id_to_record_map[through_record[through_primary_key].to_s], reflection.name, through_record.send(source)) @@ -251,12 +252,12 @@ module ActiveRecord conditions << append_conditions(options, preload_options) reflection.klass.find(:all, - :select => (options[:select] || "#{table_name}.*"), - :include => options[:include], + :select => (preload_options[:select] || options[:select] || "#{table_name}.*"), + :include => preload_options[:include] || options[:include], :conditions => [conditions, ids], :joins => options[:joins], - :group => options[:group], - :order => options[:order]) + :group => preload_options[:group] || options[:group], + :order => preload_options[:order] || options[:order]) end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 74299bd572..392d187092 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1910,6 +1910,8 @@ module ActiveRecord #:nodoc: # { :name => "foo'bar", :group_id => 4 } returns "name='foo''bar' and group_id='4'" # "name='foo''bar' and group_id='4'" returns "name='foo''bar' and group_id='4'" def sanitize_sql_for_conditions(condition) + return nil if condition.blank? + case condition when Array; sanitize_sql_array(condition) when Hash; sanitize_sql_hash_for_conditions(condition) @@ -2340,7 +2342,7 @@ module ActiveRecord #:nodoc: # Returns a hash of all the attributes with their names as keys and the values of the attributes as values. - def attributes(options = nil) + def attributes self.attribute_names.inject({}) do |attrs, name| attrs[name] = read_attribute(name) attrs diff --git a/activerecord/lib/active_record/dirty.rb b/activerecord/lib/active_record/dirty.rb index c6d89e3a05..6034963811 100644 --- a/activerecord/lib/active_record/dirty.rb +++ b/activerecord/lib/active_record/dirty.rb @@ -69,19 +69,19 @@ module ActiveRecord changed.inject({}) { |h, attr| h[attr] = attribute_change(attr); h } end - - # Clear changed attributes after they are saved. + # Attempts to +save+ the record and clears changed attributes if successful. def save_with_dirty(*args) #:nodoc: - save_without_dirty(*args) - ensure - changed_attributes.clear + if status = save_without_dirty(*args) + changed_attributes.clear + end + status end - # Clear changed attributes after they are saved. + # Attempts to <tt>save!</tt> the record and clears changed attributes if successful. def save_with_dirty!(*args) #:nodoc: - save_without_dirty!(*args) - ensure + status = save_without_dirty!(*args) changed_attributes.clear + status end private diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index af4fb6e83c..5cc9f4e197 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -8,6 +8,12 @@ module ActiveRecord end end + class DuplicateMigrationNameError < ActiveRecordError#:nodoc: + def initialize(name) + super("Multiple migrations have the name #{name}") + end + end + class UnknownMigrationVersionError < ActiveRecordError #:nodoc: def initialize(version) super("No migration with version number #{version}") @@ -440,6 +446,10 @@ module ActiveRecord if klasses.detect { |m| m.version == version } raise DuplicateMigrationVersionError.new(version) end + + if klasses.detect { |m| m.name == name.camelize } + raise DuplicateMigrationNameError.new(name.camelize) + end load(file) diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 81b99f8e96..d43ebefc3b 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -71,6 +71,18 @@ module ActiveRecord # end # end # + # + # For testing complex named scopes, you can examine the scoping options using the + # <tt>proxy_options</tt> method on the proxy itself. + # + # class Shirt < ActiveRecord::Base + # named_scope :colored, lambda { |color| + # { :conditions => { :color => color } } + # } + # end + # + # expected_options = { :conditions => { :colored => 'red' } } + # assert_equal expected_options, Shirt.colored('red').proxy_options def named_scope(name, options = {}, &block) scopes[name] = lambda do |parent_scope, *args| Scope.new(parent_scope, case options diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index d25e8cd0da..b3a75121ed 100755 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -640,7 +640,7 @@ module ActiveRecord results = finder_class.with_exclusive_scope do connection.select_all( construct_finder_sql( - :select => "#{attr_name}", + :select => "#{connection.quote_column_name(attr_name)}", :from => "#{finder_class.quoted_table_name}", :conditions => [condition_sql, *condition_params] ) diff --git a/activerecord/lib/active_record/version.rb b/activerecord/lib/active_record/version.rb index a8ee7dbeb9..1463e84764 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 = 0 - TINY = 2 + TINY = 991 STRING = [MAJOR, MINOR, TINY].join('.') end diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 67b57ceb42..3a3358e39b 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -275,6 +275,17 @@ class EagerAssociationTest < ActiveRecord::TestCase Author.find(:first, :order => 'authors.id').hello_post_comments.sort_by(&:id) end + def test_eager_with_has_many_through_join_model_with_conditions_on_top_level + assert_equal comments(:more_greetings), Author.find(authors(:david).id, :include => :comments_with_order_and_conditions).comments_with_order_and_conditions.first + end + + def test_eager_with_has_many_through_join_model_with_include + author_comments = Author.find(authors(:david).id, :include => :comments_with_include).comments_with_include.to_a + assert_no_queries do + author_comments.first.post.title + end + end + def test_eager_with_has_many_and_limit posts = Post.find(:all, :order => 'posts.id asc', :include => [ :author, :comments ], :limit => 2) assert_equal 2, posts.size @@ -592,4 +603,10 @@ class EagerAssociationTest < ActiveRecord::TestCase assert_equal 3, authors(:david).posts_with_comments.count(:conditions => "length(comments.body) > 15") end end + + def test_load_with_sti_sharing_association + assert_queries(2) do #should not do 1 query per subclass + Comment.find :all, :include => :post + end + 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 7b97afe42c..9e26e2ad58 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -48,6 +48,12 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 2, Firm.find(:first).clients.length end + def test_find_with_blank_conditions + [[], {}, nil, ""].each do |blank| + assert_equal 2, Firm.find(:first).clients.find(:all, :conditions => blank).size + end + end + def test_find_many_with_merged_options assert_equal 1, companies(:first_firm).limited_clients.size assert_equal 1, companies(:first_firm).limited_clients.find(:all).size @@ -851,4 +857,4 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert ! firm.clients.include?(client) end -end
\ No newline at end of file +end diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index 7412e63872..1266eb5036 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -78,7 +78,7 @@ class DirtyTest < ActiveRecord::TestCase end def test_association_assignment_changes_foreign_key - pirate = Pirate.create! + pirate = Pirate.create!(:catchphrase => 'jarl') pirate.parrot = Parrot.create! assert pirate.changed? assert_equal %w(parrot_id), pirate.changed @@ -115,6 +115,18 @@ class DirtyTest < ActiveRecord::TestCase end end + def test_changed_attributes_should_be_preserved_if_save_failure + pirate = Pirate.new + pirate.parrot_id = 1 + assert !pirate.save + check_pirate_after_save_failure(pirate) + + pirate = Pirate.new + pirate.parrot_id = 1 + assert_raises(ActiveRecord::RecordInvalid) { pirate.save! } + check_pirate_after_save_failure(pirate) + end + private def with_partial_updates(klass, on = true) old = klass.partial_updates? @@ -123,4 +135,11 @@ class DirtyTest < ActiveRecord::TestCase ensure klass.partial_updates = old end + + def check_pirate_after_save_failure(pirate) + assert pirate.changed? + assert pirate.parrot_id_changed? + assert_equal %w(parrot_id), pirate.changed + assert_nil pirate.parrot_id_was + end end diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 6be31b5f86..527856b4c0 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -984,6 +984,12 @@ if ActiveRecord::Base.connection.supports_migrations? end end + def test_migrator_with_duplicate_names + assert_raises(ActiveRecord::DuplicateMigrationNameError, "Multiple migrations have the name Chunky") do + ActiveRecord::Migrator.migrate(MIGRATIONS_ROOT + "/duplicate_names", nil) + end + end + def test_migrator_with_missing_version_numbers assert_raise(ActiveRecord::UnknownMigrationVersionError) do ActiveRecord::Migrator.migrate(MIGRATIONS_ROOT + "/missing", 500) diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index e99448c23e..30c074c9d8 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -112,4 +112,10 @@ class NamedScopeTest < ActiveRecord::TestCase assert_equal Topic.find(:all, scope), Topic.scoped(scope) end + + def test_proxy_options + expected_proxy_options = { :conditions => { :approved => true } } + assert_equal expected_proxy_options, Topic.approved.proxy_options + end + end diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb index e3ca8660ac..a4d9da4806 100755 --- a/activerecord/test/cases/validations_test.rb +++ b/activerecord/test/cases/validations_test.rb @@ -5,6 +5,7 @@ require 'models/reply' require 'models/person' require 'models/developer' require 'models/warehouse_thing' +require 'models/guid' # The following methods in Topic are used in test_conditional_validation_* class Topic @@ -493,6 +494,13 @@ class ValidationsTest < ActiveRecord::TestCase end end + def test_validate_uniqueness_with_columns_which_are_sql_keywords + Guid.validates_uniqueness_of :key + g = Guid.new + g.key = "foo" + assert_nothing_raised { !g.valid? } + 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/migrations/duplicate_names/20080507052938_chunky.rb b/activerecord/test/migrations/duplicate_names/20080507052938_chunky.rb new file mode 100644 index 0000000000..5fe5089e18 --- /dev/null +++ b/activerecord/test/migrations/duplicate_names/20080507052938_chunky.rb @@ -0,0 +1,7 @@ +class Chunky < ActiveRecord::Migration + def self.up + end + + def self.down + end +end diff --git a/activerecord/test/migrations/duplicate_names/20080507053028_chunky.rb b/activerecord/test/migrations/duplicate_names/20080507053028_chunky.rb new file mode 100644 index 0000000000..5fe5089e18 --- /dev/null +++ b/activerecord/test/migrations/duplicate_names/20080507053028_chunky.rb @@ -0,0 +1,7 @@ +class Chunky < ActiveRecord::Migration + def self.up + end + + def self.down + end +end diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index 2918139f7f..f63af27403 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -17,6 +17,10 @@ class Author < ActiveRecord::Base end has_many :comments, :through => :posts has_many :comments_containing_the_letter_e, :through => :posts, :source => :comments + has_many :comments_with_order_and_conditions, :through => :posts, :source => :comments, :order => 'comments.body', :conditions => "comments.body like 'Thank%'" + has_many :comments_with_include, :through => :posts, :source => :comments, :include => :post + + has_many :comments_desc, :through => :posts, :source => :comments, :order => 'comments.id DESC' has_many :limited_comments, :through => :posts, :source => :comments, :limit => 1 has_many :funky_comments, :through => :posts, :source => :comments diff --git a/activerecord/test/models/guid.rb b/activerecord/test/models/guid.rb new file mode 100644 index 0000000000..9208dc28fa --- /dev/null +++ b/activerecord/test/models/guid.rb @@ -0,0 +1,2 @@ +class Guid < ActiveRecord::Base +end
\ No newline at end of file diff --git a/activerecord/test/models/pirate.rb b/activerecord/test/models/pirate.rb index bb4d02c10f..51c8183dee 100644 --- a/activerecord/test/models/pirate.rb +++ b/activerecord/test/models/pirate.rb @@ -4,4 +4,6 @@ class Pirate < ActiveRecord::Base has_many :treasures, :as => :looter has_many :treasure_estimates, :through => :treasures, :source => :price_estimates + + validates_presence_of :catchphrase end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 818237f076..423929fd55 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -403,6 +403,10 @@ ActiveRecord::Schema.define do create_table(t, :force => true) { } end + create_table :guids, :force => true do |t| + t.column :key, :string + end + except 'SQLite' do # fk_test_has_fk should be before fk_test_has_pk create_table :fk_test_has_fk, :force => true do |t| |