Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[2.7][Translation] avoid freshness check based on content *inside* the cache #14281

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 9, 2015
Merged

[2.7][Translation] avoid freshness check based on content *inside* the cache #14281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f666657
[Translator] Cache does not take fallback locales into consideration
mpdude Apr 8, 2015
f9939d8
[Translation] avoid freshness check based on content *inside* the cache.
aitboudad Apr 8, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions 11 src/Symfony/Bundle/FrameworkBundle/Tests/Translation/TranslatorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
}
}

Expand Down
32 changes: 32 additions & 0 deletions 32 src/Symfony/Component/Translation/Tests/TranslatorCacheTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
50 changes: 10 additions & 40 deletions 50 src/Symfony/Component/Translation/Translator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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(),
Copy link
Contributor

Choose a reason for hiding this comment

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

@aitboudad While backporting the Translator test case refactorings to the 2.6 branch, I noticed that the test for this fails because we don't have that fix on 2.6 yet (different resources still use same cache).

Is that something you would want to fix on 2.6 as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no IMO it's a new feature.

'fallback_locales' => $this->fallbackLocales,
)));

return sha1(serialize($this->resources[$locale]));
return $this->cacheDir.'/catalogue.'.$locale.'.'.$catalogueHash.'.php';
}

private function doLoadCatalogue($locale)
Expand Down
Morty Proxy This is a proxified and sanitized view of the page, visit original site.