From 3e5fea2680137fa495656b8835836f2ab06be1c4 Mon Sep 17 00:00:00 2001 From: Roy Zwambag <rzwambag@gitlab.com> Date: Tue, 20 Jun 2023 23:25:55 +0000 Subject: [PATCH] Infer operation name when not explicitly given as param --- app/controllers/graphql_controller.rb | 21 +++++++++++++++++---- spec/controllers/graphql_controller_spec.rb | 7 +++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb index 3d3b7f31dfd0..f6a6648fd792 100644 --- a/app/controllers/graphql_controller.rb +++ b/app/controllers/graphql_controller.rb @@ -14,6 +14,7 @@ class GraphqlController < ApplicationController # The query string of a standard IntrospectionQuery, used to compare incoming requests for caching CACHED_INTROSPECTION_QUERY_STRING = CachedIntrospectionQuery.query_string + INTROSPECTION_QUERY_OPERATION_NAME = 'IntrospectionQuery' # If a user is using their session to access GraphQL, we need to have session # storage, since the admin-mode check is session wide. @@ -58,7 +59,7 @@ class GraphqlController < ApplicationController urgency :low, [:execute] def execute - result = if Feature.enabled?(:cache_introspection_query) && params[:operationName] == 'IntrospectionQuery' + result = if Feature.enabled?(:cache_introspection_query) && introspection_query? execute_introspection_query else multiplex? ? execute_multiplex : execute_query @@ -294,9 +295,7 @@ def execute_introspection_query end def introspection_query_can_use_cache? - graphql_query = GraphQL::Query.new(GitlabSchema, query: query, variables: build_variables(params[:variables])) - - CACHED_INTROSPECTION_QUERY_STRING == graphql_query.query_string.squish + CACHED_INTROSPECTION_QUERY_STRING == graphql_query_object.query_string.squish end def introspection_query_cache_key @@ -306,6 +305,15 @@ def introspection_query_cache_key ['introspection-query-cache', Gitlab.revision, context[:remove_deprecated]] end + def introspection_query? + if params.key?(:operationName) + params[:operationName] == INTROSPECTION_QUERY_OPERATION_NAME + else + # If we don't provide operationName param, we infer it from the query + graphql_query_object.selected_operation_name == INTROSPECTION_QUERY_OPERATION_NAME + end + end + def log_introspection_query_cache_details(can_use_introspection_query_cache) Gitlab::AppLogger.info( message: "IntrospectionQueryCache", @@ -315,4 +323,9 @@ def log_introspection_query_cache_details(can_use_introspection_query_cache) introspection_query_cache_key: introspection_query_cache_key.to_s ) end + + def graphql_query_object + @graphql_query_object ||= GraphQL::Query.new(GitlabSchema, query: query, + variables: build_variables(params[:variables])) + end end diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb index b4a7e41ccd26..da1676138ec6 100644 --- a/spec/controllers/graphql_controller_spec.rb +++ b/spec/controllers/graphql_controller_spec.rb @@ -431,6 +431,13 @@ context 'when querying an IntrospectionQuery', :use_clean_rails_memory_store_caching do let_it_be(:query) { File.read(Rails.root.join('spec/fixtures/api/graphql/introspection.graphql')) } + it 'caches IntrospectionQuery even when operationName is not given' do + expect(GitlabSchema).to receive(:execute).exactly(:once) + + post :execute, params: { query: query } + post :execute, params: { query: query } + end + it 'caches the IntrospectionQuery' do expect(GitlabSchema).to receive(:execute).exactly(:once) -- GitLab