Skip to content

Commit

Permalink
bug symfony#53057 [HttpKernel] Move @internal from `AbstractSessi…
Browse files Browse the repository at this point in the history
…onListener` class to its methods and properties (Florian-Merle)

This PR was merged into the 5.4 branch.

Discussion
----------

[HttpKernel] Move ``@internal`` from `AbstractSessionListener` class to its methods and properties

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        |
| License       | MIT

The `AbstractSessionListener` being marked as internal, its public constant `NO_AUTO_CACHE_CONTROL_HEADER` should not be used while the documentation [states it can](https://symfony.com/doc/current/http_cache.html#http-caching-and-user-sessions).

In fact, static analysis tools like psalm says there is an error with code using this constant.
```
ERROR: InternalClass - xxx.php:32:33 - Symfony\Component\HttpKernel\EventListener\AbstractSessionListener is internal to Symfony but called from xxx (see https://psalm.dev/174)
        $response->headers->set(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER, 'true');
```

~~Another solution is to make every method of the `AbstractSessionListener`  internal but this means the class could be extended.~~
~~Also, maybe the class `AbstractSessionListener` should not be internal, but I don't think so.~~
~~This is why I introduced a new interface that is not internal and allows to not introduce BC.~~

~~The documentation will need to be updated if this pull request is merged, I'd be happy to do it later.~~

As discussed, I made public/protected properties and methods internal and removed the original internal mark on the class.
This solves the issue and allows us to use the `AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER` const just like the documentation says we can.

Commits
-------

defe229 [HttpKernel] Move `@internal` from AbstractSessionListener class to its methods and properties
  • Loading branch information
fabpot committed Dec 17, 2023
2 parents 50ec299 + defe229 commit 1de8768
Showing 1 changed file with 27 additions and 2 deletions.
Expand Up @@ -36,13 +36,15 @@
*
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
* @author Tobias Schultze <http://tobion.de>
*
* @internal
*/
abstract class AbstractSessionListener implements EventSubscriberInterface, ResetInterface
{
public const NO_AUTO_CACHE_CONTROL_HEADER = 'Symfony-Session-NoAutoCacheControl';


/**
* @internal
*/
protected $container;
private $sessionUsageStack = [];
private $debug;
Expand All @@ -52,13 +54,19 @@ abstract class AbstractSessionListener implements EventSubscriberInterface, Rese
*/
private $sessionOptions;

/**
* @internal
*/
public function __construct(ContainerInterface $container = null, bool $debug = false, array $sessionOptions = [])
{
$this->container = $container;
$this->debug = $debug;
$this->sessionOptions = $sessionOptions;
}

/**
* @internal
*/
public function onKernelRequest(RequestEvent $event)
{
if (!$event->isMainRequest()) {
Expand Down Expand Up @@ -94,6 +102,9 @@ public function onKernelRequest(RequestEvent $event)
$this->sessionUsageStack[] = $session instanceof Session ? $session->getUsageIndex() : 0;
}

/**
* @internal
*/
public function onKernelResponse(ResponseEvent $event)
{
if (!$event->isMainRequest() || (!$this->container->has('initialized_session') && !$event->getRequest()->hasSession())) {
Expand Down Expand Up @@ -222,13 +233,19 @@ public function onKernelResponse(ResponseEvent $event)
}
}

/**
* @internal
*/
public function onFinishRequest(FinishRequestEvent $event)
{
if ($event->isMainRequest()) {
array_pop($this->sessionUsageStack);
}
}

/**
* @internal
*/
public function onSessionUsage(): void
{
if (!$this->debug) {
Expand Down Expand Up @@ -264,6 +281,9 @@ public function onSessionUsage(): void
throw new UnexpectedSessionUsageException('Session was used while the request was declared stateless.');
}

/**
* @internal
*/
public static function getSubscribedEvents(): array
{
return [
Expand All @@ -274,6 +294,9 @@ public static function getSubscribedEvents(): array
];
}

/**
* @internal
*/
public function reset(): void
{
if (\PHP_SESSION_ACTIVE === session_status()) {
Expand All @@ -291,6 +314,8 @@ public function reset(): void
/**
* Gets the session object.
*
* @internal
*
* @return SessionInterface|null
*/
abstract protected function getSession();
Expand Down

0 comments on commit 1de8768

Please sign in to comment.