feature #1328 [WCM] Deleting save and remove methods from reposit…

…ories (mdoutreluingne, weaverryan) This PR was merged into the 1.0-dev branch. Discussion ---------- [WCM] Deleting `save` and `remove` methods from repositories Fix #1327 In this PR I've removed the `save` and `remove` methods from the repositories, for the reasons explained here #1327. ( The add method has been renamed to save, here #1204 ) Reviews welcome! Commits ------- 6d4c62c Removed unecessary use statement dff2f97 Fixing look up of Symfony version + removing incorrect renderForm() on 2x assertion methods c3b506d Fix use statement sort 027b9f5 Add WithCustomRepositoryLegacy.php to include the `renderForm()` version 1650332 Replacement renderForm() method deprecated by render() method e919b78 Adding EntityManagerInterface use statement if not already added bbb7967 Adding EntityManagerInterface use statement 5038082 Deleting `add` and `remove` methods from repositories
symfony · Jul 10, 2023 · 9d0003b · 9d0003b
2 parents 5c47dab + 6d4c62c
commit 9d0003b
Show file tree

Hide file tree

Showing 12 changed files with 114 additions and 140 deletions.
diff --git a/src/Maker/MakeCrud.php b/src/Maker/MakeCrud.php
@@ -165,6 +165,10 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen
             Route::class,
         ]);
 
