Merge branch '39104-collect-file-paths-from-diff_files' into 'master'

Collect #file_path info from #diff_files

See merge request gitlab-org/gitlab!21446

lib/gitlab/diff/file_collection/base.rb

@@ -31,7 +31,11 @@ module Gitlab
         end
 
         def diff_files
-          @diff_files ||= diffs.decorate! { |diff| decorate_diff!(diff) }
+          raw_diff_files
+        end
+
+        def raw_diff_files
+          @raw_diff_files ||= diffs.decorate! { |diff| decorate_diff!(diff) }
         end
 
         def diff_file_paths

lib/gitlab/diff/highlight_cache.rb

@@ -70,8 +70,6 @@ module Gitlab
 
       def cacheable_files
         strong_memoize(:cacheable_files) do
-          diff_files = @diff_collection.diff_files
-
           diff_files.select { |file| cacheable?(file) && read_file(file).nil? }
         end
       end
@@ -114,7 +112,7 @@ module Gitlab
 
       def file_paths
         strong_memoize(:file_paths) do
-          @diff_collection.diffs.collect(&:file_path)
+          diff_files.collect(&:file_path)
         end
       end
 
@@ -145,6 +143,14 @@ module Gitlab
       def cacheable?(diff_file)
         diffable.present? && diff_file.text? && diff_file.diffable?
       end
+
+      def diff_files
+        # We access raw_diff_files here, as diff_files will attempt to apply the
+        #   highlighting code found in this class, leading  to a circular
+        #   reference.
+        #
+        @diff_collection.raw_diff_files
+      end
     end
   end
 end

spec/lib/gitlab/diff/highlight_cache_spec.rb

@@ -79,10 +79,8 @@ describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do
     end
   end
 
-  describe '#write_if_empty' do
+  shared_examples 'caches missing entries' do
     it 'filters the key/value list of entries to be caches for each invocation' do
-      paths = merge_request.diffs.diff_files.select(&:text?).map(&:file_path)
-
       expect(cache).to receive(:write_to_redis_hash)
         .with(hash_including(*paths))
         .once
@@ -96,6 +94,12 @@ describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do
 
       cache.write_if_empty
     end
+  end
+
+  describe '#write_if_empty' do
+    it_behaves_like 'caches missing entries' do
+      let(:paths) { merge_request.diffs.raw_diff_files.select(&:text?).map(&:file_path) }
+    end
 
     context 'different diff_collections for the same diffable' do
       before do
@@ -109,6 +113,21 @@ describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do
           .to change { Gitlab::Redis::Cache.with { |r| r.hgetall(cache_key) } }
       end
     end
+
+    context 'when cache initialized with MergeRequestDiffBatch' do
+      let(:merge_request_diff_batch) do
+        Gitlab::Diff::FileCollection::MergeRequestDiffBatch.new(
+          merge_request.merge_request_diff,
+          1,
+          10,
+          diff_options: nil)
+      end
+
+      it_behaves_like 'caches missing entries' do
+        let(:cache) { described_class.new(merge_request_diff_batch) }
+        let(:paths) { merge_request_diff_batch.raw_diff_files.select(&:text?).map(&:file_path) }
+      end
+    end
   end
 
   describe '#write_to_redis_hash' do