diff options
4 files changed, 75 insertions, 40 deletions
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 376fbca74f..d4229dfdba 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb @@ -4,6 +4,7 @@ module ActiveRecord class << self def included(base) #:nodoc: dirties_query_cache base, :insert, :update, :delete, :rollback_to_savepoint, :rollback_db_transaction + base.set_callback :checkin, :after, :disable_query_cache! end def dirties_query_cache(base, *method_names) @@ -41,6 +42,7 @@ module ActiveRecord def disable_query_cache! @query_cache_enabled = false + clear_query_cache end # Disable the query cache within the block. diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 135421819c..137396a677 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -62,17 +62,17 @@ module ActiveRecord # notably, the instance methods provided by SchemaStatements are very useful. class AbstractAdapter ADAPTER_NAME = "Abstract".freeze + include ActiveSupport::Callbacks + define_callbacks :checkout, :checkin + include Quoting, DatabaseStatements, SchemaStatements include DatabaseLimits include QueryCache - include ActiveSupport::Callbacks include ColumnDumper include Savepoints SIMPLE_INT = /\A\d+\z/ - define_callbacks :checkout, :checkin - attr_accessor :visitor, :pool attr_reader :schema_cache, :owner, :logger alias :in_use? :owner diff --git a/activerecord/lib/active_record/query_cache.rb b/activerecord/lib/active_record/query_cache.rb index c42c22ab09..23dae825c2 100644 --- a/activerecord/lib/active_record/query_cache.rb +++ b/activerecord/lib/active_record/query_cache.rb @@ -24,17 +24,10 @@ module ActiveRecord end def self.run - connection = ActiveRecord::Base.connection - enabled = connection.query_cache_enabled - connection.enable_query_cache! - - [connection, enabled] + ActiveRecord::Base.connection.enable_query_cache! end - def self.complete((connection, enabled)) - connection.clear_query_cache - connection.disable_query_cache! unless enabled - + def self.complete(_) unless ActiveRecord::Base.connected? && ActiveRecord::Base.connection.transaction_open? ActiveRecord::Base.clear_active_connections! end diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb index 29b2deea26..5b0477aabe 100644 --- a/activerecord/test/cases/query_cache_test.rb +++ b/activerecord/test/cases/query_cache_test.rb @@ -37,7 +37,7 @@ class QueryCacheTest < ActiveRecord::TestCase end def test_exceptional_middleware_clears_and_disables_cache_on_error - assert !ActiveRecord::Base.connection.query_cache_enabled, "cache off" + assert_cache :off mw = middleware { |env| Task.find 1 @@ -47,42 +47,66 @@ class QueryCacheTest < ActiveRecord::TestCase } assert_raises(RuntimeError) { mw.call({}) } - assert_equal 0, ActiveRecord::Base.connection.query_cache.length - assert !ActiveRecord::Base.connection.query_cache_enabled, "cache off" + assert_cache :off end - def test_exceptional_middleware_cleans_up_correct_cache - connection = ActiveRecord::Base.connection - called = false + def test_query_cache_across_threads + ActiveRecord::Base.connection_pool.connections.each do |conn| + assert_cache :off, conn + end + + assert !ActiveRecord::Base.connection.nil? + assert_cache :off + + middleware { + assert_cache :clean - mw = middleware { |env| - Task.find 1 Task.find 1 - assert_equal 1, connection.query_cache.length + assert_cache :dirty - # Checkin connection early + thread_1_connection = ActiveRecord::Base.connection ActiveRecord::Base.clear_active_connections! - # Make sure ActiveRecord::Base.connection doesn't checkout the same connection - ActiveRecord::Base.connection_pool.remove(connection) + assert_cache :off, thread_1_connection - called = true - } - mw.call({}) + started = Concurrent::Event.new + checked = Concurrent::Event.new - assert called - assert_equal 0, connection.query_cache.length - assert !connection.query_cache_enabled, "cache off" - end + thread_2_connection = nil + thread = Thread.new { + thread_2_connection = ActiveRecord::Base.connection - def test_exceptional_middleware_leaves_enabled_cache_alone - ActiveRecord::Base.connection.enable_query_cache! + assert_equal thread_2_connection, thread_1_connection + assert_cache :off - mw = middleware { |env| - raise "lol borked" - } - assert_raises(RuntimeError) { mw.call({}) } + middleware { + assert_cache :clean + + Task.find 1 + assert_cache :dirty + + started.set + checked.wait + + ActiveRecord::Base.clear_active_connections! + }.call({}) + } + + started.wait + + thread_1_connection = ActiveRecord::Base.connection + assert_not_equal thread_1_connection, thread_2_connection + assert_cache :dirty, thread_2_connection + checked.set + thread.join + + assert_cache :off, thread_2_connection + }.call({}) - assert ActiveRecord::Base.connection.query_cache_enabled, "cache on" + ActiveRecord::Base.connection_pool.connections.each do |conn| + assert_cache :off, conn + end + ensure + ActiveRecord::Base.clear_all_connections! end def test_middleware_delegates @@ -106,10 +130,10 @@ class QueryCacheTest < ActiveRecord::TestCase end def test_cache_enabled_during_call - assert !ActiveRecord::Base.connection.query_cache_enabled, "cache off" + assert_cache :off mw = middleware { |env| - assert ActiveRecord::Base.connection.query_cache_enabled, "cache on" + assert_cache :clean [200, {}, nil] } mw.call({}) @@ -295,6 +319,22 @@ class QueryCacheTest < ActiveRecord::TestCase ActiveRecord::QueryCache.install_executor_hooks executor lambda { |env| executor.wrap { app.call(env) } } end + + def assert_cache(state, connection = ActiveRecord::Base.connection) + case state + when :off + assert !connection.query_cache_enabled, "cache should be off" + assert connection.query_cache.empty?, "cache should be empty" + when :clean + assert connection.query_cache_enabled, "cache should be on" + assert connection.query_cache.empty?, "cache should be empty" + when :dirty + assert connection.query_cache_enabled, "cache should be on" + assert !connection.query_cache.empty?, "cache should be dirty" + else + raise "unknown state" + end + end end class QueryCacheExpiryTest < ActiveRecord::TestCase |