Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
b553b43
Make possible to send requests as anonymous users in integration tests
danxuliu Dec 3, 2020
184742e
Make possible to set body in requesttoken requests in integration tests
danxuliu Dec 3, 2020
2cc22a0
Add integration tests for user avatars
danxuliu Dec 3, 2020
1552add
Add integration tests for resized user avatars
danxuliu Dec 4, 2020
b4b3276
Add integration tests for getting guest avatars
danxuliu Dec 7, 2020
1de8dc3
Add getter for generic avatars to IAvatarManager
danxuliu Dec 7, 2020
bcce5a6
Add OCS endpoint for avatars
danxuliu Dec 7, 2020
2522439
Make possible to set body in OCS requests in integration tests
danxuliu Dec 7, 2020
b190431
Add integration tests for getting and setting generic avatars
danxuliu Dec 7, 2020
63cbd7b
fixup! Add OCS endpoint for avatars
danxuliu Dec 9, 2020
a31a7fd
fixup! Add OCS endpoint for avatars
danxuliu Dec 10, 2020
92c9fc0
Make possible to define custom avatar types
danxuliu Dec 10, 2020
a836608
fixup! Add OCS endpoint for avatars
danxuliu Dec 10, 2020
95e0177
Move registration of IAvatarProviders to IRegistrationContext
danxuliu Dec 10, 2020
f04a16a
fixup! Make possible to define custom avatar types
danxuliu Dec 14, 2020
ebf242a
fixup! Add OCS endpoint for avatars
danxuliu Dec 14, 2020
bf9169e
fixup! Add getter for generic avatars to IAvatarManager
danxuliu Dec 14, 2020
1e13309
fixup! Make possible to define custom avatar types
danxuliu Dec 14, 2020
6e43ce7
fixup! Add OCS endpoint for avatars
danxuliu Dec 14, 2020
90ac35d
fixup! Make possible to define custom avatar types
danxuliu Dec 14, 2020
5d0102e
fixup! Move registration of IAvatarProviders to IRegistrationContext
danxuliu Dec 14, 2020
ab910ec
fixup! Move registration of IAvatarProviders to IRegistrationContext
danxuliu Dec 14, 2020
a2c63ff
fixup! Make possible to define custom avatar types
danxuliu Dec 14, 2020
76e4bd2
Move deprecated ILogger to LoggerInterface in avatar private classes
danxuliu Dec 14, 2020
612dbe6
Add explicit providers for user and guest avatars
danxuliu Dec 14, 2020
0e953eb
Remove no longer needed attributes from AvatarManager
danxuliu Dec 14, 2020
0bb311f
Get the avatar provider rather than the avatar itself from the manager
danxuliu Dec 14, 2020
611881b
Add method to get the cache duration of an avatar to IAvatarProvider
danxuliu Dec 14, 2020
6c99a9a
fixup! Add method to get the cache duration of an avatar to IAvatarPr…
danxuliu Dec 14, 2020
6079793
fixup! Add explicit providers for user and guest avatars
danxuliu Dec 15, 2020
bcd2074
fixup! Move registration of IAvatarProviders to IRegistrationContext
danxuliu Dec 15, 2020
23f4c1d
fixup! Get the avatar provider rather than the avatar itself from the…
danxuliu Dec 15, 2020
e88d3b6
fixup! Make possible to define custom avatar types
danxuliu Dec 15, 2020
4a26943
fixup! Add explicit providers for user and guest avatars
danxuliu Dec 15, 2020
fa0342a
fixup! Add explicit providers for user and guest avatars
danxuliu Dec 15, 2020
ebc3f14
fixup! Get the avatar provider rather than the avatar itself from the…
danxuliu Dec 15, 2020
03eed3e
fixup! Add integration tests for getting and setting generic avatars
danxuliu Dec 15, 2020
1348cab
fixup! Add integration tests for getting and setting generic avatars
danxuliu Dec 15, 2020
8d69a4e
fixup! Add OCS endpoint for avatars
danxuliu Dec 15, 2020
4368ec0
fixup! Make possible to define custom avatar types
danxuliu Dec 15, 2020
6b8f290
fixup! Add integration tests for getting and setting generic avatars
danxuliu Dec 15, 2020
68b298e
fixup! Add method to get the cache duration of an avatar to IAvatarPr…
danxuliu Dec 16, 2020
cc7cd18
Split try/catch blocks to catch only the relevant exceptions
danxuliu Dec 16, 2020
6ed5fde
Add method to check if an avatar can be accessed by the current user
danxuliu Dec 16, 2020
65c8c22
Add method to check if an avatar can be modified by the current user
danxuliu Dec 16, 2020
135d14b
Add integration tests for unauthorized modification of avatars
danxuliu Dec 16, 2020
330fac8
Add method to get the version of an avatar
danxuliu Dec 16, 2020
68fa045
Limit the returned sizes of the avatars to a predefined set
danxuliu Dec 16, 2020
601d741
fixup! Get the avatar provider rather than the avatar itself from the…
danxuliu Dec 16, 2020
6391a2b
fixup! Add explicit providers for user and guest avatars
danxuliu Dec 16, 2020
029d22c
fixup! Add method to check if an avatar can be modified by the curren…
danxuliu Dec 16, 2020
3a4b926
fixup! Add explicit providers for user and guest avatars
danxuliu Dec 16, 2020
6895199
fixup! Make possible to define custom avatar types
danxuliu Dec 16, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fixup! Make possible to define custom avatar types
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
  • Loading branch information
