fix: warnings in producers tests (#11190)

### What ``` Error 2 https://github.com/openfoodfacts/openfoodfacts-server/actions/runs/7462755789/job/20306470532?pr=9288#step:4:15260 warning: GS1Parser error {error => bless( {message => "No GS1 DL keys found in path info",previous_exception => ""}, 'GS1::SyntaxEngine::FFI::EncoderParameterException' )} warning: GS1Parser error {error => bless( {message => "No GS1 DL keys found in path info",previous_exception => ""}, 'GS1::SyntaxEngine::FFI::EncoderParameterException' )} tests/unit/products.t ......................... ok ``` I commented this warning. It can be used for future devs. However, there were additional warnings in the code because is() expected a single comparison and not an array. After fixing this, it showed that some tests were not passing. I fixed them all (see reworks in Products.pm) but could be good if someone with GS1 knowledge could review it. Ping @hangy because I noticed that you worked on this part of the code and GS1::SyntaxEngine::FFI::GS1Encoder ### Related issue(s) and discussion - Related to #9655 --------- Co-authored-by: Open Food Facts Bot <[email protected]>
openfoodfacts · Jan 8, 2025 · 0588976 · 0588976
1 parent c0a5275
commit 0588976
Show file tree

Hide file tree

Showing 9 changed files with 164 additions and 76 deletions.
diff --git a/lib/ProductOpener/DataQualityFood.pm b/lib/ProductOpener/DataQualityFood.pm
@@ -1839,7 +1839,7 @@ sub check_categories ($product_ref) {
 
 	# some categories have an expected minimum number of ingredients
 	# push data quality error if ingredients count is lower than the expected number of ingredients
-	my ($minimum_number_of_ingredients, $category_id2)
+	my ($minimum_number_of_ingredients, $category_id3)
 		= get_inherited_property_from_categories_tags($product_ref, "minimum_number_of_ingredients:en");
 
 	if ((defined $minimum_number_of_ingredients)) {

diff --git a/lib/ProductOpener/Products.pm b/lib/ProductOpener/Products.pm
@@ -332,6 +332,53 @@ sub normalize_code_zeroes($code) {
 	return $code;
 }
 
+=head2 is_valid_upc12($code)
+
+C<is_valid_upc12()> this function validates a UPC-12 code by:
+- checking if the input is exactly 12 digits long,
+- verifying the check digit using the modulo 10 algorithm.
+
+=head3 Arguments
+
+UPC-12 Code in the Raw form: $code
+
+=head3 Return Values
+
+1 (true) if the UPC-12 code is valid, 0 (false) otherwise.
+
+=cut
+
+# use strict;
+# use warnings;
+
+sub is_valid_upc12 {
+	my ($upc) = @_;
+
+	# Check if the input is exactly 12 digits long
+	return 0 unless $upc =~ /^\d{12}$/;
+
+	# Extract the first 11 digits and the check digit
+	my $check_digit = substr($upc, -1);
+	my $upc_without_check_digit = substr($upc, 0, 11);
+
+	# Calculate the check digit
+	my $sum_odd = 0;
+	my $sum_even = 0;
+	for my $i (0 .. 10) {
+		if ($i % 2 == 0) {
+			$sum_odd += substr($upc_without_check_digit, $i, 1);
+		}
+		else {
+			$sum_even += substr($upc_without_check_digit, $i, 1);
+		}
+	}
+	my $total_sum = ($sum_odd * 3) + $sum_even;
+	my $calculated_check_digit = (10 - ($total_sum % 10)) % 10;
+
+	# Validate the check digit
+	return $check_digit == $calculated_check_digit;
+}
+
 =head2 normalize_code_with_gs1_ai($code)
 
 C<normalize_code_with_gs1_ai()> this function normalizes the product code by:
@@ -361,7 +408,19 @@ sub normalize_code_with_gs1_ai ($code) {
 
 		# Keep only digits, remove spaces, dashes and everything else
 		$code =~ s/\D//g;
+
+		# might be upc12
+		if (is_valid_upc12($code)) {
+			$code = "0" . $code;
+		}
+
+		# Check if the length of the code is 14 and the first character is '0'
+		if (length($code) == 14 && substr($code, 0, 1) eq '0') {
+			# Drop the first zero
+			$code = substr($code, 1);
+		}
 	}
+
 	return ($code, $ai_data_str);
 }
 
@@ -399,7 +458,7 @@ sub _try_normalize_code_gs1 ($code) {
 		}
 	};
 	if ($@) {
-		$log->warn("GS1Parser error", {error => $@}) if $log->is_warn();
+		# $log->warn("GS1Parser error", {error => $@}) if $log->is_warn();
 		$code = undef;
 		$ai_data_str = undef;
 	}

diff --git a/...ation/expected_test_results/api_v3_product_read/get-existing-product-gs1-ai-data-str.json b/...ation/expected_test_results/api_v3_product_read/get-existing-product-gs1-ai-data-str.json
@@ -1,5 +1,5 @@
 {
-   "code" : "04260392550101",
+   "code" : "4260392550101",
    "errors" : [],
    "product" : {
       "_id" : "4260392550101",
@@ -1106,7 +1106,7 @@
       {
          "field" : {
             "id" : "code",
-            "value" : "04260392550101"
+            "value" : "4260392550101"
          },
          "impact" : {
             "id" : "none",

diff --git a/...integration/expected_test_results/api_v3_product_read/get-existing-product-gs1-caret.json b/...integration/expected_test_results/api_v3_product_read/get-existing-product-gs1-caret.json
@@ -1,5 +1,5 @@
 {
-   "code" : "04260392550101",
+   "code" : "4260392550101",
    "errors" : [],
    "product" : {
       "_id" : "4260392550101",
@@ -1106,7 +1106,7 @@
       {
          "field" : {
             "id" : "code",
-            "value" : "04260392550101"
+            "value" : "4260392550101"
          },
          "impact" : {
             "id" : "none",

diff --git a/...egration/expected_test_results/api_v3_product_read/get-existing-product-gs1-data-uri.json b/...egration/expected_test_results/api_v3_product_read/get-existing-product-gs1-data-uri.json
@@ -1,5 +1,5 @@
 {
-   "code" : "04260392550101",
+   "code" : "4260392550101",
    "errors" : [],
    "product" : {
       "_id" : "4260392550101",
@@ -1106,7 +1106,7 @@
       {
          "field" : {
             "id" : "code",
-            "value" : "04260392550101"
+            "value" : "4260392550101"
          },
          "impact" : {
             "id" : "none",

diff --git a/.../integration/expected_test_results/api_v3_product_read/get-existing-product-gs1-fnc1.json b/.../integration/expected_test_results/api_v3_product_read/get-existing-product-gs1-fnc1.json
@@ -1,5 +1,5 @@
 {
-   "code" : "04260392550101",
+   "code" : "4260392550101",
    "errors" : [],
    "product" : {
       "_id" : "4260392550101",
@@ -1106,7 +1106,7 @@
       {
          "field" : {
             "id" : "code",
-            "value" : "04260392550101"
+            "value" : "4260392550101"
          },
          "impact" : {
             "id" : "none",

diff --git a/tests/integration/expected_test_results/api_v3_product_read/get-existing-product-gs1-gs.json b/tests/integration/expected_test_results/api_v3_product_read/get-existing-product-gs1-gs.json
@@ -1,5 +1,5 @@
 {
-   "code" : "04260392550101",
+   "code" : "4260392550101",
    "errors" : [],
    "product" : {
       "_id" : "4260392550101",
@@ -1106,7 +1106,7 @@
       {
          "field" : {
             "id" : "code",
-            "value" : "04260392550101"
+            "value" : "4260392550101"
          },
          "impact" : {
             "id" : "none",

diff --git a/...ion/expected_test_results/api_v3_product_read/get-existing-product-with-leading-zero.json b/...ion/expected_test_results/api_v3_product_read/get-existing-product-with-leading-zero.json
@@ -1,5 +1,5 @@
 {
-   "code" : "04260392550101",
+   "code" : "4260392550101",
    "errors" : [],
    "product" : {
       "_id" : "4260392550101",
@@ -1101,6 +1101,23 @@
       "lc_name" : "Product found",
       "name" : "Product found"
    },
-   "status" : "success",
-   "warnings" : []
+   "status" : "success_with_warnings",
+   "warnings" : [
+      {
+         "field" : {
+            "id" : "code",
+            "value" : "4260392550101"
+         },
+         "impact" : {
+            "id" : "none",
+            "lc_name" : "None",
+            "name" : "None"
+         },
+         "message" : {
+            "id" : "different_normalized_product_code",
+            "lc_name" : "",
+            "name" : ""
+         }
+      }
+   ]
 }
diff --git a/tests/unit/products.t b/tests/unit/products.t
@@ -41,75 +41,87 @@ is(normalize_code('0100360505082919'),
 	'0360505082919', 'should reduce GS1 AI unbracketed string to GTIN (13 digits, padded with 0)');
 
 # code normalization with GS1 AI
-is(normalize_code_with_gs1_ai('036000291452'), ('0036000291452', undef), 'GS1: should add leading 0 to valid UPC12');
-is(
-	normalize_code_with_gs1_ai('036000291455'),
-	('036000291455', undef),
-	'GS1: should not add 0 to invalid UPC12, just return as-is'
-);
-is(normalize_code_with_gs1_ai('4015533014963'), ('4015533014963', undef), 'GS1: should just return invalid EAN13');
-ok(!(defined normalize_code_with_gs1_ai(undef)), 'GS1: undef should stay undef');
-is(
-	normalize_code_with_gs1_ai(' just a simple test 4015533014963 here we go '),
-	('4015533014963', undef),
-	'GS1: barcode should always be cleaned from anything but digits'
-);
-is(
-	normalize_code_with_gs1_ai(' just a simple test 036000291452 here we go '),
-	('0036000291452', undef),
-	'GS1: should add leading 0 to cleaned valid UPC12'
-);
-is(
-	normalize_code_with_gs1_ai(' just a simple test 036000291455 here we go '),
-	('036000291455', undef),
-	'GS1: should not add leading 0 to cleaned invalid UPC12'
-);
-is(
-	normalize_code_with_gs1_ai('0104044782317112'),
-	('4044782317112', '(01)04044782317112'),
-	'GS1: should reduce GS1 AI unbracketed string to GTIN'
-);
-is(
-	normalize_code_with_gs1_ai('(01)04044782317112(17)270101'),
-	('4044782317112', '(01)04044782317112(17)270101'),
-	'GS1: should reduce GS1 AI bracketed string to GTIN'
-);
-is(
-	normalize_code_with_gs1_ai('^010404478231711217270101'),
-	('4044782317112', '(01)04044782317112(17)270101'),
-	'GS1: should reduce GS1 AI unbracketed string with ^ as FNC1 to GTIN'
-);
-is(
-	normalize_code_with_gs1_ai("\x{001d}010404478231711217270101"),
-	('4044782317112', '(01)04044782317112(17)270101'),
-	'GS1: should reduce GS1 AI unbracketed string with original FNC1 to GTIN'
-);
-is(
-	normalize_code_with_gs1_ai("\x{241d}010404478231711217270101"),
-	('4044782317112', '(01)04044782317112(17)270101'),
-	'GS1: should reduce GS1 AI unbracketed string with GS as FNC1 to GTIN'
-);
+my $returned_code;
+my $returned_ai_data_str;
+($returned_code, undef) = normalize_code_with_gs1_ai('036000291452');
+is($returned_code, '0036000291452', 'GS1: should add leading 0 to valid UPC12');
+
+($returned_code, undef) = normalize_code_with_gs1_ai('036000291455');
+is($returned_code, '036000291455', 'GS1: should not add 0 to invalid UPC12, just return as-is');
+
+($returned_code, undef) = normalize_code_with_gs1_ai('4015533014963');
+is($returned_code, '4015533014963', 'GS1: should just return invalid EAN13');
+
+($returned_code, undef) = normalize_code_with_gs1_ai(undef);
+ok(!$returned_code, 'GS1: undef should stay undef');
+
+($returned_code, undef) = normalize_code_with_gs1_ai(' just a simple test 4015533014963 here we go ');
+is($returned_code, '4015533014963', 'GS1: barcode should always be cleaned from anything but digits');
+
+($returned_code, undef) = normalize_code_with_gs1_ai(' just a simple test 036000291452 here we go ');
+is($returned_code, '0036000291452', 'GS1: should add leading 0 to cleaned valid UPC12');
+
+($returned_code, undef) = normalize_code_with_gs1_ai(' just a simple test 036000291455 here we go ');
+is($returned_code, '036000291455', 'GS1: should not add leading 0 to cleaned invalid UPC12');
+
+($returned_code, $returned_ai_data_str) = normalize_code_with_gs1_ai('0104044782317112');
+is($returned_code, '4044782317112', 'GS1: should reduce GS1 AI unbracketed string to GTIN - code');
+is($returned_ai_data_str, '(01)04044782317112', 'GS1: should reduce GS1 AI unbracketed string to GTIN - ai');
+
+($returned_code, $returned_ai_data_str) = normalize_code_with_gs1_ai('(01)04044782317112(17)270101');
+is($returned_code, '4044782317112', 'GS1: should reduce GS1 AI bracketed string to GTIN - code');
+is($returned_ai_data_str, '(01)04044782317112(17)270101', 'GS1: should reduce GS1 AI bracketed string to GTIN - ai');
+
+($returned_code, $returned_ai_data_str) = normalize_code_with_gs1_ai('^010404478231711217270101');
+is($returned_code, '4044782317112', 'GS1: should reduce GS1 AI unbracketed string with ^ as FNC1 to GTIN - code');
 is(
-	normalize_code_with_gs1_ai('https://id.gs1.org/01/04044782317112/22/2A'),
-	('4044782317112', '(01)04044782317112(22)2A'),
-	'GS1: should reduce GS1 Digital Link URI string with ^ as FNC1 to GTIN'
+	$returned_ai_data_str,
+	'(01)04044782317112(17)270101',
+	'GS1: should reduce GS1 AI unbracketed string with ^ as FNC1 to GTIN - ai'
 );
+
+# switch to double quote to interpret escape sequences
+($returned_code, $returned_ai_data_str) = normalize_code_with_gs1_ai("\x{001d}010404478231711217270101");
+is($returned_code, '4044782317112', 'GS1: should reduce GS1 AI unbracketed string with original FNC1 to GTIN - code');
 is(
-	normalize_code_with_gs1_ai('https://dalgiardino.com/01/09506000134376/10/ABC/21/123456?17=211200'),
-	('9506000134376', '(01)09506000134376(10)ABC(21)123456(17)211200'),
-	'GS1: should reduce GS1 Digital Link URI to GTIN'
+	$returned_ai_data_str,
+	'(01)04044782317112(17)270101',
+	'GS1: should reduce GS1 AI unbracketed string with original FNC1 to GTIN - ai'
 );
+
+($returned_code, $returned_ai_data_str) = normalize_code_with_gs1_ai("\x{241d}010404478231711217270101");
+is($returned_code, '4044782317112', 'GS1: should reduce GS1 AI unbracketed string with GS as FNC1 to GTIN - code');
 is(
-	normalize_code_with_gs1_ai('https://example.com/01/00012345000058?17=271200'),
-	('0012345000058', '(01)00012345000058(17)271200'),
-	'GS1: should reduce GS1 Digital Link URI to GTIN'
+	$returned_ai_data_str,
+	'(01)04044782317112(17)270101',
+	'GS1: should reduce GS1 AI unbracketed string with GS as FNC1 to GTIN - ai'
 );
+
+($returned_code, $returned_ai_data_str) = normalize_code_with_gs1_ai('https://id.gs1.org/01/04044782317112/22/2A');
+is($returned_code, '4044782317112', 'GS1: should reduce GS1 Digital Link URI string with ^ as FNC1 to GTIN - code');
+is($returned_ai_data_str, '(01)04044782317112(22)2A',
+	'GS1: should reduce GS1 Digital Link URI string with ^ as FNC1 to GTIN - ai');
+
+($returned_code, $returned_ai_data_str)
+	= normalize_code_with_gs1_ai('https://dalgiardino.com/01/09506000134376/10/ABC/21/123456?17=211200');
+is($returned_code, '9506000134376', 'GS1: should reduce GS1 Digital Link URI to GTIN 1 - code');
 is(
-	normalize_code_with_gs1_ai('https://world.openfoodfacts.org/'),
-	('', undef),
-	'GS1: non-GS1 URIs should return an empty string'
+	$returned_ai_data_str,
+	'(01)09506000134376(10)ABC(21)123456(17)211200',
+	'GS1: should reduce GS1 Digital Link URI to GTIN 1 - ai'
 );
-is(normalize_code_with_gs1_ai('http://spam.zip/'), ('', undef), 'GS1: non-GS1 URIs should return an empty string');
+
+($returned_code, $returned_ai_data_str) = normalize_code_with_gs1_ai('https://example.com/01/00012345000058?17=271200');
+is($returned_code, '0012345000058', 'GS1: should reduce GS1 Digital Link URI to GTIN 2 - code');
+is($returned_ai_data_str, '(01)00012345000058(17)271200', 'GS1: should reduce GS1 Digital Link URI to GTIN 2 - ai');
+
+($returned_code, $returned_ai_data_str) = normalize_code_with_gs1_ai('https://world.openfoodfacts.org/');
+is($returned_code, '', 'GS1: non-GS1 URIs should return an empty string 1 - code');
+ok(!$returned_ai_data_str, 'GS1: non-GS1 URIs should return an empty string 1 - ai');
+
+($returned_code, $returned_ai_data_str) = normalize_code_with_gs1_ai('http://spam.zip/');
+is($returned_code, '', 'GS1: should reduce GS1 Digital Link URI to GTIN 2 - code');
+ok(!$returned_ai_data_str, 'GS1: non-GS1 URIs should return an empty string 2 - ai');
 
 # product storage path
 is(product_path_from_id('not a real code'), 'invalid', 'non digit code should return "invalid"');