From ed84fcfcfe3c6d3143484e20a1fc3e24da541974 Mon Sep 17 00:00:00 2001
From: Oswaldo Ferreira <oswaldo@gitlab.com>
Date: Wed, 6 Jun 2018 16:17:39 -0300
Subject: Return uncached blob if there is not highlighted diff cache yet

---
 app/models/blobs_service.rb                           | 13 ++++++++++---
 lib/gitlab/diff/file.rb                               | 15 ++++++++++++---
 lib/gitlab/diff/file_collection/merge_request_diff.rb |  4 ++--
 3 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/app/models/blobs_service.rb b/app/models/blobs_service.rb
index 33d103aca26..94e4f9eb675 100644
--- a/app/models/blobs_service.rb
+++ b/app/models/blobs_service.rb
@@ -2,10 +2,11 @@
 # It acts as a thin layer which caches lightweight information
 # based on the blob content (which is not cached).
 class BlobsService
-  def initialize(project, content_sha, path)
+  def initialize(project, content_sha, path, highlighted:)
     @project = project
     @content_sha = content_sha
     @path = path
+    @highlighted = highlighted
   end
 
   def id
@@ -66,13 +67,19 @@ class BlobsService
 
   private
 
+  # We need blobs data (content) in order to highlight diffs (see
+  # Gitlab::Diff:Highlight), and we don't cache this (Blob#data) on Redis,
+  # mainly because it's a quite heavy information to cache for every blob.
+  #
+  # Therefore, in this scenario (no highlight yet) we use the uncached blob
+  # version.
   def blob
-    if cache_exists?
+    if @highlighted && cache_exists?
       # TODO: This can be a CachedBlob
       Hashie::Mash.new(read_cache)
     else
       write_cache
-      blob
+      uncached_blob
     end
   end
 
diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb
index 486df1774cc..57608782331 100644
--- a/lib/gitlab/diff/file.rb
+++ b/lib/gitlab/diff/file.rb
@@ -106,13 +106,19 @@ module Gitlab
       def new_blob
         return unless new_content_sha
 
-        BlobsService.new(repository.project, new_content_sha, file_path)
+        BlobsService.new(repository.project,
+                         new_content_sha,
+                         file_path,
+                         highlighted: highlighted?)
       end
 
       def old_blob
         return unless old_content_sha
 
-        BlobsService.new(repository.project, old_content_sha, file_path)
+        BlobsService.new(repository.project,
+                         old_content_sha,
+                         file_path,
+                         highlighted: highlighted?)
       end
 
       def content_sha
@@ -125,7 +131,6 @@ module Gitlab
 
       attr_writer :highlighted_diff_lines
 
-      # Array of Gitlab::Diff::Line objects
       def diff_lines
         @diff_lines ||= Gitlab::Diff::Parser.new.parse(raw_diff.each_line).to_a
       end
@@ -238,6 +243,10 @@ module Gitlab
 
       private
 
+      def highlighted?
+        @highlighted_diff_lines.present?
+      end
+
       # We can't use Object#try because Blob doesn't inherit from Object, but
       # from BasicObject (via SimpleDelegator).
       def try_blobs(meth)
diff --git a/lib/gitlab/diff/file_collection/merge_request_diff.rb b/lib/gitlab/diff/file_collection/merge_request_diff.rb
index 1529044d10c..350ffa51290 100644
--- a/lib/gitlab/diff/file_collection/merge_request_diff.rb
+++ b/lib/gitlab/diff/file_collection/merge_request_diff.rb
@@ -19,7 +19,7 @@ module Gitlab
           #
           diff_files = super
 
-          diff_files.each { |diff_file| cache_highlight!(diff_file) if cacheable?(diff_file) }
+          diff_files.each { |diff_file| cache_highlight!(diff_file) }
           store_highlight_cache
 
           diff_files
@@ -66,7 +66,7 @@ module Gitlab
           if highlight_cache[item_key]
             highlight_diff_file_from_cache!(diff_file, highlight_cache[item_key])
           else
-            highlight_cache[item_key] = diff_file.highlighted_diff_lines.map(&:to_hash)
+            highlight_cache[item_key] = diff_file.highlighted_diff_lines.map(&:to_hash) if cacheable?(diff_file)
           end
         end
 
-- 
cgit v1.2.1