Swift to \Swift_Mailer as abstraction

* \Swift_Mailer handles starting the transport etc properly * Fixed tests Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
nextcloud · Jun 8, 2018 · 52ea2df · 52ea2df
1 parent 4460d04
commit 52ea2df
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 33 deletions.
diff --git a/lib/private/Mail/Mailer.php b/lib/private/Mail/Mailer.php
@@ -57,7 +57,7 @@
  * @package OC\Mail
  */
 class Mailer implements IMailer {
-	/** @var \Swift_SmtpTransport|\Swift_SendmailTransport Cached transport */
+	/** @var \Swift_Mailer Cached mailer */
 	private $instance = null;
 	/** @var IConfig */
 	private $config;
@@ -220,27 +220,24 @@ protected function convertEmail(string $email): string {
 		return $name.'@'.$domain;
 	}
 
-	/**
-	 * Returns whatever transport is configured within the config
-	 *
-	 * @return \Swift_SmtpTransport|\Swift_SendmailTransport
-	 */
-	protected function getInstance() {
+	protected function getInstance(): \Swift_Mailer {
 		if (!is_null($this->instance)) {
 			return $this->instance;
 		}
 
+		$transport = null;
+
 		switch ($this->config->getSystemValue('mail_smtpmode', 'smtp')) {
 			case 'sendmail':
-				$this->instance = $this->getSendMailInstance();
+				$transport = $this->getSendMailInstance();
 				break;
 			case 'smtp':
 			default:
-				$this->instance = $this->getSmtpInstance();
+				$transport = $this->getSmtpInstance();
 				break;
 		}
 
-		return $this->instance;
+		return new \Swift_Mailer($transport);
 	}
 
 	/**
@@ -262,7 +259,7 @@ protected function getSmtpInstance(): \Swift_SmtpTransport {
 		if (!empty($smtpSecurity)) {
 			$transport->setEncryption($smtpSecurity);
 		}
-		$transport->start();
+
 		return $transport;
 	}
 
@@ -272,7 +269,7 @@ protected function getSmtpInstance(): \Swift_SmtpTransport {
 	 * @return \Swift_SendmailTransport
 	 */
 	protected function getSendMailInstance(): \Swift_SendmailTransport {
-		switch ($this->config->getSystemValue('mail_smtpmode', 'smpt')) {
+		switch ($this->config->getSystemValue('mail_smtpmode', 'smtp')) {
 			case 'qmail':
 				$binaryPath = '/var/qmail/bin/sendmail';
 				break;

diff --git a/tests/lib/Mail/MailerTest.php b/tests/lib/Mail/MailerTest.php
@@ -48,50 +48,41 @@ public function setUp() {
 		);
 	}
 
-	public function testGetMailInstance() {
-		$this->assertEquals(\Swift_MailTransport::newInstance(), self::invokePrivate($this->mailer, 'getMailinstance'));
-	}
-
 	public function testGetSendMailInstanceSendMail() {
 		$this->config
 			->expects($this->once())
 			->method('getSystemValue')
-			->with('mail_smtpmode', 'php')
+			->with('mail_smtpmode', 'smtp')
 			->will($this->returnValue('sendmail'));
 
-		$this->assertEquals(\Swift_SendmailTransport::newInstance('/usr/sbin/sendmail -bs'), self::invokePrivate($this->mailer, 'getSendMailInstance'));
+		$this->assertEquals(new \Swift_SendmailTransport('/usr/sbin/sendmail -bs'), self::invokePrivate($this->mailer, 'getSendMailInstance'));
 	}
 
 	public function testGetSendMailInstanceSendMailQmail() {
 		$this->config
 			->expects($this->once())
 			->method('getSystemValue')
-			->with('mail_smtpmode', 'php')
+			->with('mail_smtpmode', 'smtp')
 			->will($this->returnValue('qmail'));
 
-		$this->assertEquals(\Swift_SendmailTransport::newInstance('/var/qmail/bin/sendmail -bs'), self::invokePrivate($this->mailer, 'getSendMailInstance'));
+		$this->assertEquals(new \Swift_SendmailTransport('/var/qmail/bin/sendmail -bs'), self::invokePrivate($this->mailer, 'getSendMailInstance'));
 	}
 
 	public function testGetInstanceDefault() {
-		$this->assertInstanceOf('\Swift_MailTransport', self::invokePrivate($this->mailer, 'getInstance'));
-	}
-
-	public function testGetInstancePhp() {
-		$this->config
-			->expects($this->any())
-			->method('getSystemValue')
-			->will($this->returnValue('php'));
-
-		$this->assertInstanceOf('\Swift_MailTransport', self::invokePrivate($this->mailer, 'getInstance'));
+		$mailer = self::invokePrivate($this->mailer, 'getInstance');
+		$this->assertInstanceOf(\Swift_Mailer::class, $mailer);
+		$this->assertInstanceOf(\Swift_SmtpTransport::class, $mailer->getTransport());
 	}
 
 	public function testGetInstanceSendmail() {
 		$this->config
-			->expects($this->any())
 			->method('getSystemValue')
-			->will($this->returnValue('sendmail'));
+			->with('mail_smtpmode', 'smtp')
+			->willReturn('sendmail');
 
-		$this->assertInstanceOf('\Swift_Mailer', self::invokePrivate($this->mailer, 'getInstance'));
+		$mailer = self::invokePrivate($this->mailer, 'getInstance');
+		$this->assertInstanceOf(\Swift_Mailer::class, $mailer);
+		$this->assertInstanceOf(\Swift_SendmailTransport::class, $mailer->getTransport());
 	}
 
 	public function testCreateMessage() {