Merge pull request #15826 from owncloud/issue-15804-occ-user-delete-exception

Issue 15804 occ user delete exception

8 files changed, 262 insertions, 20 deletions

diff --git a/core/command/user/add.php b/core/command/user/add.php
index 60c70bf13dd..c52ec4d0090 100644
--- a/core/command/user/add.php
+++ b/core/command/user/add.php
@@ -119,7 +119,7 @@ class Add extends Command {
 		);
 
 		if ($user instanceof IUser) {
-			$output->writeln('The user "' . $user->getUID() . '" was created successfully');
+			$output->writeln('<info>The user "' . $user->getUID() . '" was created successfully</info>');
 		} else {
 			$output->writeln('<error>An error occurred while creating the user</error>');
 			return 1;
diff --git a/core/command/user/delete.php b/core/command/user/delete.php
index 53952ceb9e1..8cac03157eb 100644
--- a/core/command/user/delete.php
+++ b/core/command/user/delete.php
@@ -22,19 +22,20 @@
 
 namespace OC\Core\Command\User;
 
+use OCP\IUserManager;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Input\InputArgument;
 
 class Delete extends Command {
-	/** @var \OC\User\Manager */
+	/** @var IUserManager */
 	protected $userManager;
 
 	/**
-	 * @param \OC\User\Manager $userManager
+	 * @param IUserManager $userManager
 	 */
-	public function __construct(\OC\User\Manager $userManager) {
+	public function __construct(IUserManager $userManager) {
 		$this->userManager = $userManager;
 		parent::__construct();
 	}
@@ -51,11 +52,17 @@ class Delete extends Command {
 	}
 
 	protected function execute(InputInterface $input, OutputInterface $output) {
-		$wasSuccessful = $this->userManager->get($input->getArgument('uid'))->delete();
-		if($wasSuccessful === true) {
-			$output->writeln('The specified user was deleted');
+		$user = $this->userManager->get($input->getArgument('uid'));
+		if (is_null($user)) {
+			$output->writeln('<error>User does not exist</error>');
 			return;
 		}
+
+		if ($user->delete()) {
+			$output->writeln('<info>The specified user was deleted</info>');
+			return;
+		}
+
 		$output->writeln('<error>The specified could not be deleted. Please check the logs.</error>');
 	}
 }
diff --git a/core/command/user/lastseen.php b/core/command/user/lastseen.php
index 308b3c67ddb..92fcb1d449b 100644
--- a/core/command/user/lastseen.php
+++ b/core/command/user/lastseen.php
@@ -22,12 +22,24 @@
 
 namespace OC\Core\Command\User;
 
+use OCP\IUserManager;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Input\InputArgument;
 
 class LastSeen extends Command {
+	/** @var IUserManager */
+	protected $userManager;
+
+	/**
+	 * @param IUserManager $userManager
+	 */
+	public function __construct(IUserManager $userManager) {
+		$this->userManager = $userManager;
+		parent::__construct();
+	}
+
 	protected function configure() {
 		$this
 			->setName('user:lastseen')
@@ -40,10 +52,9 @@ class LastSeen extends Command {
 	}
 
 	protected function execute(InputInterface $input, OutputInterface $output) {
-		$userManager = \OC::$server->getUserManager();
-		$user = $userManager->get($input->getArgument('uid'));
+		$user = $this->userManager->get($input->getArgument('uid'));
 		if(is_null($user)) {
-			$output->writeln('User does not exist');
+			$output->writeln('<error>User does not exist</error>');
 			return;
 		}
 
diff --git a/core/command/user/report.php b/core/command/user/report.php
index 13bb5df69f8..5d89c0ae549 100644
--- a/core/command/user/report.php
+++ b/core/command/user/report.php
@@ -23,11 +23,23 @@
 
 namespace OC\Core\Command\User;
 
+use OCP\IUserManager;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 
 class Report extends Command {
+	/** @var IUserManager */
+	protected $userManager;
+
+	/**
+	 * @param IUserManager $userManager
+	 */
+	public function __construct(IUserManager $userManager) {
+		$this->userManager = $userManager;
+		parent::__construct();
+	}
+
 	protected function configure() {
 		$this
 			->setName('user:report')
@@ -35,6 +47,7 @@ class Report extends Command {
 	}
 
 	protected function execute(InputInterface $input, OutputInterface $output) {
+		/** @var \Symfony\Component\Console\Helper\TableHelper $table */
 		$table = $this->getHelperSet()->get('table');
 		$table->setHeaders(array('User Report', ''));
 		$userCountArray = $this->countUsers();
@@ -61,8 +74,7 @@ class Report extends Command {
 	}
 
 	private function countUsers() {
-		$userManager = \OC::$server->getUserManager();
-		return $userManager->countUsers();
+		return $this->userManager->countUsers();
 	}
 
 	private function countUserDirectories() {
diff --git a/core/command/user/resetpassword.php b/core/command/user/resetpassword.php
index 3e16c8f79a5..7c405592114 100644
--- a/core/command/user/resetpassword.php
+++ b/core/command/user/resetpassword.php
@@ -23,6 +23,7 @@
 
 namespace OC\Core\Command\User;
 
+use OCP\IUserManager;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputArgument;
@@ -31,10 +32,10 @@ use Symfony\Component\Console\Output\OutputInterface;
 
 class ResetPassword extends Command {
 
-	/** @var \OC\User\Manager */
+	/** @var IUserManager */
 	protected $userManager;
 
-	public function __construct(\OC\User\Manager $userManager) {
+	public function __construct(IUserManager $userManager) {
 		$this->userManager = $userManager;
 		parent::__construct();
 	}
@@ -60,10 +61,10 @@ class ResetPassword extends Command {
 	protected function execute(InputInterface $input, OutputInterface $output) {
 		$username = $input->getArgument('user');
 
-		/** @var $user \OC\User\User */
+		/** @var $user \OCP\IUser */
 		$user = $this->userManager->get($username);
 		if (is_null($user)) {
-			$output->writeln("<error>There is no user called " . $username . "</error>");
+			$output->writeln('<error>User does not exist</error>');
 			return 1;
 		}
 
diff --git a/core/register_command.php b/core/register_command.php
index 701fb10d1ba..75c0245a924 100644
--- a/core/register_command.php
+++ b/core/register_command.php
@@ -42,11 +42,11 @@ if (\OC::$server->getConfig()->getSystemValue('installed', false)) {
 	$application->add(new OC\Core\Command\App\Enable());
 	$application->add(new OC\Core\Command\App\ListApps());
 	$application->add(new OC\Core\Command\Maintenance\Repair($repair, \OC::$server->getConfig()));
-	$application->add(new OC\Core\Command\User\Report());
-	$application->add(new OC\Core\Command\User\ResetPassword(\OC::$server->getUserManager()));
-	$application->add(new OC\Core\Command\User\LastSeen());
-	$application->add(new OC\Core\Command\User\Delete(\OC::$server->getUserManager()));
 	$application->add(new OC\Core\Command\User\Add(\OC::$server->getUserManager(), \OC::$server->getGroupManager()));
+	$application->add(new OC\Core\Command\User\Delete(\OC::$server->getUserManager()));
+	$application->add(new OC\Core\Command\User\LastSeen(\OC::$server->getUserManager()));
+	$application->add(new OC\Core\Command\User\Report(\OC::$server->getUserManager()));
+	$application->add(new OC\Core\Command\User\ResetPassword(\OC::$server->getUserManager()));
 	$application->add(new OC\Core\Command\Background\Cron(\OC::$server->getConfig()));
 	$application->add(new OC\Core\Command\Background\WebCron(\OC::$server->getConfig()));
 	$application->add(new OC\Core\Command\Background\Ajax(\OC::$server->getConfig()));
diff --git a/tests/core/command/user/deletetest.php b/tests/core/command/user/deletetest.php
new file mode 100644
index 00000000000..bfcf031b719
--- /dev/null
+++ b/tests/core/command/user/deletetest.php
@@ -0,0 +1,106 @@
+<?php
+/**
+ * @author Joas Schilling <nickvergessen@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/>
+ *
+ */
+
+namespace Tests\Core\Command\User;
+
+
+use OC\Core\Command\User\Delete;
+use Test\TestCase;
+
+class DeleteTest extends TestCase {
+	/** @var \PHPUnit_Framework_MockObject_MockObject */
+	protected $userManager;
+	/** @var \PHPUnit_Framework_MockObject_MockObject */
+	protected $consoleInput;
+	/** @var \PHPUnit_Framework_MockObject_MockObject */
+	protected $consoleOutput;
+
+	/** @var \Symfony\Component\Console\Command\Command */
+	protected $command;
+
+	protected function setUp() {
+		parent::setUp();
+
+		$userManager = $this->userManager = $this->getMockBuilder('OCP\IUserManager')
+			->disableOriginalConstructor()
+			->getMock();
+		$this->consoleInput = $this->getMock('Symfony\Component\Console\Input\InputInterface');
+		$this->consoleOutput = $this->getMock('Symfony\Component\Console\Output\OutputInterface');
+
+		/** @var \OCP\IUserManager $userManager */
+		$this->command = new Delete($userManager);
+	}
+
+
+	public function validUserLastSeen() {
+		return [
+			[true, 'The specified user was deleted'],
+			[false, 'The specified could not be deleted'],
+		];
+	}
+
+	/**
+	 * @dataProvider validUserLastSeen
+	 *
+	 * @param bool $deleteSuccess
+	 * @param string $expectedString
+	 */
+	public function testValidUser($deleteSuccess, $expectedString) {
+		$user = $this->getMock('OCP\IUser');
+		$user->expects($this->once())
+			->method('delete')
+			->willReturn($deleteSuccess);
+
+		$this->userManager->expects($this->once())
+			->method('get')
+			->with('user')
+			->willReturn($user);
+
+		$this->consoleInput->expects($this->once())
+			->method('getArgument')
+			->with('uid')
+			->willReturn('user');
+
+		$this->consoleOutput->expects($this->once())
+			->method('writeln')
+			->with($this->stringContains($expectedString));
+
+		\Test_Helper::invokePrivate($this->command, 'execute', [$this->consoleInput, $this->consoleOutput]);
+	}
+
+	public function testInvalidUser() {
+		$this->userManager->expects($this->once())
+			->method('get')
+			->with('user')
+			->willReturn(null);
+
+		$this->consoleInput->expects($this->once())
+			->method('getArgument')
+			->with('uid')
+			->willReturn('user');
+
+		$this->consoleOutput->expects($this->once())
+			->method('writeln')
+			->with($this->stringContains('User does not exist'));
+
+		\Test_Helper::invokePrivate($this->command, 'execute', [$this->consoleInput, $this->consoleOutput]);
+	}
+}
diff --git a/tests/core/command/user/lastseentest.php b/tests/core/command/user/lastseentest.php
new file mode 100644
index 00000000000..7eda6fb27ed
--- /dev/null
+++ b/tests/core/command/user/lastseentest.php
@@ -0,0 +1,105 @@
+<?php
+/**
+ * @author Joas Schilling <nickvergessen@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/>
+ *
+ */
+
+namespace Tests\Core\Command\User;
+
+
+use OC\Core\Command\User\LastSeen;
+use Test\TestCase;
+
+class LastSeenTest extends TestCase {
+	/** @var \PHPUnit_Framework_MockObject_MockObject */
+	protected $userManager;
+	/** @var \PHPUnit_Framework_MockObject_MockObject */
+	protected $consoleInput;
+	/** @var \PHPUnit_Framework_MockObject_MockObject */
+	protected $consoleOutput;
+
+	/** @var \Symfony\Component\Console\Command\Command */
+	protected $command;
+
+	protected function setUp() {
+		parent::setUp();
+
+		$userManager = $this->userManager = $this->getMockBuilder('OCP\IUserManager')
+			->disableOriginalConstructor()
+			->getMock();
+		$this->consoleInput = $this->getMock('Symfony\Component\Console\Input\InputInterface');
+		$this->consoleOutput = $this->getMock('Symfony\Component\Console\Output\OutputInterface');
+
+		/** @var \OCP\IUserManager $userManager */
+		$this->command = new LastSeen($userManager);
+	}
+
+	public function validUserLastSeen() {
+		return [
+			[0, 'never logged in'],
+			[time(), 'last login'],
+		];
+	}
+
+	/**
+	 * @dataProvider validUserLastSeen
+	 *
+	 * @param int $lastSeen
+	 * @param string $expectedString
+	 */
+	public function testValidUser($lastSeen, $expectedString) {
+		$user = $this->getMock('OCP\IUser');
+		$user->expects($this->once())
+			->method('getLastLogin')
+			->willReturn($lastSeen);
+
+		$this->userManager->expects($this->once())
+			->method('get')
+			->with('user')
+			->willReturn($user);
+
+		$this->consoleInput->expects($this->once())
+			->method('getArgument')
+			->with('uid')
+			->willReturn('user');
+
+		$this->consoleOutput->expects($this->once())
+			->method('writeln')
+			->with($this->stringContains($expectedString));
+
+		\Test_Helper::invokePrivate($this->command, 'execute', [$this->consoleInput, $this->consoleOutput]);
+	}
+
+	public function testInvalidUser() {
+		$this->userManager->expects($this->once())
+			->method('get')
+			->with('user')
+			->willReturn(null);
+
+		$this->consoleInput->expects($this->once())
+			->method('getArgument')
+			->with('uid')
+			->willReturn('user');
+
+		$this->consoleOutput->expects($this->once())
+			->method('writeln')
+			->with($this->stringContains('User does not exist'));
+
+		\Test_Helper::invokePrivate($this->command, 'execute', [$this->consoleInput, $this->consoleOutput]);
+	}
+}