From 2e08d01b5e689d31565239cf09ee61278b19cce5 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 6 Apr 2025 14:16:11 +0200 Subject: [PATCH 01/12] [ticket/17490] Add unit tests file for email method PHPBB-17490 --- tests/messenger/method_email_test.php | 389 ++++++++++++++++++++++++++ 1 file changed, 389 insertions(+) create mode 100644 tests/messenger/method_email_test.php diff --git a/tests/messenger/method_email_test.php b/tests/messenger/method_email_test.php new file mode 100644 index 0000000000..a053658378 --- /dev/null +++ b/tests/messenger/method_email_test.php @@ -0,0 +1,389 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +use phpbb\config\config; +use phpbb\language\language; +use phpbb\language\language_file_loader; +use phpbb\messenger\method\email; +use phpbb\messenger\queue; +use phpbb\path_helper; +use phpbb\symfony_request; +use phpbb\template\assets_bag; + +class phpbb_messenger_method_email_test extends \phpbb_test_case +{ + protected config $config; + protected queue $queue; + protected $request; + + public function setUp(): void + { + global $phpbb_root_path, $phpEx; + + $assets_bag = new assets_bag(); + $cache_path = $phpbb_root_path . 'cache/' . PHPBB_ENVIRONMENT . '/twig'; + $this->config = new config([]); + $dispatcher = new \phpbb_mock_event_dispatcher(); + $filesystem = new \phpbb\filesystem\filesystem(); + $language = new language(new language_file_loader($phpbb_root_path, $phpEx)); + $this->queue = $this->createMock(queue::class); + $this->request = new phpbb_mock_request(); + $user = new \phpbb\user($language, '\phpbb\datetime'); + $path_helper = new path_helper( + new symfony_request( + new phpbb_mock_request() + ), + $this->request, + $phpbb_root_path, + $phpEx + ); + $phpbb_container = new phpbb_mock_container_builder; + $twig_extensions_collection = new \phpbb\di\service_collection($phpbb_container); + $twig = new \phpbb\template\twig\environment( + $assets_bag, + $this->config, + $filesystem, + $path_helper, + $cache_path, + null, + new \phpbb\template\twig\loader(''), + $dispatcher, + array( + 'cache' => false, + 'debug' => false, + 'auto_reload' => true, + 'autoescape' => false, + ) + ); + $twig_lexer = new \phpbb\template\twig\lexer($twig); + $extension_manager = new phpbb_mock_extension_manager( + __DIR__ . '/', + array( + 'vendor2/foo' => array( + 'ext_name' => 'vendor2/foo', + 'ext_active' => '1', + 'ext_path' => 'ext/vendor2/foo/', + ), + ) + ); + $log = $this->createMock(\phpbb\log\log_interface::class); + + $this->method_email = new email( + $assets_bag, + $this->config, + $dispatcher, + $language, + $this->queue, + $path_helper, + $this->request, + $twig_extensions_collection, + $twig_lexer, + $user, + $phpbb_root_path, + $cache_path, + $extension_manager, + $log + ); + } + + public function test_miscellaneous(): void + { + $this->assertEquals(email::NOTIFY_EMAIL, $this->method_email->get_id()); + $this->assertEquals('email', $this->method_email->get_queue_object_name()); + $this->assertFalse($this->method_email->is_enabled()); + $this->config->offsetSet('email_enable', true); + $this->assertTrue($this->method_email->is_enabled()); + } + + public function test_set_dsn_from_config() + { + $config_values = [ + 'smtp_delivery' => true, + 'smtp_host' => 'smtp.example.com', + 'smtp_username' => 'user', + 'smtp_password' => 'pass', + 'smtp_port' => 587, + ]; + foreach ($config_values as $key => $value) + { + $this->config->set($key, $value); + } + + $this->method_email->set_dsn(); + $this->assertEquals('smtp://user:pass@smtp.example.com:587', $this->method_email->get_dsn()); + + $this->config->set('smtp_host', ''); + $this->method_email->set_dsn(); + $this->assertEquals('null://null', $this->method_email->get_dsn()); + } + + public function test_set_dns() + { + $this->assertEquals('', $this->method_email->get_dsn()); + $this->method_email->set_dsn(''); + $this->assertEquals('sendmail://default', $this->method_email->get_dsn()); + + $this->method_email->set_dsn('smtp://user:pass1@smtp.example.com:587'); + $this->assertEquals('smtp://user:pass1@smtp.example.com:587', $this->method_email->get_dsn()); + } + + public function test_set_transport() + { + $this->assertNull($this->method_email->get_transport()); + $this->assertEmpty($this->method_email->get_dsn()); + + $config_values = [ + 'smtp_delivery' => true, + 'smtp_host' => 'smtp.example.com', + 'smtp_username' => 'user', + 'smtp_password' => 'pass', + 'smtp_port' => 587, + 'smtp_verify_peer' => true, + 'smtp_verify_peer_name' => true, + 'smtp_allow_self_signed' => false, + ]; + foreach ($config_values as $key => $value) + { + $this->config->set($key, $value); + } + + $this->method_email->set_transport(); + + // set_dsn() should have been called in set_transport() + $this->assertEquals('smtp://user:pass@smtp.example.com:587', $this->method_email->get_dsn()); + + $transport = $this->method_email->get_transport(); + $this->assertInstanceOf('\Symfony\Component\Mailer\Transport\Smtp\SmtpTransport', $transport); + if (method_exists($transport->getStream(), 'getStreamOptions')) + { + $this->assertEquals([ + 'verify_peer' => true, + 'verify_peer_name' => true, + 'allow_self_signed' => false, + ], $transport->getStream()->getStreamOptions()['ssl'] ?? null); + } + } + + public function test_init() + { + $this->config->set('email_package_size', 100); + $email_reflection = new \ReflectionClass($this->method_email); + $email_property = $email_reflection->getProperty('email'); + $this->assertNull($email_property->getValue($this->method_email)); + + $use_queue_property = $email_reflection->getProperty('use_queue'); + $this->assertFalse($use_queue_property->getValue($this->method_email)); + + $this->method_email->init(); + /** @var \Symfony\Component\Mime\Email $email */ + $email = $email_property->getValue($this->method_email); + $this->assertNotNull($email); + + $this->assertTrue($use_queue_property->getValue($this->method_email)); + $this->assertEmpty($email->getTo()); + + $this->method_email->to('foo@bar.com'); + $this->assertNotEmpty($email->getTo()); + + $this->method_email->init(); + + $email = $email_property->getValue($this->method_email); + $this->assertNotNull($email); + $this->assertEmpty($email->getTo()); + } + + public function test_set_addresses() + { + $email_reflection = new \ReflectionClass($this->method_email); + $email_property = $email_reflection->getProperty('email'); + $this->method_email->init(); + /** @var \Symfony\Component\Mime\Email $email */ + $email = $email_property->getValue($this->method_email); + $this->assertNotNull($email); + + $this->method_email->set_addresses([]); + $this->assertEmpty($email->getTo()); + + $this->method_email->set_addresses(['user_email' => 'foo@bar.com']); + $this->assertNotEmpty($email->getTo()); + $this->assertEquals('foo@bar.com', $email->getTo()[0]->getAddress()); + $this->assertEmpty($email->getTo()[0]->getName()); + + $this->method_email->set_addresses(['user_email' => 'bar@foo.com', 'username' => 'Bar Foo']); + + $this->assertEquals('bar@foo.com', $email->getTo()[1]->getAddress()); + $this->assertEquals('Bar Foo', $email->getTo()[1]->getName()); + } + + public function test_to() + { + $email_reflection = new \ReflectionClass($this->method_email); + $email_property = $email_reflection->getProperty('email'); + $this->method_email->init(); + /** @var \Symfony\Component\Mime\Email $email */ + $email = $email_property->getValue($this->method_email); + $this->assertNotNull($email); + + // Empty address + $this->assertEmpty($email->getTo()); + $this->method_email->to(''); + $this->assertEmpty($email->getTo()); + + // Valid address + $this->method_email->to('foo@bar.com'); + $this->assertNotEmpty($email->getTo()); + $this->assertEquals('foo@bar.com', $email->getTo()[0]->getAddress()); + $this->assertEmpty($email->getTo()[0]->getName()); + + // Valid address with name + $this->method_email->to('bar@foo.com', 'Bar Foo'); + $this->assertEquals('bar@foo.com', $email->getTo()[1]->getAddress()); + if (DIRECTORY_SEPARATOR == '\\') + { + $this->assertEmpty($email->getTo()[1]->getName()); + } + else + { + $this->assertEquals('Bar Foo', $email->getTo()[1]->getName()); + } + } + + public function test_cc() + { + $email_reflection = new \ReflectionClass($this->method_email); + $email_property = $email_reflection->getProperty('email'); + $this->method_email->init(); + /** @var \Symfony\Component\Mime\Email $email */ + $email = $email_property->getValue($this->method_email); + $this->assertNotNull($email); + + // Empty address + $this->assertEmpty($email->getCc()); + $this->method_email->cc(''); + $this->assertEmpty($email->getCc()); + + // Valid address + $this->method_email->cc('foo@bar.com'); + $this->assertNotEmpty($email->getCc()); + $this->assertEquals('foo@bar.com', $email->getCc()[0]->getAddress()); + $this->assertEmpty($email->getCc()[0]->getName()); + + // Valid address with name + $this->method_email->cc('bar@foo.com', 'Bar Foo'); + $this->assertEquals('bar@foo.com', $email->getCc()[1]->getAddress()); + $this->assertEquals('Bar Foo', $email->getCc()[1]->getName()); + } + + public function test_bcc() + { + $email_reflection = new \ReflectionClass($this->method_email); + $email_property = $email_reflection->getProperty('email'); + $this->method_email->init(); + /** @var \Symfony\Component\Mime\Email $email */ + $email = $email_property->getValue($this->method_email); + $this->assertNotNull($email); + + // Empty address + $this->assertEmpty($email->getBcc()); + $this->method_email->bcc(''); + $this->assertEmpty($email->getBcc()); + + // Valid address + $this->method_email->bcc('foo@bar.com'); + $this->assertNotEmpty($email->getBcc()); + $this->assertEquals('foo@bar.com', $email->getBcc()[0]->getAddress()); + $this->assertEmpty($email->getBcc()[0]->getName()); + + // Valid address with name + $this->method_email->bcc('bar@foo.com', 'Bar Foo'); + $this->assertEquals('bar@foo.com', $email->getBcc()[1]->getAddress()); + $this->assertEquals('Bar Foo', $email->getBcc()[1]->getName()); + } + + public function test_reply_to() + { + $email_reflection = new \ReflectionClass($this->method_email); + $email_property = $email_reflection->getProperty('email'); + $this->method_email->init(); + /** @var \Symfony\Component\Mime\Email $email */ + $email = $email_property->getValue($this->method_email); + $this->assertNotNull($email); + + // Empty address + $this->assertEmpty($email->getReplyTo()); + $this->method_email->reply_to(''); + $this->assertEmpty($email->getReplyTo()); + + // Valid address + $this->method_email->reply_to('foo@bar.com'); + $this->assertNotEmpty($email->getReplyTo()); + $this->assertEquals('foo@bar.com', $email->getReplyTo()[0]->getAddress()); + $this->assertEmpty($email->getReplyTo()[0]->getName()); + + // Valid address with name + $this->method_email->reply_to('bar@foo.com', 'Bar Foo'); + $this->assertEquals('bar@foo.com', $email->getReplyTo()[1]->getAddress()); + $this->assertEquals('Bar Foo', $email->getReplyTo()[1]->getName()); + } + + public function test_from() + { + $email_reflection = new \ReflectionClass($this->method_email); + $email_property = $email_reflection->getProperty('email'); + $this->method_email->init(); + /** @var \Symfony\Component\Mime\Email $email */ + $email = $email_property->getValue($this->method_email); + $this->assertNotNull($email); + + // Empty address + $this->assertEmpty($email->getFrom()); + $this->method_email->from(''); + $this->assertEmpty($email->getFrom()); + + // Valid address + $this->method_email->from('foo@bar.com'); + $this->assertNotEmpty($email->getFrom()); + $this->assertEquals('foo@bar.com', $email->getFrom()[0]->getAddress()); + $this->assertEmpty($email->getFrom()[0]->getName()); + + // Valid address with name + $this->method_email->from('bar@foo.com', 'Bar Foo'); + $this->assertEquals('bar@foo.com', $email->getFrom()[1]->getAddress()); + $this->assertEquals('Bar Foo', $email->getFrom()[1]->getName()); + } + + public function test_subject() + { + $email_reflection = new \ReflectionClass($this->method_email); + $email_property = $email_reflection->getProperty('email'); + $this->method_email->init(); + /** @var \Symfony\Component\Mime\Email $email */ + $email = $email_property->getValue($this->method_email); + $this->assertNotNull($email); + + // Empty subject + $this->assertEmpty($email->getSubject()); + $this->method_email->subject(''); + $this->assertEmpty($email->getSubject()); + + // Test subject + $this->method_email->subject('Test email'); + $this->assertNotEmpty($email->getSubject()); + $this->assertEquals('Test email', $email->getSubject()); + + // Overwrite subject + $this->method_email->subject('Reply to test email'); + $this->assertNotEmpty($email->getSubject()); + $this->assertEquals('Reply to test email', $email->getSubject()); + } +} From 930c87e97af0484b7fa5b03faea439370a70fe7f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 6 Apr 2025 14:16:59 +0200 Subject: [PATCH 02/12] [ticket/17490] Improve expected types and docblock info PHPBB-17490 --- phpBB/phpbb/messenger/method/email.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/phpBB/phpbb/messenger/method/email.php b/phpBB/phpbb/messenger/method/email.php index 87ad9114c1..9e972fa855 100644 --- a/phpBB/phpbb/messenger/method/email.php +++ b/phpBB/phpbb/messenger/method/email.php @@ -15,6 +15,7 @@ namespace phpbb\messenger\method; use Symfony\Component\Mailer\Transport; use Symfony\Component\Mailer\Mailer; +use Symfony\Component\Mailer\Transport\TransportInterface; use Symfony\Component\Mime\Address; use Symfony\Component\Mime\Email as symfony_email; use Symfony\Component\Mime\Header\Headers; @@ -43,10 +44,10 @@ class email extends base /** @var symfony_email */ protected $email; - /** @var Address */ + /** @var Address From address */ protected $from; - /** @var Headers */ + /** @var Headers Email headers */ protected $headers; /** @@ -124,7 +125,7 @@ class email extends base { if (!empty($user_row['user_email'])) { - $this->to($user_row['user_email'], $user_row['username'] ?: ''); + $this->to($user_row['user_email'], $user_row['username'] ?? ''); } } @@ -453,9 +454,9 @@ class email extends base /** * Get mailer transport object * - * @return \Symfony\Component\Mailer\Transport\TransportInterface Symfony Mailer transport object + * @return ?TransportInterface Symfony Mailer transport object */ - public function get_transport(): \Symfony\Component\Mailer\Transport\TransportInterface + public function get_transport(): ?TransportInterface { return $this->transport; } From 91c325174f44308c6ba46c80e9cdd065a022243f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 6 Apr 2025 14:20:04 +0200 Subject: [PATCH 03/12] [ticket/17490] Add type hints to enforce some types PHPBB-17490 --- phpBB/phpbb/messenger/method/email.php | 21 +++++++++++---------- tests/messenger/method_email_test.php | 2 -- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/phpBB/phpbb/messenger/method/email.php b/phpBB/phpbb/messenger/method/email.php index 9e972fa855..6fb3e20e68 100644 --- a/phpBB/phpbb/messenger/method/email.php +++ b/phpBB/phpbb/messenger/method/email.php @@ -15,6 +15,7 @@ namespace phpbb\messenger\method; use Symfony\Component\Mailer\Transport; use Symfony\Component\Mailer\Mailer; +use Symfony\Component\Mailer\Transport\AbstractTransport; use Symfony\Component\Mailer\Transport\TransportInterface; use Symfony\Component\Mime\Address; use Symfony\Component\Mime\Email as symfony_email; @@ -39,16 +40,16 @@ class email extends base * * Symfony Mailer transport DSN */ - protected $dsn = ''; + protected string $dsn = ''; /** @var symfony_email */ - protected $email; + protected symfony_email $email; /** @var Address From address */ - protected $from; + protected Address $from; /** @var Headers Email headers */ - protected $headers; + protected Headers $headers; /** * @var int @@ -60,16 +61,16 @@ class email extends base * symfony_email::PRIORITY_LOW * symfony_email::PRIORITY_LOWEST */ - protected $mail_priority = symfony_email::PRIORITY_NORMAL; + protected int $mail_priority = symfony_email::PRIORITY_NORMAL; /** @var \phpbb\messenger\queue */ protected $queue; /** @var Address */ - protected $reply_to; + protected Address $reply_to; - /** @var \Symfony\Component\Mailer\Transport\AbstractTransport */ - protected $transport; + /** @var AbstractTransport */ + protected AbstractTransport $transport; /** * {@inheritDoc} @@ -454,9 +455,9 @@ class email extends base /** * Get mailer transport object * - * @return ?TransportInterface Symfony Mailer transport object + * @return TransportInterface Symfony Mailer transport object */ - public function get_transport(): ?TransportInterface + public function get_transport(): TransportInterface { return $this->transport; } diff --git a/tests/messenger/method_email_test.php b/tests/messenger/method_email_test.php index a053658378..a1d8256070 100644 --- a/tests/messenger/method_email_test.php +++ b/tests/messenger/method_email_test.php @@ -139,7 +139,6 @@ class phpbb_messenger_method_email_test extends \phpbb_test_case public function test_set_transport() { - $this->assertNull($this->method_email->get_transport()); $this->assertEmpty($this->method_email->get_dsn()); $config_values = [ @@ -179,7 +178,6 @@ class phpbb_messenger_method_email_test extends \phpbb_test_case $this->config->set('email_package_size', 100); $email_reflection = new \ReflectionClass($this->method_email); $email_property = $email_reflection->getProperty('email'); - $this->assertNull($email_property->getValue($this->method_email)); $use_queue_property = $email_reflection->getProperty('use_queue'); $this->assertFalse($use_queue_property->getValue($this->method_email)); From 8dbe499e3dfdb03c5f01c7c422961373b3509afd Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 6 Apr 2025 21:25:16 +0200 Subject: [PATCH 04/12] [ticket/17490] Add extra method to support unit testing mailer Also increased unit test coverage of email method to 100%. PHPBB-17490 --- phpBB/phpbb/messenger/method/email.php | 22 +- tests/messenger/method_email_test.php | 464 +++++++++++++++++++++++-- 2 files changed, 449 insertions(+), 37 deletions(-) diff --git a/phpBB/phpbb/messenger/method/email.php b/phpBB/phpbb/messenger/method/email.php index 6fb3e20e68..3b580b6a69 100644 --- a/phpBB/phpbb/messenger/method/email.php +++ b/phpBB/phpbb/messenger/method/email.php @@ -13,6 +13,8 @@ namespace phpbb\messenger\method; +use Symfony\Component\Mailer\Exception\TransportExceptionInterface; +use Symfony\Component\Mailer\MailerInterface; use Symfony\Component\Mailer\Transport; use Symfony\Component\Mailer\Mailer; use Symfony\Component\Mailer\Transport\AbstractTransport; @@ -275,7 +277,6 @@ class email extends base */ protected function build_headers(): void { - $board_contact = trim($this->config['board_contact']); $contact_name = html_entity_decode($this->config['board_contact_name'], ENT_COMPAT); @@ -317,7 +318,6 @@ class email extends base { $this->header($header, $value); } - } /** @@ -408,7 +408,7 @@ class email extends base $package_size = $queue_data[$queue_object_name]['package_size'] ?? 0; $num_items = (!$package_size || $messages_count < $package_size) ? $messages_count : $package_size; - $mailer = new Mailer($this->transport); + $mailer = $this->get_mailer(); for ($i = 0; $i < $num_items; $i++) { @@ -437,7 +437,7 @@ class email extends base { $mailer->send($email); } - catch (\Symfony\Component\Mailer\Exception\TransportExceptionInterface $e) + catch (TransportExceptionInterface $e) { $this->error($e->getDebug()); continue; @@ -452,6 +452,16 @@ class email extends base } } + /** + * Get mailer object + * + * @return MailerInterface Symfony Mailer object + */ + protected function get_mailer(): MailerInterface + { + return new Mailer($this->transport); + } + /** * Get mailer transport object * @@ -506,7 +516,7 @@ class email extends base // Send message ... if (!$this->use_queue) { - $mailer = new Mailer($this->transport); + $mailer = $this->get_mailer(); $subject = $this->subject; $msg = $this->msg; @@ -540,7 +550,7 @@ class email extends base { $mailer->send($this->email); } - catch (\Symfony\Component\Mailer\Exception\TransportExceptionInterface $e) + catch (TransportExceptionInterface $e) { $this->error($e->getDebug()); return false; diff --git a/tests/messenger/method_email_test.php b/tests/messenger/method_email_test.php index a1d8256070..e0dfae93b5 100644 --- a/tests/messenger/method_email_test.php +++ b/tests/messenger/method_email_test.php @@ -19,45 +19,65 @@ use phpbb\messenger\queue; use phpbb\path_helper; use phpbb\symfony_request; use phpbb\template\assets_bag; +use Symfony\Component\Mime\RawMessage; class phpbb_messenger_method_email_test extends \phpbb_test_case { + protected $assets_bag; + protected $cache_path; protected config $config; + protected $dispatcher; + protected $extension_manager; + protected $language; + protected $log; + protected $path_helper; protected queue $queue; protected $request; + protected $twig_extensions_collection; + protected $twig_lexer; + protected $user; public function setUp(): void { - global $phpbb_root_path, $phpEx; + global $config, $request, $symfony_request, $user, $phpbb_root_path, $phpEx; - $assets_bag = new assets_bag(); - $cache_path = $phpbb_root_path . 'cache/' . PHPBB_ENVIRONMENT . '/twig'; - $this->config = new config([]); - $dispatcher = new \phpbb_mock_event_dispatcher(); - $filesystem = new \phpbb\filesystem\filesystem(); - $language = new language(new language_file_loader($phpbb_root_path, $phpEx)); + $this->assets_bag = new assets_bag(); + $this->cache_path = $phpbb_root_path . 'cache/' . PHPBB_ENVIRONMENT . '/twig'; + $this->config = new config([ + 'force_server_vars' => false, + ]); + $config = $this->config; + $this->dispatcher = $this->getMockBuilder('\phpbb\event\dispatcher') + ->disableOriginalConstructor() + ->getMock(); + $this->filesystem = new \phpbb\filesystem\filesystem(); + $this->language = new language(new language_file_loader($phpbb_root_path, $phpEx)); $this->queue = $this->createMock(queue::class); $this->request = new phpbb_mock_request(); - $user = new \phpbb\user($language, '\phpbb\datetime'); - $path_helper = new path_helper( - new symfony_request( - new phpbb_mock_request() - ), + $request = $this->request; + $this->symfony_request = new symfony_request(new phpbb_mock_request()); + $symfony_request = $this->symfony_request; + $this->user = new \phpbb\user($this->language, '\phpbb\datetime'); + $user = $this->user; + $user->page['root_script_path'] = 'phpbb/'; + $this->user->host = 'yourdomain.com'; + $this->path_helper = new path_helper( + $this->symfony_request, $this->request, $phpbb_root_path, $phpEx ); $phpbb_container = new phpbb_mock_container_builder; - $twig_extensions_collection = new \phpbb\di\service_collection($phpbb_container); + $this->twig_extensions_collection = new \phpbb\di\service_collection($phpbb_container); $twig = new \phpbb\template\twig\environment( - $assets_bag, + $this->assets_bag, $this->config, - $filesystem, - $path_helper, - $cache_path, + $this->filesystem, + $this->path_helper, + $this->cache_path, null, new \phpbb\template\twig\loader(''), - $dispatcher, + $this->dispatcher, array( 'cache' => false, 'debug' => false, @@ -65,8 +85,8 @@ class phpbb_messenger_method_email_test extends \phpbb_test_case 'autoescape' => false, ) ); - $twig_lexer = new \phpbb\template\twig\lexer($twig); - $extension_manager = new phpbb_mock_extension_manager( + $this->twig_lexer = new \phpbb\template\twig\lexer($twig); + $this->extension_manager = new phpbb_mock_extension_manager( __DIR__ . '/', array( 'vendor2/foo' => array( @@ -76,23 +96,23 @@ class phpbb_messenger_method_email_test extends \phpbb_test_case ), ) ); - $log = $this->createMock(\phpbb\log\log_interface::class); + $this->log = $this->createMock(\phpbb\log\log_interface::class); $this->method_email = new email( - $assets_bag, + $this->assets_bag, $this->config, - $dispatcher, - $language, + $this->dispatcher, + $this->language, $this->queue, - $path_helper, + $this->path_helper, $this->request, - $twig_extensions_collection, - $twig_lexer, - $user, + $this->twig_extensions_collection, + $this->twig_lexer, + $this->user, $phpbb_root_path, - $cache_path, - $extension_manager, - $log + $this->cache_path, + $this->extension_manager, + $this->log ); } @@ -200,6 +220,17 @@ class phpbb_messenger_method_email_test extends \phpbb_test_case $this->assertEmpty($email->getTo()); } + public function test_get_mailer() + { + $email_reflection = new \ReflectionClass($this->method_email); + $this->method_email->init(); + $this->method_email->set_transport(); + $mailer_method = $email_reflection->getMethod('get_mailer'); + + $mailer = $mailer_method->invoke($this->method_email); + $this->assertInstanceOf(\Symfony\Component\Mailer\Mailer::class, $mailer); + } + public function test_set_addresses() { $email_reflection = new \ReflectionClass($this->method_email); @@ -384,4 +415,375 @@ class phpbb_messenger_method_email_test extends \phpbb_test_case $this->assertNotEmpty($email->getSubject()); $this->assertEquals('Reply to test email', $email->getSubject()); } + + public function test_anti_abuse_headers() + { + $email_reflection = new \ReflectionClass($this->method_email); + $headers_property = $email_reflection->getProperty('headers'); + $this->method_email->init(); + + /** @var \Symfony\Component\Mime\Header\Headers $headers */ + $headers = $headers_property->getValue($this->method_email); + + $this->config->set('server_name', 'yourdomain.com'); + $this->user->data['user_id'] = 2; + $this->user->data['username'] = 'admin'; + $this->user->ip = '127.0.0.1'; + + $this->assertEmpty($headers->toArray()); + $this->method_email->anti_abuse_headers($this->config, $this->user); + + $this->assertEquals( + [ + 'X-AntiAbuse: Board servername - yourdomain.com', + 'X-AntiAbuse: User_id - 2', + 'X-AntiAbuse: Username - admin', + 'X-AntiAbuse: User IP - 127.0.0.1', + ], + $headers->toArray() + ); + } + + public function test_set_mail_priority() + { + $email_reflection = new \ReflectionClass($this->method_email); + $email_property = $email_reflection->getProperty('email'); + $this->method_email->init(); + /** @var \Symfony\Component\Mime\Email $email */ + $email = $email_property->getValue($this->method_email); + $this->assertNotNull($email); + + // Default priority + $this->assertEquals(\Symfony\Component\Mime\Email::PRIORITY_NORMAL, $email->getPriority()); + + // Highest priority + $this->method_email->set_mail_priority(\Symfony\Component\Mime\Email::PRIORITY_HIGHEST); + $this->assertEquals(\Symfony\Component\Mime\Email::PRIORITY_HIGHEST, $email->getPriority()); + } + + public function test_process_queue_not_enabled() + { + $this->method_email->init(); + + $queue_data = [ + 'email' => [ + 'data' => [ + 'message_one', + 'message_two', + ] + ] + ]; + + // Process queue will remove emails if email method is not enabled + $this->method_email->process_queue($queue_data); + $this->assertEmpty($queue_data); + } + + public function test_process_queue() + { + global $phpbb_root_path; + + $this->config->set('email_enable', true); + $this->user->data['user_id'] = 2; + $this->user->session_id = 'abcdef'; + + $this->method_email = $this->getMockBuilder($this->method_email::class) + ->setConstructorArgs([$this->assets_bag, + $this->config, + $this->dispatcher, + $this->language, + $this->queue, + $this->path_helper, + $this->request, + $this->twig_extensions_collection, + $this->twig_lexer, + $this->user, + $phpbb_root_path, + $this->cache_path, + $this->extension_manager, + $this->log + ]) + ->onlyMethods(['get_mailer']) + ->getMock(); + + $mailer_mock = $this->getMockBuilder(\Symfony\Component\Mailer\MailerInterface::class) + ->disableOriginalConstructor() + ->onlyMethods(['send']) + ->getMock(); + $sent_emails = 0; + $mailer_mock->method('send') + ->willReturnCallback(function(RawMessage $mail) use(&$sent_emails) { + if ($mail->toString() === 'throw_exception') + { + throw new \Symfony\Component\Mailer\Exception\TransportException('exception'); + } + + $sent_emails++; + }); + $this->method_email->method('get_mailer')->willReturn($mailer_mock); + + $this->method_email->init(); + $errors = []; + $this->log->method('add') + ->willReturnCallback(function($mode, $user_id, $log_ip, $log_operation, $log_time = false, $additional_data = []) use (&$errors) { + $errors[] = $additional_data[0]; + }); + + $this->dispatcher + ->expects($this->atLeastOnce()) + ->method('trigger_event') + ->willReturnCallback(function($event_name, $value_array) { + if ($event_name === 'core.notification_message_process' && $value_array['email']->toString() == 'message_three') + { + $value_array['break'] = true; + } + + return $value_array; + }); + + $queue_data = [ + 'email' => [ + 'data' => [ + ['email' => new RawMessage('message_one')], + ['email' => new RawMessage('message_two')], + ['email' => new RawMessage('message_three')], + ['email' => new RawMessage('throw_exception')], + ['email' => new RawMessage('message_four')], + ] + ] + ]; + + $this->assertEmpty($errors); + $this->method_email->process_queue($queue_data); + $this->assertEmpty($queue_data); + $this->assertEquals(3, $sent_emails); + + $this->assertEquals(['EMAIL



'], $errors); + } + + public function test_send_break() + { + $this->dispatcher + ->expects($this->atLeastOnce()) + ->method('trigger_event') + ->willReturnCallback(function($event_name, $value_array) { + if ($event_name !== 'core.notification_message_email') + { + return $value_array; + } + + $value_array['break'] = true; + return $value_array; + }); + + $this->config->set('board_email', 'admin@yourdomain.com'); + + $this->method_email->init(); + $this->method_email->to('foo@bar.com'); + $this->method_email->subject('Test email'); + $this->method_email->template('test', 'en'); + + $this->method_email->send(); + } + + public function test_send_no_queue() + { + global $phpbb_root_path; + + $this->config->set('email_enable', true); + $this->user->data['user_id'] = 2; + $this->user->session_id = 'abcdef'; + + $this->method_email = $this->getMockBuilder($this->method_email::class) + ->setConstructorArgs([$this->assets_bag, + $this->config, + $this->dispatcher, + $this->language, + $this->queue, + $this->path_helper, + $this->request, + $this->twig_extensions_collection, + $this->twig_lexer, + $this->user, + $phpbb_root_path, + $this->cache_path, + $this->extension_manager, + $this->log + ]) + ->onlyMethods(['get_mailer']) + ->getMock(); + + $mailer_mock = $this->getMockBuilder(\Symfony\Component\Mailer\MailerInterface::class) + ->disableOriginalConstructor() + ->onlyMethods(['send']) + ->getMock(); + $sent_emails = 0; + $mailer_mock->method('send') + ->willReturnCallback(function(RawMessage $mail) use(&$sent_emails) { + $sent_emails++; + }); + $this->method_email->method('get_mailer')->willReturn($mailer_mock); + + $this->config->set('board_email', 'admin@yourdomain.com'); + + $this->method_email->init(); + $errors = []; + $this->log->method('add') + ->willReturnCallback(function($mode, $user_id, $log_ip, $log_operation, $log_time = false, $additional_data = []) use (&$errors) { + $errors[] = $additional_data[0]; + }); + + $this->dispatcher + ->expects($this->atLeastOnce()) + ->method('trigger_event') + ->willReturnCallback(function($event_name, $value_array) { + return $value_array; + }); + + $this->method_email->to('foo@bar.com'); + $this->method_email->subject('Test email'); + $this->method_email->template('test', 'en'); + + $this->assertTrue($this->method_email->send()); + $this->assertEquals(1, $sent_emails); + + $this->assertEmpty($errors); + } + + public function test_send_exception() + { + global $phpbb_root_path; + + $this->config->set('email_enable', true); + $this->user->data['user_id'] = 2; + $this->user->session_id = 'abcdef'; + + $this->method_email = $this->getMockBuilder($this->method_email::class) + ->setConstructorArgs([$this->assets_bag, + $this->config, + $this->dispatcher, + $this->language, + $this->queue, + $this->path_helper, + $this->request, + $this->twig_extensions_collection, + $this->twig_lexer, + $this->user, + $phpbb_root_path, + $this->cache_path, + $this->extension_manager, + $this->log + ]) + ->onlyMethods(['get_mailer']) + ->getMock(); + + $mailer_mock = $this->getMockBuilder(\Symfony\Component\Mailer\MailerInterface::class) + ->disableOriginalConstructor() + ->onlyMethods(['send']) + ->getMock(); + $mailer_mock->method('send') + ->willReturnCallback(function(RawMessage $mail) use(&$sent_emails) { + throw new \Symfony\Component\Mailer\Exception\TransportException('exception'); + }); + $this->method_email->method('get_mailer')->willReturn($mailer_mock); + + $this->config->set('board_email', 'admin@yourdomain.com'); + + $this->method_email->init(); + $errors = []; + $this->log->method('add') + ->willReturnCallback(function($mode, $user_id, $log_ip, $log_operation, $log_time = false, $additional_data = []) use (&$errors) { + $errors[] = $additional_data[0]; + }); + + $this->dispatcher + ->expects($this->atLeastOnce()) + ->method('trigger_event') + ->willReturnCallback(function($event_name, $value_array) { + return $value_array; + }); + + $this->method_email->to('foo@bar.com'); + $this->method_email->subject('Test email'); + $this->method_email->template('test', 'en'); + + $this->assertFalse($this->method_email->send()); + + $this->assertEquals(['EMAIL



'], $errors); + } + + public function test_send_queue() + { + global $phpbb_root_path; + + $this->config->set('email_enable', true); + $this->config->set('email_package_size', 100); + $this->user->data['user_id'] = 2; + $this->user->session_id = 'abcdef'; + + $this->method_email = $this->getMockBuilder($this->method_email::class) + ->setConstructorArgs([$this->assets_bag, + $this->config, + $this->dispatcher, + $this->language, + $this->queue, + $this->path_helper, + $this->request, + $this->twig_extensions_collection, + $this->twig_lexer, + $this->user, + $phpbb_root_path, + $this->cache_path, + $this->extension_manager, + $this->log + ]) + ->onlyMethods(['get_mailer']) + ->getMock(); + + $mailer_mock = $this->getMockBuilder(\Symfony\Component\Mailer\MailerInterface::class) + ->disableOriginalConstructor() + ->onlyMethods(['send']) + ->getMock(); + $mailer_mock->method('send') + ->willReturnCallback(function(RawMessage $mail) use(&$sent_emails) { + throw new \Symfony\Component\Mailer\Exception\TransportException('exception'); + }); + $this->method_email->method('get_mailer')->willReturn($mailer_mock); + + $this->config->set('board_email', 'admin@yourdomain.com'); + + $this->method_email->init(); + $errors = []; + $this->log->method('add') + ->willReturnCallback(function($mode, $user_id, $log_ip, $log_operation, $log_time = false, $additional_data = []) use (&$errors) { + $errors[] = $additional_data[0]; + }); + + $this->dispatcher + ->expects($this->atLeastOnce()) + ->method('trigger_event') + ->willReturnCallback(function($event_name, $value_array) { + return $value_array; + }); + + // Mock queue methods + $this->queue->method('init') + ->willReturnCallback(function(string $object, int $package_size) { + $this->assertEquals('email', $object); + $this->assertEquals($this->config['email_package_size'], $package_size); + }); + $this->queue->method('put') + ->willReturnCallback(function(string $object, array $message_data) { + $this->assertEquals('email', $object); + $this->assertStringContainsString('phpBB is correctly configured to send emails', $message_data['email']->getSubject()); + }); + + $this->method_email->to('foo@bar.com'); + $this->method_email->subject('Test email'); + $this->method_email->template('test', 'en'); + + $this->assertTrue($this->method_email->send()); + + $this->assertEmpty($errors); + } } From d5717b541e1d6c3164edd835273775f71ce121bf Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 7 Apr 2025 17:11:31 +0200 Subject: [PATCH 05/12] [ticket/17490] Remove name handling for mails on windows PHPBB-17490 --- phpBB/phpbb/messenger/method/email.php | 5 +---- tests/messenger/method_email_test.php | 9 +-------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/phpBB/phpbb/messenger/method/email.php b/phpBB/phpbb/messenger/method/email.php index 3b580b6a69..7bd92538c2 100644 --- a/phpBB/phpbb/messenger/method/email.php +++ b/phpBB/phpbb/messenger/method/email.php @@ -146,10 +146,7 @@ class email extends base return; } - // If empty sendmail_path on windows, PHP changes the to line - $windows_empty_sendmail_path = !$this->config['smtp_delivery'] && DIRECTORY_SEPARATOR == '\\'; - - $to = new Address($address, $windows_empty_sendmail_path ? '' : trim($realname)); + $to = new Address($address, trim($realname)); $this->email->getTo() ? $this->email->addTo($to) : $this->email->to($to); } diff --git a/tests/messenger/method_email_test.php b/tests/messenger/method_email_test.php index e0dfae93b5..432e90939b 100644 --- a/tests/messenger/method_email_test.php +++ b/tests/messenger/method_email_test.php @@ -277,14 +277,7 @@ class phpbb_messenger_method_email_test extends \phpbb_test_case // Valid address with name $this->method_email->to('bar@foo.com', 'Bar Foo'); $this->assertEquals('bar@foo.com', $email->getTo()[1]->getAddress()); - if (DIRECTORY_SEPARATOR == '\\') - { - $this->assertEmpty($email->getTo()[1]->getName()); - } - else - { - $this->assertEquals('Bar Foo', $email->getTo()[1]->getName()); - } + $this->assertEquals('Bar Foo', $email->getTo()[1]->getName()); } public function test_cc() From e4c8984bf3a26d0833b0295c700dd8aa06959907 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 7 Apr 2025 19:39:07 +0200 Subject: [PATCH 06/12] [ticket/17490] Remove not needed returns PHPBB-17490 --- phpBB/phpbb/messenger/method/base.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/phpBB/phpbb/messenger/method/base.php b/phpBB/phpbb/messenger/method/base.php index c0c2a94d0d..0fb46b2cb3 100644 --- a/phpBB/phpbb/messenger/method/base.php +++ b/phpBB/phpbb/messenger/method/base.php @@ -216,9 +216,9 @@ abstract class base implements messenger_interface * @param string $template_path Email template path * @param string $template_dir_prefix Email template directory prefix * - * @return bool + * @return void */ - public function template(string $template_file, string $template_lang = '', string $template_path = '', string $template_dir_prefix = ''): bool + public function template(string $template_file, string $template_lang = '', string $template_path = '', string $template_dir_prefix = ''): void { $template_dir_prefix = (!$template_dir_prefix || $template_dir_prefix[0] === '/') ? $template_dir_prefix : '/' . $template_dir_prefix; @@ -290,8 +290,6 @@ abstract class base implements messenger_interface $this->template->set_filenames([ 'body' => $template_file . '.txt', ]); - - return true; } /** @@ -487,6 +485,5 @@ abstract class base implements messenger_interface */ public function header(string $header_name, mixed $header_value): void { - return; } } From 5bc4c5765e2a64eb92b1128f104691fecc0049ef Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 7 Apr 2025 19:39:23 +0200 Subject: [PATCH 07/12] [ticket/17490] Use isset instead of !empty() PHPBB-17490 --- phpBB/phpbb/messenger/method/base.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/messenger/method/base.php b/phpBB/phpbb/messenger/method/base.php index 0fb46b2cb3..c8c7129caf 100644 --- a/phpBB/phpbb/messenger/method/base.php +++ b/phpBB/phpbb/messenger/method/base.php @@ -423,7 +423,7 @@ abstract class base implements messenger_interface */ public function save_queue(): void { - if ($this->use_queue && !empty($this->queue)) + if ($this->use_queue && isset($this->queue)) { $this->queue->save(); } From aa2da5b91676311050f29efbbddb59b8dbda58fa Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 7 Apr 2025 19:39:37 +0200 Subject: [PATCH 08/12] [ticket/17490] Add missing property declaration in test PHPBB-17490 --- tests/messenger/method_email_test.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/messenger/method_email_test.php b/tests/messenger/method_email_test.php index 432e90939b..be77da02de 100644 --- a/tests/messenger/method_email_test.php +++ b/tests/messenger/method_email_test.php @@ -28,6 +28,7 @@ class phpbb_messenger_method_email_test extends \phpbb_test_case protected config $config; protected $dispatcher; protected $extension_manager; + protected email $method_email; protected $language; protected $log; protected $path_helper; From 63203f42f78612e5e833e09296562b5bb4aebfaa Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Mon, 7 Apr 2025 20:37:52 +0200 Subject: [PATCH 09/12] [ticket/17490] Add test coverage for remainder of base test PHPBB-17490 --- tests/messenger/method_base_test.php | 274 ++++++++++++++++++++++++++ tests/messenger/method_email_test.php | 17 +- 2 files changed, 289 insertions(+), 2 deletions(-) create mode 100644 tests/messenger/method_base_test.php diff --git a/tests/messenger/method_base_test.php b/tests/messenger/method_base_test.php new file mode 100644 index 0000000000..38e7a921b1 --- /dev/null +++ b/tests/messenger/method_base_test.php @@ -0,0 +1,274 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +use phpbb\config\config; +use phpbb\language\language; +use phpbb\language\language_file_loader; +use phpbb\messenger\method\email; +use phpbb\messenger\queue; +use phpbb\path_helper; +use phpbb\symfony_request; +use phpbb\template\assets_bag; + +class phpbb_messenger_method_base_test extends \phpbb_test_case +{ + protected $assets_bag; + protected $cache_path; + protected config $config; + protected $dispatcher; + protected $extension_manager; + protected email $method_email; + protected $method_base; + protected $language; + protected $log; + protected $path_helper; + protected queue $queue; + protected $request; + protected $twig_extensions_collection; + protected $twig_lexer; + protected $user; + + public function setUp(): void + { + global $config, $request, $symfony_request, $user, $phpbb_root_path, $phpEx; + + $this->assets_bag = new assets_bag(); + $this->cache_path = $phpbb_root_path . 'cache/' . PHPBB_ENVIRONMENT . '/twig'; + $this->config = new config([ + 'force_server_vars' => false, + ]); + $config = $this->config; + $this->dispatcher = $this->getMockBuilder('\phpbb\event\dispatcher') + ->disableOriginalConstructor() + ->getMock(); + $this->filesystem = new \phpbb\filesystem\filesystem(); + $this->language = new language(new language_file_loader($phpbb_root_path, $phpEx)); + $this->queue = $this->createMock(queue::class); + $this->request = new phpbb_mock_request(); + $request = $this->request; + $this->symfony_request = new symfony_request(new phpbb_mock_request()); + $symfony_request = $this->symfony_request; + $this->user = $this->getMockBuilder('\phpbb\user') + ->setConstructorArgs([$this->language, '\phpbb\datetime']) + ->getMock(); + $user = $this->user; + $user->page['root_script_path'] = 'phpbb/'; + $this->user->host = 'yourdomain.com'; + $this->path_helper = new path_helper( + $this->symfony_request, + $this->request, + $phpbb_root_path, + $phpEx + ); + $phpbb_container = new phpbb_mock_container_builder; + $this->twig_extensions_collection = new \phpbb\di\service_collection($phpbb_container); + $twig = new \phpbb\template\twig\environment( + $this->assets_bag, + $this->config, + $this->filesystem, + $this->path_helper, + $this->cache_path, + null, + new \phpbb\template\twig\loader(''), + $this->dispatcher, + array( + 'cache' => false, + 'debug' => false, + 'auto_reload' => true, + 'autoescape' => false, + ) + ); + $this->twig_lexer = new \phpbb\template\twig\lexer($twig); + $this->extension_manager = new phpbb_mock_extension_manager( + __DIR__ . '/', + array( + 'vendor2/foo' => array( + 'ext_name' => 'vendor2/foo', + 'ext_active' => '1', + 'ext_path' => 'ext/vendor2/foo/', + ), + ) + ); + $this->log = $this->createMock(\phpbb\log\log_interface::class); + + $this->method_email = new email( + $this->assets_bag, + $this->config, + $this->dispatcher, + $this->language, + $this->queue, + $this->path_helper, + $this->request, + $this->twig_extensions_collection, + $this->twig_lexer, + $this->user, + $phpbb_root_path, + $this->cache_path, + $this->extension_manager, + $this->log + ); + + $this->method_base = $this->getMockBuilder(\phpbb\messenger\method\base::class) + ->setConstructorArgs([ + $this->assets_bag, + $this->config, + $this->dispatcher, + $this->language, + $this->queue, + $this->path_helper, + $this->request, + $this->twig_extensions_collection, + $this->twig_lexer, + $this->user, + $phpbb_root_path, + $this->cache_path, + $this->extension_manager, + $this->log + ]) + ->getMockForAbstractClass(); + } + + public function test_header() + { + $this->method_base->header('X-AntiAbuse', 'Board servername - ' . $this->user->host); + $this->assertTrue(true); // No exception should be thrown + } + + public function test_set_use_queue() + { + $use_queue_property = new \ReflectionProperty($this->method_base, 'use_queue'); + $this->method_base->set_use_queue(); + $this->assertTrue($use_queue_property->getValue($this->method_base)); + $this->method_base->set_use_queue(false); + $this->assertFalse($use_queue_property->getValue($this->method_base)); + } + + public function test_error_wout_session() + { + $errors = []; + $this->log->method('add') + ->willReturnCallback(function($mode, $user_id, $log_ip, $log_operation, $log_time = false, $additional_data = []) use (&$errors) { + $errors[] = $additional_data[0]; + }); + + $this->user->data['user_id'] = 2; + $this->user->session_id = ''; + $this->user + ->expects($this->once()) + ->method('session_begin') + ->willReturnCallback(function() { + $this->assertTrue(true); + }); + + $this->method_base->error('Test error message'); + + $this->assertCount(1, $errors); + $this->assertEquals('


Test error message
', $errors[0]); + } + + public function test_save_queue() + { + $this->queue->expects($this->once()) + ->method('save'); + $this->method_base->set_use_queue(false); + $this->method_base->save_queue(); + $this->method_base->set_use_queue(true); + $this->method_base->save_queue(); + } + + public function test_template_no_lang() + { + $template_mock = $this->getMockBuilder(\phpbb\template\template::class) + ->disableOriginalConstructor() + ->getMock(); + $filenames = []; + $template_mock->method('set_filenames') + ->willReturnCallback(function($filename_array) use (&$filenames, $template_mock) { + $filenames = array_merge($filenames, $filename_array); + + return $template_mock; + }); + + $base_reflection = new \ReflectionClass($this->method_base); + $template_reflection = $base_reflection->getProperty('template'); + $template_reflection->setValue($this->method_base, $template_mock); + + $this->config->set('default_lang', 'en'); + $this->method_base->template('test'); + $this->assertEquals(['body' => 'test.txt'], $filenames); + } + + public function test_template_template_path() + { + global $phpbb_root_path; + + $template_mock = $this->getMockBuilder(\phpbb\template\template::class) + ->disableOriginalConstructor() + ->getMock(); + $filenames = []; + $template_mock->method('set_filenames') + ->willReturnCallback(function($filename_array) use (&$filenames, $template_mock) { + $filenames = array_merge($filenames, $filename_array); + + return $template_mock; + }); + $template_mock->method('set_custom_style') + ->willReturnCallback(function($path_name, $paths) use($phpbb_root_path) { + $this->assertEquals([['name' => 'en_email', 'ext_path' => 'language/en/email']], $path_name); + $this->assertEquals([$phpbb_root_path . 'language/en/email'], $paths); + }); + + $base_reflection = new \ReflectionClass($this->method_base); + $template_reflection = $base_reflection->getProperty('template'); + $template_reflection->setValue($this->method_base, $template_mock); + + $this->config->set('default_lang', 'en'); + $this->method_base->template('test', '', $phpbb_root_path . 'language/en/email'); + $this->assertEquals(['body' => 'test.txt'], $filenames); + } + + public function test_template_path_fallback() + { + global $phpbb_root_path; + + $template_mock = $this->getMockBuilder(\phpbb\template\template::class) + ->disableOriginalConstructor() + ->getMock(); + $filenames = []; + $template_mock->method('set_filenames') + ->willReturnCallback(function($filename_array) use (&$filenames, $template_mock) { + $filenames = array_merge($filenames, $filename_array); + + return $template_mock; + }); + $template_mock->method('set_custom_style') + ->willReturnCallback(function($path_name, $paths) use($phpbb_root_path) { + $this->assertEquals([ + ['name' => 'de_email', 'ext_path' => 'language/de/email'], + ['name' => 'en_email', 'ext_path' => 'language/en/email'], + ], $path_name); + $this->assertEquals([ + $phpbb_root_path . 'language/de/email', + $phpbb_root_path . 'language/en/email' + ], $paths); + }); + + $base_reflection = new \ReflectionClass($this->method_base); + $template_reflection = $base_reflection->getProperty('template'); + $template_reflection->setValue($this->method_base, $template_mock); + + $this->config->set('default_lang', 'de'); + $this->method_base->template('test', 'de'); + $this->assertEquals(['body' => 'test.txt'], $filenames); + } +} diff --git a/tests/messenger/method_email_test.php b/tests/messenger/method_email_test.php index be77da02de..c21a883f74 100644 --- a/tests/messenger/method_email_test.php +++ b/tests/messenger/method_email_test.php @@ -576,11 +576,24 @@ class phpbb_messenger_method_email_test extends \phpbb_test_case $this->method_email->to('foo@bar.com'); $this->method_email->subject('Test email'); $this->method_email->template('test', 'en'); + $this->method_email->assign_block_vars('foo', ['bar' => 'baz']); $this->method_email->send(); } - public function test_send_no_queue() + public function email_template_data(): array + { + return [ + ['test'], + ['admin_send_email'], + ['topic_notify'], + ]; + } + + /** + * @dataProvider email_template_data + */ + public function test_send_no_queue($email_template) { global $phpbb_root_path; @@ -636,7 +649,7 @@ class phpbb_messenger_method_email_test extends \phpbb_test_case $this->method_email->to('foo@bar.com'); $this->method_email->subject('Test email'); - $this->method_email->template('test', 'en'); + $this->method_email->template($email_template, 'en'); $this->assertTrue($this->method_email->send()); $this->assertEquals(1, $sent_emails); From a63d423c8632f4a328d17c744984a05c22d84df7 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 8 Apr 2025 22:37:03 +0200 Subject: [PATCH 10/12] [ticket/17490] Start implementing unit tests for jabber PHPBB-17490 --- tests/messenger/method_jabber_test.php | 311 +++++++++++++++++++++++++ 1 file changed, 311 insertions(+) create mode 100644 tests/messenger/method_jabber_test.php diff --git a/tests/messenger/method_jabber_test.php b/tests/messenger/method_jabber_test.php new file mode 100644 index 0000000000..d61cd9b1b1 --- /dev/null +++ b/tests/messenger/method_jabber_test.php @@ -0,0 +1,311 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +class phpbb_messenger_method_jabber_test extends \phpbb_test_case +{ + protected $assets_bag; + protected $cache_path; + protected config $config; + protected $dispatcher; + protected $extension_manager; + protected jabber $method_jabber; + protected $method_base; + protected $language; + protected $log; + protected $path_helper; + protected queue $queue; + protected $request; + protected $twig_extensions_collection; + protected $twig_lexer; + protected $user; + + public function setUp(): void + { + global $config, $request, $symfony_request, $user, $phpbb_root_path, $phpEx; + + $this->assets_bag = new assets_bag(); + $this->cache_path = $phpbb_root_path . 'cache/' . PHPBB_ENVIRONMENT . '/twig'; + $this->config = new config([ + 'force_server_vars' => false, + 'jab_username' => 'test', + 'jab_password' => 'password', + 'jab_use_ssl' => false, + 'jab_host' => 'localhost', + 'jab_port' => 5222, + 'jab_verify_peer' => true, + 'jab_verify_peer_name' => true, + 'jab_allow_self_signed' => false, + ]); + $config = $this->config; + $this->dispatcher = $this->getMockBuilder('\phpbb\event\dispatcher') + ->disableOriginalConstructor() + ->getMock(); + $this->filesystem = new \phpbb\filesystem\filesystem(); + $this->language = new language(new language_file_loader($phpbb_root_path, $phpEx)); + $this->queue = $this->createMock(queue::class); + $this->request = new phpbb_mock_request(); + $request = $this->request; + $this->symfony_request = new symfony_request(new phpbb_mock_request()); + $symfony_request = $this->symfony_request; + $this->user = $this->getMockBuilder('\phpbb\user') + ->setConstructorArgs([$this->language, '\phpbb\datetime']) + ->getMock(); + $user = $this->user; + $user->page['root_script_path'] = 'phpbb/'; + $this->user->host = 'yourdomain.com'; + $this->path_helper = new path_helper( + $this->symfony_request, + $this->request, + $phpbb_root_path, + $phpEx + ); + $phpbb_container = new phpbb_mock_container_builder; + $this->twig_extensions_collection = new \phpbb\di\service_collection($phpbb_container); + $twig = new \phpbb\template\twig\environment( + $this->assets_bag, + $this->config, + $this->filesystem, + $this->path_helper, + $this->cache_path, + null, + new \phpbb\template\twig\loader(''), + $this->dispatcher, + array( + 'cache' => false, + 'debug' => false, + 'auto_reload' => true, + 'autoescape' => false, + ) + ); + $this->twig_lexer = new \phpbb\template\twig\lexer($twig); + $this->extension_manager = new phpbb_mock_extension_manager( + __DIR__ . '/', + array( + 'vendor2/foo' => array( + 'ext_name' => 'vendor2/foo', + 'ext_active' => '1', + 'ext_path' => 'ext/vendor2/foo/', + ), + ) + ); + $this->log = $this->createMock(\phpbb\log\log_interface::class); + + $this->method_jabber = new jabber( + $this->assets_bag, + $this->config, + $this->dispatcher, + $this->language, + $this->queue, + $this->path_helper, + $this->request, + $this->twig_extensions_collection, + $this->twig_lexer, + $this->user, + $phpbb_root_path, + $this->cache_path, + $this->extension_manager, + $this->log + ); + } + + public function test_miscellaneous() + { + $this->method_jabber->init(); + $this->assertEquals(messenger_interface::NOTIFY_IM, $this->method_jabber->get_id()); + $this->assertEquals('jabber', $this->method_jabber->get_queue_object_name()); + $this->assertFalse($this->method_jabber->is_enabled()); + $this->config->set('jab_enable', true); + $this->assertTrue($this->method_jabber->is_enabled()); + $this->assertEquals(@extension_loaded('openssl'), $this->method_jabber->can_use_ssl()); + } + + public function test_stream_options() + { + $this->method_jabber->init(); + $this->assertEquals($this->method_jabber, $this->method_jabber->stream_options([ + 'allow_self_signed' => true, + ])); + + $stream_options_reflection = new \ReflectionProperty($this->method_jabber, 'stream_options'); + $stream_options = $stream_options_reflection->getValue($this->method_jabber); + $this->assertEquals([ + 'ssl' => [ + 'allow_self_signed' => false, + 'verify_peer' => true, + 'verify_peer_name' => true, + ], + ], $stream_options); + + $this->method_jabber->ssl(true); + + $this->assertEquals($this->method_jabber, $this->method_jabber->stream_options([ + 'allow_self_signed' => true, + ])); + $stream_options = $stream_options_reflection->getValue($this->method_jabber); + $this->assertEquals([ + 'ssl' => [ + 'allow_self_signed' => true, + 'verify_peer' => true, + 'verify_peer_name' => true, + ], + ], $stream_options); + } + + public function test_port_ssl_switch() + { + $port_reflection = new \ReflectionProperty($this->method_jabber, 'port'); + + $this->method_jabber->port(); + $this->assertEquals(5222, $port_reflection->getValue($this->method_jabber)); + + $this->method_jabber->ssl(true) + ->port(); + $this->assertEquals(5223, $port_reflection->getValue($this->method_jabber)); + } + + public function test_username() + { + $jabber_reflection = new \ReflectionClass($this->method_jabber); + $username_reflection = $jabber_reflection->getProperty('username'); + $jid_reflection = $jabber_reflection->getProperty('jid'); + + $this->method_jabber->username('foo@bar'); + $this->assertEquals(['foo', 'bar'], $jid_reflection->getValue($this->method_jabber)); + $this->assertEquals('foo', $username_reflection->getValue($this->method_jabber)); + + $this->method_jabber->username('bar@baz@qux'); + $this->assertEquals(['bar', 'baz@qux'], $jid_reflection->getValue($this->method_jabber)); + $this->assertEquals('bar', $username_reflection->getValue($this->method_jabber)); + } + + public function test_server() + { + $jabber_reflection = new \ReflectionClass($this->method_jabber); + $connect_server_reflection = $jabber_reflection->getProperty('connect_server'); + $server_reflection = $jabber_reflection->getProperty('server'); + + $this->method_jabber->server(); + $this->assertEquals('localhost', $connect_server_reflection->getValue($this->method_jabber)); + $this->assertEquals('localhost', $server_reflection->getValue($this->method_jabber)); + + $this->method_jabber->server('foobar.com'); + $this->assertEquals('foobar.com', $connect_server_reflection->getValue($this->method_jabber)); + $this->assertEquals('foobar.com', $server_reflection->getValue($this->method_jabber)); + + $this->method_jabber->username('foo@bar.com'); + $this->method_jabber->server('foobar.com'); + $this->assertEquals('foobar.com', $connect_server_reflection->getValue($this->method_jabber)); + $this->assertEquals('bar.com', $server_reflection->getValue($this->method_jabber)); + } + + public function test_encrypt_password() + { + $this->method_jabber->init(); + $this->method_jabber->password('password'); + $data = [ + 'realm' => 'example.com', + 'nonce' => '12345', + 'cnonce' => 'abcde', + 'digest-uri' => 'xmpp/example.com', + 'nc' => '00000001', + 'qop' => 'auth', + ]; + + $expected = md5(sprintf( + '%s:%s:%s:%s:%s:%s', + md5(pack('H32', md5('test:example.com:password')) . ':12345:abcde'), + $data['nonce'], + $data['nc'], + $data['cnonce'], + $data['qop'], + md5('AUTHENTICATE:xmpp/example.com') + )); + $this->assertEquals($expected, $this->method_jabber->encrypt_password($data)); + } + + public function test_parse_data() + { + $data = 'key1="value1",key2="value2",key3="value3"'; + $expected = [ + 'key1' => 'value1', + 'key2' => 'value2', + 'key3' => 'value3', + ]; + + $this->assertEquals($expected, $this->method_jabber->parse_data($data)); + } + + public function test_implode_data() + { + $data = [ + 'key1' => 'value1', + 'key2' => 'value2', + 'key3' => 'value3', + ]; + $expected = 'key1="value1",key2="value2",key3="value3"'; + + $this->assertEquals($expected, $this->method_jabber->implode_data($data)); + } + + public function test_xmlize() + { + $xml = 'content'; + $result = $this->method_jabber->xmlize($xml); + + $this->assertArrayHasKey('root', $result); + $this->assertArrayHasKey('child', $result['root'][0]['#']); + $this->assertEquals('content', $result['root'][0]['#']['child'][0]['#']); + $this->assertEquals(['key' => 'value'], $result['root'][0]['#']['child'][0]['@']); + } + + public function test_send_xml() + { + $jabber_mock = $this->getMockBuilder(jabber::class) + ->setConstructorArgs([ + $this->assets_bag, + $this->config, + $this->dispatcher, + $this->language, + $this->queue, + $this->path_helper, + $this->request, + $this->twig_extensions_collection, + $this->twig_lexer, + $this->user, + '', + '', + $this->extension_manager, + $this->log, + ]) + ->onlyMethods(['send_xml']) + ->getMock(); + + $jabber_mock->expects($this->once()) + ->method('send_xml') + ->with('Test') + ->willReturn(true); + + $this->assertTrue($jabber_mock->send_xml('Test')); + } +} From 6fcac2c40f4519a9d4755d1ed211fb4b03252284 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 10 Apr 2025 20:59:30 +0200 Subject: [PATCH 11/12] [ticket/17490] Add unit tests for messenger queue PHPBB-17490 --- tests/messenger/queue_test.php | 344 +++++++++++++++++++++++++++++++++ 1 file changed, 344 insertions(+) create mode 100644 tests/messenger/queue_test.php diff --git a/tests/messenger/queue_test.php b/tests/messenger/queue_test.php new file mode 100644 index 0000000000..e99893d577 --- /dev/null +++ b/tests/messenger/queue_test.php @@ -0,0 +1,344 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +use phpbb\config\config; +use phpbb\filesystem\exception\filesystem_exception; +use phpbb\messenger\queue; + +class phpbb_messenger_queue_test extends phpbb_test_case +{ + protected $config; + protected $dispatcher; + protected $service_collection; + + /** @var queue */ + protected $messenger_queue; + + /** + * Set up the test case + */ + protected function setUp(): void + { + $this->config = new config([ + 'last_queue_run' => time() - 30, + 'queue_interval' => 600, + ]); + $this->dispatcher = $this->getMockBuilder('phpbb\event\dispatcher') + ->disableOriginalConstructor() + ->getMock(); + $this->service_collection = $this->getMockBuilder('phpbb\di\service_collection') + ->disableOriginalConstructor() + ->onlyMethods(['getIterator', 'add_service_class']) + ->getMock(); + + $this->cache_file = __DIR__ . '/../tmp/queue_test'; + + if (file_exists($this->cache_file)) + { + @unlink($this->cache_file); + } + + $this->messenger_queue = $this->getMockBuilder('phpbb\messenger\queue') + ->setConstructorArgs([ + $this->config, + $this->dispatcher, + $this->service_collection, + $this->cache_file + ]) + ->addMethods(['get_data']) + ->getMock(); + + $this->messenger_queue->method('get_data') + ->willReturnCallback(function(){ + $data_reflection = new \ReflectionProperty(queue::class, 'data'); + return $data_reflection->getValue($this->messenger_queue); + }); + } + + public function test_init() + { + $this->messenger_queue->init('email', 5); + $this->assertEquals([ + 'email' => [ + 'package_size' => 5, + 'data' => [], + ] + ], $this->messenger_queue->get_data()); + + $this->messenger_queue->init('jabber', 9); + + $this->assertEquals([ + 'email' => [ + 'package_size' => 5, + 'data' => [], + ], + 'jabber' => [ + 'package_size' => 9, + 'data' => [], + ] + ], $this->messenger_queue->get_data()); + } + + public function test_put() + { + $this->messenger_queue->init('email', 5); + $this->assertEquals([ + 'email' => [ + 'package_size' => 5, + 'data' => [], + ] + ], $this->messenger_queue->get_data()); + + $this->messenger_queue->put('email', ['data1']); + + $this->assertEquals([ + 'email' => [ + 'package_size' => 5, + 'data' => [ + ['data1'], + ], + ], + ], $this->messenger_queue->get_data()); + } + + public function test_process_no_cache_file() + { + $this->assertFileDoesNotExist($this->cache_file); + $this->assertGreaterThan(5, time() - $this->config['last_queue_run']); + $this->messenger_queue->init('email', 5); + $this->assertEquals([ + 'email' => [ + 'package_size' => 5, + 'data' => [], + ] + ], $this->messenger_queue->get_data()); + + $this->messenger_queue->process(); + $this->assertFileDoesNotExist($this->cache_file); + $this->assertLessThan(5, time() - $this->config['last_queue_run']); + } + + public function test_process_no_queue_handling() + { + // First save queue data + $this->assertFileDoesNotExist($this->cache_file); + $this->messenger_queue->init('email', 5); + $this->messenger_queue->init('jabber', 10); + $this->assertEquals([ + 'email' => [ + 'package_size' => 5, + 'data' => [], + ], + 'jabber' => [ + 'package_size' => 10, + 'data' => [], + ] + ], $this->messenger_queue->get_data()); + + $this->messenger_queue->put('email', ['data1']); + $this->messenger_queue->put('jabber', ['data2']); + $this->messenger_queue->save(); + $this->assertFileExists($this->cache_file); + $this->assertEquals([], $this->messenger_queue->get_data()); + + $this->config['last_queue_run'] = time() - 1000; + + // Process the queue + $this->messenger_queue->process(); + } + + public function test_process_no_queue_handling_chmod_exception() + { + // First save queue data + $this->assertFileDoesNotExist($this->cache_file); + $this->messenger_queue->init('email', 5); + $this->messenger_queue->init('jabber', 10); + $this->assertEquals([ + 'email' => [ + 'package_size' => 5, + 'data' => [], + ], + 'jabber' => [ + 'package_size' => 10, + 'data' => [], + ] + ], $this->messenger_queue->get_data()); + + $this->messenger_queue->put('email', ['data1']); + $this->messenger_queue->put('jabber', ['data2']); + $this->messenger_queue->save(); + $this->assertFileExists($this->cache_file); + $this->assertEquals([], $this->messenger_queue->get_data()); + + $this->config['last_queue_run'] = time() - 1000; + + // Override the filesystem to simulate a chmod failure + $filesystem = $this->getMockBuilder('phpbb\filesystem\filesystem') + ->disableOriginalConstructor() + ->onlyMethods(['phpbb_chmod']) + ->getMock(); + $filesystem->method('phpbb_chmod') + ->will($this->throwException(new filesystem_exception('Chmod failed'))); + $filesystem_reflection = new \ReflectionProperty(queue::class, 'filesystem'); + $filesystem_reflection->setAccessible(true); + $filesystem_reflection->setValue($this->messenger_queue, $filesystem); + + // Process the queue + $this->messenger_queue->process(); + } + + public function test_process_complete() + { + // First save queue data + $this->assertFileDoesNotExist($this->cache_file); + $this->messenger_queue->init('email', 5); + $this->messenger_queue->init('jabber', 10); + $this->assertEquals([ + 'email' => [ + 'package_size' => 5, + 'data' => [], + ], + 'jabber' => [ + 'package_size' => 10, + 'data' => [], + ] + ], $this->messenger_queue->get_data()); + + $this->messenger_queue->put('email', ['data1']); + $this->messenger_queue->put('jabber', ['data2']); + $this->messenger_queue->save(); + $this->assertFileExists($this->cache_file); + $this->assertEquals([], $this->messenger_queue->get_data()); + + $this->config['last_queue_run'] = time() - 1000; + + // Prepare service iterator and messenger methods + $email_method = $this->getMockBuilder('phpbb\messenger\method\email') + ->disableOriginalConstructor() + ->onlyMethods(['get_queue_object_name', 'process_queue']) + ->getMock(); + $email_method->method('get_queue_object_name') + ->willReturn('email'); + $email_method->method('process_queue') + ->willReturnCallback(function(array &$queue_data) { + $this->assertEquals([ + 'package_size' => 5, + 'data' => [ + ['data1'], + ], + ], $queue_data['email']); + unset($queue_data['email']); + }); + $jabber_method = $this->getMockBuilder('phpbb\messenger\method\jabber') + ->disableOriginalConstructor() + ->onlyMethods(['get_queue_object_name', 'process_queue']) + ->getMock(); + $jabber_method->method('get_queue_object_name') + ->willReturn('jabber'); + $jabber_method->method('process_queue') + ->willReturnCallback(function(array &$queue_data) { + $this->assertEquals([ + 'package_size' => 10, + 'data' => [ + ['data2'], + ], + ], $queue_data['jabber']); + unset($queue_data['jabber']); + }); + + $this->service_collection->method('getIterator') + ->willReturn(new \ArrayIterator([ + 'email' => $email_method, + 'jabber' => $jabber_method, + ])); + + // Process the queue + $this->messenger_queue->process(); + $this->assertFileDoesNotExist($this->cache_file); + } + + public function test_save_no_data() + { + $this->assertFileDoesNotExist($this->cache_file); + $this->messenger_queue->save(); + $this->assertFileDoesNotExist($this->cache_file); + } + + public function test_save() + { + $this->assertFileDoesNotExist($this->cache_file); + $this->messenger_queue->init('email', 5); + $this->assertEquals([ + 'email' => [ + 'package_size' => 5, + 'data' => [], + ] + ], $this->messenger_queue->get_data()); + + $this->messenger_queue->put('email', ['data1']); + $this->messenger_queue->save(); + $this->assertFileExists($this->cache_file); + $this->assertEquals([], $this->messenger_queue->get_data()); + } + + public function test_save_twice() + { + $this->assertFileDoesNotExist($this->cache_file); + $this->messenger_queue->init('email', 5); + $this->assertEquals([ + 'email' => [ + 'package_size' => 5, + 'data' => [], + ] + ], $this->messenger_queue->get_data()); + + $this->messenger_queue->put('email', ['data1']); + $this->messenger_queue->put('jabber', ['data2']); + $this->messenger_queue->save(); + $this->assertFileExists($this->cache_file); + $this->assertEquals([], $this->messenger_queue->get_data()); + + $this->messenger_queue->put('email', ['data3']); + $this->messenger_queue->save(); + $this->assertEquals([], $this->messenger_queue->get_data()); + } + + public function test_save_chmod_fail() + { + $this->assertFileDoesNotExist($this->cache_file); + $this->messenger_queue->init('email', 5); + $this->assertEquals([ + 'email' => [ + 'package_size' => 5, + 'data' => [], + ] + ], $this->messenger_queue->get_data()); + + $this->messenger_queue->put('email', ['data1']); + + // Override the filesystem to simulate a chmod failure + $filesystem = $this->getMockBuilder('phpbb\filesystem\filesystem') + ->disableOriginalConstructor() + ->onlyMethods(['phpbb_chmod']) + ->getMock(); + $filesystem->method('phpbb_chmod') + ->will($this->throwException(new filesystem_exception('Chmod failed'))); + $filesystem_reflection = new \ReflectionProperty(queue::class, 'filesystem'); + $filesystem_reflection->setAccessible(true); + $filesystem_reflection->setValue($this->messenger_queue, $filesystem); + + $this->messenger_queue->save(); + $this->assertFileExists($this->cache_file); + $this->assertEquals([], $this->messenger_queue->get_data()); + } +} From bf2dd2cd75687c3b8ca393ae631b02fc8deee52f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 12 Apr 2025 09:02:14 +0200 Subject: [PATCH 12/12] [ticket/17490] Remove not needed isset PHPBB-17490 --- phpBB/phpbb/messenger/method/base.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/messenger/method/base.php b/phpBB/phpbb/messenger/method/base.php index c8c7129caf..f3db4155ca 100644 --- a/phpBB/phpbb/messenger/method/base.php +++ b/phpBB/phpbb/messenger/method/base.php @@ -423,7 +423,7 @@ abstract class base implements messenger_interface */ public function save_queue(): void { - if ($this->use_queue && isset($this->queue)) + if ($this->use_queue) { $this->queue->save(); }