Merge branch '340591_use_gitaly_pagination' into 'master'

Add keyset pagination for tags API

See merge request gitlab-org/gitlab!74239

app/finders/tags_finder.rb

@@ -5,9 +5,36 @@ class TagsFinder < GitRefsFinder
     super(repository, params)
   end
 
-  def execute
-    tags = repository.tags_sorted_by(sort)
+  def execute(gitaly_pagination: false)
+    tags = if gitaly_pagination
+             repository.tags_sorted_by(sort, pagination_params)
+           else
+             repository.tags_sorted_by(sort)
+           end
 
     by_search(tags)
+
+  rescue ArgumentError => e
+    raise Gitlab::Git::InvalidPageToken, "Invalid page token: #{page_token}" if e.message.include?('page token')
+
+    raise
+  end
+
+  def total
+    repository.tag_count
+  end
+
+  private
+
+  def per_page
+    params[:per_page].presence
+  end
+
+  def page_token
+    "#{Gitlab::Git::TAG_REF_PREFIX}#{@params[:page_token]}" if params[:page_token]
+  end
+
+  def pagination_params
+    { limit: per_page, page_token: page_token }
   end
 end

config/feature_flags/development/tag_list_keyset_pagination.yml

+---
+name: tag_list_keyset_pagination
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74239
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/345595
+milestone: '14.5'
+type: development
+group: group::source code
+default_enabled: false

lib/api/tags.rb

@@ -21,14 +21,17 @@ module API
         optional :order_by, type: String, values: %w[name updated], default: 'updated',
                             desc: 'Return tags ordered by `name` or `updated` fields.'
         optional :search, type: String, desc: 'Return list of tags matching the search criteria'
+        optional :page_token, type: String, desc: 'Name of tag to start the paginaition from'
         use :pagination
       end
       get ':id/repository/tags', feature_category: :source_code_management, urgency: :low do
-        tags = ::TagsFinder.new(user_project.repository,
+        tags_finder = ::TagsFinder.new(user_project.repository,
                                 sort: "#{params[:order_by]}_#{params[:sort]}",
-                                search: params[:search]).execute
+                                search: params[:search],
+                                page_token: params[:page_token],
+                                per_page: params[:per_page])
 
-        paginated_tags = paginate(::Kaminari.paginate_array(tags))
+        paginated_tags = Gitlab::Pagination::GitalyKeysetPager.new(self, user_project).paginate(tags_finder)
 
         if Feature.enabled?(:api_caching_tags, user_project, type: :development)
           present_cached paginated_tags, with: Entities::Tag, project: user_project, cache_context: -> (_tag) { user_project.cache_key }
@@ -36,6 +39,8 @@ module API
           present paginated_tags, with: Entities::Tag, project: user_project
         end
 
+      rescue Gitlab::Git::InvalidPageToken => e
+        unprocessable_entity!(e.message)
       rescue Gitlab::Git::CommandError
         service_unavailable!
       end

lib/gitlab/git.rb

@@ -17,6 +17,7 @@ module Gitlab
     OSError = Class.new(BaseError)
     UnknownRef = Class.new(BaseError)
     CommandTimedOut = Class.new(CommandError)
+    InvalidPageToken = Class.new(BaseError)
 
     class << self
       include Gitlab::EncodingHelper

lib/gitlab/pagination/gitaly_keyset_pager.rb

@@ -30,6 +30,8 @@ module Gitlab
 
         if finder.is_a?(BranchesFinder)
           Feature.enabled?(:branch_list_keyset_pagination, project, default_enabled: :yaml)
+        elsif finder.is_a?(TagsFinder)
+          Feature.enabled?(:tag_list_keyset_pagination, project, default_enabled: :yaml)
         elsif finder.is_a?(::Repositories::TreeFinder)
           Feature.enabled?(:repository_tree_gitaly_pagination, project, default_enabled: :yaml)
         else
@@ -42,6 +44,8 @@ module Gitlab
 
         if finder.is_a?(BranchesFinder)
           Feature.enabled?(:branch_list_keyset_pagination, project, default_enabled: :yaml)
+        elsif finder.is_a?(TagsFinder)
+          Feature.enabled?(:tag_list_keyset_pagination, project, default_enabled: :yaml)
         elsif finder.is_a?(::Repositories::TreeFinder)
           Feature.enabled?(:repository_tree_gitaly_pagination, project, default_enabled: :yaml)
         else

spec/finders/tags_finder_spec.rb

@@ -7,8 +7,8 @@ RSpec.describe TagsFinder do
   let_it_be(:project) { create(:project, :repository) }
   let_it_be(:repository) { project.repository }
 
-  def load_tags(params)
-    described_class.new(repository, params).execute
+  def load_tags(params, gitaly_pagination: false)
+    described_class.new(repository, params).execute(gitaly_pagination: gitaly_pagination)
   end
 
   describe '#execute' do
@@ -96,6 +96,72 @@ RSpec.describe TagsFinder do
       end
     end
 
