diff options
author | Rémy Coutable <remy@rymai.me> | 2016-07-21 12:01:47 +0300 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-07-21 12:01:47 +0300 |
commit | 4a2a127b5c4e811f24d159469fb2bd491ed3a3e7 (patch) | |
tree | 0c06a339b5af1dfe6f7fc42b8c30708e6c0c5f0d /lib | |
parent | 8ed97054835a28ead703c32d9c2ab79a15412c88 (diff) | |
parent | e5b05fe8c0e773d850c314b8317e5fa2eb75379d (diff) |
Merge branch 'artifacts-from-ref-and-build-name-api' into 'master'
API for downloading latest successful build
## What does this MR do?
Implement parts of #4255, particularly the API.
## Are there points in the code the reviewer needs to double check?
I still made it that `ref` could be either branch, tag, or even SHA with:
``` ruby
# 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
```
Because the reasons I put in:
* https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5142#note_13165543
* https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5142#note_13165921
But if you still think that it's not good to do it this way, I'll drop it and let's think about the other way to satisfy the requirement specified in https://gitlab.com/gitlab-org/gitlab-ce/issues/4255#note_13101233 It could be `status=any` or `sha=DEADBEAF`
## What are the relevant issue numbers?
Part of #4255
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [x] API support added
- Tests
- [x] Added for this feature/bug
- [ ] All builds are passing
- [ ] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [ ] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
See merge request !5347
Diffstat (limited to 'lib')
-rw-r--r-- | lib/api/builds.rb | 58 |
1 files changed, 38 insertions, 20 deletions
diff --git a/lib/api/builds.rb b/lib/api/builds.rb index d36047acd1f..657d421fe97 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,27 @@ 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 + present_artifacts!(build.artifacts_file) + end - unless artifacts_file.file_storage? - return redirect_to build.artifacts_file.url - end + # 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 + authorize_read_builds! - return not_found! unless artifacts_file.exists? + 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_artifacts!(latest_build.artifacts_file) end # Get a trace of a specific build of a project @@ -97,8 +105,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 +125,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 +143,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 +162,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 +180,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 +196,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_artifacts!(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? |