-
Notifications
You must be signed in to change notification settings - Fork 8
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
Collaborative locking #54
Merged
+5,877
−365
Merged
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
46da347
Implement separate lock type for collaborative editing
juliusknorr 451f2c5
Switch to OCP proposal
juliusknorr 4ca68cd
Stick to 7.4 for composer dependencies
juliusknorr a9e379e
Unify lock service methods to use the LockScope and propagate ETag on…
juliusknorr 62dae1e
Adjust to OCP review comments
juliusknorr ec2899a
Adapt LockScope to LockContext rename
juliusknorr 9e5f510
Add tests for etag changes
juliusknorr f1cca4d
Update file etag and propagate it with the proper path
juliusknorr d932fd0
Limit Nextcloud compatibility to 24
juliusknorr 4d70749
Extract displayname fetching
juliusknorr d0fb12c
Add separate status code if the lock is not found on unlock
juliusknorr 6612dc9
Remove unused method
juliusknorr 8b1a881
Expose token locks the same as user locks through our properties
juliusknorr d1fd060
Remove unused dependency that tries to obtain the user session too early
juliusknorr 6bd03c5
Add response data to OCS controllers
juliusknorr a271a40
Some more cleanup
juliusknorr 78e7373
Bump version for 24
juliusknorr b03835d
Fix tests
juliusknorr 363931a
Implement WebDAV lock backend
juliusknorr f61ab53
Run litmus on CI
juliusknorr 9418b4a
Make user overwrite only happen on custom locks
juliusknorr 4d3f9f2
Skip collection locking for now
juliusknorr File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,120 @@ | ||
name: Litmus | ||
|
||
on: | ||
pull_request: | ||
push: | ||
branches: | ||
- master | ||
- stable* | ||
|
||
env: | ||
APP_NAME: files_lock | ||
|
||
|
||
jobs: | ||
litmus: | ||
runs-on: ubuntu-latest | ||
strategy: | ||
fail-fast: false | ||
matrix: | ||
php-versions: ['7.4'] | ||
databases: ['mysql'] | ||
server-versions: ['master'] | ||
|
||
name: litmus ${{ matrix.php-versions }}-${{ matrix.databases }}-${{ matrix.server-versions }} | ||
|
||
services: | ||
postgres: | ||
image: postgres | ||
ports: | ||
- 4445:5432/tcp | ||
env: | ||
POSTGRES_USER: root | ||
POSTGRES_PASSWORD: rootpassword | ||
POSTGRES_DB: nextcloud | ||
options: --health-cmd pg_isready --health-interval 5s --health-timeout 2s --health-retries 5 | ||
mysql: | ||
image: mariadb:10.5 | ||
ports: | ||
- 4444:3306/tcp | ||
env: | ||
MYSQL_ROOT_PASSWORD: rootpassword | ||
options: --health-cmd="mysqladmin ping" --health-interval 5s --health-timeout 2s --health-retries 5 | ||
|
||
steps: | ||
- name: Checkout server | ||
uses: actions/checkout@v3 | ||
with: | ||
repository: nextcloud/server | ||
ref: ${{ matrix.server-versions }} | ||
|
||
- name: Checkout submodules | ||
shell: bash | ||
run: | | ||
auth_header="$(git config --local --get http.https://github.com/.extraheader)" | ||
git submodule sync --recursive | ||
git -c "http.extraheader=$auth_header" -c protocol.version=2 submodule update --init --force --recursive --depth=1 | ||
|
||
- name: Checkout app | ||
uses: actions/checkout@v3 | ||
with: | ||
path: apps/${{ env.APP_NAME }} | ||
|
||
- name: Set up php ${{ matrix.php-versions }} | ||
uses: shivammathur/setup-php@2.17.1 | ||
with: | ||
php-version: ${{ matrix.php-versions }} | ||
tools: phpunit | ||
extensions: zip, gd, mbstring, iconv, fileinfo, intl, sqlite, pdo_sqlite, mysql, pdo_mysql, pgsql, pdo_pgsql | ||
coverage: none | ||
ini-values: zend.exception_ignore_args=0 | ||
|
||
- name: Set up PHPUnit | ||
working-directory: apps/${{ env.APP_NAME }} | ||
run: composer i | ||
|
||
- name: Set up Nextcloud | ||
run: | | ||
if [ "${{ matrix.databases }}" = "mysql" ]; then | ||
export DB_PORT=4444 | ||
elif [ "${{ matrix.databases }}" = "pgsql" ]; then | ||
export DB_PORT=4445 | ||
fi | ||
mkdir data | ||
./occ maintenance:install --verbose --database=${{ matrix.databases }} --database-name=nextcloud --database-host=127.0.0.1 --database-port=$DB_PORT --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass admin | ||
./occ app:enable --force ${{ env.APP_NAME }} | ||
php -S localhost:8080 & | ||
|
||
- name: Litmus | ||
run: | | ||
mkdir -p /tmp/litmus && \ | ||
cp apps/files_lock/tests/litmus/0001-Comment-out-collection-locking-tests.patch /tmp/litmus/ && \ | ||
wget -O /tmp/litmus/litmus-0.13.tar.gz http://www.webdav.org/neon/litmus/litmus-0.13.tar.gz && \ | ||
cd /tmp/litmus && tar -xzf litmus-0.13.tar.gz | ||
|
||
cd /tmp/litmus/litmus-0.13 | ||
patch -p1 < ../0001-Comment-out-collection-locking-tests.patch | ||
./configure && make && rm -f /tmp/litmus-0.13.tar.gz | ||
|
||
cd /tmp/litmus/litmus-0.13 | ||
make URL=http://localhost:8080/remote.php/dav/files/admin CREDS="admin admin" TESTS="basic copymove props locks" check | ||
|
||
- name: Dump Nextcloud log | ||
if: failure() | ||
run: cat data/nextcloud.log | ||
|
||
- name: Upload litmus logs | ||
uses: actions/upload-artifact@v2 | ||
if: failure() | ||
with: | ||
name: Upload litmus log | ||
path: /tmp/litmus/litmus-0.13/debug.log | ||
retention-days: 5 | ||
|
||
- name: Upload nextcloud logs | ||
uses: actions/upload-artifact@v2 | ||
if: failure() | ||
with: | ||
name: Upload nextcloud log | ||
path: data/nextcloud.log | ||
retention-days: 5 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,93 @@ | ||
name: PHPUnit | ||
|
||
on: | ||
pull_request: | ||
push: | ||
branches: | ||
- master | ||
- stable* | ||
|
||
env: | ||
APP_NAME: files_lock | ||
|
||
|
||
jobs: | ||
integration: | ||
runs-on: ubuntu-latest | ||
strategy: | ||
fail-fast: false | ||
matrix: | ||
php-versions: ['7.4', '8.0', '8.1'] | ||
databases: ['sqlite', 'mysql', 'pgsql'] | ||
server-versions: ['master'] | ||
|
||
name: php${{ matrix.php-versions }}-${{ matrix.databases }}-${{ matrix.server-versions }} | ||
|
||
services: | ||
postgres: | ||
image: postgres | ||
ports: | ||
- 4445:5432/tcp | ||
env: | ||
POSTGRES_USER: root | ||
POSTGRES_PASSWORD: rootpassword | ||
POSTGRES_DB: nextcloud | ||
options: --health-cmd pg_isready --health-interval 5s --health-timeout 2s --health-retries 5 | ||
mysql: | ||
image: mariadb:10.5 | ||
ports: | ||
- 4444:3306/tcp | ||
env: | ||
MYSQL_ROOT_PASSWORD: rootpassword | ||
options: --health-cmd="mysqladmin ping" --health-interval 5s --health-timeout 2s --health-retries 5 | ||
|
||
steps: | ||
- name: Checkout server | ||
uses: actions/checkout@v3 | ||
with: | ||
repository: nextcloud/server | ||
ref: ${{ matrix.server-versions }} | ||
|
||
- name: Checkout submodules | ||
shell: bash | ||
run: | | ||
auth_header="$(git config --local --get http.https://github.com/.extraheader)" | ||
git submodule sync --recursive | ||
git -c "http.extraheader=$auth_header" -c protocol.version=2 submodule update --init --force --recursive --depth=1 | ||
|
||
- name: Checkout app | ||
uses: actions/checkout@v3 | ||
with: | ||
path: apps/${{ env.APP_NAME }} | ||
|
||
- name: Set up php ${{ matrix.php-versions }} | ||
uses: shivammathur/setup-php@2.17.1 | ||
with: | ||
php-version: ${{ matrix.php-versions }} | ||
tools: phpunit | ||
extensions: zip, gd, mbstring, iconv, fileinfo, intl, sqlite, pdo_sqlite, mysql, pdo_mysql, pgsql, pdo_pgsql | ||
coverage: none | ||
|
||
- name: Set up PHPUnit | ||
working-directory: apps/${{ env.APP_NAME }} | ||
run: composer i | ||
|
||
- name: Set up Nextcloud | ||
run: | | ||
if [ "${{ matrix.databases }}" = "mysql" ]; then | ||
export DB_PORT=4444 | ||
elif [ "${{ matrix.databases }}" = "pgsql" ]; then | ||
export DB_PORT=4445 | ||
fi | ||
mkdir data | ||
./occ maintenance:install --verbose --database=${{ matrix.databases }} --database-name=nextcloud --database-host=127.0.0.1 --database-port=$DB_PORT --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass admin | ||
./occ app:enable --force ${{ env.APP_NAME }} | ||
php -S localhost:8080 & | ||
|
||
- name: PHPUnit | ||
working-directory: ./apps/${{ env.APP_NAME }} | ||
run: ./vendor/phpunit/phpunit/phpunit -c tests/phpunit.xml | ||
|
||
- name: Dump Nextcloud log | ||
if: failure() | ||
run: cat data/nextcloud.log |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,3 +2,6 @@ | |
\.idea/ | ||
|
||
vendor/ | ||
|
||
.php-cs-fixer.cache | ||
.phpunit.result.cache |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
require_once './vendor/autoload.php'; | ||
|
||
use Nextcloud\CodingStandard\Config; | ||
|
||
$config = new Config(); | ||
$config | ||
->getFinder() | ||
// ->ignoreVCSIgnored(true) | ||
->notPath('build') | ||
->notPath('l10n') | ||
->notPath('src') | ||
->notPath('node_modules') | ||
->notPath('vendor') | ||
->in(__DIR__); | ||
return $config; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,11 +27,19 @@ Administrators can also lock files using the `./occ` command: | |
|
||
## API | ||
|
||
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. | ||
|
||
### Capability | ||
|
||
If locking is available the app will expose itself through the capabilties endpoint under the files key: | ||
``` | ||
curl http://admin:admin@nextcloud.local/ocs/v1.php/cloud/capabilities\?format\=json -H 'OCS-APIRequest: true' \ | ||
curl http://admin:admin@nextcloud.local/ocs/v1.php/cloud/capabilities\?format\=json \ | ||
-H 'OCS-APIRequest: true' \ | ||
| jq .ocs.data.capabilities.files | ||
{ | ||
... | ||
|
@@ -40,16 +48,106 @@ curl http://admin:admin@nextcloud.local/ocs/v1.php/cloud/capabilities\?format\=j | |
} | ||
``` | ||
|
||
### Fetching lock details | ||
### WebDAV: Fetching lock details | ||
|
||
WebDAV returns the following additional properties if requests through a `PROPFIND`: | ||
|
||
- `{http://nextcloud.org/ns}lock`: `true` if the file is locked, otherwise `false` | ||
- `{http://nextcloud.org/ns}lock-owner-type`: User id of the lock owner | ||
- `0` represents a manual lock by a user | ||
- `1` represents a collaboratively locked file, e.g. when being edited through Office or Text | ||
- `2` represents a WebDAV lock identified by a lock token, which will have the other properties set as if it was type 0 | ||
- `{http://nextcloud.org/ns}lock-owner`: User id of the lock owner | ||
- `{http://nextcloud.org/ns}lock-owner-displayname`: Display name of the lock owner | ||
- `{http://nextcloud.org/ns}lock-owner-editor`: App id of an app owned lock to allow clients to suggest joining the collaborative editing session through the web or direct editing | ||
- `{http://nextcloud.org/ns}lock-time`: Timestamp of the log creation time | ||
- `{http://nextcloud.org/ns}lock-timeout`: TTL of the lock in seconds staring from the creation time | ||
- `{http://nextcloud.org/ns}lock-token`: Unique lock token (to be preserved on the client side while holding the lock to sent once full webdav locking is implemented) | ||
|
||
```bash | ||
curl -X PROPFIND \ | ||
--url http://admin:admin@nextcloud.dev.local/remote.php/dav/files/admin/myfile.odt \ | ||
--data '<D:propfind xmlns:D="DAV:" xmlns:NC="http://nextcloud.org/ns"> | ||
<D:prop> | ||
<NC:lock /> | ||
<NC:lock-owner-type /> | ||
<NC:lock-owner /> | ||
<NC:lock-owner-displayname /> | ||
<NC:lock-owner-editor /> | ||
<NC:lock-time /> | ||
</D:prop> | ||
</D:propfind>' | ||
``` | ||
|
||
|
||
### WebDAV: Manually lock a file | ||
|
||
``` | ||
curl -X LOCK \ | ||
--url http://admin:admin@nextcloud.dev.local/remote.php/dav/files/admin/myfile.odt \ | ||
--header 'X-User-Lock: 1' | ||
``` | ||
|
||
#### Response | ||
|
||
The response will give back the updated properties after obtaining the lock with a `200 Success` status code or the existing lock details in case of a `423 Locked` status. | ||
|
||
```xml | ||
<?xml version="1.0"?> | ||
<d:prop | ||
xmlns:d="DAV:" | ||
xmlns:s="http://sabredav.org/ns" | ||
xmlns:oc="http://owncloud.org/ns" | ||
xmlns:nc="http://nextcloud.org/ns"> | ||
<nc:lock>1</nc:lock> | ||
<nc:lock-owner-type>0</nc:lock-owner-type> | ||
<nc:lock-owner>user1</nc:lock-owner> | ||
<nc:lock-owner-displayname>user1</nc:lock-owner-displayname> | ||
<nc:lock-owner-editor>user1</nc:lock-owner-editor> | ||
<nc:lock-time>1648046707</nc:lock-time> | ||
</d:prop> | ||
``` | ||
|
||
#### Error status codes | ||
|
||
- 423 Unable to unlock, if the lock is owned by another user | ||
|
||
|
||
|
||
|
||
Comment on lines
+115
to
+117
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason for those empty lines? 😁 |
||
### WebDAV: Manually unlock a file | ||
|
||
``` | ||
curl -X UNLOCK \ | ||
--url http://admin:admin@nextcloud.dev.local/remote.php/dav/files/admin/myfile.odt \ | ||
--header 'X-User-Lock: 1' | ||
``` | ||
|
||
#### Response | ||
|
||
```xml | ||
<?xml version="1.0"?> | ||
<d:prop | ||
xmlns:d="DAV:" | ||
xmlns:s="http://sabredav.org/ns" | ||
xmlns:oc="http://owncloud.org/ns" | ||
xmlns:nc="http://nextcloud.org/ns"> | ||
<nc:lock></nc:lock> | ||
<nc:lock-owner-type/> | ||
<nc:lock-owner/> | ||
<nc:lock-owner-displayname/> | ||
<nc:lock-owner-editor/> | ||
<nc:lock-time/> | ||
</d:prop> | ||
|
||
``` | ||
|
||
#### Error status codes | ||
|
||
- 412 Unable to unlock because the file is not locked | ||
- 423 Unable to unlock, if the lock is owned by another user | ||
|
||
### Locking a file | ||
### OCS: Locking a file | ||
|
||
`PUT /apps/files_lock/lock/{fileId}` | ||
|
||
|
@@ -87,7 +185,7 @@ curl -X PUT 'http://admin:admin@nextcloud.local/ocs/v2.php/apps/files_lock/lock/ | |
``` | ||
|
||
|
||
### Unlocking a file | ||
### OCS: Unlocking a file | ||
|
||
`DELETE /apps/files_lock/lock/{fileId}` | ||
|
||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should explain that this specifies the lock type. It's pretty self explanatory but still.