diff --git a/changelogs/unreleased/25931-gitlab-merge-request-view-crash-when-commiting-a-js-sourcemap-file.yml b/changelogs/unreleased/25931-gitlab-merge-request-view-crash-when-commiting-a-js-sourcemap-file.yml new file mode 100644 index 0000000000000000000000000000000000000000..023ec1abcfa401b6d68742d65c57228eb2118841 --- /dev/null +++ b/changelogs/unreleased/25931-gitlab-merge-request-view-crash-when-commiting-a-js-sourcemap-file.yml @@ -0,0 +1,4 @@ +--- +title: Fix timeout when MR contains large files marked as binary by .gitattributes +merge_request: +author: diff --git a/lib/gitlab/diff/file_collection/merge_request_diff.rb b/lib/gitlab/diff/file_collection/merge_request_diff.rb index 56530448f3603475b68161bf684d40a8df20f9ea..329d12f13d161210356d1d6342c01b2ca77b85c1 100644 --- a/lib/gitlab/diff/file_collection/merge_request_diff.rb +++ b/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 diff --git a/spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb b/spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb index 2a680f03476acbeeacee02ef9f5c63db323cfd28..f2bc15d39d7a19e8016ce6607d461da36600c3cd 100644 --- a/spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb +++ b/spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb @@ -1,21 +1,30 @@ 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 diff --git a/spec/services/merge_requests/merge_request_diff_cache_service_spec.rb b/spec/services/merge_requests/merge_request_diff_cache_service_spec.rb index 05cdbe5287a4dbf4b7349a277e7036de8ea31551..35804d41b4605446de979e4e91459df3239493f0 100644 --- a/spec/services/merge_requests/merge_request_diff_cache_service_spec.rb +++ b/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