Cache redis loading and streamline method calls

lib/gitlab/diff/highlight_cache.rb

@@ -11,6 +11,7 @@ module Gitlab
       def initialize(diff_collection, backend: Rails.cache)
         @backend = backend
         @diff_collection = diff_collection
+        @redis_key = diffable.cache_key if Feature.enabled?(:redis_diff_caching)
       end
 
       # - Reads from cache
@@ -58,32 +59,30 @@ module Gitlab
       #   ...it will write/update a Redis hash (HSET)
       #
       def write_to_redis_hash(hash)
-        key = diffable.cache_key
-
-        if key
-          Redis::Cache.with do |redis|
-            redis.multi do |multi|
-              hash.each do |diff_file_id, highlighted_diff_lines_hash|
-                multi.hset(key, diff_file_id, highlighted_diff_lines_hash.to_json)
-
-                # HSETs have to have their expiration date manually updated
-                #
-                multi.expire(key, EXPIRATION)
-              end
+        return unless @redis_key
+
+        Redis::Cache.with do |redis|
+          redis.multi do |multi|
+            hash.each do |diff_file_id, highlighted_diff_lines_hash|
+              multi.hset(@redis_key, diff_file_id, highlighted_diff_lines_hash.to_json)
+
+              # HSETs have to have their expiration date manually updated
+              #
+              multi.expire(@redis_key, EXPIRATION)
             end
           end
         end
       end
 
-      def read_entire_redis_hash(key)
+      def read_entire_redis_hash
         Redis::Cache.with do |redis|
-          redis.hgetall(key)
+          redis.hgetall(@redis_key)
         end
       end
 
-      def read_single_entry_from_redis_hash(key, diff_file_id)
+      def read_single_entry_from_redis_hash(diff_file_id)
         Redis::Cache.with do |redis|
-          redis.hget(key, diff_file_id)
+          redis.hget(@redis_key, diff_file_id)
         end
       end
 
@@ -106,7 +105,15 @@ module Gitlab
       end
 
       def cached_content
-        @cached_content ||= cache.read(key) || {}
+        @cached_content ||= populate_cached_content || {}
+      end
+
+      def populate_cached_content
+        if Feature.enabled?(:redis_diff_caching)
+          read_entire_redis_hash
+        else
+          cache.read(key)
+        end
       end
 
       def cacheable?(diff_file)

spec/lib/gitlab/diff/highlight_cache_spec.rb

@@ -53,6 +53,7 @@ describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do
                              fallback_diff_refs: diffs.fallback_diff_refs)
     end
 
+
     it 'does not calculate highlighting when reading from cache' do
       cache.write_if_empty
       cache.decorate(diff_file)
@@ -69,11 +70,18 @@ describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do
       expect(diff_file.highlighted_diff_lines.size).to be > 5
     end
 
-    it 'submits a single reading from the cache' do
-      cache.decorate(diff_file)
-      cache.decorate(diff_file)
+    context 'when :redis_diff_caching is not enabled' do
+      before do
+        expect(Feature).to receive(:enabled?).with(:redis_diff_caching).and_return(false)
+      end
+
+      it 'submits a single reading from the cache' do
+        expect(Feature).to receive(:enabled?).with(:redis_diff_caching).at_least(:once).and_return(false)
+
+        2.times { cache.decorate(diff_file) }
 
-      expect(backend).to have_received(:read).with(cache.key).once
+        expect(backend).to have_received(:read).with(cache.key).once
+      end
     end
   end
 
@@ -94,7 +102,7 @@ describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do
 
     context 'when :redis_diff_caching is not enabled' do
       before do
-        expect(Feature).to receive(:enabled?).with(:redis_diff_caching).and_return(false)
+        expect(Feature).to receive(:enabled?).with(:redis_diff_caching).at_least(:once).and_return(false)
       end
 
       it 'submits a single writing to the cache' do
@@ -126,7 +134,7 @@ describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do
     end
 
     it 'returns the entire contents of a Redis hash as JSON' do
-      result = cache.read_entire_redis_hash(cache_key)
+      result = cache.read_entire_redis_hash
 
       expect(result.values.first).to eq(diff_hash.values.first.to_json)
     end
@@ -142,7 +150,7 @@ describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do
 
     it 'returns highlighted diff content for a single file as JSON' do
       diff_hash.each do |file_path, value|
-        found = cache.read_single_entry_from_redis_hash(cache_key, file_path)
+        found = cache.read_single_entry_from_redis_hash(file_path)
 
         expect(found).to eq(value.to_json)
       end