diff options
author | Sean McGivern <sean@gitlab.com> | 2018-03-16 15:35:58 +0000 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2018-03-16 18:19:33 +0000 |
commit | bb226a294b12bc0430fe506bef5baea01a26de99 (patch) | |
tree | ddd9817849bed4ec260bb3a03404c368afa52b12 /lib/gitlab/diff | |
parent | 0b849e8d30ee662e84f72b853820a94a60f2276b (diff) | |
download | gitlab-ce-bb226a294b12bc0430fe506bef5baea01a26de99.tar.gz |
Ensure that we never assume old_blob or new_blob are nil44257-viewing-a-particular-commit-gives-500-error-error-undefined-method-binary
These can be a `BatchLoader` which is proxying a nil, while not being concrete
nils themselves.
Diffstat (limited to 'lib/gitlab/diff')
-rw-r--r-- | lib/gitlab/diff/file.rb | 39 |
1 files changed, 21 insertions, 18 deletions
diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb index 34b070dd375..014854da55c 100644 --- a/lib/gitlab/diff/file.rb +++ b/lib/gitlab/diff/file.rb @@ -27,8 +27,8 @@ module Gitlab @fallback_diff_refs = fallback_diff_refs # Ensure items are collected in the the batch - new_blob - old_blob + new_blob_lazy + old_blob_lazy end def position(position_marker, position_type: :text) @@ -101,25 +101,19 @@ module Gitlab end def new_blob - return unless new_content_sha - - Blob.lazy(repository.project, new_content_sha, file_path) + new_blob_lazy&.itself end def old_blob - return unless old_content_sha - - Blob.lazy(repository.project, old_content_sha, old_path) + old_blob_lazy&.itself end def content_sha 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&.itself || old_blob&.itself + new_blob || old_blob end attr_writer :highlighted_diff_lines @@ -237,17 +231,14 @@ 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). + # We can't use Object#try 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) + old_blob&.public_send(meth) || new_blob&.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?) + [old_blob, new_blob].compact end def text_position_properties(line) @@ -262,6 +253,18 @@ module Gitlab old_blob && new_blob && old_blob.id != new_blob.id 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 return DiffViewer::NotDiffable unless diffable? |