Skip to content

Conversation

@Oviglo
Copy link
Contributor

@Oviglo Oviglo commented Apr 11, 2025

Q A
Branch? 7.3
Bug fix? no
New feature? yes
Deprecations? no
Issues
License MIT

It could be useful to add 'override' property into Route attribute. In this example, I need to override 'add' function but by default, the trait's route replace the controller's route.

trait CrudTrait
{
    #[Route('/add', name: 'add', methods: ['GET', 'POST'], override: false)]
    public function add(Request $request)
    {
        // ... Logic to create a new entity
    }
}
class PostController
{
    use CrudTrait {add as crudAdd;}

    #[Route('/add', name: 'add', methods: ['GET', 'POST'])]
    public function add(Request $request, TranslatorInterface $trans)
    {
        // Custom logic for creating a new post entity
        // This will override the method from the CrudTrait
    }
}

Maybe default instead of override ?

@carsonbot
Copy link

Hey!

To help keep things organized, we don't allow "Draft" pull requests. Could you please click the "ready for review" button or close this PR and open a new one when you are done?

Note that a pull request does not have to be "perfect" or "ready for merge" when you first open it. We just want it to be ready for a first review.

Cheers!

Carsonbot

@Oviglo Oviglo marked this pull request as ready for review April 11, 2025 08:25
@carsonbot carsonbot added this to the 7.3 milestone Apr 11, 2025
@carsonbot carsonbot changed the title [WIP][Routing] New override property in Route attribute [Routing] [WIP] New override property in Route attribute Apr 11, 2025
@stof
Copy link
Member

stof commented Apr 11, 2025

Why are you exposing the add method of the trait as a public crudAdd instead of just overriding it in your class (if you need to be able to call it from your custom add, you could use as private crudAdd instead).

the issue is that your class actually looks like that right now at runtime (with an unknown order of functions as I don't know how PHP orders functions copied from traits):

class PostController
{
    #[Route('/add', name: 'add', methods: ['GET', 'POST'])]
    public function add(Request $request, TranslatorInterface $trans)
    {
        // Custom logic for creating a new post entity
        // This will override the method from the CrudTrait
    }

    #[Route('/add', name: 'add', methods: ['GET', 'POST'])]
    public function crudAdd(Request $request)
    {
        // ... Logic to create a new entity
    }
}

And so you have 2 routes with the same name defined in that class.

@GromNaN
Copy link
Member

GromNaN commented Apr 12, 2025

The solution would be to import the method as private or protected, as proposed by @stof. But that's not enough, as the route is registered even if the method is not public, and an exception is thrown by the ControllerResolver when the route is requested.
This bebavior could be changed: #60203

@GromNaN
Copy link
Member

GromNaN commented Apr 14, 2025

On second thought, I think you'd be better off rethinking the architecture of your code rather than complicating the framework to meet your needs. We already have a priority feature but it doesn't work with same-name routes.

You can move some of the add method logic into a private method in the trait (or create a service). Then redeclare the add method in your final class without retrieving the add method from the trait.

Or go the other way if the controller signature doesn't need to be modified: from add, call a private method in the trait, which you redefine in your final class.

@nicolas-grekas
Copy link
Member

👎 as discussed (thanks @stof and @GromNaN for digging what we could do).

@Oviglo
Copy link
Contributor Author

Oviglo commented Apr 14, 2025

Thanks all for your comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants