From 1be2d802e8c873ff44e2889e97cf814fb4050f43 Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Sat, 2 Sep 2017 12:37:40 +0200 Subject: [PATCH 1/8] Fix for namespaced model real-time callbacks. If symbol is expected, let's cast it to ensure we're actually getting one. --- lib/thinking_sphinx/real_time.rb | 2 +- spec/acceptance/real_time_updates_spec.rb | 11 +++++++++++ spec/internal/app/indices/admin_person_index.rb | 4 ++++ spec/internal/app/models/admin/person.rb | 2 ++ 4 files changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/thinking_sphinx/real_time.rb b/lib/thinking_sphinx/real_time.rb index 024195d4e..6b1a94c07 100644 --- a/lib/thinking_sphinx/real_time.rb +++ b/lib/thinking_sphinx/real_time.rb @@ -4,7 +4,7 @@ module Callbacks end def self.callback_for(reference, path = [], &block) - Callbacks::RealTimeCallbacks.new reference, path, &block + Callbacks::RealTimeCallbacks.new reference.to_sym, path, &block end end diff --git a/spec/acceptance/real_time_updates_spec.rb b/spec/acceptance/real_time_updates_spec.rb index baa7adb84..24f4deb41 100644 --- a/spec/acceptance/real_time_updates_spec.rb +++ b/spec/acceptance/real_time_updates_spec.rb @@ -14,4 +14,15 @@ expect(Product.search('blue fish', :indices => ['product_core']).to_a). to eq([product]) end + + it "handles inserts and updates for namespaced models" do + person = Admin::Person.create :name => 'Death' + + expect(Admin::Person.search('Death').to_a).to eq([person]) + + person.update_attributes :name => 'Mort' + + expect(Admin::Person.search('Death').to_a).to be_empty + expect(Admin::Person.search('Mort').to_a).to eq([person]) + end end diff --git a/spec/internal/app/indices/admin_person_index.rb b/spec/internal/app/indices/admin_person_index.rb index 94901e387..ea3c19bfa 100644 --- a/spec/internal/app/indices/admin_person_index.rb +++ b/spec/internal/app/indices/admin_person_index.rb @@ -1,3 +1,7 @@ ThinkingSphinx::Index.define 'admin/person', :with => :active_record do indexes name end + +ThinkingSphinx::Index.define 'admin/person', :with => :real_time, :name => 'admin_person_rt' do + indexes name +end diff --git a/spec/internal/app/models/admin/person.rb b/spec/internal/app/models/admin/person.rb index 2c48d632e..c6bbeeb29 100644 --- a/spec/internal/app/models/admin/person.rb +++ b/spec/internal/app/models/admin/person.rb @@ -1,3 +1,5 @@ class Admin::Person < ActiveRecord::Base self.table_name = 'admin_people' + + after_save ThinkingSphinx::RealTime.callback_for('admin/person') end From ec35be9017e18172f08b5971cd51e9effbdebe08 Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Sun, 10 Sep 2017 19:09:33 +0200 Subject: [PATCH 2/8] Allow use of deletion callback for rollback events. Related to #1021. --- .../callbacks/delete_callbacks.rb | 14 ++++- .../callbacks/delete_callbacks_spec.rb | 59 +++++++++++++++++++ 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/lib/thinking_sphinx/active_record/callbacks/delete_callbacks.rb b/lib/thinking_sphinx/active_record/callbacks/delete_callbacks.rb index 88983a4ba..1dbeeeadf 100644 --- a/lib/thinking_sphinx/active_record/callbacks/delete_callbacks.rb +++ b/lib/thinking_sphinx/active_record/callbacks/delete_callbacks.rb @@ -1,9 +1,19 @@ class ThinkingSphinx::ActiveRecord::Callbacks::DeleteCallbacks < ThinkingSphinx::Callbacks - callbacks :after_destroy + callbacks :after_destroy, :after_rollback def after_destroy + delete_from_sphinx + end + + def after_rollback + delete_from_sphinx + end + + private + + def delete_from_sphinx return if ThinkingSphinx::Callbacks.suspended? || instance.new_record? indices.each { |index| @@ -11,8 +21,6 @@ def after_destroy } end - private - def indices ThinkingSphinx::Configuration.instance.index_set_class.new( :classes => [instance.class] diff --git a/spec/thinking_sphinx/active_record/callbacks/delete_callbacks_spec.rb b/spec/thinking_sphinx/active_record/callbacks/delete_callbacks_spec.rb index 066ef0846..666ac136e 100644 --- a/spec/thinking_sphinx/active_record/callbacks/delete_callbacks_spec.rb +++ b/spec/thinking_sphinx/active_record/callbacks/delete_callbacks_spec.rb @@ -64,4 +64,63 @@ ThinkingSphinx::Callbacks.resume! end end + + describe '.after_rollback' do + let(:callbacks) { double('callbacks', :after_rollback => nil) } + + before :each do + allow(ThinkingSphinx::ActiveRecord::Callbacks::DeleteCallbacks). + to receive_messages :new => callbacks + end + + it "builds an object from the instance" do + expect(ThinkingSphinx::ActiveRecord::Callbacks::DeleteCallbacks). + to receive(:new).with(instance).and_return(callbacks) + + ThinkingSphinx::ActiveRecord::Callbacks::DeleteCallbacks. + after_rollback(instance) + end + + it "invokes after_rollback on the object" do + expect(callbacks).to receive(:after_rollback) + + ThinkingSphinx::ActiveRecord::Callbacks::DeleteCallbacks. + after_rollback(instance) + end + end + + describe '#after_rollback' do + let(:index_set) { double 'index set', :to_a => [index] } + let(:index) { double('index', :name => 'foo_core', + :document_id_for_key => 14, :type => 'plain', :distributed? => false) } + let(:instance) { double('instance', :id => 7, :new_record? => false) } + + before :each do + allow(ThinkingSphinx::IndexSet).to receive_messages :new => index_set + end + + it "performs the deletion for the index and instance" do + expect(ThinkingSphinx::Deletion).to receive(:perform).with(index, 7) + + callbacks.after_rollback + end + + it "doesn't do anything if the instance is a new record" do + allow(instance).to receive_messages :new_record? => true + + expect(ThinkingSphinx::Deletion).not_to receive(:perform) + + callbacks.after_rollback + end + + it 'does nothing if callbacks are suspended' do + ThinkingSphinx::Callbacks.suspend! + + expect(ThinkingSphinx::Deletion).not_to receive(:perform) + + callbacks.after_rollback + + ThinkingSphinx::Callbacks.resume! + end + end end From 52f2253de73fbb8fd2fe8d2c6df323458d5aec78 Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Tue, 19 Sep 2017 17:00:17 +0200 Subject: [PATCH 3/8] No need to delete files in the Populator. The real-time interface does that as well, so let's keep that version. The populator can focus on what it does best: populating. --- lib/thinking_sphinx/real_time/populator.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/thinking_sphinx/real_time/populator.rb b/lib/thinking_sphinx/real_time/populator.rb index 1250fe864..d6eab2cec 100644 --- a/lib/thinking_sphinx/real_time/populator.rb +++ b/lib/thinking_sphinx/real_time/populator.rb @@ -10,8 +10,6 @@ def initialize(index) def populate(&block) instrument 'start_populating' - remove_files - scope.find_in_batches(:batch_size => batch_size) do |instances| transcriber.copy *instances instrument 'populated', :instances => instances @@ -38,10 +36,6 @@ def instrument(message, options = {}) ) end - def remove_files - Dir["#{index.path}*"].each { |file| FileUtils.rm file } - end - def transcriber @transcriber ||= ThinkingSphinx::RealTime::Transcriber.new index end From 60df63108b6a67b9740f8b277886526d8aa79738 Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Mon, 25 Sep 2017 08:23:21 +1000 Subject: [PATCH 4/8] Fix warn-level logging. I can't have my own method named warn (or debug, info, etc). Makes sense. Probably space for some clear refactoring here, but this gets things working again for now. This issue was raised in #1068. --- lib/thinking_sphinx/active_record/log_subscriber.rb | 4 ++-- lib/thinking_sphinx/middlewares/valid_options.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/thinking_sphinx/active_record/log_subscriber.rb b/lib/thinking_sphinx/active_record/log_subscriber.rb index 76af6e572..5cbad52c3 100644 --- a/lib/thinking_sphinx/active_record/log_subscriber.rb +++ b/lib/thinking_sphinx/active_record/log_subscriber.rb @@ -14,9 +14,9 @@ def query(event) debug " #{identifier} #{event.payload[:query]}" end - def warn(event) + def caution(event) identifier = color 'Sphinx', GREEN, true - warn " #{identifier} #{event.payload[:guard]}" + warn " #{identifier} #{event.payload[:caution]}" end end diff --git a/lib/thinking_sphinx/middlewares/valid_options.rb b/lib/thinking_sphinx/middlewares/valid_options.rb index d2d164a9c..7ad1a5722 100644 --- a/lib/thinking_sphinx/middlewares/valid_options.rb +++ b/lib/thinking_sphinx/middlewares/valid_options.rb @@ -13,7 +13,7 @@ def check_options(options) unknown = invalid_keys options.keys return if unknown.empty? - ThinkingSphinx::Logger.log :warn, + ThinkingSphinx::Logger.log :caution, "Unexpected search options: #{unknown.inspect}" end From 83911c686d26cdd047714dc610e6e2bf01a48ba5 Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Mon, 25 Sep 2017 12:35:36 +1000 Subject: [PATCH 5/8] Update spec for previous commit. --- spec/thinking_sphinx/middlewares/valid_options_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/thinking_sphinx/middlewares/valid_options_spec.rb b/spec/thinking_sphinx/middlewares/valid_options_spec.rb index 840f0ee35..9fc3baaa1 100644 --- a/spec/thinking_sphinx/middlewares/valid_options_spec.rb +++ b/spec/thinking_sphinx/middlewares/valid_options_spec.rb @@ -17,7 +17,7 @@ it "adds a warning" do expect(ThinkingSphinx::Logger).to receive(:log). - with(:warn, "Unexpected search options: [:foo]") + with(:caution, "Unexpected search options: [:foo]") middleware.call [context] end From f12073f7f8a5f41ec6de536361c5ee2f2297b987 Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Wed, 27 Sep 2017 00:31:51 +1000 Subject: [PATCH 6/8] Adding the missed search options. --- lib/thinking_sphinx/search.rb | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/thinking_sphinx/search.rb b/lib/thinking_sphinx/search.rb index 33160613b..bb2a53885 100644 --- a/lib/thinking_sphinx/search.rb +++ b/lib/thinking_sphinx/search.rb @@ -6,6 +6,15 @@ class ThinkingSphinx::Search < Array respond_to_missing? send should should_not type ) SAFE_METHODS = %w( partition private_methods protected_methods public_methods send class ) + KNOWN_OPTIONS = ( + [ + :classes, :conditions, :geo, :group_by, :ids_only, :ignore_scopes, + :indices, :limit, :masks, :max_matches, :middleware, :offset, :order, + :order_group_by, :page, :per_page, :populate, :retry_stale, :select, + :skip_sti, :sql, :star, :with, :with_all, :without, :without_ids + ] + + ThinkingSphinx::Middlewares::SphinxQL::SELECT_OPTIONS + ).uniq DEFAULT_MASKS = [ ThinkingSphinx::Masks::PaginationMask, ThinkingSphinx::Masks::ScopesMask, @@ -25,12 +34,7 @@ def self.valid_options @valid_options end - @valid_options = [ - :classes, :conditions, :geo, :group_by, :ids_only, :ignore_scopes, :indices, - :limit, :masks, :max_matches, :middleware, :offset, :order, :order_group_by, - :page, :per_page, :populate, :retry_stale, :select, :skip_sti, :sql, :star, - :with, :with_all, :without, :without_ids - ] + @valid_options = KNOWN_OPTIONS.dup def initialize(query = nil, options = {}) query, options = nil, query if query.is_a?(Hash) From 59d55a7579ab93990efb7b64a169444778e987a6 Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Fri, 29 Sep 2017 17:00:29 +1000 Subject: [PATCH 7/8] Update HISTORY with recent changes. --- HISTORY | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/HISTORY b/HISTORY index 803bcd0d1..4d8a8ac1c 100644 --- a/HISTORY +++ b/HISTORY @@ -1,3 +1,10 @@ +Edge: +* [FIX] Real-time callback syntax for namespaced models accepts a string (as documented). +* [CHANGE] Allow use of deletion callbacks for rollback events. +* [CHANGE] Remove extra deletion code in the Populator - it's also being done by the real-time rake interface. +* [FIX] Fix up logged warnings. +* [FIX] Add missing search options to known values to avoid incorrect warnings. + 2017-08-29: 3.4.1 * [CHANGE] Treat "Lost connection to MySQL server" as a connection error (Manuel Schnitzer). * [FIX] Index normalisation will now work even when index model tables don't exist. From 887469826d5b42cafa13bf2788b78acad9a52cd7 Mon Sep 17 00:00:00 2001 From: Pat Allan Date: Fri, 29 Sep 2017 17:03:56 +1000 Subject: [PATCH 8/8] 3.4.2 --- HISTORY | 2 +- README.textile | 5 +++-- thinking-sphinx.gemspec | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/HISTORY b/HISTORY index 4d8a8ac1c..38ca1571f 100644 --- a/HISTORY +++ b/HISTORY @@ -1,4 +1,4 @@ -Edge: +2017-09-29: 3.4.2 * [FIX] Real-time callback syntax for namespaced models accepts a string (as documented). * [CHANGE] Allow use of deletion callbacks for rollback events. * [CHANGE] Remove extra deletion code in the Populator - it's also being done by the real-time rake interface. diff --git a/README.textile b/README.textile index dbf8a3968..29c8be1e1 100644 --- a/README.textile +++ b/README.textile @@ -1,11 +1,12 @@ h1. Thinking Sphinx -Thinking Sphinx is a library for connecting ActiveRecord to the Sphinx full-text search tool, and integrates closely with Rails (but also works with other Ruby web frameworks). The current release is v3.4.1. +Thinking Sphinx is a library for connecting ActiveRecord to the Sphinx full-text search tool, and integrates closely with Rails (but also works with other Ruby web frameworks). The current release is v3.4.2. h2. Upgrading Please refer to the release notes for any changes you need to make when upgrading: +* "v3.4.2":https://github.com/pat/thinking-sphinx/releases/tag/v3.4.2 * "v3.4.1":https://github.com/pat/thinking-sphinx/releases/tag/v3.4.1 * "v3.4.0":https://github.com/pat/thinking-sphinx/releases/tag/v3.4.0 * "v3.3.0":https://github.com/pat/thinking-sphinx/releases/tag/v3.3.0 @@ -25,7 +26,7 @@ It's a gem, so install it like you would any other gem. You will also need to sp 
gem 'mysql2',          '~> 0.3', :platform => :ruby
 gem 'jdbc-mysql',      '= 5.1.35', :platform => :jruby
-gem 'thinking-sphinx', '~> 3.4.1'
+gem 'thinking-sphinx', '~> 3.4.2' The MySQL gems mentioned are required for connecting to Sphinx, so please include it even when you're using PostgreSQL for your database. If you're using JRuby, there is "currently an issue with Sphinx and jdbc-mysql 5.1.36 or newer":http://sphinxsearch.com/forum/view.html?id=13939, so you'll need to stick to nothing more recent than 5.1.35. diff --git a/thinking-sphinx.gemspec b/thinking-sphinx.gemspec index efc3908f8..1ac3419fc 100644 --- a/thinking-sphinx.gemspec +++ b/thinking-sphinx.gemspec @@ -3,7 +3,7 @@ $:.push File.expand_path('../lib', __FILE__) Gem::Specification.new do |s| s.name = 'thinking-sphinx' - s.version = '3.4.1' + s.version = '3.4.2' s.platform = Gem::Platform::RUBY s.authors = ["Pat Allan"] s.email = ["pat@freelancing-gods.com"]