Merge branch '24704-restore-download-repository-path' into 'master'

Restore "download repository path" feature, now with fixes and a feature flag

Closes gitlab-workhorse#218 and #24704

See merge request gitlab-org/gitlab-ce!27275

18 files changed, 204 insertions, 76 deletions

diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss
index b90db135b4a..8fb4027bf97 100644
--- a/app/assets/stylesheets/framework/dropdowns.scss
+++ b/app/assets/stylesheets/framework/dropdowns.scss
@@ -351,6 +351,10 @@
     // Expects up to 3 digits on the badge
     margin-right: 40px;
   }
+
+  .dropdown-menu-content {
+    padding: $dropdown-item-padding-y $dropdown-item-padding-x;
+  }
 }
 
 .droplab-dropdown {
diff --git a/app/controllers/projects/repositories_controller.rb b/app/controllers/projects/repositories_controller.rb
index 4eeaeb860ee..3b4215b766e 100644
--- a/app/controllers/projects/repositories_controller.rb
+++ b/app/controllers/projects/repositories_controller.rb
@@ -23,7 +23,7 @@ class Projects::RepositoriesController < Projects::ApplicationController
       append_sha = false if @filename == shortname
     end
 
-    send_git_archive @repository, ref: @ref, format: params[:format], append_sha: append_sha
+    send_git_archive @repository, ref: @ref, path: params[:path], format: params[:format], append_sha: append_sha
   rescue => ex
     logger.error("#{self.class.name}: #{ex}")
     git_not_found!
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index 7da51da8473..2ac90eb8d9f 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -299,6 +299,10 @@ module ProjectsHelper
     }.to_json
   end
 
+  def directory?
+    @path.present?
+  end
+
   def external_classification_label_help_message
     default_label = ::Gitlab::CurrentSettings.current_application_settings
                       .external_authorization_service_default_label
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 574ce12b309..51ab2247a03 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -299,13 +299,14 @@ class Repository
     end
   end
 
-  def archive_metadata(ref, storage_path, format = "tar.gz", append_sha:)
+  def archive_metadata(ref, storage_path, format = "tar.gz", append_sha:, path: nil)
     raw_repository.archive_metadata(
       ref,
       storage_path,
       project.path,
       format,
-      append_sha: append_sha
+      append_sha: append_sha,
+      path: path
     )
   end
 
diff --git a/app/views/projects/buttons/_download.html.haml b/app/views/projects/buttons/_download.html.haml
index 4eb53faa6ff..4762045ee96 100644
--- a/app/views/projects/buttons/_download.html.haml
+++ b/app/views/projects/buttons/_download.html.haml
@@ -7,31 +7,22 @@
       = sprite_icon('download')
       %span.sr-only=  _('Select Archive Format')
       = sprite_icon("arrow-down")
-    %ul.dropdown-menu.dropdown-menu-right{ role: 'menu' }
-      %li.dropdown-header
-        #{ _('Source code') }
-      %li
-        = link_to project_archive_path(project, id: tree_join(ref, archive_prefix), format: 'zip'), rel: 'nofollow', download: '' do
-          %span=  _('Download zip')
-      %li
-        = link_to project_archive_path(project, id: tree_join(ref, archive_prefix), format: 'tar.gz'), rel: 'nofollow', download: '' do
-          %span= _('Download tar.gz')
-      %li
-        = link_to project_archive_path(project, id: tree_join(ref, archive_prefix), format: 'tar.bz2'), rel: 'nofollow', download: '' do
-          %span= _('Download tar.bz2')
-      %li
-        = link_to project_archive_path(project, id: tree_join(ref, archive_prefix), format: 'tar'), rel: 'nofollow', download: '' do
-          %span= _('Download tar')
-
+    .dropdown-menu.dropdown-menu-right{ role: 'menu' }
+      %section
+        %h5.m-0.dropdown-bold-header= _('Download source code')
+        .dropdown-menu-content
+          = render 'projects/buttons/download_links', project: project, ref: ref, archive_prefix: archive_prefix, path: nil
+      - if directory? && Feature.enabled?(:git_archive_path, default_enabled: true)
+        %section.border-top.pt-1.mt-1
+          %h5.m-0.dropdown-bold-header= _('Download this directory')
+          .dropdown-menu-content
+            = render 'projects/buttons/download_links', project: project, ref: ref, archive_prefix: archive_prefix, path: @path
       - if pipeline && pipeline.latest_builds_with_artifacts.any?
