Merge branch 'id-n-1-for-api-deployments' into 'master'

Resolve N + 1 for deployments API

See merge request gitlab-org/gitlab!57558

app/models/deployment.rb

@@ -45,6 +45,7 @@ class Deployment < ApplicationRecord
   scope :active, -> { where(status: %i[created running]) }
   scope :older_than, -> (deployment) { where('deployments.id < ?', deployment.id) }
   scope :with_deployable, -> { joins('INNER JOIN ci_builds ON ci_builds.id = deployments.deployable_id').preload(:deployable) }
+  scope :with_api_entity_associations, -> { preload({ deployable: { runner: [], tags: [], user: [], job_artifacts_archive: [] } }) }
 
   scope :finished_after, ->(date) { where('finished_at >= ?', date) }
   scope :finished_before, ->(date) { where('finished_at < ?', date) }

changelogs/unreleased/id-n-1-for-api-deployments.yml

+---
+title: Resolve N + 1 for deployments API
+merge_request: 57558
+author:
+type: performance

lib/api/deployments.rb

@@ -36,7 +36,9 @@ module API
       get ':id/deployments' do
         authorize! :read_deployment, user_project
 
-        deployments = DeploymentsFinder.new(params.merge(project: user_project)).execute
+        deployments =
+          DeploymentsFinder.new(params.merge(project: user_project))
+            .execute.with_api_entity_associations
 
         present paginate(deployments), with: Entities::Deployment
       end

lib/api/entities/user.rb

@@ -11,10 +11,10 @@ module API
         work_information(user)
       end
       expose :followers, if: ->(user, opts) { Ability.allowed?(opts[:current_user], :read_user_profile, user) } do |user|
-        user.followers.count
+        user.followers.size
       end
       expose :following, if: ->(user, opts) { Ability.allowed?(opts[:current_user], :read_user_profile, user) } do |user|
-        user.followees.count
+        user.followees.size
       end
     end
   end

spec/requests/api/deployments_spec.rb

@@ -3,22 +3,26 @@
 require 'spec_helper'
 
 RSpec.describe API::Deployments do
-  let(:user)        { create(:user) }
-  let(:non_member)  { create(:user) }
+  let_it_be(:user)        { create(:user) }
+  let_it_be(:non_member)  { create(:user) }
 
   before do
     project.add_maintainer(user)
   end
 
   describe 'GET /projects/:id/deployments' do
-    let(:project) { create(:project, :repository) }
-    let!(:deployment_1) { create(:deployment, :success, project: project, iid: 11, ref: 'master', created_at: Time.now, updated_at: Time.now) }
-    let!(:deployment_2) { create(:deployment, :success, project: project, iid: 12, ref: 'master', created_at: 1.day.ago, updated_at: 2.hours.ago) }
-    let!(:deployment_3) { create(:deployment, :success, project: project, iid: 8, ref: 'master', created_at: 2.days.ago, updated_at: 1.hour.ago) }
+    let_it_be(:project) { create(:project, :repository) }
+    let_it_be(:deployment_1) { create(:deployment, :success, project: project, iid: 11, ref: 'master', created_at: Time.now, updated_at: Time.now) }
+    let_it_be(:deployment_2) { create(:deployment, :success, project: project, iid: 12, ref: 'master', created_at: 1.day.ago, updated_at: 2.hours.ago) }
+    let_it_be(:deployment_3) { create(:deployment, :success, project: project, iid: 8, ref: 'master', created_at: 2.days.ago, updated_at: 1.hour.ago) }
+
+    def perform_request(params = {})
+      get api("/projects/#{project.id}/deployments", user), params: params
+    end
 
     context 'as member of the project' do
       it 'returns projects deployments sorted by id asc' do
-        get api("/projects/#{project.id}/deployments", user)
+        perform_request
 
         expect(response).to have_gitlab_http_status(:ok)
         expect(response).to include_pagination_headers
@@ -32,7 +36,7 @@ RSpec.describe API::Deployments do
 
       context 'with updated_at filters specified' do
         it 'returns projects deployments with last update in specified datetime range' do
-          get api("/projects/#{project.id}/deployments", user), params: { updated_before: 30.minutes.ago, updated_after: 90.minutes.ago }
+          perform_request({ updated_before: 30.minutes.ago, updated_after: 90.minutes.ago })
 
           expect(response).to have_gitlab_http_status(:ok)
           expect(response).to include_pagination_headers
@@ -42,10 +46,7 @@ RSpec.describe API::Deployments do
 
       context 'with the environment filter specifed' do
         it 'returns deployments for the environment' do
-          get(
-            api("/projects/#{project.id}/deployments", user),
-            params: { environment: deployment_1.environment.name }
-          )
+          perform_request({ environment: deployment_1.environment.name })
 
           expect(json_response.size).to eq(1)
           expect(json_response.first['iid']).to eq(deployment_1.iid)
@@ -86,6 +87,16 @@ RSpec.describe API::Deployments do
           end
         end
       end
+
+      it 'returns multiple deployments without N + 1' do
+        perform_request # warm up the cache
+
+        control_count = ActiveRecord::QueryRecorder.new { perform_request }.count
+
+        create(:deployment, :success, project: project, iid: 21, ref: 'master')
+
+        expect { perform_request }.not_to exceed_query_limit(control_count)
+      end
     end
 
     context 'as non member' do