From f666657342a1b6fcd249ca4cdda115338e9b1602 Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Wed, 8 Apr 2015 14:50:34 +0200 Subject: [PATCH 1/2] [Translator] Cache does not take fallback locales into consideration --- .../Translation/Tests/TranslatorCacheTest.php | 32 +++++++++++++++++++ .../Component/Translation/Translator.php | 7 +++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/Translation/Tests/TranslatorCacheTest.php b/src/Symfony/Component/Translation/Tests/TranslatorCacheTest.php index 24514b73dc..fdbb4f48e6 100644 --- a/src/Symfony/Component/Translation/Tests/TranslatorCacheTest.php +++ b/src/Symfony/Component/Translation/Tests/TranslatorCacheTest.php @@ -11,6 +11,7 @@ namespace Symfony\Component\Translation\Tests; +use Symfony\Component\Translation\Loader\ArrayLoader; use Symfony\Component\Translation\Translator; use Symfony\Component\Translation\MessageCatalogue; use Symfony\Component\Translation\MessageSelector; @@ -164,6 +165,37 @@ class TranslatorCacheTest extends \PHPUnit_Framework_TestCase } } + public function testDifferentCacheFilesAreUsedForDifferentSetsOfFallbackLocales() + { + /* + * Because the cache file contains a catalogue including all of its fallback + * catalogues (either "inlined" in Symfony 2.7 production or "standalone"), + * we must take the active set of fallback locales into consideration when + * loading a catalogue from the cache. + */ + $translator = new Translator('a', null, $this->tmpDir); + $translator->setFallbackLocales(array('b')); + + $translator->addLoader('array', new ArrayLoader()); + $translator->addResource('array', array('foo' => 'foo (a)'), 'a'); + $translator->addResource('array', array('bar' => 'bar (b)'), 'b'); + + $this->assertEquals('bar (b)', $translator->trans('bar')); + + // Remove fallback locale + $translator->setFallbackLocales(array()); + $this->assertEquals('bar', $translator->trans('bar')); + + // Use a fresh translator with no fallback locales, result should be the same + $translator = new Translator('a', null, $this->tmpDir); + + $translator->addLoader('array', new ArrayLoader()); + $translator->addResource('array', array('foo' => 'foo (a)'), 'a'); + $translator->addResource('array', array('bar' => 'bar (b)'), 'b'); + + $this->assertEquals('bar', $translator->trans('bar')); + } + protected function getCatalogue($locale, $messages) { $catalogue = new MessageCatalogue($locale); diff --git a/src/Symfony/Component/Translation/Translator.php b/src/Symfony/Component/Translation/Translator.php index c0d3629b36..ca9b4054f7 100644 --- a/src/Symfony/Component/Translation/Translator.php +++ b/src/Symfony/Component/Translation/Translator.php @@ -372,7 +372,7 @@ class Translator implements TranslatorInterface, TranslatorBagInterface } $this->assertValidLocale($locale); - $cacheFile = $this->cacheDir.'/catalogue.'.$locale.'.php'; + $cacheFile = $this->getCatalogueCachePath($locale); $self = $this; // required for PHP 5.3 where "$this" cannot be use()d in anonymous functions. Change in Symfony 3.0. $cache = $this->getConfigCacheFactory()->cache($cacheFile, function (ConfigCacheInterface $cache) use ($self, $locale) { @@ -503,6 +503,11 @@ EOF return sha1(serialize($this->resources[$locale])); } + private function getCatalogueCachePath($locale) + { + return $this->cacheDir.'/catalogue.'.$locale.'.'.sha1(serialize($this->fallbackLocales)).'.php'; + } + private function doLoadCatalogue($locale) { $this->catalogues[$locale] = new MessageCatalogue($locale); From f9939d80f399f11e0f05210a7bc16d0b6aa9ecbe Mon Sep 17 00:00:00 2001 From: Abdellatif Ait boudad Date: Thu, 9 Apr 2015 00:14:28 +0100 Subject: [PATCH 2/2] [Translation] avoid freshness check based on content *inside* the cache. --- .../Tests/Translation/TranslatorTest.php | 11 +++- .../Component/Translation/Translator.php | 55 ++++--------------- 2 files changed, 18 insertions(+), 48 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Translation/TranslatorTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Translation/TranslatorTest.php index ed3f0e0fda..8d331a638b 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Translation/TranslatorTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Translation/TranslatorTest.php @@ -255,12 +255,17 @@ class TranslatorTest extends \PHPUnit_Framework_TestCase __DIR__.'/../Fixtures/Resources/translations/messages.fr.yml', ), ); + $catalogueHash = sha1(serialize(array( + 'resources' => array(), + 'fallback_locales' => array(), + ))); // prime the cache - $translator = $this->getTranslator($loader, array('cache_dir' => $this->tmpDir, 'resource_files' => $resourceFiles), 'yml'); - $this->assertFalse(file_exists($this->tmpDir.'/catalogue.fr.php')); + $translator = $this->getTranslator($loader, array('cache_dir' => $this->tmpDir, 'resource_files' => $resourceFiles), 'yml'); + + $this->assertFalse(file_exists($this->tmpDir.'/catalogue.fr.'.$catalogueHash.'.php')); $translator->warmup($this->tmpDir); - $this->assertTrue(file_exists($this->tmpDir.'/catalogue.fr.php')); + $this->assertTrue(file_exists($this->tmpDir.'/catalogue.fr.'.$catalogueHash.'.php')); } } diff --git a/src/Symfony/Component/Translation/Translator.php b/src/Symfony/Component/Translation/Translator.php index ca9b4054f7..d62bfb6873 100644 --- a/src/Symfony/Component/Translation/Translator.php +++ b/src/Symfony/Component/Translation/Translator.php @@ -372,9 +372,8 @@ class Translator implements TranslatorInterface, TranslatorBagInterface } $this->assertValidLocale($locale); - $cacheFile = $this->getCatalogueCachePath($locale); $self = $this; // required for PHP 5.3 where "$this" cannot be use()d in anonymous functions. Change in Symfony 3.0. - $cache = $this->getConfigCacheFactory()->cache($cacheFile, + $cache = $this->getConfigCacheFactory()->cache($this->getCatalogueCachePath($locale), function (ConfigCacheInterface $cache) use ($self, $locale) { $self->dumpCatalogue($locale, $cache); } @@ -386,40 +385,12 @@ class Translator implements TranslatorInterface, TranslatorBagInterface } /* Read catalogue from cache. */ - $catalogue = include $cache->getPath(); - - /* - * Gracefully handle the case when the cached catalogue is in an "old" format, without a resourcesHash - */ - $resourcesHash = null; - if (is_array($catalogue)) { - list($catalogue, $resourcesHash) = $catalogue; - } - - if ($this->debug && $resourcesHash !== $this->getResourcesHash($locale)) { - /* - * This approach of resource checking has the disadvantage that a second - * type of freshness check happens based on content *inside* the cache, while - * the idea of ConfigCache is to make this check transparent to the client (and keeps - * the resources in a .meta file). - * - * Thus, we might run into the unfortunate situation that we just thought (a few lines above) - * that the cache is fresh -- and now that we look into it, we figure it's not. - * - * For now, just unlink the cache and try again. See - * https://github.com/symfony/symfony/pull/11862#issuecomment-54634631 and/or - * https://github.com/symfony/symfony/issues/7176 for possible better approaches. - */ - unlink($cacheFile); - $this->initializeCacheCatalogue($locale); - } else { - /* Initialize with catalogue from cache. */ - $this->catalogues[$locale] = $catalogue; - } + $this->catalogues[$locale] = include $cache->getPath(); } /** * This method is public because it needs to be callable from a closure in PHP 5.3. It should be made protected (or even private, if possible) in 3.0. + * * @internal */ public function dumpCatalogue($locale, ConfigCacheInterface $cache) @@ -432,15 +403,13 @@ class Translator implements TranslatorInterface, TranslatorBagInterface use Symfony\Component\Translation\MessageCatalogue; -\$resourcesHash = '%s'; \$catalogue = new MessageCatalogue('%s', %s); %s -return array(\$catalogue, \$resourcesHash); +return \$catalogue; EOF , - $this->getResourcesHash($locale), $locale, var_export($this->catalogues[$locale]->all(), true), $fallbackContent @@ -494,18 +463,14 @@ EOF return $fallbackContent; } - private function getResourcesHash($locale) - { - if (!isset($this->resources[$locale])) { - return ''; - } - - return sha1(serialize($this->resources[$locale])); - } - private function getCatalogueCachePath($locale) { - return $this->cacheDir.'/catalogue.'.$locale.'.'.sha1(serialize($this->fallbackLocales)).'.php'; + $catalogueHash = sha1(serialize(array( + 'resources' => isset($this->resources[$locale]) ? $this->resources[$locale] : array(), + 'fallback_locales' => $this->fallbackLocales, + ))); + + return $this->cacheDir.'/catalogue.'.$locale.'.'.$catalogueHash.'.php'; } private function doLoadCatalogue($locale)