fix: prevent race condition on newsletter consent confirmation

This commit is contained in:
Björn Fromme
2026-07-14 09:47:22 +02:00
parent b95944701e
commit 6c4ea07e6e
3 changed files with 39 additions and 4 deletions
@@ -38,4 +38,31 @@ class NewsletterConsentRepository extends ServiceEntityRepository
'mailjetListId' => $mailjetListId, 'mailjetListId' => $mailjetListId,
]); ]);
} }
/**
* @param list<int> $mailjetListIds
*
* @return array<int, NewsletterConsent> existing consents indexed by mailjetListId
*/
public function findByEmailAndListIds(string $email, array $mailjetListIds): array
{
if ([] === $mailjetListIds) {
return [];
}
$consents = $this->createQueryBuilder('c')
->where('c.email = :email')
->andWhere('c.mailjetListId IN (:listIds)')
->setParameter('email', mb_strtolower(trim($email)))
->setParameter('listIds', $mailjetListIds)
->getQuery()
->getResult();
$byListId = [];
foreach ($consents as $consent) {
$byListId[$consent->getMailjetListId()] = $consent;
}
return $byListId;
}
} }
+4 -3
View File
@@ -327,8 +327,7 @@ class NewsletterManager
$this->newsletterService->ensureSubscribed($confirmation->getEmail(), $mailjetListId); $this->newsletterService->ensureSubscribed($confirmation->getEmail(), $mailjetListId);
} }
$confirmation->markConfirmed(); $this->upsertConfirmedConsents($confirmation->getEmail(), $mailjetListIds, $confirmation->getFirstName(), $confirmation->getLastName(), flush: false, deletePending: false);
$this->upsertConfirmedConsents($confirmation->getEmail(), $mailjetListIds, $confirmation->getFirstName(), $confirmation->getLastName(), false, false);
$this->entityManager->remove($confirmation); $this->entityManager->remove($confirmation);
$this->entityManager->flush(); $this->entityManager->flush();
@@ -468,8 +467,10 @@ class NewsletterManager
*/ */
private function upsertConfirmedConsents(string $email, array $mailjetListIds, ?string $firstName, ?string $lastName, bool $flush = true, bool $deletePending = true): void private function upsertConfirmedConsents(string $email, array $mailjetListIds, ?string $firstName, ?string $lastName, bool $flush = true, bool $deletePending = true): void
{ {
$existingByListId = $this->consentRepository->findByEmailAndListIds($email, $mailjetListIds);
foreach ($mailjetListIds as $mailjetListId) { foreach ($mailjetListIds as $mailjetListId) {
$consent = $this->consentRepository->findOneByEmailAndListId($email, $mailjetListId); $consent = $existingByListId[$mailjetListId] ?? null;
if (null === $consent) { if (null === $consent) {
$consent = new NewsletterConsent($email, $mailjetListId, $firstName, $lastName); $consent = new NewsletterConsent($email, $mailjetListId, $firstName, $lastName);
$this->entityManager->persist($consent); $this->entityManager->persist($consent);
+8 -1
View File
@@ -166,6 +166,10 @@ class NewsletterManagerTest extends TestCase
default => null, default => null,
}; };
}); });
$consents
->method('findByEmailAndListIds')
->with('[email protected]', [1, 2])
->willReturn([1 => $existingConsent, 2 => $existingOtherConsent]);
$repository->expects(self::once())->method('deletePendingByEmail')->with('[email protected]')->willReturn(0); $repository->expects(self::once())->method('deletePendingByEmail')->with('[email protected]')->willReturn(0);
$entityManager->expects(self::never())->method('persist'); $entityManager->expects(self::never())->method('persist');
$entityManager->expects(self::once())->method('flush'); $entityManager->expects(self::once())->method('flush');
@@ -223,6 +227,7 @@ class NewsletterManagerTest extends TestCase
$mailjet->expects(self::once())->method('ensureSubscribed')->with('[email protected]', 2); $mailjet->expects(self::once())->method('ensureSubscribed')->with('[email protected]', 2);
$consents->method('findActiveByEmail')->with('[email protected]')->willReturn($existingConsent); $consents->method('findActiveByEmail')->with('[email protected]')->willReturn($existingConsent);
$consents->method('findOneByEmailAndListId')->willReturn(null); $consents->method('findOneByEmailAndListId')->willReturn(null);
$consents->method('findByEmailAndListIds')->willReturn([]);
$repository->expects(self::once())->method('deletePendingByEmail')->with('[email protected]')->willReturn(0); $repository->expects(self::once())->method('deletePendingByEmail')->with('[email protected]')->willReturn(0);
$repository->expects(self::never())->method('findPendingByEmail'); $repository->expects(self::never())->method('findPendingByEmail');
$entityManager->expects(self::once())->method('persist')->with(self::isInstanceOf(NewsletterConsent::class)); $entityManager->expects(self::once())->method('persist')->with(self::isInstanceOf(NewsletterConsent::class));
@@ -254,6 +259,7 @@ class NewsletterManagerTest extends TestCase
$mailjet->expects(self::once())->method('ensureSubscribed')->with('[email protected]', 2); $mailjet->expects(self::once())->method('ensureSubscribed')->with('[email protected]', 2);
$consents->method('findActiveByEmail')->with('[email protected]')->willReturn($existingConsent); $consents->method('findActiveByEmail')->with('[email protected]')->willReturn($existingConsent);
$consents->method('findOneByEmailAndListId')->with('[email protected]', 2)->willReturn(null); $consents->method('findOneByEmailAndListId')->with('[email protected]', 2)->willReturn(null);
$consents->method('findByEmailAndListIds')->willReturn([]);
$repository->expects(self::once())->method('deletePendingByEmail')->with('[email protected]')->willReturn(0); $repository->expects(self::once())->method('deletePendingByEmail')->with('[email protected]')->willReturn(0);
$entityManager->expects(self::once())->method('persist')->with(self::isInstanceOf(NewsletterConsent::class)); $entityManager->expects(self::once())->method('persist')->with(self::isInstanceOf(NewsletterConsent::class));
$entityManager->expects(self::once())->method('flush'); $entityManager->expects(self::once())->method('flush');
@@ -338,7 +344,7 @@ class NewsletterManagerTest extends TestCase
$repository->expects(self::once())->method('findByTokenHash')->with(hash('sha256', $token))->willReturn($confirmation); $repository->expects(self::once())->method('findByTokenHash')->with(hash('sha256', $token))->willReturn($confirmation);
$repository->expects(self::never())->method('deletePendingByEmail'); $repository->expects(self::never())->method('deletePendingByEmail');
$consents->expects(self::once())->method('findOneByEmailAndListId')->with('[email protected]', 10321569)->willReturn(null); $consents->expects(self::once())->method('findByEmailAndListIds')->with('[email protected]', [10321569])->willReturn([]);
$mailjet->expects(self::once())->method('upsertContact')->with('[email protected]', 'Mia', 'Muster'); $mailjet->expects(self::once())->method('upsertContact')->with('[email protected]', 'Mia', 'Muster');
$mailjet->expects(self::once())->method('ensureSubscribed')->with('[email protected]', 10321569); $mailjet->expects(self::once())->method('ensureSubscribed')->with('[email protected]', 10321569);
$entityManager->expects(self::once())->method('persist')->with(self::isInstanceOf(NewsletterConsent::class)); $entityManager->expects(self::once())->method('persist')->with(self::isInstanceOf(NewsletterConsent::class));
@@ -370,6 +376,7 @@ class NewsletterManagerTest extends TestCase
$repository->expects(self::once())->method('findByTokenHash')->with(hash('sha256', $token))->willReturn($confirmation); $repository->expects(self::once())->method('findByTokenHash')->with(hash('sha256', $token))->willReturn($confirmation);
$repository->expects(self::never())->method('deletePendingByEmail'); $repository->expects(self::never())->method('deletePendingByEmail');
$consents->method('findOneByEmailAndListId')->willReturn(null); $consents->method('findOneByEmailAndListId')->willReturn(null);
$consents->method('findByEmailAndListIds')->with('[email protected]', [1, 2])->willReturn([]);
$mailjet $mailjet
->expects(self::exactly(2)) ->expects(self::exactly(2))
->method('ensureSubscribed') ->method('ensureSubscribed')