From ced62e8f072d0850eca673208b6f82ffea905ee3 Mon Sep 17 00:00:00 2001
From: Alex Garel <alex@garel.org>
Date: Fri, 14 Oct 2022 19:55:16 +0200
Subject: [PATCH] fix: default redis url to empty and handle reconnects (#7540)

As in dev people don't necessarily have openfoodfacts-search running

We also changed lib to Redis which is better maintained, and handle connection timeout.
---
 .env                          |  4 +-
 cpanfile                      |  2 +-
 lib/ProductOpener/Products.pm |  4 +-
 lib/ProductOpener/Redis.pm    | 97 ++++++++++++++++++++++++++++-------
 4 files changed, 85 insertions(+), 22 deletions(-)

diff --git a/.env b/.env
index e5352638ac415..3d45a5e6feb81 100644
--- a/.env
+++ b/.env
@@ -38,7 +38,9 @@ MONGO_INITDB_ROOT_USERNAME=root
 MONGO_INITDB_ROOT_PASSWORD=test
 ROBOTOFF_URL=http://robotoff.openfoodfacts.localhost:5500 # connect to Robotoff running in separate docker-compose deployment
 EVENTS_URL=
-REDIS_URL=searchredis:6379 # Redis runs in a separate docker-compose deployment
+# use this to push products to openfoodfacts-search
+# in dev: searchredis:6379 (with openfoodfacts-search running in docker in same network)
+REDIS_URL=
 GOOGLE_CLOUD_VISION_API_KEY=
 CROWDIN_PROJECT_IDENTIFIER=
 CROWDIN_PROJECT_KEY=
diff --git a/cpanfile b/cpanfile
index 250a92ea4c438..a10c244cf5f94 100644
--- a/cpanfile
+++ b/cpanfile
@@ -64,7 +64,7 @@ requires 'JSON::Create';
 requires 'JSON::Parse';
 requires 'Data::DeepAccess';
 requires 'XML::XML2JSON';
-requires 'Redis::Client';
+requires 'Redis';
 
 
 # Mojolicious/Minion
diff --git a/lib/ProductOpener/Products.pm b/lib/ProductOpener/Products.pm
index ad195600e19fd..75a0feebd7018 100644
--- a/lib/ProductOpener/Products.pm
+++ b/lib/ProductOpener/Products.pm
@@ -1179,11 +1179,11 @@ sub store_product($user_id, $product_ref, $comment) {
 	store("$new_data_root/products/$path/changes.sto", $changes_ref);
 	log_change($product_ref, $change_ref);
 
+	$log->debug("store_product - done", { code => $code, product_id => $product_id } ) if $log->is_debug();
+
 	# index for search service
 	push_to_search_service($product_ref);
 
-	$log->debug("store_product - done", { code => $code, product_id => $product_id } ) if $log->is_debug();
-
 	return 1;
 }
 
diff --git a/lib/ProductOpener/Redis.pm b/lib/ProductOpener/Redis.pm
index 9b2977ebd5ff9..15860d941c04c 100644
--- a/lib/ProductOpener/Redis.pm
+++ b/lib/ProductOpener/Redis.pm
@@ -1,4 +1,15 @@
 
+=head1 NAME
+
+ProductOpener::Redis - functions to push informations to redis
+
+=head1 DESCRIPTION
+
+C<ProductOpener::Redis> is handling pushing info to Redis
+to communicate updates to openfoodfacts-search instance
+
+=cut
+
 package ProductOpener::Redis;
 
 use ProductOpener::PerlStandards;
@@ -14,36 +25,86 @@ BEGIN {
 
 use vars @EXPORT_OK;
 
-use Log::Any qw($log);
-use ProductOpener::Config2;
-use Redis::Client;
+use Log::Any qw/$log/;
+use ProductOpener::Config2 qw/$redis_url/;
+use Redis;
 
-sub init_redis() {
+=head2 $redis_client
+The connection to redis
+=cut
 
-	$log->debug("init_redis", {redis_url_env => $ENV{REDIS_URL}, redis_url => $ProductOpener::Config2::redis_url})
-	  if $log->is_debug();
+my $redis_client;
 
-	$log->warn("REDIS_URL env", {redis_url => $ENV{REDIS_URL}}) if $log->is_warn();
-	if ($ProductOpener::Config2::redis_url eq "") {
-		$log->warn("Redis URL not provided for search indexing", {error => $@}) if $log->is_warn();
-		return;
-	}
-	my $redis_client;
-	eval {$redis_client = Redis::Client->new(host => $ProductOpener::Config2::redis_url);};
+# tracking if we already displayed a warning
+my $sent_warning_about_missing_redis_url = 0;
+
+=head2 init_redis($is_reconnect=0)
+
+init $redis_client or re-init it if we where disconnected
+
+it is uses  ProductOpener::Config2::redis_url
+
+=cut
+
+sub init_redis() {
+	$log->debug("init_redis", {redis_url => $redis_url})
+	  if $log->is_debug();
+	eval {
+		$redis_client = Redis->new(
+			server => $redis_url,
+			# we don't want to sacrifice too much performance for redis problems
+			cnx_timeout => 1,
+			write_timeout => 1,
+		);
+	};
 	if ($@) {
 		$log->warn("Error connecting to Redis", {error => $@}) if $log->is_warn();
-	}
-	else {
-		return $redis_client;
+		$redis_client = undef;    # this ask for eventual reconnection
 	}
 	return;
 }
 
-my $redis_client = init_redis();
+=head2 push_to_search_service ($product_ref)
+
+Inform openfoodfacts-search that a product was updated.
+It uses Redis to do that.
+
+=head3 Arguments
+
+=head4 Product Object $product_ref
+The product that was updated.
+
+=cut
 
 sub push_to_search_service ($product_ref) {
-	if (defined($redis_client)) {
+
+	if (!$redis_url) {
+		# off search not activated
+		if (!$sent_warning_about_missing_redis_url) {
+			$log->warn("Redis URL not provided for search indexing") if $log->is_warn();
+			$sent_warning_about_missing_redis_url = 1;
+		}
+		return;
+	}
+
+	my $error = "";
+	if (!defined $redis_client) {
+		# we where deconnected, try again
+		$log->info("Trying to reconnect to redis");
+		init_redis();
+	}
+	if (defined $redis_client) {
 		eval {$redis_client->rpush('search_import_queue', $product_ref->{code});};
+		$error = $@;
+	}
+	else {
+		$error = "Can't connect to redis";
+	}
+	if (!($error eq "")) {
+		$log->warn("Failed to push to redis", {product_code => $product_ref->{code}, error => $error})
+		  if $log->is_warn();
+		# ask for eventual reconnection for next call
+		$redis_client = undef;
 	}
 
 	return;