From fc7bbac22cc20a77065dd5349338b8f696b5d18e Mon Sep 17 00:00:00 2001 From: ellnix <103502144+ellnix@users.noreply.github.com> Date: Wed, 28 Feb 2024 16:17:57 +0100 Subject: [PATCH 01/36] Remove redundant Ebook model in specs --- .../attributes_have_changed_spec.rb | 50 +++++++++++++++++++ spec/integration_spec.rb | 44 ---------------- spec/support/active_record_classes.rb | 22 -------- 3 files changed, 50 insertions(+), 66 deletions(-) create mode 100644 spec/integration/active_record/attributes_have_changed_spec.rb diff --git a/spec/integration/active_record/attributes_have_changed_spec.rb b/spec/integration/active_record/attributes_have_changed_spec.rb new file mode 100644 index 00000000..debd6f14 --- /dev/null +++ b/spec/integration/active_record/attributes_have_changed_spec.rb @@ -0,0 +1,50 @@ +require 'support/models/color' +require 'support/models/book' + +describe 'When record attributes have changed' do + it 'detects attribute changes' do + color = Color.new name: 'dark-blue', short_name: 'blue' + + expect(Color.ms_must_reindex?(color)).to be(true) + color.save + expect(Color.ms_must_reindex?(color)).to be(false) + + color.hex = 123_456 + expect(Color.ms_must_reindex?(color)).to be(false) + + color.not_indexed = 'strstr' + expect(Color.ms_must_reindex?(color)).to be(false) + color.name = 'red' + expect(Color.ms_must_reindex?(color)).to be(true) + color.delete + end + + it 'detects attribute changes even in a transaction' do + color = Color.new name: 'dark-blue', short_name: 'blue' + color.save + expect(color.instance_variable_get('@ms_must_reindex')).to be_nil + Color.transaction do + color.name = 'red' + color.save + color.not_indexed = 'strstr' + color.save + expect(color.instance_variable_get('@ms_must_reindex')).to be(true) + end + expect(color.instance_variable_get('@ms_must_reindex')).to be_nil + color.delete + end + + it 'detects change with ms_dirty? method' do + book = Book.new name: 'My life', author: 'Myself', premium: false, released: true + + allow(book).to receive(:ms_dirty?).and_return(true) + expect(Book.ms_must_reindex?(book)).to be(true) + + allow(book).to receive(:ms_dirty?).and_return(false) + expect(Book.ms_must_reindex?(book)).to be(false) + + allow(book).to receive(:ms_dirty?).and_return(true) + expect(Book.ms_must_reindex?(book)).to be(true) + end +end + diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index 03a176a2..e57b0d9d 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -46,50 +46,6 @@ end end -describe 'Attributes change detection' do - it 'detects attribute changes' do - color = Color.new name: 'dark-blue', short_name: 'blue' - - expect(Color.ms_must_reindex?(color)).to be(true) - color.save - expect(Color.ms_must_reindex?(color)).to be(false) - - color.hex = 123_456 - expect(Color.ms_must_reindex?(color)).to be(false) - - color.not_indexed = 'strstr' - expect(Color.ms_must_reindex?(color)).to be(false) - color.name = 'red' - expect(Color.ms_must_reindex?(color)).to be(true) - color.delete - end - - it 'detects attribute changes even in a transaction' do - color = Color.new name: 'dark-blue', short_name: 'blue' - color.save - expect(color.instance_variable_get('@ms_must_reindex')).to be_nil - Color.transaction do - color.name = 'red' - color.save - color.not_indexed = 'strstr' - color.save - expect(color.instance_variable_get('@ms_must_reindex')).to be(true) - end - expect(color.instance_variable_get('@ms_must_reindex')).to be_nil - color.delete - end - - it 'detects change with ms_dirty? method' do - ebook = Ebook.new name: 'My life', author: 'Myself', premium: false, released: true - expect(Ebook.ms_must_reindex?(ebook)).to be(true) # Because it's defined in ms_dirty? method - ebook.current_time = 10 - ebook.published_at = 8 - expect(Ebook.ms_must_reindex?(ebook)).to be(true) - ebook.published_at = 12 - expect(Ebook.ms_must_reindex?(ebook)).to be(false) - end -end - describe 'Namespaced::Model' do before(:all) do Namespaced::Model.index.delete_all_documents! diff --git a/spec/support/active_record_classes.rb b/spec/support/active_record_classes.rb index bb16768b..f373f811 100644 --- a/spec/support/active_record_classes.rb +++ b/spec/support/active_record_classes.rb @@ -9,12 +9,6 @@ create_table :mongo_documents do |t| t.string :name end - create_table :ebooks do |t| - t.string :name - t.string :author - t.boolean :premium - t.boolean :released - end end class UniqUser < ActiveRecord::Base @@ -52,19 +46,3 @@ def index! end end -class Ebook < ActiveRecord::Base - include MeiliSearch::Rails - attr_accessor :current_time, :published_at - - meilisearch synchronous: true, index_uid: safe_index_uid('eBooks') do - searchable_attributes [:name] - end - - def ms_dirty? - return true if published_at.nil? || current_time.nil? - - # Consider dirty if published date is in the past - # This doesn't make so much business sense but it's easy to test. - published_at < current_time - end -end From c5990b362b0b82df6d13c42fdd1ba193598e79e8 Mon Sep 17 00:00:00 2001 From: ellnix <103502144+ellnix@users.noreply.github.com> Date: Thu, 29 Feb 2024 13:01:52 +0100 Subject: [PATCH 02/36] Remove test of ruby's Marshal This test seems to just use Marshal.dump on an object and then Marshal.load it later, which would pass for any ruby object. --- spec/integration_spec.rb | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index e57b0d9d..67b863ba 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -620,20 +620,6 @@ end.not_to raise_error end - context 'with Marshal serialization' do - let(:found_books) { Book.search('*') } - let(:marshaled_books) { Marshal.dump(found_books) } - - it 'returns all books in the marshaled format' do - # Perform the search and marshal the results - expect(marshaled_books).to be_present - - # Load the marshaled data and check the content - loaded_books = Marshal.load(marshaled_books) - expect(loaded_books).to match_array(found_books) - end - end - context 'with Rails caching' do let(:memory_store) { ActiveSupport::Cache.lookup_store(:memory_store) } let(:cache) { Rails.cache } From c4644e502750151adbb4bfc84ceab5998baa000e Mon Sep 17 00:00:00 2001 From: ellnix <103502144+ellnix@users.noreply.github.com> Date: Thu, 29 Feb 2024 13:06:28 +0100 Subject: [PATCH 03/36] Remove test of Rails cache functionality This test seems to test that an object that is cached can be fetched back from the cache. As proof, the meilisearch method calls can be removed and the test passes just fine: ```ruby context 'with Rails caching' do let(:memory_store) { ActiveSupport::Cache.lookup_store(:memory_store) } let(:search_query) { '*' } let(:cache_key) { "book_search:#{search_query}" } before do allow(Rails).to receive(:cache).and_return(memory_store) Rails.cache.clear end fit 'caches the search results' do # Ensure the cache is empty before the test expect(Rails.cache.read(cache_key)).to be_nil # Perform the search and cache the results Rails.cache.fetch(cache_key) do search_query end # Check if the search result is cached expect(Rails.cache.read(cache_key)).to eq(search_query) end end ``` --- spec/integration_spec.rb | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index 67b863ba..cc1513ae 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -619,33 +619,6 @@ Book.index.facet_search('genre') end.not_to raise_error end - - context 'with Rails caching' do - let(:memory_store) { ActiveSupport::Cache.lookup_store(:memory_store) } - let(:cache) { Rails.cache } - - let(:search_query) { '*' } - let(:cache_key) { "book_search:#{search_query}" } - - before do - allow(Rails).to receive(:cache).and_return(memory_store) - Rails.cache.clear - end - - it 'caches the search results' do - # Ensure the cache is empty before the test - expect(Rails.cache.read(cache_key)).to be_nil - - # Perform the search and cache the results - Rails.cache.fetch(cache_key) do - Book.search(search_query) - end - - # Check if the search result is cached - not_cached_books = Book.search(search_query) - expect(Rails.cache.read(cache_key)).to match_array(not_cached_books) - end - end end describe 'Movie' do From 4c62567b8a8c64a7d29a83e7d5c1c4025f311c60 Mon Sep 17 00:00:00 2001 From: ellnix <103502144+ellnix@users.noreply.github.com> Date: Thu, 29 Feb 2024 13:14:57 +0100 Subject: [PATCH 04/36] Remove unused UniqUser class from specs --- spec/support/active_record_classes.rb | 9 --------- 1 file changed, 9 deletions(-) diff --git a/spec/support/active_record_classes.rb b/spec/support/active_record_classes.rb index f373f811..25f8bb97 100644 --- a/spec/support/active_record_classes.rb +++ b/spec/support/active_record_classes.rb @@ -2,21 +2,12 @@ Dir["#{File.dirname(__FILE__)}/models/*.rb"].sort.each { |file| require file } ar_schema.instance_exec do - create_table :uniq_users, id: false do |t| - t.string :name - end create_table :nullable_ids create_table :mongo_documents do |t| t.string :name end end -class UniqUser < ActiveRecord::Base - include MeiliSearch::Rails - - meilisearch synchronous: true, index_uid: safe_index_uid('UniqUser'), primary_key: :name -end - class NullableId < ActiveRecord::Base include MeiliSearch::Rails From 0994f36a16dabfce9572da5734845cac0b788b9a Mon Sep 17 00:00:00 2001 From: ellnix <103502144+ellnix@users.noreply.github.com> Date: Thu, 29 Feb 2024 13:15:41 +0100 Subject: [PATCH 05/36] Remove unused NullableId class from specs --- spec/support/active_record_classes.rb | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/spec/support/active_record_classes.rb b/spec/support/active_record_classes.rb index 25f8bb97..fe3761a0 100644 --- a/spec/support/active_record_classes.rb +++ b/spec/support/active_record_classes.rb @@ -2,27 +2,11 @@ Dir["#{File.dirname(__FILE__)}/models/*.rb"].sort.each { |file| require file } ar_schema.instance_exec do - create_table :nullable_ids create_table :mongo_documents do |t| t.string :name end end -class NullableId < ActiveRecord::Base - include MeiliSearch::Rails - - meilisearch synchronous: true, index_uid: safe_index_uid('NullableId'), primary_key: :custom_id, - if: :never - - def custom_id - nil - end - - def never - false - end -end - class MongoDocument < ActiveRecord::Base include MeiliSearch::Rails From 355250efab07105284e0cabdfbb15affcf34a9f4 Mon Sep 17 00:00:00 2001 From: ellnix <103502144+ellnix@users.noreply.github.com> Date: Thu, 29 Feb 2024 13:57:07 +0100 Subject: [PATCH 06/36] Remove flawed MongoDocument specs It does not seem like Mongoid::Document has bang methods, so this is spec is not reproducing a likely naming conflict. In either case, the plan is to test with real MongoDB models so this test would be removed eventually. --- spec/integration_spec.rb | 9 --------- spec/support/active_record_classes.rb | 20 -------------------- 2 files changed, 29 deletions(-) diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index cc1513ae..307c2c68 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -451,15 +451,6 @@ end end -describe 'MongoDocument' do - it 'does not have method conflicts' do - expect { MongoDocument.reindex! }.to raise_error(NameError) - expect { MongoDocument.new.index! }.to raise_error(NameError) - MongoDocument.ms_reindex! - MongoDocument.create(name: 'mongo').ms_index! - end -end - describe 'Book' do before do Book.clear_index!(true) diff --git a/spec/support/active_record_classes.rb b/spec/support/active_record_classes.rb index fe3761a0..6ebce285 100644 --- a/spec/support/active_record_classes.rb +++ b/spec/support/active_record_classes.rb @@ -1,23 +1,3 @@ require 'support/active_record_schema' Dir["#{File.dirname(__FILE__)}/models/*.rb"].sort.each { |file| require file } -ar_schema.instance_exec do - create_table :mongo_documents do |t| - t.string :name - end -end - -class MongoDocument < ActiveRecord::Base - include MeiliSearch::Rails - - meilisearch index_uid: safe_index_uid('MongoDocument') - - def self.reindex! - raise NameError, 'never reached' - end - - def index! - raise NameError, 'never reached' - end -end - From 093bfbc32deadd60e4e6ebbc10fac61f105aa5e7 Mon Sep 17 00:00:00 2001 From: ellnix <103502144+ellnix@users.noreply.github.com> Date: Thu, 29 Feb 2024 20:13:49 +0100 Subject: [PATCH 07/36] Remove unnecessary Color tests 'is_synchronous' tests a private method and only tests that the configuration was applied 'has maxValuesPerFacet set' tests meilisearch-ruby's Index class --- spec/integration_spec.rb | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index 307c2c68..72af2201 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -140,12 +140,6 @@ Color.delete_all end - it 'is synchronous' do - c = Color.new - c.valid? - expect(c.send(:ms_synchronous?)).to be(true) - end - it 'auto indexes' do blue = Color.create!(name: 'blue', short_name: 'b', hex: 0xFF0000) results = Color.search('blue') @@ -260,10 +254,6 @@ expect(facets).to eq([black, blue, green]) end - - it 'has maxValuesPerFacet set' do - expect(Color.ms_index.get_settings.dig('faceting', 'maxValuesPerFacet')).to eq(20) - end end describe 'An imaginary store' do From 26e607c98d58a6fe6cf9467dc122140925fbd820 Mon Sep 17 00:00:00 2001 From: ellnix <103502144+ellnix@users.noreply.github.com> Date: Thu, 29 Feb 2024 19:30:07 +0100 Subject: [PATCH 08/36] Move Post specs --- .../record_has_associations_spec.rb | 24 +++++++++++++++++++ spec/integration_spec.rb | 22 ----------------- 2 files changed, 24 insertions(+), 22 deletions(-) create mode 100644 spec/integration/active_record/record_has_associations_spec.rb diff --git a/spec/integration/active_record/record_has_associations_spec.rb b/spec/integration/active_record/record_has_associations_spec.rb new file mode 100644 index 00000000..b9294cae --- /dev/null +++ b/spec/integration/active_record/record_has_associations_spec.rb @@ -0,0 +1,24 @@ +require 'support/models/post' + +describe 'When a record has associations' do + before(:all) do + Post.clear_index!(true) + end + + it 'eagerly loads associations' do + post1 = Post.new(title: 'foo') + post1.comments << Comment.new(body: 'one') + post1.comments << Comment.new(body: 'two') + post1.save! + + post2 = Post.new(title: 'bar') + post2.comments << Comment.new(body: 'three') + post2.comments << Comment.new(body: 'four') + post2.save! + + assert_queries(2) do + Post.reindex! + end + end +end + diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index 72af2201..e651756d 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -112,28 +112,6 @@ end end -describe 'Posts' do - before(:all) do - Post.clear_index!(true) - end - - it 'eagerly loads associations' do - post1 = Post.new(title: 'foo') - post1.comments << Comment.new(body: 'one') - post1.comments << Comment.new(body: 'two') - post1.save! - - post2 = Post.new(title: 'bar') - post2.comments << Comment.new(body: 'three') - post2.comments << Comment.new(body: 'four') - post2.save! - - assert_queries(2) do - Post.reindex! - end - end -end - describe 'Colors' do before do Color.clear_index!(true) From 3fda1a70b87a8849077c9fc2d86c072407160790 Mon Sep 17 00:00:00 2001 From: ellnix <103502144+ellnix@users.noreply.github.com> Date: Thu, 29 Feb 2024 20:10:22 +0100 Subject: [PATCH 09/36] Move Color search specs --- spec/integration_spec.rb | 60 ---------------------------------- spec/search_spec.rb | 69 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 60 deletions(-) create mode 100644 spec/search_spec.rb diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index e651756d..a3487b70 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -125,21 +125,6 @@ expect(results).to include(blue) end - it 'returns facets distribution' do - Color.create!(name: 'blue', short_name: 'b', hex: 0xFF0000) - results = Color.search('', { facets: ['short_name'] }) - expect(results.raw_answer).not_to be_nil - expect(results.facets_distribution).not_to be_nil - expect(results.facets_distribution.size).to eq(1) - expect(results.facets_distribution['short_name']['b']).to eq(1) - end - - it 'is raw searchable' do - Color.create!(name: 'blue', short_name: 'b', hex: 0xFF0000) - results = Color.raw_search('blue') - expect(results['hits'].size).to eq(1) - expect(results['estimatedTotalHits']).to eq(1) - end it 'is able to temporarily disable auto-indexing' do Color.without_auto_index do @@ -150,23 +135,6 @@ expect(Color.search('blue').size).to eq(1) end - it 'is not searchable with non-searchable fields' do - Color.create!(name: 'blue', short_name: 'x', hex: 0xFF0000) - results = Color.search('x') - expect(results.size).to eq(0) - end - - it 'ranks with custom hex' do - Color.create!(name: 'red', short_name: 'r3', hex: 3) - Color.create!(name: 'red', short_name: 'r1', hex: 1) - Color.create!(name: 'red', short_name: 'r2', hex: 2) - results = Color.search('red') - expect(results.size).to eq(3) - expect(results[0].hex).to eq(1) - expect(results[1].hex).to eq(2) - expect(results[2].hex).to eq(3) - end - it 'updates the index if the attribute changed' do purple = Color.create!(name: 'purple', short_name: 'p') expect(Color.search('purple').size).to eq(1) @@ -194,13 +162,6 @@ expect(Color.index_uid).to eq(safe_index_uid('Color') + "_#{Rails.env}") end - it 'includes _formatted object' do - Color.create!(name: 'green', short_name: 'b', hex: 0xFF0000) - results = Color.search('gre') - expect(results.size).to eq(1) - expect(results[0].formatted).not_to be_nil - end - it 'indexes an array of documents' do json = Color.raw_search('') Color.index_documents Color.limit(1), true # reindex last color, `limit` is incompatible with the reindex! method @@ -211,27 +172,6 @@ expect { Color.new(name: 'purple').index!(true) }.to raise_error(ArgumentError) expect { Color.new(name: 'purple').remove_from_index!(true) }.to raise_error(ArgumentError) end - - it 'searches with filter' do - Color.create!(name: 'blue', short_name: 'blu', hex: 0x0000FF) - black = Color.create!(name: 'black', short_name: 'bla', hex: 0x000000) - Color.create!(name: 'green', short_name: 'gre', hex: 0x00FF00) - facets = Color.search('bl', { filter: ['short_name = bla'] }) - expect(facets.size).to eq(1) - expect(facets).to include(black) - end - - it 'searches with sorting' do - Color.delete_all - - blue = Color.create!(name: 'blue', short_name: 'blu', hex: 0x0000FF) - black = Color.create!(name: 'black', short_name: 'bla', hex: 0x000000) - green = Color.create!(name: 'green', short_name: 'gre', hex: 0x00FF00) - - facets = Color.search('*', { sort: ['name:asc'] }) - - expect(facets).to eq([black, blue, green]) - end end describe 'An imaginary store' do diff --git a/spec/search_spec.rb b/spec/search_spec.rb new file mode 100644 index 00000000..00147eff --- /dev/null +++ b/spec/search_spec.rb @@ -0,0 +1,69 @@ +require 'support/models/color' + +describe 'Search' do + before do + Color.clear_index!(true) + Color.delete_all + end + + it 'respects non-searchable attributes' do + Color.create!(name: 'blue', short_name: 'x', hex: 0xFF0000) + expect(Color.search('x')).to be_empty + end + + it 'respects ranking rules' do + third = Color.create!(hex: 3) + first = Color.create!(hex: 1) + second = Color.create!(hex: 2) + + expect(Color.search('')).to eq([first, second, third]) + end + + it 'applies filter' do + _blue = Color.create!(name: 'blue', short_name: 'blu', hex: 0x0000FF) + black = Color.create!(name: 'black', short_name: 'bla', hex: 0x000000) + _green = Color.create!(name: 'green', short_name: 'gre', hex: 0x00FF00) + + results = Color.search('bl', { filter: ['short_name = bla'] }) + expect(results).to contain_exactly(black) + end + + it 'applies sorting' do + blue = Color.create!(name: 'blue', short_name: 'blu', hex: 0x0000FF) + black = Color.create!(name: 'black', short_name: 'bla', hex: 0x000000) + green = Color.create!(name: 'green', short_name: 'gre', hex: 0x00FF00) + + results = Color.search('*', { sort: ['name:asc'] }) + + expect(results).to eq([black, blue, green]) + end + + it 'makes facets distribution accessible' do + Color.create!(name: 'blue', short_name: 'b', hex: 0xFF0000) + results = Color.search('', { facets: ['short_name'] }) + + expect(results.facets_distribution).to match( + { 'short_name' => { 'b' => 1 } } + ) + end + + + it 'results include #formatted object' do + Color.create!(name: 'green', short_name: 'b', hex: 0xFF0000) + results = Color.search('gre') + expect(results[0].formatted).to include('name' => 'green') + end +end + +describe '#raw_search' do + it 'allows for access to meilisearch-ruby search' do + Color.clear_index!(true) + Color.delete_all + Color.create!(name: 'blue', short_name: 'b', hex: 0xFF0000) + + raw_results = Color.raw_search('blue') + ms_ruby_results = Color.index.search('blue') + + expect(raw_results.keys).to match ms_ruby_results.keys + end +end From 8504a7a9cc156746aa929598b69bf92f508d1307 Mon Sep 17 00:00:00 2001 From: ellnix <103502144+ellnix@users.noreply.github.com> Date: Fri, 1 Mar 2024 14:35:34 +0100 Subject: [PATCH 10/36] Move Book specs from integration_spec.rb --- spec/configuration_spec.rb | 16 ++ spec/instance_methods_spec.rb | 49 ++++++ .../active_record/record_is_updated_spec.rb | 22 +++ spec/integration_spec.rb | 165 ------------------ spec/options_spec.rb | 66 +++++++ spec/safe_index_spec.rb | 22 +++ spec/search_spec.rb | 1 - spec/settings_spec.rb | 44 +++++ spec/support/models/book.rb | 8 + 9 files changed, 227 insertions(+), 166 deletions(-) create mode 100644 spec/instance_methods_spec.rb create mode 100644 spec/integration/active_record/record_is_updated_spec.rb create mode 100644 spec/options_spec.rb create mode 100644 spec/safe_index_spec.rb diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index ebc7e8db..938c637d 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -1,4 +1,6 @@ require 'spec_helper' +require 'support/models/book' +require 'support/models/color' describe MeiliSearch::Rails::Configuration do before { stub_const('MeiliSearch::Rails::VERSION', '0.0.1') } @@ -63,6 +65,20 @@ end end + context 'with per_environment' do + # per_environment is already enabled in testing + # no setup is required + + it 'adds a Rails env-based index suffix' do + expect(Color.index_uid).to eq(safe_index_uid('Color') + "_#{Rails.env}") + end + + it 'uses suffix in the additional index as well' do + index = Book.index(safe_index_uid('Book')) + expect(index.uid).to eq("#{safe_index_uid('Book')}_#{Rails.env}") + end + end + context 'when use Meilisearch without configuration' do around do |example| config = MeiliSearch::Rails.configuration diff --git a/spec/instance_methods_spec.rb b/spec/instance_methods_spec.rb new file mode 100644 index 00000000..bd862598 --- /dev/null +++ b/spec/instance_methods_spec.rb @@ -0,0 +1,49 @@ +require 'support/models/book' + +describe 'Instance methods' do + describe '#ms_entries' do + it 'includes conditionally enabled indexes' do + book = Book.create!( + name: 'Frankenstein', author: 'Mary Shelley', + premium: false, released: true + ) + + expect(book.ms_entries).to contain_exactly( + a_hash_including("index_uid" => safe_index_uid('SecuredBook')), + a_hash_including("index_uid" => safe_index_uid('BookAuthor')), + a_hash_including("index_uid" => safe_index_uid('Book')), + ) + end + + it 'includes conditionally disabled indexes' do + # non public book + book = Book.create!( + name: 'Frankenstein', author: 'Mary Shelley', + premium: false, released: false + ) + + expect(book.ms_entries).to contain_exactly( + a_hash_including("index_uid" => safe_index_uid('SecuredBook')), + a_hash_including("index_uid" => safe_index_uid('BookAuthor')), + # also includes book's id as if it was a public book + a_hash_including("index_uid" => safe_index_uid('Book')), + ) + end + end + + describe '#ms_index!' do + it 'returns array of tasks' do + TestUtil.reset_books! + + moby_dick = Book.create! name: 'Moby Dick', author: 'Herman Melville', premium: false, released: true + + tasks = moby_dick.ms_index! + + expect(tasks).to contain_exactly( + a_hash_including('uid'), + a_hash_including('taskUid'), + a_hash_including('taskUid') + ) + end + end +end diff --git a/spec/integration/active_record/record_is_updated_spec.rb b/spec/integration/active_record/record_is_updated_spec.rb new file mode 100644 index 00000000..66615bfb --- /dev/null +++ b/spec/integration/active_record/record_is_updated_spec.rb @@ -0,0 +1,22 @@ +require 'support/models/book' + +describe 'When record is updated' do + it 'automatically removes document from conditional indexes' do + TestUtil.reset_books! + + # add a new public book which is public (not premium but released) + book = Book.create! name: 'Public book', author: 'me', premium: false, released: true + + # should be searchable in the 'Book' index + index = Book.index(safe_index_uid('Book')) + results = index.search('Public book') + expect(results['hits']).to be_one + + # update the book and make it non-public anymore (not premium, not released) + book.update released: false + + # should be removed from the index + results = index.search('Public book') + expect(results['hits']).to be_empty + end +end diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index a3487b70..24b466b5 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -158,10 +158,6 @@ expect(Color.search('').size).to eq(1) end - it 'has a Rails env-based index name' do - expect(Color.index_uid).to eq(safe_index_uid('Color') + "_#{Rails.env}") - end - it 'indexes an array of documents' do json = Color.raw_search('') Color.index_documents Color.limit(1), true # reindex last color, `limit` is incompatible with the reindex! method @@ -359,167 +355,6 @@ end end -describe 'Book' do - before do - Book.clear_index!(true) - Book.index(safe_index_uid('BookAuthor')).delete_all_documents - Book.index(safe_index_uid('Book')).delete_all_documents - end - - it 'returns array of tasks on #ms_index!' do - moby_dick = Book.create! name: 'Moby Dick', author: 'Herman Melville', premium: false, released: true - - tasks = moby_dick.ms_index! - - expect(tasks).to contain_exactly( - a_hash_including('uid'), - a_hash_including('taskUid'), - a_hash_including('taskUid') - ) - end - - it 'indexes the book in 2 indexes of 3' do - steve_jobs = Book.create! name: 'Steve Jobs', author: 'Walter Isaacson', premium: true, released: true - results = Book.search('steve') - expect(results.size).to eq(1) - expect(results).to include(steve_jobs) - - index_author = Book.index(safe_index_uid('BookAuthor')) - expect(index_author).not_to be_nil - results = index_author.search('steve') - expect(results['hits'].length).to eq(0) - results = index_author.search('walter') - expect(results['hits'].length).to eq(1) - - # premium -> not part of the public index - index_book = Book.index(safe_index_uid('Book')) - expect(index_book).not_to be_nil - results = index_book.search('steve') - expect(results['hits'].length).to eq(0) - end - - it 'sanitizes attributes' do - _hack = Book.create! name: '"> hack0r', - author: '', premium: true, released: true - b = Book.raw_search('hack', { attributes_to_highlight: ['*'] }) - expect(b['hits'].length).to eq(1) - begin - expect(b['hits'][0]['name']).to eq('"> hack0r').and_raise(StandardError) - expect(b['hits'][0]['author']).to eq('alert(1)') - expect(b['hits'][0]['_formatted']['name']).to eq('"> hack0r') - rescue StandardError - # rails 4.2's sanitizer - begin - expect(b['hits'][0]['name']).to eq('"> hack0r').and_raise(StandardError) - expect(b['hits'][0]['author']).to eq('') - expect(b['hits'][0]['_formatted']['name']).to eq('"> hack0r') - rescue StandardError - # jruby - expect(b['hits'][0]['name']).to eq('"> hack0r') - expect(b['hits'][0]['author']).to eq('') - expect(b['hits'][0]['_formatted']['name']).to eq('"> hack0r') - end - end - end - - it 'handles removal in an extra index' do - # add a new public book which (not premium but released) - book = Book.create! name: 'Public book', author: 'me', premium: false, released: true - - # should be searchable in the 'Book' index - index = Book.index(safe_index_uid('Book')) - results = index.search('Public book') - expect(results['hits'].size).to eq(1) - - # update the book and make it non-public anymore (not premium, not released) - book.update released: false - - # should be removed from the index - results = index.search('Public book') - expect(results['hits'].size).to eq(0) - end - - it 'uses the per_environment option in the additional index as well' do - index = Book.index(safe_index_uid('Book')) - expect(index.uid).to eq("#{safe_index_uid('Book')}_#{Rails.env}") - end - - it 'searches with one typo min size' do - Book.create! name: 'The Lord of the Rings', author: 'me', premium: false, released: true - results = Book.search('Lrod') - expect(results.size).to eq(0) - - results = Book.search('Rnigs') - expect(results.size).to eq(1) - end - - it 'searches with two typo min size' do - Book.create! name: 'Dracula', author: 'me', premium: false, released: true - results = Book.search('Darclua') - expect(results.size).to eq(0) - - Book.create! name: 'Frankenstein', author: 'me', premium: false, released: true - results = Book.search('Farnkenstien') - expect(results.size).to eq(1) - end - - describe '#ms_entries' do - it 'returns all 3 indexes for a public book' do - book = Book.create!( - name: 'Frankenstein', author: 'Mary Shelley', - premium: false, released: true - ) - - expect(book.ms_entries).to contain_exactly( - a_hash_including("index_uid" => safe_index_uid('SecuredBook')), - a_hash_including("index_uid" => safe_index_uid('BookAuthor')), - a_hash_including("index_uid" => safe_index_uid('Book')), - ) - end - - it 'returns all 3 indexes for a non-public book' do - book = Book.create!( - name: 'Frankenstein', author: 'Mary Shelley', - premium: false, released: false - ) - - expect(book.ms_entries).to contain_exactly( - a_hash_including("index_uid" => safe_index_uid('SecuredBook')), - a_hash_including("index_uid" => safe_index_uid('BookAuthor')), - a_hash_including("index_uid" => safe_index_uid('Book')), - ) - end - end - - it 'returns facets using max values per facet' do - 10.times do - Book.create! name: Faker::Book.title, author: Faker::Book.author, genre: Faker::Book.genre - end - - genres = Book.distinct.pluck(:genre) - - results = Book.search('', { facets: ['genre'] }) - - expect(genres.size).to be > 3 - expect(results.facets_distribution['genre'].size).to eq(3) - end - - it 'does not error on facet_search' do - genres = %w[Legend Fiction Crime].cycle - authors = %w[A B C].cycle - - 5.times do - Book.create! name: Faker::Book.title, author: authors.next, genre: genres.next - end - - expect do - Book.index.facet_search('genre', 'Fic', filter: 'author = A') - Book.index.facet_search('genre', filter: 'author = A') - Book.index.facet_search('genre') - end.not_to raise_error - end -end - describe 'Movie' do before(:all) do Movie.clear_index!(true) diff --git a/spec/options_spec.rb b/spec/options_spec.rb new file mode 100644 index 00000000..3b9fcf26 --- /dev/null +++ b/spec/options_spec.rb @@ -0,0 +1,66 @@ +require 'support/models/color' +require 'support/models/book' + +describe 'meilisearch_options' do + describe ':if' do + it 'only indexes the record in the valid indexes' do + TestUtil.reset_books! + + Book.create! name: 'Steve Jobs', author: 'Walter Isaacson', + premium: true, released: true + + results = Book.search('steve') + expect(results).to be_one + + results = Book.index(safe_index_uid('BookAuthor')).search('walter') + expect(results['hits']).to be_one + + # premium -> not part of the public index + results = Book.index(safe_index_uid('Book')).search('steve') + expect(results['hits']).to be_empty + end + end + + describe ':auto_index' do + it 'auto indexes by default' do + Color.clear_index!(true) + Color.delete_all + + Color.create!(name: 'blue', short_name: 'b', hex: 0xFF0000) + results = Color.raw_search('blue') + expect(results['hits'].size).to eq(1) + expect(results['estimatedTotalHits']).to eq(1) + end + end + + describe ':sanitize' do + context 'when true' do + it 'sanitizes attributes' do + TestUtil.reset_books! + + Book.create! name: '"> hack0r', + author: '', premium: true, released: true + + b = Book.raw_search('hack') + + expect(b['hits'][0]).to include( + 'name' => '"> hack0r', + 'author' => '', + ) + end + + it 'keeps _formatted emphasis' do + TestUtil.reset_books! + + Book.create! name: '"> hack0r', + author: '', premium: true, released: true + + b = Book.raw_search('hack', { attributes_to_highlight: ['*'] }) + + expect(b['hits'][0]['_formatted']).to include( + 'name' => '"> hack0r', + ) + end + end + end +end diff --git a/spec/safe_index_spec.rb b/spec/safe_index_spec.rb new file mode 100644 index 00000000..9fa0fc4c --- /dev/null +++ b/spec/safe_index_spec.rb @@ -0,0 +1,22 @@ +require 'support/models/book' + +describe MeiliSearch::Rails::SafeIndex do + describe '#facet_search' do + it 'accepts all params without error' do + TestUtil.reset_books! + + genres = %w[Legend Fiction Crime].cycle + authors = %w[A B C].cycle + + 5.times do + Book.create! name: Faker::Book.title, author: authors.next, genre: genres.next + end + + expect do + Book.index.facet_search('genre', 'Fic', filter: 'author = A') + Book.index.facet_search('genre', filter: 'author = A') + Book.index.facet_search('genre') + end.not_to raise_error + end + end +end diff --git a/spec/search_spec.rb b/spec/search_spec.rb index 00147eff..4d963b61 100644 --- a/spec/search_spec.rb +++ b/spec/search_spec.rb @@ -47,7 +47,6 @@ ) end - it 'results include #formatted object' do Color.create!(name: 'green', short_name: 'b', hex: 0xFF0000) results = Color.search('gre') diff --git a/spec/settings_spec.rb b/spec/settings_spec.rb index ed5a7a8f..7e35990a 100644 --- a/spec/settings_spec.rb +++ b/spec/settings_spec.rb @@ -1,6 +1,50 @@ require 'spec_helper' +require 'support/models/book' describe MeiliSearch::Rails::IndexSettings do + describe 'faceting' do + it 'respects max values per facet' do + TestUtil.reset_books! + + 4.times do + Book.create! name: Faker::Book.title, author: Faker::Book.author, + genre: Faker::Book.unique.genre + end + + genres = Book.distinct.pluck(:genre) + + results = Book.search('', { facets: ['genre'] }) + + expect(genres.size).to be > 3 + expect(results.facets_distribution['genre'].size).to eq(3) + end + end + + describe 'typo_tolerance' do + it 'searches with one typo min size' do + TestUtil.reset_books! + + Book.create! name: 'The Lord of the Rings', author: 'me', premium: false, released: true + results = Book.search('Lrod') + expect(results).to be_empty + + results = Book.search('Rnigs') + expect(results).to be_one + end + + it 'searches with two typo min size' do + TestUtil.reset_books! + + Book.create! name: 'Dracula', author: 'me', premium: false, released: true + results = Book.search('Darclua') + expect(results).to be_empty + + Book.create! name: 'Frankenstein', author: 'me', premium: false, released: true + results = Book.search('Farnkenstien') + expect(results).to be_one + end + end + describe 'settings change detection' do let(:record) { Color.create name: 'dark-blue', short_name: 'blue' } diff --git a/spec/support/models/book.rb b/spec/support/models/book.rb index 5b92db30..ad956606 100644 --- a/spec/support/models/book.rb +++ b/spec/support/models/book.rb @@ -32,3 +32,11 @@ def public? released && !premium end end + +module TestUtil + def self.reset_books! + Book.clear_index!(true) + Book.index(safe_index_uid('BookAuthor')).delete_all_documents + Book.index(safe_index_uid('Book')).delete_all_documents + end +end From b0cc102a8f0ee49ab3a13ea7d57fc71e25b14670 Mon Sep 17 00:00:00 2001 From: ellnix <103502144+ellnix@users.noreply.github.com> Date: Fri, 1 Mar 2024 15:08:21 +0100 Subject: [PATCH 11/36] Move Color specs from integration_spec.rb --- spec/instance_methods_spec.rb | 10 +++ .../active_record/record_is_updated_spec.rb | 11 ++++ spec/integration_spec.rb | 58 ----------------- spec/model_methods_spec.rb | 64 +++++++++++++++++++ spec/options_spec.rb | 5 +- spec/search_spec.rb | 4 +- spec/support/models/color.rb | 7 ++ 7 files changed, 96 insertions(+), 63 deletions(-) create mode 100644 spec/model_methods_spec.rb diff --git a/spec/instance_methods_spec.rb b/spec/instance_methods_spec.rb index bd862598..ed76ef5f 100644 --- a/spec/instance_methods_spec.rb +++ b/spec/instance_methods_spec.rb @@ -45,5 +45,15 @@ a_hash_including('taskUid') ) end + + it 'throws error on non-persisted instances' do + expect { Color.new(name: 'purple').index!(true) }.to raise_error(ArgumentError) + end + end + + describe '#ms_remove_from_index!' do + it 'throws error on non-persisted instances' do + expect { Color.new(name: 'purple').remove_from_index!(true) }.to raise_error(ArgumentError) + end end end diff --git a/spec/integration/active_record/record_is_updated_spec.rb b/spec/integration/active_record/record_is_updated_spec.rb index 66615bfb..06e89a1a 100644 --- a/spec/integration/active_record/record_is_updated_spec.rb +++ b/spec/integration/active_record/record_is_updated_spec.rb @@ -1,6 +1,17 @@ require 'support/models/book' +require 'support/models/color' describe 'When record is updated' do + it 'updates the changed attributes on the index' do + purple = Color.create!(name: 'purple', short_name: 'p') + expect(Color.search('purple')).to be_one + expect(Color.search('pink')).to be_empty + + purple.update name: 'pink' + expect(Color.search('purple')).to be_empty + expect(Color.search('pink')).to be_one + end + it 'automatically removes document from conditional indexes' do TestUtil.reset_books! diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index 24b466b5..21250913 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -112,64 +112,6 @@ end end -describe 'Colors' do - before do - Color.clear_index!(true) - Color.delete_all - end - - it 'auto indexes' do - blue = Color.create!(name: 'blue', short_name: 'b', hex: 0xFF0000) - results = Color.search('blue') - expect(results.size).to eq(1) - expect(results).to include(blue) - end - - - it 'is able to temporarily disable auto-indexing' do - Color.without_auto_index do - Color.create!(name: 'blue', short_name: 'b', hex: 0xFF0000) - end - expect(Color.search('blue').size).to eq(0) - Color.reindex!(MeiliSearch::Rails::IndexSettings::DEFAULT_BATCH_SIZE, true) - expect(Color.search('blue').size).to eq(1) - end - - it 'updates the index if the attribute changed' do - purple = Color.create!(name: 'purple', short_name: 'p') - expect(Color.search('purple').size).to eq(1) - expect(Color.search('pink').size).to eq(0) - purple.name = 'pink' - purple.save - expect(Color.search('purple').size).to eq(0) - expect(Color.search('pink').size).to eq(1) - end - - it 'uses the specified scope' do - Color.create!(name: 'red', short_name: 'r3', hex: 3) - Color.create!(name: 'red', short_name: 'r1', hex: 1) - Color.create!(name: 'red', short_name: 'r2', hex: 2) - Color.create!(name: 'purple', short_name: 'p') - Color.clear_index!(true) - Color.where(name: 'red').reindex!(MeiliSearch::Rails::IndexSettings::DEFAULT_BATCH_SIZE, true) - expect(Color.search('').size).to eq(3) - Color.clear_index!(true) - Color.where(id: Color.first.id).reindex!(MeiliSearch::Rails::IndexSettings::DEFAULT_BATCH_SIZE, true) - expect(Color.search('').size).to eq(1) - end - - it 'indexes an array of documents' do - json = Color.raw_search('') - Color.index_documents Color.limit(1), true # reindex last color, `limit` is incompatible with the reindex! method - expect(json['hits'].count).to eq(Color.raw_search('')['hits'].count) - end - - it 'does not index non-saved document' do - expect { Color.new(name: 'purple').index!(true) }.to raise_error(ArgumentError) - expect { Color.new(name: 'purple').remove_from_index!(true) }.to raise_error(ArgumentError) - end -end - describe 'An imaginary store' do before(:all) do Product.clear_index!(true) diff --git a/spec/model_methods_spec.rb b/spec/model_methods_spec.rb new file mode 100644 index 00000000..55eccacc --- /dev/null +++ b/spec/model_methods_spec.rb @@ -0,0 +1,64 @@ +require 'support/models/color' +require 'support/models/book' + +describe 'Model methods' do + describe '.reindex!' do + it 'uses the specified scope' do + TestUtil.reset_colors! + + Color.create!(name: 'red', short_name: 'r3', hex: 3) + Color.create!(name: 'red', short_name: 'r1', hex: 1) + Color.create!(name: 'purple', short_name: 'p') + + Color.clear_index!(true) + + Color.where(name: 'red').reindex!(3, true) + expect(Color.search('').size).to eq(2) + + Color.clear_index!(true) + Color.where(id: Color.first.id).reindex!(3, true) + expect(Color.search('').size).to eq(1) + end + end + + describe '.without_auto_index' do + it 'disables auto indexing for the model' do + TestUtil.reset_colors! + + Color.without_auto_index do + Color.create!(name: 'blue', short_name: 'b', hex: 0xFF0000) + end + + expect(Color.search('blue')).to be_empty + + Color.reindex!(2, true) + expect(Color.search('blue')).to be_one + end + + it 'does not disable auto indexing for other models' do + TestUtil.reset_books! + + Color.without_auto_index do + Book.create!( + name: 'Frankenstein', author: 'Mary Shelley', + premium: false, released: true + ) + end + + expect(Book.search('Frankenstein')).to be_one + end + end + + describe '.index_documents' do + it 'updates existing documents' do + TestUtil.reset_colors! + + _blue = Color.create!(name: 'blue', short_name: 'blu', hex: 0x0000FF) + _black = Color.create!(name: 'black', short_name: 'bla', hex: 0x000000) + + json = Color.raw_search('') + Color.index_documents Color.limit(1), true # reindex last color, `limit` is incompatible with the reindex! method + expect(json['hits'].count).to eq(Color.raw_search('')['hits'].count) + end + end +end diff --git a/spec/options_spec.rb b/spec/options_spec.rb index 3b9fcf26..15d17bfa 100644 --- a/spec/options_spec.rb +++ b/spec/options_spec.rb @@ -22,9 +22,8 @@ end describe ':auto_index' do - it 'auto indexes by default' do - Color.clear_index!(true) - Color.delete_all + it 'is enabled by default' do + TestUtil.reset_colors! Color.create!(name: 'blue', short_name: 'b', hex: 0xFF0000) results = Color.raw_search('blue') diff --git a/spec/search_spec.rb b/spec/search_spec.rb index 4d963b61..d07a0961 100644 --- a/spec/search_spec.rb +++ b/spec/search_spec.rb @@ -56,8 +56,8 @@ describe '#raw_search' do it 'allows for access to meilisearch-ruby search' do - Color.clear_index!(true) - Color.delete_all + TestUtil.reset_colors! + Color.create!(name: 'blue', short_name: 'b', hex: 0xFF0000) raw_results = Color.raw_search('blue') diff --git a/spec/support/models/color.rb b/spec/support/models/color.rb index 96807212..c4118ec0 100644 --- a/spec/support/models/color.rb +++ b/spec/support/models/color.rb @@ -35,3 +35,10 @@ def will_save_change_to_short_name? false end end + +module TestUtil + def self.reset_colors! + Color.clear_index!(true) + Color.delete_all + end +end From b9a278c62d526717e6866a6695fd2fbafa40e1ba Mon Sep 17 00:00:00 2001 From: ellnix <103502144+ellnix@users.noreply.github.com> Date: Fri, 1 Mar 2024 15:23:54 +0100 Subject: [PATCH 12/36] Move Cat and Doc specs from integration_spec.rb --- spec/instance_methods_spec.rb | 16 ++++++++++++++++ spec/integration_spec.rb | 35 ---------------------------------- spec/options_spec.rb | 22 +++++++++++++++++++++ spec/support/models/animals.rb | 9 +++++++++ 4 files changed, 47 insertions(+), 35 deletions(-) diff --git a/spec/instance_methods_spec.rb b/spec/instance_methods_spec.rb index ed76ef5f..a83065a5 100644 --- a/spec/instance_methods_spec.rb +++ b/spec/instance_methods_spec.rb @@ -1,4 +1,5 @@ require 'support/models/book' +require 'support/models/animals' describe 'Instance methods' do describe '#ms_entries' do @@ -29,6 +30,21 @@ a_hash_including("index_uid" => safe_index_uid('Book')), ) end + + context 'when models share an index' do + it 'does not return instances of other models' do + TestUtil.reset_animals! + + toby_dog = Dog.create!(name: 'Toby the Dog') + taby_cat = Cat.create!(name: 'Taby the Cat') + + expect(toby_dog.ms_entries).to contain_exactly( + a_hash_including('primary_key' => /dog_\d+/)) + + expect(taby_cat.ms_entries).to contain_exactly( + a_hash_including('primary_key' => /cat_\d+/)) + end + end end describe '#ms_index!' do diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index 21250913..08b13172 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -631,41 +631,6 @@ end end -describe 'Animals' do - it 'returns only the requested type' do - Dog.create!([{ name: 'Toby the Dog' }, { name: 'Felix the Dog' }]) - Cat.create!([{ name: 'Toby the Cat' }, { name: 'Felix the Cat' }, { name: 'roar' }]) - - expect(Dog.count).to eq(2) - expect(Cat.count).to eq(3) - - expect(Cat.search('felix').size).to eq(1) - expect(Cat.search('felix').first.name).to eq('Felix the Cat') - expect(Dog.search('toby').size).to eq(1) - expect(Dog.search('Toby').first.name).to eq('Toby the Dog') - end - - it 'shares a single index' do - cat_index = Cat.index.instance_variable_get('@index').uid - dog_index = Dog.index.instance_variable_get('@index').uid - - expect(cat_index).to eq(dog_index) - end - - describe '#ms_entries' do - it 'returns the correct entry for each animal' do - toby_dog = Dog.create!(name: 'Toby the Dog') - taby_cat = Cat.create!(name: 'Taby the Cat') - - expect(toby_dog.ms_entries).to contain_exactly( - a_hash_including('primary_key' => /dog_\d+/)) - - expect(taby_cat.ms_entries).to contain_exactly( - a_hash_including('primary_key' => /cat_\d+/)) - end - end -end - describe 'Songs' do before(:all) { MeiliSearch::Rails.configuration[:per_environment] = false } diff --git a/spec/options_spec.rb b/spec/options_spec.rb index 15d17bfa..0444dc57 100644 --- a/spec/options_spec.rb +++ b/spec/options_spec.rb @@ -1,7 +1,29 @@ require 'support/models/color' require 'support/models/book' +require 'support/models/animals' describe 'meilisearch_options' do + describe ':index_uid and :primary_key (shared index)' do + it 'index uid is the same' do + cat_index = Cat.index_uid + dog_index = Dog.index_uid + + expect(cat_index).to eq(dog_index) + end + + it 'searching a type only returns its own documents' do + TestUtil.reset_animals! + + Dog.create!([{ name: 'Toby the Dog' }, { name: 'Felix the Dog' }]) + Cat.create!([{ name: 'Toby the Cat' }, { name: 'Felix the Cat' }, { name: 'roar' }]) + + expect(Cat.search('felix')).to be_one + expect(Cat.search('felix').first.name).to eq('Felix the Cat') + expect(Dog.search('toby')).to be_one + expect(Dog.search('Toby').first.name).to eq('Toby the Dog') + end + end + describe ':if' do it 'only indexes the record in the valid indexes' do TestUtil.reset_books! diff --git a/spec/support/models/animals.rb b/spec/support/models/animals.rb index a177298e..c9c8ceaa 100644 --- a/spec/support/models/animals.rb +++ b/spec/support/models/animals.rb @@ -31,3 +31,12 @@ def ms_id "dog_#{id}" end end + +module TestUtil + def self.reset_animals! + Cat.clear_index!(true) + Cat.delete_all + Dog.clear_index!(true) + Dog.delete_all + end +end From 4663f91b14a703b905479585a5e1c55e4d6a5665 Mon Sep 17 00:00:00 2001 From: ellnix <103502144+ellnix@users.noreply.github.com> Date: Fri, 1 Mar 2024 16:04:31 +0100 Subject: [PATCH 13/36] Move People specs from integration_spec.rb --- spec/instance_methods_spec.rb | 17 +++++ .../active_record/record_is_updated_spec.rb | 12 ++++ spec/integration_spec.rb | 64 ------------------- spec/model_methods_spec.rb | 19 ++++++ spec/options_spec.rb | 35 ++++++++++ spec/settings_spec.rb | 14 ++++ spec/support/models/people.rb | 7 ++ 7 files changed, 104 insertions(+), 64 deletions(-) diff --git a/spec/instance_methods_spec.rb b/spec/instance_methods_spec.rb index a83065a5..9c3707f9 100644 --- a/spec/instance_methods_spec.rb +++ b/spec/instance_methods_spec.rb @@ -1,5 +1,6 @@ require 'support/models/book' require 'support/models/animals' +require 'support/models/people' describe 'Instance methods' do describe '#ms_entries' do @@ -71,5 +72,21 @@ it 'throws error on non-persisted instances' do expect { Color.new(name: 'purple').remove_from_index!(true) }.to raise_error(ArgumentError) end + + context 'when :auto_remove is disabled' do + it 'is able to remove manually' do + TestUtil.reset_people! + + bob = People.create(first_name: 'Bob', last_name: 'Sponge', card_number: 75_801_889) + + result = People.raw_search('Bob') + expect(result['hits']).to be_one + + bob.remove_from_index! + + result = People.raw_search('Bob') + expect(result['hits']).to be_empty + end + end end end diff --git a/spec/integration/active_record/record_is_updated_spec.rb b/spec/integration/active_record/record_is_updated_spec.rb index 06e89a1a..ffbed0bc 100644 --- a/spec/integration/active_record/record_is_updated_spec.rb +++ b/spec/integration/active_record/record_is_updated_spec.rb @@ -30,4 +30,16 @@ results = index.search('Public book') expect(results['hits']).to be_empty end + + context 'when attributes have not changed' do + it 'does not call the API' do + TestUtil.reset_people! + + jane = People.create(first_name: 'Jane', last_name: 'Doe', card_number: 75_801_887) + + expect do + jane.update(first_name: 'Jane') + end.not_to change(People.index.tasks['results'], :count) + end + end end diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index 08b13172..531480c1 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -567,70 +567,6 @@ end end -describe 'People' do - before do - People.clear_index!(true) - People.delete_all - end - - before(:all) { MeiliSearch::Rails.configuration[:per_environment] = false } - - after(:all) { MeiliSearch::Rails.configuration[:per_environment] = true } - - it 'adds custom complex attribute' do - People.create(first_name: 'Jane', last_name: 'Doe', card_number: 75_801_887) - result = People.raw_search('Jane') - expect(result['hits'][0]['full_name']).to eq('Jane Doe') - end - - it 'has as uid the custom name specified' do - People.create(first_name: 'Jane', last_name: 'Doe', card_number: 75_801_887) - expect(People.index.uid).to eq(safe_index_uid('MyCustomPeople')) - end - - it 'has the chosen field as custom primary key' do - People.create(first_name: 'Jane', last_name: 'Doe', card_number: 75_801_887) - index = MeiliSearch::Rails.client.fetch_index(safe_index_uid('MyCustomPeople')) - expect(index.primary_key).to eq('card_number') - end - - it 'does not call the API if there has been no attribute change' do - People.create(first_name: 'Jane', last_name: 'Doe', card_number: 75_801_887) - - person = People.search('Jane').first - - expect do - person.update(first_name: 'Jane') - end.not_to change(People.index.tasks['results'], :size) - end - - it 'does not auto-remove' do - People.create(first_name: 'Joanna', last_name: 'Banana', card_number: 75_801_888) - joanna = People.search('Joanna')[0] - joanna.destroy - result = People.raw_search('Joanna') - expect(result['hits'].size).to eq(1) - end - - it 'is able to remove manually' do - bob = People.create(first_name: 'Bob', last_name: 'Sponge', card_number: 75_801_889) - result = People.raw_search('Bob') - expect(result['hits'].size).to eq(1) - bob.remove_from_index! - result = People.raw_search('Bob') - expect(result['hits'].size).to eq(0) - end - - it 'clears index manually' do - People.create(first_name: 'Jane', last_name: 'Doe', card_number: 75_801_887) - results = People.raw_search('') - expect(results['hits'].size).not_to eq(0) - People.clear_index!(true) - results = People.raw_search('') - expect(results['hits'].size).to eq(0) - end -end - describe 'Songs' do before(:all) { MeiliSearch::Rails.configuration[:per_environment] = false } diff --git a/spec/model_methods_spec.rb b/spec/model_methods_spec.rb index 55eccacc..83529e4c 100644 --- a/spec/model_methods_spec.rb +++ b/spec/model_methods_spec.rb @@ -1,5 +1,6 @@ require 'support/models/color' require 'support/models/book' +require 'support/models/people' describe 'Model methods' do describe '.reindex!' do @@ -21,6 +22,24 @@ end end + describe '.clear_index!' do + context 'when :auto_remove is disabled' do + it 'clears index manually' do + TestUtil.reset_people! + + People.create(first_name: 'Jane', last_name: 'Doe', card_number: 75_801_887) + + results = People.raw_search('') + expect(results['hits']).not_to be_empty + + People.clear_index!(true) + + results = People.raw_search('') + expect(results['hits']).to be_empty + end + end + end + describe '.without_auto_index' do it 'disables auto indexing for the model' do TestUtil.reset_colors! diff --git a/spec/options_spec.rb b/spec/options_spec.rb index 0444dc57..74fd478d 100644 --- a/spec/options_spec.rb +++ b/spec/options_spec.rb @@ -1,8 +1,25 @@ require 'support/models/color' require 'support/models/book' require 'support/models/animals' +require 'support/models/people' describe 'meilisearch_options' do + describe ':index_uid' do + it 'sets the index uid specified' do + TestUtil.reset_people! + People.create(first_name: 'Jane', last_name: 'Doe', card_number: 75_801_887) + expect(People.index.uid).to eq("#{safe_index_uid('MyCustomPeople')}_test") + end + end + + describe ':primary_key' do + it 'sets the primary key specified' do + TestUtil.reset_people! + People.create(first_name: 'Jane', last_name: 'Doe', card_number: 75_801_887) + expect(People.index.fetch_info.primary_key).to eq('card_number') + end + end + describe ':index_uid and :primary_key (shared index)' do it 'index uid is the same' do cat_index = Cat.index_uid @@ -54,6 +71,24 @@ end end + describe ':auto_remove' do + context 'when false' do + it 'does not remove document on destroy' do + TestUtil.reset_people! + + joanna = People.create(first_name: 'Joanna', last_name: 'Banana', card_number: 75_801_888) + + result = People.raw_search('Joanna') + expect(result['hits']).to be_one + + joanna.destroy + + result = People.raw_search('Joanna') + expect(result['hits']).to be_one + end + end + end + describe ':sanitize' do context 'when true' do it 'sanitizes attributes' do diff --git a/spec/settings_spec.rb b/spec/settings_spec.rb index 7e35990a..b3731dba 100644 --- a/spec/settings_spec.rb +++ b/spec/settings_spec.rb @@ -1,7 +1,21 @@ require 'spec_helper' require 'support/models/book' +require 'support/models/people' describe MeiliSearch::Rails::IndexSettings do + describe 'add_attribute' do + context 'with a symbol' do + it 'calls method for new attribute' do + TestUtil.reset_people! + + People.create(first_name: 'Jane', last_name: 'Doe', card_number: 75_801_887) + + result = People.raw_search('Jane') + expect(result['hits'][0]['full_name']).to eq('Jane Doe') + end + end + end + describe 'faceting' do it 'respects max values per facet' do TestUtil.reset_books! diff --git a/spec/support/models/people.rb b/spec/support/models/people.rb index 7c7c18da..590aeaf6 100644 --- a/spec/support/models/people.rb +++ b/spec/support/models/people.rb @@ -22,3 +22,10 @@ def will_save_change_to_full_name? will_save_change_to_first_name? || will_save_change_to_last_name? end end + +module TestUtil + def self.reset_people! + People.clear_index!(true) + People.delete_all + end +end From 5c7a6e41ae65c5182c855aad50116fef4a5f57b7 Mon Sep 17 00:00:00 2001 From: ellnix <103502144+ellnix@users.noreply.github.com> Date: Fri, 1 Mar 2024 16:18:45 +0100 Subject: [PATCH 14/36] Move Disabled specs from integration_spec.rb --- spec/integration_spec.rb | 23 ----------------------- spec/options_spec.rb | 27 +++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index 531480c1..1347fa9b 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -476,29 +476,6 @@ end end -describe 'Disabled' do - before(:all) do - DisabledBoolean.index.delete_all_documents! - DisabledProc.index.delete_all_documents! - DisabledSymbol.index.delete_all_documents! - end - - it 'disables the indexing using a boolean' do - DisabledBoolean.create name: 'foo' - expect(DisabledBoolean.search('').size).to eq(0) - end - - it 'disables the indexing using a proc' do - DisabledProc.create name: 'foo' - expect(DisabledProc.search('').size).to eq(0) - end - - it 'disables the indexing using a symbol' do - DisabledSymbol.create name: 'foo' - expect(DisabledSymbol.search('').size).to eq(0) - end -end - unless OLD_RAILS describe 'EnqueuedDocument' do it 'enqueues a job' do diff --git a/spec/options_spec.rb b/spec/options_spec.rb index 74fd478d..b68baa29 100644 --- a/spec/options_spec.rb +++ b/spec/options_spec.rb @@ -2,6 +2,7 @@ require 'support/models/book' require 'support/models/animals' require 'support/models/people' +require 'support/models/disabled_models' describe 'meilisearch_options' do describe ':index_uid' do @@ -89,6 +90,32 @@ end end + describe ':disable_indexing' do + it 'prevents indexing when disabled with a boolean' do + # manually trigger index creation since indexing is disabled + DisabledBoolean.index + + DisabledBoolean.create name: 'foo' + expect(DisabledBoolean.search('')).to be_empty + end + + it 'prevents indexing when disabled with a proc' do + # manually trigger index creation since indexing is disabled + DisabledProc.index + + DisabledProc.create name: 'foo' + expect(DisabledProc.search('')).to be_empty + end + + it 'prevents indexing when disabled with a symbol (method)' do + # manually trigger index creation since indexing is disabled + DisabledSymbol.index + + DisabledSymbol.create name: 'foo' + expect(DisabledSymbol.search('')).to be_empty + end + end + describe ':sanitize' do context 'when true' do it 'sanitizes attributes' do From 55bf813eadd22ef6745e86846a31934b8fc95b73 Mon Sep 17 00:00:00 2001 From: ellnix Date: Sat, 25 May 2024 16:14:07 +0200 Subject: [PATCH 15/36] Move Movie, Restaurant, and pagination specs Should look into adding pagy specs in the future, along with https://github.com/meilisearch/meilisearch-rails/issues/362 --- spec/instance_methods_spec.rb | 11 +- spec/integration_spec.rb | 179 -------------------------- spec/pagination/kaminari_spec.rb | 59 +++++++++ spec/pagination/pagy_spec.rb | 19 +++ spec/pagination/will_paginate_spec.rb | 42 ++++++ spec/settings_spec.rb | 33 +++++ spec/support/models/movie.rb | 7 + spec/support/models/restaurant.rb | 2 +- 8 files changed, 171 insertions(+), 181 deletions(-) create mode 100644 spec/pagination/kaminari_spec.rb create mode 100644 spec/pagination/pagy_spec.rb create mode 100644 spec/pagination/will_paginate_spec.rb diff --git a/spec/instance_methods_spec.rb b/spec/instance_methods_spec.rb index 9c3707f9..387311ad 100644 --- a/spec/instance_methods_spec.rb +++ b/spec/instance_methods_spec.rb @@ -1,6 +1,7 @@ require 'support/models/book' require 'support/models/animals' require 'support/models/people' +require 'support/models/movie' describe 'Instance methods' do describe '#ms_entries' do @@ -49,7 +50,15 @@ end describe '#ms_index!' do - it 'returns array of tasks' do + it 'returns array with single task with single index' do + TestUtil.reset_movies! + + task = Movie.create(title: 'Harry Potter').ms_index! + + expect(task).to contain_exactly(a_hash_including('taskUid')) + end + + it 'returns array of tasks with multiple indexes' do TestUtil.reset_books! moby_dick = Book.create! name: 'Moby Dick', author: 'Herman Melville', premium: false, released: true diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index 1347fa9b..8b89d445 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -297,185 +297,6 @@ end end -describe 'Movie' do - before(:all) do - Movie.clear_index!(true) - end - - it 'returns array of single task hash on #ms_index!' do - movie = Movie.create(title: 'Harry Potter') - - task = movie.ms_index! - - expect(task).to contain_exactly(a_hash_including('taskUid')) - end - - it 'does not return any record with typo' do - Movie.create(title: 'Harry Potter') - - expect(Movie.search('harry pottr', matching_strategy: 'all').size).to eq(0) - end -end - -describe 'Kaminari' do - before(:all) do - require 'kaminari' - MeiliSearch::Rails.configuration[:pagination_backend] = :kaminari - Restaurant.clear_index!(true) - - 10.times do - Restaurant.create( - name: Faker::Restaurant.name, - kind: Faker::Restaurant.type, - description: Faker::Restaurant.description - ) - end - - Restaurant.reindex!(MeiliSearch::Rails::IndexSettings::DEFAULT_BATCH_SIZE, true) - end - - after(:all) { MeiliSearch::Rails.configuration[:pagination_backend] = nil } - - it 'paginates' do - hits = Restaurant.search '' - expect(hits.total_count).to eq(Restaurant.raw_search('')['hits'].size) - - p1 = Restaurant.search '', page: 1, hits_per_page: 1 - expect(p1.size).to eq(1) - expect(p1[0]).to eq(hits[0]) - expect(p1.total_count).to eq(Restaurant.raw_search('')['hits'].count) - - p2 = Restaurant.search '', page: 2, hits_per_page: 1 - expect(p2.size).to eq(1) - expect(p2[0]).to eq(hits[1]) - expect(p2.total_count).to eq(Restaurant.raw_search('')['hits'].count) - end - - it 'respects both camelCase and snake_case options' do - expect(Restaurant.count).to be > 1 - - # TODO: deprecate all camelcase attributes on v1. - %i[hits_per_page hitsPerPage].each do |method| - restaurants = Restaurant.search '', { page: 1, method => 1 } - - expect(restaurants.size).to eq(1) - end - end - - it 'does not return error if pagination params are strings' do - p1 = Restaurant.search '', page: '1', hits_per_page: '1' - expect(p1.size).to eq(1) - expect(p1.total_count).to eq(Restaurant.raw_search('')['hits'].count) - - p2 = Restaurant.search '', page: '2', hits_per_page: '1' - expect(p2.size).to eq(1) - expect(p2.total_count).to eq(Restaurant.raw_search('')['hits'].count) - end - - it 'returns records less than or equal to max_total_hits' do - expect(Restaurant.search('*').size).to eq(5) - end -end - -describe 'Will_paginate' do - before(:all) do - require 'will_paginate' - MeiliSearch::Rails.configuration[:pagination_backend] = :will_paginate - Movie.clear_index!(true) - - 10.times { Movie.create(title: Faker::Movie.title) } - - Movie.reindex!(MeiliSearch::Rails::IndexSettings::DEFAULT_BATCH_SIZE, true) - end - - after(:all) { MeiliSearch::Rails.configuration[:pagination_backend] = nil } - - it 'paginates' do - hits = Movie.search '', hits_per_page: 2 - expect(hits.per_page).to eq(2) - expect(hits.total_pages).to eq(3) - expect(hits.total_entries).to eq(Movie.raw_search('')['hits'].count) - end - - it 'returns most relevant elements in the first page' do - hits = Movie.search '', hits_per_page: 2 - raw_hits = Movie.raw_search '' - expect(hits[0]['id']).to eq(raw_hits['hits'][0]['id'].to_i) - - hits = Movie.search '', hits_per_page: 2, page: 2 - raw_hits = Movie.raw_search '' - expect(hits[0]['id']).to eq(raw_hits['hits'][2]['id'].to_i) - end - - it 'does not return error if pagination params are strings' do - hits = Movie.search '', hits_per_page: '5' - expect(hits.per_page).to eq(5) - expect(hits.total_pages).to eq(1) - expect(hits.current_page).to eq(1) - - hits = Movie.search '', hits_per_page: '5', page: '2' - expect(hits.current_page).to eq(2) - end - - it 'returns records less than or equal to max_total_hits' do - expect(Movie.search('*').size).to eq(5) - end -end - -describe 'with pagination by pagy' do - before(:all) do - MeiliSearch::Rails.configuration[:pagination_backend] = :pagy - MeiliSearch::Rails.configuration[:per_environment] = false - end - - after(:all) do - MeiliSearch::Rails.configuration[:pagination_backend] = nil - MeiliSearch::Rails.configuration[:per_environment] = true - end - - it 'has meaningful error when pagy is set as the pagination_backend' do - Movie.create(title: 'Harry Potter').index!(true) - - logger = double - - allow(logger).to receive(:warn) - allow(MeiliSearch::Rails).to receive(:logger).and_return(logger) - - Movie.search('') - - expect(logger).to have_received(:warn) - .with('[meilisearch-rails] Remove `pagination_backend: :pagy` from your initializer, `pagy` it is not required for `pagy`') - end -end - -describe 'attributes_to_crop' do - before(:all) do - MeiliSearch::Rails.configuration[:per_environment] = false - - 10.times do - Restaurant.create( - name: Faker::Restaurant.name, - kind: Faker::Restaurant.type, - description: Faker::Restaurant.description - ) - end - - Restaurant.reindex!(MeiliSearch::Rails::IndexSettings::DEFAULT_BATCH_SIZE, true) - end - - after(:all) { MeiliSearch::Rails.configuration[:per_environment] = true } - - it 'includes _formatted object' do - results = Restaurant.search('') - raw_search_results = Restaurant.raw_search('') - expect(results[0].formatted).not_to be_nil - expect(results[0].formatted).to eq(raw_search_results['hits'].first['_formatted']) - expect(results.first.formatted['description'].length).to be < results.first['description'].length - expect(results.first.formatted['description']).to eq(raw_search_results['hits'].first['_formatted']['description']) - expect(results.first.formatted['description']).not_to eq(results.first['description']) - end -end - unless OLD_RAILS describe 'EnqueuedDocument' do it 'enqueues a job' do diff --git a/spec/pagination/kaminari_spec.rb b/spec/pagination/kaminari_spec.rb new file mode 100644 index 00000000..caba2176 --- /dev/null +++ b/spec/pagination/kaminari_spec.rb @@ -0,0 +1,59 @@ +require 'kaminari' +require 'support/models/restaurant' + +describe 'Pagination with kaminari' do + before(:all) do + MeiliSearch::Rails.configuration[:pagination_backend] = :kaminari + Restaurant.clear_index! + + 3.times do + Restaurant.create( + name: Faker::Restaurant.name, + kind: Faker::Restaurant.type, + description: Faker::Restaurant.description + ) + end + end + + it 'paginates' do + first, second = Restaurant.search '' + + p1 = Restaurant.search '', page: 1, hits_per_page: 1 + expect(p1).to be_one + expect(p1).to contain_exactly(first) + + p2 = Restaurant.search '', page: 2, hits_per_page: 1 + expect(p2).to be_one + expect(p2).to contain_exactly(second) + end + + it 'returns number of total results' do + hits = Restaurant.search '' + expect(hits.total_count).to eq(2) + + p1 = Restaurant.search '', page: 1, hits_per_page: 1 + expect(p1.total_count).to eq(2) + end + + it 'respects both camelCase options' do + # TODO: deprecate all camelcase attributes on v1. + restaurants = Restaurant.search '', { page: 1, hitsPerPage: 1 } + + expect(restaurants).to be_one + expect(restaurants.total_count).to be > 1 + end + + it 'accepts string options' do + p1 = Restaurant.search '', page: '1', hits_per_page: '1' + expect(p1).to be_one + expect(p1.total_count).to eq(Restaurant.raw_search('')['hits'].count) + + p2 = Restaurant.search '', page: '2', hits_per_page: '1' + expect(p2.size).to eq(1) + expect(p2.total_count).to eq(Restaurant.raw_search('')['hits'].count) + end + + it 'respects max_total_hits' do + expect(Restaurant.search('*').count).to eq(2) + end +end diff --git a/spec/pagination/pagy_spec.rb b/spec/pagination/pagy_spec.rb new file mode 100644 index 00000000..8d78f49d --- /dev/null +++ b/spec/pagination/pagy_spec.rb @@ -0,0 +1,19 @@ +require 'support/models/movie' + +describe 'Pagination with pagy' do + it 'has meaningful error when pagy is set as the pagination_backend' do + Movie.create(title: 'Harry Potter').index!(true) + + logger = double + + allow(logger).to receive(:warn) + allow(MeiliSearch::Rails).to receive(:logger).and_return(logger) + + MeiliSearch::Rails.configuration[:pagination_backend] = :pagy + + Movie.search('') + + expect(logger).to have_received(:warn) + .with('[meilisearch-rails] Remove `pagination_backend: :pagy` from your initializer, `pagy` it is not required for `pagy`') + end +end diff --git a/spec/pagination/will_paginate_spec.rb b/spec/pagination/will_paginate_spec.rb new file mode 100644 index 00000000..8c181cdc --- /dev/null +++ b/spec/pagination/will_paginate_spec.rb @@ -0,0 +1,42 @@ +require 'will_paginate' +require 'support/models/movie' + +describe 'Pagination with will_paginate' do + before(:all) do + MeiliSearch::Rails.configuration[:pagination_backend] = :will_paginate + Movie.clear_index! + + 6.times { Movie.create(title: Faker::Movie.title) } + end + + it 'paginates with sort' do + unpaged_hits = Movie.search '' + + hits = Movie.search '', hits_per_page: 2 + expect(hits).to eq(unpaged_hits[0..1]) + + hits = Movie.search '', hits_per_page: 2, page: 2 + expect(hits).to eq(unpaged_hits[2..3]) + end + + it 'returns paging metadata' do + hits = Movie.search '', hits_per_page: 2 + expect(hits.per_page).to eq(2) + expect(hits.total_pages).to eq(3) + expect(hits.total_entries).to eq(5) + end + + it 'accepts string options' do + hits = Movie.search '', hits_per_page: '5' + expect(hits.per_page).to eq(5) + expect(hits.total_pages).to eq(1) + expect(hits.current_page).to eq(1) + + hits = Movie.search '', hits_per_page: '5', page: '2' + expect(hits.current_page).to eq(2) + end + + it 'respects max_total_hits' do + expect(Movie.search('*').count).to eq(5) + end +end diff --git a/spec/settings_spec.rb b/spec/settings_spec.rb index b3731dba..33726c77 100644 --- a/spec/settings_spec.rb +++ b/spec/settings_spec.rb @@ -1,6 +1,7 @@ require 'spec_helper' require 'support/models/book' require 'support/models/people' +require 'support/models/restaurant' describe MeiliSearch::Rails::IndexSettings do describe 'add_attribute' do @@ -35,6 +36,14 @@ end describe 'typo_tolerance' do + it 'does not return any record with type when disabled' do + TestUtil.reset_movies! + + Movie.create(title: 'Harry Potter') + + expect(Movie.search('harry pottr', matching_strategy: 'all')).to be_empty + end + it 'searches with one typo min size' do TestUtil.reset_books! @@ -59,6 +68,30 @@ end end + describe 'attributes_to_crop' do + before(:all) do + 10.times do + Restaurant.create( + name: Faker::Restaurant.name, + kind: Faker::Restaurant.type, + description: Faker::Restaurant.description + ) + end + + Restaurant.reindex!(MeiliSearch::Rails::IndexSettings::DEFAULT_BATCH_SIZE, true) + end + + it 'includes _formatted object' do + results = Restaurant.search('') + raw_search_results = Restaurant.raw_search('') + expect(results[0].formatted).not_to be_nil + expect(results[0].formatted).to eq(raw_search_results['hits'].first['_formatted']) + expect(results.first.formatted['description'].length).to be < results.first['description'].length + expect(results.first.formatted['description']).to eq(raw_search_results['hits'].first['_formatted']['description']) + expect(results.first.formatted['description']).not_to eq(results.first['description']) + end + end + describe 'settings change detection' do let(:record) { Color.create name: 'dark-blue', short_name: 'blue' } diff --git a/spec/support/models/movie.rb b/spec/support/models/movie.rb index 5198a286..0f6338ec 100644 --- a/spec/support/models/movie.rb +++ b/spec/support/models/movie.rb @@ -12,3 +12,10 @@ class Movie < ActiveRecord::Base typo_tolerance enabled: false end end + +module TestUtil + def self.reset_movies! + Movie.clear_index!(true) + Movie.delete_all + end +end diff --git a/spec/support/models/restaurant.rb b/spec/support/models/restaurant.rb index d39f4071..6c36211e 100644 --- a/spec/support/models/restaurant.rb +++ b/spec/support/models/restaurant.rb @@ -13,6 +13,6 @@ class Restaurant < ActiveRecord::Base meilisearch index_uid: safe_index_uid('Restaurant') do attributes_to_crop [:description] crop_length 10 - pagination max_total_hits: 5 + pagination max_total_hits: 2 end end From 91ac1af3d91bcd1bcf6b662fe896e0a9ee95d946 Mon Sep 17 00:00:00 2001 From: ellnix Date: Sat, 25 May 2024 16:17:23 +0200 Subject: [PATCH 16/36] `require --spec-helper` in .rspec --- .rspec | 1 + 1 file changed, 1 insertion(+) diff --git a/.rspec b/.rspec index 4e1e0d2f..83e16f80 100644 --- a/.rspec +++ b/.rspec @@ -1 +1,2 @@ --color +--require spec_helper From 4cf7ad7beb69bb6a6d72fc3e5ff57c1413427b58 Mon Sep 17 00:00:00 2001 From: ellnix Date: Sat, 25 May 2024 17:33:31 +0200 Subject: [PATCH 17/36] Move 'an imaginary store' tests --- spec/integration_spec.rb | 185 --------------------------------- spec/system/tech_shop_spec.rb | 186 ++++++++++++++++++++++++++++++++++ 2 files changed, 186 insertions(+), 185 deletions(-) create mode 100644 spec/system/tech_shop_spec.rb diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index 8b89d445..289038f8 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -112,191 +112,6 @@ end end -describe 'An imaginary store' do - before(:all) do - Product.clear_index!(true) - - # Google products - @blackberry = Product.create!(name: 'blackberry', href: 'google', tags: ['decent', 'businessmen love it']) - @nokia = Product.create!(name: 'nokia', href: 'google', tags: ['decent']) - - # Amazon products - @android = Product.create!(name: 'android', href: 'amazon', tags: ['awesome']) - @samsung = Product.create!(name: 'samsung', href: 'amazon', tags: ['decent']) - @motorola = Product.create!(name: 'motorola', href: 'amazon', tags: ['decent'], - description: 'Not sure about features since I\'ve never owned one.') - - # Ebay products - @palmpre = Product.create!(name: 'palmpre', href: 'ebay', tags: ['discontinued', 'worst phone ever']) - @palm_pixi_plus = Product.create!(name: 'palm pixi plus', href: 'ebay', tags: ['terrible']) - @lg_vortex = Product.create!(name: 'lg vortex', href: 'ebay', tags: ['decent']) - @t_mobile = Product.create!(name: 't mobile', href: 'ebay', tags: ['terrible']) - - # Yahoo products - @htc = Product.create!(name: 'htc', href: 'yahoo', tags: ['decent']) - @htc_evo = Product.create!(name: 'htc evo', href: 'yahoo', tags: ['decent']) - @ericson = Product.create!(name: 'ericson', href: 'yahoo', tags: ['decent']) - - # Apple products - @iphone = Product.create!(name: 'iphone', href: 'apple', tags: ['awesome', 'poor reception'], - description: 'Puts even more features at your fingertips') - @macbook = Product.create!(name: 'macbookpro', href: 'apple') - - # Unindexed products - @sekrit = Product.create!(name: 'super sekrit', href: 'amazon', release_date: Time.now + 1.day) - @no_href = Product.create!(name: 'super sekrit too; missing href') - - # Subproducts - @camera = Camera.create!(name: 'canon eos rebel t3', href: 'canon') - - 100.times { Product.create!(name: 'crapoola', href: 'crappy', tags: ['crappy']) } - - @products_in_database = Product.all - - Product.reindex!(MeiliSearch::Rails::IndexSettings::DEFAULT_BATCH_SIZE, true) - end - - it 'is not synchronous' do - p = Product.new - p.valid? - - expect(p).not_to be_ms_synchronous - end - - it 'is able to reindex manually' do - results_before_clearing = Product.raw_search('') - expect(results_before_clearing['hits'].size).not_to be(0) - Product.clear_index!(true) - results = Product.raw_search('') - expect(results['hits'].size).to be(0) - Product.reindex!(MeiliSearch::Rails::IndexSettings::DEFAULT_BATCH_SIZE, true) - results_after_reindexing = Product.raw_search('') - expect(results_after_reindexing['hits'].size).not_to be(0) - expect(results_before_clearing['hits'].size).to be(results_after_reindexing['hits'].size) - end - - describe 'basic searching' do - it 'finds the iphone' do - results = Product.search('iphone') - expect(results.size).to eq(1) - expect(results).to include(@iphone) - end - - it 'searches case insensitively' do - results = Product.search('IPHONE') - expect(results.size).to eq(1) - expect(results).to include(@iphone) - end - - it 'finds all amazon products' do - results = Product.search('amazon') - expect(results.size).to eq(3) - expect(results).to include(@android, @samsung, @motorola) - end - - it 'finds all "palm" phones with wildcard word search' do - results = Product.search('pal') - expect(results.size).to eq(2) - expect(results).to include(@palmpre, @palm_pixi_plus) - end - - it 'searches multiple words from the same field' do - results = Product.search('palm pixi plus') - expect(results.size).to eq(1) - expect(results).to include(@palm_pixi_plus) - end - - it 'finds using phrase search' do - results = Product.search('coco "palm"') - expect(results.size).to eq(1) - expect(results).to include(@palm_pixi_plus) - end - - it 'narrows the results by searching across multiple fields' do - results = Product.search('apple iphone') - expect(results.size).to eq(2) - expect(results).to include(@iphone) - end - - it 'does not search on non-indexed fields' do - results = Product.search('features') - expect(results.size).to eq(0) - end - - it 'deletes the associated record' do - ipad = Product.create!(name: 'ipad', href: 'apple', tags: ['awesome', 'great battery'], - description: 'Big screen') - - ipad.index!(true) - results = Product.search('ipad') - expect(results.size).to eq(1) - - ipad.destroy - results = Product.search('ipad') - expect(results.size).to eq(0) - end - - context 'when a document cannot be found in ActiveRecord' do - it 'does not throw an exception' do - Product.index.add_documents!(@palmpre.attributes.merge(id: -1)) - expect { Product.search('pal').to_json }.not_to raise_error - Product.index.delete_document!(-1) - end - - it 'returns the other results if those are still available locally' do - Product.index.add_documents!(@palmpre.attributes.merge(id: -1)) - expect(JSON.parse(Product.search('pal').to_json).size).to eq(2) - Product.index.delete_document!(-1) - end - end - - it 'does not duplicate an already indexed record' do - expect(Product.search('nokia').size).to eq(1) - @nokia.index! - expect(Product.search('nokia').size).to eq(1) - @nokia.index! - @nokia.index! - expect(Product.search('nokia').size).to eq(1) - end - - it 'does not return products that are not indexable' do - @sekrit.index! - @no_href.index! - results = Product.search('sekrit') - expect(results.size).to eq(0) - end - - it 'includes items belong to subclasses' do - @camera.index! - results = Product.search('eos rebel') - expect(results.size).to eq(1) - expect(results).to include(@camera) - end - - it 'deletes a not-anymore-indexable product' do - results = Product.search('sekrit') - expect(results.size).to eq(0) - - @sekrit.release_date = Time.now - 1.day - @sekrit.save! - @sekrit.index!(true) - results = Product.search('sekrit') - expect(results.size).to eq(1) - - @sekrit.release_date = Time.now + 1.day - @sekrit.save! - @sekrit.index!(true) - results = Product.search('sekrit') - expect(results.size).to eq(0) - end - - it 'finds using synonyms' do - expect(Product.search('pomme').size).to eq(Product.search('apple').size) - expect(Product.search('m_b_p').size).to eq(Product.search('macbookpro').size) - end - end -end - unless OLD_RAILS describe 'EnqueuedDocument' do it 'enqueues a job' do diff --git a/spec/system/tech_shop_spec.rb b/spec/system/tech_shop_spec.rb new file mode 100644 index 00000000..f3602539 --- /dev/null +++ b/spec/system/tech_shop_spec.rb @@ -0,0 +1,186 @@ +require 'support/models/product' + +describe 'Tech shop' do + before(:all) do + Product.clear_index!(true) + + # Google products + @blackberry = Product.create!(name: 'blackberry', href: 'google', tags: ['decent', 'businessmen love it']) + @nokia = Product.create!(name: 'nokia', href: 'google', tags: ['decent']) + + # Amazon products + @android = Product.create!(name: 'android', href: 'amazon', tags: ['awesome']) + @samsung = Product.create!(name: 'samsung', href: 'amazon', tags: ['decent']) + @motorola = Product.create!(name: 'motorola', href: 'amazon', tags: ['decent'], + description: 'Not sure about features since I\'ve never owned one.') + + # Ebay products + @palmpre = Product.create!(name: 'palmpre', href: 'ebay', tags: ['discontinued', 'worst phone ever']) + @palm_pixi_plus = Product.create!(name: 'palm pixi plus', href: 'ebay', tags: ['terrible']) + @lg_vortex = Product.create!(name: 'lg vortex', href: 'ebay', tags: ['decent']) + @t_mobile = Product.create!(name: 't mobile', href: 'ebay', tags: ['terrible']) + + # Yahoo products + @htc = Product.create!(name: 'htc', href: 'yahoo', tags: ['decent']) + @htc_evo = Product.create!(name: 'htc evo', href: 'yahoo', tags: ['decent']) + @ericson = Product.create!(name: 'ericson', href: 'yahoo', tags: ['decent']) + + # Apple products + @iphone = Product.create!(name: 'iphone', href: 'apple', tags: ['awesome', 'poor reception'], + description: 'Puts even more features at your fingertips') + @macbook = Product.create!(name: 'macbookpro', href: 'apple') + + # Unindexed products + @sekrit = Product.create!(name: 'super sekrit', href: 'amazon', release_date: Time.now + 1.day) + @no_href = Product.create!(name: 'super sekrit too; missing href') + + # Subproducts + @camera = Camera.create!(name: 'canon eos rebel t3', href: 'canon') + + 100.times { Product.create!(name: 'crapoola', href: 'crappy', tags: ['crappy']) } + + @products_in_database = Product.all + + Product.reindex!(MeiliSearch::Rails::IndexSettings::DEFAULT_BATCH_SIZE, true) + end + + it 'is not synchronous' do + p = Product.new + p.valid? + + expect(p).not_to be_ms_synchronous + end + + it 'is able to reindex manually' do + results_before_clearing = Product.raw_search('') + expect(results_before_clearing['hits'].size).not_to be(0) + Product.clear_index!(true) + results = Product.raw_search('') + expect(results['hits'].size).to be(0) + Product.reindex!(MeiliSearch::Rails::IndexSettings::DEFAULT_BATCH_SIZE, true) + results_after_reindexing = Product.raw_search('') + expect(results_after_reindexing['hits'].size).not_to be(0) + expect(results_before_clearing['hits'].size).to be(results_after_reindexing['hits'].size) + end + + describe 'basic searching' do + it 'finds the iphone' do + results = Product.search('iphone') + expect(results.size).to eq(1) + expect(results).to include(@iphone) + end + + it 'searches case insensitively' do + results = Product.search('IPHONE') + expect(results.size).to eq(1) + expect(results).to include(@iphone) + end + + it 'finds all amazon products' do + results = Product.search('amazon') + expect(results.size).to eq(3) + expect(results).to include(@android, @samsung, @motorola) + end + + it 'finds all "palm" phones with wildcard word search' do + results = Product.search('pal') + expect(results.size).to eq(2) + expect(results).to include(@palmpre, @palm_pixi_plus) + end + + it 'searches multiple words from the same field' do + results = Product.search('palm pixi plus') + expect(results.size).to eq(1) + expect(results).to include(@palm_pixi_plus) + end + + it 'finds using phrase search' do + results = Product.search('coco "palm"') + expect(results.size).to eq(1) + expect(results).to include(@palm_pixi_plus) + end + + it 'narrows the results by searching across multiple fields' do + results = Product.search('apple iphone') + expect(results.size).to eq(2) + expect(results).to include(@iphone) + end + + it 'does not search on non-indexed fields' do + results = Product.search('features') + expect(results.size).to eq(0) + end + + it 'deletes the associated record' do + ipad = Product.create!(name: 'ipad', href: 'apple', tags: ['awesome', 'great battery'], + description: 'Big screen') + + ipad.index!(true) + results = Product.search('ipad') + expect(results.size).to eq(1) + + ipad.destroy + results = Product.search('ipad') + expect(results.size).to eq(0) + end + + context 'when a document cannot be found in ActiveRecord' do + it 'does not throw an exception' do + Product.index.add_documents!(@palmpre.attributes.merge(id: -1)) + expect { Product.search('pal').to_json }.not_to raise_error + Product.index.delete_document!(-1) + end + + it 'returns the other results if those are still available locally' do + Product.index.add_documents!(@palmpre.attributes.merge(id: -1)) + expect(JSON.parse(Product.search('pal').to_json).size).to eq(2) + Product.index.delete_document!(-1) + end + end + + it 'does not duplicate an already indexed record' do + expect(Product.search('nokia').size).to eq(1) + @nokia.index! + expect(Product.search('nokia').size).to eq(1) + @nokia.index! + @nokia.index! + expect(Product.search('nokia').size).to eq(1) + end + + it 'does not return products that are not indexable' do + @sekrit.index! + @no_href.index! + results = Product.search('sekrit') + expect(results.size).to eq(0) + end + + it 'includes items belong to subclasses' do + @camera.index! + results = Product.search('eos rebel') + expect(results.size).to eq(1) + expect(results).to include(@camera) + end + + it 'deletes a not-anymore-indexable product' do + results = Product.search('sekrit') + expect(results.size).to eq(0) + + @sekrit.release_date = Time.now - 1.day + @sekrit.save! + @sekrit.index!(true) + results = Product.search('sekrit') + expect(results.size).to eq(1) + + @sekrit.release_date = Time.now + 1.day + @sekrit.save! + @sekrit.index!(true) + results = Product.search('sekrit') + expect(results.size).to eq(0) + end + + it 'finds using synonyms' do + expect(Product.search('pomme').size).to eq(Product.search('apple').size) + expect(Product.search('m_b_p').size).to eq(Product.search('macbookpro').size) + end + end +end From a437c8e4305ae7528efaaf3b52afdf961b709c0b Mon Sep 17 00:00:00 2001 From: ellnix Date: Sat, 25 May 2024 19:16:14 +0200 Subject: [PATCH 18/36] Refactor tests This is mainly focused around removing interdependencies between tests and ensuring that there are no race conditions. --- spec/integration_spec.rb | 49 +++++++------- spec/model_methods_spec.rb | 2 + spec/options_spec.rb | 5 +- spec/settings_spec.rb | 3 +- spec/support/async_helper.rb | 6 ++ spec/system/tech_shop_spec.rb | 116 ++++++++++++++++------------------ 6 files changed, 94 insertions(+), 87 deletions(-) create mode 100644 spec/support/async_helper.rb diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index 289038f8..b436ddde 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -244,30 +244,31 @@ end end -context 'when a searchable attribute is not an attribute' do - let(:other_people_class) do - Class.new(People) do - def self.name - 'People' - end - end - end - - let(:logger) { instance_double('Logger', warn: nil) } - - before do - allow(MeiliSearch::Rails).to receive(:logger).and_return(logger) - - other_people_class.meilisearch index_uid: safe_index_uid('Others'), primary_key: :card_number do - attribute :first_name - searchable_attributes %i[first_name last_name] - end - end - - it 'warns the user' do - expect(logger).to have_received(:warn).with(/meilisearch-rails.+last_name/) - end -end +# This changes the index uid of the People class as well, making tests unrandomizable +# context 'when a searchable attribute is not an attribute' do +# let(:other_people_class) do +# Class.new(People) do +# def self.name +# 'People' +# end +# end +# end + +# let(:logger) { instance_double('Logger', warn: nil) } + +# before do +# allow(MeiliSearch::Rails).to receive(:logger).and_return(logger) + +# other_people_class.meilisearch index_uid: safe_index_uid('Others'), primary_key: :card_number do +# attribute :first_name +# searchable_attributes %i[first_name last_name] +# end +# end + +# it 'warns the user' do +# expect(logger).to have_received(:warn).with(/meilisearch-rails.+last_name/) +# end +# end context "when have a internal class defined in the app's scope" do it 'does not raise NoMethodError' do diff --git a/spec/model_methods_spec.rb b/spec/model_methods_spec.rb index 83529e4c..9853aafb 100644 --- a/spec/model_methods_spec.rb +++ b/spec/model_methods_spec.rb @@ -1,3 +1,4 @@ +require 'support/async_helper' require 'support/models/color' require 'support/models/book' require 'support/models/people' @@ -28,6 +29,7 @@ TestUtil.reset_people! People.create(first_name: 'Jane', last_name: 'Doe', card_number: 75_801_887) + AsyncHelper.await_last_task results = People.raw_search('') expect(results['hits']).not_to be_empty diff --git a/spec/options_spec.rb b/spec/options_spec.rb index b68baa29..760485a9 100644 --- a/spec/options_spec.rb +++ b/spec/options_spec.rb @@ -1,3 +1,4 @@ +require 'support/async_helper' require 'support/models/color' require 'support/models/book' require 'support/models/animals' @@ -77,12 +78,14 @@ it 'does not remove document on destroy' do TestUtil.reset_people! - joanna = People.create(first_name: 'Joanna', last_name: 'Banana', card_number: 75_801_888) + joanna = People.create(first_name: 'Joanna', last_name: 'Mason', card_number: 75_801_888) + AsyncHelper.await_last_task result = People.raw_search('Joanna') expect(result['hits']).to be_one joanna.destroy + AsyncHelper.await_last_task result = People.raw_search('Joanna') expect(result['hits']).to be_one diff --git a/spec/settings_spec.rb b/spec/settings_spec.rb index 33726c77..79c3e007 100644 --- a/spec/settings_spec.rb +++ b/spec/settings_spec.rb @@ -1,4 +1,4 @@ -require 'spec_helper' +require 'support/async_helper' require 'support/models/book' require 'support/models/people' require 'support/models/restaurant' @@ -10,6 +10,7 @@ TestUtil.reset_people! People.create(first_name: 'Jane', last_name: 'Doe', card_number: 75_801_887) + AsyncHelper.await_last_task result = People.raw_search('Jane') expect(result['hits'][0]['full_name']).to eq('Jane Doe') diff --git a/spec/support/async_helper.rb b/spec/support/async_helper.rb new file mode 100644 index 00000000..27a8be63 --- /dev/null +++ b/spec/support/async_helper.rb @@ -0,0 +1,6 @@ +module AsyncHelper + def self.await_last_task + task = MeiliSearch::Rails.client.tasks['results'].first + MeiliSearch::Rails.client.wait_for_task task['uid'] + end +end diff --git a/spec/system/tech_shop_spec.rb b/spec/system/tech_shop_spec.rb index f3602539..680be3ac 100644 --- a/spec/system/tech_shop_spec.rb +++ b/spec/system/tech_shop_spec.rb @@ -1,8 +1,10 @@ +require 'support/async_helper' require 'support/models/product' describe 'Tech shop' do before(:all) do - Product.clear_index!(true) + Product.delete_all + Product.index.delete_all_documents! # Google products @blackberry = Product.create!(name: 'blackberry', href: 'google', tags: ['decent', 'businessmen love it']) @@ -37,150 +39,142 @@ # Subproducts @camera = Camera.create!(name: 'canon eos rebel t3', href: 'canon') - 100.times { Product.create!(name: 'crapoola', href: 'crappy', tags: ['crappy']) } - - @products_in_database = Product.all - Product.reindex!(MeiliSearch::Rails::IndexSettings::DEFAULT_BATCH_SIZE, true) end - it 'is not synchronous' do - p = Product.new - p.valid? + context 'product' do + it 'defaults to asynchronous' do + p = Product.new - expect(p).not_to be_ms_synchronous - end + expect(p).not_to be_ms_synchronous + end - it 'is able to reindex manually' do - results_before_clearing = Product.raw_search('') - expect(results_before_clearing['hits'].size).not_to be(0) - Product.clear_index!(true) - results = Product.raw_search('') - expect(results['hits'].size).to be(0) - Product.reindex!(MeiliSearch::Rails::IndexSettings::DEFAULT_BATCH_SIZE, true) - results_after_reindexing = Product.raw_search('') - expect(results_after_reindexing['hits'].size).not_to be(0) - expect(results_before_clearing['hits'].size).to be(results_after_reindexing['hits'].size) + it 'supports manual indexing' do + products_before_clear = Product.raw_search('')['hits'] + expect(products_before_clear).not_to be_empty + + Product.clear_index!(true) + + products_after_clear = Product.raw_search('')['hits'] + expect(products_after_clear).to be_empty + Product.reindex!(MeiliSearch::Rails::IndexSettings::DEFAULT_BATCH_SIZE, true) + + products_after_reindex = Product.raw_search('')['hits'] + expect(products_after_reindex).not_to be_empty + expect(products_before_clear).to eq(products_after_reindex) + end end describe 'basic searching' do it 'finds the iphone' do results = Product.search('iphone') - expect(results.size).to eq(1) - expect(results).to include(@iphone) + expect(results).to contain_exactly(@iphone) end it 'searches case insensitively' do results = Product.search('IPHONE') - expect(results.size).to eq(1) - expect(results).to include(@iphone) + expect(results).to contain_exactly(@iphone) end it 'finds all amazon products' do results = Product.search('amazon') - expect(results.size).to eq(3) - expect(results).to include(@android, @samsung, @motorola) + expect(results).to contain_exactly(@android, @samsung, @motorola) end it 'finds all "palm" phones with wildcard word search' do results = Product.search('pal') - expect(results.size).to eq(2) - expect(results).to include(@palmpre, @palm_pixi_plus) + expect(results).to contain_exactly(@palmpre, @palm_pixi_plus) end it 'searches multiple words from the same field' do results = Product.search('palm pixi plus') - expect(results.size).to eq(1) - expect(results).to include(@palm_pixi_plus) + expect(results).to contain_exactly(@palm_pixi_plus) end it 'finds using phrase search' do results = Product.search('coco "palm"') - expect(results.size).to eq(1) - expect(results).to include(@palm_pixi_plus) + expect(results).to contain_exactly(@palm_pixi_plus) end it 'narrows the results by searching across multiple fields' do results = Product.search('apple iphone') - expect(results.size).to eq(2) - expect(results).to include(@iphone) + expect(results).to include(@iphone, @macbook) end it 'does not search on non-indexed fields' do - results = Product.search('features') - expect(results.size).to eq(0) + expect(Product.search('features')).to be_empty end - it 'deletes the associated record' do + it 'deletes associated document on #destroy' do ipad = Product.create!(name: 'ipad', href: 'apple', tags: ['awesome', 'great battery'], description: 'Big screen') ipad.index!(true) results = Product.search('ipad') - expect(results.size).to eq(1) + expect(results).to contain_exactly(ipad) ipad.destroy - results = Product.search('ipad') - expect(results.size).to eq(0) + AsyncHelper.await_last_task + + results = Product.raw_search('ipad')['hits'] + expect(results).to be_empty end context 'when a document cannot be found in ActiveRecord' do it 'does not throw an exception' do Product.index.add_documents!(@palmpre.attributes.merge(id: -1)) - expect { Product.search('pal').to_json }.not_to raise_error + expect { Product.search('pal') }.not_to raise_error Product.index.delete_document!(-1) end - it 'returns the other results if those are still available locally' do + it 'returns other available results' do Product.index.add_documents!(@palmpre.attributes.merge(id: -1)) - expect(JSON.parse(Product.search('pal').to_json).size).to eq(2) + expect(Product.search('pal').size).to eq(2) Product.index.delete_document!(-1) end end - it 'does not duplicate an already indexed record' do - expect(Product.search('nokia').size).to eq(1) + it 'reindexing does not duplicate record' do + expect(Product.search('nokia')).to contain_exactly(@nokia) @nokia.index! - expect(Product.search('nokia').size).to eq(1) + expect(Product.search('nokia')).to contain_exactly(@nokia) @nokia.index! @nokia.index! - expect(Product.search('nokia').size).to eq(1) + expect(Product.search('nokia')).to contain_exactly(@nokia) end it 'does not return products that are not indexable' do @sekrit.index! @no_href.index! results = Product.search('sekrit') - expect(results.size).to eq(0) + expect(results).to be_empty end - it 'includes items belong to subclasses' do + it 'includes instances of subclasses' do @camera.index! results = Product.search('eos rebel') - expect(results.size).to eq(1) - expect(results).to include(@camera) + expect(results).to contain_exactly(@camera) end - it 'deletes a not-anymore-indexable product' do + it 'deletes a document that is no longer indexable' do results = Product.search('sekrit') - expect(results.size).to eq(0) + expect(results).to be_empty - @sekrit.release_date = Time.now - 1.day - @sekrit.save! + @sekrit.update(release_date: Time.now - 1.day) @sekrit.index!(true) results = Product.search('sekrit') - expect(results.size).to eq(1) + expect(results).to contain_exactly(@sekrit) - @sekrit.release_date = Time.now + 1.day + @sekrit.update(release_date: Time.now + 1.day) @sekrit.save! @sekrit.index!(true) results = Product.search('sekrit') - expect(results.size).to eq(0) + expect(results).to be_empty end - it 'finds using synonyms' do - expect(Product.search('pomme').size).to eq(Product.search('apple').size) - expect(Product.search('m_b_p').size).to eq(Product.search('macbookpro').size) + it 'supports synonyms' do + expect(Product.search('pomme')).to eq(Product.search('apple')) + expect(Product.search('m_b_p')).to eq(Product.search('macbookpro')) end end end From 50ec330dedf028cc6a6b7ef8d5543380ef54da4b Mon Sep 17 00:00:00 2001 From: ellnix Date: Sun, 26 May 2024 09:31:18 +0200 Subject: [PATCH 19/36] Move SerializedDocument and EncodedString tests --- spec/integration_spec.rb | 26 -------------------------- spec/settings_spec.rb | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 26 deletions(-) diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index b436ddde..52ff3aca 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -1,31 +1,5 @@ require 'spec_helper' -if defined?(ActiveModel::Serializer) - describe 'SerializedDocument' do - before(:all) do - SerializedDocument.clear_index!(true) - end - - it 'pushes the name but not the other attribute' do - o = SerializedDocument.new name: 'test', skip: 'skip me' - attributes = SerializedDocument.meilisearch_settings.get_attributes(o) - expect(attributes).to eq({ name: 'test' }) - end - end -end -describe 'Encoding' do - before(:all) do - EncodedString.clear_index!(true) - end - - it 'converts to utf-8' do - EncodedString.create! - results = EncodedString.raw_search '' - expect(results['hits'].size).to eq(1) - expect(results['hits'].first['value']).to eq("\xC2\xA0\xE2\x80\xA2\xC2\xA0".force_encoding('utf-8')) - end -end - describe 'Settings change detection' do it 'detects settings changes' do expect(Color.send(:meilisearch_settings_changed?, nil, {})).to be(true) diff --git a/spec/settings_spec.rb b/spec/settings_spec.rb index 79c3e007..a448602a 100644 --- a/spec/settings_spec.rb +++ b/spec/settings_spec.rb @@ -2,6 +2,7 @@ require 'support/models/book' require 'support/models/people' require 'support/models/restaurant' +require 'support/models/specialty_models' describe MeiliSearch::Rails::IndexSettings do describe 'add_attribute' do @@ -93,6 +94,22 @@ end end + describe 'use_serializer' do + it 'only uses the attributes from the serializer' do + o = SerializedDocument.new name: 'test', skip: 'skip me' + attributes = SerializedDocument.meilisearch_settings.get_attributes(o) + expect(attributes).to eq({ name: 'test' }) + end + end + + describe 'force_utf8_encoding' do + it 'converts to utf8' do + EncodedString.create! + results = EncodedString.raw_search '' + expect(results['hits'].first).to include('value' => ' • ') + end + end + describe 'settings change detection' do let(:record) { Color.create name: 'dark-blue', short_name: 'blue' } From 702888bb7fcd108d821eb9a9e58b044df7b0e937 Mon Sep 17 00:00:00 2001 From: ellnix Date: Sun, 26 May 2024 09:45:42 +0200 Subject: [PATCH 20/36] Move NamespacedDocument specs --- .../attributes_have_changed_spec.rb | 18 ++++++++++ .../active_record/model_is_namespaced_spec.rb | 7 ++++ spec/integration_spec.rb | 35 ------------------- spec/settings_spec.rb | 10 ++++++ 4 files changed, 35 insertions(+), 35 deletions(-) create mode 100644 spec/integration/active_record/model_is_namespaced_spec.rb diff --git a/spec/integration/active_record/attributes_have_changed_spec.rb b/spec/integration/active_record/attributes_have_changed_spec.rb index debd6f14..aa11f93d 100644 --- a/spec/integration/active_record/attributes_have_changed_spec.rb +++ b/spec/integration/active_record/attributes_have_changed_spec.rb @@ -46,5 +46,23 @@ allow(book).to receive(:ms_dirty?).and_return(true) expect(Book.ms_must_reindex?(book)).to be(true) end + + it 'always updates when there is no custom _changed? function' do + m = Namespaced::Model.new(another_private_value: 2) + m.save + results = Namespaced::Model.search('42') + expect(results.size).to eq(1) + expect(results[0].id).to eq(m.id) + + m.another_private_value = 5 + m.save + + results = Namespaced::Model.search('42') + expect(results.size).to eq(0) + + results = Namespaced::Model.search('45') + expect(results.size).to eq(1) + expect(results[0].id).to eq(m.id) + end end diff --git a/spec/integration/active_record/model_is_namespaced_spec.rb b/spec/integration/active_record/model_is_namespaced_spec.rb new file mode 100644 index 00000000..0cadb4f2 --- /dev/null +++ b/spec/integration/active_record/model_is_namespaced_spec.rb @@ -0,0 +1,7 @@ +require 'support/models/specialty_models' + +describe 'When a record has associations' do + it 'has an index name without :: hierarchy' do + expect(Namespaced::Model.index_uid.include?('Namespaced_Model')).to be(true) + end +end diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index 52ff3aca..b5997d05 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -20,41 +20,6 @@ end end -describe 'Namespaced::Model' do - before(:all) do - Namespaced::Model.index.delete_all_documents! - end - - it 'has an index name without :: hierarchy' do - expect(Namespaced::Model.index_uid.include?('Namespaced_Model')).to be(true) - end - - it 'uses the block to determine attribute\'s value' do - m = Namespaced::Model.new(another_private_value: 2) - attributes = Namespaced::Model.meilisearch_settings.get_attributes(m) - expect(attributes['customAttr']).to eq(42) - expect(attributes['myid']).to eq(m.id) - end - - it 'always updates when there is no custom _changed? function' do - m = Namespaced::Model.new(another_private_value: 2) - m.save - results = Namespaced::Model.search('42') - expect(results.size).to eq(1) - expect(results[0].id).to eq(m.id) - - m.another_private_value = 5 - m.save - - results = Namespaced::Model.search('42') - expect(results.size).to eq(0) - - results = Namespaced::Model.search('45') - expect(results.size).to eq(1) - expect(results[0].id).to eq(m.id) - end -end - describe 'NestedItem' do before(:all) do NestedItem.clear_index!(true) diff --git a/spec/settings_spec.rb b/spec/settings_spec.rb index a448602a..e7f227c0 100644 --- a/spec/settings_spec.rb +++ b/spec/settings_spec.rb @@ -5,6 +5,16 @@ require 'support/models/specialty_models' describe MeiliSearch::Rails::IndexSettings do + describe 'attribute' do + context 'when passed a block' do + it 'uses the block to determine attribute\'s value' do + m = Namespaced::Model.new(another_private_value: 2) + attributes = Namespaced::Model.meilisearch_settings.get_attributes(m) + expect(attributes).to include('customAttr' => 42, 'myid' => m.id) + end + end + end + describe 'add_attribute' do context 'with a symbol' do it 'calls method for new attribute' do From 862262b1c1916285574afd619f577c93a8f86794 Mon Sep 17 00:00:00 2001 From: ellnix Date: Sun, 26 May 2024 09:50:58 +0200 Subject: [PATCH 21/36] Remove redundant `#search` method test It looks to me like this method simply tested that we define a `#search` method on models, but if we didn't define that method most of the test suite would fail. If these was a different purpose to this test I don't see it. --- spec/integration_spec.rb | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index b5997d05..2de492b6 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -209,16 +209,6 @@ # end # end -context "when have a internal class defined in the app's scope" do - it 'does not raise NoMethodError' do - Task.create(title: 'my task #1') - - expect do - Task.search('task') - end.not_to raise_error - end -end - context 'when MeiliSearch calls are deactivated' do it 'is active by default' do expect(MeiliSearch::Rails).to be_active From 5deaef93b7447e359257b8de38c23b3e1128d51f Mon Sep 17 00:00:00 2001 From: ellnix Date: Mon, 27 May 2024 07:31:14 +0200 Subject: [PATCH 22/36] Move deactivation specs --- .../meilisearch_calls_are_deactivated.rb | 29 +++++++++ spec/integration_spec.rb | 61 ------------------- spec/meilisearch/activation_spec.rb | 41 +++++++++++++ 3 files changed, 70 insertions(+), 61 deletions(-) create mode 100644 spec/integration/active_record/meilisearch_calls_are_deactivated.rb create mode 100644 spec/meilisearch/activation_spec.rb diff --git a/spec/integration/active_record/meilisearch_calls_are_deactivated.rb b/spec/integration/active_record/meilisearch_calls_are_deactivated.rb new file mode 100644 index 00000000..888ca4c5 --- /dev/null +++ b/spec/integration/active_record/meilisearch_calls_are_deactivated.rb @@ -0,0 +1,29 @@ +require 'support/models/task' + +describe 'When meilisearch calls are disabled' do + it 'does not send requests to meilisearch' do + MeiliSearch::Rails.deactivate! + + expect do + Task.create(title: 'my task #1') + Task.search('task') + end.not_to raise_error + + MeiliSearch::Rails.activate! + end + + context 'with a block' do + it 'does not interfere with prior requests' do + Task.destroy_all + Task.create!(title: 'deactivated #1') + + MeiliSearch::Rails.deactivate! do + # always 0 since the black hole will return the default values + expect(Task.search('deactivated').size).to eq(0) + end + + expect(MeiliSearch::Rails).to be_active + expect(Task.search('#1').size).to eq(1) + end + end +end diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index 2de492b6..e942469b 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -209,64 +209,3 @@ # end # end -context 'when MeiliSearch calls are deactivated' do - it 'is active by default' do - expect(MeiliSearch::Rails).to be_active - end - - describe '#deactivate!' do - context 'without block' do - before { MeiliSearch::Rails.deactivate! } - - after { MeiliSearch::Rails.activate! } - - it 'deactivates the requests and keep the state' do - expect(MeiliSearch::Rails).not_to be_active - end - - it 'responds with a black hole' do - expect(MeiliSearch::Rails.client.foo.bar.now.nil.item.issue).to be_nil - end - - it 'deactivates requests' do - expect do - Task.create(title: 'my task #1') - Task.search('task') - end.not_to raise_error - end - end - - context 'with a block' do - it 'disables only around call' do - MeiliSearch::Rails.deactivate! do - expect(MeiliSearch::Rails).not_to be_active - end - - expect(MeiliSearch::Rails).to be_active - end - - it 'works even when the instance made calls earlier' do - Task.destroy_all - Task.create!(title: 'deactivated #1') - - MeiliSearch::Rails.deactivate! do - # always 0 since the black hole will return the default values - expect(Task.search('deactivated').size).to eq(0) - end - - expect(MeiliSearch::Rails).to be_active - expect(Task.search('#1').size).to eq(1) - end - - it 'works in multi-threaded environments' do - Threads.new(5, log: $stdout).assert(20) do |_i, _r| - MeiliSearch::Rails.deactivate! do - expect(MeiliSearch::Rails).not_to be_active - end - - expect(MeiliSearch::Rails).to be_active - end - end - end - end -end diff --git a/spec/meilisearch/activation_spec.rb b/spec/meilisearch/activation_spec.rb new file mode 100644 index 00000000..edceac35 --- /dev/null +++ b/spec/meilisearch/activation_spec.rb @@ -0,0 +1,41 @@ +describe MeiliSearch::Rails do + it 'is active by default' do + expect(MeiliSearch::Rails).to be_active + end + + describe '#deactivate!' do + context 'without block' do + before { MeiliSearch::Rails.deactivate! } + + after { MeiliSearch::Rails.activate! } + + it 'deactivates the requests until activate!-ed' do + expect(MeiliSearch::Rails).not_to be_active + end + + it 'responds with a black hole' do + expect(MeiliSearch::Rails.client.foo.bar.now.nil.item.issue).to be_nil + end + end + + context 'with a block' do + it 'disables only around call' do + MeiliSearch::Rails.deactivate! do + expect(MeiliSearch::Rails).not_to be_active + end + + expect(MeiliSearch::Rails).to be_active + end + + it 'works in multi-threaded environments' do + Threads.new(5, log: $stdout).assert(20) do |_i, _r| + MeiliSearch::Rails.deactivate! do + expect(MeiliSearch::Rails).not_to be_active + end + + expect(MeiliSearch::Rails).to be_active + end + end + end + end +end From 208c9cb3828af53be3b61dbd202aa10e24f47cd5 Mon Sep 17 00:00:00 2001 From: ellnix Date: Mon, 27 May 2024 17:57:26 +0200 Subject: [PATCH 23/36] Move :raise_on_failure specs --- spec/integration_spec.rb | 42 -------------------------------------- spec/options_spec.rb | 44 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 42 deletions(-) diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index e942469b..a69b003c 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -141,48 +141,6 @@ end end -describe 'Raise on failure' do - before { Vegetable.instance_variable_set('@ms_indexes', nil) } - - it 'raises on failure' do - expect do - Fruit.search('', { filter: 'title = Nightshift' }) - end.to raise_error(MeiliSearch::ApiError) - end - - it 'does not raise on failure' do - expect do - Vegetable.search('', { filter: 'title = Kale' }) - end.not_to raise_error - end - - context 'when Meilisearch server take too long to answer' do - let(:index_instance) { instance_double(MeiliSearch::Index, settings: nil, update_settings: nil) } - let(:slow_client) { instance_double(MeiliSearch::Client, index: index_instance) } - - before do - allow(slow_client).to receive(:create_index) - allow(MeiliSearch::Rails).to receive(:client).and_return(slow_client) - end - - it 'does not raise error timeouts on reindex' do - allow(index_instance).to receive(:add_documents).and_raise(MeiliSearch::TimeoutError) - - expect do - Vegetable.create(name: 'potato') - end.not_to raise_error - end - - it 'does not raise error timeouts on data addition' do - allow(index_instance).to receive(:add_documents).and_return(nil) - - expect do - Vegetable.ms_reindex! - end.not_to raise_error - end - end -end - # This changes the index uid of the People class as well, making tests unrandomizable # context 'when a searchable attribute is not an attribute' do # let(:other_people_class) do diff --git a/spec/options_spec.rb b/spec/options_spec.rb index 760485a9..a78d8ca2 100644 --- a/spec/options_spec.rb +++ b/spec/options_spec.rb @@ -149,4 +149,48 @@ end end end + + describe ':raise_on_failure' do + context 'when true' do + it 'raises exception on failure' do + expect do + Fruit.search('', { filter: 'title = Nightshift' }) + end.to raise_error(MeiliSearch::ApiError) + end + end + + context 'when set to false' do + it 'fails without an exception' do + expect do + Vegetable.search('', { filter: 'title = Kale' }) + end.not_to raise_error + end + + context 'in case of timeout' do + let(:index_instance) { instance_double(MeiliSearch::Index, settings: nil, update_settings: nil) } + let(:slow_client) { instance_double(MeiliSearch::Client, index: index_instance) } + + before do + allow(slow_client).to receive(:create_index) + allow(MeiliSearch::Rails).to receive(:client).and_return(slow_client) + end + + it 'does not raise error timeouts on reindex' do + allow(index_instance).to receive(:add_documents).and_raise(MeiliSearch::TimeoutError) + + expect do + Vegetable.create(name: 'potato') + end.not_to raise_error + end + + it 'does not raise error timeouts on data addition' do + allow(index_instance).to receive(:add_documents).and_return(nil) + + expect do + Vegetable.ms_reindex! + end.not_to raise_error + end + end + end + end end From 88bed6f5815bdf8c9e0a1b898acef1e6e01a4549 Mon Sep 17 00:00:00 2001 From: ellnix Date: Mon, 27 May 2024 18:06:59 +0200 Subject: [PATCH 24/36] Fix a few more race conditions in tests --- spec/options_spec.rb | 3 +++ spec/pagination/kaminari_spec.rb | 3 +++ 2 files changed, 6 insertions(+) diff --git a/spec/options_spec.rb b/spec/options_spec.rb index a78d8ca2..034dfea5 100644 --- a/spec/options_spec.rb +++ b/spec/options_spec.rb @@ -97,6 +97,7 @@ it 'prevents indexing when disabled with a boolean' do # manually trigger index creation since indexing is disabled DisabledBoolean.index + AsyncHelper.await_last_task DisabledBoolean.create name: 'foo' expect(DisabledBoolean.search('')).to be_empty @@ -105,6 +106,7 @@ it 'prevents indexing when disabled with a proc' do # manually trigger index creation since indexing is disabled DisabledProc.index + AsyncHelper.await_last_task DisabledProc.create name: 'foo' expect(DisabledProc.search('')).to be_empty @@ -113,6 +115,7 @@ it 'prevents indexing when disabled with a symbol (method)' do # manually trigger index creation since indexing is disabled DisabledSymbol.index + AsyncHelper.await_last_task DisabledSymbol.create name: 'foo' expect(DisabledSymbol.search('')).to be_empty diff --git a/spec/pagination/kaminari_spec.rb b/spec/pagination/kaminari_spec.rb index caba2176..d42dc1aa 100644 --- a/spec/pagination/kaminari_spec.rb +++ b/spec/pagination/kaminari_spec.rb @@ -1,4 +1,5 @@ require 'kaminari' +require 'support/async_helper' require 'support/models/restaurant' describe 'Pagination with kaminari' do @@ -13,6 +14,8 @@ description: Faker::Restaurant.description ) end + + AsyncHelper.await_last_task end it 'paginates' do From 596a7b4f7cf41d7c7c81de5abaf79248f444f04e Mon Sep 17 00:00:00 2001 From: ellnix Date: Mon, 27 May 2024 18:25:45 +0200 Subject: [PATCH 25/36] Move EnqueuedDocument-type tests --- spec/instance_methods_spec.rb | 7 ++++ spec/integration_spec.rb | 60 ----------------------------------- spec/options_spec.rb | 53 +++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 60 deletions(-) diff --git a/spec/instance_methods_spec.rb b/spec/instance_methods_spec.rb index 387311ad..040aa356 100644 --- a/spec/instance_methods_spec.rb +++ b/spec/instance_methods_spec.rb @@ -2,6 +2,7 @@ require 'support/models/animals' require 'support/models/people' require 'support/models/movie' +require 'support/models/queued_models' describe 'Instance methods' do describe '#ms_entries' do @@ -75,6 +76,12 @@ it 'throws error on non-persisted instances' do expect { Color.new(name: 'purple').index!(true) }.to raise_error(ArgumentError) end + + it 'returns empty array when indexing is disabled' do + doc = DisabledEnqueuedDocument.create! name: 'test' + + expect(doc.ms_index!).to be_empty + end end describe '#ms_remove_from_index!' do diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index a69b003c..2d2a4d46 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -51,66 +51,6 @@ end end -unless OLD_RAILS - describe 'EnqueuedDocument' do - it 'enqueues a job' do - expect do - EnqueuedDocument.create! name: 'hellraiser' - end.to raise_error('enqueued hellraiser') - end - - it 'does not enqueue a job inside no index block' do - expect do - EnqueuedDocument.without_auto_index do - EnqueuedDocument.create! name: 'test' - end - end.not_to raise_error - end - end - - describe 'DisabledEnqueuedDocument' do - it '#ms_index! returns an empty array' do - doc = DisabledEnqueuedDocument.create! name: 'test' - - expect(doc.ms_index!).to be_empty - end - - it 'does not try to enqueue a job' do - expect do - DisabledEnqueuedDocument.create! name: 'test' - end.not_to raise_error - end - end - - describe 'ConditionallyEnqueuedDocument' do - before do - allow(MeiliSearch::Rails::MSJob).to receive(:perform_later).and_return(nil) - allow(MeiliSearch::Rails::MSCleanUpJob).to receive(:perform_later).and_return(nil) - end - - it 'does not try to enqueue an index job when :if option resolves to false' do - doc = ConditionallyEnqueuedDocument.create! name: 'test', is_public: false - - expect(MeiliSearch::Rails::MSJob).not_to have_received(:perform_later).with(doc, 'ms_index!') - end - - it 'enqueues an index job when :if option resolves to true' do - doc = ConditionallyEnqueuedDocument.create! name: 'test', is_public: true - - expect(MeiliSearch::Rails::MSJob).to have_received(:perform_later).with(doc, 'ms_index!') - end - - it 'does enqueue a remove_from_index despite :if option' do - doc = ConditionallyEnqueuedDocument.create!(name: 'test', is_public: true) - expect(MeiliSearch::Rails::MSJob).to have_received(:perform_later).with(doc, 'ms_index!') - - doc.destroy! - - expect(MeiliSearch::Rails::MSCleanUpJob).to have_received(:perform_later).with(doc.ms_entries) - end - end -end - describe 'Misconfigured Block' do it 'forces the meilisearch block' do expect do diff --git a/spec/options_spec.rb b/spec/options_spec.rb index 034dfea5..19562c5d 100644 --- a/spec/options_spec.rb +++ b/spec/options_spec.rb @@ -4,6 +4,7 @@ require 'support/models/animals' require 'support/models/people' require 'support/models/disabled_models' +require 'support/models/queued_models' describe 'meilisearch_options' do describe ':index_uid' do @@ -122,6 +123,58 @@ end end + describe ':enqueue' do + context 'when configured with a proc' do + it 'runs proc when created' do + expect do + EnqueuedDocument.create! name: 'hellraiser' + end.to raise_error('enqueued hellraiser') + end + + it 'does not run proc in without_auto_index block' do + expect do + EnqueuedDocument.without_auto_index do + EnqueuedDocument.create! name: 'test' + end + end.not_to raise_error + end + + it 'does not run proc when auto_index is disabled' do + expect do + DisabledEnqueuedDocument.create! name: 'test' + end.not_to raise_error + end + + context 'when :if is configured' do + before do + allow(MeiliSearch::Rails::MSJob).to receive(:perform_later).and_return(nil) + allow(MeiliSearch::Rails::MSCleanUpJob).to receive(:perform_later).and_return(nil) + end + + it 'does not try to enqueue an index job when :if option resolves to false' do + doc = ConditionallyEnqueuedDocument.create! name: 'test', is_public: false + + expect(MeiliSearch::Rails::MSJob).not_to have_received(:perform_later).with(doc, 'ms_index!') + end + + it 'enqueues an index job when :if option resolves to true' do + doc = ConditionallyEnqueuedDocument.create! name: 'test', is_public: true + + expect(MeiliSearch::Rails::MSJob).to have_received(:perform_later).with(doc, 'ms_index!') + end + + it 'does enqueue a remove_from_index despite :if option' do + doc = ConditionallyEnqueuedDocument.create!(name: 'test', is_public: true) + expect(MeiliSearch::Rails::MSJob).to have_received(:perform_later).with(doc, 'ms_index!') + + doc.destroy! + + expect(MeiliSearch::Rails::MSCleanUpJob).to have_received(:perform_later).with(doc.ms_entries) + end + end + end + end + describe ':sanitize' do context 'when true' do it 'sanitizes attributes' do From b328a69d5878410a0668439758c81be804a9dcda Mon Sep 17 00:00:00 2001 From: ellnix Date: Mon, 27 May 2024 18:38:54 +0200 Subject: [PATCH 26/36] Move Song specs --- spec/integration_spec.rb | 22 ---------------------- spec/settings_spec.rb | 25 +++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index 2d2a4d46..9a2ed192 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -59,28 +59,6 @@ end end -describe 'Songs' do - before(:all) { MeiliSearch::Rails.configuration[:per_environment] = false } - - after(:all) { MeiliSearch::Rails.configuration[:per_environment] = true } - - it 'targets multiple indices' do - Song.create!(name: 'Coconut nut', artist: 'Smokey Mountain', premium: false, released: true) # Only song supposed to be added to Songs index - Song.create!(name: 'Smoking hot', artist: 'Cigarettes before lunch', premium: true, released: true) - Song.create!(name: 'Floor is lava', artist: 'Volcano', premium: true, released: false) - Song.index.wait_for_task(Song.index.tasks['results'].first['uid']) - MeiliSearch::Rails.client.index(safe_index_uid('PrivateSongs')).wait_for_task(MeiliSearch::Rails.client.index(safe_index_uid('PrivateSongs')).tasks['results'].first['uid']) - results = Song.search('', index: safe_index_uid('Songs')) - expect(results.size).to eq(1) - raw_results = Song.raw_search('', index: safe_index_uid('Songs')) - expect(raw_results['hits'].size).to eq(1) - results = Song.search('', index: safe_index_uid('PrivateSongs')) - expect(results.size).to eq(3) - raw_results = Song.raw_search('', index: safe_index_uid('PrivateSongs')) - expect(raw_results['hits'].size).to eq(3) - end -end - # This changes the index uid of the People class as well, making tests unrandomizable # context 'when a searchable attribute is not an attribute' do # let(:other_people_class) do diff --git a/spec/settings_spec.rb b/spec/settings_spec.rb index e7f227c0..d17b4548 100644 --- a/spec/settings_spec.rb +++ b/spec/settings_spec.rb @@ -3,6 +3,7 @@ require 'support/models/people' require 'support/models/restaurant' require 'support/models/specialty_models' +require 'support/models/song' describe MeiliSearch::Rails::IndexSettings do describe 'attribute' do @@ -120,6 +121,30 @@ end end + describe 'add_index' do + let(:private_songs_index) { safe_index_uid('PrivateSongs') } + let(:public_songs_index) { safe_index_uid('Songs') } + + it 'targets multiple indexes' do + songs = + [ + Song.create!(name: 'Coconut nut', artist: 'Smokey Mountain', premium: false, released: true), + Song.create!(name: 'Smoking hot', artist: 'Cigarettes before lunch', premium: true, released: true), + Song.create!(name: 'Floor is lava', artist: 'Volcano', premium: true, released: false) + ] + + public_song = songs.first + + AsyncHelper.await_last_task + + public_search = Song.search('', index: public_songs_index) + expect(public_search).to contain_exactly(public_song) + + results = Song.search('', index: private_songs_index) + expect(results).to match(songs) + end + end + describe 'settings change detection' do let(:record) { Color.create name: 'dark-blue', short_name: 'blue' } From 4f1663b2f23694f64003df02bcf557f51baf13f8 Mon Sep 17 00:00:00 2001 From: ellnix Date: Mon, 27 May 2024 18:45:14 +0200 Subject: [PATCH 27/36] Move NestedItem tests --- spec/integration_spec.rb | 31 ------------------------------- spec/options_spec.rb | 27 +++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 31 deletions(-) diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index 9a2ed192..7d0982e9 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -20,37 +20,6 @@ end end -describe 'NestedItem' do - before(:all) do - NestedItem.clear_index!(true) - rescue StandardError - # not fatal - end - - it 'fetches attributes unscoped' do - i1 = NestedItem.create hidden: false - i2 = NestedItem.create hidden: true - - i1.children << NestedItem.create(hidden: true) << NestedItem.create(hidden: true) - NestedItem.where(id: [i1.id, i2.id]).reindex!(MeiliSearch::Rails::IndexSettings::DEFAULT_BATCH_SIZE, true) - - result = NestedItem.index.get_document(i1.id) - expect(result['nb_children']).to eq(2) - - result = NestedItem.raw_search('') - expect(result['hits'].size).to eq(1) - - if i2.respond_to? :update_attributes - i2.update_attributes hidden: false # rubocop:disable Rails/ActiveRecordAliases - else - i2.update hidden: false - end - - result = NestedItem.raw_search('') - expect(result['hits'].size).to eq(2) - end -end - describe 'Misconfigured Block' do it 'forces the meilisearch block' do expect do diff --git a/spec/options_spec.rb b/spec/options_spec.rb index 19562c5d..9f01d874 100644 --- a/spec/options_spec.rb +++ b/spec/options_spec.rb @@ -63,6 +63,33 @@ end end + describe ':unless' do + it 'only indexes the record if it evaluates to false' do + NestedItem.clear_index!(true) + + i1 = NestedItem.create hidden: false + i2 = NestedItem.create hidden: true + + i1.children << NestedItem.create(hidden: true) << NestedItem.create(hidden: true) + NestedItem.where(id: [i1.id, i2.id]).reindex!(MeiliSearch::Rails::IndexSettings::DEFAULT_BATCH_SIZE, true) + + result = NestedItem.index.get_document(i1.id) + expect(result['nb_children']).to eq(2) + + result = NestedItem.raw_search('') + expect(result['hits'].size).to eq(1) + + if i2.respond_to? :update_attributes + i2.update_attributes hidden: false # rubocop:disable Rails/ActiveRecordAliases + else + i2.update hidden: false + end + + result = NestedItem.raw_search('') + expect(result['hits'].size).to eq(2) + end + end + describe ':auto_index' do it 'is enabled by default' do TestUtil.reset_colors! From dcc4c08dc995c8ed602bc5beb4df01714a13d1fb Mon Sep 17 00:00:00 2001 From: ellnix Date: Mon, 27 May 2024 18:48:55 +0200 Subject: [PATCH 28/36] Move misconfiguration test --- .../ms_rails_is_included_without_block_spec.rb | 10 ++++++++++ spec/integration_spec.rb | 8 -------- 2 files changed, 10 insertions(+), 8 deletions(-) create mode 100644 spec/integration/active_record/ms_rails_is_included_without_block_spec.rb diff --git a/spec/integration/active_record/ms_rails_is_included_without_block_spec.rb b/spec/integration/active_record/ms_rails_is_included_without_block_spec.rb new file mode 100644 index 00000000..cc0de7eb --- /dev/null +++ b/spec/integration/active_record/ms_rails_is_included_without_block_spec.rb @@ -0,0 +1,10 @@ +require 'support/models/specialty_models' + +describe 'When MeiliSearch::Rails is included but not called' do + it 'raises an error' do + expect do + MisconfiguredBlock.reindex! + end.to raise_error(ArgumentError) + end +end + diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index 7d0982e9..e55c15cf 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -20,14 +20,6 @@ end end -describe 'Misconfigured Block' do - it 'forces the meilisearch block' do - expect do - MisconfiguredBlock.reindex! - end.to raise_error(ArgumentError) - end -end - # This changes the index uid of the People class as well, making tests unrandomizable # context 'when a searchable attribute is not an attribute' do # let(:other_people_class) do From de4cf44d2cf150b728b829e827ab0d26f145133b Mon Sep 17 00:00:00 2001 From: ellnix Date: Mon, 27 May 2024 19:25:26 +0200 Subject: [PATCH 29/36] Fix searchable attribute warning and its test - Searchable attribute warning only happened on the top level block, with this fix it will work on additional indexes (added with `add_index`) - This involved moving the warning method to IndexSettings which has the side effect of having no access to the model name, however the attribute name alone *should* be enough for a user to figure out the error - The searchable attribute spec created a new class which due to ActiveModel naming and our interdependence on it cannot conveniently be isolated from other tests --- lib/meilisearch-rails.rb | 29 ++++++++++++++--------------- spec/integration_spec.rb | 26 -------------------------- spec/settings_spec.rb | 20 ++++++++++++++++++++ 3 files changed, 34 insertions(+), 41 deletions(-) diff --git a/lib/meilisearch-rails.rb b/lib/meilisearch-rails.rb index 20d727a1..7099e19e 100644 --- a/lib/meilisearch-rails.rb +++ b/lib/meilisearch-rails.rb @@ -83,6 +83,20 @@ class IndexSettings def initialize(options, &block) @options = options instance_exec(&block) if block_given? + warn_searchable_missing_attributes + end + + def warn_searchable_missing_attributes + searchables = get_setting(:searchable_attributes) + attrs = get_setting(:attributes)&.keys + + if searchables.present? && attrs.present? + (searchables.map(&:to_s) - attrs.map(&:to_s)).each do |missing_searchable| + MeiliSearch::Rails.logger.warn( + "[meilisearch-rails] #{missing_searchable} declared in searchable_attributes but not in attributes. Please add it to attributes if it should be searchable." + ) + end + end end def use_serializer(serializer) @@ -463,8 +477,6 @@ def meilisearch(options = {}, &block) after_destroy_commit { |searchable| searchable.ms_enqueue_remove_from_index!(ms_synchronous?) } end end - - warn_searchable_missing_attributes end def ms_without_auto_index(&block) @@ -900,19 +912,6 @@ def ms_attribute_changed?(document, attr_name) # We don't know if the attribute has changed, so conservatively assume it has true end - - def warn_searchable_missing_attributes - searchables = meilisearch_settings.get_setting(:searchable_attributes) - attrs = meilisearch_settings.get_setting(:attributes)&.keys - - if searchables.present? && attrs.present? - (searchables.map(&:to_s) - attrs.map(&:to_s)).each do |missing_searchable| - MeiliSearch::Rails.logger.warn( - "[meilisearch-rails] #{name}##{missing_searchable} declared in searchable_attributes but not in attributes. Please add it to attributes if it should be searchable." - ) - end - end - end end # these are the instance methods included diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index e55c15cf..36160f98 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -20,29 +20,3 @@ end end -# This changes the index uid of the People class as well, making tests unrandomizable -# context 'when a searchable attribute is not an attribute' do -# let(:other_people_class) do -# Class.new(People) do -# def self.name -# 'People' -# end -# end -# end - -# let(:logger) { instance_double('Logger', warn: nil) } - -# before do -# allow(MeiliSearch::Rails).to receive(:logger).and_return(logger) - -# other_people_class.meilisearch index_uid: safe_index_uid('Others'), primary_key: :card_number do -# attribute :first_name -# searchable_attributes %i[first_name last_name] -# end -# end - -# it 'warns the user' do -# expect(logger).to have_received(:warn).with(/meilisearch-rails.+last_name/) -# end -# end - diff --git a/spec/settings_spec.rb b/spec/settings_spec.rb index d17b4548..fdce62ba 100644 --- a/spec/settings_spec.rb +++ b/spec/settings_spec.rb @@ -121,6 +121,26 @@ end end + describe 'searchable_attributes' do + # TODO: Add more searchable_attributes tests + context 'when a searchable attribute is not an attribute' do + let(:logger) { instance_double('Logger', warn: nil) } + + before do + allow(MeiliSearch::Rails).to receive(:logger).and_return(logger) + end + + it 'warns the user' do + People.meilisearch_settings.add_index(safe_index_uid('searchable_attr_spec')) do + attribute :first_name + searchable_attributes %i[first_name last_name] + end + + expect(logger).to have_received(:warn).with(/meilisearch-rails.+last_name/) + end + end + end + describe 'add_index' do let(:private_songs_index) { safe_index_uid('PrivateSongs') } let(:public_songs_index) { safe_index_uid('Songs') } From 121b98986793762d7896177bcb442e3d514b672b Mon Sep 17 00:00:00 2001 From: ellnix Date: Mon, 27 May 2024 19:42:04 +0200 Subject: [PATCH 30/36] Fix regression in #ms_entries specs The specs should now be more resilient by using regular expressions instead of index names. --- spec/instance_methods_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/instance_methods_spec.rb b/spec/instance_methods_spec.rb index 040aa356..13490ab1 100644 --- a/spec/instance_methods_spec.rb +++ b/spec/instance_methods_spec.rb @@ -13,9 +13,9 @@ ) expect(book.ms_entries).to contain_exactly( - a_hash_including("index_uid" => safe_index_uid('SecuredBook')), - a_hash_including("index_uid" => safe_index_uid('BookAuthor')), - a_hash_including("index_uid" => safe_index_uid('Book')), + a_hash_including("index_uid" => /SecuredBook/), + a_hash_including("index_uid" => /BookAuthor/), + a_hash_including("index_uid" => /Book/), ) end @@ -27,10 +27,10 @@ ) expect(book.ms_entries).to contain_exactly( - a_hash_including("index_uid" => safe_index_uid('SecuredBook')), - a_hash_including("index_uid" => safe_index_uid('BookAuthor')), + a_hash_including("index_uid" => /SecuredBook/), + a_hash_including("index_uid" => /BookAuthor/), # also includes book's id as if it was a public book - a_hash_including("index_uid" => safe_index_uid('Book')), + a_hash_including("index_uid" => /Book/), ) end From 84d355b1eb51613bb74e287819b3bde9073c00f9 Mon Sep 17 00:00:00 2001 From: ellnix Date: Mon, 27 May 2024 19:43:26 +0200 Subject: [PATCH 31/36] Merge test refactoring into main New test has been added while PR #350 was being finished. --- .github/workflows/tests.yml | 5 ++- .rubocop_todo.yml | 33 ++++------------- Gemfile | 2 +- README.md | 3 +- docker-compose.yml | 2 - lib/meilisearch-rails.rb | 3 +- lib/meilisearch/rails/multi_search/result.rb | 4 +- lib/meilisearch/rails/version.rb | 2 +- spec/integration_spec.rb | 39 ++++++++++++++++++++ spec/ms_clean_up_job_spec.rb | 2 +- spec/spec_helper.rb | 7 +++- 11 files changed, 63 insertions(+), 39 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 6482a0c4..6ef19a96 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -36,8 +36,9 @@ jobs: run: bundle exec rspec - name: Upload coverage reports to Codecov uses: codecov/codecov-action@v4 - env: - CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + if: matrix.ruby-version == '3.1' && matrix.rails-version == '7.0' + with: + token: ${{ secrets.CODECOV_TOKEN }} linter_check: name: linter-check diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 8560fb10..e3a5568c 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2024-01-10 10:49:28 UTC using RuboCop version 1.27.0. +# on 2024-04-08 13:44:25 UTC using RuboCop version 1.27.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 @@ -67,7 +67,7 @@ Lint/UnusedMethodArgument: Exclude: - 'lib/meilisearch-rails.rb' -# Offense count: 11 +# Offense count: 12 # Configuration parameters: IgnoredMethods, CountRepeatedAttributes. Metrics/AbcSize: Max: 104 @@ -81,14 +81,14 @@ Metrics/BlockLength: # Offense count: 1 # Configuration parameters: CountComments, CountAsOne. Metrics/ClassLength: - Max: 156 + Max: 157 # Offense count: 8 # Configuration parameters: IgnoredMethods. Metrics/CyclomaticComplexity: Max: 27 -# Offense count: 16 +# Offense count: 18 # Configuration parameters: CountComments, CountAsOne, ExcludedMethods, IgnoredMethods. Metrics/MethodLength: Max: 103 @@ -128,13 +128,7 @@ RSpec/BeforeAfterAll: Exclude: - 'spec/integration_spec.rb' -# Offense count: 7 -# Configuration parameters: IgnoredMetadata. -RSpec/DescribeClass: - Exclude: - - 'spec/integration_spec.rb' - -# Offense count: 46 +# Offense count: 56 # Configuration parameters: CountAsOne. RSpec/ExampleLength: Max: 19 @@ -154,24 +148,11 @@ RSpec/InstanceVariable: Exclude: - 'spec/integration_spec.rb' -# Offense count: 1 -# Configuration parameters: EnforcedStyle. -# SupportedStyles: have_received, receive -RSpec/MessageSpies: - Exclude: - - 'spec/integration_spec.rb' - # Offense count: 1 RSpec/MultipleDescribes: Exclude: - 'spec/integration_spec.rb' -# Offense count: 1 -# This cop supports safe auto-correction (--auto-correct). -RSpec/MultipleSubjects: - Exclude: - - 'spec/ms_clean_up_job_spec.rb' - # Offense count: 1 # Configuration parameters: IgnoreNameless, IgnoreSymbolicNames. RSpec/VerifiedDoubles: @@ -231,7 +212,7 @@ Style/OptionalBooleanParameter: Exclude: - 'lib/meilisearch-rails.rb' -# Offense count: 13 +# Offense count: 11 # This cop supports safe auto-correction (--auto-correct). # Configuration parameters: EnforcedStyle, ConsistentQuotesInMultiline. # SupportedStyles: single_quotes, double_quotes @@ -248,7 +229,7 @@ Style/TrailingCommaInArguments: Exclude: - 'spec/integration_spec.rb' -# Offense count: 19 +# Offense count: 20 # This cop supports safe auto-correction (--auto-correct). # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. # URISchemes: http, https diff --git a/Gemfile b/Gemfile index e674d48f..8f4e5c69 100644 --- a/Gemfile +++ b/Gemfile @@ -31,7 +31,7 @@ group :test do gem 'jdbc-sqlite3', platform: :jruby gem 'rspec', '~> 3.0' gem 'simplecov', require: 'false' - gem 'codecov', require: 'false' + gem 'simplecov-cobertura', require: 'false' gem 'threads' gem 'byebug' diff --git a/README.md b/README.md index ddd410dd..f0a38b67 100644 --- a/README.md +++ b/README.md @@ -207,6 +207,7 @@ class Book < ApplicationRecord crop_length 10 faceting max_values_per_facet: 2000 pagination max_total_hits: 1000 + proximity_precision 'byWord' end end ``` @@ -230,7 +231,7 @@ harry_book.formatted # => {"id"=>"1", "name"=>"Harry Potter", "descript 👉 Don't forget that `attributes_to_highlight`, `attributes_to_crop`, and `crop_length` can be set up in the `meilisearch` block of your model. -## 🔍 Sorted search +### 🔍 Sorted search As an example of how to use the sort option, here is how you could achieve returning all books sorted by title in ascending order: diff --git a/docker-compose.yml b/docker-compose.yml index db71719f..78c36f2a 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,5 +1,3 @@ -version: "3.7" - volumes: bundle: play_bundle: diff --git a/lib/meilisearch-rails.rb b/lib/meilisearch-rails.rb index 7099e19e..b2577a67 100644 --- a/lib/meilisearch-rails.rb +++ b/lib/meilisearch-rails.rb @@ -67,6 +67,7 @@ class IndexSettings pagination faceting typo_tolerance + proximity_precision ].freeze CAMELIZE_OPTIONS = %i[pagination faceting typo_tolerance].freeze @@ -588,7 +589,7 @@ def ms_entries_for(document:, synchronous:) ms_configurations.filter_map do |options, settings| { synchronous: synchronous || options[:synchronous], - index_uid: options[:index_uid], + index_uid: ms_index_uid(options), primary_key: primary_key }.with_indifferent_access unless ms_indexing_disabled?(options) end diff --git a/lib/meilisearch/rails/multi_search/result.rb b/lib/meilisearch/rails/multi_search/result.rb index 593856ab..e7938fcc 100644 --- a/lib/meilisearch/rails/multi_search/result.rb +++ b/lib/meilisearch/rails/multi_search/result.rb @@ -30,8 +30,8 @@ def each_hit(&block) end alias each each_hit - def each_result - @results.each + def each_result(&block) + @results.each(&block) end def to_a diff --git a/lib/meilisearch/rails/version.rb b/lib/meilisearch/rails/version.rb index 946ea846..17fe1460 100644 --- a/lib/meilisearch/rails/version.rb +++ b/lib/meilisearch/rails/version.rb @@ -2,7 +2,7 @@ module MeiliSearch module Rails - VERSION = '0.12.0' + VERSION = '0.13.0' def self.qualified_version "Meilisearch Rails (v#{VERSION})" diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index 36160f98..3f65cc19 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -20,3 +20,42 @@ end end +describe 'proximity_precision' do + before do + stub_const( + 'OtherColor', + Class.new do + include ActiveModel::Model + include MeiliSearch::Rails + end + ) + end + + context 'when the value is byWord' do + before do + OtherColor.meilisearch synchronize: true, index_uid: safe_index_uid('OtherColors') do + proximity_precision 'byWord' + end + end + + it 'sets the value byWord to proximity precision' do + AsyncHelper.await_last_task + expect(OtherColor.index.get_settings['proximityPrecision']).to eq('byWord') + end + end + + context 'when the value is byAttribute' do + before do + OtherColor.meilisearch synchronize: true, index_uid: safe_index_uid('OtherColors') do + proximity_precision 'byAttribute' + end + end + + it 'sets the value byAttribute to proximity precision' do + OtherColor.index.get_settings # induce update_settings + AsyncHelper.await_last_task + expect(OtherColor.index.get_settings['proximityPrecision']).to eq('byAttribute') + end + end +end + diff --git a/spec/ms_clean_up_job_spec.rb b/spec/ms_clean_up_job_spec.rb index ccbf687c..52292018 100644 --- a/spec/ms_clean_up_job_spec.rb +++ b/spec/ms_clean_up_job_spec.rb @@ -23,7 +23,7 @@ def create_indexed_record end let(:record_entries) do - record.ms_entries(true).each { |h| h[:index_uid] += '_test' } + record.ms_entries(true) end let(:indexes) do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 661ca86c..4d1ce533 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -4,13 +4,16 @@ unless ENV.fetch('DISABLE_COVERAGE', false) require 'simplecov' - require 'codecov' SimpleCov.start do add_filter %r{^/spec/} minimum_coverage 86.70 - formatter SimpleCov::Formatter::Codecov if ENV['CI'] + if ENV['CI'] + require 'simplecov-cobertura' + + formatter SimpleCov::Formatter::CoberturaFormatter + end end end From eefe6be68168ee155413926336d9847e868a30ed Mon Sep 17 00:00:00 2001 From: ellnix Date: Mon, 27 May 2024 20:00:41 +0200 Subject: [PATCH 32/36] Refactor proximity_precision test --- spec/integration_spec.rb | 39 ------------------------------------ spec/settings_spec.rb | 13 ++++++++++++ spec/support/models/color.rb | 1 + spec/support/models/song.rb | 2 ++ 4 files changed, 16 insertions(+), 39 deletions(-) diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index 3f65cc19..36160f98 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -20,42 +20,3 @@ end end -describe 'proximity_precision' do - before do - stub_const( - 'OtherColor', - Class.new do - include ActiveModel::Model - include MeiliSearch::Rails - end - ) - end - - context 'when the value is byWord' do - before do - OtherColor.meilisearch synchronize: true, index_uid: safe_index_uid('OtherColors') do - proximity_precision 'byWord' - end - end - - it 'sets the value byWord to proximity precision' do - AsyncHelper.await_last_task - expect(OtherColor.index.get_settings['proximityPrecision']).to eq('byWord') - end - end - - context 'when the value is byAttribute' do - before do - OtherColor.meilisearch synchronize: true, index_uid: safe_index_uid('OtherColors') do - proximity_precision 'byAttribute' - end - end - - it 'sets the value byAttribute to proximity precision' do - OtherColor.index.get_settings # induce update_settings - AsyncHelper.await_last_task - expect(OtherColor.index.get_settings['proximityPrecision']).to eq('byAttribute') - end - end -end - diff --git a/spec/settings_spec.rb b/spec/settings_spec.rb index fdce62ba..ab3b1992 100644 --- a/spec/settings_spec.rb +++ b/spec/settings_spec.rb @@ -4,6 +4,7 @@ require 'support/models/restaurant' require 'support/models/specialty_models' require 'support/models/song' +require 'support/models/color' describe MeiliSearch::Rails::IndexSettings do describe 'attribute' do @@ -113,6 +114,18 @@ end end + describe 'proximity_precision' do + it 'can be set to byWord' do + expect(Color.index.get_settings['proximityPrecision']).to eq('byWord') + end + + it 'can be set to byAttribute' do + Song.create!(name: 'Coconut nut', artist: 'Smokey Mountain', premium: false, released: true) + AsyncHelper.await_last_task + expect(Song.index.get_settings['proximityPrecision']).to eq('byAttribute') + end + end + describe 'force_utf8_encoding' do it 'converts to utf8' do EncodedString.create! diff --git a/spec/support/models/color.rb b/spec/support/models/color.rb index c4118ec0..c242fed7 100644 --- a/spec/support/models/color.rb +++ b/spec/support/models/color.rb @@ -25,6 +25,7 @@ class Color < ActiveRecord::Base ] attributes_to_highlight [:name] faceting max_values_per_facet: 20 + proximity_precision 'byWord' end def will_save_change_to_hex? diff --git a/spec/support/models/song.rb b/spec/support/models/song.rb index be164576..81b1c3c2 100644 --- a/spec/support/models/song.rb +++ b/spec/support/models/song.rb @@ -19,6 +19,8 @@ class Song < ActiveRecord::Base add_index PUBLIC_INDEX_UID, if: :public? do searchable_attributes %i[name artist] end + + proximity_precision 'byAttribute' end private From 79fe56a070af51c5a3f76dc0b2a7cc4ce196b336 Mon Sep 17 00:00:00 2001 From: ellnix Date: Mon, 27 May 2024 20:13:52 +0200 Subject: [PATCH 33/36] Remove integration spec file & private method test - Specs are all moved to their own files! - The remaining tests were unit tests on the `meilisearch_settings_changed?` method which should be tested but not this way. Postponing until the logic has been refactored out of the MeiliSearch::Rails module. - Do not require every ActiveRecord table in every test - Some require statements that were missing were added in this commit --- spec/integration_spec.rb | 22 ---------------------- spec/options_spec.rb | 2 ++ spec/settings_spec.rb | 1 + spec/support/active_record_classes.rb | 3 --- 4 files changed, 3 insertions(+), 25 deletions(-) delete mode 100644 spec/integration_spec.rb delete mode 100644 spec/support/active_record_classes.rb diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb deleted file mode 100644 index 36160f98..00000000 --- a/spec/integration_spec.rb +++ /dev/null @@ -1,22 +0,0 @@ -require 'spec_helper' - -describe 'Settings change detection' do - it 'detects settings changes' do - expect(Color.send(:meilisearch_settings_changed?, nil, {})).to be(true) - expect(Color.send(:meilisearch_settings_changed?, {}, { 'searchable_attributes' => ['name'] })).to be(true) - expect(Color.send(:meilisearch_settings_changed?, { 'searchable_attributes' => ['name'] }, - { 'searchable_attributes' => %w[name hex] })).to be(true) - expect(Color.send(:meilisearch_settings_changed?, { 'searchable_attributes' => ['name'] }, - { 'ranking_rules' => ['words', 'typo', 'proximity', 'attribute', 'sort', 'exactness', 'hex:asc'] })).to be(true) - end - - it 'does not detect settings changes' do - expect(Color.send(:meilisearch_settings_changed?, {}, {})).to be(false) - expect(Color.send(:meilisearch_settings_changed?, { 'searchableAttributes' => ['name'] }, - { searchable_attributes: ['name'] })).to be(false) - expect(Color.send(:meilisearch_settings_changed?, - { 'searchableAttributes' => ['name'], 'rankingRules' => ['words', 'typo', 'proximity', 'attribute', 'sort', 'exactness', 'hex:asc'] }, - { 'ranking_rules' => ['words', 'typo', 'proximity', 'attribute', 'sort', 'exactness', 'hex:asc'] })).to be(false) - end -end - diff --git a/spec/options_spec.rb b/spec/options_spec.rb index 9f01d874..8ad2a123 100644 --- a/spec/options_spec.rb +++ b/spec/options_spec.rb @@ -3,6 +3,8 @@ require 'support/models/book' require 'support/models/animals' require 'support/models/people' +require 'support/models/vegetable' +require 'support/models/fruit' require 'support/models/disabled_models' require 'support/models/queued_models' diff --git a/spec/settings_spec.rb b/spec/settings_spec.rb index ab3b1992..0e7a3dfd 100644 --- a/spec/settings_spec.rb +++ b/spec/settings_spec.rb @@ -159,6 +159,7 @@ let(:public_songs_index) { safe_index_uid('Songs') } it 'targets multiple indexes' do + Song.clear_index!(true) songs = [ Song.create!(name: 'Coconut nut', artist: 'Smokey Mountain', premium: false, released: true), diff --git a/spec/support/active_record_classes.rb b/spec/support/active_record_classes.rb deleted file mode 100644 index 6ebce285..00000000 --- a/spec/support/active_record_classes.rb +++ /dev/null @@ -1,3 +0,0 @@ -require 'support/active_record_schema' -Dir["#{File.dirname(__FILE__)}/models/*.rb"].sort.each { |file| require file } - From 300b7785b784a572418ac0f633cfabaf581e3880 Mon Sep 17 00:00:00 2001 From: ellnix Date: Mon, 27 May 2024 20:16:23 +0200 Subject: [PATCH 34/36] Hide ActiveRecord's create_table messages This should mean less noise in the test, and if we follow this example going foward when adding Sequel and Mongoid models the test output should be relevant only to the tests being run. --- spec/support/active_record_schema.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/support/active_record_schema.rb b/spec/support/active_record_schema.rb index 8673a122..832e0212 100644 --- a/spec/support/active_record_schema.rb +++ b/spec/support/active_record_schema.rb @@ -26,3 +26,5 @@ def ar_schema @ar_schema ||= ActiveRecord::Schema.new end + +ar_schema.verbose = false From f1b4ec3962265151ea372be12af11700f776ad9c Mon Sep 17 00:00:00 2001 From: ellnix Date: Mon, 27 May 2024 20:24:23 +0200 Subject: [PATCH 35/36] Make rubocop happy --- .rubocop_todo.yml | 88 ++++++++----------- spec/instance_methods_spec.rb | 18 ++-- .../attributes_have_changed_spec.rb | 1 - ...ms_rails_is_included_without_block_spec.rb | 1 - .../record_has_associations_spec.rb | 1 - spec/meilisearch/activation_spec.rb | 22 ++--- spec/options_spec.rb | 4 +- 7 files changed, 61 insertions(+), 74 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index e3a5568c..e0943d3a 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,27 +1,11 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2024-04-08 13:44:25 UTC using RuboCop version 1.27.0. +# on 2024-05-27 18:22:30 UTC using RuboCop version 1.27.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 # versions of RuboCop, may require this file to be generated again. -# Offense count: 1 -# This cop supports safe auto-correction (--auto-correct). -# Configuration parameters: TreatCommentsAsGroupSeparators, ConsiderPunctuation, Include. -# Include: **/*.gemfile, **/Gemfile, **/gems.rb -Bundler/OrderedGems: - Exclude: - - 'Gemfile' - -# Offense count: 1 -# This cop supports safe auto-correction (--auto-correct). -# Configuration parameters: Include. -# Include: **/*.gemspec -Gemspec/RequireMFA: - Exclude: - - 'meilisearch-rails.gemspec' - # Offense count: 1 # This cop supports safe auto-correction (--auto-correct). # Configuration parameters: EnforcedStyle. @@ -30,13 +14,12 @@ Layout/EmptyLinesAroundModuleBody: Exclude: - 'lib/meilisearch-rails.rb' -# Offense count: 3 +# Offense count: 1 # This cop supports safe auto-correction (--auto-correct). # Configuration parameters: EnforcedStyle. # SupportedStyles: symmetrical, new_line, same_line Layout/MultilineMethodCallBraceLayout: Exclude: - - 'spec/integration_spec.rb' - 'spec/ms_clean_up_job_spec.rb' # Offense count: 1 @@ -70,7 +53,7 @@ Lint/UnusedMethodArgument: # Offense count: 12 # Configuration parameters: IgnoredMethods, CountRepeatedAttributes. Metrics/AbcSize: - Max: 104 + Max: 103 # Offense count: 1 # Configuration parameters: CountComments, CountAsOne, ExcludedMethods, IgnoredMethods. @@ -81,7 +64,7 @@ Metrics/BlockLength: # Offense count: 1 # Configuration parameters: CountComments, CountAsOne. Metrics/ClassLength: - Max: 157 + Max: 169 # Offense count: 8 # Configuration parameters: IgnoredMethods. @@ -91,12 +74,12 @@ Metrics/CyclomaticComplexity: # Offense count: 18 # Configuration parameters: CountComments, CountAsOne, ExcludedMethods, IgnoredMethods. Metrics/MethodLength: - Max: 103 + Max: 102 # Offense count: 1 # Configuration parameters: CountComments, CountAsOne. Metrics/ModuleLength: - Max: 449 + Max: 437 # Offense count: 8 # Configuration parameters: IgnoredMethods. @@ -123,35 +106,54 @@ Naming/MethodParameterName: Exclude: - 'lib/meilisearch-rails.rb' -# Offense count: 20 +# Offense count: 5 RSpec/BeforeAfterAll: Exclude: - - 'spec/integration_spec.rb' + - 'spec/integration/active_record/record_has_associations_spec.rb' + - 'spec/pagination/kaminari_spec.rb' + - 'spec/pagination/will_paginate_spec.rb' + - 'spec/settings_spec.rb' + - 'spec/system/tech_shop_spec.rb' + +# Offense count: 2 +# Configuration parameters: Prefixes. +# Prefixes: when, with, without +RSpec/ContextWording: + Exclude: + - 'spec/options_spec.rb' + - 'spec/system/tech_shop_spec.rb' -# Offense count: 56 +# Offense count: 54 # Configuration parameters: CountAsOne. RSpec/ExampleLength: - Max: 19 + Max: 16 -# Offense count: 3 +# Offense count: 6 # Configuration parameters: Include, CustomTransform, IgnoreMethods, SpecSuffixOnly. # Include: **/*_spec*rb*, **/spec/**/* RSpec/FilePath: Exclude: - 'spec/configuration_spec.rb' + - 'spec/integration/active_record/meilisearch_calls_are_deactivated.rb' + - 'spec/meilisearch/activation_spec.rb' + - 'spec/safe_index_spec.rb' - 'spec/settings_spec.rb' - 'spec/utilities_spec.rb' -# Offense count: 25 +# Offense count: 29 # Configuration parameters: AssignmentOnly. RSpec/InstanceVariable: Exclude: - - 'spec/integration_spec.rb' + - 'spec/system/tech_shop_spec.rb' # Offense count: 1 RSpec/MultipleDescribes: Exclude: - - 'spec/integration_spec.rb' + - 'spec/search_spec.rb' + +# Offense count: 2 +RSpec/NestedGroups: + Max: 4 # Offense count: 1 # Configuration parameters: IgnoreNameless, IgnoreSymbolicNames. @@ -159,11 +161,6 @@ RSpec/VerifiedDoubles: Exclude: - 'spec/configuration_spec.rb' -# Offense count: 1 -Security/MarshalLoad: - Exclude: - - 'spec/integration_spec.rb' - # Offense count: 2 # This cop supports safe auto-correction (--auto-correct). # Configuration parameters: EnforcedStyle. @@ -172,13 +169,13 @@ Style/Alias: Exclude: - 'lib/meilisearch-rails.rb' -# Offense count: 2 +# Offense count: 3 # Configuration parameters: MinBodyLength. Style/GuardClause: Exclude: - 'lib/meilisearch-rails.rb' -# Offense count: 9 +# Offense count: 8 # This cop supports safe auto-correction (--auto-correct). Style/IfUnlessModifier: Exclude: @@ -212,26 +209,17 @@ Style/OptionalBooleanParameter: Exclude: - 'lib/meilisearch-rails.rb' -# Offense count: 11 +# Offense count: 5 # This cop supports safe auto-correction (--auto-correct). # Configuration parameters: EnforcedStyle, ConsistentQuotesInMultiline. # SupportedStyles: single_quotes, double_quotes Style/StringLiterals: Exclude: - - 'spec/integration_spec.rb' - 'spec/ms_clean_up_job_spec.rb' -# Offense count: 2 -# This cop supports safe auto-correction (--auto-correct). -# Configuration parameters: EnforcedStyleForMultiline. -# SupportedStylesForMultiline: comma, consistent_comma, no_comma -Style/TrailingCommaInArguments: - Exclude: - - 'spec/integration_spec.rb' - -# Offense count: 20 +# Offense count: 16 # This cop supports safe auto-correction (--auto-correct). # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. # URISchemes: http, https Layout/LineLength: - Max: 178 + Max: 170 diff --git a/spec/instance_methods_spec.rb b/spec/instance_methods_spec.rb index 13490ab1..b24c93e7 100644 --- a/spec/instance_methods_spec.rb +++ b/spec/instance_methods_spec.rb @@ -13,9 +13,9 @@ ) expect(book.ms_entries).to contain_exactly( - a_hash_including("index_uid" => /SecuredBook/), - a_hash_including("index_uid" => /BookAuthor/), - a_hash_including("index_uid" => /Book/), + a_hash_including('index_uid' => /SecuredBook/), + a_hash_including('index_uid' => /BookAuthor/), + a_hash_including('index_uid' => /Book/) ) end @@ -27,10 +27,10 @@ ) expect(book.ms_entries).to contain_exactly( - a_hash_including("index_uid" => /SecuredBook/), - a_hash_including("index_uid" => /BookAuthor/), + a_hash_including('index_uid' => /SecuredBook/), + a_hash_including('index_uid' => /BookAuthor/), # also includes book's id as if it was a public book - a_hash_including("index_uid" => /Book/), + a_hash_including('index_uid' => /Book/) ) end @@ -42,10 +42,12 @@ taby_cat = Cat.create!(name: 'Taby the Cat') expect(toby_dog.ms_entries).to contain_exactly( - a_hash_including('primary_key' => /dog_\d+/)) + a_hash_including('primary_key' => /dog_\d+/) + ) expect(taby_cat.ms_entries).to contain_exactly( - a_hash_including('primary_key' => /cat_\d+/)) + a_hash_including('primary_key' => /cat_\d+/) + ) end end end diff --git a/spec/integration/active_record/attributes_have_changed_spec.rb b/spec/integration/active_record/attributes_have_changed_spec.rb index aa11f93d..f1be37b6 100644 --- a/spec/integration/active_record/attributes_have_changed_spec.rb +++ b/spec/integration/active_record/attributes_have_changed_spec.rb @@ -65,4 +65,3 @@ expect(results[0].id).to eq(m.id) end end - diff --git a/spec/integration/active_record/ms_rails_is_included_without_block_spec.rb b/spec/integration/active_record/ms_rails_is_included_without_block_spec.rb index cc0de7eb..691f998b 100644 --- a/spec/integration/active_record/ms_rails_is_included_without_block_spec.rb +++ b/spec/integration/active_record/ms_rails_is_included_without_block_spec.rb @@ -7,4 +7,3 @@ end.to raise_error(ArgumentError) end end - diff --git a/spec/integration/active_record/record_has_associations_spec.rb b/spec/integration/active_record/record_has_associations_spec.rb index b9294cae..3da2fdf6 100644 --- a/spec/integration/active_record/record_has_associations_spec.rb +++ b/spec/integration/active_record/record_has_associations_spec.rb @@ -21,4 +21,3 @@ end end end - diff --git a/spec/meilisearch/activation_spec.rb b/spec/meilisearch/activation_spec.rb index edceac35..e81a08a0 100644 --- a/spec/meilisearch/activation_spec.rb +++ b/spec/meilisearch/activation_spec.rb @@ -1,39 +1,39 @@ describe MeiliSearch::Rails do it 'is active by default' do - expect(MeiliSearch::Rails).to be_active + expect(described_class).to be_active end describe '#deactivate!' do context 'without block' do - before { MeiliSearch::Rails.deactivate! } + before { described_class.deactivate! } - after { MeiliSearch::Rails.activate! } + after { described_class.activate! } it 'deactivates the requests until activate!-ed' do - expect(MeiliSearch::Rails).not_to be_active + expect(described_class).not_to be_active end it 'responds with a black hole' do - expect(MeiliSearch::Rails.client.foo.bar.now.nil.item.issue).to be_nil + expect(described_class.client.foo.bar.now.nil.item.issue).to be_nil end end context 'with a block' do it 'disables only around call' do - MeiliSearch::Rails.deactivate! do - expect(MeiliSearch::Rails).not_to be_active + described_class.deactivate! do + expect(described_class).not_to be_active end - expect(MeiliSearch::Rails).to be_active + expect(described_class).to be_active end it 'works in multi-threaded environments' do Threads.new(5, log: $stdout).assert(20) do |_i, _r| - MeiliSearch::Rails.deactivate! do - expect(MeiliSearch::Rails).not_to be_active + described_class.deactivate! do + expect(described_class).not_to be_active end - expect(MeiliSearch::Rails).to be_active + expect(described_class).to be_active end end end diff --git a/spec/options_spec.rb b/spec/options_spec.rb index 8ad2a123..41fbc744 100644 --- a/spec/options_spec.rb +++ b/spec/options_spec.rb @@ -216,7 +216,7 @@ expect(b['hits'][0]).to include( 'name' => '"> hack0r', - 'author' => '', + 'author' => '' ) end @@ -229,7 +229,7 @@ b = Book.raw_search('hack', { attributes_to_highlight: ['*'] }) expect(b['hits'][0]['_formatted']).to include( - 'name' => '"> hack0r', + 'name' => '"> hack0r' ) end end From cf8a17fa2041c08552ad7a40882e05e47d49d356 Mon Sep 17 00:00:00 2001 From: ellnix Date: Mon, 27 May 2024 20:32:40 +0200 Subject: [PATCH 36/36] Fix will_paginate test race condition --- spec/pagination/will_paginate_spec.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spec/pagination/will_paginate_spec.rb b/spec/pagination/will_paginate_spec.rb index 8c181cdc..87045076 100644 --- a/spec/pagination/will_paginate_spec.rb +++ b/spec/pagination/will_paginate_spec.rb @@ -1,4 +1,5 @@ require 'will_paginate' +require 'support/async_helper' require 'support/models/movie' describe 'Pagination with will_paginate' do @@ -7,6 +8,8 @@ Movie.clear_index! 6.times { Movie.create(title: Faker::Movie.title) } + + AsyncHelper.await_last_task end it 'paginates with sort' do