Fixes for chapter 5

[nicoSWD]

I was skipping through the book and saw some issues in chapter 5. I will get back to chapter 4 after this.

5.1 Application Services

} catch(UserAlreadyExistsException $e) {
   $this->render('error.html.twig', $response);
}

The line in the catch() block is missing a return statement, and a space is missing after the catch keyword.

5.3 Domain Services and Infrastructure Services

if (!$this->userRepository->has($aUsername)) {
    throw \InvalidArgumentException(
        sprintf('The user "%s" does not exist.', $aUsername)
    );
}

This is missing a new keyword.

Furthermore, how come an invalid user name throws an InvalidArgumentException while an incorrect password throws a BadCredentialsException? Shouldn't both throw the same exception as they're both related to credentials?

And last, I would not include the user's password in the exception:

throw new BadCredentialsException($aUser, $aPassword);

The password should not be required past this point, and even worse, it can be shown to the user if the exception is not being caught correctly, or it can end up in log files.

[carlosbuenosvinos]

Fixed. We fire now a UserDoesNotExistException (which is fine). The Web App could decide to print the same error (login incorrect).

About the password. $aPassword is the password written by the user, not the password of the user. ;)

[nicoSWD]

Thanks, Carlos! :)

The example code below, namely Md5HashingSignUp and SignUp need some love too. They still throw an InvalidArgumentException. Oh, and the tests need to be updated as well!

As for the password, I'm still not sure if it makes sense. I understand that if the BadCredentialsException is thrown, the password is incorrect. But it might just contain a small typo which can be easy to figure out for someone.

I'm basically just worried about usernames and passwords ending up in log files somewhere, and I don't really see the value in having an exception holding sensitive data that should not be required past this point.

@carlosbuenosvinos @theUniC @keyvanakbary

[nicoSWD]

One more:

private function __construct($anId = null)
{
    $this->id = $id ?: Uuid::uuid4()->toString();
}

$id should be $anId in the OrderId Value Object.

[nicoSWD]

In DefaultHashingSignUp:

throw new UserDoesNotExistException::fromUsername($aUsername);

The new keyword causes a syntax error.