Do not change the old behavior of user creation

core/Command/User/Add.php

@@ -32,6 +32,7 @@
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
+use Symfony\Component\Console\Output\ConsoleOutputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Question\Question;
@@ -91,7 +92,17 @@ protected function configure() {
 			);
 	}
 
+	/**
+	 * @param InputInterface $input
+	 * @param OutputInterface $output
+	 * @return int
+	 */
 	protected function execute(InputInterface $input, OutputInterface $output) {
+		$stdErr = $output;
+		if ($output instanceof ConsoleOutputInterface) {
+			// If it's available, get stdErr output
+			$stdErr = $output->getErrorOutput();
+		}
 		$uid = $input->getArgument('uid');
 		$email = $input->getOption('email');
 		$displayName = $input->getOption('display-name');
@@ -134,20 +145,20 @@ protected function execute(InputInterface $input, OutputInterface $output) {
 		try {
 			$user = $this->createUserService->createUser(['username' => $uid, 'password' => $password, 'email' => $email]);
 		} catch (InvalidEmailException $e) {
-			$output->writeln('<error>Invalid email address supplied</error>');
+			$stdErr->writeln('<error>Invalid email address supplied</error>');
 			return 1;
 		} catch (CannotCreateUserException $e) {
-			$output->writeln("<error>" . $e->getMessage() .  "</error>");
+			$stdErr->writeln("<error>" . $e->getMessage() .  "</error>");
 			return 1;
 		} catch (UserAlreadyExistsException $e) {
-			$output->writeln("<error>" . $e->getMessage() .  "</error>");
+			$stdErr->writeln("<error>" . $e->getMessage() .  "</error>");
 			return 1;
 		}
 
 		if ($user instanceof IUser) {
 			$output->writeln('<info>The user "' . $user->getUID() . '" was created successfully</info>');
 		} else {
-			$output->writeln('<error>An error occurred while creating the user</error>');
+			$stdErr->writeln('<error>An error occurred while creating the user</error>');
 			return 1;
 		}
 

lib/private/User/Service/CreateUserService.php

@@ -92,7 +92,7 @@ public function __construct(IUserSession $userSession, IGroupManager $groupManag
 	 * @throws UserAlreadyExistsException
 	 */
 	public function createUser($arguments) {
-		$username = $password = $email = '';
+		$password = $email = '';
 		if (\array_key_exists('username', $arguments)) {
 			$username = $arguments['username'];
 		} else {
@@ -125,7 +125,7 @@ public function createUser($arguments) {
 			}
 			$user = $this->userManager->createUser($username, $password);
 		} catch (\Exception $exception) {
-			throw new CannotCreateUserException("Unable to create user due to exception: {$exception->getMessage()}");
+			throw new CannotCreateUserException($exception->getMessage());
 		}
 
 		if ($user === false) {
@@ -160,13 +160,13 @@ public function createUser($arguments) {
 	 *  $groups = ['group1', 'group2']
 	 *
 	 * This function returns the list of group(s) which failed to add user to the group(s)
+	 * If the user is already in the group, function assumes the add operation successful.
 	 *
 	 * @param IUser $user
 	 * @param array $groups list of group names, example ['group1', 'group2']
-	 * @param bool $checkInGroup
 	 * @return array Returns an array of groups which failed to add user
 	 */
-	public function addUserToGroups(IUser $user, array $groups, $checkInGroup = true) {
+	public function addUserToGroups(IUser $user, array $groups) {
 		$failedToAdd = [];
 
 		if (\is_array($groups) && \count($groups) > 0) {
@@ -177,7 +177,7 @@ public function addUserToGroups(IUser $user, array $groups, $checkInGroup = true
 					$groupObject = $this->groupManager->createGroup($groupName);
 				}
 				$groupObject->addUser($user);
-				if ($checkInGroup && !$this->groupManager->isInGroup($user->getUID(), $groupName)) {
+				if (!$this->groupManager->isInGroup($user->getUID(), $groupName)) {
 					$failedToAdd[] = $groupName;
 				} else {
 					$this->logger->info('Added userid ' . $user->getUID() . ' to group ' . $groupName, ['app' => 'ocs_api']);
@@ -190,7 +190,7 @@ public function addUserToGroups(IUser $user, array $groups, $checkInGroup = true
 	/**
 	 * Check if the user exist
 	 *
-	 * This function is for convinience. It helps to use this service to check if user exist,
+	 * This function is for convenience. It helps to use this service to check if user exist,
 	 * instead of adding dependency userManager. Kindly do not delete this method.
 	 *
 	 * @param string $uid

tests/lib/User/Service/CreateUserServiceTest.php

@@ -116,20 +116,22 @@ public function testAddUserToGroups() {
 	}
 
 	/**
-	 * @expectedExceptionMessage Invalid mail address
-	 * @expectedException  \OCP\User\Exceptions\InvalidEmailException
 	 */
 	public function testInvalidEmail() {
+		$this->expectException(\OCP\User\Exceptions\InvalidEmailException::class);
+		$this->expectExceptionMessage('Invalid mail address');
+
 		$this->mailer->method('validateMailAddress')
 			->willReturn(false);
 		$this->createUserService->createUser(['username' => 'foo', 'password' => '', 'email' => 'foo@bar']);
 	}
 
 	/**
-	 * @expectedExceptionMessage A user with that name already exists.
-	 * @expectedException  \OCP\User\Exceptions\UserAlreadyExistsException
 	 */
 	public function testAlreadyExistingUser() {
+		$this->expectException(\OCP\User\Exceptions\UserAlreadyExistsException::class);
+		$this->expectExceptionMessage('A user with that name already exists.');
+
 		$this->userSendMailService->method('validateEmailAddress')
 			->willReturn(true);
 
@@ -140,26 +142,28 @@ public function testAlreadyExistingUser() {
 	}
 
 	/**
-	 * @expectedExceptionMessage Unable to create user due to exception:
-	 * @expectedException  \OCP\User\Exceptions\CannotCreateUserException
 	 */
 	public function testUserCreateException() {
+		$this->expectException(\OCP\User\Exceptions\CannotCreateUserException::class);
+		$this->expectExceptionMessage('Exception Message');
+
 		$this->userSendMailService->method('validateEmailAddress')
 			->willReturn(true);
 
 		$this->userManager->method('userExists')
 			->willReturn(false);
 
 		$this->userManager->method('createUser')
-			->willThrowException(new \Exception());
+			->willThrowException(new \Exception("Exception Message"));
 		$this->createUserService->createUser(['username' => 'foo', 'password' => '', 'email' => '[email protected]']);
 	}
 
 	/**
-	 * @expectedExceptionMessage Unable to create user.
-	 * @expectedException  \OCP\User\Exceptions\CannotCreateUserException
 	 */
 	public function testUserCreateFailed() {
+		$this->expectException(\OCP\User\Exceptions\CannotCreateUserException::class);
+		$this->expectExceptionMessage('Unable to create user.');
+
 		$this->userSendMailService->method('validateEmailAddress')
 			->willReturn(true);
 

tests/lib/User/Service/UserSendMailServiceTest.php

@@ -99,10 +99,11 @@ public function testGenerateTokenAndSendMail() {
 	}
 
 	/**
-	 * @expectedException OCP\User\Exceptions\InvalidUserTokenException
-	 * @expectedExceptionMessage The token provided is invalid.
 	 */
 	public function testcheckPasswordSetInvalidToken() {
+		$this->expectException(\OCP\User\Exceptions\InvalidUserTokenException::class);
+		$this->expectExceptionMessage('The token provided is invalid.');
+
 		$user = $this->createMock(IUser::class);
 		$user->method('getUID')
 			->willReturn('foo');
@@ -113,10 +114,11 @@ public function testcheckPasswordSetInvalidToken() {
 	}
 
 	/**
-	 * @expectedException  \OCP\User\Exceptions\UserTokenExpiredException
-	 * @expectedExceptionMessage The token provided had expired.
 	 */
 	public function testCheckPasswordSetTokenExpired() {
+		$this->expectException(\OCP\User\Exceptions\UserTokenExpiredException::class);
+		$this->expectExceptionMessage('The token provided had expired.');
+
 		$user = $this->createMock(IUser::class);
 		$user->method('getUID')
 			->willReturn('foo');
@@ -133,10 +135,11 @@ public function testCheckPasswordSetTokenExpired() {
 	}
 
 	/**
-	 * @expectedException  \OCP\User\Exceptions\UserTokenMismatchException
-	 * @expectedExceptionMessage The token provided is invalid.
 	 */
 	public function testCheckPasswordSetTokenMismatch() {
+		$this->expectException(\OCP\User\Exceptions\UserTokenMismatchException::class);
+		$this->expectExceptionMessage('The token provided is invalid.');
+
 		$user = $this->createMock(IUser::class);
 		$user->method('getUID')
 			->willReturn('foo');
@@ -187,10 +190,11 @@ public function testSendNotificationMailFail() {
 	}
 
 	/**
-	 * @expectedException  \OCP\User\Exceptions\EmailSendFailedException
-	 * @expectedExceptionMessage Email could not be sent.
 	 */
 	public function testSendNotificationMailSendFail() {
+		$this->expectException(\OCP\User\Exceptions\EmailSendFailedException::class);
+		$this->expectExceptionMessage('Email could not be sent.');
+
 		$user = $this->createMock(IUser::class);
 		$user->method('getEMailAddress')
 			->willReturn('[email protected]');