bug #29220 [Translation] make intl+icu format seamless by handling it in MessageCatalogue (nicolas-grekas)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[Translation] make intl+icu format seamless by handling it in MessageCatalogue

| Q             | A
| ------------- | ---
| Branch?       | 4.2
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #29136
| License       | MIT
| Doc PR        | -

Commits
-------

c71dfb9673 [Translation] make intl+icu format seamless by handling it in MessageCatalogue
This commit is contained in:
Nicolas Grekas 2018-11-15 12:53:51 +01:00
commit ef7545444a
11 changed files with 159 additions and 30 deletions

View File

@ -11,6 +11,8 @@
namespace Symfony\Component\Translation\Catalogue;
use Symfony\Component\Translation\MessageCatalogueInterface;
/**
* Merge operation between two catalogues as follows:
* all = source target = {x: x source x target}
@ -32,10 +34,11 @@ class MergeOperation extends AbstractOperation
'new' => array(),
'obsolete' => array(),
);
$intlDomain = $domain.MessageCatalogueInterface::INTL_DOMAIN_SUFFIX;
foreach ($this->source->all($domain) as $id => $message) {
$this->messages[$domain]['all'][$id] = $message;
$this->result->add(array($id => $message), $domain);
$this->result->add(array($id => $message), $this->source->defines($id, $intlDomain) ? $intlDomain : $domain);
if (null !== $keyMetadata = $this->source->getMetadata($id, $domain)) {
$this->result->setMetadata($id, $keyMetadata, $domain);
}
@ -45,7 +48,7 @@ class MergeOperation extends AbstractOperation
if (!$this->source->has($id, $domain)) {
$this->messages[$domain]['all'][$id] = $message;
$this->messages[$domain]['new'][$id] = $message;
$this->result->add(array($id => $message), $domain);
$this->result->add(array($id => $message), $this->target->defines($id, $intlDomain) ? $intlDomain : $domain);
if (null !== $keyMetadata = $this->target->getMetadata($id, $domain)) {
$this->result->setMetadata($id, $keyMetadata, $domain);
}

View File

@ -11,6 +11,8 @@
namespace Symfony\Component\Translation\Catalogue;
use Symfony\Component\Translation\MessageCatalogueInterface;
/**
* Target operation between two catalogues:
* intersection = source target = {x: x source x target}
@ -33,6 +35,7 @@ class TargetOperation extends AbstractOperation
'new' => array(),
'obsolete' => array(),
);
$intlDomain = $domain.MessageCatalogueInterface::INTL_DOMAIN_SUFFIX;
// For 'all' messages, the code can't be simplified as ``$this->messages[$domain]['all'] = $target->all($domain);``,
// because doing so will drop messages like {x: x ∈ source ∧ x ∉ target.all ∧ x ∈ target.fallback}
@ -46,7 +49,7 @@ class TargetOperation extends AbstractOperation
foreach ($this->source->all($domain) as $id => $message) {
if ($this->target->has($id, $domain)) {
$this->messages[$domain]['all'][$id] = $message;
$this->result->add(array($id => $message), $domain);
$this->result->add(array($id => $message), $this->target->defines($id, $intlDomain) ? $intlDomain : $domain);
if (null !== $keyMetadata = $this->source->getMetadata($id, $domain)) {
$this->result->setMetadata($id, $keyMetadata, $domain);
}
@ -59,7 +62,7 @@ class TargetOperation extends AbstractOperation
if (!$this->source->has($id, $domain)) {
$this->messages[$domain]['all'][$id] = $message;
$this->messages[$domain]['new'][$id] = $message;
$this->result->add(array($id => $message), $domain);
$this->result->add(array($id => $message), $this->target->defines($id, $intlDomain) ? $intlDomain : $domain);
if (null !== $keyMetadata = $this->target->getMetadata($id, $domain)) {
$this->result->setMetadata($id, $keyMetadata, $domain);
}

View File

@ -76,7 +76,26 @@ abstract class FileDumper implements DumperInterface
throw new RuntimeException(sprintf('Unable to create directory "%s".', $directory));
}
}
// save file
$intlDomain = $domain.MessageCatalogue::INTL_DOMAIN_SUFFIX;
$intlMessages = $messages->all($intlDomain);
if ($intlMessages) {
$intlPath = $options['path'].'/'.$this->getRelativePath($intlDomain, $messages->getLocale());
file_put_contents($intlPath, $this->formatCatalogue($messages, $intlDomain, $options));
$messages->replace(array(), $intlDomain);
try {
if ($messages->all($domain)) {
file_put_contents($fullpath, $this->formatCatalogue($messages, $domain, $options));
}
continue;
} finally {
$messages->replace($intlMessages, $intlDomain);
}
}
file_put_contents($fullpath, $this->formatCatalogue($messages, $domain, $options));
}
}

View File

@ -18,8 +18,6 @@ namespace Symfony\Component\Translation\Formatter;
*/
interface IntlFormatterInterface
{
const DOMAIN_SUFFIX = '+intl-icu';
/**
* Formats a localized message using rules defined by ICU MessageFormat.
*

View File

@ -49,7 +49,17 @@ class MessageCatalogue implements MessageCatalogueInterface, MetadataAwareInterf
*/
public function getDomains()
{
return array_keys($this->messages);
$domains = array();
$suffixLength = \strlen(self::INTL_DOMAIN_SUFFIX);
foreach ($this->messages as $domain => $messages) {
if (\strlen($domain) > $suffixLength && false !== $i = strpos($domain, self::INTL_DOMAIN_SUFFIX, -$suffixLength)) {
$domain = substr($domain, 0, $i);
}
$domains[$domain] = $domain;
}
return array_values($domains);
}
/**
@ -57,11 +67,23 @@ class MessageCatalogue implements MessageCatalogueInterface, MetadataAwareInterf
*/
public function all($domain = null)
{
if (null === $domain) {
return $this->messages;
if (null !== $domain) {
return ($this->messages[$domain.self::INTL_DOMAIN_SUFFIX] ?? array()) + ($this->messages[$domain] ?? array());
}
return isset($this->messages[$domain]) ? $this->messages[$domain] : array();
$allMessages = array();
$suffixLength = \strlen(self::INTL_DOMAIN_SUFFIX);
foreach ($this->messages as $domain => $messages) {
if (\strlen($domain) > $suffixLength && false !== $i = strpos($domain, self::INTL_DOMAIN_SUFFIX, -$suffixLength)) {
$domain = substr($domain, 0, $i);
$allMessages[$domain] = $messages + ($allMessages[$domain] ?? array());
} else {
$allMessages[$domain] = ($allMessages[$domain] ?? array()) + $messages;
}
}
return $allMessages;
}
/**
@ -77,7 +99,7 @@ class MessageCatalogue implements MessageCatalogueInterface, MetadataAwareInterf
*/
public function has($id, $domain = 'messages')
{
if (isset($this->messages[$domain][$id])) {
if (isset($this->messages[$domain][$id]) || isset($this->messages[$domain.self::INTL_DOMAIN_SUFFIX][$id])) {
return true;
}
@ -93,7 +115,7 @@ class MessageCatalogue implements MessageCatalogueInterface, MetadataAwareInterf
*/
public function defines($id, $domain = 'messages')
{
return isset($this->messages[$domain][$id]);
return isset($this->messages[$domain][$id]) || isset($this->messages[$domain.self::INTL_DOMAIN_SUFFIX][$id]);
}
/**
@ -101,6 +123,10 @@ class MessageCatalogue implements MessageCatalogueInterface, MetadataAwareInterf
*/
public function get($id, $domain = 'messages')
{
if (isset($this->messages[$domain.self::INTL_DOMAIN_SUFFIX][$id])) {
return $this->messages[$domain.self::INTL_DOMAIN_SUFFIX][$id];
}
if (isset($this->messages[$domain][$id])) {
return $this->messages[$domain][$id];
}
@ -117,7 +143,7 @@ class MessageCatalogue implements MessageCatalogueInterface, MetadataAwareInterf
*/
public function replace($messages, $domain = 'messages')
{
$this->messages[$domain] = array();
unset($this->messages[$domain], $this->messages[$domain.self::INTL_DOMAIN_SUFFIX]);
$this->add($messages, $domain);
}
@ -144,6 +170,10 @@ class MessageCatalogue implements MessageCatalogueInterface, MetadataAwareInterf
}
foreach ($catalogue->all() as $domain => $messages) {
if ($intlMessages = $catalogue->all($domain.self::INTL_DOMAIN_SUFFIX)) {
$this->add($intlMessages, $domain.self::INTL_DOMAIN_SUFFIX);
$messages = array_diff_key($messages, $intlMessages);
}
$this->add($messages, $domain);
}

View File

@ -20,6 +20,8 @@ use Symfony\Component\Config\Resource\ResourceInterface;
*/
interface MessageCatalogueInterface
{
const INTL_DOMAIN_SUFFIX = '+intl-icu';
/**
* Gets the catalogue locale.
*

View File

@ -53,6 +53,20 @@ class MergeOperationTest extends AbstractOperationTest
);
}
public function testGetResultFromIntlDomain()
{
$this->assertEquals(
new MessageCatalogue('en', array(
'messages' => array('a' => 'old_a', 'b' => 'old_b'),
'messages+intl-icu' => array('d' => 'old_d', 'c' => 'new_c'),
)),
$this->createOperation(
new MessageCatalogue('en', array('messages' => array('a' => 'old_a', 'b' => 'old_b'), 'messages+intl-icu' => array('d' => 'old_d'))),
new MessageCatalogue('en', array('messages+intl-icu' => array('a' => 'new_a', 'c' => 'new_c')))
)->getResult()
);
}
public function testGetResultWithMetadata()
{
$leftCatalogue = new MessageCatalogue('en', array('messages' => array('a' => 'old_a', 'b' => 'old_b')));

View File

@ -53,6 +53,20 @@ class TargetOperationTest extends AbstractOperationTest
);
}
public function testGetResultFromIntlDomain()
{
$this->assertEquals(
new MessageCatalogue('en', array(
'messages' => array('a' => 'old_a'),
'messages+intl-icu' => array('c' => 'new_c'),
)),
$this->createOperation(
new MessageCatalogue('en', array('messages' => array('a' => 'old_a'), 'messages+intl-icu' => array('b' => 'old_b'))),
new MessageCatalogue('en', array('messages' => array('a' => 'new_a'), 'messages+intl-icu' => array('c' => 'new_c')))
)->getResult()
);
}
public function testGetResultWithMetadata()
{
$leftCatalogue = new MessageCatalogue('en', array('messages' => array('a' => 'old_a', 'b' => 'old_b')));

View File

@ -32,6 +32,30 @@ class FileDumperTest extends TestCase
@unlink($tempDir.'/messages.en.concrete');
}
public function testDumpIntl()
{
$tempDir = sys_get_temp_dir();
$catalogue = new MessageCatalogue('en');
$catalogue->add(array('foo' => 'bar'), 'd1');
$catalogue->add(array('bar' => 'foo'), 'd1+intl-icu');
$catalogue->add(array('bar' => 'foo'), 'd2+intl-icu');
$dumper = new ConcreteFileDumper();
@unlink($tempDir.'/d2.en.concrete');
$dumper->dump($catalogue, array('path' => $tempDir));
$this->assertStringEqualsFile($tempDir.'/d1.en.concrete', 'foo=bar');
@unlink($tempDir.'/d1.en.concrete');
$this->assertStringEqualsFile($tempDir.'/d1+intl-icu.en.concrete', 'bar=foo');
@unlink($tempDir.'/d1+intl-icu.en.concrete');
$this->assertFileNotExists($tempDir.'/d2.en.concrete');
$this->assertStringEqualsFile($tempDir.'/d2+intl-icu.en.concrete', 'bar=foo');
@unlink($tempDir.'/d2+intl-icu.en.concrete');
}
public function testDumpCreatesNestedDirectoriesAndFile()
{
$tempDir = sys_get_temp_dir();
@ -56,7 +80,7 @@ class ConcreteFileDumper extends FileDumper
{
public function formatCatalogue(MessageCatalogue $messages, $domain, array $options = array())
{
return '';
return http_build_query($messages->all($domain), '', '&');
}
protected function getExtension()

View File

@ -25,9 +25,9 @@ class MessageCatalogueTest extends TestCase
public function testGetDomains()
{
$catalogue = new MessageCatalogue('en', array('domain1' => array(), 'domain2' => array()));
$catalogue = new MessageCatalogue('en', array('domain1' => array(), 'domain2' => array(), 'domain2+intl-icu' => array(), 'domain3+intl-icu' => array()));
$this->assertEquals(array('domain1', 'domain2'), $catalogue->getDomains());
$this->assertEquals(array('domain1', 'domain2', 'domain3'), $catalogue->getDomains());
}
public function testAll()
@ -37,24 +37,43 @@ class MessageCatalogueTest extends TestCase
$this->assertEquals(array('foo' => 'foo'), $catalogue->all('domain1'));
$this->assertEquals(array(), $catalogue->all('domain88'));
$this->assertEquals($messages, $catalogue->all());
$messages = array('domain1+intl-icu' => array('foo' => 'bar')) + $messages + array(
'domain2+intl-icu' => array('bar' => 'foo'),
'domain3+intl-icu' => array('biz' => 'biz'),
);
$catalogue = new MessageCatalogue('en', $messages);
$this->assertEquals(array('foo' => 'bar'), $catalogue->all('domain1'));
$this->assertEquals(array('bar' => 'foo'), $catalogue->all('domain2'));
$this->assertEquals(array('biz' => 'biz'), $catalogue->all('domain3'));
$messages = array(
'domain1' => array('foo' => 'bar'),
'domain2' => array('bar' => 'foo'),
'domain3' => array('biz' => 'biz'),
);
$this->assertEquals($messages, $catalogue->all());
}
public function testHas()
{
$catalogue = new MessageCatalogue('en', array('domain1' => array('foo' => 'foo'), 'domain2' => array('bar' => 'bar')));
$catalogue = new MessageCatalogue('en', array('domain1' => array('foo' => 'foo'), 'domain2+intl-icu' => array('bar' => 'bar')));
$this->assertTrue($catalogue->has('foo', 'domain1'));
$this->assertTrue($catalogue->has('bar', 'domain2'));
$this->assertFalse($catalogue->has('bar', 'domain1'));
$this->assertFalse($catalogue->has('foo', 'domain88'));
}
public function testGetSet()
{
$catalogue = new MessageCatalogue('en', array('domain1' => array('foo' => 'foo'), 'domain2' => array('bar' => 'bar')));
$catalogue = new MessageCatalogue('en', array('domain1' => array('foo' => 'foo'), 'domain2' => array('bar' => 'bar'), 'domain2+intl-icu' => array('bar' => 'foo')));
$catalogue->set('foo1', 'foo1', 'domain1');
$this->assertEquals('foo', $catalogue->get('foo', 'domain1'));
$this->assertEquals('foo1', $catalogue->get('foo1', 'domain1'));
$this->assertEquals('foo', $catalogue->get('bar', 'domain2'));
}
public function testAdd()
@ -75,7 +94,7 @@ class MessageCatalogueTest extends TestCase
public function testReplace()
{
$catalogue = new MessageCatalogue('en', array('domain1' => array('foo' => 'foo'), 'domain2' => array('bar' => 'bar')));
$catalogue = new MessageCatalogue('en', array('domain1' => array('foo' => 'foo'), 'domain1+intl-icu' => array('bar' => 'bar')));
$catalogue->replace($messages = array('foo1' => 'foo1'), 'domain1');
$this->assertEquals($messages, $catalogue->all('domain1'));
@ -89,16 +108,18 @@ class MessageCatalogueTest extends TestCase
$r1 = $this->getMockBuilder('Symfony\Component\Config\Resource\ResourceInterface')->getMock();
$r1->expects($this->any())->method('__toString')->will($this->returnValue('r1'));
$catalogue = new MessageCatalogue('en', array('domain1' => array('foo' => 'foo'), 'domain2' => array('bar' => 'bar')));
$catalogue = new MessageCatalogue('en', array('domain1' => array('foo' => 'foo')));
$catalogue->addResource($r);
$catalogue1 = new MessageCatalogue('en', array('domain1' => array('foo1' => 'foo1')));
$catalogue1 = new MessageCatalogue('en', array('domain1' => array('foo1' => 'foo1'), 'domain2+intl-icu' => array('bar' => 'bar')));
$catalogue1->addResource($r1);
$catalogue->addCatalogue($catalogue1);
$this->assertEquals('foo', $catalogue->get('foo', 'domain1'));
$this->assertEquals('foo1', $catalogue->get('foo1', 'domain1'));
$this->assertEquals('bar', $catalogue->get('bar', 'domain2'));
$this->assertEquals('bar', $catalogue->get('bar', 'domain2+intl-icu'));
$this->assertEquals(array($r, $r1), $catalogue->getResources());
}

View File

@ -203,14 +203,7 @@ class Translator implements LegacyTranslatorInterface, TranslatorInterface, Tran
$id = (string) $id;
$catalogue = $this->getCatalogue($locale);
$locale = $catalogue->getLocale();
$intlDomain = $this->hasIntlFormatter ? $domain.IntlFormatterInterface::DOMAIN_SUFFIX : null;
while (true) {
if (null !== $intlDomain && $catalogue->defines($id, $intlDomain)) {
return $this->formatter->formatIntl($catalogue->get($id, $intlDomain), $locale, $parameters);
}
if ($catalogue->defines($id, $domain)) {
break;
}
while (!$catalogue->defines($id, $domain)) {
if ($cat = $catalogue->getFallbackCatalogue()) {
$catalogue = $cat;
$locale = $catalogue->getLocale();
@ -219,6 +212,10 @@ class Translator implements LegacyTranslatorInterface, TranslatorInterface, Tran
}
}
if ($this->hasIntlFormatter && $catalogue->defines($id, $domain.MessageCatalogue::INTL_DOMAIN_SUFFIX)) {
return $this->formatter->formatIntl($catalogue->get($id, $domain), $locale, $parameters);
}
return $this->formatter->format($catalogue->get($id, $domain), $locale, $parameters);
}
@ -251,6 +248,10 @@ class Translator implements LegacyTranslatorInterface, TranslatorInterface, Tran
}
}
if ($this->hasIntlFormatter && $catalogue->defines($id, $domain.MessageCatalogue::INTL_DOMAIN_SUFFIX)) {
return $this->formatter->formatIntl($catalogue->get($id, $domain), $locale, array('%count%' => $number) + $parameters);
}
return $this->formatter->choiceFormat($catalogue->get($id, $domain), $number, $locale, $parameters);
}