From 597a15d7f7df10ce574dd3ba83a05c3720fc9a67 Mon Sep 17 00:00:00 2001 From: Nyholm Date: Mon, 3 Sep 2018 23:28:48 +0200 Subject: [PATCH] Use FallbackFormatter instead of support for multiple formatters --- .../Bundle/FrameworkBundle/CHANGELOG.md | 1 - .../DependencyInjection/Configuration.php | 15 +- .../FrameworkExtension.php | 3 - .../Resources/config/schema/symfony-1.0.xsd | 6 - .../Resources/config/translation.xml | 10 +- .../Component/Translation/CHANGELOG.md | 3 +- .../Formatter/FallbackFormatter.php | 78 +++++++ .../Formatter/IntlMessageFormatter.php | 9 +- .../Tests/Formatter/FallbackFormatterTest.php | 196 ++++++++++++++++++ .../Formatter/IntlMessageFormatterTest.php | 9 +- .../Translation/Tests/TranslatorTest.php | 26 --- .../Component/Translation/Translator.php | 29 +-- 12 files changed, 303 insertions(+), 82 deletions(-) create mode 100644 src/Symfony/Component/Translation/Formatter/FallbackFormatter.php create mode 100644 src/Symfony/Component/Translation/Tests/Formatter/FallbackFormatterTest.php diff --git a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md index 795915b165..6c2dea7e5e 100644 --- a/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md @@ -11,7 +11,6 @@ CHANGELOG * Deprecated the `Symfony\Bundle\FrameworkBundle\Controller\Controller` class in favor of `Symfony\Bundle\FrameworkBundle\Controller\AbstractController`. * Enabled autoconfiguration for `Psr\Log\LoggerAwareInterface` * Added new "auto" mode for `framework.session.cookie_secure` to turn it on when HTTPS is used - * Added support for configuring the `Translator` with multiple formatters. 4.1.0 ----- diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php index 5328f53fb3..6625570009 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php @@ -690,7 +690,6 @@ class Configuration implements ConfigurationInterface ->{!class_exists(FullStack::class) && class_exists(Translator::class) ? 'canBeDisabled' : 'canBeEnabled'}() ->fixXmlConfig('fallback') ->fixXmlConfig('path') - ->fixXmlConfig('domain_formatter') ->children() ->arrayNode('fallbacks') ->beforeNormalization()->ifString()->then(function ($v) { return array($v); })->end() @@ -698,19 +697,7 @@ class Configuration implements ConfigurationInterface ->defaultValue(array('en')) ->end() ->booleanNode('logging')->defaultValue(false)->end() - ->scalarNode('formatter') - ->info('The default formatter to use if none is specified.') - ->defaultValue('translator.formatter.default') - ->end() - ->arrayNode('domain_formatters') - ->info('Configure different formatters per domain.') - ->useAttributeAsKey('domain') - ->prototype('array') - ->children() - ->scalarNode('service')->cannotBeEmpty()->end() - ->end() - ->end() - ->end() + ->scalarNode('formatter')->defaultValue('translator.formatter.default')->end() ->scalarNode('default_path') ->info('The default path used to load translations') ->defaultValue('%kernel.project_dir%/translations') diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 253770e8e9..289bd67aca 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -981,9 +981,6 @@ class FrameworkExtension extends Extension $container->setAlias('translator.formatter', new Alias($config['formatter'], false)); $translator = $container->findDefinition('translator.default'); $translator->addMethodCall('setFallbackLocales', array($config['fallbacks'])); - foreach ($config['domain_formatters'] as $formatter) { - $translator->addMethodCall('addFormatter', array($formatter['domain'], new Reference($formatter['service']))); - } $container->setParameter('translator.logging', $config['logging']); $container->setParameter('translator.default_path', $config['default_path']); diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd index 9bb271cc55..a078bb67c1 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/schema/symfony-1.0.xsd @@ -185,7 +185,6 @@ - @@ -193,11 +192,6 @@ - - - - - diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/translation.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/translation.xml index 729f178319..c37e556d7f 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/translation.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/translation.xml @@ -29,13 +29,15 @@ - + - - - + + + + + diff --git a/src/Symfony/Component/Translation/CHANGELOG.md b/src/Symfony/Component/Translation/CHANGELOG.md index c0a62b1f5e..ce7e19722b 100644 --- a/src/Symfony/Component/Translation/CHANGELOG.md +++ b/src/Symfony/Component/Translation/CHANGELOG.md @@ -7,8 +7,7 @@ CHANGELOG * Started using ICU parent locales as fallback locales. * deprecated `TranslatorInterface` in favor of `Symfony\Contracts\Translation\TranslatorInterface` * deprecated `MessageSelector`, `Interval` and `PluralizationRules`; use `IdentityTranslator` instead - * Added intl message formatter. - * Added support for one formatter per domain + * Added `IntlMessageFormatter` and`FallbackMessageFormatter` 4.1.0 ----- diff --git a/src/Symfony/Component/Translation/Formatter/FallbackFormatter.php b/src/Symfony/Component/Translation/Formatter/FallbackFormatter.php new file mode 100644 index 0000000000..dfc272af37 --- /dev/null +++ b/src/Symfony/Component/Translation/Formatter/FallbackFormatter.php @@ -0,0 +1,78 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Symfony\Component\Translation\Formatter; + +use Symfony\Component\Translation\Exception\LogicException; + +class FallbackFormatter implements MessageFormatterInterface, ChoiceMessageFormatterInterface +{ + /** + * @var MessageFormatterInterface|ChoiceMessageFormatterInterface + */ + private $firstFormatter; + + /** + * @var MessageFormatterInterface|ChoiceMessageFormatterInterface + */ + private $secondFormatter; + + public function __construct(MessageFormatterInterface $firstFormatter, MessageFormatterInterface $secondFormatter) + { + $this->firstFormatter = $firstFormatter; + $this->secondFormatter = $secondFormatter; + } + + public function format($message, $locale, array $parameters = array()) + { + try { + $result = $this->firstFormatter->format($message, $locale, $parameters); + } catch (\Throwable $e) { + return $this->secondFormatter->format($message, $locale, $parameters); + } + + if ($result === $message) { + $result = $this->secondFormatter->format($message, $locale, $parameters); + } + + return $result; + } + + public function choiceFormat($message, $number, $locale, array $parameters = array()) + { + // If both support ChoiceMessageFormatterInterface + if ($this->firstFormatter instanceof ChoiceMessageFormatterInterface && $this->secondFormatter instanceof ChoiceMessageFormatterInterface) { + try { + $result = $this->firstFormatter->choiceFormat($message, $number, $locale, $parameters); + } catch (\Throwable $e) { + return $this->secondFormatter->choiceFormat($message, $number, $locale, $parameters); + } + + if ($result === $message) { + $result = $this->secondFormatter->choiceFormat($message, $number, $locale, $parameters); + } + + return $result; + } + + if ($this->firstFormatter instanceof ChoiceMessageFormatterInterface) { + return $this->firstFormatter->choiceFormat($message, $number, $locale, $parameters); + } + + if ($this->secondFormatter instanceof ChoiceMessageFormatterInterface) { + return $this->secondFormatter->choiceFormat($message, $number, $locale, $parameters); + } + + throw new LogicException(sprintf('The no formatter support plural translations.')); + } +} diff --git a/src/Symfony/Component/Translation/Formatter/IntlMessageFormatter.php b/src/Symfony/Component/Translation/Formatter/IntlMessageFormatter.php index cd44964c7e..eb27f56d7c 100644 --- a/src/Symfony/Component/Translation/Formatter/IntlMessageFormatter.php +++ b/src/Symfony/Component/Translation/Formatter/IntlMessageFormatter.php @@ -11,6 +11,8 @@ namespace Symfony\Component\Translation\Formatter; +use Symfony\Component\Translation\Exception\InvalidArgumentException; + /** * @author Guilherme Blanco * @author Abdellatif Ait boudad @@ -25,15 +27,12 @@ class IntlMessageFormatter implements MessageFormatterInterface try { $formatter = new \MessageFormatter($locale, $message); } catch (\Throwable $e) { - throw new \InvalidArgumentException('Invalid message format.', $e); - } - if (null === $formatter) { - throw new \InvalidArgumentException(sprintf('Invalid message format. Reason: %s (error #%d)', intl_get_error_message(), intl_get_error_code())); + throw new InvalidArgumentException(sprintf('Invalid message format. Reason: %s (error #%d)', intl_get_error_message(), intl_get_error_code()), 0, $e); } $message = $formatter->format($parameters); if (U_ZERO_ERROR !== $formatter->getErrorCode()) { - throw new \InvalidArgumentException(sprintf('Unable to format message. Reason: %s (error #%s)', $formatter->getErrorMessage(), $formatter->getErrorCode())); + throw new InvalidArgumentException(sprintf('Unable to format message. Reason: %s (error #%s)', $formatter->getErrorMessage(), $formatter->getErrorCode())); } return $message; diff --git a/src/Symfony/Component/Translation/Tests/Formatter/FallbackFormatterTest.php b/src/Symfony/Component/Translation/Tests/Formatter/FallbackFormatterTest.php new file mode 100644 index 0000000000..68aadb90e8 --- /dev/null +++ b/src/Symfony/Component/Translation/Tests/Formatter/FallbackFormatterTest.php @@ -0,0 +1,196 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Translation\Tests\Formatter; + +use Symfony\Component\Translation\Exception\InvalidArgumentException; +use Symfony\Component\Translation\Exception\LogicException; +use Symfony\Component\Translation\Formatter\ChoiceMessageFormatterInterface; +use Symfony\Component\Translation\Formatter\FallbackFormatter; +use Symfony\Component\Translation\Formatter\MessageFormatterInterface; + +class FallbackFormatterTest extends \PHPUnit\Framework\TestCase +{ + public function testFormatSame() + { + $first = $this->getMockBuilder(MessageFormatterInterface::class)->setMethods(array('format'))->getMock(); + $first + ->expects($this->once()) + ->method('format') + ->with('foo', 'en', array(2)) + ->willReturn('foo'); + + $second = $this->getMockBuilder(MessageFormatterInterface::class)->setMethods(array('format'))->getMock(); + $second + ->expects($this->once()) + ->method('format') + ->with('foo', 'en', array(2)) + ->willReturn('bar'); + + $this->assertEquals('bar', (new FallbackFormatter($first, $second))->format('foo', 'en', array(2))); + } + + public function testFormatDifferent() + { + $first = $this->getMockBuilder(MessageFormatterInterface::class)->setMethods(array('format'))->getMock(); + $first + ->expects($this->once()) + ->method('format') + ->with('foo', 'en', array(2)) + ->willReturn('new value'); + + $second = $this->getMockBuilder(MessageFormatterInterface::class)->setMethods(array('format'))->getMock(); + $second + ->expects($this->exactly(0)) + ->method('format') + ->withAnyParameters(); + + $this->assertEquals('new value', (new FallbackFormatter($first, $second))->format('foo', 'en', array(2))); + } + + public function testFormatException() + { + $first = $this->getMockBuilder(MessageFormatterInterface::class)->setMethods(array('format'))->getMock(); + $first + ->expects($this->once()) + ->method('format') + ->willThrowException(new InvalidArgumentException()); + + $second = $this->getMockBuilder(MessageFormatterInterface::class)->setMethods(array('format'))->getMock(); + $second + ->expects($this->once()) + ->method('format') + ->with('foo', 'en', array(2)) + ->willReturn('bar'); + + $this->assertEquals('bar', (new FallbackFormatter($first, $second))->format('foo', 'en', array(2))); + } + + public function testChoiceFormatSame() + { + $first = $this->getMockBuilder(SuperFormatterInterface::class)->setMethods(array('format', 'choiceFormat'))->getMock(); + $first + ->expects($this->once()) + ->method('choiceFormat') + ->with('foo', 1, 'en', array(2)) + ->willReturn('foo'); + + $second = $this->getMockBuilder(SuperFormatterInterface::class)->setMethods(array('format', 'choiceFormat'))->getMock(); + $second + ->expects($this->once()) + ->method('choiceFormat') + ->with('foo', 1, 'en', array(2)) + ->willReturn('bar'); + + $this->assertEquals('bar', (new FallbackFormatter($first, $second))->choiceFormat('foo', 1, 'en', array(2))); + } + + public function testChoiceFormatDifferent() + { + $first = $this->getMockBuilder(SuperFormatterInterface::class)->setMethods(array('format', 'choiceFormat'))->getMock(); + $first + ->expects($this->once()) + ->method('choiceFormat') + ->with('foo', 1, 'en', array(2)) + ->willReturn('new value'); + + $second = $this->getMockBuilder(SuperFormatterInterface::class)->setMethods(array('format', 'choiceFormat'))->getMock(); + $second + ->expects($this->exactly(0)) + ->method('choiceFormat') + ->withAnyParameters() + ->willReturn('bar'); + + $this->assertEquals('new value', (new FallbackFormatter($first, $second))->choiceFormat('foo', 1, 'en', array(2))); + } + + public function testChoiceFormatException() + { + $first = $this->getMockBuilder(SuperFormatterInterface::class)->setMethods(array('format', 'choiceFormat'))->getMock(); + $first + ->expects($this->once()) + ->method('choiceFormat') + ->willThrowException(new InvalidArgumentException()); + + $second = $this->getMockBuilder(SuperFormatterInterface::class)->setMethods(array('format', 'choiceFormat'))->getMock(); + $second + ->expects($this->once()) + ->method('choiceFormat') + ->with('foo', 1, 'en', array(2)) + ->willReturn('bar'); + + $this->assertEquals('bar', (new FallbackFormatter($first, $second))->choiceFormat('foo', 1, 'en', array(2))); + } + + public function testChoiceFormatOnlyFirst() + { + // Implements both interfaces + $first = $this->getMockBuilder(SuperFormatterInterface::class)->setMethods(array('format', 'choiceFormat'))->getMock(); + $first + ->expects($this->once()) + ->method('choiceFormat') + ->with('foo', 1, 'en', array(2)) + ->willReturn('bar'); + + // Implements only one interface + $second = $this->getMockBuilder(MessageFormatterInterface::class)->setMethods(array('format'))->getMock(); + $second + ->expects($this->exactly(0)) + ->method('format') + ->withAnyParameters() + ->willReturn('error'); + + $this->assertEquals('bar', (new FallbackFormatter($first, $second))->choiceFormat('foo', 1, 'en', array(2))); + } + + public function testChoiceFormatOnlySecond() + { + // Implements only one interface + $first = $this->getMockBuilder(MessageFormatterInterface::class)->setMethods(array('format'))->getMock(); + $first + ->expects($this->exactly(0)) + ->method('format') + ->withAnyParameters() + ->willReturn('error'); + + // Implements both interfaces + $second = $this->getMockBuilder(SuperFormatterInterface::class)->setMethods(array('format', 'choiceFormat'))->getMock(); + $second + ->expects($this->once()) + ->method('choiceFormat') + ->with('foo', 1, 'en', array(2)) + ->willReturn('bar'); + + $this->assertEquals('bar', (new FallbackFormatter($first, $second))->choiceFormat('foo', 1, 'en', array(2))); + } + + public function testChoiceFormatNoChoiceFormat() + { + // Implements only one interface + $first = $this->getMockBuilder(MessageFormatterInterface::class)->setMethods(array('format'))->getMock(); + $first + ->expects($this->exactly(0)) + ->method('format'); + + // Implements both interfaces + $second = $this->getMockBuilder(MessageFormatterInterface::class)->setMethods(array('format'))->getMock(); + $second + ->expects($this->exactly(0)) + ->method('format'); + + $this->expectException(LogicException::class); + $this->assertEquals('bar', (new FallbackFormatter($first, $second))->choiceFormat('foo', 1, 'en', array(2))); + } +} + +interface SuperFormatterInterface extends MessageFormatterInterface, ChoiceMessageFormatterInterface +{ +} diff --git a/src/Symfony/Component/Translation/Tests/Formatter/IntlMessageFormatterTest.php b/src/Symfony/Component/Translation/Tests/Formatter/IntlMessageFormatterTest.php index 268f85fe0d..29bc35a47c 100644 --- a/src/Symfony/Component/Translation/Tests/Formatter/IntlMessageFormatterTest.php +++ b/src/Symfony/Component/Translation/Tests/Formatter/IntlMessageFormatterTest.php @@ -11,13 +11,14 @@ namespace Symfony\Component\Translation\Tests\Formatter; +use Symfony\Component\Translation\Exception\InvalidArgumentException; use Symfony\Component\Translation\Formatter\IntlMessageFormatter; class IntlMessageFormatterTest extends \PHPUnit\Framework\TestCase { public function setUp() { - if (!extension_loaded('intl')) { + if (!\extension_loaded('intl')) { $this->markTestSkipped( 'The Intl extension is not available.' ); @@ -32,6 +33,12 @@ class IntlMessageFormatterTest extends \PHPUnit\Framework\TestCase $this->assertEquals($expected, trim($this->getMessageFormatter()->format($message, 'en', $arguments))); } + public function testInvalidFormat() + { + $this->expectException(InvalidArgumentException::class); + $this->getMessageFormatter()->format('{foo', 'en', array(2)); + } + public function testFormatWithNamedArguments() { if (version_compare(INTL_ICU_VERSION, '4.8', '<')) { diff --git a/src/Symfony/Component/Translation/Tests/TranslatorTest.php b/src/Symfony/Component/Translation/Tests/TranslatorTest.php index 1038bd996b..b0efefb7d0 100644 --- a/src/Symfony/Component/Translation/Tests/TranslatorTest.php +++ b/src/Symfony/Component/Translation/Tests/TranslatorTest.php @@ -12,8 +12,6 @@ namespace Symfony\Component\Translation\Tests; use PHPUnit\Framework\TestCase; -use Symfony\Component\Translation\Formatter\MessageFormatterInterface; -use Symfony\Component\Translation\Translator; use Symfony\Component\Translation\Loader\ArrayLoader; use Symfony\Component\Translation\MessageCatalogue; use Symfony\Component\Translation\Translator; @@ -569,30 +567,6 @@ class TranslatorTest extends TestCase // unchanged if it can't be found $this->assertEquals('some_message2', $translator->transChoice('some_message2', 10, array('%count%' => 10))); } - - public function testDomainSpecificFormatter() - { - $fooFormatter = $this->getMockBuilder(MessageFormatterInterface::class) - ->setMethods(array('format')) - ->getMock(); - $fooFormatter->expects($this->exactly(2)) - ->method('format') - ->with('foo', 'en', array()); - - $barFormatter = $this->getMockBuilder(MessageFormatterInterface::class) - ->setMethods(array('format')) - ->getMock(); - $barFormatter->expects($this->exactly(1)) - ->method('format') - ->with('bar', 'en', array()); - - $translator = new Translator('en', $fooFormatter); - $translator->addFormatter('bar_domain', $barFormatter); - - $translator->trans('foo'); - $translator->trans('foo', array(), 'foo_domain'); - $translator->trans('bar', array(), 'bar_domain'); - } } class StringClass diff --git a/src/Symfony/Component/Translation/Translator.php b/src/Symfony/Component/Translation/Translator.php index ce1a2e31b0..3c7b9b3ad7 100644 --- a/src/Symfony/Component/Translation/Translator.php +++ b/src/Symfony/Component/Translation/Translator.php @@ -54,9 +54,9 @@ class Translator implements TranslatorInterface, TranslatorBagInterface private $resources = array(); /** - * @var MessageFormatterInterface[] + * @var MessageFormatterInterface */ - private $formatters; + private $formatter; /** * @var string @@ -89,7 +89,7 @@ class Translator implements TranslatorInterface, TranslatorBagInterface $formatter = new MessageFormatter(); } - $this->formatters['_default'] = $formatter; + $this->formatter = $formatter; $this->cacheDir = $cacheDir; $this->debug = $debug; } @@ -137,11 +137,6 @@ class Translator implements TranslatorInterface, TranslatorBagInterface } } - public function addFormatter(string $domain, MessageFormatterInterface $formatter): void - { - $this->formatters[$domain] = $formatter; - } - /** * {@inheritdoc} */ @@ -197,7 +192,7 @@ class Translator implements TranslatorInterface, TranslatorBagInterface $domain = 'messages'; } - return $this->getFormatter($domain)->format($this->getCatalogue($locale)->get((string) $id, $domain), $locale, $parameters); + return $this->formatter->format($this->getCatalogue($locale)->get((string) $id, $domain), $locale, $parameters); } /** @@ -205,13 +200,12 @@ class Translator implements TranslatorInterface, TranslatorBagInterface */ public function transChoice($id, $number, array $parameters = array(), $domain = null, $locale = null) { - if (null === $domain) { - $domain = 'messages'; + if (!$this->formatter instanceof ChoiceMessageFormatterInterface) { + throw new LogicException(sprintf('The formatter "%s" does not support plural translations.', \get_class($this->formatter))); } - $formatter = $this->getFormatter($domain); - if (!$formatter instanceof ChoiceMessageFormatterInterface) { - throw new LogicException(sprintf('The formatter "%s" does not support plural translations.', \get_class($formatter))); + if (null === $domain) { + $domain = 'messages'; } $id = (string) $id; @@ -226,7 +220,7 @@ class Translator implements TranslatorInterface, TranslatorBagInterface } } - return $formatter->choiceFormat($catalogue->get($id, $domain), $number, $locale, $parameters); + return $this->formatter->choiceFormat($catalogue->get($id, $domain), $number, $locale, $parameters); } /** @@ -461,9 +455,4 @@ EOF return $this->configCacheFactory; } - - private function getFormatter(string $domain): MessageFormatterInterface - { - return $this->formatters[$domain] ?? $this->formatters['_default']; - } }