Allow advanced API projects filtering for admins

In order to develop better automated tools, GitLab admins need to be
able to filter projects by specific repository storage, and to sort
them by one of their statistics fields (e.g. repository_size,
storage_size, or wiki_size).

Related to https://gitlab.com/gitlab-org/gitlab/-/issues/215198

app/finders/projects_finder.rb

@@ -23,6 +23,7 @@
 #     min_access_level: integer
 #     last_activity_after: datetime
 #     last_activity_before: datetime
+#     repository_storage: string
 #
 class ProjectsFinder < UnionFinder
   include CustomAttributesFilter
@@ -75,6 +76,7 @@ class ProjectsFinder < UnionFinder
     collection = by_deleted_status(collection)
     collection = by_last_activity_after(collection)
     collection = by_last_activity_before(collection)
+    collection = by_repository_storage(collection)
     collection
   end
 
@@ -197,6 +199,14 @@ class ProjectsFinder < UnionFinder
     end
   end
 
+  def by_repository_storage(items)
+    if params[:repository_storage].present?
+      items.where(repository_storage: params[:repository_storage]) # rubocop: disable CodeReuse/ActiveRecord
+    else
+      items
+    end
+  end
+
   def sort(items)
     params[:sort].present? ? items.sort_by_attribute(params[:sort]) : items.projects_order_id_desc
   end

app/policies/global_policy.rb

@@ -105,6 +105,9 @@ class GlobalPolicy < BasePolicy
     enable :update_custom_attribute
   end
 
+  # We can't use `read_statistics` because the user may have different permissions for different projects
+  rule { admin }.enable :use_project_statistics_filters
+
   rule { external_user }.prevent :create_snippet
 end
 

changelogs/unreleased/projects-api-sort-by-statistics.yml

+---
+title: Allow advanced API projects filtering for admins
+merge_request: 32879
+author:
+type: added

db/migrate/20200605160806_add_index_on_repository_size_and_project_id_to_project_statistics.rb

+# frozen_string_literal: true
+
+class AddIndexOnRepositorySizeAndProjectIdToProjectStatistics < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :project_statistics, [:repository_size, :project_id]
+  end
+
+  def down
+    remove_concurrent_index :project_statistics, [:repository_size, :project_id]
+  end
+end

db/migrate/20200605160836_add_index_on_storage_size_and_project_id_to_project_statistics.rb

+# frozen_string_literal: true
+
+class AddIndexOnStorageSizeAndProjectIdToProjectStatistics < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :project_statistics, [:storage_size, :project_id]
+  end
+
+  def down
+    remove_concurrent_index :project_statistics, [:storage_size, :project_id]
+  end
+end

db/migrate/20200605160851_add_index_on_wiki_size_and_project_id_to_project_statistics.rb

+# frozen_string_literal: true
+
+class AddIndexOnWikiSizeAndProjectIdToProjectStatistics < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :project_statistics, [:wiki_size, :project_id]
+  end
+
+  def down
+    remove_concurrent_index :project_statistics, [:wiki_size, :project_id]
+  end
+end

db/structure.sql

@@ -10588,6 +10588,12 @@ CREATE INDEX index_project_statistics_on_namespace_id ON public.project_statisti
 
 CREATE UNIQUE INDEX index_project_statistics_on_project_id ON public.project_statistics USING btree (project_id);
 
+CREATE INDEX index_project_statistics_on_repository_size_and_project_id ON public.project_statistics USING btree (repository_size, project_id);
+
+CREATE INDEX index_project_statistics_on_storage_size_and_project_id ON public.project_statistics USING btree (storage_size, project_id);
+
+CREATE INDEX index_project_statistics_on_wiki_size_and_project_id ON public.project_statistics USING btree (wiki_size, project_id);
+
 CREATE UNIQUE INDEX index_project_tracing_settings_on_project_id ON public.project_tracing_settings USING btree (project_id);
 
 CREATE INDEX index_projects_api_created_at_id_desc ON public.projects USING btree (created_at, id DESC);
@@ -13984,6 +13990,9 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200604174558
 20200605003204
 20200605093113
+20200605160806
+20200605160836
+20200605160851
 20200608072931
 20200608075553
 20200608214008

doc/api/projects.md

@@ -45,7 +45,7 @@ GET /projects
 | --------- | ---- | -------- | ----------- |
 | `archived`                    | boolean  | no | Limit by archived status |
 | `visibility`                  | string   | no | Limit by visibility `public`, `internal`, or `private` |
-| `order_by`                    | string   | no | Return projects ordered by `id`, `name`, `path`, `created_at`, `updated_at`, or `last_activity_at` fields. Default is `created_at` |
+| `order_by`                    | string   | no | Return projects ordered by `id`, `name`, `path`, `created_at`, `updated_at`, or `last_activity_at` fields. `repository_size`, `storage_size`, or `wiki_size` fields are only allowed for admins. Default is `created_at` |
 | `sort`                        | string   | no | Return projects sorted in `asc` or `desc` order. Default is `desc` |
 | `search`                      | string   | no | Return list of projects matching the search criteria |
 | `search_namespaces`           | boolean  | no | Include ancestor namespaces when matching search criteria. Default is `false` |
