Merge branch '336998-n-1-noted-around-pipeline-webhook-event-payload-serialisation' into 'master'

Resolve "N+1 noted around pipeline webhook event payload serialisation"

See merge request gitlab-org/gitlab!67238

app/workers/pipeline_hooks_worker.rb

@@ -12,7 +12,7 @@ class PipelineHooksWorker # rubocop:disable Scalability/IdempotentWorker
 
   # rubocop: disable CodeReuse/ActiveRecord
   def perform(pipeline_id)
-    Ci::Pipeline.includes({ builds: { runner: :tags } })
+    Ci::Pipeline
       .find_by(id: pipeline_id)
       .try(:execute_hooks)
   end

lib/gitlab/data_builder/pipeline.rb

@@ -19,18 +19,40 @@ module Gitlab
           user: pipeline.user.try(:hook_attrs),
           project: pipeline.project.hook_attrs(backward: false),
           commit: pipeline.commit.try(:hook_attrs),
-          builds: Gitlab::Lazy.new { pipeline.builds.latest.map(&method(:build_hook_attrs)) }
+          builds: Gitlab::Lazy.new do
+            preload_builds(pipeline, :latest_builds)
+            pipeline.latest_builds.map(&method(:build_hook_attrs))
+          end
         )
       end
 
       def with_retried_builds
         merge(
-          builds: Gitlab::Lazy.new { @pipeline.builds.map(&method(:build_hook_attrs)) }
+          builds: Gitlab::Lazy.new do
+            preload_builds(@pipeline, :builds)
+            @pipeline.builds.map(&method(:build_hook_attrs))
+          end
         )
       end
 
       private
 
+      # rubocop: disable CodeReuse/ActiveRecord
+      def preload_builds(pipeline, association)
+        ActiveRecord::Associations::Preloader.new.preload(pipeline,
+          {
+            association => {
+              **::Ci::Pipeline::PROJECT_ROUTE_AND_NAMESPACE_ROUTE,
+              runner: :tags,
+              job_artifacts_archive: [],
+              user: [],
+              metadata: []
+            }
+          }
+        )
+      end
+      # rubocop: enable CodeReuse/ActiveRecord
+
       def hook_attrs(pipeline)
         {
           id: pipeline.id,

spec/factories/ci/builds.rb

@@ -237,8 +237,13 @@ FactoryBot.define do
         # to the job. If `build.deployment` has already been set, it doesn't
         # build a new instance.
         environment = Gitlab::Ci::Pipeline::Seed::Environment.new(build).to_resource
-        build.deployment =
-          Gitlab::Ci::Pipeline::Seed::Deployment.new(build, environment).to_resource
+
+        build.assign_attributes(
+          deployment: Gitlab::Ci::Pipeline::Seed::Deployment.new(build, environment).to_resource,
+          metadata_attributes: {
+            expanded_environment_name: environment.name
+          }
+        )
       end
     end
 

spec/lib/gitlab/data_builder/pipeline_spec.rb

@@ -131,5 +131,41 @@ RSpec.describe Gitlab::DataBuilder::Pipeline do
         expect(build_environment_data[:deployment_tier]).to eq(build.persisted_environment.try(:tier))
       end
     end
+
+    context 'avoids N+1 database queries' do
+      it "with multiple builds" do
+        # Preparing the pipeline with the minimal builds
+        pipeline = create(:ci_pipeline, user: user, project: project)
+        create(:ci_build, user: user, project: project, pipeline: pipeline)
+        create(:ci_build, :deploy_to_production, :with_deployment, user: user, project: project, pipeline: pipeline)
+
+        # We need `.to_json` as the build hook data is wrapped within `Gitlab::Lazy`
+        control_count = ActiveRecord::QueryRecorder.new { described_class.build(pipeline.reload).to_json }.count
+
+        # Adding more builds to the pipeline and serializing the data again
+        create_list(:ci_build, 3, user: user, project: project, pipeline: pipeline)
+        create(:ci_build, :start_review_app, :with_deployment, user: user, project: project, pipeline: pipeline)
+        create(:ci_build, :stop_review_app, :with_deployment, user: user, project: project, pipeline: pipeline)
+
+        expect { described_class.build(pipeline.reload).to_json }.not_to exceed_query_limit(control_count)
+      end
+
+      it "with multiple retried builds" do
+        # Preparing the pipeline with the minimal builds
+        pipeline = create(:ci_pipeline, user: user, project: project)
+        create(:ci_build, :retried, user: user, project: project, pipeline: pipeline)
+        create(:ci_build, :deploy_to_production, :retried, :with_deployment, user: user, project: project, pipeline: pipeline)
+
+        # We need `.to_json` as the build hook data is wrapped within `Gitlab::Lazy`
+        control_count = ActiveRecord::QueryRecorder.new { described_class.build(pipeline.reload).with_retried_builds.to_json }.count
+
+        # Adding more builds to the pipeline and serializing the data again
+        create_list(:ci_build, 3, :retried, user: user, project: project, pipeline: pipeline)
+        create(:ci_build, :start_review_app, :retried, :with_deployment, user: user, project: project, pipeline: pipeline)
+        create(:ci_build, :stop_review_app, :retried, :with_deployment, user: user, project: project, pipeline: pipeline)
+
+        expect { described_class.build(pipeline.reload).with_retried_builds.to_json }.not_to exceed_query_limit(control_count)
+      end
+    end
   end
 end