Merge branch 'fix-n-plus-1-in-projects-api' into 'master'

Fix N+1 in projects API

See merge request gitlab-org/gitlab!69949

lib/api/entities/project.rb

@@ -126,6 +126,10 @@ module API
       expose :keep_latest_artifacts_available?, as: :keep_latest_artifact
 
       # rubocop: disable CodeReuse/ActiveRecord
+      def self.preload_resource(project)
+        ActiveRecord::Associations::Preloader.new.preload(project, project_group_links: { group: :route })
+      end
+
       def self.preload_relation(projects_relation, options = {})
         # Preloading topics, should be done with using only `:topics`,
         # as `:topics` are defined as: `has_many :topics, through: :taggings`

lib/api/projects.rb

@@ -152,6 +152,12 @@ module API
         ProjectsFinder.new(current_user: current_user, params: project_params).execute
       end
 
+      def present_project(project, options = {})
+        options[:with].preload_resource(project) if options[:with].respond_to?(:preload_resource)
+
+        present project, options
+      end
+
       def present_projects(projects, options = {})
         verify_statistics_order_by_projects!
 
@@ -264,9 +270,9 @@ module API
         project = ::Projects::CreateService.new(current_user, attrs).execute
 
         if project.saved?
-          present project, with: Entities::Project,
-                           user_can_admin_project: can?(current_user, :admin_project, project),
-                           current_user: current_user
+          present_project project, with: Entities::Project,
+                                   user_can_admin_project: can?(current_user, :admin_project, project),
+                                   current_user: current_user
         else
           if project.errors[:limit_reached].present?
             error!(project.errors[:limit_reached], 403)
@@ -301,9 +307,9 @@ module API
         project = ::Projects::CreateService.new(user, attrs).execute
 
         if project.saved?
-          present project, with: Entities::Project,
-                           user_can_admin_project: can?(current_user, :admin_project, project),
-                           current_user: current_user
+          present_project project, with: Entities::Project,
+                                   user_can_admin_project: can?(current_user, :admin_project, project),
+                                   current_user: current_user
         else
           render_validation_error!(project)
         end
@@ -336,7 +342,7 @@ module API
 
         project, options = with_custom_attributes(user_project, options)
 
-        present project, options
+        present_project project, options
       end
 
       desc 'Fork new project for the current user or provided namespace.' do
@@ -376,9 +382,11 @@ module API
         if forked_project.errors.any?
           conflict!(forked_project.errors.messages)
         else
-          present forked_project, with: Entities::Project,
-                  user_can_admin_project: can?(current_user, :admin_project, forked_project),
-                  current_user: current_user
+          present_project forked_project, {
+            with: Entities::Project,
+            user_can_admin_project: can?(current_user, :admin_project, forked_project),
+            current_user: current_user
+          }
         end
       end
 
@@ -427,9 +435,9 @@ module API
         result = ::Projects::UpdateService.new(user_project, current_user, attrs).execute
 
         if result[:status] == :success
-          present user_project, with: Entities::Project,
-                                user_can_admin_project: can?(current_user, :admin_project, user_project),
-                                current_user: current_user
+          present_project user_project, with: Entities::Project,
+                                        user_can_admin_project: can?(current_user, :admin_project, user_project),
+                                        current_user: current_user
         else
           render_validation_error!(user_project)
         end
@@ -443,7 +451,7 @@ module API
 
         ::Projects::UpdateService.new(user_project, current_user, archived: true).execute
 
-        present user_project, with: Entities::Project, current_user: current_user
+        present_project user_project, with: Entities::Project, current_user: current_user
       end
 
       desc 'Unarchive a project' do
@@ -454,7 +462,7 @@ module API
 
         ::Projects::UpdateService.new(user_project, current_user, archived: false).execute
 
-        present user_project, with: Entities::Project, current_user: current_user
+        present_project user_project, with: Entities::Project, current_user: current_user
       end
 
       desc 'Star a project' do
@@ -467,7 +475,7 @@ module API
           current_user.toggle_star(user_project)
           user_project.reset
 
-          present user_project, with: Entities::Project, current_user: current_user
+          present_project user_project, with: Entities::Project, current_user: current_user
         end
       end
 
@@ -479,7 +487,7 @@ module API
           current_user.toggle_star(user_project)
           user_project.reset
 
-          present user_project, with: Entities::Project, current_user: current_user
+          present_project user_project, with: Entities::Project, current_user: current_user
         else
           not_modified!
         end
@@ -528,7 +536,7 @@ module API
         result = ::Projects::ForkService.new(fork_from_project, current_user).execute(user_project)
 
         if result
-          present user_project.reset, with: Entities::Project, current_user: current_user
+          present_project user_project.reset, with: Entities::Project, current_user: current_user
         else
           render_api_error!("Project already forked", 409) if user_project.forked?
         end
@@ -698,7 +706,7 @@ module API
         result = ::Projects::TransferService.new(user_project, current_user).execute(namespace)
 
         if result
-          present user_project, with: Entities::Project, current_user: current_user
+          present_project user_project, with: Entities::Project, current_user: current_user
         else
           render_api_error!("Failed to transfer project #{user_project.errors.messages}", 400)
         end

spec/requests/api/projects_spec.rb

@@ -2616,6 +2616,23 @@ RSpec.describe API::Projects do
       expect(json_response).to have_key 'service_desk_enabled'
       expect(json_response).to have_key 'service_desk_address'
     end
+
+    context 'when project is shared to multiple groups' do
+      it 'avoids N+1 queries' do
+        create(:project_group_link, project: project)
+        get api("/projects/#{project.id}", user)
+
+        control = ActiveRecord::QueryRecorder.new do
+          get api("/projects/#{project.id}", user)
+        end
+
+        create(:project_group_link, project: project)
+
+        expect do
+          get api("/projects/#{project.id}", user)
+        end.not_to exceed_query_limit(control)
+      end
+    end
   end
 
   describe 'GET /projects/:id/users' do