+        if (EntityManagerInterface::class !== $repositoryClassName) {
+            $useStatements->addUseStatement(EntityManagerInterface::class);
+        }
+
         $generator->generateController(
             $controllerClassDetails->getFullName(),
             'crud/controller/Controller.tpl.php',
@@ -258,6 +262,10 @@ public function generate(InputInterface $input, ConsoleStyle $io, Generator $gen
                 $useStatements->addUseStatement(EntityRepository::class);
             }
 
+            if (EntityManagerInterface::class !== $repositoryClassName) {
+                $useStatements->addUseStatement(EntityManagerInterface::class);
+            }
+
             $generator->generateFile(
                 'tests/Controller/'.$testClassDetails->getShortName().'.php',
                 $usesEntityManager ? 'crud/test/Test.EntityManager.tpl.php' : 'crud/test/Test.tpl.php',

diff --git a/src/Resources/skeleton/crud/controller/Controller.tpl.php b/src/Resources/skeleton/crud/controller/Controller.tpl.php
@@ -29,30 +29,18 @@ public function index(EntityManagerInterface $entityManager): Response
 <?php endif ?>
 
 <?= $generator->generateRouteForControllerMethod('/new', sprintf('%s_new', $route_name), ['GET', 'POST']) ?>
-<?php if (isset($repository_full_class_name) && $generator->repositoryHasSaveAndRemoveMethods($repository_full_class_name)) { ?>
-    public function new(Request $request, <?= $repository_class_name ?> $<?= $repository_var ?>): Response
-<?php } else { ?>
     public function new(Request $request, EntityManagerInterface $entityManager): Response
-<?php } ?>
     {
         $<?= $entity_var_singular ?> = new <?= $entity_class_name ?>();
         $form = $this->createForm(<?= $form_class_name ?>::class, $<?= $entity_var_singular ?>);
         $form->handleRequest($request);
 
-<?php if (isset($repository_full_class_name) && $generator->repositoryHasSaveAndRemoveMethods($repository_full_class_name)) { ?>
-        if ($form->isSubmitted() && $form->isValid()) {
-            $<?= $repository_var ?>->save($<?= $entity_var_singular ?>, true);
-
-            return $this->redirectToRoute('<?= $route_name ?>_index', [], Response::HTTP_SEE_OTHER);
-        }
-<?php } else { ?>
         if ($form->isSubmitted() && $form->isValid()) {
             $entityManager->persist($<?= $entity_var_singular ?>);
             $entityManager->flush();
 
             return $this->redirectToRoute('<?= $route_name ?>_index', [], Response::HTTP_SEE_OTHER);
         }
-<?php } ?>
 
 <?php if ($use_render_form) { ?>
         return $this->renderForm('<?= $templates_path ?>/new.html.twig', [
@@ -76,28 +64,16 @@ public function show(<?= $entity_class_name ?> $<?= $entity_var_singular ?>): Re
     }
 
 <?= $generator->generateRouteForControllerMethod(sprintf('/{%s}/edit', $entity_identifier), sprintf('%s_edit', $route_name), ['GET', 'POST']) ?>
-<?php if (isset($repository_full_class_name) && $generator->repositoryHasSaveAndRemoveMethods($repository_full_class_name)) { ?>
-    public function edit(Request $request, <?= $entity_class_name ?> $<?= $entity_var_singular ?>, <?= $repository_class_name ?> $<?= $repository_var ?>): Response
-<?php } else { ?>
     public function edit(Request $request, <?= $entity_class_name ?> $<?= $entity_var_singular ?>, EntityManagerInterface $entityManager): Response
-<?php } ?>
     {
         $form = $this->createForm(<?= $form_class_name ?>::class, $<?= $entity_var_singular ?>);
         $form->handleRequest($request);
 
-<?php if (isset($repository_full_class_name) && $generator->repositoryHasSaveAndRemoveMethods($repository_full_class_name)) { ?>
-        if ($form->isSubmitted() && $form->isValid()) {
-            $<?= $repository_var ?>->save($<?= $entity_var_singular ?>, true);
-
-            return $this->redirectToRoute('<?= $route_name ?>_index', [], Response::HTTP_SEE_OTHER);
-        }
-<?php } else { ?>
         if ($form->isSubmitted() && $form->isValid()) {
             $entityManager->flush();
 
             return $this->redirectToRoute('<?= $route_name ?>_index', [], Response::HTTP_SEE_OTHER);
         }
-<?php } ?>
 
 <?php if ($use_render_form) { ?>
         return $this->renderForm('<?= $templates_path ?>/edit.html.twig', [
@@ -113,22 +89,12 @@ public function edit(Request $request, <?= $entity_class_name ?> $<?= $entity_va
     }
 
 <?= $generator->generateRouteForControllerMethod(sprintf('/{%s}', $entity_identifier), sprintf('%s_delete', $route_name), ['POST']) ?>
-<?php if (isset($repository_full_class_name) && $generator->repositoryHasSaveAndRemoveMethods($repository_full_class_name)) { ?>
-    public function delete(Request $request, <?= $entity_class_name ?> $<?= $entity_var_singular ?>, <?= $repository_class_name ?> $<?= $repository_var ?>): Response
-<?php } else { ?>
     public function delete(Request $request, <?= $entity_class_name ?> $<?= $entity_var_singular ?>, EntityManagerInterface $entityManager): Response
-<?php } ?>
     {
-<?php if (isset($repository_full_class_name) && $generator->repositoryHasSaveAndRemoveMethods($repository_full_class_name)) { ?>
-        if ($this->isCsrfTokenValid('delete'.$<?= $entity_var_singular ?>->get<?= ucfirst($entity_identifier) ?>(), $request->request->get('_token'))) {
-            $<?= $repository_var ?>->remove($<?= $entity_var_singular ?>, true);
-        }
-<?php } else { ?>
         if ($this->isCsrfTokenValid('delete'.$<?= $entity_var_singular ?>->get<?= ucfirst($entity_identifier) ?>(), $request->request->get('_token'))) {
             $entityManager->remove($<?= $entity_var_singular ?>);
             $entityManager->flush();
         }
-<?php } ?>
 
         return $this->redirectToRoute('<?= $route_name ?>_index', [], Response::HTTP_SEE_OTHER);
     }

diff --git a/src/Resources/skeleton/crud/test/Test.EntityManager.tpl.php b/src/Resources/skeleton/crud/test/Test.EntityManager.tpl.php
@@ -62,7 +62,8 @@ public function testShow(): void
         $fixture->set<?= ucfirst($form_field); ?>('My Title');
 <?php endforeach; ?>
 
-        $this->repository->save($fixture, true);
+        $this->manager->persist($fixture);
+        $this->manager->flush();
 
         $this->client->request('GET', sprintf('%s%s', $this->path, $fixture->getId()));
 
@@ -108,7 +109,7 @@ public function testRemove(): void
         $fixture->set<?= ucfirst($form_field); ?>('Value');
 <?php endforeach; ?>
 
-        $$this->manager->remove($fixture);
+        $this->manager->remove($fixture);
         $this->manager->flush();
 
         $this->client->request('GET', sprintf('%s%s', $this->path, $fixture->getId()));

diff --git a/src/Resources/skeleton/crud/test/Test.tpl.php b/src/Resources/skeleton/crud/test/Test.tpl.php
@@ -10,14 +10,15 @@ class <?= $class_name ?> extends WebTestCase<?= "\n" ?>
     private KernelBrowser $client;
     private <?= "$repository_class_name " ?>$repository;
     private string $path = '<?= $route_path; ?>/';
+    private EntityManagerInterface $manager;
 
     protected function setUp(): void
     {
         $this->client = static::createClient();
         $this->repository = static::getContainer()->get('doctrine')->getRepository(<?= $entity_class_name; ?>::class);
 
         foreach ($this->repository->findAll() as $object) {
-            $this->repository->remove($object, true);
+            $this->manager->remove($object);
         }
     }
 
@@ -60,7 +61,8 @@ public function testShow(): void
         $fixture->set<?= ucfirst($form_field); ?>('My Title');
 <?php endforeach; ?>
 
-        $this->repository->save($fixture, true);
+        $this->manager->persist($fixture);
+        $this->manager->flush();
 
         $this->client->request('GET', sprintf('%s%s', $this->path, $fixture->getId()));
 
@@ -78,7 +80,8 @@ public function testEdit(): void
         $fixture->set<?= ucfirst($form_field); ?>('My Title');
 <?php endforeach; ?>
 
-        $this->repository->save($fixture, true);
+        $this->manager->persist($fixture);
+        $this->manager->flush();
 
         $this->client->request('GET', sprintf('%s%s/edit', $this->path, $fixture->getId()));
 
@@ -108,7 +111,8 @@ public function testRemove(): void
         $fixture->set<?= ucfirst($form_field); ?>('My Title');
 <?php endforeach; ?>
 
-        $this->repository->save($fixture, true);
+        $this->manager->persist($fixture);
+        $this->manager->flush();
 
         self::assertSame($originalNumObjectsInRepository + 1, count($this->repository->findAll()));
 

diff --git a/src/Resources/skeleton/doctrine/Repository.tpl.php b/src/Resources/skeleton/doctrine/Repository.tpl.php
@@ -18,26 +18,6 @@ public function __construct(ManagerRegistry $registry)
     {
         parent::__construct($registry, <?= $entity_class_name; ?>::class);
     }
-
-    public function save(<?= $entity_class_name ?> $entity, bool $flush = false): void
-    {
-        $entityManager = $this->getEntityManager();
-        $entityManager->persist($entity);
-
-        if ($flush) {
-            $entityManager->flush();
-        }
-    }
-
-    public function remove(<?= $entity_class_name ?> $entity, bool $flush = false): void
-    {
-        $entityManager = $this->getEntityManager();
-        $entityManager->remove($entity);
-
-        if ($flush) {
-            $entityManager->flush();
-        }
-    }
 <?php if ($include_example_comments): // When adding a new method without existing default comments, the blank line is automatically added.?>
 
 <?php endif; ?>
@@ -52,8 +32,8 @@ public function upgradePassword(<?= sprintf('%s ', $password_upgrade_user_interf
         }
 
         $user->setPassword($newHashedPassword);
-
-        $this->save($user, true);
+        $this->getEntityManager()->persist($user);
+        $this->getEntityManager()->flush();
     }
 
 <?php endif ?>

diff --git a/src/Util/TemplateComponentGenerator.php b/src/Util/TemplateComponentGenerator.php
@@ -43,14 +43,4 @@ public function getPropertyType(ClassNameDetails $classNameDetails): ?string
     {
         return sprintf('%s ', $classNameDetails->getShortName());
     }
-
-    /**
-     * @throws \ReflectionException
-     */
-    public function repositoryHasSaveAndRemoveMethods(string $repositoryFullClassName): bool
-    {
-        $reflectedComponents = new \ReflectionClass($repositoryFullClassName);
-
-        return $reflectedComponents->hasMethod('save') && $reflectedComponents->hasMethod('remove');
-    }
 }
diff --git a/tests/Doctrine/fixtures/expected_xml/src/Repository/UserRepository.php b/tests/Doctrine/fixtures/expected_xml/src/Repository/UserRepository.php
@@ -21,26 +21,6 @@ public function __construct(ManagerRegistry $registry)
         parent::__construct($registry, UserXml::class);
     }
 
-    public function save(UserXml $entity, bool $flush = false): void
-    {
-        $entityManager = $this->getEntityManager();
-        $entityManager->persist($entity);
-
-        if ($flush) {
-            $entityManager->flush();
-        }
-    }
-
-    public function remove(UserXml $entity, bool $flush = false): void
-    {
-        $entityManager = $this->getEntityManager();
-        $entityManager->remove($entity);
-
-        if ($flush) {
-            $entityManager->flush();
-        }
-    }
-
 //    /**
 //     * @return UserXml[] Returns an array of UserXml objects
 //     */

diff --git a/tests/Doctrine/fixtures/expected_xml/src/Repository/XOtherRepository.php b/tests/Doctrine/fixtures/expected_xml/src/Repository/XOtherRepository.php
@@ -21,26 +21,6 @@ public function __construct(ManagerRegistry $registry)
         parent::__construct($registry, XOther::class);
     }
 
-    public function save(XOther $entity, bool $flush = false): void
-    {
-        $entityManager = $this->getEntityManager();
-        $entityManager->persist($entity);
-
-        if ($flush) {
-            $entityManager->flush();
-        }
-    }
-
-    public function remove(XOther $entity, bool $flush = false): void
-    {
-        $entityManager = $this->getEntityManager();
-        $entityManager->remove($entity);
-
-        if ($flush) {
-            $entityManager->flush();
-        }
-    }
-
 //    /**
 //     * @return XOther[] Returns an array of XOther objects
 //     */

diff --git a/tests/Maker/MakeCrudTest.php b/tests/Maker/MakeCrudTest.php
@@ -175,9 +175,8 @@ public function getTestDetails(): \Generator
                 $this->assertStringContainsString('src/Form/SweetFoodType.php', $output);
 
                 $this->runCrudTest($runner, 'it_generates_basic_crud.php');
-
                 self::assertFileEquals(
-                    sprintf('%s/fixtures/%s', \dirname(__DIR__), 'make-crud/expected/WithCustomRepository.php'),
+                    sprintf('%s/fixtures/make-crud/expected/WithCustomRepository%s.php', \dirname(__DIR__), $runner->getSymfonyVersion() < 60200 ? 'Legacy' : ''),
                     $runner->getPath('src/Controller/SweetFoodController.php')
                 );
             }),

diff --git a/tests/fixtures/make-crud/SweetFoodRepository.php b/tests/fixtures/make-crud/SweetFoodRepository.php
@@ -20,22 +20,4 @@ public function __construct(ManagerRegistry $registry)
     {
         parent::__construct($registry, SweetFood::class);
     }
-
-    public function save(SweetFood $entity, bool $flush = false): void
-    {
-        ($em = $this->getEntityManager())->persist($entity);
-
-        if ($flush) {
-            $em->flush();
-        }
-    }
-
-    public function remove(SweetFood $entity, bool $flush = false): void
-    {
-        ($em = $this->getEntityManager())->remove($entity);
-
-        if ($flush) {
-            $em->flush();
-        }
-    }
 }
diff --git a/tests/fixtures/make-crud/expected/WithCustomRepository.php b/tests/fixtures/make-crud/expected/WithCustomRepository.php
@@ -5,6 +5,7 @@
 use App\Entity\SweetFood;
 use App\Form\SweetFoodType;
 use App\Repository\SweetFoodRepository;
+use Doctrine\ORM\EntityManagerInterface;
 use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
@@ -22,19 +23,20 @@ public function index(SweetFoodRepository $sweetFoodRepository): Response
     }
 
     #[Route('/new', name: 'app_sweet_food_new', methods: ['GET', 'POST'])]
-    public function new(Request $request, SweetFoodRepository $sweetFoodRepository): Response
+    public function new(Request $request, EntityManagerInterface $entityManager): Response
     {
         $sweetFood = new SweetFood();
         $form = $this->createForm(SweetFoodType::class, $sweetFood);
         $form->handleRequest($request);
 
         if ($form->isSubmitted() && $form->isValid()) {
-            $sweetFoodRepository->save($sweetFood, true);
+            $entityManager->persist($sweetFood);
+            $entityManager->flush();
 
             return $this->redirectToRoute('app_sweet_food_index', [], Response::HTTP_SEE_OTHER);
         }
 
-        return $this->renderForm('sweet_food/new.html.twig', [
+        return $this->render('sweet_food/new.html.twig', [
             'sweet_food' => $sweetFood,
             'form' => $form,
         ]);
@@ -49,28 +51,29 @@ public function show(SweetFood $sweetFood): Response
     }
 
     #[Route('/{id}/edit', name: 'app_sweet_food_edit', methods: ['GET', 'POST'])]
-    public function edit(Request $request, SweetFood $sweetFood, SweetFoodRepository $sweetFoodRepository): Response
+    public function edit(Request $request, SweetFood $sweetFood, EntityManagerInterface $entityManager): Response
     {
         $form = $this->createForm(SweetFoodType::class, $sweetFood);
         $form->handleRequest($request);
 
         if ($form->isSubmitted() && $form->isValid()) {
-            $sweetFoodRepository->save($sweetFood, true);
+            $entityManager->flush();
 
             return $this->redirectToRoute('app_sweet_food_index', [], Response::HTTP_SEE_OTHER);
         }
 
-        return $this->renderForm('sweet_food/edit.html.twig', [
+        return $this->render('sweet_food/edit.html.twig', [
             'sweet_food' => $sweetFood,
             'form' => $form,
         ]);
     }
 
     #[Route('/{id}', name: 'app_sweet_food_delete', methods: ['POST'])]
-    public function delete(Request $request, SweetFood $sweetFood, SweetFoodRepository $sweetFoodRepository): Response
+    public function delete(Request $request, SweetFood $sweetFood, EntityManagerInterface $entityManager): Response
     {
         if ($this->isCsrfTokenValid('delete'.$sweetFood->getId(), $request->request->get('_token'))) {
-            $sweetFoodRepository->remove($sweetFood, true);
+            $entityManager->remove($sweetFood);
+            $entityManager->flush();
         }
 
         return $this->redirectToRoute('app_sweet_food_index', [], Response::HTTP_SEE_OTHER);