From cd8596fe2b479282eddb220ed2e376fb738c989c Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu <heinrich@gitlab.com> Date: Thu, 21 Oct 2021 10:59:32 +0800 Subject: [PATCH] Use full text search when searching issues --- .../concerns/issuable_collections.rb | 4 +++ app/finders/issuable_finder.rb | 8 ++++++ .../concerns/pg_full_text_searchable.rb | 19 +++++++++++-- .../development/issues_full_text_search.yml | 8 ++++++ lib/gitlab/issuables_count_for_state.rb | 2 +- spec/controllers/dashboard_controller_spec.rb | 17 ++++++++++- .../projects/issues_controller_spec.rb | 16 ++++++++++- .../filtered_search/filter_issues_spec.rb | 2 ++ spec/finders/issues_finder_spec.rb | 23 +++++++++++++++ .../concerns/pg_full_text_searchable_spec.rb | 28 +++++++++++++++++++ 10 files changed, 122 insertions(+), 5 deletions(-) create mode 100644 config/feature_flags/development/issues_full_text_search.yml diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb index 4841225de08a..63ff1390d92c 100644 --- a/app/controllers/concerns/issuable_collections.rb +++ b/app/controllers/concerns/issuable_collections.rb @@ -117,6 +117,10 @@ def finder_options options[:attempt_group_search_optimizations] = true end + if collection_type == 'Issue' && Feature.enabled?(:issues_full_text_search, @project || @group, default_enabled: :yaml) + options[:attempt_full_text_search] = true + end + params.permit(finder_type.valid_params).merge(options) end end diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 3e436f309712..416db1c9c7dd 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -37,6 +37,7 @@ # attempt_project_search_optimizations: boolean # crm_contact_id: integer # crm_organization_id: integer +# attempt_full_text_search: boolean # class IssuableFinder prepend FinderWithCrossProjectAccess @@ -46,6 +47,7 @@ class IssuableFinder requires_cross_project_access unless: -> { params.project? } + FULL_TEXT_SEARCH_TERM_REGEX = /\A[\p{ASCII}|\p{Latin}]+\z/.freeze NEGATABLE_PARAMS_HELPER_KEYS = %i[project_id scope status include_subgroups].freeze attr_accessor :current_user, :params @@ -331,6 +333,8 @@ def by_search(items) return items if items.is_a?(ActiveRecord::NullRelation) return items if Feature.enabled?(:disable_anonymous_search, type: :ops) && current_user.nil? + return items.pg_full_text_search(search) if use_full_text_search? + if use_cte_for_search? cte = Gitlab::SQL::CTE.new(klass.table_name, items) @@ -341,6 +345,10 @@ def by_search(items) end # rubocop: enable CodeReuse/ActiveRecord + def use_full_text_search? + params[:attempt_full_text_search] && params[:search] =~ FULL_TEXT_SEARCH_TERM_REGEX + end + # rubocop: disable CodeReuse/ActiveRecord def by_iids(items) params[:iids].present? ? items.where(iid: params[:iids]) : items diff --git a/app/models/concerns/pg_full_text_searchable.rb b/app/models/concerns/pg_full_text_searchable.rb index fc88eff1c315..a7483e3f77c3 100644 --- a/app/models/concerns/pg_full_text_searchable.rb +++ b/app/models/concerns/pg_full_text_searchable.rb @@ -13,9 +13,9 @@ # This module sets up an after_commit hook that updates the search data # when the searchable columns are changed. # -# This also adds a `full_text_search` scope so you can do: +# This also adds a `pg_full_text_search` scope so you can do: # -# Model.full_text_search("some search term") +# Model.pg_full_text_search("some search term") module PgFullTextSearchable extend ActiveSupport::Concern @@ -82,5 +82,20 @@ def pg_full_text_searchable(columns:) update_search_data! end end + + def pg_full_text_search(search_term) + search_data_table = reflect_on_association(:search_data).klass.arel_table + + joins(:search_data).where( + Arel::Nodes::InfixOperation.new( + '@@', + search_data_table[:search_vector], + Arel::Nodes::NamedFunction.new( + 'websearch_to_tsquery', + [Arel::Nodes.build_quoted(TEXT_SEARCH_DICTIONARY), Arel::Nodes.build_quoted(search_term)] + ) + ) + ) + end end end diff --git a/config/feature_flags/development/issues_full_text_search.yml b/config/feature_flags/development/issues_full_text_search.yml new file mode 100644 index 000000000000..35af801e3ab3 --- /dev/null +++ b/config/feature_flags/development/issues_full_text_search.yml @@ -0,0 +1,8 @@ +--- +name: issues_full_text_search +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/71913 +rollout_issue_url: +milestone: '14.5' +type: development +group: group::project management +default_enabled: false diff --git a/lib/gitlab/issuables_count_for_state.rb b/lib/gitlab/issuables_count_for_state.rb index f11dd520d2da..42b549cf127b 100644 --- a/lib/gitlab/issuables_count_for_state.rb +++ b/lib/gitlab/issuables_count_for_state.rb @@ -144,7 +144,7 @@ def cache_options def params_include_filters? non_filtering_params = %i[ scope state sort group_id include_subgroups - attempt_group_search_optimizations non_archived issue_types + attempt_group_search_optimizations attempt_full_text_search non_archived issue_types ] finder.params.except(*non_filtering_params).values.any? diff --git a/spec/controllers/dashboard_controller_spec.rb b/spec/controllers/dashboard_controller_spec.rb index 8fae617ea650..dbeac9fd2408 100644 --- a/spec/controllers/dashboard_controller_spec.rb +++ b/spec/controllers/dashboard_controller_spec.rb @@ -13,7 +13,22 @@ end describe 'GET issues' do - it_behaves_like 'issuables list meta-data', :issue, :issues + context 'when issues_full_text_search is disabled' do + before do + stub_feature_flags(issues_full_text_search: false) + end + + it_behaves_like 'issuables list meta-data', :issue, :issues + end + + context 'when issues_full_text_search is enabled' do + before do + stub_feature_flags(issues_full_text_search: true) + end + + it_behaves_like 'issuables list meta-data', :issue, :issues + end + it_behaves_like 'issuables requiring filter', :issues end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index bf0b833b3111..c159768690e7 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -72,7 +72,21 @@ project.add_developer(user) end - it_behaves_like "issuables list meta-data", :issue + context 'when issues_full_text_search is disabled' do + before do + stub_feature_flags(issues_full_text_search: false) + end + + it_behaves_like 'issuables list meta-data', :issue + end + + context 'when issues_full_text_search is enabled' do + before do + stub_feature_flags(issues_full_text_search: true) + end + + it_behaves_like 'issuables list meta-data', :issue + end it_behaves_like 'set sort order from user preference' do let(:sorting_param) { 'updated_asc' } diff --git a/spec/features/issues/filtered_search/filter_issues_spec.rb b/spec/features/issues/filtered_search/filter_issues_spec.rb index edf3df7c16ef..2883030e9ac2 100644 --- a/spec/features/issues/filtered_search/filter_issues_spec.rb +++ b/spec/features/issues/filtered_search/filter_issues_spec.rb @@ -497,6 +497,8 @@ def expect_no_issues_list end it 'filters issues by searched text containing special characters' do + stub_feature_flags(issues_full_text_search: false) + issue = create(:issue, project: project, author: user, title: "issue with !@\#{$%^&*()-+") search = '!@#{$%^&*()-+' diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index c22e56c3b9ef..b00cdca34237 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -632,6 +632,29 @@ end end + context 'filtering by issue term using full-text search' do + let(:params) { { search: search_term, attempt_full_text_search: true } } + + let_it_be(:english) { create(:issue, project: project1, title: 'title', description: 'something english') } + let_it_be(:japanese) { create(:issue, project: project1, title: '日本語 title', description: 'another english description') } + + context 'with latin search term' do + let(:search_term) { 'title english' } + + it 'returns matching issues' do + expect(issues).to contain_exactly(english, japanese) + end + end + + context 'with non-latin search term' do + let(:search_term) { '日本語' } + + it 'returns matching issues' do + expect(issues).to contain_exactly(japanese) + end + end + end + context 'filtering by issues iids' do let(:params) { { iids: [issue3.iid] } } diff --git a/spec/models/concerns/pg_full_text_searchable_spec.rb b/spec/models/concerns/pg_full_text_searchable_spec.rb index 69f70b172ad2..3a6b7dbd1042 100644 --- a/spec/models/concerns/pg_full_text_searchable_spec.rb +++ b/spec/models/concerns/pg_full_text_searchable_spec.rb @@ -55,6 +55,34 @@ def self.name end end + describe '.pg_full_text_search' do + let(:english) { model_class.create!(title: 'title', description: 'something english') } + let(:with_accent) { model_class.create!(title: 'Jürgen', description: 'Ærøskøbing') } + let(:japanese) { model_class.create!(title: '日本語 title', description: 'another english description') } + + before do + model_class.pg_full_text_searchable columns: [{ name: 'title', weight: 'A' }, { name: 'description', weight: 'B' }] + + [english, with_accent, japanese].each(&:update_search_data!) + end + + it 'searches across all fields' do + expect(model_class.pg_full_text_search('title english')).to contain_exactly(english, japanese) + end + + it 'searches for exact term with quotes' do + expect(model_class.pg_full_text_search('"something english"')).to contain_exactly(english) + end + + it 'ignores accents' do + expect(model_class.pg_full_text_search('jurgen')).to contain_exactly(with_accent) + end + + it 'does not support searching by non-Latin characters' do + expect(model_class.pg_full_text_search('日本')).to be_empty + end + end + describe '#update_search_data!' do let(:model) { model_class.create!(title: 'title', description: 'description') } -- GitLab