fix: Exclude Readme.md files from file locking in the text app#8284
fix: Exclude Readme.md files from file locking in the text app#8284rikled wants to merge 6 commits intonextcloud:mainfrom
Conversation
Signed-off-by: Hendrik Leidinger <hendrik.leidinger@nextcloud.com>
Signed-off-by: Hendrik Leidinger <hendrik.leidinger@nextcloud.com>
Signed-off-by: Hendrik Leidinger <hendrik.leidinger@nextcloud.com>
Signed-off-by: Hendrik Leidinger <hendrik.leidinger@nextcloud.com>
Signed-off-by: Hendrik Leidinger <hendrik.leidinger@nextcloud.com>
Signed-off-by: Hendrik Leidinger <hendrik.leidinger@nextcloud.com>
silverkszlo
left a comment
There was a problem hiding this comment.
I've tested this by adding temporary debug logging and saw the following output in the nextcloud log, so seems to work :)
{"message":"Attempting to lock file","data":{"fileId":"89","fileName":"regular.md"}}
{"message":"Lock result","data":{"fileId":"89","fileName":"regular.md","isLocked":"true"}}
{"message":"Skipping lock for file","data":{"fileId":"86","fileName":"Readme.md","reason":"Readme.md exception"}}
| /** @psalm-suppress RedundantCastGivenDocblockType */ | ||
| if (!$readOnly && strcasecmp((string)$file->getName(), 'Readme.md') !== 0) { |
There was a problem hiding this comment.
| /** @psalm-suppress RedundantCastGivenDocblockType */ | |
| if (!$readOnly && strcasecmp((string)$file->getName(), 'Readme.md') !== 0) { | |
| if (!$readOnly && strcasecmp($file->getName(), 'Readme.md') !== 0) { |
I think if the phpunit tests fail on that it is rather due to the file mock not having calls of getName mocked.
Probably just a matter of addding in
$file->method('getName')->willReturn('MyFile.md');There was a problem hiding this comment.
When running with the real implementation it is always returning a string
https://github.com/nextcloud/server/blob/0d6c5516947cbe0837ab4731b00bbd60f37908d0/lib/public/Files/Node.php#L214
http://github.com/nextcloud/server/blob/0d6c5516947cbe0837ab4731b00bbd60f37908d0/lib/private/Files/Node/Node.php#L313
Especially since basename always returns a string, not null or anything else: https://www.php.net/manual/en/function.basename.php
📝 Summary
This PR excludes Readme files from file locking. Without it is almost impossible to delete these files as they are opened as soon as a user visits the directory of the file. This is meant as an intermediate solution until #5597 is implemented.
🏁 Checklist
npm run lint/npm run stylelint/composer run cs:check)