Add multiple UrlGenerator classes for forum/api/admin

Spent quite a while looking into the best solution here and ended up going with three separate classes. Thanks to @Luceos for the PR that got this rolling (#518). My reasoning is:

- The task of routing and URL generation is independent for each section of the app. Take Flarum\Api\Users\IndexAction for example. I don't want to generate a URL to a Flarum route... I specifically want to generate a URL to an API route. So there should be a class with that specific responsibility.
- In fact, each URL generator is slightly different, because we need to add a certain prefix to the start (e.g. /api)
- This also allows us to get rid of the "flarum.api" prefix on each route's name.
- It's still DRY, because they all extend a base class.

At the same time, I could see no reason this needed to be "interfaced", so all of the classes are concrete.

Goes a long way to fixing #123 - still just a few places left remaining with hardcoded URLs.

framework/core/src/Admin/AdminServiceProvider.php

@@ -11,7 +11,7 @@
 namespace Flarum\Admin;
 
 use Flarum\Http\RouteCollection;
-use Flarum\Http\UrlGenerator;
+use Flarum\Admin\UrlGenerator;
 use Illuminate\Support\ServiceProvider;
 use Psr\Http\Message\ServerRequestInterface;
 use Zend\Diactoros\Response\RedirectResponse;
@@ -26,7 +26,7 @@ class AdminServiceProvider extends ServiceProvider
     public function register()
     {
         $this->app->singleton(
-            'Flarum\Http\UrlGeneratorInterface',
+            UrlGenerator::class,
             function () {
                 return new UrlGenerator($this->app->make('flarum.admin.routes'));
             }
@@ -57,7 +57,7 @@ class AdminServiceProvider extends ServiceProvider
 
         $routes->get(
             '/',
-            'flarum.admin.index',
+            'index',
             $this->action('Flarum\Admin\Actions\ClientAction')
         );
     }

framework/core/src/Admin/UrlGenerator.php

@@ -0,0 +1,18 @@
+<?php
+/*
+ * This file is part of Flarum.
+ *
+ * (c) Toby Zerner <toby.zerner@gmail.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Flarum\Admin;
+
+use Flarum\Http\UrlGenerator as BaseUrlGenerator;
+
+class UrlGenerator extends BaseUrlGenerator
+{
+    protected $prefix = 'admin';
+}

framework/core/src/Api/Actions/Discussions/IndexAction.php

@@ -14,7 +14,7 @@ use Flarum\Core\Search\SearchCriteria;
 use Flarum\Core\Discussions\Search\DiscussionSearcher;
 use Flarum\Api\Actions\SerializeCollectionAction;
 use Flarum\Api\JsonApiRequest;
-use Flarum\Http\UrlGeneratorInterface;
+use Flarum\Api\UrlGenerator;
 use Tobscure\JsonApi\Document;
 
 class IndexAction extends SerializeCollectionAction
@@ -25,7 +25,7 @@ class IndexAction extends SerializeCollectionAction
     protected $searcher;
 
     /**
-     * @var UrlGeneratorInterface
+     * @var UrlGenerator
      */
     protected $url;
 
@@ -74,9 +74,9 @@ class IndexAction extends SerializeCollectionAction
 
     /**
      * @param DiscussionSearcher $searcher
-     * @param UrlGeneratorInterface $url
+     * @param UrlGenerator $url
      */
-    public function __construct(DiscussionSearcher $searcher, UrlGeneratorInterface $url)
+    public function __construct(DiscussionSearcher $searcher, UrlGenerator $url)
     {
         $this->searcher = $searcher;
         $this->url = $url;
@@ -106,7 +106,7 @@ class IndexAction extends SerializeCollectionAction
         $this->addPaginationLinks(
             $document,
             $request,
-            $request->http ? $this->url->toRoute('flarum.api.discussions.index') : '',
+            $request->http ? $this->url->toRoute('discussions.index') : '',
             $results->areMoreResults()
         );
 

framework/core/src/Api/Actions/Users/IndexAction.php

@@ -14,7 +14,7 @@ use Flarum\Core\Search\SearchCriteria;
 use Flarum\Core\Users\Search\UserSearcher;
 use Flarum\Api\Actions\SerializeCollectionAction;
 use Flarum\Api\JsonApiRequest;
-use Flarum\Http\UrlGeneratorInterface;
+use Flarum\Api\UrlGenerator;
 use Tobscure\JsonApi\Document;
 
 class IndexAction extends SerializeCollectionAction
@@ -25,7 +25,7 @@ class IndexAction extends SerializeCollectionAction
     protected $searcher;
 
     /**
-     * @var UrlGeneratorInterface
+     * @var UrlGenerator
      */
     protected $url;
 
@@ -68,9 +68,9 @@ class IndexAction extends SerializeCollectionAction
 
     /**
      * @param UserSearcher $searcher
-     * @param UrlGeneratorInterface $url
+     * @param UrlGenerator $url
      */
-    public function __construct(UserSearcher $searcher, UrlGeneratorInterface $url)
+    public function __construct(UserSearcher $searcher, UrlGenerator $url)
     {
         $this->searcher = $searcher;
         $this->url = $url;
@@ -97,7 +97,7 @@ class IndexAction extends SerializeCollectionAction
         $this->addPaginationLinks(
             $document,
             $request,
-            $this->url->toRoute('flarum.api.users.index'),
+            $this->url->toRoute('users.index'),
             $results->areMoreResults()
         );
 

framework/core/src/Api/ApiServiceProvider.php

@@ -17,7 +17,7 @@ use Flarum\Events\RegisterApiRoutes;
 use Flarum\Events\RegisterActivityTypes;
 use Flarum\Events\RegisterNotificationTypes;
 use Flarum\Http\RouteCollection;
-use Flarum\Http\UrlGenerator;
+use Flarum\Api\UrlGenerator;
 use Illuminate\Support\ServiceProvider;
 use Psr\Http\Message\ServerRequestInterface;
 
@@ -35,7 +35,7 @@ class ApiServiceProvider extends ServiceProvider
         });
 
         $this->app->singleton(
-            'Flarum\Http\UrlGeneratorInterface',
+            UrlGenerator::class,
             function () {
                 return new UrlGenerator($this->app->make('flarum.api.routes'));
             }
@@ -78,28 +78,28 @@ class ApiServiceProvider extends ServiceProvider
         // Get forum information
         $routes->get(
             '/forum',
-            'flarum.api.forum.show',
+            'forum.show',
             $this->action('Flarum\Api\Actions\Forum\ShowAction')
         );
 
         // Save forum information
         $routes->patch(
             '/forum',
-            'flarum.api.forum.update',
+            'forum.update',
             $this->action('Flarum\Api\Actions\Forum\UpdateAction')
         );
 
         // Retrieve authentication token
         $routes->post(
             '/token',
-            'flarum.api.token',
+            'token',
             $this->action('Flarum\Api\Actions\TokenAction')
         );
 
         // Send forgot password email
         $routes->post(
             '/forgot',
-            'flarum.api.forgot',
+            'forgot',
             $this->action('Flarum\Api\Actions\ForgotAction')
         );
 
@@ -112,49 +112,49 @@ class ApiServiceProvider extends ServiceProvider
         // List users
         $routes->get(
             '/users',
-            'flarum.api.users.index',
+            'users.index',
             $this->action('Flarum\Api\Actions\Users\IndexAction')
         );
 
         // Register a user
         $routes->post(
             '/users',
-            'flarum.api.users.create',
+            'users.create',
             $this->action('Flarum\Api\Actions\Users\CreateAction')
         );
 
         // Get a single user
         $routes->get(
             '/users/{id}',
-            'flarum.api.users.show',
+            'users.show',
             $this->action('Flarum\Api\Actions\Users\ShowAction')
         );
 
         // Edit a user
         $routes->patch(
             '/users/{id}',
-            'flarum.api.users.update',
+            'users.update',
             $this->action('Flarum\Api\Actions\Users\UpdateAction')
         );
 
         // Delete a user
         $routes->delete(
             '/users/{id}',
-            'flarum.api.users.delete',
+            'users.delete',
             $this->action('Flarum\Api\Actions\Users\DeleteAction')
         );
 
         // Upload avatar
         $routes->post(
             '/users/{id}/avatar',
-            'flarum.api.users.avatar.upload',
+            'users.avatar.upload',
             $this->action('Flarum\Api\Actions\Users\UploadAvatarAction')
         );
 
         // Remove avatar
         $routes->delete(
             '/users/{id}/avatar',
-            'flarum.api.users.avatar.delete',
+            'users.avatar.delete',
             $this->action('Flarum\Api\Actions\Users\DeleteAvatarAction')
         );
 
@@ -167,28 +167,28 @@ class ApiServiceProvider extends ServiceProvider
         // List activity
         $routes->get(
             '/activity',
-            'flarum.api.activity.index',
+            'activity.index',
             $this->action('Flarum\Api\Actions\Activity\IndexAction')
         );
 
         // List notifications for the current user
         $routes->get(
             '/notifications',
-            'flarum.api.notifications.index',
+            'notifications.index',
             $this->action('Flarum\Api\Actions\Notifications\IndexAction')
         );
 
         // Mark all notifications as read
         $routes->post(
             '/notifications/read',
-            'flarum.api.notifications.readAll',
+            'notifications.readAll',
             $this->action('Flarum\Api\Actions\Notifications\ReadAllAction')
         );
 
         // Mark a single notification as read
         $routes->patch(
             '/notifications/{id}',
-            'flarum.api.notifications.update',
+            'notifications.update',
             $this->action('Flarum\Api\Actions\Notifications\UpdateAction')
         );
 
@@ -201,35 +201,35 @@ class ApiServiceProvider extends ServiceProvider
         // List discussions
         $routes->get(
             '/discussions',
-            'flarum.api.discussions.index',
+            'discussions.index',
             $this->action('Flarum\Api\Actions\Discussions\IndexAction')
         );
 
         // Create a discussion
         $routes->post(
             '/discussions',
-            'flarum.api.discussions.create',
+            'discussions.create',
             $this->action('Flarum\Api\Actions\Discussions\CreateAction')
         );
 
         // Show a single discussion
         $routes->get(
             '/discussions/{id}',
-            'flarum.api.discussions.show',
+            'discussions.show',
             $this->action('Flarum\Api\Actions\Discussions\ShowAction')
         );
 
         // Edit a discussion
         $routes->patch(
             '/discussions/{id}',
-            'flarum.api.discussions.update',
+            'discussions.update',
             $this->action('Flarum\Api\Actions\Discussions\UpdateAction')
         );
 
         // Delete a discussion
         $routes->delete(
             '/discussions/{id}',
-            'flarum.api.discussions.delete',
+            'discussions.delete',
             $this->action('Flarum\Api\Actions\Discussions\DeleteAction')
         );
 
@@ -242,35 +242,35 @@ class ApiServiceProvider extends ServiceProvider
         // List posts, usually for a discussion
         $routes->get(
             '/posts',
-            'flarum.api.posts.index',
+            'posts.index',
             $this->action('Flarum\Api\Actions\Posts\IndexAction')
         );
 
         // Create a post
         $routes->post(
             '/posts',
-            'flarum.api.posts.create',
+            'posts.create',
             $this->action('Flarum\Api\Actions\Posts\CreateAction')
         );
 
         // Show a single or multiple posts by ID
         $routes->get(
             '/posts/{id}',
-            'flarum.api.posts.show',
+            'posts.show',
             $this->action('Flarum\Api\Actions\Posts\ShowAction')
         );
 
         // Edit a post
         $routes->patch(
             '/posts/{id}',
-            'flarum.api.posts.update',
+            'posts.update',
             $this->action('Flarum\Api\Actions\Posts\UpdateAction')
         );
 
         // Delete a post
         $routes->delete(
             '/posts/{id}',
-            'flarum.api.posts.delete',
+            'posts.delete',
             $this->action('Flarum\Api\Actions\Posts\DeleteAction')
         );
 
@@ -283,28 +283,28 @@ class ApiServiceProvider extends ServiceProvider
         // List groups
         $routes->get(
             '/groups',
-            'flarum.api.groups.index',
+            'groups.index',
             $this->action('Flarum\Api\Actions\Groups\IndexAction')
         );
 
         // Create a group
         $routes->post(
             '/groups',
-            'flarum.api.groups.create',
+            'groups.create',
             $this->action('Flarum\Api\Actions\Groups\CreateAction')
         );
 
         // Edit a group
         $routes->patch(
             '/groups/{id}',
-            'flarum.api.groups.update',
+            'groups.update',
             $this->action('Flarum\Api\Actions\Groups\UpdateAction')
         );
 
         // Delete a group
         $routes->delete(
             '/groups/{id}',
-            'flarum.api.groups.delete',
+            'groups.delete',
             $this->action('Flarum\Api\Actions\Groups\DeleteAction')
         );
 
@@ -317,28 +317,28 @@ class ApiServiceProvider extends ServiceProvider
         // Toggle an extension
         $routes->patch(
             '/extensions/{name}',
-            'flarum.api.extensions.update',
+            'extensions.update',
             $this->action('Flarum\Api\Actions\Extensions\UpdateAction')
         );
 
         // Uninstall an extension
         $routes->delete(
             '/extensions/{name}',
-            'flarum.api.extensions.delete',
+            'extensions.delete',
             $this->action('Flarum\Api\Actions\Extensions\DeleteAction')
         );
 
         // Update config settings
         $routes->post(
             '/config',
-            'flarum.api.config',
+            'config',
             $this->action('Flarum\Api\Actions\ConfigAction')
         );
 
         // Update a permission
         $routes->post(
             '/permission',
-            'flarum.api.permission',
+            'permission',
             $this->action('Flarum\Api\Actions\PermissionAction')
         );
 

framework/core/src/Api/UrlGenerator.php

@@ -0,0 +1,18 @@
+<?php
+/*
+ * This file is part of Flarum.
+ *
+ * (c) Toby Zerner <toby.zerner@gmail.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Flarum\Api;
+
+use Flarum\Http\UrlGenerator as BaseUrlGenerator;
+
+class UrlGenerator extends BaseUrlGenerator
+{
+    protected $prefix = 'api';
+}

framework/core/src/Core/Users/Commands/RequestPasswordResetHandler.php

@@ -17,7 +17,7 @@ use Illuminate\Contracts\Mail\Mailer;
 use Illuminate\Mail\Message;
 use Illuminate\Database\Eloquent\ModelNotFoundException;
 use Flarum\Core;
-use Flarum\Http\UrlGeneratorInterface;
+use Flarum\Forum\UrlGenerator;
 
 class RequestPasswordResetHandler
 {
@@ -37,7 +37,7 @@ class RequestPasswordResetHandler
     protected $mailer;
 
     /**
-     * @var UrlGeneratorInterface
+     * @var UrlGenerator
      */
     protected $url;
 
@@ -45,9 +45,9 @@ class RequestPasswordResetHandler
      * @param UserRepository $users
      * @param SettingsRepository $settings
      * @param Mailer $mailer
-     * @param UrlGeneratorInterface $url
+     * @param UrlGenerator $url
      */
-    public function __construct(UserRepository $users, SettingsRepository $settings, Mailer $mailer, UrlGeneratorInterface $url)
+    public function __construct(UserRepository $users, SettingsRepository $settings, Mailer $mailer, UrlGenerator $url)
     {
         $this->users = $users;
         $this->settings = $settings;

framework/core/src/Core/Users/User.php

@@ -328,7 +328,7 @@ class User extends Model
      */
     public function getAvatarUrlAttribute()
     {
-        $urlGenerator = app('Flarum\Http\UrlGeneratorInterface');
+        $urlGenerator = app('Flarum\Forum\UrlGenerator');
 
         return $this->avatar_path ? $urlGenerator->toAsset('avatars/'.$this->avatar_path) : null;
     }

framework/core/src/Forum/Actions/DiscussionAction.php

@@ -50,8 +50,8 @@ class DiscussionAction extends ClientAction
                 $queryString[] = $k . '=' . $v;
             }
 
-            return app('Flarum\Http\UrlGeneratorInterface')
+            return app('Flarum\Forum\UrlGenerator')
-                ->toRoute('flarum.forum.discussion', ['id' => $document->data->id]) .
+                ->toRoute('discussion', ['id' => $document->data->id]) .
                 ($queryString ? '?' . implode('&', $queryString) : '');
         };
 

framework/core/src/Forum/ForumServiceProvider.php

@@ -13,7 +13,7 @@ namespace Flarum\Forum;
 use Flarum\Core\Users\Guest;
 use Flarum\Events\RegisterForumRoutes;
 use Flarum\Http\RouteCollection;
-use Flarum\Http\UrlGenerator;
+use Flarum\Forum\UrlGenerator;
 use Flarum\Support\ServiceProvider;
 use Psr\Http\Message\ServerRequestInterface;
 
@@ -31,7 +31,7 @@ class ForumServiceProvider extends ServiceProvider
         });
 
         $this->app->singleton(
-            'Flarum\Http\UrlGeneratorInterface',
+            UrlGenerator::class,
             function () {
                 return new UrlGenerator($this->app->make('flarum.forum.routes'));
             }
@@ -62,67 +62,67 @@ class ForumServiceProvider extends ServiceProvider
 
         $routes->get(
             '/all',
-            'flarum.forum.index',
+            'index',
             $defaultAction = $this->action('Flarum\Forum\Actions\IndexAction')
         );
 
         $routes->get(
             '/d/{id:\d+(?:-[^/]*)?}[/{near:[^/]*}]',
-            'flarum.forum.discussion',
+            'discussion',
             $this->action('Flarum\Forum\Actions\DiscussionAction')
         );
 
         $routes->get(
             '/u/{username}[/{filter:[^/]*}]',
-            'flarum.forum.user',
+            'user',
             $this->action('Flarum\Forum\Actions\ClientAction')
         );
 
         $routes->get(
             '/settings',
-            'flarum.forum.settings',
+            'settings',
             $this->action('Flarum\Forum\Actions\ClientAction')
         );
 
         $routes->get(
             '/notifications',
-            'flarum.forum.notifications',
+            'notifications',
             $this->action('Flarum\Forum\Actions\ClientAction')
         );
 
         $routes->get(
             '/logout',
-            'flarum.forum.logout',
+            'logout',
             $this->action('Flarum\Forum\Actions\LogoutAction')
         );
 
         $routes->post(
             '/login',
-            'flarum.forum.login',
+            'login',
             $this->action('Flarum\Forum\Actions\LoginAction')
         );
 
         $routes->post(
             '/register',
-            'flarum.forum.register',
+            'register',
             $this->action('Flarum\Forum\Actions\RegisterAction')
         );
 
         $routes->get(
             '/confirm/{token}',
-            'flarum.forum.confirmEmail',
+            'confirmEmail',
             $this->action('Flarum\Forum\Actions\ConfirmEmailAction')
         );
 
         $routes->get(
             '/reset/{token}',
-            'flarum.forum.resetPassword',
+            'resetPassword',
             $this->action('Flarum\Forum\Actions\ResetPasswordAction')
         );
 
         $routes->post(
             '/reset',
-            'flarum.forum.savePassword',
+            'savePassword',
             $this->action('Flarum\Forum\Actions\SavePasswordAction')
         );
 
@@ -137,7 +137,7 @@ class ForumServiceProvider extends ServiceProvider
 
         $routes->get(
             '/',
-            'flarum.forum.default',
+            'default',
             $defaultAction
         );
     }

