Conversation
|
By analyzing the blame information on this pull request, we identified @DeepDiver1975, @PVince81 and @nickvergessen to be potential reviewers |
|
To test insert a cyclic share... see the tests on how to do that... |
There was a problem hiding this comment.
make sure share type is file?
There was a problem hiding this comment.
file or folder then... but 'other shares' should never set the file_source... but ok
There was a problem hiding this comment.
you did it on the count, but missed it here?
|
@nickvergessen @icewind1991 fixed |
There was a problem hiding this comment.
Needs to use $qb->expr()->literal(), otherwise columns file and folder are not found.
Fixes #23265 (A possibly costly) repair job to remove cyclic shares.
|
|
||
| $qb->select('fileid') | ||
| ->from('filecache') | ||
| ->where($qb->expr()->eq('path', $qb->expr()->literal('files'))); |
There was a problem hiding this comment.
Could it also happen that the real root is shared ? The one with path "".
However that was even possible to happen in the first place...
There was a problem hiding this comment.
Question 2: what happens if there's an external storage with a path "files" on it ? Would it be excluded ?
I didn't see where you check that it's indeed the user's home storage and not an external one (with the included nightmare of storage id, not being able to rely on "home::"). Maybe oc_mounts can be used for that ?
There was a problem hiding this comment.
Well this is just a easy and simplified test. On real deletion we make sure to only take the real roots into account.
There was a problem hiding this comment.
exactly as suggested by @nickvergessen this is just a very simple quick tests. That should tell us if there is potentially something to fix. For probabaly 99% of the isntallations out there this will be false so they don't have to do the expensive repair step.
|
Tested with moving a share in the DB to point at the "files" entry. The repair job deleted that share properly 👍 See my question above. |
| private function removeRootShares(IOutput $output) { | ||
| $function = function(IUser $user) use ($output) { | ||
| $userFolder = $this->rootFolder->getUserFolder($user->getUID()); | ||
| $fileId = $userFolder->getId(); |
|
👍 |
|
@owncloud/sharing should be backport this to 9.0 ? |
Yes should be backported. |
|
Will do (there are some complications due to IRepairStep incompatibilities) |
5 participants
Fixes #23265
(A possibly costly) repair job to remove cyclic shares.
CC: @nickvergessen @PVince81 @icewind1991