Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: avoid crash if ingredients services called without ingredients_lc #11055

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

stephanegigandet
Copy link
Contributor

fixes #11047

In the future, it would be a good thing to enable the services to add error messages that could be returned to the caller, when there is missing data for instance.

@stephanegigandet stephanegigandet requested a review from a team as a code owner November 25, 2024 15:12
@github-actions github-actions bot added 🥗🔍 Ingredients analysis https://wiki.openfoodfacts.org/Ingredients_Extraction_and_Analysis 🥗 Ingredients labels Nov 25, 2024
@@ -1604,7 +1604,7 @@ Array of specific ingredients.

sub parse_origins_from_text ($product_ref, $text) {

my $ingredients_lc = $product_ref->{ingredients_lc} || $product_ref->{lc};
my $ingredients_lc = $product_ref->{ingredients_lc} || $product_ref->{lc} || $product_ref->{lang};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where, if we add an Object approach, we could have a method for that on product. (would also work to define a sub get_product_lang)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm refactoring it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't you add a test for this scenario ?

@github-actions github-actions bot added API Issues related to the Open Food Facts API. More specific labels exist & should be used (API WRITE…) 🖼️ Images 🧪 tests 📍 Origins Origins are used for Eco-Score computation. We want to have structured origins. Products labels Nov 26, 2024
@stephanegigandet
Copy link
Contributor Author

@alexgarel Thank you for the feedback.

I refactored some things, and then that led to refactoring more things:

  • get_or_select_ingredients_lc() : new function called by services that expect ingredients_lc to be set
  • select_ingredients_lc() : @language_codes may not be computed is called from a service, so finding existing fields differently
    also sets ingredients_text if needed
  • APIProductServices: added a way for services to pass back error messages that can be reported in the API response
  • added tests for the parse_ingredients_text service

@stephanegigandet
Copy link
Contributor Author

/update_tests_results

Open Food Facts Bot and others added 2 commits November 27, 2024 14:49
@github-actions github-actions bot added the 🧬 Taxonomies https://wiki.openfoodfacts.org/Global_taxonomies label Nov 29, 2024
Copy link

sonarcloud bot commented Nov 29, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🥜 Allergens API Issues related to the Open Food Facts API. More specific labels exist & should be used (API WRITE…) categories 🖼️ Images 🥗🔍 Ingredients analysis https://wiki.openfoodfacts.org/Ingredients_Extraction_and_Analysis Ingredients processing 🥗 Ingredients 📍 Origins Origins are used for Eco-Score computation. We want to have structured origins. Products 🧬 Taxonomies https://wiki.openfoodfacts.org/Global_taxonomies 🧪 tests
Projects
Status: Reviewer approved
Status: In progress
Development

Successfully merging this pull request may close these issues.

crash in ingredient analysis service: Undef $lc in call to get_string_id_for_lang
2 participants