diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index ffac3a22efc09eab3cd818fa82233bed8b236cc0..65dfe4f019005fc146899b3971c95c767a466a3c 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -15,6 +15,9 @@ module Ci
scope :with_artifacts, ->() { where.not(artifacts_file: nil) }
scope :with_expired_artifacts, ->() { with_artifacts.where('artifacts_expire_at < ?', Time.now) }
scope :last_month, ->() { where('created_at > ?', Date.today - 1.month) }
+ scope :latest_successful_with_artifacts, ->() do
+ with_artifacts.success.order(id: :desc)
+ end
mount_uploader :artifacts_file, ArtifactUploader
mount_uploader :artifacts_metadata, ArtifactUploader
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index a65a826536de8f4fdc8a43a5e25b238a664035c1..f5b4124d1ee7d18108d5def5545b7e94eae73903 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -20,6 +20,14 @@ module Ci
after_touch :update_state
after_save :keep_around_commits
+ # ref can't be HEAD, can only be branch/tag name or SHA
+ scope :latest_successful_for, ->(ref) do
+ table = quoted_table_name
+ # TODO: Use `where(ref: ref).or(sha: ref)` in Rails 5
+ where("#{table}.ref = ? OR #{table}.sha = ?", ref, ref).
+ success.order(id: :desc)
+ end
+
def self.truncate_sha(sha)
sha[0...8]
end
@@ -222,7 +230,7 @@ module Ci
def keep_around_commits
return unless project
-
+
project.repository.keep_around(self.sha)
project.repository.keep_around(self.before_sha)
end
diff --git a/app/models/project.rb b/app/models/project.rb
index a805f5d97bc61bab97d16404cf5beb561bb220df..29aaaf5117f52a67f566f0b42e22fee22ff586f9 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -429,6 +429,13 @@ class Project < ActiveRecord::Base
repository.commit(ref)
end
+ # ref can't be HEAD, can only be branch/tag name or SHA
+ def latest_successful_builds_for(ref = 'master')
+ Ci::Build.joins(:pipeline).
+ merge(pipelines.latest_successful_for(ref)).
+ latest_successful_with_artifacts
+ end
+
def merge_base_commit(first_commit_id, second_commit_id)
sha = repository.merge_base(first_commit_id, second_commit_id)
repository.commit(sha) if sha
diff --git a/lib/api/builds.rb b/lib/api/builds.rb
index d36047acd1f65c03a11d39a67c1ded42f20ebd2d..bb9e8f1ae6ef7a23a6e32d3f4f1b3c9fdfa8f12e 100644
--- a/lib/api/builds.rb
+++ b/lib/api/builds.rb
@@ -52,8 +52,7 @@ module API
get ':id/builds/:build_id' do
authorize_read_builds!
- build = get_build(params[:build_id])
- return not_found!(build) unless build
+ build = get_build!(params[:build_id])
present build, with: Entities::Build,
user_can_download_artifacts: can?(current_user, :read_build, user_project)
@@ -69,18 +68,25 @@ module API
get ':id/builds/:build_id/artifacts' do
authorize_read_builds!
- build = get_build(params[:build_id])
- return not_found!(build) unless build
+ build = get_build!(params[:build_id])
- artifacts_file = build.artifacts_file
-
- unless artifacts_file.file_storage?
- return redirect_to build.artifacts_file.url
- end
+ present_artifact!(build.artifacts_file)
+ end
- return not_found! unless artifacts_file.exists?
+ # Download the artifacts file from ref_name and job
+ #
+ # Parameters:
+ # id (required) - The ID of a project
+ # ref_name (required) - The ref from repository
+ # job (required) - The name for the build
+ # Example Request:
+ # GET /projects/:id/artifacts/:ref_name/download?job=name
+ get ':id/builds/artifacts/:ref_name/download',
+ requirements: { ref_name: /.+/ } do
+ builds = user_project.latest_successful_builds_for(params[:ref_name])
+ latest_build = builds.find_by!(name: params[:job])
- present_file!(artifacts_file.path, artifacts_file.filename)
+ present_artifact!(latest_build.artifacts_file)
end
# Get a trace of a specific build of a project
@@ -97,8 +103,7 @@ module API
get ':id/builds/:build_id/trace' do
authorize_read_builds!
- build = get_build(params[:build_id])
- return not_found!(build) unless build
+ build = get_build!(params[:build_id])
header 'Content-Disposition', "infile; filename=\"#{build.id}.log\""
content_type 'text/plain'
@@ -118,8 +123,7 @@ module API
post ':id/builds/:build_id/cancel' do
authorize_update_builds!
- build = get_build(params[:build_id])
- return not_found!(build) unless build
+ build = get_build!(params[:build_id])
build.cancel
@@ -137,8 +141,7 @@ module API
post ':id/builds/:build_id/retry' do
authorize_update_builds!
- build = get_build(params[:build_id])
- return not_found!(build) unless build
+ build = get_build!(params[:build_id])
return forbidden!('Build is not retryable') unless build.retryable?
build = Ci::Build.retry(build, current_user)
@@ -157,8 +160,7 @@ module API
post ':id/builds/:build_id/erase' do
authorize_update_builds!
- build = get_build(params[:build_id])
- return not_found!(build) unless build
+ build = get_build!(params[:build_id])
return forbidden!('Build is not erasable!') unless build.erasable?
build.erase(erased_by: current_user)
@@ -176,8 +178,8 @@ module API
post ':id/builds/:build_id/artifacts/keep' do
authorize_update_builds!
- build = get_build(params[:build_id])
- return not_found!(build) unless build && build.artifacts?
+ build = get_build!(params[:build_id])
+ return not_found!(build) unless build.artifacts?
build.keep_artifacts!
@@ -192,6 +194,20 @@ module API
user_project.builds.find_by(id: id.to_i)
end
+ def get_build!(id)
+ get_build(id) || not_found!
+ end
+
+ def present_artifact!(artifacts_file)
+ if !artifacts_file.file_storage?
+ redirect_to(build.artifacts_file.url)
+ elsif artifacts_file.exists?
+ present_file!(artifacts_file.path, artifacts_file.filename)
+ else
+ not_found!
+ end
+ end
+
def filter_builds(builds, scope)
return builds if scope.nil? || scope.empty?
diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index 481416319dd9cd01ff8607045578839223679c09..355cb8fdfff1a034823352fb9d7992f24c8de5a1 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -5,7 +5,9 @@ describe Ci::Build, models: true do
let(:pipeline) do
create(:ci_pipeline, project: project,
- sha: project.commit.id)
+ sha: project.commit.id,
+ ref: 'fix',
+ status: 'success')
end
let(:build) { create(:ci_build, pipeline: pipeline) }
@@ -612,7 +614,7 @@ describe Ci::Build, models: true do
describe '#erasable?' do
subject { build.erasable? }
- it { is_expected.to eq true }
+ it { is_expected.to be_truthy }
end
describe '#erased?' do
@@ -620,7 +622,7 @@ describe Ci::Build, models: true do
subject { build.erased? }
context 'build has not been erased' do
- it { is_expected.to be false }
+ it { is_expected.to be_falsey }
end
context 'build has been erased' do
@@ -628,12 +630,13 @@ describe Ci::Build, models: true do
build.erase
end
- it { is_expected.to be true }
+ it { is_expected.to be_truthy }
end
end
context 'metadata and build trace are not available' do
let!(:build) { create(:ci_build, :success, :artifacts) }
+
before do
build.remove_artifacts_metadata!
end
@@ -655,18 +658,58 @@ describe Ci::Build, models: true do
describe '#retryable?' do
context 'when build is running' do
- before { build.run! }
+ before do
+ build.run!
+ end
- it 'should return false' do
- expect(build.retryable?).to be false
+ it 'returns false' do
+ expect(build).not_to be_retryable
end
end
context 'when build is finished' do
- before { build.success! }
+ before do
+ build.success!
+ end
+
+ it 'returns true' do
+ expect(build).to be_retryable
+ end
+ end
+ end
+
+ describe 'Project#latest_successful_builds_for' do
+ let(:build) do
+ create(:ci_build, :artifacts, :success, pipeline: pipeline)
+ end
+
+ before do
+ build
+ end
+
+ context 'with succeed pipeline' do
+ it 'returns builds from ref' do
+ builds = project.latest_successful_builds_for('fix')
+
+ expect(builds).to contain_exactly(build)
+ end
+
+ it 'returns empty relation if the build cannot be found' do
+ builds = project.latest_successful_builds_for('TAIL').all
+
+ expect(builds).to be_empty
+ end
+ end
+
+ context 'with pending pipeline' do
+ before do
+ pipeline.update(status: 'pending')
+ end
+
+ it 'returns empty relation' do
+ builds = project.latest_successful_builds_for('fix').all
- it 'should return true' do
- expect(build.retryable?).to be true
+ expect(builds).to be_empty
end
end
end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 9dc34276f188edbb0617ccb1434e3c67180e22bb..53b420d808fa8bdc59df0d51f29f4298e2f198f2 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -377,7 +377,7 @@ describe Project, models: true do
describe '#repository' do
let(:project) { create(:project) }
- it 'should return valid repo' do
+ it 'returns valid repo' do
expect(project.repository).to be_kind_of(Repository)
end
end
diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb
index f5b39c3d69857663d00bc57e201d2eb76ad882a8..47bcc0eebddc663c8392c2a36a8cc0e9ee40cf20 100644
--- a/spec/requests/api/builds_spec.rb
+++ b/spec/requests/api/builds_spec.rb
@@ -1,21 +1,26 @@
require 'spec_helper'
-describe API::API, api: true do
+describe API::API, api: true do
include ApiHelpers
let(:user) { create(:user) }
let(:api_user) { user }
let(:user2) { create(:user) }
- let!(:project) { create(:project, creator_id: user.id) }
- let!(:developer) { create(:project_member, :developer, user: user, project: project) }
- let!(:reporter) { create(:project_member, :reporter, user: user2, project: project) }
- let!(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit.id) }
- let!(:build) { create(:ci_build, pipeline: pipeline) }
+ let(:project) { create(:project, creator_id: user.id) }
+ let(:developer) { create(:project_member, :developer, user: user, project: project) }
+ let(:reporter) { create(:project_member, :reporter, user: user2, project: project) }
+ let(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit.id) }
+ let(:build) { create(:ci_build, pipeline: pipeline) }
describe 'GET /projects/:id/builds ' do
let(:query) { '' }
- before { get api("/projects/#{project.id}/builds?#{query}", api_user) }
+ before do
+ developer
+ build
+
+ get api("/projects/#{project.id}/builds?#{query}", api_user)
+ end
context 'authorized user' do
it 'should return project builds' do
@@ -77,9 +82,9 @@ describe API::API, api: true do
context 'when user is authorized' do
context 'when pipeline has builds' do
before do
- create(:ci_pipeline, project: project, sha: project.commit.id)
+ developer
+ build
create(:ci_build, pipeline: pipeline)
- create(:ci_build)
get api("/projects/#{project.id}/repository/commits/#{project.commit.id}/builds", api_user)
end
@@ -93,6 +98,8 @@ describe API::API, api: true do
context 'when pipeline has no builds' do
before do
+ developer
+
branch_head = project.commit('feature').id
get api("/projects/#{project.id}/repository/commits/#{branch_head}/builds", api_user)
end
@@ -107,8 +114,7 @@ describe API::API, api: true do
context 'when user is not authorized' do
before do
- create(:ci_pipeline, project: project, sha: project.commit.id)
- create(:ci_build, pipeline: pipeline)
+ build
get api("/projects/#{project.id}/repository/commits/#{project.commit.id}/builds", nil)
end
@@ -122,7 +128,11 @@ describe API::API, api: true do
end
describe 'GET /projects/:id/builds/:build_id' do
- before { get api("/projects/#{project.id}/builds/#{build.id}", api_user) }
+ before do
+ developer
+
+ get api("/projects/#{project.id}/builds/#{build.id}", api_user)
+ end
context 'authorized user' do
it 'should return specific build data' do
@@ -141,7 +151,11 @@ describe API::API, api: true do
end
describe 'GET /projects/:id/builds/:build_id/artifacts' do
- before { get api("/projects/#{project.id}/builds/#{build.id}/artifacts", api_user) }
+ before do
+ developer
+
+ get api("/projects/#{project.id}/builds/#{build.id}/artifacts", api_user)
+ end
context 'build with artifacts' do
let(:build) { create(:ci_build, :artifacts, pipeline: pipeline) }
@@ -172,10 +186,146 @@ describe API::API, api: true do
end
end
+ describe 'GET /projects/:id/artifacts/:ref_name/download?job=name' do
+ let(:user) { create(:user) }
+ let(:project) { create(:project) }
+ let(:pipeline) do
+ create(:ci_pipeline,
+ project: project,
+ sha: project.commit('fix').sha,
+ ref: 'fix')
+ end
+ let(:build) { create(:ci_build, :success, :artifacts, pipeline: pipeline) }
+
+ before do
+ project.team << [user, :developer]
+ end
+
+ def path_from_ref(ref = pipeline.ref, job = build.name)
+ api("/projects/#{project.id}/builds/artifacts/#{ref}/download?job=#{job}", user)
+ end
+
+ context 'not logging in' do
+ let(:user) { nil }
+
+ before do
+ get path_from_ref
+ end
+
+ it 'gives 401 for unauthorized user' do
+ expect(response).to have_http_status(401)
+ end
+ end
+
+ context 'non-existing build' do
+ def verify
+ expect(response).to have_http_status(404)
+ end
+
+ context 'has no such ref' do
+ before do
+ get path_from_ref('TAIL', build.name)
+ end
+
+ it('gives 404') { verify }
+ end
+
+ context 'has no such build' do
+ before do
+ get path_from_ref(pipeline.ref, 'NOBUILD')
+ end
+
+ it('gives 404') { verify }
+ end
+ end
+
+ context 'find proper build' do
+ def verify
+ download_headers =
+ { 'Content-Transfer-Encoding' => 'binary',
+ 'Content-Disposition' =>
+ "attachment; filename=#{build.artifacts_file.filename}" }
+
+ expect(response).to have_http_status(200)
+ expect(response.headers).to include(download_headers)
+ end
+
+ def create_new_pipeline(status)
+ new_pipeline = create(:ci_pipeline, status: 'success')
+ create(:ci_build, status, :artifacts, pipeline: new_pipeline)
+ end
+
+ context 'with sha' do
+ before do
+ get path_from_ref(pipeline.sha)
+ end
+
+ it('gives the file') { verify }
+ end
+
+ context 'with regular branch' do
+ before do
+ pipeline.update(ref: 'master',
+ sha: project.commit('master').sha)
+ end
+
+ before do
+ get path_from_ref('master')
+ end
+
+ it('gives the file') { verify }
+ end
+
+ context 'with branch name containing slash' do
+ before do
+ pipeline.update(ref: 'improve/awesome',
+ sha: project.commit('improve/awesome').sha)
+ end
+
+ before do
+ get path_from_ref('improve/awesome')
+ end
+
+ it('gives the file') { verify }
+ end
+
+ context 'with latest pipeline' do
+ before do
+ 3.times do # creating some old pipelines
+ create_new_pipeline(:success)
+ end
+ end
+
+ before do
+ get path_from_ref
+ end
+
+ it('gives the file') { verify }
+ end
+
+ context 'with success pipeline' do
+ before do
+ build # make sure pipeline was old, but still the latest success one
+ create_new_pipeline(:pending)
+ end
+
+ before do
+ get path_from_ref
+ end
+
+ it('gives the file') { verify }
+ end
+ end
+ end
+
describe 'GET /projects/:id/builds/:build_id/trace' do
let(:build) { create(:ci_build, :trace, pipeline: pipeline) }
- before { get api("/projects/#{project.id}/builds/#{build.id}/trace", api_user) }
+ before do
+ developer
+
+ get api("/projects/#{project.id}/builds/#{build.id}/trace", api_user)
+ end
context 'authorized user' do
it 'should return specific build trace' do
@@ -194,7 +344,12 @@ describe API::API, api: true do
end
describe 'POST /projects/:id/builds/:build_id/cancel' do
- before { post api("/projects/#{project.id}/builds/#{build.id}/cancel", api_user) }
+ before do
+ developer
+ reporter
+
+ post api("/projects/#{project.id}/builds/#{build.id}/cancel", api_user)
+ end
context 'authorized user' do
context 'user with :update_build persmission' do
@@ -225,7 +380,12 @@ describe API::API, api: true do
describe 'POST /projects/:id/builds/:build_id/retry' do
let(:build) { create(:ci_build, :canceled, pipeline: pipeline) }
- before { post api("/projects/#{project.id}/builds/#{build.id}/retry", api_user) }
+ before do
+ developer
+ reporter
+
+ post api("/projects/#{project.id}/builds/#{build.id}/retry", api_user)
+ end
context 'authorized user' do
context 'user with :update_build permission' do
@@ -256,6 +416,8 @@ describe API::API, api: true do
describe 'POST /projects/:id/builds/:build_id/erase' do
before do
+ developer
+
post api("/projects/#{project.id}/builds/#{build.id}/erase", user)
end
@@ -286,6 +448,8 @@ describe API::API, api: true do
describe 'POST /projects/:id/builds/:build_id/artifacts/keep' do
before do
+ developer
+
post api("/projects/#{project.id}/builds/#{build.id}/artifacts/keep", user)
end