-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[TwigBundle] Deprecate the public "twig" service to private #36739
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,3 +8,8 @@ framework: | |
|
||
twig: | ||
strict_variables: '%kernel.debug%' | ||
|
||
services: | ||
twig.alias: | ||
alias: twig | ||
public: true |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,17 +11,23 @@ | |
|
||
namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FormLoginBundle\Controller; | ||
|
||
use Symfony\Component\DependencyInjection\ContainerAwareInterface; | ||
use Symfony\Component\DependencyInjection\ContainerAwareTrait; | ||
use Psr\Container\ContainerInterface; | ||
use Symfony\Component\HttpFoundation\Request; | ||
use Symfony\Component\HttpFoundation\Response; | ||
use Symfony\Component\Security\Core\Exception\AccessDeniedException; | ||
use Symfony\Component\Security\Core\Security; | ||
use Symfony\Component\Security\Core\User\UserInterface; | ||
use Symfony\Contracts\Service\ServiceSubscriberInterface; | ||
use Twig\Environment; | ||
|
||
class LoginController implements ContainerAwareInterface | ||
class LoginController implements ServiceSubscriberInterface | ||
{ | ||
use ContainerAwareTrait; | ||
private $container; | ||
|
||
public function __construct(ContainerInterface $container) | ||
{ | ||
$this->container = $container; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not inject dependencies explicitly? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried to modify existing tests the least possible. I mean I tried to keep There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using the service subscriber system looks OK to me 🤷♂️ |
||
} | ||
|
||
public function loginAction(Request $request, UserInterface $user = null) | ||
{ | ||
|
@@ -53,4 +59,14 @@ public function secureAction() | |
{ | ||
throw new \Exception('Wrapper', 0, new \Exception('Another Wrapper', 0, new AccessDeniedException())); | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public static function getSubscribedServices() | ||
{ | ||
return [ | ||
'twig' => Environment::class, | ||
]; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,8 +11,28 @@ | |
|
||
namespace Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FormLoginBundle; | ||
|
||
use Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FormLoginBundle\Controller\LocalizedController; | ||
use Symfony\Bundle\SecurityBundle\Tests\Functional\Bundle\FormLoginBundle\Controller\LoginController; | ||
use Symfony\Component\DependencyInjection\ContainerBuilder; | ||
use Symfony\Component\HttpKernel\Bundle\Bundle; | ||
|
||
class FormLoginBundle extends Bundle | ||
{ | ||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function build(ContainerBuilder $container) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this test bundle is used with different configurations, it's easier to register those controllers as services here (it avoids duplication in config files). |
||
{ | ||
parent::build($container); | ||
|
||
$container | ||
->register(LoginController::class) | ||
->setPublic(true) | ||
->addTag('container.service_subscriber'); | ||
|
||
$container | ||
->register(LocalizedController::class) | ||
->setPublic(true) | ||
->addTag('container.service_subscriber'); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,11 @@ | ||
CHANGELOG | ||
========= | ||
|
||
5.2.0 | ||
----- | ||
|
||
* deprecated the public `twig` service to private | ||
|
||
5.0.0 | ||
----- | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't we get private services in tests with the special test container? why is this needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
twig
service is still public. We don't want to access to it directly to avoid the deprecation.