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.

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

Paranoia does not preserve optimistic locking behaviour #242

Open
elyzion opened this issue Jun 14, 2015 · 5 comments
Open

Paranoia does not preserve optimistic locking behaviour #242

elyzion opened this issue Jun 14, 2015 · 5 comments

Comments

@elyzion
Copy link

elyzion commented Jun 14, 2015

Summary

Paranoia makes use of the touch method to update the deleted_at column. This breaks existing destroy behaviour as described in the API documentation at http://api.rubyonrails.org/classes/ActiveRecord/Locking/Optimistic.html.

Detail

The web based service I am working on uses optimistic locking to prevent a user from overwriting a record or deleting a record after is had been updating by another user. This worked fine while not using acts_as_paranoid. Enabling acts_as_paranoid resulted in the lock_version being ignored on destroy calls. I took a look at the source and it turned out the paranoia uses the touch method to update the deleted_at column.

It seems like changing the paranoia implementation to make use of an update call, and not a touch call, would be the best way to go about resolving this issue. Any suggestions?

@radar
Copy link
Collaborator

radar commented Jun 17, 2015

Would this be fixed by the changes in #245? /cc @Empact.

@Empact
Copy link
Contributor

Empact commented Jun 17, 2015

Don't think #245 would help - optimistic locking is implemented via _update_record, which is run on save but not touch or update_columns.

@Empact
Copy link
Contributor

Empact commented Jun 17, 2015

Ok I might see a fix - overriding destroy_row rather than destroy itself. Will take a look at implementing it as well.

@elyzion
Copy link
Author

elyzion commented Jun 17, 2015

Thank you for taking the time to look into this @Empact !

@Empact
Copy link
Contributor

Empact commented Jun 24, 2015

I don't think there's an easy fix due to order of inclusion - I would recommend paranoia make use of "destroy_row" and then re-include other extension modules over itself - any module that extends a method defined in ActiveRecord::Persistence e.g. the counter cached associations code could just be re-included via ActiveRecord::CounterCache, once we've re-defined destroy_row.

There's some risk in this so I can't guarantee it would be successful, but it does mean paranoia has to do less reimplementation and can support more of the existing extensions in other libraries and in ActiveRecord itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants