Merge pull request #246 from components-web-app/fix/245-route-generator-parent-route

Refuse to generate a route for a page whose parent has no route (#245)

Author: silverbackdan

CLAUDE.md

@@ -171,6 +171,15 @@ Consumers:
 3. Resolves name/path conflicts with a numeric suffix
 4. Creates or updates the `Route` entity and calls `setRoute()` on the `PageData`
 
+**Generation refuses when the page has a parent but the parent has no route (#245).** If `parentPage` or `parentPageData` is set and `getParentPageRoute()` returns null, `create()` throws `UnroutedParentException` before touching anything. It used to skip the prefix silently and hand the child a bare top-level path — frequently the parent's own natural path (a child at `/2027` under an unrouted conference whose natural path is `/2027`). The parent could then never take its route: `UniqueEntity('path')` rejected it with a 422 on a later, unrelated write, naming a path the user never created. The reason is **path squatting, not rendering** — rendering depth comes from the manifest either way (see **Route path concatenation — recommended, not required**). The generator must not claim a path that is not the page's to claim, so it fails where the cause is.
+
+- `POST /_/routes/generate` returns **422** (`application/problem+json`, the message in `detail`), via `exception_to_status` in `prependApiPlatformConfig()`. 422 matches the endpoint's existing refusals (page/pageData both or neither), which are 422 validation violations. No Route is created.
+- **Pages with no parent are unaffected** and still get a top-level path.
+- **Explicit creation is unaffected.** `POST /_/routes` with a path, `DoctrineContext` and `CwaFixtureBuilder`'s `route:` argument never call the generator, so a routed child under an unrouted parent remains possible and remains live (the #224 guard). Refusing to *generate* a path is not refusing the page a route.
+- `CwaFixtureBuilder` propagates the exception out of `flush()`: a page nested under a parent that gets no route (an `isTemplate: true` page with no `route:`) must be given an explicit `route:`.
+
+Tests: `tests/Helper/Route/RouteGeneratorTest.php`; `features/main/route.feature` (refused with 422 and zero Routes, prefixed under a routed parent, explicit creation still 201).
+
 ### Caching architecture
 
 Resources are designed as **individual, piecemeal, independently cacheable entities**. The API does not bundle data into large grouped responses. Each resource (Route, Page, Layout, ComponentGroup, Component, etc.) is fetched and cached separately. When a resource changes, only that resource's cache entry is invalidated — not anything that merely references it.
@@ -253,7 +262,7 @@ Pages support sub-pages. A conference page at `/best-conference-ever` renders a
 
 A validation constraint (`Assert\Expression`) ensures both cannot be set simultaneously.
 
-`getParentPageRoute(): ?Route` is a computed helper (no DB column) returning `$parentPage?->getRoute() ?? $parentPageData?->getRoute()`. Used by `RouteGenerator` to prefix paths. Returns null gracefully when the parent is still in draft (no public Route yet).
+`getParentPageRoute(): ?Route` is a computed helper (no DB column) returning `$parentPage?->getRoute() ?? $parentPageData?->getRoute()`. Used by `RouteGenerator` to prefix paths. Returns null when the parent is still in draft (no public Route yet), in which case `RouteGenerator` refuses to generate a route for the child (#245).
 
 ### How the manifest carries parent resources
 
@@ -302,7 +311,7 @@ This means:
 
 - **No `$nested` boolean** — parent = nested, full stop. The presence of `$parentPage`/`$parentPageData` is the complete signal.
 - **Two FK properties, not one** — `AbstractPage` is a mapped superclass with no discriminator map; `?AbstractPage` cannot be a Doctrine FK target. `?Page` + `?AbstractPageData` mirrors `Route.$page`/`Route.$pageData`.
-- **`getParentPageRoute()` is computed** — no DB column; used by `RouteGenerator` only; returns null safely when the parent has no route yet.
+- **`getParentPageRoute()` is computed** — no DB column; used by `RouteGenerator` only; returns null when the parent has no route yet, and `RouteGenerator` then refuses to generate (#245).
 - **Route concatenation is recommended, not required** — `RouteGenerator` prefixes child paths for clean URLs and SEO, but the module's `<CwaPage />` renders depth from manifest data, not URL structure.
 - **`resource_iris` is `string[][]`, not `string[]`** — depth-grouped, root first. The module reads the array index as the rendering depth without any client-side traversal.
 - **Single rendering mechanism** — `<CwaPage />` uses a manifest in both public and admin/draft contexts. Both contexts use the same `/_/resource_manifest/{id}` endpoint — route path for public, UUID for admin/draft. The chain walk (`parentPage`/`parentPageData`) is a fallback only. No URL-depth dependency.
@@ -492,6 +501,7 @@ GroupBuilder
 | no `route:` on `->page()` + `isTemplate: true` | no Route created |
 | no `route:` on `->page()` without template flag | RouteGenerator called from title (slug) |
 | `->pageData(...)` inside `->nested()`, no route | RouteGenerator called → `/parent-path/slug-from-title` |
+| `->page(...)`/`->pageData(...)` inside `->nested()` of a parent that gets no route (e.g. `isTemplate: true`), no route | `UnroutedParentException` from `flush()` — pass an explicit `route:` (#245) |
 | `->pageData(...)` or `->page(...)` at top level, no route, no title | no Route created (draft) |
 
 ### Allowed components on groups
@@ -564,7 +574,7 @@ $topicBuilder->onRoutesCreated(function (array $childBuilders) use ($intro) {
 
 - **No `$nested` boolean** — parent = nested, full stop. The presence of `$parentPage`/`$parentPageData` is the complete signal.
 - **Two FK properties, not one** — `AbstractPage` is a mapped superclass with no discriminator map; `?AbstractPage` cannot be a Doctrine FK target. `?Page` + `?AbstractPageData` mirrors `Route.$page`/`Route.$pageData`.
-- **`getParentPageRoute()` is computed** — no DB column; used by `RouteGenerator` only; returns null safely when the parent has no route yet.
+- **`getParentPageRoute()` is computed** — no DB column; used by `RouteGenerator` only; returns null when the parent has no route yet, and `RouteGenerator` then refuses to generate (#245).
 - **Route concatenation is recommended, not required** — `RouteGenerator` prefixes child paths for clean URLs and SEO, but the module's `<CwaPage />` renders depth from manifest data, not URL structure.
 - **`resource_iris` is `string[][]`, not `string[]`** — depth-grouped, root first. The module reads the array index as the rendering depth without any client-side traversal.
 - **Single rendering mechanism** — `<CwaPage />` uses a manifest in both public and admin/draft contexts. Both contexts use the same `/_/resource_manifest/{id}` endpoint — route path for public, UUID for admin/draft. The chain walk (`parentPage`/`parentPageData`) is a fallback only. No URL-depth dependency.

features/bootstrap/DoctrineContext.php

@@ -76,13 +76,6 @@ final class DoctrineContext implements Context
     private JsonContext $jsonContext;
     private RouteLiveResolver $routeLiveResolver;
 
-    /**
-     * Initializes context.
-     *
-     * Every scenario gets its own context instance.
-     * You can also pass arbitrary arguments to the
-     * context constructor through behat.yml.
-     */
     public function __construct(ManagerRegistry $doctrine, JWTTokenManagerInterface $jwtManager, IriConverterInterface $iriConverter, TimestampedDataPersister $timestampedHelper, UserPasswordHasherInterface $passwordHasher, JWTEncoderInterface $jwtEncoder, RouteLiveResolver $routeLiveResolver)
     {
         $this->routeLiveResolver = $routeLiveResolver;
@@ -2102,6 +2095,18 @@ public function theResponseResourceShouldBeSavedAs($name): void
         $this->restContext->resources[$name] = $response['@id'];
     }
 
+    /**
+     * @Then there should be :count Route resources
+     */
+    public function thereShouldBeRouteResources(int $count): void
+    {
+        $this->manager->clear();
+        $actual = \count($this->manager->getRepository(Route::class)->findAll());
+        if ($actual !== $count) {
+            throw new ExpectationException(\sprintf('Expected %d Route resources, found %d.', $count, $actual), $this->minkContext->getSession()->getDriver());
+        }
+    }
+
     /**
      * @Then there should be :count ComponentPosition resources
      */

features/main/route.feature

@@ -107,6 +107,34 @@ Feature: Route resources
     And the JSON should be valid according to the schema file "route.schema.json"
     And the Route "/original" should redirect to "/unnamed-page"
 
+  @loginUser
+  Scenario: A route cannot be generated for a nested page whose parent page has no route
+    Given there is a routeless parent PageData with a component and an unrouted child Page
+    When I send a "POST" request to "/_/routes/generate" with data:
+      | page                 |
+      | resource[child_page] |
+    Then the response status code should be 422
+    And the JSON node "detail" should contain "parent page has no route"
+    And there should be 0 Route resources
+
+  @loginUser
+  Scenario: A route generated for a nested page whose parent page has a route is prefixed with the parent path
+    Given there is a PageData resource with the route path "/conference/programme" nested within the route "/conference"
+    When I send a "POST" request to "/_/routes/generate" with data:
+      | pageData            |
+      | resource[page_data] |
+    Then the response status code should be 201
+    And the JSON node "path" should be equal to the string "/conference/unnamed-page"
+
+  @loginUser
+  Scenario: A route can still be created explicitly for a nested page whose parent page has no route
+    Given there is a routeless parent PageData with a component and an unrouted child Page
+    When I send a "POST" request to "/_/routes" with data:
+      | path  | name            | page                 |
+      | /2027 | conference-2027 | resource[child_page] |
+    Then the response status code should be 201
+    And the JSON node "path" should be equal to the string "/2027"
+
   @loginUser
   Scenario: I update a route path. A new redirect will be created.
     Given there is a PageData resource with the route path "/original"

src/DependencyInjection/SilverbackApiComponentsExtension.php

@@ -21,6 +21,7 @@
 use Silverback\ApiComponentsBundle\EventListener\Form\FormSuccessEventListenerInterface;
 use Silverback\ApiComponentsBundle\Exception\ApiPlatformAuthenticationException;
 use Silverback\ApiComponentsBundle\Exception\UnparseableRequestHeaderException;
+use Silverback\ApiComponentsBundle\Exception\UnroutedParentException;
 use Silverback\ApiComponentsBundle\Exception\UserDisabledException;
 use Silverback\ApiComponentsBundle\Factory\Uploadable\MediaObjectFactory;
 use Silverback\ApiComponentsBundle\Factory\User\Mailer\ChangeEmailConfirmationEmailFactory;
@@ -365,6 +366,7 @@ private function prependApiPlatformConfig(ContainerBuilder $container, array $co
                     UnparseableRequestHeaderException::class => 400,
                     ApiPlatformAuthenticationException::class => 401,
                     UserDisabledException::class => 401,
+                    UnroutedParentException::class => 422,
                 ],
             ]
         );

src/Exception/UnroutedParentException.php

@@ -0,0 +1,19 @@
+<?php
+
+/*
+ * This file is part of the Silverback API Components Bundle Project
+ *
+ * (c) Daniel West <daniel@silverback.is>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Silverback\ApiComponentsBundle\Exception;
+
+/**
+ * @author Daniel West <daniel@silverback.is>
+ */
+class UnroutedParentException extends \RuntimeException
+{
+}

src/Helper/Route/RouteGenerator.php

@@ -16,6 +16,7 @@
 use Silverback\ApiComponentsBundle\Entity\Core\RoutableInterface;
 use Silverback\ApiComponentsBundle\Entity\Core\Route;
 use Silverback\ApiComponentsBundle\Exception\InvalidArgumentException;
+use Silverback\ApiComponentsBundle\Exception\UnroutedParentException;
 use Silverback\ApiComponentsBundle\Helper\Timestamped\TimestampedDataPersister;
 use Silverback\ApiComponentsBundle\Repository\Core\RouteRepository;
 
@@ -57,6 +58,11 @@ public function createRedirect(string $fromPath, Route $targetRoute): Route
 
     public function create(RoutableInterface $object, ?Route $route = null): Route
     {
+        $parentPageRoute = $object->getParentPageRoute();
+        if (null === $parentPageRoute && (null !== $object->getParentPage() || null !== $object->getParentPageData())) {
+            throw new UnroutedParentException('Cannot generate a route for this page because its parent page has no route. Give the parent page a route first, or create this page\'s route explicitly.');
+        }
+
         $entityManager = $this->registry->getManagerForClass($className = $object::class);
         if (!$entityManager) {
             throw new InvalidArgumentException(\sprintf('Could not find entity manager for %s', $className));
@@ -75,7 +81,7 @@ public function create(RoutableInterface $object, ?Route $route = null): Route
 
         $path = '/' . ltrim($titleSlug, '/');
 
-        if ($parentPageRoute = $object->getParentPageRoute()) {
+        if ($parentPageRoute) {
             $path = '/' . ltrim($parentPageRoute->getPath(), '/') . $path;
         }
 
@@ -88,10 +94,6 @@ public function create(RoutableInterface $object, ?Route $route = null): Route
 
         if ($existingRoute) {
             $existingRoute->setRedirect($route);
-            // When we enabled patch endpoint for route, this was required.
-            // The existing route is found in uow, perhaps this is why..
-            // Future investigation would be nice to know reasoning for this breaking tests and pageData becoming null
-            // on the $route and staying on the existingRoute only when patch enabled.
             $route->setPage($existingRoute->getPage());
             $route->setpageData($existingRoute->getPageData());
         }

tests/DependencyInjection/SilverbackApiComponentsExtensionTest.php

@@ -24,6 +24,10 @@
 use Silverback\ApiComponentsBundle\Entity\Core\RoutableInterface;
 use Silverback\ApiComponentsBundle\Entity\Core\Route;
 use Silverback\ApiComponentsBundle\Entity\Core\SiteConfigParameter;
+use Silverback\ApiComponentsBundle\Exception\ApiPlatformAuthenticationException;
+use Silverback\ApiComponentsBundle\Exception\UnparseableRequestHeaderException;
+use Silverback\ApiComponentsBundle\Exception\UnroutedParentException;
+use Silverback\ApiComponentsBundle\Exception\UserDisabledException;
 use Silverback\ApiComponentsBundle\Factory\User\Mailer\ChangeEmailConfirmationEmailFactory;
 use Silverback\ApiComponentsBundle\Factory\User\Mailer\PasswordResetEmailFactory;
 use Silverback\ApiComponentsBundle\Factory\User\Mailer\VerifyEmailFactory;
@@ -87,6 +91,21 @@ private static function minimalConfig(): array
      * @return array{0: ContainerBuilder, 1: list<string>} the built container and any warnings or
      *                                                     notices raised while loading it
      */
+    public function test_bundle_exceptions_are_mapped_to_their_http_status(): void
+    {
+        $container = new ContainerBuilder();
+        $container->prependExtensionConfig('silverback_api_components', self::minimalConfig());
+
+        (new SilverbackApiComponentsExtension())->prepend($container);
+
+        $mapped = array_merge(...array_column($container->getExtensionConfig('api_platform'), 'exception_to_status'));
+
+        self::assertSame(400, $mapped[UnparseableRequestHeaderException::class]);
+        self::assertSame(401, $mapped[ApiPlatformAuthenticationException::class]);
+        self::assertSame(401, $mapped[UserDisabledException::class]);
+        self::assertSame(422, $mapped[UnroutedParentException::class]);
+    }
+
     private function load(array $config): array
     {
         $container = new ContainerBuilder();