Merge branch 'bw-refactor-externallypaginatedarrayconnection' into 'master'

GraphQL: Refactor connections to use new GraphQL::Pagination classes

See merge request gitlab-org/gitlab!28446

app/graphql/gitlab_schema.rb

@@ -10,11 +10,12 @@ class GitlabSchema < GraphQL::Schema
   DEFAULT_MAX_DEPTH = 15
   AUTHENTICATED_MAX_DEPTH = 20
 
+  use GraphQL::Pagination::Connections
   use BatchLoader::GraphQL
   use Gitlab::Graphql::Authorize
   use Gitlab::Graphql::Present
   use Gitlab::Graphql::CallsGitaly
-  use Gitlab::Graphql::Connections
+  use Gitlab::Graphql::Pagination::Connections
   use Gitlab::Graphql::GenericTracing
   use Gitlab::Graphql::Timeout, max_seconds: Gitlab.config.gitlab.graphql_timeout
 

lib/gitlab/graphql/connections.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module Graphql
-    module Connections
-      def self.use(_schema)
-        GraphQL::Relay::BaseConnection.register_connection_implementation(
-          ActiveRecord::Relation,
-          Gitlab::Graphql::Connections::Keyset::Connection
-        )
-        GraphQL::Relay::BaseConnection.register_connection_implementation(
-          Gitlab::Graphql::FilterableArray,
-          Gitlab::Graphql::Connections::FilterableArrayConnection
-        )
-        GraphQL::Relay::BaseConnection.register_connection_implementation(
-          Gitlab::Graphql::ExternallyPaginatedArray,
-          Gitlab::Graphql::Connections::ExternallyPaginatedArrayConnection
-        )
-      end
-    end
-  end
-end

lib/gitlab/graphql/pagination/connections.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Graphql
+    module Pagination
+      module Connections
+        def self.use(schema)
+          schema.connections.add(
+            ActiveRecord::Relation,
+            Gitlab::Graphql::Pagination::Keyset::Connection)
+
+          schema.connections.add(
+            Gitlab::Graphql::FilterableArray,
+            Gitlab::Graphql::Pagination::FilterableArrayConnection)
+
+          schema.connections.add(
+            Gitlab::Graphql::ExternallyPaginatedArray,
+            Gitlab::Graphql::Pagination::ExternallyPaginatedArrayConnection)
+        end
+      end
+    end
+  end
+end

lib/gitlab/graphql/connections/externally_paginated_array_connection.rb → lib/gitlab/graphql/pagination/externally_paginated_array_connection.rb

@@ -3,20 +3,14 @@
 # Make a customized connection type
 module Gitlab
   module Graphql
-    module Connections
-      class ExternallyPaginatedArrayConnection < GraphQL::Relay::ArrayConnection
-        # As the pagination happens externally
-        # we just return all the nodes here.
-        def sliced_nodes
-          @nodes
-        end
-
+    module Pagination
+      class ExternallyPaginatedArrayConnection < GraphQL::Pagination::ArrayConnection
         def start_cursor
-          nodes.previous_cursor
+          items.previous_cursor
         end
 
         def end_cursor
-          nodes.next_cursor
+          items.next_cursor
         end
 
         def next_page?

lib/gitlab/graphql/connections/filterable_array_connection.rb → lib/gitlab/graphql/pagination/filterable_array_connection.rb

@@ -2,14 +2,14 @@
 
 module Gitlab
   module Graphql
-    module Connections
+    module Pagination
       # FilterableArrayConnection is useful especially for lazy-loaded values.
       # It allows us to call a callback only on the slice of array being
       # rendered in the "after loaded" phase.  For example we can check
       # permissions only on a small subset of items.
-      class FilterableArrayConnection < GraphQL::Relay::ArrayConnection
-        def paged_nodes
-          @filtered_nodes ||= nodes.filter_callback.call(super)
+      class FilterableArrayConnection < GraphQL::Pagination::ArrayConnection
+        def nodes
+          @nodes ||= items.filter_callback.call(super)
         end
       end
     end

