From 27572acf1ae15c31b6b9df6b0ceffedf7689e7a5 Mon Sep 17 00:00:00 2001
From: Georgi Mateev <gmateev@exactag.com>
Date: Thu, 6 Feb 2020 15:41:14 +0200
Subject: [PATCH] [BUGFIX] 1604 Fixed changes to the API, internal refactoring
- Added static runtime cache
---
Classes/Controller/MailController.php | 1 -
Classes/Controller/NewsletterController.php | 6 +-
Classes/Service/MailTemplateService.php | 367 +++++++++++---------
3 files changed, 199 insertions(+), 175 deletions(-)
diff --git a/Classes/Controller/MailController.php b/Classes/Controller/MailController.php
index 613d82fe..b00caa5b 100644
--- a/Classes/Controller/MailController.php
+++ b/Classes/Controller/MailController.php
@@ -379,7 +379,6 @@ class MailController extends ActionController {
foreach ((array) $parameters['templates'] as $key => $template) {
$mailTemplateService->setLanguage($key);
$mailTemplateService->setToAddresses($parameters['emailAddress']);
-// $mailTemplateService->setFromAddress($template['fromMail']);
$mailTemplateService->setTemplateName($parameters['selectedTemplate']);
$mailTemplateService->setExtensionKey($parameters['selectedExtension']);
$mailTemplateService->setPreviewMarkers();
diff --git a/Classes/Controller/NewsletterController.php b/Classes/Controller/NewsletterController.php
index 58d123f2..8618c144 100644
--- a/Classes/Controller/NewsletterController.php
+++ b/Classes/Controller/NewsletterController.php
@@ -426,8 +426,6 @@ class NewsletterController extends ActionController {
$mailTemplateService->setOverwrittenFromMail($parameter['fromMail']);
$mailTemplateService->setSubject($parameter['subject']);
$mailTemplateService->setOverwrittenReplyTo($parameter['replyTo']);
- $siteRootId = $mailTemplateService->getSiteRootId();
- $templateObject = $mailTemplateService->getTemplate($siteRootId);
if (!$this->request->getArgument('sendRealEmails')) {
// Send test emails
@@ -456,7 +454,7 @@ class NewsletterController extends ActionController {
'www' => ' www.example.com',
]
]);
- $mailIsSend = $mailTemplateService->sendEmail(TRUE, $templateObject);
+ $mailIsSend = $mailTemplateService->sendEmail(TRUE);
}
} else {
@@ -487,7 +485,7 @@ class NewsletterController extends ActionController {
$mailTemplateService->setMarkers(['user' => $recipient]);
// no real error handling here, one must check the MailQueue
- $mailTemplateService->sendEmail(FALSE, $templateObject);
+ $mailTemplateService->sendEmail(FALSE);
} catch (\Exception $e) {
// Invalid email address could not be loaded to queue
$errorRecipients[] = $recipient['uid'] . ' - '
diff --git a/Classes/Service/MailTemplateService.php b/Classes/Service/MailTemplateService.php
index fe4a828a..55aba627 100644
--- a/Classes/Service/MailTemplateService.php
+++ b/Classes/Service/MailTemplateService.php
@@ -63,6 +63,16 @@ class MailTemplateService {
const REGISTER_FILE = 'Register.php';
const CONFIG_PATH = 'Configuration/MailTemplates';
+ /**
+ * @var Template
+ */
+ private static $template;
+
+ /**
+ * @var array
+ */
+ private static $mailObjectCache = [];
+
/**
* @var string $toAddresses
*/
@@ -211,6 +221,97 @@ class MailTemplateService {
*/
private $subjectToSend;
+ /**
+ * @param Template $template
+ */
+ public static function setTemplate(Template $template): void {
+ self::$template = $template;
+ }
+
+ /**
+ * @return string
+ */
+ public function getOverwrittenBcc(): string {
+ return $this->overwrittenBcc;
+ }
+
+ /**
+ * @param string $overwrittenBcc
+ */
+ public function setOverwrittenBcc(string $overwrittenBcc): void {
+ $this->overwrittenBcc = $overwrittenBcc;
+ }
+
+ /**
+ * @return string
+ */
+ public function getOverwrittenCc(): string {
+ return $this->overwrittenCc;
+ }
+
+ /**
+ * @param string $overwrittenCc
+ */
+ public function setOverwrittenCc(string $overwrittenCc): void {
+ $this->overwrittenCc = $overwrittenCc;
+ }
+
+ /**
+ * @return string
+ */
+ public function getOverwrittenFromName(): string {
+ return $this->overwrittenFromName;
+ }
+
+ /**
+ * @param string $overwrittenFromName
+ */
+ public function setOverwrittenFromName(string $overwrittenFromName): void {
+ $this->overwrittenFromName = $overwrittenFromName;
+ }
+
+ /**
+ * @return string
+ */
+ public function getOverwrittenFromMail(): string {
+ return $this->overwrittenFromMail;
+ }
+
+ /**
+ * @param string $overwrittenFromMail
+ */
+ public function setOverwrittenFromMail(string $overwrittenFromMail): void {
+ $this->overwrittenFromMail = $overwrittenFromMail;
+ }
+
+ /**
+ * @return string
+ */
+ public function getOverwrittenReplyTo(): string {
+ return $this->overwrittenReplyTo;
+ }
+
+ /**
+ * @param string $overwrittenReplyTo
+ */
+ public function setOverwrittenReplyTo(string $overwrittenReplyTo): void {
+ $this->overwrittenReplyTo = $overwrittenReplyTo;
+ }
+
+ /**
+ * @return string
+ */
+ public function getOverwrittenToAddresses(): string {
+ return $this->overwrittenToAddresses;
+ }
+
+ /**
+ * @param string $overwrittenToAddresses
+ */
+ public function setOverwrittenToAddresses(string $overwrittenToAddresses): void {
+ $this->overwrittenToAddresses = $overwrittenToAddresses;
+ }
+
/**
* MailTemplateService constructor.
*
@@ -401,6 +502,34 @@ class MailTemplateService {
return $this;
}
+ /**
+ * @return string|string[]|null
+ */
+ public function getMailBodyToSend() {
+ return $this->mailBodyToSend;
+ }
+
+ /**
+ * @param string|string[]|null $mailBodyToSend
+ */
+ public function setMailBodyToSend($mailBodyToSend): void {
+ $this->mailBodyToSend = $mailBodyToSend;
+ }
+
+ /**
+ * @return string
+ */
+ public function getSubjectToSend(): string {
+ return $this->subjectToSend;
+ }
+
+ /**
+ * @param string $subjectToSend
+ */
+ public function setSubjectToSend(string $subjectToSend): void {
+ $this->subjectToSend = $subjectToSend;
+ }
+
/**
* @param array $markers
* @return MailTemplateService
@@ -674,28 +803,37 @@ class MailTemplateService {
$pageUid = $this->pid;
}
- //TODO: move to page repository
if ($pageUid === 0) {
- $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable(
- 'pages'
- );
- $rootPageRows = $queryBuilder->select('*')
- ->from('pages')
- ->where(
- $queryBuilder->expr()->eq(
- 'is_siteroot', 1
- )
+ $pageUid = $this->getAnyRootPageUid();
+ }
+ return $pageUid;
+ }
+
+ /**
+ * Gets the page uid of any root page in the page tree
+ *
+ * @return int
+ */
+ private function getAnyRootPageUid() {
+ $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable(
+ 'pages'
+ );
+ $rootPageRows = $queryBuilder->select('*')
+ ->from('pages')
+ ->where(
+ $queryBuilder->expr()->eq(
+ 'is_siteroot', 1
)
- ->andWhere(
- $queryBuilder->expr()->eq(
- 'hidden', 0
- )
+ )
+ ->andWhere(
+ $queryBuilder->expr()->eq(
+ 'hidden', 0
)
- ->execute()->fetchAll();
+ )
+ ->execute()->fetchAll();
- if ($rootPageRows && \count($rootPageRows)) {
- $pageUid = (int) $rootPageRows[0]['uid'];
- }
+ if ($rootPageRows && \count($rootPageRows)) {
+ $pageUid = (int) $rootPageRows[0]['uid'];
}
return $pageUid;
}
@@ -707,7 +845,7 @@ class MailTemplateService {
* @return null|object|Template|FALSE
* @throws \Exception
*/
- public function getTemplate(int $siteRootId) {
+ public function getTemplate($siteRootId) {
$isTemplateBlacklisted = self::isTemplateBlacklisted($this->extensionKey, $this->templateName, $siteRootId);
if ($isTemplateBlacklisted) {
throw new \Exception('The template is blacklisted');
@@ -765,12 +903,14 @@ class MailTemplateService {
}
/**
- * @param $template
+ * Sets the values to send the mail with from the template or register service
+ *
+ * @param Template|null $template
* @param $registerService
* @param $defaultTemplateContent
* @param $siteRootId
*/
- protected function setMailValuesFromObjects($template, $registerService, $defaultTemplateContent, $siteRootId): void {
+ protected function extractValuesForMail($template, $registerService, $defaultTemplateContent, $siteRootId): void {
/** @var StandaloneView $emailView */
$emailView = $this->objectManager->get(StandaloneView::class);
$emailView->assignMultiple($this->markers);
@@ -814,10 +954,7 @@ class MailTemplateService {
if ($this->fromAddress === '') {
$fromMail = $this->parseMarkers(
- $this->getTemplateSource(
- (empty($this->overwrittenFromMail) ? $template->getFromMail() : $this->overwrittenFromMail),
- $layoutId, $siteRootId
- ),
+ empty($this->overwrittenFromMail) ? $template->getFromMail() : $this->overwrittenFromMail,
$emailView
);
$this->setFromAddress($fromMail);
@@ -825,10 +962,7 @@ class MailTemplateService {
if ($this->fromName === '') {
$fromName = $this->parseMarkers(
- $this->getTemplateSource(
- (empty($this->overwrittenFromName) ? $template->getFromName() : $this->overwrittenFromName),
- $layoutId, $siteRootId
- ),
+ (empty($this->overwrittenFromName) ? $template->getFromName() : $this->overwrittenFromName),
$emailView
);
$this->setFromName($fromName);
@@ -836,10 +970,7 @@ class MailTemplateService {
if ($this->replyToAddress === '') {
$replyTo = $this->parseMarkers(
- $this->getTemplateSource(
- (empty($this->overwrittenReplyTo) ? $template->getReplyTo() : $this->overwrittenReplyTo),
- $layoutId, $siteRootId
- ),
+ (empty($this->overwrittenReplyTo) ? $template->getReplyTo() : $this->overwrittenReplyTo),
$emailView
);
$this->setReplyToAddress($replyTo);
@@ -847,10 +978,7 @@ class MailTemplateService {
if (empty($this->ccAddresses)) {
$cc = $this->parseMarkers(
- $this->getTemplateSource(
- (empty($this->overwrittenCc) ? $template->getCc() : $this->overwrittenCc),
- $layoutId, $siteRootId
- ),
+ (empty($this->overwrittenCc) ? $template->getCc() : $this->overwrittenCc),
$emailView
);
$this->setCcAddresses($cc);
@@ -858,10 +986,7 @@ class MailTemplateService {
if (empty($this->bccAddresses)) {
$bcc = $this->parseMarkers(
- $this->getTemplateSource(
- (empty($this->overwrittenBcc) ? $template->getBcc() : $this->overwrittenBcc),
- $layoutId, $siteRootId
- ),
+ (empty($this->overwrittenBcc) ? $template->getBcc() : $this->overwrittenBcc),
$emailView
);
$this->setBccAddresses($bcc);
@@ -869,10 +994,7 @@ class MailTemplateService {
if ($this->toAddresses === '') {
$toAddress = $this->parseMarkers(
- $this->getTemplateSource(
- (empty($this->overwrittenToAddresses) ? $template->getToAddress() : $this->overwrittenToAddresses),
- $layoutId, $siteRootId
- ),
+ (empty($this->overwrittenToAddresses) ? $template->getToAddress() : $this->overwrittenToAddresses),
$emailView
);
$this->setToAddresses($toAddress);
@@ -913,18 +1035,19 @@ class MailTemplateService {
* @throws \TYPO3\CMS\Extbase\Persistence\Exception\IllegalObjectTypeException
* @throws \TYPO3\CMS\Extbase\Persistence\Exception\UnknownObjectException
*/
- public function sendEmail($isPreview = FALSE, Template $template = NULL): bool {
+ public function sendEmail($isPreview = FALSE): bool {
if ($isPreview) { //TODO: remove this from here
$this->setIgnoreMailQueue(TRUE);
}
$success = FALSE;
// Get page ID
+ // TODO: this doesn't belong here. The API user needs to provide the UID
$siteRootId = $this->getSiteRootId();
- if ($template === NULL) {
+ if (self::$template === NULL) {
// Get template
try {
- $template = $this->getTemplate($siteRootId);
+ self::$template = $this->getTemplate($siteRootId);
} catch (\Exception $e) {
return FALSE;
}
@@ -932,22 +1055,23 @@ class MailTemplateService {
$registerService = GeneralUtility::makeInstance(RegisterService::class);
// get default template content from register array
- $defaultTemplateContent = $this->getDefaultTemplateContent($template, $registerService);
+ $defaultTemplateContent = $this->getDefaultTemplateContent(self::$template, $registerService);
// set the ToAddress if there are no placeholders in it
// TODO: does this belong here?
- if ($template !== NULL && \filter_var($template->getToAddress(), FILTER_VALIDATE_EMAIL)) {
- $this->setToAddresses(\trim($template->getToAddress()));
+ if (self::$template !== NULL && \filter_var(self::$template->getToAddress(), FILTER_VALIDATE_EMAIL)) {
+ $this->setToAddresses(\trim(self::$template->getToAddress()));
}
- $this->setMailValuesFromObjects($template, $registerService, $defaultTemplateContent, $siteRootId);
+ $this->extractValuesForMail(self::$template, $registerService, $defaultTemplateContent, $siteRootId);
$mail = $this->addMailToMailQueue(
$this->extensionKey, $this->templateName, $this->getSubjectToSend(), $this->getMailBodyToSend(), $this->priority,
0, 0, $this->language, $siteRootId
);
+ self::$mailObjectCache[$mail->getUid()] = $mail; // add it to cache to avoid extra DB queries
if ($this->ignoreMailQueue) {
- $success = $this->sendMailFromQueue($mail->getUid(), $mail);
+ $success = $this->sendMailFromQueue($mail->getUid());
}
//TODO: this can be avoided if the sending logic is decoupled from this function
@@ -1085,24 +1209,38 @@ class MailTemplateService {
return $mail;
}
+ /**
+ * Get the mail object by uid and check if it's blacklisted
+ * @param $uid
+ */
+ private function getMailObjectByUid($uid) {
+ $mailRepository = $this->objectManager->get(MailRepository::class);
+ $mailObject = $mailRepository->findOneByUid($uid);
+ if (!$mailObject || $mailObject->getBlacklisted()) {
+ return FALSE;
+ }
+ return $mailObject;
+ }
+
/**
* Send a Mail from the queue, identified by its id
*
* @param int $uid
- * @param null $mailToSend
* @return bool|NULL
* @throws \TYPO3\CMS\Core\Resource\Exception\ResourceDoesNotExistException
* @throws \TYPO3\CMS\Extbase\Persistence\Exception\IllegalObjectTypeException
* @throws \TYPO3\CMS\Extbase\Persistence\Exception\UnknownObjectException
*/
- public function sendMailFromQueue($uid, $mailToSend = NULL): bool {
+ public function sendMailFromQueue($uid): bool {
$mailRepository = $this->objectManager->get(MailRepository::class);
/** @var Mail $mailToSend */
- if ($mailToSend === NULL) {
- $mailToSend = $mailRepository->findOneByUid($uid);
- if (!$mailToSend || $mailToSend->getBlacklisted()) {
+ if (!isset(self::$mailObjectCache[$uid]) || !self::$mailObjectCache[$uid] instanceof Mail) {
+ $mailToSend = $this->getMailObjectByUid($uid);
+ if ($mailToSend === FALSE) {
return FALSE;
}
+ } else {
+ $mailToSend = self::$mailObjectCache[$uid];
}
$this->mailMessage->setBody($mailToSend->getMailBody(), 'text/html');
@@ -1153,121 +1291,10 @@ class MailTemplateService {
} else {
$this->mailMessage->getFailedRecipients();
}
+ unset(self::$mailObjectCache[$uid]); // free the memory
return $success;
}
- /**
- * @return string
- */
- public function getOverwrittenBcc(): string {
- return $this->overwrittenBcc;
- }
-
- /**
- * @param string $overwrittenBcc
- */
- public function setOverwrittenBcc(string $overwrittenBcc): void {
- $this->overwrittenBcc = $overwrittenBcc;
- }
-
- /**
- * @return string
- */
- public function getOverwrittenCc(): string {
- return $this->overwrittenCc;
- }
-
- /**
- * @param string $overwrittenCc
- */
- public function setOverwrittenCc(string $overwrittenCc): void {
- $this->overwrittenCc = $overwrittenCc;
- }
-
- /**
- * @return string
- */
- public function getOverwrittenFromName(): string {
- return $this->overwrittenFromName;
- }
-
- /**
- * @param string $overwrittenFromName
- */
- public function setOverwrittenFromName(string $overwrittenFromName): void {
- $this->overwrittenFromName = $overwrittenFromName;
- }
-
- /**
- * @return string
- */
- public function getOverwrittenFromMail(): string {
- return $this->overwrittenFromMail;
- }
-
- /**
- * @param string $overwrittenFromMail
- */
- public function setOverwrittenFromMail(string $overwrittenFromMail): void {
- $this->overwrittenFromMail = $overwrittenFromMail;
- }
-
- /**
- * @return string
- */
- public function getOverwrittenReplyTo(): string {
- return $this->overwrittenReplyTo;
- }
-
- /**
- * @param string $overwrittenReplyTo
- */
- public function setOverwrittenReplyTo(string $overwrittenReplyTo): void {
- $this->overwrittenReplyTo = $overwrittenReplyTo;
- }
-
- /**
- * @return string
- */
- public function getOverwrittenToAddresses(): string {
- return $this->overwrittenToAddresses;
- }
-
- /**
- * @param string $overwrittenToAddresses
- */
- public function setOverwrittenToAddresses(string $overwrittenToAddresses): void {
- $this->overwrittenToAddresses = $overwrittenToAddresses;
- }
-
- /**
- * @return string|string[]|null
- */
- public function getMailBodyToSend() {
- return $this->mailBodyToSend;
- }
-
- /**
- * @param string|string[]|null $mailBodyToSend
- */
- public function setMailBodyToSend($mailBodyToSend): void {
- $this->mailBodyToSend = $mailBodyToSend;
- }
-
- /**
- * @return string
- */
- public function getSubjectToSend(): string {
- return $this->subjectToSend;
- }
-
- /**
- * @param string $subjectToSend
- */
- public function setSubjectToSend(string $subjectToSend): void {
- $this->subjectToSend = $subjectToSend;
- }
-
/**
* Sets the fromMail property of the mailTemplateService.
* Checks validity and uses all available fallbacks
--
GitLab