From 9e479ebd3282d9b54c46697bf7e5c5779feda299 Mon Sep 17 00:00:00 2001 From: Pierre Rolland Date: Tue, 7 Apr 2020 15:35:43 +0200 Subject: [PATCH] Use factories instead of dynamic di (#11) * Fix sf4 * Tests * Added UT and comments * Tested parameters values Co-authored-by: Romain Delouard --- Command/VhostDefineCommand.php | 23 +- .../OlaRabbitMqAdminToolkitExtension.php | 56 +---- Exception/ConfigurationNotFoundException.php | 7 + Exception/ConnectionNotFoundException.php | 7 + Resources/config/services.xml | 9 +- Tests/Command/VhostDefineCommandTest.php | 30 +-- .../OlaRabbitMqAdminToolkitExtensionTest.php | 205 +++++++++++++----- Tests/VhostConfigurationFactoryTest.php | 72 ++++++ VhostConfigurationFactory.php | 61 ++++++ 9 files changed, 322 insertions(+), 148 deletions(-) create mode 100644 Exception/ConfigurationNotFoundException.php create mode 100644 Exception/ConnectionNotFoundException.php create mode 100644 Tests/VhostConfigurationFactoryTest.php create mode 100644 VhostConfigurationFactory.php diff --git a/Command/VhostDefineCommand.php b/Command/VhostDefineCommand.php index 15b1501..6fa8a43 100644 --- a/Command/VhostDefineCommand.php +++ b/Command/VhostDefineCommand.php @@ -2,10 +2,9 @@ namespace Ola\RabbitMqAdminToolkitBundle\Command; -use Ola\RabbitMqAdminToolkitBundle\DependencyInjection\OlaRabbitMqAdminToolkitExtension; use Ola\RabbitMqAdminToolkitBundle\VhostConfiguration; +use Ola\RabbitMqAdminToolkitBundle\VhostConfigurationFactory; use Ola\RabbitMqAdminToolkitBundle\VhostHandler; -use Psr\Container\ContainerInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -16,7 +15,7 @@ class VhostDefineCommand extends Command { protected static $defaultName = 'rabbitmq:vhost:define'; - private ContainerInterface $serviceLocator; + private VhostConfigurationFactory $vhostConfigurationFactory; private array $vhostList; @@ -25,14 +24,14 @@ class VhostDefineCommand extends Command private bool $silentFailure; public function __construct( - ContainerInterface $serviceLocator, + VhostConfigurationFactory $vhostConfigurationFactory, array $vhostList, VhostHandler $vhostHandler, bool $silentFailure ) { parent::__construct(); - $this->serviceLocator = $serviceLocator; + $this->vhostConfigurationFactory = $vhostConfigurationFactory; $this->vhostList = $vhostList; $this->vhostHandler = $vhostHandler; $this->silentFailure = $silentFailure; @@ -104,19 +103,7 @@ private function getVhostList(InputInterface $input): array */ private function getVhostConfiguration(string $vhost): VhostConfiguration { - $serviceName = sprintf( - OlaRabbitMqAdminToolkitExtension::VHOST_MANAGER_SERVICE_TEMPLATE, - $vhost - ); - - if (!$this->serviceLocator->has($serviceName)) { - throw new \InvalidArgumentException(sprintf( - 'No configuration service found for vhost : "%s"', - $vhost - )); - } - - return $this->serviceLocator->get($serviceName); + return $this->vhostConfigurationFactory->getVhostConfiguration($vhost); } /** diff --git a/DependencyInjection/OlaRabbitMqAdminToolkitExtension.php b/DependencyInjection/OlaRabbitMqAdminToolkitExtension.php index d6997c0..aeaaf5b 100644 --- a/DependencyInjection/OlaRabbitMqAdminToolkitExtension.php +++ b/DependencyInjection/OlaRabbitMqAdminToolkitExtension.php @@ -2,13 +2,8 @@ namespace Ola\RabbitMqAdminToolkitBundle\DependencyInjection; -use Ola\RabbitMqAdminToolkitBundle\ClientFactory; -use Ola\RabbitMqAdminToolkitBundle\VhostConfiguration; -use RabbitMq\ManagementApi\Client; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\Config\FileLocator; -use Symfony\Component\DependencyInjection\Definition; -use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\HttpKernel\DependencyInjection\Extension; use Symfony\Component\DependencyInjection\Loader; @@ -20,8 +15,6 @@ class OlaRabbitMqAdminToolkitExtension extends Extension { const PARAMETER_TEMPLATE = 'ola_rabbit_mq_admin_toolkit.%s'; - const CONNECTION_SERVICE_TEMPLATE = 'ola_rabbit_mq_admin_toolkit.connection.%s'; - const VHOST_MANAGER_SERVICE_TEMPLATE = 'ola_rabbit_mq_admin_toolkit.configuration.%s'; /** * {@inheritdoc} @@ -33,54 +26,11 @@ public function load(array $configs, ContainerBuilder $container) $container->setParameter(sprintf(self::PARAMETER_TEMPLATE, 'vhost_list'), array_keys($config['vhosts'])); $container->setParameter(sprintf(self::PARAMETER_TEMPLATE, 'silent_failure'), $config['silent_failure']); - - $this->loadConnections($config['connections'], $container); - $this->loadVhostManagers($config['vhosts'], $container, $config['delete_allowed']); + $container->setParameter(sprintf(self::PARAMETER_TEMPLATE, 'connections'), $config['connections']); + $container->setParameter(sprintf(self::PARAMETER_TEMPLATE, 'vhosts'), $config['vhosts']); + $container->setParameter(sprintf(self::PARAMETER_TEMPLATE, 'delete_allowed'), $config['delete_allowed']); $loader = new Loader\XmlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config')); $loader->load('services.xml'); } - - /** - * @param array $connections - * @param ContainerBuilder $container - */ - private function loadConnections(array $connections, ContainerBuilder $container): void - { - foreach ($connections as $name => $uri) { - $parsedUri = parse_url($uri); - - $definition = new Definition(Client::class, [ - $parsedUri['scheme'], - $parsedUri['host'], - $parsedUri['user'], - $parsedUri['pass'], - $parsedUri['port'] ?? 80 - ]); - - $definition->setFactory([ClientFactory::class, 'getClient']); // necessary to have the exception handling - $definition->setPublic(true); - $container->setDefinition(sprintf(self::CONNECTION_SERVICE_TEMPLATE, $name), $definition); - } - } - - /** - * @param array $vhosts - * @param ContainerBuilder $container - * @param bool $deleteAllowed - */ - private function loadVhostManagers(array $vhosts, ContainerBuilder $container, bool $deleteAllowed): void - { - foreach ($vhosts as $name => $vhost) { - $definition = new Definition(VhostConfiguration::class, [ - new Reference(sprintf(self::CONNECTION_SERVICE_TEMPLATE, $vhost['connection'])), - !empty($vhost['name']) ? $vhost['name'] : $name, - $vhost, - $deleteAllowed - ]); - $definition->setPublic(true); - $definition->addTag('ola_rabbit_mq_admin_toolkit.vhost_configuration'); - $container->setDefinition(sprintf(self::VHOST_MANAGER_SERVICE_TEMPLATE, $name), $definition); - } - } } diff --git a/Exception/ConfigurationNotFoundException.php b/Exception/ConfigurationNotFoundException.php new file mode 100644 index 0000000..a472c00 --- /dev/null +++ b/Exception/ConfigurationNotFoundException.php @@ -0,0 +1,7 @@ + + + + + %ola_rabbit_mq_admin_toolkit.delete_allowed% + %ola_rabbit_mq_admin_toolkit.connections% + %ola_rabbit_mq_admin_toolkit.vhosts% + @@ -19,7 +26,7 @@ - + %ola_rabbit_mq_admin_toolkit.vhost_list% %ola_rabbit_mq_admin_toolkit.silent_failure% diff --git a/Tests/Command/VhostDefineCommandTest.php b/Tests/Command/VhostDefineCommandTest.php index 442d0c9..c4d82c6 100644 --- a/Tests/Command/VhostDefineCommandTest.php +++ b/Tests/Command/VhostDefineCommandTest.php @@ -4,16 +4,16 @@ use Ola\RabbitMqAdminToolkitBundle\Command\VhostDefineCommand; use Ola\RabbitMqAdminToolkitBundle\VhostConfiguration; +use Ola\RabbitMqAdminToolkitBundle\VhostConfigurationFactory; use Ola\RabbitMqAdminToolkitBundle\VhostHandler; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; -use Symfony\Component\DependencyInjection\ContainerInterface; class VhostDefineCommandTest extends TestCase { - private ObjectProphecy $container; + private ObjectProphecy $vhostConfigurationFactory; private ObjectProphecy $configuration; private ObjectProphecy $handler; @@ -24,7 +24,7 @@ class VhostDefineCommandTest extends TestCase public function setUp(): void { $this->configuration = $this->prophesize(VhostConfiguration::class); - $this->container = $this->prophesize(ContainerInterface::class); + $this->vhostConfigurationFactory = $this->prophesize(VhostConfigurationFactory::class); $this->handler = $this->prophesize(VhostHandler::class); $this->defineCommand(false); @@ -32,16 +32,16 @@ public function setUp(): void public function test_execute_withoutDefaultVhost(): void { - $this->expectException(\InvalidArgumentException::class); + $this->expectException(\Exception::class); - $this->container->has('ola_rabbit_mq_admin_toolkit.configuration.foo')->willReturn(false); + $this->vhostConfigurationFactory->getVhostConfiguration('foo')->willThrow(\Exception::class); $this->commandTester->execute(['command' => 'rabbitmq:define:vhost']); } public function test_execute_withoutDefaultVhostButSilentFailure(): void { - $this->container->has('ola_rabbit_mq_admin_toolkit.configuration.foo')->willReturn(false); + $this->vhostConfigurationFactory->getVhostConfiguration('foo')->willThrow(\Exception::class); $this->defineCommand(true); $this->assertEquals(0, $this->commandTester->execute(['command' => 'rabbitmq:define:vhost'])); @@ -52,9 +52,7 @@ public function test_execute_creationWithDefaultVhost(): void $this->handler->exists($this->configuration)->willReturn(false); $this->handler->define($this->configuration)->shouldBeCalled(); - $this->container->has('ola_rabbit_mq_admin_toolkit.configuration.foo')->willReturn(true); - $this->container->get('ola_rabbit_mq_admin_toolkit.configuration.foo') - ->willReturn($this->configuration->reveal()); + $this->vhostConfigurationFactory->getVhostConfiguration('foo')->willReturn($this->configuration->reveal()); $this->commandTester->execute(['command' => 'rabbitmq:define:vhost']); @@ -66,9 +64,7 @@ public function test_execute_creationWithSpecificVhost(): void $this->handler->exists($this->configuration)->willReturn(false); $this->handler->define($this->configuration)->shouldBeCalled(); - $this->container->has('ola_rabbit_mq_admin_toolkit.configuration.bar')->willReturn(true); - $this->container->get('ola_rabbit_mq_admin_toolkit.configuration.bar') - ->willReturn($this->configuration->reveal()); + $this->vhostConfigurationFactory->getVhostConfiguration('bar')->willReturn($this->configuration->reveal()); $this->commandTester->execute(['vhost' => 'bar']); @@ -80,9 +76,7 @@ public function test_execute_updateWithDefaultVhost(): void $this->handler->exists($this->configuration)->willReturn(true); $this->handler->define($this->configuration)->shouldBeCalled(); - $this->container->has('ola_rabbit_mq_admin_toolkit.configuration.foo')->willReturn(true); - $this->container->get('ola_rabbit_mq_admin_toolkit.configuration.foo') - ->willReturn($this->configuration->reveal()); + $this->vhostConfigurationFactory->getVhostConfiguration('foo')->willReturn($this->configuration->reveal()); $this->commandTester->execute(['command' => 'rabbitmq:define:vhost']); @@ -94,9 +88,7 @@ public function test_execute_updateWithSpecificVhost(): void $this->handler->exists($this->configuration)->willReturn(true); $this->handler->define($this->configuration)->shouldBeCalled(); - $this->container->has('ola_rabbit_mq_admin_toolkit.configuration.bar')->willReturn(true); - $this->container->get('ola_rabbit_mq_admin_toolkit.configuration.bar') - ->willReturn($this->configuration->reveal()); + $this->vhostConfigurationFactory->getVhostConfiguration('bar')->willReturn($this->configuration->reveal()); $this->commandTester->execute(['vhost' => 'bar']); @@ -107,7 +99,7 @@ private function defineCommand(bool $silentFailure): void { $this->application = new Application(); $this->command = new VhostDefineCommand( - $this->container->reveal(), + $this->vhostConfigurationFactory->reveal(), ['foo'], $this->handler->reveal(), $silentFailure diff --git a/Tests/DependencyInjection/OlaRabbitMqAdminToolkitExtensionTest.php b/Tests/DependencyInjection/OlaRabbitMqAdminToolkitExtensionTest.php index 1d7c6b7..fc28ff3 100644 --- a/Tests/DependencyInjection/OlaRabbitMqAdminToolkitExtensionTest.php +++ b/Tests/DependencyInjection/OlaRabbitMqAdminToolkitExtensionTest.php @@ -17,75 +17,166 @@ protected function getContainerExtensions(): array public function test_load_successfull() { - $this->load([ - 'delete_allowed' => true, - 'connections' => [ - 'default' => 'http://user:password@localhost', - ], - 'vhosts' => [ - 'test' => [ - 'name' => '/test', - 'connection' => 'default', - 'permissions' => [ - 'lafourchette' => NULL, + $connections = [ + 'default' => 'http://user:password@localhost', + ]; + $vhosts = [ + 'test' => [ + 'name' => '/test', + 'connection' => 'default', + 'permissions' => [ + 'lafourchette' => NULL, + ], + 'exchanges' => [ + 'exchange.a' => NULL, + 'exchange.b' => [ + 'type' => 'direct', ], - 'exchanges' => [ - 'exchange.a' => NULL, - 'exchange.b' => [ - 'type' => 'direct', + 'exchange.c' => NULL, + ], + 'queues' => [ + 'queue.a.sharded' => [ + 'name' => 'queue.a.{modulus}', + 'modulus' => 10, + 'bindings' => [ + [ + 'exchange' => 'exchange.a', + 'routing_key' => 'a.{modulus}.#', + ], + [ + 'exchange' => 'exchange.b', + 'routing_key' => 'b.#', + ], ], - 'exchange.c' => NULL, - ], - 'queues' => [ - 'queue.a.sharded' => [ - 'name' => 'queue.a.{modulus}', - 'modulus' => 10, - 'bindings' => [ - [ - 'exchange' => 'exchange.a', - 'routing_key' => 'a.{modulus}.#', - ], - [ - 'exchange' => 'exchange.b', - 'routing_key' => 'b.#', - ], + ], + 'queue.b.{modulus}' => [ + 'bindings' => [ + [ + 'exchange' => 'exchange.a', + 'routing_key' => 'a.#', + ], + [ + 'exchange' => 'exchange.b', + 'routing_key' => 'b.{modulus}.#', + ], + [ + 'exchange' => 'exchange.c', + 'routing_key' => 'c.#', ], ], - 'queue.b.{modulus}' => [ - 'bindings' => [ - [ - 'exchange' => 'exchange.a', - 'routing_key' => 'a.#', - ], - [ - 'exchange' => 'exchange.b', - 'routing_key' => 'b.{modulus}.#', - ], - [ - 'exchange' => 'exchange.c', - 'routing_key' => 'c.#', - ], + ], + 'queue.c' => [ + 'bindings' => [ + [ + 'exchange' => 'exchange.a', + 'routing_key' => 'a.#', + ], + [ + 'exchange' => 'exchange.c', + 'routing_key' => 'c.#', ], ], - 'queue.c' => [ - 'bindings' => [ - [ - 'exchange' => 'exchange.a', - 'routing_key' => 'a.#', - ], - [ - 'exchange' => 'exchange.c', - 'routing_key' => 'c.#', - ], + ], + ], + ], + ]; + + $expectedVhosts = [ + 'test' => [ + 'name' => '/test', + 'connection' => 'default', + 'permissions' => [ + 'lafourchette' => [ + 'configure' => '.*', + 'read' => '.*', + 'write' => '.*' + ], + ], + 'exchanges' => [ + 'exchange.a' => [ + 'name' => 'exchange.a', + 'type' => 'topic', + 'durable' => true + ], + 'exchange.b' => [ + 'type' => 'direct', + 'name' => 'exchange.b', + 'durable' => true + ], + 'exchange.c' => [ + 'name' => 'exchange.c', + 'type' => 'topic', + 'durable' => true + ], + ], + 'queues' => [ + 'queue.a.sharded' => [ + 'name' => 'queue.a.{modulus}', + 'modulus' => 10, + 'bindings' => [ + [ + 'exchange' => 'exchange.a', + 'routing_key' => 'a.{modulus}.#', + ], + [ + 'exchange' => 'exchange.b', + 'routing_key' => 'b.#', + ], + ], + 'durable' => true, + 'arguments' => [] + ], + 'queue.b.{modulus}' => [ + 'bindings' => [ + [ + 'exchange' => 'exchange.a', + 'routing_key' => 'a.#', + ], + [ + 'exchange' => 'exchange.b', + 'routing_key' => 'b.{modulus}.#', + ], + [ + 'exchange' => 'exchange.c', + 'routing_key' => 'c.#', ], ], + 'name' => 'queue.b.{modulus}', + 'durable' => true, + 'modulus' => null, + 'arguments' => [] + + ], + 'queue.c' => [ + 'bindings' => [ + [ + 'exchange' => 'exchange.a', + 'routing_key' => 'a.#', + ], + [ + 'exchange' => 'exchange.c', + 'routing_key' => 'c.#', + ], + ], + 'name' => 'queue.c', + 'durable' => true, + 'modulus' => null, + 'arguments' => [] ], ], ], + ]; + + $this->load([ + 'delete_allowed' => true, + 'connections' => $connections, + 'vhosts' => $vhosts, ]); - $this->assertContainerBuilderHasService('ola_rabbit_mq_admin_toolkit.connection.default'); - $this->assertContainerBuilderHasService('ola_rabbit_mq_admin_toolkit.configuration.test'); - $this->assertContainerBuilderHasParameter('ola_rabbit_mq_admin_toolkit.vhost_list'); + + $this->assertContainerBuilderHasParameter('ola_rabbit_mq_admin_toolkit.vhost_list', ['test']); + $this->assertContainerBuilderHasParameter('ola_rabbit_mq_admin_toolkit.vhosts', $expectedVhosts); + $this->assertContainerBuilderHasParameter('ola_rabbit_mq_admin_toolkit.connections', $connections); + $this->assertContainerBuilderHasParameter('ola_rabbit_mq_admin_toolkit.delete_allowed', true); } public function dataProvider_load_failBecauseModulusIsImproperlyDefined(): array diff --git a/Tests/VhostConfigurationFactoryTest.php b/Tests/VhostConfigurationFactoryTest.php new file mode 100644 index 0000000..d642956 --- /dev/null +++ b/Tests/VhostConfigurationFactoryTest.php @@ -0,0 +1,72 @@ +clientFactory = $this->prophesize(ClientFactory::class); + $this->vhostConfigurationFactory = new VhostConfigurationFactory( + $this->clientFactory->reveal(), + true, + [ + 'default' => 'https://login:pwd@rabbitmq.local:443' + ], + [ + 'default' => [ + 'connection' => 'default' + ], + 'with_connection_not_found' => [ + 'connection' => 'not_found' + ] + ] + ); + } + + public function testWithVhostNotFound() + { + $this->expectException(ConfigurationNotFoundException::class); + $this->clientFactory->getClient(Argument::cetera())->shouldNotBeCalled(); + + $this->vhostConfigurationFactory->getVhostConfiguration('not_found'); + } + + public function testWithConnectionNotFound() + { + $this->expectException(ConnectionNotFoundException::class); + $this->clientFactory->getClient(Argument::cetera())->shouldNotBeCalled(); + + $this->vhostConfigurationFactory->getVhostConfiguration('with_connection_not_found'); + } + + public function testWithWorkingScenario() + { + $this + ->clientFactory + ->getClient( + 'https', + 'rabbitmq.local', + 'login', + 'pwd', + 443 + )->shouldBeCalledTimes(1); + + $result = $this->vhostConfigurationFactory->getVhostConfiguration('default'); + + $this->assertInstanceOf(VhostConfiguration::class, $result); + } +} diff --git a/VhostConfigurationFactory.php b/VhostConfigurationFactory.php new file mode 100644 index 0000000..69e9d15 --- /dev/null +++ b/VhostConfigurationFactory.php @@ -0,0 +1,61 @@ +clientFactory = $clientFactory; + $this->deleteAllowed = $deleteAllowed; + $this->connections = $connections; + $this->vhosts = $vhosts; + } + + public function getVhostConfiguration(string $vhostName): VhostConfiguration + { + $vhostConfiguration = $this->vhosts[$vhostName] ?? null; + + if (null === $vhostConfiguration) { + throw new ConfigurationNotFoundException(sprintf('No vhost configuration found for vhost %s', $vhostName)); + } + + $connectionUri = $this->connections[$vhostConfiguration['connection']] ?? null; + + if (null === $connectionUri) { + throw new ConnectionNotFoundException(sprintf('No connection found for vhost %s', $vhostName)); + } + + $parsedUri = parse_url($connectionUri); + + return new VhostConfiguration( + $this->clientFactory->getClient( + $parsedUri['scheme'], + $parsedUri['host'], + $parsedUri['user'], + $parsedUri['pass'], + $parsedUri['port'] ?? 80 + ), + $vhostConfiguration['name'] ?? $vhostName, + $vhostConfiguration, + $this->deleteAllowed + ); + } +}