fix: clear the admin URL generator before reusing it

Changes

- Call `unsetAll()` before `setController()` in `DashboardController` and
  `SecurityContractCrudController`
- Add `DashboardControllerTest`, and put `SecurityContractCrudController` into
  the admin smoke test's provider

Why

EasyAdmin registers `AdminUrlGenerator` as `shared: no`, so each injection point
gets its own instance — but both consumers here are shared, so that instance
lives as long as they do, which in a worker is longer than one request. It
accumulates route parameters as it is used. `AppExtension` and
`RepoAdvisoryService` already opened with `unsetAll()`; these two were the
inconsistency, and inconsistency is what rots.

Neither site was covered. The dashboard test asserts where the redirect lands
rather than that it merely redirects, because the failure mode worth catching is
silent: `unsetAll()` placed after `setController()` wipes the controller back
out and produces a URL pointing somewhere else without anything throwing. Both
new tests fail against that arrangement.

Author: turegjorup

CHANGELOG.md

@@ -40,7 +40,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - Implement `ResetInterface` on `LeantimeService`, whose memoised user
     directory has to stay a cache — `resolveUserName()` runs in a loop over
     tickets — but must not outlive the request
-  - Cover all three with tests: the factories had none
+  - Call `unsetAll()` before `setController()` on the injected
+    `AdminUrlGenerator` in `DashboardController` and
+    `SecurityContractCrudController`, which `AppExtension` and
+    `RepoAdvisoryService` already did. The instance is held for as long as its
+    consumer, which in a worker outlives the request
+  - Cover all of it with tests: the factories had none, and neither the
+    dashboard nor the Security Contract CRUD was in the admin smoke test
 - [#96](https://github.com/itk-dev/devops_itksites/pull/96)
   Show the Service Agreements monthly price as Danish kroner,
   `12.500,50 kr.`, on index and detail

src/Controller/Admin/DashboardController.php

@@ -28,6 +28,7 @@ public function __construct(
     public function index(): Response
     {
         $d = $this->adminUrlGenerator
+            ->unsetAll()
             ->setController(ServerCrudController::class)->setAction(Crud::PAGE_INDEX)
             ->generateUrl();
 

src/Controller/Admin/SecurityContractCrudController.php

@@ -132,6 +132,7 @@ public function syncAll(): RedirectResponse
 
         return $this->redirect(
             $this->adminUrlGenerator
+                ->unsetAll()
                 ->setController(static::class)
                 ->setAction(Crud::PAGE_INDEX)
                 ->generateUrl()

tests/Controller/Admin/AdminSmokeTest.php

@@ -17,6 +17,7 @@
 use App\Controller\Admin\OIDCCrudController;
 use App\Controller\Admin\PackageCrudController;
 use App\Controller\Admin\PackageVersionCrudController;
+use App\Controller\Admin\SecurityContractCrudController;
 use App\Controller\Admin\ServerCrudController;
 use App\Controller\Admin\ServiceCertificateCrudController;
 use App\Controller\Admin\SiteCrudController;
@@ -68,6 +69,7 @@ public static function crudControllerProvider(): iterable
         yield 'OIDC' => [OIDCCrudController::class];
         yield 'Package' => [PackageCrudController::class];
         yield 'PackageVersion' => [PackageVersionCrudController::class];
+        yield 'SecurityContract' => [SecurityContractCrudController::class];
         yield 'Server' => [ServerCrudController::class];
         yield 'ServiceCertificate' => [ServiceCertificateCrudController::class];
         yield 'Site' => [SiteCrudController::class];

tests/Controller/Admin/DashboardControllerTest.php

@@ -0,0 +1,68 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Controller\Admin;
+
+use App\Controller\Admin\ServerCrudController;
+use App\Entity\User;
+use EasyCorp\Bundle\EasyAdminBundle\Config\Crud;
+use EasyCorp\Bundle\EasyAdminBundle\Router\AdminUrlGenerator;
+use Hautelook\AliceBundle\PhpUnit\RefreshDatabaseTrait;
+use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
+
+/**
+ * The dashboard exists only to redirect to the Server index.
+ *
+ * It builds that URL on an injected AdminUrlGenerator, and the chain starts
+ * with unsetAll() because the generator instance is held for as long as this
+ * controller is — which, in a worker, is longer than one request. Order matters
+ * there: unsetAll() after setController() rather than before would wipe the
+ * controller back out and produce a URL pointing nowhere in particular, without
+ * anything throwing. Hence asserting where the redirect actually lands.
+ */
+class DashboardControllerTest extends WebTestCase
+{
+    use RefreshDatabaseTrait;
+
+    public function testItRedirectsToTheServerIndex(): void
+    {
+        $client = static::createClient();
+
+        $user = static::getContainer()->get('doctrine')->getManager()
+            ->getRepository(User::class)->findOneBy([]);
+        $client->loginUser($user);
+
+        $client->request('GET', '/admin');
+
+        $this->assertResponseRedirects();
+
+        // Ask EasyAdmin what that URL should be rather than hard-coding the
+        // slug, so this keeps working if the route path changes.
+        $expected = static::getContainer()->get(AdminUrlGenerator::class)
+            ->setController(ServerCrudController::class)
+            ->setAction(Crud::PAGE_INDEX)
+            ->generateUrl();
+
+        $location = (string) $client->getResponse()->headers->get('Location');
+        $this->assertSame(
+            parse_url($expected, \PHP_URL_PATH),
+            parse_url($location, \PHP_URL_PATH),
+            'the redirect must still land on the Server index'
+        );
+    }
+
+    public function testTheRedirectTargetLoads(): void
+    {
+        $client = static::createClient();
+
+        $user = static::getContainer()->get('doctrine')->getManager()
+            ->getRepository(User::class)->findOneBy([]);
+        $client->loginUser($user);
+
+        $client->request('GET', '/admin');
+        $client->followRedirect();
+
+        $this->assertResponseIsSuccessful();
+    }
+}