Merge branch...

Merge branch '25931-gitlab-merge-request-view-crash-when-commiting-a-js-sourcemap-file' into 'master'

Resolve "Gitlab merge request view crash when commiting a js sourcemap file"

Closes #25931

See merge request !8310

app/models/merge_request.rb

@@ -198,7 +198,9 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def diff_size
-    diffs(diff_options).size
+    opts = diff_options || {}
+
+    raw_diffs(opts).size
   end
 
   def diff_base_commit

changelogs/unreleased/25931-gitlab-merge-request-view-crash-when-commiting-a-js-sourcemap-file.yml

+---
+title: Fix timeout when MR contains large files marked as binary by .gitattributes
+merge_request:
+author:

lib/gitlab/diff/file_collection/merge_request_diff.rb

@@ -61,7 +61,10 @@ module Gitlab
         end
 
         def cacheable?(diff_file)
-          @merge_request_diff.present? && diff_file.blob && diff_file.blob.text?
+          @merge_request_diff.present? &&
+            diff_file.blob &&
+            diff_file.blob.text? &&
+            @project.repository.diffable?(diff_file.blob)
         end
 
         def cache_key

spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb

 require 'spec_helper'
 
 describe Gitlab::Diff::FileCollection::MergeRequestDiff do
-  let(:merge_request) { create :merge_request }
+  let(:merge_request) { create(:merge_request) }
+  let(:diff_files) { described_class.new(merge_request.merge_request_diff, diff_options: nil).diff_files }
 
-  it 'does not hightlight binary files' do
+  it 'does not highlight binary files' do
     allow_any_instance_of(Gitlab::Diff::File).to receive(:blob).and_return(double("text?" => false))
 
     expect_any_instance_of(Gitlab::Diff::File).not_to receive(:highlighted_diff_lines)
 
-    described_class.new(merge_request.merge_request_diff, diff_options: nil).diff_files
+    diff_files
   end
 
-  it 'does not hightlight file if blob is not accessable' do
+  it 'does not highlight file if blob is not accessable' do
     allow_any_instance_of(Gitlab::Diff::File).to receive(:blob).and_return(nil)
 
     expect_any_instance_of(Gitlab::Diff::File).not_to receive(:highlighted_diff_lines)
 
-    described_class.new(merge_request.merge_request_diff, diff_options: nil).diff_files
+    diff_files
+  end
+
+  it 'does not files marked as undiffable in .gitattributes' do
+    allow_any_instance_of(Repository).to receive(:diffable?).and_return(false)
+
+    expect_any_instance_of(Gitlab::Diff::File).not_to receive(:highlighted_diff_lines)
+
+    diff_files
   end
 end

spec/services/merge_requests/merge_request_diff_cache_service_spec.rb

@@ -11,6 +11,7 @@ describe MergeRequests::MergeRequestDiffCacheService do
       expect(Rails.cache).to receive(:read).with(cache_key).and_return({})
       expect(Rails.cache).to receive(:write).with(cache_key, anything)
       allow_any_instance_of(Gitlab::Diff::File).to receive(:blob).and_return(double("text?" => true))
+      allow_any_instance_of(Repository).to receive(:diffable?).and_return(true)
 
       subject.execute(merge_request)
     end