Fix building the tree, raise coverage to 100%, minor refactoring (#143)

yiisoft · Dec 25, 2021 · 071a3c2 · 071a3c2
1 parent 428be6d
commit 071a3c2
Show file tree

Hide file tree

Showing 14 changed files with 370 additions and 46 deletions.
diff --git a/composer.json b/composer.json
@@ -34,6 +34,7 @@
         "roave/infection-static-analysis-plugin": "^1.10",
         "spatie/phpunit-watcher": "^1.23",
         "vimeo/psalm": "^4.12",
+        "yiisoft/di": "^1.0",
         "yiisoft/dummy-provider": "^1.0.0",
         "yiisoft/test-support": "^1.3"
     },

diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -1,20 +1,32 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd" backupGlobals="false" colors="true" verbose="true" bootstrap="vendor/autoload.php" failOnRisky="true" failOnWarning="true">
-  <coverage>
-    <include>
-      <directory>./</directory>
-    </include>
-    <exclude>
-      <directory>./tests</directory>
-      <directory>./vendor</directory>
-    </exclude>
-  </coverage>
-  <php>
-    <ini name="error_reporting" value="-1"/>
-  </php>
-  <testsuites>
-    <testsuite name="Yii Router tests">
-      <directory>./tests/</directory>
-    </testsuite>
-  </testsuites>
+
+<phpunit
+    bootstrap="vendor/autoload.php"
+    colors="true"
+    verbose="true"
+    failOnRisky="true"
+    failOnWarning="true"
+    convertErrorsToExceptions="true"
+    convertNoticesToExceptions="true"
+    convertWarningsToExceptions="true"
+    stopOnFailure="false"
+    executionOrder="random"
+    resolveDependencies="true"
+>
+    <php>
+        <ini name="error_reporting" value="-1"/>
+    </php>
+
+    <testsuites>
+        <testsuite name="Yii Router tests">
+            <directory>./tests</directory>
+        </testsuite>
+    </testsuites>
+
+    <coverage>
+        <include>
+            <directory>./src</directory>
+            <directory>./config</directory>
+        </include>
+    </coverage>
 </phpunit>
diff --git a/src/Group.php b/src/Group.php
@@ -9,6 +9,7 @@
 use Yiisoft\Middleware\Dispatcher\MiddlewareDispatcher;
 
 use function get_class;
+use function gettype;
 use function in_array;
 use function is_object;
 

diff --git a/src/Route.php b/src/Route.php
@@ -9,6 +9,8 @@
 use Yiisoft\Http\Method;
 use Yiisoft\Middleware\Dispatcher\MiddlewareDispatcher;
 
+use function in_array;
+
 /**
  * Route defines a mapping from URL to callback / name and vice versa.
  */
