From 016ea1e14c91d0c6fcf83cabb67d36d52adb39f8 Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Tue, 18 Aug 2015 17:41:40 +0100 Subject: [PATCH 1/3] Extract a CallableResolver from the Invoker class --- src/CallableResolver.php | 121 +++++++++++++++++++++++++++++++++ src/Invoker.php | 109 +++++------------------------ tests/CallableResolverTest.php | 76 +++++++++++++++++++++ 3 files changed, 215 insertions(+), 91 deletions(-) create mode 100644 src/CallableResolver.php create mode 100644 tests/CallableResolverTest.php diff --git a/src/CallableResolver.php b/src/CallableResolver.php new file mode 100644 index 0000000..abee5dc --- /dev/null +++ b/src/CallableResolver.php @@ -0,0 +1,121 @@ + + */ +class CallableResolver +{ + /** + * @var ContainerInterface + */ + private $container; + + public function __construct(ContainerInterface $container) + { + $this->container = $container; + } + + /** + * Resolve the given callable into a real PHP callable. + * + * @param callable|string|array $callable + * + * @return callable Real PHP callable. + * + * @throws NotCallableException + */ + public function resolve($callable) + { + $callable = $this->resolveFromContainer($callable); + + if (! is_callable($callable)) { + throw new NotCallableException(sprintf( + '%s is not a callable', + is_object($callable) ? 'Instance of ' . get_class($callable) : var_export($callable, true) + )); + } + + return $callable; + } + + /** + * @param callable|string|array $callable + * @return callable + * @throws NotCallableException + */ + private function resolveFromContainer($callable) + { + $isStaticCallToNonStaticMethod = false; + + // If it's already a callable there is nothing to do + if (is_callable($callable)) { + $isStaticCallToNonStaticMethod = $this->isStaticCallToNonStaticMethod($callable); + if (! $isStaticCallToNonStaticMethod) { + return $callable; + } + } + + // The callable is a container entry name + if (is_string($callable)) { + if ($this->container->has($callable)) { + return $this->container->get($callable); + } else { + throw new NotCallableException(sprintf( + '%s is neither a callable or a valid container entry', + $callable + )); + } + } + + // The callable is an array whose first item is a container entry name + // e.g. ['some-container-entry', 'methodToCall'] + if (is_array($callable) && is_string($callable[0])) { + if ($this->container->has($callable[0])) { + $callable[0] = $this->container->get($callable[0]); + return $callable; + } elseif ($isStaticCallToNonStaticMethod) { + throw new NotCallableException(sprintf( + 'Cannot call %s::%s() because %s() is not a static method and "%s" is not a container entry', + $callable[0], + $callable[1], + $callable[1], + $callable[0] + )); + } else { + throw new NotCallableException(sprintf( + 'Cannot call %s on %s because it is not a class nor a valid container entry', + $callable[1], + $callable[0] + )); + } + } + + // Unrecognized stuff, we let it fail later + return $callable; + } + + /** + * Check if the callable represents a static call to a non-static method. + * + * @param mixed $callable + * @return bool + */ + private function isStaticCallToNonStaticMethod($callable) + { + if (is_array($callable) && is_string($callable[0])) { + list($class, $method) = $callable; + $reflection = new \ReflectionMethod($class, $method); + + return ! $reflection->isStatic(); + } + + return false; + } +} diff --git a/src/Invoker.php b/src/Invoker.php index 83da882..aa739b7 100644 --- a/src/Invoker.php +++ b/src/Invoker.php @@ -30,10 +30,19 @@ class Invoker implements InvokerInterface */ private $container; + /** + * @var CallableResolver|null + */ + private $callableResolver; + public function __construct(ParameterResolver $parameterResolver = null, ContainerInterface $container = null) { $this->parameterResolver = $parameterResolver ?: $this->createParameterResolver(); $this->container = $container; + + if ($container) { + $this->callableResolver = new CallableResolver($container); + } } /** @@ -41,10 +50,16 @@ public function __construct(ParameterResolver $parameterResolver = null, Contain */ public function call($callable, array $parameters = array()) { - if ($this->container) { - $callable = $this->resolveCallableFromContainer($callable); + if ($this->callableResolver) { + $callable = $this->callableResolver->resolve($callable); + } + + if (! is_callable($callable)) { + throw new NotCallableException(sprintf( + '%s is not a callable', + is_object($callable) ? 'Instance of ' . get_class($callable) : var_export($callable, true) + )); } - $this->assertIsCallable($callable); $callableReflection = CallableReflection::create($callable); @@ -104,94 +119,6 @@ public function getContainer() return $this->container; } - /** - * @param callable|string|array $callable - * @return callable - * @throws NotCallableException - */ - private function resolveCallableFromContainer($callable) - { - $isStaticCallToNonStaticMethod = false; - - // If it's already a callable there is nothing to do - if (is_callable($callable)) { - $isStaticCallToNonStaticMethod = $this->isStaticCallToNonStaticMethod($callable); - if (! $isStaticCallToNonStaticMethod) { - return $callable; - } - } - - // The callable is a container entry name - if (is_string($callable)) { - if ($this->container->has($callable)) { - return $this->container->get($callable); - } else { - throw new NotCallableException(sprintf( - '%s is neither a callable or a valid container entry', - $callable - )); - } - } - - // The callable is an array whose first item is a container entry name - // e.g. ['some-container-entry', 'methodToCall'] - if (is_array($callable) && is_string($callable[0])) { - if ($this->container->has($callable[0])) { - $callable[0] = $this->container->get($callable[0]); - return $callable; - } elseif ($isStaticCallToNonStaticMethod) { - throw new NotCallableException(sprintf( - 'Cannot call %s::%s() because %s() is not a static method and "%s" is not a container entry', - $callable[0], - $callable[1], - $callable[1], - $callable[0] - )); - } else { - throw new NotCallableException(sprintf( - 'Cannot call %s on %s because it is not a class nor a valid container entry', - $callable[1], - $callable[0] - )); - } - } - - // Unrecognized stuff, we let it fail later - return $callable; - } - - /** - * @param callable $callable - * @throws NotCallableException - */ - private function assertIsCallable($callable) - { - if (! is_callable($callable)) { - throw new NotCallableException(sprintf( - '%s is not a callable', - is_object($callable) ? 'Instance of ' . get_class($callable) : var_export($callable, true) - )); - } - } - - /** - * Check if the callable represents a static call to a non-static method. - * - * @param mixed $callable - * @return bool - */ - private function isStaticCallToNonStaticMethod($callable) - { - if (is_array($callable) && is_string($callable[0])) { - list($class, $method) = $callable; - $reflection = new \ReflectionMethod($class, $method); - - return ! $reflection->isStatic(); - } - - return false; - } - private function assertMandatoryParametersAreResolved($parameters, ReflectionFunctionAbstract $reflection) { $parameterCount = $reflection->getNumberOfRequiredParameters(); diff --git a/tests/CallableResolverTest.php b/tests/CallableResolverTest.php new file mode 100644 index 0000000..267cfbb --- /dev/null +++ b/tests/CallableResolverTest.php @@ -0,0 +1,76 @@ +container = new ArrayContainer; + $this->resolver = new CallableResolver($this->container); + } + + /** + * @test + */ + public function resolves_callable_from_container() + { + $callable = new CallableSpy; + $this->container->set('thing-to-call', $callable); + + $this->assertSame($callable, $this->resolver->resolve('thing-to-call')); + } + + /** + * @test + */ + public function resolves_array_callable_from_container() + { + $fixture = new InvokerTestFixture; + $this->container->set('thing-to-call', $fixture); + + $result = $this->resolver->resolve(array('thing-to-call', 'foo')); + + $result(); + $this->assertTrue($fixture->wasCalled); + } + + /** + * @test + */ + public function resolve_array_callable_from_container_with_class_name() + { + $fixture = new InvokerTestFixture; + $this->container->set('Invoker\Test\InvokerTestFixture', $fixture); + + $result = $this->resolver->resolve(array('Invoker\Test\InvokerTestFixture', 'foo')); + + $result(); + $this->assertTrue($fixture->wasCalled); + } + + /** + * @test + * @expectedException \Invoker\Exception\NotCallableException + * @expectedExceptionMessage foo is neither a callable or a valid container entry + */ + public function throws_resolving_non_callable_from_container() + { + $resolver = new CallableResolver(new ArrayContainer); + $resolver->resolve('foo'); + } +} From 72f5f5df354f823b24b26b60db18c96c1faafb69 Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Wed, 19 Aug 2015 07:06:48 +0100 Subject: [PATCH 2/3] Let users get the callable resolver --- src/Invoker.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Invoker.php b/src/Invoker.php index aa739b7..1a49cff 100644 --- a/src/Invoker.php +++ b/src/Invoker.php @@ -119,6 +119,14 @@ public function getContainer() return $this->container; } + /** + * @return CallableResolver|null Returns null if no container was given in the constructor. + */ + public function getCallableResolver() + { + return $this->callableResolver; + } + private function assertMandatoryParametersAreResolved($parameters, ReflectionFunctionAbstract $reflection) { $parameterCount = $reflection->getNumberOfRequiredParameters(); From 86a6236aaeb837c80aacc206fcb9561799688e9a Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Wed, 19 Aug 2015 07:21:17 +0100 Subject: [PATCH 3/3] Add more tests for CallableResolver --- tests/CallableResolverTest.php | 42 +++++++++++++++++++++++++++++++--- tests/InvokerTest.php | 1 + 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/tests/CallableResolverTest.php b/tests/CallableResolverTest.php index 267cfbb..de98406 100644 --- a/tests/CallableResolverTest.php +++ b/tests/CallableResolverTest.php @@ -24,12 +24,32 @@ public function setUp() $this->resolver = new CallableResolver($this->container); } + /** + * @test + */ + public function resolves_function_callable() + { + $result = $this->resolver->resolve('strlen'); + + $this->assertSame(strlen('Hello world!'), $result('Hello world!')); + } + + /** + * @test + */ + public function resolves_namespaced_function_callable() + { + $result = $this->resolver->resolve(__NAMESPACE__ . '\foo'); + + $this->assertEquals('bar', $result()); + } + /** * @test */ public function resolves_callable_from_container() { - $callable = new CallableSpy; + $callable = function () {}; $this->container->set('thing-to-call', $callable); $this->assertSame($callable, $this->resolver->resolve('thing-to-call')); @@ -38,7 +58,18 @@ public function resolves_callable_from_container() /** * @test */ - public function resolves_array_callable_from_container() + public function resolves_invokable_class_from_container() + { + $callable = new CallableSpy; + $this->container->set('Invoker\Test\Mock\CallableSpy', $callable); + + $this->assertSame($callable, $this->resolver->resolve('Invoker\Test\Mock\CallableSpy')); + } + + /** + * @test + */ + public function resolves_method_call_service_from_container() { $fixture = new InvokerTestFixture; $this->container->set('thing-to-call', $fixture); @@ -52,7 +83,7 @@ public function resolves_array_callable_from_container() /** * @test */ - public function resolve_array_callable_from_container_with_class_name() + public function resolves_method_call_class_from_container() { $fixture = new InvokerTestFixture; $this->container->set('Invoker\Test\InvokerTestFixture', $fixture); @@ -74,3 +105,8 @@ public function throws_resolving_non_callable_from_container() $resolver->resolve('foo'); } } + +function foo() +{ + return 'bar'; +} diff --git a/tests/InvokerTest.php b/tests/InvokerTest.php index f4e26bf..3c44c9c 100644 --- a/tests/InvokerTest.php +++ b/tests/InvokerTest.php @@ -282,6 +282,7 @@ class InvokerTestFixture public $wasCalled = false; public function foo() { + // Use this to make sure we are not called from a static context $this->wasCalled = true; return 'bar'; }