diff options
Diffstat (limited to 'spec/lib/gitlab/pagination/keyset')
5 files changed, 193 insertions, 38 deletions
diff --git a/spec/lib/gitlab/pagination/keyset/column_order_definition_spec.rb b/spec/lib/gitlab/pagination/keyset/column_order_definition_spec.rb index 69384e0c501..778244677ef 100644 --- a/spec/lib/gitlab/pagination/keyset/column_order_definition_spec.rb +++ b/spec/lib/gitlab/pagination/keyset/column_order_definition_spec.rb @@ -140,8 +140,8 @@ RSpec.describe Gitlab::Pagination::Keyset::ColumnOrderDefinition do described_class.new( attribute_name: :name, column_expression: Project.arel_table[:name], - order_expression: Gitlab::Database.nulls_last_order('merge_request_metrics.merged_at', :desc), - reversed_order_expression: Gitlab::Database.nulls_first_order('merge_request_metrics.merged_at', :asc), + order_expression: MergeRequest::Metrics.arel_table[:merged_at].desc.nulls_last, + reversed_order_expression: MergeRequest::Metrics.arel_table[:merged_at].asc.nulls_first, order_direction: :desc, nullable: :nulls_last, # null values are always last distinct: false @@ -161,8 +161,8 @@ RSpec.describe Gitlab::Pagination::Keyset::ColumnOrderDefinition do described_class.new( attribute_name: :name, column_expression: Project.arel_table[:name], - order_expression: Gitlab::Database.nulls_last_order('merge_request_metrics.merged_at', :desc), - reversed_order_expression: Gitlab::Database.nulls_first_order('merge_request_metrics.merged_at', :asc), + order_expression: MergeRequest::Metrics.arel_table[:merged_at].desc.nulls_last, + reversed_order_expression: MergeRequest::Metrics.arel_table[:merged_at].asc.nulls_first, order_direction: :desc, nullable: true, distinct: false @@ -175,8 +175,8 @@ RSpec.describe Gitlab::Pagination::Keyset::ColumnOrderDefinition do described_class.new( attribute_name: :name, column_expression: Project.arel_table[:name], - order_expression: Gitlab::Database.nulls_last_order('merge_request_metrics.merged_at', :desc), - reversed_order_expression: Gitlab::Database.nulls_first_order('merge_request_metrics.merged_at', :asc), + order_expression: MergeRequest::Metrics.arel_table[:merged_at].desc.nulls_last, + reversed_order_expression: MergeRequest::Metrics.arel_table[:merged_at].asc.nulls_first, order_direction: :desc, nullable: :nulls_last, distinct: true @@ -191,8 +191,8 @@ RSpec.describe Gitlab::Pagination::Keyset::ColumnOrderDefinition do described_class.new( attribute_name: :name, column_expression: Project.arel_table[:name], - order_expression: Gitlab::Database.nulls_last_order('merge_request_metrics.merged_at', :desc), - reversed_order_expression: Gitlab::Database.nulls_first_order('merge_request_metrics.merged_at', :asc), + order_expression: MergeRequest::Metrics.arel_table[:merged_at].desc.nulls_last, + reversed_order_expression: MergeRequest::Metrics.arel_table[:merged_at].asc.nulls_first, order_direction: :desc, nullable: :nulls_last, # null values are always last distinct: false diff --git a/spec/lib/gitlab/pagination/keyset/in_operator_optimization/query_builder_spec.rb b/spec/lib/gitlab/pagination/keyset/in_operator_optimization/query_builder_spec.rb index 58db22e5a9c..9f2ac9a953d 100644 --- a/spec/lib/gitlab/pagination/keyset/in_operator_optimization/query_builder_spec.rb +++ b/spec/lib/gitlab/pagination/keyset/in_operator_optimization/query_builder_spec.rb @@ -24,12 +24,12 @@ RSpec.describe Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder let_it_be(:issues) do [ create(:issue, project: project_1, created_at: three_weeks_ago, relative_position: 5), - create(:issue, project: project_1, created_at: two_weeks_ago), + create(:issue, project: project_1, created_at: two_weeks_ago, relative_position: nil), create(:issue, project: project_2, created_at: two_weeks_ago, relative_position: 15), - create(:issue, project: project_2, created_at: two_weeks_ago), - create(:issue, project: project_3, created_at: four_weeks_ago), + create(:issue, project: project_2, created_at: two_weeks_ago, relative_position: nil), + create(:issue, project: project_3, created_at: four_weeks_ago, relative_position: nil), create(:issue, project: project_4, created_at: five_weeks_ago, relative_position: 10), - create(:issue, project: project_5, created_at: four_weeks_ago) + create(:issue, project: project_5, created_at: four_weeks_ago, relative_position: nil) ] end @@ -121,8 +121,8 @@ RSpec.describe Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( attribute_name: :relative_position, column_expression: Issue.arel_table[:relative_position], - order_expression: Gitlab::Database.nulls_last_order('relative_position', :desc), - reversed_order_expression: Gitlab::Database.nulls_first_order('relative_position', :asc), + order_expression: Issue.arel_table[:relative_position].desc.nulls_last, + reversed_order_expression: Issue.arel_table[:relative_position].asc.nulls_first, order_direction: :desc, nullable: :nulls_last, distinct: false @@ -155,6 +155,31 @@ RSpec.describe Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder it_behaves_like 'correct ordering examples' end + + context 'with condition "relative_position IS NULL"' do + let(:base_scope) { Issue.where(relative_position: nil) } + let(:scope) { base_scope.order(order) } + + let(:in_operator_optimization_options) do + { + array_scope: Project.where(namespace_id: top_level_group.self_and_descendants.select(:id)).select(:id), + array_mapping_scope: -> (id_expression) { Issue.merge(base_scope.dup).where(Issue.arel_table[:project_id].eq(id_expression)) }, + finder_query: -> (_relative_position_expression, id_expression) { Issue.where(Issue.arel_table[:id].eq(id_expression)) } + } + end + + context 'when iterating records one by one' do + let(:batch_size) { 1 } + + it_behaves_like 'correct ordering examples' + end + + context 'when iterating records with LIMIT 3' do + let(:batch_size) { 3 } + + it_behaves_like 'correct ordering examples' + end + end end context 'when ordering by issues.created_at DESC, issues.id ASC' do @@ -239,7 +264,7 @@ RSpec.describe Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder end it 'raises error when unsupported scope is passed' do - scope = Issue.order(Issue.arel_table[:id].lower.desc) + scope = Issue.order(Arel::Nodes::NamedFunction.new('UPPER', [Issue.arel_table[:id]])) options = { scope: scope, diff --git a/spec/lib/gitlab/pagination/keyset/iterator_spec.rb b/spec/lib/gitlab/pagination/keyset/iterator_spec.rb index 09cbca2c1cb..d62d20d2d2c 100644 --- a/spec/lib/gitlab/pagination/keyset/iterator_spec.rb +++ b/spec/lib/gitlab/pagination/keyset/iterator_spec.rb @@ -19,8 +19,8 @@ RSpec.describe Gitlab::Pagination::Keyset::Iterator do Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( attribute_name: column, column_expression: klass.arel_table[column], - order_expression: ::Gitlab::Database.nulls_order(column, direction, nulls_position), - reversed_order_expression: ::Gitlab::Database.nulls_order(column, reverse_direction, reverse_nulls_position), + order_expression: klass.arel_table[column].public_send(direction).public_send(nulls_position), # rubocop:disable GitlabSecurity/PublicSend + reversed_order_expression: klass.arel_table[column].public_send(reverse_direction).public_send(reverse_nulls_position), # rubocop:disable GitlabSecurity/PublicSend order_direction: direction, nullable: nulls_position, distinct: false @@ -99,7 +99,7 @@ RSpec.describe Gitlab::Pagination::Keyset::Iterator do iterator.each_batch(of: 2) { |rel| positions.concat(rel.pluck(:relative_position, :id)) } - expect(positions).to eq(project.issues.reorder(::Gitlab::Database.nulls_last_order('relative_position', 'ASC')).order(id: :asc).pluck(:relative_position, :id)) + expect(positions).to eq(project.issues.reorder(Issue.arel_table[:relative_position].asc.nulls_last).order(id: :asc).pluck(:relative_position, :id)) end end @@ -111,7 +111,7 @@ RSpec.describe Gitlab::Pagination::Keyset::Iterator do iterator.each_batch(of: 2) { |rel| positions.concat(rel.pluck(:relative_position, :id)) } - expect(positions).to eq(project.issues.reorder(::Gitlab::Database.nulls_first_order('relative_position', 'DESC')).order(id: :desc).pluck(:relative_position, :id)) + expect(positions).to eq(project.issues.reorder(Issue.arel_table[:relative_position].desc.nulls_first).order(id: :desc).pluck(:relative_position, :id)) end end @@ -123,7 +123,7 @@ RSpec.describe Gitlab::Pagination::Keyset::Iterator do iterator.each_batch(of: 2) { |rel| positions.concat(rel.pluck(:relative_position, :id)) } - expect(positions).to eq(project.issues.reorder(::Gitlab::Database.nulls_first_order('relative_position', 'ASC')).order(id: :asc).pluck(:relative_position, :id)) + expect(positions).to eq(project.issues.reorder(Issue.arel_table[:relative_position].asc.nulls_first).order(id: :asc).pluck(:relative_position, :id)) end end @@ -136,7 +136,7 @@ RSpec.describe Gitlab::Pagination::Keyset::Iterator do iterator.each_batch(of: 2) { |rel| positions.concat(rel.pluck(:relative_position, :id)) } - expect(positions).to eq(project.issues.reorder(::Gitlab::Database.nulls_last_order('relative_position', 'DESC')).order(id: :desc).pluck(:relative_position, :id)) + expect(positions).to eq(project.issues.reorder(Issue.arel_table[:relative_position].desc.nulls_last).order(id: :desc).pluck(:relative_position, :id)) end end diff --git a/spec/lib/gitlab/pagination/keyset/order_spec.rb b/spec/lib/gitlab/pagination/keyset/order_spec.rb index 1bed8e542a2..abbb3a21cd4 100644 --- a/spec/lib/gitlab/pagination/keyset/order_spec.rb +++ b/spec/lib/gitlab/pagination/keyset/order_spec.rb @@ -262,8 +262,8 @@ RSpec.describe Gitlab::Pagination::Keyset::Order do Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( attribute_name: 'year', column_expression: table['year'], - order_expression: Gitlab::Database.nulls_last_order('year', :asc), - reversed_order_expression: Gitlab::Database.nulls_first_order('year', :desc), + order_expression: table[:year].asc.nulls_last, + reversed_order_expression: table[:year].desc.nulls_first, order_direction: :asc, nullable: :nulls_last, distinct: false @@ -271,8 +271,8 @@ RSpec.describe Gitlab::Pagination::Keyset::Order do Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( attribute_name: 'month', column_expression: table['month'], - order_expression: Gitlab::Database.nulls_last_order('month', :asc), - reversed_order_expression: Gitlab::Database.nulls_first_order('month', :desc), + order_expression: table[:month].asc.nulls_last, + reversed_order_expression: table[:month].desc.nulls_first, order_direction: :asc, nullable: :nulls_last, distinct: false @@ -328,8 +328,8 @@ RSpec.describe Gitlab::Pagination::Keyset::Order do Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( attribute_name: 'year', column_expression: table['year'], - order_expression: Gitlab::Database.nulls_first_order('year', :asc), - reversed_order_expression: Gitlab::Database.nulls_last_order('year', :desc), + order_expression: table[:year].asc.nulls_first, + reversed_order_expression: table[:year].desc.nulls_last, order_direction: :asc, nullable: :nulls_first, distinct: false @@ -337,9 +337,9 @@ RSpec.describe Gitlab::Pagination::Keyset::Order do Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( attribute_name: 'month', column_expression: table['month'], - order_expression: Gitlab::Database.nulls_first_order('month', :asc), + order_expression: table[:month].asc.nulls_first, order_direction: :asc, - reversed_order_expression: Gitlab::Database.nulls_last_order('month', :desc), + reversed_order_expression: table[:month].desc.nulls_last, nullable: :nulls_first, distinct: false ), @@ -441,6 +441,47 @@ RSpec.describe Gitlab::Pagination::Keyset::Order do end end + context 'when ordering by the named function LOWER' do + let(:order) do + Gitlab::Pagination::Keyset::Order.build([ + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'title', + column_expression: Arel::Nodes::NamedFunction.new("LOWER", [table['title'].desc]), + order_expression: table['title'].lower.desc, + nullable: :not_nullable, + distinct: false + ), + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'id', + column_expression: table['id'], + order_expression: table['id'].desc, + nullable: :not_nullable, + distinct: true + ) + ]) + end + + let(:table_data) do + <<-SQL + VALUES (1, 'A') + SQL + end + + let(:query) do + <<-SQL + SELECT id, title + FROM (#{table_data}) my_table (id, title) + ORDER BY #{order}; + SQL + end + + subject { run_query(query) } + + it "uses downcased value for encoding and decoding a cursor" do + expect(order.cursor_attributes_for_node(subject.first)['title']).to eq("a") + end + end + context 'when the passed cursor values do not match with the order definition' do let(:order) do Gitlab::Pagination::Keyset::Order.build([ diff --git a/spec/lib/gitlab/pagination/keyset/simple_order_builder_spec.rb b/spec/lib/gitlab/pagination/keyset/simple_order_builder_spec.rb index 5af86cb2dc0..4f1d380ab0a 100644 --- a/spec/lib/gitlab/pagination/keyset/simple_order_builder_spec.rb +++ b/spec/lib/gitlab/pagination/keyset/simple_order_builder_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe Gitlab::Pagination::Keyset::SimpleOrderBuilder do let(:ordered_scope) { described_class.build(scope).first } let(:order_object) { Gitlab::Pagination::Keyset::Order.extract_keyset_order_object(ordered_scope) } + let(:column_definition) { order_object.column_definitions.first } subject(:sql_with_order) { ordered_scope.to_sql } @@ -16,11 +17,25 @@ RSpec.describe Gitlab::Pagination::Keyset::SimpleOrderBuilder do end it 'sets the column definition distinct and not nullable' do - column_definition = order_object.column_definitions.first - expect(column_definition).to be_not_nullable expect(column_definition).to be_distinct end + + context "when the order scope's model uses default_scope" do + let(:scope) do + model = Class.new(ApplicationRecord) do + self.table_name = 'events' + + default_scope { reorder(nil) } # rubocop:disable Cop/DefaultScope + end + + model.reorder(nil) + end + + it 'orders by primary key' do + expect(sql_with_order).to end_with('ORDER BY "events"."id" DESC') + end + end end context 'when primary key order present' do @@ -39,8 +54,6 @@ RSpec.describe Gitlab::Pagination::Keyset::SimpleOrderBuilder do end it 'sets the column definition for created_at non-distinct and nullable' do - column_definition = order_object.column_definitions.first - expect(column_definition.attribute_name).to eq('created_at') expect(column_definition.nullable?).to eq(true) # be_nullable calls non_null? method for some reason expect(column_definition).not_to be_distinct @@ -59,14 +72,80 @@ RSpec.describe Gitlab::Pagination::Keyset::SimpleOrderBuilder do let(:scope) { Project.where(id: [1, 2, 3]).order(namespace_id: :asc, id: :asc) } it 'sets the column definition for namespace_id non-distinct and non-nullable' do - column_definition = order_object.column_definitions.first - expect(column_definition.attribute_name).to eq('namespace_id') expect(column_definition).to be_not_nullable expect(column_definition).not_to be_distinct end end + context 'when ordering by a column with the lower named function' do + let(:scope) { Project.where(id: [1, 2, 3]).order(Project.arel_table[:name].lower.desc) } + + it 'sets the column definition for name' do + expect(column_definition.attribute_name).to eq('name') + expect(column_definition.column_expression.expressions.first.name).to eq('name') + expect(column_definition.column_expression.name).to eq('LOWER') + end + + it 'adds extra primary key order as tie-breaker' do + expect(sql_with_order).to end_with('ORDER BY LOWER("projects"."name") DESC, "projects"."id" DESC') + end + end + + context "NULLS order given as as an Arel literal" do + context 'when NULLS LAST order is given without a tie-breaker' do + let(:scope) { Project.order(Project.arel_table[:created_at].asc.nulls_last) } + + it 'sets the column definition for created_at appropriately' do + expect(column_definition.attribute_name).to eq('created_at') + end + + it 'orders by primary key' do + expect(sql_with_order) + .to end_with('ORDER BY "projects"."created_at" ASC NULLS LAST, "projects"."id" DESC') + end + end + + context 'when NULLS FIRST order is given with a tie-breaker' do + let(:scope) { Issue.order(Issue.arel_table[:relative_position].desc.nulls_first).order(id: :asc) } + + it 'sets the column definition for created_at appropriately' do + expect(column_definition.attribute_name).to eq('relative_position') + end + + it 'orders by the given primary key' do + expect(sql_with_order) + .to end_with('ORDER BY "issues"."relative_position" DESC NULLS FIRST, "issues"."id" ASC') + end + end + end + + context "NULLS order given as as an Arel node" do + context 'when NULLS LAST order is given without a tie-breaker' do + let(:scope) { Project.order(Project.arel_table[:created_at].asc.nulls_last) } + + it 'sets the column definition for created_at appropriately' do + expect(column_definition.attribute_name).to eq('created_at') + end + + it 'orders by primary key' do + expect(sql_with_order).to end_with('ORDER BY "projects"."created_at" ASC NULLS LAST, "projects"."id" DESC') + end + end + + context 'when NULLS FIRST order is given with a tie-breaker' do + let(:scope) { Issue.order(Issue.arel_table[:relative_position].desc.nulls_first).order(id: :asc) } + + it 'sets the column definition for created_at appropriately' do + expect(column_definition.attribute_name).to eq('relative_position') + end + + it 'orders by the given primary key' do + expect(sql_with_order).to end_with('ORDER BY "issues"."relative_position" DESC NULLS FIRST, "issues"."id" ASC') + end + end + end + context 'return :unable_to_order symbol when order cannot be built' do subject(:success) { described_class.build(scope).last } @@ -76,10 +155,20 @@ RSpec.describe Gitlab::Pagination::Keyset::SimpleOrderBuilder do it { is_expected.to eq(false) } end - context 'when NULLS LAST order is given' do - let(:scope) { Project.order(::Gitlab::Database.nulls_last_order('created_at', 'ASC')) } + context 'when an invalid NULLS order is given' do + using RSpec::Parameterized::TableSyntax - it { is_expected.to eq(false) } + where(:scope) do + [ + lazy { Project.order(Arel.sql('projects.updated_at created_at Asc Nulls Last')) }, + lazy { Project.order(Arel.sql('projects.created_at ZZZ NULLS FIRST')) }, + lazy { Project.order(Arel.sql('projects.relative_position ASC NULLS LAST')) } + ] + end + + with_them do + it { is_expected.to eq(false) } + end end context 'when more than 2 columns are given for the order' do |