refactor Projects::JobsController

app/controllers/projects/jobs_controller.rb

 class Projects::JobsController < Projects::ApplicationController
-  prepend EE::Projects::JobsController
+  include SendFileUpload
 
   before_action :build, except: [:index, :cancel_all]
 
   before_action :authorize_read_build!,
-    only: [:index, :show, :status, :raw, :trace]
+                only: [:index, :show, :status, :raw, :trace]
   before_action :authorize_update_build!,
-    except: [:index, :show, :status, :raw, :trace, :cancel_all, :erase]
+                except: [:index, :show, :status, :raw, :trace, :cancel_all, :erase]
   before_action :authorize_erase_build!, only: [:erase]
 
   layout 'project'
@@ -27,10 +27,10 @@ class Projects::JobsController < Projects::ApplicationController
         @builds
       end
     @builds = @builds.includes([
-      { pipeline: :project },
-      :project,
-      :tags
-    ])
+                                 { pipeline: :project },
+                                 :project,
+                                 :tags
+                               ])
     @builds = @builds.page(params[:page]).per(30).without_count
   end
 
@@ -55,8 +55,8 @@ class Projects::JobsController < Projects::ApplicationController
         Gitlab::PollingInterval.set_header(response, interval: 10_000)
 
         render json: BuildSerializer
-          .new(project: @project, current_user: @current_user)
-          .represent(@build, {}, BuildDetailsEntity)
+                 .new(project: @project, current_user: @current_user)
+                 .represent(@build, {}, BuildDetailsEntity)
       end
     end
   end
@@ -105,25 +105,31 @@ class Projects::JobsController < Projects::ApplicationController
 
   def status
     render json: BuildSerializer
-      .new(project: @project, current_user: @current_user)
-      .represent_status(@build)
+             .new(project: @project, current_user: @current_user)
+             .represent_status(@build)
   end
 
   def erase
     if @build.erase(erased_by: current_user)
       redirect_to project_job_path(project, @build),
-                notice: "Job has been successfully erased!"
+                  notice: "Job has been successfully erased!"
     else
       respond_422
     end
   end
 
   def raw
-    build.trace.read do |stream|
-      if stream.file?
-        send_file stream.path, type: 'text/plain; charset=utf-8', disposition: 'inline'
-      else
-        render_404
+    if trace_artifact_file
+      send_upload(trace_artifact_file,
+                  send_params: raw_send_params,
+                  redirect_params: raw_redirect_params)
+    else
+      build.trace.read do |stream|
+        if stream.file?
+          send_file stream.path, type: 'text/plain; charset=utf-8', disposition: 'inline'
+        else
+          render_404
+        end
       end
     end
   end
@@ -138,6 +144,18 @@ class Projects::JobsController < Projects::ApplicationController
     return access_denied! unless can?(current_user, :erase_build, build)
   end
 
+  def raw_send_params
+    { type: 'text/plain; charset=utf-8', disposition: 'inline' }
+  end
+
+  def raw_redirect_params
+    { query: { 'response-content-type' => 'text/plain; charset=utf-8', 'response-content-disposition' => 'inline' } }
+  end
+
+  def trace_artifact_file
+    @trace_artifact_file ||= build.job_artifacts_trace&.file
+  end
+
   def build
     @build ||= project.builds.find(params[:id])
       .present(current_user: current_user)

ee/app/controllers/ee/projects/jobs_controller.rb

-module EE
-  module Projects
-    module JobsController
-      extend ActiveSupport::Concern
-      include SendFileUpload
-
-      def raw
-        if trace_artifact_file
-          send_upload(trace_artifact_file,
-                      send_params: raw_send_params,
-                      redirect_params: raw_redirect_params)
-        else
-          super
-        end
-      end
-
-      private
-
-      def raw_send_params
-        { type: 'text/plain; charset=utf-8', disposition: 'inline' }
-      end
-
-      def raw_redirect_params
-        { query: { 'response-content-type' => 'text/plain; charset=utf-8', 'response-content-disposition' => 'inline' } }
-      end
-
-      def trace_artifact_file
-        @trace_artifact_file ||= build.job_artifacts_trace&.file
-      end
-    end
-  end
-end

ee/spec/controllers/projects/jobs_controller_spec.rb

