From d8c70fc4543fbf7b406be0a0139e88df05884534 Mon Sep 17 00:00:00 2001 From: Thomas Vargiu Date: Wed, 1 Feb 2017 00:46:48 +0100 Subject: [PATCH 1/5] Added --ignore-unresolved param in ConfigDumperCommand --- src/Tool/ConfigDumper.php | 41 +++++++++-- src/Tool/ConfigDumperCommand.php | 28 ++++++-- .../ObjectWithObjectScalarDependency.php | 15 ++++ test/Tool/ConfigDumperCommandTest.php | 44 ++++++++++++ test/Tool/ConfigDumperTest.php | 72 +++++++++++++++++++ 5 files changed, 189 insertions(+), 11 deletions(-) create mode 100644 test/TestAsset/ObjectWithObjectScalarDependency.php diff --git a/src/Tool/ConfigDumper.php b/src/Tool/ConfigDumper.php index 24661e7e..9b59e2fe 100644 --- a/src/Tool/ConfigDumper.php +++ b/src/Tool/ConfigDumper.php @@ -7,6 +7,7 @@ namespace Zend\ServiceManager\Tool; +use Interop\Container\ContainerInterface; use ReflectionClass; use ReflectionParameter; use Traversable; @@ -25,13 +26,35 @@ class ConfigDumper return %s; EOC; + /** + * @var ContainerInterface + */ + protected $container; + + /** + * @param ContainerInterface $container + */ + public function __construct(ContainerInterface $container = null) + { + $this->container = $container; + } + + /** + * @param ContainerInterface $container + */ + public function setContainer(ContainerInterface $container) + { + $this->container = $container; + } + /** * @param array $config * @param string $className + * @param bool $ignoreUnresolved * @return array * @throws InvalidArgumentException for invalid $className */ - public function createDependencyConfig(array $config, $className) + public function createDependencyConfig(array $config, $className, $ignoreUnresolved = false) { $this->validateClassName($className); @@ -60,11 +83,19 @@ function (ReflectionParameter $argument) { return $this->createInvokable($config, $className); } - $config[ConfigAbstractFactory::class][$className] = []; + $classConfig = []; foreach ($constructorArguments as $constructorArgument) { $argumentType = $constructorArgument->getClass(); if (is_null($argumentType)) { + if ($ignoreUnresolved) { + // don't throw an exception, just return the previous config + return $config; + } + // don't throw an exception if the class is an already defined service + if ($this->container && $this->container->has($className)) { + return $config; + } throw new InvalidArgumentException(sprintf( 'Cannot create config for constructor argument "%s", ' . 'it has no type hint, or non-class/interface type hint', @@ -72,10 +103,12 @@ function (ReflectionParameter $argument) { )); } $argumentName = $argumentType->getName(); - $config = $this->createDependencyConfig($config, $argumentName); - $config[ConfigAbstractFactory::class][$className][] = $argumentName; + $config = $this->createDependencyConfig($config, $argumentName, $ignoreUnresolved); + $classConfig[] = $argumentName; } + $config[ConfigAbstractFactory::class][$className] = $classConfig; + return $config; } diff --git a/src/Tool/ConfigDumperCommand.php b/src/Tool/ConfigDumperCommand.php index 22fac0ca..9c3e65e8 100644 --- a/src/Tool/ConfigDumperCommand.php +++ b/src/Tool/ConfigDumperCommand.php @@ -26,6 +26,8 @@ class ConfigDumperCommand Arguments: -h|--help|help This usage message + -i|--ignore-unresolved Ignore classes with unresolved + direct dependencies. Path to a config file for which to generate configuration. If the file does not exist, it will be created. If it does exist, it must return an array, and the file will be @@ -81,7 +83,11 @@ public function __invoke(array $args) $dumper = new ConfigDumper(); try { - $config = $dumper->createDependencyConfig($arguments->config, $arguments->class); + $config = $dumper->createDependencyConfig( + $arguments->config, + $arguments->class, + $arguments->ignoreUnresolved + ); } catch (Exception\InvalidArgumentException $e) { $this->helper->writeErrorMessage(sprintf( 'Unable to create config for "%s": %s', @@ -117,6 +123,12 @@ private function parseArgs(array $args) return $this->createHelpArgument(); } + $ignoreUnresolved = false; + if (in_array($arg1, ['-i', '--ignore-unresolved'], true)) { + $ignoreUnresolved = true; + $arg1 = array_shift($args); + } + if (! count($args)) { return $this->createErrorArgument('Missing class name'); } @@ -157,7 +169,7 @@ private function parseArgs(array $args) )); } - return $this->createArguments(self::COMMAND_DUMP, $configFile, $config, $class); + return $this->createArguments(self::COMMAND_DUMP, $configFile, $config, $class, $ignoreUnresolved); } /** @@ -178,15 +190,17 @@ private function help($resource = STDOUT) * which it will be written. * @param array $config Parsed configuration. * @param string $class Name of class to reflect. + * @param bool $ignoreUnresolved If to ignore classes with unresolved direct dependencies. * @return \stdClass */ - private function createArguments($command, $configFile, $config, $class) + private function createArguments($command, $configFile, $config, $class, $ignoreUnresolved) { return (object) [ - 'command' => $command, - 'configFile' => $configFile, - 'config' => $config, - 'class' => $class, + 'command' => $command, + 'configFile' => $configFile, + 'config' => $config, + 'class' => $class, + 'ignoreUnresolved' => $ignoreUnresolved, ]; } diff --git a/test/TestAsset/ObjectWithObjectScalarDependency.php b/test/TestAsset/ObjectWithObjectScalarDependency.php new file mode 100644 index 00000000..88b5102f --- /dev/null +++ b/test/TestAsset/ObjectWithObjectScalarDependency.php @@ -0,0 +1,15 @@ + ['-i'], + 'long' => ['--ignore-unresolved'], + ]; + } + /** * @dataProvider helpArguments */ @@ -110,6 +119,41 @@ public function testGeneratesConfigFileWhenProvidedConfigurationFileNotFound() $this->assertEquals([], $factoryConfig[InvokableObject::class]); } + /** + * @dataProvider ignoreUnresolvedArguments + */ + public function testGeneratesConfigFileIgnoringUnresolved($argument) + { + $command = $this->command; + vfsStream::newDirectory('config', 0775) + ->at($this->configDir); + $config = vfsStream::url('project/config/test.config.php'); + + $this->helper->writeLine('[DONE] Changes written to ' . $config)->shouldBeCalled(); + + $this->assertEquals(0, $command([$argument, $config, ObjectWithObjectScalarDependency::class])); + + $generated = include $config; + $this->assertInternalType('array', $generated); + $this->assertArrayHasKey(ConfigAbstractFactory::class, $generated); + $factoryConfig = $generated[ConfigAbstractFactory::class]; + $this->assertInternalType('array', $factoryConfig); + $this->assertArrayHasKey(SimpleDependencyObject::class, $factoryConfig); + $this->assertArrayHasKey(InvokableObject::class, $factoryConfig); + $this->assertContains(InvokableObject::class, $factoryConfig[SimpleDependencyObject::class]); + $this->assertEquals([], $factoryConfig[InvokableObject::class]); + + $this->assertArrayHasKey(ObjectWithObjectScalarDependency::class, $factoryConfig); + $this->assertContains( + SimpleDependencyObject::class, + $factoryConfig[ObjectWithObjectScalarDependency::class] + ); + $this->assertContains( + ObjectWithScalarDependency::class, + $factoryConfig[ObjectWithObjectScalarDependency::class] + ); + } + public function testEmitsErrorWhenConfigurationFileDoesNotReturnArray() { $command = $this->command; diff --git a/test/Tool/ConfigDumperTest.php b/test/Tool/ConfigDumperTest.php index d3287e43..fb48329b 100644 --- a/test/Tool/ConfigDumperTest.php +++ b/test/Tool/ConfigDumperTest.php @@ -7,6 +7,7 @@ namespace ZendTest\ServiceManager\Tool; +use Interop\Container\ContainerInterface; use PHPUnit_Framework_TestCase as TestCase; use Zend\ServiceManager\AbstractFactory\ConfigAbstractFactory; use Zend\ServiceManager\Exception\InvalidArgumentException; @@ -16,6 +17,7 @@ use ZendTest\ServiceManager\TestAsset\DoubleDependencyObject; use ZendTest\ServiceManager\TestAsset\FailingFactory; use ZendTest\ServiceManager\TestAsset\InvokableObject; +use ZendTest\ServiceManager\TestAsset\ObjectWithObjectScalarDependency; use ZendTest\ServiceManager\TestAsset\ObjectWithScalarDependency; use ZendTest\ServiceManager\TestAsset\SecondComplexDependencyObject; use ZendTest\ServiceManager\TestAsset\SimpleDependencyObject; @@ -101,6 +103,76 @@ public function testCreateDependencyConfigWithoutTypeHintedParameterExcepts() ); } + public function testCreateDependencyConfigWithContainerAndNoServiceWithoutTypeHintedParameterExcepts() + { + self::expectException(InvalidArgumentException::class); + self::expectExceptionMessage( + 'Cannot create config for constructor argument "aName", ' + . 'it has no type hint, or non-class/interface type hint' + ); + $container = $this->prophesize(ContainerInterface::class); + $container->has(ObjectWithScalarDependency::class) + ->shouldBeCalled() + ->willReturn(false); + $this->dumper->setContainer($container->reveal()); + $config = $this->dumper->createDependencyConfig( + [ConfigAbstractFactory::class => []], + ObjectWithScalarDependency::class + ); + } + + public function testCreateDependencyConfigWithContainerWithoutTypeHintedParameter() + { + $container = $this->prophesize(ContainerInterface::class); + $container->has(ObjectWithScalarDependency::class) + ->shouldBeCalled() + ->willReturn(true); + $this->dumper->setContainer($container->reveal()); + $config = $this->dumper->createDependencyConfig( + [ConfigAbstractFactory::class => []], + ObjectWithObjectScalarDependency::class + ); + self::assertEquals( + [ + ConfigAbstractFactory::class => [ + SimpleDependencyObject::class => [ + InvokableObject::class, + ], + InvokableObject::class => [], + ObjectWithObjectScalarDependency::class => [ + SimpleDependencyObject::class, + ObjectWithScalarDependency::class, + ], + ] + ], + $config + ); + } + + public function testCreateDependencyConfigWithoutTypeHintedParameterIgnoringUnresolved() + { + $config = $this->dumper->createDependencyConfig( + [ConfigAbstractFactory::class => []], + ObjectWithObjectScalarDependency::class, + true + ); + self::assertEquals( + [ + ConfigAbstractFactory::class => [ + SimpleDependencyObject::class => [ + InvokableObject::class, + ], + InvokableObject::class => [], + ObjectWithObjectScalarDependency::class => [ + SimpleDependencyObject::class, + ObjectWithScalarDependency::class, + ], + ] + ], + $config + ); + } + public function testCreateDependencyConfigWorksWithExistingConfig() { $config = [ From c9df6a3d076d08d25931a8811fba65a87608a80c Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 15 Feb 2017 09:14:17 -0600 Subject: [PATCH 2/5] Incorporate feedback from review - Use private visibility for `$container` property - Remove `setContainer()` method; updated tests to create a new `ConfigDumper` instance using the mock container, instead of using setter injection. - Updated copyrights on licence docblocks of changed files. --- src/Tool/ConfigDumper.php | 12 ++---------- src/Tool/ConfigDumperCommand.php | 2 +- .../ObjectWithObjectScalarDependency.php | 2 +- test/Tool/ConfigDumperCommandTest.php | 6 +++--- test/Tool/ConfigDumperTest.php | 15 ++++++++++----- 5 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/Tool/ConfigDumper.php b/src/Tool/ConfigDumper.php index 9b59e2fe..1a881820 100644 --- a/src/Tool/ConfigDumper.php +++ b/src/Tool/ConfigDumper.php @@ -1,7 +1,7 @@ container = $container; } - /** - * @param ContainerInterface $container - */ - public function setContainer(ContainerInterface $container) - { - $this->container = $container; - } - /** * @param array $config * @param string $className diff --git a/src/Tool/ConfigDumperCommand.php b/src/Tool/ConfigDumperCommand.php index 9c3e65e8..5d6830fe 100644 --- a/src/Tool/ConfigDumperCommand.php +++ b/src/Tool/ConfigDumperCommand.php @@ -1,7 +1,7 @@ ['-i'], - 'long' => ['--ignore-unresolved'], + 'short' => ['-i'], + 'long' => ['--ignore-unresolved'], ]; } diff --git a/test/Tool/ConfigDumperTest.php b/test/Tool/ConfigDumperTest.php index fb48329b..817f31e2 100644 --- a/test/Tool/ConfigDumperTest.php +++ b/test/Tool/ConfigDumperTest.php @@ -1,7 +1,7 @@ has(ObjectWithScalarDependency::class) ->shouldBeCalled() ->willReturn(false); - $this->dumper->setContainer($container->reveal()); - $config = $this->dumper->createDependencyConfig( + + $dumper = new ConfigDumper($container->reveal()); + + $config = $dumper->createDependencyConfig( [ConfigAbstractFactory::class => []], ObjectWithScalarDependency::class ); @@ -127,11 +129,14 @@ public function testCreateDependencyConfigWithContainerWithoutTypeHintedParamete $container->has(ObjectWithScalarDependency::class) ->shouldBeCalled() ->willReturn(true); - $this->dumper->setContainer($container->reveal()); - $config = $this->dumper->createDependencyConfig( + + $dumper = new ConfigDumper($container->reveal()); + + $config = $dumper->createDependencyConfig( [ConfigAbstractFactory::class => []], ObjectWithObjectScalarDependency::class ); + self::assertEquals( [ ConfigAbstractFactory::class => [ From 975d2040178c1bf70af8c9600ba0711a5a859241 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 15 Feb 2017 09:16:33 -0600 Subject: [PATCH 3/5] Added CHANGELOG for #176 --- CHANGELOG.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e03f919..45ace95a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,11 @@ All notable changes to this project will be documented in this file, in reverse ### Added -- Nothing. +- [#176](https://github.com/zendframework/zend-servicemanager/pull/176) adds + the options `-i` or `--ignore-unresolved` to the shipped + `generate-deps-for-config-factory` command. This flag allows it to build + configuration for classes resolved by the `ConfigAbstractFactory` that + typehint on interfaces, which was previously unsupported. ### Deprecated From c5b3d9cacfcc9a6c65e9142f98aca0e8f9d5efa4 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 15 Feb 2017 09:19:00 -0600 Subject: [PATCH 4/5] Added `-i` switch to usage string for generate-deps-for-config-factory Re-aligned all items in the Arguments section as well, due to `--ignore-unresolved` being a longer string. --- src/Tool/ConfigDumperCommand.php | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Tool/ConfigDumperCommand.php b/src/Tool/ConfigDumperCommand.php index 5d6830fe..29d13b5f 100644 --- a/src/Tool/ConfigDumperCommand.php +++ b/src/Tool/ConfigDumperCommand.php @@ -21,19 +21,19 @@ class ConfigDumperCommand const HELP_TEMPLATE = <<< EOH Usage: - %s [-h|--help|help] + %s [-h|--help|help] [-i|--ignore-unresolved] Arguments: - -h|--help|help This usage message - -i|--ignore-unresolved Ignore classes with unresolved - direct dependencies. - Path to a config file for which to generate configuration. - If the file does not exist, it will be created. If it does - exist, it must return an array, and the file will be - updated with new configuration. - Name of the class to reflect and for which to generate - dependency configuration. + -h|--help|help This usage message + -i|--ignore-unresolved Ignore classes with unresolved direct dependencies. + Path to a config file for which to generate + configuration. If the file does not exist, it will + be created. If it does exist, it must return an + array, and the file will be updated with new + configuration. + Name of the class to reflect and for which to + generate dependency configuration. Reads the provided configuration file (creating it if it does not exist), and injects it with ConfigAbstractFactory dependency configuration for From 309dc00adad4d7e55c79cd452ad5ca80bb4aa21a Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 15 Feb 2017 09:25:12 -0600 Subject: [PATCH 5/5] Documented `-i`/`--ignore-unresolved` flag. --- doc/book/console-tools.md | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/doc/book/console-tools.md b/doc/book/console-tools.md index 914d01bb..549d1545 100644 --- a/doc/book/console-tools.md +++ b/doc/book/console-tools.md @@ -9,17 +9,19 @@ document details each. $ ./vendor/bin/generate-deps-for-config-factory Usage: - generate-deps-for-config-factory [-h|--help|help] + generate-deps-for-config-factory [-h|--help|help] [-i|--ignore-unresolved] Arguments: - -h|--help|help This usage message - Path to a config file for which to generate configuration. - If the file does not exist, it will be created. If it does - exist, it must return an array, and the file will be - updated with new configuration. - Name of the class to reflect and for which to generate - dependency configuration. + -h|--help|help This usage message + -i|--ignore-unresolved Ignore classes with unresolved direct dependencies. + Path to a config file for which to generate + configuration. If the file does not exist, it will + be created. If it does exist, it must return an + array, and the file will be updated with new + configuration. + Name of the class to reflect and for which to + generate dependency configuration. Reads the provided configuration file (creating it if it does not exist), @@ -33,6 +35,13 @@ will read the named configuration file (creating it if it does not exist), and merge any configuration it generates with the return values of that file, writing the changes back to the original file. +Since 3.2.1, the tool also supports the `-i` or `--ignore-unresolved` flag. +Use these flags when you have typehints to classes that cannot be resolved. +When you omit the flag, such classes will cause the tool to fail with an +exception message. By adding the flag, you can have it continue and produce +configuration. This option is particularly useful when typehints are on +interfaces or resolve to services served by other abstract factories. + ## generate-factory-for-class ```bash