diff --git a/app/views/projects/diffs/_content.html.haml b/app/views/projects/diffs/_content.html.haml
index 59844bc00cdda25427e5a3a8151e57784d857b53..ec1c434a4b8f9812d73ad47c0c06ce84ce195c01 100644
--- a/app/views/projects/diffs/_content.html.haml
+++ b/app/views/projects/diffs/_content.html.haml
@@ -6,7 +6,7 @@
- elsif blob.truncated?
.nothing-here-block The file could not be displayed because it is too large.
- elsif blob.readable_text?
- - if !diff_file.repository.diffable?(blob)
+ - if !diff_file.diffable?
.nothing-here-block This diff was suppressed by a .gitattributes entry.
- elsif diff_file.collapsed?
- url = url_for(params.merge(action: :diff_for_path, old_path: diff_file.old_path, new_path: diff_file.new_path, file_identifier: diff_file.file_identifier))
diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb
index 2aef7fdaa350e33492a8ee05703cdbd83d8ffc1e..4212a0dbe2e01cf3001f2ed8f1d8bfb446d1ea5a 100644
--- a/lib/gitlab/diff/file.rb
+++ b/lib/gitlab/diff/file.rb
@@ -58,19 +58,19 @@ module Gitlab
diff_refs&.head_sha
end
- def content_sha
- return old_content_sha if deleted_file?
- return @content_sha if defined?(@content_sha)
+ def new_content_sha
+ return if deleted_file?
+ return @new_content_sha if defined?(@new_content_sha)
refs = diff_refs || fallback_diff_refs
- @content_sha = refs&.head_sha
+ @new_content_sha = refs&.head_sha
end
- def content_commit
- return @content_commit if defined?(@content_commit)
+ def new_content_commit
+ return @new_content_commit if defined?(@new_content_commit)
- sha = content_sha
- @content_commit = repository.commit(sha) if sha
+ sha = new_content_commit
+ @new_content_commit = repository.commit(sha) if sha
end
def old_content_sha
@@ -88,13 +88,13 @@ module Gitlab
@old_content_commit = repository.commit(sha) if sha
end
- def blob
- return @blob if defined?(@blob)
+ def new_blob
+ return @new_blob if defined?(@new_blob)
- sha = content_sha
- return @blob = nil unless sha
+ sha = new_content_sha
+ return @new_blob = nil unless sha
- repository.blob_at(sha, file_path)
+ @new_blob = repository.blob_at(sha, file_path)
end
def old_blob
@@ -106,6 +106,18 @@ module Gitlab
@old_blob = repository.blob_at(sha, old_path)
end
+ def content_sha
+ new_content_sha || old_content_sha
+ end
+
+ def content_commit
+ new_content_commit || old_content_commit
+ end
+
+ def blob
+ new_blob || old_blob
+ end
+
attr_writer :highlighted_diff_lines
# Array of Gitlab::Diff::Line objects
@@ -153,6 +165,18 @@ module Gitlab
def file_identifier
"#{file_path}-#{new_file?}-#{deleted_file?}-#{renamed_file?}"
end
+
+ def diffable?
+ repository.attributes(file_path).fetch('diff') { true }
+ end
+
+ def binary?
+ old_blob&.binary? || new_blob&.binary?
+ end
+
+ def text?
+ !binary?
+ end
end
end
end
diff --git a/lib/gitlab/diff/file_collection/merge_request_diff.rb b/lib/gitlab/diff/file_collection/merge_request_diff.rb
index 9a58b500a2ccc54a23ea85447a2d304699db02a2..fcda1fe2233c0a5f9782fe78f3b0d493e3d2b2e6 100644
--- a/lib/gitlab/diff/file_collection/merge_request_diff.rb
+++ b/lib/gitlab/diff/file_collection/merge_request_diff.rb
@@ -66,10 +66,7 @@ module Gitlab
end
def cacheable?(diff_file)
- @merge_request_diff.present? &&
- diff_file.blob &&
- diff_file.blob.text? &&
- @project.repository.diffable?(diff_file.blob)
+ @merge_request_diff.present? && diff_file.text? && diff_file.diffable?
end
def cache_key
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 9d6adbdb4ac480b7fe0747485289fedf839e51f4..85695d0a4df9fcf407bd441850e8a32717ea2cdb 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -962,11 +962,6 @@ module Gitlab
end
end
- # Checks if the blob should be diffable according to its attributes
- def diffable?(blob)
- attributes(blob.path).fetch('diff') { blob.text? }
- end
-
# Returns the Git attributes for the given file path.
#
# See `Gitlab::Git::Attributes` for more information.
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 f2bc15d39d7a19e8016ce6607d461da36600c3cd..d81774c8b8fbfab408523f832e8a227ab8edb186 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
@@ -5,15 +5,7 @@ describe Gitlab::Diff::FileCollection::MergeRequestDiff do
let(:diff_files) { described_class.new(merge_request.merge_request_diff, diff_options: nil).diff_files }
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)
-
- diff_files
- end
-
- it 'does not highlight file if blob is not accessable' do
- allow_any_instance_of(Gitlab::Diff::File).to receive(:blob).and_return(nil)
+ allow_any_instance_of(Gitlab::Diff::File).to receive(:text?).and_return(false)
expect_any_instance_of(Gitlab::Diff::File).not_to receive(:highlighted_diff_lines)
@@ -21,7 +13,7 @@ describe Gitlab::Diff::FileCollection::MergeRequestDiff do
end
it 'does not files marked as undiffable in .gitattributes' do
- allow_any_instance_of(Repository).to receive(:diffable?).and_return(false)
+ allow_any_instance_of(Gitlab::Diff::File).to receive(:diffable?).and_return(false)
expect_any_instance_of(Gitlab::Diff::File).not_to receive(:highlighted_diff_lines)
diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb
index 050689b7c9ae14174299a080162571d34016b979..a9953bb0d0166cd8c658c40b86ea1f808deeecf6 100644
--- a/spec/lib/gitlab/diff/file_spec.rb
+++ b/spec/lib/gitlab/diff/file_spec.rb
@@ -6,7 +6,7 @@ describe Gitlab::Diff::File, lib: true do
let(:project) { create(:project, :repository) }
let(:commit) { project.commit(sample_commit.id) }
let(:diff) { commit.raw_diffs.first }
- let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: project.repository) }
+ let(:diff_file) { described_class.new(diff, diff_refs: commit.diff_refs, repository: project.repository) }
describe '#diff_lines' do
let(:diff_lines) { diff_file.diff_lines }
@@ -63,11 +63,33 @@ describe Gitlab::Diff::File, lib: true do
end
end
- describe '#blob' do
+ describe '#new_blob' do
it 'returns blob of new commit' do
- data = diff_file.blob.data
+ data = diff_file.new_blob.data
expect(data).to include('raise RuntimeError, "System commands must be given as an array of strings"')
end
end
+
+ describe '#diffable?' do
+ let(:commit) { project.commit('1a0b36b3cdad1d2ee32457c102a8c0b7056fa863') }
+ let(:diffs) { commit.diffs }
+
+ before do
+ info_dir_path = File.join(project.repository.path_to_repo, 'info')
+
+ FileUtils.mkdir(info_dir_path) unless File.exist?(info_dir_path)
+ File.write(File.join(info_dir_path, 'attributes'), "*.md -diff\n")
+ end
+
+ it "returns true for files that do not have attributes" do
+ diff_file = diffs.diff_file_with_new_path('LICENSE')
+ expect(diff_file.diffable?).to be_truthy
+ end
+
+ it "returns false for files that have been marked as not being diffable in attributes" do
+ diff_file = diffs.diff_file_with_new_path('README.md')
+ expect(diff_file.diffable?).to be_falsey
+ end
+ end
end
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 26215381cc47355fcb5a13fb5b0f1f57662ba283..e1e4aa9fde947fdf64f9f3492de5291cbf4f147f 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -1235,47 +1235,6 @@ describe Gitlab::Git::Repository, seed_helper: true do
end
end
- describe '#diffable' do
- info_dir_path = attributes_path = File.join(SEED_STORAGE_PATH, TEST_REPO_PATH, 'info')
- attributes_path = File.join(info_dir_path, 'attributes')
-
- before(:all) do
- FileUtils.mkdir(info_dir_path) unless File.exist?(info_dir_path)
- File.write(attributes_path, "*.md -diff\n")
- end
-
- it "should return true for files which are text and do not have attributes" do
- blob = Gitlab::Git::Blob.find(
- repository,
- '33bcff41c232a11727ac6d660bd4b0c2ba86d63d',
- 'LICENSE'
- )
- expect(repository.diffable?(blob)).to be_truthy
- end
-
- it "should return false for binary files which do not have attributes" do
- blob = Gitlab::Git::Blob.find(
- repository,
- '33bcff41c232a11727ac6d660bd4b0c2ba86d63d',
- 'files/images/logo-white.png'
- )
- expect(repository.diffable?(blob)).to be_falsey
- end
-
- it "should return false for text files which have been marked as not being diffable in attributes" do
- blob = Gitlab::Git::Blob.find(
- repository,
- '33bcff41c232a11727ac6d660bd4b0c2ba86d63d',
- 'README.md'
- )
- expect(repository.diffable?(blob)).to be_falsey
- end
-
- after(:all) do
- FileUtils.rm_rf(info_dir_path)
- end
- end
-
describe '#tag_exists?' do
it 'returns true for an existing tag' do
tag = repository.tag_names.first
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 935f4710851863018a370d7474c6e39e2c1a6701..bb46e1dd9abdc5abc1222037c10c51be0db6b687 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
@@ -10,8 +10,8 @@ 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)
+ allow_any_instance_of(Gitlab::Diff::File).to receive(:text?).and_return(true)
+ allow_any_instance_of(Gitlab::Diff::File).to receive(:diffable?).and_return(true)
subject.execute(merge_request)
end