From e9ce739b74c1357d4ca8fc0136377b4f15de7f6e Mon Sep 17 00:00:00 2001
From: Kyle M Hall <kyle@bywatersolutions.com>
Date: Thu, 8 Jun 2023 11:57:47 -0400
Subject: [PATCH] Bug 33964: Use Email::Sender::Transport::SMTP::Persistent for
 sending email

As described in bug 30013, some outgoing SMTP services ( such as Gmail ) do not like Koha's current behavior of initiating a new connection for each email sent.  If we switch from Email::Sender::Transport::SMTP to Email::Sender::Transport::SMTP::Persistent and store the object for the duration of the message queue processing, this should solve that issue.

Signed-off-by: Sam Lau <samalau@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
---
 C4/Letters.pm                     | 11 ++++++++---
 Koha/SMTP/Server.pm               |  6 +++---
 cpanfile                          |  1 +
 t/db_dependent/Koha/SMTP/Server.t |  4 ++--
 4 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/C4/Letters.pm b/C4/Letters.pm
index 7100c4ea71..e190f90c08 100644
--- a/C4/Letters.pm
+++ b/C4/Letters.pm
@@ -980,6 +980,8 @@ sub SendQueuedMessages {
     my $limit = $params->{limit};
     my $where = $params->{where};
 
+    my $smtp_transports = {};
+
     my $count_messages = 0;
     my $unsent_messages = Koha::Notice::Messages->search({
         status  => 'pending',
@@ -1009,7 +1011,7 @@ sub SendQueuedMessages {
         # This is just begging for subclassing
         next if ( lc($message->{'message_transport_type'}) eq 'rss' );
         if ( lc( $message->{'message_transport_type'} ) eq 'email' ) {
-            my $rv = _send_message_by_email( $message, $params->{'username'}, $params->{'password'}, $params->{'method'} );
+            my $rv = _send_message_by_email( $message, $params->{'username'}, $params->{'password'}, $params->{'method'}, $smtp_transports );
             $count_messages++ if $rv;
         }
         elsif ( lc( $message->{'message_transport_type'} ) eq 'sms' ) {
@@ -1315,7 +1317,7 @@ sub _get_unsent_messages {
 
 sub _send_message_by_email {
     my $message = shift or return;
-    my ($username, $password, $method) = @_;
+    my ( $username, $password, $method, $smtp_transports ) = @_;
 
     my $patron;
     my $to_address = $message->{'to_address'};
@@ -1467,8 +1469,11 @@ sub _send_message_by_email {
       if !$message->{to_address}
       || $message->{to_address} ne $email->email->header('To');
 
+    $smtp_transports->{ $smtp_server->id } ||= $smtp_server->transport;
+    my $smtp_transport = $smtp_transports->{ $smtp_server->id };
+
     try {
-        $email->send_or_die({ transport => $smtp_server->transport });
+        $email->send_or_die({ transport => $smtp_transport });
 
         _set_message_status(
             {
diff --git a/Koha/SMTP/Server.pm b/Koha/SMTP/Server.pm
index d357b89e24..44d3158aa8 100644
--- a/Koha/SMTP/Server.pm
+++ b/Koha/SMTP/Server.pm
@@ -61,7 +61,7 @@ sub store {
     $email->transport($transport);
     $email->send_or_die;
 
-Returns an I<Email::Sender::Transport::SMTP> object that can be used directly
+Returns an I<Email::Sender::Transport::SMTP::Persistent> object that can be used directly
 with Email::Sender.
 
 =cut
@@ -88,8 +88,8 @@ sub transport {
 
     $params->{debug} = $self->debug;
 
-    require Email::Sender::Transport::SMTP;
-    my $transport = Email::Sender::Transport::SMTP->new( $params );
+    require Email::Sender::Transport::SMTP::Persistent;
+    my $transport = Email::Sender::Transport::SMTP::Persistent->new( $params );
 
     return $transport;
 }
diff --git a/cpanfile b/cpanfile
index 3dddcad362..ef93a5002e 100644
--- a/cpanfile
+++ b/cpanfile
@@ -41,6 +41,7 @@ requires 'Email::Address', '>= 1.908';
 requires 'Email::Date', '1.103';
 requires 'Email::MessageID', '1.406';
 requires 'Email::Sender', '1.300030';
+requires 'Email::Sender::Transport::SMTP::Persistent', '2.6'
 requires 'Email::Stuffer', '0.014';
 requires 'Exception::Class', '1.38';
 requires 'File::Slurp', '9999.13';
diff --git a/t/db_dependent/Koha/SMTP/Server.t b/t/db_dependent/Koha/SMTP/Server.t
index 5ca2488455..bb84b8da6a 100755
--- a/t/db_dependent/Koha/SMTP/Server.t
+++ b/t/db_dependent/Koha/SMTP/Server.t
@@ -44,13 +44,13 @@ subtest 'transport() tests' => sub {
 
     my $transport = $server->transport;
 
-    is( ref($transport), 'Email::Sender::Transport::SMTP', 'Type is correct' );
+    is( ref($transport), 'Email::Sender::Transport::SMTP::Persistent', 'Type is correct' );
     is( $transport->ssl, 0, 'SSL is not set' );
 
     $server->set({ ssl_mode => '1' })->store;
     $transport = $server->transport;
 
-    is( ref($transport), 'Email::Sender::Transport::SMTP', 'Type is correct' );
+    is( ref($transport), 'Email::Sender::Transport::SMTP::Persistent', 'Type is correct' );
     is( $transport->ssl, '1', 'SSL is set' );
     is( $transport->debug, '0', 'Debug setting honoured (disabled)' );
 
-- 
2.39.5