Commit 016a2b6e authored by Douwe Maan's avatar Douwe Maan

Merge branch...

Merge branch '41468-error-500-trying-to-view-a-merge-request-json-undefined-method-binary-for-nil-nilclass' into 'master'

Resolve "Error 500 trying to view a merge request JSON: undefined method `binary?' for nil:NilClass"

Closes #41468

See merge request gitlab-org/gitlab-ce!16193
parents bdad21d6 528b5eeb
---
title: Fix viewing merge request diffs where the underlying blobs are unavailable
merge_request:
author:
type: fixed
......@@ -116,8 +116,10 @@ module Gitlab
new_content_sha || old_content_sha
end
# Use #itself to check the value wrapped by a BatchLoader instance, rather
# than if the BatchLoader instance itself is falsey.
def blob
new_blob || old_blob
new_blob&.itself || old_blob&.itself
end
attr_writer :highlighted_diff_lines
......@@ -173,7 +175,7 @@ module Gitlab
end
def binary?
has_binary_notice? || old_blob&.binary? || new_blob&.binary?
has_binary_notice? || try_blobs(:binary?)
end
def text?
......@@ -181,15 +183,15 @@ module Gitlab
end
def external_storage_error?
old_blob&.external_storage_error? || new_blob&.external_storage_error?
try_blobs(:external_storage_error?)
end
def stored_externally?
old_blob&.stored_externally? || new_blob&.stored_externally?
try_blobs(:stored_externally?)
end
def external_storage
old_blob&.external_storage || new_blob&.external_storage
try_blobs(:external_storage)
end
def content_changed?
......@@ -204,15 +206,15 @@ module Gitlab
end
def size
[old_blob&.size, new_blob&.size].compact.sum
valid_blobs.map(&:size).sum
end
def raw_size
[old_blob&.raw_size, new_blob&.raw_size].compact.sum
valid_blobs.map(&:raw_size).sum
end
def raw_binary?
old_blob&.raw_binary? || new_blob&.raw_binary?
try_blobs(:raw_binary?)
end
def raw_text?
......@@ -235,6 +237,19 @@ module Gitlab
private
# The blob instances are instances of BatchLoader, which means calling
# &. directly on them won't work. Object#try also won't work, because Blob
# doesn't inherit from Object, but from BasicObject (via SimpleDelegator).
def try_blobs(meth)
old_blob&.itself&.public_send(meth) || new_blob&.itself&.public_send(meth)
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
[old_blob, new_blob].reject(&:nil?)
end
def text_position_properties(line)
{ old_line: line.old_line, new_line: line.new_line }
end
......
......@@ -431,4 +431,29 @@ describe Gitlab::Diff::File do
end
end
end
context 'when neither blob exists' do
let(:blank_diff_refs) { Gitlab::Diff::DiffRefs.new(base_sha: Gitlab::Git::BLANK_SHA, head_sha: Gitlab::Git::BLANK_SHA) }
let(:diff_file) { described_class.new(diff, diff_refs: blank_diff_refs, repository: project.repository) }
describe '#blob' do
it 'returns a concrete nil so it can be used in boolean expressions' do
actual = diff_file.blob && true
expect(actual).to be_nil
end
end
describe '#binary?' do
it 'returns false' do
expect(diff_file).not_to be_binary
end
end
describe '#size' do
it 'returns zero' do
expect(diff_file.size).to be_zero
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment