Skip to content

Commit

Permalink
Fixes #30062: remove old avatar get endpoint
Browse files Browse the repository at this point in the history
  • Loading branch information
DeepDiver1975 committed Oct 10, 2018
1 parent 438615a commit 5c038c3
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 147 deletions.
43 changes: 0 additions & 43 deletions core/Controller/AvatarController.php
Original file line number Diff line number Diff line change
Expand Up @@ -103,49 +103,6 @@ public function __construct($appName,
$this->logger = $logger;
}

/**
* @NoAdminRequired
* @NoCSRFRequired
*
* @param string $userId
* @param int $size
* @return DataResponse|DataDisplayResponse
*/
public function getAvatar($userId, $size) {
if ($size > 2048) {
$size = 2048;
} elseif ($size <= 0) {
$size = 64;
}

try {
$avatar = $this->avatarManager->getAvatar($userId)->getFile($size);
$resp = new DataDisplayResponse($avatar->getContent(),
Http::STATUS_OK,
['Content-Type' => $avatar->getMimeType()]);
$resp->setETag($avatar->getEtag());
} catch (NotFoundException $e) {
$user = $this->userManager->get($userId);
$resp = new DataResponse([
'data' => [
'displayname' => $user->getDisplayName(),
],
]);
} catch (\Exception $e) {
$resp = new DataResponse([
'data' => [
'displayname' => '',
],
]);
}

$resp->addHeader('Pragma', 'public');
$resp->cacheFor(0);
$resp->setLastModified(new \DateTime('now', new \DateTimeZone('GMT')));

return $resp;
}

/**
* @NoAdminRequired
*
Expand Down
8 changes: 5 additions & 3 deletions core/js/jquery.avatar.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,11 @@

var $div = this;

var url = OC.generateUrl(
'/avatar/{user}/{size}',
{user: user, size: Math.ceil(size * window.devicePixelRatio)});
var url = OC.getRootPath() +
'/remote.php/dav/avatars' +
'/' + encodeURIComponent(user) +
'/' + encodeURIComponent(Math.ceil(size * window.devicePixelRatio).toString()) +
'.jpeg';

// If the displayname is not defined we use the old code path
if (typeof(displayname) === 'undefined') {
Expand Down
1 change: 0 additions & 1 deletion core/routes.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
['name' => 'lost#resetform', 'url' => '/lostpassword/reset/form/{token}/{userId}', 'verb' => 'GET'],
['name' => 'lost#setPassword', 'url' => '/lostpassword/set/{token}/{userId}', 'verb' => 'POST'],
['name' => 'user#getDisplayNames', 'url' => '/displaynames', 'verb' => 'POST'],
['name' => 'avatar#getAvatar', 'url' => '/avatar/{userId}/{size}', 'verb' => 'GET'],
['name' => 'avatar#deleteAvatar', 'url' => '/avatar/', 'verb' => 'DELETE'],
['name' => 'avatar#postCroppedAvatar', 'url' => '/avatar/cropped', 'verb' => 'POST'],
['name' => 'avatar#getTmpAvatar', 'url' => '/avatar/tmp', 'verb' => 'GET'],
Expand Down
4 changes: 2 additions & 2 deletions core/templates/layout.user.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@
} ?>">
<?php if ($_['userAvatarSet']): ?>
<img alt="" width="32" height="32"
src="<?php p(\OC::$server->getURLGenerator()->linkToRoute('core.avatar.getAvatar', ['userId' => $_['user_uid'], 'size' => 32]));?>"
srcset="<?php p(\OC::$server->getURLGenerator()->linkToRoute('core.avatar.getAvatar', ['userId' => $_['user_uid'], 'size' => 64]));?> 2x, <?php p(\OC::$server->getURLGenerator()->linkToRoute('core.avatar.getAvatar', ['userId' => $_['user_uid'], 'size' => 128]));?> 4x"
src="<?php p(\OC::$server->getURLGenerator()->linkTo('', 'remote.php') . '/dav/avatars/' . \rawurlencode($_['user_uid']) . '/32.jpeg');?>"
srcset="<?php p(\OC::$server->getURLGenerator()->linkTo('', 'remote.php') . '/dav/avatars/' . \rawurlencode($_['user_uid']) . '/32.jpeg');?> 2x, <?php p(\OC::$server->getURLGenerator()->linkTo('', 'remote.php') . '/dav/avatars/' . \rawurlencode($_['user_uid']) . '/32.jpeg');?> 4x"
>
<?php endif; ?>
</div>
Expand Down
19 changes: 11 additions & 8 deletions settings/js/panels/profile.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,12 @@ function updateAvatar (hidedefault) {
}
$displaydiv.css({'background-color': ''});
$displaydiv.avatar(OC.currentUser, 145, true);
$.get(OC.generateUrl(
'/avatar/{user}/{size}',
{user: OC.currentUser, size: 1}
), function (result) {
var url = OC.getRootPath() +
'/remote.php/dav/avatars' +
'/' + encodeURIComponent(OC.getCurrentUser().uid) +
'/96.jpeg';

$.get(url, function (result) {
if (typeof(result) === 'string') {
// Show the delete button when the avatar is custom
$('#removeavatar').removeClass('hidden').addClass('inlineblock');
Expand Down Expand Up @@ -334,10 +336,11 @@ $(document).ready(function () {
});

// does the user have a custom avatar? if he does show #removeavatar
$.get(OC.generateUrl(
'/avatar/{user}/{size}',
{user: OC.currentUser, size: 1}
), function (result) {
var url = OC.getRootPath() +
'/remote.php/dav/avatars' +
'/' + encodeURIComponent(OC.getCurrentUser().uid) +
'/1.jpeg';
$.get(url, function (result) {
if (typeof(result) === 'string') {
// Show the delete button when the avatar is custom
$('#removeavatar').removeClass('hidden').addClass('inlineblock');
Expand Down
90 changes: 0 additions & 90 deletions tests/Core/Controller/AvatarControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -130,96 +130,6 @@ public function tearDown() {
parent::tearDown();
}

/**
* Fetch an avatar if a user has no avatar
*/
public function testGetAvatarNoAvatar() {
$this->avatarManager->expects($this->any())->method('getAvatar')->willReturn($this->avatarMock);
$this->avatarMock->expects($this->any())->method('getFile')->will($this->throwException(new NotFoundException()));
$response = $this->avatarController->getAvatar('userId', 32);

//Comment out until JS is fixed
//$this->assertEquals(Http::STATUS_NOT_FOUND, $response->getStatus());
$this->assertEquals(Http::STATUS_OK, $response->getStatus());
$this->assertEquals('displayName', $response->getData()['data']['displayname']);
}

