diff --git a/lib/rights_api.rb b/lib/rights_api.rb index cb5c811..435c7a7 100644 --- a/lib/rights_api.rb +++ b/lib/rights_api.rb @@ -4,8 +4,11 @@ module RightsAPI end require "rights_api/app" +require "rights_api/cursor" require "rights_api/database" +require "rights_api/order" require "rights_api/query" +require "rights_api/query_parser" require "rights_api/result" require "rights_api/result/error_result" require "rights_api/schema" diff --git a/lib/rights_api/app.rb b/lib/rights_api/app.rb index 547e20e..d987c22 100644 --- a/lib/rights_api/app.rb +++ b/lib/rights_api/app.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require "cgi" require "sinatra" require "sinatra/json" require "sinatra/reloader" if development? diff --git a/lib/rights_api/cursor.rb b/lib/rights_api/cursor.rb new file mode 100644 index 0000000..dd4dfae --- /dev/null +++ b/lib/rights_api/cursor.rb @@ -0,0 +1,114 @@ +# frozen_string_literal: true + +require "base64" +require "json" +require "sequel" + +# Given a list of sort fields -- Array of symbols +# and a list of field -> value mappings (the last result) +# create a string that can be decoded into +# a WHERE list + +# Tries to store just enough information so the next query can +# pick up where the current one left off. The Base64-encoded cursor string +# is a serialized array containing the current offset and one value for each +# sort parameter (explicit or default) used in the query. + +# CURSOR LIFECYCLE +# - At the beginning of the query a Cursor is created with the "cursor" URL parameter, or +# nil if none was supplied (indicating first page of results, i.e. no previous query). +# - Query calls `cursor.where` to create a WHERE clause based on the decoded values +# from the previous result (in effect saying "WHERE fields > last_result") +# - Query calls `cursor.offset` for the current page of results. +# - Query calls `cursor.encode` to calculate a new offset and new last_result values +# dictated by the current ORDER BY. + +# IVAR SEMANTICS +# - `offset` the (zero-based) offset into the overall results set produced by `where`, or perhaps +# "give me the results at offset N (by using values X Y and Z)" +# - `values` the X Y and Z from above, these are the relevant values from the previous result +# if there was one. +# It is possibly counterintuitive that X Y and Z are NOT at offset N. Offset N is the location +# of the NEXT record. + +# CAVEATS +# Relies on the search parameters being unchanged between queries, +# if they do change then the results are undefined. +module RightsAPI + class Cursor + OFFSET_KEY = "off" + LAST_ROW_KEY = "last" + attr_reader :values, :offset + + # @param cursor_string [String] the URL parameter to decode + # @return [Array] of the form [offset, "val1", "val2" ...] + def self.decode(cursor_string) + JSON.parse(Base64.urlsafe_decode64(cursor_string)) + end + + # JSON-encode and Base64-encode an object + # @param arg [Object] a serializable object (always an Array in this class) + # @return [String] + def self.encode(arg) + Base64.urlsafe_encode64(JSON.generate(arg)) + end + + def initialize(cursor_string: nil) + @offset = 0 + @values = [] + if cursor_string + @values = self.class.decode cursor_string + @offset = @values.shift + end + end + + # Generate zero or one WHERE clauses that will generate a pseudo-OFFSET + # based on ORDER BY parameters. + # ORDER BY a, b, c TRANSLATES TO + # WHERE (a > 1) + # OR (a = 1 AND b > 2) + # OR (a = 1 AND b = 2 AND c > 3) + # @param model [Class] Sequel::Model subclass for the table being queried, + # only used for qualifying column names in the WHERE clause. + # @param order [Array] the current query's ORDER BY + # @return [Array] zero or one Sequel literals + def where(model:, order:) + return [] if values.empty? + + # Create one OR clause for each ORDER. + # Each OR clause is a series of AND clauses. + # The last element of each AND clause is < or >, the others are = + # The first AND clause has only the first ORDER parameter. + # Each subsequent one adds one ORDER parameter. + or_clause = [] + order.count.times do |order_index| + # Take a slice of ORDER of size order_index + 1 + and_clause = order[0, order_index + 1].each_with_index.map do |ord, i| + # in which each element is a "col op val" string and the last is an inequality + op = if i == order_index + ord.asc? ? ">" : "<" + else + "=" + end + "#{model.table_name}.#{ord.column}#{op}'#{values[i]}'" + end + or_clause << "(" + and_clause.join(" AND ") + ")" + end + [Sequel.lit(or_clause.join(" OR "))] + end + + # Encode the offset and the relevant values from the last result row + # (i.e. those used in the current ORDER BY) + # @param order [Array] the current query's ORDER BY + # @param rows [Sequel::Dataset] the result of the current query + # @return [String] + def encode(order:, rows:) + data = [offset + rows.count] + row = rows.last + order.each do |ord| + data << row[ord.column] + end + self.class.encode data + end + end +end diff --git a/lib/rights_api/model_extensions.rb b/lib/rights_api/model_extensions.rb index 75c9a75..136e12e 100644 --- a/lib/rights_api/model_extensions.rb +++ b/lib/rights_api/model_extensions.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require_relative "order" + module RightsAPI module ModelExtensions # Overridden by classes that want to do some kind of #eager or #eager_graph @@ -19,9 +21,9 @@ def default_key end # For use in ORDER BY clause. - # @return [Sequel::SQL::QualifiedIdentifier] + # @return [Array] def default_order - query_for_field field: default_key + [Order.new(column: default_key)] end # @param field [String, Symbol] diff --git a/lib/rights_api/models/access_statement_map.rb b/lib/rights_api/models/access_statement_map.rb index f2cd142..9d127df 100644 --- a/lib/rights_api/models/access_statement_map.rb +++ b/lib/rights_api/models/access_statement_map.rb @@ -9,6 +9,13 @@ def self.default_key :attr_access_id end + def self.default_order + [ + Order.new(column: :a_attr), + Order.new(column: :a_access_profile) + ] + end + # @param [String, Symbol] field # @return [Sequel::SQL::Expression] def self.query_for_field(field:) diff --git a/lib/rights_api/models/rights_current.rb b/lib/rights_api/models/rights_current.rb index a6f78f1..11ef80b 100644 --- a/lib/rights_api/models/rights_current.rb +++ b/lib/rights_api/models/rights_current.rb @@ -30,10 +30,14 @@ def self.query_for_field(field:) super field: field end - # rights_current and rights_log should order by timestamp - # @return [Sequel::SQL::Expression] + # rights_current and rights_log should order by htid, timestamp + # @return [Array] def self.default_order - qualify field: :time + [ + Order.new(column: :namespace), + Order.new(column: :id), + Order.new(column: :time) + ] end def to_h diff --git a/lib/rights_api/models/rights_log.rb b/lib/rights_api/models/rights_log.rb index 3330436..5c3c0ac 100644 --- a/lib/rights_api/models/rights_log.rb +++ b/lib/rights_api/models/rights_log.rb @@ -10,7 +10,7 @@ class RightsLog < Sequel::Model(:rights_log) many_to_one :attribute_obj, class: :"RightsAPI::Attribute", key: :attr many_to_one :reason_obj, class: :"RightsAPI::Reason", key: :reason many_to_one :source_obj, class: :"RightsAPI::Source", key: :source - set_primary_key [:namespace, :id] + set_primary_key [:namespace, :id, :time] # Maybe TOO eager. This makes us partially responsible for the fact that rights_current.source # has an embedded access_profile. @@ -31,10 +31,14 @@ def self.query_for_field(field:) super field: field end - # rights_current and rights_log should order by timestamp - # @return [Sequel::SQL::Expression] + # rights_current and rights_log should order by htid, timestamp + # @return [Array] def self.default_order - qualify field: :time + [ + Order.new(column: :namespace), + Order.new(column: :id), + Order.new(column: :time) + ] end def to_h diff --git a/lib/rights_api/order.rb b/lib/rights_api/order.rb new file mode 100644 index 0000000..b66a9cd --- /dev/null +++ b/lib/rights_api/order.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +# A class that encapsulates the field and ASC/DESC properties of a +# single ORDER BY argument. + +module RightsAPI + class Order + attr_reader :column + # @param column [Symbol] the field to ORDER BY + # @param asc [Boolean] true if ASC, false if DESC + def initialize(column:, asc: true) + @column = column + @asc = asc + end + + # @return [Boolean] is the order direction ASC? + def asc? + @asc + end + + # @return [Sequel::SQL::OrderedExpression] + def to_sequel(model:) + if asc? + Sequel.asc(model.qualify(field: column)) + else + Sequel.desc(model.qualify(field: column)) + end + end + end +end diff --git a/lib/rights_api/query.rb b/lib/rights_api/query.rb index 7ab8c52..1c0b9e9 100644 --- a/lib/rights_api/query.rb +++ b/lib/rights_api/query.rb @@ -1,16 +1,15 @@ # frozen_string_literal: true require "benchmark" -require "cgi" +require_relative "cursor" require_relative "error" require_relative "query_parser" require_relative "result" -require_relative "services" module RightsAPI class Query - attr_reader :model, :params, :parser, :total, :dataset + attr_reader :model, :params, :parser, :total # @param model [Class] Sequel::Model subclass for the table being queried # @param params [Hash] CGI parameters submitted to the Sinatra frontend @@ -19,29 +18,35 @@ def initialize(model:, params: {}) @params = params @parser = QueryParser.new(model: model) @total = 0 - @dataset = nil end # @return [Result] def run + dataset = nil # This may raise QueryParserError parser.parse(params: params) time_delta = Benchmark.realtime do - @dataset = model.base_dataset + dataset = model.base_dataset parser.where.each do |where| - @dataset = dataset.where(where) + dataset = dataset.where(where) end - # Save this here because offset and limit may alter the count. + dataset = dataset.order(*(parser.order.map { |order| order.to_sequel(model: model) })) + # Save this here because limit and cursor would otherwise alter the count. @total = dataset.count - @dataset = dataset.order(*parser.order) - .offset(parser.offset) - .limit(parser.limit) - .all + # Apply the cursor to get to the offset we want + parser.cursor.where(model: model, order: parser.order).each do |where| + dataset = dataset.where(where) + end + dataset = dataset.limit(parser.limit).all end - result = Result.new(offset: parser.offset, total: total, milliseconds: 1000 * time_delta) + result = Result.new(offset: parser.cursor.offset, total: total, milliseconds: 1000 * time_delta) dataset.each do |row| result.add! row: row.to_h end + if result.more? + cursor = parser.cursor.encode(order: parser.order, rows: dataset) + result.cursor = cursor + end result end end diff --git a/lib/rights_api/query_parser.rb b/lib/rights_api/query_parser.rb index 8a3cde4..3b352cb 100644 --- a/lib/rights_api/query_parser.rb +++ b/lib/rights_api/query_parser.rb @@ -2,7 +2,9 @@ require "sequel" +require_relative "cursor" require_relative "error" +require_relative "order" # Processes the Hash of URL parameters passed to the API into an # Array of WHERE constraints, as well as LIMIT, and OFFSET values. @@ -11,16 +13,15 @@ module RightsAPI class QueryParser DEFAULT_LIMIT = 1000 - DEFAULT_OFFSET = 0 - attr_reader :params, :model, :where, :order, :offset, :limit + attr_reader :params, :model, :where, :order, :limit # @param model [Class] Sequel::Model subclass for the table being queried def initialize(model:) @model = model @where = [] @order = [] + @cursor = nil @limit = DEFAULT_LIMIT - @offset = DEFAULT_OFFSET end def parse(params: {}) @@ -28,18 +29,24 @@ def parse(params: {}) params.each do |key, values| key = key.to_sym case key - when :offset - parse_offset(values: values) + when :cursor + parse_cursor(values: values) when :limit parse_limit(values: values) else parse_parameter(key: key, values: values) end end - @order = [model.default_order] if @order.empty? + # Always tack on the default order even if it is redundant. + # The cursor implementation requires that there be an intrinsic order. + @order += model.default_order self end + def cursor + @cursor ||= Cursor.new + end + private # Parses a general search parameter and appends the resulting Sequel @@ -55,9 +62,16 @@ def parse_parameter(key:, values:) end end - # Extract a single integer that can be passed to dataset.offset. - def parse_offset(values:) - @offset = parse_int_value(values: values, type: "OFFSET") + # Parse cursor value into an auxiliary WHERE clause + def parse_cursor(values:) + if values.count > 1 + raise QueryParserError, "multiple cursor values (#{values})" + end + begin + @cursor = Cursor.new(cursor_string: values.first) + rescue ArgumentError => e + raise QueryParserError, "cannot decode cursor: #{e.message}" + end end # Extract a single integer that can be passed to dataset.limit. @@ -72,6 +86,8 @@ def parse_limit(values:) # @param type [String] "OFFSET" or "LIMIT", used only for reporting errors. # @return [Integer] def parse_int_value(values:, type:) + return values.last if values.last.is_a? Integer + value = values.last.to_i # Make sure the offset can make a round-trip conversion between Int and String # https://stackoverflow.com/a/1235891 diff --git a/lib/rights_api/result.rb b/lib/rights_api/result.rb index e2d1303..5c49325 100644 --- a/lib/rights_api/result.rb +++ b/lib/rights_api/result.rb @@ -5,7 +5,7 @@ # and then data is populated according to the requested offset and limit. module RightsAPI class Result - attr_reader :offset, :total, :start, :end, :milliseconds, :data + attr_reader :offset, :total, :start, :end, :milliseconds, :cursor, :data # @param offset [Integer] The offset=x URL parameter. # @param total [Integer] The total number of results, regardless of paging. @@ -13,6 +13,7 @@ def initialize(offset: 0, total: 0, milliseconds: 0.0) @offset = offset @total = total @milliseconds = milliseconds + @cursor = nil @start = 0 @end = 0 @data = [] @@ -32,6 +33,15 @@ def add!(row:) self end + # @return [Boolean] are there any more results after this one? + def more? + @end < @total + end + + def cursor=(arg) + @cursor = arg unless arg.nil? + end + # @return [Hash] def to_h h = { @@ -41,6 +51,7 @@ def to_h "milliseconds" => @milliseconds, "data" => @data } + h["cursor"] = @cursor unless @cursor.nil? finalize h end diff --git a/spec/integration/rights_log_spec.rb b/spec/integration/rights_log_spec.rb index a58d663..9098f68 100644 --- a/spec/integration/rights_log_spec.rb +++ b/spec/integration/rights_log_spec.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require "climate_control" require "rack/test" require "shared_examples" @@ -16,16 +17,39 @@ before(:each) { get(rights_api_endpoint + "rights_log/bogus.no_such_id") } it_behaves_like "empty response" end + end - context "with no HTID" do - before(:each) { get(rights_api_endpoint + "rights_log") } - it_behaves_like "nonempty rights response" + context "with no HTID" do + before(:each) { get(rights_api_endpoint + "rights_log") } + it_behaves_like "nonempty rights response" - it "obeys default sort order `time ASC`" do - response = parse_json(last_response.body) - sorted = response[:data].sort_by { |a| a[:time] } - expect(response[:data]).to eq(sorted) + it "obeys default sort order `namespace, id, time`" do + response = parse_json(last_response.body) + sorted = response[:data].sort do |a, b| + a[:namespace] <=> b[:namespace] || + collator.compare(a[:id], b[:id]) || + a[:time] <=> b[:time] end + expect(response[:data]).to eq(sorted) + end + end + + context "with a cursor" do + before(:each) { get(rights_api_endpoint + "rights_log?limit=2") } + it_behaves_like "nonempty rights response" + + it "returns a cursor" do + response = parse_json(last_response.body) + expect(response[:cursor]).to be_a(String) + end + + it "produces next page of results when cursor is used" do + response_1 = parse_json(last_response.body) + cursor = response_1[:cursor] + get(rights_api_endpoint + "rights_log?limit=2&cursor=#{cursor}") + response_2 = parse_json(last_response.body) + expect(response_2[:total]).to eq(response_1[:total]) + expect(response_2[:start]).to eq(3) end end end diff --git a/spec/integration/rights_spec.rb b/spec/integration/rights_spec.rb index 3097e4c..7c0ecd8 100644 --- a/spec/integration/rights_spec.rb +++ b/spec/integration/rights_spec.rb @@ -6,24 +6,49 @@ RSpec.describe "/rights" do include Rack::Test::Methods - context "with a valid HTID" do - before(:each) { get(rights_api_endpoint + "rights/test.pd_google") } - it_behaves_like "nonempty rights response" - end + describe "/rights_log/HTID" do + context "with a valid HTID" do + before(:each) { get(rights_api_endpoint + "rights/test.pd_google") } + it_behaves_like "nonempty rights response" + end - context "with an invalid HTID" do - before(:each) { get(rights_api_endpoint + "rights/bogus.no_such_id") } - it_behaves_like "empty response" + context "with an invalid HTID" do + before(:each) { get(rights_api_endpoint + "rights/bogus.no_such_id") } + it_behaves_like "empty response" + end end context "with no HTID" do before(:each) { get(rights_api_endpoint + "rights") } it_behaves_like "nonempty rights response" - it "obeys default sort order `time ASC`" do + it "obeys default sort order `namespace, id, time`" do response = parse_json(last_response.body) - sorted = response[:data].sort_by { |a| a[:time] } + sorted = response[:data].sort do |a, b| + a[:namespace] <=> b[:namespace] || + collator.compare(a[:id], b[:id]) || + a[:time] <=> b[:time] + end expect(response[:data]).to eq(sorted) end end + + context "with a cursor" do + before(:each) { get(rights_api_endpoint + "rights?limit=2") } + it_behaves_like "nonempty rights response" + + it "returns a cursor" do + response = parse_json(last_response.body) + expect(response[:cursor]).to be_a(String) + end + + it "produces next page of results when cursor is used" do + response_1 = parse_json(last_response.body) + cursor = response_1[:cursor] + get(rights_api_endpoint + "rights?limit=2&cursor=#{cursor}") + response_2 = parse_json(last_response.body) + expect(response_2[:total]).to eq(response_1[:total]) + expect(response_2[:start]).to eq(3) + end + end end diff --git a/spec/integration/support_table_spec.rb b/spec/integration/support_table_spec.rb index f982b92..2fd1343 100644 --- a/spec/integration/support_table_spec.rb +++ b/spec/integration/support_table_spec.rb @@ -42,24 +42,24 @@ end # With OFFSET - SUPPORT_TABLES.each do |table| - describe "/#{table}/offset=" do - context "with a valid offset value" do - before(:each) { get(rights_api_endpoint + table + "?offset=1") } - it_behaves_like "nonempty #{table} response" - - it "returns data set starting at index 2" do - response = parse_json(last_response.body) - expect(response[:start]).to eq(2) - end - end - - context "with a bogus offset value" do - before(:each) { get(rights_api_endpoint + table + "?offset=x") } - it_behaves_like "400 response" - end - end - end + # SUPPORT_TABLES.each do |table| + # describe "/#{table}/offset=" do + # context "with a valid offset value" do + # before(:each) { get(rights_api_endpoint + table + "?offset=1") } + # it_behaves_like "nonempty #{table} response" + # + # it "returns data set starting at index 2" do + # response = parse_json(last_response.body) + # expect(response[:start]).to eq(2) + # end + # end + # + # context "with a bogus offset value" do + # before(:each) { get(rights_api_endpoint + table + "?offset=x") } + # it_behaves_like "400 response" + # end + # end + # end # With LIMIT SUPPORT_TABLES.each do |table| diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b235fbf..2916534 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -33,6 +33,12 @@ def rights_api_endpoint "/v1/" end +module RightsAPI + VALID_CURSOR = "WzIsMl0=" + ANOTHER_VALID_CURSOR = "WzQsNF0=" + INVALID_CURSOR = "%" +end + # BEGIN RSPEC BOILERPLATE # This file was generated by the `rspec --init` command. Conventionally, all diff --git a/spec/unit/cursor_spec.rb b/spec/unit/cursor_spec.rb new file mode 100644 index 0000000..6f44126 --- /dev/null +++ b/spec/unit/cursor_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +module RightsAPI + RSpec.describe Cursor do + let(:empty_cursor) { described_class.new } + + describe ".encode" do + context "with an object" do + it "returns a string" do + expect(described_class.encode({})).to be_a(String) + end + end + + context "with nil" do + it "returns a string" do + expect(described_class.encode(nil)).to be_a(String) + end + end + end + + describe ".decode" do + context "with a valid cursor string" do + it "returns an object" do + expect(described_class.decode(VALID_CURSOR)).to be_a(Array) + end + end + + context "with a bogus cursor string" do + it "raises ArgumentError" do + expect { described_class.decode(INVALID_CURSOR) }.to raise_error(ArgumentError) + end + end + end + + describe "#initialize" do + context "with a valid cursor string" do + it "creates RightsAPI::Cursor instance" do + expect(described_class.new(cursor_string: VALID_CURSOR)).to be_a(RightsAPI::Cursor) + end + end + + context "with an empty cursor string" do + it "creates RightsAPI::Cursor instance" do + expect(empty_cursor).to be_a(RightsAPI::Cursor) + end + end + + context "with a bogus cursor string" do + it "raises ArgumentError" do + expect { described_class.new(INVALID_CURSOR) }.to raise_error(ArgumentError) + end + end + end + + describe "#encode" do + it "returns the expected cursor string" do + expected = described_class.encode([1, "test", "cc-by-3.0_google", "2009-01-01 05:00:00 +0000"]) + row = {namespace: "test", id: "cc-by-3.0_google", time: "2009-01-01 05:00:00 +0000"} + expect(empty_cursor.encode(order: RightsCurrent.default_order, rows: [row])).to eq(expected) + end + end + + describe "#where" do + context "with no results" do + it "returns an empty Array" do + expect(empty_cursor.where(model: RightsCurrent, order: [])).to match_array([]) + end + end + + it "produces a one-element Array with a Sequel literal" do + cursor_string = described_class.encode([0, "test", "cc-by-3.0_google", "2009-01-01 05:00:00 +0000"]) + cursor = described_class.new(cursor_string: cursor_string) + expected = <<~END.delete("\n") + (rights_current.namespace>'test') OR + (rights_current.namespace='test' AND rights_current.id>'cc-by-3.0_google') OR + (rights_current.namespace='test' AND rights_current.id='cc-by-3.0_google' AND rights_current.time>'2009-01-01 05:00:00 +0000') + END + expect(cursor.where(model: RightsCurrent, order: RightsCurrent.default_order).first.to_s).to eq(expected) + end + end + end +end diff --git a/spec/unit/models/access_profile_spec.rb b/spec/unit/models/access_profile_spec.rb index 9d2524a..95c465c 100644 --- a/spec/unit/models/access_profile_spec.rb +++ b/spec/unit/models/access_profile_spec.rb @@ -11,8 +11,8 @@ module RightsAPI end describe "#default_order" do - it "returns SQL expression" do - expect(described_class.default_order).to be_a(Sequel::SQL::QualifiedIdentifier) + it "returns Array" do + expect(described_class.default_order).to be_a(Array) end end diff --git a/spec/unit/models/access_statement_map_spec.rb b/spec/unit/models/access_statement_map_spec.rb index 83051ae..a7f9ce3 100644 --- a/spec/unit/models/access_statement_map_spec.rb +++ b/spec/unit/models/access_statement_map_spec.rb @@ -17,8 +17,8 @@ module RightsAPI end describe "#default_order" do - it "returns SQL expression" do - expect(described_class.default_order).to be_a(Sequel::SQL::Expression) + it "returns Array" do + expect(described_class.default_order).to be_a(Array) end end diff --git a/spec/unit/models/access_statement_spec.rb b/spec/unit/models/access_statement_spec.rb index 741f167..bb4df2a 100644 --- a/spec/unit/models/access_statement_spec.rb +++ b/spec/unit/models/access_statement_spec.rb @@ -17,8 +17,8 @@ module RightsAPI end describe "#default_order" do - it "returns SQL expression" do - expect(described_class.default_order).to be_a(Sequel::SQL::QualifiedIdentifier) + it "returns Array" do + expect(described_class.default_order).to be_a(Array) end end diff --git a/spec/unit/models/attribute_spec.rb b/spec/unit/models/attribute_spec.rb index ec16dc5..b611bbe 100644 --- a/spec/unit/models/attribute_spec.rb +++ b/spec/unit/models/attribute_spec.rb @@ -17,8 +17,8 @@ module RightsAPI end describe "#default_order" do - it "returns SQL expression" do - expect(described_class.default_order).to be_a(Sequel::SQL::QualifiedIdentifier) + it "returns Array" do + expect(described_class.default_order).to be_a(Array) end end diff --git a/spec/unit/models/reason_spec.rb b/spec/unit/models/reason_spec.rb index b4726a1..ba021e5 100644 --- a/spec/unit/models/reason_spec.rb +++ b/spec/unit/models/reason_spec.rb @@ -17,8 +17,8 @@ module RightsAPI end describe "#default_order" do - it "returns SQL expression" do - expect(described_class.default_order).to be_a(Sequel::SQL::QualifiedIdentifier) + it "returns Array" do + expect(described_class.default_order).to be_a(Array) end end diff --git a/spec/unit/models/rights_current_spec.rb b/spec/unit/models/rights_current_spec.rb index c9a43a5..0925277 100644 --- a/spec/unit/models/rights_current_spec.rb +++ b/spec/unit/models/rights_current_spec.rb @@ -17,8 +17,11 @@ module RightsAPI end describe "#default_order" do - it "returns timestamp" do - expect(described_class.default_order).to be_a(Sequel::SQL::Expression) + it "returns Array of RightsAPI::Order" do + expect(described_class.default_order).to be_a(Array) + described_class.default_order.each do |order| + expect(order).to be_a(RightsAPI::Order) + end end end diff --git a/spec/unit/models/rights_log_spec.rb b/spec/unit/models/rights_log_spec.rb index 416233e..9083d6d 100644 --- a/spec/unit/models/rights_log_spec.rb +++ b/spec/unit/models/rights_log_spec.rb @@ -17,8 +17,11 @@ module RightsAPI end describe "#default_order" do - it "returns timestamp" do - expect(described_class.default_order).to be_a(Sequel::SQL::Expression) + it "returns Array of RightsAPI::Order" do + expect(described_class.default_order).to be_a(Array) + described_class.default_order.each do |order| + expect(order).to be_a(RightsAPI::Order) + end end end diff --git a/spec/unit/models/source_spec.rb b/spec/unit/models/source_spec.rb index 4746655..ad6d3ba 100644 --- a/spec/unit/models/source_spec.rb +++ b/spec/unit/models/source_spec.rb @@ -17,8 +17,8 @@ module RightsAPI end describe "#default_order" do - it "returns SQL expression" do - expect(described_class.default_order).to be_a(Sequel::SQL::QualifiedIdentifier) + it "returns Array" do + expect(described_class.default_order).to be_a(Array) end end diff --git a/spec/unit/order_spec.rb b/spec/unit/order_spec.rb new file mode 100644 index 0000000..0550e63 --- /dev/null +++ b/spec/unit/order_spec.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +module RightsAPI + RSpec.describe Order do + let(:order) { described_class.new(column: :id) } + let(:order_desc) { described_class.new(column: :id, asc: false) } + + describe ".new" do + it "creates a #{described_class}" do + expect(order).to be_a(described_class) + end + + it "has the expected attribute reader" do + %i[column].each do |reader| + expect(order.send(reader)).not_to be_nil + end + end + end + + describe "#asc?" do + context "with default direction" do + it "returns true" do + expect(order.asc?).to eq true + end + end + + context "with asc false" do + it "returns false" do + expect(order_desc.asc?).to eq false + end + end + end + + describe "#to_sequel" do + context "with default direction" do + it "returns Sequel::SQL::OrderedExpression with #descending == false" do + expect(order.to_sequel(model: RightsCurrent)).to be_a(Sequel::SQL::OrderedExpression) + expect(order.to_sequel(model: RightsCurrent).descending).to eq(false) + end + end + + context "with asc false" do + it "returns Sequel::SQL::OrderedExpression with #descending == true" do + expect(order_desc.to_sequel(model: RightsCurrent)).to be_a(Sequel::SQL::OrderedExpression) + expect(order_desc.to_sequel(model: RightsCurrent).descending).to eq(true) + end + end + end + end +end diff --git a/spec/unit/query_parser_spec.rb b/spec/unit/query_parser_spec.rb index 3c03cd2..44d307c 100644 --- a/spec/unit/query_parser_spec.rb +++ b/spec/unit/query_parser_spec.rb @@ -1,11 +1,8 @@ # frozen_string_literal: true -require "sequel" - module RightsAPI RSpec.describe(QueryParser) do let(:query_parser) { described_class.new(model: Attribute) } - # let(:id_query) { described_class.new(params: "id=some+id", table_name: "rights") } describe ".new" do it "creates a #{described_class}" do @@ -13,23 +10,23 @@ module RightsAPI end it "has the expected attribute readers" do - %i[model where order offset limit].each do |reader| + %i[model where order limit].each do |reader| expect(query_parser.send(reader)).not_to be_nil end end end describe "#parse" do - it "parses empty query into zero WHERE clauses" do - expect(query_parser.parse.where.count).to eq(0) + it "parses cursor" do + expect(query_parser.parse(params: {cursor: [VALID_CURSOR]}).cursor).to be_a(RightsAPI::Cursor) end - it "parses id query into one WHERE clause" do - expect(query_parser.parse(params: {id: ["1"]}).where.count).to eq(1) + it "raises on bogus cursor" do + expect { query_parser.parse(params: {cursor: [INVALID_CURSOR]}) }.to raise_error(QueryParserError) end - it "parses OFFSET query" do - expect(query_parser.parse(params: {offset: ["1"]}).offset).to eq(1) + it "raises on multiple cursors" do + expect { query_parser.parse(params: {cursor: [VALID_CURSOR, ANOTHER_VALID_CURSOR]}) }.to raise_error(QueryParserError) end it "parses LIMIT query" do @@ -39,9 +36,11 @@ module RightsAPI it "raises on bogus LIMIT queries" do expect { query_parser.parse(params: {limit: ["a"]}) }.to raise_error(QueryParserError) end + end - it "raises on bogus OFFSET queries" do - expect { query_parser.parse(params: {offset: ["a"]}) }.to raise_error(QueryParserError) + describe "#cursor" do + it "returns a RightsAPI::Cursor" do + expect(query_parser.cursor).to be_a(RightsAPI::Cursor) end end end diff --git a/spec/unit/query_spec.rb b/spec/unit/query_spec.rb index b5b2994..ebda2a1 100644 --- a/spec/unit/query_spec.rb +++ b/spec/unit/query_spec.rb @@ -6,6 +6,7 @@ module RightsAPI RSpec.describe(Query) do let(:query) { described_class.new(model: Attribute) } let(:query_with_params) { described_class.new(model: Attribute, params: {id: [1]}) } + let(:query_with_limit) { described_class.new(model: Attribute, params: {limit: [2]}) } describe ".new" do it "creates a Query" do @@ -25,6 +26,13 @@ module RightsAPI expect(query.run).to be_a_kind_of(Result) end end + + context "with a limit" do + it "returns a Result with a Cursor" do + expect(query_with_limit.run).to be_a_kind_of(Result) + expect(query_with_limit.run.cursor).to be_a(String) + end + end end end end diff --git a/spec/unit/result_spec.rb b/spec/unit/result_spec.rb index b24fb18..8816c4a 100644 --- a/spec/unit/result_spec.rb +++ b/spec/unit/result_spec.rb @@ -47,6 +47,32 @@ module RightsAPI end end + describe "#more?" do + context "with a total greater than rows added" do + it "returns true" do + res = described_class.new(total: 4) + 2.times { |i| res.add! row: {} } + expect(res.more?).to be true + end + end + + context "with a total equal to rows added" do + it "returns false" do + res = described_class.new(total: 4) + 4.times { |i| res.add! row: {} } + expect(res.more?).to be false + end + end + end + + describe "#cursor=" do + it "sets the cursor" do + res = described_class.new(total: 1) + res.cursor = "cursor" + expect(res.cursor).to eq "cursor" + end + end + describe "#to_h" do it "returns a hash" do expect(result.to_h).to be_a(Hash)