Skip to content

Commit

Permalink
feature #31202 [FrameworkBundle] WebTestCase KernelBrowser::getContai…
Browse files Browse the repository at this point in the history
…ner null return type (Simperfit)

This PR was merged into the 4.4 branch.

Discussion
----------

[FrameworkBundle] WebTestCase KernelBrowser::getContainer null return type

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | yes <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #25920  <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        |  <!-- required for new features -->

<!--
Write a short README entry for your feature/bugfix here (replace this comment block.)
This will help people understand your PR and can be used as a start of the Doc PR.
Additionally:
 - Bug fixes must be submitted against the lowest branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against the master branch.
-->

As @stof suggested in the #25920 I started deprecating the behaviour of returning null when the container is non booted / null.

If this is not wanted we should close both the PR and the issue ;).

Commits
-------

e169e1a [FrameworkBundle] WebTestCase KernelBrowser::getContainer null return type
  • Loading branch information
fabpot committed Sep 27, 2019
2 parents 86d7efa + e169e1a commit 979be29
Show file tree
Hide file tree
Showing 9 changed files with 87 additions and 34 deletions.
55 changes: 28 additions & 27 deletions UPGRADE-4.4.md
Expand Up @@ -60,7 +60,7 @@ DependencyInjection
```php
new Definition('%my_class%');
```

DoctrineBridge
--------------
* Deprecated injecting `ClassMetadataFactory` in `DoctrineExtractor`, an instance of `EntityManagerInterface` should be
Expand Down Expand Up @@ -96,7 +96,7 @@ FrameworkBundle
* Deprecated `routing.loader.service`, use `routing.loader.container` instead.
* Not tagging service route loaders with `routing.route_loader` has been deprecated.
* Overriding the methods `KernelTestCase::tearDown()` and `WebTestCase::tearDown()` without the `void` return-type is deprecated.

HttpClient
----------

Expand Down Expand Up @@ -139,7 +139,7 @@ HttpKernel
}
```

As many bundles must be compatible with a range of Symfony versions, the current
As many bundles must be compatible with a range of Symfony versions, the current
directory convention is not deprecated yet, but it will be in the future.

* Deprecated the second and third argument of `KernelInterface::locateResource`
Expand All @@ -148,6 +148,7 @@ HttpKernel
fallback directories. Resources like service definitions are usually loaded relative to the
current directory or with a glob pattern. The fallback directories have never been advocated
so you likely do not use those in any app based on the SF Standard or Flex edition.
* Getting the container from a non-booted kernel is deprecated

Lock
----
Expand All @@ -171,7 +172,7 @@ MonologBridge
--------------

* The `RouteProcessor` has been marked final.

Process
-------

Expand All @@ -195,14 +196,14 @@ Security
* Implementations of `PasswordEncoderInterface` and `UserPasswordEncoderInterface` should add a new `needsRehash()` method
* Deprecated returning a non-boolean value when implementing `Guard\AuthenticatorInterface::checkCredentials()`. Please explicitly return `false` to indicate invalid credentials.
* Deprecated passing more than one attribute to `AccessDecisionManager::decide()` and `AuthorizationChecker::isGranted()` (and indirectly the `is_granted()` Twig and ExpressionLanguage function)

**Before**
```php
if ($this->authorizationChecker->isGranted(['ROLE_USER', 'ROLE_ADMIN'])) {
// ...
}
```

**After**
```php
if ($this->authorizationChecker->isGranted(new Expression("has_role('ROLE_USER') or has_role('ROLE_ADMIN')"))) {}
Expand Down Expand Up @@ -230,18 +231,18 @@ TwigBridge
* Deprecated to pass `$rootDir` and `$fileLinkFormatter` as 5th and 6th argument respectively to the
`DebugCommand::__construct()` method, swap the variables position.
* Deprecated accepting STDIN implicitly when using the `lint:twig` command, use `lint:twig -` (append a dash) instead to make it explicit.

TwigBundle
----------

* Deprecated `twig.exception_controller` configuration option, set it to "null" and use `framework.error_controller` instead:

