diff options
author | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2017-10-03 12:34:24 +0200 |
---|---|---|
committer | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2017-10-03 12:34:24 +0200 |
commit | 8cbfe3aea64a11f8de0e28e35fc1fc298128158e (patch) | |
tree | a12867e59b7ae7584f3a67a406f832bd7cab7ee8 | |
parent | ca0e39048f0e92bd615c45bfe555800910396cfd (diff) | |
download | gitlab-ce-8cbfe3aea64a11f8de0e28e35fc1fc298128158e.tar.gz |
Redirect to pages daemon
-rw-r--r-- | app/controllers/projects/artifacts_controller.rb | 18 | ||||
-rw-r--r-- | app/helpers/artifact_helper.rb | 31 | ||||
-rw-r--r-- | app/models/ci/artifact_blob.rb | 25 | ||||
-rw-r--r-- | app/models/concerns/routable.rb | 4 | ||||
-rw-r--r-- | app/views/projects/artifacts/_tree_file.html.haml | 4 | ||||
-rw-r--r-- | config/initializers/1_settings.rb | 19 | ||||
-rw-r--r-- | spec/controllers/projects/artifacts_controller_spec.rb | 13 | ||||
-rw-r--r-- | spec/fixtures/ci_build_artifacts.zip | bin | 106633 -> 106365 bytes | |||
-rw-r--r-- | spec/models/ci/artifact_blob_spec.rb | 43 |
9 files changed, 102 insertions, 55 deletions
diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb index eb010923466..a8f56acc73b 100644 --- a/app/controllers/projects/artifacts_controller.rb +++ b/app/controllers/projects/artifacts_controller.rb @@ -29,13 +29,17 @@ class Projects::ArtifactsController < Projects::ApplicationController blob = @entry.blob conditionally_expand_blob(blob) - respond_to do |format| - format.html do - render 'file' - end - - format.json do - render_blob_json(blob) + if blob.external_link? + redirect_to blob.external_url(@project, build) + else + respond_to do |format| + format.html do + render 'file' + end + + format.json do + render_blob_json(blob) + end end end end diff --git a/app/helpers/artifact_helper.rb b/app/helpers/artifact_helper.rb deleted file mode 100644 index 04c336c7e5d..00000000000 --- a/app/helpers/artifact_helper.rb +++ /dev/null @@ -1,31 +0,0 @@ -module ArtifactHelper - include GitlabRoutingHelper - - def link_to_artifact(project, job, file) - if external_url?(file.blob) - html_artifact_url(project, job, file.blob) - else - file_project_job_artifacts_path(project, job, path: file.path) - end - end - - def external_url?(blob) - blob.name.end_with?(".html") && - pages_config.enabled && - pages_config.artifacts_server - end - - private - - def html_artifact_url(project, job, blob) - http = pages_config.https ? "https://" : "http://" - domain = "#{project.namespace.to_param}.#{pages_config.host}/" - path = "-/jobs/#{job.id}/artifacts/#{blob.path}" - - http + domain + path - end - - def pages_config - Gitlab.config.pages - end -end diff --git a/app/models/ci/artifact_blob.rb b/app/models/ci/artifact_blob.rb index b35febc9ac5..e03d11284c0 100644 --- a/app/models/ci/artifact_blob.rb +++ b/app/models/ci/artifact_blob.rb @@ -2,6 +2,8 @@ module Ci class ArtifactBlob include BlobLike + EXTENTIONS_SERVED_BY_PAGES = %w[.html .htm .txt].freeze + attr_reader :entry def initialize(entry) @@ -17,6 +19,7 @@ module Ci def size entry.metadata[:size] end + alias_method :external_size, :size def data "Build artifact #{path}" @@ -30,6 +33,26 @@ module Ci :build_artifact end - alias_method :external_size, :size + def external_url(project, job) + return unless external_link? + + components = project.full_path_components + components << "-/jobs/#{job.id}/artifacts/file/#{path}" + artifact_path = components[1..-1].join('/') + + "#{pages_config.protocol}://#{components[0]}.#{pages_config.host}/#{artifact_path}" + end + + def external_link? + pages_config.enabled && + pages_config.artifacts_server && + EXTENTIONS_SERVED_BY_PAGES.include?(File.extname(name)) + end + + private + + def pages_config + Gitlab.config.pages + end end end diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb index f5048d17d80..12e93be2104 100644 --- a/app/models/concerns/routable.rb +++ b/app/models/concerns/routable.rb @@ -106,6 +106,10 @@ module Routable RequestStore[full_path_key] ||= uncached_full_path end + def full_path_components + full_path.split('/') + end + def expires_full_path_cache RequestStore.delete(full_path_key) if RequestStore.active? @full_path = nil diff --git a/app/views/projects/artifacts/_tree_file.html.haml b/app/views/projects/artifacts/_tree_file.html.haml index b2f7504aadc..a15c520d8c6 100644 --- a/app/views/projects/artifacts/_tree_file.html.haml +++ b/app/views/projects/artifacts/_tree_file.html.haml @@ -1,6 +1,6 @@ - blob = file.blob -- is_external_link = external_link?(blob) -- path_to_file = link_to_artifact(@project, @build, file) +- is_external_link = blob.external_link? +- path_to_file = file_project_job_artifacts_path(@project, @build, path: file.path) %tr.tree-item.js-artifact-tree-row{ data: { link: path_to_file, external_link: "#{is_external_link}" } } %td.tree-item-file-name diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 27c1ecc7b23..5328644dfb2 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -316,15 +316,16 @@ Settings.registry['path'] = Settings.absolute(Settings.registry['path # Pages # Settings['pages'] ||= Settingslogic.new({}) -Settings.pages['enabled'] = false if Settings.pages['enabled'].nil? -Settings.pages['path'] = Settings.absolute(Settings.pages['path'] || File.join(Settings.shared['path'], "pages")) -Settings.pages['https'] = false if Settings.pages['https'].nil? -Settings.pages['host'] ||= "example.com" -Settings.pages['port'] ||= Settings.pages.https ? 443 : 80 -Settings.pages['protocol'] ||= Settings.pages.https ? "https" : "http" -Settings.pages['url'] ||= Settings.__send__(:build_pages_url) -Settings.pages['external_http'] ||= false unless Settings.pages['external_http'].present? -Settings.pages['external_https'] ||= false unless Settings.pages['external_https'].present? +Settings.pages['enabled'] = false if Settings.pages['enabled'].nil? +Settings.pages['path'] = Settings.absolute(Settings.pages['path'] || File.join(Settings.shared['path'], "pages")) +Settings.pages['https'] = false if Settings.pages['https'].nil? +Settings.pages['host'] ||= "example.com" +Settings.pages['port'] ||= Settings.pages.https ? 443 : 80 +Settings.pages['protocol'] ||= Settings.pages.https ? "https" : "http" +Settings.pages['url'] ||= Settings.__send__(:build_pages_url) +Settings.pages['external_http'] ||= false unless Settings.pages['external_http'].present? +Settings.pages['external_https'] ||= false unless Settings.pages['external_https'].present? +Settings.pages['artifacts_server'] ||= Settings.pages['enabled'] # # Git LFS diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb index caa63e7bd22..bb22e74a83c 100644 --- a/spec/controllers/projects/artifacts_controller_spec.rb +++ b/spec/controllers/projects/artifacts_controller_spec.rb @@ -1,8 +1,8 @@ require 'spec_helper' describe Projects::ArtifactsController do - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } + set(:user) { create(:user) } + set(:project) { create(:project, :repository) } let(:pipeline) do create(:ci_pipeline, @@ -15,7 +15,7 @@ describe Projects::ArtifactsController do let(:job) { create(:ci_build, :success, :artifacts, pipeline: pipeline) } before do - project.team << [user, :developer] + project.add_developer(user) sign_in(user) end @@ -47,11 +47,16 @@ describe Projects::ArtifactsController do end describe 'GET file' do + before do + allow(Gitlab.config.pages).to receive(:enabled).and_return(true) + allow(Gitlab.config.pages).to receive(:artifacts_server).and_return(true) + end + context 'when the file exists' do it 'renders the file view' do get :file, namespace_id: project.namespace, project_id: project, job_id: job, path: 'ci_artifacts.txt' - expect(response).to render_template('projects/artifacts/file') + expect(response).to have_http_status(302) end end diff --git a/spec/fixtures/ci_build_artifacts.zip b/spec/fixtures/ci_build_artifacts.zip Binary files differindex 17c2b8298db..dae976d918e 100644 --- a/spec/fixtures/ci_build_artifacts.zip +++ b/spec/fixtures/ci_build_artifacts.zip diff --git a/spec/models/ci/artifact_blob_spec.rb b/spec/models/ci/artifact_blob_spec.rb index a10a8af5303..3ea11a3bd54 100644 --- a/spec/models/ci/artifact_blob_spec.rb +++ b/spec/models/ci/artifact_blob_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Ci::ArtifactBlob do - let(:build) { create(:ci_build, :artifacts) } + set(:build) { create(:ci_build, :artifacts) } let(:entry) { build.artifacts_metadata_entry('other_artifacts_0.1.2/another-subdirectory/banana_sample.gif') } subject { described_class.new(entry) } @@ -41,4 +41,45 @@ describe Ci::ArtifactBlob do expect(subject.external_storage).to eq(:build_artifact) end end + + describe '#url' do + before do + allow(Gitlab.config.pages).to receive(:enabled).and_return(true) + allow(Gitlab.config.pages).to receive(:artifacts_server).and_return(true) + end + + context '.gif extension' do + it 'returns nil' do + expect(subject.external_url(build.project, build)).to be_nil + end + end + + context 'txt extensions' do + let(:entry) { build.artifacts_metadata_entry('other_artifacts_0.1.2/doc_sample.txt') } + + it 'returns a URL' do + url = subject.external_url(build.project, build) + + expect(url).not_to be_nil + expect(url).to start_with("http") + expect(url).to match Gitlab.config.pages.host + expect(url).to end_with(entry.path) + end + end + end + + describe '#external_link?' do + before do + allow(Gitlab.config.pages).to receive(:enabled).and_return(true) + allow(Gitlab.config.pages).to receive(:artifacts_server).and_return(true) + end + + it { is_expected.not_to be_external_link } + + context 'txt extensions' do + let(:entry) { build.artifacts_metadata_entry('other_artifacts_0.1.2/doc_sample.txt') } + + it { is_expected.to be_external_link } + end + end end |