Merge branch 'git-archive-refactor' into 'master'

Refactor 'git archive' hand-off to gitlab-workhorse

We have a nicer way now to hand off HTTP responses to gitlab-workhorse.

Companion MR: https://gitlab.com/gitlab-org/gitlab-workhorse/merge_requests/36

See merge request !2675

11 files changed, 77 insertions, 71 deletions

diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION
index d2b13eb644d..ef5e4454454 100644
--- a/GITLAB_WORKHORSE_VERSION
+++ b/GITLAB_WORKHORSE_VERSION
@@ -1 +1 @@
-0.6.4
+0.6.5
diff --git a/app/controllers/projects/repositories_controller.rb b/app/controllers/projects/repositories_controller.rb
index ba9aea1c165..5c7614cfbaf 100644
--- a/app/controllers/projects/repositories_controller.rb
+++ b/app/controllers/projects/repositories_controller.rb
@@ -11,7 +11,9 @@ class Projects::RepositoriesController < Projects::ApplicationController
   end
 
   def archive
-    render json: ArchiveRepositoryService.new(@project, params[:ref], params[:format]).execute
+    RepositoryArchiveCacheWorker.perform_async
+    headers.store(*Gitlab::Workhorse.send_git_archive(@project, params[:ref], params[:format]))
+    head :ok
   rescue => ex
     logger.error("#{self.class.name}: #{ex}")
     return git_not_found!
diff --git a/app/services/archive_repository_service.rb b/app/services/archive_repository_service.rb
deleted file mode 100644
index 2160bf13e6d..00000000000
--- a/app/services/archive_repository_service.rb
+++ /dev/null
@@ -1,23 +0,0 @@
-class ArchiveRepositoryService
-  attr_reader :project, :ref, :format
-
-  def initialize(project, ref, format)
-    format ||= 'tar.gz'
-    @project, @ref, @format = project, ref, format.downcase
-  end
-
-  def execute(options = {})
-    RepositoryArchiveCacheWorker.perform_async
-
-    metadata = project.repository.archive_metadata(ref, storage_path, format)
-    raise "Repository or ref not found" if metadata.empty?
-
-    metadata
-  end
-
-  private
-
-  def storage_path
-    Gitlab.config.gitlab.repository_downloads_path
-  end
-end
diff --git a/doc/install/installation.md b/doc/install/installation.md
index 7ae73450afb..7f16e6ff5c4 100644
--- a/doc/install/installation.md
+++ b/doc/install/installation.md
@@ -353,7 +353,7 @@ GitLab Shell is an SSH access and repository management software developed speci
     cd /home/git
     sudo -u git -H git clone https://gitlab.com/gitlab-org/gitlab-workhorse.git
     cd gitlab-workhorse
-    sudo -u git -H git checkout 0.6.4
+    sudo -u git -H git checkout 0.6.5
     sudo -u git -H make
 
 ### Initialize Database and Activate Advanced Features
diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb
index c95d2d2001d..0d0f0d4616d 100644
--- a/lib/api/repositories.rb
+++ b/lib/api/repositories.rb
@@ -98,11 +98,8 @@ module API
         authorize! :download_code, user_project
 
         begin
-          ArchiveRepositoryService.new(
-            user_project,
-            params[:sha],
-            params[:format]
-          ).execute
+          RepositoryArchiveCacheWorker.perform_async
+          header *Gitlab::Workhorse.send_git_archive(user_project, params[:sha], params[:format])
         rescue
           not_found!('File')
         end
diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb
index a23120a4176..c3ddd4c2680 100644
--- a/lib/gitlab/workhorse.rb
+++ b/lib/gitlab/workhorse.rb
@@ -3,19 +3,38 @@ require 'json'
 
 module Gitlab
   class Workhorse
+    SEND_DATA_HEADER = 'Gitlab-Workhorse-Send-Data'
+
     class << self
       def send_git_blob(repository, blob)
