Merge pull request #27 from danadesrosiers/fast-register

Fast register
danadesrosiers · May 17, 2018 · d6d1d3f · d6d1d3f
2 parents cc3bead + ccc4904
commit d6d1d3f
Show file tree

Hide file tree

Showing 13 changed files with 100 additions and 13 deletions.
diff --git a/README.md b/README.md
@@ -55,6 +55,12 @@ annot.cache
 -----------
 An instance of a class that implements Doctrine\Common\Cache\Cache.  This is the cache that will be used by the AnnotationReader to cache annotations so they don't have to be parsed every time.  Make sure to include Doctrine Cache as it is not a required dependency of this project.
 
+annot.base_uri (Enables Faster Controller Registration)
+--------------
+This is the base uri of all requests.  Basically, it's the part of the URI that isn't included in `$_SERVER['REQUEST_URI']`.  If your bootstrap file is at the root of htdocs, the value is "/".  If your bootstrap file lives in a directory called "api", all your API's URIs are prefixed with "/api" and that is value you must specify for annot.uri.
+
+annot.base_uri enables faster registration of controllers.  Silex has to register every endpoint in your app on every request.  If you have a lot of endpoints, that could be a significant overhead on each and every request.  Silex Annotations can improve this by filtering the controllers that need to be registered using the `prefix` on the `Controller` annotation.  We only need to register the endpoints in the Controller if the prefix matches the URI.  In this way, Silex Annotations allows FASTER routing than pure Silex. 
+
 Annotate Controllers
 ====================
 Create your controller.  The following is an example demonstrating the use of annotations to register an endpoint.

