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

Revert "Merge branch '24704-download-repository-path' into 'master'"

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

app/assets/stylesheets/framework/dropdowns.scss

@@ -287,7 +287,7 @@
     list-style: none;
     padding: 0 1px;
 
-    a:not(.btn),
+    a,
     button,
     .menu-item {
       @include dropdown-link;
@@ -351,10 +351,6 @@
     // 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 {

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, path: params[:path], format: params[:format], append_sha: append_sha
+    send_git_archive @repository, ref: @ref, format: params[:format], append_sha: append_sha
   rescue => ex
     logger.error("#{self.class.name}: #{ex}")
     git_not_found!

app/helpers/projects_helper.rb

@@ -299,10 +299,6 @@ 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

app/models/repository.rb

@@ -299,14 +299,13 @@ class Repository
     end
   end
 
-  def archive_metadata(ref, storage_path, format = "tar.gz", append_sha:, path: nil)
+  def archive_metadata(ref, storage_path, format = "tar.gz", append_sha:)
     raw_repository.archive_metadata(
       ref,
       storage_path,
       project.path,
       format,
-      append_sha: append_sha,
+      append_sha: append_sha
-      path: path
     )
   end
 

app/views/projects/buttons/_download.html.haml

@@ -8,20 +8,30 @@
       %span.sr-only=  _('Select Archive Format')
       = sprite_icon("arrow-down")
     %ul.dropdown-menu.dropdown-menu-right{ role: 'menu' }
-      %li.dropdown-bold-header= _('Download source code')
+      %li.dropdown-header
-      %li.dropdown-menu-content
+        #{ _('Source code') }
-        = render 'projects/buttons/download_links', project: project, ref: ref, archive_prefix: archive_prefix, path: nil
+      %li
-      - if directory?
+        = link_to project_archive_path(project, id: tree_join(ref, archive_prefix), format: 'zip'), rel: 'nofollow', download: '' do
-        %li.separator
+          %span=  _('Download zip')
-        %li.dropdown-bold-header= _('Download this directory')
+      %li
-        %li.dropdown-menu-content
+        = link_to project_archive_path(project, id: tree_join(ref, archive_prefix), format: 'tar.gz'), rel: 'nofollow', download: '' do
-          = render 'projects/buttons/download_links', project: project, ref: ref, archive_prefix: archive_prefix, path: @path
+          %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')
+
       - if pipeline && pipeline.latest_builds_with_artifacts.any?
-        %li.separator
+        %li.dropdown-header Artifacts
-        %li.dropdown-bold-header= _('Download artifacts')
         - unless pipeline.latest?
-          %span.unclickable= ci_status_for_statuseable(project.pipeline_for(ref))
+          - latest_pipeline = project.pipeline_for(ref)
-        %li.dropdown-header= _('Previous Artifacts')
+          %li
+            .unclickable= ci_status_for_statuseable(latest_pipeline)
+          %li.dropdown-header Previous Artifacts
         - 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: ''
+            = link_to latest_succeeded_project_artifacts_path(project, "#{ref}/download", job: job.name), rel: 'nofollow', download: '' do
+              %span
+                #{s_('DownloadArtifacts|Download')} '#{job.name}'

app/views/projects/buttons/_download_links.html.haml

@@ -1,9 +0,0 @@
-%ul
-  %li.d-inline-block.m-0.p-0
-    = link_to 'zip', project_archive_path(project, id: tree_join(ref, archive_prefix), path: path, format: 'zip'), rel: 'nofollow', download: '', class: 'btn btn-primary btn-xs'
-  %li.d-inline-block.m-0.p-0
-    = link_to 'tar.gz', project_archive_path(project, id: tree_join(ref, archive_prefix), path: path, format: 'tar.gz'), rel: 'nofollow', download: '', class: 'btn btn-xs'
-  %li.d-inline-block.m-0.p-0
-    = link_to 'tar.bz2', project_archive_path(project, id: tree_join(ref, archive_prefix), path: path, format: 'tar.bz2'), rel: 'nofollow', download: '', class: 'btn btn-xs'
-  %li.d-inline-block.m-0.p-0
-    = link_to 'tar', project_archive_path(project, id: tree_join(ref, archive_prefix), path: path, format: 'tar'), rel: 'nofollow', download: '', class: 'btn btn-xs'

changelogs/unreleased/24704-download-repository-path.yml

@@ -1,5 +0,0 @@
----
-title: Download a folder from repository
-merge_request: 26532
-author: kiameisomabes
-type: added

doc/user/project/repository/index.md

@@ -241,24 +241,4 @@ 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

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:, path: nil)
+      def archive_metadata(ref, storage_path, project_path, format = "tar.gz", append_sha:)
         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, path: path)
+        prefix = archive_prefix(ref, commit.id, project_path, append_sha: append_sha)
 
         {
           'ArchivePrefix' => prefix,
@@ -248,14 +248,13 @@ 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:, path:)
+      def archive_prefix(ref, sha, project_path, append_sha:)
         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

