diff --git a/app/assets/javascripts/search/topbar/components/app.vue b/app/assets/javascripts/search/topbar/components/app.vue index 416657056e7b0f410e6b953372d774c96c1096de..3aa699a8725496ab0a4c630958d822d71b13c150 100644 --- a/app/assets/javascripts/search/topbar/components/app.vue +++ b/app/assets/javascripts/search/topbar/components/app.vue @@ -1,7 +1,7 @@ <script> import { GlButton } from '@gitlab/ui'; // eslint-disable-next-line no-restricted-imports -import { mapState, mapActions } from 'vuex'; +import { mapState, mapActions, mapGetters } from 'vuex'; import { InternalEvents } from '~/tracking'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { s__ } from '~/locale'; @@ -13,6 +13,7 @@ import { LS_REGEX_HANDLE, } from '~/search/store/constants'; import { loadDataFromLS } from '~/search/store/utils'; +import { SCOPE_BLOB } from '~/search/sidebar/constants'; import { SYNTAX_OPTIONS_ADVANCED_DOCUMENT, SYNTAX_OPTIONS_ZOEKT_DOCUMENT } from '../constants'; import SearchTypeIndicator from './search_type_indicator.vue'; @@ -43,6 +44,7 @@ export default { }, computed: { ...mapState(['query', 'searchType', 'defaultBranchName', 'urlQuery']), + ...mapGetters(['currentScope']), search: { get() { return this.query ? this.query.search : ''; @@ -68,6 +70,13 @@ export default { isRegexButtonVisible() { return this.searchType === ZOEKT_SEARCH_TYPE && this.isDefaultBranch; }, + isMultiMatch() { + return ( + this.glFeatures.zoektMultimatchFrontend && + this.searchType === ZOEKT_SEARCH_TYPE && + this.currentScope === SCOPE_BLOB + ); + }, }, created() { this.preloadStoredFrequentItems(); @@ -89,6 +98,11 @@ export default { this.applyQuery(); } }, + submitSearch() { + if (!this.isMultiMatch) { + this.applyQuery(); + } + }, }, }; </script> @@ -114,7 +128,7 @@ export default { :regex-button-state="regexEnabled" :regex-button-handler="regexButtonHandler" :placeholder="$options.i18n.searchPlaceholder" - @keydown.enter.stop.prevent="applyQuery" + @keydown.enter.stop.prevent="submitSearch" /> </div> </section> diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 9e4c69e467bee8911be5f9a950612eb25a1c8733..bd136b38d743ff3b1d1da3b7623df80e674cf069 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -65,35 +65,10 @@ def show @search_level = @search_service_presenter.level @search_type = search_type + @scope = @search_service_presenter.scope - @global_search_duration_s = Benchmark.realtime do - @scope = @search_service_presenter.scope - @search_results = @search_service_presenter.search_results - @search_objects = @search_service_presenter.search_objects - @search_highlight = @search_service_presenter.search_highlight - end - - return if @search_results.respond_to?(:failed?) && @search_results.failed?(@scope) - - Gitlab::Metrics::GlobalSearchSlis.record_apdex( - elapsed: @global_search_duration_s, - search_type: @search_type, - search_level: @search_level, - search_scope: @scope - ) - - increment_search_counters - ensure - if @search_type - # If we raise an error somewhere in the @global_search_duration_s benchmark block, we will end up here - # with a 200 status code, but an empty @global_search_duration_s. - Gitlab::Metrics::GlobalSearchSlis.record_error_rate( - error: @global_search_duration_s.nil? || (status < 200 || status >= 400), - search_type: @search_type, - search_level: @search_level, - search_scope: @scope - ) - end + # separate following lines to method that is conditionally triggered when not zoekt multi-result search + haml_search_results unless multi_match?(scope: @scope, search_type: search_type) end def count @@ -156,6 +131,40 @@ def opensearch; end private + def multi_match?(search_type:, scope:) # rubocop: disable Lint/UnusedMethodArgument -- This is being overridden in EE + false + end + + def haml_search_results + @global_search_duration_s = Benchmark.realtime do + @search_results = @search_service_presenter.search_results + @search_objects = @search_service_presenter.search_objects + @search_highlight = @search_service_presenter.search_highlight + end + + return if @search_results.respond_to?(:failed?) && @search_results.failed?(@scope) + + Gitlab::Metrics::GlobalSearchSlis.record_apdex( + elapsed: @global_search_duration_s, + search_type: @search_type, + search_level: @search_level, + search_scope: @scope + ) + + increment_search_counters + ensure + if @search_type + # If we raise an error somewhere in the @global_search_duration_s benchmark block, we will end up here + # with a 200 status code, but an empty @global_search_duration_s. + Gitlab::Metrics::GlobalSearchSlis.record_error_rate( + error: @global_search_duration_s.nil? || (status < 200 || status >= 400), + search_type: @search_type, + search_level: @search_level, + search_scope: @scope + ) + end + end + def update_scope_for_code_search return if params[:scope] == 'blobs' return unless params[:search].present? diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index e7f86c368ed6fbf89387fd1cdf1743ec10ec7039..75362adbc83bb19f1bba858e2cb06b68b7c4ac00 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -258,6 +258,12 @@ def should_show_zoekt_results?(_scope, _search_type) private + def formatted_count(scope) + return "0" if @timeout + + @search_results&.formatted_count(scope) || "0" + end + # Autocomplete results for various settings pages def default_autocomplete [ @@ -446,7 +452,7 @@ def search_filter_link(scope, label, data: {}, search: {}) if @scope == scope li_class = 'active' - count = @timeout ? 0 : @search_results.formatted_count(scope) + count = formatted_count(scope) else badge_class = 'js-search-count hidden' badge_data = { url: search_count_path(search_params) } @@ -469,7 +475,7 @@ def search_filter_link_json(scope, label, data, search) result = { label: label, scope: scope_name, data: data, link: search_path(search_params), active: active_scope } if active_scope - result[:count] = !@timeout ? @search_results.formatted_count(scope_name) : "0" + result[:count] = formatted_count(scope_name) end result[:count_link] = search_count_path(search_params) unless active_scope diff --git a/ee/app/controllers/ee/search_controller.rb b/ee/app/controllers/ee/search_controller.rb index 51fcfe0e63e9ea44a2751a03ea6b3a86fcbafee2..8bc3036c380a5b997128b72d0f9579a27f1b0450 100644 --- a/ee/app/controllers/ee/search_controller.rb +++ b/ee/app/controllers/ee/search_controller.rb @@ -79,6 +79,11 @@ def aggregations private + override :multi_match? + def multi_match?(search_type:, scope:) + scope == 'blobs' && search_type == 'zoekt' && ::Feature.enabled?(:zoekt_multimatch_frontend, current_user) + end + override :default_sort def default_sort if search_service.use_elasticsearch? diff --git a/ee/spec/controllers/ee/search_controller_spec.rb b/ee/spec/controllers/ee/search_controller_spec.rb index ab67fc2c0e05275cdf917ab0b5d5d17a70abb0df..af3f38950a8b3d7eeeaa2b28ff6081ae8aee6001 100644 --- a/ee/spec/controllers/ee/search_controller_spec.rb +++ b/ee/spec/controllers/ee/search_controller_spec.rb @@ -162,6 +162,41 @@ end end end + + context 'when zoekt is enabled' do + let_it_be(:group) { create(:group) } + + before do + stub_ee_application_setting(zoekt_search_enabled: true) + allow(user).to receive(:has_zoekt_indexed_namespace?).and_return(true) + end + + context 'when multi match should be returned' do + before do + allow_next_instance_of(SearchService) do |search_service| + allow(search_service).to receive(:search_type).and_return('zoekt') + allow(search_service).to receive(:use_zoekt?).and_return(true) + allow(search_service).to receive(:scope).and_return('blobs') + end + end + + it 'does not call haml_search_results' do + expect(controller).not_to receive(:haml_search_results) + get :show, params: { scope: 'blobs', search: 'test', group_id: group.id } + end + + context 'when the feature flag is disabled' do + before do + stub_feature_flags(zoekt_multimatch_frontend: false) + end + + it 'calls haml_search_results' do + expect(controller).to receive(:haml_search_results) + get :show, params: { scope: 'blobs', search: 'test', group_id: group.id } + end + end + end + end end describe 'GET #autocomplete' do @@ -384,4 +419,64 @@ def request def context(event) [Gitlab::Tracking::ServicePingContext.new(data_source: :redis_hll, event: event).to_context.to_json] end + + describe '#multi_match?' do + subject(:controller_instance) { described_class.new } + + let(:current_user) { user } + let(:scope) { 'blobs' } + let(:search_type) { 'zoekt' } + + before do + allow(controller_instance).to receive(:current_user).and_return(current_user) + end + + context 'when the feature flag is enabled, scope is "blobs", and search_type is "zoekt"' do + before do + allow(Feature).to receive(:enabled?).with(:zoekt_multimatch_frontend, current_user).and_return(true) + end + + it 'returns true' do + result = controller_instance.send(:multi_match?, search_type: search_type, scope: scope) + expect(result).to be(true) + end + end + + context 'when the feature flag is disabled' do + before do + allow(Feature).to receive(:enabled?).with(:zoekt_multimatch_frontend, current_user).and_return(false) + end + + it 'returns false' do + result = controller_instance.send(:multi_match?, search_type: search_type, scope: scope) + expect(result).to be(false) + end + end + + context 'when scope is not "blobs"' do + let(:scope) { 'other_scope' } + + before do + allow(Feature).to receive(:enabled?).with(:zoekt_multimatch_frontend, current_user).and_return(true) + end + + it 'returns false' do + result = controller_instance.send(:multi_match?, search_type: search_type, scope: scope) + expect(result).to be(false) + end + end + + context 'when search_type is not "zoekt"' do + let(:search_type) { 'other_search' } + + before do + allow(Feature).to receive(:enabled?).with(:zoekt_multimatch_frontend, current_user).and_return(true) + end + + it 'returns false' do + result = controller_instance.send(:multi_match?, search_type: search_type, scope: scope) + expect(result).to be(false) + end + end + end end diff --git a/ee/spec/features/search/elastic/project_search_spec.rb b/ee/spec/features/search/elastic/project_search_spec.rb index 29b33a5b073406bcca589333e68f30acb5a1e7c0..4e9e9b2047b9f60552b9999d0289e964d6425d70 100644 --- a/ee/spec/features/search/elastic/project_search_spec.rb +++ b/ee/spec/features/search/elastic/project_search_spec.rb @@ -160,14 +160,19 @@ let(:results) { Search::Zoekt::SearchResults.new(user, query, ::Project.id_in(project.id), node_id: zoekt_node.id) } before do + sign_in(user) + # this tests tests only non-multimatch search + # multimatch search is tested in jest + stub_feature_flags(zoekt_multimatch_frontend: false) + allow_next_instance_of(SearchService) do |service| allow(service).to receive(:search_service).and_return(search_service) allow(service).to receive(:show_epics?).and_return(false) allow(service).to receive(:search_results).and_return(results) - allow(::Gitlab::Search::Zoekt::Client.instance).to receive(:search) - .and_return(Gitlab::Search::Zoekt::Response.new({ Error: 'failed to parse query' })) end + allow(::Gitlab::Search::Zoekt::Client.instance).to receive(:search) + .and_return(Gitlab::Search::Zoekt::Response.new({ Error: 'failed to parse query' })) visit search_path(search: query, project_id: project.id) end diff --git a/ee/spec/features/search/zoekt/search_spec.rb b/ee/spec/features/search/zoekt/search_spec.rb index 7c5800e03231e04a574cc4ef233ee4523e6baedf..ee8cca64ea03a3cc85a7c2deb06da4cc40db1e2d 100644 --- a/ee/spec/features/search/zoekt/search_spec.rb +++ b/ee/spec/features/search/zoekt/search_spec.rb @@ -52,11 +52,11 @@ def choose_project(project) end it 'finds files with a regex search and allows filtering down again by project' do - # Temporary: There is no results in the current version with the FF true - # WIP stub_feature_flags(zoekt_multimatch_frontend: false) - submit_search('user.*egex') + select_search_scope('Code') + wait_for_all_requests + submit_search('user.*egex') expect(page).to have_selector('.file-content .blob-content', count: 2) expect(page).to have_button('Copy file path') @@ -75,8 +75,13 @@ def choose_project(project) end it 'displays that exact code search is enabled' do - submit_search('test') + stub_feature_flags(zoekt_multimatch_frontend: false) + + choose_project(project1) + select_search_scope('Code') + submit_search('test') + expect(page).to have_link('Exact code search (powered by Zoekt)', href: help_page_path('user/search/exact_code_search.md')) end diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index 21caa4de6d39cdfbf5ebf9f6dfeeb8666618a348..d96bd6c53b58e932e795f1aa9402817494a7e841 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -789,4 +789,26 @@ def request end end end + + context 'when CE edition', unless: Gitlab.ee? do + describe '#multi_match?' do + using RSpec::Parameterized::TableSyntax + + where(:search_type, :scope) do + 'basic' | 'blobs' + 'advanced' | 'blobs' + 'zoekt' | 'blobs' + 'basic' | 'issues' + 'advanced' | 'issues' + 'zoekt' | 'issues' + end + + with_them do + it 'returns false' do + result = subject.send(:multi_match?, search_type: search_type, scope: scope) + expect(result).to be(false) + end + end + end + end end diff --git a/spec/frontend/search/topbar/components/app_spec.js b/spec/frontend/search/topbar/components/app_spec.js index df3d0f86b1f47df1455a8293e1c688d7cda79307..90a299268f8335028085390d3e06281ef2e0c499 100644 --- a/spec/frontend/search/topbar/components/app_spec.js +++ b/spec/frontend/search/topbar/components/app_spec.js @@ -31,19 +31,32 @@ describe('GlobalSearchTopbar', () => { preloadStoredFrequentItems: jest.fn(), }; - const createComponent = (initialState = {}, defaultBranchName = '', stubs = {}) => { + const getterSpies = { + currentScope: jest.fn(), + }; + + const createComponent = ({ + initialState = {}, + defaultBranchName = '', + stubs = {}, + featureFlag = {}, + } = {}) => { const store = new Vuex.Store({ state: { query: MOCK_QUERY, ...initialState, }, actions: actionSpies, + getters: getterSpies, }); wrapper = shallowMount(GlobalSearchTopbar, { store, propsData: { defaultBranchName }, stubs, + provide: { + glFeatures: featureFlag, + }, }); }; @@ -67,9 +80,11 @@ describe('GlobalSearchTopbar', () => { `('Seachbox options for searchType: $searchType', ({ searchType, hasRegexSearch }) => { beforeEach(() => { createComponent({ - query: { repository_ref: 'master' }, - searchType, - defaultBranchName: 'master', + initialState: { + query: { repository_ref: 'master' }, + searchType, + defaultBranchName: 'master', + }, }); }); @@ -93,7 +108,7 @@ describe('GlobalSearchTopbar', () => { ${'zoekt'} | ${true} `('syntax options drawer with searchType: $searchType', ({ searchType, showSyntaxOptions }) => { beforeEach(() => { - createComponent({ query: { repository_ref: '' }, searchType }); + createComponent({ initialState: { query: { repository_ref: '' }, searchType } }); }); it('renders button correctly', () => { @@ -111,7 +126,7 @@ describe('GlobalSearchTopbar', () => { ${'zoekt'} | ${SYNTAX_OPTIONS_ZOEKT_DOCUMENT} `('syntax options drawer with searchType: $searchType', ({ searchType, documentPath }) => { beforeEach(() => { - createComponent({ query: { repository_ref: '' }, searchType }); + createComponent({ initialState: { query: { repository_ref: '' }, searchType } }); }); it('renders drawer with correct document', () => { @@ -123,10 +138,13 @@ describe('GlobalSearchTopbar', () => { it('dispatched correct click action', () => { const drawerToggleSpy = jest.fn(); - createComponent({ query: { repository_ref: '' }, searchType: 'advanced' }, '', { - MarkdownDrawer: stubComponent(MarkdownDrawer, { - methods: { toggleDrawer: drawerToggleSpy }, - }), + createComponent({ + initialState: { query: { repository_ref: '' }, searchType: 'advanced' }, + stubs: { + MarkdownDrawer: stubComponent(MarkdownDrawer, { + methods: { toggleDrawer: drawerToggleSpy }, + }), + }, }); findSyntaxOptionButton().vm.$emit('click'); @@ -148,7 +166,10 @@ describe('GlobalSearchTopbar', () => { `the syntax option based on component state`, ({ state, defaultBranchName, hasSyntaxOptions }) => { beforeEach(() => { - createComponent({ ...state }, defaultBranchName); + createComponent({ + initialState: { ...state }, + defaultBranchName, + }); }); describe(`repository: ${state.query.repository_ref}, searchType: ${state.searchType}`, () => { @@ -165,41 +186,57 @@ describe('GlobalSearchTopbar', () => { }); describe('actions', () => { - beforeEach(() => { - createComponent(); - }); - - it('clicking search button inside search box calls applyQuery', async () => { - await nextTick(); + describe.each` + FF | scope | searchType | called + ${{ zoektMultimatchFrontend: false }} | ${'blobs'} | ${'zoekt'} | ${true} + ${{ zoektMultimatchFrontend: false }} | ${'issues'} | ${'advanced'} | ${true} + ${{ zoektMultimatchFrontend: false }} | ${'blobs'} | ${'advanced'} | ${true} + ${{ zoektMultimatchFrontend: true }} | ${'issues'} | ${'advanced'} | ${true} + ${{ zoektMultimatchFrontend: true }} | ${'issues'} | ${'advanced'} | ${true} + ${{ zoektMultimatchFrontend: true }} | ${'blobs'} | ${'zoekt'} | ${false} + `('hitting enter inside search box', ({ FF, scope, searchType, called }) => { + beforeEach(() => { + getterSpies.currentScope = jest.fn(() => scope); + createComponent({ + featureFlag: FF, + initialState: { searchType }, + }); + }); - findGlSearchBox().vm.$emit('keydown', new KeyboardEvent({ key: ENTER_KEY })); - expect(actionSpies.applyQuery).toHaveBeenCalled(); + it(`calls applyQuery ${called ? '' : 'NOT '}`, async () => { + await nextTick(); + findGlSearchBox().vm.$emit('keydown', new KeyboardEvent({ key: ENTER_KEY })); + expect(actionSpies.applyQuery).toHaveBeenCalledTimes(called ? 1 : 0); + }); }); - }); - describe.each` - search | reload - ${''} | ${0} - ${'test'} | ${1} - `('clicking regular expression button', ({ search, reload }) => { - beforeEach(() => { - createComponent({ query: { search }, searchType: 'zoekt' }, '', { GlSearchBoxByType }); - }); + describe.each` + search | reload + ${''} | ${0} + ${'test'} | ${1} + `('clicking regular expression button', ({ search, reload }) => { + beforeEach(() => { + createComponent({ + initialState: { query: { search }, searchType: 'zoekt' }, + stubs: { GlSearchBoxByType }, + }); + }); - it(`calls setQuery and ${!reload ? 'NOT ' : ''}applyQuery if there is a search term`, () => { - findRegulareExpressionToggle().vm.$emit('click'); - expect(actionSpies.setQuery).toHaveBeenCalled(); - expect(actionSpies.applyQuery).toHaveBeenCalledTimes(reload); + it(`calls setQuery and ${!reload ? 'NOT ' : ''}applyQuery if there is a search term`, () => { + findRegulareExpressionToggle().vm.$emit('click'); + expect(actionSpies.setQuery).toHaveBeenCalled(); + expect(actionSpies.applyQuery).toHaveBeenCalledTimes(reload); + }); }); - }); - describe('onCreate', () => { - beforeEach(() => { - createComponent(); - }); + describe('onCreate', () => { + beforeEach(() => { + createComponent(); + }); - it('calls preloadStoredFrequentItems', () => { - expect(actionSpies.preloadStoredFrequentItems).toHaveBeenCalled(); + it('calls preloadStoredFrequentItems', () => { + expect(actionSpies.preloadStoredFrequentItems).toHaveBeenCalled(); + }); }); }); }); diff --git a/spec/helpers/search_helper_spec.rb b/spec/helpers/search_helper_spec.rb index 2d694293af5e16a71e10ac264242c1aa7ccfc88b..270cb7d09b8e0002511aad722ae7343258f195ef 100644 --- a/spec/helpers/search_helper_spec.rb +++ b/spec/helpers/search_helper_spec.rb @@ -1236,4 +1236,34 @@ def simple_sanitize(str) expect(should_show_zoekt_results?(:some_scope, :some_type)).to be false end end + + describe '#formatted_count' do + context 'when @timeout is set' do + it 'returns "0"' do + @timeout = true + @scope = 'projects' + + expect(formatted_count(@scope)).to eq("0") + end + end + + context 'when @search_results is defined' do + it 'delegates formatted_count to @search_results' do + @scope = 'projects' + @search_results = double + + allow(@search_results).to receive(:formatted_count).with(@scope) + expect(@search_results).to receive(:formatted_count).with(@scope) + + formatted_count(@scope) + end + end + + context 'when @search_results is not defined' do + it 'returns "0"' do + @scope = 'projects' + expect(formatted_count(@scope)).to eq("0") + end + end + end end