From d7b48d0229dee31f22dedde0fd7550c51de10f78 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Fri, 18 Feb 2022 08:51:14 +0000 Subject: [PATCH] FIX: Handle `nil` values in `DistributedCache#defer_get_set` (stable) (#15980) Themes often cache `nil` values in a DistributedCache. This bug meant that we were re-calculating some values on every request, AND triggering message-bus publishing on every request. This fix should provide a significant performance improvement for busy sites. --- lib/distributed_cache.rb | 2 +- spec/lib/distributed_cache_spec.rb | 31 ++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 spec/lib/distributed_cache_spec.rb diff --git a/lib/distributed_cache.rb b/lib/distributed_cache.rb index 2597c0a3e5..89e6058d11 100644 --- a/lib/distributed_cache.rb +++ b/lib/distributed_cache.rb @@ -21,7 +21,7 @@ class DistributedCache < MessageBus::DistributedCache end def defer_get_set(k, &block) - return self[k] if self[k] + return self[k] if hash.key? k value = block.call self.defer_set(k, value) value diff --git a/spec/lib/distributed_cache_spec.rb b/spec/lib/distributed_cache_spec.rb new file mode 100644 index 0000000000..91f0022a29 --- /dev/null +++ b/spec/lib/distributed_cache_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe "DistributedCache extensions" do + let(:cache) { DistributedCache.new('mytest') } + + it "can defer_get_set" do + messages = MessageBus.track_publish("/distributed_hash") do + cache.defer_get_set("key") { "value" } + end + expect(messages.size).to eq(1) + expect(cache["key"]).to eq("value") + end + + it "works correctly for nil values" do + block_called_counter = 0 + messages = MessageBus.track_publish("/distributed_hash") do + 2.times do + cache.defer_get_set("key") do + block_called_counter += 1 + nil + end + end + end + + expect(block_called_counter).to eq(1) + expect(messages.size).to eq(1) + expect(cache["key"]).to eq(nil) + end +end