diff options
Diffstat (limited to 'activerecord')
12 files changed, 41 insertions, 26 deletions
diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb index 9b0b50977d..3d79e540b8 100644 --- a/activerecord/lib/active_record/associations/association_scope.rb +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -68,11 +68,11 @@ module ActiveRecord foreign_key = join_keys.foreign_key value = transform_value(owner[foreign_key]) - scope = scope.where(table.name => { key => value }) + scope = apply_scope(scope, table, key, value) if reflection.type polymorphic_type = transform_value(owner.class.base_class.name) - scope = scope.where(table.name => { reflection.type => polymorphic_type }) + scope = apply_scope(scope, table, reflection.type, polymorphic_type) end scope @@ -91,10 +91,10 @@ module ActiveRecord if reflection.type value = transform_value(next_reflection.klass.base_class.name) - scope = scope.where(table.name => { reflection.type => value }) + scope = apply_scope(scope, table, reflection.type, value) end - scope = scope.joins(join(foreign_table, constraint)) + scope.joins!(join(foreign_table, constraint)) end class ReflectionProxy < SimpleDelegator # :nodoc: @@ -165,6 +165,14 @@ module ActiveRecord scope end + def apply_scope(scope, table, key, value) + if scope.table == table + scope.where!(key => value) + else + scope.where!(table.name => { key => value }) + end + end + def eval_scope(reflection, table, scope, owner) reflection.build_scope(table).instance_exec(owner, &scope) end diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 0001b804a8..1a424896fe 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -59,7 +59,9 @@ module ActiveRecord r.send(reflection.association_primary_key) end.values_at(*ids).compact if records.size != ids.size - klass.all.raise_record_not_found_exception!(ids, records.size, ids.size, reflection.association_primary_key) + found_ids = records.map { |record| record.send(reflection.association_primary_key) } + not_found_ids = ids - found_ids + klass.all.raise_record_not_found_exception!(ids, records.size, ids.size, reflection.association_primary_key, not_found_ids) else replace(records) 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 c11b7b012f..0759f4d2b3 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -270,7 +270,7 @@ module ActiveRecord # Connections must be leased while holding the main pool mutex. This is # an internal subclass that also +.leases+ returned connections while # still in queue's critical section (queue synchronizes with the same - # +@lock+ as the main pool) so that a returned connection is already + # <tt>@lock</tt> as the main pool) so that a returned connection is already # leased and there is no need to re-enter synchronized block. class ConnectionLeasingQueue < Queue # :nodoc: include BiasableQueue @@ -326,8 +326,6 @@ module ActiveRecord @spec = spec @checkout_timeout = (spec.config[:checkout_timeout] && spec.config[:checkout_timeout].to_f) || 5 - @reaper = Reaper.new(self, (spec.config[:reaping_frequency] && spec.config[:reaping_frequency].to_f)) - @reaper.run # default max pool size to 5 @size = (spec.config[:pool] && spec.config[:pool].to_i) || 5 @@ -340,7 +338,7 @@ module ActiveRecord # then that +thread+ does indeed own that +conn+. However, an absence of a such # mapping does not mean that the +thread+ doesn't own the said connection. In # that case +conn.owner+ attr should be consulted. - # Access and modification of +@thread_cached_conns+ does not require + # Access and modification of <tt>@thread_cached_conns</tt> does not require # synchronization. @thread_cached_conns = Concurrent::Map.new(initial_capacity: @size) @@ -357,6 +355,9 @@ module ActiveRecord @available = ConnectionLeasingQueue.new self @lock_thread = false + + @reaper = Reaper.new(self, spec.config[:reaping_frequency] && spec.config[:reaping_frequency].to_f) + @reaper.run end def lock_thread=(lock_thread) @@ -736,10 +737,10 @@ module ActiveRecord # Implementation detail: the connection returned by +acquire_connection+ # will already be "+connection.lease+ -ed" to the current thread. def acquire_connection(checkout_timeout) - # NOTE: we rely on +@available.poll+ and +try_to_checkout_new_connection+ to + # NOTE: we rely on <tt>@available.poll</tt> and +try_to_checkout_new_connection+ to # +conn.lease+ the returned connection (and to do this in a +synchronized+ # section). This is not the cleanest implementation, as ideally we would - # <tt>synchronize { conn.lease }</tt> in this method, but by leaving it to +@available.poll+ + # <tt>synchronize { conn.lease }</tt> in this method, but by leaving it to <tt>@available.poll</tt> # and +try_to_checkout_new_connection+ we can piggyback on +synchronize+ sections # of the said methods and avoid an additional +synchronize+ overhead. if conn = @available.poll || try_to_checkout_new_connection @@ -763,7 +764,7 @@ module ActiveRecord end end - # If the pool is not at a +@size+ limit, establish new connection. Connecting + # If the pool is not at a <tt>@size</tt> limit, establish new connection. Connecting # to the DB is done outside main synchronized section. #-- # Implementation constraint: a newly established connection returned by this diff --git a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb index ecf5201d12..41d93c4322 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "concurrent/map" + module ActiveRecord module ConnectionAdapters # :nodoc: module QueryCache diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 626c50470e..5da9573052 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -330,7 +330,7 @@ module ActiveRecord # of results obtained should be provided in the +result_size+ argument and # the expected number of results should be provided in the +expected_size+ # argument. - def raise_record_not_found_exception!(ids = nil, result_size = nil, expected_size = nil, key = primary_key) # :nodoc: + def raise_record_not_found_exception!(ids = nil, result_size = nil, expected_size = nil, key = primary_key, not_found_ids = nil) # :nodoc: conditions = arel.where_sql(@klass) conditions = " [#{conditions}]" if conditions name = @klass.name @@ -344,8 +344,8 @@ module ActiveRecord raise RecordNotFound.new(error, name, key, ids) else error = "Couldn't find all #{name.pluralize} with '#{key}': ".dup - error << "(#{ids.join(", ")})#{conditions} (found #{result_size} results, but was looking for #{expected_size})" - + error << "(#{ids.join(", ")})#{conditions} (found #{result_size} results, but was looking for #{expected_size})." + error << " Couldn't find #{name.pluralize(not_found_ids.size)} with #{key.to_s.pluralize(not_found_ids.size)} #{not_found_ids.join(', ')}." if not_found_ids raise RecordNotFound.new(error, name, primary_key, ids) end end diff --git a/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb b/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb index a2e74efc2b..5681ccdd23 100644 --- a/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb @@ -22,7 +22,7 @@ module ActiveRecord configuration.merge("encoding" => encoding) establish_connection configuration rescue ActiveRecord::StatementInvalid => error - if /database .* already exists/.match?(error.message) + if error.cause.is_a?(PG::DuplicateDatabase) raise DatabaseAlreadyExists else raise @@ -136,7 +136,7 @@ module ActiveRecord ensure tempfile.close end - FileUtils.mv(tempfile.path, filename) + FileUtils.cp(tempfile.path, filename) end end end diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index f91f0cdf12..b2f5e39e09 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -472,7 +472,7 @@ module ActiveRecord # if it's associated with a transaction, then the state of the Active Record # object will be updated to reflect the current state of the transaction. # - # The +@transaction_state+ variable stores the states of the associated + # The <tt>@transaction_state</tt> variable stores the states of the associated # transaction. This relies on the fact that a transaction can only be in # one rollback or commit (otherwise a list of states would be required). # Each Active Record object inside of a transaction carries that transaction's 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 c5e35a146b..4c2723addc 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -891,7 +891,8 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase company = companies(:rails_core) ids = [Developer.first.id, -9999] e = assert_raises(ActiveRecord::RecordNotFound) { company.developer_ids = ids } - assert_match(/Couldn't find all Developers with 'id'/, e.message) + msg = "Couldn't find all Developers with 'id': (1, -9999) (found 1 results, but was looking for 2). Couldn't find Developer with id -9999." + assert_equal(msg, e.message) end def test_collection_singular_ids_setter_raises_exception_when_invalid_ids_set_with_changed_primary_key @@ -905,7 +906,8 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase author = authors(:david) ids = [categories(:general).name, "Unknown"] e = assert_raises(ActiveRecord::RecordNotFound) { author.essay_category_ids = ids } - assert_equal "Couldn't find all Categories with 'name': (General, Unknown) (found 1 results, but was looking for 2)", e.message + msg = "Couldn't find all Categories with 'name': (General, Unknown) (found 1 results, but was looking for 2). Couldn't find Category with name Unknown." + assert_equal msg, e.message end def test_build_a_model_from_hm_through_association_with_where_clause diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 2f68bc5141..9d3d5353ff 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -404,7 +404,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase end def test_has_many_through_polymorphic_has_one - assert_equal Tagging.find(1, 2).sort_by(&:id), authors(:david).taggings_2 + assert_equal Tagging.find(1, 2).sort_by(&:id), authors(:david).taggings_2.sort_by(&:id) end def test_has_many_through_polymorphic_has_many diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index fb3d691d51..a602f83d8c 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -466,11 +466,10 @@ class DirtyTest < ActiveRecord::TestCase def test_save_should_not_save_serialized_attribute_with_partial_writes_if_not_present with_partial_writes(Topic) do - Topic.create!(author_name: "Bill", content: { a: "a" }) - topic = Topic.select("id, author_name").first + topic = Topic.create!(author_name: "Bill", content: { a: "a" }) + topic = Topic.select("id, author_name").find(topic.id) topic.update_columns author_name: "John" - topic = Topic.first - assert_not_nil topic.content + assert_not_nil topic.reload.content end end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index a0d87dfb90..55496147c1 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -1153,7 +1153,7 @@ class FinderTest < ActiveRecord::TestCase e = assert_raises(ActiveRecord::RecordNotFound) do model.find "Hello", "World!" end - assert_equal "Couldn't find all MercedesCars with 'name': (Hello, World!) (found 0 results, but was looking for 2)", e.message + assert_equal "Couldn't find all MercedesCars with 'name': (Hello, World!) (found 0 results, but was looking for 2).", e.message end end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 05420a4240..47749c07d2 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -192,6 +192,7 @@ ActiveRecord::Schema.define do t.string :resource_type t.integer :developer_id t.datetime :deleted_at + t.integer :comments end create_table :companies, force: true do |t| |