diff --git a/src/AnnotationService.php b/src/AnnotationService.php
@@ -100,11 +100,24 @@ public function discoverControllers($dir)
      */
     public function registerControllers($controllers)
     {
-        foreach ($controllers as $fqcn) {
-            $this->registerController($fqcn);
+        foreach ($controllers as $prefix => $controllerNames) {
+            if (!is_array($controllerNames)) {
+                $controllerNames = [$controllerNames];
+            }
+            foreach ($controllerNames as $fqcn) {
+                if (strlen($prefix) == 0 || $this->prefixMatchesUri($prefix)) {
+                    $this->registerController($fqcn);
+                }
+            }
         }
     }
 
+    public function prefixMatchesUri($prefix)
+    {
+        return ($this->app->offsetExists('annot.base_uri')
+            && strpos($_SERVER['REQUEST_URI'], $this->app['annot.base_uri'].$prefix) === 0);
+    }
+
     /**
      * Recursively walk the file tree starting from $dir to find potential controller class files.
      * Returns array of fully qualified class names.
@@ -131,19 +144,32 @@ public function getFiles($dir, $namespace='', $files=array())
                         $pathInfo = pathinfo($entry);
                         $className = trim($namespace.$pathInfo['filename']);
                         if (class_exists($className)) {
-                            $files[] = $className;
+                            $reflectionClass = new ReflectionClass($className);
+                            $annotationClassName = "\\DDesrosiers\\SilexAnnotations\\Annotations\\Controller";
+                            $controllerAnnotation = $this->reader->getClassAnnotation($reflectionClass, $annotationClassName);
+
+                            if ($this->hasPrefix($controllerAnnotation)) {
+                                $files[$controllerAnnotation->getPrefix()][] = $className;
+                            } else {
+                                $files[] = $className;
+                            }
                         }
                     }
                 }
             }
             closedir($handle);
         }
-        usort($files, function ($a, $b) {
-            return (string) $a > (string) $b ? 1 : -1;
-        });
+
         return $files;
     }
 
+    public function hasPrefix(Controller $controllerAnnotation = null)
+    {
+        $hasPrefix = $controllerAnnotation instanceof Controller && strlen($controllerAnnotation->prefix) > 0;
+
+        return $this->app->offsetExists('annot.base_uri') && $hasPrefix;
+    }
+
     /**
      * Register the controller if a Controller annotation exists in the class doc block or $controllerAnnotation is provided.
      *

diff --git a/src/AnnotationServiceProvider.php b/src/AnnotationServiceProvider.php
@@ -10,7 +10,6 @@
 
 namespace DDesrosiers\SilexAnnotations;
 
-use DDesrosiers\SilexAnnotations\Annotations\Controller;
 use Doctrine\Common\Annotations\AnnotationRegistry;
 use RuntimeException;
 use Pimple\Container;
@@ -108,5 +107,7 @@ function (Application $app, $controllerName, $methodName, $separator) {
         );
 
         $app['annot.controllerNamespace'] = '';
+
+        $app['annot.base_uri'] = '';
     }
 }
diff --git a/src/Annotations/Controller.php b/src/Annotations/Controller.php
@@ -33,4 +33,11 @@ public function process(Application $app, ReflectionClass $reflectionClass)
         $annotationService->processMethodAnnotations($reflectionClass, $controllerCollection);
         $app->mount($this->prefix, $controllerCollection);
     }
+
+    public function getPrefix()
+    {
+        // the prefix might not start with a forward slash, but the REQUEST_URI always will
+        // make sure we always have a forward slash so the comparison to REQUEST_URI works as expected
+        return ($this->prefix[0] !== '/') ? "/$this->prefix" : $this->prefix;
+    }
 } 
diff --git a/test/AnnotationDirArrayTestBase.php b/test/AnnotationDirArrayTestBase.php
@@ -79,6 +79,7 @@ protected function assertEndPointStatus($method, $uri, $status, $annotationOptio
 
     protected function makeRequest($method, $uri, $annotationOptions = array())
     {
+        $_SERVER['REQUEST_URI'] = $uri;
         $this->getClient($annotationOptions);
         $this->client->request($method, $uri, array(), array(), $this->requestOptions);
         $response = $this->client->getResponse();

diff --git a/test/AnnotationServiceDirArrayTest.php b/test/AnnotationServiceDirArrayTest.php
@@ -21,14 +21,18 @@ class AnnotationServiceDirArrayTest extends AnnotationDirArrayTestBase
     public function testServiceControllerDirArray()
     {
         $this->assertEndPointStatus(self::GET_METHOD, '/test/test1', self::STATUS_OK);
+        $this->setup();
         $this->assertEndPointStatus(self::GET_METHOD, '/test2/test1', self::STATUS_OK);
     }
 
     public function testIsolationOfControllerModifiersDirArray()
     {        
         $this->assertEndPointStatus(self::GET_METHOD, '/before/test', self::STATUS_ERROR);
+        $this->setup();
         $this->assertEndPointStatus(self::GET_METHOD, '/before2/test', self::STATUS_ERROR);
+        $this->setup();
         $this->assertEndPointStatus(self::GET_METHOD, '/test2/test1', self::STATUS_OK);
+        $this->setup();
         $this->assertEndPointStatus(self::GET_METHOD, '/test/test1', self::STATUS_OK);
     }
 
@@ -86,13 +90,14 @@ public function testControllerCache()
 
         // spot check a URI from each directory
         $this->assertEndPointStatus(self::GET_METHOD, '/test/test1', self::STATUS_OK);
+        $this->setup();
         $this->assertEndPointStatus(self::GET_METHOD, '/test2/test1', self::STATUS_OK);
 
         // check that we got the controllers from cache
         $this->assertTrue($cache->wasFetched($cacheKey1));
         $this->assertTrue($cache->wasFetched($cacheKey2));
 
-        $this->assertCount(13, $cache->fetch($cacheKey1));
+        $this->assertCount(14, $cache->fetch($cacheKey1));
         $this->assertCount(13, $cache->fetch($cacheKey2));
     }
 }

diff --git a/test/AnnotationServiceProviderTest.php b/test/AnnotationServiceProviderTest.php
@@ -73,13 +73,13 @@ public function testControllerCache()
         $this->app['debug'] = false;
         $service = $this->registerAnnotations();
         $service->discoverControllers(self::$CONTROLLER_DIR);
-        $this->assertCount(13, $cache->fetch($cacheKey));
+        $this->assertCount(14, $cache->fetch($cacheKey));
 
         $files = $service->discoverControllers(self::$CONTROLLER_DIR);
         $this->assertTrue($cache->wasFetched($cacheKey));
         $this->assertContains(self::CONTROLLER_NAMESPACE."SubDir\\SubDirTestController", $files);
-        $this->assertContains(self::CONTROLLER_NAMESPACE."TestController", $files);
-        $this->assertCount(13, $files);
+        $this->assertContains(self::CONTROLLER_NAMESPACE."TestController", $files['/test']);
+        $this->assertCount(14, $files);
     }
 }
 

diff --git a/test/AnnotationServiceTest.php b/test/AnnotationServiceTest.php
@@ -25,7 +25,8 @@ public function testServiceController()
 
     public function testIsolationOfControllerModifiers()
     {
-        $this->assertEndPointStatus(self::GET_METHOD, '/before/test', self::STATUS_ERROR);
+        $this->assertEndPointStatus(self::GET_METHOD, '/test/before', self::STATUS_ERROR);
+        $this->setup();
         $this->assertEndPointStatus(self::GET_METHOD, '/test/test1', self::STATUS_OK);
     }
 
@@ -43,7 +44,7 @@ public function cacheTestProvider()
             array('Array'),                                // string identifier
             array(new ApcCache()),                         // proper implementation of Cache
             array('Fake', 'RuntimeException'),             // invalid cache string
-            array(new InvalidCache(), 'RuntimeException')  // class that does not implement Cache
+            array(new NotCache(), 'RuntimeException')  // class that does not implement Cache
         );
     }
 
@@ -61,4 +62,16 @@ public function testCache($cache, $exception=null)
             $this->assertEquals($exception, get_class($e));
         }
     }
+
+    public function testFastRegister()
+    {
+        $this->assertEndPointStatus(self::GET_METHOD, '/two/test', self::STATUS_OK);
+        // there are 35 routes, but only 2 are registered (the ones with prefix '/' and '/two')
+        $this->assertEquals(2, count($this->app['routes']->all()));
+    }
+}
+
+class NotCache
+{
+
 }
diff --git a/test/AnnotationTestBase.php b/test/AnnotationTestBase.php
@@ -79,6 +79,7 @@ protected function assertEndPointStatus($method, $uri, $status, $annotationOptio
 
     protected function makeRequest($method, $uri, $annotationOptions = array())
     {
+        $_SERVER['REQUEST_URI'] = $uri;
         $this->getClient($annotationOptions);
         $this->client->request($method, $uri, array(), array(), $this->requestOptions);
         $response = $this->client->getResponse();

diff --git a/test/Annotations/ModifierTest.php b/test/Annotations/ModifierTest.php
@@ -32,6 +32,7 @@ public function testHttpsNoArgs()
         // testing a modifier that has no arguments
         // we make the request as http, but it should be redirected to a Http request
         $this->registerAnnotations();
+        $_SERVER['REQUEST_URI'] = '/test/requirehttps/modifier';
         $request = Request::create('http://test.com/test/requirehttps/modifier');
         $response = $this->app->handle($request);
         $this->assertEquals(301, $response->getStatusCode());

diff --git a/test/Annotations/RequireHttpTest.php b/test/Annotations/RequireHttpTest.php
@@ -24,6 +24,7 @@ public function testHttps()
     {
         // we make the request as https, but it should be redirected to a http request
         $this->registerAnnotations();
+        $_SERVER['REQUEST_URI'] = '/test/requirehttp';
         $request = Request::create('https://test.com/test/requirehttp');
         $response = $this->app->handle($request);
         $this->assertEquals(301, $response->getStatusCode());
@@ -39,6 +40,7 @@ public function testHttpsCollection()
     {
         // we make the request as https, but it should be redirected to a http request
         $this->registerAnnotations();
+        $_SERVER['REQUEST_URI'] = '/requirehttp/test';
         $request = Request::create('https://test.com/requirehttp/test');
         $response = $this->app->handle($request);
         $this->assertEquals(301, $response->getStatusCode());

diff --git a/test/Annotations/RequireHttpsTest.php b/test/Annotations/RequireHttpsTest.php
@@ -24,6 +24,7 @@ public function testHttp()
     {
         // we make the request as http, but it should be redirected to a https request
         $this->registerAnnotations();
+        $_SERVER['REQUEST_URI'] = '/test/requirehttps';
         $request = Request::create('https://test.com/test/requirehttps');
         $response = $this->app->handle($request);
         $this->assertEquals(301, $response->getStatusCode());
@@ -39,6 +40,7 @@ public function testHttpCollection()
     {
         // we make the request as http, but it should be redirected to a https request
         $this->registerAnnotations();
+        $_SERVER['REQUEST_URI'] = '/test/requirehttps';
         $request = Request::create('https://test.com/test/requirehttps');
         $response = $this->app->handle($request);
         $this->assertEquals(301, $response->getStatusCode());

diff --git a/test/Controller/TestController2.php b/test/Controller/TestController2.php
@@ -0,0 +1,22 @@
+<?php
+
+namespace DDesrosiers\Test\SilexAnnotations\Controller;
+
+use DDesrosiers\SilexAnnotations\Annotations as SLX;
+use Symfony\Component\HttpFoundation\Response;
+
+/**
+ * @SLX\Controller(prefix="/two")
+ */
+class TestController2
+{
+    /**
+     * @SLX\Route(
+     *      @SLX\Request(method="GET", uri="test")
+     * )
+     */
+    public function test1()
+    {
+        return new Response();
+    }
+}