Skip to content

Commit

Permalink
bug #29220 [Translation] make intl+icu format seamless by handling it…
Browse files Browse the repository at this point in the history
… 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
-------

c71dfb9 [Translation] make intl+icu format seamless by handling it in MessageCatalogue
  • Loading branch information
nicolas-grekas committed Nov 15, 2018
2 parents 88891d5 + c71dfb9 commit ef75454
Show file tree
Hide file tree
Showing 11 changed files with 159 additions and 30 deletions.
Expand Up @@ -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}
Expand All @@ -32,10 +34,11 @@ protected function processDomain($domain)
'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);
}
Expand All @@ -45,7 +48,7 @@ protected function processDomain($domain)
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);
}
Expand Down
Expand Up @@ -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}
Expand All @@ -33,6 +35,7 @@ protected function processDomain($domain)
'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}
Expand All @@ -46,7 +49,7 @@ protected function processDomain($domain)
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);
}
Expand All @@ -59,7 +62,7 @@ protected function processDomain($domain)
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);
}
Expand Down
21 changes: 20 additions & 1 deletion src/Symfony/Component/Translation/Dumper/FileDumper.php
Expand Up @@ -76,7 +76,26 @@ public function dump(MessageCatalogue $messages, $options = array())
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));
}
}
Expand Down
Expand Up @@ -18,8 +18,6 @@
*/
interface IntlFormatterInterface
{
const DOMAIN_SUFFIX = '+intl-icu';

/**
* Formats a localized message using rules defined by ICU MessageFormat.
*
Expand Down
44 changes: 37 additions & 7 deletions src/Symfony/Component/Translation/MessageCatalogue.php
Expand Up @@ -49,19 +49,41 @@ public function getLocale()
*/
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);
}

/**
* {@inheritdoc}
*/
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());
}

$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 isset($this->messages[$domain]) ? $this->messages[$domain] : array();
return $allMessages;
}

/**
Expand All @@ -77,7 +99,7 @@ public function set($id, $translation, $domain = 'messages')
*/
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;
}

Expand All @@ -93,14 +115,18 @@ public function has($id, $domain = 'messages')
*/
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]);
}

/**
* {@inheritdoc}
*/
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];
}
Expand All @@ -117,7 +143,7 @@ public function get($id, $domain = 'messages')
*/
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);
}
Expand All @@ -144,6 +170,10 @@ public function addCatalogue(MessageCatalogueInterface $catalogue)
}

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);
}

Expand Down
Expand Up @@ -20,6 +20,8 @@
*/
interface MessageCatalogueInterface
{
const INTL_DOMAIN_SUFFIX = '+intl-icu';

/**
* Gets the catalogue locale.
*
Expand Down
Expand Up @@ -53,6 +53,20 @@ public function testGetResultFromSingleDomain()
);
}

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')));
Expand Down
Expand Up @@ -53,6 +53,20 @@ public function testGetResultFromSingleDomain()
);
}

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')));
Expand Down
Expand Up @@ -32,6 +32,30 @@ public function testDump()
@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();
Expand All @@ -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()
Expand Down
35 changes: 28 additions & 7 deletions src/Symfony/Component/Translation/Tests/MessageCatalogueTest.php
Expand Up @@ -25,9 +25,9 @@ public function testGetLocale()

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()
Expand All @@ -37,24 +37,43 @@ public function testAll()
$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()
Expand All @@ -75,7 +94,7 @@ public function testAdd()

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'));
Expand All @@ -89,16 +108,18 @@ public function testAddCatalogue()
$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());
}
Expand Down

0 comments on commit ef75454

Please sign in to comment.