Properly handle deleted users in UserMountCache#27028
Conversation
When encountering entries from a deleted user, discard such entries.
|
@PVince81, thanks for your PR! By analyzing the history of the files in this pull request, we identified @DeepDiver1975 to be a potential reviewer. |
|
Testes passed on Jenkins |
| private function dbRowToMountInfo(array $row) { | ||
| $user = $this->userManager->get($row['user_id']); | ||
| if ($user === null) { | ||
| // user does not exist any more, delete all mounts of that user directly |
There was a problem hiding this comment.
This feels misplaced. I'm talking strictly about the code, not the behaviour. I'd expect the function to convert the row, not to delete things.
Maybe adding a private function deleteSharesOfMissingUser or similar, and move it to the getMountsForUser function just before starting the conversion might look better.
There was a problem hiding this comment.
The bug in question was from yet another code path using dbRowToMountInfo.
Another alternative would be to use a repair step I guess.
There was a problem hiding this comment.
I don't like to use a repair step for this because this could happen often and we can't keep on repairing over and over.
I don't have better alternatives, so let's say "this is private stuff" 😄
|
👍 |
|
Maybe some day we'll use foreign keys and it will all be the job of cascading delete... |
|
Tests pass on Jenkins, results unpublished. |
|
I'll take care of the backports after the freeze. |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
When encountering entries from a deleted user, discard such entries.
Related Issue
Fixes #27012
Motivation and Context
How Has This Been Tested?
#27012 (comment)
Comments can be posted and no exceptions after the fix.
Also, the bogus entries with "unexist" are removed from
oc_mounts.Screenshots (if appropriate):
Types of changes
Checklist:
Please review @butonic @jvillafanez @IljaN @VicDeo