lib/gitlab/workhorse.rb

@@ -63,26 +63,13 @@ module Gitlab
         ]
       end
 
-      def send_git_archive(repository, ref:, format:, append_sha:, path: nil)
+      def send_git_archive(repository, ref:, format:, append_sha:)
         format ||= 'tar.gz'
         format = format.downcase
-        metadata = repository.archive_metadata(ref, Gitlab.config.gitlab.repository_downloads_path, format, append_sha: append_sha, path: path)
+        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?
 
-        raise "Repository or ref not found" if metadata.empty?
+        params['GitalyServer'] = gitaly_server_hash(repository)
-
-        params = {
-          'GitalyServer' => gitaly_server_hash(repository),
-          'ArchivePath' => metadata['ArchivePath'],
-          'GetArchiveRequest' => encode_binary(
-            Gitaly::GetArchiveRequest.new(
-              repository: repository.gitaly_repository,
-              commit_id: metadata['CommitId'],
-              prefix: metadata['ArchivePrefix'],
-              format: archive_format(format),
-              path: path.presence || ""
-            ).to_proto
-          )
-        }
 
         # If present DisableCache must be a Boolean. Otherwise workhorse ignores it.
         params['DisableCache'] = true if git_archive_cache_disabled?
@@ -233,10 +220,6 @@ module Gitlab
         Base64.urlsafe_encode64(JSON.dump(hash))
       end
 
-      def encode_binary(binary)
-        Base64.urlsafe_encode64(binary)
-      end
-
       def gitaly_server_hash(repository)
         {
           address: Gitlab::GitalyClient.address(repository.project.repository_storage),
@@ -255,19 +238,6 @@ 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
     end
   end
 end

locale/gitlab.pot

@@ -3109,10 +3109,19 @@ msgstr ""
 msgid "Download asset"
 msgstr ""
 
-msgid "Download source code"
+msgid "Download tar"
 msgstr ""
 
-msgid "Download this directory"
+msgid "Download tar.bz2"
+msgstr ""
+
+msgid "Download tar.gz"
+msgstr ""
+
+msgid "Download zip"
+msgstr ""
+
+msgid "DownloadArtifacts|Download"
 msgstr ""
 
 msgid "DownloadCommit|Email Patches"
@@ -6262,9 +6271,6 @@ msgstr ""
 msgid "Preview payload"
 msgstr ""
 
-msgid "Previous Artifacts"
-msgstr ""
-
 msgid "Prioritize"
 msgstr ""
 

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 build.name, href: href
+        expect(page).to have_link "Download '#{build.name}'", href: href
       end
     end
   end

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 build.name, href: href
+      expect(page).to have_link "Download '#{build.name}'", href: href
     end
   end
 end

spec/features/projects/show/download_buttons_spec.rb

@@ -35,10 +35,11 @@ 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 build.name, href: href
+        expect(page).to have_link "Download '#{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

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 build.name, href: href
+        expect(page).to have_link "Download '#{build.name}'", href: href
       end
     end
   end

spec/lib/gitlab/git/repository_spec.rb

@@ -152,14 +152,13 @@ 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, path: path) }
+    subject(:metadata) { repository.archive_metadata(ref, storage_path, 'gitlab-git-test', format, append_sha: append_sha) }
 
     it 'sets CommitId to the commit SHA' do
       expect(metadata['CommitId']).to eq(SeedRepo::LastCommit::ID)
@@ -177,14 +176,6 @@ 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

spec/lib/gitlab/workhorse_spec.rb

@@ -16,12 +16,20 @@ describe Gitlab::Workhorse do
     let(:ref) { 'master' }
     let(:format) { 'zip' }
     let(:storage_path) { Gitlab.config.gitlab.repository_downloads_path }
-    let(:path) { 'some/path' }
+    let(:base_params) { repository.archive_metadata(ref, storage_path, format, append_sha: nil) }
-    let(:metadata) { repository.archive_metadata(ref, storage_path, format, append_sha: nil, path: path) }
+    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(:cache_disabled) { false }
 
     subject do
-      described_class.send_git_archive(repository, ref: ref, format: format, append_sha: nil, path: path)
+      described_class.send_git_archive(repository, ref: ref, format: format, append_sha: nil)
     end
 
     before do
@@ -33,22 +41,7 @@ describe Gitlab::Workhorse do
 
       expect(key).to eq('Gitlab-Workhorse-Send-Data')
       expect(command).to eq('git-archive')
-      expect(params).to eq({
+      expect(params).to include(gitaly_params)
-        'GitalyServer' => {
-          address: Gitlab::GitalyClient.address(project.repository_storage),
-          token: Gitlab::GitalyClient.token(project.repository_storage)
-        },
-        'ArchivePath' => metadata['ArchivePath'],
-        'GetArchiveRequest' => Base64.urlsafe_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
 
     context 'when archive caching is disabled' do