-        %li.dropdown-header Artifacts
-        - unless pipeline.latest?
-          - latest_pipeline = project.pipeline_for(ref)
-          %li
-            .unclickable= ci_status_for_statuseable(latest_pipeline)
-          %li.dropdown-header Previous Artifacts
-        - pipeline.latest_builds_with_artifacts.each do |job|
-          %li
-            = link_to latest_succeeded_project_artifacts_path(project, "#{ref}/download", job: job.name), rel: 'nofollow', download: '' do
-              %span
-                #{s_('DownloadArtifacts|Download')} '#{job.name}'
+        %section.border-top.pt-1.mt-1
+          %h5.m-0.dropdown-bold-header= _('Download artifacts')
+          - unless pipeline.latest?
+            %span.unclickable= ci_status_for_statuseable(project.pipeline_for(ref))
+          %h6.m-0.dropdown-header= _('Previous Artifacts')
+          %ul
+            - pipeline.latest_builds_with_artifacts.each do |job|
+              %li= link_to job.name, latest_succeeded_project_artifacts_path(project, "#{ref}/download", job: job.name), rel: 'nofollow', download: ''
diff --git a/app/views/projects/buttons/_download_links.html.haml b/app/views/projects/buttons/_download_links.html.haml
new file mode 100644
index 00000000000..7f2cd8e109e
--- /dev/null
+++ b/app/views/projects/buttons/_download_links.html.haml
@@ -0,0 +1,5 @@
+- formats = [['zip', 'btn-primary'], ['tar.gz'], ['tar.bz2'], ['tar']]
+
+.d-flex.justify-content-between
+  - formats.each do |(fmt, extra_class)|
+    = link_to fmt, project_archive_path(project, id: tree_join(ref, archive_prefix), path: path, format: fmt), rel: 'nofollow', download: '', class: "btn btn-xs #{extra_class}"
diff --git a/changelogs/unreleased/24704-download-repository-path.yml b/changelogs/unreleased/24704-download-repository-path.yml
new file mode 100644
index 00000000000..ff3082bec45
--- /dev/null
+++ b/changelogs/unreleased/24704-download-repository-path.yml
@@ -0,0 +1,5 @@
+---
+title: Download a folder from repository
+merge_request: 26532
+author: kiameisomabes
+type: added
diff --git a/doc/user/project/repository/img/download_source_code.png b/doc/user/project/repository/img/download_source_code.png
new file mode 100644
index 00000000000..17f2cb4b3e8
--- /dev/null
+++ b/doc/user/project/repository/img/download_source_code.png
diff --git a/doc/user/project/repository/index.md b/doc/user/project/repository/index.md
index 22d912cd9d1..718566a539f 100644
--- a/doc/user/project/repository/index.md
+++ b/doc/user/project/repository/index.md
@@ -241,4 +241,24 @@ Projects that contain a `.xcodeproj` or `.xcworkspace` directory can now be clon
 in Xcode using the new **Open in Xcode** button, located next to the Git URL
 used for cloning your project. The button is only shown on macOS.
 
+## Download Source Code
+
+Source code stored in the repository can be downloaded.
+
+By clicking the download icon, a dropdown will open with links to download the following:
+
+![Download source code](img/download_source_code.png)
+
+- **Source Code:**
+  This allows users to download the source code on branch they're currently
+  viewing. Available zip, tar, tar.gz and tar.bz2.
+- **Directory:**
+  > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/24704) in GitLab 11.10
+
+  Only shows up when viewing a sub-directory. This allows users to download
+  the specific directory they're currently viewing. Also available in zip, tar,
+  tar.gz and tar.bz2.
+- **Artifacts:**
+  This allows users to download the artifacts of the latest CI build.
+
 [jupyter]: https://jupyter.org
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index be9e926728c..a22e3c4b9dd 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -231,12 +231,12 @@ module Gitlab
         end
       end
 
-      def archive_metadata(ref, storage_path, project_path, format = "tar.gz", append_sha:)
+      def archive_metadata(ref, storage_path, project_path, format = "tar.gz", append_sha:, path: nil)
         ref ||= root_ref
         commit = Gitlab::Git::Commit.find(self, ref)
         return {} if commit.nil?
 
