DI in avatar code

* DI in avatar code
* Use the node API
* More unit tests
* Unit tests no longer require DB

lib/private/avatar.php

@@ -29,7 +29,9 @@
 
 namespace OC;
 
-use OC\Files\Filesystem;
+use OCP\Files\Folder;
+use OCP\Files\File;
+use OCP\IL10N;
 use OC_Image;
 
 /**
@@ -37,19 +39,21 @@ use OC_Image;
  */
 
 class Avatar implements \OCP\IAvatar {
-	/** @var Files\View  */
+	/** @var Folder */
-	private $view;
+	private $folder;
+
+	/** @var IL10N */
+	private $l;
 
 	/**
 	 * constructor
-	 * @param string $user user to do avatar-management with
+	 *
-	 * @throws \Exception In case the username is potentially dangerous
+	 * @param Folder $folder The folder where the avatars are
+	 * @param IL10N $l
 	 */
-	public function __construct ($user) {
+	public function __construct (Folder $folder, IL10N $l) {
-		if(!Filesystem::isValidPath($user)) {
+		$this->folder = $folder;
-			throw new \Exception('Username may not contain slashes');
+		$this->l = $l;
-		}
-		$this->view = new \OC\Files\View('/'.$user);
 	}
 
 	/**
@@ -58,21 +62,25 @@ class Avatar implements \OCP\IAvatar {
 	 * @return boolean|\OCP\IImage containing the avatar or false if there's no image
 	*/
 	public function get ($size = 64) {
-		if ($this->view->file_exists('avatar.jpg')) {
+		if ($this->folder->nodeExists('avatar.jpg')) {
 			$ext = 'jpg';
-		} elseif ($this->view->file_exists('avatar.png')) {
+		} elseif ($this->folder->nodeExists('avatar.png')) {
 			$ext = 'png';
 		} else {
 			return false;
 		}
 
 		$avatar = new OC_Image();
-		if ($this->view->file_exists('avatar.' . $size . '.' . $ext)) {
+		if ($this->folder->nodeExists('avatar.' . $size . '.' . $ext)) {
-			$avatar->loadFromData($this->view->file_get_contents('avatar.' . $size . '.' . $ext));
+			/** @var File $node */
+			$node = $this->folder->get('avatar.' . $size . '.' . $ext);
+			$avatar->loadFromData($node->getContent());
 		} else {
-			$avatar->loadFromData($this->view->file_get_contents('avatar.' . $ext));
+			/** @var File $node */
+			$node = $this->folder->get('avatar.' . $ext);
+			$avatar->loadFromData($node->getContent());
 			$avatar->resize($size);
-			$this->view->file_put_contents('avatar.' . $size . '.' . $ext, $avatar->data());
+			$this->folder->newFile('avatar.' . $size . '.' . $ext)->putContent($avatar->data());
 		}
 		return $avatar;
 	}
@@ -83,7 +91,7 @@ class Avatar implements \OCP\IAvatar {
 	 * @return bool
 	 */
 	public function exists() {
-		return $this->view->file_exists('avatar.jpg') || $this->view->file_exists('avatar.png');
+		return $this->folder->nodeExists('avatar.jpg') || $this->folder->nodeExists('avatar.png');
 	}
 
 	/**
@@ -107,22 +115,19 @@ class Avatar implements \OCP\IAvatar {
 			$type = 'jpg';
 		}
 		if ($type !== 'jpg' && $type !== 'png') {
-			$l = \OC::$server->getL10N('lib');
+			throw new \Exception($this->l->t("Unknown filetype"));
-			throw new \Exception($l->t("Unknown filetype"));
 		}
 
 		if (!$img->valid()) {
-			$l = \OC::$server->getL10N('lib');
+			throw new \Exception($this->l->t("Invalid image"));
-			throw new \Exception($l->t("Invalid image"));
 		}
 
 		if (!($img->height() === $img->width())) {
 			throw new \OC\NotSquareException();
 		}
 
-		$this->view->unlink('avatar.jpg');
+		$this->remove();
-		$this->view->unlink('avatar.png');
+		$this->folder->newFile('avatar.'.$type)->putContent($data);
-		$this->view->file_put_contents('avatar.'.$type, $data);
 	}
 
 	/**
@@ -130,7 +135,11 @@ class Avatar implements \OCP\IAvatar {
 	 * @return void
 	*/
 	public function remove () {
-		$this->view->unlink('avatar.jpg');
+		try {
-		$this->view->unlink('avatar.png');
+			$this->folder->get('avatar.jpg')->delete();
+		} catch (\OCP\Files\NotFoundException $e) {}
+		try {
+			$this->folder->get('avatar.png')->delete();
+		} catch (\OCP\Files\NotFoundException $e) {}
 	}
 }

lib/private/avatarmanager.php

@@ -27,13 +27,33 @@
 namespace OC;
 
 use OCP\IAvatarManager;
-use OC\Avatar;
+use OCP\IUserManager;
+use OCP\Files\IRootFolder;
+use OCP\IL10N;
 
 /**
  * This class implements methods to access Avatar functionality
  */
 class AvatarManager implements IAvatarManager {
 
+	/** @var  IUserManager */
+	private $userManager;
+
+	/** @var  IRootFolder */
+	private $rootFolder;
+
+	/** @var IL10N */
+	private $l;
+
+	public function __construct(
+			IUserManager $userManager,
+			IRootFolder $rootFolder,
+			IL10N $l) {
+		$this->userManager = $userManager;
+		$this->rootFolder = $rootFolder;
+		$this->l = $l;
+	}
+
 	/**
 	 * return a user specific instance of \OCP\IAvatar
 	 * @see \OCP\IAvatar
@@ -42,6 +62,9 @@ class AvatarManager implements IAvatarManager {
 	 * @throws \Exception In case the username is potentially dangerous
 	 */
 	public function getAvatar($user) {
-		return new Avatar($user);
+		if (!$this->userManager->userExists($user)) {
+			throw new \Exception('user does not exist');
+		}
+		return new Avatar($this->rootFolder->getUserFolder($user)->getParent(), $this->l);
 	}
 }

lib/private/helper.php

@@ -196,10 +196,10 @@ class OC_Helper {
 	 * shows whether the user has an avatar
 	 * @param string $user username
 	 * @return bool avatar set or not
+	 * @deprecated 9.0.0 Use \OC::$server->getAvatarManager()->getAvatar($user)->exists();
 	**/
 	public static function userAvatarSet($user) {
-		$avatar = new \OC\Avatar($user);
+		return \OC::$server->getAvatarManager()->getAvatar($user)->exists();
-		return $avatar->exists();
 	}
 
 	/**

lib/private/server.php

@@ -291,8 +291,12 @@ class Server extends SimpleContainer implements IServerContainer {
 				$c->getConfig()
 			);
 		});
-		$this->registerService('AvatarManager', function ($c) {
+		$this->registerService('AvatarManager', function (Server $c) {
-			return new AvatarManager();
+			return new AvatarManager(
+				$c->getUserManager(),
+				$c->getRootFolder(),
+				$c->getL10N('lib')
+			);
 		});
 		$this->registerService('Logger', function (Server $c) {
 			$logClass = $c->query('AllConfig')->getSystemValue('log_type', 'owncloud');

tests/lib/avatar.php

@@ -1,94 +0,0 @@
-<?php
-
-/**
- * Copyright (c) 2013 Christopher Schäpers <christopher@schaepers.it>
- * This file is licensed under the Affero General Public License version 3 or
- * later.
- * See the COPYING-README file.
- */
-
-use OC\Avatar;
-
-/**
- * Class Test_Avatar
- *
- * @group DB
- */
-class Test_Avatar extends \Test\TestCase {
-	private static $trashBinStatus;
-
-	/** @var  @var string */
-	private $user;
-
-	protected function setUp() {
-		parent::setUp();
-
-		$this->user = $this->getUniqueID();
-		$storage = new \OC\Files\Storage\Temporary(array());
-		\OC\Files\Filesystem::mount($storage, array(), '/' . $this->user . '/');
-	}
-
-	public static function setUpBeforeClass() {
-		self::$trashBinStatus = \OC_App::isEnabled('files_trashbin');
-		\OC_App::disable('files_trashbin');
-	}
-
-	public static function tearDownAfterClass() {
-		if (self::$trashBinStatus) {
-			\OC_App::enable('files_trashbin');
-		}
-	}
-
-	/**
-	 * @return array
-	 */
-	public function traversalProvider() {
-		return [
-			['Pot\..\entiallyDangerousUsername'],
-			['Pot/..\entiallyDangerousUsername'],
-			['PotentiallyDangerousUsername/..'],
-			['PotentiallyDangerousUsername\../'],
-			['/../PotentiallyDangerousUsername'],
-		];
-	}
-
-	/**
-	 * @dataProvider traversalProvider
-	 * @expectedException \Exception
-	 * @expectedExceptionMessage Username may not contain slashes
-	 * @param string $dangerousUsername
-	 */
-	public function testAvatarTraversal($dangerousUsername) {
-		new Avatar($dangerousUsername);
-	}
-
-	public function testAvatar() {
-
-		$avatar = new Avatar($this->user);
-
-		$this->assertEquals(false, $avatar->get());
-
-		$expected = new OC_Image(\OC::$SERVERROOT . '/tests/data/testavatar.png');
-		$expected->resize(64);
-		$avatar->set($expected->data());
-		$this->assertEquals($expected->data(), $avatar->get()->data());
-
-		$avatar->remove();
-		$this->assertEquals(false, $avatar->get());
-	}
-
-	public function testAvatarApi() {
-		$avatarManager = \OC::$server->getAvatarManager();
-		$avatar = $avatarManager->getAvatar($this->user);
-
-		$this->assertEquals(false, $avatar->get());
-
-		$expected = new OC_Image(\OC::$SERVERROOT . '/tests/data/testavatar.png');
-		$expected->resize(64);
-		$avatar->set($expected->data());
-		$this->assertEquals($expected->data(), $avatar->get()->data());
-
-		$avatar->remove();
-		$this->assertEquals(false, $avatar->get());
-	}
-}

tests/lib/avatarmanagertest.php

@@ -0,0 +1,76 @@
+<?php
+/**
+ * @author Roeland Jago Douma <rullzer@owncloud.com>
+ *
+ * @copyright Copyright (c) 2015, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * 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, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+use OC\AvatarManager;
+use OCP\Files\IRootFolder;
+use OCP\IUserManager;
+
+class AvatarManagerTest extends \Test\TestCase {
+	/** @var  IRootFolder */
+	private $rootFolder;
+
+	/** @var  AvatarManager */
+	private $avatarManager;
+
+	/** @var  IUserManager */
+	private $userManager;
+
+	public function setUp() {
+		parent::setUp();
+
+		$this->rootFolder = $this->getMock('\OCP\Files\IRootFolder');
+		$this->userManager = $this->getMock('\OCP\IUserManager');
+		$l = $this->getMock('\OCP\IL10N');
+		$l->method('t')->will($this->returnArgument(0));
+		$this->avatarManager = new \OC\AvatarManager(
+				$this->userManager,
+				$this->rootFolder,
+				$l);;
+	}
+
+	/**
+	 * @expectedException Exception
+	 * @expectedExceptionMessage user does not exist
+	 */
+	public function testGetAvatarInvalidUser() {
+		$this->avatarManager->getAvatar('invalidUser');
+	}
+
+	public function testGetAvatarValidUser() {
+		$this->userManager->expects($this->once())
+			->method('userExists')
+			->with('validUser')
+			->willReturn(true);
+
+		$folder = $this->getMock('\OCP\Files\Folder');
+		$this->rootFolder->expects($this->once())
+			->method('getUserFolder')
+			->with('validUser')
+			->willReturn($folder);
+
+		$folder->expects($this->once())
+			->method('getParent')
+			->will($this->returnSelf());
+
+		$this->avatarManager->getAvatar('validUser');
+
+	}
+
+}

tests/lib/avatartest.php

@@ -0,0 +1,122 @@
+<?php
+
+/**
+ * Copyright (c) 2013 Christopher Schäpers <christopher@schaepers.it>
+ * This file is licensed under the Affero General Public License version 3 or
+ * later.
+ * See the COPYING-README file.
+ */
+
+use OC\Avatar;
+use OCP\Files\Folder;
+
+class AvatarTest extends \Test\TestCase {
+	/** @var  Folder */
+	private $folder;
+
+	/** @var  \OC\Avatar */
+	private $avatar;
+
+	public function setUp() {
+		parent::setUp();
+
+		$this->folder = $this->getMock('\OCP\Files\Folder');
+		$l = $this->getMock('\OCP\IL10N');
+		$l->method('t')->will($this->returnArgument(0));
+		$this->avatar = new \OC\Avatar($this->folder, $l);
+
+	}
+
+	public function testGetNoAvatar() {
+		$this->assertEquals(false, $this->avatar->get());
+	}
+
+	public function testGetAvatarSizeMatch() {
+		$this->folder->method('nodeExists')
+			->will($this->returnValueMap([
+				['avatar.jpg', true],
+				['avatar.128.jpg', true],
+			]));
+
+		$expected = new OC_Image(\OC::$SERVERROOT . '/tests/data/testavatar.png');
+
+		$file = $this->getMock('\OCP\Files\File');
+		$file->method('getContent')->willReturn($expected->data());
+		$this->folder->method('get')->with('avatar.128.jpg')->willReturn($file);
+
+		$this->assertEquals($expected->data(), $this->avatar->get(128)->data());
+	}
+
+	public function testGetAvatarNoSizeMatch() {
+		$this->folder->method('nodeExists')
+			->will($this->returnValueMap([
+				['avatar.png', true],
+				['avatar.32.png', false],
+			]));
+
+		$expected = new OC_Image(\OC::$SERVERROOT . '/tests/data/testavatar.png');
+		$expected2 = new OC_Image(\OC::$SERVERROOT . '/tests/data/testavatar.png');
+		$expected2->resize(32);
+
+		$file = $this->getMock('\OCP\Files\File');
+		$file->method('getContent')->willReturn($expected->data());
+		$this->folder->method('get')->with('avatar.png')->willReturn($file);
+
+		$newFile = $this->getMock('\OCP\Files\File');
+		$newFile->expects($this->once())
+			->method('putContent')
+			->with($expected2->data());
+		$this->folder->expects($this->once())
+			->method('newFile')
+			->with('avatar.32.png')
+			->willReturn($newFile);
+
+		$this->assertEquals($expected2->data(), $this->avatar->get(32)->data());
+	}
+
+	public function testExistsNo() {
+		$this->assertFalse($this->avatar->exists());
+	}
+
+	public function testExiststJPG() {
+		$this->folder->method('nodeExists')
+			->will($this->returnValueMap([
+				['avatar.jpg', true],
+				['avatar.png', false],
+			]));
+		$this->assertTrue($this->avatar->exists());
+	}
+
+	public function testExistsPNG() {
+		$this->folder->method('nodeExists')
+			->will($this->returnValueMap([
+				['avatar.jpg', false],
+				['avatar.png', true],
+			]));
+		$this->assertTrue($this->avatar->exists());
+	}
+
+	public function testSetAvatar() {
+		$oldFile = $this->getMock('\OCP\Files\File');
+		$this->folder->method('get')
+			->will($this->returnValueMap([
+				['avatar.jpg', $oldFile],
+				['avatar.png', $oldFile],
+			]));
+		$oldFile->expects($this->exactly(2))->method('delete');
+
+		$newFile = $this->getMock('\OCP\Files\File');
+		$this->folder->expects($this->once())
+			->method('newFile')
+			->with('avatar.png')
+			->willReturn($newFile);
+
+		$image = new OC_Image(\OC::$SERVERROOT . '/tests/data/testavatar.png');
+		$newFile->expects($this->once())
+			->method('putContent')
+			->with($image->data());
+
+		$this->avatar->set($image->data());
+	}
+
+}