pat / thinking-sphinx

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

Updating associated models with SQL-backed models and delta indexes #1182

Closed JasonBarnabe closed 3 years ago

JasonBarnabe commented 3 years ago

A Script has a many-to-many relationship with a User. The User's name is part of the Script index.

I'm upgrading to thinking-sphinx 5.0.0. I've added:

ThinkingSphinx::Callbacks.append(self, behaviours: [:sql, :deltas])

to Script; works good, updates to Script trigger a delta job.

I'm having trouble making updates to User trigger deltas jobs for the associated Scripts. I've tried

ThinkingSphinx::Callbacks.append(self, :script, :behaviours => [:sql, :deltas]) {|instance| instance.scripts}

and

ThinkingSphinx::Callbacks.append(self, :script, :behaviours => [:sql, :deltas], :path => [:scripts])

in the User model - doesn't seem to do anything. How do I get this to work?

pat commented 3 years ago

I've pushed a commit to develop that will allow the second form you've listed - using :path - to work. You're welcome to give it a spin, but hopefully - as noted in #1183 - I'll have 5.1.0 released soon, which will include this :)

JasonBarnabe commented 3 years ago

@pat Thanks for the fix. It works good on an update, but when I delete a User, I get Can't modify frozen hash:

/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activemodel-6.0.3.4/lib/active_model/attribute_set/builder.rb:44:in `[]='
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activemodel-6.0.3.4/lib/active_model/attribute_set.rb:49:in `write_from_user'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activerecord-6.0.3.4/lib/active_record/attribute_methods/write.rb:43:in `_write_attribute'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activerecord-6.0.3.4/lib/active_record/attribute_methods/write.rb:21:in `delta='
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activemodel-6.0.3.4/lib/active_model/attribute_assignment.rb:50:in `public_send'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activemodel-6.0.3.4/lib/active_model/attribute_assignment.rb:50:in `_assign_attribute'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activemodel-6.0.3.4/lib/active_model/attribute_assignment.rb:43:in `block in _assign_attributes'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activemodel-6.0.3.4/lib/active_model/attribute_assignment.rb:42:in `each'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activemodel-6.0.3.4/lib/active_model/attribute_assignment.rb:42:in `_assign_attributes'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activerecord-6.0.3.4/lib/active_record/attribute_assignment.rb:21:in `_assign_attributes'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activemodel-6.0.3.4/lib/active_model/attribute_assignment.rb:35:in `assign_attributes'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activerecord-6.0.3.4/lib/active_record/persistence.rb:620:in `block in update'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activerecord-6.0.3.4/lib/active_record/transactions.rb:375:in `block in with_transaction_returning_status'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/abstract/database_statements.rb:280:in `block in transaction'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/abstract/transaction.rb:280:in `block in within_new_transaction'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activesupport-6.0.3.4/lib/active_support/concurrency/load_interlock_aware_monitor.rb:26:in `block (2 levels) in synchronize'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activesupport-6.0.3.4/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `handle_interrupt'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activesupport-6.0.3.4/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `block in synchronize'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activesupport-6.0.3.4/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `handle_interrupt'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activesupport-6.0.3.4/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `synchronize'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/abstract/transaction.rb:278:in `within_new_transaction'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/abstract/database_statements.rb:280:in `transaction'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activerecord-6.0.3.4/lib/active_record/transactions.rb:212:in `transaction'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activerecord-6.0.3.4/lib/active_record/transactions.rb:366:in `with_transaction_returning_status'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/gems/activerecord-6.0.3.4/lib/active_record/persistence.rb:619:in `update'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/bundler/gems/thinking-sphinx-9c5f5db4d994/lib/thinking_sphinx/active_record/callbacks/association_delta_callbacks.rb:9:in `block in after_commit'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/bundler/gems/thinking-sphinx-9c5f5db4d994/lib/thinking_sphinx/active_record/callbacks/association_delta_callbacks.rb:9:in `each'
/home/jason/.rbenv/versions/2.6.6/lib/ruby/gems/2.6.0/bundler/gems/thinking-sphinx-9c5f5db4d994/lib/thinking_sphinx/active_record/callbacks/association_delta_callbacks.rb:9:in `after_commit'
JasonBarnabe commented 3 years ago

Also... not sure if I'm doing it wrong, but if a new User is added to the Script, it doesn't seem to be indexing it.

pat commented 3 years ago

So I ended up getting the Greasy Spoon code working on my machine to test out these issues myself :)

The deletion issue was because when a user is deleted, the associated scripts are deleted as well - and so, the script instances are frozen. I've just pushed a commit that handles this - it won't fire off a delta update for the script, because it won't exist, but the callbacks in Script should cover off marking the sphinx record as deleted.

As for updates: given it's actually the Author model that ties the two together, I'd recommend adding the TS association callbacks to Author as well:

ThinkingSphinx::Callbacks.append(self, :script, :behaviours => [:sql, :deltas], :path => [:script])
JasonBarnabe commented 3 years ago

Delete issue is fixed, thanks.

It's not actually a new User that's the issue; it's a new ScriptVersion. I lied to keep things simple. You can check out https://github.com/JasonBarnabe/greasyfork/pull/817 for what I have now. rails test test/system/scripts/additional_info_test.rb:42 fails.

JasonBarnabe commented 3 years ago

Nevermind, the test was wrong, not thinking-sphinx. Thanks for the prompt fixes!

pat commented 3 years ago

No worries, thanks for the testing :)