Skip to content

Commit 2d02ee1

Browse files
ljonesflljonesfl
committed
fixes
1 parent 804fc2e commit 2d02ee1

3 files changed

Lines changed: 149 additions & 28 deletions

File tree

‎src/Routing/RouteMap.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,12 @@ class RouteMap
4646
public string $Name;
4747
public array $Payload;
4848

49+
/** @var Router|null Reference to router for name validation */
50+
private ?Router $_router = null;
51+
52+
/** @var string|null HTTP method for this route (GET, POST, etc.) */
53+
private ?string $_method = null;
54+
4955
/**
5056
* RouteMap constructor.
5157
* @param $path string route path i.e. /part/new or /part/:id
@@ -79,6 +85,19 @@ public function __construct( string $path, callable $function, string|array $fil
7985
$this->Payload = [];
8086
}
8187

88+
/**
89+
* Set the router and method for this route (used for name duplicate detection)
90+
* @param Router $router
91+
* @param string $method HTTP method (GET, POST, PUT, DELETE)
92+
* @return RouteMap
93+
*/
94+
public function setRouterContext( Router $router, string $method ): RouteMap
95+
{
96+
$this->_router = $router;
97+
$this->_method = $method;
98+
return $this;
99+
}
100+
82101
/**
83102
* @return string
84103
*/
@@ -190,11 +209,20 @@ public function getName()
190209
}
191210

192211
/**
212+
* Set the name for this route and register it with the router for duplicate detection.
213+
*
193214
* @param mixed $name
194215
* @return RouteMap
216+
* @throws Exceptions\DuplicateRouteException If name is already in use
195217
*/
196218
public function setName( $name ) : RouteMap
197219
{
220+
// If router context is set, register the name for duplicate detection
221+
if( $this->_router && $this->_method )
222+
{
223+
$this->_router->registerRouteName( $name, $this->_method, $this->Path, $this );
224+
}
225+
198226
$this->Name = $name;
199227
return $this;
200228
}

‎src/Routing/Router.php‎

Lines changed: 52 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -230,15 +230,18 @@ protected function addRoute( array &$routes, string $method, string $routeName,
230230
$routeName = substr( $routeName, 0, -1 );
231231
}
232232

233-
$route = new RouteMap( $routeName, $function, $filters ?? '' );
233+
$route = new RouteMap( $routeName, $function, $filters ?? '' );
234234

235-
// Set the route name BEFORE checking for duplicates
235+
// Set router context so the route can register names for duplicate detection
236+
$route->setRouterContext( $this, $method );
237+
238+
// Set the route name if provided (this will trigger duplicate name check)
236239
if( $name )
237240
{
238241
$route->setName( $name );
239242
}
240243

