diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index d325b57..c668351 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2026-10-17 03:36:52 UTC using RuboCop version 1.92.0. +# on 2026-10-17 03:42:44 UTC using RuboCop version 1.92.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -44,23 +44,23 @@ Lint/UselessAssignment: Exclude: - 'spec/mongoid/full_text_search_spec.rb' -# Offense count: 5 +# Offense count: 8 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes. Metrics/AbcSize: - Max: 111 + Max: 98 -# Offense count: 13 +# Offense count: 15 # Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns. # AllowedMethods: refine Metrics/BlockLength: Max: 644 -# Offense count: 5 +# Offense count: 6 # Configuration parameters: AllowedMethods, AllowedPatterns. Metrics/CyclomaticComplexity: - Max: 35 + Max: 21 -# Offense count: 5 +# Offense count: 8 # Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns. Metrics/MethodLength: Max: 53 @@ -68,12 +68,12 @@ Metrics/MethodLength: # Offense count: 1 # Configuration parameters: CountComments, CountAsOne. Metrics/ModuleLength: - Max: 262 + Max: 199 # Offense count: 5 # Configuration parameters: AllowedMethods, AllowedPatterns. Metrics/PerceivedComplexity: - Max: 37 + Max: 23 # Offense count: 1 Naming/ConstantName: @@ -124,12 +124,12 @@ Style/FormatString: Style/FormatStringToken: EnforcedStyle: unannotated -# Offense count: 2 +# Offense count: 1 Style/MultilineBlockChain: Exclude: - 'lib/mongoid/full_text_search.rb' -# Offense count: 2 +# Offense count: 1 # This cop supports unsafe autocorrection (--autocorrect-all). # Configuration parameters: EnforcedStyle, AllowedMethods, AllowedPatterns. # SupportedStyles: predicate, comparison @@ -138,12 +138,13 @@ Style/NumericPredicate: - 'spec/**/*' - 'lib/mongoid/full_text_search.rb' -# Offense count: 1 +# Offense count: 2 # Configuration parameters: AllowedMethods. # AllowedMethods: respond_to_missing? Style/OptionalBooleanParameter: Exclude: - 'lib/mongoid/full_text_search.rb' + - 'lib/mongoid/full_text_search/ngrams.rb' # Offense count: 1 # This cop supports unsafe autocorrection (--autocorrect-all). @@ -153,7 +154,7 @@ Style/SymbolProc: Exclude: - 'spec/models/filtered_artwork.rb' -# Offense count: 22 +# Offense count: 24 # This cop supports safe autocorrection (--autocorrect). # Configuration parameters: AllowHeredoc, AllowURI, AllowQualifiedName, URISchemes, AllowRBSInlineAnnotation, AllowCopDirectives, AllowedPatterns, SplitStrings. # URISchemes: http, https diff --git a/CHANGELOG.md b/CHANGELOG.md index 97da880..72c47e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ### 0.9.0 (Next) +* [#60](https://github.com/mongoid/mongoid_fulltext/pull/60): Extract ngram calculation and index-definition planning for maintainability without changing model APIs or search behavior - [@tomasc](https://github.com/tomasc), [@dblock](https://github.com/dblock). * [#59](https://github.com/mongoid/mongoid_fulltext/pull/59): Document and cover class-level indexing timeout options - [@dblock](https://github.com/dblock). * [#52](https://github.com/mongoid/mongoid_fulltext/pull/52): Fix full-text index creation before any documents exist - [@dblock](https://github.com/dblock). * [#58](https://github.com/mongoid/mongoid_fulltext/pull/58): Support localized fields with locale-tagged ngrams in one index collection; existing localized indexes must be rebuilt - [@tomasc](https://github.com/tomasc), [@dblock](https://github.com/dblock). diff --git a/lib/mongoid/full_text_search.rb b/lib/mongoid/full_text_search.rb index e51c7cb..aec73cc 100644 --- a/lib/mongoid/full_text_search.rb +++ b/lib/mongoid/full_text_search.rb @@ -7,8 +7,9 @@ require 'mongoid/full_text_search/indexable' end -require 'unicode_utils' require 'cgi' +require 'mongoid/full_text_search/ngrams' +require 'mongoid/full_text_search/index_definition' module Mongoid::FullTextSearch extend ActiveSupport::Concern @@ -109,26 +110,11 @@ def fulltext_search_ensure_indexes(index_name, config) coll.indexes.create('document_id' => 1) end - # The order of filters matters when the same index is used from two or more collections. - filter_indexes = (config[:filters] || []).map do |key, _value| - ["filter_values.#{key}", 1] - end.sort_by { |filter_index| filter_index[0] } - - index_definition = [['ngram', 1], ['score', -1]].concat(filter_indexes) - index_definition.unshift(['locale', 1]) if localized_fulltext_index?(config) - - # Since the definition of the index could have changed, we'll clean up by - # removing any indexes that aren't on the exact. - correct_keys = index_definition.map { |field_def| field_def[0] } - all_filter_keys = filter_indexes.map { |field_def| field_def[0] } - coll.indexes.each do |idef| - keys = idef['key'].keys - next unless keys.member?('ngram') - - all_filter_keys |= keys.find_all { |key| key.starts_with?('filter_values.') } - next unless keys & correct_keys != correct_keys - - Mongoid.logger.info "Dropping #{idef['name']} [#{keys & correct_keys} <=> #{correct_keys}]" if Mongoid.logger + existing_indexes = [] + coll.indexes.each { |index| existing_indexes << index } + definition = IndexDefinition.new(config[:filters], localized_fulltext_index?(config), existing_indexes) + definition.obsolete_indexes.each do |idef| + Mongoid.logger.info "Dropping #{idef['name']} [#{idef['key'].keys} <=> #{definition.keys.map(&:first)}]" if Mongoid.logger if Mongoid::Compatibility::Version.mongoid5_or_newer? coll.indexes.drop_one(idef['key']) else @@ -136,12 +122,7 @@ def fulltext_search_ensure_indexes(index_name, config) end end - if all_filter_keys.length > filter_indexes.length - filter_indexes = all_filter_keys.map { |key| [key, 1] }.sort_by { |filter_index| filter_index[0] } - index_definition = [['ngram', 1], ['score', -1]].concat(filter_indexes) - index_definition.unshift(['locale', 1]) if localized_fulltext_index?(config) - end - + index_definition = definition.keys Mongoid.logger.info "Ensuring fts_index on #{coll.name}: #{index_definition}" if Mongoid.logger if Mongoid::Compatibility::Version.mongoid5_or_newer? coll.indexes.create_one(Hash[index_definition], name: 'fts_index') @@ -243,81 +224,7 @@ def instantiate_mapreduce_results(results, options) end def all_ngrams(str, config, bound_number_returned = true) - return {} if str.nil? - - if config[:remove_accents] - if defined?(UnicodeUtils) - str = UnicodeUtils.nfkd(str) - elsif defined?(DiacriticsFu) - str = DiacriticsFu.escape(str) - end - end - - # Remove any characters that aren't in the alphabet and aren't word separators - alphabet = config[:alphabet] - filtered_str = str.mb_chars.downcase.to_s.split('').find_all do |ch| - (alphabet.is_a?(Regexp) ? alphabet =~ ch : alphabet[ch]) || config[:word_separators][ch] - end.join('') - - # Sample across the full string, including both endpoints when taking multiple ngrams. - # Three 3-letter ngrams from 'abcdefghijk' are 'abc', 'efg', and 'ijk'. - last_start = filtered_str.length - config[:ngram_width] - ngram_count = last_start + 1 - ngram_count = [ngram_count, config[:max_ngrams_to_search]].min if bound_number_returned - - # Create an array of records of the form {:ngram => x, :score => y} for all ngrams that occur in the - # input string using the sample positions. Let score(x,y) be the score of string x - # compared with string y - assigning scores to ngrams with the square root-based scoring function - # below and multiplying scores of matching ngrams together yields a score function that has the - # property that score(x,y) > score(x,z) for any string z containing y and score(x,y) > score(x,z) - # for any string z contained in y. - ngram_array = (0...ngram_count).map do |sample| - i = ngram_count == 1 ? 0 : (sample * last_start.to_f / (ngram_count - 1)).round - score = if i == 0 || (config[:apply_prefix_scoring_to_all_words] && \ - config[:word_separators].key?(filtered_str[i - 1].chr)) - Math.sqrt(1 + 1.0 / filtered_str.length) - else - Math.sqrt(2.0 / filtered_str.length) - end - { ngram: filtered_str[i..i + config[:ngram_width] - 1], score: score } - end - - # If an ngram appears multiple times in the query string, keep the max score - ngram_array = ngram_array.group_by { |h| h[:ngram] }.map { |key, values| { ngram: key, score: values.map { |v| v[:score] }.max } } - - if config[:index_short_prefixes] || config[:index_full_words] - split_regex_def = config[:word_separators].keys.map { |k| Regexp.escape(k) }.join - split_regex = Regexp.compile("[#{split_regex_def}]") - all_words = filtered_str.split(split_regex) - end - - # Add 'short prefix' records to the array: prefixes of the string that are length (ngram_width - 1) - if config[:index_short_prefixes] - prefixes_seen = {} - all_words.each do |word| - next if word.length < config[:ngram_width] - 1 - - prefix = word[0...config[:ngram_width] - 1] - if prefixes_seen[prefix].nil? && (config[:stop_words][word].nil? || word == filtered_str) - ngram_array << { ngram: prefix, score: 1 + 1.0 / filtered_str.length } - prefixes_seen[prefix] = true - end - end - end - - # Add records to the array of ngrams for each full word in the string that isn't a stop word - if config[:index_full_words] - full_words_seen = {} - all_words.each do |word| - if word.length > 1 && full_words_seen[word].nil? && (config[:stop_words][word].nil? || word == filtered_str) - ngram_array << { ngram: word, score: 1 + 1.0 / filtered_str.length } - full_words_seen[word] = true - end - end - end - - # If an ngram appears as any combination of full word, short prefix, and ngram, keep the sum of the two scores - Hash[ngram_array.group_by { |h| h[:ngram] }.map { |key, values| [key, values.map { |v| v[:score] }.sum] }] + Ngrams.calculate(str, config, bound_number_returned) end def remove_from_ngram_index diff --git a/lib/mongoid/full_text_search/index_definition.rb b/lib/mongoid/full_text_search/index_definition.rb new file mode 100644 index 0000000..8792f97 --- /dev/null +++ b/lib/mongoid/full_text_search/index_definition.rb @@ -0,0 +1,31 @@ +module Mongoid + module FullTextSearch + # Plans search keys and obsolete indexes while retaining shared-model filters. + class IndexDefinition + attr_reader :keys, :obsolete_indexes + + def initialize(filters, localized, existing_indexes) + filter_keys = (filters || {}).map { |key, _value| "filter_values.#{key}" }.sort + @keys = build_keys(filter_keys, localized) + correct_keys = @keys.map(&:first) + all_filter_keys = filter_keys.dup + @obsolete_indexes = existing_indexes.find_all do |index| + keys = index['key'].keys + next false unless keys.member?('ngram') + + all_filter_keys |= keys.find_all { |key| key.start_with?('filter_values.') } + keys & correct_keys != correct_keys + end + @keys = build_keys(all_filter_keys.sort, localized) if all_filter_keys.length > filter_keys.length + end + + private + + def build_keys(filter_keys, localized) + keys = [['ngram', 1], ['score', -1]].concat(filter_keys.map { |key| [key, 1] }) + keys.unshift(['locale', 1]) if localized + keys + end + end + end +end diff --git a/lib/mongoid/full_text_search/ngrams.rb b/lib/mongoid/full_text_search/ngrams.rb new file mode 100644 index 0000000..749ccd8 --- /dev/null +++ b/lib/mongoid/full_text_search/ngrams.rb @@ -0,0 +1,93 @@ +require 'active_support' +require 'active_support/core_ext/enumerable' +require 'active_support/core_ext/string/multibyte' +require 'unicode_utils' + +module Mongoid + module FullTextSearch + # Calculates query and document ngram scores without accessing persistence. + module Ngrams + module_function + + def calculate(str, config, bound_number_returned = true) + return {} if str.nil? + + filtered_str = normalize(str, config) + ngram_array = sliding_windows(filtered_str, config, bound_number_returned) + + # Repeated windows contribute their maximum score, not their sum. + ngram_array = ngram_array.group_by { |h| h[:ngram] }.map do |key, values| + { ngram: key, score: values.map { |v| v[:score] }.max } + end + + if config[:index_short_prefixes] || config[:index_full_words] + split_regex_def = config[:word_separators].keys.map { |k| Regexp.escape(k) }.join + all_words = filtered_str.split(Regexp.compile("[#{split_regex_def}]")) + ngram_array.concat(short_prefixes(all_words, filtered_str, config)) if config[:index_short_prefixes] + ngram_array.concat(full_words(all_words, filtered_str, config)) if config[:index_full_words] + end + + # Window, prefix, and full-word contributions add to the final score. + Hash[ngram_array.group_by { |h| h[:ngram] }.map { |key, values| [key, values.map { |v| v[:score] }.sum] }] + end + + def normalize(str, config) + if config[:remove_accents] + if defined?(UnicodeUtils) + str = UnicodeUtils.nfkd(str) + elsif defined?(DiacriticsFu) + str = DiacriticsFu.escape(str) + end + end + + alphabet = config[:alphabet] + str.mb_chars.downcase.to_s.split('').find_all do |ch| + (alphabet.is_a?(Regexp) ? alphabet =~ ch : alphabet[ch]) || config[:word_separators][ch] + end.join('') + end + + def sliding_windows(str, config, bound_number_returned) + last_start = str.length - config[:ngram_width] + ngram_count = last_start + 1 + ngram_count = [ngram_count, config[:max_ngrams_to_search]].min if bound_number_returned + + # Sample both endpoints: three windows from 'abcdefghijk' are 'abc', 'efg', 'ijk'. + (0...ngram_count).map do |sample| + i = ngram_count == 1 ? 0 : (sample * last_start.to_f / (ngram_count - 1)).round + score = if i.zero? || (config[:apply_prefix_scoring_to_all_words] && \ + config[:word_separators].key?(str[i - 1].chr)) + Math.sqrt(1 + 1.0 / str.length) + else + Math.sqrt(2.0 / str.length) + end + { ngram: str[i..i + config[:ngram_width] - 1], score: score } + end + end + + def short_prefixes(words, str, config) + prefixes_seen = {} + words.each_with_object([]) do |word, records| + next if word.length < config[:ngram_width] - 1 + + prefix = word[0...config[:ngram_width] - 1] + if prefixes_seen[prefix].nil? && (config[:stop_words][word].nil? || word == str) + records << { ngram: prefix, score: 1 + 1.0 / str.length } + prefixes_seen[prefix] = true + end + end + end + + def full_words(words, str, config) + full_words_seen = {} + words.each_with_object([]) do |word, records| + if word.length > 1 && full_words_seen[word].nil? && (config[:stop_words][word].nil? || word == str) + records << { ngram: word, score: 1 + 1.0 / str.length } + full_words_seen[word] = true + end + end + end + + private_class_method :normalize, :sliding_windows, :short_prefixes, :full_words + end + end +end diff --git a/spec/mongoid/ngram_sampling_spec.rb b/spec/mongoid/ngram_sampling_spec.rb index 3965dd6..bea3b0e 100644 --- a/spec/mongoid/ngram_sampling_spec.rb +++ b/spec/mongoid/ngram_sampling_spec.rb @@ -7,6 +7,11 @@ ) end + it 'preserves the model calculation entry point and its unbounded option' do + expect(Mongoid::FullTextSearch::Ngrams).to receive(:calculate).with('abcdef', config, false).and_call_original + expect(BasicArtwork.all_ngrams('abcdef', config, false).keys).to eq(%w[abc bcd cde def]) + end + it 'selects the documented evenly spaced sample including the last ngram' do expect(BasicArtwork.all_ngrams('abcdefghijk', config).keys).to eq(%w[abc efg ijk]) expect(BasicArtwork.all_ngrams('abcdefghij', config).keys).to eq(%w[abc efg hij]) diff --git a/spec/unit/index_definition_spec.rb b/spec/unit/index_definition_spec.rb new file mode 100644 index 0000000..26b931c --- /dev/null +++ b/spec/unit/index_definition_spec.rb @@ -0,0 +1,48 @@ +require 'rspec' +require 'mongoid/full_text_search/index_definition' + +describe Mongoid::FullTextSearch::IndexDefinition do + it 'builds the nonlocalized search keys without filters' do + definition = described_class.new(nil, false, []) + expect(definition.keys).to eq([['ngram', 1], ['score', -1]]) + expect(definition.obsolete_indexes).to eq([]) + end + + it 'sorts filters and prefixes localized keys with locale' do + definition = described_class.new({ z: true, a: true }, true, []) + expect(definition.keys).to eq([['locale', 1], ['ngram', 1], ['score', -1], ['filter_values.a', 1], ['filter_values.z', 1]]) + end + + it 'retains shared-model filters and identifies incomplete search indexes' do + old = { 'name' => 'fts_index', 'key' => { 'ngram' => 1, 'score' => -1, 'filter_values.z' => 1 } } + unrelated = { 'name' => 'document_id_1', 'key' => { 'document_id' => 1 } } + definition = described_class.new({ a: true }, true, [unrelated, old]) + expect(definition.keys).to eq([['locale', 1], ['ngram', 1], ['score', -1], ['filter_values.a', 1], ['filter_values.z', 1]]) + expect(definition.obsolete_indexes).to eq([old]) + end + + it 'preserves a complete index that includes other models filters' do + existing = { 'key' => { 'ngram' => 1, 'score' => -1, 'filter_values.a' => 1, 'filter_values.z' => 1 } } + definition = described_class.new({ a: true }, false, [existing]) + expect(definition.obsolete_indexes).to eq([]) + expect(definition.keys).to eq(existing['key'].to_a) + end + + it 'preserves direction-insensitive matching rather than changing index policy' do + existing = { 'key' => { 'ngram' => -1, 'score' => 1 } } + expect(described_class.new(nil, false, [existing]).obsolete_indexes).to eq([]) + end + + it 'identifies a search index whose keys are in the wrong order' do + existing = { 'key' => { 'score' => -1, 'ngram' => 1 } } + expect(described_class.new(nil, false, [existing]).obsolete_indexes).to eq([existing]) + end + + it 'does not mutate existing definitions or filter configuration' do + filters = { a: true }.freeze + existing = [{ 'key' => { 'ngram' => 1, 'filter_values.z' => 1 } }] + original = Marshal.dump(existing) + described_class.new(filters, true, existing) + expect(Marshal.dump(existing)).to eq(original) + end +end diff --git a/spec/unit/ngrams_spec.rb b/spec/unit/ngrams_spec.rb new file mode 100644 index 0000000..3f44891 --- /dev/null +++ b/spec/unit/ngrams_spec.rb @@ -0,0 +1,73 @@ +require 'rspec' +require 'mongoid/full_text_search/ngrams' + +describe Mongoid::FullTextSearch::Ngrams do + let(:config) do + { + alphabet: /[\p{L}\p{M}\p{N}]/, + word_separators: { ' ' => ' ', '-' => '-' }, + ngram_width: 3, + max_ngrams_to_search: 3, + remove_accents: true, + apply_prefix_scoring_to_all_words: true, + index_full_words: false, + index_short_prefixes: false, + stop_words: { 'the' => true } + } + end + + it 'handles nil and strings shorter than a window' do + [nil, '', 'ab'].each { |text| expect(described_class.calculate(text, config)).to eq({}) } + end + + it 'samples endpoints and preserves exact prefix and interior scores' do + expect(described_class.calculate('abcdefghijk', config)).to eq( + 'abc' => Math.sqrt(1 + 1.0 / 11), + 'efg' => Math.sqrt(2.0 / 11), + 'ijk' => Math.sqrt(2.0 / 11) + ) + end + + it 'uses every window for unbounded indexing' do + expect(described_class.calculate('abcdef', config, false).keys).to eq(%w[abc bcd cde def]) + end + + it 'keeps the maximum score of repeated windows rather than adding them' do + expect(described_class.calculate('aaaa', config, false)).to eq('aaa' => Math.sqrt(1 + 1.0 / 4)) + end + + it 'adds window and full-word scores for the same term only once each' do + options = config.merge(index_full_words: true) + expect(described_class.calculate('cat cat', options, false)).to eq( + 'cat' => Math.sqrt(1 + 1.0 / 7) + (1 + 1.0 / 7), + 'at ' => Math.sqrt(2.0 / 7), + 't c' => Math.sqrt(2.0 / 7), + ' ca' => Math.sqrt(2.0 / 7) + ) + end + + it 'combines short-prefix and full-word bonuses without duplicate prefixes' do + options = config.merge(ngram_width: 4, index_short_prefixes: true, index_full_words: true) + ngrams = described_class.calculate('cat cats', options, false) + expect(ngrams['cat']).to eq(2 * (1 + 1.0 / 8)) + end + + it 'suppresses stop-word bonuses unless the word is the whole input' do + options = config.merge(index_full_words: true, index_short_prefixes: true) + expect(described_class.calculate('the cat', options)['th']).to be_nil + expect(described_class.calculate('the cat', options)['the']).to eq(Math.sqrt(1 + 1.0 / 7)) + expect(described_class.calculate('the', options)['th']).to eq(1 + 1.0 / 3) + end + + it 'normalizes Unicode and preserves configured separators' do + expect(described_class.calculate("CAF\u00c9-\u4e2d\u6587", config).keys).to eq(['caf', "e\u0301-", "-\u4e2d\u6587"]) + end + + it 'does not mutate configuration or source text' do + text = 'Cat'.freeze + original = Marshal.dump(config) + described_class.calculate(text, config.freeze) + expect(Marshal.dump(config)).to eq(original) + expect(text).to eq('Cat') + end +end