diff --git a/.github/composer-config.json b/.github/composer-config.json index 1b82f7c5db..25d2bcd027 100644 --- a/.github/composer-config.json +++ b/.github/composer-config.json @@ -6,6 +6,7 @@ "symfony/http-kernel": "source", "symfony/messenger": "source", "symfony/notifier": "source", + "symfony/translation": "source", "symfony/validator": "source", "*": "dist" } diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index 9cddc28f86..17282c683c 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -1360,7 +1360,7 @@ class FrameworkExtension extends Extension $parentPackages = ['symfony/framework-bundle', 'symfony/translation', 'symfony/http-client']; foreach ($classToServices as $class => $service) { - $package = sprintf('symfony/%s-translation', substr($service, \strlen('translation.provider_factory.'))); + $package = sprintf('symfony/%s-translation-provider', substr($service, \strlen('translation.provider_factory.'))); if (!$container->hasDefinition('http_client') || !ContainerBuilder::willBeAvailable($package, $class, $parentPackages)) { $container->removeDefinition($service); diff --git a/src/Symfony/Component/Translation/Bridge/Loco/composer.json b/src/Symfony/Component/Translation/Bridge/Loco/composer.json index 6b4cc805fd..759930a0ff 100644 --- a/src/Symfony/Component/Translation/Bridge/Loco/composer.json +++ b/src/Symfony/Component/Translation/Bridge/Loco/composer.json @@ -1,7 +1,7 @@ { - "name": "symfony/loco-translation", + "name": "symfony/loco-translation-provider", "type": "symfony-bridge", - "description": "Symfony Loco Translation Bridge", + "description": "Symfony Loco Translation Provider Bridge", "keywords": ["loco", "translation", "provider"], "homepage": "https://symfony.com", "license": "MIT", diff --git a/src/Symfony/Component/Translation/Bridge/Loco/phpunit.xml.dist b/src/Symfony/Component/Translation/Bridge/Loco/phpunit.xml.dist index c58cb50d0e..5bfb7a9c2c 100644 --- a/src/Symfony/Component/Translation/Bridge/Loco/phpunit.xml.dist +++ b/src/Symfony/Component/Translation/Bridge/Loco/phpunit.xml.dist @@ -13,7 +13,7 @@ - + ./Tests/ diff --git a/src/Symfony/Component/Translation/Exception/MissingRequiredOptionException.php b/src/Symfony/Component/Translation/Exception/MissingRequiredOptionException.php new file mode 100644 index 0000000000..e17283dcc3 --- /dev/null +++ b/src/Symfony/Component/Translation/Exception/MissingRequiredOptionException.php @@ -0,0 +1,25 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Translation\Exception; + +/** + * @author Oskar Stark + */ +class MissingRequiredOptionException extends IncompleteDsnException +{ + public function __construct(string $option, string $dsn = null, ?\Throwable $previous = null) + { + $message = sprintf('The option "%s" is required but missing.', $option); + + parent::__construct($message, $dsn, $previous); + } +} diff --git a/src/Symfony/Component/Translation/Exception/UnsupportedSchemeException.php b/src/Symfony/Component/Translation/Exception/UnsupportedSchemeException.php index d53f3b0118..f96b5cdc1f 100644 --- a/src/Symfony/Component/Translation/Exception/UnsupportedSchemeException.php +++ b/src/Symfony/Component/Translation/Exception/UnsupportedSchemeException.php @@ -19,7 +19,7 @@ class UnsupportedSchemeException extends LogicException private const SCHEME_TO_PACKAGE_MAP = [ 'loco' => [ 'class' => Bridge\Loco\Provider\LocoProviderFactory::class, - 'package' => 'symfony/loco-translation', + 'package' => 'symfony/loco-translation-provider', ], ]; @@ -31,14 +31,14 @@ class UnsupportedSchemeException extends LogicException } $package = self::SCHEME_TO_PACKAGE_MAP[$provider] ?? null; if ($package && !class_exists($package['class'])) { - parent::__construct(sprintf('Unable to synchronize translations via "%s" as the providers is not installed; try running "composer require %s".', $provider, $package['package'])); + parent::__construct(sprintf('Unable to synchronize translations via "%s" as the provider is not installed; try running "composer require %s".', $provider, $package['package'])); return; } $message = sprintf('The "%s" scheme is not supported', $dsn->getScheme()); if ($name && $supported) { - $message .= sprintf('; supported schemes for translation providers "%s" are: "%s"', $name, implode('", "', $supported)); + $message .= sprintf('; supported schemes for translation provider "%s" are: "%s"', $name, implode('", "', $supported)); } parent::__construct($message.'.'); diff --git a/src/Symfony/Component/Translation/Provider/Dsn.php b/src/Symfony/Component/Translation/Provider/Dsn.php index 67f805c9d8..820cabfb3a 100644 --- a/src/Symfony/Component/Translation/Provider/Dsn.php +++ b/src/Symfony/Component/Translation/Provider/Dsn.php @@ -12,11 +12,11 @@ namespace Symfony\Component\Translation\Provider; use Symfony\Component\Translation\Exception\InvalidArgumentException; +use Symfony\Component\Translation\Exception\MissingRequiredOptionException; /** - * @author Mathieu Santostefano - * - * @experimental in 5.3 + * @author Fabien Potencier + * @author Oskar Stark */ final class Dsn { @@ -25,23 +25,14 @@ final class Dsn private $user; private $password; private $port; - private $options; private $path; - private $dsn; + private $options; + private $originalDsn; - public function __construct(string $scheme, string $host, ?string $user = null, ?string $password = null, ?int $port = null, array $options = [], ?string $path = null) + public function __construct(string $dsn) { - $this->scheme = $scheme; - $this->host = $host; - $this->user = $user; - $this->password = $password; - $this->port = $port; - $this->options = $options; - $this->path = $path; - } + $this->originalDsn = $dsn; - public static function fromString(string $dsn): self - { if (false === $parsedDsn = parse_url($dsn)) { throw new InvalidArgumentException(sprintf('The "%s" translation provider DSN is invalid.', $dsn)); } @@ -49,21 +40,18 @@ final class Dsn if (!isset($parsedDsn['scheme'])) { throw new InvalidArgumentException(sprintf('The "%s" translation provider DSN must contain a scheme.', $dsn)); } + $this->scheme = $parsedDsn['scheme']; if (!isset($parsedDsn['host'])) { throw new InvalidArgumentException(sprintf('The "%s" translation provider DSN must contain a host (use "default" by default).', $dsn)); } + $this->host = $parsedDsn['host']; - $user = isset($parsedDsn['user']) ? urldecode($parsedDsn['user']) : null; - $password = isset($parsedDsn['pass']) ? urldecode($parsedDsn['pass']) : null; - $port = $parsedDsn['port'] ?? null; - $path = $parsedDsn['path'] ?? null; - parse_str($parsedDsn['query'] ?? '', $query); - - $dsnObject = new self($parsedDsn['scheme'], $parsedDsn['host'], $user, $password, $port, $query, $path); - $dsnObject->dsn = $dsn; - - return $dsnObject; + $this->user = '' !== ($parsedDsn['user'] ?? '') ? urldecode($parsedDsn['user']) : null; + $this->password = '' !== ($parsedDsn['pass'] ?? '') ? urldecode($parsedDsn['pass']) : null; + $this->port = $parsedDsn['port'] ?? null; + $this->path = $parsedDsn['path'] ?? null; + parse_str($parsedDsn['query'] ?? '', $this->options); } public function getScheme(): string @@ -96,6 +84,20 @@ final class Dsn return $this->options[$key] ?? $default; } + public function getRequiredOption(string $key) + { + if (!\array_key_exists($key, $this->options) || '' === trim($this->options[$key])) { + throw new MissingRequiredOptionException($key); + } + + return $this->options[$key]; + } + + public function getOptions(): array + { + return $this->options; + } + public function getPath(): ?string { return $this->path; @@ -103,6 +105,6 @@ final class Dsn public function getOriginalDsn(): string { - return $this->dsn; + return $this->originalDsn; } } diff --git a/src/Symfony/Component/Translation/Provider/NullProvider.php b/src/Symfony/Component/Translation/Provider/NullProvider.php index f180a5030c..785fcaa601 100644 --- a/src/Symfony/Component/Translation/Provider/NullProvider.php +++ b/src/Symfony/Component/Translation/Provider/NullProvider.php @@ -23,7 +23,7 @@ class NullProvider implements ProviderInterface { public function __toString(): string { - return NullProviderFactory::SCHEME.'://default'; + return 'null'; } public function write(TranslatorBagInterface $translatorBag, bool $override = false): void diff --git a/src/Symfony/Component/Translation/Provider/NullProviderFactory.php b/src/Symfony/Component/Translation/Provider/NullProviderFactory.php index 81a02df6b2..6ddbd8572f 100644 --- a/src/Symfony/Component/Translation/Provider/NullProviderFactory.php +++ b/src/Symfony/Component/Translation/Provider/NullProviderFactory.php @@ -20,19 +20,17 @@ use Symfony\Component\Translation\Exception\UnsupportedSchemeException; */ final class NullProviderFactory extends AbstractProviderFactory { - const SCHEME = 'null'; - public function create(Dsn $dsn): ProviderInterface { - if (self::SCHEME === $dsn->getScheme()) { + if ('null' === $dsn->getScheme()) { return new NullProvider(); } - throw new UnsupportedSchemeException($dsn, self::SCHEME, $this->getSupportedSchemes()); + throw new UnsupportedSchemeException($dsn, 'null', $this->getSupportedSchemes()); } protected function getSupportedSchemes(): array { - return [self::SCHEME]; + return ['null']; } } diff --git a/src/Symfony/Component/Translation/Provider/TranslationProviderCollectionFactory.php b/src/Symfony/Component/Translation/Provider/TranslationProviderCollectionFactory.php index 5c0b1eab7c..43f4a344c8 100644 --- a/src/Symfony/Component/Translation/Provider/TranslationProviderCollectionFactory.php +++ b/src/Symfony/Component/Translation/Provider/TranslationProviderCollectionFactory.php @@ -37,7 +37,7 @@ class TranslationProviderCollectionFactory $providers = []; foreach ($config as $name => $currentConfig) { $providers[$name] = $this->fromDsnObject( - Dsn::fromString($currentConfig['dsn']), + new Dsn($currentConfig['dsn']), !$currentConfig['locales'] ? $this->enabledLocales : $currentConfig['locales'], !$currentConfig['domains'] ? [] : $currentConfig['domains'] ); diff --git a/src/Symfony/Component/Translation/Test/ProviderFactoryTestCase.php b/src/Symfony/Component/Translation/Test/ProviderFactoryTestCase.php index 872f374eaf..6d5f4b7bf7 100644 --- a/src/Symfony/Component/Translation/Test/ProviderFactoryTestCase.php +++ b/src/Symfony/Component/Translation/Test/ProviderFactoryTestCase.php @@ -72,7 +72,18 @@ abstract class ProviderFactoryTestCase extends TestCase { $factory = $this->createFactory(); - $this->assertSame($expected, $factory->supports(Dsn::fromString($dsn))); + $this->assertSame($expected, $factory->supports(new Dsn($dsn))); + } + + /** + * @dataProvider createProvider + */ + public function testCreate(string $expected, string $dsn) + { + $factory = $this->createFactory(); + $provider = $factory->create(new Dsn($dsn)); + + $this->assertSame($expected, (string) $provider); } /** @@ -82,7 +93,7 @@ abstract class ProviderFactoryTestCase extends TestCase { $factory = $this->createFactory(); - $dsn = Dsn::fromString($dsn); + $dsn = new Dsn($dsn); $this->expectException(UnsupportedSchemeException::class); if (null !== $message) { @@ -92,17 +103,6 @@ abstract class ProviderFactoryTestCase extends TestCase $factory->create($dsn); } - /** - * @dataProvider createProvider - */ - public function testCreate(string $expected, string $dsn) - { - $factory = $this->createFactory(); - $provider = $factory->create(Dsn::fromString($dsn)); - - $this->assertSame($expected, (string) $provider); - } - /** * @dataProvider incompleteDsnProvider */ @@ -110,7 +110,7 @@ abstract class ProviderFactoryTestCase extends TestCase { $factory = $this->createFactory(); - $dsn = Dsn::fromString($dsn); + $dsn = new Dsn($dsn); $this->expectException(IncompleteDsnException::class); if (null !== $message) { diff --git a/src/Symfony/Component/Translation/Test/ProviderTestCase.php b/src/Symfony/Component/Translation/Test/ProviderTestCase.php index eae116c89f..4eb08604ba 100644 --- a/src/Symfony/Component/Translation/Test/ProviderTestCase.php +++ b/src/Symfony/Component/Translation/Test/ProviderTestCase.php @@ -14,7 +14,6 @@ namespace Symfony\Component\Translation\Test; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; use Symfony\Component\HttpClient\MockHttpClient; -use Symfony\Component\Translation\Bridge\Loco\Provider\LocoProvider; use Symfony\Component\Translation\Dumper\XliffFileDumper; use Symfony\Component\Translation\Loader\LoaderInterface; use Symfony\Component\Translation\Provider\ProviderInterface; @@ -45,7 +44,7 @@ abstract class ProviderTestCase extends TestCase /** * @dataProvider toStringProvider */ - public function testToString(LocoProvider $provider, string $expected) + public function testToString(ProviderInterface $provider, string $expected) { $this->assertSame($expected, (string) $provider); } diff --git a/src/Symfony/Component/Translation/Tests/Provider/DsnTest.php b/src/Symfony/Component/Translation/Tests/Provider/DsnTest.php index e6cf8a388d..d1ec35fc2f 100644 --- a/src/Symfony/Component/Translation/Tests/Provider/DsnTest.php +++ b/src/Symfony/Component/Translation/Tests/Provider/DsnTest.php @@ -13,69 +13,142 @@ namespace Symfony\Component\Translation\Tests\Provider; use PHPUnit\Framework\TestCase; use Symfony\Component\Translation\Exception\InvalidArgumentException; +use Symfony\Component\Translation\Exception\MissingRequiredOptionException; use Symfony\Component\Translation\Provider\Dsn; -class DsnTest extends TestCase +final class DsnTest extends TestCase { /** - * @dataProvider fromStringProvider + * @dataProvider constructProvider */ - public function testFromString(string $string, Dsn $expectedDsn): void + public function testConstruct(string $dsnString, string $scheme, string $host, ?string $user = null, ?string $password = null, ?int $port = null, array $options = [], ?string $path = null) { - $actualDsn = Dsn::fromString($string); + $dsn = new Dsn($dsnString); + $this->assertSame($dsnString, $dsn->getOriginalDsn()); - $this->assertSame($expectedDsn->getScheme(), $actualDsn->getScheme()); - $this->assertSame($expectedDsn->getHost(), $actualDsn->getHost()); - $this->assertSame($expectedDsn->getPort(), $actualDsn->getPort()); - $this->assertSame($expectedDsn->getUser(), $actualDsn->getUser()); - $this->assertSame($expectedDsn->getPassword(), $actualDsn->getPassword()); - $this->assertSame($expectedDsn->getPath(), $actualDsn->getPath()); - $this->assertSame($expectedDsn->getOption('from'), $actualDsn->getOption('from')); - - $this->assertSame($string, $actualDsn->getOriginalDsn()); + $this->assertSame($scheme, $dsn->getScheme()); + $this->assertSame($host, $dsn->getHost()); + $this->assertSame($user, $dsn->getUser()); + $this->assertSame($password, $dsn->getPassword()); + $this->assertSame($port, $dsn->getPort()); + $this->assertSame($path, $dsn->getPath()); + $this->assertSame($options, $dsn->getOptions()); } - public function fromStringProvider(): iterable + public function constructProvider(): iterable { yield 'simple dsn' => [ 'scheme://localhost', - new Dsn('scheme', 'localhost', null, null, null, [], null), + 'scheme', + 'localhost', + ]; + + yield 'simple dsn including @ sign, but no user/password/token' => [ + 'scheme://@localhost', + 'scheme', + 'localhost', + ]; + + yield 'simple dsn including : sign and @ sign, but no user/password/token' => [ + 'scheme://:@localhost', + 'scheme', + 'localhost', + ]; + + yield 'simple dsn including user, : sign and @ sign, but no password' => [ + 'scheme://user1:@localhost', + 'scheme', + 'localhost', + 'user1', + ]; + + yield 'simple dsn including : sign, password, and @ sign, but no user' => [ + 'scheme://:pass@localhost', + 'scheme', + 'localhost', + null, + 'pass', ]; yield 'dsn with user and pass' => [ 'scheme://u$er:pa$s@localhost', - new Dsn('scheme', 'localhost', 'u$er', 'pa$s', null, [], null), + 'scheme', + 'localhost', + 'u$er', + 'pa$s', ]; yield 'dsn with user and pass and custom port' => [ 'scheme://u$er:pa$s@localhost:8000', - new Dsn('scheme', 'localhost', 'u$er', 'pa$s', '8000', [], null), + 'scheme', + 'localhost', + 'u$er', + 'pa$s', + 8000, ]; yield 'dsn with user and pass, custom port and custom path' => [ 'scheme://u$er:pa$s@localhost:8000/channel', - new Dsn('scheme', 'localhost', 'u$er', 'pa$s', '8000', [], '/channel'), + 'scheme', + 'localhost', + 'u$er', + 'pa$s', + 8000, + [], + '/channel', + ]; + + yield 'dsn with user and pass, custom port, custom path and custom option' => [ + 'scheme://u$er:pa$s@localhost:8000/channel?from=FROM', + 'scheme', + 'localhost', + 'u$er', + 'pa$s', + 8000, + [ + 'from' => 'FROM', + ], + '/channel', ]; yield 'dsn with user and pass, custom port, custom path and custom options' => [ - 'scheme://u$er:pa$s@localhost:8000/channel?from=FROM', - new Dsn('scheme', 'localhost', 'u$er', 'pa$s', '8000', ['from' => 'FROM'], '/channel'), + 'scheme://u$er:pa$s@localhost:8000/channel?from=FROM&to=TO', + 'scheme', + 'localhost', + 'u$er', + 'pa$s', + 8000, + [ + 'from' => 'FROM', + 'to' => 'TO', + ], + '/channel', ]; - yield 'dsn with user and pass that contains an urlencoded character' => [ - 'scheme://u$er:p%2Fa$s@localhost', - new Dsn('scheme', 'localhost', 'u$er', 'p/a$s'), + yield 'dsn with user and pass, custom port, custom path and custom options and custom options keep the same order' => [ + 'scheme://u$er:pa$s@localhost:8000/channel?to=TO&from=FROM', + 'scheme', + 'localhost', + 'u$er', + 'pa$s', + 8000, + [ + 'to' => 'TO', + 'from' => 'FROM', + ], + '/channel', ]; } /** * @dataProvider invalidDsnProvider */ - public function testInvalidDsn(string $dsn, string $exceptionMessage): void + public function testInvalidDsn(string $dsnString, string $exceptionMessage) { $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage($exceptionMessage); - Dsn::fromString($dsn); + + new Dsn($dsnString); } public function invalidDsnProvider(): iterable @@ -96,13 +169,94 @@ class DsnTest extends TestCase ]; } - public function testGetOption(): void + /** + * @dataProvider getOptionProvider + */ + public function testGetOption($expected, string $dsnString, string $option, ?string $default = null) { - $options = ['with_value' => 'some value', 'nullable' => null]; - $dsn = new Dsn('scheme', 'localhost', 'u$er', 'pa$s', '8000', $options, '/channel'); + $dsn = new Dsn($dsnString); - $this->assertSame('some value', $dsn->getOption('with_value')); - $this->assertSame('default', $dsn->getOption('nullable', 'default')); - $this->assertSame('default', $dsn->getOption('not_existent_property', 'default')); + $this->assertSame($expected, $dsn->getOption($option, $default)); + } + + public function getOptionProvider(): iterable + { + yield [ + 'foo', + 'scheme://localhost?with_value=foo', + 'with_value', + ]; + + yield [ + '', + 'scheme://localhost?empty=', + 'empty', + ]; + + yield [ + '0', + 'scheme://localhost?zero=0', + 'zero', + ]; + + yield [ + 'default-value', + 'scheme://localhost?option=value', + 'non_existent_property', + 'default-value', + ]; + } + + /** + * @dataProvider getRequiredOptionProvider + */ + public function testGetRequiredOption(string $expectedValue, string $options, string $option) + { + $dsn = new Dsn(sprintf('scheme://localhost?%s', $options)); + + $this->assertSame($expectedValue, $dsn->getRequiredOption($option)); + } + + public function getRequiredOptionProvider(): iterable + { + yield [ + 'value', + 'with_value=value', + 'with_value', + ]; + + yield [ + '0', + 'timeout=0', + 'timeout', + ]; + } + + /** + * @dataProvider getRequiredOptionThrowsMissingRequiredOptionExceptionProvider + */ + public function testGetRequiredOptionThrowsMissingRequiredOptionException(string $expectedExceptionMessage, string $options, string $option) + { + $dsn = new Dsn(sprintf('scheme://localhost?%s', $options)); + + $this->expectException(MissingRequiredOptionException::class); + $this->expectExceptionMessage($expectedExceptionMessage); + + $dsn->getRequiredOption($option); + } + + public function getRequiredOptionThrowsMissingRequiredOptionExceptionProvider(): iterable + { + yield [ + 'The option "foo_bar" is required but missing.', + 'with_value=value', + 'foo_bar', + ]; + + yield [ + 'The option "with_empty_string" is required but missing.', + 'with_empty_string=', + 'with_empty_string', + ]; } } diff --git a/src/Symfony/Component/Translation/Tests/Provider/NullProviderFactoryTest.php b/src/Symfony/Component/Translation/Tests/Provider/NullProviderFactoryTest.php index 6f35ebad66..08e690bab2 100644 --- a/src/Symfony/Component/Translation/Tests/Provider/NullProviderFactoryTest.php +++ b/src/Symfony/Component/Translation/Tests/Provider/NullProviderFactoryTest.php @@ -28,11 +28,11 @@ class NullProviderFactoryTest extends TestCase { $this->expectException(UnsupportedSchemeException::class); - (new NullProviderFactory())->create(new Dsn('foo', '')); + (new NullProviderFactory())->create(new Dsn('foo://localhost')); } public function testCreate() { - $this->assertInstanceOf(NullProvider::class, (new NullProviderFactory())->create(new Dsn('null', ''))); + $this->assertInstanceOf(NullProvider::class, (new NullProviderFactory())->create(new Dsn('null://null'))); } }