Skip to content

[HttpKernel] Dont implement ServiceSubscriberInterface on *SessionListener #22207

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.

Sign up for GitHub

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
Mar 31, 2017
Merged

[HttpKernel] Dont implement ServiceSubscriberInterface on *SessionListener #22207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 8 additions & 2 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/session.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,14 @@

<service id="session_listener" class="Symfony\Component\HttpKernel\EventListener\SessionListener">
<tag name="kernel.event_subscriber" />
<tag name="container.service_subscriber" id="session" />
<argument type="service" id="container" />
<argument type="service">
<service class="Symfony\Component\DependencyInjection\ServiceLocator">
<tag name="container.service_locator" />
<argument type="collection">
<argument key="session" type="service" id="session" on-invalid="ignore" />
</argument>
</service>
</argument>
</service>

<service id="session.save_listener" class="Symfony\Component\HttpKernel\EventListener\SaveSessionListener">
Expand Down
10 changes: 8 additions & 2 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/test.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,14 @@

<service id="test.session.listener" class="Symfony\Component\HttpKernel\EventListener\TestSessionListener">
<tag name="kernel.event_subscriber" />
<tag name="container.service_subscriber" id="session" />
<argument type="service" id="container" />
<argument type="service">
<service class="Symfony\Component\DependencyInjection\ServiceLocator">
<tag name="container.service_locator" />
Copy link
Member

Choose a reason for hiding this comment

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

AFAIK, tagging anonymous services inside an argument does not work, as this Definition is not registered directly in the container (well, I think the XMLFileLoader converts inlined definition to normal private services with a random id, which is why it works here, but creating an inlined definition in PHP would not work as it would stay inline). Should we keep it like this ?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's an issue when using findTaggedServiceIds, but ServiceLocatorTagPass visits recursively all definitions by using AbstractRecursivePass, so it works :)

<argument type="collection">
<argument key="session" type="service" id="session" on-invalid="ignore" />
</argument>
</service>
</argument>
</service>
</services>
</container>
14 changes: 1 addition & 13 deletions src/Symfony/Component/HttpKernel/EventListener/SessionListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
namespace Symfony\Component\HttpKernel\EventListener;

use Psr\Container\ContainerInterface;
use Symfony\Component\DependencyInjection\ServiceSubscriberInterface;
use Symfony\Component\HttpFoundation\Session\SessionInterface;

/**
* Sets the session in the request.
Expand All @@ -22,7 +20,7 @@
*
* @final since version 3.3
*/
class SessionListener extends AbstractSessionListener implements ServiceSubscriberInterface
class SessionListener extends AbstractSessionListener
{
private $container;

Expand All @@ -39,14 +37,4 @@ protected function getSession()

return $this->container->get('session');
}

/**
* {@inheritdoc}
*/
public static function getSubscribedServices()
{
return array(
'session' => '?'.SessionInterface::class,
);
}
}
14 changes: 1 addition & 13 deletions src/Symfony/Component/HttpKernel/EventListener/TestSessionListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
namespace Symfony\Component\HttpKernel\EventListener;

use Psr\Container\ContainerInterface;
use Symfony\Component\DependencyInjection\ServiceSubscriberInterface;
use Symfony\Component\HttpFoundation\Session\SessionInterface;

/**
* Sets the session in the request.
Expand All @@ -22,7 +20,7 @@
*
* @final since version 3.3
*/
class TestSessionListener extends AbstractTestSessionListener implements ServiceSubscriberInterface
class TestSessionListener extends AbstractTestSessionListener
{
private $container;

Expand All @@ -39,14 +37,4 @@ protected function getSession()

return $this->container->get('session');
}

/**
* {@inheritdoc}
*/
public static function getSubscribedServices()
{
return array(
'session' => '?'.SessionInterface::class,
);
}
}
12 changes: 12 additions & 0 deletions src/Symfony/Component/HttpKernel/Tests/EventListener/TestSessionListenerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@
namespace Symfony\Component\HttpKernel\Tests\EventListener;

use PHPUnit\Framework\TestCase;
use Symfony\Component\DependencyInjection\ServiceSubscriberInterface;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
use Symfony\Component\HttpKernel\EventListener\SessionListener;
use Symfony\Component\HttpKernel\EventListener\TestSessionListener;
use Symfony\Component\HttpFoundation\Session\SessionInterface;

/**
Expand Down Expand Up @@ -79,6 +82,15 @@ public function testUnstartedSessionIsNotSave()
$this->filterResponse(new Request());
}

public function testDoesNotImplementServiceSubscriberInterface()
{
$this->assertTrue(interface_exists(ServiceSubscriberInterface::class));
$this->assertTrue(class_exists(SessionListener::class));
$this->assertTrue(class_exists(TestSessionListener::class));
$this->assertFalse(is_subclass_of(SessionListener::class, ServiceSubscriberInterface::class), 'Implementing ServiceSubscriberInterface would create a dep on the DI component, which eg Silex cannot afford');
$this->assertFalse(is_subclass_of(TestSessionListener::class, ServiceSubscriberInterface::class, 'Implementing ServiceSubscriberInterface would create a dep on the DI component, which eg Silex cannot afford'));
}

private function filterResponse(Request $request, $type = HttpKernelInterface::MASTER_REQUEST)
{
$request->setSession($this->session);
Expand Down