From c8b3c66ae7aa33ba01f649fca68d010677a48039 Mon Sep 17 00:00:00 2001 From: Kyle M Hall 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 Signed-off-by: Marcel de Rooy Signed-off-by: Tomas Cohen Arazi (cherry picked from commit e9ce739b74c1357d4ca8fc0136377b4f15de7f6e) Signed-off-by: Fridolin Somers --- 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 object that can be used directly +Returns an I 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