From 5f3df596ccb6429ba05fe1b7f1fdb2c88f5ee060 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Sat, 3 Jun 2017 23:06:14 +0200 Subject: [PATCH 1/5] Add test to demonstrate #23034 --- .../Tests/Translation/TranslatorTest.php | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Translation/TranslatorTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Translation/TranslatorTest.php index 361bcf001b560..c5142cbc01a2b 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Translation/TranslatorTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Translation/TranslatorTest.php @@ -135,6 +135,51 @@ public function testGetDefaultLocale() $this->assertSame('en', $translator->getLocale()); } + /** @dataProvider getDebugModeAndCacheDirCombinations */ + public function testResourceFilesOptionLoadsBeforeOtherAddedResources($debug, $enableCache) + { + $someCatalogue = $this->getCatalogue('some_locale', array()); + + $loader = $this->getMockBuilder('Symfony\Component\Translation\Loader\LoaderInterface')->getMock(); + + $loader->expects($this->at(0)) + ->method('load') + /* The "messages.some_locale.loader" is passed via the resource_file option and shall be loaded first */ + ->with('messages.some_locale.loader', 'some_locale', 'messages') + ->willReturn($someCatalogue); + + $loader->expects($this->at(1)) + ->method('load') + /* This resource is added by an addResource() call and shall be loaded after the resource_files */ + ->with('second_resource.some_locale.loader', 'some_locale', 'messages') + ->willReturn($someCatalogue); + + $options = array( + 'resource_files' => array('some_locale' => array('messages.some_locale.loader')), + 'debug' => $debug + ); + + if ($enableCache) { + $options['cache_dir'] = $this->tmpDir; + } + + /** @var Translator $translator */ + $translator = $this->createTranslator($loader, $options); + $translator->addResource('loader', 'second_resource.some_locale.loader', 'some_locale', 'messages'); + + $translator->trans('some_message', array(), null, 'some_locale'); + } + + public function getDebugModeAndCacheDirCombinations() + { + return array( + array(false, false), + array(true, false), + array(false, true), + array(true, true), + ); + } + protected function getCatalogue($locale, $messages, $resources = array()) { $catalogue = new MessageCatalogue($locale); From ddc7a61d2624312b6793e64dac44380b9f4653a9 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Sat, 3 Jun 2017 23:24:46 +0200 Subject: [PATCH 2/5] Fix resource loading order inconsistency reported in #23034 --- .../Translation/Translator.php | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Translation/Translator.php b/src/Symfony/Bundle/FrameworkBundle/Translation/Translator.php index fba6d70d6978b..a4ef3c694dcec 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Translation/Translator.php +++ b/src/Symfony/Bundle/FrameworkBundle/Translation/Translator.php @@ -37,6 +37,15 @@ class Translator extends BaseTranslator implements WarmableInterface */ private $resourceLocales; + /** + * Holds parameters from addResource() calls so we can add these resources + * after the ones passed in the resource_files option (which are added + * lazily in loadResources() as well). + * + * @var array + */ + private $otherResources = array(); + /** * Constructor. * @@ -93,6 +102,11 @@ public function warmUp($cacheDir) } } + public function addResource($format, $resource, $locale, $domain = null) + { + $this->otherResources[] = array($format, $resource, $locale, $domain); + } + /** * {@inheritdoc} */ @@ -118,9 +132,15 @@ private function loadResources() foreach ($files as $key => $file) { // filename is domain.locale.format list($domain, $locale, $format) = explode('.', basename($file), 3); - $this->addResource($format, $file, $locale, $domain); + parent::addResource($format, $file, $locale, $domain); unset($this->options['resource_files'][$locale][$key]); } } + + foreach ($this->otherResources as $key => $params) { + list($format, $resource, $locale, $domain) = $params; + parent::addResource($format, $resource, $locale, $domain); + unset($this->otherResources[$key]); + } } } From 82f004112488edd654de41c3551fef3d9ba894ee Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Sat, 3 Jun 2017 23:30:14 +0200 Subject: [PATCH 3/5] fabbot.io --- .../Bundle/FrameworkBundle/Tests/Translation/TranslatorTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Translation/TranslatorTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Translation/TranslatorTest.php index c5142cbc01a2b..75f4281952227 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Translation/TranslatorTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Translation/TranslatorTest.php @@ -156,7 +156,7 @@ public function testResourceFilesOptionLoadsBeforeOtherAddedResources($debug, $e $options = array( 'resource_files' => array('some_locale' => array('messages.some_locale.loader')), - 'debug' => $debug + 'debug' => $debug, ); if ($enableCache) { From 31c245dfd5e16395d116c158b9502d6d16ca8e4f Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Mon, 5 Jun 2017 15:33:19 +0200 Subject: [PATCH 4/5] Clean up and simplify lazy resource initialization --- .../Translation/Translator.php | 33 ++++++++----------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Translation/Translator.php b/src/Symfony/Bundle/FrameworkBundle/Translation/Translator.php index a4ef3c694dcec..7914afd37afc5 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Translation/Translator.php +++ b/src/Symfony/Bundle/FrameworkBundle/Translation/Translator.php @@ -38,13 +38,12 @@ class Translator extends BaseTranslator implements WarmableInterface private $resourceLocales; /** - * Holds parameters from addResource() calls so we can add these resources - * after the ones passed in the resource_files option (which are added - * lazily in loadResources() as well). + * Holds parameters from addResource() calls so we can defer the acutal + * parent::addResource() calls until initialize() is executed. * * @var array */ - private $otherResources = array(); + private $resources = array(); /** * Constructor. @@ -74,9 +73,7 @@ public function __construct(ContainerInterface $container, MessageSelector $sele $this->options = array_merge($this->options, $options); $this->resourceLocales = array_keys($this->options['resource_files']); - if (null !== $this->options['cache_dir'] && $this->options['debug']) { - $this->loadResources(); - } + $this->addResourceFiles($this->options['resource_files']); parent::__construct($container->getParameter('kernel.default_locale'), $selector, $this->options['cache_dir'], $this->options['debug']); } @@ -104,7 +101,7 @@ public function warmUp($cacheDir) public function addResource($format, $resource, $locale, $domain = null) { - $this->otherResources[] = array($format, $resource, $locale, $domain); + $this->resources[] = array($format, $resource, $locale, $domain); } /** @@ -118,7 +115,12 @@ protected function initializeCatalogue($locale) protected function initialize() { - $this->loadResources(); + foreach ($this->resources as $key => $params) { + list($format, $resource, $locale, $domain) = $params; + parent::addResource($format, $resource, $locale, $domain); + } + $this->resources = array(); + foreach ($this->loaderIds as $id => $aliases) { foreach ($aliases as $alias) { $this->addLoader($alias, $this->container->get($id)); @@ -126,21 +128,14 @@ protected function initialize() } } - private function loadResources() + private function addResourceFiles($filesByLocale) { - foreach ($this->options['resource_files'] as $locale => $files) { + foreach ($filesByLocale as $locale => $files) { foreach ($files as $key => $file) { // filename is domain.locale.format list($domain, $locale, $format) = explode('.', basename($file), 3); - parent::addResource($format, $file, $locale, $domain); - unset($this->options['resource_files'][$locale][$key]); + $this->addResource($format, $file, $locale, $domain); } } - - foreach ($this->otherResources as $key => $params) { - list($format, $resource, $locale, $domain) = $params; - parent::addResource($format, $resource, $locale, $domain); - unset($this->otherResources[$key]); - } } } From 2260f67c74d6554126fe0c29b2ea9dd0d2f19df5 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Thu, 8 Jun 2017 10:27:01 +0200 Subject: [PATCH 5/5] Fix tpyo --- src/Symfony/Bundle/FrameworkBundle/Translation/Translator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Translation/Translator.php b/src/Symfony/Bundle/FrameworkBundle/Translation/Translator.php index 7914afd37afc5..9fdfb645d372a 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Translation/Translator.php +++ b/src/Symfony/Bundle/FrameworkBundle/Translation/Translator.php @@ -38,7 +38,7 @@ class Translator extends BaseTranslator implements WarmableInterface private $resourceLocales; /** - * Holds parameters from addResource() calls so we can defer the acutal + * Holds parameters from addResource() calls so we can defer the actual * parent::addResource() calls until initialize() is executed. * * @var array