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.
This commit is contained in:
Toby Zerner 2015-10-02 17:35:29 +09:30
parent 9e91ada4a8
commit f255d318ef
15 changed files with 124 additions and 85 deletions

View File

@ -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')
);
}

View File

@ -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';
}

View File

@ -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()
);

View File

@ -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()
);

View File

@ -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')
);

18
src/Api/UrlGenerator.php Normal file
View File

@ -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';
}

View File

@ -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;

View File

@ -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;
}

View File

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

View File

@ -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
);
}

View File

@ -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);
}

View File

@ -88,9 +88,13 @@ class RouteCollection
public function getPath($name, array $parameters = [])
{
$parts = $this->reverse[$name][0];
array_walk($parts, [$this, 'fixPathPart'], $parameters);
if (isset($this->reverse[$name])) {
$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");
}
}

View File

@ -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";
}
}

View File

@ -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>

View File

@ -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">