Using Fedora's If-Unmodified-Since header to prevent clobbering another client's writes #556
Conversation
…er client's writes
escowles
force-pushed
the
locking-so-nice-we-lock-it-twice
branch
from
f368b9d
to
e1417ed
Compare
|
@HackMasterA are you still putting one token into your forms in Figgy? If this PR goes through it might be better to put them all into the form, since Fedora would have 2. @escowles This PR looks good to me. I think this approach with the separate lock makes the most sense given the time boundaries. |
There was a problem hiding this comment.
I like this approach much better.
I don't think we need to pass optimistic_locking_enabled? down to the orm converter anymore, because once you've got the optimistic_lock_token property populated it will be discarded when a resource is created if that field is not defined on the resource. This is how the casting works in other orm converters so I think it should work that way here. Can you try it without checking that value? It would make the code more consistent and reduce new lines of code here.
| return unless lastmod | ||
|
|
||
| token = Valkyrie::Persistence::OptimisticLockToken.new(adapter_id: "native-#{adapter.id}", token: DateTime.parse(lastmod.to_s).httpdate) | ||
| resource.optimistic_lock_token << token |
There was a problem hiding this comment.
Access the property via its constant, Valkyrie::Persistence::Attributes::OPTIMISTIC_LOCK
|
@HackMasterA I'll see what I can do about not passing the optimistic_locking_enabled down, and using the OPTIMISTIC_LOCK constant (I couldn't get that to work with |
|
@escowles yeah I'm not sure about making it work with |
|
@HackMasterA Ready for re-review — both of those were easy to do! |
|
yay! |
Adding a second lock token for Fedora to hold the system-managed fedora:lastModified value, for use with the
If-Unmodified-Sinceheader.Fixes #522
Alternative to #531