diff --git a/lib/algoliasearch-rails.rb b/lib/algoliasearch-rails.rb index dc3051a3..2f315c7f 100644 --- a/lib/algoliasearch-rails.rb +++ b/lib/algoliasearch-rails.rb @@ -739,19 +739,21 @@ def algolia_index_name(options = nil) end def algolia_must_reindex?(object) + # use +algolia_dirty?+ method if implemented + return object.send(:algolia_dirty?) if (object.respond_to?(:algolia_dirty?)) + # Loop over each index to see if a attribute used in records has changed algolia_configurations.each do |options, settings| next if options[:slave] || options[:replica] return true if algolia_object_id_changed?(object, options) settings.get_attribute_names(object).each do |k| - changed_method = attribute_changed_method(k) - return true if !object.respond_to?(changed_method) || object.send(changed_method) + return true if algolia_attribute_changed?(object, k) + # return true if !object.respond_to?(changed_method) || object.send(changed_method) end [options[:if], options[:unless]].each do |condition| case condition when nil when String, Symbol - changed_method = attribute_changed_method(condition) - return true if !object.respond_to?(changed_method) || object.send(changed_method) + return true if algolia_attribute_changed?(object, condition) else # if the :if, :unless condition is a anything else, # we have no idea whether we should reindex or not @@ -760,6 +762,7 @@ def algolia_must_reindex?(object) end end end + # By default, we don't reindex return false end @@ -825,8 +828,8 @@ def algolia_object_id_of(o, options = nil) end def algolia_object_id_changed?(o, options = nil) - m = attribute_changed_method(algolia_object_id_method(options)) - o.respond_to?(m) ? o.send(m) : false + changed = algolia_attribute_changed?(o, algolia_object_id_method(options)) + changed.nil? ? false : changed end def algoliasearch_settings_changed?(prev, current) @@ -920,13 +923,48 @@ def algolia_find_in_batches(batch_size, &block) end end - def attribute_changed_method(attr) - if defined?(::ActiveRecord) && ActiveRecord::VERSION::MAJOR >= 5 && ActiveRecord::VERSION::MINOR >= 1 || - (defined?(::ActiveRecord) && ActiveRecord::VERSION::MAJOR > 5) - "will_save_change_to_#{attr}?" - else - "#{attr}_changed?" + def algolia_attribute_changed?(object, attr_name) + # if one of two method is implemented, we return its result + # true/false means whether it has changed or not + # +#{attr_name}_changed?+ always defined for automatic attributes but deprecated after Rails 5.2 + # +will_save_change_to_#{attr_name}?+ should be use instead for Rails 5.2+, also defined for automatic attributes. + # If none of the method are defined, it's a dynamic attribute + + method_name = "#{attr_name}_changed?" + if object.respond_to?(method_name) + # If +#{attr_name}_changed?+ respond we want to see if the method is user defined or if it's automatically + # defined by Rails. + # If it's user-defined, we call it. + # If it's automatic we check ActiveRecord version to see if this method is deprecated + # and try to call +will_save_change_to_#{attr_name}?+ instead. + # See: https://github.com/algolia/algoliasearch-rails/pull/338 + # This feature is not compatible with Ruby 1.8 + # In this case, we always call #{attr_name}_changed? + if Object.const_defined?(:RUBY_VERSION) && RUBY_VERSION.to_f < 1.9 + return object.send(method_name) + end + unless automatic_changed_method?(object, method_name) && automatic_changed_method_deprecated? + return object.send(method_name) + end end + + if object.respond_to?("will_save_change_to_#{attr_name}?") + return object.send("will_save_change_to_#{attr_name}?") + end + + # Nil means we don't know if the attribute has changed + nil + end + + def automatic_changed_method?(object, method_name) + raise ArgumentError.new("Method #{method_name} doesn't exist on #{object.class.name}") unless object.respond_to?(method_name) + file = object.method(method_name).source_location[0] + file.end_with?("active_model/attribute_methods.rb") + end + + def automatic_changed_method_deprecated? + (defined?(::ActiveRecord) && ActiveRecord::VERSION::MAJOR >= 5 && ActiveRecord::VERSION::MINOR >= 1) || + (defined?(::ActiveRecord) && ActiveRecord::VERSION::MAJOR > 5) end end diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index 64ab27de..915d6232 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -83,6 +83,12 @@ t.boolean :premium t.boolean :released end + create_table :ebooks do |t| + t.string :name + t.string :author + t.boolean :premium + t.boolean :released + end create_table :disabled_booleans do |t| t.string :name end @@ -155,6 +161,7 @@ class Camera < Product class Color < ActiveRecord::Base include AlgoliaSearch + attr_accessor :not_indexed algoliasearch :synchronous => true, :index_name => safe_index_name("Color"), :per_environment => true do attributesToIndex [:name] @@ -166,6 +173,14 @@ class Color < ActiveRecord::Base # we're using all attributes of the Color class + the _tag "extra" attribute end + + def hex_changed? + false + end + + def will_save_change_to_short_name? + false + end end class DisabledBoolean < ActiveRecord::Base @@ -372,6 +387,22 @@ def public? end end +class Ebook < ActiveRecord::Base + include AlgoliaSearch + attr_accessor :current_time, :published_at + + algoliasearch :synchronous => true, :index_name => safe_index_name("eBooks")do + attributesToIndex [:name] + end + + def algolia_dirty? + return true if self.published_at.nil? || self.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. + self.published_at < self.current_time + end +end + class EncodedString < ActiveRecord::Base include AlgoliaSearch @@ -532,6 +563,56 @@ class SerializedObject < ActiveRecord::Base end +describe 'Change detection' do + + it "should detect attribute changes" do + color = Color.new :name => "dark-blue", :short_name => "blue" + + Color.algolia_must_reindex?(color).should == true + color.save + Color.algolia_must_reindex?(color).should == false + + color.hex = 123456 + Color.algolia_must_reindex?(color).should == false + + color.not_indexed = "strstr" + Color.algolia_must_reindex?(color).should == false + color.name = "red" + Color.algolia_must_reindex?(color).should == true + + color.delete + end + + it "should detect change with algolia_dirty? method" do + ebook = Ebook.new :name => "My life", :author => "Myself", :premium => false, :released => true + + Ebook.algolia_must_reindex?(ebook).should == true # Because it's defined in algolia_dirty? method + ebook.current_time = 10 + ebook.published_at = 8 + Ebook.algolia_must_reindex?(ebook).should == true + ebook.published_at = 12 + Ebook.algolia_must_reindex?(ebook).should == false + end + + it "should know if the _changed? method is user-defined", :skip => Object.const_defined?(:RUBY_VERSION) && RUBY_VERSION.to_f < 1.9 do + color = Color.new :name => "dark-blue", :short_name => "blue" + + expect { Color.send(:automatic_changed_method?, color, :something_that_doesnt_exist) }.to raise_error(ArgumentError) + + Color.send(:automatic_changed_method?, color, :name_changed?).should == true + Color.send(:automatic_changed_method?, color, :hex_changed?).should == false + + Color.send(:automatic_changed_method?, color, :will_save_change_to_short_name?).should == false + + if Color.send(:automatic_changed_method_deprecated?) + Color.send(:automatic_changed_method?, color, :will_save_change_to_name?).should == true + Color.send(:automatic_changed_method?, color, :will_save_change_to_hex?).should == true + end + + end + +end + describe 'Namespaced::Model' do it "should have an index name without :: hierarchy" do Namespaced::Model.index_name.should == "Namespaced_Model"