framework/core/src/Forum/UrlGenerator.php

@@ -1,5 +1,4 @@
 <?php
-
 /*
  * This file is part of Flarum.
  *
@@ -9,11 +8,10 @@
  * file that was distributed with this source code.
  */
 
-namespace Flarum\Http;
+namespace Flarum\Forum;
 
-interface UrlGeneratorInterface
+use Flarum\Http\UrlGenerator as BaseUrlGenerator;
+
+class UrlGenerator extends BaseUrlGenerator
 {
-    public function toRoute($name, $parameters = []);
-
-    public function toAsset($path);
 }

framework/core/src/Http/RouteCollection.php

@@ -88,9 +88,13 @@ class RouteCollection
 
     public function getPath($name, array $parameters = [])
     {
-        $parts = $this->reverse[$name][0];
+        if (isset($this->reverse[$name])) {
-        array_walk($parts, [$this, 'fixPathPart'], $parameters);
+            $parts = $this->reverse[$name][0];
+            array_walk($parts, [$this, 'fixPathPart'], $parameters);
 
-        return '/' . ltrim(implode('', $parts), '/');
+            return '/' . ltrim(implode('', $parts), '/');
+        }
+
+        throw new \RuntimeException("Route $name not found");
     }
 }

framework/core/src/Http/UrlGenerator.php

