Test file locking

[nickvergessen]

• Depends on: Testing app fake locks: [testing] Allow to lock a file without the lock being killed at the end of the request core#22823
• Test skips for broken client versions (Test file locking #134 (comment))
• Test skips for non DB-locking backends

Test description

Step	User
2	Enable QA testing app
3	Create dir/subdir/
4	Populate locks
5	Try to upload dir/subdir/file2.dat
6	Remove locks
7	Upload dir/subdir/file2.dat

Cases

Case	Description	Result	Test
0	Exclusive lock on dir/	Can not upload	❌ - File upload fails in 7 as well
1	Shared lock on dir/	Can upload	✅
2	Exclusive lock on dir/subdir/	Can not upload	❌ - File upload fails in 7 as well
3	Shared lock on dir/subdir/	Can upload	✅
4	Exclusive lock on dir/ + Shared lock on dir/subdir/	Can not upload	❌ - File upload fails in 7 as well
5	Shared lock on dir/ + Exclusive lock on dir/subdir/	Can not upload	❌ - File upload fails in 7 as well
6	Shared lock on dir/ and dir/subdir/	Can upload	✅

[nickvergessen]

Okay, I seem to have found out the issue.
When we sync the folders in step 3, they don't have a md5 (etag)set, also the folders have RDNVW as permission.
when we now do a second sync, the permissions are set correctly to RDNVCK.
But when we already moved a file into the folder, the file will fail because C is not in the permissions.

Not sure if "adding a second sync" is the way to go, or if the client should update the etag and permissions on the first sync?

[guruz]

When we sync the folders in step 3, they don't have a md5 (etag)set,

That's normal and intended, the next sync would update them via PROPFIND.

also the folders have RDNVW as permission.

That permission string is created by who? The server? Because I don't see it retrieved in PropagateRemoteMkdir and I don't find in the client where that could be created

FYI @ogoffart @ckamm

[nickvergessen]

also the folders have RDNVW as permission.

That permission string is created by who? The server?

I don't think so, because a propget always says permissions RDNVCK for me. So I assume that is client discovery.

[nickvergessen]

Here are the full sync logs of a try:
https://gist.github.com/nickvergessen/8f5b8ea746a32a7a934b

[nickvergessen]

Okay, so I changed the test to work with folders only, instead of files. Tests still failed, then I removed the skeleton files, to make sure, that the test always only ever deals with files, and see there: test passes.

So it seems that some variable is not reset correctly, after dealing with a file before reusing it for a folder. (All cheers to @guruz for that hint)

[nickvergessen]

I can't seem to find the root cause of this. I can only confirm that two things fix the issue:

1. Don't block the first sync of the file
2. Don't sync a files but folders only

I hope that helps the desktop guy stepping in here.

[ckamm]

@nickvergessen A potential fix was merged to master 7bd4f95b8c200e0fc2d5eff2bc5ab2428356a2ba, see owncloud/client#4548

[nickvergessen]

Yes, confirmed that it works with that commit. Nice find!

[nickvergessen]

The test is written in a way, that it will not fail on 9.0 and when using another locking then DB, so it should be good to merge @PVince81

[PVince81]

👍