-        prefix = archive_prefix(ref, commit.id, project_path, append_sha: append_sha)
+        prefix = archive_prefix(ref, commit.id, project_path, append_sha: append_sha, path: path)
 
         {
           'ArchivePrefix' => prefix,
@@ -248,13 +248,14 @@ module Gitlab
 
       # This is both the filename of the archive (missing the extension) and the
       # name of the top-level member of the archive under which all files go
-      def archive_prefix(ref, sha, project_path, append_sha:)
+      def archive_prefix(ref, sha, project_path, append_sha:, path:)
         append_sha = (ref != sha) if append_sha.nil?
 
         formatted_ref = ref.tr('/', '-')
 
         prefix_segments = [project_path, formatted_ref]
         prefix_segments << sha if append_sha
+        prefix_segments << path.tr('/', '-').gsub(%r{^/|/$}, '') if path
 
         prefix_segments.join('-')
       end
diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb
index 0c2acac3d1e..46a7b5b982a 100644
--- a/lib/gitlab/workhorse.rb
+++ b/lib/gitlab/workhorse.rb
@@ -63,16 +63,34 @@ module Gitlab
         ]
       end
 
-      def send_git_archive(repository, ref:, format:, append_sha:)
+      def send_git_archive(repository, ref:, format:, append_sha:, path: nil)
+        path_enabled = Feature.enabled?(:git_archive_path, default_enabled: true)
+        path = nil unless path_enabled
+
         format ||= 'tar.gz'
         format = format.downcase
-        params = repository.archive_metadata(ref, Gitlab.config.gitlab.repository_downloads_path, format, append_sha: append_sha)
-        raise "Repository or ref not found" if params.empty?
 
-        params['GitalyServer'] = gitaly_server_hash(repository)
+        metadata = repository.archive_metadata(
+          ref,
+          Gitlab.config.gitlab.repository_downloads_path,
+          format,
+          append_sha: append_sha,
+          path: path
+        )
 
-        # If present DisableCache must be a Boolean. Otherwise workhorse ignores it.
+        raise "Repository or ref not found" if metadata.empty?
+
+        params =
+          if path_enabled
+            send_git_archive_params(repository, metadata, path, archive_format(format))
+          else
+            metadata
+          end
+
+        # If present, DisableCache must be a Boolean. Otherwise
+        # workhorse ignores it.
         params['DisableCache'] = true if git_archive_cache_disabled?
