diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 78ed12db516..bd4ba384b29 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -117,10 +117,6 @@ class ApplicationController < ActionController::Base
render file: Rails.root.join("public", "404"), layout: false, status: "404"
end
- def render_410
- render file: Rails.root.join("public", "410"), layout: false, status: "410"
- end
-
def no_cache_headers
response.headers["Cache-Control"] = "no-cache, no-store, max-age=0, must-revalidate"
response.headers["Pragma"] = "no-cache"
diff --git a/app/controllers/projects/builds_controller.rb b/app/controllers/projects/builds_controller.rb
index 79d774195f8..77934ff9962 100644
--- a/app/controllers/projects/builds_controller.rb
+++ b/app/controllers/projects/builds_controller.rb
@@ -79,9 +79,9 @@ class Projects::BuildsController < Projects::ApplicationController
def raw
if @build.has_trace_file?
- send_file @build.path_to_trace, type: 'text/plain; charset=utf-8', disposition: 'inline'
+ send_file @build.trace_file_path, type: 'text/plain; charset=utf-8', disposition: 'inline'
else
- render_410
+ render_404
end
end
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 8a8f848d328..f219cee4a62 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -217,7 +217,7 @@ module Ci
end
def raw_trace
- if File.file?(path_to_trace)
+ if File.exist?(path_to_trace)
File.read(path_to_trace)
elsif has_old_trace_file?
# Temporary fix for build trace data integrity
@@ -274,6 +274,14 @@ module Ci
end
end
+ def trace_file_path
+ if has_old_trace_file?
+ old_path_to_trace
+ else
+ path_to_trace
+ end
+ end
+
def dir_to_trace
File.join(
Settings.gitlab_ci.builds_path,
diff --git a/public/410.html b/public/410.html
deleted file mode 100644
index bba436b7b7d..00000000000
--- a/public/410.html
+++ /dev/null
@@ -1,65 +0,0 @@
-
-
-
-
- The page you're looking for is gone (410)
-
-
-
-
-
-
- 410
-
-
-
The page you're looking for is gone.
-
-
Make sure the address is correct and that the page hasn't moved.
-
Please contact your GitLab administrator if you think this is a mistake.
-
-
-
diff --git a/spec/features/projects/builds_spec.rb b/spec/features/projects/builds_spec.rb
index 0cfeb2e57d8..9030ce9dc67 100644
--- a/spec/features/projects/builds_spec.rb
+++ b/spec/features/projects/builds_spec.rb
@@ -1,4 +1,5 @@
require 'spec_helper'
+require 'tempfile'
describe "Builds" do
let(:artifacts_file) { fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') }
@@ -6,7 +7,7 @@ describe "Builds" do
before do
login_as(:user)
@commit = FactoryGirl.create :ci_pipeline
- @build = FactoryGirl.create :ci_build, pipeline: @commit
+ @build = FactoryGirl.create :ci_build, :trace, pipeline: @commit
@build2 = FactoryGirl.create :ci_build
@project = @commit.project
@project.team << [@user, :developer]
@@ -156,7 +157,6 @@ describe "Builds" do
context 'Build raw trace' do
before do
@build.run!
- @build.trace = 'BUILD TRACE'
visit namespace_project_build_path(@project.namespace, @project, @build)
end
@@ -255,35 +255,101 @@ describe "Builds" do
end
end
- describe "GET /:project/builds/:id/raw" do
- context "Build from project" do
- before do
- Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile')
- @build.run!
- @build.trace = 'BUILD TRACE'
- visit namespace_project_build_path(@project.namespace, @project, @build)
- page.within('.js-build-sidebar') { click_link 'Raw' }
+ describe 'GET /:project/builds/:id/raw' do
+ context 'access source' do
+ context 'build from project' do
+ before do
+ Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile')
+ @build.run!
+ visit namespace_project_build_path(@project.namespace, @project, @build)
+ page.within('.js-build-sidebar') { click_link 'Raw' }
+ end
+
+ it 'sends the right headers' do
+ expect(page.status_code).to eq(200)
+ expect(page.response_headers['Content-Type']).to eq('text/plain; charset=utf-8')
+ expect(page.response_headers['X-Sendfile']).to eq(@build.path_to_trace)
+ end
end
- it 'sends the right headers' do
- expect(page.status_code).to eq(200)
- expect(page.response_headers['Content-Type']).to eq('text/plain; charset=utf-8')
- expect(page.response_headers['X-Sendfile']).to eq(@build.path_to_trace)
+ context 'build from other project' do
+ before do
+ Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile')
+ @build2.run!
+ visit raw_namespace_project_build_path(@project.namespace, @project, @build2)
+ end
+
+ it 'sends the right headers' do
+ expect(page.status_code).to eq(404)
+ end
end
end
- context "Build from other project" do
- before do
- Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile')
- @build2.run!
- @build2.trace = 'BUILD TRACE'
- visit raw_namespace_project_build_path(@project.namespace, @project, @build2)
- puts page.status_code
- puts current_url
+ context 'storage form' do
+ let (:existing_file) { Tempfile.new('existing-trace-file').path }
+ let (:non_existing_file) do
+ file = Tempfile.new('non-existing-trace-file')
+ path = file.path
+ file.unlink
+ path
end
- it 'sends the right headers' do
- expect(page.status_code).to eq(404)
+ context 'when build has trace in file' do
+ before do
+ Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile')
+ @build.run!
+ visit namespace_project_build_path(@project.namespace, @project, @build)
+
+ allow_any_instance_of(Project).to receive(:ci_id).and_return(nil)
+ allow_any_instance_of(Ci::Build).to receive(:path_to_trace).and_return(existing_file)
+ allow_any_instance_of(Ci::Build).to receive(:old_path_to_trace).and_return(non_existing_file)
+
+ page.within('.js-build-sidebar') { click_link 'Raw' }
+ end
+
+ it 'sends the right headers' do
+ expect(page.status_code).to eq(200)
+ expect(page.response_headers['Content-Type']).to eq('text/plain; charset=utf-8')
+ expect(page.response_headers['X-Sendfile']).to eq(existing_file)
+ end
+ end
+
+ context 'when build has trace in old file' do
+ before do
+ Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile')
+ @build.run!
+ visit namespace_project_build_path(@project.namespace, @project, @build)
+
+ allow_any_instance_of(Project).to receive(:ci_id).and_return(999)
+ allow_any_instance_of(Ci::Build).to receive(:path_to_trace).and_return(non_existing_file)
+ allow_any_instance_of(Ci::Build).to receive(:old_path_to_trace).and_return(existing_file)
+
+ page.within('.js-build-sidebar') { click_link 'Raw' }
+ end
+
+ it 'sends the right headers' do
+ expect(page.status_code).to eq(200)
+ expect(page.response_headers['Content-Type']).to eq('text/plain; charset=utf-8')
+ expect(page.response_headers['X-Sendfile']).to eq(existing_file)
+ end
+ end
+
+ context 'when build has trace in DB' do
+ before do
+ Capybara.current_session.driver.header('X-Sendfile-Type', 'X-Sendfile')
+ @build.run!
+ visit namespace_project_build_path(@project.namespace, @project, @build)
+
+ allow_any_instance_of(Project).to receive(:ci_id).and_return(nil)
+ allow_any_instance_of(Ci::Build).to receive(:path_to_trace).and_return(non_existing_file)
+ allow_any_instance_of(Ci::Build).to receive(:old_path_to_trace).and_return(non_existing_file)
+
+ page.within('.js-build-sidebar') { click_link 'Raw' }
+ end
+
+ it 'sends the right headers' do
+ expect(page.status_code).to eq(404)
+ end
end
end
end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 1dd26750edd..bce18b4e99e 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -28,41 +28,30 @@ describe Ci::Build, models: true do
context 'when there is a trace' do
context 'when trace is stored in file' do
- before do
- build.trace = test_trace
- build.save
- end
+ let(:build_with_trace) { create(:ci_build, :trace) }
- it { expect(build.has_trace_file?).to be_truthy }
- it { expect(build.trace).to eq(test_trace) }
+ it { expect(build_with_trace.has_trace_file?).to be_truthy }
+ it { expect(build_with_trace.trace).to eq('BUILD TRACE') }
end
context 'when trace is stored in old file' do
before do
- build.trace = test_trace
- build.save
-
- build.project.ci_id = 999
- build.project.save
-
- FileUtils.mkdir_p(build.old_dir_to_trace)
- FileUtils.mv(build.path_to_trace, build.old_path_to_trace)
+ allow(build.project).to receive(:ci_id).and_return(999)
+ allow(File).to receive(:exist?).with(build.path_to_trace).and_return(false)
+ allow(File).to receive(:exist?).with(build.old_path_to_trace).and_return(true)
+ allow(File).to receive(:read).with(build.old_path_to_trace).and_return(test_trace)
end
it { expect(build.has_trace_file?).to be_truthy }
it { expect(build.trace).to eq(test_trace) }
end
- context 'when there is stored in DB' do
- class Ci::Build
- def write_db_trace=(trace)
- write_attribute :trace, trace
- end
- end
-
+ context 'when trace is stored in DB' do
before do
- build.write_db_trace = test_trace
- build.save
+ allow(build.project).to receive(:ci_id).and_return(nil)
+ allow(build).to receive(:read_attribute).with(:trace).and_return(test_trace)
+ allow(File).to receive(:exist?).with(build.path_to_trace).and_return(false)
+ allow(File).to receive(:exist?).with(build.old_path_to_trace).and_return(false)
end
it { expect(build.has_trace_file?).to be_falsey }
@@ -70,4 +59,24 @@ describe Ci::Build, models: true do
end
end
end
+
+ describe '#trace_file_path' do
+ context 'when trace is stored in file' do
+ before do
+ allow(build).to receive(:has_trace_file?).and_return(true)
+ allow(build).to receive(:has_old_trace_file?).and_return(false)
+ end
+
+ it { expect(build.trace_file_path).to eq(build.path_to_trace) }
+ end
+
+ context 'when trace is stored in old file' do
+ before do
+ allow(build).to receive(:has_trace_file?).and_return(true)
+ allow(build).to receive(:has_old_trace_file?).and_return(true)
+ end
+
+ it { expect(build.trace_file_path).to eq(build.old_path_to_trace) }
+ end
+ end
end