fix: UI and tests

config/config.sample.php

@@ -396,7 +396,6 @@
 /**
  * Enable or disable SMTP class debugging
  */
-# TODO: unused / deprecated
 'mail_smtpdebug' => false,
 
 /**
@@ -439,7 +438,7 @@
 
 /**
  * Define the SMTP security style
- * Depends on `mail_smtpmode`. Specify when you are using `ssl` or `tls`.
+ * Depends on `mail_smtpmode`. Specify when you are using `ssl` or not.
  * Leave empty for no encryption.
  */
 'mail_smtpsecure' => '',

lib/private/Mail/Logger.php

@@ -0,0 +1,20 @@
+<?php
+
+namespace OC\Mail;
+
+use Psr\Log\AbstractLogger;
+
+class Logger extends AbstractLogger {
+	private array $log;
+
+	public function log($level, $message, array $context = []) {
+		$this->log[] = [$level, $message, $context];
+	}
+
+	/**
+	 * @throws \JsonException
+	 */
+	public function toJSON(): string {
+		return json_encode($this->log, JSON_THROW_ON_ERROR);
+	}
+}

lib/private/Mail/Mailer.php

@@ -27,6 +27,7 @@
 use OCP\IConfig;
 use OCP\Mail\IMailer;
 use OCP\ILogger;
+use Psr\Log\LoggerInterface;
 use Symfony\Component\Mailer\Exception\TransportExceptionInterface;
 use Symfony\Component\Mailer\Transport\SendmailTransport;
 use Symfony\Component\Mailer\Transport\Smtp\EsmtpTransport;
@@ -86,19 +87,31 @@ public function createMessage(): Message {
 	 * has been supplied.)
 	 */
 	public function send(Message $message): array {
-		# TODO: implement this ????
-		$debugMode = $this->config->getSystemValue('mail_smtpdebug', false);
-
 		if (!\is_array($message->getFrom()) || \count($message->getFrom()) === 0) {
 			$message->setFrom([\OCP\Util::getDefaultEmailAddress($this->defaults->getName())]);
 		}
 
-		$failedRecipients = [];
+		$debugMode = $this->config->getSystemValue('mail_smtpdebug', false);
+		$logger = $debugMode ? new Logger() : null;
 
 		try {
-			$this->getInstance()->send($message->getMessage());
+			$this->getInstance($logger ?? null)->send($message->getMessage());
 		} catch (TransportExceptionInterface $e) {
-			# TODO: handle exception
+			$this->logger->logException($e);
+
+			# in case of exception it is expected that none of the mails has been sent
+			$failedRecipients = [];
+
+			$recipients = array_merge($message->getTo(), $message->getCc(), $message->getBcc());
+			array_walk($recipients, static function ($value, $key) use (&$failedRecipients) {
+				if (is_numeric($key)) {
+					$failedRecipients[] = $value;
+				} else {
+					$failedRecipients[] = $key;
+				}
+			});
+
+			return $failedRecipients;
 		}
 
 		$allRecipients = [];
@@ -118,10 +131,11 @@ public function send(Message $message): array {
 			'app' => 'core',
 			'from' => \json_encode($message->getFrom()),
 			'recipients' => \json_encode($allRecipients),
-			'subject' => $message->getSubject()
+			'subject' => $message->getSubject(),
+			'mail_log' => ($logger !== null) ? $logger->toJSON() : null,
 		]);
 