@@ -365,7 +367,7 @@ public function __debugInfo()
     private function getDispatcherWithMiddlewares(): MiddlewareDispatcher
     {
         if ($this->dispatcher === null) {
-            throw new RuntimeException(sprintf('There is no dispatcher in the route %s', $this->getData('name')));
+            throw new RuntimeException(sprintf('There is no dispatcher in the route %s.', $this->getData('name')));
         }
 
         if ($this->dispatcher->hasMiddlewares()) {

diff --git a/src/RouteCollection.php b/src/RouteCollection.php
@@ -8,6 +8,10 @@
 use Psr\Http\Message\ResponseFactoryInterface;
 use Yiisoft\Http\Method;
 
+use function array_key_exists;
+use function in_array;
+use function is_array;
+
 /**
  * @psalm-type Items = array<array-key,array|string>
  */
@@ -129,7 +133,9 @@ private function injectGroup(Group $group, array &$tree, string $prefix = '', st
                     continue;
                 }
                 /** @psalm-suppress PossiblyNullArrayOffset Checked group prefix on not empty above */
-                $tree[$item->getData('prefix')] = [];
+                if (!isset($tree[$item->getData('prefix')])) {
+                    $tree[$item->getData('prefix')] = [];
+                }
                 /**
                  * @psalm-suppress MixedArgumentTypeCoercion
                  * @psalm-suppress MixedArgument,PossiblyNullArrayOffset
@@ -149,17 +155,8 @@ private function injectGroup(Group $group, array &$tree, string $prefix = '', st
                 $this->processCors($group, $host, $pattern, $modifiedItem, $tree);
             }
 
-            if (empty($tree[$group->getData('prefix')])) {
-                $tree[] = $modifiedItem->getData('name');
-            } else {
-                /**
-                 * @psalm-suppress MixedArrayAssignment,PossiblyNullArrayOffset
-                 * Checked group prefix on not empty above
-                 */
-                $tree[$group->getData('prefix')][] = $modifiedItem->getData('name');
-            }
-
             $routeName = $modifiedItem->getData('name');
+            $tree[] = $routeName;
             if (isset($this->routes[$routeName]) && !$modifiedItem->getData('override')) {
                 throw new InvalidArgumentException("A route with name '$routeName' already exists.");
             }
@@ -190,16 +187,10 @@ private function processCors(
         }
         if ($isNotDuplicate) {
             $optionsRoute = $optionsRoute->middleware($middleware);
-            if (empty($tree[$group->getData('prefix')])) {
-                $tree[] = $optionsRoute->getData('name');
-            } else {
-                /**
-                 * @psalm-suppress MixedArrayAssignment,PossiblyNullArrayOffset
-                 * Checked group prefix on not empty above
-                 */
-                $tree[$group->getData('prefix')][] = $optionsRoute->getData('name');
-            }
-            $this->routes[$optionsRoute->getData('name')] = $optionsRoute->action(
+
+            $routeName = $optionsRoute->getData('name');
+            $tree[] = $routeName;
+            $this->routes[$routeName] = $optionsRoute->action(
                 static fn (ResponseFactoryInterface $responseFactory) => $responseFactory->createResponse(204)
             );
         }

diff --git a/tests/ConfigTest.php b/tests/ConfigTest.php
@@ -0,0 +1,56 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Yiisoft\Router\Tests;
+
+use Nyholm\Psr7\Uri;
+use PHPUnit\Framework\TestCase;
+use Yiisoft\Di\Container;
+use Yiisoft\Di\ContainerConfig;
+use Yiisoft\Di\StateResetter;
+use Yiisoft\Router\CurrentRoute;
+use Yiisoft\Router\Route;
+use Yiisoft\Router\RouteCollector;
+use Yiisoft\Router\RouteCollectorInterface;
+
+final class ConfigTest extends TestCase
+{
+    public function testRouteCollector(): void
+    {
+        $container = $this->createContainer();
+
+        $routerCollector = $container->get(RouteCollectorInterface::class);
+        $this->assertInstanceOf(RouteCollector::class, $routerCollector);
+    }
+
+    public function testCurrentRoute(): void
+    {
+        $container = $this->createContainer();
+
+        $currentRoute = $container->get(CurrentRoute::class);
+        $currentRoute->setRouteWithArguments(Route::get('/main'), ['name' => 'hello']);
+        $currentRoute->setUri(new Uri('http://example.com/'));
+
+        $container->get(StateResetter::class)->reset();
+
+        $this->assertNull($currentRoute->getName());
+        $this->assertNull($currentRoute->getUri());
+        $this->assertSame([], $currentRoute->getArguments());
+    }
+
+    private function createContainer(): Container
+    {
+        return new Container(
+            ContainerConfig::create()->withDefinitions(
+                $this->getCommonDefinitions()
+            )
+        );
+    }
+
+    private function getCommonDefinitions(): array
+    {
+        $params = [];
+        return require dirname(__DIR__) . '/config/common.php';
+    }
+}
diff --git a/tests/GroupTest.php b/tests/GroupTest.php
@@ -21,6 +21,8 @@
 use Yiisoft\Router\RouteCollection;
 use Yiisoft\Router\RouteCollector;
 use Yiisoft\Router\Tests\Support\Container;
+use Yiisoft\Router\Tests\Support\TestMiddleware1;
+use Yiisoft\Router\Tests\Support\TestMiddleware2;
 
 final class GroupTest extends TestCase
 {
@@ -43,6 +45,17 @@ public function testAddMiddleware(): void
         $this->assertSame($middleware2, $group->getData('middlewareDefinitions')[1]);
     }
 
+    public function testDisabledMiddlewareDefinitions(): void
+    {
+        $group = Group::create()
+            ->middleware(TestMiddleware1::class)
+            ->middleware(TestMiddleware2::class)
+            ->disableMiddleware(TestMiddleware1::class);
+
+        $this->assertCount(1, $group->getData('middlewareDefinitions'));
+        $this->assertSame(TestMiddleware2::class, $group->getData('middlewareDefinitions')[0]);
+    }
+
     public function testRoutesAfterMiddleware(): void
     {
         $group = Group::create();
@@ -385,6 +398,15 @@ static function () {
         $this->assertInstanceOf(Route::class, $routeCollection->getRoute('OPTIONS /v1/post'));
     }
 
+    public function testMiddlewareAfterRoutes(): void
+    {
+        $group = Group::create()->routes(Route::get('/info')->action(static fn () => 'info'));
+
+        $this->expectException(RuntimeException::class);
+        $this->expectExceptionMessage('middleware() can not be used after routes().');
+        $group->middleware(static fn () => new Response());
+    }
+
     private function getRequestHandler(): RequestHandlerInterface
     {
         return new class () implements RequestHandlerInterface {

diff --git a/tests/MatchingResultTest.php b/tests/MatchingResultTest.php
@@ -12,6 +12,7 @@
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Server\RequestHandlerInterface;
+use RuntimeException;
 use Yiisoft\Http\Method;
 use Yiisoft\Middleware\Dispatcher\MiddlewareDispatcher;
 use Yiisoft\Middleware\Dispatcher\MiddlewareFactory;
@@ -71,6 +72,15 @@ public function testProcessFailure(): void
         $this->assertSame(404, $response->getStatusCode());
     }
 
+    public function testRouteOnFailure(): void
+    {
+        $result = MatchingResult::fromFailure([Method::GET, Method::HEAD]);
+
+        $this->expectException(RuntimeException::class);
+        $this->expectExceptionMessage('There is no route in the matching result.');
+        $result->route();
+    }
+
     private function getMiddleware(): callable
     {
         return static function () {

diff --git a/tests/RouteCollectionTest.php b/tests/RouteCollectionTest.php
@@ -112,13 +112,18 @@ public function testGetRouterTree(): void
                     ->routes(
                         Route::get('/posts', $this->getDispatcher())->name('/posts'),
                         Route::get('/post/{sile}')->name('/post/view')
-                    )->namePrefix('/v1')
+                    )->namePrefix('/v1'),
+                Group::create('/v1')
+                    ->routes(
+                        Route::get('/tags', $this->getDispatcher())->name('/tags'),
+                        Route::get('/tag/{slug}')->name('/tag/view'),
+                    )->namePrefix('/v1'),
             )->namePrefix('/api');
 
         $group2 = Group::create('/api')
             ->routes(
                 Route::get('/posts', $this->getDispatcher())->name('/posts'),
-                Route::get('/post/{sile}')->name('/post/view')
+                Route::get('/post/{sile}')->name('/post/view'),
             )->namePrefix('/api');
 
         $collector = new RouteCollector();
@@ -127,8 +132,22 @@ public function testGetRouterTree(): void
 
         $routeCollection = new RouteCollection($collector);
         $routeTree = $routeCollection->getRouteTree();
-        $this->assertCount(5, $routeTree);
-        $this->assertArrayHasKey('/v1', $routeTree);
+
+        $this->assertSame(
+            [
+                '[/api/test] GET /api/test',
+                '[/api/image] GET /api/images/{sile}',
+                '/v1' => [
+                    '[/api/v1/posts] GET /api/v1/posts',
+                    '[/api/v1/post/view] GET /api/v1/post/{sile}',
+                    '[/api/v1/tags] GET /api/v1/tags',
+                    '[/api/v1/tag/view] GET /api/v1/tag/{slug}',
+                ],
+                '[/api/posts] GET /api/posts',
+                '[/api/post/view] GET /api/post/{sile}',
+            ],
+            $routeTree
+        );
     }
 
     public function testGetRoutes(): void