diff --git a/.coderabbit.yaml b/.coderabbit.yaml index f249121a..0ce9e85a 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -73,7 +73,3 @@ reviews: # code_guidelines: # filePatterns: # - ".github/AGENT.md" - -checks: - docstring_coverage: - enabled: false diff --git a/composer.json b/composer.json index 632038f8..e696c132 100644 --- a/composer.json +++ b/composer.json @@ -78,7 +78,8 @@ "webklex/php-imap": "^6.2", "ext-imap": "*", "tatevikgr/rss-feed": "dev-main", - "ext-pdo": "*" + "ext-pdo": "*", + "ezyang/htmlpurifier": "^4.19" }, "require-dev": { "phpunit/phpunit": "^9.5", diff --git a/config/PhpCodeSniffer/ruleset.xml b/config/PhpCodeSniffer/ruleset.xml index fdba2edf..7541e406 100644 --- a/config/PhpCodeSniffer/ruleset.xml +++ b/config/PhpCodeSniffer/ruleset.xml @@ -30,7 +30,9 @@ - + + 0 + diff --git a/config/parameters.yml.dist b/config/parameters.yml.dist index 88f0b2b2..41c9a20b 100644 --- a/config/parameters.yml.dist +++ b/config/parameters.yml.dist @@ -87,3 +87,5 @@ parameters: env(MAILQUEUE_THROTTLE): '5' messaging.max_process_time: '%%env(MESSAGING_MAX_PROCESS_TIME)%%' env(MESSAGING_MAX_PROCESS_TIME): '600' + messaging.max_mail_size: '%%env(MAX_MAILSIZE)%%' + env(MAX_MAILSIZE): '209715200' diff --git a/config/services.yml b/config/services.yml index 59f32e7a..ffb20ce7 100644 --- a/config/services.yml +++ b/config/services.yml @@ -50,3 +50,21 @@ services: lazy: true tags: - { name: 'doctrine.dbal.schema_filter', connection: 'default' } + + HTMLPurifier_Config: + class: HTMLPurifier_Config + factory: [ 'HTMLPurifier_Config', 'createDefault' ] + calls: + - [ set, [ 'Cache.SerializerPath', '%kernel.cache_dir%/htmlpurifier' ] ] + - [ set, [ 'HTML.ForbiddenElements', [ 'script', 'style' ] ] ] + - [ set, [ 'CSS.Disable', true ] ] + - [ set, [ 'URI.DisableJavaScript', true ] ] + - [ set, [ 'URI.DisableDataURI', true ] ] + - [ set, [ 'HTML.Doctype', 'HTML5' ] ] + - [ set, [ 'HTML.Allowed', 'p,br,b,strong,i,em,u,a[href|title],ul,ol,li,blockquote,img[src|alt|title],span,div'] ] + + HTMLPurifier: + class: HTMLPurifier + arguments: + - '@HTMLPurifier_Config' + diff --git a/config/services/managers.yml b/config/services/managers.yml index 282f5e1d..75475459 100644 --- a/config/services/managers.yml +++ b/config/services/managers.yml @@ -110,3 +110,7 @@ services: PhpList\Core\Domain\Messaging\Service\Manager\SendProcessManager: autowire: true autoconfigure: true + + PhpList\Core\Domain\Messaging\Service\Manager\MessageDataManager: + autowire: true + autoconfigure: true diff --git a/config/services/messenger.yml b/config/services/messenger.yml index 79076663..110129d5 100644 --- a/config/services/messenger.yml +++ b/config/services/messenger.yml @@ -29,6 +29,11 @@ services: autoconfigure: true tags: [ 'messenger.message_handler' ] + PhpList\Core\Domain\Messaging\MessageHandler\CampaignProcessorMessageHandler: + autowire: true + arguments: + $maxMailSize: '%messaging.max_mail_size%' + PhpList\Core\Domain\Subscription\MessageHandler\DynamicTableMessageHandler: autowire: true autoconfigure: true diff --git a/config/services/repositories.yml b/config/services/repositories.yml index f71343a1..ea1f0001 100644 --- a/config/services/repositories.yml +++ b/config/services/repositories.yml @@ -140,3 +140,8 @@ services: parent: PhpList\Core\Domain\Common\Repository\AbstractRepository arguments: - PhpList\Core\Domain\Messaging\Model\SendProcess + + PhpList\Core\Domain\Messaging\Repository\MessageDataRepository: + parent: PhpList\Core\Domain\Common\Repository\AbstractRepository + arguments: + - PhpList\Core\Domain\Messaging\Model\MessageData diff --git a/src/Domain/Analytics/Service/LinkTrackService.php b/src/Domain/Analytics/Service/LinkTrackService.php index ad2df206..902092f6 100644 --- a/src/Domain/Analytics/Service/LinkTrackService.php +++ b/src/Domain/Analytics/Service/LinkTrackService.php @@ -9,6 +9,7 @@ use PhpList\Core\Domain\Analytics\Model\LinkTrack; use PhpList\Core\Domain\Analytics\Repository\LinkTrackRepository; use PhpList\Core\Domain\Messaging\Model\Message; +use PhpList\Core\Domain\Messaging\Model\Message\MessageContent; class LinkTrackService { @@ -38,15 +39,12 @@ public function isExtractAndSaveLinksApplicable(): bool * @return LinkTrack[] The saved LinkTrack entities * @throws MissingMessageIdException */ - public function extractAndSaveLinks(Message $message, int $userId): array + public function extractAndSaveLinks(MessageContent $content, int $userId, ?int $messageId = null): array { if (!$this->isExtractAndSaveLinksApplicable()) { return []; } - $content = $message->getContent(); - $messageId = $message->getId(); - if ($messageId === null) { throw new MissingMessageIdException(); } diff --git a/src/Domain/Identity/Service/PermissionChecker.php b/src/Domain/Identity/Service/PermissionChecker.php index 8fc241b7..9986ff59 100644 --- a/src/Domain/Identity/Service/PermissionChecker.php +++ b/src/Domain/Identity/Service/PermissionChecker.php @@ -70,6 +70,8 @@ private function resolveRelatedEntity(DomainModel $resource, string $relatedClas } if ($resource instanceof Message && $relatedClass === SubscriberList::class) { + // todo: check which one is correct + // return $resource->getListMessages()->map(fn(ListMessage $lm) => $lm->getList())->toArray(); return $resource->getListMessages()->map(fn($lm) => $lm->getSubscriberList())->toArray(); } diff --git a/src/Domain/Messaging/Exception/MessageSizeLimitExceededException.php b/src/Domain/Messaging/Exception/MessageSizeLimitExceededException.php new file mode 100644 index 00000000..80053f4b --- /dev/null +++ b/src/Domain/Messaging/Exception/MessageSizeLimitExceededException.php @@ -0,0 +1,31 @@ +actualSize; + } + + public function getMaxSize(): int + { + return $this->maxSize; + } +} diff --git a/src/Domain/Messaging/MessageHandler/CampaignProcessorMessageHandler.php b/src/Domain/Messaging/MessageHandler/CampaignProcessorMessageHandler.php index f0e4b5b1..12d45c30 100644 --- a/src/Domain/Messaging/MessageHandler/CampaignProcessorMessageHandler.php +++ b/src/Domain/Messaging/MessageHandler/CampaignProcessorMessageHandler.php @@ -5,6 +5,7 @@ namespace PhpList\Core\Domain\Messaging\MessageHandler; use Doctrine\ORM\EntityManagerInterface; +use PhpList\Core\Domain\Messaging\Exception\MessageSizeLimitExceededException; use PhpList\Core\Domain\Messaging\Message\CampaignProcessorMessage; use PhpList\Core\Domain\Messaging\Message\SyncCampaignProcessorMessage; use PhpList\Core\Domain\Messaging\Model\Message; @@ -14,14 +15,19 @@ use PhpList\Core\Domain\Messaging\Repository\MessageRepository; use PhpList\Core\Domain\Messaging\Repository\UserMessageRepository; use PhpList\Core\Domain\Messaging\Service\Handler\RequeueHandler; +use PhpList\Core\Domain\Messaging\Service\Manager\MessageDataManager; use PhpList\Core\Domain\Messaging\Service\MaxProcessTimeLimiter; +use PhpList\Core\Domain\Messaging\Service\MessagePrecacheService; use PhpList\Core\Domain\Messaging\Service\MessageProcessingPreparator; use PhpList\Core\Domain\Messaging\Service\RateLimitedCampaignMailer; +use PhpList\Core\Domain\Configuration\Service\Manager\EventLogManager; use PhpList\Core\Domain\Subscription\Model\Subscriber; use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberHistoryManager; use PhpList\Core\Domain\Subscription\Service\Provider\SubscriberProvider; use Psr\Log\LoggerInterface; +use Psr\SimpleCache\CacheInterface; use Symfony\Component\Messenger\Attribute\AsMessageHandler; +use Symfony\Component\Mime\Email; use Symfony\Contracts\Translation\TranslatorInterface; use Throwable; @@ -32,42 +38,27 @@ #[AsMessageHandler] class CampaignProcessorMessageHandler { - private RateLimitedCampaignMailer $mailer; - private EntityManagerInterface $entityManager; - private SubscriberProvider $subscriberProvider; - private MessageProcessingPreparator $messagePreparator; - private LoggerInterface $logger; - private UserMessageRepository $userMessageRepository; - private MaxProcessTimeLimiter $timeLimiter; - private RequeueHandler $requeueHandler; - private TranslatorInterface $translator; - private SubscriberHistoryManager $subscriberHistoryManager; - private MessageRepository $messageRepository; + private ?int $maxMailSize; public function __construct( - RateLimitedCampaignMailer $mailer, - EntityManagerInterface $entityManager, - SubscriberProvider $subscriberProvider, - MessageProcessingPreparator $messagePreparator, - LoggerInterface $logger, - UserMessageRepository $userMessageRepository, - MaxProcessTimeLimiter $timeLimiter, - RequeueHandler $requeueHandler, - TranslatorInterface $translator, - SubscriberHistoryManager $subscriberHistoryManager, - MessageRepository $messageRepository, + private readonly RateLimitedCampaignMailer $mailer, + private readonly EntityManagerInterface $entityManager, + private readonly SubscriberProvider $subscriberProvider, + private readonly MessageProcessingPreparator $messagePreparator, + private readonly LoggerInterface $logger, + private readonly CacheInterface $cache, + private readonly UserMessageRepository $userMessageRepository, + private readonly MaxProcessTimeLimiter $timeLimiter, + private readonly RequeueHandler $requeueHandler, + private readonly TranslatorInterface $translator, + private readonly SubscriberHistoryManager $subscriberHistoryManager, + private readonly MessageRepository $messageRepository, + private readonly EventLogManager $eventLogManager, + private readonly MessageDataManager $messageDataManager, + private readonly MessagePrecacheService $precacheService, + ?int $maxMailSize = null, ) { - $this->mailer = $mailer; - $this->entityManager = $entityManager; - $this->subscriberProvider = $subscriberProvider; - $this->messagePreparator = $messagePreparator; - $this->logger = $logger; - $this->userMessageRepository = $userMessageRepository; - $this->timeLimiter = $timeLimiter; - $this->requeueHandler = $requeueHandler; - $this->translator = $translator; - $this->subscriberHistoryManager = $subscriberHistoryManager; - $this->messageRepository = $messageRepository; + $this->maxMailSize = $maxMailSize ?? 0; } public function __invoke(CampaignProcessorMessage|SyncCampaignProcessorMessage $message): void @@ -82,6 +73,8 @@ public function __invoke(CampaignProcessorMessage|SyncCampaignProcessorMessage $ return; } + $messageContent = $this->precacheService->getOrCacheBaseMessageContent($campaign); + $this->updateMessageStatus($campaign, MessageStatus::Prepared); $subscribers = $this->subscriberProvider->getSubscribersForMessage($campaign); @@ -111,7 +104,7 @@ public function __invoke(CampaignProcessorMessage|SyncCampaignProcessorMessage $ continue; } - $this->handleEmailSending($campaign, $subscriber, $userMessage); + $this->handleEmailSending($campaign, $subscriber, $userMessage, $messageContent); } if ($stoppedEarly && $this->requeueHandler->handle($campaign)) { @@ -142,7 +135,7 @@ private function updateUserMessageStatus(UserMessage $userMessage, UserMessageSt $this->entityManager->flush(); } - private function handleInvalidEmail(UserMessage $userMessage, Subscriber $subscriber, mixed $campaign): void + private function handleInvalidEmail(UserMessage $userMessage, Subscriber $subscriber, Message $campaign): void { $this->updateUserMessageStatus($userMessage, UserMessageStatus::InvalidEmailAddress); $this->unconfirmSubscriber($subscriber); @@ -159,14 +152,25 @@ private function handleInvalidEmail(UserMessage $userMessage, Subscriber $subscr ); } - private function handleEmailSending(mixed $campaign, Subscriber $subscriber, UserMessage $userMessage): void - { - $processed = $this->messagePreparator->processMessageLinks($campaign, $subscriber->getId()); + private function handleEmailSending( + Message $campaign, + Subscriber $subscriber, + UserMessage $userMessage, + Message\MessageContent $precachedContent, + ): void { + $processed = $this->messagePreparator->processMessageLinks($campaign->getId(), $precachedContent, $subscriber); try { - $email = $this->mailer->composeEmail($processed, $subscriber); + $email = $this->mailer->composeEmail($campaign, $subscriber, $processed); $this->mailer->send($email); + $this->checkMessageSizeOrSuspendCampaign($campaign, $email, $subscriber->hasHtmlEmail()); + $this->updateUserMessageStatus($userMessage, UserMessageStatus::Sent); + } catch (MessageSizeLimitExceededException $e) { + // stop after the first message if size is exceeded + $this->updateMessageStatus($campaign, MessageStatus::Suspended); $this->updateUserMessageStatus($userMessage, UserMessageStatus::Sent); + + throw $e; } catch (Throwable $e) { $this->updateUserMessageStatus($userMessage, UserMessageStatus::NotSent); $this->logger->error($e->getMessage(), [ @@ -178,4 +182,57 @@ private function handleEmailSending(mixed $campaign, Subscriber $subscriber, Use ])); } } + + private function checkMessageSizeOrSuspendCampaign( + Message $campaign, + Email $email, + bool $hasHtmlEmail + ): void { + if ($this->maxMailSize <= 0) { + return; + } + $sizeName = $hasHtmlEmail ? 'htmlsize' : 'textsize'; + $cacheKey = sprintf('messaging.size.%d.%s', $campaign->getId(), $sizeName); + if (!$this->cache->has($cacheKey)) { + $size = $this->calculateEmailSize($email); + $this->messageDataManager->setMessageData($campaign, $sizeName, $size); + $this->cache->set($cacheKey, $size); + } + + $size = $this->cache->get($cacheKey); + if ($size <= $this->maxMailSize) { + return; + } + + $this->logger->warning(sprintf( + 'Message too large (%d is over %d), suspending campaign %d', + $size, + $this->maxMailSize, + $campaign->getId() + )); + + $this->eventLogManager->log('send', sprintf( + 'Message too large (%d is over %d), suspending', + $size, + $this->maxMailSize + )); + + $this->eventLogManager->log('send', sprintf( + 'Campaign %d suspended. Message too large', + $campaign->getId() + )); + + throw new MessageSizeLimitExceededException($size, $this->maxMailSize); + } + + private function calculateEmailSize(Email $email): int + { + $size = 0; + + foreach ($email->toIterable() as $line) { + $size += strlen($line); + } + + return $size; + } } diff --git a/src/Domain/Messaging/Repository/MessageDataRepository.php b/src/Domain/Messaging/Repository/MessageDataRepository.php index 6f9b4910..51b27a04 100644 --- a/src/Domain/Messaging/Repository/MessageDataRepository.php +++ b/src/Domain/Messaging/Repository/MessageDataRepository.php @@ -7,8 +7,14 @@ use PhpList\Core\Domain\Common\Repository\AbstractRepository; use PhpList\Core\Domain\Common\Repository\CursorPaginationTrait; use PhpList\Core\Domain\Common\Repository\Interfaces\PaginatableRepositoryInterface; +use PhpList\Core\Domain\Messaging\Model\MessageData; class MessageDataRepository extends AbstractRepository implements PaginatableRepositoryInterface { use CursorPaginationTrait; + + public function findByIdAndName(int $messageId, string $name): ?MessageData + { + return $this->findOneBy(['id' => $messageId, 'name' => $name]); + } } diff --git a/src/Domain/Messaging/Service/Manager/MessageDataManager.php b/src/Domain/Messaging/Service/Manager/MessageDataManager.php new file mode 100644 index 00000000..760fda60 --- /dev/null +++ b/src/Domain/Messaging/Service/Manager/MessageDataManager.php @@ -0,0 +1,96 @@ +normalizeValueByName($name, $value); + + // todo: remove this once we have a proper way to handle targetlists +// if ($name === 'targetlist' && is_array($value)) { +// $this->listMessageRepository->removeAllListAssociationsForMessage($campaign); +// +// if (!empty($value['all']) || !empty($value['allactive'])) { +// // todo: should be with $GLOBALS['subselect'] filter for access control +// foreach ($this->subscriberListRepository->getAllActive() as $list) { +// $listMessage = (new ListMessage()) +// ->setMessage($campaign) +// ->setList($list); +// $this->listMessageRepository->persist($listMessage); +// } +// // once we used "all" to set all, unset it, to avoid confusion trying to unselect lists +// unset($value['all']); +// } else { +// foreach ($value as $listId => $val) { +// // see #16940 - ignore a list called "unselect" which is there to allow unselecting all +// if ($listId !== 'unselect') { +// $list = $this->subscriberListRepository->find($listId); +// $listMessage = (new ListMessage()) +// ->setMessage($campaign) +// ->setList($list); +// $this->listMessageRepository->persist($listMessage); +// } +// } +// } +// } + + if (is_array($value) || is_object($value)) { + $value = 'SER:' . serialize($value); + } + + $entity = $this->getOrCreateMessageDataEntity($campaign, $name); + $entity->setData($value !== null ? (string) $value : null); + } + + private function normalizeValueByName(string $name, mixed $value) + { + return match ($name) { + 'subject', 'campaigntitle' => is_string($value) ? strip_tags($value) : $value, + 'message' => is_string($value) ? $this->purifier->purify($value) : $value, + 'excludelist' => is_array($value) ? array_filter($value, fn ($val) => is_numeric($val)) : $value, + 'footer' => is_string($value) ? preg_replace('//', '', $value) : $value, + default => $value, + }; + } + + private function getOrCreateMessageDataEntity(Message $campaign, string $name) + { + $entity = $this->messageDataRepository->findByIdAndName($campaign->getId(), $name); + if (!$entity instanceof MessageData) { + $entity = (new MessageData()) + ->setId($campaign->getId()) + ->setName($name); + $this->entityManager->persist($entity); + } + + return $entity; + } +} diff --git a/src/Domain/Messaging/Service/MessagePrecacheService.php b/src/Domain/Messaging/Service/MessagePrecacheService.php new file mode 100644 index 00000000..cc1c7dc9 --- /dev/null +++ b/src/Domain/Messaging/Service/MessagePrecacheService.php @@ -0,0 +1,137 @@ +getId()); + + $cached = $this->getFromCache($cacheKey); + if ($cached !== null) { + return $cached; + } + + $content = $campaign->getContent(); + $subject = $content->getSubject(); + $html = $content->getText(); + $text = $content->getTextMessage(); + $footer = $content->getFooter(); + + // If content contains a [URL:...] token, try to fetch and replace with remote content + if (is_string($html) && preg_match('/\[URL:([^\s\]]+)\]/i', $html, $match)) { + $remoteUrl = $match[1]; + $fetched = $this->fetchRemoteContent($remoteUrl); + if ($fetched !== null) { + $html = str_replace($match[0], $fetched, $html); + } + } + + // Replace basic placeholders [subject],[id],[fromname],[fromemail] + $replacements = $this->buildBasicReplacements($campaign, $subject); + $html = $this->applyReplacements($html, $replacements); + $text = $this->applyReplacements($text, $replacements); + $footer = $this->applyReplacements($footer, $replacements); + + $snapshot = [ + 'subject' => $subject, + 'text' => $html, + 'textMessage' => $text, + 'footer' => $footer, + ]; + + $this->cache->set($cacheKey, $snapshot); + + return new Message\MessageContent($subject, $html, $text, $footer); + } + + private function fetchRemoteContent(string $url): ?string + { + $ctx = stream_context_create([ + 'http' => ['timeout' => 5], + 'https' => ['timeout' => 5], + ]); + + // Ignore warnings from file_get_contents only inside this block + set_error_handler(static function () { + return true; + }); + + try { + $data = file_get_contents($url, false, $ctx); + } finally { + restore_error_handler(); + } + + if ($data === false) { + return null; + } + + return $data; + } + + private function buildBasicReplacements(Message $campaign, string $subject): array + { + [$fromName, $fromEmail] = $this->parseFromField($campaign->getOptions()->getFromField()); + return [ + '[subject]' => $subject, + '[id]' => (string)($campaign->getId() ?? ''), + '[fromname]' => $fromName, + '[fromemail]' => $fromEmail, + ]; + } + + private function parseFromField(string $fromField): array + { + $email = ''; + if (preg_match('/([^\s<>"]+@[^\s<>"]+)/', $fromField, $match)) { + $email = str_replace(['<', '>'], '', $match[0]); + } + $name = trim(str_replace([$email, '"'], ['', ''], $fromField)); + $name = trim(str_replace(['<', '>'], '', $name)); + return [$name, $email]; + } + + private function applyReplacements(?string $input, array $replacements): ?string + { + if ($input === null) { + return null; + } + return str_ireplace(array_keys($replacements), array_values($replacements), $input); + } + + private function getFromCache(string $cacheKey): ?Message\MessageContent + { + $cached = $this->cache->get($cacheKey); + if (is_array($cached) + && array_key_exists('subject', $cached) + && array_key_exists('text', $cached) + && array_key_exists('textMessage', $cached) + && array_key_exists('footer', $cached) + ) { + return new Message\MessageContent( + $cached['subject'], + $cached['text'], + $cached['textMessage'], + $cached['footer'] + ); + } + + return null; + } +} diff --git a/src/Domain/Messaging/Service/MessageProcessingPreparator.php b/src/Domain/Messaging/Service/MessageProcessingPreparator.php index 5255914d..aee1d9e7 100644 --- a/src/Domain/Messaging/Service/MessageProcessingPreparator.php +++ b/src/Domain/Messaging/Service/MessageProcessingPreparator.php @@ -5,32 +5,27 @@ namespace PhpList\Core\Domain\Messaging\Service; use PhpList\Core\Domain\Analytics\Service\LinkTrackService; +use PhpList\Core\Domain\Configuration\Service\UserPersonalizer; use PhpList\Core\Domain\Messaging\Model\Message; +use PhpList\Core\Domain\Messaging\Model\Message\MessageContent; use PhpList\Core\Domain\Messaging\Repository\MessageRepository; +use PhpList\Core\Domain\Subscription\Model\Subscriber; use PhpList\Core\Domain\Subscription\Repository\SubscriberRepository; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Contracts\Translation\TranslatorInterface; class MessageProcessingPreparator { - // phpcs:ignore Generic.Commenting.Todo // @todo: create functionality to track - public const LINT_TRACK_ENDPOINT = '/api/v2/link-track'; - private SubscriberRepository $subscriberRepository; - private MessageRepository $messageRepository; - private LinkTrackService $linkTrackService; - private TranslatorInterface $translator; + public const LINK_TRACK_ENDPOINT = '/api/v2/link-track'; public function __construct( - SubscriberRepository $subscriberRepository, - MessageRepository $messageRepository, - LinkTrackService $linkTrackService, - TranslatorInterface $translator, + private readonly SubscriberRepository $subscriberRepository, + private readonly MessageRepository $messageRepository, + private readonly LinkTrackService $linkTrackService, + private readonly TranslatorInterface $translator, + private readonly UserPersonalizer $userPersonalizer, ) { - $this->subscriberRepository = $subscriberRepository; - $this->messageRepository = $messageRepository; - $this->linkTrackService = $linkTrackService; - $this->translator = $translator; } public function ensureSubscribersHaveUuid(OutputInterface $output): void @@ -66,40 +61,44 @@ public function ensureCampaignsHaveUuid(OutputInterface $output): void /** * Process message content to extract URLs and replace them with link track URLs */ - public function processMessageLinks(Message $message, int $userId): Message - { + public function processMessageLinks( + int $campaignId, + MessageContent $content, + Subscriber $subscriber + ): MessageContent { if (!$this->linkTrackService->isExtractAndSaveLinksApplicable()) { - return $message; + return $content; } - $savedLinks = $this->linkTrackService->extractAndSaveLinks($message, $userId); + $savedLinks = $this->linkTrackService->extractAndSaveLinks($content, $subscriber->getId(), $campaignId); if (empty($savedLinks)) { - return $message; + return $content; } - $content = $message->getContent(); $htmlText = $content->getText(); $footer = $content->getFooter(); - + // todo: check other configured data that should be used in mail formatting/creation if ($htmlText !== null) { $htmlText = $this->replaceLinks($savedLinks, $htmlText); + $htmlText = $this->userPersonalizer->personalize($htmlText, $subscriber->getEmail()); $content->setText($htmlText); } if ($footer !== null) { $footer = $this->replaceLinks($savedLinks, $footer); + $footer = $this->userPersonalizer->personalize($footer, $subscriber->getEmail()); $content->setFooter($footer); } - return $message; + return $content; } private function replaceLinks(array $savedLinks, string $htmlText): string { foreach ($savedLinks as $linkTrack) { $originalUrl = $linkTrack->getUrl(); - $trackUrl = '/' . self::LINT_TRACK_ENDPOINT . '?id=' . $linkTrack->getId(); + $trackUrl = self::LINK_TRACK_ENDPOINT . '?id=' . $linkTrack->getId(); $htmlText = str_replace('href="' . $originalUrl . '"', 'href="' . $trackUrl . '"', $htmlText); } diff --git a/src/Domain/Messaging/Service/RateLimitedCampaignMailer.php b/src/Domain/Messaging/Service/RateLimitedCampaignMailer.php index 7691f970..de2b73c1 100644 --- a/src/Domain/Messaging/Service/RateLimitedCampaignMailer.php +++ b/src/Domain/Messaging/Service/RateLimitedCampaignMailer.php @@ -20,7 +20,7 @@ public function __construct(MailerInterface $mailer, SendRateLimiter $limiter) $this->limiter = $limiter; } - public function composeEmail(Message $processed, Subscriber $subscriber): Email + public function composeEmail(Message $processed, Subscriber $subscriber, Message\MessageContent $content): Email { $email = new Email(); if ($processed->getOptions()->getFromField() !== '') { @@ -33,9 +33,10 @@ public function composeEmail(Message $processed, Subscriber $subscriber): Email return $email ->to($subscriber->getEmail()) - ->subject($processed->getContent()->getSubject()) - ->text($processed->getContent()->getTextMessage()) - ->html($processed->getContent()->getText()); + ->subject($content->getSubject()) + // todo: check HTML2Text functionality + ->text($content->getTextMessage()) + ->html($content->getText()); } /** diff --git a/src/Domain/Subscription/Repository/SubscriberListRepository.php b/src/Domain/Subscription/Repository/SubscriberListRepository.php index bb0fb678..d8910751 100644 --- a/src/Domain/Subscription/Repository/SubscriberListRepository.php +++ b/src/Domain/Subscription/Repository/SubscriberListRepository.php @@ -45,4 +45,12 @@ public function getListsByMessage(Message $message): array ->getQuery() ->getResult(); } + + public function getAllActive(): array + { + return $this->createQueryBuilder('l') + ->where('l.active = true') + ->getQuery() + ->getResult(); + } } diff --git a/src/Domain/Subscription/Service/Manager/SubscriberAttributeManager.php b/src/Domain/Subscription/Service/Manager/SubscriberAttributeManager.php index 75b9e3e2..4426fe70 100644 --- a/src/Domain/Subscription/Service/Manager/SubscriberAttributeManager.php +++ b/src/Domain/Subscription/Service/Manager/SubscriberAttributeManager.php @@ -38,7 +38,6 @@ public function createOrUpdate( SubscriberAttributeDefinition $definition, ?string $value = null ): SubscriberAttributeValue { - // phpcs:ignore Generic.Commenting.Todo // todo: clarify which attributes can be created/updated $subscriberAttribute = $this->attributeRepository ->findOneBySubscriberAndAttribute($subscriber, $definition); diff --git a/tests/Unit/Domain/Analytics/Service/LinkTrackServiceTest.php b/tests/Unit/Domain/Analytics/Service/LinkTrackServiceTest.php index c768b056..9c42163e 100644 --- a/tests/Unit/Domain/Analytics/Service/LinkTrackServiceTest.php +++ b/tests/Unit/Domain/Analytics/Service/LinkTrackServiceTest.php @@ -44,7 +44,7 @@ public function testExtractAndSaveLinksWithNoLinks(): void $this->linkTrackRepository->expects(self::never())->method('persist'); - $result = $this->subject->extractAndSaveLinks($message, $userId); + $result = $this->subject->extractAndSaveLinks($messageContent, $userId, $messageId); self::assertEmpty($result); } @@ -71,7 +71,7 @@ public function testExtractAndSaveLinksWithLinks(): void return null; }); - $result = $this->subject->extractAndSaveLinks($message, $userId); + $result = $this->subject->extractAndSaveLinks($messageContent, $userId, $messageId); self::assertCount(2, $result); self::assertSame('https://example.com', $result[0]->getUrl()); @@ -100,7 +100,7 @@ public function testExtractAndSaveLinksWithFooter(): void return null; }); - $result = $this->subject->extractAndSaveLinks($message, $userId); + $result = $this->subject->extractAndSaveLinks($messageContent, $userId, $messageId); self::assertCount(2, $result); self::assertSame('https://example.com', $result[0]->getUrl()); @@ -128,7 +128,7 @@ public function testExtractAndSaveLinksWithDuplicateLinks(): void return null; }); - $result = $this->subject->extractAndSaveLinks($message, $userId); + $result = $this->subject->extractAndSaveLinks($messageContent, $userId, $messageId); self::assertCount(1, $result); self::assertSame('https://example.com', $result[0]->getUrl()); @@ -155,7 +155,7 @@ public function testExtractAndSaveLinksWithNullText(): void return null; }); - $result = $this->subject->extractAndSaveLinks($message, $userId); + $result = $this->subject->extractAndSaveLinks($messageContent, $userId, $messageId); self::assertCount(1, $result); self::assertSame('https://footer.com', $result[0]->getUrl()); @@ -175,7 +175,7 @@ public function testExtractAndSaveLinksWithMessageWithoutId(): void $this->expectException(MissingMessageIdException::class); $this->expectExceptionMessage('Message must have an ID'); - $this->subject->extractAndSaveLinks($message, $userId); + $this->subject->extractAndSaveLinks($messageContent, $userId, $message->getId()); } public function testIsExtractAndSaveLinksApplicableWhenClickTrackIsTrue(): void @@ -221,7 +221,7 @@ public function testExtractAndSaveLinksWithExistingLink(): void $this->linkTrackRepository->expects(self::never()) ->method('persist'); - $result = $this->subject->extractAndSaveLinks($message, $userId); + $result = $this->subject->extractAndSaveLinks($messageContent, $userId, $message->getId()); self::assertCount(1, $result); self::assertSame($existingLinkTrack, $result[0]); diff --git a/tests/Unit/Domain/Messaging/MessageHandler/CampaignProcessorMessageHandlerTest.php b/tests/Unit/Domain/Messaging/MessageHandler/CampaignProcessorMessageHandlerTest.php index e50d89fa..a565f558 100644 --- a/tests/Unit/Domain/Messaging/MessageHandler/CampaignProcessorMessageHandlerTest.php +++ b/tests/Unit/Domain/Messaging/MessageHandler/CampaignProcessorMessageHandlerTest.php @@ -6,6 +6,7 @@ use Doctrine\ORM\EntityManagerInterface; use Exception; +use PhpList\Core\Domain\Configuration\Service\Manager\EventLogManager; use PhpList\Core\Domain\Messaging\Message\CampaignProcessorMessage; use PhpList\Core\Domain\Messaging\MessageHandler\CampaignProcessorMessageHandler; use PhpList\Core\Domain\Messaging\Model\Message; @@ -15,8 +16,10 @@ use PhpList\Core\Domain\Messaging\Repository\MessageRepository; use PhpList\Core\Domain\Messaging\Repository\UserMessageRepository; use PhpList\Core\Domain\Messaging\Service\Handler\RequeueHandler; +use PhpList\Core\Domain\Messaging\Service\Manager\MessageDataManager; use PhpList\Core\Domain\Messaging\Service\MaxProcessTimeLimiter; use PhpList\Core\Domain\Messaging\Service\MessageProcessingPreparator; +use PhpList\Core\Domain\Messaging\Service\MessagePrecacheService; use PhpList\Core\Domain\Messaging\Service\RateLimitedCampaignMailer; use PhpList\Core\Domain\Subscription\Model\Subscriber; use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberHistoryManager; @@ -24,6 +27,7 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; +use Psr\SimpleCache\CacheInterface; use Symfony\Component\Mime\Email; use Symfony\Component\Translation\Translator; use Symfony\Contracts\Translation\TranslatorInterface; @@ -37,10 +41,8 @@ class CampaignProcessorMessageHandlerTest extends TestCase private LoggerInterface|MockObject $logger; private CampaignProcessorMessageHandler $handler; private MessageRepository|MockObject $messageRepository; - private UserMessageRepository|MockObject $userMessageRepository; - private MaxProcessTimeLimiter|MockObject $timeLimiter; - private RequeueHandler|MockObject $requeueHandler; private TranslatorInterface|MockObject $translator; + private MessagePrecacheService|MockObject $precacheService; protected function setUp(): void { @@ -50,13 +52,14 @@ protected function setUp(): void $this->messagePreparator = $this->createMock(MessageProcessingPreparator::class); $this->logger = $this->createMock(LoggerInterface::class); $this->messageRepository = $this->createMock(MessageRepository::class); - $this->userMessageRepository = $this->createMock(UserMessageRepository::class); - $this->timeLimiter = $this->createMock(MaxProcessTimeLimiter::class); - $this->requeueHandler = $this->createMock(RequeueHandler::class); + $userMessageRepository = $this->createMock(UserMessageRepository::class); + $timeLimiter = $this->createMock(MaxProcessTimeLimiter::class); + $requeueHandler = $this->createMock(RequeueHandler::class); $this->translator = $this->createMock(Translator::class); + $this->precacheService = $this->createMock(MessagePrecacheService::class); - $this->timeLimiter->method('start'); - $this->timeLimiter->method('shouldStop')->willReturn(false); + $timeLimiter->method('start'); + $timeLimiter->method('shouldStop')->willReturn(false); $this->handler = new CampaignProcessorMessageHandler( mailer: $this->mailer, @@ -64,12 +67,17 @@ protected function setUp(): void subscriberProvider: $this->subscriberProvider, messagePreparator: $this->messagePreparator, logger: $this->logger, - userMessageRepository: $this->userMessageRepository, - timeLimiter: $this->timeLimiter, - requeueHandler: $this->requeueHandler, + cache: $this->createMock(CacheInterface::class), + userMessageRepository: $userMessageRepository, + timeLimiter: $timeLimiter, + requeueHandler: $requeueHandler, translator: $this->translator, subscriberHistoryManager: $this->createMock(SubscriberHistoryManager::class), messageRepository: $this->messageRepository, + eventLogManager: $this->createMock(EventLogManager::class), + messageDataManager: $this->createMock(MessageDataManager::class), + precacheService: $this->precacheService, + maxMailSize: 0, ); } @@ -156,7 +164,9 @@ public function testInvokeWithInvalidSubscriberEmail(): void public function testInvokeWithValidSubscriberEmail(): void { - $campaign = $this->createCampaignMock(); + $campaign = $this->createMock(Message::class); + $content = $this->createContentMock(); + $campaign->method('getContent')->willReturn($content); $metadata = $this->createMock(MessageMetadata::class); $campaign->method('getMetadata')->willReturn($metadata); $campaign->method('getId')->willReturn(1); @@ -176,15 +186,20 @@ public function testInvokeWithValidSubscriberEmail(): void $this->messagePreparator->expects($this->once()) ->method('processMessageLinks') - ->with($campaign, 1) - ->willReturn($campaign); + ->willReturn($content); $this->mailer->expects($this->once()) ->method('composeEmail') - ->with($campaign, $subscriber) - ->willReturnCallback(function ($processed, $sub) use ($campaign, $subscriber) { - $this->assertSame($campaign, $processed); + ->with( + $this->identicalTo($campaign), + $this->identicalTo($subscriber), + $this->identicalTo($content) + ) + ->willReturnCallback(function ($camp, $sub, $proc) use ($campaign, $subscriber, $content) { + $this->assertSame($campaign, $camp); $this->assertSame($subscriber, $sub); + $this->assertSame($content, $proc); + return (new Email()) ->from('news@example.com') ->to('test@example.com') @@ -208,8 +223,10 @@ public function testInvokeWithValidSubscriberEmail(): void public function testInvokeWithMailerException(): void { - $campaign = $this->createCampaignMock(); + $campaign = $this->createMock(Message::class); + $content = $this->createContentMock(); $metadata = $this->createMock(MessageMetadata::class); + $campaign->method('getContent')->willReturn($content); $campaign->method('getMetadata')->willReturn($metadata); $campaign->method('getId')->willReturn(123); @@ -221,6 +238,11 @@ public function testInvokeWithMailerException(): void $subscriber->method('getEmail')->willReturn('test@example.com'); $subscriber->method('getId')->willReturn(1); + $this->precacheService->expects($this->once()) + ->method('getOrCacheBaseMessageContent') + ->with($campaign) + ->willReturn($content); + $this->subscriberProvider->expects($this->once()) ->method('getSubscribersForMessage') ->with($campaign) @@ -228,8 +250,8 @@ public function testInvokeWithMailerException(): void $this->messagePreparator->expects($this->once()) ->method('processMessageLinks') - ->with($campaign, 1) - ->willReturn($campaign); + ->with(123, $content, $subscriber) + ->willReturn($content); $exception = new Exception('Test exception'); $this->mailer->expects($this->once()) @@ -255,6 +277,7 @@ public function testInvokeWithMailerException(): void public function testInvokeWithMultipleSubscribers(): void { $campaign = $this->createCampaignMock(); + $content = $this->createContentMock(); $metadata = $this->createMock(MessageMetadata::class); $campaign->method('getMetadata')->willReturn($metadata); $campaign->method('getId')->willReturn(1); @@ -282,7 +305,7 @@ public function testInvokeWithMultipleSubscribers(): void $this->messagePreparator->expects($this->exactly(2)) ->method('processMessageLinks') - ->willReturn($campaign); + ->willReturn($content); $this->mailer->expects($this->exactly(2)) ->method('send'); @@ -302,14 +325,20 @@ public function testInvokeWithMultipleSubscribers(): void private function createCampaignMock(): Message|MockObject { $campaign = $this->createMock(Message::class); - $content = $this->createMock(MessageContent::class); + $content = $this->createContentMock(); + $campaign->method('getContent')->willReturn($content); + return $campaign; + } + + private function createContentMock(): MessageContent|MockObject + { + $content = $this->createMock(MessageContent::class); + $content->method('getSubject')->willReturn('Test Subject'); $content->method('getTextMessage')->willReturn('Test text message'); $content->method('getText')->willReturn('

Test HTML message

'); - - $campaign->method('getContent')->willReturn($content); - - return $campaign; + + return $content; } } diff --git a/tests/Unit/Domain/Messaging/Service/MessageProcessingPreparatorTest.php b/tests/Unit/Domain/Messaging/Service/MessageProcessingPreparatorTest.php index f2a29d49..b7530895 100644 --- a/tests/Unit/Domain/Messaging/Service/MessageProcessingPreparatorTest.php +++ b/tests/Unit/Domain/Messaging/Service/MessageProcessingPreparatorTest.php @@ -6,6 +6,7 @@ use PhpList\Core\Domain\Analytics\Model\LinkTrack; use PhpList\Core\Domain\Analytics\Service\LinkTrackService; +use PhpList\Core\Domain\Configuration\Service\UserPersonalizer; use PhpList\Core\Domain\Messaging\Model\Message\MessageContent; use PhpList\Core\Domain\Messaging\Repository\MessageRepository; use PhpList\Core\Domain\Messaging\Service\MessageProcessingPreparator; @@ -22,6 +23,7 @@ class MessageProcessingPreparatorTest extends TestCase private SubscriberRepository&MockObject $subscriberRepository; private MessageRepository&MockObject $messageRepository; private LinkTrackService&MockObject $linkTrackService; + private UserPersonalizer&MockObject $userPersonalizer; private OutputInterface&MockObject $output; private MessageProcessingPreparator $preparator; @@ -30,6 +32,13 @@ protected function setUp(): void $this->subscriberRepository = $this->createMock(SubscriberRepository::class); $this->messageRepository = $this->createMock(MessageRepository::class); $this->linkTrackService = $this->createMock(LinkTrackService::class); + $this->userPersonalizer = $this->createMock(UserPersonalizer::class); + // Ensure personalization returns original text so assertions on replaced links remain valid + $this->userPersonalizer + ->method('personalize') + ->willReturnCallback(function (string $text) { + return $text; + }); $this->output = $this->createMock(OutputInterface::class); $this->preparator = new MessageProcessingPreparator( @@ -37,6 +46,7 @@ protected function setUp(): void messageRepository: $this->messageRepository, linkTrackService: $this->linkTrackService, translator: new Translator('en'), + userPersonalizer: $this->userPersonalizer, ); } @@ -118,8 +128,10 @@ public function testEnsureCampaignsHaveUuidWithCampaigns(): void public function testProcessMessageLinksWhenLinkTrackingNotApplicable(): void { - $message = $this->createMock(Message::class); - $userId = 123; + $messageContent = $this->createMock(MessageContent::class); + $subscriber = $this->createMock(Subscriber::class); + $subscriber->method('getId')->willReturn(123); + $subscriber->method('getEmail')->willReturn('test@example.com'); $this->linkTrackService->expects($this->once()) ->method('isExtractAndSaveLinksApplicable') @@ -128,18 +140,21 @@ public function testProcessMessageLinksWhenLinkTrackingNotApplicable(): void $this->linkTrackService->expects($this->never()) ->method('extractAndSaveLinks'); - $message->expects($this->never()) - ->method('getContent'); + $messageContent->expects($this->never()) + ->method('getText'); - $result = $this->preparator->processMessageLinks($message, $userId); + $result = $this->preparator->processMessageLinks(1, $messageContent, $subscriber); - $this->assertSame($message, $result); + $this->assertSame($messageContent, $result); } public function testProcessMessageLinksWhenNoLinksExtracted(): void { - $message = $this->createMock(Message::class); - $userId = 123; + $messageId = 1; + $messageContent = $this->createMock(MessageContent::class); + $subscriber = $this->createMock(Subscriber::class); + $subscriber->method('getId')->willReturn(123); + $subscriber->method('getEmail')->willReturn('test@example.com'); $this->linkTrackService->expects($this->once()) ->method('isExtractAndSaveLinksApplicable') @@ -147,22 +162,23 @@ public function testProcessMessageLinksWhenNoLinksExtracted(): void $this->linkTrackService->expects($this->once()) ->method('extractAndSaveLinks') - ->with($message, $userId) + ->with($messageContent, 123, $messageId) ->willReturn([]); - $message->expects($this->never()) - ->method('getContent'); + $messageContent->expects($this->never()) + ->method('getText'); - $result = $this->preparator->processMessageLinks($message, $userId); + $result = $this->preparator->processMessageLinks($messageId, $messageContent, $subscriber); - $this->assertSame($message, $result); + $this->assertSame($messageContent, $result); } public function testProcessMessageLinksWithLinksExtracted(): void { - $message = $this->createMock(Message::class); $content = $this->createMock(MessageContent::class); - $userId = 123; + $subscriber = $this->createMock(Subscriber::class); + $subscriber->method('getId')->willReturn(123); + $subscriber->method('getEmail')->willReturn('test@example.com'); $linkTrack1 = $this->createMock(LinkTrack::class); $linkTrack1->method('getId')->willReturn(1); @@ -177,11 +193,9 @@ public function testProcessMessageLinksWithLinksExtracted(): void $this->linkTrackService->method('isExtractAndSaveLinksApplicable')->willReturn(true); $this->linkTrackService ->method('extractAndSaveLinks') - ->with($message, $userId) + ->with($content, 123, 1) ->willReturn($savedLinks); - $message->method('getContent')->willReturn($content); - $htmlContent = 'Link 1 Link 2'; $content->method('getText')->willReturn($htmlContent); @@ -190,14 +204,14 @@ public function testProcessMessageLinksWithLinksExtracted(): void $content->expects($this->once()) ->method('setText') - ->with($this->stringContains(MessageProcessingPreparator::LINT_TRACK_ENDPOINT . '?id=1')); + ->with($this->stringContains(MessageProcessingPreparator::LINK_TRACK_ENDPOINT . '?id=1')); $content->expects($this->once()) ->method('setFooter') - ->with($this->stringContains(MessageProcessingPreparator::LINT_TRACK_ENDPOINT . '?id=1')); + ->with($this->stringContains(MessageProcessingPreparator::LINK_TRACK_ENDPOINT . '?id=1')); - $result = $this->preparator->processMessageLinks($message, $userId); + $result = $this->preparator->processMessageLinks(1, $content, $subscriber); - $this->assertSame($message, $result); + $this->assertSame($content, $result); } } diff --git a/tests/Unit/Domain/Messaging/Service/RateLimitedCampaignMailerTest.php b/tests/Unit/Domain/Messaging/Service/RateLimitedCampaignMailerTest.php index 6e2011ff..97d4e158 100644 --- a/tests/Unit/Domain/Messaging/Service/RateLimitedCampaignMailerTest.php +++ b/tests/Unit/Domain/Messaging/Service/RateLimitedCampaignMailerTest.php @@ -46,7 +46,7 @@ public function testComposeEmailSetsHeadersAndBody(): void $subscriber = new Subscriber(); $this->setSubscriberEmail($subscriber, 'user@example.com'); - $email = $this->sut->composeEmail($message, $subscriber); + $email = $this->sut->composeEmail($message, $subscriber, $message->getContent()); $this->assertInstanceOf(Email::class, $email); $this->assertSame('user@example.com', $email->getTo()[0]->getAddress()); @@ -70,7 +70,7 @@ public function testComposeEmailWithoutOptionalHeaders(): void $subscriber = new Subscriber(); $this->setSubscriberEmail($subscriber, 'user2@example.com'); - $email = $this->sut->composeEmail($message, $subscriber); + $email = $this->sut->composeEmail($message, $subscriber, $message->getContent()); $this->assertSame('user2@example.com', $email->getTo()[0]->getAddress()); $this->assertSame('No headers', $email->getSubject());