/**
* Fetch the user's avatar
*/
public function testGetAvatar() {
$this->avatarMock->expects($this->any())->method('getFile')->willReturn($this->avatarFile);
$this->avatarManager->expects($this->any())->method('getAvatar')->with('userId')->willReturn($this->avatarMock);

$response = $this->avatarController->getAvatar('userId', 32);

$this->assertEquals(Http::STATUS_OK, $response->getStatus());
$this->assertArrayHasKey('Content-Type', $response->getHeaders());
$this->assertEquals('image type', $response->getHeaders()['Content-Type']);

$this->assertEquals('my etag', $response->getETag());
}

/**
* Fetch the avatar of a non-existing user
*/
public function testGetAvatarNoUser() {
$this->avatarManager
->expects($this->any())
->method('getAvatar')
->with('userDoesNotExist')
->will($this->throwException(new \Exception('user does not exist')));

$response = $this->avatarController->getAvatar('userDoesNotExist', 32);

//Comment out until JS is fixed
//$this->assertEquals(Http::STATUS_NOT_FOUND, $response->getStatus());
$this->assertEquals(Http::STATUS_OK, $response->getStatus());
$this->assertEquals('', $response->getData()['data']['displayname']);
}

/**
* Make sure we get the correct size
*/
public function testGetAvatarSize() {
$this->avatarMock->expects($this->once())
->method('getFile')
->with($this->equalTo(32))
->willReturn($this->avatarFile);

$this->avatarManager->expects($this->any())->method('getAvatar')->willReturn($this->avatarMock);

$this->avatarController->getAvatar('userId', 32);
}

/**
* We cannot get avatars that are 0 or negative
*/
public function testGetAvatarSizeMin() {
$this->avatarMock->expects($this->once())
->method('getFile')
->with($this->equalTo(64))
->willReturn($this->avatarFile);

$this->avatarManager->expects($this->any())->method('getAvatar')->willReturn($this->avatarMock);

$this->avatarController->getAvatar('userId', 0);
}

/**
* We do not support avatars larger than 2048*2048
*/
public function testGetAvatarSizeMax() {
$this->avatarMock->expects($this->once())
->method('getFile')
->with($this->equalTo(2048))
->willReturn($this->avatarFile);

$this->avatarManager->expects($this->any())->method('getAvatar')->willReturn($this->avatarMock);

$this->avatarController->getAvatar('userId', 2049);
}

/**
* Remove an avatar
*/
Expand Down

0 comments on commit 5c038c3

Please sign in to comment.