Skip to content

Commit

Permalink
When using pretty URLs, redirect ugly URLs automatically
Browse files Browse the repository at this point in the history
  • Loading branch information
javiereguiluz committed Jan 11, 2025
1 parent b76eca4 commit 79d26cb
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 4 deletions.
1 change: 1 addition & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ jobs:
- name: Run tests
continue-on-error: ${{ matrix.stability == 'dev' || matrix.symfony_version == '7.2' }}
env:
USE_PRETTY_URLS: "0"
SYMFONY_DEPRECATIONS_HELPER: "max[total]=999999&ignoreFile=./tests/baseline-ignore.txt"
run: |
./vendor/bin/simple-phpunit --exclude-group pretty_urls
Expand Down
2 changes: 2 additions & 0 deletions config/services.php
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@
->arg(4, service('router'))
->arg(5, service('cache.easyadmin'))
->arg(6, service(AdminRouteGenerator::class))
->arg(7, '%kernel.build_dir%')
->arg(8, service(CrudControllerRegistry::class))
->tag('kernel.event_subscriber')

->set(ControllerFactory::class)
Expand Down
59 changes: 56 additions & 3 deletions src/EventListener/AdminRouterSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,18 @@

namespace EasyCorp\Bundle\EasyAdminBundle\EventListener;

use EasyCorp\Bundle\EasyAdminBundle\Cache\CacheWarmer;
use EasyCorp\Bundle\EasyAdminBundle\Config\Option\EA;
use EasyCorp\Bundle\EasyAdminBundle\Contracts\Controller\CrudControllerInterface;
use EasyCorp\Bundle\EasyAdminBundle\Contracts\Controller\DashboardControllerInterface;
use EasyCorp\Bundle\EasyAdminBundle\Contracts\Router\AdminRouteGeneratorInterface;
use EasyCorp\Bundle\EasyAdminBundle\Factory\AdminContextFactory;
use EasyCorp\Bundle\EasyAdminBundle\Factory\ControllerFactory;
use EasyCorp\Bundle\EasyAdminBundle\Registry\CrudControllerRegistry;
use EasyCorp\Bundle\EasyAdminBundle\Router\AdminRouteGenerator;
use Psr\Cache\CacheItemPoolInterface;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface;
use Symfony\Component\HttpKernel\Event\ControllerEvent;
Expand Down Expand Up @@ -40,8 +43,10 @@ class AdminRouterSubscriber implements EventSubscriberInterface
private RequestMatcherInterface $requestMatcher;
private CacheItemPoolInterface $cache;
private AdminRouteGeneratorInterface $adminRouteGenerator;
private CrudControllerRegistry $crudControllerRegistry;
private bool $requestAlreadyProcessedAsPrettyUrl = false;

