From aa0cadfa84c4c192889f79980cba6c54d7f99e85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Thu, 8 Feb 2024 11:40:48 +0100 Subject: [PATCH] Add conflict resolution for generated API + add test Implemented additional code to handle version conflicts when editing accompanying period work. By keeping track of the current version and returning an HTTP conflict response when it doesn't match with the provided entity version, users are properly alerted to update their entity before continuing. Furthermore, adjusted BadRequestHttpException to match correct arguments order and introduced entity version as query parameter for the URL. ensure kernel is shutdown after generating data --- .../Controller/AbstractCRUDController.php | 19 +++ .../CRUD/Controller/ApiController.php | 4 +- .../AccompanyingCourseWorkApiController.php | 28 +--- .../vuejs/AccompanyingCourseWorkEdit/store.js | 3 +- .../ConflictTest.php | 157 ++++++++++++++++++ 5 files changed, 183 insertions(+), 28 deletions(-) create mode 100644 src/Bundle/ChillPersonBundle/Tests/Controller/AccompanyingCoursWorkApiController/ConflictTest.php diff --git a/src/Bundle/ChillMainBundle/CRUD/Controller/AbstractCRUDController.php b/src/Bundle/ChillMainBundle/CRUD/Controller/AbstractCRUDController.php index f3982d0d5..9ee0d9e19 100644 --- a/src/Bundle/ChillMainBundle/CRUD/Controller/AbstractCRUDController.php +++ b/src/Bundle/ChillMainBundle/CRUD/Controller/AbstractCRUDController.php @@ -15,10 +15,14 @@ use Chill\MainBundle\CRUD\Resolver\Resolver; use Chill\MainBundle\Pagination\PaginatorFactory; use Chill\MainBundle\Pagination\PaginatorInterface; use Chill\MainBundle\Security\Authorization\AuthorizationHelper; +use Doctrine\DBAL\LockMode; +use Doctrine\ORM\EntityManagerInterface; +use Doctrine\ORM\OptimisticLockException; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpKernel\Exception\ConflictHttpException; use Symfony\Component\Serializer\SerializerInterface; use Symfony\Component\Validator\Validator\ValidatorInterface; use Symfony\Contracts\Translation\TranslatorInterface; @@ -173,6 +177,21 @@ abstract class AbstractCRUDController extends AbstractController if (null === $e) { throw $this->createNotFoundException(sprintf('The object %s for id %s is not found', $this->getEntityClass(), $id)); } + if ($request->query->has('entity_version')) { + $expectedVersion = $request->query->getInt('entity_version'); + + try { + $manager = $this->getDoctrine()->getManagerForClass($this->getEntityClass()); + + if ($manager instanceof EntityManagerInterface) { + $manager->lock($e, LockMode::OPTIMISTIC, $expectedVersion); + } else { + throw new \LogicException('This manager does not allow locking.'); + } + } catch (OptimisticLockException $e) { + throw new ConflictHttpException('Sorry, but someone else has already changed this entity. Please refresh the page and apply the changes again', $e); + } + } return $e; } diff --git a/src/Bundle/ChillMainBundle/CRUD/Controller/ApiController.php b/src/Bundle/ChillMainBundle/CRUD/Controller/ApiController.php index 78bdc96d5..a6675424e 100644 --- a/src/Bundle/ChillMainBundle/CRUD/Controller/ApiController.php +++ b/src/Bundle/ChillMainBundle/CRUD/Controller/ApiController.php @@ -135,7 +135,7 @@ class ApiController extends AbstractCRUDController try { $entity = $this->deserialize($action, $request, $_format, $entity); } catch (NotEncodableValueException $e) { - throw new BadRequestHttpException('invalid json', 400, $e); + throw new BadRequestHttpException('invalid json', $e, 400); } $errors = $this->validate($action, $request, $_format, $entity); @@ -153,7 +153,7 @@ class ApiController extends AbstractCRUDController return $response; } - $this->getDoctrine()->getManager()->flush(); + $this->getDoctrine()->getManagerForClass($this->getEntityClass())->flush(); $response = $this->onAfterFlush($action, $request, $_format, $entity, $errors); diff --git a/src/Bundle/ChillPersonBundle/Controller/AccompanyingCourseWorkApiController.php b/src/Bundle/ChillPersonBundle/Controller/AccompanyingCourseWorkApiController.php index 00d2f9026..47e46cb6b 100644 --- a/src/Bundle/ChillPersonBundle/Controller/AccompanyingCourseWorkApiController.php +++ b/src/Bundle/ChillPersonBundle/Controller/AccompanyingCourseWorkApiController.php @@ -15,23 +15,15 @@ use Chill\MainBundle\CRUD\Controller\ApiController; use Chill\MainBundle\Serializer\Model\Collection; use Chill\MainBundle\Serializer\Model\Counter; use Chill\PersonBundle\Repository\AccompanyingPeriod\AccompanyingPeriodWorkRepository; -use Doctrine\DBAL\LockMode; -use Doctrine\ORM\EntityManagerInterface; -use Doctrine\ORM\OptimisticLockException; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\HttpKernel\Exception\ConflictHttpException; use Symfony\Component\Routing\Annotation\Route; -use Symfony\Contracts\Translation\TranslatorInterface; class AccompanyingCourseWorkApiController extends ApiController { - public function __construct( - private readonly AccompanyingPeriodWorkRepository $accompanyingPeriodWorkRepository, - private readonly EntityManagerInterface $em, - private readonly TranslatorInterface $translator, - ) {} + public function __construct(private readonly AccompanyingPeriodWorkRepository $accompanyingPeriodWorkRepository) + { + } /** * @Route("/api/1.0/person/accompanying-period/work/my-near-end") @@ -75,18 +67,4 @@ class AccompanyingCourseWorkApiController extends ApiController return parent::getContextForSerialization($action, $request, $_format, $entity); } - - public function entityPut($action, Request $request, $id, string $_format): Response - { - $entity = $this->accompanyingPeriodWorkRepository->findBy(['id' => $id])[0]; - $expectedVersion = $entity->getVersion(); - - try { - $this->em->lock($entity, LockMode::OPTIMISTIC, $expectedVersion); - - return parent::entityPut($action, $request, $id, $_format); - } catch (OptimisticLockException) { - throw new ConflictHttpException($this->translator->trans('Sorry, but someone else has already changed this entity. Please refresh the page and apply the changes again')); - } - } } diff --git a/src/Bundle/ChillPersonBundle/Resources/public/vuejs/AccompanyingCourseWorkEdit/store.js b/src/Bundle/ChillPersonBundle/Resources/public/vuejs/AccompanyingCourseWorkEdit/store.js index bec89d088..44be6d5f2 100644 --- a/src/Bundle/ChillPersonBundle/Resources/public/vuejs/AccompanyingCourseWorkEdit/store.js +++ b/src/Bundle/ChillPersonBundle/Resources/public/vuejs/AccompanyingCourseWorkEdit/store.js @@ -503,7 +503,8 @@ const store = createStore({ submit({ getters, state, commit }, callback) { let payload = getters.buildPayload, - url = `/api/1.0/person/accompanying-course/work/${state.work.id}.json`, + params = new URLSearchParams({'entity_version': state.version}), + url = `/api/1.0/person/accompanying-course/work/${state.work.id}.json?${params}`, errors = [] ; diff --git a/src/Bundle/ChillPersonBundle/Tests/Controller/AccompanyingCoursWorkApiController/ConflictTest.php b/src/Bundle/ChillPersonBundle/Tests/Controller/AccompanyingCoursWorkApiController/ConflictTest.php new file mode 100644 index 000000000..b16560831 --- /dev/null +++ b/src/Bundle/ChillPersonBundle/Tests/Controller/AccompanyingCoursWorkApiController/ConflictTest.php @@ -0,0 +1,157 @@ +getClientAuthenticated(); + + $currentVersion = $work->getVersion(); + + $client->request( + 'PUT', + "/api/1.0/person/accompanying-course/work/{$work->getid()}.json?entity_version={$currentVersion}", + content: json_encode([ + 'type' => 'accompanying_period_work', + 'id' => $work->getId(), + 'startDate' => [ + 'datetime' => '2023-12-15T00:00:00+01:00', + ], + 'endDate' => null, + 'note' => 'This is a note', + 'accompanyingPeriodWorkEvaluations' => [], + 'goals' => [], + 'handlingThirdParty' => null, + 'persons' => [[ + 'type' => 'person', + 'id' => $personId, + ], + ], + 'privateComment' => '', + 'refferers' => [], + 'results' => [], + 'thirdParties' => [], + ], JSON_THROW_ON_ERROR) + ); + + self::assertResponseIsSuccessful(); + $w = json_decode($client->getResponse()->getContent(), true, 512, JSON_THROW_ON_ERROR); + + self::assertEquals($work->getVersion() + 1, $w['version']); + } + + /** + * @dataProvider generateAccompanyingPeriodWork + */ + public function testWhenEditingAccompanyingPeriodWorkWithPreviousVersionAnHttpConflictResponseOccurs(AccompanyingPeriod\AccompanyingPeriodWork $work, int $personId): void + { + $client = $this->getClientAuthenticated(); + + $previous = $work->getVersion() - 1; + + $client->request( + 'PUT', + "/api/1.0/person/accompanying-course/work/{$work->getid()}.json?entity_version={$previous}", + content: json_encode([ + 'type' => 'accompanying_period_work', + 'id' => $work->getId(), + 'startDate' => [ + 'datetime' => '2023-12-15T00:00:00+01:00', + ], + 'endDate' => null, + 'note' => 'This is a note', + 'accompanyingPeriodWorkEvaluations' => [], + 'goals' => [], + 'handlingThirdParty' => null, + 'persons' => [[ + 'type' => 'person', + 'id' => $personId, + ], + ], + 'privateComment' => '', + 'refferers' => [], + 'results' => [], + 'thirdParties' => [], + ], JSON_THROW_ON_ERROR) + ); + + self::assertResponseStatusCodeSame(409); + } + + public function generateAccompanyingPeriodWork(): iterable + { + self::bootKernel(); + $em = self::$container->get(EntityManagerInterface::class); + $userRepository = self::$container->get(UserRepositoryInterface::class); + $user = $userRepository->findOneByUsernameOrEmail('center a_social'); + + $period = new AccompanyingPeriod(); + $em->persist($period); + $period->addPerson(($p = new Person())->setFirstName('test')->setLastName('test') + ->setBirthdate(new \DateTime('1980-01-01'))->setGender(Person::BOTH_GENDER)); + $em->persist($p); + $issue = (new SocialIssue())->setTitle(['fr' => 'test']); + $em->persist($issue); + $action = (new SocialAction())->setIssue($issue); + $em->persist($action); + + $work = new AccompanyingPeriod\AccompanyingPeriodWork(); + $work + ->setAccompanyingPeriod($period) + ->setStartDate(new \DateTimeImmutable()) + ->addPerson($p) + ->setSocialAction($action) + ->setCreatedBy($user) + ->setUpdatedBy($user) + ; + $em->persist($work); + + $em->flush(); + + self::ensureKernelShutdown(); + + yield [$work, $p->getId()]; + } +}