From 776cea4c00d883cafc2bc5381f3b61b146a93976 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Thu, 13 Oct 2016 14:19:52 +0100 Subject: [PATCH] Handle case where deployment ref no longer exists Keep-around refs for deployments were only introduced in 8.10, so any deployment created in 8.9 could have a SHA pointing to a commit that no longer exists in the repository. We can't do anything useful with those deployments, so make `#includes_commit?` always return false for those. --- app/models/deployment.rb | 9 ++++++++- spec/models/deployment_spec.rb | 9 +++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 82b27b78229..f63cc179b9e 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -40,7 +40,14 @@ class Deployment < ActiveRecord::Base def includes_commit?(commit) return false unless commit - project.repository.is_ancestor?(commit.id, sha) + # Before 8.10, deployments didn't have keep-around refs. Any deployment + # created before then could have a `sha` referring to a commit that no + # longer exists in the repository, so just ignore those. + begin + project.repository.is_ancestor?(commit.id, sha) + rescue Rugged::OdbError + false + end end def update_merge_request_metrics! diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index bfff639ad78..01a4a53a264 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -38,5 +38,14 @@ describe Deployment, models: true do expect(deployment.includes_commit?(commit)).to be true end end + + context 'when the SHA for the deployment does not exist in the repo' do + it 'returns false' do + deployment.update(sha: Gitlab::Git::BLANK_SHA) + commit = project.commit + + expect(deployment.includes_commit?(commit)).to be false + end + end end end -- GitLab