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

Compatibility problems with optimistic locking / lock_version #163

Closed
saizai opened this issue Jun 21, 2012 · 7 comments
Closed

Compatibility problems with optimistic locking / lock_version #163

saizai opened this issue Jun 21, 2012 · 7 comments

Comments

@saizai
Copy link
Contributor

saizai commented Jun 21, 2012

Suppose you have a Widget with a lock_version column.

If you try to do widget.previous_version.save!, you'll get a ActiveRecord::StaleObjectError.

I'm not sure how to override this. Ideally what should happen is that the newly reified object should have the same staleness / lock properties as the original object, so that if widget is OK to save then widget.previous_version should be too (since it hasn't changed out from under you).

Any ideas for how to implement this?

@batter
Copy link
Collaborator

batter commented Oct 21, 2013

@saizai - I know this is a little late, but is there any chance you could provide a failing test to work with?

@Ganasist
Copy link

I have the same issue just now, has anyone figured this out yet? Keep throwing

ActiveRecord::StaleObjectError

whenever I try to undo something with Papertrail. Would be nice to have both functionalities on the same model.

@batter
Copy link
Collaborator

batter commented Nov 26, 2013

To quote the rails docs for ActiveRecord::StaleObjectError:

Raised on attempt to save stale record. Record is stale when it’s being saved in another query after instantiation, for example, when two users edit the same wiki page and one starts editing and saves the page before the other.

Read more about optimistic locking in ActiveRecord::Locking module RDoc.

@justinlyman
Copy link

When using optimistic locking a lock_version integer is saved in the database, if an update is attempted and the lock version is a lower number that the current lock version you will get the StaleObjectError. The problem with using optimistic locking with paper_trail is that when you go back to a previous version it tries to save with the old lock version, which causes the StaleObjectError. If you don't have paper_trail save the lock_version, then it sets it to nil when you try and reify the previous version, which gets interpreted as 0 which will cause the StaleObjectError. The only workaround I have found to to change the lock_version attribute after reifying to the current lock_version and then saving the object.

@batter
Copy link
Collaborator

batter commented Feb 5, 2014

@justinlyman - Good to know. I don't know if it makes sense to hard code that into PaperTrail since it is technically monkey hacking around the way that db locking is supposed to work, right? If you think it does make sense, feel free to make a pull request.

@wdiechmann
Copy link

using the Rails conventional lock_version, I did:

model.lock_version += 1 if model.respond_to? :lock_version

in version_concern.rb, line 125 just before returning 'model'

@jaredbeck
Copy link
Member

The only workaround I have found to to change the lock_version attribute after reifying to the current lock_version and then saving the object. -Justin

I'm not sure I'd describe that as a workaround. I haven't used optimistic locking personally, but based on the documentation it sounds like you're using it as intended; you want to update a record, so you must increment the lock_version.

I'm adding a note to the readme about this. (#550)

I did: model.lock_version += 1 if model.respond_to? :lock_version in version_concern.rb, line 125 just before returning 'model' -Walther

I think this would work in a single-threaded app, but without concurrency control would not be sufficient in a multi-threaded app.

I don't know if it makes sense to .. code that into PaperTrail .. -Ben

I agree. If PaperTrail ever adds a high-level undo function, this is something we'll have to keep in mind.

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

6 participants