From 3b5d940fe5868af0f31989054e8fb7d2d9d11ec9 Mon Sep 17 00:00:00 2001
From: fatkodima <fatkodima123@gmail.com>
Date: Mon, 5 Feb 2018 18:46:55 +0200
Subject: Add missing instrumentation to RedisCacheStore#read_multi
---
.../lib/active_support/cache/redis_cache_store.rb | 6 ++-
activesupport/test/cache/behaviors.rb | 1 +
.../behaviors/cache_instrumentation_behavior.rb | 60 +++++++++++++++++++++
.../test/cache/cache_store_write_multi_test.rb | 62 ----------------------
activesupport/test/cache/stores/file_store_test.rb | 1 +
.../test/cache/stores/mem_cache_store_test.rb | 1 +
.../test/cache/stores/memory_store_test.rb | 1 +
.../test/cache/stores/redis_cache_store_test.rb | 1 +
8 files changed, 70 insertions(+), 63 deletions(-)
create mode 100644 activesupport/test/cache/behaviors/cache_instrumentation_behavior.rb
delete mode 100644 activesupport/test/cache/cache_store_write_multi_test.rb
diff --git a/activesupport/lib/active_support/cache/redis_cache_store.rb b/activesupport/lib/active_support/cache/redis_cache_store.rb
index 596541087f..c21ade8670 100644
--- a/activesupport/lib/active_support/cache/redis_cache_store.rb
+++ b/activesupport/lib/active_support/cache/redis_cache_store.rb
@@ -187,7 +187,11 @@ module ActiveSupport
# fetched values.
def read_multi(*names)
if mget_capable?
- read_multi_mget(*names)
+ instrument(:read_multi, names, options) do |payload|
+ read_multi_mget(*names).tap do |results|
+ payload[:hits] = results.keys
+ end
+ end
else
super
end
diff --git a/activesupport/test/cache/behaviors.rb b/activesupport/test/cache/behaviors.rb
index 745dc09e2c..2d39976be3 100644
--- a/activesupport/test/cache/behaviors.rb
+++ b/activesupport/test/cache/behaviors.rb
@@ -3,6 +3,7 @@
require_relative "behaviors/autoloading_cache_behavior"
require_relative "behaviors/cache_delete_matched_behavior"
require_relative "behaviors/cache_increment_decrement_behavior"
+require_relative "behaviors/cache_instrumentation_behavior"
require_relative "behaviors/cache_store_behavior"
require_relative "behaviors/cache_store_version_behavior"
require_relative "behaviors/connection_pool_behavior"
diff --git a/activesupport/test/cache/behaviors/cache_instrumentation_behavior.rb b/activesupport/test/cache/behaviors/cache_instrumentation_behavior.rb
new file mode 100644
index 0000000000..4e8ff60eb3
--- /dev/null
+++ b/activesupport/test/cache/behaviors/cache_instrumentation_behavior.rb
@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+
+module CacheInstrumentationBehavior
+ def test_fetch_multi_uses_write_multi_entries_store_provider_interface
+ assert_called_with(@cache, :write_multi_entries) do
+ @cache.fetch_multi "a", "b", "c" do |key|
+ key * 2
+ end
+ end
+ end
+
+ def test_write_multi_instrumentation
+ writes = { "a" => "aa", "b" => "bb" }
+
+ events = with_instrumentation "write_multi" do
+ @cache.write_multi(writes)
+ end
+
+ assert_equal %w[ cache_write_multi.active_support ], events.map(&:name)
+ assert_nil events[0].payload[:super_operation]
+ assert_equal({ "a" => "aa", "b" => "bb" }, events[0].payload[:key])
+ end
+
+ def test_instrumentation_with_fetch_multi_as_super_operation
+ @cache.write("b", "bb")
+
+ events = with_instrumentation "read_multi" do
+ @cache.fetch_multi("a", "b") { |key| key * 2 }
+ end
+
+ assert_equal %w[ cache_read_multi.active_support ], events.map(&:name)
+ assert_equal :fetch_multi, events[0].payload[:super_operation]
+ assert_equal ["b"], events[0].payload[:hits]
+ end
+
+ def test_read_multi_instrumentation
+ @cache.write("b", "bb")
+
+ events = with_instrumentation "read_multi" do
+ @cache.read_multi("a", "b") { |key| key * 2 }
+ end
+
+ assert_equal %w[ cache_read_multi.active_support ], events.map(&:name)
+ assert_equal ["b"], events[0].payload[:hits]
+ end
+
+ private
+ def with_instrumentation(method)
+ event_name = "cache_#{method}.active_support"
+
+ [].tap do |events|
+ ActiveSupport::Notifications.subscribe event_name do |*args|
+ events << ActiveSupport::Notifications::Event.new(*args)
+ end
+ yield
+ end
+ ensure
+ ActiveSupport::Notifications.unsubscribe event_name
+ end
+end
diff --git a/activesupport/test/cache/cache_store_write_multi_test.rb b/activesupport/test/cache/cache_store_write_multi_test.rb
deleted file mode 100644
index 7d606e3f7b..0000000000
--- a/activesupport/test/cache/cache_store_write_multi_test.rb
+++ /dev/null
@@ -1,62 +0,0 @@
-# frozen_string_literal: true
-
-require "abstract_unit"
-require "active_support/cache"
-
-class CacheStoreWriteMultiEntriesStoreProviderInterfaceTest < ActiveSupport::TestCase
- setup do
- @cache = ActiveSupport::Cache.lookup_store(:null_store)
- end
-
- test "fetch_multi uses write_multi_entries store provider interface" do
- assert_called_with(@cache, :write_multi_entries) do
- @cache.fetch_multi "a", "b", "c" do |key|
- key * 2
- end
- end
- end
-end
-
-class CacheStoreWriteMultiInstrumentationTest < ActiveSupport::TestCase
- setup do
- @cache = ActiveSupport::Cache.lookup_store(:memory_store)
- end
-
- test "instrumentation" do
- writes = { "a" => "aa", "b" => "bb" }
-
- events = with_instrumentation "write_multi" do
- @cache.write_multi(writes)
- end
-
- assert_equal %w[ cache_write_multi.active_support ], events.map(&:name)
- assert_nil events[0].payload[:super_operation]
- assert_equal({ "a" => "aa", "b" => "bb" }, events[0].payload[:key])
- end
-
- test "instrumentation with fetch_multi as super operation" do
- @cache.write("b", "bb")
-
- events = with_instrumentation "read_multi" do
- @cache.fetch_multi("a", "b") { |key| key * 2 }
- end
-
- assert_equal %w[ cache_read_multi.active_support ], events.map(&:name)
- assert_equal :fetch_multi, events[0].payload[:super_operation]
- assert_equal ["b"], events[0].payload[:hits]
- end
-
- private
- def with_instrumentation(method)
- event_name = "cache_#{method}.active_support"
-
- [].tap do |events|
- ActiveSupport::Notifications.subscribe event_name do |*args|
- events << ActiveSupport::Notifications::Event.new(*args)
- end
- yield
- end
- ensure
- ActiveSupport::Notifications.unsubscribe event_name
- end
-end
diff --git a/activesupport/test/cache/stores/file_store_test.rb b/activesupport/test/cache/stores/file_store_test.rb
index 67eff9b94f..c3c35a7bcc 100644
--- a/activesupport/test/cache/stores/file_store_test.rb
+++ b/activesupport/test/cache/stores/file_store_test.rb
@@ -30,6 +30,7 @@ class FileStoreTest < ActiveSupport::TestCase
include LocalCacheBehavior
include CacheDeleteMatchedBehavior
include CacheIncrementDecrementBehavior
+ include CacheInstrumentationBehavior
include AutoloadingCacheBehavior
def test_clear
diff --git a/activesupport/test/cache/stores/mem_cache_store_test.rb b/activesupport/test/cache/stores/mem_cache_store_test.rb
index 3e2316f217..f426a37c66 100644
--- a/activesupport/test/cache/stores/mem_cache_store_test.rb
+++ b/activesupport/test/cache/stores/mem_cache_store_test.rb
@@ -49,6 +49,7 @@ class MemCacheStoreTest < ActiveSupport::TestCase
include CacheStoreVersionBehavior
include LocalCacheBehavior
include CacheIncrementDecrementBehavior
+ include CacheInstrumentationBehavior
include EncodedKeyCacheBehavior
include AutoloadingCacheBehavior
include ConnectionPoolBehavior
diff --git a/activesupport/test/cache/stores/memory_store_test.rb b/activesupport/test/cache/stores/memory_store_test.rb
index 3981f05331..72fafc187b 100644
--- a/activesupport/test/cache/stores/memory_store_test.rb
+++ b/activesupport/test/cache/stores/memory_store_test.rb
@@ -14,6 +14,7 @@ class MemoryStoreTest < ActiveSupport::TestCase
include CacheStoreVersionBehavior
include CacheDeleteMatchedBehavior
include CacheIncrementDecrementBehavior
+ include CacheInstrumentationBehavior
def test_prune_size
@cache.write(1, "aaaaaaaaaa") && sleep(0.001)
diff --git a/activesupport/test/cache/stores/redis_cache_store_test.rb b/activesupport/test/cache/stores/redis_cache_store_test.rb
index 62752d2c65..22b519daf1 100644
--- a/activesupport/test/cache/stores/redis_cache_store_test.rb
+++ b/activesupport/test/cache/stores/redis_cache_store_test.rb
@@ -107,6 +107,7 @@ module ActiveSupport::Cache::RedisCacheStoreTests
include CacheStoreVersionBehavior
include LocalCacheBehavior
include CacheIncrementDecrementBehavior
+ include CacheInstrumentationBehavior
include AutoloadingCacheBehavior
end
--
cgit v1.2.3