Merge branch 'ld-simplify-gitlab_schema-execute' into 'master'

Remove `GitlabSchema#execute`, and small tidy ups related to max limits

See merge request gitlab-org/gitlab!67590

app/graphql/gitlab_schema.rb

@@ -3,9 +3,9 @@
 class GitlabSchema < GraphQL::Schema
   # Currently an IntrospectionQuery has a complexity of 179.
   # These values will evolve over time.
-  DEFAULT_MAX_COMPLEXITY   = 200
-  AUTHENTICATED_COMPLEXITY = 250
-  ADMIN_COMPLEXITY         = 300
+  DEFAULT_MAX_COMPLEXITY = 200
+  AUTHENTICATED_MAX_COMPLEXITY = 250
+  ADMIN_MAX_COMPLEXITY = 300
 
   DEFAULT_MAX_DEPTH = 15
   AUTHENTICATED_MAX_DEPTH = 20
@@ -20,9 +20,6 @@ class GitlabSchema < GraphQL::Schema
   query_analyzer Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer.new
   query_analyzer Gitlab::Graphql::QueryAnalyzers::RecursionAnalyzer.new
 
-  max_complexity DEFAULT_MAX_COMPLEXITY
-  max_depth DEFAULT_MAX_DEPTH
-
   query Types::QueryType
   mutation Types::MutationType
   subscription Types::SubscriptionType
@@ -36,20 +33,13 @@ class GitlabSchema < GraphQL::Schema
       kwargs[:max_complexity] ||= max_query_complexity(kwargs[:context]) unless kwargs.key?(:max_complexity)
 
       queries.each do |query|
-        query[:max_complexity] ||= max_query_complexity(kwargs[:context]) unless query.key?(:max_complexity)
-        query[:max_depth] = max_query_depth(kwargs[:context])
+        query[:max_complexity] ||= max_query_complexity(query[:context]) unless query.key?(:max_complexity)
+        query[:max_depth] = max_query_depth(query[:context]) unless query.key?(:max_depth)
       end
 
       super(queries, **kwargs)
     end
 
-    def execute(query_str = nil, **kwargs)
-      kwargs[:max_complexity] ||= max_query_complexity(kwargs[:context])
-      kwargs[:max_depth] ||= max_query_depth(kwargs[:context])
-
-      super(query_str, **kwargs)
-    end
-
     def get_type(type_name)
       type_name = Gitlab::GlobalId::Deprecations.apply_to_graphql_name(type_name)
 
@@ -142,9 +132,9 @@ class GitlabSchema < GraphQL::Schema
       current_user = ctx&.fetch(:current_user, nil)
 
       if current_user&.admin
-        ADMIN_COMPLEXITY
+        ADMIN_MAX_COMPLEXITY
       elsif current_user
-        AUTHENTICATED_COMPLEXITY
+        AUTHENTICATED_MAX_COMPLEXITY
       else
         DEFAULT_MAX_COMPLEXITY
       end

app/graphql/types/query_type.rb

@@ -129,7 +129,7 @@ module Types
           description: "Find runners visible to the current user.",
           feature_flag: :runner_graphql_query
 
-    field :ci_config, resolver: Resolvers::Ci::ConfigResolver, complexity: 126 # AUTHENTICATED_COMPLEXITY / 2 + 1
+    field :ci_config, resolver: Resolvers::Ci::ConfigResolver, complexity: 126 # AUTHENTICATED_MAX_COMPLEXITY / 2 + 1
 
     def design_management
       DesignManagementObject.new(nil)

lib/tasks/gitlab/graphql.rake

@@ -56,9 +56,9 @@ namespace :gitlab do
           color = case complexity
                   when 0..GitlabSchema::DEFAULT_MAX_COMPLEXITY
                     :green
-                  when GitlabSchema::DEFAULT_MAX_COMPLEXITY..GitlabSchema::AUTHENTICATED_COMPLEXITY
+                  when GitlabSchema::DEFAULT_MAX_COMPLEXITY..GitlabSchema::AUTHENTICATED_MAX_COMPLEXITY
                     :yellow
-                  when GitlabSchema::AUTHENTICATED_COMPLEXITY..GitlabSchema::ADMIN_COMPLEXITY
+                  when GitlabSchema::AUTHENTICATED_MAX_COMPLEXITY..GitlabSchema::ADMIN_MAX_COMPLEXITY
                     :orange
                   else
                     :red

spec/deprecation_toolkit_env.rb

@@ -56,6 +56,7 @@ module DeprecationToolkitEnv
   def self.allowed_kwarg_warning_paths
     %w[
       actionpack-6.1.3.2/lib/action_dispatch/routing/route_set.rb
+      graphql-1.11.8/lib/graphql/schema.rb
     ]
   end
 

spec/graphql/gitlab_schema_spec.rb

@@ -36,75 +36,66 @@ RSpec.describe GitlabSchema do
   end
 
   describe '.execute' do