lib/gitlab/graphql/connections/keyset/conditions/base_condition.rb → lib/gitlab/graphql/pagination/keyset/conditions/base_condition.rb

@@ -2,7 +2,7 @@
 
 module Gitlab
   module Graphql
-    module Connections
+    module Pagination
       module Keyset
         module Conditions
           class BaseCondition

lib/gitlab/graphql/connections/keyset/conditions/not_null_condition.rb → lib/gitlab/graphql/pagination/keyset/conditions/not_null_condition.rb

@@ -2,7 +2,7 @@
 
 module Gitlab
   module Graphql
-    module Connections
+    module Pagination
       module Keyset
         module Conditions
           class NotNullCondition < BaseCondition

lib/gitlab/graphql/connections/keyset/conditions/null_condition.rb → lib/gitlab/graphql/pagination/keyset/conditions/null_condition.rb

@@ -2,7 +2,7 @@
 
 module Gitlab
   module Graphql
-    module Connections
+    module Pagination
       module Keyset
         module Conditions
           class NullCondition < BaseCondition

lib/gitlab/graphql/connections/keyset/connection.rb → lib/gitlab/graphql/pagination/keyset/connection.rb

@@ -27,21 +27,21 @@
 #
 module Gitlab
   module Graphql
-    module Connections
+    module Pagination
       module Keyset
-        class Connection < GraphQL::Relay::BaseConnection
+        class Connection < GraphQL::Pagination::ActiveRecordRelationConnection
           include Gitlab::Utils::StrongMemoize
 
-          def cursor_from_node(node)
+          def cursor_for(node)
             encoded_json_from_ordering(node)
           end
 
           def sliced_nodes
             @sliced_nodes ||=
               begin
-                OrderInfo.validate_ordering(ordered_nodes, order_list)
+                OrderInfo.validate_ordering(ordered_items, order_list)
 
-                sliced = ordered_nodes
+                sliced = ordered_items
                 sliced = slice_nodes(sliced, before, :before) if before.present?
                 sliced = slice_nodes(sliced, after, :after) if after.present?
 
@@ -49,12 +49,12 @@ module Gitlab
               end
           end
 
-          def paged_nodes
+          def nodes
             # These are the nodes that will be loaded into memory for rendering
             # So we're ok loading them into memory here as that's bound to happen
             # anyway. Having them ready means we can modify the result while
             # rendering the fields.
-            @paged_nodes ||= load_paged_nodes.to_a
+            @nodes ||= load_paged_nodes.to_a
           end
 
           private
@@ -85,31 +85,31 @@ module Gitlab
             @limit_value ||= [first, last, max_page_size].compact.min
           end
 
-          def ordered_nodes
-            strong_memoize(:order_nodes) do
-              unless nodes.primary_key.present?
+          def ordered_items
+            strong_memoize(:ordered_items) do
+              unless items.primary_key.present?
                 raise ArgumentError.new('Relation must have a primary key')
               end
 
-              list = OrderInfo.build_order_list(nodes)
+              list = OrderInfo.build_order_list(items)
 
               # ensure there is a primary key ordering
-              if list&.last&.attribute_name != nodes.primary_key
-                nodes.order(arel_table[nodes.primary_key].desc) # rubocop: disable CodeReuse/ActiveRecord
+              if list&.last&.attribute_name != items.primary_key
+                items.order(arel_table[items.primary_key].desc) # rubocop: disable CodeReuse/ActiveRecord
               else
-                nodes
+                items
               end
             end
           end
 
           def order_list
             strong_memoize(:order_list) do
-              OrderInfo.build_order_list(ordered_nodes)
+              OrderInfo.build_order_list(ordered_items)
             end
           end
 
           def arel_table
-            nodes.arel_table
+            items.arel_table
           end
 
           # Storing the current order values in the cursor allows us to

lib/gitlab/graphql/connections/keyset/order_info.rb → lib/gitlab/graphql/pagination/keyset/order_info.rb

