From 7fd99ae2a4424cf996adcc1a3c3f2a753c0ec5aa Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 29 Jul 2022 12:11:29 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../new_graphql_keyset_pagination.yml | 8 - .../keyset/conditions/base_condition.rb | 62 -- .../keyset/conditions/not_null_condition.rb | 55 -- .../keyset/conditions/null_condition.rb | 39 -- .../graphql/pagination/keyset/connection.rb | 97 +-- .../keyset/generic_keyset_pagination.rb | 98 --- .../graphql/pagination/keyset/order_info.rb | 124 ---- .../pagination/keyset/query_builder.rb | 73 -- .../conditions/not_null_condition_spec.rb | 115 --- .../keyset/conditions/null_condition_spec.rb | 95 --- .../keyset/connection_generic_keyset_spec.rb | 415 ----------- .../pagination/keyset/connection_spec.rb | 663 ++++++++---------- .../pagination/keyset/order_info_spec.rb | 118 ---- .../pagination/keyset/query_builder_spec.rb | 135 ---- .../graphql/type_name_deprecations_spec.rb | 11 +- .../api/graphql/group/group_members_spec.rb | 18 - .../graphql/project/project_members_spec.rb | 18 - spec/requests/api/graphql_spec.rb | 53 +- 18 files changed, 335 insertions(+), 1862 deletions(-) delete mode 100644 config/feature_flags/development/new_graphql_keyset_pagination.yml delete mode 100644 lib/gitlab/graphql/pagination/keyset/conditions/base_condition.rb delete mode 100644 lib/gitlab/graphql/pagination/keyset/conditions/not_null_condition.rb delete mode 100644 lib/gitlab/graphql/pagination/keyset/conditions/null_condition.rb delete mode 100644 lib/gitlab/graphql/pagination/keyset/generic_keyset_pagination.rb delete mode 100644 lib/gitlab/graphql/pagination/keyset/order_info.rb delete mode 100644 lib/gitlab/graphql/pagination/keyset/query_builder.rb delete mode 100644 spec/lib/gitlab/graphql/pagination/keyset/conditions/not_null_condition_spec.rb delete mode 100644 spec/lib/gitlab/graphql/pagination/keyset/conditions/null_condition_spec.rb delete mode 100644 spec/lib/gitlab/graphql/pagination/keyset/connection_generic_keyset_spec.rb delete mode 100644 spec/lib/gitlab/graphql/pagination/keyset/order_info_spec.rb delete mode 100644 spec/lib/gitlab/graphql/pagination/keyset/query_builder_spec.rb diff --git a/config/feature_flags/development/new_graphql_keyset_pagination.yml b/config/feature_flags/development/new_graphql_keyset_pagination.yml deleted file mode 100644 index 7f1c73756b3..00000000000 --- a/config/feature_flags/development/new_graphql_keyset_pagination.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: new_graphql_keyset_pagination -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56751 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/323730 -milestone: '13.10' -type: development -group: group::optimize -default_enabled: true diff --git a/lib/gitlab/graphql/pagination/keyset/conditions/base_condition.rb b/lib/gitlab/graphql/pagination/keyset/conditions/base_condition.rb deleted file mode 100644 index 6645dac36fa..00000000000 --- a/lib/gitlab/graphql/pagination/keyset/conditions/base_condition.rb +++ /dev/null @@ -1,62 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Graphql - module Pagination - module Keyset - module Conditions - class BaseCondition - # @param [Arel::Table] arel_table for the relation being ordered - # @param [Array] order_list of extracted orderings - # @param [Array] values from the decoded cursor - # @param [Array] operators determining sort comparison - # @param [Symbol] before_or_after indicates whether we want - # items :before the cursor or :after the cursor - def initialize(arel_table, order_list, values, operators, before_or_after) - @arel_table = arel_table - @order_list = order_list - @values = values - @operators = operators - @before_or_after = before_or_after - - @before_or_after = :after unless [:after, :before].include?(@before_or_after) - end - - def build - raise NotImplementedError - end - - private - - attr_reader :arel_table, :order_list, :values, :operators, :before_or_after - - def table_condition(order_info, value, operator) - if order_info.named_function - target = order_info.named_function - - if target.try(:name)&.casecmp('lower') == 0 - value = value&.downcase - end - else - target = arel_table[order_info.attribute_name] - end - - case operator - when '>' - target.gt(value) - when '<' - target.lt(value) - when '=' - target.eq(value) - when 'is_null' - target.eq(nil) - when 'is_not_null' - target.not_eq(nil) - end - end - end - end - end - end - end -end diff --git a/lib/gitlab/graphql/pagination/keyset/conditions/not_null_condition.rb b/lib/gitlab/graphql/pagination/keyset/conditions/not_null_condition.rb deleted file mode 100644 index ec70f5c5a24..00000000000 --- a/lib/gitlab/graphql/pagination/keyset/conditions/not_null_condition.rb +++ /dev/null @@ -1,55 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Graphql - module Pagination - module Keyset - module Conditions - class NotNullCondition < BaseCondition - def build - conditions = [first_attribute_condition] - - # If there is only one order field, we can assume it - # does not contain NULLs, and don't need additional - # conditions - unless order_list.count == 1 - conditions << [second_attribute_condition, final_condition] - end - - conditions.join - end - - private - - # ex: "(relative_position > 23)" - def first_attribute_condition - <<~SQL - (#{table_condition(order_list.first, values.first, operators.first).to_sql}) - SQL - end - - # ex: " OR (relative_position = 23 AND id > 500)" - def second_attribute_condition - <<~SQL - OR ( - #{table_condition(order_list.first, values.first, '=').to_sql} - AND - #{table_condition(order_list[1], values[1], operators[1]).to_sql} - ) - SQL - end - - # ex: " OR (relative_position IS NULL)" - def final_condition - if before_or_after == :after - <<~SQL - OR (#{table_condition(order_list.first, nil, 'is_null').to_sql}) - SQL - end - end - end - end - end - end - end -end diff --git a/lib/gitlab/graphql/pagination/keyset/conditions/null_condition.rb b/lib/gitlab/graphql/pagination/keyset/conditions/null_condition.rb deleted file mode 100644 index 1aae1020e79..00000000000 --- a/lib/gitlab/graphql/pagination/keyset/conditions/null_condition.rb +++ /dev/null @@ -1,39 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Graphql - module Pagination - module Keyset - module Conditions - class NullCondition < BaseCondition - def build - [first_attribute_condition, final_condition].join - end - - private - - # ex: "(relative_position IS NULL AND id > 500)" - def first_attribute_condition - <<~SQL - ( - #{table_condition(order_list.first, nil, 'is_null').to_sql} - AND - #{table_condition(order_list[1], values[1], operators[1]).to_sql} - ) - SQL - end - - # ex: " OR (relative_position IS NOT NULL)" - def final_condition - if before_or_after == :before - <<~SQL - OR (#{table_condition(order_list.first, nil, 'is_not_null').to_sql}) - SQL - end - end - end - end - end - end - end -end diff --git a/lib/gitlab/graphql/pagination/keyset/connection.rb b/lib/gitlab/graphql/pagination/keyset/connection.rb index 3e119a39e6d..b074c273996 100644 --- a/lib/gitlab/graphql/pagination/keyset/connection.rb +++ b/lib/gitlab/graphql/pagination/keyset/connection.rb @@ -29,7 +29,6 @@ module Gitlab include Gitlab::Utils::StrongMemoize include ::Gitlab::Graphql::ConnectionCollectionMethods prepend ::Gitlab::Graphql::ConnectionRedaction - prepend GenericKeysetPagination # rubocop: disable Naming/PredicateName # https://relay.dev/graphql/connections.htm#sec-undefined.PageInfo.Fields @@ -58,19 +57,13 @@ module Gitlab def has_next_page strong_memoize(:has_next_page) do if before - # If `before` is specified, that points to a specific record, - # even if it's the last one. Since we're asking for `before`, - # then the specific record we're pointing to is in the - # next page true elsif first case sliced_nodes when Array sliced_nodes.size > limit_value else - # If we count the number of requested items plus one (`limit_value + 1`), - # then if we get `limit_value + 1` then we know there is a next page - relation_count(set_limit(sliced_nodes, limit_value + 1)) == limit_value + 1 + sliced_nodes.limit(1).offset(limit_value).exists? # rubocop: disable CodeReuse/ActiveRecord end else false @@ -80,20 +73,15 @@ module Gitlab # rubocop: enable Naming/PredicateName def cursor_for(node) - encoded_json_from_ordering(node) + order = Gitlab::Pagination::Keyset::Order.extract_keyset_order_object(items) + encode(order.cursor_attributes_for_node(node).to_json) end def sliced_nodes - @sliced_nodes ||= - begin - OrderInfo.validate_ordering(ordered_items, order_list) unless loaded?(ordered_items) - - sliced = ordered_items - sliced = slice_nodes(sliced, before, :before) if before.present? - sliced = slice_nodes(sliced, after, :after) if after.present? - - sliced - end + sliced = ordered_items + sliced = slice_nodes(sliced, before, :before) if before.present? + sliced = slice_nodes(sliced, after, :after) if after.present? + sliced end def nodes @@ -104,6 +92,20 @@ module Gitlab @nodes ||= limited_nodes.to_a end + def items + original_items = super + return original_items if Gitlab::Pagination::Keyset::Order.keyset_aware?(original_items) + + strong_memoize(:keyset_pagination_items) do + rebuilt_items_with_keyset_order, success = + Gitlab::Pagination::Keyset::SimpleOrderBuilder.build(original_items) + + raise(Gitlab::Pagination::Keyset::UnsupportedScopeOrder) unless success + + rebuilt_items_with_keyset_order + end + end + private # Apply `first` and `last` to `sliced_nodes` @@ -129,11 +131,11 @@ module Gitlab # rubocop: disable CodeReuse/ActiveRecord def slice_nodes(sliced, encoded_cursor, before_or_after) - decoded_cursor = ordering_from_encoded_json(encoded_cursor) - builder = QueryBuilder.new(arel_table, order_list, decoded_cursor, before_or_after) - ordering = builder.conditions + order = Gitlab::Pagination::Keyset::Order.extract_keyset_order_object(sliced) + order = order.reversed_order if before_or_after == :before - sliced.where(*ordering).where.not(id: decoded_cursor['id']) + decoded_cursor = ordering_from_encoded_json(encoded_cursor) + order.apply_cursor_conditions(sliced, decoded_cursor) end # rubocop: enable CodeReuse/ActiveRecord @@ -157,57 +159,10 @@ module Gitlab raise ArgumentError, 'Relation must have a primary key' end - list = OrderInfo.build_order_list(items) - - if loaded?(items) && !before.present? && !after.present? - @order_list = list.presence || [OrderInfo.new(items.primary_key)] - - # already sorted, or trivially sorted - next items if list.present? || items.size <= 1 - - pkey = items.primary_key.to_sym - next items.sort_by { |item| item[pkey] }.reverse - end - - # ensure there is a primary key ordering - if list&.last&.attribute_name != items.primary_key - items.order(arel_table[items.primary_key].desc) # rubocop: disable CodeReuse/ActiveRecord - else - items - end + items end end - def order_list - strong_memoize(:order_list) do - OrderInfo.build_order_list(ordered_items) - end - end - - def arel_table - items.arel_table - end - - # Storing the current order values in the cursor allows us to - # make an intelligent decision on handling NULL values. - # Otherwise we would either need to fetch the record first, - # or fetch it in the SQL, significantly complicating it. - def encoded_json_from_ordering(node) - ordering = { 'id' => node[:id].to_s } - - order_list.each do |field| - field_name = field.try(:attribute_name) || field - field_value = node[field_name] - ordering[field_name] = if field_value.is_a?(Time) - field_value.to_s(:inspect) - else - field_value.to_s - end - end - - encode(ordering.to_json) - end - def ordering_from_encoded_json(cursor) Gitlab::Json.parse(decode(cursor)) rescue JSON::ParserError diff --git a/lib/gitlab/graphql/pagination/keyset/generic_keyset_pagination.rb b/lib/gitlab/graphql/pagination/keyset/generic_keyset_pagination.rb deleted file mode 100644 index 9beb40ddd7e..00000000000 --- a/lib/gitlab/graphql/pagination/keyset/generic_keyset_pagination.rb +++ /dev/null @@ -1,98 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Graphql - module Pagination - module Keyset - # https://gitlab.com/gitlab-org/gitlab/-/issues/334973 - # Use the generic keyset implementation if the given ActiveRecord scope supports it. - # Note: this module is temporary, at some point it will be merged with Keyset::Connection - module GenericKeysetPagination - extend ActiveSupport::Concern - - # rubocop: disable Naming/PredicateName - # rubocop: disable CodeReuse/ActiveRecord - def has_next_page - return super unless Gitlab::Pagination::Keyset::Order.keyset_aware?(items) - - strong_memoize(:generic_keyset_pagination_has_next_page) do - if before - true - elsif first - case sliced_nodes - when Array - sliced_nodes.size > limit_value - else - sliced_nodes.limit(1).offset(limit_value).exists? - end - else - false - end - end - end - - # rubocop: enable CodeReuse/ActiveRecord - def ordered_items - raise ArgumentError, 'Relation must have a primary key' unless items.primary_key.present? - - return super unless Gitlab::Pagination::Keyset::Order.keyset_aware?(items) - - items - end - - def cursor_for(node) - return super unless Gitlab::Pagination::Keyset::Order.keyset_aware?(items) - - order = Gitlab::Pagination::Keyset::Order.extract_keyset_order_object(items) - encode(order.cursor_attributes_for_node(node).to_json) - end - - def slice_nodes(sliced, encoded_cursor, before_or_after) - return super unless Gitlab::Pagination::Keyset::Order.keyset_aware?(sliced) - - order = Gitlab::Pagination::Keyset::Order.extract_keyset_order_object(sliced) - order = order.reversed_order if before_or_after == :before - - decoded_cursor = ordering_from_encoded_json(encoded_cursor) - order.apply_cursor_conditions(sliced, decoded_cursor) - end - - def sliced_nodes - return super unless Gitlab::Pagination::Keyset::Order.keyset_aware?(items) - - sliced = ordered_items - sliced = slice_nodes(sliced, before, :before) if before.present? - sliced = slice_nodes(sliced, after, :after) if after.present? - sliced - end - - def items - original_items = super - return original_items if Feature.disabled?(:new_graphql_keyset_pagination) || Gitlab::Pagination::Keyset::Order.keyset_aware?(original_items) - - strong_memoize(:generic_keyset_pagination_items) do - rebuilt_items_with_keyset_order, success = Gitlab::Pagination::Keyset::SimpleOrderBuilder.build(original_items) - - if success - rebuilt_items_with_keyset_order - else - if original_items.is_a?(ActiveRecord::Relation) - old_keyset_pagination_usage.increment({ model: original_items.model.to_s }) - end - - original_items - end - end - end - - def old_keyset_pagination_usage - @old_keyset_pagination_usage ||= Gitlab::Metrics.counter( - :old_keyset_pagination_usage, - 'The number of times the old keyset pagination code was used' - ) - end - end - end - end - end -end diff --git a/lib/gitlab/graphql/pagination/keyset/order_info.rb b/lib/gitlab/graphql/pagination/keyset/order_info.rb deleted file mode 100644 index 57e85ebe7f6..00000000000 --- a/lib/gitlab/graphql/pagination/keyset/order_info.rb +++ /dev/null @@ -1,124 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Graphql - module Pagination - module Keyset - class OrderInfo - attr_reader :attribute_name, :sort_direction, :named_function - - def initialize(order_value) - @attribute_name, @sort_direction, @named_function = - if order_value.is_a?(String) - extract_nulls_last_order(order_value) - else - extract_attribute_values(order_value) - end - end - - def operator_for(before_or_after) - case before_or_after - when :before - sort_direction == :asc ? '<' : '>' - when :after - sort_direction == :asc ? '>' : '<' - end - end - - # Only allow specific node types - def self.build_order_list(relation) - order_list = relation.order_values.select do |value| - supported_order_value?(value) - end - - order_list.map { |info| OrderInfo.new(info) } - end - - def self.validate_ordering(relation, order_list) - if order_list.empty? - raise ArgumentError, 'A minimum of 1 ordering field is required' - end - - if order_list.count > 2 - # Keep in mind an order clause for primary key is added if one is not present - # lib/gitlab/graphql/pagination/keyset/connection.rb:97 - raise ArgumentError, 'A maximum of 2 ordering fields are allowed' - end - - # make sure the last ordering field is non-nullable - attribute_name = order_list.last&.attribute_name - - if relation.columns_hash[attribute_name].null - raise ArgumentError, "Column `#{attribute_name}` must not allow NULL" - end - - if order_list.last.attribute_name != relation.primary_key - raise ArgumentError, "Last ordering field must be the primary key, `#{relation.primary_key}`" - end - end - - def self.supported_order_value?(order_value) - return true if order_value.is_a?(Arel::Nodes::Ascending) || order_value.is_a?(Arel::Nodes::Descending) - return false unless order_value.is_a?(String) - - tokens = order_value.downcase.split - - tokens.last(2) == %w(nulls last) && tokens.count == 4 - end - - private - - def extract_nulls_last_order(order_value) - tokens = order_value.downcase.split - - column_reference = tokens.first - sort_direction = tokens[1] == 'asc' ? :asc : :desc - - # Handles the case when the order value is coming from another table. - # Example: table_name.column_name - # Query the value using the fully qualified column name: pass table_name.column_name as the named_function - if fully_qualified_column_reference?(column_reference) - [column_reference, sort_direction, Arel.sql(column_reference)] - else - [column_reference, sort_direction, nil] - end - end - - # Example: table_name.column_name - def fully_qualified_column_reference?(attribute) - attribute.to_s.count('.') == 1 - end - - def extract_attribute_values(order_value) - if ordering_by_lower?(order_value) - [order_value.expr.expressions[0].name.to_s, order_value.direction, order_value.expr] - elsif ordering_by_case?(order_value) - ['case_order_value', order_value.direction, order_value.expr] - elsif ordering_by_array_position?(order_value) - ['array_position', order_value.direction, order_value.expr] - else - [order_value.expr.name, order_value.direction, nil] - end - end - - # determine if ordering using LOWER, eg. "ORDER BY LOWER(boards.name)" - def ordering_by_lower?(order_value) - order_value.expr.is_a?(Arel::Nodes::NamedFunction) && order_value.expr&.name&.downcase == 'lower' - end - - # determine if ordering using ARRAY_POSITION, eg. "ORDER BY ARRAY_POSITION(Array[4,3,1,2]::smallint, state)" - def ordering_by_array_position?(order_value) - order_value.expr.is_a?(Arel::Nodes::NamedFunction) && order_value.expr&.name&.downcase == 'array_position' - end - - # determine if ordering using CASE - def ordering_by_case?(order_value) - order_value.expr.is_a?(Arel::Nodes::Case) - end - end - end - end - end -end - -Gitlab::Graphql::Pagination::Keyset::OrderInfo.prepend_mod_with('Gitlab::Graphql::Pagination::Keyset::OrderInfo') diff --git a/lib/gitlab/graphql/pagination/keyset/query_builder.rb b/lib/gitlab/graphql/pagination/keyset/query_builder.rb deleted file mode 100644 index a2f53ae83dd..00000000000 --- a/lib/gitlab/graphql/pagination/keyset/query_builder.rb +++ /dev/null @@ -1,73 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Graphql - module Pagination - module Keyset - class QueryBuilder - def initialize(arel_table, order_list, decoded_cursor, before_or_after) - @arel_table = arel_table - @order_list = order_list - @decoded_cursor = decoded_cursor - @before_or_after = before_or_after - - if order_list.empty? - raise ArgumentError, 'No ordering scopes have been supplied' - end - end - - # Based on whether the main field we're ordering on is NULL in the - # cursor, we can more easily target our query condition. - # We assume that the last ordering field is unique, meaning - # it will not contain NULLs. - # We currently only support two ordering fields. - # - # Example of the conditions for - # relation: Issue.order(relative_position: :asc).order(id: :asc) - # after cursor: relative_position: 1500, id: 500 - # - # when cursor[relative_position] is not NULL - # - # ("issues"."relative_position" > 1500) - # OR ( - # "issues"."relative_position" = 1500 - # AND - # "issues"."id" > 500 - # ) - # OR ("issues"."relative_position" IS NULL) - # - # when cursor[relative_position] is NULL - # - # "issues"."relative_position" IS NULL - # AND - # "issues"."id" > 500 - # - def conditions - attr_values = order_list.map do |field| - name = field.try(:attribute_name) || field - decoded_cursor[name] - end - - if order_list.count == 1 && attr_values.first.nil? - raise Gitlab::Graphql::Errors::ArgumentError, 'Before/after cursor invalid: `nil` was provided as only sortable value' - end - - if order_list.count == 1 || attr_values.first.present? - Keyset::Conditions::NotNullCondition.new(arel_table, order_list, attr_values, operators, before_or_after).build - else - Keyset::Conditions::NullCondition.new(arel_table, order_list, attr_values, operators, before_or_after).build - end - end - - private - - attr_reader :arel_table, :order_list, :decoded_cursor, :before_or_after - - def operators - order_list.map { |field| field.operator_for(before_or_after) } - end - end - end - end - end -end diff --git a/spec/lib/gitlab/graphql/pagination/keyset/conditions/not_null_condition_spec.rb b/spec/lib/gitlab/graphql/pagination/keyset/conditions/not_null_condition_spec.rb deleted file mode 100644 index eecdaa3409f..00000000000 --- a/spec/lib/gitlab/graphql/pagination/keyset/conditions/not_null_condition_spec.rb +++ /dev/null @@ -1,115 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Graphql::Pagination::Keyset::Conditions::NotNullCondition do - describe '#build' do - let(:operators) { ['>', '>'] } - let(:before_or_after) { :after } - let(:condition) { described_class.new(arel_table, order_list, values, operators, before_or_after) } - - context 'when there is only one ordering field' do - let(:arel_table) { Issue.arel_table } - let(:order_list) { [double(named_function: nil, attribute_name: 'id')] } - let(:values) { [500] } - let(:operators) { ['>'] } - - it 'generates a single condition sql' do - expected_sql = <<~SQL - ("issues"."id" > 500) - SQL - - expect(condition.build.squish).to eq expected_sql.squish - end - end - - context 'when ordering by a column attribute' do - let(:arel_table) { Issue.arel_table } - let(:order_list) { [double(named_function: nil, attribute_name: 'relative_position'), double(named_function: nil, attribute_name: 'id')] } - let(:values) { [1500, 500] } - - shared_examples ':after condition' do - it 'generates :after sql' do - expected_sql = <<~SQL - ("issues"."relative_position" > 1500) - OR ( - "issues"."relative_position" = 1500 - AND - "issues"."id" > 500 - ) - OR ("issues"."relative_position" IS NULL) - SQL - - expect(condition.build.squish).to eq expected_sql.squish - end - end - - context 'when :after' do - it_behaves_like ':after condition' - end - - context 'when :before' do - let(:before_or_after) { :before } - - it 'generates :before sql' do - expected_sql = <<~SQL - ("issues"."relative_position" > 1500) - OR ( - "issues"."relative_position" = 1500 - AND - "issues"."id" > 500 - ) - SQL - - expect(condition.build.squish).to eq expected_sql.squish - end - end - - context 'when :foo' do - let(:before_or_after) { :foo } - - it_behaves_like ':after condition' - end - end - - context 'when ordering by LOWER' do - let(:arel_table) { Project.arel_table } - let(:relation) { Project.order(arel_table['name'].lower.asc).order(:id) } - let(:order_list) { Gitlab::Graphql::Pagination::Keyset::OrderInfo.build_order_list(relation) } - let(:values) { ['Test', 500] } - - context 'when :after' do - it 'generates :after sql' do - expected_sql = <<~SQL - (LOWER("projects"."name") > 'test') - OR ( - LOWER("projects"."name") = 'test' - AND - "projects"."id" > 500 - ) - OR (LOWER("projects"."name") IS NULL) - SQL - - expect(condition.build.squish).to eq expected_sql.squish - end - end - - context 'when :before' do - let(:before_or_after) { :before } - - it 'generates :before sql' do - expected_sql = <<~SQL - (LOWER("projects"."name") > 'test') - OR ( - LOWER("projects"."name") = 'test' - AND - "projects"."id" > 500 - ) - SQL - - expect(condition.build.squish).to eq expected_sql.squish - end - end - end - end -end diff --git a/spec/lib/gitlab/graphql/pagination/keyset/conditions/null_condition_spec.rb b/spec/lib/gitlab/graphql/pagination/keyset/conditions/null_condition_spec.rb deleted file mode 100644 index 582f96299ec..00000000000 --- a/spec/lib/gitlab/graphql/pagination/keyset/conditions/null_condition_spec.rb +++ /dev/null @@ -1,95 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Graphql::Pagination::Keyset::Conditions::NullCondition do - describe '#build' do - let(:values) { [nil, 500] } - let(:operators) { [nil, '>'] } - let(:before_or_after) { :after } - let(:condition) { described_class.new(arel_table, order_list, values, operators, before_or_after) } - - context 'when ordering by a column attribute' do - let(:arel_table) { Issue.arel_table } - let(:order_list) { [double(named_function: nil, attribute_name: 'relative_position'), double(named_function: nil, attribute_name: 'id')] } - - shared_examples ':after condition' do - it 'generates sql' do - expected_sql = <<~SQL - ( - "issues"."relative_position" IS NULL - AND - "issues"."id" > 500 - ) - SQL - - expect(condition.build.squish).to eq expected_sql.squish - end - end - - context 'when :after' do - it_behaves_like ':after condition' - end - - context 'when :before' do - let(:before_or_after) { :before } - - it 'generates :before sql' do - expected_sql = <<~SQL - ( - "issues"."relative_position" IS NULL - AND - "issues"."id" > 500 - ) - OR ("issues"."relative_position" IS NOT NULL) - SQL - - expect(condition.build.squish).to eq expected_sql.squish - end - end - - context 'when :foo' do - let(:before_or_after) { :foo } - - it_behaves_like ':after condition' - end - end - - context 'when ordering by LOWER' do - let(:arel_table) { Project.arel_table } - let(:relation) { Project.order(arel_table['name'].lower.asc).order(:id) } - let(:order_list) { Gitlab::Graphql::Pagination::Keyset::OrderInfo.build_order_list(relation) } - - context 'when :after' do - it 'generates sql' do - expected_sql = <<~SQL - ( - LOWER("projects"."name") IS NULL - AND - "projects"."id" > 500 - ) - SQL - - expect(condition.build.squish).to eq expected_sql.squish - end - end - - context 'when :before' do - let(:before_or_after) { :before } - - it 'generates :before sql' do - expected_sql = <<~SQL - ( - LOWER("projects"."name") IS NULL - AND - "projects"."id" > 500 - ) - OR (LOWER("projects"."name") IS NOT NULL) - SQL - - expect(condition.build.squish).to eq expected_sql.squish - end - end - end - end -end diff --git a/spec/lib/gitlab/graphql/pagination/keyset/connection_generic_keyset_spec.rb b/spec/lib/gitlab/graphql/pagination/keyset/connection_generic_keyset_spec.rb deleted file mode 100644 index 8a2b5ae0d38..00000000000 --- a/spec/lib/gitlab/graphql/pagination/keyset/connection_generic_keyset_spec.rb +++ /dev/null @@ -1,415 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Graphql::Pagination::Keyset::Connection do - include GraphqlHelpers - - # https://gitlab.com/gitlab-org/gitlab/-/issues/334973 - # The spec will be merged with connection_spec.rb in the future. - let(:nodes) { Project.all.order(id: :asc) } - let(:arguments) { {} } - let(:context) { GraphQL::Query::Context.new(query: query_double, values: nil, object: nil) } - - let_it_be(:column_order_id) { Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(attribute_name: 'id', order_expression: Project.arel_table[:id].asc) } - let_it_be(:column_order_id_desc) { Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(attribute_name: 'id', order_expression: Project.arel_table[:id].desc) } - let_it_be(:column_order_updated_at) { Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(attribute_name: 'updated_at', order_expression: Project.arel_table[:updated_at].asc) } - let_it_be(:column_order_created_at) { Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(attribute_name: 'created_at', order_expression: Project.arel_table[:created_at].asc) } - let_it_be(:column_order_last_repo) do - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'last_repository_check_at', - column_expression: Project.arel_table[:last_repository_check_at], - order_expression: Project.arel_table[:last_repository_check_at].asc.nulls_last, - reversed_order_expression: Project.arel_table[:last_repository_check_at].desc.nulls_last, - order_direction: :asc, - nullable: :nulls_last, - distinct: false) - end - - let_it_be(:column_order_last_repo_desc) do - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'last_repository_check_at', - column_expression: Project.arel_table[:last_repository_check_at], - order_expression: Project.arel_table[:last_repository_check_at].desc.nulls_last, - reversed_order_expression: Project.arel_table[:last_repository_check_at].asc.nulls_last, - order_direction: :desc, - nullable: :nulls_last, - distinct: false) - end - - subject(:connection) do - described_class.new(nodes, **{ context: context, max_page_size: 3 }.merge(arguments)) - end - - def encoded_cursor(node) - described_class.new(nodes, context: context).cursor_for(node) - end - - def decoded_cursor(cursor) - Gitlab::Json.parse(Base64Bp.urlsafe_decode64(cursor)) - end - - describe "With generic keyset order support" do - let(:nodes) { Project.all.order(Gitlab::Pagination::Keyset::Order.build([column_order_id])) } - - it_behaves_like 'a connection with collection methods' - - it_behaves_like 'a redactable connection' do - let_it_be(:projects) { create_list(:project, 2) } - let(:unwanted) { projects.second } - end - - describe '#cursor_for' do - let(:project) { create(:project) } - let(:cursor) { connection.cursor_for(project) } - - it 'returns an encoded ID' do - expect(decoded_cursor(cursor)).to eq('id' => project.id.to_s) - end - - context 'when an order is specified' do - let(:nodes) { Project.all.order(Gitlab::Pagination::Keyset::Order.build([column_order_id])) } - - it 'returns the encoded value of the order' do - expect(decoded_cursor(cursor)).to include('id' => project.id.to_s) - end - end - - context 'when multiple orders are specified' do - let(:nodes) { Project.all.order(Gitlab::Pagination::Keyset::Order.build([column_order_updated_at, column_order_created_at, column_order_id])) } - - it 'returns the encoded value of the order' do - expect(decoded_cursor(cursor)).to include('updated_at' => project.updated_at.to_s(:inspect)) - end - end - end - - describe '#sliced_nodes' do - let(:projects) { create_list(:project, 4) } - - context 'when before is passed' do - let(:arguments) { { before: encoded_cursor(projects[1]) } } - - it 'only returns the project before the selected one' do - expect(subject.sliced_nodes).to contain_exactly(projects.first) - end - - context 'when the sort order is descending' do - let(:nodes) { Project.all.order(Gitlab::Pagination::Keyset::Order.build([column_order_id_desc])) } - - it 'returns the correct nodes' do - expect(subject.sliced_nodes).to contain_exactly(*projects[2..]) - end - end - end - - context 'when after is passed' do - let(:arguments) { { after: encoded_cursor(projects[1]) } } - - it 'only returns the project before the selected one' do - expect(subject.sliced_nodes).to contain_exactly(*projects[2..]) - end - - context 'when the sort order is descending' do - let(:nodes) { Project.all.order(Gitlab::Pagination::Keyset::Order.build([column_order_id_desc])) } - - it 'returns the correct nodes' do - expect(subject.sliced_nodes).to contain_exactly(projects.first) - end - end - end - - context 'when both before and after are passed' do - let(:arguments) do - { - after: encoded_cursor(projects[1]), - before: encoded_cursor(projects[3]) - } - end - - it 'returns the expected set' do - expect(subject.sliced_nodes).to contain_exactly(projects[2]) - end - end - - shared_examples 'nodes are in ascending order' do - context 'when no cursor is passed' do - let(:arguments) { {} } - - it 'returns projects in ascending order' do - expect(subject.sliced_nodes).to eq(ascending_nodes) - end - end - - context 'when before cursor value is not NULL' do - let(:arguments) { { before: encoded_cursor(ascending_nodes[2]) } } - - it 'returns all projects before the cursor' do - expect(subject.sliced_nodes).to eq(ascending_nodes.first(2)) - end - end - - context 'when after cursor value is not NULL' do - let(:arguments) { { after: encoded_cursor(ascending_nodes[1]) } } - - it 'returns all projects after the cursor' do - expect(subject.sliced_nodes).to eq(ascending_nodes.last(3)) - end - end - - context 'when before and after cursor' do - let(:arguments) { { before: encoded_cursor(ascending_nodes.last), after: encoded_cursor(ascending_nodes.first) } } - - it 'returns all projects after the cursor' do - expect(subject.sliced_nodes).to eq(ascending_nodes[1..3]) - end - end - end - - shared_examples 'nodes are in descending order' do - context 'when no cursor is passed' do - let(:arguments) { {} } - - it 'only returns projects in descending order' do - expect(subject.sliced_nodes).to eq(descending_nodes) - end - end - - context 'when before cursor value is not NULL' do - let(:arguments) { { before: encoded_cursor(descending_nodes[2]) } } - - it 'returns all projects before the cursor' do - expect(subject.sliced_nodes).to eq(descending_nodes.first(2)) - end - end - - context 'when after cursor value is not NULL' do - let(:arguments) { { after: encoded_cursor(descending_nodes[1]) } } - - it 'returns all projects after the cursor' do - expect(subject.sliced_nodes).to eq(descending_nodes.last(3)) - end - end - - context 'when before and after cursor' do - let(:arguments) { { before: encoded_cursor(descending_nodes.last), after: encoded_cursor(descending_nodes.first) } } - - it 'returns all projects after the cursor' do - expect(subject.sliced_nodes).to eq(descending_nodes[1..3]) - end - end - end - - context 'when multiple orders with nil values are defined' do - let_it_be(:project1) { create(:project, last_repository_check_at: 10.days.ago) } # Asc: project5 Desc: project3 - let_it_be(:project2) { create(:project, last_repository_check_at: nil) } # Asc: project1 Desc: project1 - let_it_be(:project3) { create(:project, last_repository_check_at: 5.days.ago) } # Asc: project3 Desc: project5 - let_it_be(:project4) { create(:project, last_repository_check_at: nil) } # Asc: project2 Desc: project2 - let_it_be(:project5) { create(:project, last_repository_check_at: 20.days.ago) } # Asc: project4 Desc: project4 - - context 'when ascending' do - let_it_be(:order) { Gitlab::Pagination::Keyset::Order.build([column_order_last_repo, column_order_id]) } - let_it_be(:nodes) { Project.order(order) } - let_it_be(:ascending_nodes) { [project5, project1, project3, project2, project4] } - - it_behaves_like 'nodes are in ascending order' - - context 'when before cursor value is NULL' do - let(:arguments) { { before: encoded_cursor(project4) } } - - it 'returns all projects before the cursor' do - expect(subject.sliced_nodes).to eq([project5, project1, project3, project2]) - end - end - - context 'when after cursor value is NULL' do - let(:arguments) { { after: encoded_cursor(project2) } } - - it 'returns all projects after the cursor' do - expect(subject.sliced_nodes).to eq([project4]) - end - end - end - - context 'when descending' do - let_it_be(:order) { Gitlab::Pagination::Keyset::Order.build([column_order_last_repo_desc, column_order_id]) } - let_it_be(:nodes) { Project.order(order) } - let_it_be(:descending_nodes) { [project3, project1, project5, project2, project4] } - - it_behaves_like 'nodes are in descending order' - - context 'when before cursor value is NULL' do - let(:arguments) { { before: encoded_cursor(project4) } } - - it 'returns all projects before the cursor' do - expect(subject.sliced_nodes).to eq([project3, project1, project5, project2]) - end - end - - context 'when after cursor value is NULL' do - let(:arguments) { { after: encoded_cursor(project2) } } - - it 'returns all projects after the cursor' do - expect(subject.sliced_nodes).to eq([project4]) - end - end - end - end - - context 'when ordering by similarity' do - let_it_be(:project1) { create(:project, name: 'test') } - let_it_be(:project2) { create(:project, name: 'testing') } - let_it_be(:project3) { create(:project, name: 'tests') } - let_it_be(:project4) { create(:project, name: 'testing stuff') } - let_it_be(:project5) { create(:project, name: 'test') } - - let_it_be(:nodes) do - # Note: sorted_by_similarity_desc scope internally supports the generic keyset order. - Project.sorted_by_similarity_desc('test', include_in_select: true) - end - - let_it_be(:descending_nodes) { nodes.to_a } - - it_behaves_like 'nodes are in descending order' - end - - context 'when an invalid cursor is provided' do - let(:arguments) { { before: Base64Bp.urlsafe_encode64('invalidcursor', padding: false) } } - - it 'raises an error' do - expect { subject.sliced_nodes }.to raise_error(Gitlab::Graphql::Errors::ArgumentError) - end - end - end - - describe '#nodes' do - let_it_be(:all_nodes) { create_list(:project, 5) } - - let(:paged_nodes) { subject.nodes } - - it_behaves_like 'connection with paged nodes' do - let(:paged_nodes_size) { 3 } - end - - context 'when both are passed' do - let(:arguments) { { first: 2, last: 2 } } - - it 'raises an error' do - expect { paged_nodes }.to raise_error(Gitlab::Graphql::Errors::ArgumentError) - end - end - - context 'when primary key is not in original order' do - let(:nodes) { Project.order(last_repository_check_at: :desc) } - - it 'is added to end' do - sliced = subject.sliced_nodes - - order_sql = sliced.order_values.last.to_sql - - expect(order_sql).to end_with(Project.arel_table[:id].desc.to_sql) - end - end - - context 'when there is no primary key' do - before do - stub_const('NoPrimaryKey', Class.new(ActiveRecord::Base)) - NoPrimaryKey.class_eval do - self.table_name = 'no_primary_key' - self.primary_key = nil - end - end - - let(:nodes) { NoPrimaryKey.all } - - it 'raises an error' do - expect(NoPrimaryKey.primary_key).to be_nil - expect { subject.sliced_nodes }.to raise_error(ArgumentError, 'Relation must have a primary key') - end - end - end - - describe '#has_previous_page and #has_next_page' do - # using a list of 5 items with a max_page of 3 - let_it_be(:project_list) { create_list(:project, 5) } - let_it_be(:nodes) { Project.order(Gitlab::Pagination::Keyset::Order.build([column_order_id])) } - - context 'when default query' do - let(:arguments) { {} } - - it 'has no previous, but a next' do - expect(subject.has_previous_page).to be_falsey - expect(subject.has_next_page).to be_truthy - end - end - - context 'when before is first item' do - let(:arguments) { { before: encoded_cursor(project_list.first) } } - - it 'has no previous, but a next' do - expect(subject.has_previous_page).to be_falsey - expect(subject.has_next_page).to be_truthy - end - end - - describe 'using `before`' do - context 'when before is the last item' do - let(:arguments) { { before: encoded_cursor(project_list.last) } } - - it 'has no previous, but a next' do - expect(subject.has_previous_page).to be_falsey - expect(subject.has_next_page).to be_truthy - end - end - - context 'when before and last specified' do - let(:arguments) { { before: encoded_cursor(project_list.last), last: 2 } } - - it 'has a previous and a next' do - expect(subject.has_previous_page).to be_truthy - expect(subject.has_next_page).to be_truthy - end - end - - context 'when before and last does request all remaining nodes' do - let(:arguments) { { before: encoded_cursor(project_list[1]), last: 3 } } - - it 'has a previous and a next' do - expect(subject.has_previous_page).to be_falsey - expect(subject.has_next_page).to be_truthy - expect(subject.nodes).to eq [project_list[0]] - end - end - end - - describe 'using `after`' do - context 'when after is the first item' do - let(:arguments) { { after: encoded_cursor(project_list.first) } } - - it 'has a previous, and a next' do - expect(subject.has_previous_page).to be_truthy - expect(subject.has_next_page).to be_truthy - end - end - - context 'when after and first specified' do - let(:arguments) { { after: encoded_cursor(project_list.first), first: 2 } } - - it 'has a previous and a next' do - expect(subject.has_previous_page).to be_truthy - expect(subject.has_next_page).to be_truthy - end - end - - context 'when before and last does request all remaining nodes' do - let(:arguments) { { after: encoded_cursor(project_list[2]), last: 3 } } - - it 'has a previous but no next' do - expect(subject.has_previous_page).to be_truthy - expect(subject.has_next_page).to be_falsey - end - end - end - end - end -end diff --git a/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb b/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb index 6574b3e3131..b54c618d8e0 100644 --- a/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb +++ b/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb @@ -5,10 +5,38 @@ require 'spec_helper' RSpec.describe Gitlab::Graphql::Pagination::Keyset::Connection do include GraphqlHelpers + # https://gitlab.com/gitlab-org/gitlab/-/issues/334973 + # The spec will be merged with connection_spec.rb in the future. let(:nodes) { Project.all.order(id: :asc) } let(:arguments) { {} } let(:context) { GraphQL::Query::Context.new(query: query_double, values: nil, object: nil) } + let_it_be(:column_order_id) { Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(attribute_name: 'id', order_expression: Project.arel_table[:id].asc) } + let_it_be(:column_order_id_desc) { Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(attribute_name: 'id', order_expression: Project.arel_table[:id].desc) } + let_it_be(:column_order_updated_at) { Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(attribute_name: 'updated_at', order_expression: Project.arel_table[:updated_at].asc) } + let_it_be(:column_order_created_at) { Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(attribute_name: 'created_at', order_expression: Project.arel_table[:created_at].asc) } + let_it_be(:column_order_last_repo) do + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'last_repository_check_at', + column_expression: Project.arel_table[:last_repository_check_at], + order_expression: Project.arel_table[:last_repository_check_at].asc.nulls_last, + reversed_order_expression: Project.arel_table[:last_repository_check_at].desc.nulls_last, + order_direction: :asc, + nullable: :nulls_last, + distinct: false) + end + + let_it_be(:column_order_last_repo_desc) do + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'last_repository_check_at', + column_expression: Project.arel_table[:last_repository_check_at], + order_expression: Project.arel_table[:last_repository_check_at].desc.nulls_last, + reversed_order_expression: Project.arel_table[:last_repository_check_at].asc.nulls_last, + order_direction: :desc, + nullable: :nulls_last, + distinct: false) + end + subject(:connection) do described_class.new(nodes, **{ context: context, max_page_size: 3 }.merge(arguments)) end @@ -21,414 +49,293 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::Connection do Gitlab::Json.parse(Base64Bp.urlsafe_decode64(cursor)) end - # see: https://gitlab.com/gitlab-org/gitlab/-/issues/297358 - context 'the relation has been preloaded' do - let(:projects) { Project.all.preload(:issues) } - let(:nodes) { projects.first.issues } + describe "with generic keyset order support" do + let(:nodes) { Project.all.order(Gitlab::Pagination::Keyset::Order.build([column_order_id])) } - before do - project = create(:project) - create_list(:issue, 3, project: project) + it_behaves_like 'a connection with collection methods' + + it_behaves_like 'a redactable connection' do + let_it_be(:projects) { create_list(:project, 2) } + let(:unwanted) { projects.second } end - it 'is loaded' do - expect(nodes).to be_loaded - end + describe '#cursor_for' do + let(:project) { create(:project) } + let(:cursor) { connection.cursor_for(project) } - it 'does not error when accessing pagination information' do - connection.first = 2 - - expect(connection).to have_attributes( - has_previous_page: false, - has_next_page: true - ) - end - - it 'can generate cursors' do - connection.send(:ordered_items) # necessary to generate the order-list - - expect(connection.cursor_for(nodes.first)).to be_a(String) - end - - it 'can read the next page' do - connection.send(:ordered_items) # necessary to generate the order-list - ordered = nodes.reorder(id: :desc) - next_page = described_class.new(nodes, - context: context, - max_page_size: 3, - after: connection.cursor_for(ordered.second)) - - expect(next_page.sliced_nodes).to contain_exactly(ordered.third) - end - end - - it_behaves_like 'a connection with collection methods' - - it_behaves_like 'a redactable connection' do - let_it_be(:projects) { create_list(:project, 2) } - let(:unwanted) { projects.second } - end - - describe '#cursor_for' do - let(:project) { create(:project) } - let(:cursor) { connection.cursor_for(project) } - - it 'returns an encoded ID' do - expect(decoded_cursor(cursor)).to eq('id' => project.id.to_s) - end - - context 'when SimpleOrderBuilder cannot build keyset paginated query' do - it 'increments the `old_keyset_pagination_usage` counter', :prometheus do - expect(Gitlab::Pagination::Keyset::SimpleOrderBuilder).to receive(:build).and_return([false, nil]) - - decoded_cursor(cursor) - - counter = Gitlab::Metrics.registry.get(:old_keyset_pagination_usage) - expect(counter.get(model: 'Project')).to eq(1) - end - end - - context 'when an order is specified' do - let(:nodes) { Project.order(:updated_at) } - - it 'returns the encoded value of the order' do - expect(decoded_cursor(cursor)).to include('updated_at' => project.updated_at.to_s(:inspect)) + it 'returns an encoded ID' do + expect(decoded_cursor(cursor)).to eq('id' => project.id.to_s) end - it 'includes the :id even when not specified in the order' do - expect(decoded_cursor(cursor)).to include('id' => project.id.to_s) - end - end + context 'when an order is specified' do + let(:nodes) { Project.all.order(Gitlab::Pagination::Keyset::Order.build([column_order_id])) } - context 'when multiple orders are specified' do - let(:nodes) { Project.order(:updated_at).order(:created_at) } - - it 'returns the encoded value of the order' do - expect(decoded_cursor(cursor)).to include('updated_at' => project.updated_at.to_s(:inspect)) - end - end - - context 'when multiple orders with SQL are specified' do - let(:nodes) { Project.order(Arel.sql('projects.updated_at IS NULL')).order(:updated_at).order(:id) } - - it 'returns the encoded value of the order' do - expect(decoded_cursor(cursor)).to include('updated_at' => project.updated_at.to_s(:inspect)) - end - end - end - - describe '#sliced_nodes' do - let(:projects) { create_list(:project, 4) } - - context 'when before is passed' do - let(:arguments) { { before: encoded_cursor(projects[1]) } } - - it 'only returns the project before the selected one' do - expect(subject.sliced_nodes).to contain_exactly(projects.first) + it 'returns the encoded value of the order' do + expect(decoded_cursor(cursor)).to include('id' => project.id.to_s) + end end - context 'when the sort order is descending' do - let(:nodes) { Project.all.order(id: :desc) } + context 'when multiple orders are specified' do + let(:nodes) { Project.all.order(Gitlab::Pagination::Keyset::Order.build([column_order_updated_at, column_order_created_at, column_order_id])) } - it 'returns the correct nodes' do - expect(subject.sliced_nodes).to contain_exactly(*projects[2..]) + it 'returns the encoded value of the order' do + expect(decoded_cursor(cursor)).to include('updated_at' => project.updated_at.to_s(:inspect)) end end end - context 'when after is passed' do - let(:arguments) { { after: encoded_cursor(projects[1]) } } + describe '#sliced_nodes' do + let(:projects) { create_list(:project, 4) } - it 'only returns the project before the selected one' do - expect(subject.sliced_nodes).to contain_exactly(*projects[2..]) - end + context 'when before is passed' do + let(:arguments) { { before: encoded_cursor(projects[1]) } } - context 'when the sort order is descending' do - let(:nodes) { Project.all.order(id: :desc) } - - it 'returns the correct nodes' do + it 'only returns the project before the selected one' do expect(subject.sliced_nodes).to contain_exactly(projects.first) end - end - end - context 'when both before and after are passed' do - let(:arguments) do - { - after: encoded_cursor(projects[1]), - before: encoded_cursor(projects[3]) - } - end + context 'when the sort order is descending' do + let(:nodes) { Project.all.order(Gitlab::Pagination::Keyset::Order.build([column_order_id_desc])) } - it 'returns the expected set' do - expect(subject.sliced_nodes).to contain_exactly(projects[2]) - end - end - - shared_examples 'nodes are in ascending order' do - context 'when no cursor is passed' do - let(:arguments) { {} } - - it 'returns projects in ascending order' do - expect(subject.sliced_nodes).to eq(ascending_nodes) + it 'returns the correct nodes' do + expect(subject.sliced_nodes).to contain_exactly(*projects[2..]) + end end end - context 'when before cursor value is not NULL' do - let(:arguments) { { before: encoded_cursor(ascending_nodes[2]) } } + context 'when after is passed' do + let(:arguments) { { after: encoded_cursor(projects[1]) } } - it 'returns all projects before the cursor' do - expect(subject.sliced_nodes).to eq(ascending_nodes.first(2)) + it 'only returns the project before the selected one' do + expect(subject.sliced_nodes).to contain_exactly(*projects[2..]) + end + + context 'when the sort order is descending' do + let(:nodes) { Project.all.order(Gitlab::Pagination::Keyset::Order.build([column_order_id_desc])) } + + it 'returns the correct nodes' do + expect(subject.sliced_nodes).to contain_exactly(projects.first) + end end end - context 'when after cursor value is not NULL' do - let(:arguments) { { after: encoded_cursor(ascending_nodes[1]) } } + context 'when both before and after are passed' do + let(:arguments) do + { + after: encoded_cursor(projects[1]), + before: encoded_cursor(projects[3]) + } + end - it 'returns all projects after the cursor' do - expect(subject.sliced_nodes).to eq(ascending_nodes.last(3)) + it 'returns the expected set' do + expect(subject.sliced_nodes).to contain_exactly(projects[2]) end end - context 'when before and after cursor' do - let(:arguments) { { before: encoded_cursor(ascending_nodes.last), after: encoded_cursor(ascending_nodes.first) } } + shared_examples 'nodes are in ascending order' do + context 'when no cursor is passed' do + let(:arguments) { {} } - it 'returns all projects after the cursor' do - expect(subject.sliced_nodes).to eq(ascending_nodes[1..3]) + it 'returns projects in ascending order' do + expect(subject.sliced_nodes).to eq(ascending_nodes) + end end - end - end - shared_examples 'nodes are in descending order' do - context 'when no cursor is passed' do - let(:arguments) { {} } + context 'when before cursor value is not NULL' do + let(:arguments) { { before: encoded_cursor(ascending_nodes[2]) } } - it 'only returns projects in descending order' do - expect(subject.sliced_nodes).to eq(descending_nodes) + it 'returns all projects before the cursor' do + expect(subject.sliced_nodes).to eq(ascending_nodes.first(2)) + end + end + + context 'when after cursor value is not NULL' do + let(:arguments) { { after: encoded_cursor(ascending_nodes[1]) } } + + it 'returns all projects after the cursor' do + expect(subject.sliced_nodes).to eq(ascending_nodes.last(3)) + end + end + + context 'when before and after cursor' do + let(:arguments) { { before: encoded_cursor(ascending_nodes.last), after: encoded_cursor(ascending_nodes.first) } } + + it 'returns all projects after the cursor' do + expect(subject.sliced_nodes).to eq(ascending_nodes[1..3]) + end end end - context 'when before cursor value is not NULL' do - let(:arguments) { { before: encoded_cursor(descending_nodes[2]) } } + shared_examples 'nodes are in descending order' do + context 'when no cursor is passed' do + let(:arguments) { {} } - it 'returns all projects before the cursor' do - expect(subject.sliced_nodes).to eq(descending_nodes.first(2)) + it 'only returns projects in descending order' do + expect(subject.sliced_nodes).to eq(descending_nodes) + end + end + + context 'when before cursor value is not NULL' do + let(:arguments) { { before: encoded_cursor(descending_nodes[2]) } } + + it 'returns all projects before the cursor' do + expect(subject.sliced_nodes).to eq(descending_nodes.first(2)) + end + end + + context 'when after cursor value is not NULL' do + let(:arguments) { { after: encoded_cursor(descending_nodes[1]) } } + + it 'returns all projects after the cursor' do + expect(subject.sliced_nodes).to eq(descending_nodes.last(3)) + end + end + + context 'when before and after cursor' do + let(:arguments) { { before: encoded_cursor(descending_nodes.last), after: encoded_cursor(descending_nodes.first) } } + + it 'returns all projects after the cursor' do + expect(subject.sliced_nodes).to eq(descending_nodes[1..3]) + end end end - context 'when after cursor value is not NULL' do - let(:arguments) { { after: encoded_cursor(descending_nodes[1]) } } + context 'when multiple orders with nil values are defined' do + let_it_be(:project1) { create(:project, last_repository_check_at: 10.days.ago) } # Asc: project5 Desc: project3 + let_it_be(:project2) { create(:project, last_repository_check_at: nil) } # Asc: project1 Desc: project1 + let_it_be(:project3) { create(:project, last_repository_check_at: 5.days.ago) } # Asc: project3 Desc: project5 + let_it_be(:project4) { create(:project, last_repository_check_at: nil) } # Asc: project2 Desc: project2 + let_it_be(:project5) { create(:project, last_repository_check_at: 20.days.ago) } # Asc: project4 Desc: project4 - it 'returns all projects after the cursor' do - expect(subject.sliced_nodes).to eq(descending_nodes.last(3)) + context 'when ascending' do + let_it_be(:order) { Gitlab::Pagination::Keyset::Order.build([column_order_last_repo, column_order_id]) } + let_it_be(:nodes) { Project.order(order) } + let_it_be(:ascending_nodes) { [project5, project1, project3, project2, project4] } + + it_behaves_like 'nodes are in ascending order' + + context 'when before cursor value is NULL' do + let(:arguments) { { before: encoded_cursor(project4) } } + + it 'returns all projects before the cursor' do + expect(subject.sliced_nodes).to eq([project5, project1, project3, project2]) + end + end + + context 'when after cursor value is NULL' do + let(:arguments) { { after: encoded_cursor(project2) } } + + it 'returns all projects after the cursor' do + expect(subject.sliced_nodes).to eq([project4]) + end + end + end + + context 'when descending' do + let_it_be(:order) { Gitlab::Pagination::Keyset::Order.build([column_order_last_repo_desc, column_order_id]) } + let_it_be(:nodes) { Project.order(order) } + let_it_be(:descending_nodes) { [project3, project1, project5, project2, project4] } + + it_behaves_like 'nodes are in descending order' + + context 'when before cursor value is NULL' do + let(:arguments) { { before: encoded_cursor(project4) } } + + it 'returns all projects before the cursor' do + expect(subject.sliced_nodes).to eq([project3, project1, project5, project2]) + end + end + + context 'when after cursor value is NULL' do + let(:arguments) { { after: encoded_cursor(project2) } } + + it 'returns all projects after the cursor' do + expect(subject.sliced_nodes).to eq([project4]) + end + end end end - context 'when before and after cursor' do - let(:arguments) { { before: encoded_cursor(descending_nodes.last), after: encoded_cursor(descending_nodes.first) } } + context 'when ordering by similarity' do + let_it_be(:project1) { create(:project, name: 'test') } + let_it_be(:project2) { create(:project, name: 'testing') } + let_it_be(:project3) { create(:project, name: 'tests') } + let_it_be(:project4) { create(:project, name: 'testing stuff') } + let_it_be(:project5) { create(:project, name: 'test') } - it 'returns all projects after the cursor' do - expect(subject.sliced_nodes).to eq(descending_nodes[1..3]) - end - end - end - - context 'when ordering uses LOWER' do - let!(:project1) { create(:project, name: 'A') } # Asc: project1 Desc: project4 - let!(:project2) { create(:project, name: 'c') } # Asc: project5 Desc: project2 - let!(:project3) { create(:project, name: 'b') } # Asc: project3 Desc: project3 - let!(:project4) { create(:project, name: 'd') } # Asc: project2 Desc: project5 - let!(:project5) { create(:project, name: 'a') } # Asc: project4 Desc: project1 - - context 'when ascending' do - let(:nodes) do - Project.order(Arel::Table.new(:projects)['name'].lower.asc).order(id: :asc) + let_it_be(:nodes) do + # Note: sorted_by_similarity_desc scope internally supports the generic keyset order. + Project.sorted_by_similarity_desc('test', include_in_select: true) end - let(:ascending_nodes) { [project1, project5, project3, project2, project4] } - - it_behaves_like 'nodes are in ascending order' - end - - context 'when descending' do - let(:nodes) do - Project.order(Arel::Table.new(:projects)['name'].lower.desc).order(id: :desc) - end - - let(:descending_nodes) { [project4, project2, project3, project5, project1] } + let_it_be(:descending_nodes) { nodes.to_a } it_behaves_like 'nodes are in descending order' end - end - context 'NULLS order' do - using RSpec::Parameterized::TableSyntax + context 'when an invalid cursor is provided' do + let(:arguments) { { before: Base64Bp.urlsafe_encode64('invalidcursor', padding: false) } } - let_it_be(:issue1) { create(:issue, relative_position: nil) } - let_it_be(:issue2) { create(:issue, relative_position: 100) } - let_it_be(:issue3) { create(:issue, relative_position: 200) } - let_it_be(:issue4) { create(:issue, relative_position: nil) } - let_it_be(:issue5) { create(:issue, relative_position: 300) } - - context 'when ascending NULLS LAST (ties broken by id DESC implicitly)' do - let(:ascending_nodes) { [issue2, issue3, issue5, issue4, issue1] } - - where(:nodes) do - [ - lazy { Issue.order(Issue.arel_table[:relative_position].asc.nulls_last) } - ] - end - - with_them do - it_behaves_like 'nodes are in ascending order' - end - end - - context 'when descending NULLS LAST (ties broken by id DESC implicitly)' do - let(:descending_nodes) { [issue5, issue3, issue2, issue4, issue1] } - - where(:nodes) do - [ - lazy { Issue.order(Issue.arel_table[:relative_position].desc.nulls_last) } -] - end - - with_them do - it_behaves_like 'nodes are in descending order' - end - end - - context 'when ascending NULLS FIRST with a tie breaker' do - let(:ascending_nodes) { [issue1, issue4, issue2, issue3, issue5] } - - where(:nodes) do - [ - lazy { Issue.order(Issue.arel_table[:relative_position].asc.nulls_first).order(id: :asc) } -] - end - - with_them do - it_behaves_like 'nodes are in ascending order' - end - end - - context 'when descending NULLS FIRST with a tie breaker' do - let(:descending_nodes) { [issue1, issue4, issue5, issue3, issue2] } - - where(:nodes) do - [ - lazy { Issue.order(Issue.arel_table[:relative_position].desc.nulls_first).order(id: :asc) } -] - end - - with_them do - it_behaves_like 'nodes are in descending order' + it 'raises an error' do + expect { subject.sliced_nodes }.to raise_error(Gitlab::Graphql::Errors::ArgumentError) end end end - context 'when ordering by similarity' do - let!(:project1) { create(:project, name: 'test') } - let!(:project2) { create(:project, name: 'testing') } - let!(:project3) { create(:project, name: 'tests') } - let!(:project4) { create(:project, name: 'testing stuff') } - let!(:project5) { create(:project, name: 'test') } + describe '#nodes' do + let_it_be(:all_nodes) { create_list(:project, 5) } - let(:nodes) do - Project.sorted_by_similarity_desc('test', include_in_select: true) + let(:paged_nodes) { subject.nodes } + + it_behaves_like 'connection with paged nodes' do + let(:paged_nodes_size) { 3 } end - let(:descending_nodes) { nodes.to_a } + context 'when both are passed' do + let(:arguments) { { first: 2, last: 2 } } - it_behaves_like 'nodes are in descending order' - end - - context 'when an invalid cursor is provided' do - let(:arguments) { { before: Base64Bp.urlsafe_encode64('invalidcursor', padding: false) } } - - it 'raises an error' do - expect { subject.sliced_nodes }.to raise_error(Gitlab::Graphql::Errors::ArgumentError) - end - end - end - - describe '#nodes' do - let_it_be(:all_nodes) { create_list(:project, 5) } - - let(:paged_nodes) { subject.nodes } - - it_behaves_like 'connection with paged nodes' do - let(:paged_nodes_size) { 3 } - end - - context 'when both are passed' do - let(:arguments) { { first: 2, last: 2 } } - - it 'raises an error' do - expect { paged_nodes }.to raise_error(Gitlab::Graphql::Errors::ArgumentError) - end - end - - context 'when primary key is not in original order' do - let(:nodes) { Project.order(last_repository_check_at: :desc) } - - before do - stub_feature_flags(new_graphql_keyset_pagination: false) - end - - it 'is added to end' do - sliced = subject.sliced_nodes - - order_sql = sliced.order_values.last.to_sql - - expect(order_sql).to end_with(Project.arel_table[:id].desc.to_sql) - end - end - - context 'when there is no primary key' do - before do - stub_const('NoPrimaryKey', Class.new(ActiveRecord::Base)) - NoPrimaryKey.class_eval do - self.table_name = 'no_primary_key' - self.primary_key = nil + it 'raises an error' do + expect { paged_nodes }.to raise_error(Gitlab::Graphql::Errors::ArgumentError) end end - let(:nodes) { NoPrimaryKey.all } + context 'when primary key is not in original order' do + let(:nodes) { Project.order(last_repository_check_at: :desc) } - it 'raises an error' do - expect(NoPrimaryKey.primary_key).to be_nil - expect { subject.sliced_nodes }.to raise_error(ArgumentError, 'Relation must have a primary key') + it 'is added to end' do + sliced = subject.sliced_nodes + + order_sql = sliced.order_values.last.to_sql + + expect(order_sql).to end_with(Project.arel_table[:id].desc.to_sql) + end end - end - end - describe '#has_previous_page and #has_next_page' do - # using a list of 5 items with a max_page of 3 - let_it_be(:project_list) { create_list(:project, 5) } - let_it_be(:nodes) { Project.order(:id) } + context 'when there is no primary key' do + before do + stub_const('NoPrimaryKey', Class.new(ActiveRecord::Base)) + NoPrimaryKey.class_eval do + self.table_name = 'no_primary_key' + self.primary_key = nil + end + end - context 'when default query' do - let(:arguments) { {} } + let(:nodes) { NoPrimaryKey.all } - it 'has no previous, but a next' do - expect(subject.has_previous_page).to be_falsey - expect(subject.has_next_page).to be_truthy + it 'raises an error' do + expect(NoPrimaryKey.primary_key).to be_nil + expect { subject.sliced_nodes }.to raise_error(ArgumentError, 'Relation must have a primary key') + end end end - context 'when before is first item' do - let(:arguments) { { before: encoded_cursor(project_list.first) } } + describe '#has_previous_page and #has_next_page' do + # using a list of 5 items with a max_page of 3 + let_it_be(:project_list) { create_list(:project, 5) } + let_it_be(:nodes) { Project.order(Gitlab::Pagination::Keyset::Order.build([column_order_id])) } - it 'has no previous, but a next' do - expect(subject.has_previous_page).to be_falsey - expect(subject.has_next_page).to be_truthy - end - end - - describe 'using `before`' do - context 'when before is the last item' do - let(:arguments) { { before: encoded_cursor(project_list.last) } } + context 'when default query' do + let(:arguments) { {} } it 'has no previous, but a next' do expect(subject.has_previous_page).to be_falsey @@ -436,51 +343,71 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::Connection do end end - context 'when before and last specified' do - let(:arguments) { { before: encoded_cursor(project_list.last), last: 2 } } + context 'when before is first item' do + let(:arguments) { { before: encoded_cursor(project_list.first) } } - it 'has a previous and a next' do - expect(subject.has_previous_page).to be_truthy - expect(subject.has_next_page).to be_truthy - end - end - - context 'when before and last does request all remaining nodes' do - let(:arguments) { { before: encoded_cursor(project_list[1]), last: 3 } } - - it 'has a previous and a next' do + it 'has no previous, but a next' do expect(subject.has_previous_page).to be_falsey expect(subject.has_next_page).to be_truthy - expect(subject.nodes).to eq [project_list[0]] - end - end - end - - describe 'using `after`' do - context 'when after is the first item' do - let(:arguments) { { after: encoded_cursor(project_list.first) } } - - it 'has a previous, and a next' do - expect(subject.has_previous_page).to be_truthy - expect(subject.has_next_page).to be_truthy end end - context 'when after and first specified' do - let(:arguments) { { after: encoded_cursor(project_list.first), first: 2 } } + describe 'using `before`' do + context 'when before is the last item' do + let(:arguments) { { before: encoded_cursor(project_list.last) } } - it 'has a previous and a next' do - expect(subject.has_previous_page).to be_truthy - expect(subject.has_next_page).to be_truthy + it 'has no previous, but a next' do + expect(subject.has_previous_page).to be_falsey + expect(subject.has_next_page).to be_truthy + end + end + + context 'when before and last specified' do + let(:arguments) { { before: encoded_cursor(project_list.last), last: 2 } } + + it 'has a previous and a next' do + expect(subject.has_previous_page).to be_truthy + expect(subject.has_next_page).to be_truthy + end + end + + context 'when before and last does request all remaining nodes' do + let(:arguments) { { before: encoded_cursor(project_list[1]), last: 3 } } + + it 'has a previous and a next' do + expect(subject.has_previous_page).to be_falsey + expect(subject.has_next_page).to be_truthy + expect(subject.nodes).to eq [project_list[0]] + end end end - context 'when before and last does request all remaining nodes' do - let(:arguments) { { after: encoded_cursor(project_list[2]), last: 3 } } + describe 'using `after`' do + context 'when after is the first item' do + let(:arguments) { { after: encoded_cursor(project_list.first) } } - it 'has a previous but no next' do - expect(subject.has_previous_page).to be_truthy - expect(subject.has_next_page).to be_falsey + it 'has a previous, and a next' do + expect(subject.has_previous_page).to be_truthy + expect(subject.has_next_page).to be_truthy + end + end + + context 'when after and first specified' do + let(:arguments) { { after: encoded_cursor(project_list.first), first: 2 } } + + it 'has a previous and a next' do + expect(subject.has_previous_page).to be_truthy + expect(subject.has_next_page).to be_truthy + end + end + + context 'when before and last does request all remaining nodes' do + let(:arguments) { { after: encoded_cursor(project_list[2]), last: 3 } } + + it 'has a previous but no next' do + expect(subject.has_previous_page).to be_truthy + expect(subject.has_next_page).to be_falsey + end end end end diff --git a/spec/lib/gitlab/graphql/pagination/keyset/order_info_spec.rb b/spec/lib/gitlab/graphql/pagination/keyset/order_info_spec.rb deleted file mode 100644 index 40ee47ece49..00000000000 --- a/spec/lib/gitlab/graphql/pagination/keyset/order_info_spec.rb +++ /dev/null @@ -1,118 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Graphql::Pagination::Keyset::OrderInfo do - describe '#build_order_list' do - let(:order_list) { described_class.build_order_list(relation) } - - context 'when multiple orders with SQL is specified' do - let(:relation) { Project.order(Arel.sql('projects.updated_at IS NULL')).order(:updated_at).order(:id) } - - it 'ignores the SQL order' do - expect(order_list.count).to eq 2 - expect(order_list.first.attribute_name).to eq 'updated_at' - expect(order_list.first.operator_for(:after)).to eq '>' - expect(order_list.last.attribute_name).to eq 'id' - expect(order_list.last.operator_for(:after)).to eq '>' - end - end - - context 'when order contains NULLS LAST' do - let(:relation) { Project.order(Arel.sql('projects.updated_at Asc Nulls Last')).order(:id) } - - it 'does not ignore the SQL order' do - expect(order_list.count).to eq 2 - expect(order_list.first.attribute_name).to eq 'projects.updated_at' - expect(order_list.first.operator_for(:after)).to eq '>' - expect(order_list.last.attribute_name).to eq 'id' - expect(order_list.last.operator_for(:after)).to eq '>' - end - end - - context 'when order contains invalid formatted NULLS LAST ' do - let(:relation) { Project.order(Arel.sql('projects.updated_at created_at Asc Nulls Last')).order(:id) } - - it 'ignores the SQL order' do - expect(order_list.count).to eq 1 - end - end - - context 'when order contains LOWER' do - let(:relation) { Project.order(Arel::Table.new(:projects)['name'].lower.asc).order(:id) } - - it 'does not ignore the SQL order' do - expect(order_list.count).to eq 2 - expect(order_list.first.attribute_name).to eq 'name' - expect(order_list.first.named_function).to be_kind_of(Arel::Nodes::NamedFunction) - expect(order_list.first.named_function.to_sql).to eq 'LOWER("projects"."name")' - expect(order_list.first.operator_for(:after)).to eq '>' - expect(order_list.last.attribute_name).to eq 'id' - expect(order_list.last.operator_for(:after)).to eq '>' - end - end - - context 'when ordering by CASE', :aggregate_failuers do - let(:relation) { Project.order(Arel::Nodes::Case.new(Project.arel_table[:pending_delete]).when(true).then(100).else(1000).asc) } - - it 'assigns the right attribute name, named function, and direction' do - expect(order_list.count).to eq 1 - expect(order_list.first.attribute_name).to eq 'case_order_value' - expect(order_list.first.named_function).to be_kind_of(Arel::Nodes::Case) - expect(order_list.first.sort_direction).to eq :asc - end - end - - context 'when ordering by ARRAY_POSITION', :aggregate_failuers do - let(:array_position) { Arel::Nodes::NamedFunction.new('ARRAY_POSITION', [Arel.sql("ARRAY[1,0]::smallint[]"), Project.arel_table[:auto_cancel_pending_pipelines]]) } - let(:relation) { Project.order(array_position.asc) } - - it 'assigns the right attribute name, named function, and direction' do - expect(order_list.count).to eq 1 - expect(order_list.first.attribute_name).to eq 'array_position' - expect(order_list.first.named_function).to be_kind_of(Arel::Nodes::NamedFunction) - expect(order_list.first.sort_direction).to eq :asc - end - end - end - - describe '#validate_ordering' do - let(:order_list) { described_class.build_order_list(relation) } - - context 'when number of ordering fields is 0' do - let(:relation) { Project.all } - - it 'raises an error' do - expect { described_class.validate_ordering(relation, order_list) } - .to raise_error(ArgumentError, 'A minimum of 1 ordering field is required') - end - end - - context 'when number of ordering fields is over 2' do - let(:relation) { Project.order(last_repository_check_at: :desc).order(updated_at: :desc).order(:id) } - - it 'raises an error' do - expect { described_class.validate_ordering(relation, order_list) } - .to raise_error(ArgumentError, 'A maximum of 2 ordering fields are allowed') - end - end - - context 'when the second (or first) column is nullable' do - let(:relation) { Project.order(last_repository_check_at: :desc).order(updated_at: :desc) } - - it 'raises an error' do - expect { described_class.validate_ordering(relation, order_list) } - .to raise_error(ArgumentError, "Column `updated_at` must not allow NULL") - end - end - - context 'for last ordering field' do - let(:relation) { Project.order(namespace_id: :desc) } - - it 'raises error if primary key is not last field' do - expect { described_class.validate_ordering(relation, order_list) } - .to raise_error(ArgumentError, "Last ordering field must be the primary key, `#{relation.primary_key}`") - end - end - end -end diff --git a/spec/lib/gitlab/graphql/pagination/keyset/query_builder_spec.rb b/spec/lib/gitlab/graphql/pagination/keyset/query_builder_spec.rb deleted file mode 100644 index 31c02fd43e8..00000000000 --- a/spec/lib/gitlab/graphql/pagination/keyset/query_builder_spec.rb +++ /dev/null @@ -1,135 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Graphql::Pagination::Keyset::QueryBuilder do - context 'when number of ordering fields is 0' do - it 'raises an error' do - expect { described_class.new(Issue.arel_table, [], {}, :after) } - .to raise_error(ArgumentError, 'No ordering scopes have been supplied') - end - end - - describe '#conditions' do - let(:relation) { Issue.order(relative_position: :desc).order(:id) } - let(:order_list) { Gitlab::Graphql::Pagination::Keyset::OrderInfo.build_order_list(relation) } - let(:arel_table) { Issue.arel_table } - let(:builder) { described_class.new(arel_table, order_list, decoded_cursor, before_or_after) } - let(:before_or_after) { :after } - - context 'when only a single ordering' do - let(:relation) { Issue.order(id: :desc) } - - context 'when the value is nil' do - let(:decoded_cursor) { { 'id' => nil } } - - it 'raises an error' do - expect { builder.conditions } - .to raise_error(Gitlab::Graphql::Errors::ArgumentError, 'Before/after cursor invalid: `nil` was provided as only sortable value') - end - end - - context 'when value is not nil' do - let(:decoded_cursor) { { 'id' => 100 } } - let(:conditions) { builder.conditions } - - context 'when :after' do - it 'generates the correct condition' do - expect(conditions.strip).to eq '("issues"."id" < 100)' - end - end - - context 'when :before' do - let(:before_or_after) { :before } - - it 'generates the correct condition' do - expect(conditions.strip).to eq '("issues"."id" > 100)' - end - end - end - end - - context 'when two orderings' do - let(:decoded_cursor) { { 'relative_position' => 1500, 'id' => 100 } } - - context 'when no values are nil' do - context 'when :after' do - it 'generates the correct condition' do - conditions = builder.conditions - - expect(conditions).to include '"issues"."relative_position" < 1500' - expect(conditions).to include '"issues"."id" > 100' - expect(conditions).to include 'OR ("issues"."relative_position" IS NULL)' - end - end - - context 'when :before' do - let(:before_or_after) { :before } - - it 'generates the correct condition' do - conditions = builder.conditions - - expect(conditions).to include '("issues"."relative_position" > 1500)' - expect(conditions).to include '"issues"."id" < 100' - expect(conditions).to include '"issues"."relative_position" = 1500' - end - end - end - - context 'when first value is nil' do - let(:decoded_cursor) { { 'relative_position' => nil, 'id' => 100 } } - - context 'when :after' do - it 'generates the correct condition' do - conditions = builder.conditions - - expect(conditions).to include '"issues"."relative_position" IS NULL' - expect(conditions).to include '"issues"."id" > 100' - end - end - - context 'when :before' do - let(:before_or_after) { :before } - - it 'generates the correct condition' do - conditions = builder.conditions - - expect(conditions).to include '"issues"."relative_position" IS NULL' - expect(conditions).to include '"issues"."id" < 100' - expect(conditions).to include 'OR ("issues"."relative_position" IS NOT NULL)' - end - end - end - end - - context 'when sorting using LOWER' do - let(:relation) { Project.order(Arel::Table.new(:projects)['name'].lower.asc).order(:id) } - let(:arel_table) { Project.arel_table } - let(:decoded_cursor) { { 'name' => 'Test', 'id' => 100 } } - - context 'when no values are nil' do - context 'when :after' do - it 'generates the correct condition' do - conditions = builder.conditions - - expect(conditions).to include '(LOWER("projects"."name") > \'test\')' - expect(conditions).to include '"projects"."id" > 100' - expect(conditions).to include 'OR (LOWER("projects"."name") IS NULL)' - end - end - - context 'when :before' do - let(:before_or_after) { :before } - - it 'generates the correct condition' do - conditions = builder.conditions - - expect(conditions).to include '(LOWER("projects"."name") < \'test\')' - expect(conditions).to include '"projects"."id" < 100' - expect(conditions).to include 'LOWER("projects"."name") = \'test\'' - end - end - end - end - end -end diff --git a/spec/lib/gitlab/graphql/type_name_deprecations_spec.rb b/spec/lib/gitlab/graphql/type_name_deprecations_spec.rb index 9fa2d59c093..0505e709a3b 100644 --- a/spec/lib/gitlab/graphql/type_name_deprecations_spec.rb +++ b/spec/lib/gitlab/graphql/type_name_deprecations_spec.rb @@ -1,23 +1,16 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'test_prof/recipes/rspec/let_it_be' require_relative '../../../support/helpers/type_name_deprecation_helpers' -TestProf::BeforeAll.adapter = Class.new do - def begin_transaction; end - - def rollback_transaction; end -end.new - RSpec.describe Gitlab::Graphql::TypeNameDeprecations do include TypeNameDeprecationHelpers - let_it_be(:deprecation_1) do + let(:deprecation_1) do described_class::NameDeprecation.new(old_name: 'Foo::Model', new_name: 'Bar', milestone: '9.0') end - let_it_be(:deprecation_2) do + let(:deprecation_2) do described_class::NameDeprecation.new(old_name: 'Baz', new_name: 'Qux::Model', milestone: '10.0') end diff --git a/spec/requests/api/graphql/group/group_members_spec.rb b/spec/requests/api/graphql/group/group_members_spec.rb index 1ff5b134e92..bab8d5b770c 100644 --- a/spec/requests/api/graphql/group/group_members_spec.rb +++ b/spec/requests/api/graphql/group/group_members_spec.rb @@ -64,24 +64,6 @@ RSpec.describe 'getting group members information' do expect_array_response(user_2) end - - context 'when the use_keyset_aware_user_search_query FF is off' do - before do - stub_feature_flags(use_keyset_aware_user_search_query: false) - end - - it 'raises error on the 2nd page due to missing cursor data' do - fetch_members(args: { search: 'Same Name', first: 1 }) - - # user_2 because the "old" order was undeterministic (insert order), no tie-breaker column - expect_array_response(user_2) - - next_cursor = graphql_data_at(:group, :groupMembers, :pageInfo, :endCursor) - fetch_members(args: { search: 'Same Name', first: 1, after: next_cursor }) - - expect(graphql_errors.first['message']).to include('PG::UndefinedColumn') - end - end end end end diff --git a/spec/requests/api/graphql/project/project_members_spec.rb b/spec/requests/api/graphql/project/project_members_spec.rb index 4225c3ad3e8..97a79ab3b0e 100644 --- a/spec/requests/api/graphql/project/project_members_spec.rb +++ b/spec/requests/api/graphql/project/project_members_spec.rb @@ -48,24 +48,6 @@ RSpec.describe 'getting project members information' do expect_array_response(user_2) end - - context 'when the use_keyset_aware_user_search_query FF is off' do - before do - stub_feature_flags(use_keyset_aware_user_search_query: false) - end - - it 'raises error on the 2nd page due to missing cursor data' do - fetch_members(project: parent_project, args: { search: 'Same Name', first: 1 }) - - # user_2 because the "old" order was undeterministic (insert order), no tie-breaker column - expect_array_response(user_2) - - next_cursor = graphql_data_at(:project, :projectMembers, :pageInfo, :endCursor) - fetch_members(project: parent_project, args: { search: 'Same Name', first: 1, after: next_cursor }) - - expect(graphql_errors.first['message']).to include('PG::UndefinedColumn') - end - end end end end diff --git a/spec/requests/api/graphql_spec.rb b/spec/requests/api/graphql_spec.rb index d94257c61eb..1c1ae73ddfe 100644 --- a/spec/requests/api/graphql_spec.rb +++ b/spec/requests/api/graphql_spec.rb @@ -463,50 +463,21 @@ RSpec.describe 'GraphQL' do ) end - context 'when new_graphql_keyset_pagination feature flag is off' do - before do - stub_feature_flags(new_graphql_keyset_pagination: false) - end + it 'paginates datetimes correctly when they have millisecond data' do + execute_query + first_page = graphql_data + edges = first_page.dig(*issues_edges) + cursor = first_page.dig(*end_cursor) - it 'paginates datetimes correctly when they have millisecond data' do - # let's make sure we're actually querying a timestamp, just in case - expect(Gitlab::Graphql::Pagination::Keyset::QueryBuilder) - .to receive(:new).with(anything, anything, hash_including('created_at'), anything).and_call_original + expect(edges.count).to eq(6) + expect(edges.last['node']['iid']).to eq(issues[4].iid.to_s) - execute_query - first_page = graphql_data - edges = first_page.dig(*issues_edges) - cursor = first_page.dig(*end_cursor) + execute_query(after: cursor) + second_page = graphql_data + edges = second_page.dig(*issues_edges) - expect(edges.count).to eq(6) - expect(edges.last['node']['iid']).to eq(issues[4].iid.to_s) - - execute_query(after: cursor) - second_page = graphql_data - edges = second_page.dig(*issues_edges) - - expect(edges.count).to eq(4) - expect(edges.last['node']['iid']).to eq(issues[0].iid.to_s) - end - end - - context 'when new_graphql_keyset_pagination feature flag is on' do - it 'paginates datetimes correctly when they have millisecond data' do - execute_query - first_page = graphql_data - edges = first_page.dig(*issues_edges) - cursor = first_page.dig(*end_cursor) - - expect(edges.count).to eq(6) - expect(edges.last['node']['iid']).to eq(issues[4].iid.to_s) - - execute_query(after: cursor) - second_page = graphql_data - edges = second_page.dig(*issues_edges) - - expect(edges.count).to eq(4) - expect(edges.last['node']['iid']).to eq(issues[0].iid.to_s) - end + expect(edges.count).to eq(4) + expect(edges.last['node']['iid']).to eq(issues[0].iid.to_s) end end end