From c44b7c067d1128d687a7aace80a7f624e6d7ca27 Mon Sep 17 00:00:00 2001 From: Maksim Kotlyar Date: Sat, 3 Nov 2018 12:42:15 +0200 Subject: [PATCH 1/2] [fs] Use enqueue/dsn to parse DSN --- docs/transport/filesystem.md | 4 +- pkg/dsn/Dsn.php | 14 +++++ pkg/dsn/Tests/DsnTest.php | 17 +++++ pkg/fs/FsConnectionFactory.php | 63 +++++++++---------- .../Tests/FsConnectionFactoryConfigTest.php | 18 ++++-- pkg/fs/composer.json | 1 + 6 files changed, 76 insertions(+), 41 deletions(-) diff --git a/docs/transport/filesystem.md b/docs/transport/filesystem.md index a57b93e5d..d958149f7 100644 --- a/docs/transport/filesystem.md +++ b/docs/transport/filesystem.md @@ -35,10 +35,10 @@ $connectionFactory = new FsConnectionFactory('file:'); $connectionFactory = new FsConnectionFactory('/path/to/queue/dir'); // same as above -$connectionFactory = new FsConnectionFactory('file://path/to/queue/dir'); +$connectionFactory = new FsConnectionFactory('file:///path/to/queue/dir'); // with options -$connectionFactory = new FsConnectionFactory('file://path/to/queue/dir?pre_fetch_count=1'); +$connectionFactory = new FsConnectionFactory('file:///path/to/queue/dir?pre_fetch_count=1'); // as an array $connectionFactory = new FsConnectionFactory([ diff --git a/pkg/dsn/Dsn.php b/pkg/dsn/Dsn.php index d5e01fc7d..77b974a6e 100644 --- a/pkg/dsn/Dsn.php +++ b/pkg/dsn/Dsn.php @@ -165,6 +165,20 @@ public function getInt(string $name, int $default = null): ?int return (int) $value; } + public function getOctal(string $name, int $default = null): ?int + { + $value = $this->getQueryParameter($name); + if (null === $value) { + return $default; + } + + if (false == preg_match('/^[\+\-]?[0-9]*$/', $value)) { + throw InvalidQueryParameterTypeException::create($name, 'integer'); + } + + return intval($value, 8); + } + public function getFloat(string $name, float $default = null): ?float { $value = $this->getQueryParameter($name); diff --git a/pkg/dsn/Tests/DsnTest.php b/pkg/dsn/Tests/DsnTest.php index 05abc4890..644c3934b 100644 --- a/pkg/dsn/Tests/DsnTest.php +++ b/pkg/dsn/Tests/DsnTest.php @@ -107,6 +107,16 @@ public function testShouldParseQueryParameterAsInt(string $parameter, int $expec $this->assertSame($expected, $dsn->getInt('aName')); } + /** + * @dataProvider provideOctalQueryParameters + */ + public function testShouldParseQueryParameterAsOctalInt(string $parameter, int $expected) + { + $dsn = new Dsn('foo:?aName='.$parameter); + + $this->assertSame($expected, $dsn->getOctal('aName')); + } + public function testShouldReturnDefaultIntIfNotSet() { $dsn = new Dsn('foo:'); @@ -204,6 +214,13 @@ public static function provideIntQueryParameters() yield ['+123', 123]; yield ['-123', -123]; + + yield ['010', 10]; + } + + public static function provideOctalQueryParameters() + { + yield ['010', 8]; } public static function provideFloatQueryParameters() diff --git a/pkg/fs/FsConnectionFactory.php b/pkg/fs/FsConnectionFactory.php index c35c52e5a..829014e38 100644 --- a/pkg/fs/FsConnectionFactory.php +++ b/pkg/fs/FsConnectionFactory.php @@ -4,6 +4,7 @@ namespace Enqueue\Fs; +use Enqueue\Dsn\Dsn; use Interop\Queue\ConnectionFactory; use Interop\Queue\Context; @@ -27,23 +28,31 @@ class FsConnectionFactory implements ConnectionFactory * or * * file: - create queue files in tmp dir. - * file://home/foo/enqueue - * file://home/foo/enqueue?pre_fetch_count=20&chmod=0777 + * file:///home/foo/enqueue + * file:///home/foo/enqueue?pre_fetch_count=20&chmod=0777 * * @param array|string|null $config */ public function __construct($config = 'file:') { if (empty($config) || 'file:' === $config) { - $config = ['path' => sys_get_temp_dir().'/enqueue']; + $config = $this->parseDsn('file://'.sys_get_temp_dir().'/enqueue'); } elseif (is_string($config)) { - $config = $this->parseDsn($config); + if ('/' === $config[0]) { + $config = $this->parseDsn('file://'.$config); + } else { + $config = $this->parseDsn($config); + } } elseif (is_array($config)) { } else { throw new \LogicException('The config must be either an array of options, a DSN string or null'); } $this->config = array_replace($this->defaultConfig(), $config); + + if (empty($this->config['path'])) { + throw new \LogicException('The path option must be set.'); + } } /** @@ -61,39 +70,23 @@ public function createContext(): Context private function parseDsn(string $dsn): array { - if ($dsn && '/' === $dsn[0]) { - return ['path' => $dsn]; - } - - if (false === strpos($dsn, 'file:')) { - throw new \LogicException(sprintf('The given DSN "%s" is not supported. Must start with "file:".', $dsn)); - } - - $dsn = substr($dsn, 7); - - $path = parse_url($dsn, PHP_URL_PATH); - $query = parse_url($dsn, PHP_URL_QUERY); - - if ('/' != $path[0]) { - throw new \LogicException(sprintf('Failed to parse DSN path "%s". The path must start with "/"', $path)); + $dsn = new Dsn($dsn); + + $supportedSchemes = ['file']; + if (false == in_array($dsn->getSchemeProtocol(), $supportedSchemes, true)) { + throw new \LogicException(sprintf( + 'The given scheme protocol "%s" is not supported. It must be one of "%s"', + $dsn->getSchemeProtocol(), + implode('", "', $supportedSchemes) + )); } - if ($query) { - $config = []; - parse_str($query, $config); - } - - if (isset($config['pre_fetch_count'])) { - $config['pre_fetch_count'] = (int) $config['pre_fetch_count']; - } - - if (isset($config['chmod'])) { - $config['chmod'] = intval($config['chmod'], 8); - } - - $config['path'] = $path; - - return $config; + return array_filter(array_replace($dsn->getQuery(), [ + 'path' => $dsn->getPath(), + 'pre_fetch_count' => $dsn->getInt('pre_fetch_count'), + 'chmod' => $dsn->getOctal('chmod'), + 'polling_interval' => $dsn->getInt('polling_interval'), + ]), function ($value) { return null !== $value; }); } private function defaultConfig(): array diff --git a/pkg/fs/Tests/FsConnectionFactoryConfigTest.php b/pkg/fs/Tests/FsConnectionFactoryConfigTest.php index c79581c51..59193d26d 100644 --- a/pkg/fs/Tests/FsConnectionFactoryConfigTest.php +++ b/pkg/fs/Tests/FsConnectionFactoryConfigTest.php @@ -21,10 +21,10 @@ public function testThrowNeitherArrayStringNorNullGivenAsConfig() new FsConnectionFactory(new \stdClass()); } - public function testThrowIfSchemeIsNotAmqp() + public function testThrowIfSchemeIsNotFileScheme() { $this->expectException(\LogicException::class); - $this->expectExceptionMessage('The given DSN "http://example.com" is not supported. Must start with "file:'); + $this->expectExceptionMessage('The given scheme protocol "http" is not supported. It must be one of "file"'); new FsConnectionFactory('http://example.com'); } @@ -32,9 +32,19 @@ public function testThrowIfSchemeIsNotAmqp() public function testThrowIfDsnCouldNotBeParsed() { $this->expectException(\LogicException::class); - $this->expectExceptionMessage('Failed to parse DSN path ":@/". The path must start with "/"'); + $this->expectExceptionMessage('The DSN is invalid.'); - new FsConnectionFactory('file://:@/'); + new FsConnectionFactory('foo'); + } + + public function testThrowIfArrayConfigGivenWithEmptyPath() + { + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('The path option must be set.'); + + new FsConnectionFactory([ + 'path' => null, + ]); } /** diff --git a/pkg/fs/composer.json b/pkg/fs/composer.json index b0328b168..13d3df8a7 100644 --- a/pkg/fs/composer.json +++ b/pkg/fs/composer.json @@ -8,6 +8,7 @@ "require": { "php": "^7.1.3", "queue-interop/queue-interop": "0.7.x-dev", + "enqueue/dsn": "0.9.x-dev", "symfony/filesystem": "^3.4|^4", "makasim/temp-file": "^0.2@stable" }, From d60432966b51049394a93f3640dd50498f5000ad Mon Sep 17 00:00:00 2001 From: Maksim Kotlyar Date: Sat, 3 Nov 2018 12:54:36 +0200 Subject: [PATCH 2/2] [dsn] add more octal tests --- pkg/dsn/Dsn.php | 2 +- pkg/dsn/Tests/DsnTest.php | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/pkg/dsn/Dsn.php b/pkg/dsn/Dsn.php index 77b974a6e..81828e7d1 100644 --- a/pkg/dsn/Dsn.php +++ b/pkg/dsn/Dsn.php @@ -172,7 +172,7 @@ public function getOctal(string $name, int $default = null): ?int return $default; } - if (false == preg_match('/^[\+\-]?[0-9]*$/', $value)) { + if (false == preg_match('/^0[\+\-]?[0-7]*$/', $value)) { throw InvalidQueryParameterTypeException::create($name, 'integer'); } diff --git a/pkg/dsn/Tests/DsnTest.php b/pkg/dsn/Tests/DsnTest.php index 644c3934b..a81cdcfd1 100644 --- a/pkg/dsn/Tests/DsnTest.php +++ b/pkg/dsn/Tests/DsnTest.php @@ -134,6 +134,33 @@ public function testThrowIfQueryParameterNotInt() $dsn->getInt('aName'); } + public function testThrowIfQueryParameterNotOctalButString() + { + $dsn = new Dsn('foo:?aName=notInt'); + + $this->expectException(InvalidQueryParameterTypeException::class); + $this->expectExceptionMessage('The query parameter "aName" has invalid type. It must be "integer"'); + $dsn->getOctal('aName'); + } + + public function testThrowIfQueryParameterNotOctalButDecimal() + { + $dsn = new Dsn('foo:?aName=123'); + + $this->expectException(InvalidQueryParameterTypeException::class); + $this->expectExceptionMessage('The query parameter "aName" has invalid type. It must be "integer"'); + $dsn->getOctal('aName'); + } + + public function testThrowIfQueryParameterInvalidOctal() + { + $dsn = new Dsn('foo:?aName=0128'); + + $this->expectException(InvalidQueryParameterTypeException::class); + $this->expectExceptionMessage('The query parameter "aName" has invalid type. It must be "integer"'); + $dsn->getOctal('aName'); + } + /** * @dataProvider provideFloatQueryParameters */