[WIP][Routing] Add support for 'priority' option for annotated Routes #11747
Conversation
|
Btw, we added this only for annotations, because I think that's the only real use-case where this is important. I thought this would be initially done in SensioFrameworkExtraBundle since my use-case is to make life easier there, but the code ended up living in core :). Thanks! |
|
I don't think the implementation actually works. |
|
This would also require a lot of tests. For example that the routes without priority actually don't change order. |
|
I implemented this first in one of my personal project and checked there the behaviour, and it change the order inside the collection. The problem here, that the code is in sf, but the annotations are in the frameworkextra-bundle. I'm opened to any suggestion, how to test. Or you mean just simply create routes and pass them to the load? |
| @@ -104,6 +119,8 @@ public function setRouteAnnotationClass($class) | |||
| */ | |||
| public function load($class, $type = null) | |||
| { | |||
| $hasPriority = false; | |||
|
I will extract the sorting to a separate function so that it can be tested, cause at the moment I don't see any other option for that. |
| @@ -123,6 +140,9 @@ public function load($class, $type = null) | |||
| foreach ($this->reader->getMethodAnnotations($method) as $annot) { | |||
| if ($annot instanceof $this->routeAnnotationClass) { | |||
| $this->addRoute($collection, $annot, $globals, $class, $method); | |||
| if (array_key_exists('priority',$annot->getOptions())) { | |||
| $this->routeWithPriority = true; | |||
There was a problem hiding this comment.
The loop can be left then since all following routes wouldn't change the routeWithPriority property any more.
There was a problem hiding this comment.
you mean check if routeWithPriority still false?
There was a problem hiding this comment.
Never mind, I didn't see the call to addRoute() first.
| @@ -58,6 +59,15 @@ public function load($path, $type = null) | |||
| } | |||
| } | |||
|
|
|||
| if ($this->loader->hasRouteWithPriority()) { | |||
| $collection->getIterator()->uasort(function(Route $a, Route $b) { | |||
If the given route option parameter has a priority key, it will be sorted. The use case is described in the linked ticket.
If the changes are acceptable, then I will create the changes also for the Doc