From fa57631212c0ca820c46a5033c721f4fabb6d280 Mon Sep 17 00:00:00 2001
From: Chad Woolley <cwoolley@gitlab.com>
Date: Sun, 17 Jul 2022 17:43:33 -0700
Subject: [PATCH] Allow pagination=none for recursive tree API

- Allows pagination to be set to 'none' for
  projects repository tree API, if recursive
  is set to true.
- Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/365996

Changelog: changed
MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/92624
---
 lib/api/repositories.rb                       | 10 +++++--
 lib/gitlab/pagination/gitaly_keyset_pager.rb  |  8 +++++-
 .../pagination/gitaly_keyset_pager_spec.rb    | 14 ++++++++++
 spec/requests/api/repositories_spec.rb        | 26 +++++++++++++++++++
 4 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb
index 2e21f591667c4d..356c5584dbaa13 100644
--- a/lib/api/repositories.rb
+++ b/lib/api/repositories.rb
@@ -99,11 +99,17 @@ def compare_cache_key(current_user, user_project, target_project, params)
         optional :recursive, type: Boolean, default: false, desc: 'Used to get a recursive tree'
 
         use :pagination
-        optional :pagination, type: String, values: %w(legacy keyset), default: 'legacy', desc: 'Specify the pagination method'
+        optional :pagination, type: String, values: %w(legacy keyset none), default: 'legacy', desc: 'Specify the pagination method ("none" is only valid if "recursive" is true)'
 
-        given pagination: -> (value) { value == 'keyset' } do
+        given pagination: ->(value) { value == 'keyset' } do
           optional :page_token, type: String, desc: 'Record from which to start the keyset pagination'
         end
+
+        given pagination: ->(value) { value == 'none' } do
+          given recursive: ->(value) { value == false } do
+            validates([:pagination], except_values: { value: 'none', message: 'cannot be "none" unless "recursive" is true' })
+          end
+        end
       end
       get ':id/repository/tree', urgency: :low do
         tree_finder = ::Repositories::TreeFinder.new(user_project, declared_params(include_missing: false))
diff --git a/lib/gitlab/pagination/gitaly_keyset_pager.rb b/lib/gitlab/pagination/gitaly_keyset_pager.rb
index 8bbc9a93610a0e..1f1061fe4f1e3d 100644
--- a/lib/gitlab/pagination/gitaly_keyset_pager.rb
+++ b/lib/gitlab/pagination/gitaly_keyset_pager.rb
@@ -12,9 +12,11 @@ def initialize(request_context, project)
         @project = project
       end
 
-      # It is expected that the given finder will respond to `execute` method with `gitaly_pagination: true` option
+      # It is expected that the given finder will respond to `execute` method with `gitaly_pagination:` option
       # and supports pagination via gitaly.
       def paginate(finder)
+        return finder.execute(gitaly_pagination: false) if no_pagination?
+
         return paginate_via_gitaly(finder) if keyset_pagination_enabled?(finder)
         return paginate_first_page_via_gitaly(finder) if paginate_first_page?(finder)
 
@@ -26,6 +28,10 @@ def paginate(finder)
 
       private
 
+      def no_pagination?
+        params[:pagination] == 'none'
+      end
+
       def keyset_pagination_enabled?(finder)
         return false unless params[:pagination] == "keyset"
 
diff --git a/spec/lib/gitlab/pagination/gitaly_keyset_pager_spec.rb b/spec/lib/gitlab/pagination/gitaly_keyset_pager_spec.rb
index dcb8138bdded29..0bafd436bd0e9d 100644
--- a/spec/lib/gitlab/pagination/gitaly_keyset_pager_spec.rb
+++ b/spec/lib/gitlab/pagination/gitaly_keyset_pager_spec.rb
@@ -126,5 +126,19 @@
         end
       end
     end
+
+    context 'with "none" pagination option' do
+      let(:expected_result) { double(:result) }
+      let(:query) { { pagination: 'none' } }
+
+      it 'uses offset pagination' do
+        expect(finder).to receive(:execute).with(gitaly_pagination: false).and_return(expected_result)
+        expect(Kaminari).not_to receive(:paginate_array)
+        expect(Gitlab::Pagination::OffsetPagination).not_to receive(:new)
+
+        actual_result = pager.paginate(finder)
+        expect(actual_result).to eq(expected_result)
+      end
+    end
   end
 end
diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb
index d6d2bd5baf23f8..e4b91a1bc57c17 100644
--- a/spec/requests/api/repositories_spec.rb
+++ b/spec/requests/api/repositories_spec.rb
@@ -92,6 +92,32 @@
           expect(json_response.map { |t| t["id"] }).not_to include(page_token)
         end
       end
+
+      context 'with pagination=none' do
+        context 'with recursive=1' do
+          it 'returns unpaginated recursive project paths tree' do
+            get api("#{route}?recursive=1&pagination=none", current_user)
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(json_response).to be_an Array
+            expect(response).not_to include_pagination_headers
+            expect(json_response[4]['name']).to eq('html')
+            expect(json_response[4]['path']).to eq('files/html')
+            expect(json_response[4]['type']).to eq('tree')
+            expect(json_response[4]['mode']).to eq('040000')
+          end
+        end
+
+        context 'with recursive=0' do
+          it 'returns 400' do
+            get api("#{route}?recursive=0&pagination=none", current_user)
+
+            expect(response).to have_gitlab_http_status(:bad_request)
+            expect(json_response['error'])
+              .to eq('pagination cannot be "none" unless "recursive" is true')
+          end
+        end
+      end
     end
 
     context 'when unauthenticated', 'and project is public' do
-- 
GitLab