@@ -13,10 +13,11 @@ namespace Flarum\Http;
 
 use Flarum\Core;
 
-class UrlGenerator implements UrlGeneratorInterface
+class UrlGenerator
 {
     protected $routes;
 
+    protected $prefix;
 
     public function __construct(RouteCollection $routes)
     {
@@ -28,11 +29,11 @@ class UrlGenerator implements UrlGeneratorInterface
         $path = $this->routes->getPath($name, $parameters);
         $path = ltrim($path, '/');
 
-        return Core::url() . "/$path";
+        return Core::url($this->prefix) . "/$path";
     }
 
     public function toAsset($path)
     {
-        return Core::url() . "/assets/$path";
+        return Core::url($this->prefix) . "/assets/$path";
     }
 }

framework/core/views/index.blade.php

@@ -1,5 +1,5 @@
 <?php
-$url = app('Flarum\Http\UrlGeneratorInterface');
+$url = app('Flarum\Forum\UrlGenerator');
 ?>
 <div class="container">
     <h2>All Discussions</h2>
@@ -7,7 +7,7 @@ $url = app('Flarum\Http\UrlGeneratorInterface');
     <ul>
         @foreach ($document->data as $discussion)
             <li>
-                <a href="{{ $url->toRoute('flarum.forum.discussion', [
+                <a href="{{ $url->toRoute('discussion', [
                     'id' => $discussion->id . '-' . $discussion->attributes->title
                 ]) }}">
                     {{ $discussion->attributes->title }}
@@ -16,5 +16,5 @@ $url = app('Flarum\Http\UrlGeneratorInterface');
         @endforeach
     </ul>
 
-    <a href="{{ $url->toRoute('flarum.forum.index') }}?page={{ $page + 1 }}">Next Page &raquo;</a>
+    <a href="{{ $url->toRoute('index') }}?page={{ $page + 1 }}">Next Page &raquo;</a>
 </div>

framework/core/views/reset.blade.php

@@ -11,7 +11,7 @@
   <body>
     <h1>Reset Your Password</h1>
 
-    <form class="form-horizontal" role="form" method="POST" action="{{ app('Flarum\Http\UrlGeneratorInterface')->toRoute('flarum.forum.savePassword') }}">
+    <form class="form-horizontal" role="form" method="POST" action="{{ app('Flarum\Forum\UrlGenerator')->toRoute('savePassword') }}">
       <input type="hidden" name="token" value="{{ $token }}">
 
       <div class="form-group">