danxuliu committed Dec 15, 2020
commit e88d3b6a4ca54cbd40abf53fc5e4b0f814732d2b
3 changes: 3 additions & 0 deletions core/Controller/GenericAvatarController.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\Http\FileDisplayResponse;
use OCP\AppFramework\Http\Response;
use OCP\AvatarProviderException;
use OCP\Files\NotFoundException;
use OCP\IAvatarManager;
use OCP\IL10N;
Expand Down Expand Up @@ -90,6 +91,8 @@ public function getAvatar(string $avatarType, string $avatarId, int $size): Resp
);
} catch (\InvalidArgumentException $e) {
return new DataResponse([], Http::STATUS_NOT_FOUND);
} catch (AvatarProviderException $e) {
return new DataResponse([], Http::STATUS_NOT_FOUND);
} catch (NotFoundException $e) {
return new DataResponse([], Http::STATUS_NOT_FOUND);
} catch (\Exception $e) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • This is a bit generic – is it possible to catch a more specific exception?
  • If catching that generic I would highly recommend logging the exception to help our futures selves debug any issues with this code :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit generic – is it possible to catch a more specific exception?

We could catch OCP\Files\NotFoundException as it is thrown by getFile(), InvalidArgumentException as it is thrown by IAvatarManager and whatever is thrown by IAvatarProvider::getAvatar(). I wonder whether other exceptions should be catched or just left to bubble up and generate an error 500 (maybe even the exception thrown by IAvatarProvider::getAvatar() if it ends being just a generic Exception).

If catching that generic I would highly recommend logging the exception to help our futures selves debug any issues with this code :)

👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of AvatarException I named it AvatarProviderException, as that is more specific and gives room to add in the future an AvatarException for avatar specific errors if needed.

In the end I removed the log, as now a generic exception is no longer caught, but an AvatarProviderException, which would be thrown if the provider can not get an avatar for the given id. I think that belongs to the realm of expected errors that do not need to be logged (but if I am wrong it can be added back, of course ;-) ).

Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@
'OCP\\Authentication\\TwoFactorAuth\\TwoFactorException' => $baseDir . '/lib/public/Authentication/TwoFactorAuth/TwoFactorException.php',
'OCP\\Authentication\\TwoFactorAuth\\TwoFactorProviderDisabled' => $baseDir . '/lib/public/Authentication/TwoFactorAuth/TwoFactorProviderDisabled.php',
'OCP\\AutoloadNotAllowedException' => $baseDir . '/lib/public/AutoloadNotAllowedException.php',
'OCP\\AvatarProviderException' => $baseDir . '/lib/public/AvatarProviderException.php',
'OCP\\BackgroundJob' => $baseDir . '/lib/public/BackgroundJob.php',
'OCP\\BackgroundJob\\IJob' => $baseDir . '/lib/public/BackgroundJob/IJob.php',
'OCP\\BackgroundJob\\IJobList' => $baseDir . '/lib/public/BackgroundJob/IJobList.php',
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
'OCP\\Authentication\\TwoFactorAuth\\TwoFactorException' => __DIR__ . '/../../..' . '/lib/public/Authentication/TwoFactorAuth/TwoFactorException.php',
'OCP\\Authentication\\TwoFactorAuth\\TwoFactorProviderDisabled' => __DIR__ . '/../../..' . '/lib/public/Authentication/TwoFactorAuth/TwoFactorProviderDisabled.php',
'OCP\\AutoloadNotAllowedException' => __DIR__ . '/../../..' . '/lib/public/AutoloadNotAllowedException.php',
'OCP\\AvatarProviderException' => __DIR__ . '/../../..' . '/lib/public/AvatarProviderException.php',
'OCP\\BackgroundJob' => __DIR__ . '/../../..' . '/lib/public/BackgroundJob.php',
'OCP\\BackgroundJob\\IJob' => __DIR__ . '/../../..' . '/lib/public/BackgroundJob/IJob.php',
'OCP\\BackgroundJob\\IJobList' => __DIR__ . '/../../..' . '/lib/public/BackgroundJob/IJobList.php',
Expand Down
39 changes: 39 additions & 0 deletions lib/public/AvatarProviderException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<?php

declare(strict_types=1);

/**
* @copyright Copyright (c) 2020, Daniel Calviño Sánchez ([email protected])
*
* @author Daniel Calviño Sánchez <[email protected]>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

namespace OCP;

/**
* Generic exception thrown when an AvatarProvider can not perform an action
*
* @since 21.0.0
*/
class AvatarProviderException extends \RuntimeException {

public function __construct(string $message = "", int $code = 0, \Exception $previous = null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function __construct(string $message = "", int $code = 0, \Exception $previous = null) {
public function __construct(string $message = '', int $code = 0, \Exception $previous = null) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and for $previous we should allow \Throwable, not just \Exception

parent::__construct($message, $code, $previous);
}
}
3 changes: 2 additions & 1 deletion lib/public/IAvatarProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ interface IAvatarProvider {
*
* @param string $id the identifier of the avatar
* @return IAvatar the avatar instance
* @throws \Exception if an error occurred while getting the avatar
* @throws AvatarProviderException if an error occurred while getting the
* avatar
* @since 21.0.0
*/
public function getAvatar(string $id): IAvatar;
Expand Down