Commit 879b8e84 authored by Andreas Brandl's avatar Andreas Brandl

Efficiently check for next page

This assumes pagination always comes last and we don't have to return an
AR relation from the pagination code. Otherwise we'd have to perform two
queries.
parent 00dfe044
......@@ -82,7 +82,6 @@ module API
def present_projects(projects, options = {})
projects = reorder_projects(projects)
projects = apply_filters(projects)
projects = paginate(projects)
projects, options = with_custom_attributes(projects, options)
options = options.reverse_merge(
......@@ -93,7 +92,10 @@ module API
)
options[:with] = Entities::BasicProjectDetails if params[:simple]
present options[:with].prepare_relation(projects, options), options
projects = options[:with].prepare_relation(projects, options)
projects = paginate(projects)
present projects, options
end
def translate_params_for_compatibility(params)
......
......@@ -11,21 +11,30 @@ module Gitlab
end
def paginate(relation)
relation = relation.limit(page.per_page) # rubocop: disable CodeReuse/ActiveRecord
# Validate an assumption we're making (TODO: subject to be removed)
check_order!(relation)
apply_headers(relation.last)
# This performs the database query and retrieves records
# We retrieve one record more to check if we have data beyond this page
all_records = relation.limit(page.per_page + 1).to_a # rubocop: disable CodeReuse/ActiveRecord
records_for_page = all_records.first(page.per_page)
relation
# If we retrieved more records than belong on this page,
# we know there's a next page
there_is_more = all_records.size > records_for_page.size
apply_headers(records_for_page.last, there_is_more)
records_for_page
end
private
def apply_headers(last_record_in_page)
def apply_headers(last_record_in_page, there_is_more)
end_reached = last_record_in_page.nil? || !there_is_more
lower_bounds = last_record_in_page&.slice(page.order_by.keys)
next_page = page.next(lower_bounds, last_record_in_page.nil?)
next_page = page.next(lower_bounds, end_reached)
request.apply_headers(next_page)
end
......
......@@ -5,22 +5,16 @@ require 'spec_helper'
describe Gitlab::Pagination::Keyset::Pager do
let(:relation) { Project.all.order(id: :asc) }
let(:request) { double('request', page: page, apply_headers: nil) }
let(:page) { double('page', per_page: 20, order_by: { id: :asc }, lower_bounds: nil, next: nil) }
let(:page) { Gitlab::Pagination::Keyset::Page.new(order_by: { id: :asc }, per_page: 20) }
let(:next_page) { double('next page') }
before_all do
create_list(:project, 5)
create_list(:project, 25)
end
describe '#paginate' do
subject { described_class.new(request).paginate(relation) }
it 'applies a limit' do
expect(relation).to receive(:limit).with(page.per_page).and_call_original
subject
end
it 'loads the result relation only once' do
expect do
subject
......@@ -28,15 +22,27 @@ describe Gitlab::Pagination::Keyset::Pager do
end
it 'passes information about next page to request' do
lower_bounds = relation.last.slice(:id)
lower_bounds = relation.limit(20).last.slice(:id)
expect(page).to receive(:next).with(lower_bounds, false).and_return(next_page)
expect(request).to receive(:apply_headers).with(next_page)
subject
end
it 'returns the limited relation' do
expect(subject).to eq(relation.limit(20))
context 'while retrieving the last page' do
let(:relation) { Project.where('id >= ?', Project.maximum(:id) - 10).order(id: :asc) }
it 'indicates this is the last page' do
expect(request).to receive(:apply_headers) do |next_page|
expect(next_page.end_reached?).to be_truthy
end
subject
end
end
it 'returns an array with the loaded records' do
expect(subject).to eq(relation.limit(20).to_a)
end
end
end
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