gstreamer0.10-ffmpeg
gstreamer0.10-plugins-good
packages.
We've identified that UserManager::register()
handles two things that might change for different reasons. These are its two responsibilities: one, creating and sending a confirmation email and two, setting up the data for a user and saving it to the database.
We're now going to follow the advice of SRP and "separate those things that change for different reasons".
The first thing I want to do is rename register()
to create()
... or you could use save()
... or even rename the entire class itself. The point is: I want to make its responsibility more clear: to set all the required data on the user object and save it to the database.
Right click on register()
, go to Refactor->Rename and call this create()
.
... lines 1 - 12 | |
class UserManager | |
{ | |
... lines 15 - 27 | |
public function create(User $user, string $plainPassword): void | |
{ | |
... lines 30 - 53 | |
} | |
... lines 55 - 59 | |
} |
When I hit enter, over in RegistrationController
, PhpStorm renamed the method there too.
... lines 1 - 13 | |
class RegistrationController extends AbstractController | |
{ | |
... lines 16 - 18 | |
public function signup(Request $request, UserManager $userManager) | |
{ | |
... lines 21 - 23 | |
if ($form->isSubmitted() && $form->isValid()) { | |
... lines 25 - 31 | |
$userManager->create($user, $plainPassword); | |
... lines 33 - 36 | |
} | |
... lines 38 - 41 | |
} | |
... lines 43 - 62 | |
} |
Next, let's move the email-related logic into a new class in the Service/
directory... though, it doesn't matter where this lives. Create a new PHP class called, how about, ConfirmationEmailSender
. This class will need two services: the router so it can generate the link and mailer. Add a public function __construct()
with those two arguments: MailerInterface $mailer
, and RouterInterface $router
. Hit Alt + Enter and go to "Initialize properties" to create both of those properties and set them. We don't need this extra PHPDoc up here.
... lines 1 - 4 | |
use Symfony\Component\Mailer\MailerInterface; | |
use Symfony\Component\Routing\RouterInterface; | |
class ConfirmationEmailSender | |
{ | |
private MailerInterface $mailer; | |
private RouterInterface $router; | |
public function __construct(MailerInterface $mailer, RouterInterface $router) | |
{ | |
$this->mailer = $mailer; | |
$this->router = $router; | |
} | |
} |
Now we can create a public function called, how about, send()
, with a User
object argument that will return void
.
... lines 1 - 10 | |
class ConfirmationEmailSender | |
{ | |
... lines 13 - 21 | |
public function send(User $user): void | |
{ | |
... lines 24 - 37 | |
} | |
} |
For the inside of this, let's go steal all of the email-related logic from UserManager
. So... copy the $confirmationLink
and $confirmationEmail
parts... delete those... and paste. Yes PhpStorm: I definitely want you to import the use
statements for me.
The last line we need to steal is the $mailer->send()
line. Paste that into the new class.
... lines 1 - 5 | |
use Symfony\Bridge\Twig\Mime\TemplatedEmail; | |
... line 7 | |
use Symfony\Component\Routing\Generator\UrlGeneratorInterface; | |
... lines 9 - 10 | |
class ConfirmationEmailSender | |
{ | |
... lines 13 - 21 | |
public function send(User $user): void | |
{ | |
$confirmationLink = $this->router->generate('check_confirmation_link', [ | |
'token' => $user->getConfirmationToken() | |
], UrlGeneratorInterface::ABSOLUTE_URL); | |
$confirmationEmail = (new TemplatedEmail()) | |
->from('staff@example.com') | |
->to($user->getEmail()) | |
->subject('Confirm your account') | |
->htmlTemplate('emails/registration_confirmation.html.twig') | |
->context([ | |
'confirmationLink' => $confirmationLink | |
]); | |
$this->mailer->send($confirmationEmail); | |
} | |
} |
Very nice! Let's celebrate by cleaning things up in UserManager
: we can remove the last two arguments of the constructor - $router
and $mailer
- their properties... and even some use
statements on top.
... lines 1 - 8 | |
class UserManager | |
{ | |
... lines 11 - 13 | |
public function __construct(UserPasswordEncoderInterface $passwordEncoder, EntityManagerInterface $entityManager) | |
{ | |
... lines 16 - 17 | |
} | |
... line 19 | |
public function create(User $user, string $plainPassword): void | |
{ | |
$token = $this->createToken(); | |
$user->setConfirmationToken($token); | |
$user->setPassword( | |
$this->passwordEncoder->encodePassword($user, $plainPassword) | |
); | |
$this->entityManager->persist($user); | |
$this->entityManager->flush(); | |
} | |
... lines 32 - 36 | |
} |
Done! Now... let's see... who should be responsible for creating and setting the confirmation token on the User? I'm... not exactly sure. But let's invert that question: who should not be responsible for creating the token?
That's a bit easier: it probably doesn't make sense for the service whose only responsibility is creating an email... to also be responsible for generating this cryptographically-secure token and saving it to the database. Yes, this service does deal with the confirmation link... but it feels like that logic would change for very different reasons than the email itself.
So if we discard ConfirmationEmailSender
from our options, then there's only one logical place left UserManager::create()
. And... it makes sense: this method sets up new User
objects with all the data they need and then saves them. You could also choose to isolate the confirmation token creation logic into a third class... there's no right or wrong answer, which is what makes this stuff so darn tricky! But over optimizing, by splitting things into too many pieces, is also something that we do not want to do. We'll talk more about that in the next chapter.
Anyways, now that we've split all of our code into two places, over in RegistrationController
, we need to call both methods. Autowire a new argument into the method: ConfirmationEmailSender $confirmationEmailSender
. Then, below, right after we call $userManager->create()
, say $confirmationEmailSender->send()
and pass the $user
object.
... lines 1 - 13 | |
class RegistrationController extends AbstractController | |
{ | |
... lines 17 - 19 | |
public function signup(Request $request, UserManager $userManager, ConfirmationEmailSender $confirmationEmailSender) | |
{ | |
... lines 22 - 24 | |
if ($form->isSubmitted() && $form->isValid()) { | |
... lines 26 - 32 | |
$userManager->create($user, $plainPassword); | |
$confirmationEmailSender->send($user); | |
... lines 35 - 38 | |
} | |
... lines 40 - 43 | |
} | |
... lines 45 - 64 | |
} |
Done! Our original feature - sending a confirmation email - is now implemented in a more SRP-friendly way.
By the way, if you don't like that you need to call two methods whenever you're registering a new user... I kind of agree! And it's no problem: you could extract these two calls into a new class... maybe called UserRegistrationHandler
.
It's one responsibility would be to "orchestrate" all the tasks related to registering a user. This is just one responsibility - not many - because it's not actually doing any of the real work. So, for example, if we needed to make a change to the confirmation email... or change how users are persisted to the database... neither of those would require us to need to modify this new class. The new class would only change if we added some new "step" to user registration - like sending an API call to our newsletter service.
Anyways, now that we've refactored to be SRP-compliant, we get to enjoy our hard work by finally adding the new feature that our team asked for: the ability to resend a confirmation email.
If you downloaded the course code from this page, you should have a tutorial/
directory with a ResendConfirmationController
file inside. Copy this, go up to the Controller/
directory... and paste. This comes with the boilerplate needed for an endpoint that a user could POST to in order to resend their confirmation email.
... lines 1 - 8 | |
class ResendConfirmationController extends AbstractController | |
{ | |
/** | |
* @Route("/resend-confirmation", methods={"POST"}) | |
*/ | |
public function resend() | |
{ | |
$this->denyAccessUnlessGranted('ROLE_USER'); | |
$user = $this->getUser(); | |
// TODO: send confirmation email | |
return new Response(null, 204); | |
} | |
} |
But... the actual sending of that confirmation email is still a "TODO". Remove that comment, autowire the ConfirmationEmailSender
service... and then say $confirmationEmailSender->send($user)
.
... lines 1 - 4 | |
use App\Service\ConfirmationEmailSender; | |
... lines 6 - 9 | |
class ResendConfirmationController extends AbstractController | |
{ | |
... lines 12 - 14 | |
public function resend(ConfirmationEmailSender $confirmationEmailSender) | |
{ | |
... lines 17 - 19 | |
$confirmationEmailSender->send($user); | |
... lines 21 - 22 | |
} | |
} |
It's that easy! I won't bother testing this... but I will repeat the words that every developer loves to say: "it should work".
The important thing is that, thanks to our new organization, if, for example, a marketing person did want to tweak the subject on our welcome email, we can make that change without messing around near code that saves things to the database or hashes passwords.
But... I have more that I want to say about SRP... like the risks of over-optimizing, which violates a concept called cohesion. I also think that, thanks to inspiration from Dan North, there's an easier way to think about SRP. I'll explain all of that next.
Hey theNaschkatze,
Good question :) Actually, manager is a bad name to use because it really does not have much value in its name. Well, I bet people start using the "manager" suffix after Doctrine's EntityManager and so abuse this name in their own projects for their own services, but usually, there might be a better name for your services. Basically, manager is the same as service. So you can think about manager as just a service, nothing special, just a fancy name. But that's my personal opinion on it.
Cheers!
Why ConfirmationEmailSender is a concrete class and not an interface? Currently the caller is tied to this specific implementation and then to the specific template and synchronous mailer. Maybe I want to change the template based on the user who has just signed in, e.g. standard user, business user, gold user? Or I have recommendation links and depending on who recommended my site the template will differ (there even might not be any confirmation email for some users, and NullSender will be the way to use then)? Maybe I want to save the email in DB so a cron job will pick it up or want to send to some emails queue.
What I'm trying to say is that a caller should not care about the implementation. It should only care that it has a contract with an interface that the confirmation will be sent "somehow".
What do you think about it?
Cheers!
Hey Fabien,
those are goods points, and if your application requires such flexibility, then, adding an interface, so you can change the implementation based on your business rules makes perfect sense, but I wouldn't add an interface to protect my code from changes that I'm not sure about. I don't like to add unnecessary abstractions which only makes more difficult to understand your code. My advice here is "keep it simple" and let changes hit you first, then, you can adapt your code and improve its design based on real things not on a guess.
Cheers!
// composer.json
{
"require": {
"php": ">=8.1",
"ext-ctype": "*",
"ext-iconv": "*",
"composer/package-versions-deprecated": "^1.11", // 1.11.99.1
"doctrine/annotations": "^1.0", // 1.12.1
"doctrine/doctrine-bundle": "^2", // 2.3.1
"doctrine/doctrine-migrations-bundle": "^3", // 3.1.1
"doctrine/orm": "^2", // 2.8.4
"knplabs/knp-time-bundle": "^1.15", // v1.16.0
"phpdocumentor/reflection-docblock": "^5.2", // 5.2.2
"sensio/framework-extra-bundle": "^6.0", // v6.1.2
"symfony/console": "5.2.*", // v5.2.6
"symfony/dotenv": "5.2.*", // v5.2.4
"symfony/flex": "^1.9", // v1.18.7
"symfony/form": "5.2.*", // v5.2.6
"symfony/framework-bundle": "5.2.*", // v5.2.6
"symfony/http-client": "5.2.*", // v5.2.6
"symfony/mailer": "5.2.*", // v5.2.6
"symfony/property-access": "5.2.*", // v5.2.4
"symfony/property-info": "5.2.*", // v5.2.4
"symfony/security-bundle": "5.2.*", // v5.2.6
"symfony/serializer": "5.2.*", // v5.2.4
"symfony/twig-bundle": "5.2.*", // v5.2.4
"symfony/validator": "5.2.*", // v5.2.6
"symfony/webpack-encore-bundle": "^1.6", // v1.11.1
"symfony/yaml": "5.2.*", // v5.2.5
"twig/cssinliner-extra": "^3.3", // v3.3.0
"twig/extra-bundle": "^2.12|^3.0", // v3.3.0
"twig/twig": "^2.12|^3.0" // v3.3.0
},
"require-dev": {
"doctrine/doctrine-fixtures-bundle": "^3.2", // 3.4.0
"fakerphp/faker": "^1.13", // v1.14.1
"symfony/debug-bundle": "^5.2", // v5.2.4
"symfony/maker-bundle": "^1.13", // v1.30.2
"symfony/monolog-bundle": "^3.0", // v3.7.0
"symfony/stopwatch": "^5.2", // v5.2.4
"symfony/var-dumper": "^5.2", // v5.2.6
"symfony/web-profiler-bundle": "^5.2" // v5.2.6
}
}
I did all of the OOP courses in the track (until this point), but non of them explained me what is the manager folder for. I only knew services and entities... Entities store data and services do some work with the data.... but manager?