Commit d14aeefd authored by Rémy Coutable's avatar Rémy Coutable

Merge branch '345329-revert-lower_relation_max_count_limit-ff' into 'master'

Remove reference to lower_relation_max_count_limit

See merge request gitlab-org/gitlab!75482
parents 11968f2d 8a3163c3
---
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
...@@ -4,7 +4,6 @@ module Kaminari ...@@ -4,7 +4,6 @@ module Kaminari
# Active Record specific page scope methods implementations # Active Record specific page scope methods implementations
module ActiveRecordRelationMethodsWithLimit module ActiveRecordRelationMethodsWithLimit
MAX_COUNT_LIMIT = 10_000 MAX_COUNT_LIMIT = 10_000
MAX_COUNT_NEW_LOWER_LIMIT = 1_000
# This is a modified version of # 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 # https://github.com/kaminari/kaminari/blob/c5186f5d9b7f23299d115408e62047447fd3189d/kaminari-activerecord/lib/kaminari/activerecord/active_record_relation_methods.rb#L17-L41
...@@ -22,8 +21,7 @@ module Kaminari ...@@ -22,8 +21,7 @@ module Kaminari
return @total_count = (current_page - 1) * limit_value + @records.length if @records.any? && (@records.length < limit_value) return @total_count = (current_page - 1) * limit_value + @records.length if @records.any? && (@records.length < limit_value)
end end
max_limit = Feature.enabled?(:lower_relation_max_count_limit, type: :ops) ? MAX_COUNT_NEW_LOWER_LIMIT : MAX_COUNT_LIMIT limit = options.fetch(:limit, MAX_COUNT_LIMIT).to_i
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 # #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) c = except(:offset, :limit, :order)
# Remove includes only if they are irrelevant # Remove includes only if they are irrelevant
......
...@@ -29,7 +29,7 @@ module Gitlab ...@@ -29,7 +29,7 @@ module Gitlab
return pagination_data unless Feature.enabled?(:api_kaminari_count_with_limit, type: :ops) return pagination_data unless Feature.enabled?(:api_kaminari_count_with_limit, type: :ops)
limited_total_count = pagination_data.total_count_with_limit limited_total_count = pagination_data.total_count_with_limit
if limited_total_count > max_limit if limited_total_count > Kaminari::ActiveRecordRelationMethods::MAX_COUNT_LIMIT
# The call to `total_count_with_limit` memoizes `@arel` because of a call to `references_eager_loaded_tables?` # 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 # We need to call `reset` because `without_count` relies on `@arel` being unmemoized
pagination_data.reset.without_count pagination_data.reset.without_count
...@@ -38,14 +38,6 @@ module Gitlab ...@@ -38,14 +38,6 @@ module Gitlab
end end
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) def needs_pagination?(relation)
return true unless relation.respond_to?(:current_page) return true unless relation.respond_to?(:current_page)
return true if params[:page].present? && relation.current_page != params[:page].to_i return true if params[:page].present? && relation.current_page != params[:page].to_i
......
...@@ -82,7 +82,7 @@ RSpec.describe Gitlab::Pagination::OffsetPagination do ...@@ -82,7 +82,7 @@ RSpec.describe Gitlab::Pagination::OffsetPagination do
context 'when the api_kaminari_count_with_limit feature flag is enabled' do context 'when the api_kaminari_count_with_limit feature flag is enabled' do
before do before do
stub_feature_flags(api_kaminari_count_with_limit: true, lower_relation_max_count_limit: false) stub_feature_flags(api_kaminari_count_with_limit: true)
end end
context 'when resources count is less than MAX_COUNT_LIMIT' do context 'when resources count is less than MAX_COUNT_LIMIT' do
...@@ -120,41 +120,6 @@ RSpec.describe Gitlab::Pagination::OffsetPagination do ...@@ -120,41 +120,6 @@ RSpec.describe Gitlab::Pagination::OffsetPagination do
end end
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 it 'does not return the total headers when excluding them' do
expect_no_header('X-Total') expect_no_header('X-Total')
expect_no_header('X-Total-Pages') expect_no_header('X-Total-Pages')
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment