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

Add public API for owner based file locking #31676

Merged
merged 2 commits into from
Apr 8, 2022
Merged

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Mar 23, 2022

This adds a lightweight public interface for handling manual file locking within the files_lock app and to allow other apps like Text, Collabora or ONLYOFFICE to integrate with it.

Required for nextcloud/files_lock#54

Locks are separated into three different types:

  • 0 User owned manual lock:
    This lock type is initiated by a user manually through the WebUI or Clients and will limit editing capabilities on the file to the lock owning user.
  • 1 App owned lock:
    This lock type is created by collaborative apps like Text or Office to avoid outside changes through WevDAV or other apps.
  • 2 Token owned lock: (not implemented yet) This lock type will bind the ownership to the provided lock token. Any request that aims to modify the file will be required to sent the token, the user itself is not able to write to files without the token. This will allow to limit the locking to an individual client.

The OCP parts are mainly for exposing the ability to lock/unlock for apps and to give the files_lock app a way to register and then be triggered by the apps while the actual locking implementation is kept in the LockProvider and DAV Plugin from files_lock app.

Registering through IBootstrap was considered, but decided against as this is only limited to the one app.

App integrations

@juliusknorr juliusknorr added this to the Nextcloud 24 milestone Mar 24, 2022
@juliusknorr juliusknorr added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Mar 24, 2022
@juliusknorr juliusknorr requested review from julien-nc, a team, nickvergessen, ArtificialOwl and vanpertsch and removed request for a team March 24, 2022 09:42
@juliusknorr juliusknorr marked this pull request as ready for review March 24, 2022 09:42
@juliusknorr juliusknorr mentioned this pull request Mar 24, 2022
8 tasks
@juliusknorr juliusknorr self-assigned this Mar 24, 2022
lib/private/Files/Lock/LockManager.php Show resolved Hide resolved
lib/public/Files/Lock/LockScope.php Outdated Show resolved Hide resolved
lib/public/Files/Lock/LockScope.php Outdated Show resolved Hide resolved
lib/public/Files/Lock/ILockProvider.php Show resolved Hide resolved
lib/public/DirectEditing/IManager.php Outdated Show resolved Hide resolved
lib/private/Server.php Outdated Show resolved Hide resolved
lib/public/Files/Lock/ILockProvider.php Outdated Show resolved Hide resolved
lib/private/Files/Lock/LockManager.php Outdated Show resolved Hide resolved
lib/public/Files/Lock/ILockManager.php Show resolved Hide resolved
lib/public/Files/Lock/ILock.php Outdated Show resolved Hide resolved
Copy link
Member

@julien-nc julien-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

lib/public/Files/Lock/LockScope.php Outdated Show resolved Hide resolved
@skjnldsv skjnldsv mentioned this pull request Mar 24, 2022
@blizzz blizzz mentioned this pull request Mar 31, 2022
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr
Copy link
Member Author

🏓 for another review round, would be great to get this in for the next beta

Copy link
Member

@julien-nc julien-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understood the concept of the LockScope class after reading its implementation but I don't feel it matches the name (maybe it's only me). What do you think about renaming it to LockInfo, LockData or LockContext?

Other than that, nicely done!

@juliusknorr
Copy link
Member Author

Right, LockContext seems far better fitting indeed, will rename that.

@blizzz blizzz mentioned this pull request Apr 7, 2022
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@PVince81 PVince81 added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Apr 8, 2022
@PVince81
Copy link
Member

PVince81 commented Apr 8, 2022

failed tests unrelated, merging

@PVince81 PVince81 merged commit 0d7d28e into master Apr 8, 2022
@PVince81 PVince81 deleted the enh/ocp-owner-lock branch April 8, 2022 15:43
@blizzz blizzz mentioned this pull request Apr 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement feature: locking high
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants