Skip to content

G
gitlab-ce
Commit ff60273a authored by Andreas Brandl's avatar Andreas Brandl
Browse files
Options

Remove keyset pagination 

This removes the unused keyset pagination implementation. It has never
been matured enough to get enabled. This is a preparation to
re-implement it properly.
parent 92490c32
Hide whitespace changes
Inline Side-by-side
Showing with 1 addition and 337 deletions
lib/api/helpers/pagination.rb
View file @ ff60273a
......@@ -4,13 +4,7 @@ module API
module Helpers
module Pagination
def paginate(relation)
strategy = if params[:pagination] == 'keyset' && Feature.enabled?('api_keyset_pagination')
KeysetPaginationStrategy
else
DefaultPaginationStrategy
end
strategy.new(self).paginate(relation)
DefaultPaginationStrategy.new(self).paginate(relation)
end
class Base
......@@ -40,148 +34,6 @@ module API
end
end
class KeysetPaginationInfo
attr_reader :relation, :request_context
def initialize(relation, request_context)
# This is because it's rather complex to support multiple values with possibly different sort directions
# (and we don't need this in the API)
if relation.order_values.size > 1
raise "Pagination only supports ordering by a single column." \
"The following columns were given: #{relation.order_values.map { |v| v.expr.name }}"
end
@relation = relation
@request_context = request_context
end
def fields
keys.zip(values).reject { |_, v| v.nil? }.to_h
end
def column_for_order_by(relation)
relation.order_values.first&.expr&.name
end
# Sort direction (`:asc` or `:desc`)
def sort
@sort ||= if order_by_primary_key?
# Default order is by id DESC
:desc
else
# API defaults to DESC order if param `sort` not present
request_context.params[:sort]&.to_sym || :desc
end
end
# Do we only sort by primary key?
def order_by_primary_key?
keys.size == 1 && keys.first == primary_key
end
def primary_key
relation.model.primary_key.to_sym
end
def sort_ascending?
sort == :asc
end
# Build hash of request parameters for a given record (relevant to pagination)
def params_for(record)
return {} unless record
keys.each_with_object({}) do |key, h|
h["ks_prev_#{key}".to_sym] = record.attributes[key.to_s]
end
end
private
# All values present in request parameters that correspond to #keys.
def values
@values ||= keys.map do |key|
request_context.params["ks_prev_#{key}".to_sym]
end
end
# All keys relevant to pagination.
# This always includes the primary key. Optionally, the `order_by` key is prepended.
def keys
@keys ||= [column_for_order_by(relation), primary_key].compact.uniq
end
end
class KeysetPaginationStrategy < Base
attr_reader :request_context
delegate :params, :header, :request, to: :request_context
def initialize(request_context)
@request_context = request_context
end
# rubocop: disable CodeReuse/ActiveRecord
def paginate(relation)
pagination = KeysetPaginationInfo.new(relation, request_context)
paged_relation = relation.limit(per_page)
if conds = conditions(pagination)
paged_relation = paged_relation.where(*conds)
end
# In all cases: sort by primary key (possibly in addition to another sort column)
paged_relation = paged_relation.order(pagination.primary_key => pagination.sort)
add_default_pagination_headers
if last_record = paged_relation.last
next_page_params = pagination.params_for(last_record)
add_navigation_links(next_page_params)
end
paged_relation
end
# rubocop: enable CodeReuse/ActiveRecord
private
def conditions(pagination)
fields = pagination.fields
return if fields.empty?
placeholder = fields.map { '?' }
comp = if pagination.sort_ascending?
'>'
else
'<'
end
[
# Row value comparison:
# (A, B) < (a, b) <=> (A < a) OR (A = a AND B < b)
# <=> A <= a AND ((A < a) OR (A = a AND B < b))
"(#{fields.keys.join(',')}) #{comp} (#{placeholder.join(',')})",
*fields.values
]
end
def add_default_pagination_headers
header 'X-Per-Page', per_page.to_s
end
def add_navigation_links(next_page_params)
header 'X-Next-Page', page_href(next_page_params)
header 'Link', link_for('next', next_page_params)
end
def link_for(rel, next_page_params)
%(<#{page_href(next_page_params)}>; rel="#{rel}")
end
end
class DefaultPaginationStrategy < Base
attr_reader :request_context
delegate :params, :header, :request, to: :request_context
......
spec/lib/api/helpers/pagination_spec.rb
View file @ ff60273a
......@@ -15,194 +15,6 @@ describe API::Helpers::Pagination do
Class.new.include(described_class).new
end
describe '#paginate (keyset pagination)' do
let(:value) { spy('return value') }
let(:base_query) do
{
pagination: 'keyset',
foo: 'bar',
bar: 'baz'
}
end
let(:query) { base_query }
before do
allow(subject).to receive(:header).and_return(value)
allow(subject).to receive(:params).and_return(query)
allow(subject).to receive(:request).and_return(double(url: "#{incoming_api_projects_url}?#{query.to_query}"))
end
context 'when resource can be paginated' do
let!(:projects) do
[
create(:project, name: 'One'),
create(:project, name: 'Two'),
create(:project, name: 'Three')
].sort_by { |e| -e.id } # sort by id desc (this is the default sort order for the API)
end
describe 'first page' do
let(:query) { base_query.merge(per_page: 2) }
it 'returns appropriate amount of resources' do
expect(subject.paginate(resource).count).to eq 2
end
it 'returns the first two records (by id desc)' do
expect(subject.paginate(resource)).to eq(projects[0..1])
end
it 'adds appropriate headers' do
expect_header('X-Per-Page', '2')
expect_header('X-Next-Page', "#{incoming_api_projects_url}?#{query.merge(ks_prev_id: projects[1].id).to_query}")
expect_header('Link', anything) do |_key, val|
expect(val).to include('rel="next"')
end
subject.paginate(resource)
end
end
describe 'second page' do
let(:query) { base_query.merge(per_page: 2, ks_prev_id: projects[1].id) }
it 'returns appropriate amount of resources' do
expect(subject.paginate(resource).count).to eq 1
end
it 'returns the third record' do
expect(subject.paginate(resource)).to eq(projects[2..2])
end
it 'adds appropriate headers' do
expect_header('X-Per-Page', '2')
expect_header('X-Next-Page', "#{incoming_api_projects_url}?#{query.merge(ks_prev_id: projects[2].id).to_query}")
expect_header('Link', anything) do |_key, val|
expect(val).to include('rel="next"')
end
subject.paginate(resource)
end
end
describe 'third page' do
let(:query) { base_query.merge(per_page: 2, ks_prev_id: projects[2].id) }
it 'returns appropriate amount of resources' do
expect(subject.paginate(resource).count).to eq 0
end
it 'adds appropriate headers' do
expect_header('X-Per-Page', '2')
expect_no_header('X-Next-Page')
expect(subject).not_to receive(:header).with('Link')
subject.paginate(resource)
end
end
context 'if order' do
context 'is not present' do
let(:query) { base_query.merge(per_page: 2) }
it 'is not present it adds default order(:id) desc' do
resource.order_values = []
paginated_relation = subject.paginate(resource)
expect(resource.order_values).to be_empty
expect(paginated_relation.order_values).to be_present
expect(paginated_relation.order_values.size).to eq(1)
expect(paginated_relation.order_values.first).to be_descending
expect(paginated_relation.order_values.first.expr.name).to eq 'id'
end
end
context 'is present' do
let(:resource) { Project.all.order(name: :desc) }
let!(:projects) do
[
create(:project, name: 'One'),
create(:project, name: 'Two'),
create(:project, name: 'Three'),
create(:project, name: 'Three'), # Note the duplicate name
create(:project, name: 'Four'),
create(:project, name: 'Five'),
create(:project, name: 'Six')
]
# if we sort this by name descending, id descending, this yields:
# {
# 2 => "Two",
# 4 => "Three",
# 3 => "Three",
# 7 => "Six",
# 1 => "One",
# 5 => "Four",
# 6 => "Five"
# }
#
# (key is the id)
end
it 'also orders by primary key' do
paginated_relation = subject.paginate(resource)
expect(paginated_relation.order_values).to be_present
expect(paginated_relation.order_values.size).to eq(2)
expect(paginated_relation.order_values.first).to be_descending
expect(paginated_relation.order_values.first.expr.name).to eq 'name'
expect(paginated_relation.order_values.second).to be_descending
expect(paginated_relation.order_values.second.expr.name).to eq 'id'
end
it 'returns the right records (first page)' do
result = subject.paginate(resource)
expect(result.first).to eq(projects[1])
expect(result.second).to eq(projects[3])
end
describe 'second page' do
let(:query) { base_query.merge(ks_prev_id: projects[3].id, ks_prev_name: projects[3].name, per_page: 2) }
it 'returns the right records (second page)' do
result = subject.paginate(resource)
expect(result.first).to eq(projects[2])
expect(result.second).to eq(projects[6])
end
it 'returns the right link to the next page' do
expect_header('X-Per-Page', '2')
expect_header('X-Next-Page', "#{incoming_api_projects_url}?#{query.merge(ks_prev_id: projects[6].id, ks_prev_name: projects[6].name).to_query}")
expect_header('Link', anything) do |_key, val|
expect(val).to include('rel="next"')
end
subject.paginate(resource)
end
end
describe 'third page' do
let(:query) { base_query.merge(ks_prev_id: projects[6].id, ks_prev_name: projects[6].name, per_page: 5) }
it 'returns the right records (third page), note increased per_page' do
result = subject.paginate(resource)
expect(result.size).to eq(3)
expect(result.first).to eq(projects[0])
expect(result.second).to eq(projects[4])
expect(result.last).to eq(projects[5])
end
end
end
end
end
end
describe '#paginate (default offset-based pagination)' do
let(:value) { spy('return value') }
let(:base_query) { { foo: 'bar', bar: 'baz' } }
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7