From e935a2c0e03554955da714f67224eb6b23e8d7e9 Mon Sep 17 00:00:00 2001 From: Lee Richmond Date: Sun, 2 May 2021 19:45:24 -0400 Subject: [PATCH] WIP - Add cursor-based stable ID pagination 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? --- lib/graphiti.rb | 1 + lib/graphiti/adapters/abstract.rb | 4 + lib/graphiti/adapters/active_record.rb | 26 ++ lib/graphiti/configuration.rb | 2 + lib/graphiti/errors.rb | 12 + lib/graphiti/query.rb | 17 +- lib/graphiti/resource/configuration.rb | 37 ++ lib/graphiti/resource/dsl.rb | 6 +- lib/graphiti/scoping/paginate.rb | 59 +++- lib/graphiti/scoping/sort.rb | 13 +- lib/graphiti/util/cursor.rb | 27 ++ lib/graphiti/util/serializer_attributes.rb | 39 +++ spec/fixtures/legacy.rb | 3 +- spec/fixtures/poro.rb | 33 +- .../rails/cursor_pagination_spec.rb | 116 ++++++ spec/pagination_spec.rb | 123 ++++++- spec/serialization_spec.rb | 330 ++++++++++++++++++ spec/sorting_spec.rb | 82 +++++ 18 files changed, 911 insertions(+), 19 deletions(-) create mode 100644 lib/graphiti/util/cursor.rb create mode 100644 spec/integration/rails/cursor_pagination_spec.rb diff --git a/lib/graphiti.rb b/lib/graphiti.rb index 35eaed6c..67ffaf9a 100644 --- a/lib/graphiti.rb +++ b/lib/graphiti.rb @@ -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" diff --git a/lib/graphiti/adapters/abstract.rb b/lib/graphiti/adapters/abstract.rb index c567bedd..bc142b74 100644 --- a/lib/graphiti/adapters/abstract.rb +++ b/lib/graphiti/adapters/abstract.rb @@ -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, current_page, per_page) + 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 diff --git a/lib/graphiti/adapters/active_record.rb b/lib/graphiti/adapters/active_record.rb index 1b6f61ac..1005094e 100644 --- a/lib/graphiti/adapters/active_record.rb +++ b/lib/graphiti/adapters/active_record.rb @@ -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 diff --git a/lib/graphiti/configuration.rb b/lib/graphiti/configuration.rb index 59f2ed15..1b2741cf 100644 --- a/lib/graphiti/configuration.rb +++ b/lib/graphiti/configuration.rb @@ -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 @@ -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) diff --git a/lib/graphiti/errors.rb b/lib/graphiti/errors.rb index e2273476..5d2c45e4 100644 --- a/lib/graphiti/errors.rb +++ b/lib/graphiti/errors.rb @@ -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 diff --git a/lib/graphiti/query.rb b/lib/graphiti/query.rb index e02e764b..fbff0b74 100644 --- a/lib/graphiti/query.rb +++ b/lib/graphiti/query.rb @@ -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 @@ -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) @@ -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 diff --git a/lib/graphiti/resource/configuration.rb b/lib/graphiti/resource/configuration.rb index caf22940..e2f6f4fc 100644 --- a/lib/graphiti/resource/configuration.rb +++ b/lib/graphiti/resource/configuration.rb @@ -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? @@ -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, @@ -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? @@ -205,6 +233,7 @@ def config sort_all: nil, sorts: {}, pagination: nil, + cursor_pagination: nil, after_graph_persist: {}, before_commit: {}, after_commit: {}, @@ -252,6 +281,10 @@ def pagination config[:pagination] end + def cursor_pagination + config[:cursor_pagination] + end + def default_filters config[:default_filters] end @@ -298,6 +331,10 @@ def pagination self.class.pagination end + def cursor_pagination + self.class.cursor_pagination + end + def attributes self.class.attributes end diff --git a/lib/graphiti/resource/dsl.rb b/lib/graphiti/resource/dsl.rb index 96f11850..407ffddf 100644 --- a/lib/graphiti/resource/dsl.rb +++ b/lib/graphiti/resource/dsl.rb @@ -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) @@ -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 diff --git a/lib/graphiti/scoping/paginate.rb b/lib/graphiti/scoping/paginate.rb index 8a0a355f..e9b045cc 100644 --- a/lib/graphiti/scoping/paginate.rb +++ b/lib/graphiti/scoping/paginate.rb @@ -1,6 +1,7 @@ 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 @@ -8,6 +9,8 @@ def apply .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 @@ -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 @@ -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 diff --git a/lib/graphiti/scoping/sort.rb b/lib/graphiti/scoping/sort.rb index b81c8a07..85f970ea 100644 --- a/lib/graphiti/scoping/sort.rb +++ b/lib/graphiti/scoping/sort.rb @@ -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 @@ -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 diff --git a/lib/graphiti/util/cursor.rb b/lib/graphiti/util/cursor.rb new file mode 100644 index 00000000..e10997de --- /dev/null +++ b/lib/graphiti/util/cursor.rb @@ -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 diff --git a/lib/graphiti/util/serializer_attributes.rb b/lib/graphiti/util/serializer_attributes.rb index c12a4df9..27e530e8 100644 --- a/lib/graphiti/util/serializer_attributes.rb +++ b/lib/graphiti/util/serializer_attributes.rb @@ -28,6 +28,45 @@ def apply existing = @serializer.send(applied_method) @serializer.send(:"#{applied_method}=", [@name] | existing) + + # NB could use some refacotring + @serializer.meta do + # Not a remote resource and requested/enabled + if @resource.respond_to?(:cursor_paginatable?) && + @resource.cursor_paginatable? && + @proxy.query.cursor? + + parts = [] + has_cursorable_part = false + if @proxy.query.sorts.any? + @proxy.query.sorts.each do |sort| + attribute, direction = Array(sort)[0] + cursorable = !!@resource.sorts[attribute][:cursorable] + parts << {attribute: attribute, direction: direction} + if cursorable + has_cursorable_part = true + break + end + end + end + + unless has_cursorable_part + parts << {attribute: @resource.default_cursor, direction: :asc} + end + + parts.each do |part| + config = @resource.get_attr!(part[:attribute], :sortable, request: true) + + part[:value] = if config[:proc] + instance_eval(&config[:proc]) + else + @object.public_send(part[:attribute]) + end + end + + {cursor: Util::Cursor.encode(parts)} + end + end end private diff --git a/spec/fixtures/legacy.rb b/spec/fixtures/legacy.rb index 60024b8c..657ba6a0 100644 --- a/spec/fixtures/legacy.rb +++ b/spec/fixtures/legacy.rb @@ -333,12 +333,13 @@ class AuthorResource < ApplicationResource attribute :float_age, :float attribute :decimal_age, :big_decimal attribute :active, :boolean - attribute :last_login, :datetime, only: [:filterable] + attribute :last_login, :datetime, only: [:filterable, :sortable] attribute :created_at, :datetime, only: [:filterable] attribute :created_at_date, :date, only: [:filterable] attribute :identifier, :uuid filter :last_login, allow_nil: true + sort :last_login, cursorable: true has_many :books belongs_to :state diff --git a/spec/fixtures/poro.rb b/spec/fixtures/poro.rb index 8debdf0c..9a3c128f 100644 --- a/spec/fixtures/poro.rb +++ b/spec/fixtures/poro.rb @@ -97,11 +97,28 @@ def apply_sorting(records, params) end def apply_pagination(records, params) - return records unless params[:per] + if params.key?(:after) + start_at = 0 + + if (after = params[:after]) + after = after[0] # no multisort for PORO + start_at = records.index do |r| + r.send(after[:attribute]) == after[:value] + end + 1 + end + + end_at = if (per = params[:per]) + start_at + (per - 1) + else + 999 + end + else + return records unless params[:per] + start_at = (params[:page] - 1) * (params[:per]) + end_at = (params[:page] * params[:per]) - 1 + return [] if end_at < 0 + end - start_at = (params[:page] - 1) * (params[:per]) - end_at = (params[:page] * params[:per]) - 1 - return [] if end_at < 0 records[start_at..end_at] end end @@ -109,7 +126,7 @@ def apply_pagination(records, params) class Base include ActiveModel::Validations - attr_accessor :id + attr_accessor :id, :created_at def self.create(attrs = {}) record = new(attrs) @@ -117,7 +134,9 @@ def self.create(attrs = {}) if record.valid? id = attrs[:id] || DB.data[type].length + 1 attrs[:id] = id + attrs[:created_at] = attrs[:created_at] || Time.now record.id = id + record.created_at = attrs[:created_at] DB.data[type] << attrs end @@ -296,6 +315,10 @@ def paginate(scope, current_page, per_page) scope.merge!(page: current_page, per: per_page) end + def cursor_paginate(scope, after, size) + scope.merge!(after: after, per: size) + end + def filter(scope, name, value) scope[:conditions] ||= {} scope[:conditions][name] = value diff --git a/spec/integration/rails/cursor_pagination_spec.rb b/spec/integration/rails/cursor_pagination_spec.rb new file mode 100644 index 00000000..e5784c1f --- /dev/null +++ b/spec/integration/rails/cursor_pagination_spec.rb @@ -0,0 +1,116 @@ +if ENV["APPRAISAL_INITIALIZED"] + RSpec.describe "cursor pagination", type: :controller do + include GraphitiSpecHelpers + + controller(ApplicationController) do + def index + records = resource.all(params) + render jsonapi: records + end + + def resource + Legacy::AuthorResource + end + end + + before do + allow(controller.request.env).to receive(:[]) + .with(anything).and_call_original + allow(controller.request.env).to receive(:[]) + .with("PATH_INFO") { path } + end + + let(:path) { "/legacy/authors" } + + let!(:author1) { Legacy::Author.create!(age: 10, last_login: 4.day.ago) } + let!(:author2) { Legacy::Author.create!(age: 20, last_login: 2.days.ago) } + let!(:author3) { Legacy::Author.create!(age: 20, last_login: 3.days.ago) } + let!(:author4) { Legacy::Author.create!(age: 30, last_login: 1.days.ago) } + + around do |e| + original = Legacy::AuthorResource.cursor_paginatable + Legacy::AuthorResource.cursor_paginatable = true + begin + e.run + ensure + Legacy::AuthorResource.cursor_paginatable = original + end + end + + def decode(cursor) + Graphiti::Util::Cursor.decode(Legacy::AuthorResource.new, cursor) + end + + # don't go through 'd' helper b/c it is memoized + def ids + json["data"].map { |d| d["id"].to_i } + end + + it "renders a cursor in meta" do + do_index({}) + decoded = decode(json["data"][0]["meta"]["cursor"]) + expect(decoded).to eq([{attribute: :id, direction: "asc", value: 1}]) + decoded = decode(json["data"][1]["meta"]["cursor"]) + expect(decoded).to eq([{attribute: :id, direction: "asc", value: 2}]) + end + + describe "using a rendered cursor" do + context "when paginating after" do + context "basic" do + it "works" do + do_index({}) + cursor = json["data"][1]["meta"]["cursor"] + do_index(page: {after: cursor}) + expect(ids).to eq([author3.id, author4.id]) + end + + it "respects page size" do + do_index({}) + cursor = json["data"][1]["meta"]["cursor"] + do_index(page: {after: cursor, size: 1}) + expect(ids).to eq([author3.id]) + end + end + + context "when given sort param" do + context "that is cursorable" do + it "works asc" do + do_index(sort: "last_login") + expect(ids).to eq([author1.id, author3.id, author2.id, author4.id]) + cursor = json["data"][0]["meta"]["cursor"] + do_index(sort: "last_login", page: {after: cursor}) + expect(ids).to eq([author3.id, author2.id, author4.id]) + end + + it "works desc" do + do_index(sort: "-last_login") + expect(ids).to eq([4, 2, 3, 1]) + cursor = json["data"][0]["meta"]["cursor"] + do_index(sort: "-last_login", page: {after: cursor}) + expect(ids).to eq([author2.id, author3.id, author1.id]) + end + end + + context "that is not cursorable" do + it "works asc" do + do_index(sort: "age") + expect(ids).to eq([1, 2, 3, 4]) + cursor = json["data"][1]["meta"]["cursor"] # author2, age 20 + do_index(sort: "age", page: {after: cursor}) + expect(ids).to eq([author3.id, author4.id]) + end + + it "works desc" do + do_index(sort: "-age") + # this order because id is secondary sort + expect(ids).to eq([4, 2, 3, 1]) + cursor = json["data"][1]["meta"]["cursor"] # author2, age 20 + do_index(sort: "-age", page: {after: cursor}) + expect(ids).to eq([author3.id, author1.id]) + end + end + end + end + end + end +end diff --git a/spec/pagination_spec.rb b/spec/pagination_spec.rb index db537902..3fb85441 100644 --- a/spec/pagination_spec.rb +++ b/spec/pagination_spec.rb @@ -7,12 +7,10 @@ subject(:ids) { records.map(&:id) } - before do - PORO::Employee.create - PORO::Employee.create - PORO::Employee.create - PORO::Employee.create - end + let!(:employee1) { PORO::Employee.create } + let!(:employee2) { PORO::Employee.create } + let!(:employee3) { PORO::Employee.create } + let!(:employee4) { PORO::Employee.create } it "applies default pagination" do resource.class_eval do @@ -104,4 +102,117 @@ end end end + + + context "when cursor pagination" do + before do + resource.cursor_paginatable = true + end + + def encode(attribute, value) + Graphiti::Util::Cursor.encode([{ + attribute: attribute, + value: value, + direction: :asc + }]) + end + + context "when simple case - by id" do + context "and 'after' given" do + before do + params[:page] = {after: encode(:id, employee2.id)} + end + + it "goes through typecasting" do + expect_any_instance_of(resource.adapter).to receive(:cursor_paginate) + .with(anything, [hash_including(value: employee2.id)], 20) + .and_call_original + ids + end + + it "works" do + expect(ids).to eq([employee3.id, employee4.id]) + end + end + + context "when page[size] is passed" do + context "with 'after' param" do + before do + params[:page] = { + after: encode(:id, employee1.id), + size: 2 + } + end + + it "is respected" do + expect(ids).to eq([2, 3]) + end + end + end + end + + context "when a datetime" do + before do + resource.attribute :created_at, :datetime + resource.sort :created_at, cursorable: true + end + + context "and 'after' given" do + let(:nano_created_at) { employee2.created_at.iso8601(6) } + + before do + params[:page] = { + after: encode(:created_at, nano_created_at) + } + end + + it "passes the datetime with nanosecond precision" do + expect_any_instance_of(resource.adapter).to receive(:cursor_paginate) + .with(anything, [hash_including(value: nano_created_at)], 20) + .and_call_original + ids + end + + it "works" do + expect(ids).to eq([employee3.id, employee4.id]) + end + end + end + + context "when custom .cursor_pagination proc" do + before do + resource.cursor_paginate do |scope, after, size, context| + Graphiti.context[:after_spy] = after + Graphiti.context[:size_spy] = size + Graphiti.context[:context_correct_spy] = Graphiti.context[:object] == context + scope.merge!(after: after, per: size) + end + + params[:page] = { + after: encode(:id, employee1.id) + } + end + + it "is called correctly" do + expect(ids).to eq([employee2.id, employee3.id, employee4.id]) + expect(Graphiti.context[:after_spy]) + .to eq([{attribute: :id, value: employee1.id, direction: "asc"}]) + expect(Graphiti.context[:size_spy]).to eq(20) + expect(Graphiti.context[:context_correct_spy]).to eq(true) + end + end + + context "when disabled" do + before do + resource.cursor_paginatable = false + params[:page] = {after: "abc123"} + end + + it "raises friendly error" do + expect { + ids + }.to raise_error(Graphiti::Errors::UnsupportedCursorPagination) + end + end + end end diff --git a/spec/serialization_spec.rb b/spec/serialization_spec.rb index 917038ad..c159df57 100644 --- a/spec/serialization_spec.rb +++ b/spec/serialization_spec.rb @@ -1866,4 +1866,334 @@ def admin? end end end + + context "when cursor pagination turned on" do + let!(:employee1) { PORO::Employee.create(age: 10) } + let!(:employee2) { PORO::Employee.create(age: 20) } + + before do + resource.cursor_paginatable = true + resource.attribute :created_at, :datetime + end + + def encode(attribute, value) + Graphiti::Util::Cursor.encode([{ + attribute: attribute, + value: value, + direction: "asc" + }]) + end + + def decode(cursor) + Graphiti::Util::Cursor.decode(resource.new, cursor) + end + + it "renders the cursor in resource meta" do + render + expect(decode(json["data"][0]["meta"]["cursor"])) + .to eq([{attribute: :id, direction: "asc", value: employee1.id}]) + expect(decode(json["data"][1]["meta"]["cursor"])) + .to eq([{attribute: :id, direction: "asc", value: employee2.id}]) + end + + context "when default_cursor is specified" do + before do + resource.default_cursor = :created_at + end + + it "is respected" do + render + expect(decode(json["data"][0]["meta"]["cursor"])) + .to eq([{ + attribute: :created_at, + direction: "asc", + value: employee1.created_at.iso8601(6) + }]) + expect(decode(json["data"][1]["meta"]["cursor"])) + .to eq([{ + attribute: :created_at, + direction: "asc", + value: employee2.created_at.iso8601(6) + }]) + end + end + + context "when a sort is passed in the request" do + before do + resource.attribute :age, :integer + params[:sort] = "-age" + end + + context "and the sort is not cursorable" do + # We will multisort in this case, see sorting spec + it "renders the default cursor" do + render + + expect(decode(json["data"][0]["meta"]["cursor"])) + .to eq([ + {attribute: :age, direction: "desc", value: employee2.age}, + {attribute: :id, direction: "asc", value: employee2.id} + ]) + expect(decode(json["data"][1]["meta"]["cursor"])) + .to eq([ + {attribute: :age, direction: "desc", value: employee1.age}, + {attribute: :id, direction: "asc", value: employee1.id} + ]) + end + end + + context "and it is cursorable" do + before do + resource.sort :created_at, cursorable: true + params[:sort] = "-created_at" + end + + it "renders a cursor with the given sort" do + render + + expect(decode(json["data"][0]["meta"]["cursor"])) + .to eq([{ + attribute: :created_at, + direction: "desc", + value: employee2.created_at.iso8601(6) + }]) + expect(decode(json["data"][1]["meta"]["cursor"])) + .to eq([{ + attribute: :created_at, + direction: "desc", + value: employee1.created_at.iso8601(6) + }]) + end + end + + context "when multiple sorts passed in the request" do + before do + resource.attribute :age, :integer + params[:sort] = "-age,created_at" + end + + context "and the last sort is cursorable" do + before do + resource.sort :created_at, cursorable: true + end + + it "adds multiple attributes - but not the default - to the cursor" do + render + + expect(decode(json["data"][0]["meta"]["cursor"])) + .to eq([ + { + attribute: :age, + direction: "desc", + value: employee2.age + }, + { + attribute: :created_at, + direction: "asc", + value: employee2.created_at.iso8601(6) + } + ]) + + expect(decode(json["data"][1]["meta"]["cursor"])) + .to eq([ + { + attribute: :age, + direction: "desc", + value: employee1.age + }, + { + attribute: :created_at, + direction: "asc", + value: employee1.created_at.iso8601(6) + } + ]) + end + end + + context "and the last sort is not cursorable" do + it "adds the default cursor to the payload" do + render + + expect(decode(json["data"][0]["meta"]["cursor"])) + .to eq([ + { + attribute: :age, + direction: "desc", + value: employee2.age + }, + { + attribute: :created_at, + direction: "asc", + value: employee2.created_at.iso8601(6) + }, + { + attribute: :id, + direction: "asc", + value: employee2.id + } + ]) + + expect(decode(json["data"][1]["meta"]["cursor"])) + .to eq([ + { + attribute: :age, + direction: "desc", + value: employee1.age + }, + { + attribute: :created_at, + direction: "asc", + value: employee1.created_at.iso8601(6) + }, + { + attribute: :id, + direction: "asc", + value: employee1.id + } + ]) + end + end + end + end + + context "when .cursor_paginatable == false" do + before do + resource.cursor_paginatable = false + end + + it "does not render meta/cursor" do + render + expect(json["data"][0]).to_not have_key("meta") + expect(json["data"][1]).to_not have_key("meta") + end + + context "but cursor_on_demand is true and cursor requested" do + around do |e| + original = Graphiti.config.cursor_on_demand + Graphiti.config.cursor_on_demand = true + begin + e.run + ensure + Graphiti.config.cursor_on_demand = original + end + end + + before do + params[:cursor] = true + end + + it "still does not render meta/cursor" do + render + expect(json["data"][0]).to_not have_key("meta") + expect(json["data"][1]).to_not have_key("meta") + end + end + end + + context "when cursor_on_demand" do + around do |e| + original = Graphiti.config.cursor_on_demand + Graphiti.config.cursor_on_demand = true + begin + e.run + ensure + Graphiti.config.cursor_on_demand = original + end + end + + context "and cursor param is passed" do + before do + params[:cursor] = true + end + + it "renders cursor in meta" do + render + expect(json["data"][0]["meta"]["cursor"]).to be_present + expect(json["data"][1]["meta"]["cursor"]).to be_present + end + + context "but resource is not cursor paginatable" do + before do + resource.cursor_paginatable = false + end + + it "does not render meta/cursor" do + render + expect(json["data"][0]).to_not have_key("meta") + expect(json["data"][1]).to_not have_key("meta") + end + end + end + + context "and cursor param is not passed" do + it "does not render meta/cursor" do + render + expect(json["data"][0]).to_not have_key("meta") + expect(json["data"][1]).to_not have_key("meta") + end + end + end + + context "when the cursor has special rendering logic" do + before do + resource.attribute :timestamp, :datetime do + @object.created_at + end + + resource.sort :timestamp, cursorable: true + resource.default_cursor = :timestamp + end + + it "is honored" do + render + + expect(decode(json["data"][0]["meta"]["cursor"])) + .to eq([ + { + attribute: :timestamp, + direction: "asc", + value: employee1.created_at.iso8601(6) + } + ]) + expect(decode(json["data"][1]["meta"]["cursor"])) + .to eq([ + { + attribute: :timestamp, + direction: "asc", + value: employee2.created_at.iso8601(6) + } + ]) + end + + context "but the attribute is unreadable" do + before do + resource.attribute :timestamp, :datetime, readable: false do + @object.created_at + end + end + + it "still honors the serialization block" do + render + + expect(decode(json["data"][0]["meta"]["cursor"])) + .to eq([ + { + attribute: :timestamp, + direction: "asc", + value: employee1.created_at.iso8601(6) + } + ]) + + expect(decode(json["data"][1]["meta"]["cursor"])) + .to eq([ + { + attribute: :timestamp, + direction: "asc", + value: employee2.created_at.iso8601(6) + } + ]) + end + end + end + end end diff --git a/spec/sorting_spec.rb b/spec/sorting_spec.rb index ade318bc..06a83e99 100644 --- a/spec/sorting_spec.rb +++ b/spec/sorting_spec.rb @@ -327,4 +327,86 @@ def admin? end end end + + context "when cursor pagination is on" do + before do + resource.cursor_paginatable = true + end + + context "and given an uncursorable sort" do + before do + params[:sort] = "-age" + end + + it "applies multisort with default cursor" do + expect_any_instance_of(resource.adapter).to receive(:order) + .with(anything, :age, :desc).and_call_original + expect_any_instance_of(resource.adapter).to receive(:order) + .with(anything, :id, :asc).and_call_original + ids + end + + context "with custom default cursor" do + before do + resource.sort :created_at, :datetime, cursorable: true + resource.default_cursor = :created_at + end + + it "is applied correctly as multisort" do + expect_any_instance_of(resource.adapter).to receive(:order) + .with(anything, :age, :desc).and_call_original + expect_any_instance_of(resource.adapter).to receive(:order) + .with(anything, :created_at, :asc).and_call_original + ids + end + end + end + + context "and given a cursorable sort" do + before do + resource.sort :created_at, :datetime, cursorable: true + params[:sort] = "-created_at" + end + + it "does not add the default cursor to sorts" do + expect_any_instance_of(resource.adapter).to receive(:order) + .with(anything, :created_at, :desc).and_call_original + ids + end + + context "that has custom sorting logic" do + before do + resource.sort :identifier, :integer, cursorable: true do |scope, dir| + scope[:sort] ||= [] + scope[:sort] << {id: dir} + scope + end + params[:sort] = "-identifier" + end + + it "honors the custom sort logic" do + expect(ids).to eq([2, 1]) + end + + it "does not add the default cursor to sorts" do + expect(PORO::DB).to receive(:all) + .with(hash_including(sort: [{id: :desc}])) + .and_call_original + ids + end + end + end + + context "and given the default cursor as sort" do + before do + params[:sort] = "-id" + end + + it "does not add the default cursor to sorts" do + expect_any_instance_of(resource.adapter).to receive(:order) + .with(anything, :id, :desc).once.and_call_original + ids + end + end + end end