@@ -2,7 +2,7 @@
 
 module Gitlab
   module Graphql
-    module Connections
+    module Pagination
       module Keyset
         class OrderInfo
           attr_reader :attribute_name, :sort_direction, :named_function

lib/gitlab/graphql/connections/keyset/query_builder.rb → lib/gitlab/graphql/pagination/keyset/query_builder.rb

@@ -2,7 +2,7 @@
 
 module Gitlab
   module Graphql
-    module Connections
+    module Pagination
       module Keyset
         class QueryBuilder
           def initialize(arel_table, order_list, decoded_cursor, before_or_after)

spec/graphql/gitlab_schema_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 describe GitlabSchema do
-  let_it_be(:implementations) { GraphQL::Relay::BaseConnection::CONNECTION_IMPLEMENTATIONS }
+  let_it_be(:connections) { GitlabSchema.connections.all_wrappers }
   let(:user) { build :user }
 
   it 'uses batch loading' do
@@ -34,22 +34,22 @@ describe GitlabSchema do
     expect(described_class.query).to eq(::Types::QueryType)
   end
 
-  it 'paginates active record relations using `Connections::Keyset::Connection`' do
-    connection = implementations[ActiveRecord::Relation.name]
+  it 'paginates active record relations using `Pagination::Keyset::Connection`' do
+    connection = connections[ActiveRecord::Relation]
 
-    expect(connection).to eq(Gitlab::Graphql::Connections::Keyset::Connection)
+    expect(connection).to eq(Gitlab::Graphql::Pagination::Keyset::Connection)
   end
 
-  it 'paginates ExternallyPaginatedArray using `Connections::ExternallyPaginatedArrayConnection`' do
-    connection = implementations[Gitlab::Graphql::ExternallyPaginatedArray.name]
+  it 'paginates ExternallyPaginatedArray using `Pagination::ExternallyPaginatedArrayConnection`' do
+    connection = connections[Gitlab::Graphql::ExternallyPaginatedArray]
 
-    expect(connection).to eq(Gitlab::Graphql::Connections::ExternallyPaginatedArrayConnection)
+    expect(connection).to eq(Gitlab::Graphql::Pagination::ExternallyPaginatedArrayConnection)
   end
 
-  it 'paginates FilterableArray using `Connections::FilterableArrayConnection`' do
-    connection = implementations[Gitlab::Graphql::FilterableArray.name]
+  it 'paginates FilterableArray using `Pagination::FilterableArrayConnection`' do
+    connection = connections[Gitlab::Graphql::FilterableArray]
 
-    expect(connection).to eq(Gitlab::Graphql::Connections::FilterableArrayConnection)
+    expect(connection).to eq(Gitlab::Graphql::Pagination::FilterableArrayConnection)
   end
 
   describe '.execute' do

spec/lib/gitlab/graphql/connections/externally_paginated_array_connection_spec.rb → spec/lib/gitlab/graphql/pagination/externally_paginated_array_connection_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe Gitlab::Graphql::Connections::ExternallyPaginatedArrayConnection do
+describe Gitlab::Graphql::Pagination::ExternallyPaginatedArrayConnection do
   let(:prev_cursor) { 1 }
   let(:next_cursor) { 6 }
   let(:values) { [2, 3, 4, 5] }
@@ -10,21 +10,13 @@ describe Gitlab::Graphql::Connections::ExternallyPaginatedArrayConnection do
   let(:arguments) { {} }
 
   subject(:connection) do
-    described_class.new(all_nodes, arguments)
+    described_class.new(all_nodes, { max_page_size: values.size }.merge(arguments))
   end
 
-  describe '#sliced_nodes' do
-    let(:sliced_nodes) { connection.sliced_nodes }
+  describe '#nodes' do
+    let(:paged_nodes) { connection.nodes }
 
-    it 'returns all the nodes' do
-      expect(connection.sliced_nodes).to eq(values)
-    end
-  end
-
-  describe '#paged_nodes' do
-    let(:paged_nodes) { connection.send(:paged_nodes) }
-
-    it_behaves_like "connection with paged nodes" do
+    it_behaves_like 'connection with paged nodes' do
       let(:paged_nodes_size) { values.size }
     end
   end

