Merge branch 'sh-optimize-archive-controller-ref-names' into 'master'

Optimize ref name lookups in archive downloads

See merge request gitlab-org/gitlab!23890

app/controllers/projects/repositories_controller.rb

@@ -81,7 +81,7 @@ class Projects::RepositoriesController < Projects::ApplicationController
 
   def assign_archive_vars
     if params[:id]
-      @ref, @filename = extract_ref(params[:id])
+      @ref, @filename = extract_ref_and_filename(params[:id])
     else
       @ref = params[:ref]
       @filename = nil
@@ -89,6 +89,26 @@ class Projects::RepositoriesController < Projects::ApplicationController
   rescue InvalidPathError
     render_404
   end
+
+  # path can be of the form:
+  # master
+  # master/first.zip
+  # master/first/second.tar.gz
+  # master/first/second/third.zip
+  #
+  # In the archive case, we know that the last value is always the filename, so we
+  # do a greedy match to extract the ref. This avoid having to pull all ref names
+  # from Redis.
+  def extract_ref_and_filename(id)
+    path = id.strip
+    data = path.match(/(.*)\/(.*)/)
+
+    if data
+      [data[1], data[2]]
+    else
+      [path, nil]
+    end
+  end
 end
 
 Projects::RepositoriesController.prepend_if_ee('EE::Projects::RepositoriesController')

changelogs/unreleased/sh-optimize-archive-controller-ref-names.yml

+---
+title: Optimize ref name lookups in archive downloads
+merge_request: 23890
+author:
+type: performance

spec/controllers/projects/repositories_controller_spec.rb

@@ -17,6 +17,7 @@ describe Projects::RepositoriesController do
 
     context 'as a user' do
       let(:user) { create(:user) }
+      let(:archive_name) { "#{project.path}-master" }
 
       before do
         project.add_developer(user)
@@ -30,9 +31,18 @@ describe Projects::RepositoriesController do
       end
 
       it 'responds with redirect to the short name archive if fully qualified' do
-        get :archive, params: { namespace_id: project.namespace, project_id: project, id: "master/#{project.path}-master" }, format: "zip"
+        get :archive, params: { namespace_id: project.namespace, project_id: project, id: "master/#{archive_name}" }, format: "zip"
 
         expect(assigns(:ref)).to eq("master")
+        expect(assigns(:filename)).to eq(archive_name)
+        expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-archive:")
+      end
+
+      it 'responds with redirect for a path with multiple slashes' do
+        get :archive, params: { namespace_id: project.namespace, project_id: project, id: "improve/awesome/#{archive_name}" }, format: "zip"
+
+        expect(assigns(:ref)).to eq("improve/awesome")
+        expect(assigns(:filename)).to eq(archive_name)
         expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-archive:")
       end