From 32d3b178f828d3533e18eed7da3df18017d219dd Mon Sep 17 00:00:00 2001 From: Daniel Seifert Date: Tue, 4 Jun 2024 14:37:41 +0200 Subject: [PATCH] improve code --- .../Controller/Admin/MailCheckMenu.php | 2 +- .../Controller/Admin/MailConfigCheck.php | 10 +-- Application/Controller/Admin/MailTester.php | 9 ++- Application/Controller/Admin/SmtpChecker.php | 50 ++++++------ Application/Controller/Admin/SpfChecker.php | 81 +++++++------------ Application/Model/DmarcResult.php | 4 +- Application/Model/SpfResult.php | 8 +- composer.json | 1 + 8 files changed, 76 insertions(+), 89 deletions(-) diff --git a/Application/Controller/Admin/MailCheckMenu.php b/Application/Controller/Admin/MailCheckMenu.php index 01330e6..c820211 100644 --- a/Application/Controller/Admin/MailCheckMenu.php +++ b/Application/Controller/Admin/MailCheckMenu.php @@ -22,7 +22,7 @@ class MailCheckMenu extends AdminListController { protected $_sThisTemplate = '@'.Constants::OXID_MODULE_ID.'/admin/mailCheckMenu'; - public function render() + public function render(): string { $this->setEditObjectId('foo'); return parent::render(); diff --git a/Application/Controller/Admin/MailConfigCheck.php b/Application/Controller/Admin/MailConfigCheck.php index e96ef82..e481b8a 100644 --- a/Application/Controller/Admin/MailConfigCheck.php +++ b/Application/Controller/Admin/MailConfigCheck.php @@ -26,9 +26,9 @@ use OxidEsales\Eshop\Core\Registry; class MailConfigCheck extends AdminDetailsController { protected $_sThisTemplate = '@'.Constants::OXID_MODULE_ID.'/admin/mailConfigCheck'; - protected $testMailAddress = 'test@example.com'; + protected string $testMailAddress = 'test@example.com'; - public function render() + public function render(): string { $this->checkDataAreSet(); $this->addTplParam('shop', Registry::getConfig()->getActiveShop()); @@ -36,7 +36,7 @@ class MailConfigCheck extends AdminDetailsController return parent::render(); } - protected function checkDataAreSet() + protected function checkDataAreSet(): void { try { /** @var Shop $shop */ @@ -64,12 +64,12 @@ class MailConfigCheck extends AdminDetailsController } } - public function checkConfiguration() + public function checkConfiguration(): void { $this->getCurrentMailer(); } - protected function getCurrentMailer() + protected function getCurrentMailer(): void { try { $shop = Registry::getConfig()->getActiveShop(); diff --git a/Application/Controller/Admin/MailTester.php b/Application/Controller/Admin/MailTester.php index 670d8cf..7723405 100644 --- a/Application/Controller/Admin/MailTester.php +++ b/Application/Controller/Admin/MailTester.php @@ -16,6 +16,7 @@ namespace D3\MailConfigChecker\Application\Controller\Admin; use Assert\Assert; use D3\MailConfigChecker\Application\Model\Constants; use D3\MailConfigChecker\Application\Model\Exception\d3TranslatableLazyAssertionException; +use Exception; use OxidEsales\Eshop\Application\Controller\Admin\AdminDetailsController; use OxidEsales\Eshop\Application\Model\Shop; use OxidEsales\Eshop\Core\Email; @@ -25,7 +26,7 @@ class MailTester extends AdminDetailsController { protected $_sThisTemplate = '@'.Constants::OXID_MODULE_ID.'/admin/mailTester'; - public function sendMail() + public function sendMail(): void { try { $request = Registry::getRequest(); @@ -45,12 +46,12 @@ class MailTester extends AdminDetailsController $mail->setFrom($from); $mail->sendEmail($to, $subject, $body); $this->addTplParam('success', true); - } catch (\Exception $e) { + } catch ( Exception $e) { Registry::getUtilsView()->addErrorToDisplay(nl2br($e->getMessage())); } } - protected function assertContent() + protected function assertContent(): void { $request = Registry::getRequest(); $lang = Registry::getLang(); @@ -76,7 +77,7 @@ class MailTester extends AdminDetailsController ->verifyNow(); } - public function getMailAddressList() + public function getMailAddressList(): array { /** @var Shop $shop */ $shop = Registry::getConfig()->getActiveShop(); diff --git a/Application/Controller/Admin/SmtpChecker.php b/Application/Controller/Admin/SmtpChecker.php index 311a8f3..0793ec0 100644 --- a/Application/Controller/Admin/SmtpChecker.php +++ b/Application/Controller/Admin/SmtpChecker.php @@ -27,18 +27,18 @@ use PEAR_Error; class SmtpChecker extends AdminDetailsController { - protected $debug = true; + protected bool $debug = true; - protected $host; - protected $port; - protected $user; - protected $pwd; - protected $from; - protected $to; + protected string $host; + protected int $port; + protected string $user; + protected string $pwd; + protected string $from; + protected ?string $to = null; - protected $smtp; - protected $action; - protected $log = []; + protected Net_SMTP $smtp; + protected string $action = ''; + protected array $log = []; public function __construct() { @@ -69,19 +69,19 @@ class SmtpChecker extends AdminDetailsController $this->addTplParam('smtpLog', $this->log); } - public function getTemplateName() + public function getTemplateName(): string { return '@'.Constants::OXID_MODULE_ID.'/admin/smtpCheck'; } - public function render() + public function render(): ?string { $this->addTplParam('shop', Registry::getConfig()->getActiveShop()); return parent::render(); } - public function getMailAddressList() + public function getMailAddressList(): array { /** @var Shop $shop */ $shop = Registry::getConfig()->getActiveShop(); @@ -97,7 +97,7 @@ class SmtpChecker extends AdminDetailsController ); } - public function sendMail() + public function sendMail(): void { try { $this->hostIsAvailable(); @@ -120,7 +120,7 @@ class SmtpChecker extends AdminDetailsController * @throws InvalidArgumentException * @return void */ - protected function hostIsAvailable() + protected function hostIsAvailable(): void { $this->action = __FUNCTION__; Assert::that( @@ -133,7 +133,7 @@ class SmtpChecker extends AdminDetailsController $this->smtp->setDebug($this->debug, [$this, 'dumpDebug']); } - protected function connect() + protected function connect(): void { $this->action = __FUNCTION__; Assert::that( @@ -150,7 +150,7 @@ class SmtpChecker extends AdminDetailsController * @throws InvalidArgumentException * @return void */ - protected function auth() + protected function auth(): void { $this->action = __FUNCTION__; Assert::that( @@ -163,7 +163,7 @@ class SmtpChecker extends AdminDetailsController ); } - protected function setFrom() + protected function setFrom(): void { $this->action = __FUNCTION__; Assert::that( @@ -176,8 +176,12 @@ class SmtpChecker extends AdminDetailsController ); } - protected function setRecipient() + protected function setRecipient(): void { + if (!$this->to) { + return; + } + $this->action = __FUNCTION__; Assert::that( $this->smtp->rcptTo($this->to), @@ -189,7 +193,7 @@ class SmtpChecker extends AdminDetailsController ); } - protected function sendContent() + protected function sendContent(): void { if (!Registry::getRequest()->getRequestEscapedParameter('sendmail')) { return; @@ -208,13 +212,13 @@ class SmtpChecker extends AdminDetailsController ); } - protected function disconnect() + protected function disconnect(): void { $this->action = __FUNCTION__; $this->smtp->disconnect(); } - public function dumpDebug($smtp, $message) + public function dumpDebug($smtp, $message): void { unset($smtp); @@ -224,7 +228,7 @@ class SmtpChecker extends AdminDetailsController $this->log[$this->action][] = trim(htmlentities($message)); } - public function formatMessage(array $arguments) + public function formatMessage(array $arguments): ?string { /** @var PEAR_Error|true $response */ $response = $arguments['value']; diff --git a/Application/Controller/Admin/SpfChecker.php b/Application/Controller/Admin/SpfChecker.php index 3388cee..728ddc3 100644 --- a/Application/Controller/Admin/SpfChecker.php +++ b/Application/Controller/Admin/SpfChecker.php @@ -21,6 +21,7 @@ use D3\MailAuthenticationCheck\Model\DMARCResult; use D3\MailConfigChecker\Application\Model\Constants; use D3\MailConfigChecker\Application\Model\DmarcResult as OxDmarcResult; use D3\MailConfigChecker\Application\Model\SpfResult; +use LogicException; use Mika56\SPFCheck\DNS\DNSRecordGetter; use Mika56\SPFCheck\Model\Query; use Mika56\SPFCheck\Model\Result; @@ -33,14 +34,14 @@ class SpfChecker extends AdminDetailsController { protected $_sThisTemplate = '@'.Constants::OXID_MODULE_ID.'/admin/spfCheck'; - public function render() + public function render(): ?string { $this->checkSpf(); $this->checkDmarc(); return parent::render(); } - protected function checkSpf() + protected function checkSpf(): void { $result = []; $mailDomains = $this->getMailDomains(); @@ -54,7 +55,7 @@ class SpfChecker extends AdminDetailsController $this->addTplParam('spf_result', $result); } - protected function checkDmarc() + protected function checkDmarc(): void { $result = []; $mailDomains = $this->getMailDomains(); @@ -80,13 +81,13 @@ class SpfChecker extends AdminDetailsController function ($mailAddress) { $mailAddress = trim($mailAddress); try { - if (! strstr($mailAddress, '@')) { + if ( ! str_contains( $mailAddress, '@' ) ) { throw oxNew(InvalidArgumentException::class); } $addressChunks = explode('@', $mailAddress); return array_pop($addressChunks); - } catch (InvalidArgumentException $e) { + } catch (InvalidArgumentException) { return ''; } }, @@ -100,25 +101,17 @@ class SpfChecker extends AdminDetailsController ); } - protected function checkSpfByDomain($domain, &$summarize) + protected function checkSpfByDomain($domain, &$summarize): void { $checker = new SPFCheck(new DNSRecordGetter()); $query = new Query('', $domain); $result = $checker->getResult($query); - switch ($result->getResult()) { - case Result::FAIL: - case Result::NEUTRAL: - case Result::PASS: - case Result::SOFTFAIL: - $status = SpfResult::SET; - break; - case Result::NONE: - $status = SpfResult::MISSING; - break; - default: - $status = SpfResult::ERROR; - } + $status = match ( $result->getResult() ) { + Result::FAIL, Result::NEUTRAL, Result::PASS, Result::SOFTFAIL => SpfResult::SET, + Result::NONE => SpfResult::MISSING, + default => SpfResult::ERROR, + }; $rawRecord = ($record = $result->getRecord()) ? $record->getRawRecord() : @@ -127,52 +120,40 @@ class SpfChecker extends AdminDetailsController $summarize[$domain] = oxNew(SpfResult::class, $status, $rawRecord); } - public function getSpfStatusColor(SpfResult $result) + public function getSpfStatusColor(SpfResult $result): string { - switch ($result->getStatus()) { - case SpfResult::SET: - return 'success'; - case SpfResult::ERROR: - return 'warning'; - default: - return 'danger'; - } + return match ( $result->getStatus() ) { + SpfResult::SET => 'success', + SpfResult::ERROR => 'warning', + default => 'danger', + }; } - public function checkDmarcByDomain($domain, &$summarize) + public function checkDmarcByDomain($domain, &$summarize): void { try { $check = new DMARCCheck(new DNSRecordGetter()); $query = new Query('', $domain); $record = $check->getResult($query)->getRecord(); - switch ( $record->getRejectPolicy()->getValue() ) { - case DMARCResult::REJECT_QUARANTINE: - case DMARCResult::REJECT_REJECT: - $status = OxDmarcResult::SET; - break; - case DMARCResult::REJECT_NONE: - $status = OxDmarcResult::MISSING; - break; - default: - $status = OxDmarcResult::ERROR; - } + $status = match ( $record->getRejectPolicy()->getValue() ) { + DMARCResult::REJECT_QUARANTINE, DMARCResult::REJECT_REJECT => OxDmarcResult::SET, + DMARCResult::REJECT_NONE => OxDmarcResult::MISSING, + default => OxDmarcResult::ERROR, + }; $summarize[$domain] = oxNew( OxDmarcResult::class, $status, $record->getRawRecord()); - } catch (\LogicException) { + } catch ( LogicException) { $summarize[$domain] = oxNew( OxDmarcResult::class, OxDmarcResult::MISSING, ''); } } - public function getDmarcStatusColor(OxDmarcResult $result) + public function getDmarcStatusColor(OxDmarcResult $result): string { - switch ($result->getStatus()) { - case SpfResult::SET: - return 'success'; - case SpfResult::ERROR: - return 'warning'; - default: - return 'danger'; - } + return match ( $result->getStatus() ) { + SpfResult::SET => 'success', + SpfResult::ERROR => 'warning', + default => 'danger', + }; } } diff --git a/Application/Model/DmarcResult.php b/Application/Model/DmarcResult.php index 5d6e0a4..c32f1fb 100644 --- a/Application/Model/DmarcResult.php +++ b/Application/Model/DmarcResult.php @@ -30,12 +30,12 @@ class DmarcResult $this->record = $record; } - public function getStatus() + public function getStatus(): string { return $this->status; } - public function getRecord() + public function getRecord(): ?string { return $this->record; } diff --git a/Application/Model/SpfResult.php b/Application/Model/SpfResult.php index e8e1cfe..28bb255 100644 --- a/Application/Model/SpfResult.php +++ b/Application/Model/SpfResult.php @@ -21,8 +21,8 @@ class SpfResult public const MISSING = 'missing'; public const ERROR = 'error'; - protected $status; - protected $record; + protected string $status; + protected ?string $record; public function __construct(string $status, ?string $record = null) { @@ -30,12 +30,12 @@ class SpfResult $this->record = $record; } - public function getStatus() + public function getStatus(): string { return $this->status; } - public function getRecord() + public function getRecord(): ?string { return $this->record; } diff --git a/composer.json b/composer.json index 3458ebf..7731668 100644 --- a/composer.json +++ b/composer.json @@ -34,6 +34,7 @@ "oxid-esales/oxideshop-ce": "7.0 - 7.1", "pear/net_smtp": "^1.11", "mika56/spfcheck": "^2.1.1", + "d3/mailauthenticationcheck": "^0.1.0", "beberlei/assert": "^3.3" }, "require-dev": {