+        params['GitalyServer'] = gitaly_server_hash(repository)
 
         [
           SEND_DATA_HEADER,
@@ -216,10 +234,19 @@ module Gitlab
 
       protected
 
+      # This is the outermost encoding of a senddata: header. It is safe for
+      # inclusion in HTTP response headers
       def encode(hash)
         Base64.urlsafe_encode64(JSON.dump(hash))
       end
 
+      # This is for encoding individual fields inside the senddata JSON that
+      # contain binary data. In workhorse, the corresponding struct field should
+      # be type []byte
+      def encode_binary(binary)
+        Base64.encode64(binary)
+      end
+
       def gitaly_server_hash(repository)
         {
           address: Gitlab::GitalyClient.address(repository.project.repository_storage),
@@ -238,6 +265,34 @@ module Gitlab
       def git_archive_cache_disabled?
         ENV['WORKHORSE_ARCHIVE_CACHE_DISABLED'].present? || Feature.enabled?(:workhorse_archive_cache_disabled)
       end
+
+      def archive_format(format)
+        case format
+        when "tar.bz2", "tbz", "tbz2", "tb2", "bz2"
+          Gitaly::GetArchiveRequest::Format::TAR_BZ2
+        when "tar"
+          Gitaly::GetArchiveRequest::Format::TAR
+        when "zip"
+          Gitaly::GetArchiveRequest::Format::ZIP
+        else
+          Gitaly::GetArchiveRequest::Format::TAR_GZ
+        end
+      end
+
+      def send_git_archive_params(repository, metadata, path, format)
+        {
+          'ArchivePath' => metadata['ArchivePath'],
+          'GetArchiveRequest' => encode_binary(
+            Gitaly::GetArchiveRequest.new(
+              repository: repository.gitaly_repository,
+              commit_id: metadata['CommitId'],
+              prefix: metadata['ArchivePrefix'],
+              format: format,
+              path: path.presence || ""
+            ).to_proto
+          )
+        }
+      end
     end
   end
 end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index da09f305400..fc412a936e9 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -3298,19 +3298,10 @@ msgstr ""
 msgid "Download export"
 msgstr ""
 
-msgid "Download tar"
+msgid "Download source code"
 msgstr ""
 
-msgid "Download tar.bz2"
-msgstr ""
-
-msgid "Download tar.gz"
-msgstr ""
-
-msgid "Download zip"
-msgstr ""
-
-msgid "DownloadArtifacts|Download"
+msgid "Download this directory"
 msgstr ""
 
 msgid "DownloadCommit|Email Patches"
@@ -6679,6 +6670,9 @@ msgstr ""
 msgid "Preview payload"
 msgstr ""
 
+msgid "Previous Artifacts"
+msgstr ""
+
 msgid "Prioritize"
 msgstr ""
 
diff --git a/spec/features/projects/branches/download_buttons_spec.rb b/spec/features/projects/branches/download_buttons_spec.rb
index c8dc72a34ec..3e75890725e 100644
--- a/spec/features/projects/branches/download_buttons_spec.rb
+++ b/spec/features/projects/branches/download_buttons_spec.rb
@@ -35,7 +35,7 @@ describe 'Download buttons in branches page' do
       it 'shows download artifacts button' do
         href = latest_succeeded_project_artifacts_path(project, 'binary-encoding/download', job: 'build')
 
-        expect(page).to have_link "Download '#{build.name}'", href: href
+        expect(page).to have_link build.name, href: href
       end
     end
   end
diff --git a/spec/features/projects/files/download_buttons_spec.rb b/spec/features/projects/files/download_buttons_spec.rb
index 03cb3530e2b..111972a6b00 100644
--- a/spec/features/projects/files/download_buttons_spec.rb
+++ b/spec/features/projects/files/download_buttons_spec.rb
@@ -30,7 +30,7 @@ describe 'Projects > Files > Download buttons in files tree' do
     it 'shows download artifacts button' do
       href = latest_succeeded_project_artifacts_path(project, "#{project.default_branch}/download", job: 'build')
 
-      expect(page).to have_link "Download '#{build.name}'", href: href
+      expect(page).to have_link build.name, href: href
     end
   end
 end
diff --git a/spec/features/projects/show/download_buttons_spec.rb b/spec/features/projects/show/download_buttons_spec.rb
index 3a2dcc5aa55..fee5f8001b0 100644
--- a/spec/features/projects/show/download_buttons_spec.rb
+++ b/spec/features/projects/show/download_buttons_spec.rb
@@ -35,11 +35,10 @@ describe 'Projects > Show > Download buttons' do
       it 'shows download artifacts button' do
         href = latest_succeeded_project_artifacts_path(project, "#{project.default_branch}/download", job: 'build')
 
-        expect(page).to have_link "Download '#{build.name}'", href: href
+        expect(page).to have_link build.name, href: href
       end
 
       it 'download links have download attribute' do
-        expect(page).to have_selector('a', text: 'Download')
         page.all('a', text: 'Download').each do |link|
           expect(link[:download]).to eq ''
         end
diff --git a/spec/features/projects/tags/download_buttons_spec.rb b/spec/features/projects/tags/download_buttons_spec.rb
index fbfd8cee7aa..4c8ec53836a 100644
--- a/spec/features/projects/tags/download_buttons_spec.rb
+++ b/spec/features/projects/tags/download_buttons_spec.rb
@@ -36,7 +36,7 @@ describe 'Download buttons in tags page' do
       it 'shows download artifacts button' do
         href = latest_succeeded_project_artifacts_path(project, "#{tag}/download", job: 'build')
 
-        expect(page).to have_link "Download '#{build.name}'", href: href
+        expect(page).to have_link build.name, href: href
       end
     end
   end
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 088f8acf554..778950c95e4 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -152,13 +152,14 @@ describe Gitlab::Git::Repository, :seed_helper do
     let(:append_sha) { true }
     let(:ref) { 'master' }
     let(:format) { nil }
+    let(:path) { nil }
 
     let(:expected_extension) { 'tar.gz' }
     let(:expected_filename) { "#{expected_prefix}.#{expected_extension}" }
     let(:expected_path) { File.join(storage_path, cache_key, expected_filename) }
     let(:expected_prefix) { "gitlab-git-test-#{ref}-#{SeedRepo::LastCommit::ID}" }
 
-    subject(:metadata) { repository.archive_metadata(ref, storage_path, 'gitlab-git-test', format, append_sha: append_sha) }
+    subject(:metadata) { repository.archive_metadata(ref, storage_path, 'gitlab-git-test', format, append_sha: append_sha, path: path) }
 
     it 'sets CommitId to the commit SHA' do
       expect(metadata['CommitId']).to eq(SeedRepo::LastCommit::ID)
@@ -176,6 +177,14 @@ describe Gitlab::Git::Repository, :seed_helper do
       expect(metadata['ArchivePath']).to eq(expected_path)
     end
 
+    context 'path is set' do
+      let(:path) { 'foo/bar' }
+
+      it 'appends the path to the prefix' do
+        expect(metadata['ArchivePrefix']).to eq("#{expected_prefix}-foo-bar")
+      end
+    end
+
     context 'append_sha varies archive path and filename' do
       where(:append_sha, :ref, :expected_prefix) do
         sha = SeedRepo::LastCommit::ID
diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb
index d02d9be5c5c..f8332757fcd 100644
--- a/spec/lib/gitlab/workhorse_spec.rb
+++ b/spec/lib/gitlab/workhorse_spec.rb
@@ -16,40 +16,80 @@ describe Gitlab::Workhorse do
     let(:ref) { 'master' }
     let(:format) { 'zip' }
     let(:storage_path) { Gitlab.config.gitlab.repository_downloads_path }
-    let(:base_params) { repository.archive_metadata(ref, storage_path, format, append_sha: nil) }
-    let(:gitaly_params) do
-      base_params.merge(
-        'GitalyServer' => {
-          'address' => Gitlab::GitalyClient.address(project.repository_storage),
-          'token' => Gitlab::GitalyClient.token(project.repository_storage)
-        },
-        'GitalyRepository' => repository.gitaly_repository.to_h.deep_stringify_keys
-      )
-    end
+    let(:path) { 'some/path' if Feature.enabled?(:git_archive_path, default_enabled: true) }
+    let(:metadata) { repository.archive_metadata(ref, storage_path, format, append_sha: nil, path: path) }
     let(:cache_disabled) { false }
 
     subject do
-      described_class.send_git_archive(repository, ref: ref, format: format, append_sha: nil)
+      described_class.send_git_archive(repository, ref: ref, format: format, append_sha: nil, path: path)
     end
 
     before do
       allow(described_class).to receive(:git_archive_cache_disabled?).and_return(cache_disabled)
     end
 
-    it 'sets the header correctly' do
-      key, command, params = decode_workhorse_header(subject)
+    context 'feature flag disabled' do
+      before do
+        stub_feature_flags(git_archive_path: false)
+      end
 
-      expect(key).to eq('Gitlab-Workhorse-Send-Data')
-      expect(command).to eq('git-archive')
-      expect(params).to include(gitaly_params)
+      it 'sets the header correctly' do
+        key, command, params = decode_workhorse_header(subject)
+
+        expected_params = metadata.merge(
+          'GitalyRepository' => repository.gitaly_repository.to_h,
+          'GitalyServer' => {
+            address: Gitlab::GitalyClient.address(project.repository_storage),
+            token: Gitlab::GitalyClient.token(project.repository_storage)
+          }
+        )
+
+        expect(key).to eq('Gitlab-Workhorse-Send-Data')
+        expect(command).to eq('git-archive')
+        expect(params).to eq(expected_params.deep_stringify_keys)
+      end
+
+      context 'when archive caching is disabled' do
+        let(:cache_disabled) { true }
+
+        it 'tells workhorse not to use the cache' do
+          _, _, params = decode_workhorse_header(subject)
+          expect(params).to include({ 'DisableCache' => true })
+        end
+      end
     end
 
-    context 'when archive caching is disabled' do
-      let(:cache_disabled) { true }
+    context 'feature flag enabled' do
+      it 'sets the header correctly' do
+        key, command, params = decode_workhorse_header(subject)
+
+        expect(key).to eq('Gitlab-Workhorse-Send-Data')
+        expect(command).to eq('git-archive')
+        expect(params).to eq({
+          'GitalyServer' => {
+            address: Gitlab::GitalyClient.address(project.repository_storage),
+            token: Gitlab::GitalyClient.token(project.repository_storage)
+          },
+          'ArchivePath' => metadata['ArchivePath'],
+          'GetArchiveRequest' => Base64.encode64(
+            Gitaly::GetArchiveRequest.new(
+              repository: repository.gitaly_repository,
+              commit_id: metadata['CommitId'],
+              prefix: metadata['ArchivePrefix'],
+              format: Gitaly::GetArchiveRequest::Format::ZIP,
+              path: path
+            ).to_proto
+          )
+        }.deep_stringify_keys)
+      end
 
-      it 'tells workhorse not to use the cache' do
-        _, _, params = decode_workhorse_header(subject)
-        expect(params).to include({ 'DisableCache' => true })
+      context 'when archive caching is disabled' do
+        let(:cache_disabled) { true }
+
+        it 'tells workhorse not to use the cache' do
+          _, _, params = decode_workhorse_header(subject)
+          expect(params).to include({ 'DisableCache' => true })
+        end
       end
     end