From 79fe18d9e7290fb880f1feb5f2c9f3f96b2d74fe Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 17 Dec 2015 14:24:43 +0100
Subject: [PATCH] Move build artifacts implementation to separate controller

---
 .../projects/builds/artifacts_controller.rb   | 36 +++++++++++++++++++
 app/controllers/projects/builds_controller.rb | 27 --------------
 app/models/ci/build.rb                        |  2 +-
 config/routes.rb                              | 10 +++++-
 4 files changed, 46 insertions(+), 29 deletions(-)
 create mode 100644 app/controllers/projects/builds/artifacts_controller.rb

diff --git a/app/controllers/projects/builds/artifacts_controller.rb b/app/controllers/projects/builds/artifacts_controller.rb
new file mode 100644
index 00000000000..fd2195d2460
--- /dev/null
+++ b/app/controllers/projects/builds/artifacts_controller.rb
@@ -0,0 +1,36 @@
+class Projects::Builds::ArtifactsController < Projects::ApplicationController
+  layout 'project'
+  before_action :authorize_download_build_artifacts!
+
+  def download
+    unless artifacts_file.file_storage?
+      return redirect_to artifacts_file.url
+    end
+
+    unless artifacts_file.exists?
+      return not_found!
+    end
+
+    send_file artifacts_file.path, disposition: 'attachment'
+  end
+
+  private
+
+  def build
+    @build ||= project.builds.unscoped.find_by!(id: params[:build_id])
+  end
+
+  def artifacts_file
+    @artifacts_file ||= build.artifacts_file
+  end
+
+  def authorize_download_build_artifacts!
+    unless can?(current_user, :download_build_artifacts, @project)
+      if current_user.nil?
+        return authenticate_user!
+      else
+        return render_404
+      end
+    end
+  end
+end
diff --git a/app/controllers/projects/builds_controller.rb b/app/controllers/projects/builds_controller.rb
index 39d3ba26ba2..0e965966ffa 100644
--- a/app/controllers/projects/builds_controller.rb
+++ b/app/controllers/projects/builds_controller.rb
@@ -2,7 +2,6 @@ class Projects::BuildsController < Projects::ApplicationController
   before_action :build, except: [:index, :cancel_all]
 
   before_action :authorize_manage_builds!, except: [:index, :show, :status]
-  before_action :authorize_download_build_artifacts!, only: [:download]
 
   layout "project"
 
@@ -51,18 +50,6 @@ class Projects::BuildsController < Projects::ApplicationController
     redirect_to build_path(build)
   end
 
-  def download
-    unless artifacts_file.file_storage?
-      return redirect_to artifacts_file.url
-    end
-
-    unless artifacts_file.exists?
-      return not_found!
-    end
-
-    send_file artifacts_file.path, disposition: 'attachment'
-  end
-
   def status
     render json: @build.to_json(only: [:status, :id, :sha, :coverage], methods: :sha)
   end
@@ -79,10 +66,6 @@ class Projects::BuildsController < Projects::ApplicationController
     @build ||= project.builds.unscoped.find_by!(id: params[:id])
   end
 
-  def artifacts_file
-    build.artifacts_file
-  end
-
   def build_path(build)
     namespace_project_build_path(build.project.namespace, build.project, build)
   end
@@ -92,14 +75,4 @@ class Projects::BuildsController < Projects::ApplicationController
       return page_404
     end
   end
-
-  def authorize_download_build_artifacts!
-    unless can?(current_user, :download_build_artifacts, @project)
-      if current_user.nil?
-        return authenticate_user!
-      else
-        return render_404
-      end
-    end
-  end
 end
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index a2dca97ad31..b928416aa11 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -326,7 +326,7 @@ module Ci
     def download_url
       if artifacts_file.exists?
         Gitlab::Application.routes.url_helpers.
-          download_namespace_project_build_path(project.namespace, project, self)
+          download_namespace_project_build_artifacts_path(project.namespace, project, self)
       end
     end
 
diff --git a/config/routes.rb b/config/routes.rb
index 05d6ff1e884..f639f89189d 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -603,10 +603,18 @@ Rails.application.routes.draw do
 
           member do
             get :status
-            post :cancel
             get :download
+            post :cancel
             post :retry
           end
+
+          scope module: :builds do
+            resources :artifacts do
+              collection do
+                get :download
+              end
+            end
+          end
         end
 
         resources :hooks, only: [:index, :create, :destroy], constraints: { id: /\d+/ } do