Merge branch '31031-gitaly-n-1-queries-in-api-version-groups-id' into 'master'

Resolve "Gitaly N+1 queries in /api/:version/groups/:id"

Closes #31031

See merge request gitlab-org/gitlab!20023

app/finders/group_projects_finder.rb

@@ -11,6 +11,7 @@
 #   options:
 #     only_owned: boolean
 #     only_shared: boolean
+#     limit: integer
 #   params:
 #     sort: string
 #     visibility_level: int
@@ -20,6 +21,8 @@
 #     non_archived: boolean
 #
 class GroupProjectsFinder < ProjectsFinder
+  DEFAULT_PROJECTS_LIMIT = 100
+
   attr_reader :group, :options
 
   def initialize(group:, params: {}, options: {}, current_user: nil, project_ids_relation: nil)
@@ -32,8 +35,19 @@ class GroupProjectsFinder < ProjectsFinder
     @options = options
   end
 
+  def execute
+    collection = super
+    limit(collection)
+  end
+
   private
 
+  def limit(collection)
+    limit = options[:limit]
+
+    limit.present? ? collection.with_limit(limit) : collection
+  end
+
   def init_collection
     projects = if current_user
                  collection_with_user

app/models/project.rb

@@ -443,6 +443,7 @@ class Project < ApplicationRecord
   scope :with_merge_requests_available_for_user, ->(current_user) { with_feature_available_for_user(:merge_requests, current_user) }
   scope :with_merge_requests_enabled, -> { with_feature_enabled(:merge_requests) }
   scope :with_remote_mirrors, -> { joins(:remote_mirrors).where(remote_mirrors: { enabled: true }).distinct }
+  scope :with_limit, -> (maximum) { limit(maximum) }
 
   scope :with_group_runners_enabled, -> do
     joins(:ci_cd_settings)

changelogs/unreleased/31031-gitaly-n-1-queries-in-api-version-groups-id.yml

+---
+title: Limit number of projects displayed in GET /groups/:id API
+merge_request: 20023
+author:
+type: deprecated

doc/api/groups.md

@@ -5,6 +5,8 @@
 Get a list of visible groups for the authenticated user. When accessed without
 authentication, only public groups are returned.
 
