Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ActiveJob::DeserializationError issue #422

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix ActiveJob::DeserializationError issue #422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3bc24da
Fix ActiveJob::DeserializationError issue
dsgh Mar 8, 2022
4875e1b
Merge branch 'master' into dsgh/fix-deindex
DevinCodes Mar 11, 2022
2c547e2
Add spec for passing in id
dsgh Mar 14, 2022
4f33cfd
Merge branch 'master' into dsgh/fix-deindex
DevinCodes Nov 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions lib/algoliasearch-rails.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,8 @@ def algoliasearch(options = {}, &block)
raise ArgumentError.new("Cannot use a enqueue if the `synchronous` option if set") if options[:synchronous]
proc = if options[:enqueue] == true
Proc.new do |record, remove|
AlgoliaJob.perform_later(record, remove ? 'algolia_remove_from_index!' : 'algolia_index!')
record_or_object_id = remove ? algolia_object_id_of(record) : record
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dsgh @DevinCodes I think this line needs to be more general. If a single model instance is included in two indexes, and has a unique objectID in each of those indexes, this will not remove the record from each Algolia index in which it's included.

To remove the instance from each index in which it's included, we need to specify the Algolia index options for each relevant index in this call to algolia_object_id_of. i.e. this should probably be an array of objectID values, one per relevant index

Copy link

@duhaime duhaime Feb 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For what it's worth, we have a pretty complex setup in which several models are in several indices, and many of those indices include multiple models. Some of the models also use custom objectID values in indexes (so that e.g. Message=3 and User=3 will not have colliding objectIDs in the indices in which both are present).

When a record needs to be deleted, we find each of the indexes in which the record is included, and find the name of each of those indexes. Then we send that 2D array of [[object_id, index_name], ...] to our ActiveJob, from which we can retrieve the relevant index with index = Algolia::Index.new(index_name) and then remove the object with index.delete_object(object_id). This ensures that objects with custom object ids that are included in several indices are properly removed from each when the model instance is destroyed

AlgoliaJob.perform_later(record.class.to_s, record_or_object_id, remove)
end
elsif options[:enqueue].respond_to?(:call)
options[:enqueue]
Expand Down Expand Up @@ -624,7 +625,7 @@ def algolia_index!(object, synchronous = false)

def algolia_remove_from_index!(object, synchronous = false)
return if algolia_without_auto_index_scope
object_id = algolia_object_id_of(object)
object_id = (object.is_a?(String) || object.is_a?(Numeric)) ? object : algolia_object_id_of(object)
raise ArgumentError.new("Cannot index a record with a blank objectID") if object_id.blank?
algolia_configurations.each do |options, settings|
next if algolia_indexing_disabled?(options)
Expand Down
10 changes: 8 additions & 2 deletions lib/algoliasearch/algolia_job.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,14 @@ module AlgoliaSearch
class AlgoliaJob < ::ActiveJob::Base
queue_as :algoliasearch

def perform(record, method)
record.send(method)
def perform(record_class, record_or_object_id, remove)
if remove
object_id = record_or_object_id
record_class.constantize.algolia_remove_from_index!(object_id)
else
record = record_or_object_id
record.algolia_index!
end
end
end
end
16 changes: 16 additions & 0 deletions spec/integration_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1561,3 +1561,19 @@ class ForwardToReplicasTwo < ActiveRecord::Base
}.to raise_error(ArgumentError)
end
end

describe 'algolia_remove_from_index!' do
it "allows passing in the id as well" do
# index a Product
nokia = Product.create!(:name => 'nokia', :href => "google", :tags => ['decent'])
Product.reindex(AlgoliaSearch::IndexSettings::DEFAULT_BATCH_SIZE, true)
sleep 5

# verify it was indexed
expect(Product.search('nokia').size).to eq(1)

# remove from index by passing in id rather than the record
Product.algolia_remove_from_index!(nokia.id, true)
expect(Product.search('nokia').size).to eq(0)
end
end