Ensure that we never assume old_blob or new_blob are nil
These can be a `BatchLoader` which is proxying a nil, while not being concrete nils themselves.
This commit is contained in:
parent
0b849e8d30
commit
bb226a294b
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Fix viewing diffs on old merge requests
|
||||||
|
merge_request: 17805
|
||||||
|
author:
|
||||||
|
type: fixed
|
|
@ -27,8 +27,8 @@ module Gitlab
|
||||||
@fallback_diff_refs = fallback_diff_refs
|
@fallback_diff_refs = fallback_diff_refs
|
||||||
|
|
||||||
# Ensure items are collected in the the batch
|
# Ensure items are collected in the the batch
|
||||||
new_blob
|
new_blob_lazy
|
||||||
old_blob
|
old_blob_lazy
|
||||||
end
|
end
|
||||||
|
|
||||||
def position(position_marker, position_type: :text)
|
def position(position_marker, position_type: :text)
|
||||||
|
@ -101,25 +101,19 @@ module Gitlab
|
||||||
end
|
end
|
||||||
|
|
||||||
def new_blob
|
def new_blob
|
||||||
return unless new_content_sha
|
new_blob_lazy&.itself
|
||||||
|
|
||||||
Blob.lazy(repository.project, new_content_sha, file_path)
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def old_blob
|
def old_blob
|
||||||
return unless old_content_sha
|
old_blob_lazy&.itself
|
||||||
|
|
||||||
Blob.lazy(repository.project, old_content_sha, old_path)
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def content_sha
|
def content_sha
|
||||||
new_content_sha || old_content_sha
|
new_content_sha || old_content_sha
|
||||||
end
|
end
|
||||||
|
|
||||||
# Use #itself to check the value wrapped by a BatchLoader instance, rather
|
|
||||||
# than if the BatchLoader instance itself is falsey.
|
|
||||||
def blob
|
def blob
|
||||||
new_blob&.itself || old_blob&.itself
|
new_blob || old_blob
|
||||||
end
|
end
|
||||||
|
|
||||||
attr_writer :highlighted_diff_lines
|
attr_writer :highlighted_diff_lines
|
||||||
|
@ -237,17 +231,14 @@ module Gitlab
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
# The blob instances are instances of BatchLoader, which means calling
|
# We can't use Object#try because Blob doesn't inherit from Object, but
|
||||||
# &. directly on them won't work. Object#try also won't work, because Blob
|
# from BasicObject (via SimpleDelegator).
|
||||||
# doesn't inherit from Object, but from BasicObject (via SimpleDelegator).
|
|
||||||
def try_blobs(meth)
|
def try_blobs(meth)
|
||||||
old_blob&.itself&.public_send(meth) || new_blob&.itself&.public_send(meth)
|
old_blob&.public_send(meth) || new_blob&.public_send(meth)
|
||||||
end
|
end
|
||||||
|
|
||||||
# We can't use #compact for the same reason we can't use &., but calling
|
|
||||||
# #nil? explicitly does work because it is proxied to the blob itself.
|
|
||||||
def valid_blobs
|
def valid_blobs
|
||||||
[old_blob, new_blob].reject(&:nil?)
|
[old_blob, new_blob].compact
|
||||||
end
|
end
|
||||||
|
|
||||||
def text_position_properties(line)
|
def text_position_properties(line)
|
||||||
|
@ -262,6 +253,18 @@ module Gitlab
|
||||||
old_blob && new_blob && old_blob.id != new_blob.id
|
old_blob && new_blob && old_blob.id != new_blob.id
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def new_blob_lazy
|
||||||
|
return unless new_content_sha
|
||||||
|
|
||||||
|
Blob.lazy(repository.project, new_content_sha, file_path)
|
||||||
|
end
|
||||||
|
|
||||||
|
def old_blob_lazy
|
||||||
|
return unless old_content_sha
|
||||||
|
|
||||||
|
Blob.lazy(repository.project, old_content_sha, old_path)
|
||||||
|
end
|
||||||
|
|
||||||
def simple_viewer_class
|
def simple_viewer_class
|
||||||
return DiffViewer::NotDiffable unless diffable?
|
return DiffViewer::NotDiffable unless diffable?
|
||||||
|
|
||||||
|
|
|
@ -455,5 +455,17 @@ describe Gitlab::Diff::File do
|
||||||
expect(diff_file.size).to be_zero
|
expect(diff_file.size).to be_zero
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe '#different_type?' do
|
||||||
|
it 'returns false' do
|
||||||
|
expect(diff_file).not_to be_different_type
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe '#content_changed?' do
|
||||||
|
it 'returns false' do
|
||||||
|
expect(diff_file).not_to be_content_changed
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in New Issue