Skip to content

[Bridge\Doctrine][FrameworkBundle] Deprecate some remaining uses of ContainerAwareTrait #24409

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 5, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions src/Symfony/Bridge/Doctrine/ManagerRegistry.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@
namespace Symfony\Bridge\Doctrine;

use ProxyManager\Proxy\LazyLoadingInterface;
use Psr\Container\ContainerInterface;
use Symfony\Component\DependencyInjection\Container;
use Symfony\Component\DependencyInjection\ContainerAwareInterface;
use Symfony\Component\DependencyInjection\ContainerAwareTrait;
use Symfony\Component\DependencyInjection\ContainerInterface as SymfonyContainerInterface;
use Doctrine\Common\Persistence\AbstractManagerRegistry;

/**
Expand All @@ -24,7 +25,21 @@
*/
abstract class ManagerRegistry extends AbstractManagerRegistry implements ContainerAwareInterface
{
use ContainerAwareTrait;
/**
* @var ContainerInterface
*/
protected $container;

/**
* @deprecated since version 3.4, to be removed in 4.0 alongside with the ContainerAwareInterface type.
* @final since version 3.4
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't it just be deprecated?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so: @deprecated doesn't trigger a deprecation when a method is overridden but the parent is not called. @final does.

*/
public function setContainer(SymfonyContainerInterface $container = null)
{
@trigger_error(sprintf('The "%s()" method is deprecated since version 3.4 and will be removed in 4.0. Inject a PSR-11 container using the constructor instead.', __METHOD__), E_USER_DEPRECATED);

$this->container = $container;
}

/**
* {@inheritdoc}
Expand Down
7 changes: 6 additions & 1 deletion src/Symfony/Bridge/Doctrine/Tests/ManagerRegistryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public function testResetService()
$container = new \LazyServiceProjectServiceContainer();

$registry = new TestManagerRegistry('name', array(), array('defaultManager' => 'foo'), 'defaultConnection', 'defaultManager', 'proxyInterfaceName');
$registry->setContainer($container);
$registry->setTestContainer($container);

$foo = $container->get('foo');
$foo->bar = 123;
Expand All @@ -46,6 +46,11 @@ public function testResetService()

class TestManagerRegistry extends ManagerRegistry
{
public function setTestContainer($container)
{
$this->container = $container;
}

public function getAliasNamespace($alias)
{
return 'Foo';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,7 @@ protected function createController($controller)
$resolvedController = parent::createController($controller);

if (1 === substr_count($controller, ':') && is_array($resolvedController)) {
if ($resolvedController[0] instanceof ContainerAwareInterface) {
$resolvedController[0]->setContainer($this->container);
}

if ($resolvedController[0] instanceof AbstractController && null !== $previousContainer = $resolvedController[0]->setContainer($this->container)) {
$resolvedController[0]->setContainer($previousContainer);
}
$resolvedController[0] = $this->configureController($resolvedController[0]);
}

return $resolvedController;
Expand All @@ -65,9 +59,19 @@ protected function createController($controller)
*/
protected function instantiateController($class)
{
$controller = parent::instantiateController($class);
return $this->configureController(parent::instantiateController($class));
}

private function configureController($controller)
{
if ($controller instanceof ContainerAwareInterface) {
// @deprecated switch, to be removed in 4.0 where these classes
// won't implement ContainerAwareInterface anymore
switch (\get_class($controller)) {
case RedirectController::class:
case TemplateController::class:
return $controller;
}
$controller->setContainer($this->container);
}
if ($controller instanceof AbstractController && null !== $previousContainer = $controller->setContainer($this->container)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
namespace Symfony\Bundle\FrameworkBundle\Controller;

use Symfony\Component\DependencyInjection\ContainerAwareInterface;
use Symfony\Component\DependencyInjection\ContainerAwareTrait;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
Expand All @@ -23,10 +23,37 @@
* Redirects a request to another URL.
*
* @author Fabien Potencier <fabien@symfony.com>
*
* @final since version 3.4
*/
class RedirectController implements ContainerAwareInterface
{
use ContainerAwareTrait;
/**
* @deprecated since version 3.4, to be removed in 4.0
*/
protected $container;

private $router;
private $httpPort;
private $httpsPort;

public function __construct(UrlGeneratorInterface $router = null, $httpPort = null, $httpsPort = null)
{
$this->router = $router;
$this->httpPort = $httpPort;
$this->httpsPort = $httpsPort;
}

/**
* @deprecated since version 3.4, to be removed in 4.0 alongside with the ContainerAwareInterface type.
*/
public function setContainer(ContainerInterface $container = null)
{
@trigger_error(sprintf('The "%s()" method is deprecated since version 3.4 and will be removed in 4.0. Inject an UrlGeneratorInterface using the constructor instead.', __METHOD__), E_USER_DEPRECATED);

$this->container = $container;
$this->router = $container->get('router');
}

/**
* Redirects to another route with the given name.
Expand Down Expand Up @@ -61,7 +88,7 @@ public function redirectAction(Request $request, $route, $permanent = false, $ig
}
}

return new RedirectResponse($this->container->get('router')->generate($route, $attributes, UrlGeneratorInterface::ABSOLUTE_URL), $permanent ? 301 : 302);
return new RedirectResponse($this->router->generate($route, $attributes, UrlGeneratorInterface::ABSOLUTE_URL), $permanent ? 301 : 302);
}

/**
Expand Down Expand Up @@ -115,8 +142,11 @@ public function urlRedirectAction(Request $request, $path, $permanent = false, $
if (null === $httpPort) {
if ('http' === $request->getScheme()) {
$httpPort = $request->getPort();
} elseif ($this->container->hasParameter('request_listener.http_port')) {
} elseif ($this->container && $this->container->hasParameter('request_listener.http_port')) {
@trigger_error(sprintf('Passing the http port as a container parameter is deprecated since Symfony 3.4 and won\'t be possible in 4.0. Pass it to the constructor of the "%s" class instead.', __CLASS__), E_USER_DEPRECATED);
$httpPort = $this->container->getParameter('request_listener.http_port');
} else {
$httpPort = $this->httpPort;
}
}

Expand All @@ -127,8 +157,11 @@ public function urlRedirectAction(Request $request, $path, $permanent = false, $
if (null === $httpsPort) {
if ('https' === $request->getScheme()) {
$httpsPort = $request->getPort();
} elseif ($this->container->hasParameter('request_listener.https_port')) {
} elseif ($this->container && $this->container->hasParameter('request_listener.https_port')) {
@trigger_error(sprintf('Passing the https port as a container parameter is deprecated since Symfony 3.4 and won\'t be possible in 4.0. Pass it to the constructor of the "%s" class instead.', __CLASS__), E_USER_DEPRECATED);
$httpsPort = $this->container->getParameter('request_listener.https_port');
} else {
$httpsPort = $this->httpsPort;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,48 @@
namespace Symfony\Bundle\FrameworkBundle\Controller;

use Symfony\Component\DependencyInjection\ContainerAwareInterface;
use Symfony\Component\DependencyInjection\ContainerAwareTrait;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Templating\EngineInterface;
use Twig\Environment;

/**
* TemplateController.
*
* @author Fabien Potencier <fabien@symfony.com>
*
* @final since version 3.4
*/
class TemplateController implements ContainerAwareInterface
{
use ContainerAwareTrait;
/**
* @deprecated since version 3.4, to be removed in 4.0
*/
protected $container;

private $twig;
private $templating;

public function __construct(Environment $twig = null, EngineInterface $templating = null)
{
$this->twig = $twig;
$this->templating = $templating;
}

/**
* @deprecated since version 3.4, to be removed in 4.0 alongside with the ContainerAwareInterface type.
*/
public function setContainer(ContainerInterface $container = null)
{
@trigger_error(sprintf('The "%s()" method is deprecated since version 3.4 and will be removed in 4.0. Inject a Twig Environment or an EngineInterface using the constructor instead.', __METHOD__), E_USER_DEPRECATED);

if ($container->has('templating')) {
$this->templating = $container->get('templating');
} elseif ($container->has('twig')) {
$this->twig = $container->get('twig');
}
$this->container = $container;
}

/**
* Renders a template.
Expand All @@ -36,10 +67,10 @@ class TemplateController implements ContainerAwareInterface
*/
public function templateAction($template, $maxAge = null, $sharedAge = null, $private = null)
{
if ($this->container->has('templating')) {
$response = $this->container->get('templating')->renderResponse($template);
} elseif ($this->container->has('twig')) {
$response = new Response($this->container->get('twig')->render($template));
if ($this->templating) {
$response = new Response($this->templating->render($template));
} elseif ($this->twig) {
$response = new Response($this->twig->render($template));
} else {
throw new \LogicException('You can not use the TemplateController if the Templating Component or the Twig Bundle are not available.');
}
Expand Down
11 changes: 11 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/routing.xml
Original file line number Diff line number Diff line change
Expand Up @@ -110,5 +110,16 @@
<argument type="service" id="router.request_context" on-invalid="ignore" />
<argument type="service" id="logger" on-invalid="ignore" />
</service>

<service id="Symfony\Bundle\FrameworkBundle\Controller\RedirectController" public="true">
<argument type="service" id="router" />
<argument>%request_listener.http_port%</argument>
<argument>%request_listener.https_port%</argument>
</service>

<service id="Symfony\Bundle\FrameworkBundle\Controller\TemplateController" public="true">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this service belongs in routing.xml. It has nothing to do with routing?!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These services are useful only when a route references them, that's why I made it this way.
Did I miss something else?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't one use them without routing using fragments?

<argument type="service" id="twig" on-invalid="ignore" />
<argument type="service" id="templating" on-invalid="ignore" />
</service>
</services>
</container>
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Symfony\Component\HttpFoundation\ParameterBag;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Exception\HttpException;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
use Symfony\Bundle\FrameworkBundle\Controller\RedirectController;
use Symfony\Bundle\FrameworkBundle\Tests\TestCase;

Expand Down Expand Up @@ -66,23 +67,14 @@ public function testRoute($permanent, $ignoreAttributes, $expectedCode, $expecte

$request->attributes = new ParameterBag($attributes);

$router = $this->getMockBuilder('Symfony\Component\Routing\RouterInterface')->getMock();
$router = $this->getMockBuilder(UrlGeneratorInterface::class)->getMock();
$router
->expects($this->once())
->method('generate')
->with($this->equalTo($route), $this->equalTo($expectedAttributes))
->will($this->returnValue($url));

$container = $this->getMockBuilder('Symfony\Component\DependencyInjection\ContainerInterface')->getMock();

$container
->expects($this->once())
->method('get')
->with($this->equalTo('router'))
->will($this->returnValue($router));

$controller = new RedirectController();
$controller->setContainer($container);
$controller = new RedirectController($router);

$returnResponse = $controller->redirectAction($request, $route, $permanent, $ignoreAttributes);

Expand Down Expand Up @@ -130,7 +122,7 @@ public function testFullURL()
$this->assertEquals(302, $returnResponse->getStatusCode());
}

public function testUrlRedirectDefaultPortParameters()
public function testUrlRedirectDefaultPorts()
{
$host = 'www.example.com';
$baseUrl = '/base';
Expand All @@ -151,6 +143,30 @@ public function testUrlRedirectDefaultPortParameters()
$this->assertRedirectUrl($returnValue, $expectedUrl);
}

/**
* @group legacy
*/
public function testUrlRedirectDefaultPortParameters()
{
$host = 'www.example.com';
$baseUrl = '/base';
$path = '/redirect-path';
$httpPort = 1080;
$httpsPort = 1443;

$expectedUrl = "https://$host:$httpsPort$baseUrl$path";
$request = $this->createRequestObject('http', $host, $httpPort, $baseUrl);
$controller = $this->createLegacyRedirectController(null, $httpsPort);
$returnValue = $controller->urlRedirectAction($request, $path, false, 'https');
$this->assertRedirectUrl($returnValue, $expectedUrl);

$expectedUrl = "http://$host:$httpPort$baseUrl$path";
$request = $this->createRequestObject('https', $host, $httpPort, $baseUrl);
$controller = $this->createLegacyRedirectController($httpPort);
$returnValue = $controller->urlRedirectAction($request, $path, false, 'http');
$this->assertRedirectUrl($returnValue, $expectedUrl);
}

public function urlRedirectProvider()
{
return array(
Expand Down Expand Up @@ -256,6 +272,14 @@ private function createRequestObject($scheme, $host, $port, $baseUrl, $queryStri
}

private function createRedirectController($httpPort = null, $httpsPort = null)
{
return new RedirectController(null, $httpPort, $httpsPort);
}

/**
* @deprecated
*/
private function createLegacyRedirectController($httpPort = null, $httpsPort = null)
{
$container = $this->getMockBuilder('Symfony\Component\DependencyInjection\ContainerInterface')->getMock();

Expand Down
Loading