Only run callback on changed and on update

[kalsan]

Hi Pat

Maybe it would be a good idea to only run real-time callbacks when an actual update is written to the DB. I see three different cases:

• after_save callback is executed and saved_changes? returns true -> execute TS callback
• after_save callback is executed and saved_changes? returns false -> do nothing
• after_touch callback is executed: execute TS callback

This behavior is equivalent to the way Rails appears handle the DB. This method should greatly speedup empty updates.

What do think, should this be integrated in thinking-sphinx?

Cheers! Kalsan

[kalsan]

PS - this only applies to direct callbacks and not to those for another depending model (i.e. when specifying path in the callback is necessary).

[pat]

Hi @kalsan - I think I'm mostly on board with these changes, if you wanted to create a PR?

I guess my only concern is that I kinda expect callbacks to always be invoked whether or not the database changes. But that said, if there's no changes to the DB, there's not much point sending updates to Sphinx (and if the Sphinx data is somehow out-of-date, then that's what rake ts:index is for).

[kalsan]

Hi @pat

While I managed to make it work in most of the parts in my application, I would not dare to generalize it to the library. I spent a while reflecting on this and there are many, many edge cases. My knowledge of possible applications of TS is highly insufficient to even approach the possibility space.

However I can provide an example in the case where a callback is called for the model itself:

ThinkingSphinx::Callbacks.append(self, behaviours: [:real_time]) { |model| model.saved_changes? ? [model] : [] }

The tricky part is where the save of a model triggers callback for other classes. Stuff like: ThinkingSphinx::Callbacks.append(self, :tree, behaviours: [:real_time], path: [:trees]) in apple.rb In such cases, we need to know whether the apple needs to know when the tree has changed. Only then can it be implemented in a generic way.

Therefore unfortunately I cannot provide a PR.

Best Kalsan

[pat]

No worries - I appreciate you looking into it! And upon reflection, I'm not too surprised that it gets caught in edge cases and app-specific challenges.

Should someone else come across this issue and think that they can figure a generic approach out, they're welcome to chime in here with questions, or offer a PR - but for now I'm happy to accept the edge-cases make it too fiddly, and so I'm closing the issue.