Keyset pagination (id based)

First attempt of adding keyset pagination based on the id column.

lib/api/helpers/pagination.rb

@@ -4,7 +4,12 @@ module API
   module Helpers
     module Pagination
       def paginate(relation)
-        ::Gitlab::Pagination::OffsetPagination.new(self).paginate(relation)
+        if params[:pagination] == "keyset"
+          request_context = Gitlab::Pagination::Keyset::RequestContext.new(self)
+          Gitlab::Pagination::Keyset.paginate(request_context, relation)
+        else
+          Gitlab::Pagination::OffsetPagination.new(self).paginate(relation)
+        end
       end
     end
   end

lib/gitlab/pagination/keyset.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Pagination
+    module Keyset
+      def self.paginate(request_context, relation)
+        paged_relation = Gitlab::Pagination::Keyset::Pager.new(request_context).paginate(relation)
+
+        request_context.apply_headers(paged_relation)
+
+        paged_relation.relation
+      end
+    end
+  end
+end

lib/gitlab/pagination/keyset/page.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Pagination
+    module Keyset
+      class Page
+        DEFAULT_PAGE_SIZE = 20
+
+        attr_reader :last_value, :column
+
+        def initialize(last_value, column: :id, per_page: DEFAULT_PAGE_SIZE, is_first_page: false)
+          @last_value = last_value
+          @column = column
+          @per_page = per_page || DEFAULT_PAGE_SIZE
+          @is_first_page = is_first_page
+        end
+
+        def per_page
+          return DEFAULT_PAGE_SIZE if @per_page <= 0
+
+          [@per_page, DEFAULT_PAGE_SIZE].min
+        end
+
+        def empty?
+          last_value.nil? && !first_page?
+        end
+
+        def first_page?
+          @is_first_page
+        end
+      end
+    end
+  end
+end

lib/gitlab/pagination/keyset/paged_relation.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Pagination
+    module Keyset
+      class PagedRelation
+        attr_reader :relation, :page
+
+        def initialize(relation, page)
+          @relation = relation
+          @page = page
+        end
+
+        # return Page information for next page
+        def next_page
+          last_record_in_page = relation.last
+          last_value = last_record_in_page&.read_attribute(page.column)
+
+          Page.new(last_value, column: page.column, per_page: page.per_page)
+        end
+      end
+    end
+  end
+end

lib/gitlab/pagination/keyset/pager.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Pagination
+    module Keyset
+      class Pager
+        attr_reader :request
+
+        def initialize(request)
+          @request = request
+        end
+
+        def paginate(relation)
+          paged_relation = relation.limit(page.per_page).reorder(page.column => :asc) # rubocop: disable CodeReuse/ActiveRecord
+
+          if val = page.last_value
+            # TODO: check page.column is valid
+            paged_relation = paged_relation.where("#{page.column} > ?", val) # rubocop: disable CodeReuse/ActiveRecord
+          end
+
+          PagedRelation.new(paged_relation, page)
+        end
+
+        private
+
+        def page
+          @page ||= request.page
+        end
+      end
+    end
+  end
+end

