refactor: Expose a public exception when no user is found#61544
refactor: Expose a public exception when no user is found#61544CarlSchwan wants to merge 1 commit into
Conversation
CarlSchwan
requested review from
a team,
blizzz and
nickvergessen
as code owners
CarlSchwan
added
technical debt
CarlSchwan
requested review from
ArtificialOwl,
come-nc,
icewind1991 and
provokateurin
and removed request for
a team
CarlSchwan
force-pushed
the
carl/nouserexception
branch
from
c476514 to
efa25e8
Compare
susnux
left a comment
susnux
left a comment
There was a problem hiding this comment.
All throw UserNotFoundException must be reverted to throw NoUserFoundException otherwise legacy code cannot catch them.
|
|
||
| if ($currentUser === null) { | ||
| throw new NoUserException(); | ||
| throw new UserNotFoundException(); |
There was a problem hiding this comment.
This does not work.
Old code still catches NoUserException throwing UserNotFoundException is the parent class.
Meaning the catch does not work anymore as the now thrown exception is less specific.
There was a problem hiding this comment.
The throw UserNotFoundException I ported were all in commands and are just to display a nice error message when running occ and are not catched. I left the one in lib/private/Files/Filesystem.php and lib/private/Files/Node/Root.php as before.
The one in Session can be argue for. It's at the moment not documented and the method is not used https://github.com/search?q=org%3Anextcloud+setImpersonatingUserID&type=code
There was a problem hiding this comment.
The throw UserNotFoundException I ported were all in commands and are just to display a nice error message when running occ and are not catched.
That makes sense
I left the one in lib/private/Files/Filesystem.php and lib/private/Files/Node/Root.php as before.
Then ok!
CarlSchwan
force-pushed
the
carl/nouserexception
branch
from
efa25e8 to
8a0c1c6
Compare
And use this new exception everywhere outside of where the old one was throw. Signed-off-by: Carl Schwan <carlschwan@kde.org>
CarlSchwan
force-pushed
the
carl/nouserexception
branch
from
8a0c1c6 to
8ecce9d
Compare
| use OCP\Session\Exceptions\SessionNotAvailableException; | ||
| use OCP\User\Events\PostLoginEvent; | ||
| use OCP\User\Events\UserFirstTimeLoggedInEvent; | ||
| use OCP\User\Exceptions\UserNotFoundException; |
There was a problem hiding this comment.
Stray import?
Edit: CS agrees
4 participants
And use this new exception everywhere outside of where the old one was throw.
Summary
TODO
Checklist
3. to review, feature component)stable32)AI (if applicable)