From 09b6cc28bf2bd7c37289d5e9a3e04a04a1ec0db3 Mon Sep 17 00:00:00 2001
From: Matthew Draper <matthew@trebex.net>
Date: Mon, 24 Oct 2016 13:53:00 -0500
Subject: Clear query cache during checkin, instead of an execution callback

It doesn't make sense for the query cache to persist while a connection
moves through the pool and is assigned to a new thread.

[Samuel Cochran & Matthew Draper]
---
 activerecord/test/cases/query_cache_test.rb | 96 ++++++++++++++++++++---------
 1 file changed, 68 insertions(+), 28 deletions(-)

(limited to 'activerecord/test/cases')

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
-- 
cgit v1.2.3