TASK: Fix constraints for #[Flow\Route] annotation

Neos.Flow/Classes/Annotations/Route.php

@@ -30,22 +30,38 @@
 final class Route
 {
     /**
-     * @param string $uriPattern The uri-pattern for the route without leading '/'. Must not contain `{@action}` or `{@controller}`.
-     * @param string|null $name (default null) The name ouf the route as it shows up in the route:list command
-     * @param array $httpMethods (default []) List of http verbs like 'GET', 'POST', 'PUT', 'DELETE', if not specified 'any' is used
-     * @param array $defaults (default []) Values to set for this route. Dan define arguments but also specify the `@format` if required.
+     * Magic route values cannot be set as default nor be contained as segments like `{\@action}` or `{\@controller}` in the uriPattern.
+     * The magic route value `\@format` is allowed if necessary.
+     */
+    private const PRESERVED_DEFAULTS = ['@package', '@subpackage', '@controller', '@action'];
+
+    /**
+     * @param string $uriPattern The uri-pattern for the route without leading '/'.
+     * @param string $name (default null) The name ouf the route as it shows up in the route:list command
+     * @param array $httpMethods (default []) List of http verbs like 'GET', 'POST', 'PUT', 'DELETE', if not specified any will be matched
+     * @param array $defaults (default []) Values to set for this route.
      */
     public function __construct(
         public readonly string $uriPattern,
-        public readonly ?string $name = null,
+        public readonly string $name = '',
         public readonly array $httpMethods = [],
         public readonly array $defaults = [],
     ) {
-        if (str_contains($uriPattern, '{@controller}') || str_contains($uriPattern, '{@action}')) {
-            throw new \DomainException(sprintf('It is not allowed to override {@controller} or {@action} in route annotations "%s"', $uriPattern), 1711129634);
+        if ($uriPattern === '' || str_starts_with($uriPattern, '/')) {
+            throw new \DomainException(sprintf('Uri pattern must not be empty or begin with a slash: "%s"', $uriPattern), 1711529592);
+        }
+        foreach ($httpMethods as $httpMethod) {
+            if ($httpMethod === '' || ctype_lower($httpMethod)) {
+                throw new \DomainException(sprintf('Http method must not be empty or be lower case: "%s"', $httpMethod), 1711530485);
+            }
         }
-        if (in_array(array_keys($defaults), ['@package', '@subpackage', '@controller', '@action'])) {
-            throw new \DomainException(sprintf('It is not allowed to override @package, @controller, @subpackage and @action in route annotation defaults "%s"', $uriPattern), 1711129638);
+        foreach (self::PRESERVED_DEFAULTS as $preservedDefaultName) {
+            if (str_contains($uriPattern, sprintf('{%s}', $preservedDefaultName))) {
+                throw new \DomainException(sprintf('It is not allowed to use "%s" in the uri pattern "%s"', $preservedDefaultName, $uriPattern), 1711129634);
+            }
+            if (array_key_exists($preservedDefaultName, $defaults)) {
+                throw new \DomainException(sprintf('It is not allowed to override "%s" as default', $preservedDefaultName), 1711129638);
+            }
         }
     }
 }

Neos.Flow/Tests/Unit/Mvc/Routing/AttributeRoutesProviderTest.php

@@ -76,7 +76,12 @@ public function routesFromAnnotationAreCreatedWhenClassNamesMatch(): void
             ->with('Vendor\Example\Controller\ExampleController', 'specialAction', Flow\Route::class)
             ->willReturn([
                 new Flow\Route(uriPattern: 'my/path'),
-                new Flow\Route(uriPattern: 'my/other/path', name: 'specialRoute', defaults: ['test' => 'foo'], httpMethods: ['get', 'post'])
+                new Flow\Route(
+                    uriPattern: 'my/other/path',
+                    name: 'specialRoute',
+                    httpMethods: ['GET', 'POST'],
+                    defaults: ['test' => 'foo']
+                )
             ]);
 
         $this->mockObjectManager->expects($this->once())
@@ -111,7 +116,7 @@ public function routesFromAnnotationAreCreatedWhenClassNamesMatch(): void
             '@format' => 'html',
             'test' => 'foo',
         ]);
-        $expectedRoute2->setHttpMethods(['get', 'post']);
+        $expectedRoute2->setHttpMethods(['GET', 'POST']);
 
         $this->assertEquals(
             Routes::create($expectedRoute1, $expectedRoute2),

Neos.Flow/Tests/Unit/Mvc/Routing/Dto/RouteAnnotationTest.php

@@ -35,19 +35,6 @@ public function simpleRoutes()
         self::assertSame(['test' => 'foo'], $route->defaults);
     }
 
-    /**
-     * @test
-     */
-    public function httpMethodIsUpperCase()
-    {
-        /** @see \Neos\Flow\Mvc\Routing\Route::matches() where we do case-sensitive comparison */
-        $route = new Flow\Route(
-            uriPattern: 'my/other/path',
-            httpMethods: ['post']
-        );
-        self::assertSame(['POST'], $route->httpMethods);
-    }
-
     /**
      * @test
      */
@@ -83,7 +70,7 @@ public function uriPatternMustNotStartWithLeadingSlash()
      */
     public function uriPatternMustNotBeEmpty()
     {
-        $this->expectExceptionCode(1711529595);
+        $this->expectExceptionCode(1711529592);
 
         new Flow\Route(uriPattern: '');
     }
@@ -97,4 +84,18 @@ public function httpMethodMustNotBeEmptyString()
 
         new Flow\Route(uriPattern: 'foo', httpMethods: ['']);
     }
+
+    /**
+     * @test
+     */
+    public function httpMethodMustBeUpperCase()
+    {
+        $this->expectExceptionCode(1711530485);
+
+        /** @see \Neos\Flow\Mvc\Routing\Route::matches() where we do case-sensitive comparison against uppercase */
+        new Flow\Route(
+            uriPattern: 'my/other/path',
+            httpMethods: ['post']
+        );
+    }
 }