241-
// Check for duplicate routes if strict mode is enabled
244+
// Check for duplicate path+method combinations if strict mode is enabled
242245
if( $this->_strictMode )
243246
{
244247
$this->checkDuplicateRoute( $route, $method );
@@ -249,6 +252,51 @@ protected function addRoute( array &$routes, string $method, string $routeName,
249252
return $route;
250253
}
251254

255+
/**
256+
* Register a route name and check for duplicates (called by RouteMap::setName).
257+
*
258+
* This method is called when a route name is set via the fluent API,
259+
* allowing duplicate name detection to work properly even when names
260+
* are set after route registration.
261+
*
262+
* @param string $name The route name to register
263+
* @param string $method The HTTP method (GET, POST, PUT, DELETE)
264+
* @param string $path The route path
265+
* @param RouteMap $route The route being named
266+
* @return void
267+
* @throws Exceptions\DuplicateRouteException If name is already in use
268+
*/
269+
public function registerRouteName( string $name, string $method, string $path, RouteMap $route ): void
270+
{
271+
// Only check for duplicates if strict mode is enabled
272+
if( !$this->_strictMode )
273+
{
274+
$this->_registeredNames[ $name ] = "{$method}:{$path}";
275+
return;
276+
}
277+
278+
// Check if this name is already registered
279+
if( isset( $this->_registeredNames[ $name ] ) )
280+
{
281+
// Parse the original route's signature to get method and path
282+
$originalSignature = $this->_registeredNames[ $name ];
283+
list( $originalMethod, $originalPath ) = explode( ':', $originalSignature, 2 );
284+
285+
throw new Exceptions\DuplicateRouteException(
286+
$originalMethod, // First route method
287+
$originalPath, // First route path
288+
$this->_registeredRoutes[ $originalSignature ] ?? 'unknown controller',
289+
$method, // Second route method
290+
$path, // Second route path
291+
$this->extractControllerInfo( $route ),
292+
$name
293+
);
294+
}
295+
296+
// Register the name
297+
$this->_registeredNames[ $name ] = "{$method}:{$path}";
298+
}
299+
252300
/**
253301
* Check if a route is a duplicate and throw exception if found.
254302
*
@@ -277,32 +325,8 @@ protected function checkDuplicateRoute( RouteMap $route, string $method ): void
277325
);
278326
}
279327

280-
// Check for duplicate route name
281-
$name = $route->getName();
282-
if( $name && isset( $this->_registeredNames[ $name ] ) )
283-
{
284-
// Parse the original route's signature to get method and path
285-
$originalSignature = $this->_registeredNames[ $name ];
286-
list( $originalMethod, $originalPath ) = explode( ':', $originalSignature, 2 );
287-
288-
throw new Exceptions\DuplicateRouteException(
289-
$originalMethod, // First route method
290-
$originalPath, // First route path
291-
$this->_registeredRoutes[ $originalSignature ],
292-
$method, // Second route method
293-
$path, // Second route path
294-
$this->extractControllerInfo( $route ),
295-
$name
296-
);
297-
}
298-
299-
// Register this route
328+
// Register this route (name registration is handled separately by registerRouteName)
300329
$this->_registeredRoutes[ $signature ] = $this->extractControllerInfo( $route );
301-
302-
if( $name )
303-
{
304-
$this->_registeredNames[ $name ] = $signature;
305-
}
306330
}
307331

308332
/**

‎tests/unit/RouterTest.php‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -643,4 +643,73 @@ public function testGenerateUrlNotFound()
643643
$url = $this->Router->generateUrl( 'nonexistent', [] );
644644
$this->assertNull( $url );
645645
}
646+
647+
public function testDuplicateRoutePath()
648+
{
649+
$this->expectException( Routing\Exceptions\DuplicateRouteException::class );
650+
651+
// First route
652+
$this->Router->get( '/users/:id', function() { return 'first'; } );
653+
654+
// Second route with same path and method
655+
$this->Router->get( '/users/:id', function() { return 'second'; } );
656+
}
657+
658+
public function testDuplicateRouteNameViaFluentApi()
659+
{
660+
$this->expectException( Routing\Exceptions\DuplicateRouteException::class );
661+
$this->expectExceptionMessage( 'user.show' );
662+
663+
// First route with name
664+
$this->Router->get( '/users/:id', function() { return 'first'; } )
665+
->setName( 'user.show' );
666+
667+
// Second route with same name (via fluent API)
668+
$this->Router->get( '/users/:slug', function() { return 'second'; } )
669+
->setName( 'user.show' );
670+
}
671+
672+
public function testDuplicateRouteNameViaParameter()
673+
{
674+
$this->expectException( Routing\Exceptions\DuplicateRouteException::class );
675+
$this->expectExceptionMessage( 'user.show' );
676+
677+
// First route with name via parameter
678+
$this->Router->get( '/users/:id', function() { return 'first'; }, null, 'user.show' );
679+
680+
// Second route with same name via parameter
681+
$this->Router->get( '/users/:slug', function() { return 'second'; }, null, 'user.show' );
682+
}
683+
684+
public function testDuplicateRouteNameAcrossMethods()
685+
{
686+
$this->expectException( Routing\Exceptions\DuplicateRouteException::class );
687+
$this->expectExceptionMessage( 'user.action' );
688+
689+
// First route: GET /users
690+
$this->Router->get( '/users', function() { return 'get'; } )
691+
->setName( 'user.action' );
692+
693+
// Second route: POST /users with same name
694+
$this->Router->post( '/users', function() { return 'post'; } )
695+
->setName( 'user.action' );
696+
}
697+
698+
public function testStrictModeDisabled()
699+
{
700+
// Disable strict mode
701+
$this->Router->setStrictMode( false );
702+
703+
// These should NOT throw exceptions now
704+
$this->Router->get( '/users/:id', function() { return 'first'; } );
705+
$this->Router->get( '/users/:id', function() { return 'second'; } );
706+
707+
$this->Router->get( '/posts/:id', function() { return 'first'; } )
708+
->setName( 'post.show' );
709+
$this->Router->get( '/posts/:slug', function() { return 'second'; } )
710+
->setName( 'post.show' );
711+
712+
// Just verify no exception was thrown
713+
$this->assertTrue( true );
714+
}
646715
}

0 commit comments

Comments
 (0)