diff options
-rw-r--r-- | app/controllers/projects/artifacts_controller.rb | 9 | ||||
-rw-r--r-- | app/controllers/projects/build_artifacts_controller.rb | 9 | ||||
-rw-r--r-- | app/models/project.rb | 18 | ||||
-rw-r--r-- | lib/api/job_artifacts.rb | 7 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 162 |
5 files changed, 122 insertions, 83 deletions
diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb index 1a91e07b97f..9a0997e92ee 100644 --- a/app/controllers/projects/artifacts_controller.rb +++ b/app/controllers/projects/artifacts_controller.rb @@ -85,20 +85,15 @@ class Projects::ArtifactsController < Projects::ApplicationController end end - # rubocop: disable CodeReuse/ActiveRecord def build_from_id - project.builds.find_by(id: params[:job_id]) if params[:job_id] + project.get_build(params[:job_id]) if params[:job_id] end - # rubocop: enable CodeReuse/ActiveRecord - # rubocop: disable CodeReuse/ActiveRecord def build_from_ref return unless @ref_name - builds = project.latest_successful_builds_for(@ref_name) - builds.find_by(name: params[:job]) + project.latest_successful_build_for(params[:job], @ref_name) end - # rubocop: enable CodeReuse/ActiveRecord def artifacts_file @artifacts_file ||= build&.artifacts_file_for_type(params[:file_type] || :archive) diff --git a/app/controllers/projects/build_artifacts_controller.rb b/app/controllers/projects/build_artifacts_controller.rb index 7d4d566499c..d3d5ba5c75d 100644 --- a/app/controllers/projects/build_artifacts_controller.rb +++ b/app/controllers/projects/build_artifacts_controller.rb @@ -44,18 +44,13 @@ class Projects::BuildArtifactsController < Projects::ApplicationController @job ||= job_from_id || job_from_ref end - # rubocop: disable CodeReuse/ActiveRecord def job_from_id - project.builds.find_by(id: params[:build_id]) if params[:build_id] + project.get_build(params[:build_id]) if params[:build_id] end - # rubocop: enable CodeReuse/ActiveRecord - # rubocop: disable CodeReuse/ActiveRecord def job_from_ref return unless @ref_name - jobs = project.latest_successful_builds_for(@ref_name) - jobs.find_by(name: params[:job]) + project.latest_successful_build_for(params[:job], @ref_name) end - # rubocop: enable CodeReuse/ActiveRecord end diff --git a/app/models/project.rb b/app/models/project.rb index cab173503ce..a66ed6736ca 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -647,19 +647,19 @@ class Project < ActiveRecord::Base end # ref can't be HEAD, can only be branch/tag name or SHA - def latest_successful_builds_for(ref = default_branch) + def latest_successful_build_for(job_name, ref = default_branch) latest_pipeline = ci_pipelines.latest_successful_for(ref) + return unless latest_pipeline - if latest_pipeline - latest_pipeline.builds.latest.with_artifacts_archive - else - builds.none - end + latest_pipeline.builds.latest.with_artifacts_archive.find_by(name: job_name) end - def latest_successful_build_for(job_name, ref = default_branch) - builds = latest_successful_builds_for(ref) - builds.find_by!(name: job_name) + def latest_successful_build_for!(job_name, ref = default_branch) + latest_successful_build_for(job_name, ref) || raise(ActiveRecord::RecordNotFound.new("Couldn't find job #{job_name}")) + end + + def get_build(id) + builds.find_by(id: id) end def merge_base_commit(first_commit_id, second_commit_id) diff --git a/lib/api/job_artifacts.rb b/lib/api/job_artifacts.rb index a4068a200b3..933bd067e26 100644 --- a/lib/api/job_artifacts.rb +++ b/lib/api/job_artifacts.rb @@ -23,17 +23,14 @@ module API requires :job, type: String, desc: 'The name for the job' end route_setting :authentication, job_token_allowed: true - # rubocop: disable CodeReuse/ActiveRecord get ':id/jobs/artifacts/:ref_name/download', requirements: { ref_name: /.+/ } do authorize_download_artifacts! - builds = user_project.latest_successful_builds_for(params[:ref_name]) - latest_build = builds.find_by!(name: params[:job]) + latest_build = user_project.latest_successful_build_for!(params[:job], params[:ref_name]) present_carrierwave_file!(latest_build.artifacts_file) end - # rubocop: enable CodeReuse/ActiveRecord desc 'Download a specific file from artifacts archive from a ref' do detail 'This feature was introduced in GitLab 11.5' @@ -48,7 +45,7 @@ module API requirements: { ref_name: /.+/ } do authorize_download_artifacts! - build = user_project.latest_successful_build_for(params[:job], params[:ref_name]) + build = user_project.latest_successful_build_for!(params[:job], params[:ref_name]) path = Gitlab::Ci::Build::Artifacts::Path .new(params[:artifact_path]) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index d1ab0bdba29..3044150bca8 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1105,13 +1105,13 @@ describe Project do describe '#pipeline_for' do let(:project) { create(:project, :repository) } - let!(:pipeline) { create_pipeline } + let!(:pipeline) { create_pipeline(project) } shared_examples 'giving the correct pipeline' do it { is_expected.to eq(pipeline) } context 'return latest' do - let!(:pipeline2) { create_pipeline } + let!(:pipeline2) { create_pipeline(project) } it { is_expected.to eq(pipeline2) } end @@ -1128,13 +1128,6 @@ describe Project do it_behaves_like 'giving the correct pipeline' end - - def create_pipeline - create(:ci_pipeline, - project: project, - ref: 'master', - sha: project.commit('master').sha) - end end describe '#builds_enabled' do @@ -1922,38 +1915,21 @@ describe Project do end end - describe '#latest_successful_builds_for and #latest_successful_build_for' do - def create_pipeline(status = 'success') - create(:ci_pipeline, project: project, - sha: project.commit.sha, - ref: project.default_branch, - status: status) - end - - def create_build(new_pipeline = pipeline, name = 'test') - create(:ci_build, :success, :artifacts, - pipeline: new_pipeline, - status: new_pipeline.status, - name: name) - end - + describe '#latest_successful_build_for' do let(:project) { create(:project, :repository) } - let(:pipeline) { create_pipeline } + let(:pipeline) { create_pipeline(project) } context 'with many builds' do it 'gives the latest builds from latest pipeline' do - pipeline1 = create_pipeline - pipeline2 = create_pipeline + pipeline1 = create_pipeline(project) + pipeline2 = create_pipeline(project) create_build(pipeline1, 'test') create_build(pipeline1, 'test2') build1_p2 = create_build(pipeline2, 'test') - build2_p2 = create_build(pipeline2, 'test2') + create_build(pipeline2, 'test2') - latest_builds = project.latest_successful_builds_for - single_build = project.latest_successful_build_for(build1_p2.name) - - expect(latest_builds).to contain_exactly(build2_p2, build1_p2) - expect(single_build).to eq(build1_p2) + expect(project.latest_successful_build_for(build1_p2.name)) + .to eq(build1_p2) end end @@ -1962,53 +1938,115 @@ describe Project do context 'standalone pipeline' do it 'returns builds for ref for default_branch' do - builds = project.latest_successful_builds_for - single_build = project.latest_successful_build_for(build.name) + expect(project.latest_successful_build_for(build.name)) + .to eq(build) + end - expect(builds).to contain_exactly(build) - expect(single_build).to eq(build) + it 'returns empty relation if the build cannot be found' do + expect(project.latest_successful_build_for('TAIL')) + .to be_nil end + end - it 'returns empty relation if the build cannot be found for #latest_successful_builds_for' do - builds = project.latest_successful_builds_for('TAIL') + context 'with some pending pipeline' do + before do + create_build(create_pipeline(project, 'pending')) + end - expect(builds).to be_kind_of(ActiveRecord::Relation) - expect(builds).to be_empty + it 'gives the latest build from latest pipeline' do + expect(project.latest_successful_build_for(build.name)) + .to eq(build) end + end + end - it 'returns exception if the build cannot be found for #latest_successful_build_for' do - expect { project.latest_successful_build_for(build.name, 'TAIL') }.to raise_error(ActiveRecord::RecordNotFound) + context 'with pending pipeline' do + it 'returns empty relation' do + pipeline.update(status: 'pending') + pending_build = create_build(pipeline) + + expect(project.latest_successful_build_for(pending_build.name)).to be_nil + end + end + end + + describe '#latest_successful_build_for!' do + let(:project) { create(:project, :repository) } + let(:pipeline) { create_pipeline(project) } + + context 'with many builds' do + it 'gives the latest builds from latest pipeline' do + pipeline1 = create_pipeline(project) + pipeline2 = create_pipeline(project) + create_build(pipeline1, 'test') + create_build(pipeline1, 'test2') + build1_p2 = create_build(pipeline2, 'test') + create_build(pipeline2, 'test2') + + expect(project.latest_successful_build_for(build1_p2.name)) + .to eq(build1_p2) + end + end + + context 'with succeeded pipeline' do + let!(:build) { create_build } + + context 'standalone pipeline' do + it 'returns builds for ref for default_branch' do + expect(project.latest_successful_build_for!(build.name)) + .to eq(build) + end + + it 'returns exception if the build cannot be found' do + expect { project.latest_successful_build_for!(build.name, 'TAIL') } + .to raise_error(ActiveRecord::RecordNotFound) end end context 'with some pending pipeline' do before do - create_build(create_pipeline('pending')) + create_build(create_pipeline(project, 'pending')) end it 'gives the latest build from latest pipeline' do - latest_builds = project.latest_successful_builds_for - last_single_build = project.latest_successful_build_for(build.name) - - expect(latest_builds).to contain_exactly(build) - expect(last_single_build).to eq(build) + expect(project.latest_successful_build_for!(build.name)) + .to eq(build) end end end context 'with pending pipeline' do - before do + it 'returns empty relation' do pipeline.update(status: 'pending') - create_build(pipeline) + pending_build = create_build(pipeline) + + expect { project.latest_successful_build_for!(pending_build.name) } + .to raise_error(ActiveRecord::RecordNotFound) end + end + end - it 'returns empty relation' do - builds = project.latest_successful_builds_for + describe '#get_build' do + let(:project) { create(:project, :repository) } + let(:ci_pipeline) { create(:ci_pipeline, project: project) } + + context 'when build exists' do + context 'build is associated with project' do + let(:build) { create(:ci_build, :success, pipeline: ci_pipeline) } + + it { expect(project.get_build(build.id)).to eq(build) } + end - expect(builds).to be_kind_of(ActiveRecord::Relation) - expect(builds).to be_empty + context 'build is not associated with project' do + let(:build) { create(:ci_build, :success) } + + it { expect(project.get_build(build.id)).to be_nil } end end + + context 'build does not exists' do + it { expect(project.get_build(rand 100)).to be_nil } + end end describe '#import_status' do @@ -4390,4 +4428,18 @@ describe Project do def rugged_config rugged_repo(project.repository).config end + + def create_pipeline(project, status = 'success') + create(:ci_pipeline, project: project, + sha: project.commit.sha, + ref: project.default_branch, + status: status) + end + + def create_build(new_pipeline = pipeline, name = 'test') + create(:ci_build, :success, :artifacts, + pipeline: new_pipeline, + status: new_pipeline.status, + name: name) + end end |