From 2f03853fb9407e23efb1ea151f53918dc6b5d733 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 25 Aug 2016 21:39:14 +0200 Subject: [PATCH] AvatarController cleanup * Use all DI components * Let the AppFramework resolve the AvatarController * Update unit tests * Unit tests no longer require DB --- core/Application.php | 13 -- core/Controller/AvatarController.php | 44 +++--- .../Core/Controller/AvatarControllerTest.php | 145 ++++++++++-------- 3 files changed, 106 insertions(+), 96 deletions(-) diff --git a/core/Application.php b/core/Application.php index e8c924432d..55bbc49797 100644 --- a/core/Application.php +++ b/core/Application.php @@ -83,19 +83,6 @@ class Application extends App { $c->query('Defaults') ); }); - $container->registerService('AvatarController', function(SimpleContainer $c) { - return new AvatarController( - $c->query('AppName'), - $c->query('Request'), - $c->query('AvatarManager'), - $c->query('Cache'), - $c->query('L10N'), - $c->query('UserManager'), - $c->query('UserSession'), - $c->query('UserFolder'), - $c->query('Logger') - ); - }); $container->registerService('LoginController', function(SimpleContainer $c) { return new LoginController( $c->query('AppName'), diff --git a/core/Controller/AvatarController.php b/core/Controller/AvatarController.php index 6fc08ec3c1..3aa002634d 100644 --- a/core/Controller/AvatarController.php +++ b/core/Controller/AvatarController.php @@ -31,14 +31,16 @@ use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\DataDisplayResponse; +use OCP\Files\File; +use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; use OCP\IAvatarManager; +use OCP\ICache; use OCP\ILogger; use OCP\IL10N; use OCP\IRequest; use OCP\IUserManager; use OCP\IUserSession; -use OCP\Files\Folder; /** * Class AvatarController @@ -50,7 +52,7 @@ class AvatarController extends Controller { /** @var IAvatarManager */ protected $avatarManager; - /** @var \OC\Cache\File */ + /** @var ICache */ protected $cache; /** @var IL10N */ @@ -62,41 +64,44 @@ class AvatarController extends Controller { /** @var IUserSession */ protected $userSession; - /** @var Folder */ - protected $userFolder; + /** @var IRootFolder */ + protected $rootFolder; /** @var ILogger */ protected $logger; + /** @var string */ + protected $userId; + /** * @param string $appName * @param IRequest $request * @param IAvatarManager $avatarManager - * @param \OC\Cache\File $cache + * @param ICache $cache * @param IL10N $l10n * @param IUserManager $userManager - * @param IUserSession $userSession - * @param Folder $userFolder + * @param IRootFolder $rootFolder * @param ILogger $logger + * @param string $userId */ public function __construct($appName, IRequest $request, IAvatarManager $avatarManager, - \OC\Cache\File $cache, + ICache $cache, IL10N $l10n, IUserManager $userManager, - IUserSession $userSession, - Folder $userFolder = null, - ILogger $logger) { + IRootFolder $rootFolder, + ILogger $logger, + $userId) { parent::__construct($appName, $request); $this->avatarManager = $avatarManager; $this->cache = $cache; $this->l = $l10n; $this->userManager = $userManager; - $this->userSession = $userSession; - $this->userFolder = $userFolder; + $this->rootFolder = $rootFolder; $this->logger = $logger; + $this->userId = $userId; } /** @@ -156,8 +161,9 @@ class AvatarController extends Controller { if (isset($path)) { $path = stripslashes($path); - $node = $this->userFolder->get($path); - if (!($node instanceof \OCP\Files\File)) { + $userFolder = $this->rootFolder->getUserFolder($this->userId); + $node = $userFolder->get($path); + if (!($node instanceof File)) { return new DataResponse(['data' => ['message' => $this->l->t('Please select a file.')]], Http::STATUS_OK, $headers); } if ($node->getSize() > 20*1024*1024) { @@ -240,10 +246,8 @@ class AvatarController extends Controller { * @return DataResponse */ public function deleteAvatar() { - $userId = $this->userSession->getUser()->getUID(); - try { - $avatar = $this->avatarManager->getAvatar($userId); + $avatar = $this->avatarManager->getAvatar($this->userId); $avatar->remove(); return new DataResponse(); } catch (\Exception $e) { @@ -285,8 +289,6 @@ class AvatarController extends Controller { * @return DataResponse */ public function postCroppedAvatar($crop) { - $userId = $this->userSession->getUser()->getUID(); - if (is_null($crop)) { return new DataResponse(['data' => ['message' => $this->l->t("No crop data provided")]], Http::STATUS_BAD_REQUEST); @@ -308,7 +310,7 @@ class AvatarController extends Controller { $image = new \OC_Image($tmpAvatar); $image->crop($crop['x'], $crop['y'], round($crop['w']), round($crop['h'])); try { - $avatar = $this->avatarManager->getAvatar($userId); + $avatar = $this->avatarManager->getAvatar($this->userId); $avatar->set($image); // Clean up $this->cache->remove('tmpAvatar'); diff --git a/tests/Core/Controller/AvatarControllerTest.php b/tests/Core/Controller/AvatarControllerTest.php index d45d061823..a275a8bd16 100644 --- a/tests/Core/Controller/AvatarControllerTest.php +++ b/tests/Core/Controller/AvatarControllerTest.php @@ -31,67 +31,83 @@ function is_uploaded_file($filename) { namespace Tests\Core\Controller; -use OC\Core\Application; -use OCP\AppFramework\IAppContainer; +use OC\Core\Controller\AvatarController; use OCP\AppFramework\Http; +use OCP\Files\Cache\ICache; use OCP\Files\File; +use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; -use OCP\IUser; use OCP\IAvatar; -use Punic\Exception; -use Test\Traits\UserTrait; +use OCP\IAvatarManager; +use OCP\IL10N; +use OCP\ILogger; +use OCP\IRequest; +use OCP\IUser; +use OCP\IUserManager; /** * Class AvatarControllerTest * - * @group DB - * * @package OC\Core\Controller */ class AvatarControllerTest extends \Test\TestCase { - use UserTrait; - - /** @var IAppContainer */ - private $container; /** @var \OC\Core\Controller\AvatarController */ private $avatarController; - /** @var IAvatar */ + /** @var IAvatar|\PHPUnit_Framework_MockObject_MockObject */ private $avatarMock; - /** @var IUser */ + /** @var IUser|\PHPUnit_Framework_MockObject_MockObject */ private $userMock; - /** @var File */ + /** @var File|\PHPUnit_Framework_MockObject_MockObject */ private $avatarFile; + + /** @var IAvatarManager|\PHPUnit_Framework_MockObject_MockObject */ + private $avatarManager; + /** @var ICache|\PHPUnit_Framework_MockObject_MockObject */ + private $cache; + /** @var IL10N|\PHPUnit_Framework_MockObject_MockObject */ + private $l; + /** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */ + private $userManager; + /** @var IRootFolder|\PHPUnit_Framework_MockObject_MockObject */ + private $rootFolder; + /** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */ + private $logger; + /** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */ + private $request; protected function setUp() { parent::setUp(); - $this->createUser('userid', 'pass'); - $this->loginAsUser('userid'); - - $app = new Application; - $this->container = $app->getContainer(); - $this->container['AppName'] = 'core'; - $this->container['AvatarManager'] = $this->getMockBuilder('OCP\IAvatarManager')->getMock(); - $this->container['Cache'] = $this->getMockBuilder('OC\Cache\File') + + $this->avatarManager = $this->getMockBuilder('OCP\IAvatarManager')->getMock(); + $this->cache = $this->getMockBuilder('OCP\ICache') ->disableOriginalConstructor()->getMock(); - $this->container['L10N'] = $this->getMockBuilder('OCP\IL10N')->getMock(); - $this->container['L10N']->method('t')->will($this->returnArgument(0)); - $this->container['UserManager'] = $this->getMockBuilder('OCP\IUserManager')->getMock(); - $this->container['UserSession'] = $this->getMockBuilder('OCP\IUserSession')->getMock(); - $this->container['Request'] = $this->getMockBuilder('OCP\IRequest')->getMock(); - $this->container['UserFolder'] = $this->getMockBuilder('OCP\Files\Folder')->getMock(); - $this->container['Logger'] = $this->getMockBuilder('OCP\ILogger')->getMock(); + $this->l = $this->getMockBuilder('OCP\IL10N')->getMock(); + $this->l->method('t')->will($this->returnArgument(0)); + $this->userManager = $this->getMockBuilder('OCP\IUserManager')->getMock(); + $this->request = $this->getMockBuilder('OCP\IRequest')->getMock(); + $this->rootFolder = $this->getMockBuilder('OCP\Files\IRootFolder')->getMock(); + $this->logger = $this->getMockBuilder('OCP\ILogger')->getMock(); $this->avatarMock = $this->getMockBuilder('OCP\IAvatar')->getMock(); $this->userMock = $this->getMockBuilder('OCP\IUser')->getMock(); - $this->avatarController = $this->container['AvatarController']; + $this->avatarController = new AvatarController( + 'core', + $this->request, + $this->avatarManager, + $this->cache, + $this->l, + $this->userManager, + $this->rootFolder, + $this->logger, + 'userid' + ); // Configure userMock $this->userMock->method('getDisplayName')->willReturn('displayName'); $this->userMock->method('getUID')->willReturn('userId'); - $this->container['UserManager']->method('get') + $this->userManager->method('get') ->willReturnMap([['userId', $this->userMock]]); - $this->container['UserSession']->method('getUser')->willReturn($this->userMock); $this->avatarFile = $this->getMockBuilder('OCP\Files\File')->getMock(); $this->avatarFile->method('getContent')->willReturn('image data'); @@ -100,7 +116,6 @@ class AvatarControllerTest extends \Test\TestCase { } public function tearDown() { - $this->logout(); parent::tearDown(); } @@ -108,7 +123,7 @@ class AvatarControllerTest extends \Test\TestCase { * Fetch an avatar if a user has no avatar */ public function testGetAvatarNoAvatar() { - $this->container['AvatarManager']->method('getAvatar')->willReturn($this->avatarMock); + $this->avatarManager->method('getAvatar')->willReturn($this->avatarMock); $this->avatarMock->method('getFile')->will($this->throwException(new NotFoundException())); $response = $this->avatarController->getAvatar('userId', 32); @@ -123,7 +138,7 @@ class AvatarControllerTest extends \Test\TestCase { */ public function testGetAvatar() { $this->avatarMock->method('getFile')->willReturn($this->avatarFile); - $this->container['AvatarManager']->method('getAvatar')->with('userId')->willReturn($this->avatarMock); + $this->avatarManager->method('getAvatar')->with('userId')->willReturn($this->avatarMock); $response = $this->avatarController->getAvatar('userId', 32); @@ -131,14 +146,14 @@ class AvatarControllerTest extends \Test\TestCase { $this->assertArrayHasKey('Content-Type', $response->getHeaders()); $this->assertEquals('image type', $response->getHeaders()['Content-Type']); - $this->assertEquals('my etag', $response->getEtag()); + $this->assertEquals('my etag', $response->getETag()); } /** * Fetch the avatar of a non-existing user */ public function testGetAvatarNoUser() { - $this->container['AvatarManager'] + $this->avatarManager ->method('getAvatar') ->with('userDoesNotExist') ->will($this->throwException(new \Exception('user does not exist'))); @@ -160,7 +175,7 @@ class AvatarControllerTest extends \Test\TestCase { ->with($this->equalTo(32)) ->willReturn($this->avatarFile); - $this->container['AvatarManager']->method('getAvatar')->willReturn($this->avatarMock); + $this->avatarManager->method('getAvatar')->willReturn($this->avatarMock); $this->avatarController->getAvatar('userId', 32); } @@ -174,7 +189,7 @@ class AvatarControllerTest extends \Test\TestCase { ->with($this->equalTo(64)) ->willReturn($this->avatarFile); - $this->container['AvatarManager']->method('getAvatar')->willReturn($this->avatarMock); + $this->avatarManager->method('getAvatar')->willReturn($this->avatarMock); $this->avatarController->getAvatar('userId', 0); } @@ -188,7 +203,7 @@ class AvatarControllerTest extends \Test\TestCase { ->with($this->equalTo(2048)) ->willReturn($this->avatarFile); - $this->container['AvatarManager']->method('getAvatar')->willReturn($this->avatarMock); + $this->avatarManager->method('getAvatar')->willReturn($this->avatarMock); $this->avatarController->getAvatar('userId', 2049); } @@ -197,7 +212,7 @@ class AvatarControllerTest extends \Test\TestCase { * Remove an avatar */ public function testDeleteAvatar() { - $this->container['AvatarManager']->method('getAvatar')->willReturn($this->avatarMock); + $this->avatarManager->method('getAvatar')->willReturn($this->avatarMock); $response = $this->avatarController->deleteAvatar(); $this->assertEquals(Http::STATUS_OK, $response->getStatus()); @@ -208,9 +223,9 @@ class AvatarControllerTest extends \Test\TestCase { */ public function testDeleteAvatarException() { $this->avatarMock->method('remove')->will($this->throwException(new \Exception("foo"))); - $this->container['AvatarManager']->method('getAvatar')->willReturn($this->avatarMock); + $this->avatarManager->method('getAvatar')->willReturn($this->avatarMock); - $this->container['Logger']->expects($this->once()) + $this->logger->expects($this->once()) ->method('logException') ->with(new \Exception("foo")); $expectedResponse = new Http\DataResponse(['data' => ['message' => 'An error occurred. Please contact your admin.']], Http::STATUS_BAD_REQUEST); @@ -229,7 +244,7 @@ class AvatarControllerTest extends \Test\TestCase { * Fetch tmp avatar */ public function testTmpAvatarValid() { - $this->container['Cache']->method('get')->willReturn(file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.jpg')); + $this->cache->method('get')->willReturn(file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.jpg')); $response = $this->avatarController->getTmpAvatar(); $this->assertEquals(Http::STATUS_OK, $response->getStatus()); @@ -255,11 +270,11 @@ class AvatarControllerTest extends \Test\TestCase { $this->assertTrue($copyRes); //Create file in cache - $this->container['Cache']->method('get')->willReturn(file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.jpg')); + $this->cache->method('get')->willReturn(file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.jpg')); //Create request return $reqRet = ['error' => [0], 'tmp_name' => [$fileName], 'size' => [filesize(\OC::$SERVERROOT.'/tests/data/testimage.jpg')]]; - $this->container['Request']->method('getUploadedFile')->willReturn($reqRet); + $this->request->method('getUploadedFile')->willReturn($reqRet); $response = $this->avatarController->postAvatar(null); @@ -276,7 +291,7 @@ class AvatarControllerTest extends \Test\TestCase { public function testPostAvatarInvalidFile() { //Create request return $reqRet = ['error' => [1], 'tmp_name' => ['foo']]; - $this->container['Request']->method('getUploadedFile')->willReturn($reqRet); + $this->request->method('getUploadedFile')->willReturn($reqRet); $response = $this->avatarController->postAvatar(null); @@ -293,11 +308,11 @@ class AvatarControllerTest extends \Test\TestCase { $this->assertTrue($copyRes); //Create file in cache - $this->container['Cache']->method('get')->willReturn(file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.gif')); + $this->cache->method('get')->willReturn(file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.gif')); //Create request return $reqRet = ['error' => [0], 'tmp_name' => [$fileName], 'size' => filesize(\OC::$SERVERROOT.'/tests/data/testimage.gif')]; - $this->container['Request']->method('getUploadedFile')->willReturn($reqRet); + $this->request->method('getUploadedFile')->willReturn($reqRet); $response = $this->avatarController->postAvatar(null); @@ -315,7 +330,9 @@ class AvatarControllerTest extends \Test\TestCase { $file = $this->getMockBuilder('OCP\Files\File') ->disableOriginalConstructor()->getMock(); $file->method('getContent')->willReturn(file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.jpg')); - $this->container['UserFolder']->method('get')->willReturn($file); + $userFolder = $this->getMockBuilder('OCP\Files\Folder')->getMock(); + $this->rootFolder->method('getUserFolder')->with('userid')->willReturn($userFolder); + $userFolder->method('get')->willReturn($file); //Create request return $response = $this->avatarController->postAvatar('avatar.jpg'); @@ -329,7 +346,9 @@ class AvatarControllerTest extends \Test\TestCase { */ public function testPostAvatarFromNoFile() { $file = $this->getMockBuilder('OCP\Files\Node')->getMock(); - $this->container['UserFolder'] + $userFolder = $this->getMockBuilder('OCP\Files\Folder')->getMock(); + $this->rootFolder->method('getUserFolder')->with('userid')->willReturn($userFolder); + $userFolder ->method('get') ->with('folder') ->willReturn($file); @@ -345,15 +364,17 @@ class AvatarControllerTest extends \Test\TestCase { * Test what happens if the upload of the avatar fails */ public function testPostAvatarException() { - $this->container['Cache']->expects($this->once()) + $this->cache->expects($this->once()) ->method('set') ->will($this->throwException(new \Exception("foo"))); $file = $this->getMockBuilder('OCP\Files\File') ->disableOriginalConstructor()->getMock(); $file->method('getContent')->willReturn(file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.jpg')); - $this->container['UserFolder']->method('get')->willReturn($file); + $userFolder = $this->getMockBuilder('OCP\Files\Folder')->getMock(); + $this->rootFolder->method('getUserFolder')->with('userid')->willReturn($userFolder); + $userFolder->method('get')->willReturn($file); - $this->container['Logger']->expects($this->once()) + $this->logger->expects($this->once()) ->method('logException') ->with(new \Exception("foo")); $expectedResponse = new Http\DataResponse(['data' => ['message' => 'An error occurred. Please contact your admin.']], Http::STATUS_OK); @@ -383,10 +404,10 @@ class AvatarControllerTest extends \Test\TestCase { * Test with non square crop */ public function testPostCroppedAvatarNoSquareCrop() { - $this->container['Cache']->method('get')->willReturn(file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.jpg')); + $this->cache->method('get')->willReturn(file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.jpg')); $this->avatarMock->method('set')->will($this->throwException(new \OC\NotSquareException)); - $this->container['AvatarManager']->method('getAvatar')->willReturn($this->avatarMock); + $this->avatarManager->method('getAvatar')->willReturn($this->avatarMock); $response = $this->avatarController->postCroppedAvatar(['x' => 0, 'y' => 0, 'w' => 10, 'h' => 11]); $this->assertEquals(Http::STATUS_BAD_REQUEST, $response->getStatus()); @@ -396,8 +417,8 @@ class AvatarControllerTest extends \Test\TestCase { * Check for proper reply on proper crop argument */ public function testPostCroppedAvatarValidCrop() { - $this->container['Cache']->method('get')->willReturn(file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.jpg')); - $this->container['AvatarManager']->method('getAvatar')->willReturn($this->avatarMock); + $this->cache->method('get')->willReturn(file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.jpg')); + $this->avatarManager->method('getAvatar')->willReturn($this->avatarMock); $response = $this->avatarController->postCroppedAvatar(['x' => 0, 'y' => 0, 'w' => 10, 'h' => 10]); $this->assertEquals(Http::STATUS_OK, $response->getStatus()); @@ -408,12 +429,12 @@ class AvatarControllerTest extends \Test\TestCase { * Test what happens if the cropping of the avatar fails */ public function testPostCroppedAvatarException() { - $this->container['Cache']->method('get')->willReturn(file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.jpg')); + $this->cache->method('get')->willReturn(file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.jpg')); $this->avatarMock->method('set')->will($this->throwException(new \Exception('foo'))); - $this->container['AvatarManager']->method('getAvatar')->willReturn($this->avatarMock); + $this->avatarManager->method('getAvatar')->willReturn($this->avatarMock); - $this->container['Logger']->expects($this->once()) + $this->logger->expects($this->once()) ->method('logException') ->with(new \Exception('foo')); $expectedResponse = new Http\DataResponse(['data' => ['message' => 'An error occurred. Please contact your admin.']], Http::STATUS_BAD_REQUEST); @@ -428,7 +449,7 @@ class AvatarControllerTest extends \Test\TestCase { $fileName = \OC::$SERVERROOT.'/tests/data/testimage.jpg'; //Create request return $reqRet = ['error' => [0], 'tmp_name' => [$fileName], 'size' => [21*1024*1024]]; - $this->container['Request']->method('getUploadedFile')->willReturn($reqRet); + $this->request->method('getUploadedFile')->willReturn($reqRet); $response = $this->avatarController->postAvatar(null);