minor symfony#14281 [2.7][Translation] avoid freshness check based on…

… content *inside* the cache (mpdude, aitboudad) This PR was merged into the 2.7 branch. Discussion ---------- [2.7][Translation] avoid freshness check based on content *inside* the cache | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Fixed tickets | ~ | Tests pass? | yes | License | MIT it's mainly based on symfony#14268 Commits ------- f9939d8 [Translation] avoid freshness check based on content *inside* the cache. f666657 [Translator] Cache does not take fallback locales into consideration
watermanio · Apr 9, 2015 · b5da2ae · b5da2ae
2 parents 94eb384 + f9939d8
commit b5da2ae
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 43 deletions.
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Translation/TranslatorTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Translation/TranslatorTest.php
@@ -255,12 +255,17 @@ public function testWarmup()
                 __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/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 @@ public function testLoadCatalogueWithCachingWithInvalidLocale()
         }
     }
 
+    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
@@ -372,9 +372,8 @@ private function initializeCacheCatalogue($locale)
         }
 
         $this->assertValidLocale($locale);
-        $cacheFile = $this->cacheDir.'/catalogue.'.$locale.'.php';
         $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 @@ function (ConfigCacheInterface $cache) use ($self, $locale) {
         }
 
         /* 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 @@ public function dumpCatalogue($locale, ConfigCacheInterface $cache)
 
 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,13 +463,14 @@ private function getFallbackContent(MessageCatalogue $catalogue)
         return $fallbackContent;
     }
 
-    private function getResourcesHash($locale)
+    private function getCatalogueCachePath($locale)
     {
-        if (!isset($this->resources[$locale])) {
-            return '';
-        }
+        $catalogueHash = sha1(serialize(array(
+            'resources' => isset($this->resources[$locale]) ? $this->resources[$locale] : array(),
+            'fallback_locales' => $this->fallbackLocales,
+        )));
 
-        return sha1(serialize($this->resources[$locale]));
+        return $this->cacheDir.'/catalogue.'.$locale.'.'.$catalogueHash.'.php';
     }
 
     private function doLoadCatalogue($locale)