-require 'spec_helper'
-
-describe Projects::JobsController do
-  include ApiHelpers
-  include HttpIOHelpers
-
-  let(:project) { create(:project, :public) }
-  let(:pipeline) { create(:ci_pipeline, project: project) }
-
-  describe 'GET trace.json' do
-    context 'when trace artifact is in ObjectStorage' do
-      let!(:job) { create(:ci_build, :success, :trace_artifact, pipeline: pipeline) }
-
-      before do
-        allow_any_instance_of(JobArtifactUploader).to receive(:file_storage?) { false }
-        allow_any_instance_of(JobArtifactUploader).to receive(:url) { remote_trace_url }
-        allow_any_instance_of(JobArtifactUploader).to receive(:size) { remote_trace_size }
-      end
-
-      context 'when there are no network issues' do
-        before do
-          stub_remote_trace_206
-
-          get_trace
-        end
-
-        it 'returns a trace' do
-          expect(response).to have_gitlab_http_status(:ok)
-          expect(json_response['id']).to eq job.id
-          expect(json_response['status']).to eq job.status
-          expect(json_response['html']).to eq(job.trace.html)
-        end
-      end
-
-      context 'when there is a network issue' do
-        before do
-          stub_remote_trace_500
-        end
-
-        it 'returns a trace' do
-          expect { get_trace }.to raise_error(Gitlab::Ci::Trace::HttpIO::FailedToGetChunkError)
-        end
-      end
-    end
-
-    def get_trace
-      get :trace, namespace_id: project.namespace,
-                  project_id: project,
-                  id: job.id,
-                  format: :json
-    end
-  end
-
-  describe 'GET raw' do
-    subject do
-      post :raw, namespace_id: project.namespace,
-                 project_id: project,
-                 id: job.id
-    end
-
-    context 'when the trace artifact is in ObjectStorage' do
-      let!(:job) { create(:ci_build, :trace_artifact, pipeline: pipeline) }
-
-      before do
-        allow_any_instance_of(JobArtifactUploader).to receive(:file_storage?) { false }
-      end
-
-      it 'redirect to the trace file url' do
-        expect(subject).to redirect_to(job.job_artifacts_trace.file.url)
-      end
-    end
-  end
-end

spec/controllers/projects/jobs_controller_spec.rb

+# coding: utf-8
 require 'spec_helper'
 
 describe Projects::JobsController do
   include ApiHelpers
+  include HttpIOHelpers
 
   let(:project) { create(:project, :public) }
   let(:pipeline) { create(:ci_pipeline, project: project) }
@@ -203,6 +205,41 @@ describe Projects::JobsController do
       end
     end
 
+    context 'when trace artifact is in ObjectStorage' do
+      let!(:job) { create(:ci_build, :success, :trace_artifact, pipeline: pipeline) }
+
+      before do
+        allow_any_instance_of(JobArtifactUploader).to receive(:file_storage?) { false }
+        allow_any_instance_of(JobArtifactUploader).to receive(:url) { remote_trace_url }
+        allow_any_instance_of(JobArtifactUploader).to receive(:size) { remote_trace_size }
+      end
+
+      context 'when there are no network issues' do
+        before do
+          stub_remote_trace_206
+
+          get_trace
+        end
+
+        it 'returns a trace' do
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['id']).to eq job.id
+          expect(json_response['status']).to eq job.status
+          expect(json_response['html']).to eq(job.trace.html)
+        end
+      end
+
+      context 'when there is a network issue' do
+        before do
+          stub_remote_trace_500
+        end
+
+        it 'returns a trace' do
+          expect { get_trace }.to raise_error(Gitlab::Ci::Trace::HttpIO::FailedToGetChunkError)
+        end
+      end
+    end
+
     def get_trace
       get :trace, namespace_id: project.namespace,
                   project_id: project,
@@ -483,5 +520,17 @@ describe Projects::JobsController do
                  project_id: project,
                  id: job.id
     end
+
+    context 'when the trace artifact is in ObjectStorage' do
+      let!(:job) { create(:ci_build, :trace_artifact, pipeline: pipeline) }
+
+      before do
+        allow_any_instance_of(JobArtifactUploader).to receive(:file_storage?) { false }
+      end
+
+      it 'redirect to the trace file url' do
+        expect(subject).to redirect_to(job.job_artifacts_trace.file.url)
+      end
+    end
   end
 end