+    context 'with Gitaly pagination' do
+      subject { load_tags(params, gitaly_pagination: true) }
+
+      context 'by page_token and per_page' do
+        let(:params) { { page_token: 'v1.0.0', per_page: 1 } }
+
+        it 'filters tags' do
+          result = subject
+
+          expect(result.map(&:name)).to eq(%w(v1.1.0))
+        end
+      end
+
+      context 'by next page_token and per_page' do
+        let(:params) { { page_token: 'v1.1.0', per_page: 2 } }
+
+        it 'filters branches' do
+          result = subject
+
+          expect(result.map(&:name)).to eq(%w(v1.1.1))
+        end
+      end
+
+      context 'by per_page only' do
+        let(:params) { { per_page: 2 } }
+
+        it 'filters branches' do
+          result = subject
+
+          expect(result.map(&:name)).to eq(%w(v1.0.0 v1.1.0))
+        end
+      end
+
+      context 'by page_token only' do
+        let(:params) { { page_token: 'feature' } }
+
+        it 'raises an error' do
+          expect do
+            subject
+          end.to raise_error(Gitlab::Git::InvalidPageToken, 'Invalid page token: refs/tags/feature')
+        end
+      end
+
+      context 'pagination and sort' do
+        context 'by per_page' do
+          let(:params) { { sort: 'updated_desc', per_page: 5 } }
+
+          it 'filters branches' do
+            result = subject
+
+            expect(result.map(&:name)).to eq(%w(v1.1.1 v1.1.0 v1.0.0))
+          end
+        end
+
+        context 'by page_token and per_page' do
+          let(:params) { { sort: 'updated_desc', page_token: 'v1.1.1', per_page: 2 } }
+
+          it 'filters branches' do
+            result = subject
+
+            expect(result.map(&:name)).to eq(%w(v1.1.0 v1.0.0))
+          end
+        end
+      end
+    end
+
     context 'when Gitaly is unavailable' do
       it 'raises an exception' do
         expect(Gitlab::GitalyClient).to receive(:call).and_raise(GRPC::Unavailable)

spec/requests/api/tags_spec.rb

@@ -17,6 +17,10 @@ RSpec.describe API::Tags do
   end
 
   describe 'GET /projects/:id/repository/tags' do
+    before do
+      stub_feature_flags(tag_list_keyset_pagination: false)
+    end
+
     shared_examples "get repository tags" do
       let(:route) { "/projects/#{project_id}/repository/tags" }
 
@@ -143,6 +147,55 @@ RSpec.describe API::Tags do
           expect(expected_tag['release']['description']).to eq(description)
         end
       end
+
+      context 'with keyset pagination on', :aggregate_errors do
+        before do
+          stub_feature_flags(tag_list_keyset_pagination: true)
+        end
+
+        context 'with keyset pagination option' do
+          let(:base_params) { { pagination: 'keyset' } }
+
+          context 'with gitaly pagination params' do
+            context 'with high limit' do
+              let(:params) { base_params.merge(per_page: 100) }
+
+              it 'returns all repository tags' do
+                get api(route, user), params: params
+
+                expect(response).to have_gitlab_http_status(:ok)
+                expect(response).to match_response_schema('public_api/v4/tags')
+                expect(response.headers).not_to include('Link')
+                tag_names = json_response.map { |x| x['name'] }
+                expect(tag_names).to match_array(project.repository.tag_names)
+              end
+            end
+
+            context 'with low limit' do
+              let(:params) { base_params.merge(per_page: 2) }
+
+              it 'returns limited repository tags' do
+                get api(route, user), params: params
+
+                expect(response).to have_gitlab_http_status(:ok)
+                expect(response).to match_response_schema('public_api/v4/tags')
+                expect(response.headers).to include('Link')
+                tag_names = json_response.map { |x| x['name'] }
+                expect(tag_names).to match_array(%w(v1.1.0 v1.1.1))
+              end
+            end
+
+            context 'with missing page token' do
+              let(:params) { base_params.merge(page_token: 'unknown') }
+
+              it_behaves_like '422 response' do
+                let(:request) { get api(route, user), params: params }
+                let(:message) { 'Invalid page token: refs/tags/unknown' }
+              end
+            end
+          end
+        end
+      end
     end
 
     context ":api_caching_tags flag enabled", :use_clean_rails_memory_store_caching do

spec/support/shared_examples/requests/api/status_shared_examples.rb

@@ -77,6 +77,24 @@ RSpec.shared_examples '412 response' do
   end
 end
 
+RSpec.shared_examples '422 response' do
+  let(:message) { nil }
+
+  before do
+    # Fires the request
+    request
+  end
+
+  it 'returns 422' do
+    expect(response).to have_gitlab_http_status(:unprocessable_entity)
+    expect(json_response).to be_an Object
+
+    if message.present?
+      expect(json_response['message']).to eq(message)
+    end
+  end
+end
+
 RSpec.shared_examples '503 response' do
   before do
     # Fires the request