From 5cb61e0a077721b04cab7fbf19ff5c356bf4bbe0 Mon Sep 17 00:00:00 2001 From: Tomokazu Kiyohara Date: Thu, 31 Mar 2022 11:26:13 +0000 Subject: [PATCH 1/4] fix: make sync opt to be selectable for auto setting update logic --- lib/algoliasearch-rails.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/algoliasearch-rails.rb b/lib/algoliasearch-rails.rb index 94f6bce..6acf7ae 100644 --- a/lib/algoliasearch-rails.rb +++ b/lib/algoliasearch-rails.rb @@ -800,7 +800,8 @@ def algolia_ensure_init(options = nil, settings = nil, index_settings = nil) replicas = index_settings.delete(:replicas) || index_settings.delete('replicas') index_settings[:replicas] = replicas unless replicas.nil? || options[:inherit] - @algolia_indexes[settings].set_settings!(index_settings) + set_settings_method = options[:synchronous] ? :set_settings! : :set_settings + @algolia_indexes[settings].send(set_settings_method, index_settings) end @algolia_indexes[settings] From a7226470cc59a10ec8e493b336cf249420d3b95a Mon Sep 17 00:00:00 2001 From: Tomokazu Kiyohara Date: Mon, 11 Apr 2022 13:08:55 +0000 Subject: [PATCH 2/4] add specs --- spec/integration_spec.rb | 82 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index 6ab0970..4a4182c 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -144,6 +144,20 @@ class DisabledIndexing < ActiveRecord::Base end end +class EnableCheckSettingsSynchronously < ActiveRecord::Base + include AlgoliaSearch + + algoliasearch :check_settings => true, :synchronous => true do + end +end + +class EnableCheckSettingsAsynchronously < ActiveRecord::Base + include AlgoliaSearch + + algoliasearch :check_settings => true, :synchronous => false do + end +end + class Product < ActiveRecord::Base include AlgoliaSearch @@ -366,6 +380,74 @@ def public? end end +describe 'EnableCheckSettingsSynchronously' do + describe 'has settings changes' do + before(:each) do + allow(EnableCheckSettingsSynchronously).to receive(:algoliasearch_settings_changed?).and_return(true) + end + + it 'should call set_setting!' do + expect_any_instance_of(Algolia::Search::Index).to receive(:set_settings!) + EnableCheckSettingsSynchronously.send(:algolia_ensure_init) + end + + it 'should not call set_setting' do + expect_any_instance_of(Algolia::Search::Index).not_to receive(:set_settings) + EnableCheckSettingsSynchronously.send(:algolia_ensure_init) + end + end + + describe 'has no settings changes' do + before(:each) do + allow(EnableCheckSettingsSynchronously).to receive(:algoliasearch_settings_changed?).and_return(false) + end + + it 'should not call set_setting!' do + expect_any_instance_of(Algolia::Search::Index).not_to receive(:set_settings!) + EnableCheckSettingsSynchronously.send(:algolia_ensure_init) + end + + it 'should not call set_setting' do + expect_any_instance_of(Algolia::Search::Index).not_to receive(:set_settings) + EnableCheckSettingsSynchronously.send(:algolia_ensure_init) + end + end +end + +describe 'EnableCheckSettingsAsynchronously' do + describe 'has settings changes' do + before(:each) do + allow(EnableCheckSettingsAsynchronously).to receive(:algoliasearch_settings_changed?).and_return(true) + end + + it 'should call set_setting' do + expect_any_instance_of(Algolia::Search::Index).to receive(:set_settings) + EnableCheckSettingsAsynchronously.send(:algolia_ensure_init) + end + + it 'should not call set_setting!' do + expect_any_instance_of(Algolia::Search::Index).not_to receive(:set_settings) + EnableCheckSettingsAsynchronously.send(:algolia_ensure_init) + end + end + + describe 'has no settings changes' do + before(:each) do + allow(EnableCheckSettingsAsynchronously).to receive(:algoliasearch_settings_changed?).and_return(false) + end + + it 'should not call set_setting!' do + expect_any_instance_of(Algolia::Search::Index).not_to receive(:set_settings!) + EnableCheckSettingsAsynchronously.send(:algolia_ensure_init) + end + + it 'should not call set_setting' do + expect_any_instance_of(Algolia::Search::Index).not_to receive(:set_settings) + EnableCheckSettingsAsynchronously.send(:algolia_ensure_init) + end + end +end + describe 'SequelBook' do before(:all) do SequelBook.clear_index!(true) From 79577ca4515bc1ab8707a6da5df23c9b503f4d66 Mon Sep 17 00:00:00 2001 From: Tomokazu Kiyohara Date: Fri, 15 Apr 2022 11:38:42 +0000 Subject: [PATCH 3/4] fix meaningless tests (add cache avoiding) --- spec/integration_spec.rb | 52 ++++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index 4a4182c..960a58b 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -144,20 +144,6 @@ class DisabledIndexing < ActiveRecord::Base end end -class EnableCheckSettingsSynchronously < ActiveRecord::Base - include AlgoliaSearch - - algoliasearch :check_settings => true, :synchronous => true do - end -end - -class EnableCheckSettingsAsynchronously < ActiveRecord::Base - include AlgoliaSearch - - algoliasearch :check_settings => true, :synchronous => false do - end -end - class Product < ActiveRecord::Base include AlgoliaSearch @@ -381,6 +367,19 @@ def public? end describe 'EnableCheckSettingsSynchronously' do + before(:each) do + # NOTE: + # Redefine below class *each* time to avoid the cache in the class. + # If the cahce is ready, algolia_ensure_init call neither set_settings nor set_settings! ever. + Object.send(:remove_const, :EnableCheckSettingsSynchronously) if Object.constants.include?(:EnableCheckSettingsSynchronously) + class EnableCheckSettingsSynchronously < ActiveRecord::Base + include AlgoliaSearch + + algoliasearch :check_settings => true, :synchronous => true do + end + end + end + describe 'has settings changes' do before(:each) do allow(EnableCheckSettingsSynchronously).to receive(:algoliasearch_settings_changed?).and_return(true) @@ -391,8 +390,8 @@ def public? EnableCheckSettingsSynchronously.send(:algolia_ensure_init) end - it 'should not call set_setting' do - expect_any_instance_of(Algolia::Search::Index).not_to receive(:set_settings) + it 'should call set_setting' do # NOTE: set_setting! call set_setting internally. + expect_any_instance_of(Algolia::Search::Index).to receive(:set_settings).and_call_original EnableCheckSettingsSynchronously.send(:algolia_ensure_init) end end @@ -415,18 +414,31 @@ def public? end describe 'EnableCheckSettingsAsynchronously' do + before(:each) do + # NOTE: + # Redefine below class *each* time to avoid the cache in the class. + # If the cahce is ready, algolia_ensure_init call neither set_settings nor set_settings! ever. + Object.send(:remove_const, :EnableCheckSettingsAsynchronously) if Object.constants.include?(:EnableCheckSettingsAsynchronously) + class EnableCheckSettingsAsynchronously < ActiveRecord::Base + include AlgoliaSearch + + algoliasearch :check_settings => true, :synchronous => false do + end + end + end + describe 'has settings changes' do before(:each) do allow(EnableCheckSettingsAsynchronously).to receive(:algoliasearch_settings_changed?).and_return(true) end - it 'should call set_setting' do - expect_any_instance_of(Algolia::Search::Index).to receive(:set_settings) + it 'should not call set_setting!' do + expect_any_instance_of(Algolia::Search::Index).not_to receive(:set_settings!) EnableCheckSettingsAsynchronously.send(:algolia_ensure_init) end - it 'should not call set_setting!' do - expect_any_instance_of(Algolia::Search::Index).not_to receive(:set_settings) + it 'should call set_setting' do + expect_any_instance_of(Algolia::Search::Index).to receive(:set_settings).and_call_original EnableCheckSettingsAsynchronously.send(:algolia_ensure_init) end end From 8ea6d0d8d4a886a5d03ac0d08bcd6910e7786b18 Mon Sep 17 00:00:00 2001 From: Tomokazu Kiyohara Date: Wed, 11 May 2022 13:01:59 +0000 Subject: [PATCH 4/4] fix test point (change from set_setting! to wait_task) --- spec/integration_spec.rb | 30 ++++++------------------------ 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index cefa0b0..cf67154 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -385,13 +385,9 @@ class EnableCheckSettingsSynchronously < ActiveRecord::Base allow(EnableCheckSettingsSynchronously).to receive(:algoliasearch_settings_changed?).and_return(true) end - it 'should call set_setting!' do - expect_any_instance_of(Algolia::Search::Index).to receive(:set_settings!) - EnableCheckSettingsSynchronously.send(:algolia_ensure_init) - end - - it 'should call set_setting' do # NOTE: set_setting! call set_setting internally. - expect_any_instance_of(Algolia::Search::Index).to receive(:set_settings).and_call_original + it 'should call set_setting with wait_task(sync)' do + expect_any_instance_of(Algolia::Search::Index).to receive(:set_settings).and_call_original # wait_task use this return val + expect_any_instance_of(Algolia::Search::Index).to receive(:wait_task) EnableCheckSettingsSynchronously.send(:algolia_ensure_init) end end @@ -401,11 +397,6 @@ class EnableCheckSettingsSynchronously < ActiveRecord::Base allow(EnableCheckSettingsSynchronously).to receive(:algoliasearch_settings_changed?).and_return(false) end - it 'should not call set_setting!' do - expect_any_instance_of(Algolia::Search::Index).not_to receive(:set_settings!) - EnableCheckSettingsSynchronously.send(:algolia_ensure_init) - end - it 'should not call set_setting' do expect_any_instance_of(Algolia::Search::Index).not_to receive(:set_settings) EnableCheckSettingsSynchronously.send(:algolia_ensure_init) @@ -432,13 +423,9 @@ class EnableCheckSettingsAsynchronously < ActiveRecord::Base allow(EnableCheckSettingsAsynchronously).to receive(:algoliasearch_settings_changed?).and_return(true) end - it 'should not call set_setting!' do - expect_any_instance_of(Algolia::Search::Index).not_to receive(:set_settings!) - EnableCheckSettingsAsynchronously.send(:algolia_ensure_init) - end - - it 'should call set_setting' do - expect_any_instance_of(Algolia::Search::Index).to receive(:set_settings).and_call_original + it 'should call set_setting without wait_task(sync)' do + expect_any_instance_of(Algolia::Search::Index).to receive(:set_settings) + expect_any_instance_of(Algolia::Search::Index).not_to receive(:wait_task) EnableCheckSettingsAsynchronously.send(:algolia_ensure_init) end end @@ -448,11 +435,6 @@ class EnableCheckSettingsAsynchronously < ActiveRecord::Base allow(EnableCheckSettingsAsynchronously).to receive(:algoliasearch_settings_changed?).and_return(false) end - it 'should not call set_setting!' do - expect_any_instance_of(Algolia::Search::Index).not_to receive(:set_settings!) - EnableCheckSettingsAsynchronously.send(:algolia_ensure_init) - end - it 'should not call set_setting' do expect_any_instance_of(Algolia::Search::Index).not_to receive(:set_settings) EnableCheckSettingsAsynchronously.send(:algolia_ensure_init)