search

cs3org / wopiserver

A vendor-neutral application gateway compatible with the WOPI specifications.
Apache License 2.0
55 stars 26 forks source link

Ensure UnlockAndRelock is atomic #86

Closed glpatcern closed 2 years ago

glpatcern commented 2 years ago

Following https://github.com/cs3org/wopiserver/pull/85, we need to make the unlock/relock operation atomic.

This could be achieved by stating that the refreshLock operation is allowed to alter the lock payload on top of refreshing the lock, or by introducing a new unlockAndRelock CS3 API that acts like refreshLock and in addition changes the payload of a lock.

To be discussed with @micbar.

micbar commented 2 years ago

Thanks @glpatcern for the ticket. I would follow the approach of a separate API Call because it makes the implementation cleaner and better to implement on the storage drivers.

glpatcern commented 2 years ago

I tend to agree, though I shall say that the current specs were not so thoroughly spelled out for the RefreshLock API, and a change around those lines would be needed:

   // Refreshes the lock metadata of a storage resource.
   // MUST return CODE_NOT_FOUND if the reference does not exist.
-  // MUST return CODE_FAILED_PRECONDITION if the reference is not locked
-  // or if the caller does not hold the lock.
+  // MUST return CODE_FAILED_PRECONDITION if the reference is not locked,
+  // if the caller does not hold the lock, or if the lock metadata does not match.

I still have to check how the eos driver was implemented, as with the current specs it would be legitimate for the storage driver to actually do what unlockAndRelock require, without needing an extra API.

micbar commented 2 years ago

I created a PR against the cs3apis.