Before:
```yaml
twig:
exception_controller: 'App\Controller\MyExceptionController'
```

After:
```yaml
twig:
Expand All @@ -250,36 +251,36 @@ TwigBundle
framework:
error_controller: 'App\Controller\MyExceptionController'
```
The new default exception controller will also change the error response content according to

The new default exception controller will also change the error response content according to
https://tools.ietf.org/html/rfc7807 for `json`, `xml`, `atom` and `txt` formats:

Before:
```json
{
"error": {
"code": 404,
"message": "Sorry, the page you are looking for could not be found"
}
{
"error": {
"code": 404,
"message": "Sorry, the page you are looking for could not be found"
}
}
```

After:
```json
{
{
"title": "Not Found",
"status": 404,
"status": 404,
"detail": "Sorry, the page you are looking for could not be found"
}
```

* Deprecated the `ExceptionController` and `PreviewErrorController` controllers, use `ErrorController` from the HttpKernel component instead
* Deprecated all built-in error templates, use the error renderer mechanism of the `ErrorRenderer` component
* Deprecated loading custom error templates in non-html formats. Custom HTML error pages based on Twig keep working as before:
* Deprecated loading custom error templates in non-html formats. Custom HTML error pages based on Twig keep working as before:

Before (`templates/bundles/TwigBundle/Exception/error.jsonld.twig`):
```twig
{
{
"@id": "https://example.com",
"@type": "error",
"@context": {
Expand All @@ -289,7 +290,7 @@ TwigBundle
}
}
```

