Commit 9c9e88da authored by Douwe Maan's avatar Douwe Maan

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
parents 2a829885 6cbd3caf
......@@ -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
......
......@@ -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
......
---
title: Fix error when using invalid branch name when creating a new pipeline
merge_request: 7324
author:
......@@ -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
......
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