Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP - Add cursor-based stable ID pagination #357

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QQ: I don't see :before implemented here thus far, correct? Is adding that query param for a later stage of this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I have some before code locally but removed it before pushing up (and forgot this).

If you have IDs 1-100 and ordering ID asc, you end up saying select * from employees where id < 100 limit 1 which gives you 1 instead of 99. The linked solution is in the PR (SQL query the opposite order then reverse in-memory) but that was a little more than I had time for at the moment - and I think it's an open question if this is "worth it" with the extra complexity, or if we should only implement offset-based for now - so pulled it out.


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 = [email protected][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