Skip to content

Commit

Permalink
-
Browse files Browse the repository at this point in the history
  • Loading branch information
javiereguiluz committed Jan 11, 2025
1 parent 79d26cb commit 7483cd6
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 54 deletions.
1 change: 0 additions & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ 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
4 changes: 4 additions & 0 deletions config/services.php
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,13 @@
->arg(0, tagged_iterator(EasyAdminExtension::TAG_DASHBOARD_CONTROLLER))
->arg(1, tagged_iterator(EasyAdminExtension::TAG_CRUD_CONTROLLER))
->arg(2, service('cache.easyadmin'))
->arg(3, service('filesystem'))
->arg(4, '%kernel.build_dir%')

->set(AdminRouteLoader::class)
->arg(0, service(AdminRouteGenerator::class))
->arg(1, service('filesystem'))
->arg(2, '%kernel.build_dir%')
->tag('routing.loader', ['type' => AdminRouteLoader::ROUTE_LOADER_TYPE])

->set(UrlSigner::class)
Expand Down
14 changes: 9 additions & 5 deletions src/EventListener/AdminRouterSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class AdminRouterSubscriber implements EventSubscriberInterface
private CacheItemPoolInterface $cache;
private AdminRouteGeneratorInterface $adminRouteGenerator;
private CrudControllerRegistry $crudControllerRegistry;
private string $buildDir;
private bool $requestAlreadyProcessedAsPrettyUrl = false;

public function __construct(AdminContextFactory $adminContextFactory, ControllerFactory $controllerFactory, ControllerResolverInterface $controllerResolver, UrlGeneratorInterface $urlGenerator, RequestMatcherInterface $requestMatcher, CacheItemPoolInterface $cache, AdminRouteGenerator $adminRouteGenerator, string $buildDir, CrudControllerRegistry $crudControllerRegistry)
Expand Down Expand Up @@ -102,10 +103,10 @@ public function onKernelRequestPrettyUrls(RequestEvent $event): void
// edge-case: in some scenarios, admin routes are generated by the custom route loader
// and their information is cached but then removed from the cache (e.g. when running
// 'rm -fr var/cache/* && bin/console cache:clear'). If that's the case, regenerate the
// admin routes to force saving them in the cache again.
// admin routes (only if the app uses pretty URLs) to force saving them in the cache again.
// see https://github.com/EasyCorp/EasyAdminBundle/issues/6680
$adminRoutes = $this->cache->getItem(AdminRouteGenerator::CACHE_KEY_ROUTE_TO_FQCN)->get();
if (null === $adminRoutes) {
if (null === $adminRoutes && $this->adminRouteGenerator->usesPrettyUrls()) {
$this->adminRouteGenerator->generateAll();
}

Expand Down Expand Up @@ -142,17 +143,20 @@ public function onKernelRequest(RequestEvent $event): void
return;
}

// return early if this is not a URL associated with EasyAdmin
$request = $event->getRequest();
if (null === $dashboardControllerFqcn = $this->getDashboardControllerFqcn($request)) {
return;
}

// if this is a ugly URL from legacy EasyAdmin versions and the application
// uses pretty URLs, redirect to the equivalent pretty URL
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 {
if (is_subclass_of($entityFqcnOrCrudControllerFqcn, CrudControllerInterface::class)) {
$crudControllerFqcn = $entityFqcnOrCrudControllerFqcn;
} else {
$crudControllerFqcn = $this->crudControllerRegistry->findCrudFqcnByEntityFqcn($entityFqcnOrCrudControllerFqcn);
}

$prettyUrlRoute = $this->adminRouteGenerator->findRouteName($dashboardControllerFqcn, $crudControllerFqcn, $request->query->get(EA::CRUD_ACTION, ''));
Expand Down
25 changes: 6 additions & 19 deletions src/Router/AdminRouteGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use EasyCorp\Bundle\EasyAdminBundle\Config\Option\EA;
use EasyCorp\Bundle\EasyAdminBundle\Contracts\Router\AdminRouteGeneratorInterface;
use Psr\Cache\CacheItemPoolInterface;
use Symfony\Component\Filesystem\Filesystem;
use Symfony\Component\Routing\Route;
use Symfony\Component\Routing\RouteCollection;

Expand Down Expand Up @@ -59,31 +60,19 @@ final class AdminRouteGenerator implements AdminRouteGeneratorInterface
],
];

private ?bool $applicationUsesPrettyUrls = null;

