Adding support for a Lock API

[glpatcern]

As recently discussed, this is a minimal API that would initially be used by the WOPI server to manage its locks.

The idea is to implement the SHARED lock type only for now, as that's the closest to the current WOPI server implementation. At a later stage, the WRITE lock (enforced by sync/webdav clients) would be more appropriate for the WOPI server.

Fixes #6. At least strives to ;-) (and comments are welcome to have as complete an API as possible)

[labkode]

@wkloucek we need a dummy implementation (like method not supported), and then we can jump into the real implementation, would be able to provide it?

[wkloucek]

@wkloucek we need a dummy implementation (like method not supported), and then we can jump into the real implementation, would be able to provide it?

I'll have a look tomorrow

[glpatcern]

I'll have a look tomorrow

Thanks, feel free to ping me as I'm also adding something (having WOPI as a "reference" for now)

[wkloucek]

How can we specify / document some implementation details that can not be directly mapped in the protobuf defintions?

For example:

• when a file is locked, only the lock holder must be able to create a version of that file
• SetLock needs to be executed atomically to prevent the issuing of two locks for the same file (no race conditions)

@glpatcern Do we need a IsLocked method? I think there will be a lot of places where we need to check if a file is locked and I don't exactly see how one would to that with the current method set (is GetLock even allowed to be called for non-lock-holders? If yes, it could be used for checking if a lock is in place, but people could then also introspect the lock).

[wkloucek]

also, please have a look at cs3org/reva#2350

[glpatcern]

How can we specify / document some implementation details that can not be directly mapped in the protobuf defintions?

Good questions, also the lock implementation itself is to be defined - and I added some comments detailing a possible reference implementation. I don't think we can do more than commenting what SHOULD / MUST / etc. be done at this stage.

For example:

* when a file is locked, only the lock holder must be able to create a version of that file

(that is implied by the lock types definitions, but yeah maybe I'll spell it out more explicitly)

* `SetLock` needs to be executed atomically to prevent the issuing of two locks for the same file (no race conditions)

same here

@glpatcern Do we need a IsLocked method?

As you say GetLock is supposed to do that, and can be executed by anyone with read access to the underlying resource. Again will add some more comments, but my take would be:

• GetLock can be called by anyone with read access - and the lock metadata is accessible, yes, as the whole file and other arbritraryMetadata is accessible as well, so not violating any access here.
• SetLock can be called by anyone with write access to the resource.
• RefreshLock: need to have write access plus be the lock holder.
• Unlock: as RefreshLock.

Also thanks for the dummy implementations and the non-dummy wiring in the gateway ;-)

[wkloucek]

Good questions, also the lock implementation itself is to be defined - and I added some comments detailing a possible reference implementation. I don't think we can do more than commenting what SHOULD / MUST / etc. be done at this stage.

You're right, that should be the place to put it

@glpatcern Do we need a IsLocked method?

As you say GetLock is supposed to do that, and can be executed by anyone with read access to the underlying resource. Again will add some more comments, but my take would be:

• GetLock can be called by anyone with read access - and the lock metadata is accessible, yes, as the whole file and other arbritraryMetadata is accessible as well, so not violating any access here.
• SetLock can be called by anyone with write access to the resource.
• RefreshLock: need to have write access plus be the lock holder.
• Unlock: as RefreshLock.

You're right, it's all coherent

[wkloucek]

@butonic could you please have a look at this since you're the author of #6

[refs]

* `GetLock` can be called by anyone with read access - and the lock metadata is accessible, yes, as the whole file and other arbritraryMetadata is accessible as well, so not violating any access here.

* `SetLock` can be called by anyone with write access to the resource.

* `RefreshLock`: need to have write access plus be the lock `holder`.

* `Unlock`: as `RefreshLock`.

Also thanks for the dummy implementations and the non-dummy wiring in the gateway ;-)

I think these are pretty good guidelines, and a concise very concise writeup 👌

[glpatcern]

OK, now the specs are more spelled out - and I suggest a slight change in my last commit: GetLock to return NOT_FOUND when a resource is not locked (in addition to the obvious resource not found), as GetLock could be called to probe if a file is locked. The other APIs use PRECONDITION_FAILED instead.

[refs]

OK, now the specs are more spelled out - and I suggest a slight change in my last commit: GetLock to return NOT_FOUND when a resource is not locked (in addition to the obvious resource not found), as GetLock could be called to probe if a file is locked. The other APIs use PRECONDITION_FAILED instead.

Do you mean perhaps "GetLock to return NOT_FOUND when a resource is locked"?

[glpatcern]

Do you mean perhaps "GetLock to return NOT_FOUND when a resource is locked"?

No, really when it's not locked. If it's locked and the user has read access, GetLock would return the lock metadata.

[refs]

Do you mean perhaps "GetLock to return NOT_FOUND when a resource is locked"?

No, really when it's not locked. If it's locked and the user has read access, GetLock would return the lock metadata.

I see! thanks for clarifying :)