Merge branch 'improve-preload-of-vuln-api-ee' into 'master'

Fix pagination and preloading of resources

Closes #7970

See merge request gitlab-org/gitlab-ee!7945

ee/app/controllers/groups/security/vulnerabilities_controller.rb

@@ -6,17 +6,13 @@ class Groups::Security::VulnerabilitiesController < Groups::ApplicationControlle
   def index
     @vulnerabilities = group.all_vulnerabilities.ordered
       .page(params[:page])
-      .per(10)
-      .to_a
-
-    ::Gitlab::Vulnerabilities::OccurrencesPreloader.new.preload(@vulnerabilities) # rubocop:disable CodeReuse/ActiveRecord
 
     respond_to do |format|
       format.json do
         render json: Vulnerabilities::OccurrenceSerializer
           .new(current_user: @current_user)
           .with_pagination(request, response)
-          .represent(@vulnerabilities)
+          .represent(@vulnerabilities, preload: true)
       end
     end
   end

ee/app/models/vulnerabilities/occurrence.rb

@@ -7,6 +7,8 @@ module Vulnerabilities
 
     self.table_name = "vulnerability_occurrences"
 
+    paginates_per 10
+
     # Used for both severity and confidence
     LEVELS = {
       undefined: 0,
@@ -62,6 +64,10 @@ module Vulnerabilities
     scope :ordered, -> { order("severity desc", :id) }
     scope :counted_by_report_and_severity, -> { group(:report_type, :severity).count }
 
+    scope :all_preloaded, -> do
+      preload(:scanner, :identifiers, :project)
+    end
+
     def feedback(feedback_type:)
       params = {
         project_id: project_id,
@@ -75,7 +81,7 @@ module Vulnerabilities
         categories = items.group_by { |i| i[:category] }
         fingerprints = items.group_by { |i| i[:project_fingerprint] }
 
-        VulnerabilityFeedback.where(
+        VulnerabilityFeedback.all_preloaded.where(
           project_id: project_ids.keys,
           category: categories.keys,
           project_fingerprint: fingerprints.keys).find_each do |feedback|

ee/app/models/vulnerability_feedback.rb

@@ -18,4 +18,8 @@ class VulnerabilityFeedback < ActiveRecord::Base
   validates :project_fingerprint, presence: true, uniqueness: { scope: [:project_id, :category, :feedback_type] }
 
   scope :with_associations, -> { includes(:pipeline, :issue, :author) }
+
+  scope :all_preloaded, -> do
+    preload(:author, :project, :issue, :pipeline)
+  end
 end

ee/app/serializers/vulnerabilities/occurrence_serializer.rb

@@ -2,4 +2,16 @@ class Vulnerabilities::OccurrenceSerializer < BaseSerializer
   include WithPagination
 
   entity Vulnerabilities::OccurrenceEntity
+
+  def represent(resource, opts = {})
+    if paginated?
+      resource = paginator.paginate(resource)
+    end
+
+    if opts.delete(:preload)
+      resource = Gitlab::Vulnerabilities::OccurrencesPreloader.preload!(resource)
+    end
+
+    super(resource, opts)
+  end
 end

ee/lib/gitlab/vulnerabilities/occurrences_preloader.rb

@@ -7,10 +7,12 @@ module Gitlab
   # vulnerabilities (occurrences).
   module Vulnerabilities
     class OccurrencesPreloader
-      def preload(occurrences)
+      def self.preload!(occurrences)
+        occurrences.all_preloaded.tap do |occurrences|
           occurrences.each(&:issue_feedback)
           occurrences.each(&:dismissal_feedback)
         end
       end
     end
+  end
 end

ee/spec/controllers/groups/security/vulnerabilities_controller_spec.rb

@@ -86,24 +86,37 @@ describe Groups::Security::VulnerabilitiesController do
         end
 
         context 'with vulnerability feedback' do
+          it "avoids N+1 queries" do
+            create_vulnerabilities(2, project_dev)
+
+            control_count = ActiveRecord::QueryRecorder.new { get_summary }
+
+            create_vulnerabilities(2, project_guest)
+
+            expect { get_summary }.not_to exceed_all_query_limit(control_count)
+          end
+
+          private
+
           def get_summary
             get :index, group_id: group, format: :json
           end
 
-          it "avoids N+1 queries" do
-            control_count = ActiveRecord::QueryRecorder.new { get_summary }
-
-            # Create feedback
-            project_dev.vulnerabilities.each do |occ|
+          def create_vulnerabilities(count, project)
+            pipeline = create(:ci_empty_pipeline, project: project)
+            vulnerabilities = create_list(:vulnerabilities_occurrence, count, project: project)
+            vulnerabilities.each do |occurrence|
               create(:vulnerability_feedback, :sast, :dismissal,
-                     project: project_dev, project_fingerprint: occ.project_fingerprint)
+                     pipeline: pipeline,
+                     project: project_dev,
+                     project_fingerprint: occurrence.project_fingerprint)
 
               create(:vulnerability_feedback, :sast, :issue,
-                     issue: create(:issue, project: project),
-                     project: project_dev, project_fingerprint: occ.project_fingerprint)
+                     pipeline: pipeline,
+                     issue: create(:issue, project: project_dev),
+                     project: project_dev,
+                     project_fingerprint: occurrence.project_fingerprint)
             end
-
-            expect { get_summary }.not_to exceed_query_limit(control_count)
           end
         end
       end