After (`App\ErrorRenderer\JsonLdErrorRenderer`):
```php
class JsonLdErrorRenderer implements ErrorRendererInterface
Expand All @@ -298,7 +299,7 @@ TwigBundle
{
return 'jsonld';
}

public function render(FlattenException $exception): string
{
return json_encode([
Expand All @@ -323,7 +324,7 @@ Validator
* Deprecated using anything else than a `string` as the code of a `ConstraintViolation`, a `string` type-hint will
be added to the constructor of the `ConstraintViolation` class and to the `ConstraintViolationBuilder::setCode()`
method in 5.0.
* Deprecated passing an `ExpressionLanguage` instance as the second argument of `ExpressionValidator::__construct()`.
* Deprecated passing an `ExpressionLanguage` instance as the second argument of `ExpressionValidator::__construct()`.
Pass it as the first argument instead.
* The `Length` constraint expects the `allowEmptyString` option to be defined
when the `min` option is used.
Expand All @@ -343,7 +344,7 @@ WebServerBundle
---------------

* The bundle is deprecated and will be removed in 5.0.

Yaml
----

Expand Down
1 change: 1 addition & 0 deletions UPGRADE-5.0.md
Expand Up @@ -288,6 +288,7 @@ HttpFoundation
use `Symfony\Component\Mime\FileinfoMimeTypeGuesser` instead.
* `ApacheRequest` has been removed, use the `Request` class instead.
* The third argument of the `HeaderBag::get()` method has been removed, use method `all()` instead.
* Getting the container from a non-booted kernel is not possible anymore.

HttpKernel
----------
Expand Down
14 changes: 9 additions & 5 deletions src/Symfony/Bundle/FrameworkBundle/Test/KernelTestCase.php
Expand Up @@ -127,11 +127,15 @@ protected static function createKernel(array $options = [])
protected static function ensureKernelShutdown()
{
if (null !== static::$kernel) {
$container = static::$kernel->getContainer();
static::$kernel->shutdown();
static::$booted = false;
if ($container instanceof ResetInterface) {
$container->reset();
$isBooted = (new \ReflectionClass(static::$kernel))->getProperty('booted');
$isBooted->setAccessible(true);
if ($isBooted->getValue(static::$kernel)) {
$container = static::$kernel->getContainer();
static::$kernel->shutdown();
static::$booted = false;
if ($container instanceof ResetInterface) {
$container->reset();
}
}
}
static::$container = null;
Expand Down
Expand Up @@ -14,6 +14,7 @@
use Psr\Log\NullLogger;
use Symfony\Component\Config\Loader\LoaderInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\Filesystem\Filesystem;
use Symfony\Component\HttpKernel\Kernel;

Expand Down Expand Up @@ -96,4 +97,13 @@ protected function getKernelParameters(): array

return $parameters;
}

public function getContainer(): ContainerInterface
{
if (!$this->booted) {
throw new \LogicException('Cannot access the container on a non-booted kernel. Did you forget to boot it?');
}

return parent::getContainer();
}
}
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Bundle\SecurityBundle\Tests\Functional\app;

use Symfony\Component\Config\Loader\LoaderInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\Filesystem\Filesystem;
use Symfony\Component\HttpKernel\Kernel;

Expand Down Expand Up @@ -98,4 +99,13 @@ protected function getKernelParameters(): array

return $parameters;
}

public function getContainer(): ContainerInterface
{
if (!$this->booted) {
throw new \LogicException('Cannot access the container on a non-booted kernel. Did you forget to boot it?');
}

return parent::getContainer();
}
}
1 change: 1 addition & 0 deletions src/Symfony/Component/HttpKernel/CHANGELOG.md
Expand Up @@ -14,6 +14,7 @@ CHANGELOG
so you likely do not use those in any app based on the SF Standard or Flex edition.
* Marked all dispatched event classes as `@final`
* Added `ErrorController` to enable the preview and error rendering mechanism
* Getting the container from a non-booted kernel is deprecated.

4.3.0
-----
Expand Down
4 changes: 4 additions & 0 deletions src/Symfony/Component/HttpKernel/Kernel.php
Expand Up @@ -380,6 +380,10 @@ public function getProjectDir()
*/
public function getContainer()
{
if (!$this->booted) {
@trigger_error('Getting the container from a non-booted kernel is deprecated since Symfony 4.4.', E_USER_DEPRECATED);
}

return $this->container;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/HttpKernel/KernelInterface.php
Expand Up @@ -124,7 +124,7 @@ public function getRootDir();
/**
* Gets the current container.
*
* @return ContainerInterface|null A ContainerInterface instance or null when the Kernel is shutdown
* @return ContainerInterface
*/
public function getContainer();

Expand Down
24 changes: 23 additions & 1 deletion src/Symfony/Component/HttpKernel/Tests/KernelTest.php
Expand Up @@ -46,6 +46,17 @@ public function testConstructor()
$this->assertEquals($debug, $kernel->isDebug());
$this->assertFalse($kernel->isBooted());
$this->assertLessThanOrEqual(microtime(true), $kernel->getStartTime());
}

/**
* @group legacy
* @expectedDeprecation Getting the container from a non-booted kernel is deprecated since Symfony 4.4.
*/
public function testGetContainerForANonBootedKernel()
{
$kernel = new KernelForTest('test_env', true);

$this->assertFalse($kernel->isBooted());
$this->assertNull($kernel->getContainer());
}

Expand All @@ -61,7 +72,6 @@ public function testClone()
$this->assertEquals($debug, $clone->isDebug());
$this->assertFalse($clone->isBooted());
$this->assertLessThanOrEqual(microtime(true), $clone->getStartTime());
$this->assertNull($clone->getContainer());
}

public function testClassNameValidityGetter()
Expand Down Expand Up @@ -486,6 +496,18 @@ public function testTerminateReturnsSilentlyIfKernelIsNotBooted()
$kernel->terminate(Request::create('/'), new Response());
}

/**
* @group legacy
* @expectedDeprecation Getting the container from a non-booted kernel is deprecated since Symfony 4.4.
*/
public function testDeprecatedNullKernel()
{
$kernel = $this->getKernel();
$kernel->shutdown();

$this->assertNull($kernel->getContainer());
}

public function testTerminateDelegatesTerminationOnlyForTerminableInterface()
{
// does not implement TerminableInterface
Expand Down

0 comments on commit 979be29

Please sign in to comment.