-		return $failedRecipients;
+		return [];
 	}
 
 	/**
@@ -157,30 +171,30 @@ protected function convertEmail(string $email): string {
 		return $name.'@'.$domain;
 	}
 
-	protected function getInstance(): TransportInterface {
+	protected function getInstance(?LoggerInterface $logger = null): TransportInterface {
 		if ($this->instance !== null) {
 			return $this->instance;
 		}
 
 		$mailMode = $this->config->getSystemValue('mail_smtpmode', 'php');
 		if ($mailMode === 'smtp') {
-			$transport = $this->getSmtpInstance();
+			$transport = $this->getSmtpInstance($logger ?? null);
 		} else {
-			$transport = $this->getSendMailInstance();
+			$transport = $this->getSendMailInstance($logger ?? null);
 		}
 
 		$this->instance = $transport;
 
 		return $this->instance;
 	}
 
-	protected function getSmtpInstance(): EsmtpTransport {
+	protected function getSmtpInstance(LoggerInterface $logger): EsmtpTransport {
 		$timeout = $this->config->getSystemValue('mail_smtptimeout', 10);
 		$host = $this->config->getSystemValue('mail_smtphost', '127.0.0.1');
 		$port = $this->config->getSystemValue('mail_smtpport', 25);
 		$smtpSecurity = $this->config->getSystemValue('mail_smtpsecure', '');
 		$tls = $smtpSecurity === 'ssl' ? true : null;
-		$transport = new EsmtpTransport($host, $port, $tls);
+		$transport = new EsmtpTransport($host, $port, $tls, null, $logger);
 		if ($this->config->getSystemValue('mail_smtpauth', false)) {
 			$transport->setUsername($this->config->getSystemValue('mail_smtpname', ''));
 			$transport->setPassword($this->config->getSystemValue('mail_smtppassword', ''));
@@ -193,15 +207,14 @@ protected function getSmtpInstance(): EsmtpTransport {
 		return $transport;
 	}
 
-	protected function getSendMailInstance(): SendmailTransport {
+	protected function getSendMailInstance(?LoggerInterface $logger = null): SendmailTransport {
 		$i = $this->config->getSystemValue('mail_smtpmode', 'sendmail');
 		if ($i === 'qmail') {
 			$binaryPath = '/var/qmail/bin/sendmail';
 		} else {
 			$binaryPath = '/usr/sbin/sendmail';
 		}
 
-		# TODO: add logger ???
-		return new SendmailTransport($binaryPath . ' -bs');
+		return new SendmailTransport($binaryPath . ' -bs', null, $logger);
 	}
 }

lib/private/Mail/Message.php

@@ -32,14 +32,11 @@
  */
 class Message {
 	private Email $message;
-	/**
-	 * @var Address[]
-	 */
-	private array $from;
-	private array $replyTo;
-	private array $to;
-	private array $cc;
-	private array $bcc;
+	private array $from = [];
+	private array $replyTo = [];
+	private array $to = [];
+	private array $cc = [];
+	private array $bcc = [];
 
 	public function __construct(Email $swiftMessage) {
 		$this->message = $swiftMessage;

settings/Controller/MailSettingsController.php

@@ -160,36 +160,44 @@ public function sendTestMail() {
 			$email = $this->userSession->getUser()->getEMailAddress();
 		}
 
-		if (!empty($email)) {
-			try {
-				$message = $this->mailer->createMessage();
-				$message->setTo([$email => $this->userSession->getUser()->getDisplayName()]);
-				$message->setFrom([$this->defaultMailAddress]);
-				$message->setSubject($this->l10n->t('test email settings'));
-				$message->setPlainBody('If you received this email, the settings seem to be correct.');
-				$this->mailer->send($message);
-			} catch (\Exception $e) {
-				return [
-					'data' => [
-						'message' => (string) $this->l10n->t('A problem occurred while sending the email. Please revise your settings. (Error: %s)', [$e->getMessage()]),
+		if (empty($email)) {
+			return ['data' =>
+				['message' =>
+					(string) $this->l10n->t('You need to set your user email before being able to send test emails.'),
+				],
+				'status' => 'error'
+			];
+		}
+
+		try {
+			$message = $this->mailer->createMessage();
+			$message->setTo([$email => $this->userSession->getUser()->getDisplayName()]);
+			$message->setFrom([$this->defaultMailAddress]);
+			$message->setSubject($this->l10n->t('test email settings'));
+			$message->setPlainBody('If you received this email, the settings seem to be correct.');
+			$failed = $this->mailer->send($message);
+			if (empty($failed)) {
+				return ['data' =>
+					['message' =>
+						(string) $this->l10n->t('Email sent')
 					],
-					'status' => 'error',
+					'status' => 'success'
 				];
 			}
 
-			return ['data' =>
-				['message' =>
-					(string) $this->l10n->t('Email sent')
+			return [
+				'data' => [
+					'message' => (string) $this->l10n->t('A problem occurred while sending the email. Please revise your settings. (Error: %s)', ['not delivered']),
+				],
+				'status' => 'error',
+			];
+		} catch (\Exception $e) {
+			return [
+				'data' => [
+					'message' => (string) $this->l10n->t('A problem occurred while sending the email. Please revise your settings. (Error: %s)', [$e->getMessage()]),
 				],
-				'status' => 'success'
+				'status' => 'error',
 			];
 		}
-
-		return ['data' =>
-			['message' =>
-				(string) $this->l10n->t('You need to set your user email before being able to send test emails.'),
-			],
-			'status' => 'error'
-		];
 	}
 }

settings/templates/panels/admin/mail.php

@@ -3,7 +3,6 @@
 $mail_smtpsecure = [
 	''		=> $l->t('None'),
 	'ssl'	=> $l->t('SSL/TLS'),
-	'tls'	=> $l->t('STARTTLS'),
 ];
 $mail_smtpmode = [
 	'php',

tests/lib/Mail/MailerTest.php

@@ -8,21 +8,25 @@
 
 namespace Test\Mail;
 
+use Exception;
 use OC\Mail\Mailer;
 use OC_Defaults;
 use OCP\IConfig;
 use OCP\ILogger;
+use PHPUnit\Framework\MockObject\MockObject;
 use Symfony\Component\Mailer\Transport\SendmailTransport;
 use Symfony\Component\Mime\Email;
 use Test\TestCase;
 use OC\Mail\Message;
+use function array_merge;
+use function json_encode;
 
 class MailerTest extends TestCase {
-	/** @var IConfig | \PHPUnit\Framework\MockObject\MockObject */
+	/** @var IConfig | MockObject */
 	private $config;
 	/** @var OC_Defaults */
 	private $defaults;
-	/** @var ILogger | \PHPUnit\Framework\MockObject\MockObject */
+	/** @var ILogger | MockObject */
 	private $logger;
 	/** @var Mailer */
 	private $mailer;
@@ -76,9 +80,9 @@ public function testGetInstanceSendmail(): void {
 	}
 
 	public function testSendInvalidMailException(): void {
-		$this->expectException(\Exception::class);
+		$this->expectException(Exception::class);
 
-		/** @var Message | \PHPUnit\Framework\MockObject\MockObject $message */
+		/** @var Message | MockObject $message */
 		$message = $this->getMockBuilder(Message::class)
 			->disableOriginalConstructor()->getMock();
 		$message->expects($this->once())
@@ -118,7 +122,7 @@ public function testLogEntry(): void {
 
 		$this->mailer->method('getInstance')->willReturn($this->createMock(SendmailTransport::class));
 
-		/** @var Message | \PHPUnit\Framework\MockObject\MockObject $message */
+		/** @var Message | MockObject $message */
 		$message = $this->getMockBuilder(Message::class)
 			->disableOriginalConstructor()->getMock();
 		$message->expects($this->once())
@@ -140,9 +144,10 @@ public function testLogEntry(): void {
 			->method('debug')
 			->with('Sent mail from "{from}" to "{recipients}" with subject "{subject}"', [
 				'app' => 'core',
-				'from' => \json_encode($from),
-				'recipients' => \json_encode(\array_merge($to, $cc, $bcc)),
-				'subject' => 'Email subject'
+				'from' => json_encode($from),
+				'recipients' => json_encode(array_merge($to, $cc, $bcc)),
+				'subject' => 'Email subject',
+				'mail_log' => null
 			]);
 
 		$this->mailer->send($message);

tests/lib/Mail/MessageTest.php

@@ -14,8 +14,7 @@
 use Test\TestCase;
 
 class MessageTest extends TestCase {
-	/** @var Email */
-	private $symfonyMail;
+	private Email $symfonyMail;
 	private Message $message;
 
 	public function setUp(): void {