From ec169c6384b44291551eb7998b4f7f1102a8af07 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Sat, 7 Dec 2019 18:07:50 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- app/finders/group_projects_finder.rb | 14 +++++++ app/models/project.rb | 1 + ...gitaly-n-1-queries-in-api-version-groups-id.yml | 5 +++ doc/api/groups.md | 48 +++++++++++++++++++++- lib/api/entities.rb | 12 +++++- spec/finders/group_projects_finder_spec.rb | 16 ++++++++ spec/models/project_spec.rb | 8 ++++ spec/requests/api/groups_spec.rb | 45 ++++++++++++++++++++ 8 files changed, 145 insertions(+), 4 deletions(-) create mode 100644 changelogs/unreleased/31031-gitaly-n-1-queries-in-api-version-groups-id.yml diff --git a/app/finders/group_projects_finder.rb b/app/finders/group_projects_finder.rb index 8ab5072fdc6..dd8b2f29425 100644 --- a/app/finders/group_projects_finder.rb +++ b/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 diff --git a/app/models/project.rb b/app/models/project.rb index 301fcfc8c98..6ee300dc7f5 100644 --- a/app/models/project.rb +++ b/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) diff --git a/changelogs/unreleased/31031-gitaly-n-1-queries-in-api-version-groups-id.yml b/changelogs/unreleased/31031-gitaly-n-1-queries-in-api-version-groups-id.yml new file mode 100644 index 00000000000..56ab804e5f6 --- /dev/null +++ b/changelogs/unreleased/31031-gitaly-n-1-queries-in-api-version-groups-id.yml @@ -0,0 +1,5 @@ +--- +title: Limit number of projects displayed in GET /groups/:id API +merge_request: 20023 +author: +type: deprecated diff --git a/doc/api/groups.md b/doc/api/groups.md index 94f46b11a0f..32e2a88f25b 100644 --- a/doc/api/groups.md +++ b/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: " 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: " "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. diff --git a/lib/api/entities.rb b/lib/api/entities.rb index ead2ea34227..1297f8e87a2 100644 --- a/lib/api/entities.rb +++ b/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 diff --git a/spec/finders/group_projects_finder_spec.rb b/spec/finders/group_projects_finder_spec.rb index b291b5d4b90..0a7ca5211e1 100644 --- a/spec/finders/group_projects_finder_spec.rb +++ b/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 diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 37604a557ea..d06e87451bd 100644 --- a/spec/models/project_spec.rb +++ b/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) } diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index cb97398805a..c7b35d6f50d 100644 --- a/spec/requests/api/groups_spec.rb +++ b/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 -- cgit v1.2.3