Commit e8b0d6c5 authored by Alex Kalderimis's avatar Alex Kalderimis

Add offset paginated relation

This separates out the responsibility of pagination from the relation,
and is necessary to test resolvers using `Resolver.single` if they
return an offset paginated relation.

The reason for this is that the `single` resolver calls `super.first`
(more or less), but on a connection, `#first` is the value of the
`first:` argument, not the first item in the collection.

For `BaseResolver.single` to work correctly with offset pagination, we
need to return `relations` from `resolve`, not connections.
parent b53afe12
......@@ -118,7 +118,7 @@ module Resolvers
end
def offset_pagination(relation)
::Gitlab::Graphql::Pagination::OffsetActiveRecordRelationConnection.new(relation)
::Gitlab::Graphql::Pagination::OffsetPaginatedRelation.new(relation)
end
override :object
......
......@@ -68,7 +68,7 @@ module Mutations
def find_object(parent:, id:)
::Resolvers::IterationsResolver.new(object: parent, context: context, field: nil)
.resolve(id: id).items.first
.resolve(id: id).first
end
def validate_arguments!(args)
......
......@@ -33,13 +33,13 @@ RSpec.describe Resolvers::BoardListIssuesResolver do
end
it 'accepts epic global id' do
result = resolve_board_list_issues({ filters: { epic_id: epic.to_global_id } }).items
result = resolve_board_list_issues({ filters: { epic_id: epic.to_global_id } })
expect(result).to match_array([issue])
end
it 'accepts epic wildcard id' do
result = resolve_board_list_issues({ filters: { epic_wildcard_id: 'NONE' } }).items
result = resolve_board_list_issues({ filters: { epic_wildcard_id: 'NONE' } })
expect(result).to match_array([])
end
......@@ -51,13 +51,13 @@ RSpec.describe Resolvers::BoardListIssuesResolver do
let_it_be(:issue_without_iteration) { create(:issue, project: project, labels: [label]) }
it 'accepts iteration title' do
result = resolve_board_list_issues({ filters: { iteration_title: iteration.title } }).items
result = resolve_board_list_issues({ filters: { iteration_title: iteration.title } })
expect(result).to contain_exactly(issue_with_iteration)
end
it 'accepts iteration wildcard id' do
result = resolve_board_list_issues({ filters: { iteration_wildcard_id: 'NONE' } }).items
result = resolve_board_list_issues({ filters: { iteration_wildcard_id: 'NONE' } })
expect(result).to contain_exactly(issue_without_iteration)
end
......
......@@ -25,7 +25,7 @@ RSpec.describe Resolvers::BoardListsResolver do
end
it 'returns a list of board lists' do
lists = resolve_board_lists.items
lists = resolve_board_lists
expect(lists.count).to eq 3
expect(lists.map(&:list_type)).to eq %w(closed assignee milestone)
......
......@@ -36,7 +36,7 @@ RSpec.describe Resolvers::Boards::BoardListEpicsResolver do
end
it 'returns epics on the board list ordered by position on the board' do
expect(result.items).to eq([list1_epic2, list1_epic1])
expect(result.to_a).to eq([list1_epic2, list1_epic1])
end
end
end
......@@ -42,7 +42,9 @@ RSpec.describe Resolvers::Boards::EpicBoardsResolver do
end
it 'returns epic boards in the group ordered by name' do
expect(result).to eq([epic_board2, epic_board1])
expect(result)
.to contain_exactly(epic_board2, epic_board1)
.and be_sorted(:name, :asc)
end
context 'when epic_boards flag is disabled' do
......
......@@ -17,8 +17,9 @@ RSpec.describe Resolvers::Boards::EpicListsResolver do
describe '#resolve' do
let(:args) { {} }
let(:resolver) { described_class }
subject(:result) { resolve(described_class, ctx: { current_user: user }, obj: epic_board, args: args) }
subject(:result) { resolve(resolver, ctx: { current_user: user }, obj: epic_board, args: args) }
before do
stub_licensed_features(epics: true)
......@@ -34,14 +35,15 @@ RSpec.describe Resolvers::Boards::EpicListsResolver do
end
it 'returns epic lists for the board' do
expect(result.items).to match_array([epic_list1, epic_list2])
expect(result).to match_array([epic_list1, epic_list2])
end
context 'when list ID param is set' do
context 'when resolving a single item' do
let(:args) { { id: epic_list1.to_global_id } }
let(:resolver) { described_class.single }
it 'returns an array with single epic list' do
expect(result.items).to match_array([epic_list1])
expect(result).to eq epic_list1
end
end
end
......
......@@ -370,7 +370,7 @@ RSpec.describe GitlabSchema.types['Project'] do
resolve_field(:compliance_frameworks, p)
end
end
frameworks = results.flat_map(&:items)
frameworks = results.flat_map(&:to_a)
expect(frameworks).to match_array(projects.flat_map(&:compliance_management_frameworks))
end
......
......@@ -5,6 +5,10 @@ module Gitlab
module Pagination
module Connections
def self.use(schema)
schema.connections.add(
::Gitlab::Graphql::Pagination::OffsetPaginatedRelation,
::Gitlab::Graphql::Pagination::OffsetActiveRecordRelationConnection)
schema.connections.add(
ActiveRecord::Relation,
Gitlab::Graphql::Pagination::Keyset::Connection)
......
# frozen_string_literal: true
# Marker class to enable us to choose the correct
# connection type during resolution
module Gitlab
module Graphql
module Pagination
class OffsetPaginatedRelation < SimpleDelegator
end
end
end
end
......@@ -278,7 +278,7 @@ RSpec.describe Resolvers::BaseResolver do
let(:instance) { resolver_instance(resolver) }
it 'is sugar for OffsetActiveRecordRelationConnection.new' do
expect(instance.offset_pagination(User.none)).to be_a(::Gitlab::Graphql::Pagination::OffsetActiveRecordRelationConnection)
expect(instance.offset_pagination(User.none)).to be_a(::Gitlab::Graphql::Pagination::OffsetPaginatedRelation)
end
end
end
......@@ -23,19 +23,19 @@ RSpec.describe Resolvers::BoardListIssuesResolver do
it 'returns the issues in the correct order' do
# by relative_position and then ID
issues = resolve_board_list_issues.items
issues = resolve_board_list_issues
expect(issues.map(&:id)).to eq [issue3.id, issue1.id, issue2.id]
end
it 'finds only issues matching filters' do
result = resolve_board_list_issues(args: { filters: { label_name: [label.title], not: { label_name: [label2.title] } } }).items
result = resolve_board_list_issues(args: { filters: { label_name: [label.title], not: { label_name: [label2.title] } } })
expect(result).to match_array([issue1, issue3])
end
it 'finds only issues matching search param' do
result = resolve_board_list_issues(args: { filters: { search: issue1.title } }).items
result = resolve_board_list_issues(args: { filters: { search: issue1.title } })
expect(result).to match_array([issue1])
end
......
......@@ -21,7 +21,7 @@ RSpec.describe Resolvers::BoardListsResolver do
end
it 'does not create the backlog list' do
lists = resolve_board_lists.items
lists = resolve_board_lists
expect(lists.count).to eq 1
expect(lists[0].list_type).to eq 'closed'
......@@ -38,7 +38,7 @@ RSpec.describe Resolvers::BoardListsResolver do
let!(:backlog_list) { create(:backlog_list, board: board) }
it 'returns a list of board lists' do
lists = resolve_board_lists.items
lists = resolve_board_lists
expect(lists.count).to eq 3
expect(lists.map(&:list_type)).to eq %w(backlog label closed)
......@@ -50,7 +50,7 @@ RSpec.describe Resolvers::BoardListsResolver do
end
it 'returns the complete list of board lists for this user' do
lists = resolve_board_lists.items
lists = resolve_board_lists
expect(lists.count).to eq 3
end
......@@ -58,7 +58,7 @@ RSpec.describe Resolvers::BoardListsResolver do
context 'when querying for a single list' do
it 'returns specified list' do
list = resolve_board_lists(args: { id: global_id_of(label_list) }).items
list = resolve_board_lists(args: { id: global_id_of(label_list) })
expect(list).to eq [label_list]
end
......@@ -69,13 +69,13 @@ RSpec.describe Resolvers::BoardListsResolver do
external_label = create(:group_label, group: group)
external_list = create(:list, board: external_board, label: external_label)
list = resolve_board_lists(args: { id: global_id_of(external_list) }).items
list = resolve_board_lists(args: { id: global_id_of(external_list) })
expect(list).to eq List.none
end
it 'raises an argument error if list ID is not valid' do
expect { resolve_board_lists(args: { id: 'test' }).items }
expect { resolve_board_lists(args: { id: 'test' }) }
.to raise_error(Gitlab::Graphql::Errors::ArgumentError)
end
end
......
......@@ -195,11 +195,11 @@ RSpec.describe Resolvers::IssuesResolver do
let_it_be(:priority_issue4) { create(:issue, project: project) }
it 'sorts issues ascending' do
expect(resolve_issues(sort: :priority_asc).items).to eq([priority_issue3, priority_issue1, priority_issue2, priority_issue4])
expect(resolve_issues(sort: :priority_asc).to_a).to eq([priority_issue3, priority_issue1, priority_issue2, priority_issue4])
end
it 'sorts issues descending' do
expect(resolve_issues(sort: :priority_desc).items).to eq([priority_issue1, priority_issue3, priority_issue2, priority_issue4])
expect(resolve_issues(sort: :priority_desc).to_a).to eq([priority_issue1, priority_issue3, priority_issue2, priority_issue4])
end
end
......@@ -214,11 +214,11 @@ RSpec.describe Resolvers::IssuesResolver do
let_it_be(:label_issue4) { create(:issue, project: project) }
it 'sorts issues ascending' do
expect(resolve_issues(sort: :label_priority_asc).items).to eq([label_issue3, label_issue1, label_issue2, label_issue4])
expect(resolve_issues(sort: :label_priority_asc).to_a).to eq([label_issue3, label_issue1, label_issue2, label_issue4])
end
it 'sorts issues descending' do
expect(resolve_issues(sort: :label_priority_desc).items).to eq([label_issue2, label_issue3, label_issue1, label_issue4])
expect(resolve_issues(sort: :label_priority_desc).to_a).to eq([label_issue2, label_issue3, label_issue1, label_issue4])
end
end
......@@ -231,11 +231,11 @@ RSpec.describe Resolvers::IssuesResolver do
let_it_be(:milestone_issue3) { create(:issue, project: project, milestone: late_milestone) }
it 'sorts issues ascending' do
expect(resolve_issues(sort: :milestone_due_asc).items).to eq([milestone_issue2, milestone_issue3, milestone_issue1])
expect(resolve_issues(sort: :milestone_due_asc).to_a).to eq([milestone_issue2, milestone_issue3, milestone_issue1])
end
it 'sorts issues descending' do
expect(resolve_issues(sort: :milestone_due_desc).items).to eq([milestone_issue3, milestone_issue2, milestone_issue1])
expect(resolve_issues(sort: :milestone_due_desc).to_a).to eq([milestone_issue3, milestone_issue2, milestone_issue1])
end
end
......
......@@ -30,7 +30,17 @@ RSpec.describe Resolvers::MergeRequestsResolver do
end
describe '#resolve' do
let(:queries_per_project) { 3 }
# One for the initial auth, then MRs, and the load of project and project_feature (for further auth):
# SELECT MAX("project_authorizations"."access_level") AS maximum_access_level,
# "project_authorizations"."user_id" AS project_authorizations_user_id
# FROM "project_authorizations"
# WHERE "project_authorizations"."project_id" = 2 AND "project_authorizations"."user_id" = 2
# GROUP BY "project_authorizations"."user_id"
# SELECT "merge_requests".* FROM "merge_requests" WHERE "merge_requests"."target_project_id" = 2
# AND "merge_requests"."iid" = 1 ORDER BY "merge_requests"."id" DESC
# SELECT "projects".* FROM "projects" WHERE "projects"."id" = 2
# SELECT "project_features".* FROM "project_features" WHERE "project_features"."project_id" = 2
let(:queries_per_project) { 4 }
context 'no arguments' do
it 'returns all merge requests' do
......@@ -72,15 +82,17 @@ RSpec.describe Resolvers::MergeRequestsResolver do
expect(result).to contain_exactly(merge_request_1, merge_request_2, merge_request_3)
end
it 'can batch-resolve merge requests from different projects' do
it 'can batch-resolve merge requests from different projects', :request_store, :use_clean_rails_memory_store_caching do
# 2 queries for project_authorizations, and 2 for merge_requests
result = batch_sync(max_queries: queries_per_project * 2) do
resolve_mr(project, iids: [iid_1]) +
resolve_mr(project, iids: [iid_2]) +
resolve_mr(other_project, iids: [other_iid])
results = batch_sync(max_queries: queries_per_project * 2) do
a = resolve_mr(project, iids: [iid_1])
b = resolve_mr(project, iids: [iid_2])
c = resolve_mr(other_project, iids: [other_iid])
[a, b, c].flat_map(&:to_a)
end
expect(result).to contain_exactly(merge_request_1, merge_request_2, other_merge_request)
expect(results).to contain_exactly(merge_request_1, merge_request_2, other_merge_request)
end
it 'resolves an unknown iid to be empty' do
......@@ -134,9 +146,9 @@ RSpec.describe Resolvers::MergeRequestsResolver do
it 'takes more than one argument' do
mrs = [merge_request_3, merge_request_4]
branches = mrs.map(&:target_branch)
result = resolve_mr(project, target_branches: branches )
result = resolve_mr(project, target_branches: branches)
expect(result.compact).to match_array(mrs)
expect(result).to match_array(mrs)
end
end
......@@ -173,7 +185,7 @@ RSpec.describe Resolvers::MergeRequestsResolver do
it 'returns merge requests merged between the given period' do
result = resolve_mr(project, merged_after: 20.days.ago, merged_before: 5.days.ago)
expect(result).to eq([merge_request_1])
expect(result).to contain_exactly(merge_request_1)
end
it 'does not return anything' do
......@@ -187,7 +199,7 @@ RSpec.describe Resolvers::MergeRequestsResolver do
it 'filters merge requests by milestone title' do
result = resolve_mr(project, milestone_title: milestone.title)
expect(result).to eq([merge_request_with_milestone])
expect(result).to contain_exactly(merge_request_with_milestone)
end
it 'does not find anything' do
......@@ -203,18 +215,29 @@ RSpec.describe Resolvers::MergeRequestsResolver do
result = resolve_mr(project, source_branches: [merge_request_4.source_branch], state: 'locked')
expect(result.compact).to contain_exactly(merge_request_4)
expect(result).to contain_exactly(merge_request_4)
end
end
describe 'sorting' do
let(:mrs) do
[
merge_request_with_milestone, merge_request_6, merge_request_5, merge_request_4,
merge_request_3, merge_request_2, merge_request_1
]
end
context 'when sorting by created' do
it 'sorts merge requests ascending' do
expect(resolve_mr(project, sort: 'created_asc')).to eq [merge_request_1, merge_request_2, merge_request_3, merge_request_4, merge_request_5, merge_request_6, merge_request_with_milestone]
expect(resolve_mr(project, sort: 'created_asc'))
.to match_array(mrs)
.and be_sorted(:created_at, :asc)
end
it 'sorts merge requests descending' do
expect(resolve_mr(project, sort: 'created_desc')).to eq [merge_request_with_milestone, merge_request_6, merge_request_5, merge_request_4, merge_request_3, merge_request_2, merge_request_1]
expect(resolve_mr(project, sort: 'created_desc'))
.to match_array(mrs)
.and be_sorted(:created_at, :desc)
end
end
......@@ -225,11 +248,19 @@ RSpec.describe Resolvers::MergeRequestsResolver do
end
it 'sorts merge requests ascending' do
expect(resolve_mr(project, sort: :merged_at_asc)).to eq [merge_request_1, merge_request_3, merge_request_with_milestone, merge_request_6, merge_request_5, merge_request_4, merge_request_2]
expect(resolve_mr(project, sort: :merged_at_asc))
.to match_array(mrs)
.and be_sorted(->(mr) { [merged_at(mr, 1.year.from_now).to_i, -mr.id] })
end
it 'sorts merge requests descending' do
expect(resolve_mr(project, sort: :merged_at_desc)).to eq [merge_request_3, merge_request_1, merge_request_with_milestone, merge_request_6, merge_request_5, merge_request_4, merge_request_2]
expect(resolve_mr(project, sort: :merged_at_desc))
.to match_array(mrs)
.and be_sorted(->(mr) { [-merged_at(mr).to_i, -mr.id] })
end
def merged_at(mr, when_nil = nil)
mr.metrics.merged_at || when_nil
end
context 'when label filter is given and the optimized_issuable_label_filter feature flag is off' do
......
......@@ -12,12 +12,12 @@ RSpec.describe Resolvers::ReleaseMilestonesResolver do
end
describe '#resolve' do
it "returns an OffsetActiveRecordRelationConnection" do
expect(resolved).to be_a(::Gitlab::Graphql::Pagination::OffsetActiveRecordRelationConnection)
it "uses offset-pagination" do
expect(resolved).to be_a(::Gitlab::Graphql::Pagination::OffsetPaginatedRelation)
end
it "includes the release's milestones in the returned OffsetActiveRecordRelationConnection" do
expect(resolved.items).to eq(release.milestones.order_by_dates_and_title)
expect(resolved.to_a).to eq(release.milestones.order_by_dates_and_title)
end
end
end
# frozen_string_literal: true
require 'spec_helper'
# Tests that our connections are correctly mapped.
RSpec.describe ::Gitlab::Graphql::Pagination::Connections do
include GraphqlHelpers
before(:all) do
ActiveRecord::Schema.define do
create_table :testing_pagination_nodes, force: true do |t|
t.integer :value, null: false
end
end
end
after(:all) do
ActiveRecord::Schema.define do
drop_table :testing_pagination_nodes, force: true
end
end
let_it_be(:node_model) do
Class.new(ActiveRecord::Base) do
self.table_name = 'testing_pagination_nodes'
end
end
let(:query_string) { 'query { items(first: 2) { nodes { value } } }' }
let(:user) { nil }
let(:node) { Struct.new(:value) }
let(:node_type) do
Class.new(::GraphQL::Schema::Object) do
graphql_name 'Node'
field :value, GraphQL::INT_TYPE, null: false
end
end
let(:query_type) do
item_values = nodes
query_factory do |t|
t.field :items, node_type.connection_type, null: true
t.define_method :items do
item_values
end
end
end
shared_examples 'it maps to a specific connection class' do |connection_type|
let(:raw_values) { [1, 7, 42] }
it "maps to #{connection_type.name}" do
expect(connection_type).to receive(:new).and_call_original
results = execute_query(query_type).to_h
expect(graphql_dig_at(results, :data, :items, :nodes, :value)).to eq [1, 7]
end
end
describe 'OffsetPaginatedRelation' do
before do
# Expect to be ordered by an explicit ordering.
raw_values.each_with_index { |value, id| node_model.create!(id: id, value: value) }
end
let(:nodes) { ::Gitlab::Graphql::Pagination::OffsetPaginatedRelation.new(node_model.order(value: :asc)) }
include_examples 'it maps to a specific connection class', Gitlab::Graphql::Pagination::OffsetActiveRecordRelationConnection
end
describe 'ActiveRecord::Relation' do
before do
# Expect to be ordered by ID descending
[3, 2, 1].zip(raw_values) { |id, value| node_model.create!(id: id, value: value) }
end
let(:nodes) { node_model.all }
include_examples 'it maps to a specific connection class', Gitlab::Graphql::Pagination::Keyset::Connection
end
describe 'ExternallyPaginatedArray' do
let(:nodes) { ::Gitlab::Graphql::ExternallyPaginatedArray.new(nil, nil, node.new(1), node.new(7)) }
include_examples 'it maps to a specific connection class', Gitlab::Graphql::Pagination::ExternallyPaginatedArrayConnection
end
describe 'Array' do
let(:nodes) { raw_values.map { |x| node.new(x) } }
include_examples 'it maps to a specific connection class', Gitlab::Graphql::Pagination::ArrayConnection
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment