From aa7b1cfc5b3319373a4b56c755b1fc1d4cbaff02 Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Wed, 4 Sep 2019 17:42:48 +0000 Subject: Upgrade GraphQL gem to 1.8.17 - Due to https://github.com/exAspArk/batch-loader/pull/32, we changed BatchLoader.for into BatchLoader::GraphQL.for - since our results are wrapped in a BatchLoader::GraphQL, calling `sync` during authorization is required to get real object - `graphql` now has it's own authorization system. Our `authorized?` method conflicted and required renaming --- Gemfile | 2 +- Gemfile.lock | 4 ++-- app/graphql/mutations/notes/create/base.rb | 4 ++-- app/graphql/resolvers/full_path_resolver.rb | 2 +- app/graphql/resolvers/issues_resolver.rb | 4 +--- app/graphql/resolvers/merge_requests_resolver.rb | 6 ++++-- app/graphql/resolvers/namespace_projects_resolver.rb | 4 +--- .../graphql/authorize/authorize_field_service.rb | 8 ++++---- lib/gitlab/graphql/authorize/authorize_resource.rb | 10 ++++++++-- lib/gitlab/graphql/loaders/batch_lfs_oid_loader.rb | 2 +- lib/gitlab/graphql/loaders/batch_model_loader.rb | 2 +- .../graphql/loaders/batch_project_statistics_loader.rb | 2 +- .../loaders/batch_root_storage_statistics_loader.rb | 2 +- lib/gitlab/graphql/loaders/pipeline_for_sha_loader.rb | 2 +- spec/graphql/gitlab_schema_spec.rb | 8 ++++---- .../concerns/mutations/resolves_project_spec.rb | 2 +- spec/graphql/resolvers/group_resolver_spec.rb | 4 ++-- spec/graphql/resolvers/issues_resolver_spec.rb | 2 +- spec/graphql/resolvers/merge_requests_resolver_spec.rb | 12 ++++++------ .../resolvers/namespace_projects_resolver_spec.rb | 2 +- spec/graphql/resolvers/project_resolver_spec.rb | 4 ++-- .../graphql/authorize/authorize_resource_spec.rb | 12 ++++++------ .../graphql/loaders/batch_lfs_oid_loader_spec.rb | 2 +- .../gitlab/graphql/loaders/batch_model_loader_spec.rb | 6 +++--- .../graphql/loaders/pipeline_for_sha_loader_spec.rb | 2 +- .../graphql/mutations/merge_requests/set_wip_spec.rb | 2 +- spec/support/helpers/graphql_helpers.rb | 18 +++++++++++++++--- .../graphql/notes_on_noteables_shared_examples.rb | 2 +- 28 files changed, 74 insertions(+), 58 deletions(-) diff --git a/Gemfile b/Gemfile index bc573434063..6bfb873c159 100644 --- a/Gemfile +++ b/Gemfile @@ -83,7 +83,7 @@ gem 'grape-entity', '~> 0.7.1' gem 'rack-cors', '~> 1.0.0', require: 'rack/cors' # GraphQL API -gem 'graphql', '= 1.8.4' +gem 'graphql', '= 1.8.17' gem 'graphiql-rails', '~> 1.4.10' gem 'apollo_upload_server', '~> 2.0.0.beta3' gem 'graphql-docs', '~> 1.6.0', group: [:development, :test] diff --git a/Gemfile.lock b/Gemfile.lock index f0b3d722326..451348af280 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -377,7 +377,7 @@ GEM graphiql-rails (1.4.10) railties sprockets-rails - graphql (1.8.4) + graphql (1.8.17) graphql-docs (1.6.0) commonmarker (~> 0.16) escape_utils (~> 1.2) @@ -1110,7 +1110,7 @@ DEPENDENCIES grape-path-helpers (~> 1.1) grape_logging (~> 1.7) graphiql-rails (~> 1.4.10) - graphql (= 1.8.4) + graphql (= 1.8.17) graphql-docs (~> 1.6.0) grpc (~> 1.19.0) haml_lint (~> 0.31.0) diff --git a/app/graphql/mutations/notes/create/base.rb b/app/graphql/mutations/notes/create/base.rb index d3a5dae2188..cf9f74a63d8 100644 --- a/app/graphql/mutations/notes/create/base.rb +++ b/app/graphql/mutations/notes/create/base.rb @@ -18,8 +18,6 @@ module Mutations required: true, description: copy_field_description(Types::Notes::NoteType, :body) - private - def resolve(args) noteable = authorized_find!(id: args[:noteable_id]) @@ -37,6 +35,8 @@ module Mutations } end + private + def create_note_params(noteable, args) { noteable: noteable, diff --git a/app/graphql/resolvers/full_path_resolver.rb b/app/graphql/resolvers/full_path_resolver.rb index 972f318c806..2afd0411ea6 100644 --- a/app/graphql/resolvers/full_path_resolver.rb +++ b/app/graphql/resolvers/full_path_resolver.rb @@ -11,7 +11,7 @@ module Resolvers end def model_by_full_path(model, full_path) - BatchLoader.for(full_path).batch(key: model) do |full_paths, loader, args| + BatchLoader::GraphQL.for(full_path).batch(key: model) do |full_paths, loader, args| # `with_route` avoids an N+1 calculating full_path args[:key].where_full_path_in(full_paths).with_route.each do |model_instance| loader.call(model_instance.full_path, model_instance) diff --git a/app/graphql/resolvers/issues_resolver.rb b/app/graphql/resolvers/issues_resolver.rb index 6988b451ec3..dd104e83f43 100644 --- a/app/graphql/resolvers/issues_resolver.rb +++ b/app/graphql/resolvers/issues_resolver.rb @@ -41,13 +41,11 @@ module Resolvers type Types::IssueType, null: true - alias_method :project, :object - def resolve(**args) # The project could have been loaded in batch by `BatchLoader`. # At this point we need the `id` of the project to query for issues, so # make sure it's loaded and not `nil` before continuing. - project.sync if project.respond_to?(:sync) + project = object.respond_to?(:sync) ? object.sync : object return Issue.none if project.nil? # Will need to be be made group & namespace aware with diff --git a/app/graphql/resolvers/merge_requests_resolver.rb b/app/graphql/resolvers/merge_requests_resolver.rb index b84e60066e1..1740d614b69 100644 --- a/app/graphql/resolvers/merge_requests_resolver.rb +++ b/app/graphql/resolvers/merge_requests_resolver.rb @@ -25,8 +25,10 @@ module Resolvers # rubocop: disable CodeReuse/ActiveRecord def batch_load(iid) - BatchLoader.for(iid.to_s).batch(key: project) do |iids, loader, args| - args[:key].merge_requests.where(iid: iids).each do |mr| + BatchLoader::GraphQL.for(iid.to_s).batch(key: project) do |iids, loader, args| + arg_key = args[:key].respond_to?(:sync) ? args[:key].sync : args[:key] + + arg_key.merge_requests.where(iid: iids).each do |mr| loader.call(mr.iid.to_s, mr) end end diff --git a/app/graphql/resolvers/namespace_projects_resolver.rb b/app/graphql/resolvers/namespace_projects_resolver.rb index 677ea808aeb..f5b60f91be6 100644 --- a/app/graphql/resolvers/namespace_projects_resolver.rb +++ b/app/graphql/resolvers/namespace_projects_resolver.rb @@ -9,13 +9,11 @@ module Resolvers type Types::ProjectType, null: true - alias_method :namespace, :object - def resolve(include_subgroups:) # The namespace could have been loaded in batch by `BatchLoader`. # At this point we need the `id` or the `full_path` of the namespace # to query for projects, so make sure it's loaded and not `nil` before continuing. - namespace.sync if namespace.respond_to?(:sync) + namespace = object.respond_to?(:sync) ? object.sync : object return Project.none if namespace.nil? if include_subgroups diff --git a/lib/gitlab/graphql/authorize/authorize_field_service.rb b/lib/gitlab/graphql/authorize/authorize_field_service.rb index 3b5dde2fde5..0b11ea2f608 100644 --- a/lib/gitlab/graphql/authorize/authorize_field_service.rb +++ b/lib/gitlab/graphql/authorize/authorize_field_service.rb @@ -54,14 +54,14 @@ module Gitlab # The field is a built-in/scalar type, or a list of scalars # authorize using the parent's object parent_typed_object.object - elsif resolved_type.respond_to?(:object) - # The field is a type representing a single object, we'll authorize - # against the object directly - resolved_type.object elsif @field.connection? || resolved_type.is_a?(Array) # The field is a connection or a list of non-built-in types, we'll # authorize each element when rendering nil + elsif resolved_type.respond_to?(:object) + # The field is a type representing a single object, we'll authorize + # against the object directly + resolved_type.object else # Resolved type is a single object that might not be loaded yet by # the batchloader, we'll authorize that diff --git a/lib/gitlab/graphql/authorize/authorize_resource.rb b/lib/gitlab/graphql/authorize/authorize_resource.rb index ef5caaf5b0e..6844367454f 100644 --- a/lib/gitlab/graphql/authorize/authorize_resource.rb +++ b/lib/gitlab/graphql/authorize/authorize_resource.rb @@ -29,19 +29,25 @@ module Gitlab def authorized_find!(*args) object = find_object(*args) + object = object.sync if object.respond_to?(:sync) + authorize!(object) object end def authorize!(object) - unless authorized?(object) + unless authorized_resource?(object) raise Gitlab::Graphql::Errors::ResourceNotAvailable, "The resource that you are attempting to access does not exist or you don't have permission to perform this action" end end - def authorized?(object) + # this was named `#authorized?`, however it conflicts with the native + # graphql gem version + # TODO consider adopting the gem's built in authorization system + # https://gitlab.com/gitlab-org/gitlab-ee/issues/13984 + def authorized_resource?(object) # Sanity check. We don't want to accidentally allow a developer to authorize # without first adding permissions to authorize against if self.class.required_permissions.empty? diff --git a/lib/gitlab/graphql/loaders/batch_lfs_oid_loader.rb b/lib/gitlab/graphql/loaders/batch_lfs_oid_loader.rb index 8f34e58a771..67511c124e4 100644 --- a/lib/gitlab/graphql/loaders/batch_lfs_oid_loader.rb +++ b/lib/gitlab/graphql/loaders/batch_lfs_oid_loader.rb @@ -9,7 +9,7 @@ module Gitlab end def find - BatchLoader.for(blob_id).batch(key: repository) do |blob_ids, loader, batch_args| + BatchLoader::GraphQL.for(blob_id).batch(key: repository) do |blob_ids, loader, batch_args| Gitlab::Git::Blob.batch_lfs_pointers(batch_args[:key], blob_ids).each do |loaded_blob| loader.call(loaded_blob.id, loaded_blob.lfs_oid) end diff --git a/lib/gitlab/graphql/loaders/batch_model_loader.rb b/lib/gitlab/graphql/loaders/batch_model_loader.rb index 50d3293fcbb..164fe74148c 100644 --- a/lib/gitlab/graphql/loaders/batch_model_loader.rb +++ b/lib/gitlab/graphql/loaders/batch_model_loader.rb @@ -12,7 +12,7 @@ module Gitlab # rubocop: disable CodeReuse/ActiveRecord def find - BatchLoader.for({ model: model_class, id: model_id.to_i }).batch do |loader_info, loader| + BatchLoader::GraphQL.for({ model: model_class, id: model_id.to_i }).batch do |loader_info, loader| per_model = loader_info.group_by { |info| info[:model] } per_model.each do |model, info| ids = info.map { |i| i[:id] } diff --git a/lib/gitlab/graphql/loaders/batch_project_statistics_loader.rb b/lib/gitlab/graphql/loaders/batch_project_statistics_loader.rb index 5e151f4dbd7..449f4160a6c 100644 --- a/lib/gitlab/graphql/loaders/batch_project_statistics_loader.rb +++ b/lib/gitlab/graphql/loaders/batch_project_statistics_loader.rb @@ -11,7 +11,7 @@ module Gitlab end def find - BatchLoader.for(project_id).batch do |project_ids, loader| + BatchLoader::GraphQL.for(project_id).batch do |project_ids, loader| ProjectStatistics.for_project_ids(project_ids).each do |statistics| loader.call(statistics.project_id, statistics) end diff --git a/lib/gitlab/graphql/loaders/batch_root_storage_statistics_loader.rb b/lib/gitlab/graphql/loaders/batch_root_storage_statistics_loader.rb index a0312366d66..366aa74d435 100644 --- a/lib/gitlab/graphql/loaders/batch_root_storage_statistics_loader.rb +++ b/lib/gitlab/graphql/loaders/batch_root_storage_statistics_loader.rb @@ -11,7 +11,7 @@ module Gitlab end def find - BatchLoader.for(namespace_id).batch do |namespace_ids, loader| + BatchLoader::GraphQL.for(namespace_id).batch do |namespace_ids, loader| Namespace::RootStorageStatistics.for_namespace_ids(namespace_ids).each do |statistics| loader.call(statistics.namespace_id, statistics) end diff --git a/lib/gitlab/graphql/loaders/pipeline_for_sha_loader.rb b/lib/gitlab/graphql/loaders/pipeline_for_sha_loader.rb index 81c5cabf451..70344392138 100644 --- a/lib/gitlab/graphql/loaders/pipeline_for_sha_loader.rb +++ b/lib/gitlab/graphql/loaders/pipeline_for_sha_loader.rb @@ -11,7 +11,7 @@ module Gitlab end def find_last - BatchLoader.for(sha).batch(key: project) do |shas, loader, args| + BatchLoader::GraphQL.for(sha).batch(key: project) do |shas, loader, args| pipelines = args[:key].ci_pipelines.latest_for_shas(shas) pipelines.each do |pipeline| diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb index dec6b23d72a..0a27bbecfef 100644 --- a/spec/graphql/gitlab_schema_spec.rb +++ b/spec/graphql/gitlab_schema_spec.rb @@ -129,7 +129,7 @@ describe GitlabSchema do result = described_class.object_from_id(user.to_global_id.to_s) - expect(result.__sync).to eq(user) + expect(result.sync).to eq(user) end it 'batchloads the queries' do @@ -138,7 +138,7 @@ describe GitlabSchema do expect do [described_class.object_from_id(user1.to_global_id), - described_class.object_from_id(user2.to_global_id)].map(&:__sync) + described_class.object_from_id(user2.to_global_id)].map(&:sync) end.not_to exceed_query_limit(1) end end @@ -149,7 +149,7 @@ describe GitlabSchema do result = described_class.object_from_id(note.to_global_id) - expect(result.__sync).to eq(note) + expect(result.sync).to eq(note) end it 'batchloads the queries' do @@ -158,7 +158,7 @@ describe GitlabSchema do expect do [described_class.object_from_id(note1.to_global_id), - described_class.object_from_id(note2.to_global_id)].map(&:__sync) + described_class.object_from_id(note2.to_global_id)].map(&:sync) end.not_to exceed_query_limit(1) end end diff --git a/spec/graphql/mutations/concerns/mutations/resolves_project_spec.rb b/spec/graphql/mutations/concerns/mutations/resolves_project_spec.rb index 19f5a8907a2..aa0f5c55902 100644 --- a/spec/graphql/mutations/concerns/mutations/resolves_project_spec.rb +++ b/spec/graphql/mutations/concerns/mutations/resolves_project_spec.rb @@ -14,6 +14,6 @@ describe Mutations::ResolvesProject do project = create(:project) expect(Resolvers::ProjectResolver).to receive(:new).with(object: nil, context: context).and_call_original - expect(mutation.resolve_project(full_path: project.full_path)).to eq(project) + expect(mutation.resolve_project(full_path: project.full_path).sync).to eq(project) end end diff --git a/spec/graphql/resolvers/group_resolver_spec.rb b/spec/graphql/resolvers/group_resolver_spec.rb index 5eb9cd06d15..7dec9ac1aa5 100644 --- a/spec/graphql/resolvers/group_resolver_spec.rb +++ b/spec/graphql/resolvers/group_resolver_spec.rb @@ -12,7 +12,7 @@ describe Resolvers::GroupResolver do it 'batch-resolves groups by full path' do paths = [group1.full_path, group2.full_path] - result = batch(max_queries: 1) do + result = batch_sync(max_queries: 1) do paths.map { |path| resolve_group(path) } end @@ -20,7 +20,7 @@ describe Resolvers::GroupResolver do end it 'resolves an unknown full_path to nil' do - result = batch { resolve_group('unknown/project') } + result = batch_sync { resolve_group('unknown/project') } expect(result).to be_nil end diff --git a/spec/graphql/resolvers/issues_resolver_spec.rb b/spec/graphql/resolvers/issues_resolver_spec.rb index 798fe00de97..d122c081069 100644 --- a/spec/graphql/resolvers/issues_resolver_spec.rb +++ b/spec/graphql/resolvers/issues_resolver_spec.rb @@ -110,7 +110,7 @@ describe Resolvers::IssuesResolver do context "when passing a non existent, batch loaded project" do let(:project) do - BatchLoader.for("non-existent-path").batch do |_fake_paths, loader, _| + BatchLoader::GraphQL.for("non-existent-path").batch do |_fake_paths, loader, _| loader.call("non-existent-path", nil) end end diff --git a/spec/graphql/resolvers/merge_requests_resolver_spec.rb b/spec/graphql/resolvers/merge_requests_resolver_spec.rb index ab3c426b2cd..97b8e5ed41c 100644 --- a/spec/graphql/resolvers/merge_requests_resolver_spec.rb +++ b/spec/graphql/resolvers/merge_requests_resolver_spec.rb @@ -17,7 +17,7 @@ describe Resolvers::MergeRequestsResolver do describe '#resolve' do it 'batch-resolves by target project full path and individual IID' do - result = batch(max_queries: 2) do + result = batch_sync(max_queries: 2) do resolve_mr(project, iid: iid_1) + resolve_mr(project, iid: iid_2) end @@ -25,7 +25,7 @@ describe Resolvers::MergeRequestsResolver do end it 'batch-resolves by target project full path and IIDS' do - result = batch(max_queries: 2) do + result = batch_sync(max_queries: 2) do resolve_mr(project, iids: [iid_1, iid_2]) end @@ -33,7 +33,7 @@ describe Resolvers::MergeRequestsResolver do end it 'can batch-resolve merge requests from different projects' do - result = batch(max_queries: 3) do + result = batch_sync(max_queries: 3) do resolve_mr(project, iid: iid_1) + resolve_mr(project, iid: iid_2) + resolve_mr(other_project, iid: other_iid) @@ -43,13 +43,13 @@ describe Resolvers::MergeRequestsResolver do end it 'resolves an unknown iid to be empty' do - result = batch { resolve_mr(project, iid: -1) } + result = batch_sync { resolve_mr(project, iid: -1) } - expect(result).to be_empty + expect(result.compact).to be_empty end it 'resolves empty iids to be empty' do - result = batch { resolve_mr(project, iids: []) } + result = batch_sync { resolve_mr(project, iids: []) } expect(result).to be_empty end diff --git a/spec/graphql/resolvers/namespace_projects_resolver_spec.rb b/spec/graphql/resolvers/namespace_projects_resolver_spec.rb index 47591445fc0..639cc69650b 100644 --- a/spec/graphql/resolvers/namespace_projects_resolver_spec.rb +++ b/spec/graphql/resolvers/namespace_projects_resolver_spec.rb @@ -46,7 +46,7 @@ describe Resolvers::NamespaceProjectsResolver do context "when passing a non existent, batch loaded namespace" do let(:namespace) do - BatchLoader.for("non-existent-path").batch do |_fake_paths, loader, _| + BatchLoader::GraphQL.for("non-existent-path").batch do |_fake_paths, loader, _| loader.call("non-existent-path", nil) end end diff --git a/spec/graphql/resolvers/project_resolver_spec.rb b/spec/graphql/resolvers/project_resolver_spec.rb index 4fdbb3aa43e..d0fc2957909 100644 --- a/spec/graphql/resolvers/project_resolver_spec.rb +++ b/spec/graphql/resolvers/project_resolver_spec.rb @@ -12,7 +12,7 @@ describe Resolvers::ProjectResolver do it 'batch-resolves projects by full path' do paths = [project1.full_path, project2.full_path] - result = batch(max_queries: 1) do + result = batch_sync(max_queries: 1) do paths.map { |path| resolve_project(path) } end @@ -20,7 +20,7 @@ describe Resolvers::ProjectResolver do end it 'resolves an unknown full_path to nil' do - result = batch { resolve_project('unknown/project') } + result = batch_sync { resolve_project('unknown/project') } expect(result).to be_nil end diff --git a/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb b/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb index 50138d272c4..23762666ba8 100644 --- a/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb +++ b/spec/lib/gitlab/graphql/authorize/authorize_resource_spec.rb @@ -46,9 +46,9 @@ describe Gitlab::Graphql::Authorize::AuthorizeResource do end end - describe '#authorized?' do + describe '#authorized_resource?' do it 'is true' do - expect(loading_resource.authorized?(project)).to be(true) + expect(loading_resource.authorized_resource?(project)).to be(true) end end end @@ -72,9 +72,9 @@ describe Gitlab::Graphql::Authorize::AuthorizeResource do end end - describe '#authorized?' do + describe '#authorized_resource?' do it 'is false' do - expect(loading_resource.authorized?(project)).to be(false) + expect(loading_resource.authorized_resource?(project)).to be(false) end end end @@ -121,9 +121,9 @@ describe Gitlab::Graphql::Authorize::AuthorizeResource do end end - describe '#authorized?' do + describe '#authorized_resource?' do it 'raises a comprehensive error message' do - expect { loading_resource.authorized?(project) }.to raise_error(error) + expect { loading_resource.authorized_resource?(project) }.to raise_error(error) end end end diff --git a/spec/lib/gitlab/graphql/loaders/batch_lfs_oid_loader_spec.rb b/spec/lib/gitlab/graphql/loaders/batch_lfs_oid_loader_spec.rb index 46dd1777285..22d8aa4274a 100644 --- a/spec/lib/gitlab/graphql/loaders/batch_lfs_oid_loader_spec.rb +++ b/spec/lib/gitlab/graphql/loaders/batch_lfs_oid_loader_spec.rb @@ -12,7 +12,7 @@ describe Gitlab::Graphql::Loaders::BatchLfsOidLoader do it 'batch-resolves LFS blob IDs' do expect(Gitlab::Git::Blob).to receive(:batch_lfs_pointers).once.and_call_original - result = batch do + result = batch_sync do [blob, otherblob].map { |b| described_class.new(repository, b.id).find } end diff --git a/spec/lib/gitlab/graphql/loaders/batch_model_loader_spec.rb b/spec/lib/gitlab/graphql/loaders/batch_model_loader_spec.rb index 4609593ef6a..a4bbd868558 100644 --- a/spec/lib/gitlab/graphql/loaders/batch_model_loader_spec.rb +++ b/spec/lib/gitlab/graphql/loaders/batch_model_loader_spec.rb @@ -9,8 +9,8 @@ describe Gitlab::Graphql::Loaders::BatchModelLoader do issue_result = described_class.new(Issue, issue.id).find user_result = described_class.new(User, user.id).find - expect(issue_result.__sync).to eq(issue) - expect(user_result.__sync).to eq(user) + expect(issue_result.sync).to eq(issue) + expect(user_result.sync).to eq(user) end it 'only queries once per model' do @@ -21,7 +21,7 @@ describe Gitlab::Graphql::Loaders::BatchModelLoader do expect do [described_class.new(User, other_user.id).find, described_class.new(User, user.id).find, - described_class.new(Issue, issue.id).find].map(&:__sync) + described_class.new(Issue, issue.id).find].map(&:sync) end.not_to exceed_query_limit(2) end end diff --git a/spec/lib/gitlab/graphql/loaders/pipeline_for_sha_loader_spec.rb b/spec/lib/gitlab/graphql/loaders/pipeline_for_sha_loader_spec.rb index 927476cc655..136027736c3 100644 --- a/spec/lib/gitlab/graphql/loaders/pipeline_for_sha_loader_spec.rb +++ b/spec/lib/gitlab/graphql/loaders/pipeline_for_sha_loader_spec.rb @@ -10,7 +10,7 @@ describe Gitlab::Graphql::Loaders::PipelineForShaLoader do pipeline2 = create(:ci_pipeline, project: project, ref: project.default_branch, sha: project.commit.sha) pipeline3 = create(:ci_pipeline, project: project, ref: 'improve/awesome', sha: project.commit('improve/awesome').sha) - result = batch(max_queries: 1) do + result = batch_sync(max_queries: 1) do [pipeline1.sha, pipeline3.sha].map { |sha| described_class.new(project, sha).find_last } end diff --git a/spec/requests/api/graphql/mutations/merge_requests/set_wip_spec.rb b/spec/requests/api/graphql/mutations/merge_requests/set_wip_spec.rb index d75f0df9fd3..3a8a2bae939 100644 --- a/spec/requests/api/graphql/mutations/merge_requests/set_wip_spec.rb +++ b/spec/requests/api/graphql/mutations/merge_requests/set_wip_spec.rb @@ -13,7 +13,7 @@ describe 'Setting WIP status of a merge request' do project_path: project.full_path, iid: merge_request.iid.to_s } - graphql_mutation(:merge_request_set_wip, variables.merge(input)) + graphql_mutation(:merge_request_set_wip, variables.merge(input), "clientMutationId\nerrors\nmergeRequest { id\ntitle }") end def mutation_response diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index d86371d70b9..beb346b2855 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -34,6 +34,14 @@ module GraphqlHelpers end end + # BatchLoader::GraphQL returns a wrapper, so we need to :sync in order + # to get the actual values + def batch_sync(max_queries: nil, &blk) + result = batch(max_queries: nil, &blk) + + result.is_a?(Array) ? result.map(&:sync) : result&.sync + end + def graphql_query_for(name, attributes = {}, fields = nil) <<~QUERY { @@ -114,7 +122,11 @@ module GraphqlHelpers FIELDS end - def all_graphql_fields_for(class_name, parent_types = Set.new) + def all_graphql_fields_for(class_name, parent_types = Set.new, max_depth: 3) + # pulling _all_ fields can generate a _huge_ query (like complexity 180,000), + # and significantly increase spec runtime. so limit the depth by default + return if max_depth <= 0 + allow_unlimited_graphql_complexity allow_unlimited_graphql_depth @@ -133,9 +145,9 @@ module GraphqlHelpers if nested_fields?(field) fields = - all_graphql_fields_for(singular_field_type, parent_types | [type]) + all_graphql_fields_for(singular_field_type, parent_types | [type], max_depth: max_depth - 1) - "#{name} { #{fields} }" + "#{name} { #{fields} }" unless fields.blank? else name end diff --git a/spec/support/shared_examples/graphql/notes_on_noteables_shared_examples.rb b/spec/support/shared_examples/graphql/notes_on_noteables_shared_examples.rb index 323d1c51ffd..9a60825855f 100644 --- a/spec/support/shared_examples/graphql/notes_on_noteables_shared_examples.rb +++ b/spec/support/shared_examples/graphql/notes_on_noteables_shared_examples.rb @@ -46,7 +46,7 @@ shared_context 'exposing regular notes on a noteable in GraphQL' do discussions { edges { node { - #{all_graphql_fields_for('Discussion')} + #{all_graphql_fields_for('Discussion', max_depth: 4)} } } } -- cgit v1.2.1