From 0197f991e80c23842098158f5c9a05c72499eeaf Mon Sep 17 00:00:00 2001 From: Alex Garel Date: Thu, 20 Jun 2024 13:13:53 +0200 Subject: [PATCH 1/7] feat: static_params for scripts Also fix a bug if sort_by is a script with minus in front, and create a real personal sort script and bettre display errors if storing scripts fails --- app/_types.py | 3 ++- app/config.py | 11 +++++++++-- app/es_scripts.py | 23 +++++++++++++++++++---- app/query.py | 9 ++++++++- data/config/openfoodfacts.yml | 21 +++++++++++++++++---- tests/unit/data/openfoodfacts_config.yml | 21 +++++++++++++++++---- 6 files changed, 72 insertions(+), 16 deletions(-) diff --git a/app/_types.py b/app/_types.py index 8195883d..6a6f1909 100644 --- a/app/_types.py +++ b/app/_types.py @@ -289,7 +289,8 @@ def sort_by_scripts_needs_params(self): raise ValueError("`sort_params` must be a dict") # verifies keys are those expected request_keys = set(self.sort_params.keys()) - expected_keys = set(self.index_config.scripts[self.sort_by].params.keys()) + sort_sign, sort_by = self.sign_sort_by + expected_keys = set(self.index_config.scripts[sort_by].params.keys()) if request_keys != expected_keys: missing = expected_keys - request_keys missing_str = ("missing keys: " + ", ".join(missing)) if missing else "" diff --git a/app/config.py b/app/config.py index 9049c22f..5318bc9b 100644 --- a/app/config.py +++ b/app/config.py @@ -295,7 +295,6 @@ class ScriptConfig(BaseModel): ] params: ( Annotated[ - # FIXME: not sure of the type here dict[str, Any], Field( description="Params for the scripts. We need this to retrieve and validate parameters" @@ -303,7 +302,15 @@ class ScriptConfig(BaseModel): ] | None ) - # TODO: do we want to add a list of mandatory parameters ? + static_params: ( + Annotated[ + dict[str, Any], + Field( + description="Additional params for the scripts that can't be supplied by the API (constants)" + ), + ] + | None + ) # Or some type checking/transformation ? diff --git a/app/es_scripts.py b/app/es_scripts.py index 6780894c..722c33ee 100644 --- a/app/es_scripts.py +++ b/app/es_scripts.py @@ -1,8 +1,12 @@ """Module to manage ES scripts that can be used for personalized sorting """ +import elasticsearch + from app import config -from app.utils import connection +from app.utils import connection, get_logger + +logger = get_logger(__name__) def get_script_prefix(index_id: str): @@ -35,10 +39,11 @@ def _store_script( script_id: str, script: config.ScriptConfig, index_config: config.IndexConfig ): """Store a script in Elasticsearch.""" + params = dict((script.params or {}), **(script.static_params or {})) payload = { "lang": script.lang.value, "source": script.source, - "params": script.params, + "params": params, } # hardcode context to scoring for the moment context = "score" @@ -53,7 +58,17 @@ def sync_scripts(index_id: str, index_config: config.IndexConfig) -> dict[str, i # remove them _remove_scripts(current_ids, index_config) # store scripts + stored_scripts = 0 if index_config.scripts: for script_id, script in index_config.scripts.items(): - _store_script(get_script_id(index_id, script_id), script, index_config) - return {"removed": len(current_ids), "added": len(index_config.scripts or [])} + try: + _store_script(get_script_id(index_id, script_id), script, index_config) + stored_scripts += 1 + except elasticsearch.ApiError as e: + logger.error( + "Unable to store script %s, got exception %s: %s", + script_id, + e, + e.body, + ) + return {"removed": len(current_ids), "added": stored_scripts} diff --git a/app/query.py b/app/query.py index 97c67f63..eaec2d43 100644 --- a/app/query.py +++ b/app/query.py @@ -1,4 +1,5 @@ import elastic_transport +import elasticsearch import luqum.exceptions from elasticsearch_dsl import A, Q, Search from elasticsearch_dsl.aggs import Agg @@ -265,12 +266,14 @@ def parse_sort_by_script( if script is None: raise ValueError(f"Unknown script '{sort_by}'") script_id = get_script_id(index_id, sort_by) + # join params and static params + script_params = dict((params or {}), **(script.static_params or {})) return { "_script": { "type": "number", "script": { "id": script_id, - "params": params, + "params": script_params, }, "order": "desc" if operator == "-" else "asc", } @@ -402,6 +405,10 @@ def execute_query( debug = SearchResponseDebug(query=query.to_dict()) try: results = query.execute() + except elasticsearch.ApiError as e: + logger.error("Error while running query: %s %s", str(e), str(e.body)) + errors.append(SearchResponseError(title="es_api_error", description=str(e))) + return ErrorSearchResponse(debug=debug, errors=errors) except elastic_transport.ConnectionError as e: errors.append( SearchResponseError(title="es_connection_error", description=str(e)) diff --git a/data/config/openfoodfacts.yml b/data/config/openfoodfacts.yml index c22741af..dd2c26b5 100644 --- a/data/config/openfoodfacts.yml +++ b/data/config/openfoodfacts.yml @@ -8,11 +8,24 @@ indices: number_of_shards: 4 scripts: personal_score: - lang: expression - source: | - 1 + # see https://www.elastic.co/guide/en/elasticsearch/painless/8.14/index.html + lang: painless + source: |- + int nova_index = (doc['nova_group'].size() != 0) ? doc['nova_group'].value.intValue() : 4; + String nutri_index = (doc['nutriscore_grade'].size() != 0) ? doc['nutriscore_grade'].value : 'e'; + String eco_index = (doc['ecoscore_grade'].size() != 0) ? doc['ecoscore_grade'].value : 'e'; + return ( + params['nova_to_score'][nova_index] * params['nova_group'] + + params['grades_to_score'].getOrDefault(nutri_index, 0) * params['nutri_score'] + + params['grades_to_score'].getOrDefault(eco_index, 0) * params['eco_score'] + ); params: - preferences: 2 + eco_score: 1 + nutri_score: 1 + nova_group: 1 + static_params: + nova_to_score: [100, 100, 100, 75, 4] + grades_to_score: {"a": 100, "b": 75, "c": 50, "d": 25,"e": 0} fields: code: required: true diff --git a/tests/unit/data/openfoodfacts_config.yml b/tests/unit/data/openfoodfacts_config.yml index c22741af..dd2c26b5 100644 --- a/tests/unit/data/openfoodfacts_config.yml +++ b/tests/unit/data/openfoodfacts_config.yml @@ -8,11 +8,24 @@ indices: number_of_shards: 4 scripts: personal_score: - lang: expression - source: | - 1 + # see https://www.elastic.co/guide/en/elasticsearch/painless/8.14/index.html + lang: painless + source: |- + int nova_index = (doc['nova_group'].size() != 0) ? doc['nova_group'].value.intValue() : 4; + String nutri_index = (doc['nutriscore_grade'].size() != 0) ? doc['nutriscore_grade'].value : 'e'; + String eco_index = (doc['ecoscore_grade'].size() != 0) ? doc['ecoscore_grade'].value : 'e'; + return ( + params['nova_to_score'][nova_index] * params['nova_group'] + + params['grades_to_score'].getOrDefault(nutri_index, 0) * params['nutri_score'] + + params['grades_to_score'].getOrDefault(eco_index, 0) * params['eco_score'] + ); params: - preferences: 2 + eco_score: 1 + nutri_score: 1 + nova_group: 1 + static_params: + nova_to_score: [100, 100, 100, 75, 4] + grades_to_score: {"a": 100, "b": 75, "c": 50, "d": 25,"e": 0} fields: code: required: true From 0452d35a905678fd3403c798eec7b1833758b0d4 Mon Sep 17 00:00:00 2001 From: Alex Garel Date: Thu, 20 Jun 2024 13:16:22 +0200 Subject: [PATCH 2/7] feat: support script option in sort_by --- frontend/README.md | 4 ++ frontend/public/off.html | 17 ++--- frontend/src/mixins/history.ts | 6 +- frontend/src/mixins/search-ctl.ts | 100 ++++++++++++++++++--------- frontend/src/search-a-licious.ts | 1 + frontend/src/search-sort-script.ts | 55 +++++++++++++++ frontend/src/search-sort.ts | 24 ++++--- frontend/src/test/search-bar_test.ts | 12 +++- frontend/src/utils/constants.ts | 5 +- 9 files changed, 169 insertions(+), 55 deletions(-) create mode 100644 frontend/src/search-sort-script.ts diff --git a/frontend/README.md b/frontend/README.md index ff642f99..a0b19412 100644 --- a/frontend/README.md +++ b/frontend/README.md @@ -28,6 +28,10 @@ The project is currently composed of several widgets. * you must add searchalicious-sort-field elements inside to add sort options * with a field= to indicate the field * the label is the text inside the element + * or a searchalicious-sort-script + * with a script= to indicate a script + * and a params= which is a either a json encoded object, + or a key in localStorage prefixed with "local:" * you can add element to slot `label` to change the label **IMPORTANT:** diff --git a/frontend/public/off.html b/frontend/public/off.html index 69d93fa2..562d06c6 100644 --- a/frontend/public/off.html +++ b/frontend/public/off.html @@ -186,7 +186,7 @@ const targetValue = preferences[element.name] ?? element.value; const options = Array.from(element.getElementsByTagName("option")); options.forEach(item => { - if (item.value === targetValue) { + if (item.value.toString() === targetValue) { item.selected = true; } }); @@ -221,7 +221,7 @@ for (let i = 0; i < preferences_form_elements.length; i++) { const preference = preferences_form_elements[i]; if (preference.nodeName === "SELECT") { - preferences[preference.name] = preference.value; + preferences[preference.name] = parseInt(preference.value); } } localStorage.setItem("off_preferences", JSON.stringify(preferences)); @@ -301,6 +301,7 @@ Products with the best Nutri-Score Recently added products Recently modified products + Personal preferences @@ -316,20 +317,20 @@