From 05889765c863cfe72530c88c11774b571df8f85b Mon Sep 17 00:00:00 2001 From: benbenben2 <110821832+benbenben2@users.noreply.github.com> Date: Wed, 8 Jan 2025 19:22:08 +0100 Subject: [PATCH] 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 --- lib/ProductOpener/DataQualityFood.pm | 2 +- lib/ProductOpener/Products.pm | 61 +++++++- .../get-existing-product-gs1-ai-data-str.json | 4 +- .../get-existing-product-gs1-caret.json | 4 +- .../get-existing-product-gs1-data-uri.json | 4 +- .../get-existing-product-gs1-fnc1.json | 4 +- .../get-existing-product-gs1-gs.json | 4 +- ...et-existing-product-with-leading-zero.json | 23 ++- tests/unit/products.t | 134 ++++++++++-------- 9 files changed, 164 insertions(+), 76 deletions(-) diff --git a/lib/ProductOpener/DataQualityFood.pm b/lib/ProductOpener/DataQualityFood.pm index 79a9733fc2772..2f6c5bd53806d 100644 --- 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 index f441518b62509..95ba9f34c6be9 100644 --- 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 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 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/tests/integration/expected_test_results/api_v3_product_read/get-existing-product-gs1-ai-data-str.json b/tests/integration/expected_test_results/api_v3_product_read/get-existing-product-gs1-ai-data-str.json index 71cd0f464f50f..357172cc4ed77 100644 --- a/tests/integration/expected_test_results/api_v3_product_read/get-existing-product-gs1-ai-data-str.json +++ b/tests/integration/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/tests/integration/expected_test_results/api_v3_product_read/get-existing-product-gs1-caret.json b/tests/integration/expected_test_results/api_v3_product_read/get-existing-product-gs1-caret.json index 1ae982623c0be..89df708f0e45e 100644 --- a/tests/integration/expected_test_results/api_v3_product_read/get-existing-product-gs1-caret.json +++ b/tests/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/tests/integration/expected_test_results/api_v3_product_read/get-existing-product-gs1-data-uri.json b/tests/integration/expected_test_results/api_v3_product_read/get-existing-product-gs1-data-uri.json index 1ae982623c0be..89df708f0e45e 100644 --- a/tests/integration/expected_test_results/api_v3_product_read/get-existing-product-gs1-data-uri.json +++ b/tests/integration/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/tests/integration/expected_test_results/api_v3_product_read/get-existing-product-gs1-fnc1.json b/tests/integration/expected_test_results/api_v3_product_read/get-existing-product-gs1-fnc1.json index 71cd0f464f50f..357172cc4ed77 100644 --- a/tests/integration/expected_test_results/api_v3_product_read/get-existing-product-gs1-fnc1.json +++ b/tests/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 index 1ae982623c0be..89df708f0e45e 100644 --- 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/tests/integration/expected_test_results/api_v3_product_read/get-existing-product-with-leading-zero.json b/tests/integration/expected_test_results/api_v3_product_read/get-existing-product-with-leading-zero.json index e1abf80e777d1..357172cc4ed77 100644 --- a/tests/integration/expected_test_results/api_v3_product_read/get-existing-product-with-leading-zero.json +++ b/tests/integration/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 index 51fded8755da5..01199676f1fde 100644 --- 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"');