Follow feedback on the merge request

5 files changed, 15 insertions, 19 deletions

diff --git a/app/models/commit.rb b/app/models/commit.rb
index 337236b30d5..66c6ae4dacf 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -1,6 +1,6 @@
 class Commit
   extend ActiveModel::Naming
-  extend Gitlab::Cache::RequestStoreWrap
+  extend Gitlab::Cache::RequestCache
 
   include ActiveModel::Conversion
   include Noteable
diff --git a/changelogs/unreleased/request-store-wrap.yml b/changelogs/unreleased/request-store-wrap.yml
index f8672ad0b93..8017054b77b 100644
--- a/changelogs/unreleased/request-store-wrap.yml
+++ b/changelogs/unreleased/request-store-wrap.yml
@@ -1,4 +1,4 @@
 ---
-title: Add RequestStoreWrap which makes caching with RequestStore easier
+title: Add RequestCache which makes caching with RequestStore easier
 merge_request: 12920
 author:
diff --git a/lib/gitlab/cache/request_store_wrap.rb b/lib/gitlab/cache/request_cache.rb
index e10ff235934..b012c876560 100644
--- a/lib/gitlab/cache/request_store_wrap.rb
+++ b/lib/gitlab/cache/request_cache.rb
@@ -8,7 +8,7 @@ module Gitlab
     # A simple example:
     #
     # class UserAccess
-    #   extend Gitlab::Cache::RequestStoreWrap
+    #   extend Gitlab::Cache::RequestCache
     #
     #   request_store_wrap_key do
     #     [user&.id, project&.id]
@@ -26,7 +26,7 @@ module Gitlab
     # Here's another example using customized method level values:
     #
     # class Commit
-    #   extend Gitlab::Cache::RequestStoreWrap
+    #   extend Gitlab::Cache::RequestCache
     #
     #   def author
     #     User.find_by_any_email(author_email.downcase)
@@ -36,12 +36,12 @@ module Gitlab
     #
     # So that we could have different strategies for different methods
     #
-    module RequestStoreWrap
+    module RequestCache
       def self.extended(klass)
         return if klass < self
 
         extension = Module.new
-        klass.const_set(:RequestStoreWrapExtension, extension)
+        klass.const_set(:RequestCacheExtension, extension)
         klass.prepend(extension)
       end
 
@@ -54,30 +54,26 @@ module Gitlab
       end
 
       def request_store_wrap(method_name, &method_key_block)
-        const_get(:RequestStoreWrapExtension).module_eval do
+        const_get(:RequestCacheExtension).module_eval do
+          cache_key_method_name = "#{method_name}_cache_key"
+
           define_method(method_name) do |*args|
             store =
               if RequestStore.active?
                 RequestStore.store
               else
                 ivar_name = # ! and ? cannot be used as ivar name
-                  "@#{method_name.to_s.tr('!', "\u2605").tr('?', "\u2606")}"
+                  "@cache_#{method_name.to_s.tr('!?', "\u2605\u2606")}"
 
                 instance_variable_get(ivar_name) ||
                   instance_variable_set(ivar_name, {})
               end
 
-            key = send("#{method_name}_cache_key", args)
+            key = __send__(cache_key_method_name, args)
 
-            if store.key?(key)
-              store[key]
-            else
-              store[key] = super(*args)
-            end
+            store.fetch(key) { store[key] = super(*args) }
           end
 
-          cache_key_method_name = "#{method_name}_cache_key"
-
           define_method(cache_key_method_name) do |args|
             klass = self.class
 
diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb
index 9c066627011..a30dfe0f6bf 100644
--- a/lib/gitlab/user_access.rb
+++ b/lib/gitlab/user_access.rb
@@ -1,6 +1,6 @@
 module Gitlab
   class UserAccess
-    extend Gitlab::Cache::RequestStoreWrap
+    extend Gitlab::Cache::RequestCache
 
     request_store_wrap_key do
       [user&.id, project&.id]
diff --git a/spec/lib/gitlab/cache/request_store_wrap_spec.rb b/spec/lib/gitlab/cache/request_cache_spec.rb
index d63d958900a..62f914cf121 100644
--- a/spec/lib/gitlab/cache/request_store_wrap_spec.rb
+++ b/spec/lib/gitlab/cache/request_cache_spec.rb
@@ -1,9 +1,9 @@
 require 'spec_helper'
 
-describe Gitlab::Cache::RequestStoreWrap, :request_store do
+describe Gitlab::Cache::RequestCache, :request_store do
   let(:klass) do
     Class.new do
-      extend Gitlab::Cache::RequestStoreWrap
+      extend Gitlab::Cache::RequestCache
 
       attr_accessor :id, :name, :result, :extra