Merge branch '292993-fix-n-1-query-for-vulnerabilitytype' into 'master'

Fix N+1 query in VulnerabilitiesResolver

See merge request gitlab-org/gitlab!72985

ee/app/graphql/ee/types/group_type.rb

@@ -45,6 +45,7 @@ module EE
         field :vulnerabilities,
               ::Types::VulnerabilityType.connection_type,
               null: true,
+              extras: [:lookahead],
               description: 'Vulnerabilities reported on the projects in the group and its subgroups.',
               resolver: ::Resolvers::VulnerabilitiesResolver
 

ee/app/graphql/ee/types/project_type.rb

@@ -13,6 +13,7 @@ module EE
         field :vulnerabilities,
               ::Types::VulnerabilityType.connection_type,
               null: true,
+              extras: [:lookahead],
               description: 'Vulnerabilities reported on the project.',
               resolver: ::Resolvers::VulnerabilitiesResolver
 

ee/app/graphql/ee/types/query_type.rb

@@ -17,6 +17,7 @@ module EE
         field :vulnerabilities,
               ::Types::VulnerabilityType.connection_type,
               null: true,
+              extras: [:lookahead],
               description: "Vulnerabilities reported on projects on the current user's instance security dashboard.",
               resolver: ::Resolvers::VulnerabilitiesResolver
 

ee/app/graphql/resolvers/vulnerabilities_resolver.rb

@@ -3,6 +3,7 @@
 module Resolvers
   class VulnerabilitiesResolver < VulnerabilitiesBaseResolver
     include Gitlab::Utils::StrongMemoize
+    include LooksAhead
 
     type Types::VulnerabilityType, null: true
 
@@ -49,7 +50,7 @@ module Resolvers
                           "the response only matches entries for a `reportType` "\
                           "that includes #{::Vulnerabilities::Finding::REPORT_TYPES_WITH_LOCATION_IMAGE.map { |type| "`#{type}`" }.join(', ')}."
 
-    def resolve(**args)
+    def resolve_with_lookahead(**args)
       return Vulnerability.none unless vulnerable
 
       args[:scanner_id] = resolve_gids(args[:scanner_id], ::Vulnerabilities::Scanner) if args[:scanner_id]
@@ -59,10 +60,20 @@ module Resolvers
         .with_created_issue_links_and_issues
     end
 
+    def unconditional_includes
+      [:findings]
+    end
+
+    def preloads
+      {
+        has_solutions: [{ findings: [:remediations] }]
+      }
+    end
+
     private
 
     def vulnerabilities(params)
-      Security::VulnerabilitiesFinder.new(vulnerable, params).execute
+      apply_lookahead(Security::VulnerabilitiesFinder.new(vulnerable, params).execute)
     end
   end
 end

ee/spec/graphql/types/vulnerability_type_spec.rb

@@ -5,7 +5,7 @@ require 'spec_helper'
 RSpec.describe GitlabSchema.types['Vulnerability'] do
   let_it_be(:project) { create(:project) }
   let_it_be(:user) { create(:user) }
-  let_it_be(:vulnerability) { create(:vulnerability, project: project) }
+  let_it_be(:vulnerability) { create(:vulnerability, :with_remediation, project: project) }
   let_it_be(:fields) do
     %i[userPermissions
        id
@@ -95,17 +95,37 @@ RSpec.describe GitlabSchema.types['Vulnerability'] do
 
     context 'N+1 queries' do
       it 'avoids N+1 database queries' do
-        pending('See: https://gitlab.com/gitlab-org/gitlab/-/issues/292993')
+        # Execute the query once so we don't count selecting Projects and Namespaces
+        GitlabSchema.execute(query, context: { current_user: user })
+
+        # Count queries for the baseline which is a single Vulnerability with a remediation
+        # Should be 10
         control_count = ActiveRecord::QueryRecorder.new { GitlabSchema.execute(query, context: { current_user: user }) }.count
+        expect(control_count).to eq(10)
 
+        create(:vulnerability, :with_remediation, project: project)
         create(:vulnerability, :with_remediation, project: project)
         create(:vulnerability, :with_remediation, project: project)
 
-        expect { GitlabSchema.execute(query, context: { current_user: user }) }.not_to exceed_query_limit(control_count)
+        # Every additional Vulnerability seems to add TWO more database calls similar to
+        # 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" = 315
+        #   AND
+        #   "project_authorizations"."user_id" = 409
+        # GROUP BY
+        #   "project_authorizations"."user_id"
+        #
+        # I have no idea where do they come from or if we could batch them
+        # so we have to increase the control_count by 2 * number of Vulnerabilities
+        expect { GitlabSchema.execute(query, context: { current_user: user }) }.not_to exceed_query_limit(control_count + (3 * 2))
 
         result = GitlabSchema.execute(query, context: { current_user: user }).to_h
         vulnerability = result.dig('data', 'project', 'vulnerabilities', 'nodes').first
-        expect(vulnerability['hasSolution']).to be_truthy
+        expect(vulnerability['hasSolutions']).to be_truthy
       end
     end
   end