Add hard return-type on User::getCredential() (#316)

ro0NL · web-flow · commit 673bf76cad08 · 2019-03-30T18:41:00.000+01:00
diff --git a/Credential/Anonymous.php b/Credential/Anonymous.php
@@ -15,14 +15,4 @@ public function __invoke(ChangeCredential $event): bool
     {
         throw new \BadMethodCallException('An anonymous credential cannot be changed.');
     }
-
-    public static function getUsernameField(): string
-    {
-        throw new \BadMethodCallException('An anonymous credential has no username field.');
-    }
-
-    public function getUsername(): string
-    {
-        throw new \BadMethodCallException('An anonymous credential has no username.');
-    }
 }
diff --git a/Credential/Credential.php b/Credential/Credential.php
@@ -12,8 +12,4 @@
 interface Credential
 {
     public function __invoke(ChangeCredential $event): bool;
-
-    public static function getUsernameField(): string;
-
-    public function getUsername(): string;
 }
diff --git a/Credential/Email.php b/Credential/Email.php
@@ -9,7 +9,7 @@
 /**
  * @author Roland Franssen <franssen.roland@gmail.com>
  */
-final class Email implements Credential
+final class Email implements UsernameCredential
 {
     use EmailAsUsername;
 
diff --git a/Credential/EmailPassword.php b/Credential/EmailPassword.php
@@ -9,7 +9,7 @@
 /**
  * @author Roland Franssen <franssen.roland@gmail.com>
  */
-final class EmailPassword implements PasswordProtectedCredential
+final class EmailPassword implements UsernameCredential, PasswordProtectedCredential
 {
     use EmailAsUsername;
     use PasswordProtection;
diff --git a/Credential/Nickname.php b/Credential/Nickname.php
@@ -9,7 +9,7 @@
 /**
  * @author Roland Franssen <franssen.roland@gmail.com>
  */
-final class Nickname implements Credential
+final class Nickname implements UsernameCredential
 {
     use NicknameAsUsername;
 
diff --git a/Credential/NicknamePassword.php b/Credential/NicknamePassword.php
@@ -9,7 +9,7 @@
 /**
  * @author Roland Franssen <franssen.roland@gmail.com>
  */
-final class NicknamePassword implements PasswordProtectedCredential
+final class NicknamePassword implements UsernameCredential, PasswordProtectedCredential
 {
     use NicknameAsUsername;
     use PasswordProtection;
diff --git a/Credential/PasswordProtectedCredential.php b/Credential/PasswordProtectedCredential.php
@@ -7,12 +7,14 @@
 use MsgPhp\User\Password\PasswordAlgorithm;
 
 /**
- * Represents a password protected credential. The password is usually a *hashed* value (thus secret).
+ * Represents a user credential that is password protected. The password is usually a *hashed* value (thus secret).
  *
  * @author Roland Franssen <franssen.roland@gmail.com>
  */
 interface PasswordProtectedCredential extends Credential
 {
+    public static function getPasswordField(): string;
+
     public function getPassword(): string;
 
     public function getPasswordAlgorithm(): ?PasswordAlgorithm;
diff --git a/Credential/PasswordProtection.php b/Credential/PasswordProtection.php
@@ -16,6 +16,11 @@ trait PasswordProtection
      */
     private $password;
 
+    public static function getPasswordField(): string
+    {
+        return 'password';
+    }
+
     public function getPassword(): string
     {
         return $this->password;
diff --git a/Credential/Token.php b/Credential/Token.php
@@ -9,7 +9,7 @@
 /**
  * @author Roland Franssen <franssen.roland@gmail.com>
  */
-final class Token implements Credential
+final class Token implements UsernameCredential
 {
     /**
      * @var string
diff --git a/Credential/UsernameCredential.php b/Credential/UsernameCredential.php
@@ -0,0 +1,17 @@
+<?php
+
+declare(strict_types=1);
+
+namespace MsgPhp\User\Credential;
+
+/**
+ * Represents a user credential that is bound to a known username.
+ *
+ * @author Roland Franssen <franssen.roland@gmail.com>
+ */
+interface UsernameCredential extends Credential
+{
+    public static function getUsernameField(): string;
+
+    public function getUsername(): string;
+}
diff --git a/Infrastructure/Console/Command/AddUserRoleCommand.php b/Infrastructure/Console/Command/AddUserRoleCommand.php
@@ -49,7 +49,7 @@ public function onMessageReceived($message): void
             $this->io->success('Created role '.$message->role->getName());
         }
         if ($message instanceof UserRoleAdded) {
-            $this->io->success(sprintf('Added role %s to user %s', $message->userRole->getRoleName(), $message->userRole->getUser()->getCredential()->getUsername()));
+            $this->io->success('Added role '.$message->userRole->getRoleName().' to user '.self::getUsername($message->userRole->getUser()));
         }
     }
 
@@ -75,7 +75,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
                 throw new \UnexpectedValueException('Role name must be a string.');
             }
 
-            if (!$input->isInteractive() || !$this->io->confirm(sprintf('Role "%s" does not exists. Create it now?', $roleName))) {
+            if (!$input->isInteractive() || !$this->io->confirm('Role <comment>'.$roleName.'</comment> does not exists. Create it now?')) {
                 throw $e;
             }
 
@@ -86,7 +86,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
             ]), $this->io);
 
             if (0 !== $result) {
-                throw new \RuntimeException(sprintf('Cannot create role "%s". Something went wrong.', $roleName));
+                throw new \RuntimeException('Cannot create role "'.$roleName.'". Something went wrong.');
             }
 
             $role = $this->getRole($input, $this->io);
diff --git a/Infrastructure/Console/Command/ChangeUserCredentialCommand.php b/Infrastructure/Console/Command/ChangeUserCredentialCommand.php
@@ -48,7 +48,7 @@ public function __construct(DomainObjectFactory $factory, DomainMessageBus $bus,
     public function onMessageReceived($message): void
     {
         if ($message instanceof UserCredentialChanged) {
-            $this->io->success('Changed user credential for '.$message->user->getCredential()->getUsername());
+            $this->io->success('Changed user credential for '.self::getUsername($message->user));
 
             $rows = [];
             $changes = array_diff((array) $message->newCredential, $oldValues = (array) $message->oldCredential);
diff --git a/Infrastructure/Console/Command/ConfirmUserCommand.php b/Infrastructure/Console/Command/ConfirmUserCommand.php
@@ -26,7 +26,7 @@ final class ConfirmUserCommand extends UserCommand
     public function onMessageReceived($message): void
     {
         if ($message instanceof UserConfirmed) {
-            $this->io->success('Confirmed user '.$message->user->getCredential()->getUsername());
+            $this->io->success('Confirmed user '.self::getUsername($message->user));
         }
     }
 
diff --git a/Infrastructure/Console/Command/CreateUserCommand.php b/Infrastructure/Console/Command/CreateUserCommand.php
@@ -48,7 +48,7 @@ public function __construct(DomainObjectFactory $factory, DomainMessageBus $bus,
     public function onMessageReceived($message): void
     {
         if ($message instanceof UserCreated) {
-            $this->io->success('Created user '.$message->user->getCredential()->getUsername());
+            $this->io->success('Created user '.UserCommand::getUsername($message->user));
         }
     }
 
diff --git a/Infrastructure/Console/Command/DeleteRoleCommand.php b/Infrastructure/Console/Command/DeleteRoleCommand.php
@@ -42,12 +42,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         $this->io = new SymfonyStyle($input, $output);
         $roleName = $this->getRole($input, $this->io)->getName();
 
-        if ($input->isInteractive()) {
-            $this->io->note('Deleting role '.$roleName);
-
-            if (!$this->io->confirm('Are you sure?')) {
-                return 0;
-            }
+        if ($input->isInteractive() && !$this->io->confirm('Are you sure you want to delete <comment>'.$roleName.'</comment>?')) {
+            return 0;
         }
 
         $this->dispatch(DeleteRole::class, compact('roleName'));
diff --git a/Infrastructure/Console/Command/DeleteUserCommand.php b/Infrastructure/Console/Command/DeleteUserCommand.php
@@ -26,7 +26,7 @@ final class DeleteUserCommand extends UserCommand
     public function onMessageReceived($message): void
     {
         if ($message instanceof UserDeleted) {
-            $this->io->success('Deleted user '.$message->user->getCredential()->getUsername());
+            $this->io->success('Deleted user '.self::getUsername($message->user));
         }
     }
 
@@ -43,12 +43,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         $user = $this->getUser($input, $this->io);
         $userId = $user->getId();
 
-        if ($input->isInteractive()) {
-            $this->io->note('Deleting user '.$user->getCredential()->getUsername().' with ID '.$userId->toString());
-
-            if (!$this->io->confirm('Are you sure?')) {
-                return 0;
-            }
+        if ($input->isInteractive() && !$this->io->confirm('Are you sure you want to delete <comment>'.self::getUsername($user).'</comment>?')) {
+            return 0;
         }
 
         $this->dispatch(DeleteUser::class, compact('userId'));
diff --git a/Infrastructure/Console/Command/DeleteUserRoleCommand.php b/Infrastructure/Console/Command/DeleteUserRoleCommand.php
@@ -26,7 +26,7 @@ final class DeleteUserRoleCommand extends UserRoleCommand
     public function onMessageReceived($message): void
     {
         if ($message instanceof UserRoleDeleted) {
-            $this->io->success(sprintf('Deleted role %s from user %s', $message->userRole->getRoleName(), $message->userRole->getUser()->getCredential()->getUsername()));
+            $this->io->success('Deleted role '.$message->userRole->getRoleName().' from user '.self::getUsername($message->userRole->getUser()));
         }
     }
 
diff --git a/Infrastructure/Console/Command/DisableUserCommand.php b/Infrastructure/Console/Command/DisableUserCommand.php
@@ -26,7 +26,7 @@ final class DisableUserCommand extends UserCommand
     public function onMessageReceived($message): void
     {
         if ($message instanceof UserDisabled) {
-            $this->io->success('Disabled user '.$message->user->getCredential()->getUsername());
+            $this->io->success('Disabled user '.self::getUsername($message->user));
         }
     }
 
diff --git a/Infrastructure/Console/Command/EnableUserCommand.php b/Infrastructure/Console/Command/EnableUserCommand.php
@@ -26,7 +26,7 @@ final class EnableUserCommand extends UserCommand
     public function onMessageReceived($message): void
     {
         if ($message instanceof UserEnabled) {
-            $this->io->success('Enabled user '.$message->user->getCredential()->getUsername());
+            $this->io->success('Enabled user '.self::getUsername($message->user));
         }
     }
 
diff --git a/Infrastructure/Console/Command/SynchronizeUsernamesCommand.php b/Infrastructure/Console/Command/SynchronizeUsernamesCommand.php
@@ -60,7 +60,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
                 $this->repository->save($username);
             }
 
-            $rows[] = sprintf('Added username <info>%s</info> for user <info>%s</info>', $usernameValue, $username->getUser()->getId()->toString());
+            $rows[] = 'Added username <info>'.$usernameValue.'</info> for user <info>'.$username->getUser()->getId()->toString().'</info>';
             ++$added;
         }
 
@@ -69,7 +69,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
                 $this->repository->delete($unknownUsername);
             }
 
-            $rows[] = sprintf('Deleted username <info>%s</info> from user <info>%s</info>', (string) $unknownUsername, $unknownUsername->getUser()->getId()->toString());
+            $rows[] = 'Deleted username <info>'.$unknownUsername.'</info> from user <info>'.$unknownUsername->getUser()->getId()->toString().'</info>';
             ++$deleted;
         }
 
diff --git a/Infrastructure/Console/Command/UserCommand.php b/Infrastructure/Console/Command/UserCommand.php
@@ -8,6 +8,7 @@
 use MsgPhp\Domain\Message\DomainMessageBus;
 use MsgPhp\Domain\Message\MessageDispatchingTrait;
 use MsgPhp\Domain\Message\MessageReceiving;
+use MsgPhp\User\Credential\UsernameCredential;
 use MsgPhp\User\Repository\UserRepository;
 use MsgPhp\User\User;
 use MsgPhp\User\UserId;
@@ -45,6 +46,13 @@ public function onMessageReceived($message): void
     {
     }
 
+    final public static function getUsername(User $user): string
+    {
+        $credential = $user->getCredential();
+
+        return $credential instanceof UsernameCredential ? $credential->getUsername() : $user->getId()->toString();
+    }
+
     protected function configure(): void
     {
         $this
diff --git a/Infrastructure/Security/DataCollector.php b/Infrastructure/Security/DataCollector.php
@@ -5,6 +5,7 @@
 namespace MsgPhp\User\Infrastructure\Security;
 
 use MsgPhp\Domain\Exception\EntityNotFoundException;
+use MsgPhp\User\Credential\UsernameCredential;
 use MsgPhp\User\Repository\UserRepository;
 use MsgPhp\User\UserId;
 use Symfony\Bundle\SecurityBundle\DataCollector\SecurityDataCollector as BaseDataCollector;
@@ -80,9 +81,13 @@ private function getUsername(UserId $id): string
         }
 
         try {
-            return $this->repository->find($id)->getCredential()->getUsername();
+            $credential = $this->repository->find($id)->getCredential();
+            if ($credential instanceof UsernameCredential) {
+                return $credential->getUsername();
+            }
         } catch (EntityNotFoundException $e) {
-            return $id->toString();
         }
+
+        return $id->toString();
     }
 }
diff --git a/Model/AbstractCredential.php b/Model/AbstractCredential.php
@@ -17,12 +17,13 @@ trait AbstractCredential
      */
     private $credential;
 
-    private function onChangeCredentialEvent(ChangeCredential $event): bool
+    public function getCredential(): Credential
     {
-        if (!\is_callable($this->credential)) {
-            throw new \LogicException(sprintf('Credential "%s" must be an invokable to apply event "%s".', \get_class($this->credential), \get_class($event)));
-        }
+        return $this->credential;
+    }
 
+    private function onChangeCredentialEvent(ChangeCredential $event): bool
+    {
         return ($this->credential)($event);
     }
 }
diff --git a/Model/AbstractPasswordCredential.php b/Model/AbstractPasswordCredential.php
diff --git a/Model/EmailCredential.php b/Model/EmailCredential.php
@@ -18,11 +18,6 @@ trait EmailCredential
      */
     private $credential;
 
-    public function getCredential(): Email
-    {
-        return $this->credential;
-    }
-
     public function getEmail(): string
     {
         return $this->credential->getUsername();
diff --git a/Model/EmailPasswordCredential.php b/Model/EmailPasswordCredential.php
@@ -11,18 +11,15 @@
  */
 trait EmailPasswordCredential
 {
-    use AbstractPasswordCredential;
-    use EmailCredential {
-        EmailCredential::onChangeCredentialEvent insteadof AbstractPasswordCredential;
-    }
+    use EmailCredential;
 
     /**
      * @var EmailPassword
      */
     private $credential;
 
-    public function getCredential(): EmailPassword
+    public function getPassword(): string
     {
-        return $this->credential;
+        return $this->credential->getPassword();
     }
 }
diff --git a/Model/NicknameCredential.php b/Model/NicknameCredential.php
@@ -18,11 +18,6 @@ trait NicknameCredential
      */
     private $credential;
 
-    public function getCredential(): Nickname
-    {
-        return $this->credential;
-    }
-
     public function getNickname(): string
     {
         return $this->credential->getUsername();
diff --git a/Model/NicknamePasswordCredential.php b/Model/NicknamePasswordCredential.php
diff --git a/Model/TokenCredential.php b/Model/TokenCredential.php
diff --git a/Tests/Infrastructure/Doctrine/ObjectMappingsTest.php b/Tests/Infrastructure/Doctrine/ObjectMappingsTest.php
diff --git a/User.php b/User.php

Original file line number	Diff line number	Diff line change
`@@ -15,14 +15,4 @@ public function __invoke(ChangeCredential $event): bool`
`15`	`15`	`{`
`16`	`16`	`throw new \BadMethodCallException('An anonymous credential cannot be changed.');`
`17`	`17`	`}`
`18`		`-`
`19`		`- public static function getUsernameField(): string`
`20`		`- {`
`21`		`- throw new \BadMethodCallException('An anonymous credential has no username field.');`
`22`		`- }`
`23`		`-`
`24`		`- public function getUsername(): string`
`25`		`- {`
`26`		`- throw new \BadMethodCallException('An anonymous credential has no username.');`
`27`		`- }`
`28`	`18`	`}`
Original file line number	Diff line number	Diff line change
`@@ -12,8 +12,4 @@`
`12`	`12`	`interface Credential`
`13`	`13`	`{`
`14`	`14`	`public function __invoke(ChangeCredential $event): bool;`
`15`		`-`
`16`		`- public static function getUsernameField(): string;`
`17`		`-`
`18`		`- public function getUsername(): string;`
`19`	`15`	`}`
Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@`
`9`	`9`	`/**`
`10`	`10`	`* @author Roland Franssen <[email protected]>`
`11`	`11`	`*/`
`12`		`-final class Email implements Credential`
	`12`	`+final class Email implements UsernameCredential`
`13`	`13`	`{`
`14`	`14`	`use EmailAsUsername;`
`15`	`15`
Original file line number	Diff line number	Diff line change
`@@ -7,12 +7,14 @@`
`7`	`7`	`use MsgPhp\User\Password\PasswordAlgorithm;`
`8`	`8`
`9`	`9`	`/**`
`10`		`- * Represents a password protected credential. The password is usually a hashed value (thus secret).`
	`10`	`+ * Represents a user credential that is password protected. The password is usually a hashed value (thus secret).`
`11`	`11`	`*`
`12`	`12`	`* @author Roland Franssen <[email protected]>`
`13`	`13`	`*/`
`14`	`14`	`interface PasswordProtectedCredential extends Credential`
`15`	`15`	`{`
	`16`	`+ public static function getPasswordField(): string;`
	`17`	`+`
`16`	`18`	`public function getPassword(): string;`
`17`	`19`
`18`	`20`	`public function getPasswordAlgorithm(): ?PasswordAlgorithm;`