public function __construct(AdminContextFactory $adminContextFactory, ControllerFactory $controllerFactory, ControllerResolverInterface $controllerResolver, UrlGeneratorInterface $urlGenerator, RequestMatcherInterface $requestMatcher, CacheItemPoolInterface $cache, AdminRouteGenerator $adminRouteGenerator)
public function __construct(AdminContextFactory $adminContextFactory, ControllerFactory $controllerFactory, ControllerResolverInterface $controllerResolver, UrlGeneratorInterface $urlGenerator, RequestMatcherInterface $requestMatcher, CacheItemPoolInterface $cache, AdminRouteGenerator $adminRouteGenerator, string $buildDir, CrudControllerRegistry $crudControllerRegistry)
{
$this->adminContextFactory = $adminContextFactory;
$this->controllerFactory = $controllerFactory;
Expand All @@ -50,6 +55,8 @@ public function __construct(AdminContextFactory $adminContextFactory, Controller
$this->requestMatcher = $requestMatcher;
$this->cache = $cache;
$this->adminRouteGenerator = $adminRouteGenerator;
$this->buildDir = $buildDir;

Check failure on line 58 in src/EventListener/AdminRouterSubscriber.php

View workflow job for this annotation

GitHub Actions / phpstan

Access to an undefined property EasyCorp\Bundle\EasyAdminBundle\EventListener\AdminRouterSubscriber::$buildDir.
$this->crudControllerRegistry = $crudControllerRegistry;
}

public static function getSubscribedEvents(): array
Expand All @@ -67,8 +74,29 @@ public static function getSubscribedEvents(): array
public function onKernelRequestPrettyUrls(RequestEvent $event): void
{
$request = $event->getRequest();
if (false === $request->attributes->has(EA::ROUTE_CREATED_BY_EASYADMIN)) {
return;
if (false === $request->attributes->getBoolean(EA::ROUTE_CREATED_BY_EASYADMIN)) {
// at this point, the incoming request can be:
// (1) A dashboard URL (they don't have the EasyAdmin route attributes) of an app using pretty URLs
// (2) An ugly URL from EasyAdmin (they all use the main dashboard URL and select the action to execute via query params)
// (3) a regular Symfony request (not related to EasyAdmin)

// check if the URL includes the 'crudControllerFqcn' query parameter
// if it does, this is case (2) and we don't handle it as a pretty URL
if ($request->query->has(EA::CRUD_CONTROLLER_FQCN)) {
return;
}

// this can be case (1) or (3). Sadly, the dashboard routes don't include
// the EasyAdmin route attributes because they are regular Symfony routes.
// I can't find any way (inside our custom route loader, in a compiler pass, etc.) to add the
// custom EasyAdmin route defaults/attributes to other existing Symfony routes. So, we have to
// check if the route of the current request matches any of the cached dashboard routes
$dashboardRoutesCachePath = $this->buildDir.'/'.CacheWarmer::DASHBOARD_ROUTES_CACHE;

Check failure on line 94 in src/EventListener/AdminRouterSubscriber.php

View workflow job for this annotation

GitHub Actions / phpstan

Access to an undefined property EasyCorp\Bundle\EasyAdminBundle\EventListener\AdminRouterSubscriber::$buildDir.
$dashboardControllerRoutes = !file_exists($dashboardRoutesCachePath) ? [] : require $dashboardRoutesCachePath;
// this is not a cached dashboard route, so this is case (3) a regular Symfony request
if (!array_key_exists($request->attributes->get('_route'), $dashboardControllerRoutes)) {
return;
}
}

// edge-case: in some scenarios, admin routes are generated by the custom route loader
Expand All @@ -82,6 +110,10 @@ public function onKernelRequestPrettyUrls(RequestEvent $event): void
}

$dashboardControllerFqcn = $request->attributes->get(EA::DASHBOARD_CONTROLLER_FQCN);
if (null === $dashboardControllerFqcn) {
$dashboardCallableAsString = $dashboardControllerRoutes[$request->attributes->get('_route')];

Check failure on line 114 in src/EventListener/AdminRouterSubscriber.php

View workflow job for this annotation

GitHub Actions / phpstan

Variable $dashboardControllerRoutes might not be defined.
[$dashboardControllerFqcn, ] = explode('::', $dashboardCallableAsString);
}
if (null === $dashboardControllerInstance = $this->getDashboardControllerInstance($dashboardControllerFqcn, $request)) {
return;
}
Expand All @@ -97,6 +129,7 @@ public function onKernelRequestPrettyUrls(RequestEvent $event): void
}

$request->attributes->set(EA::CONTEXT_REQUEST_ATTRIBUTE, $adminContext);
$this->requestAlreadyProcessedAsPrettyUrl = true;
}

