Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
moved management of the locale from the Session class to the Request …
…class

The locale management does not require sessions anymore.

In the Symfony2 spirit, the locale should be part of your URLs. If this is the case
(via the special _locale request attribute), Symfony will store it in the request
(getLocale()).

This feature is now also configurable/replaceable at will as everything is now managed
by the new LocaleListener event listener.

How to upgrade:

The default locale configuration has been moved from session to the main configuration:

Before:

framework:
    session:
        default_locale: en

After:

framework:
    default_locale: en

Whenever you want to get the current locale, call getLocale() on the request (was on the
session before).
  • Loading branch information
fabpot committed Oct 8, 2011
1 parent 8b55541 commit 74bc699
Show file tree
Hide file tree
Showing 26 changed files with 264 additions and 152 deletions.
1 change: 1 addition & 0 deletions CHANGELOG-2.1.md
Expand Up @@ -59,6 +59,7 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c

### HttpFoundation

* [BC BREAK] moved management of the locale from the Session class to the Request class
* added a generic access to the PHP built-in filter mechanism: ParameterBag::filter()
* made FileBinaryMimeTypeGuesser command configurable
* added Request::getUser() and Request::getPassword()
Expand Down
43 changes: 36 additions & 7 deletions UPGRADE-2.1.md
Expand Up @@ -3,11 +3,40 @@ UPGRADE FROM 2.0 to 2.1

* assets_base_urls and base_urls merging strategy has changed

Unlike most configuration blocks, successive values for
``assets_base_urls`` will overwrite each other instead of being merged.
This behavior was chosen because developers will typically define base
URL's for each environment. Given that most projects tend to inherit
configurations (e.g. ``config_test.yml`` imports ``config_dev.yml``)
and/or share a common base configuration (i.e. ``config.yml``), merging
could yield a set of base URL's for multiple environments.
Unlike most configuration blocks, successive values for
``assets_base_urls`` will overwrite each other instead of being merged.
This behavior was chosen because developers will typically define base
URL's for each environment. Given that most projects tend to inherit
configurations (e.g. ``config_test.yml`` imports ``config_dev.yml``)
and/or share a common base configuration (i.e. ``config.yml``), merging
could yield a set of base URL's for multiple environments.

* moved management of the locale from the Session class to the Request class

Configuring the default locale:

Before:

framework:
session:
default_locale: fr

After:

framework:
default_locale: fr

Retrieving the locale from a Twig template:

Before: {{ app.request.session.locale }}

This comment has been minimized.

Copy link
@Seldaek

Seldaek Oct 18, 2011

Member

I currently do app.session.locale which also works fine btw

After: {{ app.request.locale }}

Retrieving the locale from a PHP template:

Before: $view['session']->getLocale()
After: $view['request']->getLocale()

Retrieving the locale from PHP code:

Before: $session->getLocale()
After: $request->getLocale()
Expand Up @@ -51,6 +51,7 @@ public function getConfigTreeBuilder()
->scalarNode('secret')->isRequired()->end()
->scalarNode('ide')->defaultNull()->end()
->booleanNode('test')->end()
->scalarNode('default_locale')->defaultValue('en')->end()
->end()
;

Expand Down Expand Up @@ -161,7 +162,6 @@ private function addSessionSection(ArrayNodeDefinition $rootNode)
->canBeUnset()
->children()
->booleanNode('auto_start')->defaultFalse()->end()
->scalarNode('default_locale')->defaultValue('en')->end()
->scalarNode('storage_id')->defaultValue('session.storage.native')->end()
->scalarNode('name')->end()
->scalarNode('lifetime')->end()
Expand Down
Expand Up @@ -65,6 +65,8 @@ public function load(array $configs, ContainerBuilder $container)

$container->setParameter('kernel.trust_proxy_headers', $config['trust_proxy_headers']);

$container->setParameter('kernel.default_locale', $config['default_locale']);

