Skip to content

Commit

Permalink
feature #19452 Remove the new SecurityUserValueResolver (weaverryan)
Browse files Browse the repository at this point in the history
This PR was merged into the 3.2-dev branch.

Discussion
----------

Remove the new SecurityUserValueResolver

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no (the feature hasn't been released yet)
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

Hi guys!

This is a revert for #18510 (ping @iltar), which is a nice idea, but will have some big practical implications:

1) You are only allowed to type-hint the argument with `UserInterface` exactly. For the 90% of Symfony project's that user a User entity for their User, this will be weird: I'll receive a `UserInterface`, that immediately call methods on it that aren't in the interface (and also, your IDE won't give you auto-completion). And as #18510 mentions, we can't allow people to type-hint their concrete `User` class, because this will conflict with SensioFWExtraBundle ParamConverter if there is a user id in the URL

2) Deprecating and removing `$this->getUser()` in a controller is a step back. Where we can, let's make controllers and services act *more* like each other. You can't call `$this->getUser()` in a service, but at least if you look at this method in `Controller`, you can see that it uses `security.token_storage` - which is how you will get the User object if you need it from within services.

Sorry for being late on this original issue! It looked good to me at first :).

Cheers!

Commits
-------

da7daee Removing the Controller::getUser() deprecation
  • Loading branch information
fabpot committed Oct 15, 2016
2 parents 870c302 + da7daee commit f9bb4ab
Show file tree
Hide file tree
Showing 5 changed files with 0 additions and 22 deletions.
2 changes: 0 additions & 2 deletions UPGRADE-3.2.md
Expand Up @@ -27,8 +27,6 @@ FrameworkBundle
* The `Resources/public/images/*` files have been removed.
* The `Resources/public/css/*.css` files have been removed (they are now inlined
in TwigBundle).
* The `Controller::getUser()` method has been deprecated and will be removed in
Symfony 4.0; typehint the security user object in the action instead.

Console
-------
Expand Down
3 changes: 0 additions & 3 deletions UPGRADE-4.0.md
Expand Up @@ -124,9 +124,6 @@ FrameworkBundle
`serializer.mapping.cache.apc` and `serializer.mapping.cache.doctrine.apc`
have been removed. APCu should now be automatically used when available.

* The `Controller::getUser()` method has been removed in favor of the ability
to typehint the security user object in the action.

HttpFoundation
---------------

Expand Down
2 changes: 0 additions & 2 deletions src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
Expand Up @@ -11,8 +11,6 @@ CHANGELOG
* Removed `symfony/asset` from the list of required dependencies in `composer.json`
* The `Resources/public/images/*` files have been removed.
* The `Resources/public/css/*.css` files have been removed (they are now inlined in TwigBundle).
* The `Controller::getUser()` method has been deprecated and will be removed in
Symfony 4.0; typehint the security user object in the action instead.
* Added possibility to prioritize form type extensions with `'priority'` attribute on tags `form.type_extension`

3.1.0
Expand Down
5 changes: 0 additions & 5 deletions src/Symfony/Bundle/FrameworkBundle/Controller/Controller.php
Expand Up @@ -22,7 +22,6 @@
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Security\Csrf\CsrfToken;
use Symfony\Component\Form\Extension\Core\Type\FormType;
use Symfony\Component\Form\Form;
Expand Down Expand Up @@ -367,16 +366,12 @@ protected function getDoctrine()
*
* @return mixed
*
* @deprecated as of 3.2 and will be removed in 4.0. You can typehint your method argument with Symfony\Component\Security\Core\User\UserInterface instead.
*
* @throws \LogicException If SecurityBundle is not available
*
* @see TokenInterface::getUser()
*/
protected function getUser()
{
@trigger_error(sprintf('%s() is deprecated as of 3.2 and will be removed in 4.0. You can typehint your method argument with %s instead.', __METHOD__, UserInterface::class), E_USER_DEPRECATED);

if (!$this->container->has('security.token_storage')) {
throw new \LogicException('The SecurityBundle is not registered in your application.');
}
Expand Down
Expand Up @@ -56,9 +56,6 @@ public function testForward()
$this->assertEquals('xml--fr', $response->getContent());
}

/**
* @group legacy
*/
public function testGetUser()
{
$user = new User('user', 'pass');
Expand All @@ -70,9 +67,6 @@ public function testGetUser()
$this->assertSame($controller->getUser(), $user);
}

/**
* @group legacy
*/
public function testGetUserAnonymousUserConvertedToNull()
{
$token = new AnonymousToken('default', 'anon.');
Expand All @@ -83,9 +77,6 @@ public function testGetUserAnonymousUserConvertedToNull()
$this->assertNull($controller->getUser());
}

/**
* @group legacy
*/
public function testGetUserWithEmptyTokenStorage()
{
$controller = new TestController();
Expand All @@ -95,7 +86,6 @@ public function testGetUserWithEmptyTokenStorage()
}

/**
* @group legacy
* @expectedException \LogicException
* @expectedExceptionMessage The SecurityBundle is not registered in your application.
*/
Expand Down

0 comments on commit f9bb4ab

Please sign in to comment.