/**
Expand All @@ -105,11 +138,31 @@ public function onKernelRequestPrettyUrls(RequestEvent $event): void
*/
public function onKernelRequest(RequestEvent $event): void
{
if ($this->requestAlreadyProcessedAsPrettyUrl) {
return;
}

$request = $event->getRequest();
if (null === $dashboardControllerFqcn = $this->getDashboardControllerFqcn($request)) {
return;
}

if ($this->adminRouteGenerator->usesPrettyUrls()) {
$entityFqcnOrCrudControllerFqcn = $request->query->get(EA::CRUD_CONTROLLER_FQCN);
if (!is_subclass_of($entityFqcnOrCrudControllerFqcn, CrudControllerInterface::class)) {
$crudControllerFqcn = $this->crudControllerRegistry->findCrudFqcnByEntityFqcn($entityFqcnOrCrudControllerFqcn);
} else {
$crudControllerFqcn = $entityFqcnOrCrudControllerFqcn;
}

$prettyUrlRoute = $this->adminRouteGenerator->findRouteName($dashboardControllerFqcn, $crudControllerFqcn, $request->query->get(EA::CRUD_ACTION, ''));
$request->query->remove(EA::CRUD_CONTROLLER_FQCN);

$event->setResponse(new RedirectResponse($this->urlGenerator->generate($prettyUrlRoute, $request->query->all())));

return;
}

if (null === $dashboardControllerInstance = $this->getDashboardControllerInstance($dashboardControllerFqcn, $request)) {
return;
}
Expand Down
16 changes: 16 additions & 0 deletions src/Router/AdminRouteGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,22 @@ public function __construct(

public function generateAll(): RouteCollection
{
// this is ugly and only used in tests, but I can't find any other way of solving this problem.
// Details about the problem to solve: EasyAdmin must support both ugly and
// pretty URLs and the user must not configure anything to enable pretty URLs.
// Pretty URLs are generated automatically via a custom route loader but:
// * there's no way to know if the custom route loader was run or not
// * we can't set a container parameter dynamically because ParameterBag is frozen
// * we can't store something in the cache because it's not reliable (see https://github.com/symfony/symfony/issues/59445)
// * etc.
// This way, in tests that don't use pretty URLs we set this env var to force
// not to generate pretty URLs. This is needed because otherwise, the pretty URLs
// will be generated in AdminRouteSubscriber.
// All this will be greatly simplified in EasyAdmin 5.x when pretty URLs will be mandatory.
if ('0' === $_SERVER['USE_PRETTY_URLS']) {
return new RouteCollection();
}

$collection = new RouteCollection();
$adminRoutes = $this->generateAdminRoutes();

Expand Down
36 changes: 36 additions & 0 deletions tests/Controller/PrettyUrls/PrettyUrlsControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@
namespace EasyCorp\Bundle\EasyAdminBundle\Tests\Controller\PrettyUrls;

use EasyCorp\Bundle\EasyAdminBundle\Config\Action;
use EasyCorp\Bundle\EasyAdminBundle\Config\Option\EA;
use EasyCorp\Bundle\EasyAdminBundle\Router\AdminUrlGenerator;
use EasyCorp\Bundle\EasyAdminBundle\Tests\PrettyUrlsTestApplication\Controller\BlogPostCrudController;
use EasyCorp\Bundle\EasyAdminBundle\Tests\PrettyUrlsTestApplication\Controller\CategoryCrudController;
use EasyCorp\Bundle\EasyAdminBundle\Tests\PrettyUrlsTestApplication\Controller\DashboardController;
use EasyCorp\Bundle\EasyAdminBundle\Tests\PrettyUrlsTestApplication\Entity\Category;
use EasyCorp\Bundle\EasyAdminBundle\Tests\PrettyUrlsTestApplication\Kernel;
use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;

Expand All @@ -19,6 +22,26 @@ public static function getKernelClass(): string
return Kernel::class;
}

/**
* @dataProvider provideUglyUrlRedirects
*/
public function testUglyUrlsAreRedirectedToPrettyUrls(string $crudControllerFqcn, string $crudAction, ?int $entityId, array $extraQueryParameters, string $expectedPrettyUrlRedirect)
{
$client = static::createClient();
$client->followRedirects();

$queryParameters = array_merge([
EA::CRUD_CONTROLLER_FQCN => $crudControllerFqcn,
EA::CRUD_ACTION => $crudAction,
EA::ENTITY_ID => $entityId,
], $extraQueryParameters);

$uglyUrl = sprintf('/admin/pretty/urls?%s', http_build_query($queryParameters));
$client->request('GET', $uglyUrl);

$this->assertSame($expectedPrettyUrlRedirect, $client->getRequest()->getUri());
}

public function testGeneratedRoutes()
{
// the generated routes are:
Expand Down Expand Up @@ -292,4 +315,17 @@ public static function provideDefaultPageUrls(): iterable
yield 'Read' => ['/admin/pretty/urls/category/1'];
yield 'Update' => ['/admin/pretty/urls/category/1/edit'];
}

public static function provideUglyUrlRedirects(): iterable
{
yield [CategoryCrudController::class, Action::INDEX, null, [], 'http://localhost/admin/pretty/urls/category'];
yield [Category::class, Action::INDEX, null, [], 'http://localhost/admin/pretty/urls/category'];
yield [Category::class, Action::INDEX, null, ['page' => 2, 'foo' => 'bar'], 'http://localhost/admin/pretty/urls/category?foo=bar&page=2'];
yield [CategoryCrudController::class, Action::DETAIL, 1, [], 'http://localhost/admin/pretty/urls/category/1'];
yield [Category::class, Action::DETAIL, 1, [], 'http://localhost/admin/pretty/urls/category/1'];
yield [CategoryCrudController::class, Action::NEW, null, [], 'http://localhost/admin/pretty/urls/category/new'];
yield [Category::class, Action::NEW, null, [], 'http://localhost/admin/pretty/urls/category/new'];
yield [CategoryCrudController::class, Action::EDIT, 1, [], 'http://localhost/admin/pretty/urls/category/1/edit'];
yield [Category::class, Action::EDIT, 1, [], 'http://localhost/admin/pretty/urls/category/1/edit'];
}
}
3 changes: 2 additions & 1 deletion tests/bootstrap.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Symfony\Bundle\FrameworkBundle\Console\Application;
use Symfony\Component\Console\Input\ArrayInput;
use Symfony\Component\Console\Output\ConsoleOutput;
use Symfony\Component\Filesystem\Filesystem;

// needed to avoid encoding issues when running tests on different platforms
setlocale(\LC_ALL, 'en_US.UTF-8');
Expand Down Expand Up @@ -45,7 +46,7 @@
}

// delete the existing cache directory to avoid issues
(new Symfony\Component\Filesystem\Filesystem())->remove($kernel->getCacheDir());
(new Filesystem())->remove($kernel->getCacheDir());

$application = new Application($kernel);
$application->setAutoExit(false);
Expand Down

0 comments on commit 79d26cb

Please sign in to comment.