Redirect to pages daemon

Conflicts:
	app/views/projects/artifacts/_tree_file.html.haml

9 files changed, 102 insertions, 55 deletions

diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb
index a93b72a646b..eeb968ef010 100644
--- a/app/controllers/projects/artifacts_controller.rb
+++ b/app/controllers/projects/artifacts_controller.rb
@@ -26,13 +26,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 d2f6b6be5c6..90a21b8df60 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 10e238b6ba1..0319fb0af65 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_url?(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 1792cf092a6..4cbc801cc21 100644
--- a/config/initializers/1_settings.rb
+++ b/config/initializers/1_settings.rb
@@ -364,15 +364,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']
 
 #
 # Geo
diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb
index 5824ac4c74b..5517d983280 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
index 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