if (!empty($config['test'])) {
$loader->load('test.xml');
}
Expand Down Expand Up @@ -280,7 +282,6 @@ private function registerSessionConfiguration(array $config, ContainerBuilder $c

// session
$container->getDefinition('session_listener')->addArgument($config['auto_start']);
$container->setParameter('session.default_locale', $config['default_locale']);

// session storage
$container->setAlias('session.storage', $config['storage_id']);
Expand Down
@@ -0,0 +1,63 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bundle\FrameworkBundle\EventListener;

use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\Routing\RouterInterface;

/**
* Initializes the locale based on the current request.
*
* @author Fabien Potencier <fabien@symfony.com>
*/
class LocaleListener
{
private $router;
private $defaultLocale;

public function __construct($defaultLocale = 'en', RouterInterface $router = null)
{
$this->defaultLocale = $defaultLocale;
$this->router = $router;
}

public function onEarlyKernelRequest(GetResponseEvent $event)
{
$request = $event->getRequest();
if ($request->hasPreviousSession()) {

This comment has been minimized.

Copy link
@kriswallsmith

kriswallsmith Oct 11, 2011

Contributor

This is being called before the session listener has a chance to call setSession... therefore this is always returning false.

This comment has been minimized.

Copy link
@abdeltiflouardi

abdeltiflouardi Oct 18, 2011

yes of course, would you change Session and Locale listener.

$request->setDefaultLocale($request->getSession()->get('_locale', $this->defaultLocale));
} else {
$request->setDefaultLocale($this->defaultLocale);
}
}

public function onKernelRequest(GetResponseEvent $event)
{
if (HttpKernelInterface::MASTER_REQUEST !== $event->getRequestType()) {
return;
}

$request = $event->getRequest();
if ($locale = $request->attributes->get('_locale')) {
$request->setLocale($locale);

if ($request->hasPreviousSession()) {
$request->getSession()->set('_locale', $request->getLocale());
}
}

if (null !== $this->router) {
$this->router->getContext()->setParameter('_locale', $request->getLocale());
}
}
}
Expand Up @@ -88,19 +88,6 @@ public function onKernelRequest(GetResponseEvent $event)

throw new MethodNotAllowedHttpException($e->getAllowedMethods(), $message, $e);
}

if (HttpKernelInterface::MASTER_REQUEST === $event->getRequestType()) {
$context = $this->router->getContext();
$session = $request->getSession();
if ($locale = $request->attributes->get('_locale')) {
if ($session) {
$session->setLocale($locale);
}
$context->setParameter('_locale', $locale);
} elseif ($session) {
$context->setParameter('_locale', $session->getLocale());
}
}
}

private function parametersToString(array $parameters)
Expand Down
Expand Up @@ -25,6 +25,7 @@
<xsd:attribute name="trust-proxy-headers" type="xsd:string" />
<xsd:attribute name="ide" type="xsd:string" />
<xsd:attribute name="secret" type="xsd:string" />
<xsd:attribute name="default-locale" type="xsd:string" />
</xsd:complexType>

<xsd:complexType name="form">
Expand Down Expand Up @@ -72,7 +73,6 @@

<xsd:complexType name="session">
<xsd:attribute name="storage-id" type="xsd:string" />
<xsd:attribute name="default-locale" type="xsd:string" />
<xsd:attribute name="name" type="xsd:string" />
<xsd:attribute name="lifetime" type="xsd:integer" />
<xsd:attribute name="path" type="xsd:string" />
Expand Down
Expand Up @@ -14,7 +14,6 @@
<services>
<service id="session" class="%session.class%">
<argument type="service" id="session.storage" />
<argument>%session.default_locale%</argument>
</service>

<service id="session.storage.native" class="%session.storage.native.class%" public="false">
Expand Down
Expand Up @@ -37,7 +37,6 @@
<argument key="debug">%kernel.debug%</argument>
<argument key="charset">%kernel.charset%</argument>
</argument>
<argument type="service" id="session" on-invalid="ignore" />
</service>

<service id="translator" class="%translator.identity.class%">
Expand Down
8 changes: 8 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/web.xml
Expand Up @@ -8,6 +8,7 @@
<parameter key="controller_resolver.class">Symfony\Bundle\FrameworkBundle\Controller\ControllerResolver</parameter>
<parameter key="controller_name_converter.class">Symfony\Bundle\FrameworkBundle\Controller\ControllerNameParser</parameter>
<parameter key="response_listener.class">Symfony\Component\HttpKernel\EventListener\ResponseListener</parameter>
<parameter key="locale_listener.class">Symfony\Bundle\FrameworkBundle\EventListener\LocaleListener</parameter>
</parameters>

<services>
Expand All @@ -27,5 +28,12 @@
<tag name="kernel.event_listener" event="kernel.response" method="onKernelResponse" />
<argument>%kernel.charset%</argument>
</service>

