From c71dfb96737ed3d79b664dcc2138a1c3929354c0 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 14 Nov 2018 14:56:01 +0100 Subject: [PATCH] [Translation] make intl+icu format seamless by handling it in MessageCatalogue --- .../Translation/Catalogue/MergeOperation.php | 7 ++- .../Translation/Catalogue/TargetOperation.php | 7 ++- .../Translation/Dumper/FileDumper.php | 21 ++++++++- .../Formatter/IntlFormatterInterface.php | 2 - .../Translation/MessageCatalogue.php | 44 ++++++++++++++++--- .../Translation/MessageCatalogueInterface.php | 2 + .../Tests/Catalogue/MergeOperationTest.php | 14 ++++++ .../Tests/Catalogue/TargetOperationTest.php | 14 ++++++ .../Tests/Dumper/FileDumperTest.php | 26 ++++++++++- .../Tests/MessageCatalogueTest.php | 35 ++++++++++++--- .../Component/Translation/Translator.php | 17 +++---- 11 files changed, 159 insertions(+), 30 deletions(-) diff --git a/src/Symfony/Component/Translation/Catalogue/MergeOperation.php b/src/Symfony/Component/Translation/Catalogue/MergeOperation.php index 6db3f801f3..8dd4026176 100644 --- a/src/Symfony/Component/Translation/Catalogue/MergeOperation.php +++ b/src/Symfony/Component/Translation/Catalogue/MergeOperation.php @@ -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); } diff --git a/src/Symfony/Component/Translation/Catalogue/TargetOperation.php b/src/Symfony/Component/Translation/Catalogue/TargetOperation.php index f3b0a29dfb..6264525c97 100644 --- a/src/Symfony/Component/Translation/Catalogue/TargetOperation.php +++ b/src/Symfony/Component/Translation/Catalogue/TargetOperation.php @@ -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); } diff --git a/src/Symfony/Component/Translation/Dumper/FileDumper.php b/src/Symfony/Component/Translation/Dumper/FileDumper.php index 5b10e4520e..46657d5590 100644 --- a/src/Symfony/Component/Translation/Dumper/FileDumper.php +++ b/src/Symfony/Component/Translation/Dumper/FileDumper.php @@ -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)); } } diff --git a/src/Symfony/Component/Translation/Formatter/IntlFormatterInterface.php b/src/Symfony/Component/Translation/Formatter/IntlFormatterInterface.php index eb8590db3d..5fc5d97d24 100644 --- a/src/Symfony/Component/Translation/Formatter/IntlFormatterInterface.php +++ b/src/Symfony/Component/Translation/Formatter/IntlFormatterInterface.php @@ -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. * diff --git a/src/Symfony/Component/Translation/MessageCatalogue.php b/src/Symfony/Component/Translation/MessageCatalogue.php index c36ea30ec0..7a7657c0a4 100644 --- a/src/Symfony/Component/Translation/MessageCatalogue.php +++ b/src/Symfony/Component/Translation/MessageCatalogue.php @@ -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); } diff --git a/src/Symfony/Component/Translation/MessageCatalogueInterface.php b/src/Symfony/Component/Translation/MessageCatalogueInterface.php index e0dbb2bd96..f3d3f5ea9c 100644 --- a/src/Symfony/Component/Translation/MessageCatalogueInterface.php +++ b/src/Symfony/Component/Translation/MessageCatalogueInterface.php @@ -20,6 +20,8 @@ use Symfony\Component\Config\Resource\ResourceInterface; */ interface MessageCatalogueInterface { + const INTL_DOMAIN_SUFFIX = '+intl-icu'; + /** * Gets the catalogue locale. * diff --git a/src/Symfony/Component/Translation/Tests/Catalogue/MergeOperationTest.php b/src/Symfony/Component/Translation/Tests/Catalogue/MergeOperationTest.php index 8b51c15dae..9a68847882 100644 --- a/src/Symfony/Component/Translation/Tests/Catalogue/MergeOperationTest.php +++ b/src/Symfony/Component/Translation/Tests/Catalogue/MergeOperationTest.php @@ -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'))); diff --git a/src/Symfony/Component/Translation/Tests/Catalogue/TargetOperationTest.php b/src/Symfony/Component/Translation/Tests/Catalogue/TargetOperationTest.php index 271d17fb8f..83b0550d45 100644 --- a/src/Symfony/Component/Translation/Tests/Catalogue/TargetOperationTest.php +++ b/src/Symfony/Component/Translation/Tests/Catalogue/TargetOperationTest.php @@ -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'))); diff --git a/src/Symfony/Component/Translation/Tests/Dumper/FileDumperTest.php b/src/Symfony/Component/Translation/Tests/Dumper/FileDumperTest.php index d1524d6e53..908f22d748 100644 --- a/src/Symfony/Component/Translation/Tests/Dumper/FileDumperTest.php +++ b/src/Symfony/Component/Translation/Tests/Dumper/FileDumperTest.php @@ -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() diff --git a/src/Symfony/Component/Translation/Tests/MessageCatalogueTest.php b/src/Symfony/Component/Translation/Tests/MessageCatalogueTest.php index 1ab8246969..3339f3db20 100644 --- a/src/Symfony/Component/Translation/Tests/MessageCatalogueTest.php +++ b/src/Symfony/Component/Translation/Tests/MessageCatalogueTest.php @@ -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()); } diff --git a/src/Symfony/Component/Translation/Translator.php b/src/Symfony/Component/Translation/Translator.php index eca5674e67..90f09ce02a 100644 --- a/src/Symfony/Component/Translation/Translator.php +++ b/src/Symfony/Component/Translation/Translator.php @@ -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); }