We've just created a new branch based off of Symfony's master branch. And now, we're ready to create the amazing new TargetPathHelper class. But... where should it live? It's related to Security... which means it could live in the Security component or SecurityBundle.
Components Versus Bundles
As a general rule, most code should live in a component so that it's reusable even outside of the framework. But, sometimes, you'll write code that's really integrated with the framework. That code will live in the bundle. My best advice... don't over-think it: it usually becomes pretty obvious if you put something in the wrong spot.
Press Shift+Shift and search for a file that's closely related to our new feature: TargetPathTrait. Ok, this lives in the Security component. I'll double click on the directory to move there. At first, it seems like TargetPathHelper should live right here. And that's where I would put it at first. I say "at first" because, if you started coding, you'd notice a problem.
What problem? This new class will ultimately use the FirewallMap class internally to do its work. There are two FirwallMap classes: one lives in the Security component, and the other lives in SecurityBundle. After digging a little bit, you'd find out that we will need to use the one from SecurityBundle.
And here's why that's important: a class in a component can depend on classes from other components. But, it can never depend on a class from a bundle. Because our new TargetPathHelper needs a class from SecurityBundle, it can't live in the component: it must live in the bundle.
If you get this wrong, no big deal: someone will help you out on your pull request.
Go find SecurityBundle and look inside Security. Hey! Here are the FirewallMap and FirewallConfig classes we'll be using! That's a good sign! Create the new PHP class: TargetPathHelper. Add our first public function, how about just savePath() with a string $uri argument:
| // ... lines 1 - 2 | |
| namespace Symfony\Bundle\SecurityBundle\Security; | |
| class TargetPathHelper | |
| { | |
| public function savePath(string $uri) | |
| { | |
| } | |
| } |
Symfony 4.0 and above requires PHP 7.1, so you should use scalar type-hints and return types. But, Symfony does not use the void return type.
All About PHPDoc
Because this is a public function, we should add some PHPDoc to describe it. Add a clear, but short description above this:
| // ... lines 1 - 13 | |
| class TargetPathHelper | |
| { | |
| /** | |
| * Sets the target path the user should be redirected to after authentication. | |
| * | |
| * @param string $uri The URI to set as the target path | |
| */ | |
| public function savePath(string $uri) | |
| { | |
| } | |
| } |
Actually, Symfony does not have a lot of PHPDoc... which might seem weird at first. The reason is that we don't want to maintain too much documentation inside the code - we use a separate repository for documentation.
Oh, and, thanks to the string type-hint, the @param documentation is totally redundant and should be removed... unless there's some valuable extra info that you want to say about it. I'll keep it and add some extra notes... even though it doesn't add a lot of extra context:
| // ... lines 1 - 13 | |
| class TargetPathHelper | |
| { | |
| /** | |
| // ... lines 17 - 18 | |
| * @param string $uri The URI to set as the target path | |
| */ | |
| public function savePath(string $uri) | |
| { | |
| } | |
| } |
Also every PHP file in Symfony should have a copyright header on top. Grab that from another file and paste it here:
| // ... lines 1 - 2 | |
| /* | |
| * This file is part of the Symfony package. | |
| * | |
| * (c) Fabien Potencier <fabien@symfony.com> | |
| * | |
| * For the full copyright and license information, please view the LICENSE | |
| * file that was distributed with this source code. | |
| */ | |
| namespace Symfony\Bundle\SecurityBundle\Security; | |
| class TargetPathHelper | |
| { | |
| // ... lines 16 - 24 | |
| } |
Don't worry too much about these details: it's easy to add them later if you forget.
Injecting the Services we Need
To make life nicer, use TargetPathTrait on top of the class:
| // ... lines 1 - 13 | |
| use Symfony\Component\Security\Http\Util\TargetPathTrait; | |
| class TargetPathHelper | |
| { | |
| use TargetPathTrait; | |
| // ... lines 19 - 28 | |
| } |
Then all we need to do is say $this->saveTargetPath():
| // ... lines 1 - 15 | |
| class TargetPathHelper | |
| { | |
| use TargetPathTrait; | |
| // ... lines 19 - 24 | |
| public function savePath(string $uri) | |
| { | |
| $this->saveTargetPath(); | |
| } | |
| } |
But... hmm... this needs 3 arguments: the session, provider key - which is the firewall name - and the URI. We know that we can get the firewall name by using the FirewallMap service. Let's add some constructor arguments: SessionInterface $session and FirewallMap - the one from SecurityBundle - $firewallMap:
| // ... lines 1 - 13 | |
| use Symfony\Component\HttpFoundation\Session\SessionInterface; | |
| // ... lines 15 - 16 | |
| class TargetPathHelper | |
| { | |
| // ... lines 19 - 24 | |
| public function __construct(SessionInterface $session, FirewallMap $firewallMap) | |
| { | |
| // ... lines 27 - 28 | |
| } | |
| // ... lines 30 - 44 | |
| } |
I'll press Alt+Enter and select initialize fields to create those properties and set them:
| // ... lines 1 - 16 | |
| class TargetPathHelper | |
| { | |
| // ... lines 19 - 20 | |
| private $session; | |
| private $firewallMap; | |
| public function __construct(SessionInterface $session, FirewallMap $firewallMap) | |
| { | |
| $this->session = $session; | |
| $this->firewallMap = $firewallMap; | |
| } | |
| // ... lines 30 - 44 | |
| } |
Make sure to remove the PHPDoc above each property: this is redundant thanks to the constructor type-hints.
To calculate the provider key, create a new private function: getProviderKey() that will return a string. For now, just put a TODO:
| // ... lines 1 - 16 | |
| class TargetPathHelper | |
| { | |
| // ... lines 19 - 40 | |
| private function getProviderKey(): string | |
| { | |
| // TODO | |
| } | |
| } |
Back up in setTargetPath(), pass $this->session, $this->getProviderKey() and the $uri:
| // ... lines 1 - 16 | |
| class TargetPathHelper | |
| { | |
| // ... lines 19 - 35 | |
| public function savePath(string $uri) | |
| { | |
| $this->saveTargetPath($this->session, $this->getProviderKey(), $uri); | |
| } | |
| // ... lines 40 - 44 | |
| } |
Awesome! Look back at getProviderKey():
| // ... lines 1 - 16 | |
| class TargetPathHelper | |
| { | |
| // ... lines 19 - 40 | |
| private function getProviderKey(): string | |
| { | |
| // TODO | |
| } | |
| } |
I didn't add any PHPDoc to this function, but that's not because I'm lazy. Or... not entirely because I'm lazy. Really, it's for two reasons. First, this is a private function, and those are typically not documented inside Symfony. And second, we already have the return type - no reason to duplicate it!
Lets finish this function. To get the firewall name, we need to use the FirewallMap, call getFirewallConfig() and pass it the request. Ok: $firewallConfig = $this->firewallMap->getFirewallConfig(). But, hmm... we don't have the Request object! No problem: add a third constructor arg: RequestStack $requestStack. I'll hit Alt+Enter again to create that property and set it. Clean off the PHPDoc, then head back down:
| // ... lines 1 - 13 | |
| use Symfony\Component\HttpFoundation\RequestStack; | |
| // ... lines 15 - 17 | |
| class TargetPathHelper | |
| { | |
| // ... lines 20 - 25 | |
| private $requestStack; | |
| public function __construct(SessionInterface $session, FirewallMap $firewallMap, RequestStack $requestStack) | |
| { | |
| // ... lines 30 - 31 | |
| $this->requestStack = $requestStack; | |
| } | |
| // ... lines 34 - 54 | |
| } |
Normally, when you use RequestStack, you call its getCurrentRequest() method to get the request. But, in this case, I'm going to use another method: $this->requestStack->getMasterRequest():
| // ... lines 1 - 17 | |
| class TargetPathHelper | |
| { | |
| // ... lines 20 - 44 | |
| private function getProviderKey(): string | |
| { | |
| $firewallConfig = $this->firewallMap->getFirewallConfig($this->requestStack->getMasterRequest()); | |
| // ... lines 48 - 53 | |
| } | |
| } |
I'm not 100% sure that this is correct. The whole topic of requests and sub-requests is pretty complex. But, basically, Symfony's security firewall only operates on the outer, "master" request. So, to find the active firewall, that's what we should use. If I'm wrong, hopefully someone will tell me on the pull request.
Next, if you look at the getFirewallConfig() method, it's possible that this will return null. Code for that: if null === $firewallConfig:
| // ... lines 1 - 17 | |
| class TargetPathHelper | |
| { | |
| // ... lines 20 - 44 | |
| private function getProviderKey(): string | |
| { | |
| $firewallConfig = $this->firewallMap->getFirewallConfig($this->requestStack->getMasterRequest()); | |
| if (null === $firewallConfig) { | |
| // ... line 50 | |
| } | |
| // ... lines 52 - 53 | |
| } | |
| } |
This is another Symfony coding convention: we use Yoda conditionals!
Mysterious, Symfony's coding conventions are. Herh herh herh.
But hey! If the force isn't strong with you today, don't worry: Symfony has a magic way of fixing coding convention problems that I'll show you later.
If there is no firewall config for some reason, throw a new LogicException with as clear a message as possible:
| // ... lines 1 - 17 | |
| class TargetPathHelper | |
| { | |
| // ... lines 20 - 44 | |
| private function getProviderKey(): string | |
| { | |
| $firewallConfig = $this->firewallMap->getFirewallConfig($this->requestStack->getMasterRequest()); | |
| if (null === $firewallConfig) { | |
| throw new \LogicException('Could not find firewall config for the current request'); | |
| } | |
| // ... lines 52 - 53 | |
| } | |
| } |
Why a LogicException? Well, it seems to make sense - something went wrong... logically. And usually, the exact exception class won't matter. If it does matter, someone will tell you when reviewing your PR.
Finally, at the bottom, return $firewallConfig->getName():
| // ... lines 1 - 17 | |
| class TargetPathHelper | |
| { | |
| // ... lines 20 - 44 | |
| private function getProviderKey(): string | |
| { | |
| $firewallConfig = $this->firewallMap->getFirewallConfig($this->requestStack->getMasterRequest()); | |
| if (null === $firewallConfig) { | |
| throw new \LogicException('Could not find firewall config for the current request'); | |
| } | |
| return $firewallConfig->getName(); | |
| } | |
| } |
That should be it!
While we're here, let's add one more function: getPath() that will return a string. Inside, return $this->getTargetPath() with $this->session and $this->getProviderKey():
| // ... lines 1 - 17 | |
| class TargetPathHelper | |
| { | |
| // ... lines 20 - 47 | |
| public function getPath(): string | |
| { | |
| return $this->getTargetPath($this->session, $this->getProviderKey()); | |
| } | |
| // ... lines 52 - 62 | |
| } |
This time, I will add some PHPDoc. I don't need @return - that's redundant - but I will add a description about what this method does:
| // ... lines 1 - 17 | |
| class TargetPathHelper | |
| { | |
| // ... lines 20 - 44 | |
| /** | |
| * Returns the URL (if any) the user visited that forced them to login. | |
| */ | |
| public function getPath(): string | |
| { | |
| return $this->getTargetPath($this->session, $this->getProviderKey()); | |
| } | |
| // ... lines 52 - 62 | |
| } |
Making the Class final
And... we're done! Yea, we still need to write a test & add some config to register this as a service: we'll do that next. But, this class should work!
However... I am going to make one, ahem, final change: add final:
| // ... lines 1 - 17 | |
| final class TargetPathHelper | |
| { | |
| // ... lines 20 - 62 | |
| } |
Making this class final means that nobody is allowed to subclass it. Why are we doing this? Because, in the future, if the class is final, it will be easier to make changes to it without breaking backwards compatibility. Basically, if you allow a class to be sub-classed, you have to be a bit more careful when making certain changes. Making classes final is a good "default" for new Symfony classes.
Of course, if there is a legitimate use-case for some to sub-class this, then you don't need to make it final. But, while we can easily remove final later, we can't add final in the future, at least not without jumping through a few extra hoops to avoid breaking backwards compatibility.
Ok, let's add some service config & a test!
Is is a bug?
In the last code block, when the page is initially loaded, we have lines 20-62 hidden. Then you click arrows and have lines 20-41 hidden. Then you click arrows again and have no more lines hidden. But that is not true! Line 20 is still hidden, but no arrows - one has to click the square on the top of the code block.
How did I found this bug? I wasn't into context of the tutorial because I only needed to check one thing out and I didn't remember that you were using TargetPathTrait in your Helper. I just needed to check something in the section "Making the Class final". So I scrolled down to investigate this one code block and I couldn't figure out where the hell are saveTargetPath() and getTargetPath() coming from. Maybe he used a trait? No, there is no trait. And after few minutes I recognized the line 20 was missing.