GraphQL: Avoid ConnectionType gotchas

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

• Close this issue

When defining a ConnectionType, the GraphQL docs suggest the following:

# Make a customized connection type
class Types::PostConnectionWithTotalCountType < GraphQL::Types::Relay::BaseConnection
  edge_type(PostEdgeType)

  field :total_count, Integer, null: false
  def total_count
    # - `object` is the Connection
    # - `object.items` is the original collection of Posts
    object.items.size
  end
end

However, this can mess up complex queries, such as those containing GROUP BY (see !36259 (comment 388827630)):

0> relation = object.items
=> #<ActiveRecord::Relation []>

0> relation.loaded?
=> nil

0> relation.size
=> {}

This results in an error:

   (0.4ms)  SELECT COUNT(*) AS count_all, "issues"."id" AS issues_id FROM "issues" WHERE "issues"."project_id" = $1 AND ("issues"."state_id" IN (1)) AND (EXISTS (SELECT "label_links".* FROM "label_links" WHERE (label_links.target_type = 'Issue' AND label_links.target_id = issues.id) AND (label_links.label_id = 1) LIMIT 1)) GROUP BY "issues"."id" ORDER BY relative_position ASC /*application:test,controller:graphql,action:execute,correlation_id:52f7c200-70de-4561-9138-5950a4ed371b*/  [["project_id", 1]]
  ↳ app/graphql/types/issue_connection_type.rb:10:in `count'
NoMethodError (undefined method `to_i' for {}:Hash):
  lib/gitlab/graphql/generic_tracing.rb:40:in `with_labkit_tracing'
  lib/gitlab/graphql/generic_tracing.rb:30:in `platform_trace'
  lib/gitlab/graphql/generic_tracing.rb:40:in `with_labkit_tracing'
  lib/gitlab/graphql/generic_tracing.rb:30:in `platform_trace'
  app/graphql/gitlab_schema.rb:41:in `multiplex'

Another way it can be messed up is by virtual columns. The following error can occur:

ArgumentError (A maximum of 2 ordering fields are allowed):
  lib/gitlab/graphql/pagination/keyset/order_info.rb:45:in `validate_ordering'
  lib/gitlab/graphql/pagination/keyset/connection.rb:85:in `sliced_nodes'
  lib/gitlab/graphql/pagination/keyset/connection.rb:119:in `block in limited_nodes'

@digitalmoksha explains:

What's happening here is that we're running into a limitation of the keyset pagination. It can only handle relatively simple ordering, on two fields with one of those fields being the primary key. Putting the bandaid ordering can get you past that (because the keyset understands the actual relation ordering, not the SQL), but you don't get the results that you expect.

Instead of returning the relation directly

service.execute

you need to wrap it in offset connection, which will use the older style offset pagination, which doesn't care about the ordering.

Gitlab::Graphql::Pagination::OffsetActiveRecordRelationConnection.new(service.execute)

We purposefully default all graphql connections to use keyset pagination, and fallback to offset pagination when we must.

We could introduce the following to ensure someone else doesn't get hit with the same gotchas:

• update docs with an example of a ConnectionType
• create an abstract ConnectionType::Base type class with count including reorder(nil) so it'd be ideally straightforward to introduce these to more connections