Skip to content

Commit

Permalink
[translation][performances] move loading resources into Translator in…
Browse files Browse the repository at this point in the history 
…itialize.
  • Loading branch information
@aitboudad @fabpot
aitboudad authored and fabpot committed Mar 13, 2015
1 parent 6963887 commit bf5c583
Show file tree
Hide file tree
Showing 6 changed files with 89 additions and 27 deletions.
12 changes: 10 additions & 2 deletions src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
View file
Expand Up @@ -40,6 +40,7 @@ class FrameworkExtension extends Extension
*
* @param array $configs
* @param ContainerBuilder $container
*
* @throws LogicException
*/
public function load(array $configs, ContainerBuilder $container)
Expand Down Expand Up @@ -686,6 +687,8 @@ private function registerTranslatorConfiguration(array $config, ContainerBuilder
foreach ($dirs as $dir) {
$container->addResource(new DirectoryResource($dir));
}

$files = array();
$finder = Finder::create()
->files()
->filter(function (\SplFileInfo $file) {
Expand All @@ -695,10 +698,15 @@ private function registerTranslatorConfiguration(array $config, ContainerBuilder
;

foreach ($finder as $file) {
// filename is domain.locale.format
list($domain, $locale, $format) = explode('.', $file->getBasename(), 3);
$translator->addMethodCall('addResource', array($format, (string) $file, $locale, $domain));
if (!isset($files[$locale])) {
$files[$locale] = array();
}

$files[$locale][] = (string) $file;
}

$translator->replaceArgument(4, $files);
}
}

Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bundle/FrameworkBundle/Resources/config/translation.xml
View file
Expand Up @@ -44,6 +44,7 @@
<argument key="cache_dir">%kernel.cache_dir%/translations</argument>
<argument key="debug">%kernel.debug%</argument>
</argument>
<argument type="collection" /> <!-- translation resources -->
</service>

<service id="translator.logging" class="Symfony\Component\Translation\LoggingTranslator" public="false">
Expand Down
11 changes: 2 additions & 9 deletions src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php
View file
Expand Up @@ -218,18 +218,11 @@ public function testAssets()
public function testTranslator()
{
$container = $this->createContainerFromFile('full');

$this->assertTrue($container->hasDefinition('translator.default'), '->registerTranslatorConfiguration() loads translation.xml');
$this->assertEquals('translator.default', (string) $container->getAlias('translator'), '->registerTranslatorConfiguration() redefines translator service from identity to real translator');
$resources = $container->getDefinition('translator.default')->getArgument(4);

$resources = array();
foreach ($container->getDefinition('translator.default')->getMethodCalls() as $call) {
if ('addResource' == $call[0]) {
$resources[] = $call[1];
}
}

$files = array_map(function ($resource) { return realpath($resource[1]); }, $resources);
$files = array_map(function ($resource) { return realpath($resource); }, $resources['en']);
$ref = new \ReflectionClass('Symfony\Component\Validator\Validation');
$this->assertContains(
strtr(dirname($ref->getFileName()).'/Resources/translations/validators.en.xlf', '/', DIRECTORY_SEPARATOR),
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bundle/FrameworkBundle/Tests/Fixtures/Resources/translations/messages.fr.yml
View file
@@ -0,0 +1 @@
folder: répertoire
62 changes: 51 additions & 11 deletions src/Symfony/Bundle/FrameworkBundle/Tests/Translation/TranslatorTest.php
View file
Expand Up @@ -95,13 +95,50 @@ public function testTransWithCaching()
public function testTransWithCachingWithInvalidLocale()
{
$loader = $this->getMock('Symfony\Component\Translation\Loader\LoaderInterface');
$translator = $this->getTranslator($loader, array('cache_dir' => $this->tmpDir), '\Symfony\Bundle\FrameworkBundle\Tests\Translation\TranslatorWithInvalidLocale');
$translator = $this->getTranslator($loader, array('cache_dir' => $this->tmpDir), array(), 'loader', '\Symfony\Bundle\FrameworkBundle\Tests\Translation\TranslatorWithInvalidLocale');
$translator->setLocale('invalid locale');

$this->setExpectedException('\InvalidArgumentException');
$translator->trans('foo');
}

public function testLoadRessourcesWithCaching()
{
$loader = new \Symfony\Component\Translation\Loader\YamlFileLoader();
$resourceFiles = array(
'fr' => array(
__DIR__.'/../Fixtures/Resources/translations/messages.fr.yml',
),
);

// prime the cache
$translator = $this->getTranslator($loader, array('cache_dir' => $this->tmpDir), $resourceFiles, 'yml');
$translator->setLocale('fr');

$this->assertEquals('répertoire', $translator->trans('folder'));

// do it another time as the cache is primed now
$translator = $this->getTranslator($loader, array('cache_dir' => $this->tmpDir), array(), 'yml');
$translator->setLocale('fr');

$this->assertEquals('répertoire', $translator->trans('folder'));
}

public function testLoadRessourcesWithoutCaching()
{
$loader = new \Symfony\Component\Translation\Loader\YamlFileLoader();
$resourceFiles = array(
'fr' => array(
__DIR__.'/../Fixtures/Resources/translations/messages.fr.yml',
),
);

$translator = $this->getTranslator($loader, array(), $resourceFiles, 'yml');
$translator->setLocale('fr');

$this->assertEquals('répertoire', $translator->trans('folder'));
}

protected function getCatalogue($locale, $messages)
{
$catalogue = new MessageCatalogue($locale);
Expand Down Expand Up @@ -182,22 +219,25 @@ protected function getContainer($loader)
return $container;
}

public function getTranslator($loader, $options = array(), $translatorClass = '\Symfony\Bundle\FrameworkBundle\Translation\Translator')
public function getTranslator($loader, $options = array(), $resources = array(), $loaderFomat = 'loader', $translatorClass = '\Symfony\Bundle\FrameworkBundle\Translation\Translator')
{
$translator = new $translatorClass(
$this->getContainer($loader),
new MessageSelector(),
array('loader' => array('loader')),
$options
array($loaderFomat => array($loaderFomat)),
$options,
$resources
);

$translator->addResource('loader', 'foo', 'fr');
$translator->addResource('loader', 'foo', 'en');
$translator->addResource('loader', 'foo', 'es');
$translator->addResource('loader', 'foo', 'pt-PT'); // European Portuguese
$translator->addResource('loader', 'foo', 'pt_BR'); // Brazilian Portuguese
$translator->addResource('loader', 'foo', 'fr.UTF-8');
$translator->addResource('loader', 'foo', 'sr@latin'); // Latin Serbian
if ('loader' === $loaderFomat) {
$translator->addResource('loader', 'foo', 'fr');
$translator->addResource('loader', 'foo', 'en');
$translator->addResource('loader', 'foo', 'es');
$translator->addResource('loader', 'foo', 'pt-PT'); // European Portuguese
$translator->addResource('loader', 'foo', 'pt_BR'); // Brazilian Portuguese
$translator->addResource('loader', 'foo', 'fr.UTF-8');
$translator->addResource('loader', 'foo', 'sr@latin'); // Latin Serbian
}

return $translator;
}
Expand Down
29 changes: 24 additions & 5 deletions src/Symfony/Bundle/FrameworkBundle/Translation/Translator.php
View file
Expand Up @@ -24,6 +24,7 @@ class Translator extends BaseTranslator
{
protected $container;
protected $loaderIds;
protected $resourceFiles;

protected $options = array(
'cache_dir' => null,
Expand All @@ -38,17 +39,19 @@ class Translator extends BaseTranslator
* * cache_dir: The cache directory (or null to disable caching)
* * debug: Whether to enable debugging or not (false by default)
*
* @param ContainerInterface $container A ContainerInterface instance
* @param MessageSelector $selector The message selector for pluralization
* @param array $loaderIds An array of loader Ids
* @param array $options An array of options
* @param ContainerInterface $container A ContainerInterface instance
* @param MessageSelector $selector The message selector for pluralization
* @param array $loaderIds An array of loader Ids
* @param array $options An array of options
* @param array $resourceFiles An array of resource directories
*
* @throws \InvalidArgumentException
*/
public function __construct(ContainerInterface $container, MessageSelector $selector, $loaderIds = array(), array $options = array())
public function __construct(ContainerInterface $container, MessageSelector $selector, $loaderIds = array(), array $options = array(), $resourceFiles = array())
{
$this->container = $container;
$this->loaderIds = $loaderIds;
$this->resourceFiles = $resourceFiles;

// check option names
if ($diff = array_diff(array_keys($options), array_keys($this->options))) {
Expand All @@ -66,6 +69,7 @@ public function __construct(ContainerInterface $container, MessageSelector $sele
protected function initializeCatalogue($locale)
{
$this->initialize();
$this->loadResources($locale);
parent::initializeCatalogue($locale);
}

Expand All @@ -77,4 +81,19 @@ protected function initialize()
}
}
}

private function loadResources($locale)
{
$locales = array_merge(array($locale), $this->computeFallbackLocales($locale));
foreach ($locales as $locale) {
if (isset($this->resourceFiles[$locale])) {
foreach ($this->resourceFiles[$locale] as $file) {
// filename is domain.locale.format
list($domain, $locale, $format) = explode('.', basename($file), 3);
$this->addResource($format, $file, $locale, $domain);
}
unset($this->resourceFiles[$locale]);
}
}
}
}

3 comments on commit bf5c583

@oscherler
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this change (I think), I’m now unable to add translation resources from an event listener (or from anywhere short of a compiler pass, for that matter,) because addResource doesn’t do anything useful anymore: everything seems to be done when instantiating the class, and there’s no way to rebuild the catalogues in case you add resources later-on. I’m really stuck.

@xabbuh
Copy link
Member

@xabbuh xabbuh commented on bf5c583 Jun 19, 2018

Choose a reason for hiding this comment

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

Please do not comment on commits as your input is like to get lost, but open a new issue instead.

@oscherler
Copy link
Contributor

Choose a reason for hiding this comment

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

Oops, sorry. Here’s the issue: #27654.

Please sign in to comment.