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

Already on GitHub? Sign in to your account

Delta index runs even if object was not persisted #19

Closed
enrico opened this Issue Jan 29, 2012 · 4 comments

Comments

Projects
None yet
2 participants
Contributor

enrico commented Jan 29, 2012

I just exposed some APIs to my app, and discovered this problem. When I invoke my API to create a new record, i.e.

business.customers.create(:first_name => 'foo', ...)

and there is a validation error (i.e. email is already taken). I get an exception on my log:

Started POST "/api/customers" for 127.0.0.1 at 2012-01-29 14:10:49 -0600
  User Load (0.7ms)  SELECT `users`.* FROM `users` WHERE `users`.`authentication_token` = 'vZSDZumq7doM4tgxoTdSHHXR3m1Pqs8C' AND `users`.`current_state` IS NULL LIMIT 1
  Business Load (0.6ms)  SELECT `businesses`.* FROM `businesses` WHERE `businesses`.`id` = 1 LIMIT 1
   (0.1ms)  BEGIN
   (0.3ms)  SELECT 1 FROM `customers` WHERE (LOWER(`customers`.`email`) = LOWER('foo@bar.com') AND `customers`.`business_id` = 1) LIMIT 1
   (0.1ms)  COMMIT
     (0.4ms)  SELECT COUNT(*) FROM `delayed_jobs` WHERE `delayed_jobs`.`handler` = '--- !ruby/object:ThinkingSphinx::Deltas::DeltaJob \nindices: \n- customer_delta\n' AND `delayed_jobs`.`locked_at` IS NULL
You have a nil object when you didn't expect it!
You might have expected an instance of Array.
The error occurred while evaluating nil.*

I believe this is due to the fact that delayed delta checks whether the delta column has been toggled(which in my case was true), but not whether the object has been persisted.

I wanted to provide a patch, but I'm not sure where to add the check: I found 2 places that look like they need it:

delta.rb: line 45

delayed_delta.rb: line 63-65

am I wrong? any feedback?

Owner

pat commented Jan 31, 2012

Hi Enrico

Not sure if you're right - it's certainly possible, but it would mean a bug elsewhere. The delta indexing is fired by the after_commit callback on the model - so it means that this callback is being fired even when validations fail, I guess? Which versions of Thinking Sphinx and Rails are you using?

Contributor

enrico commented Jan 31, 2012

I'm using rails 3.1.3.
I guess you're right, not really a bug here, since your code relies on an after-commit hook...
I tried to create an invalid user, first invoking User.create(), then by using the parent: business.users.create().

Even if the user doesn't get created in either case, there's a commit (not a rollback) being performed when I invoke create from the parent model, which I guess is triggering ts-delayed-delta to do its thing...

Look at my log differences when I create a user normally and 'thru the 'parent' business:

ruby-1.9.3-p0 :012 > u=User.create(:email=>'foo@bar', :first_name=>'foo', :last_name => 'bar')
   (0.2ms)  BEGIN
   (0.4ms)  SELECT 1 FROM `users` WHERE `users`.`email` = BINARY 'foo@bar' LIMIT 1
   (0.1ms)  ROLLBACK
 => #<User id: nil, business_id: nil, email: "foo@bar", encrypted_password: "", first_name: "foo", last_name: "bar", phone: nil, current_state: "signup", abilities_mask: 0, failed_attempts: 0, unlock_token: nil, locked_at: nil, confirmation_token: nil, confirmed_at: nil, confirmation_sent_at: nil, unconfirmed_email: nil, reset_password_token: nil, reset_password_sent_at: nil, sign_in_count: 0, current_sign_in_at: nil, last_sign_in_at: nil, current_sign_in_ip: nil, last_sign_in_ip: nil, authentication_token: nil, google_auth_secret: nil, created_at: nil, updated_at: nil, deleted_at: nil> 
ruby-1.9.3-p0 :013 > u.errors.messages
 => {:email=>["is invalid"]} 
ruby-1.9.3-p0 :014 >
ruby-1.9.3-p0 :014 >
ruby-1.9.3-p0 :014 > u=Business.first.users.create(:email=>'foo@bar', :first_name=>'foo', :last_name => 'bar')
  Business Load (0.6ms)  SELECT `businesses`.* FROM `businesses` LIMIT 1
   (0.1ms)  BEGIN
   (0.2ms)  SELECT 1 FROM `users` WHERE `users`.`email` = BINARY 'foo@bar' LIMIT 1
   (0.1ms)  COMMIT
 => #<User id: nil, business_id: 1, email: "foo@bar", encrypted_password: "", first_name: "foo", last_name: "bar", phone: nil, current_state: "signup", abilities_mask: 0, failed_attempts: 0, unlock_token: nil, locked_at: nil, confirmation_token: nil, confirmed_at: nil, confirmation_sent_at: nil, unconfirmed_email: nil, reset_password_token: nil, reset_password_sent_at: nil, sign_in_count: 0, current_sign_in_at: nil, last_sign_in_at: nil, current_sign_in_ip: nil, last_sign_in_ip: nil, authentication_token: nil, google_auth_secret: nil, created_at: nil, updated_at: nil, deleted_at: nil> 
ruby-1.9.3-p0 :015 > u.errors.messages
 => {:email=>["is invalid"]} 
ruby-1.9.3-p0 :016 > 
Owner

pat commented Feb 1, 2012

Perhaps this is a bug in Rails? after_commit should only apply to the object in question, surely... although, not seeing the delta get fired in the output you've just supplied.

Contributor

enrico commented Feb 1, 2012

Pat,
the reason you're not seeing the delayed delta fire is because delayed delta runs on the customers (and not the users) object off of business. I purposely tried to run this on a non-sphinx-indexed model to see if it was related to ts-delayed-delta or not...Looks like a rails issue to me.... I'll retry with 3.2 and see what I get...

@pat pat closed this May 28, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment