Merge branch 'sarnold-lower-max-relation-pagination-count' into 'master'

Lower max relation pagination count with feature flag

See merge request gitlab-org/gitlab!69620

config/feature_flags/ops/lower_relation_max_count_limit.yml

+---
+name: lower_relation_max_count_limit
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69620
+rollout_issue_url:
+milestone: '14.3'
+type: ops
+group: group::verify
+default_enabled: false

config/initializers/kaminari_active_record_relation_methods_with_limit.rb

@@ -4,6 +4,7 @@ module Kaminari
   # Active Record specific page scope methods implementations
   module ActiveRecordRelationMethodsWithLimit
     MAX_COUNT_LIMIT = 10_000
+    MAX_COUNT_NEW_LOWER_LIMIT = 1_000
 
     # This is a modified version of
     # https://github.com/kaminari/kaminari/blob/c5186f5d9b7f23299d115408e62047447fd3189d/kaminari-activerecord/lib/kaminari/activerecord/active_record_relation_methods.rb#L17-L41
@@ -21,7 +22,8 @@ module Kaminari
         return @total_count = (current_page - 1) * limit_value + @records.length if @records.any? && (@records.length < limit_value)
       end
 
-      limit = options.fetch(:limit, MAX_COUNT_LIMIT).to_i
+      max_limit = Feature.enabled?(:lower_relation_max_count_limit, type: :ops) ? MAX_COUNT_NEW_LOWER_LIMIT : MAX_COUNT_LIMIT
+      limit = options.fetch(:limit, max_limit).to_i
       # #count overrides the #select which could include generated columns referenced in #order, so skip #order here, where it's irrelevant to the result anyway
       c = except(:offset, :limit, :order)
       # Remove includes only if they are irrelevant

lib/gitlab/pagination/offset_pagination.rb

@@ -29,7 +29,7 @@ module Gitlab
         return pagination_data unless Feature.enabled?(:api_kaminari_count_with_limit, type: :ops)
 
         limited_total_count = pagination_data.total_count_with_limit
-        if limited_total_count > Kaminari::ActiveRecordRelationMethods::MAX_COUNT_LIMIT
+        if limited_total_count > max_limit
           # The call to `total_count_with_limit` memoizes `@arel` because of a call to `references_eager_loaded_tables?`
           # We need to call `reset` because `without_count` relies on `@arel` being unmemoized
           pagination_data.reset.without_count
@@ -38,6 +38,14 @@ module Gitlab
         end
       end
 
+      def max_limit
+        if Feature.enabled?(:lower_relation_max_count_limit, type: :ops)
+          Kaminari::ActiveRecordRelationMethods::MAX_COUNT_NEW_LOWER_LIMIT
+        else
+          Kaminari::ActiveRecordRelationMethods::MAX_COUNT_LIMIT
+        end
+      end
+
       def needs_pagination?(relation)
         return true unless relation.respond_to?(:current_page)
         return true if params[:page].present? && relation.current_page != params[:page].to_i

spec/lib/gitlab/pagination/offset_pagination_spec.rb

@@ -82,7 +82,7 @@ RSpec.describe Gitlab::Pagination::OffsetPagination do
 
         context 'when the api_kaminari_count_with_limit feature flag is enabled' do
           before do
-            stub_feature_flags(api_kaminari_count_with_limit: true)
+            stub_feature_flags(api_kaminari_count_with_limit: true, lower_relation_max_count_limit: false)
           end
 
           context 'when resources count is less than MAX_COUNT_LIMIT' do
@@ -120,6 +120,41 @@ RSpec.describe Gitlab::Pagination::OffsetPagination do
             end
           end
 
+          context 'when lower_relation_max_count_limit FF is enabled' do
+            before do
+              stub_feature_flags(lower_relation_max_count_limit: true)
+            end
+
+            it_behaves_like 'paginated response'
+            it_behaves_like 'response with pagination headers'
+
+            context 'when limit is met' do
+              before do
+                stub_const("::Kaminari::ActiveRecordRelationMethods::MAX_COUNT_NEW_LOWER_LIMIT", 2)
+              end
+
+              it_behaves_like 'paginated response'
+
+              it 'does not return the X-Total and X-Total-Pages headers' do
+                expect_no_header('X-Total')
+                expect_no_header('X-Total-Pages')
+                expect_header('X-Per-Page', '2')
+                expect_header('X-Page', '1')
+                expect_header('X-Next-Page', '2')
+                expect_header('X-Prev-Page', '')
+
+                expect_header('Link', anything) do |_key, val|
+                  expect(val).to include(%Q(<#{incoming_api_projects_url}?#{query.merge(page: 1).to_query}>; rel="first"))
+                  expect(val).to include(%Q(<#{incoming_api_projects_url}?#{query.merge(page: 2).to_query}>; rel="next"))
+                  expect(val).not_to include('rel="last"')
+                  expect(val).not_to include('rel="prev"')
+                end
+
+                subject.paginate(resource)
+              end
+            end
+          end
+
           it 'does not return the total headers when excluding them' do
             expect_no_header('X-Total')
             expect_no_header('X-Total-Pages')