From 51822b128b882717ed6efed174126994b9adbafc Mon Sep 17 00:00:00 2001 From: Max Date: Fri, 19 May 2017 16:48:00 +0200 Subject: [PATCH] [Bugfix/Enhancement] Ability to provide named parameters for formatter configuration (#22) * Added failing Unit Test for named arguments in Formatter configuration * Added ability to pass named parameters within formatter configuration * Added PHP 7.1 to `.travis.yml` --- .travis.yml | 6 +- .../Service/MonologServiceFactory.php | 190 +++++++++++------- .../Service/MonologServiceFactoryTest.php | 21 ++ 3 files changed, 148 insertions(+), 69 deletions(-) diff --git a/.travis.yml b/.travis.yml index 8b808a4..d8cc781 100644 --- a/.travis.yml +++ b/.travis.yml @@ -23,6 +23,10 @@ matrix: env: DEPS=lowest - php: 7.0 env: DEPS=latest + - php: 7.1 + env: DEPS=lowest + - php: 7.1 + env: DEPS=latest install: - if [[ $DEPS == 'latest' ]]; then travis_retry composer update --no-interaction --prefer-source ; fi @@ -32,4 +36,4 @@ script: - vendor/bin/phpunit --coverage-text notifications: - email: false \ No newline at end of file + email: false diff --git a/src/EnliteMonolog/Service/MonologServiceFactory.php b/src/EnliteMonolog/Service/MonologServiceFactory.php index 77c0e39..c73c221 100644 --- a/src/EnliteMonolog/Service/MonologServiceFactory.php +++ b/src/EnliteMonolog/Service/MonologServiceFactory.php @@ -9,7 +9,6 @@ use Closure; use Exception; use Interop\Container\ContainerInterface; -use Monolog\Formatter\LineFormatter; use Monolog\Handler\HandlerInterface; use Monolog\Logger; use Monolog\Formatter\FormatterInterface; @@ -83,68 +82,50 @@ public function createHandler($container, MonologOptions $options, $handler) { if (is_string($handler) && $container->has($handler)) { return $container->get($handler); - } else { - if (!isset($handler['name'])) { - throw new RuntimeException('Cannot create logger handler'); - } - - if (!class_exists($handler['name'])) { - throw new RuntimeException('Cannot create logger handler (' . $handler['name'] . ')'); - } + } - if (isset($handler['args'])) { - if (!is_array($handler['args'])) { - throw new RuntimeException('Arguments of handler(' . $handler['name'] . ') must be array'); - } - $reflection = new \ReflectionClass($handler['name']); + if (!isset($handler['name'])) { + throw new RuntimeException('Cannot create logger handler'); + } - if (isset($handler['args']['handler'])) { - foreach ($options->getHandlers() as $key => $option) { - if ($handler['args']['handler'] == $key) { - $handler['args']['handler'] = $this->createHandler($container, $options, $option); - break; - } - } - } + $handlerClassName = $handler['name']; - $parameters = array(); - $handlerOptions = $handler['args']; + if (!class_exists($handlerClassName)) { + throw new RuntimeException('Cannot create logger handler (' . $handlerClassName . ')'); + } - $requiredArgsCount = $reflection->getConstructor()->getNumberOfRequiredParameters(); + $arguments = array_key_exists('args', $handler) ? $handler['args'] : array(); - if ($requiredArgsCount > count($handlerOptions)) { - throw new RuntimeException(sprintf('Handler(%s) requires at least %d params. Only %d passed.', $handler['name'], $requiredArgsCount, count($handlerOptions))); - } + if (!is_array($arguments)) { + throw new RuntimeException('Arguments of handler(' . $handlerClassName . ') must be array'); + } - foreach($reflection->getConstructor()->getParameters() as $parameter) { - if (!$parameter->isOptional() && !isset($handlerOptions[$parameter->getName()])) { - $argumentValue = array_shift($handlerOptions); - } elseif (isset($handlerOptions[$parameter->getName()])) { - $argumentValue = $handlerOptions[$parameter->getName()]; - unset($handlerOptions[$parameter->getName()]); - } else { - $argumentValue = $parameter->getDefaultValue(); - } - $parameters[$parameter->getPosition()] = $argumentValue; + if (isset($arguments['handler'])) { + foreach ($options->getHandlers() as $key => $option) { + if ($arguments['handler'] == $key) { + $arguments['handler'] = $this->createHandler($container, $options, $option); + break; } - - /** @var HandlerInterface $instance */ - $instance = $reflection->newInstanceArgs($parameters); - } else { - $class = $handler['name']; - - /** @var HandlerInterface $instance */ - $instance = new $class(); } + } - if (isset($handler['formatter'])) { - $formatter = $this->createFormatter($container, $handler['formatter']); - $instance->setFormatter($formatter); - } + try { + /** @var HandlerInterface $instance */ + $instance = $this->createInstanceFromArguments($handlerClassName, $arguments); + } catch (\InvalidArgumentException $exception) { + throw new RuntimeException(sprintf( + 'Handler(%s) has an invalid argument configuration', + $handlerClassName + ), 0, $exception); + } - return $instance; + if (isset($handler['formatter'])) { + $formatter = $this->createFormatter($container, $handler['formatter']); + $instance->setFormatter($formatter); } + + return $instance; } /** @@ -159,29 +140,35 @@ public function createFormatter($container, $formatter) { if (is_string($formatter) && $container->has($formatter)) { return $container->get($formatter); - } else { - if (!isset($formatter['name'])) { - throw new RuntimeException('Cannot create logger formatter'); - } + } + + if (!isset($formatter['name'])) { + throw new RuntimeException('Cannot create logger formatter'); + } - if (!class_exists($formatter['name'])) { - throw new RuntimeException('Cannot create logger formatter (' . $formatter['name'] . ')'); - } + $formatterClassName = $formatter['name']; - if (isset($formatter['args'])) { - if (!is_array($formatter['args'])) { - throw new RuntimeException('Arguments of formatter(' . $formatter['name'] . ') must be array'); - } + if (!class_exists($formatter['name'])) { + throw new RuntimeException('Cannot create logger formatter (' . $formatterClassName . ')'); + } - $reflection = new \ReflectionClass($formatter['name']); + $arguments = array_key_exists('args', $formatter) ? $formatter['args'] : array(); - return call_user_func_array(array($reflection, 'newInstance'), $formatter['args']); - } + if (!is_array($arguments)) { + throw new RuntimeException('Arguments of formatter(' . $formatterClassName . ') must be array'); + } - $class = $formatter['name']; + try { + /** @var FormatterInterface $instance */ + $instance = $this->createInstanceFromArguments($formatterClassName, $arguments); + } catch (\InvalidArgumentException $exception) { + throw new RuntimeException(sprintf( + 'Formatter(%s) has an invalid argument configuration', + $formatterClassName + ), 0, $exception); + } - return new $class(); - } + return $instance; } /** @@ -218,4 +205,71 @@ public function createProcessor($container, $processor) throw new RuntimeException('Unknown processor type, must be a Closure or the FQCN of an invokable class'); } + + /** + * Handles the constructor arguments and if they're named, just sort them to fit constructor ordering. + * + * @param string $className + * @param array $arguments + * + * @return object + * @throws \InvalidArgumentException If given arguments are not valid for provided className constructor. + */ + private function createInstanceFromArguments($className, array $arguments) + { + $reflection = new \ReflectionClass($className); + $constructor = $reflection->getConstructor(); + + // There is no or at least a non-accessible constructor for provided class name, + // therefore there is no need to handle arguments anyway + if ($constructor === null) { + return $reflection->newInstanceArgs($arguments); + } + + if (!$constructor->isPublic()) { + throw new \InvalidArgumentException(sprintf( + '%s::__construct is not accessible', + $className + )); + } + + $requiredArgsCount = $constructor->getNumberOfRequiredParameters(); + $argumentCount = count($arguments); + + if ($requiredArgsCount > $argumentCount) { + throw new \InvalidArgumentException(sprintf( + '%s::__construct() requires at least %d arguments; %d given', + $className, + $requiredArgsCount, + $argumentCount + )); + } + + // Arguments supposed to be ordered + if (isset($arguments[0])) { + return $reflection->newInstanceArgs($arguments); + } + + $parameters = array(); + + foreach($constructor->getParameters() as $parameter) { + $parameterName = $parameter->getName(); + + if (array_key_exists($parameterName, $arguments)) { + $parameters[$parameter->getPosition()] = $arguments[$parameterName]; + continue; + } + + if (!$parameter->isOptional()) { + throw new \InvalidArgumentException(sprintf( + 'Missing at least one required parameters `%s`', + $parameterName + )); + } + + $parameters[$parameter->getPosition()] = $parameter->getDefaultValue(); + } + + return $reflection->newInstanceArgs($parameters); + } } diff --git a/test/EnliteMonologTest/Service/MonologServiceFactoryTest.php b/test/EnliteMonologTest/Service/MonologServiceFactoryTest.php index f5527a5..91e2139 100644 --- a/test/EnliteMonologTest/Service/MonologServiceFactoryTest.php +++ b/test/EnliteMonologTest/Service/MonologServiceFactoryTest.php @@ -240,6 +240,27 @@ public function testCreateFormatterWithoutArguments() self::assertInstanceOf('\Monolog\Formatter\LineFormatter', $actual); } + public function testCreateFormatterWithNamedArguments() + { + $serviceManager = new ServiceManager(); + $factory = new MonologServiceFactory(); + + $dateFormat = 'Y-m-d\TH:i:sZ'; + $actual = $factory->createFormatter($serviceManager, array( + 'name' => '\Monolog\Formatter\LineFormatter', + 'args' => array( + 'dateFormat' => $dateFormat, + ), + )); + + self::assertInstanceOf('\Monolog\Formatter\LineFormatter', $actual); + $reflection = new \ReflectionClass($actual); + $property = $reflection->getProperty('dateFormat'); + $property->setAccessible(true); + self::assertSame($dateFormat, $property->getValue($actual), 'Unable to set arguments by name'); + } + + public function testCreateLoggerWithProcessor() { $options = new MonologOptions();