summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2016-11-10 14:00:55 +0000
committerDouwe Maan <douwe@gitlab.com>2016-11-10 14:00:55 +0000
commit9c9e88da56246c6efcd6a52a14f360e9a71a63e6 (patch)
treecbf89daef9331109e9e0361c0466637b73de9705
parent2a829885e2e7aecfdd2ac55fff5d1ff08a25ef0f (diff)
parent6cbd3cafd3c3e7e318f84483460ba5c6af3fc4cf (diff)
downloadgitlab-ce-9c9e88da56246c6efcd6a52a14f360e9a71a63e6.tar.gz
Merge branch 'fix/error-when-invalid-branch-for-new-pipeline-used' into 'master'
Fix error when using invalid branch name when creating a new pipeline ## What does this MR do? This MR fixes `500` error when creating a new pipeline though user interface ("Run pipeline") ## Are there points in the code the reviewer needs to double check? Is this a good approach to catch those exceptions on `Repository` level? ## Does this MR meet the acceptance criteria? - [x] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added - Tests - [x] Added for this feature/bug ## What are the relevant issue numbers? Closes #23982 See merge request !7324
-rw-r--r--app/controllers/projects/pipelines_controller.rb4
-rw-r--r--app/models/repository.rb12
-rw-r--r--changelogs/unreleased/fix-error-when-invalid-branch-for-new-pipeline-used.yml4
-rw-r--r--spec/models/repository_spec.rb49
4 files changed, 62 insertions, 7 deletions
diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb
index 371cc3787fb..533af80aee0 100644
--- a/app/controllers/projects/pipelines_controller.rb
+++ b/app/controllers/projects/pipelines_controller.rb
@@ -18,7 +18,9 @@ class Projects::PipelinesController < Projects::ApplicationController
end
def create
- @pipeline = Ci::CreatePipelineService.new(project, current_user, create_params).execute(ignore_skip_ci: true, save_on_errors: false)
+ @pipeline = Ci::CreatePipelineService
+ .new(project, current_user, create_params)
+ .execute(ignore_skip_ci: true, save_on_errors: false)
unless @pipeline.persisted?
render 'new'
return
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 7d06ce1e85b..fe991904601 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -84,15 +84,17 @@ class Repository
def commit(ref = 'HEAD')
return nil unless exists?
+
commit =
if ref.is_a?(Gitlab::Git::Commit)
ref
else
Gitlab::Git::Commit.find(raw_repository, ref)
end
+
commit = ::Commit.new(commit, @project) if commit
commit
- rescue Rugged::OdbError
+ rescue Rugged::OdbError, Rugged::TreeError
nil
end
@@ -232,6 +234,8 @@ class Repository
def ref_exists?(ref)
rugged.references.exist?(ref)
+ rescue Rugged::ReferenceError
+ false
end
def update_ref!(name, newrev, oldrev)
@@ -270,11 +274,7 @@ class Repository
end
def kept_around?(sha)
- begin
- ref_exists?(keep_around_ref_name(sha))
- rescue Rugged::ReferenceError
- false
- end
+ ref_exists?(keep_around_ref_name(sha))
end
def tag_names
diff --git a/changelogs/unreleased/fix-error-when-invalid-branch-for-new-pipeline-used.yml b/changelogs/unreleased/fix-error-when-invalid-branch-for-new-pipeline-used.yml
new file mode 100644
index 00000000000..ad6aa214f0f
--- /dev/null
+++ b/changelogs/unreleased/fix-error-when-invalid-branch-for-new-pipeline-used.yml
@@ -0,0 +1,4 @@
+---
+title: Fix error when using invalid branch name when creating a new pipeline
+merge_request: 7324
+author:
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 12989d4db53..fe26b4ac18c 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -113,6 +113,26 @@ describe Repository, models: true do
end
end
+ describe '#ref_exists?' do
+ context 'when ref exists' do
+ it 'returns true' do
+ expect(repository.ref_exists?('refs/heads/master')).to be true
+ end
+ end
+
+ context 'when ref does not exist' do
+ it 'returns false' do
+ expect(repository.ref_exists?('refs/heads/non-existent')).to be false
+ end
+ end
+
+ context 'when ref format is incorrect' do
+ it 'returns false' do
+ expect(repository.ref_exists?('refs/heads/invalid:master')).to be false
+ end
+ end
+ end
+
describe '#last_commit_for_path' do
subject { repository.last_commit_for_path(sample_commit.id, '.gitignore').id }
@@ -197,6 +217,35 @@ describe Repository, models: true do
end
end
+ describe '#commit' do
+ context 'when ref exists' do
+ it 'returns commit object' do
+ expect(repository.commit('master'))
+ .to be_an_instance_of Commit
+ end
+ end
+
+ context 'when ref does not exist' do
+ it 'returns nil' do
+ expect(repository.commit('non-existent-ref')).to be_nil
+ end
+ end
+
+ context 'when ref is not valid' do
+ context 'when preceding tree element exists' do
+ it 'returns nil' do
+ expect(repository.commit('master:ref')).to be_nil
+ end
+ end
+
+ context 'when preceding tree element does not exist' do
+ it 'returns nil' do
+ expect(repository.commit('non-existent:ref')).to be_nil
+ end
+ end
+ end
+ end
+
describe "#commit_dir" do
it "commits a change that creates a new directory" do
expect do