Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Symfony v6 Controller getDoctrine() #94

Closed
94noni opened this issue Jan 4, 2022 · 10 comments · Fixed by #99
Closed

Symfony v6 Controller getDoctrine() #94

94noni opened this issue Jan 4, 2022 · 10 comments · Fixed by #99

Comments

@94noni
Copy link

94noni commented Jan 4, 2022

Hi

According to https://github.com/symfony/symfony/blob/6.0/UPGRADE-6.0.md, getDoctrine() is being deprecated
I have a lot of code $em = $this->getDoctrine()->getManager(); in my controllers, I would like to know if there is, or how to add, a rule to swap from this code to EntityManagerInterface $entityManager constructor injection or controller action injection

thank you

@TomasVotruba
Copy link
Member

Hi, could you share before and after example?

@94noni
Copy link
Author

94noni commented Jan 5, 2022

Hi thanks for reply
Yes I ll post back soon :)

@94noni
Copy link
Author

94noni commented Jan 5, 2022

@TomasVotruba here you are:

// src/Controller/SomeController.php

use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
+ use Doctrine\ORM\EntityManagerInterface;

 class SomeController extends AbstractController 
 {
-    public function index()
+    public function index(EntityManagerInterface $em)
     {
-         $em = $this->getDoctrine()->getManager();
           $em->getRepository(...);
     }
 }

$em injected because the var was named $em in the controller's action code

@TomasVotruba
Copy link
Member

The constructor injection would be better, but I get the idea. Thanks 👍

That's all methods or is there any other diff you can give me?

@94noni
Copy link
Author

94noni commented Jan 5, 2022

I've written example of controller action injection as this project is an evolution of an old symfony v2 app to v3,4,5 now 6
so old controller with lot of actions methods

On some new controllers, I prefer one invoke action with constructor injection that is why I showed an exemple of action injection, as I know rector can already do constructor injection

of course it could have already some arguments in the function so the EntityManagerInterface should be added after

for what I can read, there is also the dispatchMessage() of the same class from symfony :)

@TomasVotruba
Copy link
Member

so old controller with lot of actions methods

It might be handy to split such god class with custom simple Rector rule 👍

On some new controllers, I prefer one invoke action with constructor injection that is why I showed an exemple of action injection, as I know rector can already do constructor injection

The thing is, we have internal services for adding dependencies to constructor. We can re-use those easily. For any other modification, you can add custom rule to move your dependencies to places of need.

for what I can read, there is also the dispatchMessage() of the same class from symfony :)

Could you share diff for that one?

@94noni
Copy link
Author

94noni commented Jan 5, 2022

@TomasVotruba, for the latest part, kind of similar to the entity manager interface but for the messenger bus:

// src/Controller/SomeController.php

use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
+ use Symfony\Component\Messenger\MessageBusInterface;

 class SomeController extends AbstractController 
 {
-    public function index()
+    public function index(MessageBusInterface $bus)
     {
-        $this->dispatchMessage(...);
+        $bus->dispatch(...);
     }
 }

$bus is total arbitrary name, can be $messageBus or whatever


yes I need to improve this "old" code behavior split with more new logic/practice of coding

But I am new to rector I am testing step by step ;) and so far so good, this tool is a must have thank you so much

@TomasVotruba
Copy link
Member

But I am new to rector I am testing step by step ;) and so far so good, this tool is a must have thank you so much

That's the best approach. Start slowly then build up every day step by step :) thank you for positive feedback 👍


I'm looking into it... related pull-requests:

@TomasVotruba
Copy link
Member

Fixed in #97 ad #99

@94noni
Copy link
Author

94noni commented Jan 6, 2022

I will be pleased to test it live and report back, will check the next release :)
thank you @TomasVotruba

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants