From 13f92521c22f63140adbf933737e0c5dc988a8d4 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzegorz.bizon@ntsn.pl>
Date: Wed, 3 Feb 2016 14:12:33 +0100
Subject: [PATCH] Move build eraseable API to proper API context

---
 lib/api/builds.rb                   | 22 +++++++++++++++++++++-
 lib/ci/api/builds.rb                | 19 +------------------
 spec/requests/api/builds_spec.rb    | 25 +++++++++++++++++++++++++
 spec/requests/ci/api/builds_spec.rb | 12 ------------
 4 files changed, 47 insertions(+), 31 deletions(-)

diff --git a/lib/api/builds.rb b/lib/api/builds.rb
index a8bd3842ce4..06ec03fc4e0 100644
--- a/lib/api/builds.rb
+++ b/lib/api/builds.rb
@@ -115,13 +115,33 @@ module API
         authorize_update_builds!
 
         build = get_build(params[:build_id])
-        return forbidden!('Build is not retryable') unless build && build.retryable?
+        return not_found!(build) unless build
+        return forbidden!('Build is not retryable') unless build.retryable?
 
         build = Ci::Build.retry(build)
 
         present build, with: Entities::Build,
                        user_can_download_artifacts: can?(current_user, :read_build, user_project)
       end
+
+      # Erase build (remove artifacts and build trace)
+      #
+      # Parameters:
+      #   id (required) - the id of a project
+      #   build_id (required) - the id of a build
+      # example Request:
+      #  delete  /projects/:id/build/:build_id/content
+      delete ':id/builds/:build_id/content' do
+        authorize_manage_builds!
+
+        build = get_build(params[:build_id])
+        return not_found!(build) unless build
+        return forbidden!('Build is not eraseable!') unless build.eraseable?
+
+        build.erase!
+        present build, with: Entities::Build,
+                       user_can_download_artifacts: can?(current_user, :download_build_artifacts, user_project)
+      end
     end
 
     helpers do
diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb
index f37a96da93c..2e9a5d311f9 100644
--- a/lib/ci/api/builds.rb
+++ b/lib/ci/api/builds.rb
@@ -146,7 +146,7 @@ module Ci
           present_file!(artifacts_file.path, artifacts_file.filename)
         end
 
-        # Remove the artifacts file from build
+        # Remove the artifacts file from build - Runners only
         #
         # Parameters:
         #   id (required) - The ID of a build
@@ -163,23 +163,6 @@ module Ci
           build.remove_artifacts_file!
           build.remove_artifacts_metadata!
         end
-
-        # Erase build (remove artifacts and build trace)
-        #
-        # Parameters:
-        #   id (required) - The ID of a build
-        #   token (required) - The build authorization token
-        # Headers:
-        #   BUILD-TOKEN (required) - The build authorization token, the same as token
-        # Example Request:
-        #   DELETE /builds/:id/content
-        delete ':id/content' do
-          build = Ci::Build.find_by_id(params[:id])
-          not_found! unless build
-          authenticate_build_token!(build)
-
-          build.erase!
-        end
       end
     end
   end
diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb
index 6c07802db8b..56ab303268f 100644
--- a/spec/requests/api/builds_spec.rb
+++ b/spec/requests/api/builds_spec.rb
@@ -169,4 +169,29 @@ describe API::API, api: true  do
       end
     end
   end
+
+  describe 'DELETE /projects/:id/builds/:build_id/content' do
+    before do
+      delete api("/projects/#{project.id}/builds/#{build.id}/content", user)
+    end
+
+    context 'build is eraseable' do
+      let(:build) { create(:ci_build_with_trace, :artifacts, :success, project: project, commit: commit) }
+
+      it 'should erase build content' do
+        expect(response.status).to eq 200
+        expect(build.trace).to be_empty
+        expect(build.artifacts_file.exists?).to be_falsy
+        expect(build.artifacts_metadata.exists?).to be_falsy
+      end
+    end
+
+    context 'build is not eraseable' do
+      let(:build) { create(:ci_build_with_trace, project: project, commit: commit) }
+
+      it 'should respond with forbidden' do
+        expect(response.status).to eq 403
+      end
+    end
+  end
 end
diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb
index 4246ff38fa2..8eb9881649c 100644
--- a/spec/requests/ci/api/builds_spec.rb
+++ b/spec/requests/ci/api/builds_spec.rb
@@ -156,18 +156,6 @@ describe Ci::API::API do
       end
     end
 
-    describe 'DELETE /builds/:id/content' do
-      let(:build) { create(:ci_build_with_trace, :artifacts, :success) }
-      before { delete ci_api("/builds/#{build.id}/content"), token: build.token }
-
-      it 'should erase build content' do
-        expect(response.status).to eq 200
-        expect(build.trace).to be_empty
-        expect(build.artifacts_file.exists?).to be_falsy
-        expect(build.artifacts_metadata.exists?).to be_falsy
-      end
-    end
-
     context "Artifacts" do
       let(:file_upload) { fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') }
       let(:file_upload2) { fixture_file_upload(Rails.root + 'spec/fixtures/dk.png', 'image/gif') }
-- 
GitLab