+By default, this request returns 20 results at a time because the API results [are paginated](README.md#pagination).
+
 Parameters:
 
 | Attribute                | Type              | Required | Description |
@@ -106,6 +108,8 @@ GET /groups?custom_attributes[key]=value&custom_attributes[other_key]=other_valu
 Get a list of visible direct subgroups in this group.
 When accessed without authentication, only public groups are returned.
 
+By default, this request returns 20 results at a time because the API results [are paginated](README.md#pagination).
+
 Parameters:
 
 | Attribute                | Type              | Required | Description |
@@ -154,8 +158,9 @@ GET /groups/:id/subgroups
 
 ## List a group's projects
 
-Get a list of projects in this group. When accessed without authentication, only
-public projects are returned.
+Get a list of projects in this group. When accessed without authentication, only public projects are returned.
+
+By default, this request returns 20 results at a time because the API results [are paginated](README.md#pagination).
 
 ```
 GET /groups/:id/projects
@@ -247,6 +252,13 @@ Parameters:
 curl --header "PRIVATE-TOKEN: <your_access_token>" https://gitlab.example.com/api/v4/groups/4
 ```
 
+This endpoint returns:
+
+- All projects and shared projects in GitLab 12.5 and earlier.
+- A maximum of 100 projects and shared projects [in GitLab 12.6](https://gitlab.com/gitlab-org/gitlab/issues/31031)
+  and later. To get the details of all projects within a group, use the
+  [list a group's projects endpoint](#list-a-groups-projects) instead.
+
 Example response:
 
 ```json
@@ -439,6 +451,18 @@ Example response:
 }
 ```
 
+### Disabling the results limit
+
+The 100 results limit can be disabled if it breaks integrations developed using GitLab
+12.4 and earlier.
+
+To disable the limit while migrating to using the [list a group's projects](#list-a-groups-projects) endpoint, ask a GitLab administrator
+with Rails console access to run the following command:
+
+```ruby
+Feature.disable(:limit_projects_in_groups_api)
+```
+
 ## New group
 
 Creates a new project group. Available only for users who can create groups.
@@ -518,6 +542,13 @@ curl --request PUT --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab
 
 ```
 
+This endpoint returns:
+
+- All projects and shared projects in GitLab 12.5 and earlier.
+- A maximum of 100 projects and shared projects [in GitLab 12.6](https://gitlab.com/gitlab-org/gitlab/issues/31031)
+  and later. To get the details of all projects within a group, use the
+  [list a group's projects endpoint](#list-a-groups-projects) instead.
+
 Example response:
 
 ```json
@@ -577,6 +608,19 @@ Example response:
 }
 ```
 
+### Disabling the results limit
+
+The 100 results limit can be disabled if it breaks integrations developed using GitLab
+12.4 and earlier.
+
+To disable the limit while migrating to using the
+[list a group's projects](#list-a-groups-projects) endpoint, ask a GitLab administrator
+with Rails console access to run the following command:
+
+```ruby
+Feature.disable(:limit_projects_in_groups_api)
+```
+
 ## Remove group
 
 Removes group with all projects inside. Only available to group owners and administrators.

lib/api/entities.rb

@@ -415,7 +415,7 @@ module API
         projects = GroupProjectsFinder.new(
           group: group,
           current_user: options[:current_user],
-          options: { only_owned: true }
+          options: { only_owned: true, limit: projects_limit }
         ).execute
 
         Entities::Project.preload_and_batch_count!(projects)
@@ -425,11 +425,19 @@ module API
         projects = GroupProjectsFinder.new(
           group: group,
           current_user: options[:current_user],
-          options: { only_shared: true }
+          options: { only_shared: true, limit: projects_limit }
         ).execute
 
         Entities::Project.preload_and_batch_count!(projects)
       end
+
+      def projects_limit
+        if ::Feature.enabled?(:limit_projects_in_groups_api, default_enabled: true)
+          GroupProjectsFinder::DEFAULT_PROJECTS_LIMIT
+        else
+          nil
+        end
+      end
     end
 
     class DiffRefs < Grape::Entity

spec/finders/group_projects_finder_spec.rb

@@ -168,4 +168,20 @@ describe GroupProjectsFinder do
       end
     end
   end
+
+  describe 'limiting' do
+    context 'without limiting' do
+      it 'returns all projects' do
+        expect(subject.count).to eq(3)
+      end
+    end
+
+    context 'with limiting' do
+      let(:options) { { limit: 1 } }
+
+      it 'returns only the number of projects specified by the limit' do
+        expect(subject.count).to eq(1)
+      end
+    end
+  end
 end

spec/models/project_spec.rb

@@ -1349,6 +1349,14 @@ describe Project do
     end
   end
 
+  describe '.with_limit' do
+    it 'limits the number of projects returned' do
+      create_list(:project, 3)
+
+      expect(described_class.with_limit(1).count).to eq(1)
+    end
+  end
+
   describe '.visible_to_user' do
     let!(:project) { create(:project, :private) }
     let!(:user)    { create(:user) }

spec/requests/api/groups_spec.rb

@@ -488,6 +488,51 @@ describe API::Groups do
         expect(response).to have_gitlab_http_status(404)
       end
     end
+
+    context 'limiting the number of projects and shared_projects in the response' do
+      let(:limit) { 1 }
+
+      before do
+        stub_const("GroupProjectsFinder::DEFAULT_PROJECTS_LIMIT", limit)
+
+        # creates 3 public projects
+        create_list(:project, 3, :public, namespace: group1)
+
+        # creates 3 shared projects
+        public_group = create(:group, :public)
+        projects_to_be_shared = create_list(:project, 3, :public, namespace: public_group)
+
+        projects_to_be_shared.each do |project|
+          create(:project_group_link, project: project, group: group1)
+        end
+      end
+
+      context 'when limiting feature is enabled' do
+        before do
+          stub_feature_flags(limit_projects_in_groups_api: true)
+        end
+
+        it 'limits projects and shared_projects' do
+          get api("/groups/#{group1.id}")
+
+          expect(json_response['projects'].count).to eq(limit)
+          expect(json_response['shared_projects'].count).to eq(limit)
+        end
+      end
+
+      context 'when limiting feature is not enabled' do
+        before do
+          stub_feature_flags(limit_projects_in_groups_api: false)
+        end
+
+        it 'does not limit projects and shared_projects' do
+          get api("/groups/#{group1.id}")
+
+          expect(json_response['projects'].count).to eq(3)
+          expect(json_response['shared_projects'].count).to eq(3)
+        end
+      end
+    end
   end
 
   describe 'PUT /groups/:id' do