From 47018a829ceed958c9765ec7678eae476a51c18e Mon Sep 17 00:00:00 2001
From: Martin <hongzeqin@gmail.com>
Date: Sun, 5 Aug 2018 22:42:16 +0800
Subject: Support skip nil for cache fetch (#25437)

* test case for fetch cache miss with skip_nil

* abondon nil cache if skip_nil specified

* ensure not cache key for skip nil

* add document with skip_nil for Store#fetch

* add a new change log entry for #25437
---
 activesupport/CHANGELOG.md                                 |  7 +++++++
 activesupport/lib/active_support/cache.rb                  | 10 +++++++++-
 activesupport/test/cache/behaviors/cache_store_behavior.rb |  7 +++++++
 3 files changed, 23 insertions(+), 1 deletion(-)

(limited to 'activesupport')

diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 8808b38aac..c966cf45d5 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -179,5 +179,12 @@
 
     *Eileen M. Uchitelle*, *Aaron Patterson*
 
+*   Support not to cache `nil` for `ActiveSupport::Cache#fetch`
+
+        cache.fetch('bar', skip_nil: true) { nil }
+        cache.exist?('bar') # => false
+
+    *Martin Hong*
+
 
 Please check [5-2-stable](https://github.com/rails/rails/blob/5-2-stable/activesupport/CHANGELOG.md) for previous changes.
diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb
index d769e2c8ea..8e516de4c9 100644
--- a/activesupport/lib/active_support/cache.rb
+++ b/activesupport/lib/active_support/cache.rb
@@ -229,6 +229,14 @@ module ActiveSupport
       # ask whether you should force a cache write. Otherwise, it's clearer to
       # just call <tt>Cache#write</tt>.
       #
+      # Setting <tt>skip_nil: true</tt> will not cache nil result:
+      #
+      #   cache.fetch('foo') { nil }
+      #   cache.fetch('bar', skip_nil: true) { nil }
+      #   cache.exist?('foo') # => true
+      #   cache.exist?('bar') # => false
+      #
+      #
       # Setting <tt>compress: false</tt> disables compression of the cache entry.
       #
       # Setting <tt>:expires_in</tt> will set an expiration time on the cache.
@@ -695,7 +703,7 @@ module ActiveSupport
             yield(name)
           end
 
-          write(name, result, options)
+          write(name, result, options) unless result.nil? && options[:skip_nil]
           result
         end
     end
diff --git a/activesupport/test/cache/behaviors/cache_store_behavior.rb b/activesupport/test/cache/behaviors/cache_store_behavior.rb
index f9153ffe2a..ae272e87d7 100644
--- a/activesupport/test/cache/behaviors/cache_store_behavior.rb
+++ b/activesupport/test/cache/behaviors/cache_store_behavior.rb
@@ -52,6 +52,13 @@ module CacheStoreBehavior
     end
   end
 
+  def test_fetch_cache_miss_with_skip_nil
+    assert_not_called(@cache, :write) do
+      assert_nil @cache.fetch("foo", skip_nil: true) { nil }
+      assert_equal false, @cache.exist?("foo")
+    end
+  end
+
   def test_fetch_with_forced_cache_miss_with_block
     @cache.write("foo", "bar")
     assert_equal "foo_bar", @cache.fetch("foo", force: true) { "foo_bar" }
-- 
cgit v1.2.3