Merge branch '332890-projects-api-find-projects-support-similarity-order' into 'master'

Add similarity sort to search projects API

See merge request gitlab-org/gitlab!64342

app/models/preloaders/user_max_access_level_in_projects_preloader.rb

@@ -10,9 +10,13 @@ module Preloaders
     end
 
     def execute
+      # Use reselect to override the existing select to prevent
+      # the error `subquery has too many columns`
+      # NotificationsController passes in an Array so we need to check the type
+      project_ids = @projects.is_a?(ActiveRecord::Relation) ? @projects.reselect(:id) : @projects
       access_levels = @user
         .project_authorizations
-        .where(project_id: @projects)
+        .where(project_id: project_ids)
         .group(:project_id)
         .maximum(:access_level)
 

doc/api/projects.md

@@ -51,7 +51,7 @@ GET /projects
 | `last_activity_before`                     | datetime | **{dotted-circle}** No | Limit results to projects with last_activity before specified time. Format: ISO 8601 `YYYY-MM-DDTHH:MM:SSZ` |
 | `membership`                               | boolean  | **{dotted-circle}** No | Limit by projects that the current user is a member of. |
 | `min_access_level`                         | integer  | **{dotted-circle}** No | Limit by current user minimal [access level](members.md#valid-access-levels). |
-| `order_by`                                 | string   | **{dotted-circle}** No | Return projects ordered by `id`, `name`, `path`, `created_at`, `updated_at`, or `last_activity_at` fields. `repository_size`, `storage_size`, `packages_size` or `wiki_size` fields are only allowed for admins. Default is `created_at`. |
+| `order_by`                                 | string   | **{dotted-circle}** No | Return projects ordered by `id`, `name`, `path`, `created_at`, `updated_at`, `last_activity_at`, or `similarity` fields. `repository_size`, `storage_size`, `packages_size` or `wiki_size` fields are only allowed for admins. `similarity` ([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/332890) in GitLab 14.1) is only available when searching and is limited to projects that the current user is a member of. Default is `created_at`. |
 | `owned`                                    | boolean  | **{dotted-circle}** No | Limit by projects explicitly owned by the current user. |
 | `repository_checksum_failed` **(PREMIUM)** | boolean  | **{dotted-circle}** No | Limit projects where the repository checksum calculation has failed ([Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/6137) in [GitLab Premium](https://about.gitlab.com/pricing/) 11.2). |
 | `repository_storage`                       | string   | **{dotted-circle}** No | Limit results to projects stored on `repository_storage`. _(admins only)_ |

lib/api/entities/project_with_access.rb

@@ -26,8 +26,10 @@ module API
       # rubocop: disable CodeReuse/ActiveRecord
       def self.preload_relation(projects_relation, options = {})
         relation = super(projects_relation, options)
-        project_ids = relation.select('projects.id')
-        namespace_ids = relation.select(:namespace_id)
+        # use reselect to override the existing select and
+        # prevent an error `subquery has too many columns`
+        project_ids = relation.reselect('projects.id')
+        namespace_ids = relation.reselect(:namespace_id)
 
         options[:project_members] = options[:current_user]
           .project_members

lib/api/groups.rb

@@ -128,10 +128,6 @@ module API
         groups.reorder(group_without_similarity_options) # rubocop: disable CodeReuse/ActiveRecord
       end
 
-      def order_by_similarity?
-        params[:order_by] == 'similarity' && params[:search].present?
-      end
-
       def group_without_similarity_options
         order_options = { params[:order_by] => params[:sort] }
         order_options['name'] = order_options.delete('similarity') if order_options.has_key?('similarity')

lib/api/helpers.rb

@@ -577,6 +577,10 @@ module API
       Gitlab::AppLogger.warn("Redis tracking event failed for event: #{event_name}, message: #{error.message}")
     end
 
+    def order_by_similarity?(allow_unauthorized: true)
+      params[:order_by] == 'similarity' && params[:search].present? && (allow_unauthorized || current_user.present?)
+    end
+
     protected
 
     def project_finder_params_visibility_ce

lib/api/projects.rb

@@ -45,6 +45,20 @@ module API
                             end
       end
 
+      def support_order_by_similarity!(attrs)
+        return unless params[:order_by] == 'similarity'
+
+        if order_by_similarity?(allow_unauthorized: false)
+          # Limit to projects the current user is a member of.
+          # Do not include all public projects because it
+          # could cause long running queries
+          attrs[:non_public] = true
+          attrs[:sort] = params['order_by']
+        else
+          params[:order_by] = 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
@@ -93,8 +107,8 @@ module API
 
       params :sort_params do
         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."
+                            values: %w[id name path created_at updated_at last_activity_at similarity] + 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. Similarity is available when searching and is limited to projects the user has access to."
         optional :sort, type: String, values: %w[asc desc], default: 'desc',
                         desc: 'Return projects sorted in ascending and descending order'
       end
@@ -131,16 +145,17 @@ module API
       end
 
       def load_projects
-        params = project_finder_params
-        verify_project_filters!(params)
+        project_params = project_finder_params
+        support_order_by_similarity!(project_params)
+        verify_project_filters!(project_params)
 
-        ProjectsFinder.new(current_user: current_user, params: params).execute
+        ProjectsFinder.new(current_user: current_user, params: project_params).execute
       end
 
       def present_projects(projects, options = {})
         verify_statistics_order_by_projects!
 
-        projects = reorder_projects(projects)
+        projects = reorder_projects(projects) unless order_by_similarity?(allow_unauthorized: false)
         projects = apply_filters(projects)
 
         records, options = paginate_with_strategies(projects, options[:request_scope]) do |projects|

spec/lib/api/helpers_spec.rb

@@ -581,4 +581,40 @@ RSpec.describe API::Helpers do
       end
     end
   end
+
+  describe '#order_by_similarity?' do
+    where(:params, :allow_unauthorized, :current_user_set, :expected) do
+      {}                                          | false | false | false
+      {}                                          | true  | false | false
+      {}                                          | false | true  | false
+      {}                                          | true  | true  | false
+      { order_by: 'similarity' }                  | false | false | false
+      { order_by: 'similarity' }                  | true  | false | false
+      { order_by: 'similarity' }                  | true  | true  | false
+      { order_by: 'similarity' }                  | false | true  | false
+      { search: 'test' }                          | false | false | false
+      { search: 'test' }                          | true  | false | false
+      { search: 'test' }                          | true  | true  | false
+      { search: 'test' }                          | false | true  | false
+      { order_by: 'similarity', search: 'test' }  | false | false | false
+      { order_by: 'similarity', search: 'test' }  | true  | false | true
+      { order_by: 'similarity', search: 'test' }  | true  | true  | true
+      { order_by: 'similarity', search: 'test' }  | false | true  | true
+    end
+
+    with_them do
+      let_it_be(:user) { create(:user) }
+
+      before do
+        u = current_user_set ? user : nil
+        subject.instance_variable_set(:@current_user, u)
+
+        allow(subject).to receive(:params).and_return(params)
+      end
+
+      it 'returns the expected result' do
+        expect(subject.order_by_similarity?(allow_unauthorized: allow_unauthorized)).to eq(expected)
+      end
+    end
+  end
 end

spec/requests/api/projects_spec.rb

@@ -700,52 +700,112 @@ RSpec.describe API::Projects do
       end
     end
 
-    context 'sorting by project statistics' do
-      %w(repository_size storage_size wiki_size packages_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 'sorting' do
+      context 'by project statistics' do
+        %w(repository_size storage_size wiki_size packages_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 '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 }
+              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)
+                  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 "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 }
+            context 'non-admin user' do
+              let(:current_user) { user }
+
+              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.first['id']).to eq(project.id)
+                expect(json_response.map { |project| project['id'] }).to eq(user_projects.map(&:id).sort.reverse)
               end
             end
           end
+        end
+      end
 
-          context 'non-admin user' do
-            let(:current_user) { user }
+      context 'by similarity', :aggregate_failures do
+        let_it_be(:group_with_projects) { create(:group) }
+        let_it_be(:project_1) { create(:project, name: 'Project', path: 'project', group: group_with_projects) }
+        let_it_be(:project_2) { create(:project, name: 'Test Project', path: 'test-project', group: group_with_projects) }
+        let_it_be(:project_3) { create(:project, name: 'Test', path: 'test', group: group_with_projects) }
+        let_it_be(:project_4) { create(:project, :public, name: 'Test Public Project') }
 
-            it 'returns projects ordered normally' do
-              get api('/projects', current_user), params: { order_by: order_by }
+        let(:current_user) { user }
+        let(:params) { { order_by: 'similarity', search: 'test' } }
 
-              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(user_projects.map(&:id).sort.reverse)
-            end
+        subject { get api('/projects', current_user), params: params }
+
+        before do
+          group_with_projects.add_owner(current_user)
+        end
+
+        it 'returns non-public items based ordered by similarity' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response).to include_pagination_headers
+          expect(json_response.length).to eq(2)
+
+          project_names = json_response.map { |proj| proj['name'] }
+          expect(project_names).to contain_exactly('Test', 'Test Project')
+        end
+
+        context 'when `search` parameter is not given' do
+          let(:params) { { order_by: 'similarity' } }
+
+          it 'returns items ordered by created_at descending' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(response).to include_pagination_headers
+            expect(json_response.length).to eq(8)
+
+            project_names = json_response.map { |proj| proj['name'] }
+            expect(project_names).to contain_exactly(project.name, project2.name, 'second_project', 'public_project', 'Project', 'Test Project', 'Test Public Project', 'Test')
+          end
+        end
+
+        context 'when called anonymously' do
+          let(:current_user) { nil }
+
+          it 'returns items ordered by created_at descending' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(response).to include_pagination_headers
+            expect(json_response.length).to eq(1)
+
+            project_names = json_response.map { |proj| proj['name'] }
+            expect(project_names).to contain_exactly('Test Public Project')
           end
         end
       end