From ec2b4bb65da6f5b6a5ab813349049806c1b3851e Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Wed, 4 Sep 2019 21:57:37 +0000 Subject: Upgrade graphql gem to 1.9.10 - `edge_nodes` needs to get called on the object - added `include GlobalID::Identification` in a couple places - renamed `object` to `item` in spec due to conflict --- Gemfile | 2 +- Gemfile.lock | 4 ++-- .../graphql/authorize/authorize_field_service.rb | 4 ++-- .../graphql/representation/submodule_tree_entry.rb | 2 ++ lib/gitlab/graphql/representation/tree_entry.rb | 2 ++ spec/graphql/features/authorization_spec.rb | 28 +++++++++++----------- .../authorize/authorize_field_service_spec.rb | 3 ++- .../graphql/connections/keyset_connection_spec.rb | 2 +- spec/requests/api/graphql/gitlab_schema_spec.rb | 2 +- .../mutations/merge_requests/set_wip_spec.rb | 11 ++++++++- 10 files changed, 37 insertions(+), 23 deletions(-) diff --git a/Gemfile b/Gemfile index 1d4e87ee221..3e3a8c5dde2 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.17' +gem 'graphql', '~> 1.9.11' 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 f0370798374..6add217bc32 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -377,7 +377,7 @@ GEM graphiql-rails (1.4.10) railties sprockets-rails - graphql (1.8.17) + graphql (1.9.11) 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.17) + graphql (~> 1.9.11) graphql-docs (~> 1.6.0) grpc (~> 1.19.0) haml_lint (~> 0.31.0) diff --git a/lib/gitlab/graphql/authorize/authorize_field_service.rb b/lib/gitlab/graphql/authorize/authorize_field_service.rb index 0b11ea2f608..c7f430490d6 100644 --- a/lib/gitlab/graphql/authorize/authorize_field_service.rb +++ b/lib/gitlab/graphql/authorize/authorize_field_service.rb @@ -74,9 +74,9 @@ module Gitlab # Authorizing fields representing scalars, or a simple field with an object resolved_type if allowed_access?(current_user, authorizing_object) elsif @field.connection? - # A connection with pagination, modify the visible nodes in on the + # A connection with pagination, modify the visible nodes on the # connection type in place - resolved_type.edge_nodes.to_a.keep_if { |node| allowed_access?(current_user, node) } + resolved_type.object.edge_nodes.to_a.keep_if { |node| allowed_access?(current_user, node) } resolved_type elsif resolved_type.is_a? Array # A simple list of rendered types each object being an object to authorize diff --git a/lib/gitlab/graphql/representation/submodule_tree_entry.rb b/lib/gitlab/graphql/representation/submodule_tree_entry.rb index 65716dff75d..8d17cb9eecc 100644 --- a/lib/gitlab/graphql/representation/submodule_tree_entry.rb +++ b/lib/gitlab/graphql/representation/submodule_tree_entry.rb @@ -4,6 +4,8 @@ module Gitlab module Graphql module Representation class SubmoduleTreeEntry < SimpleDelegator + include GlobalID::Identification + class << self def decorate(submodules, tree) repository = tree.repository diff --git a/lib/gitlab/graphql/representation/tree_entry.rb b/lib/gitlab/graphql/representation/tree_entry.rb index 7ea83db5876..7e347081a9b 100644 --- a/lib/gitlab/graphql/representation/tree_entry.rb +++ b/lib/gitlab/graphql/representation/tree_entry.rb @@ -4,6 +4,8 @@ module Gitlab module Graphql module Representation class TreeEntry < SimpleDelegator + include GlobalID::Identification + class << self def decorate(entries, repository) return if entries.nil? diff --git a/spec/graphql/features/authorization_spec.rb b/spec/graphql/features/authorization_spec.rb index c427893f9cc..9a60ff3b78c 100644 --- a/spec/graphql/features/authorization_spec.rb +++ b/spec/graphql/features/authorization_spec.rb @@ -8,10 +8,10 @@ describe 'Gitlab::Graphql::Authorization' do let(:permission_single) { :foo } let(:permission_collection) { [:foo, :bar] } let(:test_object) { double(name: 'My name') } - let(:query_string) { '{ object() { name } }' } + let(:query_string) { '{ item() { name } }' } let(:result) { execute_query(query_type)['data'] } - subject { result['object'] } + subject { result['item'] } shared_examples 'authorization with a single permission' do it 'returns the protected field when user has permission' do @@ -54,7 +54,7 @@ describe 'Gitlab::Graphql::Authorization' do describe 'with a single permission' do let(:query_type) do query_factory do |query| - query.field :object, type, null: true, resolve: ->(obj, args, ctx) { test_object }, authorize: permission_single + query.field :item, type, null: true, resolve: ->(obj, args, ctx) { test_object }, authorize: permission_single end end @@ -65,7 +65,7 @@ describe 'Gitlab::Graphql::Authorization' do let(:query_type) do permissions = permission_collection query_factory do |qt| - qt.field :object, type, null: true, resolve: ->(obj, args, ctx) { test_object } do + qt.field :item, type, null: true, resolve: ->(obj, args, ctx) { test_object } do authorize permissions end end @@ -78,7 +78,7 @@ describe 'Gitlab::Graphql::Authorization' do describe 'Field authorizations when field is a built in type' do let(:query_type) do query_factory do |query| - query.field :object, type, null: true, resolve: ->(obj, args, ctx) { test_object } + query.field :item, type, null: true, resolve: ->(obj, args, ctx) { test_object } end end @@ -131,7 +131,7 @@ describe 'Gitlab::Graphql::Authorization' do describe 'Type authorizations' do let(:query_type) do query_factory do |query| - query.field :object, type, null: true, resolve: ->(obj, args, ctx) { test_object } + query.field :item, type, null: true, resolve: ->(obj, args, ctx) { test_object } end end @@ -168,7 +168,7 @@ describe 'Gitlab::Graphql::Authorization' do let(:query_type) do query_factory do |query| - query.field :object, type, null: true, resolve: ->(obj, args, ctx) { test_object }, authorize: permission_2 + query.field :item, type, null: true, resolve: ->(obj, args, ctx) { test_object }, authorize: permission_2 end end @@ -176,7 +176,7 @@ describe 'Gitlab::Graphql::Authorization' do end describe 'type authorizations when applied to a relay connection' do - let(:query_string) { '{ object() { edges { node { name } } } }' } + let(:query_string) { '{ item() { edges { node { name } } } }' } let(:second_test_object) { double(name: 'Second thing') } let(:type) do @@ -187,11 +187,11 @@ describe 'Gitlab::Graphql::Authorization' do let(:query_type) do query_factory do |query| - query.field :object, type.connection_type, null: true, resolve: ->(obj, args, ctx) { [test_object, second_test_object] } + query.field :item, type.connection_type, null: true, resolve: ->(obj, args, ctx) { [test_object, second_test_object] } end end - subject { result.dig('object', 'edges') } + subject { result.dig('item', 'edges') } it 'returns only the elements visible to the user' do permit(permission_single) @@ -207,13 +207,13 @@ describe 'Gitlab::Graphql::Authorization' do describe 'limiting connections with multiple objects' do let(:query_type) do query_factory do |query| - query.field :object, type.connection_type, null: true, resolve: ->(obj, args, ctx) do + query.field :item, type.connection_type, null: true, resolve: ->(obj, args, ctx) do [test_object, second_test_object] end end end - let(:query_string) { '{ object(first: 1) { edges { node { name } } } }' } + let(:query_string) { '{ item(first: 1) { edges { node { name } } } }' } it 'only checks permissions for the first object' do expect(Ability).to receive(:allowed?).with(user, permission_single, test_object) { true } @@ -233,11 +233,11 @@ describe 'Gitlab::Graphql::Authorization' do let(:query_type) do query_factory do |query| - query.field :object, [type], null: true, resolve: ->(obj, args, ctx) { [test_object] } + query.field :item, [type], null: true, resolve: ->(obj, args, ctx) { [test_object] } end end - subject { result['object'].first } + subject { result['item'].first } include_examples 'authorization with a single permission' end diff --git a/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb b/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb index 7a7ae373058..aada9285b31 100644 --- a/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb +++ b/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb @@ -32,7 +32,8 @@ describe Gitlab::Graphql::Authorize::AuthorizeFieldService do let(:presented_type) { double('parent type', object: presented_object) } let(:query_type) { GraphQL::ObjectType.new } let(:schema) { GraphQL::Schema.define(query: query_type, mutation: nil)} - let(:context) { GraphQL::Query::Context.new(query: OpenStruct.new(schema: schema), values: { current_user: current_user }, object: nil) } + let(:query_context) { OpenStruct.new(schema: schema) } + let(:context) { GraphQL::Query::Context.new(query: OpenStruct.new(schema: schema, context: query_context), values: { current_user: current_user }, object: nil) } subject(:resolved) { service.authorized_resolve.call(presented_type, {}, context) } context 'scalar types' do diff --git a/spec/lib/gitlab/graphql/connections/keyset_connection_spec.rb b/spec/lib/gitlab/graphql/connections/keyset_connection_spec.rb index fefa2881b18..4eb121794e1 100644 --- a/spec/lib/gitlab/graphql/connections/keyset_connection_spec.rb +++ b/spec/lib/gitlab/graphql/connections/keyset_connection_spec.rb @@ -8,7 +8,7 @@ describe Gitlab::Graphql::Connections::KeysetConnection do end def encoded_property(value) - Base64.strict_encode64(value.to_s) + Base64Bp.urlsafe_encode64(value.to_s, padding: false) end describe '#cursor_from_nodes' do diff --git a/spec/requests/api/graphql/gitlab_schema_spec.rb b/spec/requests/api/graphql/gitlab_schema_spec.rb index 28676bb02f4..e1eb7c7f738 100644 --- a/spec/requests/api/graphql/gitlab_schema_spec.rb +++ b/spec/requests/api/graphql/gitlab_schema_spec.rb @@ -120,7 +120,7 @@ describe 'GitlabSchema configurations' do query_string: query, variables: {}.to_s, complexity: 181, - depth: 0, + depth: 13, duration: 7 } 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 3a8a2bae939..bbc477ba485 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,16 @@ 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), "clientMutationId\nerrors\nmergeRequest { id\ntitle }") + graphql_mutation(:merge_request_set_wip, variables.merge(input), + <<-QL.strip_heredoc + clientMutationId + errors + mergeRequest { + id + title + } + QL + ) end def mutation_response -- cgit v1.2.1