pat / thinking-sphinx

Sphinx/Manticore plugin for ActiveRecord/Rails
http://freelancing-gods.com/thinking-sphinx
MIT License
1.63k stars 468 forks source link

Returning another model in the callback block #1176

Closed kalsan closed 4 years ago

kalsan commented 4 years ago

Hi and thanks for the update to 5.0.0! While the documentation is pretty clear about how to migrate, I'm getting interesting behaviour and I'm wondering whether it is a bug or if I'm doing something wrong.

Assume a set of Apple belongs_to a Tree. The latter has a label and each apple also indexes the name of its parent tree such that when you search for the name of a specific tree, its apples are returned as well. However, having a callback for each apple makes the tree update slow, which is why I use a block to only trigger the callback if a field such as the label has changed.

In 4.x, I achieved this with the following code, which worked fine and updated all needed apple_core indices when a tree was updated:

[models/tree.rb]
after_save ThinkingSphinx::RealTime.callback_for(:apple) { |model| (model.saved_changes.keys - %w[updated_at]).any? ? model.apples : [] }

[Edit]: The code above still works fine unter 5.0.0, but it's outdated.

For 5.x, I tried the following:

ThinkingSphinx::Callbacks.append(self, behaviours: [:real_time]) { |model| (model.saved_changes.keys - %w[updated_at]).any? ? model.apples : [] }

However, with this, Thinking Sphinx attempts to insert an Apple into the tree_core database, resulting in an exception.

Next attempt:

ThinkingSphinx::Callbacks.append(Apple, behaviours: [:real_time]) { |model| (model.saved_changes.keys - %w[updated_at]).any? ? model.apples : [] }

However using this method, the callback would not get triggered.

What would be the new way to achieve this?

Cheers and thanks, Kalsan

pat commented 4 years ago

Thanks for reporting this Kalsan… it seems the docs need some work, and the suggested code wasn't quite right when dealing with associations. But I think I've got a way forward:

There's an optional second argument (before the options hash) in ThinkingSphinx::Callbacks.append which is for the reference of the index that should be processed - references are the first argument when you define an index. So, in this case it could be :tree or :apple. It defaults to the underscored, lowercased name of the class passed in as the first argument - but if you want to invoke the processing of an associated index, then you'll need to specify it.

So, from the perspective of your Tree model where you only want to fire the indexing of apples in this specific case, it could look like the following:

ThinkingSphinx::Callbacks.append(
  self, :apple, behaviours: [:real_time]
) do |model|
  (model.saved_changes.keys - %w[updated_at]).any? ? model.apples : []
end

The :path option is only useful for uncustomised behaviour - i.e. if you're not providing a block. If there's a block, the :path option is ignored. You weren't using it, so that's fine, I'm more making this note for anyone else who comes across this issue :)

I'm working on some doc updates now to make all of this more clear!

kalsan commented 4 years ago

Hi pat

Thanks for your very complete answer! This helps indeed.

Best Kalsan