From 256e1e7f0c6c8a0424ac8a192d5d349e066043ec Mon Sep 17 00:00:00 2001 From: maddievn Date: Wed, 7 Aug 2024 10:16:42 +0200 Subject: [PATCH 01/10] Add priority_searchable concern to select search type Changelog: changed EE: true # Conflicts: # ee/app/services/ee/search/global_service.rb # ee/app/services/ee/search/group_service.rb # ee/app/services/ee/search/project_service.rb --- .../concerns/search/priority_searchable.rb | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 ee/app/services/concerns/search/priority_searchable.rb diff --git a/ee/app/services/concerns/search/priority_searchable.rb b/ee/app/services/concerns/search/priority_searchable.rb new file mode 100644 index 00000000000000..514015b42cb896 --- /dev/null +++ b/ee/app/services/concerns/search/priority_searchable.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Search + module PrioritySearchable + include ::Search::Elasticsearchable + include ::Search::ZoektSearchable + + def execute + case search_type + when 'zoekt' + zoekt_search_results + when 'advanced' + elasticsearch_results + else + super + end + end + + def search_type + return params[:search_type] if params[:search_type] + return 'zoekt' if scope == 'blobs' && use_zoekt? + return 'advanced' if use_elasticsearch? + + 'basic' + end + end +end -- GitLab From 93181184bfd4dd2b3746f3fa89f11c34bf78a9a1 Mon Sep 17 00:00:00 2001 From: maddievn Date: Wed, 7 Aug 2024 17:00:58 +0200 Subject: [PATCH 02/10] Update frontend to show when advanced search is disabled --- app/assets/javascripts/search/topbar/components/app.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/search/topbar/components/app.vue b/app/assets/javascripts/search/topbar/components/app.vue index 416657056e7b0f..3eb9cefc109f90 100644 --- a/app/assets/javascripts/search/topbar/components/app.vue +++ b/app/assets/javascripts/search/topbar/components/app.vue @@ -42,7 +42,7 @@ export default { }; }, computed: { - ...mapState(['query', 'searchType', 'defaultBranchName', 'urlQuery']), + ...mapState(['query', 'searchType', 'advancedSearchAvailable', 'defaultBranchName', 'urlQuery']), search: { get() { return this.query ? this.query.search : ''; -- GitLab From f181d0e7ce0d89498ff02abd61e5cb150f95a4c3 Mon Sep 17 00:00:00 2001 From: maddievn Date: Wed, 7 Aug 2024 17:38:07 +0200 Subject: [PATCH 03/10] Add zoekt available --- app/assets/javascripts/search/topbar/components/app.vue | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/search/topbar/components/app.vue b/app/assets/javascripts/search/topbar/components/app.vue index 3eb9cefc109f90..0ba46ac906e234 100644 --- a/app/assets/javascripts/search/topbar/components/app.vue +++ b/app/assets/javascripts/search/topbar/components/app.vue @@ -42,7 +42,14 @@ export default { }; }, computed: { - ...mapState(['query', 'searchType', 'advancedSearchAvailable', 'defaultBranchName', 'urlQuery']), + ...mapState([ + 'query', + 'searchType', + 'advancedSearchAvailable', + 'zoektAvailable', + 'defaultBranchName', + 'urlQuery', + ]), search: { get() { return this.query ? this.query.search : ''; -- GitLab From ae04be456f42eb47321a775750b6143eb8efb97e Mon Sep 17 00:00:00 2001 From: maddievn Date: Tue, 13 Aug 2024 09:48:50 +0200 Subject: [PATCH 04/10] Apply reviewer feedback --- app/assets/javascripts/search/topbar/components/app.vue | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/assets/javascripts/search/topbar/components/app.vue b/app/assets/javascripts/search/topbar/components/app.vue index 0ba46ac906e234..992bb6aa21bd41 100644 --- a/app/assets/javascripts/search/topbar/components/app.vue +++ b/app/assets/javascripts/search/topbar/components/app.vue @@ -45,8 +45,6 @@ export default { ...mapState([ 'query', 'searchType', - 'advancedSearchAvailable', - 'zoektAvailable', 'defaultBranchName', 'urlQuery', ]), -- GitLab From 9f7194e777e36478acf5fe1c16ca5d3d723396f3 Mon Sep 17 00:00:00 2001 From: maddievn Date: Tue, 13 Aug 2024 10:11:49 +0200 Subject: [PATCH 05/10] Fix lint issue --- app/assets/javascripts/search/topbar/components/app.vue | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/app/assets/javascripts/search/topbar/components/app.vue b/app/assets/javascripts/search/topbar/components/app.vue index 992bb6aa21bd41..416657056e7b0f 100644 --- a/app/assets/javascripts/search/topbar/components/app.vue +++ b/app/assets/javascripts/search/topbar/components/app.vue @@ -42,12 +42,7 @@ export default { }; }, computed: { - ...mapState([ - 'query', - 'searchType', - 'defaultBranchName', - 'urlQuery', - ]), + ...mapState(['query', 'searchType', 'defaultBranchName', 'urlQuery']), search: { get() { return this.query ? this.query.search : ''; -- GitLab From e749d4f467ebd4cb225d8bf74222e7c910376de2 Mon Sep 17 00:00:00 2001 From: maddievn Date: Thu, 15 Aug 2024 10:23:14 +0200 Subject: [PATCH 06/10] Rename to AdvancedAndZoektSearchable --- .../concerns/search/priority_searchable.rb | 27 ------------------- 1 file changed, 27 deletions(-) delete mode 100644 ee/app/services/concerns/search/priority_searchable.rb diff --git a/ee/app/services/concerns/search/priority_searchable.rb b/ee/app/services/concerns/search/priority_searchable.rb deleted file mode 100644 index 514015b42cb896..00000000000000 --- a/ee/app/services/concerns/search/priority_searchable.rb +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -module Search - module PrioritySearchable - include ::Search::Elasticsearchable - include ::Search::ZoektSearchable - - def execute - case search_type - when 'zoekt' - zoekt_search_results - when 'advanced' - elasticsearch_results - else - super - end - end - - def search_type - return params[:search_type] if params[:search_type] - return 'zoekt' if scope == 'blobs' && use_zoekt? - return 'advanced' if use_elasticsearch? - - 'basic' - end - end -end -- GitLab From ed4b32976f7375d87a5a42b83e927ca3f189efe6 Mon Sep 17 00:00:00 2001 From: maddievn Date: Thu, 15 Aug 2024 11:03:13 +0200 Subject: [PATCH 07/10] Deprecate basic_search parameter and add documentation for search_type Changelog: changed EE: true --- doc/api/search.md | 4 ++++ doc/user/search/exact_code_search.md | 4 ++++ doc/user/search/index.md | 6 ++++++ .../concerns/search/elasticsearchable.rb | 2 -- .../concerns/search/zoekt_searchable.rb | 2 +- .../controllers/ee/search_controller_spec.rb | 2 +- .../concerns/search/elasticsearchable_spec.rb | 8 -------- ee/spec/services/search/group_service_spec.rb | 13 +++++++------ .../services/search/project_service_spec.rb | 19 ++++++++++++++----- .../search_service_shared_examples.rb | 11 ----------- lib/api/search.rb | 1 - spec/requests/api/search_spec.rb | 4 ++-- 12 files changed, 39 insertions(+), 37 deletions(-) diff --git a/doc/api/search.md b/doc/api/search.md index ff044079e03264..d3c17f45408af9 100644 --- a/doc/api/search.md +++ b/doc/api/search.md @@ -21,6 +21,10 @@ these additional scopes are available for the [advanced search](#advanced-search - `blobs` - `notes` +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/161999) in GitLab 17.4. + +You can disable using advanced search by specifying `search_type=basic`. + ## Advanced search API Search for a [term](../user/search/advanced_search.md#syntax) across the entire GitLab instance. diff --git a/doc/user/search/exact_code_search.md b/doc/user/search/exact_code_search.md index bffbd273df8d42..e85316255a0ac2 100644 --- a/doc/user/search/exact_code_search.md +++ b/doc/user/search/exact_code_search.md @@ -45,6 +45,10 @@ This feature is available for testing, but not ready for production use. With the Zoekt search API, you can use the [search API](../../api/search.md) for exact code search. When this feature is disabled, [advanced search](advanced_search.md) or [basic search](index.md) is used instead. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/161999) in GitLab 17.4. + +By specifying `search_type=zoekt`, `search_type=advanced` or `search_type=basic`, you can control which search type is used. + By default, the Zoekt search API is disabled on GitLab.com to avoid breaking changes. To request access to this feature, contact GitLab. diff --git a/doc/user/search/index.md b/doc/user/search/index.md index 8362d2e2051549..9824e0a5d47c28 100644 --- a/doc/user/search/index.md +++ b/doc/user/search/index.md @@ -21,6 +21,12 @@ For code search, GitLab uses these types in this order: or when you search against a non-default branch. This type does not support group or global search. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/161999) in GitLab 17.4. + +By specifying `search_type=zoekt`, `search_type=advanced` or `search_type=basic` as a URL parameter, you can control which search type is used. + +> - [Deprecated](https://link-to-issue) in GitLab 17.4. + When exact code search or advanced search is enabled, you can still use basic search by specifying the `basic_search=true` URL parameter. diff --git a/ee/app/services/concerns/search/elasticsearchable.rb b/ee/app/services/concerns/search/elasticsearchable.rb index 35a7de0c46382a..c103b05e0ede88 100644 --- a/ee/app/services/concerns/search/elasticsearchable.rb +++ b/ee/app/services/concerns/search/elasticsearchable.rb @@ -5,8 +5,6 @@ module Elasticsearchable SCOPES_ADVANCED_SEARCH_ALWAYS_ENABLED = %w[users].freeze def use_elasticsearch? - return false if params[:basic_search] - ::Gitlab::CurrentSettings.search_using_elasticsearch?(scope: elasticsearchable_scope) end diff --git a/ee/app/services/concerns/search/zoekt_searchable.rb b/ee/app/services/concerns/search/zoekt_searchable.rb index 21b37554b12cda..3acb1becc86a48 100644 --- a/ee/app/services/concerns/search/zoekt_searchable.rb +++ b/ee/app/services/concerns/search/zoekt_searchable.rb @@ -5,7 +5,7 @@ module ZoektSearchable def use_zoekt? # TODO: rename to search_code_with_zoekt? # https://gitlab.com/gitlab-org/gitlab/-/issues/421619 - return false if params[:basic_search] || skip_api? + return false if skip_api? return false unless ::Search::Zoekt.enabled_for_user?(current_user) && zoekt_searchable_scope? return false if Feature.enabled?(:disable_zoekt_search_for_saas, root_ancestor) diff --git a/ee/spec/controllers/ee/search_controller_spec.rb b/ee/spec/controllers/ee/search_controller_spec.rb index ef67393c3132d8..ab67fc2c0e0527 100644 --- a/ee/spec/controllers/ee/search_controller_spec.rb +++ b/ee/spec/controllers/ee/search_controller_spec.rb @@ -374,7 +374,7 @@ def request expect(payload[:metadata]['meta.search.type']).to eq(search_type) end - get :show, params: { search: 'hello world', basic_search: true } + get :show, params: { search: 'hello world', search_type: search_type } end end end diff --git a/ee/spec/services/concerns/search/elasticsearchable_spec.rb b/ee/spec/services/concerns/search/elasticsearchable_spec.rb index c1fbc35dfa7b46..9fe01622e26802 100644 --- a/ee/spec/services/concerns/search/elasticsearchable_spec.rb +++ b/ee/spec/services/concerns/search/elasticsearchable_spec.rb @@ -33,14 +33,6 @@ def elasticsearchable_scope allow(Gitlab::CurrentSettings).to receive(:search_using_elasticsearch?).and_return(true) end - context 'when basic_search param is passed in' do - let(:params) { { basic_search: true } } - - it 'is false' do - expect(class_instance).not_to be_use_elasticsearch - end - end - context 'when scope is epics' do let(:params) { { scope: 'epics' } } diff --git a/ee/spec/services/search/group_service_spec.rb b/ee/spec/services/search/group_service_spec.rb index c5354ac01356f3..449ff368334237 100644 --- a/ee/spec/services/search/group_service_spec.rb +++ b/ee/spec/services/search/group_service_spec.rb @@ -142,14 +142,12 @@ context 'when searching with Zoekt', :zoekt_settings_enabled do let(:service) do - described_class.new(user, group, search: 'foobar', scope: scope, - basic_search: basic_search, page: page, source: source) + described_class.new(user, group, search: 'foobar', scope: scope, page: page, source: source) end let(:source) { nil } let(:use_zoekt) { true } let(:scope) { 'blobs' } - let(:basic_search) { nil } let(:page) { nil } let(:zoekt_nodes) { create_list(:zoekt_node, 2) } let(:circuit_breaker) { instance_double(::Search::Zoekt::CircuitBreaker) } @@ -183,6 +181,7 @@ it 'returns a Search::Zoekt::SearchResults' do expect(service.use_zoekt?).to eq(true) + expect(service.search_type).to eq('zoekt') expect(service.zoekt_searchable_scope).to eq(group) expect(service.execute).to be_kind_of(::Search::Zoekt::SearchResults) end @@ -219,11 +218,13 @@ end end - context 'when basic_search is requested' do - let(:basic_search) { true } + context 'when basic search is requested' do + let(:service) do + described_class.new(user, group, search: 'foobar', scope: scope, page: page, source: source, search_type: 'basic') + end it 'does not search with Zoekt' do - expect(service.use_zoekt?).to eq(false) + expect(service.search_type).to eq('basic') expect(service.execute).not_to be_kind_of(::Search::Zoekt::SearchResults) end end diff --git a/ee/spec/services/search/project_service_spec.rb b/ee/spec/services/search/project_service_spec.rb index a013eda4065082..1a505dea2d1df5 100644 --- a/ee/spec/services/search/project_service_spec.rb +++ b/ee/spec/services/search/project_service_spec.rb @@ -97,7 +97,6 @@ project, search: 'foobar', scope: scope, - basic_search: basic_search, advanced_search: advanced_search, source: source ) @@ -106,7 +105,6 @@ let(:search_code_with_zoekt) { true } let(:user_preference_enabled_zoekt) { true } let(:scope) { 'blobs' } - let(:basic_search) { nil } let(:advanced_search) { nil } let(:zoekt_nodes) { create_list(:zoekt_node, 2) } let(:circuit_breaker) { instance_double(::Search::Zoekt::CircuitBreaker) } @@ -126,6 +124,7 @@ it 'searches with Zoekt' do expect(service.use_zoekt?).to eq(true) + expect(service.search_type).to eq('zoekt') expect(service.zoekt_searchable_scope).to eq(project) expect(service.execute).to be_kind_of(::Search::Zoekt::SearchResults) end @@ -173,11 +172,21 @@ end end - context 'when basic_search is requested' do - let(:basic_search) { true } + context 'when basic search is requested' do + let(:service) do + described_class.new( + (anonymous_user ? nil : user), + project, + search: 'foobar', + scope: scope, + advanced_search: advanced_search, + search_type: 'basic', + source: source + ) + end it 'does not search with Zoekt' do - expect(service.use_zoekt?).to eq(false) + expect(service.search_type).to eq('basic') expect(service.execute).not_to be_kind_of(::Search::Zoekt::SearchResults) end end diff --git a/ee/spec/support/shared_examples/services/search_service_shared_examples.rb b/ee/spec/support/shared_examples/services/search_service_shared_examples.rb index c0081eec741ee8..b5c6e991c87537 100644 --- a/ee/spec/support/shared_examples/services/search_service_shared_examples.rb +++ b/ee/spec/support/shared_examples/services/search_service_shared_examples.rb @@ -34,17 +34,6 @@ expect(service.use_elasticsearch?).to eq(:value) end - - context 'when requesting basic_search' do - let(:params) { { search: '*', basic_search: 'true' } } - - it 'returns false' do - expect(Gitlab::CurrentSettings) - .not_to receive(:search_using_elasticsearch?) - - expect(service.use_elasticsearch?).to eq(false) - end - end end describe '#execute' do diff --git a/lib/api/search.rb b/lib/api/search.rb index ad7f46ccae7af6..456d6f1f06774d 100644 --- a/lib/api/search.rb +++ b/lib/api/search.rb @@ -53,7 +53,6 @@ def search_service(additional_params = {}) state: params[:state], confidential: params[:confidential], snippets: snippets?, - basic_search: params[:basic_search], num_context_lines: params[:num_context_lines], search_type: params[:search_type], page: params[:page], diff --git a/spec/requests/api/search_spec.rb b/spec/requests/api/search_spec.rb index e21eb374be3f03..fbbc732ac24f3c 100644 --- a/spec/requests/api/search_spec.rb +++ b/spec/requests/api/search_spec.rb @@ -766,9 +766,9 @@ def request context 'when requesting basic search' do it 'passes the parameter to search service' do - expect(SearchService).to receive(:new).with(user, hash_including(basic_search: 'true')) + expect(SearchService).to receive(:new).with(user, hash_including(search_type: 'basic')) - get api(endpoint, user), params: { scope: 'issues', search: 'awesome', basic_search: 'true' } + get api(endpoint, user), params: { scope: 'issues', search: 'awesome', search_type: 'basic' } end end -- GitLab From 8382822499e40d99bb3243a9be441c4fdf4a062b Mon Sep 17 00:00:00 2001 From: Madelein van Niekerk Date: Mon, 2 Sep 2024 14:48:46 +0000 Subject: [PATCH 08/10] Apply 4 suggestion(s) to 2 file(s) Co-authored-by: Ashraf Khamis --- doc/user/search/exact_code_search.md | 6 ++---- doc/user/search/index.md | 13 +++++++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/doc/user/search/exact_code_search.md b/doc/user/search/exact_code_search.md index e85316255a0ac2..e65ee61c8d2413 100644 --- a/doc/user/search/exact_code_search.md +++ b/doc/user/search/exact_code_search.md @@ -43,11 +43,9 @@ For more information, see the history. This feature is available for testing, but not ready for production use. With the Zoekt search API, you can use the [search API](../../api/search.md) for exact code search. -When this feature is disabled, [advanced search](advanced_search.md) or [basic search](index.md) is used instead. -> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/161999) in GitLab 17.4. - -By specifying `search_type=zoekt`, `search_type=advanced` or `search_type=basic`, you can control which search type is used. +If you want to use [advanced search](advanced_search.md) or basic search instead, see +[specify a search type](index.md#specify-a-search-type). By default, the Zoekt search API is disabled on GitLab.com to avoid breaking changes. To request access to this feature, contact GitLab. diff --git a/doc/user/search/index.md b/doc/user/search/index.md index 9824e0a5d47c28..b54c58264bf44c 100644 --- a/doc/user/search/index.md +++ b/doc/user/search/index.md @@ -21,14 +21,19 @@ For code search, GitLab uses these types in this order: or when you search against a non-default branch. This type does not support group or global search. +## Specify a search type + > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/161999) in GitLab 17.4. -By specifying `search_type=zoekt`, `search_type=advanced` or `search_type=basic` as a URL parameter, you can control which search type is used. +To specify a search type, set the `search_type` URL parameter: + +- `search_type=zoekt` for [exact code search](exact_code_search.md) +- `search_type=advanced` for [advanced search](advanced_search.md) +- `search_type=basic` for basic search -> - [Deprecated](https://link-to-issue) in GitLab 17.4. +`search_type` replaces the deprecated `basic_search` parameter. +For more information, see [issue 477333](https://gitlab.com/gitlab-org/gitlab/-/issues/477333). -When exact code search or advanced search is enabled, you can still use -basic search by specifying the `basic_search=true` URL parameter. ## Global search scopes -- GitLab From 17d81a7aea0357430b0ba782dbe6d636544cc96e Mon Sep 17 00:00:00 2001 From: Madelein van Niekerk Date: Mon, 2 Sep 2024 14:55:30 +0000 Subject: [PATCH 09/10] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: Ashraf Khamis --- doc/api/search.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/doc/api/search.md b/doc/api/search.md index d3c17f45408af9..cec57e76352827 100644 --- a/doc/api/search.md +++ b/doc/api/search.md @@ -21,9 +21,8 @@ these additional scopes are available for the [advanced search](#advanced-search - `blobs` - `notes` -> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/161999) in GitLab 17.4. - -You can disable using advanced search by specifying `search_type=basic`. +If you want to use basic search instead, see +[specify a search type](../user/search/index.md#specify-a-search-type). ## Advanced search API -- GitLab From 857d76e224b67ae41a0f95be76764f6c203e093e Mon Sep 17 00:00:00 2001 From: Madelein van Niekerk Date: Tue, 3 Sep 2024 07:07:10 +0000 Subject: [PATCH 10/10] Apply 3 suggestion(s) to 2 file(s) Co-authored-by: Ashraf Khamis --- doc/user/search/exact_code_search.md | 1 - doc/user/search/index.md | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/doc/user/search/exact_code_search.md b/doc/user/search/exact_code_search.md index e65ee61c8d2413..99280226b7d14e 100644 --- a/doc/user/search/exact_code_search.md +++ b/doc/user/search/exact_code_search.md @@ -43,7 +43,6 @@ For more information, see the history. This feature is available for testing, but not ready for production use. With the Zoekt search API, you can use the [search API](../../api/search.md) for exact code search. - If you want to use [advanced search](advanced_search.md) or basic search instead, see [specify a search type](index.md#specify-a-search-type). diff --git a/doc/user/search/index.md b/doc/user/search/index.md index b54c58264bf44c..5fd5376e2ce2fe 100644 --- a/doc/user/search/index.md +++ b/doc/user/search/index.md @@ -25,7 +25,7 @@ For code search, GitLab uses these types in this order: > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/161999) in GitLab 17.4. -To specify a search type, set the `search_type` URL parameter: +To specify a search type, set the `search_type` URL parameter as follows: - `search_type=zoekt` for [exact code search](exact_code_search.md) - `search_type=advanced` for [advanced search](advanced_search.md) @@ -34,7 +34,6 @@ To specify a search type, set the `search_type` URL parameter: `search_type` replaces the deprecated `basic_search` parameter. For more information, see [issue 477333](https://gitlab.com/gitlab-org/gitlab/-/issues/477333). - ## Global search scopes DETAILS: -- GitLab