<service id="locale_listener" class="%locale_listener.class%">
<tag name="kernel.event_listener" event="kernel.request" method="onEarlyKernelRequest" priority="255" />
<tag name="kernel.event_listener" event="kernel.request" method="onKernelRequest" priority="-1" />
<argument>%kernel.default_locale%</argument>
<argument type="service" id="router" on-invalid="ignore" />
</service>
</services>
</container>
Expand Up @@ -46,6 +46,16 @@ public function getParameter($key, $default = null)
return $this->request->get($key, $default);
}

/**
* Returns the locale
*
* @return string
*/
public function getLocale()
{
return $this->request->getLocale();
}

/**
* Returns the canonical name of this helper.
*
Expand Down
Expand Up @@ -46,16 +46,6 @@ public function get($name, $default = null)
return $this->session->get($name, $default);
}

/**
* Returns the locale
*
* @return string
*/
public function getLocale()
{
return $this->session->getLocale();
}

public function getFlash($name, $default = null)
{
return $this->session->getFlash($name, $default);
Expand Down
Expand Up @@ -2,6 +2,7 @@

$container->loadFromExtension('framework', array(
'secret' => 's3cr3t',
'default_locale' => 'fr',
'form' => null,
'csrf_protection' => array(
'enabled' => true,
Expand All @@ -19,7 +20,6 @@
),
'session' => array(
'auto_start' => true,
'default_locale' => 'fr',
'storage_id' => 'session.storage.native',
'name' => '_SYMFONY',
'lifetime' => 86400,
Expand Down
Expand Up @@ -6,13 +6,13 @@
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd
http://symfony.com/schema/dic/symfony http://symfony.com/schema/dic/symfony/symfony-1.0.xsd">

<framework:config secret="s3cr3t" ide="file%%link%%format">
<framework:config secret="s3cr3t" ide="file%%link%%format" default-locale="fr">
<framework:csrf-protection enabled="true" field-name="_csrf" />
<framework:form />
<framework:esi enabled="true" />
<framework:profiler only-exceptions="true" />
<framework:router resource="%kernel.root_dir%/config/routing.xml" type="xml" />
<framework:session auto-start="true" default-locale="fr" storage-id="session.storage.native" name="_SYMFONY" lifetime="86400" path="/" domain="example.com" secure="true" httponly="true" />
<framework:session auto-start="true" storage-id="session.storage.native" name="_SYMFONY" lifetime="86400" path="/" domain="example.com" secure="true" httponly="true" />
<framework:templating assets-version="SomeVersionScheme" cache="/path/to/cache" >
<framework:loader>loader.foo</framework:loader>
<framework:loader>loader.bar</framework:loader>
Expand Down
@@ -1,5 +1,6 @@
framework:
secret: s3cr3t
default_locale: fr
form: ~
csrf_protection:
enabled: true
Expand All @@ -13,7 +14,6 @@ framework:
type: xml
session:
auto_start: true
default_locale: fr
storage_id: session.storage.native
name: _SYMFONY
lifetime: 86400
Expand Down
Expand Up @@ -76,8 +76,7 @@ public function testSession()
$container = $this->createContainerFromFile('full');

$this->assertTrue($container->hasDefinition('session'), '->registerSessionConfiguration() loads session.xml');
$this->assertEquals('fr', $container->getParameter('session.default_locale'));
$this->assertEquals('%session.default_locale%', $container->getDefinition('session')->getArgument(1));
$this->assertEquals('fr', $container->getParameter('kernel.default_locale'));
$this->assertTrue($container->getDefinition('session_listener')->getArgument(1));
$this->assertEquals('session.storage.native', (string) $container->getAlias('session.storage'));

Expand Down

2 comments on commit 74bc699

@schmittjoh
Copy link
Contributor

Choose a reason for hiding this comment

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

The Request doesn't feel like the correct object, a container-scoped context holder would be a better fit imo.

This change has the side-effect that users now have to think about the scope of their services if they need the locale. They cannot just change the dependency from the session to the request, doing that will likely lead to a scope error, and cause some wtfs.

@mahono
Copy link

@mahono mahono commented on 74bc699 Oct 11, 2011

Choose a reason for hiding this comment

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

maybe the session should still have the locale setter/getter for BC to keep upgrading easy?

Please sign in to comment.