Skip to content

Commit

Permalink
WIP - Add cursor-based stable ID pagination
Browse files Browse the repository at this point in the history
This code is more for reference than review. The intent of this PR is
to increase transparency and collaboration so we can get input from the
community on the best direction to head - as with everything, there are
tradeoffs.

[This JSON:API "Cursor Pagination" Profile](https://jsonapi.org/profiles/ethanresnick/cursor-pagination/)
explains one use case. The other is that we're in the process of adding
GraphQL support to Graphiti, and cursors are more common in the GraphQL
world.

There are really two ways to do cursor-based pagination, and the
JSON:API proposal only considers one. We can do this will offset-based
cursors, or "stable IDS". This is best explained by [this post on
Stable Relation Connections](https://graphql-ruby.org/pagination/stable_relation_connections)
in GraphQL-Pro. Vanilla `graphql-ruby` does offset-based, Pro adds
support for stable IDs.

The third thing to consider is omitting cursors and supporting the
`page[offset]` parameter. This is the simplest thing that supports the
most use cases, though downsides are noted above.

This PR implements stable IDs. That means we need to tell Graphiti which
attributes are unique, incrementing keys. I've defaulted this to `id`,
though it's notable that will need to be overridden if using
non-incrementing UUIDs. So, the code:

```ruby
class EmployeeResource < ApplicationResource
  # Tell Graphiti we are opting-in to cursor pagination
  self.cursor_paginatable = true

  # Tell Graphiti this is a stable ID that can be used as cursor
  sort :created_at, :datetime, cursor: true

  # Override the default cursor from 'id' to 'created_at'
  self.default_cursor = :created_at
end
```

(*NB: One reason to have `cursor_paginatable` a separate flag from
`default_cursor` is so `ApplicationResource` can say "all resources
should support cursor pagination" but then throw helpful errors if
`id` is a UUID or we elsewise don't have a default stable cursor
defined*)

This will cause us to render base64 encoded cursors:

```ruby
{

  data: [
    {
      id: "10",
      type: "employees",
      attributes: { ... },
      meta: { cursor: "abc123" }
    }
  ]
}
```

Which can be used as offsets with `?page[after]=abc123`. This would
by default cause the SQL query `SELECT * FROM employees WHERE id > 10`.
So far so good.

The client might also pass a sort. So `sort=-id` (ID descending) would
cause the reverse query `...where id < 10`.

A little trickier: the client might pass a sort on an attribute that
is not the cursor. This is one reason we want to flag the sorts with
`cursor: true` - so the user can

```ruby
sort :created_at, cursor: true
```

Then call

```
?sort=created_at
```

Which will then use `created_at` as the cursor which means

```
?sort=created_at&page[after]=abc123
```

Will fire

```
SELECT * from employees where created_at > ? order by created_at asc
```

OK but now let's say the user tries to sort on something that ISN'T a
stable ID:

```
?sort=age
```

Under-the-hood we will turn this into a multi-sort like `age,id`. Then
when paging `after` the SQL would be something like:

```
SELECT * FROM employees
WHERE age > 40 OR (age = 40 AND id > 10)
ORDER BY age ASC, id ASC
```

Finally, we need to consider `datetime`. By default we render to the
second (e.g. `created_at.iso8601`) but to be a stable ID we need to
render to nanosecond precision (`created_at.iso8601(6)`). So the
serializer block will be honored, even if the attribute is unreadable:

```ruby
attribute :timestamp, :datetime, readable: false do
  @object.created_at
end
```

But we override the typecasting logic that would normally call
`.iso8601` and instead call `.iso8601(6)`. For everything else
*we omit typecasting entirely* since cursors should be referencing "raw"
values and there is no need to case to and fro.

There are three downsides to stable ID cursor pagination:

* The developer needs to know all this, and has a little more work to
do (like specifying `cursor: true`).
* The `OR` logic above would be specific to the `ActiveRecord` adapter.
Adapters in general do not have an `OR` concept, and I'm not sure there
is a good general-purpose one. This means stable IDs only work when
limiting sort capabilities and/or only using `ActiveRecord`.
* The `before` cursor is complicated. We need to reverse the direction
of the clauses, then re-reverse the records in memory. See
[SQL Option 2: Double Reverse](https://blog.reactioncommerce.com/how-to-implement-graphql-pagination-and-sorting/).
This feels like it might be buggy down the line.

For these reasons I propose:

* Default to offset-based cursors
* Opt-in to stable IDs by specifying `.default_cursor`
* This all goes through a `cursor_paginate` adapter method (alternative
is we can call the relevant adapter methods like `filter_integer_gt` and
introduce an "OR" concept, but this feels shaky). Implementing this
will be non-trivial for non-AR datastores.
* This means adapters need an "offset" concept

This seems to give us the best balance of ease-of-use (offset-based) and
opt-in power (stable-id-based). It also allows us to more easily say
"all endpoints implement cursor-based pagination" (so we avoid having
to remember to specify cursors in all resources).

But, there are enough moving pieces here this is usually where I stop
and get advice from people smarter than me. This is often @wadetandy
but also includes basically anyone reading this PR. So, what are your
thoughts?
  • Loading branch information
Lee Richmond committed May 3, 2021
1 parent 9ab7fa8 commit caaf203
Show file tree
Hide file tree
Showing 18 changed files with 911 additions and 19 deletions.
1 change: 1 addition & 0 deletions lib/graphiti.rb
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ def self.setup!
require "graphiti/util/link"
require "graphiti/util/remote_serializer"
require "graphiti/util/remote_params"
require "graphiti/util/cursor"
require "graphiti/adapters/null"
require "graphiti/adapters/graphiti_api"
require "graphiti/extensions/extra_attribute"
Expand Down
4 changes: 4 additions & 0 deletions lib/graphiti/adapters/abstract.rb
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,10 @@ def paginate(scope, current_page, per_page)
raise "you must override #paginate in an adapter subclass"
end

def cursor_paginate(scope, after, size)
raise "you must override #cursor_paginate in an adapter subclass"
end

# @param scope the scope object we are chaining
# @param [Symbol] attr corresponding stat attribute name
# @return [Numeric] the count of the scope
Expand Down
26 changes: 26 additions & 0 deletions lib/graphiti/adapters/active_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,32 @@ def paginate(scope, current_page, per_page)
scope.page(current_page).per(per_page)
end

def cursor_paginate(scope, after, size)
clause = nil
after.each_with_index do |part, index|
method = part[:direction] == "asc" ? :filter_gt : :filter_lt

if index.zero?
clause = public_send \
method,
scope,
part[:attribute],
[part[:value]]
else
sub_scope = filter_eq \
scope,
after[index - 1][:attribute],
[after[index - 1][:value]]
sub_scope = filter_gt \
sub_scope,
part[:attribute],
[part[:value]]
clause = clause.or(sub_scope)
end
end
paginate(clause, 1, size)
end

# (see Adapters::Abstract#count)
def count(scope, attr)
if attr.to_sym == :total
Expand Down
2 changes: 2 additions & 0 deletions lib/graphiti/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class Configuration
attr_accessor :links_on_demand
attr_accessor :pagination_links_on_demand
attr_accessor :pagination_links
attr_accessor :cursor_on_demand
attr_accessor :typecast_reads
attr_accessor :raise_on_missing_sidepost

Expand All @@ -29,6 +30,7 @@ def initialize
@links_on_demand = false
@pagination_links_on_demand = false
@pagination_links = false
@cursor_on_demand = false
@typecast_reads = true
@raise_on_missing_sidepost = true
self.debug = ENV.fetch("GRAPHITI_DEBUG", true)
Expand Down
12 changes: 12 additions & 0 deletions lib/graphiti/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,18 @@ def message
end
end

class UnsupportedCursorPagination < Base
def initialize(resource)
@resource = resource
end

def message
<<~MSG
It looks like you are passing cursor pagination params, but #{@resource.class.name} does not support cursor pagination.
MSG
end
end

class UnsupportedPageSize < Base
def initialize(size, max)
@size, @max = size, max
Expand Down
17 changes: 15 additions & 2 deletions lib/graphiti/query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,16 @@ def pagination_links?
end
end

def cursor?
return false if [:json, :xml, "json", "xml"].include?(params[:format])

if Graphiti.config.cursor_on_demand
[true, "true"].include?(@params[:cursor])
else
@resource.cursor_paginatable?
end
end

def debug_requested?
!!@params[:debug]
end
Expand Down Expand Up @@ -185,6 +195,8 @@ def sorts

def pagination
@pagination ||= begin
valid_params = Scoping::Paginate::VALID_QUERY_PARAMS

{}.tap do |hash|
(@params[:page] || {}).each_pair do |name, value|
if legacy_nested?(name)
Expand All @@ -193,8 +205,9 @@ def pagination
end
elsif nested?(name)
hash[name.to_s.split(".").last.to_sym] = value
elsif top_level? && [:number, :size].include?(name.to_sym)
hash[name.to_sym] = value.to_i
elsif top_level? && valid_params.include?(name.to_sym)
value = value.to_i if [:size, :number].include?(name.to_sym)
hash[name.to_sym] = value
end
end
end
Expand Down
37 changes: 37 additions & 0 deletions lib/graphiti/resource/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,30 @@ def remote=(val)
}
end

def cursor_paginatable=(val)
super

unless default_cursor?
if sorts.key?(:id)
type = attributes[:id][:type]
canonical = Graphiti::Types[type][:canonical_name]
if canonical == :integer
self.default_cursor = :id
end
end
end
end

def default_cursor=(val)
super

if attributes.key?(val)
sort val, cursorable: true
else
raise "friendly error about not an attribute"
end
end

def model
klass = super
unless klass || abstract_class?
Expand Down Expand Up @@ -82,6 +106,9 @@ class << self
:serializer,
:default_page_size,
:default_sort,
:default_cursor,
:cursor_paginatable,
:cursorable_attributes,
:max_page_size,
:attributes_readable_by_default,
:attributes_writable_by_default,
Expand Down Expand Up @@ -120,6 +147,7 @@ def self.inherited(klass)
unless klass.config[:attributes][:id]
klass.attribute :id, :integer_id
end

klass.stat total: [:count]

if defined?(::Rails) && ::Rails.env.development?
Expand Down Expand Up @@ -205,6 +233,7 @@ def config
sort_all: nil,
sorts: {},
pagination: nil,
cursor_pagination: nil,
after_graph_persist: {},
before_commit: {},
after_commit: {},
Expand Down Expand Up @@ -252,6 +281,10 @@ def pagination
config[:pagination]
end

def cursor_pagination
config[:cursor_pagination]
end

def default_filters
config[:default_filters]
end
Expand Down Expand Up @@ -298,6 +331,10 @@ def pagination
self.class.pagination
end

def cursor_pagination
self.class.cursor_pagination
end

def attributes
self.class.attributes
end
Expand Down
6 changes: 5 additions & 1 deletion lib/graphiti/resource/dsl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def sort(name, *args, &blk)
if get_attr(name, :sortable, raise_error: :only_unsupported)
config[:sorts][name] = {
proc: blk
}.merge(opts.slice(:only))
}.merge(opts.slice(:only, :cursorable))
elsif (type = args[0])
attribute name, type, only: [:sortable]
sort(name, opts, &blk)
Expand All @@ -82,6 +82,10 @@ def paginate(&blk)
config[:pagination] = blk
end

def cursor_paginate(&blk)
config[:cursor_pagination] = blk
end

def stat(symbol_or_hash, &blk)
dsl = Stats::DSL.new(new.adapter, symbol_or_hash)
dsl.instance_eval(&blk) if blk
Expand Down
59 changes: 56 additions & 3 deletions lib/graphiti/scoping/paginate.rb
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
module Graphiti
class Scoping::Paginate < Scoping::Base
DEFAULT_PAGE_SIZE = 20
VALID_QUERY_PARAMS = [:number, :size, :before, :after]

def apply
if size > resource.max_page_size
raise Graphiti::Errors::UnsupportedPageSize
.new(size, resource.max_page_size)
elsif requested? && @opts[:sideload_parent_length].to_i > 1
raise Graphiti::Errors::UnsupportedPagination
elsif cursor? && !resource.cursor_paginatable?
raise Graphiti::Errors::UnsupportedCursorPagination.new(resource)
else
super
end
Expand All @@ -28,17 +31,57 @@ def apply?

# @return [Proc, Nil] the custom pagination proc
def custom_scope
resource.pagination
cursor? ? resource.cursor_pagination : resource.pagination
end

# Apply default pagination proc via the Resource adapter
def apply_standard_scope
resource.adapter.paginate(@scope, number, size)
if cursor?
# NB put in abstract adapter?

# if after_cursor
# clause = nil
# after_cursor.each_with_index do |part, index|
# method = part[:direction] == "asc" ? :filter_gt : :filter_lt

# if index.zero?
# clause = resource.adapter.public_send(method, @scope, part[:attribute], [part[:value]])
# else
# sub_scope = resource.adapter
# .filter_eq(@scope, after_cursor[index-1][:attribute], [after_cursor[index-1][:value]])
# sub_scope = resource.adapter.filter_gt(sub_scope, part[:attribute], [part[:value]])

# # NB - AR specific (use offset?)
# # maybe in PR ask feedback
# clause = clause.or(sub_scope)
# end
# end
# @scope = clause
# end
# resource.adapter.paginate(@scope, 1, size)
resource.adapter.cursor_paginate(@scope, after_cursor, size)
else
resource.adapter.paginate(@scope, number, size)
end
end

# Apply the custom pagination proc
def apply_custom_scope
resource.instance_exec(@scope, number, size, resource.context, &custom_scope)
if cursor?
resource.instance_exec \
@scope,
after_cursor,
size,
resource.context,
&custom_scope
else
resource.instance_exec \
@scope,
number,
size,
resource.context,
&custom_scope
end
end

private
Expand All @@ -58,5 +101,15 @@ def number
def size
(page_param[:size] || resource.default_page_size || DEFAULT_PAGE_SIZE).to_i
end

def after_cursor
if (after = page_param[:after])
Util::Cursor.decode(resource, after)
end
end

def cursor?
!!page_param[:after]
end
end
end
13 changes: 12 additions & 1 deletion lib/graphiti/scoping/sort.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,10 @@ def apply_custom_scope
private

def each_sort
sort_param.each do |sort_hash|
sorts = sort_param
add_cursor_pagination_fallback(sorts)

sorts.each do |sort_hash|
attribute = sort_hash.keys.first
direction = sort_hash.values.first
yield attribute, direction
Expand Down Expand Up @@ -82,5 +85,13 @@ def sort_hash(attr)

{key => value}
end

def add_cursor_pagination_fallback(sorts)
if sorts.present? && @resource.cursor_paginatable?
sort_key = sorts.last.keys[0]
cursorable = !!@resource.sorts[sort_key][:cursorable]
sorts << {@resource.default_cursor => :asc} unless cursorable
end
end
end
end
27 changes: 27 additions & 0 deletions lib/graphiti/util/cursor.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
module Graphiti
module Util
module Cursor
def self.encode(parts)
parts.each do |part|
part[:value] = part[:value].iso8601(6) if part[:value].is_a?(Time)
end
Base64.encode64(parts.to_json)
end

def self.decode(resource, cursor)
parts = JSON.parse(Base64.decode64(cursor)).map(&:symbolize_keys)
parts.each do |part|
part[:attribute] = part[:attribute].to_sym
config = resource.get_attr!(part[:attribute], :sortable, request: true)
value = part[:value]
part[:value] = if config[:type] == :datetime
Dry::Types["json.date_time"][value].iso8601(6)
else
resource.typecast(part[:attribute], value, :sortable)
end
end
parts
end
end
end
end
Loading

0 comments on commit caaf203

Please sign in to comment.