@@ -65,6 +65,7 @@ GET /projects
 | `id_before`                   | integer  | no | Limit results to projects with IDs less than the specified ID |
 | `last_activity_after`         | datetime | no | Limit results to projects with last_activity after specified time. Format: ISO 8601 YYYY-MM-DDTHH:MM:SSZ |
 | `last_activity_before`        | datetime | no | Limit results to projects with last_activity before specified time. Format: ISO 8601 YYYY-MM-DDTHH:MM:SSZ |
+| `repository_storage`          | string   | no | Limit results to projects stored on repository_storage. Available for admins only. |
 
 NOTE: **Note:**
 This endpoint supports [keyset pagination](README.md#keyset-based-pagination) for selected `order_by` options.

lib/api/helpers.rb

@@ -543,6 +543,7 @@ module API
       finder_params[:id_before] = params[:id_before] if params[:id_before]
       finder_params[:last_activity_after] = params[:last_activity_after] if params[:last_activity_after]
       finder_params[:last_activity_before] = params[:last_activity_before] if params[:last_activity_before]
+      finder_params[:repository_storage] = params[:repository_storage] if params[:repository_storage]
       finder_params
     end
 

lib/api/helpers/projects_helpers.rb

@@ -6,6 +6,8 @@ module API
       extend ActiveSupport::Concern
       extend Grape::API::Helpers
 
+      STATISTICS_SORT_PARAMS = %w[storage_size repository_size wiki_size].freeze
+
       params :optional_project_params_ce do
         optional :description, type: String, desc: 'The description of the project'
         optional :build_git_strategy, type: String, values: %w(fetch clone), desc: 'The Git strategy. Defaults to `fetch`'

lib/api/projects.rb

@@ -17,6 +17,7 @@ module API
         projects = projects.with_issues_available_for_user(current_user) if params[:with_issues_enabled]
         projects = projects.with_merge_requests_enabled if params[:with_merge_requests_enabled]
         projects = projects.with_statistics if params[:statistics]
+        projects = projects.joins(:statistics) if params[:order_by].include?('project_statistics') # rubocop: disable CodeReuse/ActiveRecord
 
         lang = params[:with_programming_language]
         projects = projects.with_programming_language(lang) if lang
@@ -28,6 +29,20 @@ module API
         attrs.delete(:repository_storage) unless can?(current_user, :change_repository_storage, project)
       end
 
+      def verify_project_filters!(attrs)
+        attrs.delete(:repository_storage) unless can?(current_user, :use_project_statistics_filters)
+      end
+
+      def verify_statistics_order_by_projects!
+        return unless Helpers::ProjectsHelpers::STATISTICS_SORT_PARAMS.include?(params[:order_by])
+
+        params[:order_by] = if can?(current_user, :use_project_statistics_filters)
+                              "project_statistics.#{params[:order_by]}"
+                            else
+                              route.params['order_by'][:default]
+                            end
+      end
+
       def delete_project(user_project)
         destroy_conditionally!(user_project) do
           ::Projects::DestroyService.new(user_project, current_user, {}).async_execute
@@ -52,8 +67,9 @@ module API
       end
 
       params :sort_params do
-        optional :order_by, type: String, values: %w[id name path created_at updated_at last_activity_at],
-                            default: 'created_at', desc: 'Return projects ordered by field'
+        optional :order_by, type: String,
+                            values: %w[id name path created_at updated_at last_activity_at] + Helpers::ProjectsHelpers::STATISTICS_SORT_PARAMS,
+                            default: 'created_at', desc: "Return projects ordered by field. #{Helpers::ProjectsHelpers::STATISTICS_SORT_PARAMS.join(', ')} are only available to admins."
         optional :sort, type: String, values: %w[asc desc], default: 'desc',
                         desc: 'Return projects sorted in ascending and descending order'
       end
@@ -75,6 +91,7 @@ module API
         optional :id_before, type: Integer, desc: 'Limit results to projects with IDs less than the specified ID'
         optional :last_activity_after, type: DateTime, desc: 'Limit results to projects with last_activity after specified time. Format: ISO 8601 YYYY-MM-DDTHH:MM:SSZ'
         optional :last_activity_before, type: DateTime, desc: 'Limit results to projects with last_activity before specified time. Format: ISO 8601 YYYY-MM-DDTHH:MM:SSZ'
+        optional :repository_storage, type: String, desc: 'Which storage shard the repository is on. Available only to admins'
 
         use :optional_filter_params_ee
       end
@@ -88,10 +105,15 @@ module API
       end
 
       def load_projects
-        ProjectsFinder.new(current_user: current_user, params: project_finder_params).execute
+        params = project_finder_params
+        verify_project_filters!(params)
+
+        ProjectsFinder.new(current_user: current_user, params: params).execute
       end
 
       def present_projects(projects, options = {})
+        verify_statistics_order_by_projects!
+
         projects = reorder_projects(projects)
         projects = apply_filters(projects)
 

spec/finders/projects_finder_spec.rb

@@ -262,6 +262,17 @@ RSpec.describe ProjectsFinder, :do_not_mock_admin_mode do
         it { is_expected.to match_array([public_project]) }
       end
 
+      describe 'filter by repository_storage' do
+        let(:params) { { repository_storage: 'nfs-05' } }
+        let!(:project) { create(:project, :public) }
+
+        before do
+          project.update_columns(repository_storage: 'nfs-05')
+        end
+
+        it { is_expected.to match_array([project]) }
+      end
+
       describe 'sorting' do
         let(:params) { { sort: 'name_asc' } }
 

spec/policies/global_policy_spec.rb

@@ -130,6 +130,24 @@ describe GlobalPolicy do
     end
   end
 
+  describe 'using project statistics filters' do
+    context 'regular user' do
+      it { is_expected.not_to be_allowed(:use_project_statistics_filters) }
+    end
+
+    context 'admin' do
+      let(:current_user) { create(:user, :admin) }
+
+      context 'when admin mode is enabled', :enable_admin_mode do
+        it { is_expected.to be_allowed(:use_project_statistics_filters) }
+      end
+
+      context 'when admin mode is disabled' do
+        it { is_expected.to be_disallowed(:use_project_statistics_filters) }
+      end
+    end
+  end
+
   shared_examples 'access allowed when terms accepted' do |ability|
     it { is_expected.not_to be_allowed(ability) }
 

spec/requests/api/projects_spec.rb

@@ -584,6 +584,85 @@ describe API::Projects do
       end
     end
 
+    context 'sorting by project statistics' do
+      %w(repository_size storage_size wiki_size).each do |order_by|
+        context "sorting by #{order_by}" do
+          before do
+            ProjectStatistics.update_all(order_by => 100)
+            project4.statistics.update_columns(order_by => 10)
+            project.statistics.update_columns(order_by => 200)
+          end
+
+          context 'admin user' do
+            let(:current_user) { admin }
+
+            context "when sorting by #{order_by} ascendingly" do
+              it 'returns a properly sorted list of projects' do
+                get api('/projects', current_user), params: { order_by: order_by, sort: :asc }
+
+                expect(response).to have_gitlab_http_status(:ok)
+                expect(response).to include_pagination_headers
+                expect(json_response).to be_an Array
+                expect(json_response.first['id']).to eq(project4.id)
+              end
+            end
+
+            context "when sorting by #{order_by} descendingly" do
+              it 'returns a properly sorted list of projects' do
+                get api('/projects', current_user), params: { order_by: order_by, sort: :desc }
+
+                expect(response).to have_gitlab_http_status(:ok)
+                expect(response).to include_pagination_headers
+                expect(json_response).to be_an Array
+                expect(json_response.first['id']).to eq(project.id)
+              end
+            end
+          end
+
+          context 'non-admin user' do
+            let(:current_user) { user }
+            let(:projects) { [public_project, project, project2, project3] }
+
+            it 'returns projects ordered normally' do
+              get api('/projects', current_user), params: { order_by: order_by }
+
+              expect(response).to have_gitlab_http_status(:ok)
+              expect(response).to include_pagination_headers
+              expect(json_response).to be_an Array
+              expect(json_response.map { |project| project['id'] }).to eq(projects.map(&:id).reverse)
+            end
+          end
+        end
+      end
+    end
+
+    context 'filtering by repository_storage' do
+      before do
+        [project, project3].each { |proj| proj.update_columns(repository_storage: 'nfs-11') }
+        # Since we don't actually have Gitaly configured with an nfs-11 storage, an error would be raised
+        # when we present the projects in a response, as we ask Gitaly for stuff like default branch and Gitaly
+        # is not configured for a nfs-11 storage. So we trick Rails into thinking the storage for these projects
+        # is still default (in reality, it is).
+        allow_any_instance_of(Project).to receive(:repository_storage).and_return('default')
+      end
+
+      context 'admin user' do
+        it_behaves_like 'projects response' do
+          let(:filter) { { repository_storage: 'nfs-11' } }
+          let(:current_user) { admin }
+          let(:projects) { [project, project3] }
+        end
+      end
+
+      context 'non-admin user' do
+        it_behaves_like 'projects response' do
+          let(:filter) { { repository_storage: 'nfs-11' } }
+          let(:current_user) { user }
+          let(:projects) { [public_project, project, project2, project3] }
+        end
+      end
+    end
+
     context 'with keyset pagination' do
       let(:current_user) { user }
       let(:projects) { [public_project, project, project2, project3] }