From 19bb00d9e181292eaabcb38440c9fa7735574389 Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Thu, 12 Sep 2024 11:14:22 -0300
Subject: [PATCH] Bug 35655: Only use STOMP if JobsNotificationMethod set to do
 so

This patch makes the background jobs (jobs from now on) connect step not
try to connect to the STOMP broker, unless explicitly told to do so.

This is achieved by using a newly introduced system preference
`JobsNotificationMethod` which falls back to STOMP if undefined.

The "about" page now has 3 states for

+ Message broker: 	Using SQL polling
+ Message broker: 	Using SQL polling (Fallback, Error connecting to RabbitMQ)
+ Message broker: 	Using RabbitMQ

The goal for this patch is to stop flooding the logs on very busy sites
when Rabbit is not being used. This shouldn't predate any efforts on
making the STOMP based jobs brokering mechanism solid and usable.

To test:
1. On clean `main`, run:
   $ ktd --shell
2. Open the logs on a separate terminal:
   $ ktd --shell
  k$ tail -f /var/log/koha/kohadev/*.log
3. Stage some MARC for import
=> SUCCESS: No warnings about STOMP
4. Import the staged records
=> SUCCESS: No warnings about STOMP
5. Stop rabbitmq:
  k$ sudo service rabbitmq-server stop
6. Repeat 2-4
=> FAIL: The logs say it couldn't connect to STOMP
7. Apply this patches and run:
  k$ restart_all
8a. See the "about" page, should read:
    Message broker: Using SQL polling (Fallback, Error connecting to RabbitMQ)
8b. Repeat 2-4
    should work with one warning in the
9. Start rabbit:
  k$ sudo service rabbitmq-server start
  k$ restart_all (not really necessary)
10a. See the "about" page, should read:
    Message broker: 	Using RabbitMQ
10b. Repeat 2-4
=> SUCCESS: No warnings about STOMP
11. Change the `JobsNotificationMethod` system preference to *polling*
   and stop rabbit:
  k$ sudo service rabbitmq-server stop
  k$ restart_all
12a. See the "about" page, should read:
Message broker: 	Using SQL polling
12b. Repeat 2-4
=> SUCCESS: No warnings about STOMP, not even on startup
13. Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 Koha/BackgroundJob.pm                         | 24 ++++++++++++-------
 about.pl                                      | 13 ++++++----
 .../intranet-tmpl/prog/en/modules/about.tt    |  4 +++-
 3 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/Koha/BackgroundJob.pm b/Koha/BackgroundJob.pm
index 33a6872ec0..c21fdd2510 100644
--- a/Koha/BackgroundJob.pm
+++ b/Koha/BackgroundJob.pm
@@ -63,6 +63,10 @@ Connect to the message broker using default guest/guest credential
 
 sub connect {
     my ( $self );
+    my $notification_method = C4::Context->preference('JobsNotificationMethod') // 'STOMP';
+    return undef
+        unless $notification_method eq 'STOMP';
+
     my $hostname = 'localhost';
     my $port = '61613';
     my $config = C4::Context->config('message_broker');
@@ -77,8 +81,17 @@ sub connect {
         $credentials->{passcode} = $config->{password} if $config->{password};
         $credentials->{host} = $config->{vhost} if $config->{vhost};
     }
-    my $stomp = Net::Stomp->new( { hostname => $hostname, port => $port } );
-    $stomp->connect( $credentials );
+
+    my $stomp;
+
+    try {
+        $stomp = Net::Stomp->new( { hostname => $hostname, port => $port } );
+        $stomp->connect( $credentials );
+    } catch {
+        warn "Cannot connect to broker " . $_;
+        $stomp = undef;
+    };
+
     return $stomp;
 }
 
@@ -123,12 +136,7 @@ sub enqueue {
 
     $job_args->{job_id} = $self->id;
 
-    my $conn;
-    try {
-        $conn = $self->connect;
-    } catch {
-        warn "Cannot connect to broker " . $_;
-    };
+    my $conn = $self->connect;
     return $self->id unless $conn;
 
     $json_args = $json->encode($job_args);
diff --git a/about.pl b/about.pl
index 6233d8be23..4e54b5a2fd 100755
--- a/about.pl
+++ b/about.pl
@@ -906,10 +906,15 @@ sub message_broker_check {
     my $template = shift;
     {
         # BackgroundJob - test connection to message broker
-        eval { Koha::BackgroundJob->connect; };
-        if ($@) {
-            warn $@;
-            $template->param( warnConnectBroker => $@ );
+        my $conn = Koha::BackgroundJob->connect;
+        if (! $conn) {
+            if (C4::Context->preference('JobsNotificationMethod') eq 'STOMP' ) {
+                $template->param( warnConnectBroker => 'Error connecting' );
+            } else {
+                $template->param(
+                    warnConnectBroker => C4::Context->preference('JobsNotificationMethod')
+                );
+            }
         }
     }
   }
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/about.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/about.tt
index 0713215662..13e7c652c3 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/about.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/about.tt
@@ -202,7 +202,9 @@
             </tr>
             <tr>
                 <th scope="row">Message broker: </th>
-                [% IF warnConnectBroker %]
+                [% IF warnConnectBroker == 'Error connecting' %]
+                    <td class="text-bg-warning"><strong>Using SQL polling (Fallback, Error connecting to RabbitMQ)</strong></td>
+                [% ELSIF warnConnectBroker %]
                     <td class="text-bg-warning"><strong>Using SQL polling</strong></td>
                 [% ELSE %]
                     <td class="text-bg-info">Using RabbitMQ</td>
-- 
2.39.5