lib/gitlab/pagination/keyset/request_context.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Pagination
+    module Keyset
+      class RequestContext
+        attr_reader :request
+
+        REQUEST_PARAM = :id_after
+
+        def initialize(request)
+          @request = request
+        end
+
+        # extracts Paging information from request parameters
+        def page
+          last_value = request.params[REQUEST_PARAM]
+
+          Page.new(last_value, per_page: request.params[:per_page], is_first_page: !last_value.nil?)
+        end
+
+        def apply_headers(paged_relation)
+          next_page = paged_relation.next_page
+          links = pagination_links(next_page)
+
+          request.header('Links', links.join(', '))
+        end
+
+        private
+
+        def pagination_links(next_page)
+          [].tap do |links|
+            links << %(<#{page_href}>; rel="first")
+            links << %(<#{page_href(next_page)}>; rel="next") unless next_page.empty?
+          end
+        end
+
+        def base_request_uri
+          @base_request_uri ||= URI.parse(request.request.url).tap do |uri|
+            uri.host = Gitlab.config.gitlab.host
+            uri.port = Gitlab.config.gitlab.port
+          end
+        end
+
+        def query_params_for(page)
+          if page && !page.empty?
+            request.params.merge(REQUEST_PARAM => page.last_value)
+          else
+            request.params.except(REQUEST_PARAM)
+          end
+        end
+
+        def page_href(page = nil)
+          base_request_uri.tap do |uri|
+            uri.query = query_params_for(page).to_query
+          end.to_s
+        end
+      end
+    end
+  end
+end

spec/lib/api/helpers/pagination_spec.rb

@@ -10,6 +10,13 @@ describe API::Helpers::Pagination do
     let(:offset_pagination) { double("offset pagination") }
     let(:expected_result) { double("result") }
 
+    before do
+      allow(subject).to receive(:params).and_return(params)
+    end
+
+    context 'for offset pagination' do
+      let(:params) { {} }
+
       it 'delegates to OffsetPagination' do
         expect(::Gitlab::Pagination::OffsetPagination).to receive(:new).with(subject).and_return(offset_pagination)
         expect(offset_pagination).to receive(:paginate).with(relation).and_return(expected_result)
@@ -19,4 +26,19 @@ describe API::Helpers::Pagination do
         expect(result).to eq(expected_result)
       end
     end
+
+    context 'for keyset pagination' do
+      let(:params) { { pagination: 'keyset' } }
+      let(:request_context) { double('request context') }
+
+      it 'delegates to KeysetPagination' do
+        expect(Gitlab::Pagination::Keyset::RequestContext).to receive(:new).with(subject).and_return(request_context)
+        expect(Gitlab::Pagination::Keyset).to receive(:paginate).with(request_context, relation).and_return(expected_result)
+
+        result = subject.paginate(relation)
+
+        expect(result).to eq(expected_result)
+      end
+    end
+  end
 end

spec/lib/gitlab/pagination/keyset/page_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Pagination::Keyset::Page do
+  describe '#per_page' do
+    it 'limits to a maximum of 20 records per page' do
+      per_page = described_class.new(double, per_page: 21).per_page
+
+      expect(per_page).to eq(described_class::DEFAULT_PAGE_SIZE)
+    end
+
+    it 'uses default value when given 0' do
+      per_page = described_class.new(double, per_page: 0).per_page
+
+      expect(per_page).to eq(described_class::DEFAULT_PAGE_SIZE)
+    end
+
+    it 'uses default value when given negative values' do
+      per_page = described_class.new(double, per_page: -1).per_page
+
+      expect(per_page).to eq(described_class::DEFAULT_PAGE_SIZE)
+    end
+  end
+end

spec/lib/gitlab/pagination/keyset/paged_relation_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Pagination::Keyset::PagedRelation do
+  before_all do
+    create_list(:project, 10)
+  end
+
+  let(:relation) { Project.all.limit(page.per_page) }
+  let(:page) { double('page', column: :id, per_page: 5) }
+
+  describe '#next_page' do
+    subject { described_class.new(relation, page).next_page }
+
+    it 'retrieves the last record on the page to establish a last_value for the page' do
+      next_page = subject
+
+      expect(next_page.last_value).to eq(relation.last.id)
+      expect(next_page.column).to eq(page.column)
+      expect(next_page.per_page).to eq(page.per_page)
+    end
+
+    context 'when the page is empty' do
+      let(:relation) { Project.none }
+
+      it 'returns a Page indicating its emptiness' do
+        next_page = subject
+
+        expect(next_page.empty?).to be_truthy
+        expect(next_page.column).to eq(page.column)
+        expect(next_page.per_page).to eq(page.per_page)
+      end
+    end
+  end
+end

spec/lib/gitlab/pagination/keyset/pager_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Pagination::Keyset::Pager do
+  let(:relation) { Project.all }
+  let(:request) { double('request', page: page) }
+  let(:page) { double('page', per_page: 20, column: :id, last_value: 10) }
+
+  describe '#paginate' do
+    subject { described_class.new(request).paginate(relation) }
+
+    it 'applies a limit' do
+      allow(relation).to receive(:order).and_return(relation)
+      expect(relation).to receive(:limit).with(page.per_page).and_call_original
+
+      subject
+    end
+
+    it 'sorts by pagination order' do
+      allow(relation).to receive(:limit).and_return(relation)
+      expect(relation).to receive(:reorder).with(page.column => :asc).and_call_original
+
+      subject
+    end
+
+    context 'without paging information' do
+      let(:page) { double('page', per_page: 20, column: :id, last_value: nil) }
+
+      it 'considers this the first page and does not apply any filter' do
+        allow(relation).to receive(:limit).and_return(relation)
+
+        expect(relation).not_to receive(:where)
+
+        subject
+      end
+    end
+
+    it 'applies a filter based on the paging information' do
+      allow(relation).to receive(:limit).and_return(relation)
+      allow(relation).to receive(:order).and_return(relation)
+
+      expect(relation).to receive(:where).with('id > ?', 10).and_call_original
+
+      subject
+    end
+
+    it 'adds limit, order,where to the query' do
+      expect(subject.relation).to eq(Project.where('id > ?', page.last_value).limit(page.per_page).order(id: :asc))
+    end
+
+    it 'passes through the page information' do
+      expect(subject.page).to eq(page)
+    end
+  end
+end

spec/lib/gitlab/pagination/keyset/request_context_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Pagination::Keyset::RequestContext do
+  let(:request) { double('request', params: params) }
+  let(:params) { { id_after: 5, per_page: 10 } }
+
+  describe '#page' do
+    subject { described_class.new(request).page }
+
+    it 'extracts last_value information' do
+      page = subject
+
+      expect(page.last_value).to eq(params[:id_after])
+    end
+
+    it 'extracts per_page information' do
+      page = subject
+
+      expect(page.per_page).to eq(params[:per_page])
+    end
+
+    context 'with no id_after value present' do
+      let(:params) { { id_after: 5, per_page: 10 } }
+
+      it 'indicates this is the first page' do
+        page = subject
+
+        expect(page.first_page?).to be_truthy
+      end
+    end
+  end
+
+  describe '#apply_headers' do
+    let(:paged_relation) { double('paged relation', next_page: next_page) }
+    let(:request) { double('request', url: "http://#{Gitlab.config.gitlab.host}/api/v4/projects?foo=bar") }
+    let(:params) { { foo: 'bar' } }
+    let(:request_context) { double('request context', params: params, request: request) }
+    let(:next_page) { double('next page', last_value: 42, empty?: false) }
+
+    subject { described_class.new(request_context).apply_headers(paged_relation) }
+
+    it 'sets Links header with a link to the first page' do
+      orig_uri = URI.parse(request_context.request.url)
+
+      expect(request_context).to receive(:header) do |name, header|
+        expect(name).to eq('Links')
+
+        first_link, _ = /<([^>]+)>; rel="first"/.match(header).captures
+
+        URI.parse(first_link).tap do |uri|
+          expect(uri.host).to eq(orig_uri.host)
+          expect(uri.path).to eq(orig_uri.path)
+
+          query = CGI.parse(uri.query)
+          expect(query.except('id_after')).to eq(CGI.parse(orig_uri.query).except("id_after"))
+          expect(query['id_after']).to be_empty
+        end
+      end
+
+      subject
+    end
+
+    it 'sets Links header with a link to the next page' do
+      orig_uri = URI.parse(request_context.request.url)
+
+      expect(request_context).to receive(:header) do |name, header|
+        expect(name).to eq('Links')
+
+        first_link, _ = /<([^>]+)>; rel="next"/.match(header).captures
+
+        URI.parse(first_link).tap do |uri|
+          expect(uri.host).to eq(orig_uri.host)
+          expect(uri.path).to eq(orig_uri.path)
+
+          query = CGI.parse(uri.query)
+          expect(query.except('id_after')).to eq(CGI.parse(orig_uri.query).except("id_after"))
+          expect(query['id_after']).to eq(["42"])
+        end
+      end
+
+      subject
+    end
+  end
+end

spec/lib/gitlab/pagination/keyset_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Pagination::Keyset do
+  describe '.paginate' do
+    subject { described_class.paginate(request_context, relation) }
+
+    let(:request_context) { instance_double(Gitlab::Pagination::Keyset::RequestContext, apply_headers: nil) }
+    let(:pager) { instance_double(Gitlab::Pagination::Keyset::Pager, paginate: paged_relation)}
+    let(:relation) { double('relation') }
+    let(:paged_relation) { double('paged relation', relation: double) }
+
+    before do
+      allow(Gitlab::Pagination::Keyset::Pager).to receive(:new).with(request_context).and_return(pager)
+    end
+
+    it 'applies headers' do
+      expect(request_context).to receive(:apply_headers).with(paged_relation)
+
+      subject
+    end
+
+    it 'returns the paginated relation' do
+      expect(subject).to eq(paged_relation.relation)
+    end
+
+    it 'paginates the relation' do
+      expect(pager).to receive(:paginate).with(relation).and_return(paged_relation)
+
+      subject
+    end
+  end
+end