ankane/searchkick

should_index? is not checked on delete callback

michelson opened this issue · 3 comments

I've been debugging Searchkick 5.2.3 and I've noticed that the client hits the service when records are deleted. I've configured this with callbacks: :async and I have should_reindex returning false, but it seems that this is not checked when records are deleted. Is this by design?

To reproduce
Use this code to reproduce when possible:

require "bundler/inline"

gemfile do
  source "https://rubygems.org"

  gem "activerecord", require: "active_record"
  gem "activejob", require: "active_job"
  gem "sqlite3"
  gem "searchkick", git: "https://github.com/ankane/searchkick.git"
  # uncomment one
  # gem "elasticsearch"
  gem "opensearch-ruby"
end

puts "Searchkick version: #{Searchkick::VERSION}"
# puts "Server version: #{Searchkick.server_version}"

ActiveRecord::Base.establish_connection adapter: "sqlite3", database: ":memory:"
ActiveJob::Base.queue_adapter = :inline

ActiveRecord::Schema.define do
  create_table :products do |t|
    t.string :name
  end
end

class Product < ActiveRecord::Base
  searchkick  callbacks: :async
  def should_index?
    false
  end
end


# Searchkick.disable_callbacks

Product.create!(name: "Test")
Product.last.delete

if this is by design is there any alternative to disable the callbacks on delete?

Hi @michelson, this is by design (should_index? may have previously returned true, so the index needs updated). You can customize the logic with a manual callback:

class Product < ApplicationRecord
  searchkick callbacks: false

  after_commit do
    reindex(mode: :async) if !destroyed?
  end
end

great, thanks!

Hi @ankane, it seems that callbacks: :async also hit the service for record creation when the should_index? is false.
Not sure if that is also by design. Does it make sense that should_index? is not considered for those callbacks? If so, I can do some help by documenting it.

BTW I'm currently using your proposed solution and it works fine.

Example:

require "bundler/inline"

gemfile do
  source "https://rubygems.org"

  gem "activerecord", require: "active_record"
  gem "activejob", require: "active_job"
  gem "sqlite3"
  gem "searchkick", git: "https://github.com/ankane/searchkick.git"
  # uncomment one
  # gem "elasticsearch"
  gem "opensearch-ruby"
end

puts "Searchkick version: #{Searchkick::VERSION}"
# puts "Server version: #{Searchkick.server_version}"

ActiveRecord::Base.establish_connection adapter: "sqlite3", database: ":memory:"
ActiveJob::Base.queue_adapter = :inline

ActiveRecord::Schema.define do
  create_table :products do |t|
    t.string :name
  end
end

class Product < ActiveRecord::Base
  searchkick  callbacks: :async
  def should_index?
    false
  end
end


# Searchkick.disable_callbacks

a = Product.create!(name: "Test") # will hit the service here
# a.update(name: "e")
# Product.last.delete