From 238d8154321cf2161ee433ac2d7f5756b60e301d Mon Sep 17 00:00:00 2001 From: Augusto Pascutti Date: Thu, 7 Feb 2013 22:48:45 -0200 Subject: [PATCH 1/3] More unit tests for *By* routine. --- .../Respect/Rest/Routines/AbstractRoutine.php | 5 ++ .../Rest/Routines/AbstractSyncedRoutine.php | 7 ++- .../Routines/AbstractSyncedRoutineTest.php | 56 ++++++++++++++++++- tests/legacy/Respect/Rest/Routines/ByTest.php | 34 +++++++---- 4 files changed, 84 insertions(+), 18 deletions(-) diff --git a/library/Respect/Rest/Routines/AbstractRoutine.php b/library/Respect/Rest/Routines/AbstractRoutine.php index aca2b54..f5b784c 100644 --- a/library/Respect/Rest/Routines/AbstractRoutine.php +++ b/library/Respect/Rest/Routines/AbstractRoutine.php @@ -20,4 +20,9 @@ public function __construct($callback) $this->callback = $callback; } + protected function getCallback() + { + return $this->callback; + } + } diff --git a/library/Respect/Rest/Routines/AbstractSyncedRoutine.php b/library/Respect/Rest/Routines/AbstractSyncedRoutine.php index 5bd9635..5e09b48 100644 --- a/library/Respect/Rest/Routines/AbstractSyncedRoutine.php +++ b/library/Respect/Rest/Routines/AbstractSyncedRoutine.php @@ -21,10 +21,11 @@ public function getParameters() /** Returns a concrete ReflectionFunctionAbstract for this routine callback */ protected function getReflection() { - if (is_array($this->callback)) - return new ReflectionMethod($this->callback[0], $this->callback[1]); + $callback = $this->getCallback(); + if (is_array($callback)) + return new ReflectionMethod($callback[0], $callback[1]); else - return new ReflectionFunction($this->callback); + return new ReflectionFunction($callback); } } diff --git a/tests/legacy/Respect/Rest/Routines/AbstractSyncedRoutineTest.php b/tests/legacy/Respect/Rest/Routines/AbstractSyncedRoutineTest.php index 2324ceb..f4106f0 100644 --- a/tests/legacy/Respect/Rest/Routines/AbstractSyncedRoutineTest.php +++ b/tests/legacy/Respect/Rest/Routines/AbstractSyncedRoutineTest.php @@ -5,8 +5,6 @@ * @covers Respect\Rest\Routines\ParamSynced * @author Nick Lombard */ -use \ReflectionParameterr; - class AbstractSyncedRoutineTest extends \PHPUnit_Framework_TestCase { /** @@ -37,7 +35,7 @@ protected function tearDown() } /** - * @covers Respect\Rest\Routines\AbstractSyncedRoutine::getParameters + * @covers Respect\Rest\Routines\AbstractSyncedRoutine */ public function testGetParameters() { @@ -48,4 +46,56 @@ public function testGetParameters() $this->assertEquals('blogId', $parameters[1]->name); $this->assertInstanceOf('ReflectionParameter', $parameters[0]); } + + /** + * @covers Respect\Rest\Routines\AbstractSyncedRoutine + * @covers Respect\Rest\Routines\AbstractRoutine + */ + public function test_getParameters_with_an_array() + { + $class = 'Respect\Rest\Routines\AbstractSyncedRoutine'; + $callback = array('DateTime', 'createFromFormat'); + $stub = $this->getMockBuilder($class) + ->setMethods(array('getCallback')) + ->disableOriginalConstructor() + ->getMock(); + $stub->expects($this->once()) + ->method('getCallback') + ->will($this->returnValue($callback)); + + $this->assertContainsOnlyInstancesOf( + $expected = 'ReflectionParameter', + $result = $stub->getParameters() + ); + $this->assertCount( + $expected = 3, + $result + ); + } + + /** + * @covers Respect\Rest\Routines\AbstractSyncedRoutine + * @covers Respect\Rest\Routines\AbstractRoutine + */ + public function test_getParameters_with_function() + { + $class = 'Respect\Rest\Routines\AbstractSyncedRoutine'; + $callback = function($name) { return 'Hello '.$name; }; + $stub = $this->getMockBuilder($class) + ->setMethods(array('getCallback')) + ->disableOriginalConstructor() + ->getMock(); + $stub->expects($this->once()) + ->method('getCallback') + ->will($this->returnValue($callback)); + + $this->assertContainsOnlyInstancesOf( + $expected = 'ReflectionParameter', + $result = $stub->getParameters() + ); + $this->assertCount( + $expected = 1, + $result + ); + } } diff --git a/tests/legacy/Respect/Rest/Routines/ByTest.php b/tests/legacy/Respect/Rest/Routines/ByTest.php index db5c8c3..34ae1e9 100644 --- a/tests/legacy/Respect/Rest/Routines/ByTest.php +++ b/tests/legacy/Respect/Rest/Routines/ByTest.php @@ -1,7 +1,8 @@ assertEquals('from by callback', $routine->by($request, $params)); + } + + /** + * @covers Respect\Rest\Routines\By + */ + public function test_by_on_a_route() { - $request = @new Request(); - $params = array(); - $alias = &$this->object; - $this->assertEquals('from by callback', - $alias->by($request, $params)); + $router = new Router(); + $router->get('/', function() { return 'route'; }) + ->by(function() { return 'by'; }); + // By does not affect the output of the route. + $this->assertEquals( + $expected = 'route', + (string) $router->dispatch('GET', '/') + ); } } From acb94373554363b2e52c626b4de0f8026cbc6050 Mon Sep 17 00:00:00 2001 From: Augusto Pascutti Date: Fri, 8 Feb 2013 00:16:48 -0200 Subject: [PATCH 2/3] Make routines accept instances that are callable. In other words, instances of classes that implement the __invoke() method. To accept class names instead of only instances we need to change the validation of callable objects in Routine\AbstractRoutine, something that would be nice to have. :) Also created a *src* directory inside tests to put classes related to tests only (stubs). Let me know if this bothers you a lot, I still think that this is better than declaring multiple namespaces on files. --- README.md | 5 +++ .../Rest/Routines/AbstractSyncedRoutine.php | 16 ++++++++-- tests/bootstrap.php | 1 + .../Routines/AbstractSyncedRoutineTest.php | 32 +++++++++++++++++-- tests/legacy/Respect/Rest/Routines/ByTest.php | 20 ++++++++++++ .../legacy/Respect/Rest/Routines/WhenTest.php | 25 ++++++++++++++- .../src/Stubs/Routines/ByClassWithInvoke.php | 14 ++++++++ tests/src/Stubs/Routines/WhenAlwaysTrue.php | 14 ++++++++ 8 files changed, 122 insertions(+), 5 deletions(-) create mode 100644 tests/src/Stubs/Routines/ByClassWithInvoke.php create mode 100644 tests/src/Stubs/Routines/WhenAlwaysTrue.php diff --git a/README.md b/README.md index a51937d..3d2637b 100644 --- a/README.md +++ b/README.md @@ -291,6 +291,11 @@ Respect\Rest uses a different approach to validate route parameters: })->when(function($documentId) { return is_numeric($documentId) && $documentId > 0; }); + // Routines can also be called using class and method names. + $r3->get('/documents/*', function($documentId) { + /** do something */ + })->when('SomeClass_name', 'someMethod_name'); + // You can also pass any instance that implements the __invoke() magic method to any routine. ``` 1. This will match the route only if the callback on *when* is matched. diff --git a/library/Respect/Rest/Routines/AbstractSyncedRoutine.php b/library/Respect/Rest/Routines/AbstractSyncedRoutine.php index 5e09b48..f23078d 100644 --- a/library/Respect/Rest/Routines/AbstractSyncedRoutine.php +++ b/library/Respect/Rest/Routines/AbstractSyncedRoutine.php @@ -5,6 +5,8 @@ use InvalidArgumentException; use ReflectionFunction; use ReflectionMethod; +use ReflectionObject; +use Closure; use Respect\Rest\Routes\AbstractRoute; /** Base class for routines that sync parameters */ @@ -15,7 +17,15 @@ abstract class AbstractSyncedRoutine extends AbstractRoutine implements ParamSyn public function getParameters() { - return $this->getReflection()->getParameters(); + $reflection = $this->getReflection(); + if (!$reflection instanceof ReflectionObject) + return $this->getReflection()->getParameters(); + + $constructorReflection = $reflection->getConstructor(); + if (is_null($constructorReflection)) + return array(); + else + return $constructorReflection->getParameters(); } /** Returns a concrete ReflectionFunctionAbstract for this routine callback */ @@ -24,8 +34,10 @@ protected function getReflection() $callback = $this->getCallback(); if (is_array($callback)) return new ReflectionMethod($callback[0], $callback[1]); - else + else if ($callback instanceof Closure) return new ReflectionFunction($callback); + else + return new ReflectionObject($callback); } } diff --git a/tests/bootstrap.php b/tests/bootstrap.php index b3276df..8c0df0d 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -12,6 +12,7 @@ $paths[] = $path; } +$paths[] = realpath(__DIR__.DIRECTORY_SEPARATOR.'src'); natsort($paths); array_unshift($paths, dirname(__DIR__) .'/library'); set_include_path(implode(PATH_SEPARATOR, array_unique($paths))); diff --git a/tests/legacy/Respect/Rest/Routines/AbstractSyncedRoutineTest.php b/tests/legacy/Respect/Rest/Routines/AbstractSyncedRoutineTest.php index f4106f0..9a85ec3 100644 --- a/tests/legacy/Respect/Rest/Routines/AbstractSyncedRoutineTest.php +++ b/tests/legacy/Respect/Rest/Routines/AbstractSyncedRoutineTest.php @@ -1,6 +1,8 @@ @@ -59,7 +61,7 @@ public function test_getParameters_with_an_array() ->setMethods(array('getCallback')) ->disableOriginalConstructor() ->getMock(); - $stub->expects($this->once()) + $stub->expects($this->any()) ->method('getCallback') ->will($this->returnValue($callback)); @@ -85,7 +87,7 @@ public function test_getParameters_with_function() ->setMethods(array('getCallback')) ->disableOriginalConstructor() ->getMock(); - $stub->expects($this->once()) + $stub->expects($this->any()) ->method('getCallback') ->will($this->returnValue($callback)); @@ -98,4 +100,30 @@ public function test_getParameters_with_function() $result ); } + + /** + * @covers Respect\Rest\Routines\AbstractSyncedRoutine + * @covers Respect\Rest\Routines\AbstractRoutine + */ + public function test_getParameters_with_callable_instance() + { + $stub = new ByClassWithInvoke; + $this->assertTrue( + is_callable($stub), + 'Callable instance does not pass the is_callable test.' + ); + $class = 'Respect\Rest\Routines\AbstractSyncedRoutine'; + $callback = function($name) { return 'Hello '.$name; }; + $routine = $this->getMockBuilder($class) + ->setMethods(array('getCallback')) + ->disableOriginalConstructor() + ->getMock(); + $routine->expects($this->any()) + ->method('getCallback') + ->will($this->returnValue($stub)); + $this->assertCount( + $expected = 0, + $result = $routine->getParameters() + ); + } } diff --git a/tests/legacy/Respect/Rest/Routines/ByTest.php b/tests/legacy/Respect/Rest/Routines/ByTest.php index 34ae1e9..6e4d2b5 100644 --- a/tests/legacy/Respect/Rest/Routines/ByTest.php +++ b/tests/legacy/Respect/Rest/Routines/ByTest.php @@ -3,6 +3,7 @@ use Respect\Rest\Request, Respect\Rest\Router; +use Stubs\Routines\ByClassWithInvoke; /** * @covers Respect\Rest\Routines\By @@ -55,4 +56,23 @@ public function test_by_on_a_route() (string) $router->dispatch('GET', '/') ); } + + public function test_by_with_a_callable_class_on_a_route() + { + $router = new Router; + $routine = new ByClassWithInvoke; + $router->get('/', function() { return 'route'; }) + ->by($routine); + // By does not affect the output of the route. + $this->assertEquals( + $expected = 'route', + (string) $router->dispatch('GET', '/') + ); + $this->assertAttributeEquals( + $expected = true, + 'invoked', + $routine, + 'Routine was not invoked!' + ); + } } diff --git a/tests/legacy/Respect/Rest/Routines/WhenTest.php b/tests/legacy/Respect/Rest/Routines/WhenTest.php index e8ab1d3..9e2d355 100644 --- a/tests/legacy/Respect/Rest/Routines/WhenTest.php +++ b/tests/legacy/Respect/Rest/Routines/WhenTest.php @@ -1,6 +1,10 @@ @@ -53,6 +57,25 @@ public function testWhen() $this->assertFalse($alias->when($request, $params)); $this->assertArrayHasKey('HTTP/1.1 400', $header); } + + public function test_when_with_a_callable_class_within_a_route() + { + $router = new Router; + $routine = new WhenAlwaysTrue; + $router->get('/', function() { return 'route'; }) + ->by($routine); + // By does not affect the output of the route. + $this->assertEquals( + $expected = 'route', + (string) $router->dispatch('GET', '/') + ); + $this->assertAttributeEquals( + $expected = true, + 'invoked', + $routine, + 'Routine was not invoked!' + ); + } } if (!function_exists(__NAMESPACE__.'\\header')) { diff --git a/tests/src/Stubs/Routines/ByClassWithInvoke.php b/tests/src/Stubs/Routines/ByClassWithInvoke.php new file mode 100644 index 0000000..2285762 --- /dev/null +++ b/tests/src/Stubs/Routines/ByClassWithInvoke.php @@ -0,0 +1,14 @@ +invoked = true; + return __CLASS__; + } +} \ No newline at end of file diff --git a/tests/src/Stubs/Routines/WhenAlwaysTrue.php b/tests/src/Stubs/Routines/WhenAlwaysTrue.php new file mode 100644 index 0000000..845dd2a --- /dev/null +++ b/tests/src/Stubs/Routines/WhenAlwaysTrue.php @@ -0,0 +1,14 @@ +invoked = true; + return true; + } +} \ No newline at end of file From a0a8ec4513c482cdf3d217b221b2d4e806fb9783 Mon Sep 17 00:00:00 2001 From: Augusto Pascutti Date: Sat, 9 Feb 2013 19:07:50 -0200 Subject: [PATCH 3/3] Accept routines passed as callable class names. - Creates an execute() method in AbstractSyncedRoutine; - Creates test for Respect\Rest\Routines\AbstractRoutine: - Add provider for valid constructor arguments - Add assert for getCallback() method on AbstractRoutine. --- .../Respect/Rest/Routines/AbstractRoutine.php | 8 +-- .../Rest/Routines/AbstractSyncedRoutine.php | 47 ++++++++++++--- library/Respect/Rest/Routines/By.php | 2 +- library/Respect/Rest/Routines/Through.php | 2 +- library/Respect/Rest/Routines/When.php | 2 +- tests/legacy/Respect/Rest/Routines/ByTest.php | 21 +++++++ .../Rest/Routines/AbstractRoutineTest.php | 58 +++++++++++++++++++ tests/src/Stubs/Routines/AbstractRoutine.php | 20 +++++++ 8 files changed, 144 insertions(+), 16 deletions(-) create mode 100644 tests/library/Respect/Rest/Routines/AbstractRoutineTest.php create mode 100644 tests/src/Stubs/Routines/AbstractRoutine.php diff --git a/library/Respect/Rest/Routines/AbstractRoutine.php b/library/Respect/Rest/Routines/AbstractRoutine.php index f5b784c..2952c01 100644 --- a/library/Respect/Rest/Routines/AbstractRoutine.php +++ b/library/Respect/Rest/Routines/AbstractRoutine.php @@ -3,9 +3,6 @@ namespace Respect\Rest\Routines; use InvalidArgumentException; -use ReflectionFunction; -use ReflectionMethod; -use Respect\Rest\Routes\AbstractRoute; /** Base class for callback routines */ abstract class AbstractRoutine implements Routinable @@ -15,8 +12,12 @@ abstract class AbstractRoutine implements Routinable public function __construct($callback) { + if (is_string($callback) && class_exists($callback) && method_exists($callback, '__invoke')) + return $this->callback = $callback; + if (!is_callable($callback)) throw new InvalidArgumentException('Routine callback must be... guess what... callable!'); + $this->callback = $callback; } @@ -24,5 +25,4 @@ protected function getCallback() { return $this->callback; } - } diff --git a/library/Respect/Rest/Routines/AbstractSyncedRoutine.php b/library/Respect/Rest/Routines/AbstractSyncedRoutine.php index f23078d..f9203d6 100644 --- a/library/Respect/Rest/Routines/AbstractSyncedRoutine.php +++ b/library/Respect/Rest/Routines/AbstractSyncedRoutine.php @@ -6,29 +6,56 @@ use ReflectionFunction; use ReflectionMethod; use ReflectionObject; +use ReflectionClass; use Closure; -use Respect\Rest\Routes\AbstractRoute; +use Respect\Rest\Routes\AbstractRoute, + Respect\Rest\Request; /** Base class for routines that sync parameters */ abstract class AbstractSyncedRoutine extends AbstractRoutine implements ParamSynced { - + /** + * @var Reflector + */ protected $reflection; + /** + * Return parameters that can be used with the routine. + * + * @return array + */ public function getParameters() { $reflection = $this->getReflection(); - if (!$reflection instanceof ReflectionObject) + if (!$reflection instanceof ReflectionObject && !$reflection instanceof ReflectionClass) return $this->getReflection()->getParameters(); - $constructorReflection = $reflection->getConstructor(); - if (is_null($constructorReflection)) - return array(); - else - return $constructorReflection->getParameters(); + return array(); + } + + /** + * Executes the routine and return its result. + * + * @param Respect\Rest\Request $request + * @param array $params + * @return mixed + */ + public function execute(Request $request, $params) + { + $callback = $this->getCallback(); + if (is_string($callback)) { + $reflection = $this->getReflection(); + $routineInstance = $reflection->newInstanceArgs($params); + return $routineInstance(); + } + return call_user_func_array($callback, $params); } - /** Returns a concrete ReflectionFunctionAbstract for this routine callback */ + /** + * Returns a concrete ReflectionFunctionAbstract for this routine callback. + * + * @return Reflector + */ protected function getReflection() { $callback = $this->getCallback(); @@ -36,6 +63,8 @@ protected function getReflection() return new ReflectionMethod($callback[0], $callback[1]); else if ($callback instanceof Closure) return new ReflectionFunction($callback); + else if (is_string($callback)) + return new ReflectionClass($callback); else return new ReflectionObject($callback); } diff --git a/library/Respect/Rest/Routines/By.php b/library/Respect/Rest/Routines/By.php index 5793da1..338caf6 100644 --- a/library/Respect/Rest/Routines/By.php +++ b/library/Respect/Rest/Routines/By.php @@ -10,7 +10,7 @@ class By extends AbstractSyncedRoutine implements ProxyableBy public function by(Request $request, $params) { - return call_user_func_array($this->callback, $params); + return $this->execute($request, $params); } } diff --git a/library/Respect/Rest/Routines/Through.php b/library/Respect/Rest/Routines/Through.php index 116c943..aa15a35 100644 --- a/library/Respect/Rest/Routines/Through.php +++ b/library/Respect/Rest/Routines/Through.php @@ -10,7 +10,7 @@ class Through extends AbstractSyncedRoutine implements ProxyableThrough public function through(Request $request, $params) { - return call_user_func_array($this->callback, $params); + return $this->execute($request, $params); } } diff --git a/library/Respect/Rest/Routines/When.php b/library/Respect/Rest/Routines/When.php index c5a29c3..8b1d4a4 100644 --- a/library/Respect/Rest/Routines/When.php +++ b/library/Respect/Rest/Routines/When.php @@ -10,7 +10,7 @@ class When extends AbstractSyncedRoutine implements ProxyableWhen public function when(Request $request, $params) { - $valid = call_user_func_array($this->callback, $params); + $valid = $this->execute($request, $params); if (!$valid) header('HTTP/1.1 400'); diff --git a/tests/legacy/Respect/Rest/Routines/ByTest.php b/tests/legacy/Respect/Rest/Routines/ByTest.php index 6e4d2b5..e0e201a 100644 --- a/tests/legacy/Respect/Rest/Routines/ByTest.php +++ b/tests/legacy/Respect/Rest/Routines/ByTest.php @@ -44,6 +44,7 @@ public function test_by_with_an_anonymous_function() /** * @covers Respect\Rest\Routines\By + * @covers Respect\Rest\Routines\AbstractSyncedRoutine */ public function test_by_on_a_route() { @@ -57,6 +58,26 @@ public function test_by_on_a_route() ); } + /** + * @covers Respect\Rest\Routines\By + * @covers Respect\Rest\Routines\AbstractSyncedRoutine + */ + public function test_by_on_a_route_with_classname() + { + $router = new Router(); + $router->get('/', function() { return 'route'; }) + ->by('Stubs\Routines\ByClassWithInvoke'); + // By does not affect the output of the route. + $this->assertEquals( + $expected = 'route', + (string) $router->dispatch('GET', '/') + ); + } + + /** + * @covers Respect\Rest\Routines\By + * @covers Respect\Rest\Routines\AbstractSyncedRoutine + */ public function test_by_with_a_callable_class_on_a_route() { $router = new Router; diff --git a/tests/library/Respect/Rest/Routines/AbstractRoutineTest.php b/tests/library/Respect/Rest/Routines/AbstractRoutineTest.php new file mode 100644 index 0000000..840ac43 --- /dev/null +++ b/tests/library/Respect/Rest/Routines/AbstractRoutineTest.php @@ -0,0 +1,58 @@ +assertInstanceOf( + 'Respect\Rest\Routines\AbstractRoutine', + $result = new Stub($argument) + ); + $this->assertSame( + $expected = $argument, + $result = $result->getCallback() + ); + } + + public function provide_valid_constructor_arguments() + { + return array( + array(function() { return 'Hello'; }), // an anonymous function + array(array('DateTime', 'createFromFormat')), // a class-method callable pair + array(new InstanceWithInvoke), // instance of a callable class + array('Stubs\Routines\WhenAlwaysTrue') // a callable class name + ); + } + + /** + * @dataProvider provide_invalid_constructor_arguments + * @covers Respect\Rest\Routines\AbstractRoutine + * @expectedException InvalidArgumentException + * @expectedExceptionMessage Routine callback must be... guess what... callable! + */ + public function test_invalid_constructor_arguments($argument) + { + $result = new Stub($argument); + } + + public function provide_invalid_constructor_arguments() + { + return array( + array('this_function_name_does_not_exist'), + array(new \StdClass), // an instance that is not callable + ); + } +} diff --git a/tests/src/Stubs/Routines/AbstractRoutine.php b/tests/src/Stubs/Routines/AbstractRoutine.php new file mode 100644 index 0000000..91f21af --- /dev/null +++ b/tests/src/Stubs/Routines/AbstractRoutine.php @@ -0,0 +1,20 @@ +callback; + } +}