-    context 'with different types of users' do
-      context 'when no context' do
-        it 'returns DEFAULT_MAX_COMPLEXITY' do
-          expect(GraphQL::Schema)
-            .to receive(:execute)
-            .with('query', hash_including(max_complexity: GitlabSchema::DEFAULT_MAX_COMPLEXITY))
-
-          described_class.execute('query')
+    describe 'setting query `max_complexity` and `max_depth`' do
+      subject(:result) { described_class.execute('query', kwargs).query }
+
+      shared_examples 'sets default limits' do
+        specify do
+          expect(result).to have_attributes(
+            max_complexity: GitlabSchema::DEFAULT_MAX_COMPLEXITY,
+            max_depth: GitlabSchema::DEFAULT_MAX_DEPTH
+          )
         end
       end
 
-      context 'when no user' do
-        it 'returns DEFAULT_MAX_COMPLEXITY' do
-          expect(GraphQL::Schema)
-            .to receive(:execute)
-            .with('query', hash_including(max_complexity: GitlabSchema::DEFAULT_MAX_COMPLEXITY))
+      context 'with no context' do
+        let(:kwargs) { {} }
 
-          described_class.execute('query', context: {})
-        end
-
-        it 'returns DEFAULT_MAX_DEPTH' do
-          expect(GraphQL::Schema)
-            .to receive(:execute)
-            .with('query', hash_including(max_depth: GitlabSchema::DEFAULT_MAX_DEPTH))
-
-          described_class.execute('query', context: {})
-        end
+        include_examples 'sets default limits'
       end
 
-      context 'when a logged in user' do
-        it 'returns AUTHENTICATED_COMPLEXITY' do
-          expect(GraphQL::Schema).to receive(:execute)
-            .with('query', hash_including(max_complexity: GitlabSchema::AUTHENTICATED_COMPLEXITY))
+      context 'with no :current_user' do
+        let(:kwargs) { { context: {} } }
 
-          described_class.execute('query', context: { current_user: user })
-        end
+        include_examples 'sets default limits'
+      end
 
-        it 'returns AUTHENTICATED_MAX_DEPTH' do
-          expect(GraphQL::Schema).to receive(:execute)
-            .with('query', hash_including(max_depth: GitlabSchema::AUTHENTICATED_MAX_DEPTH))
+      context 'with anonymous user' do
+        let(:kwargs) { { context: { current_user: nil } } }
 
-          described_class.execute('query', context: { current_user: user })
-        end
+        include_examples 'sets default limits'
       end
 
-      context 'when an admin user' do
-        it 'returns ADMIN_COMPLEXITY' do
-          user = build :user, :admin
-
-          expect(GraphQL::Schema).to receive(:execute)
-            .with('query', hash_including(max_complexity: GitlabSchema::ADMIN_COMPLEXITY))
+      context 'with a logged in user' do
+        let(:kwargs) { { context: { current_user: user } } }
 
-          described_class.execute('query', context: { current_user: user })
+        it 'sets authenticated user limits' do
+          expect(result).to have_attributes(
+            max_complexity: GitlabSchema::AUTHENTICATED_MAX_COMPLEXITY,
+            max_depth: GitlabSchema::AUTHENTICATED_MAX_DEPTH
+          )
         end
       end
 
-      context 'when max_complexity passed on the query' do
-        it 'returns what was passed on the query' do
-          expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_complexity: 1234))
+      context 'with an admin user' do
+        let(:kwargs) { { context: { current_user: build(:user, :admin) } } }
 
-          described_class.execute('query', max_complexity: 1234)
+        it 'sets admin/authenticated user limits' do
+          expect(result).to have_attributes(
+            max_complexity: GitlabSchema::ADMIN_MAX_COMPLEXITY,
+            max_depth: GitlabSchema::AUTHENTICATED_MAX_DEPTH
+          )
         end
       end
 
-      context 'when max_depth passed on the query' do
-        it 'returns what was passed on the query' do
-          expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_depth: 1234))
+      context 'when limits passed as kwargs' do
+        let(:kwargs) { { max_complexity: 1234, max_depth: 4321 } }
 
-          described_class.execute('query', max_depth: 1234)
+        it 'sets limits from the kwargs' do
+          expect(result).to have_attributes(
+            max_complexity: 1234,
+            max_depth: 4321
+          )
         end
       end
     end

spec/requests/api/graphql_spec.rb

@@ -385,7 +385,7 @@ RSpec.describe 'GraphQL' do
     context 'authenticated user' do
       subject { post_graphql(query, current_user: user) }
 
-      it 'does not raise an error as it uses the `AUTHENTICATED_COMPLEXITY`' do
+      it 'does not raise an error as it uses the `AUTHENTICATED_MAX_COMPLEXITY`' do
         subject
 
         expect(graphql_errors).to be_nil