Merge branch 'optimize-project-users-api' into 'master'

Improve the performance of project/users API

See merge request gitlab-org/gitlab!64528

config/feature_flags/development/sort_by_project_users_by_project_authorizations_user_id.yml

+---
+name: sort_by_project_users_by_project_authorizations_user_id
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/64528
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/334167
+milestone: '14.1'
+type: development
+group: group::optimize
+default_enabled: false

db/post_migrate/20210621155328_replace_project_authorizations_project_id_index.rb

+# frozen_string_literal: true
+
+class ReplaceProjectAuthorizationsProjectIdIndex < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  OLD_INDEX_NAME = 'index_project_authorizations_on_project_id'
+  NEW_INDEX_NAME = 'index_project_authorizations_on_project_id_user_id'
+
+  def up
+    add_concurrent_index(:project_authorizations, [:project_id, :user_id], name: NEW_INDEX_NAME)
+    remove_concurrent_index_by_name(:project_authorizations, OLD_INDEX_NAME)
+  end
+
+  def down
+    add_concurrent_index(:project_authorizations, :project_id, name: OLD_INDEX_NAME)
+    remove_concurrent_index_by_name(:project_authorizations, NEW_INDEX_NAME)
+  end
+end

db/schema_migrations/20210621155328

+d08fdc3da5fe1a7bf20af5fbe42438fc43ebdf1299c61159740df7644e4ea117
\ No newline at end of file

db/structure.sql

@@ -24251,7 +24251,7 @@ CREATE UNIQUE INDEX index_project_aliases_on_name ON project_aliases USING btree
 
 CREATE INDEX index_project_aliases_on_project_id ON project_aliases USING btree (project_id);
 
-CREATE INDEX index_project_authorizations_on_project_id ON project_authorizations USING btree (project_id);
+CREATE INDEX index_project_authorizations_on_project_id_user_id ON project_authorizations USING btree (project_id, user_id);
 
 CREATE UNIQUE INDEX index_project_auto_devops_on_project_id ON project_auto_devops USING btree (project_id);
 

lib/api/projects.rb

@@ -608,6 +608,10 @@ module API
         users = users.search(params[:search]) if params[:search].present?
         users = users.where_not_in(params[:skip_users]) if params[:skip_users].present?
 
+        if Feature.enabled?(:sort_by_project_users_by_project_authorizations_user_id, user_project, default_enabled: :yaml)
+          users = users.order('project_authorizations.user_id' => :asc) # rubocop: disable CodeReuse/ActiveRecord
+        end
+
         present paginate(users), with: Entities::UserBasic
       end
 

spec/requests/api/projects_spec.rb

@@ -2464,6 +2464,14 @@ RSpec.describe API::Projects do
 
   describe 'GET /projects/:id/users' do
     shared_examples_for 'project users response' do
+      let(:reporter_1) { create(:user) }
+      let(:reporter_2) { create(:user) }
+
+      before do
+        project.add_reporter(reporter_1)
+        project.add_reporter(reporter_2)
+      end
+
       it 'returns the project users' do
         get api("/projects/#{project.id}/users", current_user)
 
@@ -2472,12 +2480,15 @@ RSpec.describe API::Projects do
         expect(response).to have_gitlab_http_status(:ok)
         expect(response).to include_pagination_headers
         expect(json_response).to be_an Array
-        expect(json_response.size).to eq(1)
+        expect(json_response.size).to eq(3)
 
         first_user = json_response.first
         expect(first_user['username']).to eq(user.username)
         expect(first_user['name']).to eq(user.name)
         expect(first_user.keys).to include(*%w[name username id state avatar_url web_url])
+
+        ids = json_response.map { |raw_user| raw_user['id'] }
+        expect(ids).to eq([user.id, reporter_1.id, reporter_2.id])
       end
     end
 
@@ -2490,9 +2501,26 @@ RSpec.describe API::Projects do
 
     context 'when authenticated' do
       context 'valid request' do
-        it_behaves_like 'project users response' do
-          let(:project) { project4 }
-          let(:current_user) { user4 }
+        context 'when sort_by_project_authorizations_user_id FF is off' do
+          before do
+            stub_feature_flags(sort_by_project_users_by_project_authorizations_user_id: false)
+          end
+
+          it_behaves_like 'project users response' do
+            let(:project) { project4 }
+            let(:current_user) { user4 }
+          end
+        end
+
+        context 'when sort_by_project_authorizations_user_id FF is on' do
+          before do
+            stub_feature_flags(sort_by_project_users_by_project_authorizations_user_id: true)
+          end
+
+          it_behaves_like 'project users response' do
+            let(:project) { project4 }
+            let(:current_user) { user4 }
+          end
         end
       end