Allow overriding routes (#2577)

framework/core/src/Extend/Frontend.php

@@ -31,6 +31,7 @@ class Frontend implements ExtenderInterface
     private $css = [];
     private $js;
     private $routes = [];
+    private $removedRoutes = [];
     private $content = [];
 
     public function __construct(string $frontend)
@@ -59,6 +60,13 @@ class Frontend implements ExtenderInterface
         return $this;
     }
 
+    public function removeRoute(string $name)
+    {
+        $this->removedRoutes[] = compact('name');
+
+        return $this;
+    }
+
     /**
      * @param callable|string $callback
      * @return $this
@@ -141,7 +149,7 @@ class Frontend implements ExtenderInterface
 
     private function registerRoutes(Container $container)
     {
-        if (empty($this->routes)) {
+        if (empty($this->routes) && empty($this->removedRoutes)) {
             return;
         }
 
@@ -151,6 +159,10 @@ class Frontend implements ExtenderInterface
                 /** @var RouteHandlerFactory $factory */
                 $factory = $container->make(RouteHandlerFactory::class);
 
+                foreach ($this->removedRoutes as $route) {
+                    $collection->removeRoute('GET', $route['name']);
+                }
+
                 foreach ($this->routes as $route) {
                     $collection->get(
                         $route['path'],

framework/core/src/Extend/Routes.php

@@ -19,6 +19,7 @@ class Routes implements ExtenderInterface
     private $appName;
 
     private $routes = [];
+    private $removedRoutes = [];
 
     public function __construct($appName)
     {
@@ -62,9 +63,16 @@ class Routes implements ExtenderInterface
         return $this;
     }
 
+    public function remove(string $method, string $name)
+    {
+        $this->removedRoutes[] = compact('method', 'name');
+
+        return $this;
+    }
+
     public function extend(Container $container, Extension $extension = null)
     {
-        if (empty($this->routes)) {
+        if (empty($this->routes) && empty($this->removedRoutes)) {
             return;
         }
 
@@ -74,6 +82,10 @@ class Routes implements ExtenderInterface
                 /** @var RouteHandlerFactory $factory */
                 $factory = $container->make(RouteHandlerFactory::class);
 
+                foreach ($this->removedRoutes as $route) {
+                    $collection->removeRoute($route['method'], $route['name']);
+                }
+
                 foreach ($this->routes as $route) {
                     $collection->addRoute(
                         $route['method'],

framework/core/src/Forum/ForumServiceProvider.php

@@ -203,8 +203,8 @@ class ForumServiceProvider extends AbstractServiceProvider
         $factory = $this->app->make(RouteHandlerFactory::class);
         $defaultRoute = $this->app->make('flarum.settings')->get('default_route');
 
-        if (isset($routes->getRouteData()[0]['GET'][$defaultRoute]['handler'])) {
+        if (isset($routes->getRoutes()['GET'][$defaultRoute]['handler'])) {
-            $toDefaultController = $routes->getRouteData()[0]['GET'][$defaultRoute]['handler'];
+            $toDefaultController = $routes->getRoutes()['GET'][$defaultRoute]['handler'];
         } else {
             $toDefaultController = $factory->toForum(Content\Index::class);
         }

framework/core/src/Http/RouteCollection.php

@@ -30,6 +30,16 @@ class RouteCollection
      */
     protected $routeParser;
 
+    /**
+     * @var array
+     */
+    protected $routes = [];
+
+    /**
+     * @var array
+     */
+    protected $pendingRoutes = [];
+
     public function __construct()
     {
         $this->dataGenerator = new DataGenerator\GroupCountBased;
@@ -63,19 +73,50 @@ class RouteCollection
 
     public function addRoute($method, $path, $name, $handler)
     {
-        $routeDatas = $this->routeParser->parse($path);
+        if (isset($this->routes[$method][$name])) {
-
+            throw new \RuntimeException("Route $name on method $method already exists");
-        foreach ($routeDatas as $routeData) {
-            $this->dataGenerator->addRoute($method, $routeData, ['name' => $name, 'handler' => $handler]);
         }
 
-        $this->reverse[$name] = $routeDatas;
+        $this->routes[$method][$name] = $this->pendingRoutes[$method][$name] = compact('path', 'handler');
 
         return $this;
     }
 
+    public function removeRoute(string $method, string $name): self
+    {
+        unset($this->routes[$method][$name], $this->pendingRoutes[$method][$name]);
+
+        return $this;
+    }
+
+    protected function applyRoutes(): void
+    {
+        foreach ($this->pendingRoutes as $method => $routes) {
+            foreach ($routes as $name => $route) {
+                $routeDatas = $this->routeParser->parse($route['path']);
+
+                foreach ($routeDatas as $routeData) {
+                    $this->dataGenerator->addRoute($method, $routeData, ['name' => $name, 'handler' => $route['handler']]);
+                }
+
+                $this->reverse[$name] = $routeDatas;
+            }
+        }
+
+        $this->pendingRoutes = [];
+    }
+
+    public function getRoutes(): array
+    {
+        return $this->routes;
+    }
+
     public function getRouteData()
     {
+        if (! empty($this->pendingRoutes)) {
+            $this->applyRoutes();
+        }
+
         return $this->dataGenerator->getData();
     }
 
@@ -88,6 +129,10 @@ class RouteCollection
 
     public function getPath($name, array $parameters = [])
     {
+        if (! empty($this->pendingRoutes)) {
+            $this->applyRoutes();
+        }
+
         if (isset($this->reverse[$name])) {
             $maxMatches = 0;
             $matchingParts = $this->reverse[$name][0];

framework/core/tests/integration/extenders/RoutesTest.php

@@ -47,6 +47,41 @@ class RoutesTest extends TestCase
         $this->assertEquals(200, $response->getStatusCode());
         $this->assertEquals('Hello Flarumites!', $response->getBody());
     }
+
+    /**
+     * @test
+     */
+    public function existing_route_can_be_removed()
+    {
+        $this->extend(
+            (new Extend\Routes('api'))
+                ->remove('GET', 'forum.show')
+        );
+
+        $response = $this->send(
+            $this->request('GET', '/api')
+        );
+
+        $this->assertEquals(404, $response->getStatusCode());
+    }
+
+    /**
+     * @test
+     */
+    public function custom_route_can_override_existing_route_if_removed()
+    {
+        $this->extend(
+            (new Extend\Routes('api'))
+                ->remove('GET', 'forum.show')
+                ->get('/', 'forum.show', CustomRoute::class)
+        );
+
+        $response = $this->send(
+            $this->request('GET', '/api')
+        );
+
+        $this->assertEquals('Hello Flarumites!', $response->getBody());
+    }
 }
 
 class CustomRoute implements RequestHandlerInterface

framework/core/tests/unit/Http/RouteCollectionTest.php

@@ -0,0 +1,47 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Tests\unit\Http;
+
+use Flarum\Http\RouteCollection;
+use Flarum\Tests\unit\TestCase;
+
+class RouteCollectionTest extends TestCase
+{
+    /** @test */
+    public function can_add_routes()
+    {
+        $routeCollection = (new RouteCollection)
+            ->addRoute('GET', '/index', 'index', function () {
+                echo 'index';
+            })
+            ->addRoute('DELETE', '/posts', 'forum.posts.delete', function () {
+                echo 'delete posts';
+            });
+
+        $this->assertEquals('/index', $routeCollection->getPath('index'));
+        $this->assertEquals('/posts', $routeCollection->getPath('forum.posts.delete'));
+    }
+
+    /** @test */
+    public function can_add_routes_late()
+    {
+        $routeCollection = (new RouteCollection)->addRoute('GET', '/index', 'index', function () {
+            echo 'index';
+        });
+
+        $this->assertEquals('/index', $routeCollection->getPath('index'));
+
+        $routeCollection->addRoute('DELETE', '/posts', 'forum.posts.delete', function () {
+            echo 'delete posts';
+        });
+
+        $this->assertEquals('/posts', $routeCollection->getPath('forum.posts.delete'));
+    }
+}