public function __construct(
private iterable $dashboardControllers,
private iterable $crudControllers,
private CacheItemPoolInterface $cache,
private Filesystem $filesystem,
private string $buildDir,
) {
}

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 All @@ -102,9 +91,7 @@ public function generateAll(): RouteCollection
// TODO: remove this method in EasyAdmin 5.x
public function usesPrettyUrls(): bool
{
$cachedAdminRoutes = $this->cache->getItem(self::CACHE_KEY_FQCN_TO_ROUTE)->get();

return null !== $cachedAdminRoutes && [] !== $cachedAdminRoutes;
return $this->applicationUsesPrettyUrls ??= $this->filesystem->exists(sprintf('%s/%s', $this->buildDir, AdminRouteLoader::PRETTY_URLS_CONTEXT_FILE_NAME));
}

public function findRouteName(string $dashboardFqcn, string $crudControllerFqcn, string $actionName): ?string
Expand Down
23 changes: 23 additions & 0 deletions src/Router/AdminRouteLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use EasyCorp\Bundle\EasyAdminBundle\Contracts\Router\AdminRouteGeneratorInterface;
use Symfony\Component\Config\Loader\Loader;
use Symfony\Component\Filesystem\Filesystem;
use Symfony\Component\Routing\RouteCollection;

/**
Expand All @@ -12,9 +13,13 @@
final class AdminRouteLoader extends Loader
{
public const ROUTE_LOADER_TYPE = 'easyadmin.routes';
/** @internal don't use this in your application */
public const PRETTY_URLS_CONTEXT_FILE_NAME = 'easyadmin/application_uses_pretty_urls.txt';

public function __construct(
private AdminRouteGeneratorInterface $adminRouteGenerator,
private Filesystem $filesystem,
private string $buildDir,
) {
parent::__construct(null);
}
Expand All @@ -26,6 +31,24 @@ public function supports($resource, ?string $type = null): bool

public function load($resource, ?string $type = null): RouteCollection
{
// this is ugly, 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.
// In som parts of this bundle, we need to know if the custom route loader was
// run to decide if pretty URLs should be used or not.
//
// This can't be solved in any of these ways:
// * the router doesn't provide any feature to know if a route loader was run
// * 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)
// * we can't create a "marker service" set inside the custom route loader because
// this only works when the route loader is run the first time (next times, the routes are cached)
// * etc.
//
// The only reliable way is to create a "marker file" in the cache that is not deleted.
// All this will be greatly simplified in EasyAdmin 5.x when pretty URLs will be mandatory.
$this->filesystem->dumpFile(sprintf('%s/%s', $this->buildDir, self::PRETTY_URLS_CONTEXT_FILE_NAME), '');

return $this->adminRouteGenerator->generateAll();
}
}
58 changes: 29 additions & 29 deletions tests/Controller/PrettyUrls/PrettyUrlsControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,26 +22,6 @@ 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 @@ -301,6 +281,26 @@ public function testAdminUrlGeneratorUsePrettyUrls()
$this->assertSame('http://localhost/admin/pretty/urls/blog-post', $blogPostIndexUrl);
}

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

$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->getResponse()->headers->get('Location'));
}

public static function provideActiveMenuUrls(): iterable
{
yield ['/admin/pretty/urls/category'];
Expand All @@ -318,14 +318,14 @@ public static function provideDefaultPageUrls(): iterable

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'];
yield [CategoryCrudController::class, Action::INDEX, null, [], '/admin/pretty/urls/category'];
yield [Category::class, Action::INDEX, null, [], '/admin/pretty/urls/category'];
yield [Category::class, Action::INDEX, null, ['page' => 2, 'foo' => 'bar'], '/admin/pretty/urls/category?page=2&foo=bar'];
yield [CategoryCrudController::class, Action::DETAIL, 1, [], '/admin/pretty/urls/category/1'];
yield [Category::class, Action::DETAIL, 1, [], '/admin/pretty/urls/category/1'];
yield [CategoryCrudController::class, Action::NEW, null, [], '/admin/pretty/urls/category/new'];
yield [Category::class, Action::NEW, null, [], '/admin/pretty/urls/category/new'];
yield [CategoryCrudController::class, Action::EDIT, 1, [], '/admin/pretty/urls/category/1/edit'];
yield [Category::class, Action::EDIT, 1, [], '/admin/pretty/urls/category/1/edit'];
}
}

0 comments on commit 7483cd6

Please sign in to comment.