Use paginate_and_retrieve! for keyset pagination

* API::Helpers::Pagination#paginate only exposes offset pagination
* API::Helpers::Pagination#paginate_and_retrieve! supports both types

I wanted to make it explicit that keyset pagination returns an array
instead of a relation. This does not match the behavior of offset
pagination, hence not mixing those in #paginate.

lib/api/helpers/pagination.rb

@@ -3,8 +3,21 @@
 module API
   module Helpers
     module Pagination
+      # This returns an ActiveRecord relation
       def paginate(relation)
-        return Gitlab::Pagination::OffsetPagination.new(self).paginate(relation) unless keyset_pagination_enabled?
+        Gitlab::Pagination::OffsetPagination.new(self).paginate(relation)
+      end
+
+      # This applies pagination and executes the query
+      # It always returns an array instead of an ActiveRecord relation
+      def paginate_and_retrieve!(relation)
+        offset_or_keyset_pagination(relation).to_a
+      end
+
+      private
+
+      def offset_or_keyset_pagination(relation)
+        return paginate(relation) unless keyset_pagination_enabled?
 
         request_context = Gitlab::Pagination::Keyset::RequestContext.new(self)
 
@@ -15,14 +28,6 @@ module API
         Gitlab::Pagination::Keyset.paginate(request_context, relation)
       end
 
-      # This applies pagination and executes the query
-      # It always returns an array instead of an ActiveRecord relation
-      def paginate_and_retrieve!(relation)
-        paginate(relation).to_a
-      end
-
-      private
-
       def keyset_pagination_enabled?
         params[:pagination] == 'keyset' && Feature.enabled?(:api_keyset_pagination, default_enabled: true)
       end

spec/lib/api/helpers/pagination_spec.rb

@@ -5,25 +5,39 @@ require 'spec_helper'
 describe API::Helpers::Pagination do
   subject { Class.new.include(described_class).new }
 
+  let(:expected_result) { double("result", to_a: double) }
+  let(:relation) { double("relation") }
+  let(:params) { {} }
+
+  before do
+    allow(subject).to receive(:params).and_return(params)
+  end
+
   describe '#paginate' do
-    let(:relation) { double("relation") }
     let(:offset_pagination) { double("offset pagination") }
-    let(:expected_result) { double("result") }
 
-    before do
-      allow(subject).to receive(:params).and_return(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)
+
+      result = subject.paginate(relation)
+
+      expect(result).to eq(expected_result)
     end
+  end
 
+  describe '#paginate_and_retrieve!' do
     context 'for offset pagination' do
-      let(:params) { {} }
+      before do
+        allow(Gitlab::Pagination::Keyset).to receive(:available?).and_return(false)
+      end
 
-      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)
+      it 'delegates to paginate' do
+        expect(subject).to receive(:paginate).with(relation).and_return(expected_result)
 
-        result = subject.paginate(relation)
+        result = subject.paginate_and_retrieve!(relation)
 
-        expect(result).to eq(expected_result)
+        expect(result).to eq(expected_result.to_a)
       end
     end
 
@@ -33,38 +47,28 @@ describe API::Helpers::Pagination do
 
       before do
         allow(Gitlab::Pagination::Keyset::RequestContext).to receive(:new).with(subject).and_return(request_context)
-        allow(Gitlab::Pagination::Keyset).to receive(:available?).and_return(true)
       end
 
-      it 'delegates to KeysetPagination' do
-        expect(Gitlab::Pagination::Keyset).to receive(:paginate).with(request_context, relation).and_return(expected_result)
+      context 'when keyset pagination is available' do
+        it 'delegates to KeysetPagination' do
+          expect(Gitlab::Pagination::Keyset).to receive(:available?).and_return(true)
+          expect(Gitlab::Pagination::Keyset).to receive(:paginate).with(request_context, relation).and_return(expected_result)
 
-        result = subject.paginate(relation)
+          result = subject.paginate_and_retrieve!(relation)
 
-        expect(result).to eq(expected_result)
+          expect(result).to eq(expected_result.to_a)
+        end
       end
 
-      it 'renders a 501 error if keyset pagination isnt available yet' do
-        expect(Gitlab::Pagination::Keyset).to receive(:available?).with(request_context, relation).and_return(false)
-        expect(Gitlab::Pagination::Keyset).not_to receive(:paginate)
-        expect(subject).to receive(:error!).with(/not yet available/, 405)
+      context 'when keyset pagination is not available' do
+        it 'renders a 501 error if keyset pagination isnt available yet' do
+          expect(Gitlab::Pagination::Keyset).to receive(:available?).with(request_context, relation).and_return(false)
+          expect(Gitlab::Pagination::Keyset).not_to receive(:paginate)
+          expect(subject).to receive(:error!).with(/not yet available/, 405)
 
-        subject.paginate(relation)
+          subject.paginate_and_retrieve!(relation)
+        end
       end
     end
   end
-
-  describe '#paginate_and_retrieve!' do
-    let(:relation) { double("relation") }
-
-    let(:paginated_result) { double }
-    let(:result) { double }
-
-    it 'applies pagination and returns an array' do
-      expect(subject).to receive(:paginate).with(relation).and_return(paginated_result)
-      expect(paginated_result).to receive(:to_a).and_return(result)
-
-      expect(subject.paginate_and_retrieve!(relation)).to eq(result)
-    end
-  end
 end