-        params_hash = {
+        params = {
           'RepoPath' => repository.path_to_repo,
           'BlobId' => blob.id,
         }
-        params = Base64.urlsafe_encode64(JSON.dump(params_hash))
 
         [
-          'Gitlab-Workhorse-Send-Data',
-          "git-blob:#{params}",
+          SEND_DATA_HEADER,
+          "git-blob:#{encode(params)}",
         ]
       end
+
+      def send_git_archive(project, ref, format)
+        format ||= 'tar.gz'
+        format.downcase!
+        params = project.repository.archive_metadata(ref, Gitlab.config.gitlab.repository_downloads_path, format)
+        raise "Repository or ref not found" if params.empty?
+
+        [
+          SEND_DATA_HEADER,
+          "git-archive:#{encode(params)}",
+        ]
+      end
+      
+      protected
+      
+      def encode(hash)
+        Base64.urlsafe_encode64(JSON.dump(hash))
+      end
     end
   end
 end
diff --git a/spec/controllers/projects/repositories_controller_spec.rb b/spec/controllers/projects/repositories_controller_spec.rb
index 18a30033ed8..09ec4f18f9d 100644
--- a/spec/controllers/projects/repositories_controller_spec.rb
+++ b/spec/controllers/projects/repositories_controller_spec.rb
@@ -8,15 +8,10 @@ describe Projects::RepositoriesController do
     before do
       sign_in(user)
       project.team << [user, :developer]
-
-      allow(ArchiveRepositoryService).to receive(:new).and_return(service)
     end
 
-    let(:service) { ArchiveRepositoryService.new(project, "master", "zip") }
-
-    it "executes ArchiveRepositoryService" do
-      expect(ArchiveRepositoryService).to receive(:new).with(project, "master", "zip")
-      expect(service).to receive(:execute)
+    it "uses Gitlab::Workhorse" do
+      expect(Gitlab::Workhorse).to receive(:send_git_archive).with(project, "master", "zip")
 
       get :archive, namespace_id: project.namespace.path, project_id: project.path, ref: "master", format: "zip"
     end
@@ -24,7 +19,7 @@ describe Projects::RepositoriesController do
     context "when the service raises an error" do
 
       before do
-        allow(service).to receive(:execute).and_raise("Archive failed")
+        allow(Gitlab::Workhorse).to receive(:send_git_archive).and_raise("Archive failed")
       end
 
       it "renders Not Found" do
diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb
new file mode 100644
index 00000000000..d940bf05061
--- /dev/null
+++ b/spec/lib/gitlab/workhorse_spec.rb
@@ -0,0 +1,18 @@
+require 'spec_helper'
+
+describe Gitlab::Workhorse, lib: true do
+  let(:project) { create(:project) }
+  let(:subject) { Gitlab::Workhorse }
+
+  describe "#send_git_archive" do
+    context "when the repository doesn't have an archive file path" do
+      before do
+        allow(project.repository).to receive(:archive_metadata).and_return(Hash.new)
+      end
+
+      it "raises an error" do
+        expect { subject.send_git_archive(project, "master", "zip") }.to raise_error(RuntimeError)
+      end
+    end
+  end
+end
diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb
index 4911cdd9da6..0ae63b0afec 100644
--- a/spec/requests/api/repositories_spec.rb
+++ b/spec/requests/api/repositories_spec.rb
@@ -4,6 +4,7 @@ require 'mime/types'
 describe API::API, api: true  do
   include ApiHelpers
   include RepoHelpers
+  include WorkhorseHelpers
 
   let(:user) { create(:user) }
   let(:user2) { create(:user) }
@@ -91,21 +92,27 @@ describe API::API, api: true  do
       get api("/projects/#{project.id}/repository/archive", user)
       repo_name = project.repository.name.gsub("\.git", "")
       expect(response.status).to eq(200)
-      expect(json_response['ArchivePath']).to match(/#{repo_name}\-[^\.]+\.tar.gz/)
+      type, params = workhorse_send_data
+      expect(type).to eq('git-archive')
+      expect(params['ArchivePath']).to match(/#{repo_name}\-[^\.]+\.tar.gz/)
     end
 
     it "should get the archive.zip" do
       get api("/projects/#{project.id}/repository/archive.zip", user)
       repo_name = project.repository.name.gsub("\.git", "")
       expect(response.status).to eq(200)
-      expect(json_response['ArchivePath']).to match(/#{repo_name}\-[^\.]+\.zip/)
+      type, params = workhorse_send_data
+      expect(type).to eq('git-archive')
+      expect(params['ArchivePath']).to match(/#{repo_name}\-[^\.]+\.zip/)
     end
 
     it "should get the archive.tar.bz2" do
       get api("/projects/#{project.id}/repository/archive.tar.bz2", user)
       repo_name = project.repository.name.gsub("\.git", "")
       expect(response.status).to eq(200)
-      expect(json_response['ArchivePath']).to match(/#{repo_name}\-[^\.]+\.tar.bz2/)
+      type, params = workhorse_send_data
+      expect(type).to eq('git-archive')
+      expect(params['ArchivePath']).to match(/#{repo_name}\-[^\.]+\.tar.bz2/)
     end
 
     it "should return 404 for invalid sha" do
diff --git a/spec/services/archive_repository_service_spec.rb b/spec/services/archive_repository_service_spec.rb
deleted file mode 100644
index bd871605c66..00000000000
--- a/spec/services/archive_repository_service_spec.rb
+++ /dev/null
@@ -1,25 +0,0 @@
-require 'spec_helper'
-
-describe ArchiveRepositoryService, services: true do
-  let(:project) { create(:project) }
-  subject { ArchiveRepositoryService.new(project, "master", "zip") }
-
-  describe "#execute" do
-    it "cleans old archives" do
-      expect(RepositoryArchiveCacheWorker).to receive(:perform_async)
-
-      subject.execute(timeout: 0.0)
-    end
-
-    context "when the repository doesn't have an archive file path" do
-      before do
-        allow(project.repository).to receive(:archive_metadata).and_return(Hash.new)
-      end
-
-      it "raises an error" do
-        expect { subject.execute(timeout: 0.0) }.to raise_error(RuntimeError)
-      end
-    end
-
-  end
-end
diff --git a/spec/support/workhorse_helpers.rb b/spec/support/workhorse_helpers.rb
new file mode 100644
index 00000000000..107b6e30924
--- /dev/null
+++ b/spec/support/workhorse_helpers.rb
@@ -0,0 +1,16 @@
+module WorkhorseHelpers
+  extend self
+
+  def workhorse_send_data
+    @_workhorse_send_data ||= begin
+      header = response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]
+      split_header = header.split(':')
+      type = split_header.shift
+      header = split_header.join(':')
+      [
+        type,
+        JSON.parse(Base64.urlsafe_decode64(header)),
+      ]
+    end
+  end
+end