diff options
Diffstat (limited to 'activerecord')
19 files changed, 262 insertions, 31 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 3cf2f6a4e9..16c525d211 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,25 @@ +## Rails 3.2.3 (unreleased) ## + +* Added find_or_create_by_{attribute}! dynamic method. *Andrew White* + +* Whitelist all attribute assignment by default. Change the default for newly generated applications to whitelist all attribute assignment. Also update the generated model classes so users are reminded of the importance of attr_accessible. *NZKoz* + +* Update ActiveRecord::AttributeMethods#attribute_present? to return false for empty strings. *Jacobkg* + +* Fix associations when using per class databases. *larskanis* + +* Revert setting NOT NULL constraints in add_timestamps *fxn* + +* Fix mysql to use proper text types. Fixes #3931. *kennyj* + +* Fix #5069 - Protect foreign key from mass assignment through association builder. *byroot* + + +## Rails 3.2.2 (March 1, 2012) ## + +* No changes. + + ## Rails 3.2.1 (January 26, 2012) ## * The threshold for auto EXPLAIN is ignored if there's no logger. *fxn* @@ -192,7 +214,8 @@ *Brian Durand* -## Rails 3.1.3 (unreleased) ## + +## Rails 3.1.3 (November 20, 2011) ## * Perf fix: If we're deleting all records in an association, don't add a IN(..) clause to the query. *GH 3672* @@ -205,7 +228,8 @@ *Christos Zisopoulos and Kenny J* -## Rails 3.1.2 (unreleased) ## + +## Rails 3.1.2 (November 18, 2011) ## * Fix bug with PostgreSQLAdapter#indexes. When the search path has multiple schemas, spaces were not being stripped from the schema names after the first. @@ -252,6 +276,7 @@ *Kenny J* + ## Rails 3.1.1 (October 7, 2011) ## * Add deprecation for the preload_associations method. Fixes #3022. diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 0efa111d12..8ebb27b682 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1513,8 +1513,8 @@ module ActiveRecord # * <tt>Developer#projects.size</tt> # * <tt>Developer#projects.find(id)</tt> # * <tt>Developer#projects.exists?(...)</tt> - # * <tt>Developer#projects.build</tt> (similar to <tt>Project.new("project_id" => id)</tt>) - # * <tt>Developer#projects.create</tt> (similar to <tt>c = Project.new("project_id" => id); c.save; c</tt>) + # * <tt>Developer#projects.build</tt> (similar to <tt>Project.new("developer_id" => id)</tt>) + # * <tt>Developer#projects.create</tt> (similar to <tt>c = Project.new("developer_id" => id); c.save; c</tt>) # The declaration may include an options hash to specialize the behavior of the association. # # === Options 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 9657cb081d..53d49fef2e 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -73,7 +73,9 @@ module ActiveRecord # association def build_through_record(record) @through_records[record.object_id] ||= begin - through_record = through_association.build(construct_join_attributes(record)) + ensure_mutable + + through_record = through_association.build through_record.send("#{source_reflection.name}=", record) through_record end diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb index f95e5337c2..fd0e90aaf0 100644 --- a/activerecord/lib/active_record/associations/through_association.rb +++ b/activerecord/lib/active_record/associations/through_association.rb @@ -37,9 +37,7 @@ module ActiveRecord # situation it is more natural for the user to just create or modify their join records # directly as required. def construct_join_attributes(*records) - if source_reflection.macro != :belongs_to - raise HasManyThroughCantAssociateThroughHasOneOrManyReflection.new(owner, reflection) - end + ensure_mutable join_attributes = { source_reflection.foreign_key => @@ -73,6 +71,12 @@ module ActiveRecord !owner[through_reflection.foreign_key].nil? end + def ensure_mutable + if source_reflection.macro != :belongs_to + raise HasManyThroughCantAssociateThroughHasOneOrManyReflection.new(owner, reflection) + end + end + def ensure_not_nested if reflection.nested? raise HasManyThroughNestedAssociationsAreReadonly.new(owner, reflection) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 9c4d4cb274..9746efc7d1 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -208,6 +208,9 @@ module ActiveRecord #:nodoc: # # Now 'Bob' exist and is an 'admin' # User.find_or_create_by_name('Bob', :age => 40) { |u| u.admin = true } # + # Adding an exclamation point (!) on to the end of <tt>find_or_create_by_</tt> will + # raise an <tt>ActiveRecord::RecordInvalid</tt> error if the new record is invalid. + # # Use the <tt>find_or_initialize_by_</tt> finder if you want to return a new record without # saving it first. Protected attributes won't be set unless they are given in a block. # @@ -439,7 +442,7 @@ module ActiveRecord #:nodoc: if self == ActiveRecord::Base ActiveRecord::Base else - connection_handler.connection_pools[name] ? self : superclass.arel_engine + connection_handler.retrieve_connection_pool(self) ? self : superclass.arel_engine end end end 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 698da34d26..3af285c7c8 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -95,10 +95,11 @@ module ActiveRecord @reserved_connections[current_connection_id] ||= checkout end - # Check to see if there is an active connection in this connection - # pool. + # Is there an open connection that is being used for the current thread? def active_connection? - active_connections.any? + @reserved_connections.fetch(current_connection_id) { + return false + }.in_use? end # Signal that the thread is finished with the current connection. @@ -225,8 +226,9 @@ connection. For example: ActiveRecord::Base.connection.close # - ConnectionTimeoutError: no connection can be obtained from the pool # within the timeout period. def checkout - # Checkout an available connection synchronize do + waited_time = 0 + loop do conn = @connections.find { |c| c.lease } @@ -242,17 +244,25 @@ connection. For example: ActiveRecord::Base.connection.close return conn end - @queue.wait(@timeout) + if waited_time >= @timeout + raise ConnectionTimeoutError, "could not obtain a database connection#{" within #{@timeout} seconds" if @timeout} (waited #{waited_time} seconds). The max pool size is currently #{@size}; consider increasing it." + end - if(active_connections.size < @connections.size) - next - else + # Sometimes our wait can end because a connection is available, + # but another thread can snatch it up first. If timeout hasn't + # passed but no connection is avail, looks like that happened -- + # loop and wait again, for the time remaining on our timeout. + before_wait = Time.now + @queue.wait( [@timeout - waited_time, 0].max ) + waited_time += (Time.now - before_wait) + + # Will go away in Rails 4, when we don't clean up + # after leaked connections automatically anymore. Right now, clean + # up after we've returned from a 'wait' if it looks like it's + # needed, then loop and try again. + if(active_connections.size >= @connections.size) clear_stale_cached_connections! - if @size == active_connections.size - raise ConnectionTimeoutError, "could not obtain a database connection#{" within #{@timeout} seconds" if @timeout}. The max pool size is currently #{@size}; consider increasing it." - end end - end end end @@ -268,11 +278,27 @@ connection. For example: ActiveRecord::Base.connection.close conn.expire @queue.signal end + + release conn end end private + def release(conn) + thread_id = nil + + if @reserved_connections[current_connection_id] == conn + thread_id = current_connection_id + else + thread_id = @reserved_connections.keys.find { |k| + @reserved_connections[k] == conn + } + end + + @reserved_connections.delete thread_id if thread_id + end + def new_connection ActiveRecord::Base.send(spec.adapter_method, spec.config) end @@ -344,9 +370,7 @@ connection. For example: ActiveRecord::Base.connection.close connection_pools.values.any? { |pool| pool.active_connection? } end - # Returns any connections in use by the current thread back to the pool, - # and also returns connections to the pool cached by threads that are no - # longer alive. + # Returns any connections in use by the current thread back to the pool. def clear_active_connections! @connection_pools.each_value {|pool| pool.release_connection } end diff --git a/activerecord/lib/active_record/dynamic_finder_match.rb b/activerecord/lib/active_record/dynamic_finder_match.rb index b309df9b1b..80b17a27df 100644 --- a/activerecord/lib/active_record/dynamic_finder_match.rb +++ b/activerecord/lib/active_record/dynamic_finder_match.rb @@ -18,6 +18,10 @@ module ActiveRecord when /^find_by_([_a-zA-Z]\w*)\!$/ bang = true names = $1 + when /^find_or_create_by_([_a-zA-Z]\w*)\!$/ + bang = true + instantiator = :create + names = $1 when /^find_or_(initialize|create)_by_([_a-zA-Z]\w*)$/ instantiator = $1 == 'initialize' ? :new : :create names = $2 @@ -52,5 +56,13 @@ module ActiveRecord def bang? @bang end + + def save_record? + @instantiator == :create + end + + def save_method + bang? ? :save! : :save + end end end diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 0305e561c8..9a34927857 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -290,7 +290,7 @@ module ActiveRecord r.assign_attributes(unprotected_attributes_for_create, :without_protection => true) end yield(record) if block_given? - record.save if match.instantiator == :create + record.send(match.save_method) if match.save_record? end record diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index fab2cd77f0..f5bfa3603a 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -270,7 +270,7 @@ module ActiveRecord arel.having(*@having_values.uniq.reject{|h| h.blank?}) unless @having_values.empty? arel.take(connection.sanitize_limit(@limit_value)) if @limit_value - arel.skip(@offset_value) if @offset_value + arel.skip(@offset_value.to_i) if @offset_value arel.group(*@group_values.uniq.reject{|g| g.blank?}) unless @group_values.empty? 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 6836a2b4d0..4489c3e638 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -44,17 +44,33 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end def test_associate_existing - posts(:thinking); people(:david) # Warm cache + post = posts(:thinking) + person = people(:david) assert_queries(1) do - posts(:thinking).people << people(:david) + post.people << person end assert_queries(1) do - assert posts(:thinking).people.include?(people(:david)) + assert post.people.include?(person) end - assert posts(:thinking).reload.people(true).include?(people(:david)) + assert post.reload.people(true).include?(person) + end + + def test_associate_existing_with_strict_mass_assignment_sanitizer + SecureReader.mass_assignment_sanitizer = :strict + + SecureReader.new + + post = posts(:thinking) + person = people(:david) + + assert_queries(1) do + post.secure_people << person + end + ensure + SecureReader.mass_assignment_sanitizer = :logger end def test_associate_existing_record_twice_should_add_to_target_twice diff --git a/activerecord/test/cases/connection_pool_test.rb b/activerecord/test/cases/connection_pool_test.rb index d170a13b23..5cecfa90e7 100644 --- a/activerecord/test/cases/connection_pool_test.rb +++ b/activerecord/test/cases/connection_pool_test.rb @@ -3,7 +3,11 @@ require "cases/helper" module ActiveRecord module ConnectionAdapters class ConnectionPoolTest < ActiveRecord::TestCase + attr_reader :pool + def setup + super + # Keep a duplicate pool so we do not bother others @pool = ConnectionPool.new ActiveRecord::Base.connection_pool.spec @@ -18,6 +22,72 @@ module ActiveRecord end end + def teardown + super + @pool.disconnect! + end + + def active_connections(pool) + pool.connections.find_all(&:in_use?) + end + + def test_checkout_after_close + connection = pool.connection + assert connection.in_use? + + connection.close + assert !connection.in_use? + + assert pool.connection.in_use? + end + + def test_released_connection_moves_between_threads + thread_conn = nil + + Thread.new { + pool.with_connection do |conn| + thread_conn = conn + end + }.join + + assert thread_conn + + Thread.new { + pool.with_connection do |conn| + assert_equal thread_conn, conn + end + }.join + end + + def test_with_connection + assert_equal 0, active_connections(pool).size + + main_thread = pool.connection + assert_equal 1, active_connections(pool).size + + Thread.new { + pool.with_connection do |conn| + assert conn + assert_equal 2, active_connections(pool).size + end + assert_equal 1, active_connections(pool).size + }.join + + main_thread.close + assert_equal 0, active_connections(pool).size + end + + def test_active_connection_in_use + assert !pool.active_connection? + main_thread = pool.connection + + assert pool.active_connection? + + main_thread.close + + assert !pool.active_connection? + end + def test_active_connection? assert !@pool.active_connection? assert @pool.connection diff --git a/activerecord/test/cases/dynamic_finder_match_test.rb b/activerecord/test/cases/dynamic_finder_match_test.rb index e576870317..db619faa83 100644 --- a/activerecord/test/cases/dynamic_finder_match_test.rb +++ b/activerecord/test/cases/dynamic_finder_match_test.rb @@ -83,6 +83,14 @@ module ActiveRecord assert_equal :create, m.instantiator end + def test_find_or_create! + m = DynamicFinderMatch.match(:find_or_create_by_foo!) + assert_equal :first, m.finder + assert m.bang?, 'should be banging' + assert_equal %w{ foo }, m.attribute_names + assert_equal :create, m.instantiator + end + def test_find_or_initialize m = DynamicFinderMatch.match(:find_or_initialize_by_foo) assert_equal :first, m.finder diff --git a/activerecord/test/cases/finder_respond_to_test.rb b/activerecord/test/cases/finder_respond_to_test.rb index 235805a67c..810c1500cc 100644 --- a/activerecord/test/cases/finder_respond_to_test.rb +++ b/activerecord/test/cases/finder_respond_to_test.rb @@ -56,6 +56,16 @@ class FinderRespondToTest < ActiveRecord::TestCase assert_respond_to Topic, :find_or_create_by_title_and_author_name end + def test_should_respond_to_find_or_create_from_one_attribute_bang + ensure_topic_method_is_not_cached(:find_or_create_by_title!) + assert_respond_to Topic, :find_or_create_by_title! + end + + def test_should_respond_to_find_or_create_from_two_attributes_bang + ensure_topic_method_is_not_cached(:find_or_create_by_title_and_author_name!) + assert_respond_to Topic, :find_or_create_by_title_and_author_name! + end + def test_should_not_respond_to_find_by_one_missing_attribute assert !Topic.respond_to?(:find_by_undertitle) end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 5a67fcdc0a..4705252c05 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -862,6 +862,28 @@ class FinderTest < ActiveRecord::TestCase assert another.persisted? end + def test_find_or_create_from_one_attribute_bang + number_of_companies = Company.count + assert_raises(ActiveRecord::RecordInvalid) { Company.find_or_create_by_name!("") } + assert_equal number_of_companies, Company.count + sig38 = Company.find_or_create_by_name!("38signals") + assert_equal number_of_companies + 1, Company.count + assert_equal sig38, Company.find_or_create_by_name!("38signals") + assert sig38.persisted? + end + + def test_find_or_create_from_two_attributes_bang + number_of_companies = Company.count + assert_raises(ActiveRecord::RecordInvalid) { Company.find_or_create_by_name_and_firm_id!("", 17) } + assert_equal number_of_companies, Company.count + sig38 = Company.find_or_create_by_name_and_firm_id!("38signals", 17) + assert_equal number_of_companies + 1, Company.count + assert_equal sig38, Company.find_or_create_by_name_and_firm_id!("38signals", 17) + assert sig38.persisted? + assert_equal "38signals", sig38.name + assert_equal 17, sig38.firm_id + end + def test_find_or_create_from_two_attributes_with_one_being_an_aggregate number_of_customers = Customer.count created_customer = Customer.find_or_create_by_balance_and_name(Money.new(123), "Elizabeth") @@ -1182,6 +1204,10 @@ class FinderTest < ActiveRecord::TestCase end end + def test_finder_with_offset_string + assert_nothing_raised(ActiveRecord::StatementInvalid) { Topic.find(:all, :offset => "3") } + end + protected def bind(statement, *vars) if vars.first.is_a?(Hash) diff --git a/activerecord/test/cases/multiple_db_test.rb b/activerecord/test/cases/multiple_db_test.rb index e704322b5d..b31d971309 100644 --- a/activerecord/test/cases/multiple_db_test.rb +++ b/activerecord/test/cases/multiple_db_test.rb @@ -85,6 +85,12 @@ class MultipleDbTest < ActiveRecord::TestCase end def test_arel_table_engines - assert_equal Entrant.arel_engine, Bird.arel_engine + assert_not_equal Entrant.arel_engine, Bird.arel_engine + assert_not_equal Entrant.arel_engine, Course.arel_engine + end + + def test_connection + assert_equal Entrant.arel_engine.connection, Bird.arel_engine.connection + assert_not_equal Entrant.arel_engine.connection, Course.arel_engine.connection end end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index bf1eb6386a..5e0caf5fce 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -449,6 +449,18 @@ class RelationTest < ActiveRecord::TestCase assert_equal authors(:david), authors.find_or_create_by_name(:name => 'David') end + def test_dynamic_find_or_create_by_attributes_bang + authors = Author.scoped + + assert_raises(ActiveRecord::RecordInvalid) { authors.find_or_create_by_name!('') } + + lifo = authors.find_or_create_by_name!('Lifo') + assert_equal "Lifo", lifo.name + assert lifo.persisted? + + assert_equal authors(:david), authors.find_or_create_by_name!(:name => 'David') + end + def test_find_id authors = Author.scoped diff --git a/activerecord/test/models/person.rb b/activerecord/test/models/person.rb index 36eb08d02f..dcfea3170c 100644 --- a/activerecord/test/models/person.rb +++ b/activerecord/test/models/person.rb @@ -1,8 +1,10 @@ class Person < ActiveRecord::Base has_many :readers + has_many :secure_readers has_one :reader has_many :posts, :through => :readers + has_many :secure_posts, :through => :secure_readers has_many :posts_with_no_comments, :through => :readers, :source => :post, :include => :comments, :conditions => 'comments.id is null' has_many :references diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 1cab78d8c7..0fc22ac6a3 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -115,8 +115,10 @@ class Post < ActiveRecord::Base has_many :named_categories, :through => :standard_categorizations has_many :readers + has_many :secure_readers has_many :readers_with_person, :include => :person, :class_name => "Reader" has_many :people, :through => :readers + has_many :secure_people, :through => :secure_readers has_many :single_people, :through => :readers has_many :people_with_callbacks, :source=>:person, :through => :readers, :before_add => lambda {|owner, reader| log(:added, :before, reader.first_name) }, diff --git a/activerecord/test/models/reader.rb b/activerecord/test/models/reader.rb index 0207a2bd92..59005ac604 100644 --- a/activerecord/test/models/reader.rb +++ b/activerecord/test/models/reader.rb @@ -3,3 +3,12 @@ class Reader < ActiveRecord::Base belongs_to :person, :inverse_of => :readers belongs_to :single_person, :class_name => 'Person', :foreign_key => :person_id, :inverse_of => :reader end + +class SecureReader < ActiveRecord::Base + self.table_name = "readers" + + belongs_to :secure_post, :class_name => "Post", :foreign_key => "post_id" + belongs_to :secure_person, :inverse_of => :secure_readers, :class_name => "Person", :foreign_key => "person_id" + + attr_accessible nil +end |