spec/lib/gitlab/graphql/connections/filterable_array_connection_spec.rb → spec/lib/gitlab/graphql/pagination/filterable_array_connection_spec.rb

@@ -2,19 +2,19 @@
 
 require 'spec_helper'
 
-describe Gitlab::Graphql::Connections::FilterableArrayConnection do
+describe Gitlab::Graphql::Pagination::FilterableArrayConnection do
   let(:callback) { proc { |nodes| nodes } }
   let(:all_nodes) { Gitlab::Graphql::FilterableArray.new(callback, 1, 2, 3, 4, 5) }
   let(:arguments) { {} }
 
   subject(:connection) do
-    described_class.new(all_nodes, arguments, max_page_size: 3)
+    described_class.new(all_nodes, { max_page_size: 3 }.merge(arguments))
   end
 
-  describe '#paged_nodes' do
-    let(:paged_nodes) { subject.paged_nodes }
+  describe '#nodes' do
+    let(:paged_nodes) { subject.nodes }
 
-    it_behaves_like "connection with paged nodes" do
+    it_behaves_like 'connection with paged nodes' do
       let(:paged_nodes_size) { 3 }
     end
 
@@ -22,7 +22,7 @@ describe Gitlab::Graphql::Connections::FilterableArrayConnection do
       let(:callback) { proc { |nodes| nodes[1..-1] } }
 
       it 'does not return filtered elements' do
-        expect(subject.paged_nodes).to contain_exactly(all_nodes[1], all_nodes[2])
+        expect(subject.nodes).to contain_exactly(all_nodes[1], all_nodes[2])
       end
     end
   end

spec/lib/gitlab/graphql/connections/keyset/conditions/not_null_condition_spec.rb → spec/lib/gitlab/graphql/pagination/keyset/conditions/not_null_condition_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe Gitlab::Graphql::Connections::Keyset::Conditions::NotNullCondition do
+describe Gitlab::Graphql::Pagination::Keyset::Conditions::NotNullCondition do
   describe '#build' do
     let(:operators) { ['>', '>'] }
     let(:before_or_after) { :after }
@@ -75,7 +75,7 @@ describe Gitlab::Graphql::Connections::Keyset::Conditions::NotNullCondition do
     context 'when ordering by LOWER' do
       let(:arel_table) { Project.arel_table }
       let(:relation)   { Project.order(arel_table['name'].lower.asc).order(:id) }
-      let(:order_list) { Gitlab::Graphql::Connections::Keyset::OrderInfo.build_order_list(relation) }
+      let(:order_list) { Gitlab::Graphql::Pagination::Keyset::OrderInfo.build_order_list(relation) }
       let(:values)     { ['Test', 500] }
 
       context 'when :after' do

spec/lib/gitlab/graphql/connections/keyset/conditions/null_condition_spec.rb → spec/lib/gitlab/graphql/pagination/keyset/conditions/null_condition_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe Gitlab::Graphql::Connections::Keyset::Conditions::NullCondition do
+describe Gitlab::Graphql::Pagination::Keyset::Conditions::NullCondition do
   describe '#build' do
     let(:values) { [nil, 500] }
     let(:operators) { [nil, '>'] }
@@ -58,7 +58,7 @@ describe Gitlab::Graphql::Connections::Keyset::Conditions::NullCondition do
     context 'when ordering by LOWER' do
       let(:arel_table) { Project.arel_table }
       let(:relation)   { Project.order(arel_table['name'].lower.asc).order(:id) }
-      let(:order_list) { Gitlab::Graphql::Connections::Keyset::OrderInfo.build_order_list(relation) }
+      let(:order_list) { Gitlab::Graphql::Pagination::Keyset::OrderInfo.build_order_list(relation) }
 
       context 'when :after' do
         it 'generates sql' do

spec/lib/gitlab/graphql/connections/keyset/connection_spec.rb → spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb

@@ -2,25 +2,28 @@
 
 require 'spec_helper'
 
-describe Gitlab::Graphql::Connections::Keyset::Connection do
+describe Gitlab::Graphql::Pagination::Keyset::Connection do
   let(:nodes) { Project.all.order(id: :asc) }
   let(:arguments) { {} }
+  let(:query_type) { GraphQL::ObjectType.new }
+  let(:schema) { GraphQL::Schema.define(query: query_type, mutation: nil)}
+  let(:context) { GraphQL::Query::Context.new(query: OpenStruct.new(schema: schema), values: nil, object: nil) }
 
   subject(:connection) do
-    described_class.new(nodes, arguments, max_page_size: 3)
+    described_class.new(nodes, { context: context, max_page_size: 3 }.merge(arguments))
   end
 
   def encoded_cursor(node)
-    described_class.new(nodes, {}).cursor_from_node(node)
+    described_class.new(nodes, { context: context }).cursor_for(node)
   end
 
   def decoded_cursor(cursor)
     JSON.parse(Base64Bp.urlsafe_decode64(cursor))
   end
 
-  describe '#cursor_from_nodes' do
+  describe '#cursor_for' do
     let(:project) { create(:project) }
-    let(:cursor)  { connection.cursor_from_node(project) }
+    let(:cursor)  { connection.cursor_for(project) }
 
     it 'returns an encoded ID' do
       expect(decoded_cursor(cursor)).to eq('id' => project.id.to_s)
@@ -264,11 +267,11 @@ describe Gitlab::Graphql::Connections::Keyset::Connection do
     end
   end
 
-  describe '#paged_nodes' do
+  describe '#nodes' do
     let_it_be(:all_nodes) { create_list(:project, 5) }
-    let(:paged_nodes) { subject.paged_nodes }
+    let(:paged_nodes) { subject.nodes }
 
-    it_behaves_like "connection with paged nodes" do
+    it_behaves_like 'connection with paged nodes' do
       let(:paged_nodes_size) { 3 }
     end
 

spec/lib/gitlab/graphql/connections/keyset/order_info_spec.rb → spec/lib/gitlab/graphql/pagination/keyset/order_info_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe Gitlab::Graphql::Connections::Keyset::OrderInfo do
+describe Gitlab::Graphql::Pagination::Keyset::OrderInfo do
   describe '#build_order_list' do
     let(:order_list) { described_class.build_order_list(relation) }
 

spec/lib/gitlab/graphql/connections/keyset/query_builder_spec.rb → spec/lib/gitlab/graphql/pagination/keyset/query_builder_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe Gitlab::Graphql::Connections::Keyset::QueryBuilder do
+describe Gitlab::Graphql::Pagination::Keyset::QueryBuilder do
   context 'when number of ordering fields is 0' do
     it 'raises an error' do
       expect { described_class.new(Issue.arel_table, [], {}, :after) }
@@ -12,7 +12,7 @@ describe Gitlab::Graphql::Connections::Keyset::QueryBuilder do
 
   describe '#conditions' do
     let(:relation) { Issue.order(relative_position: :desc).order(:id) }
-    let(:order_list) { Gitlab::Graphql::Connections::Keyset::OrderInfo.build_order_list(relation) }
+    let(:order_list) { Gitlab::Graphql::Pagination::Keyset::OrderInfo.build_order_list(relation) }
     let(:arel_table) { Issue.arel_table }
     let(:builder) { described_class.new(arel_table, order_list, decoded_cursor, before_or_after) }
     let(:before_or_after) { :after }

spec/support/helpers/graphql_helpers.rb

@@ -378,8 +378,9 @@ module GraphqlHelpers
 
   def execute_query(query_type)
     schema = Class.new(GraphQL::Schema) do
+      use GraphQL::Pagination::Connections
       use Gitlab::Graphql::Authorize
-      use Gitlab::Graphql::Connections
+      use Gitlab::Graphql::Pagination::Connections
 
       query(query_type)
     end