From 7e207c8901be2cfd24afc7d9970e4a0ca42bd0b9 Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 31 Mar 2022 16:29:18 +0530 Subject: [PATCH 1/7] fix: Call Redisearch index creation functions on enabling redisearch in settings --- .../e_commerce_settings.json | 12 ++++++- .../e_commerce_settings.py | 17 ++++++++++ erpnext/e_commerce/redisearch_utils.py | 34 +++++++++---------- erpnext/templates/pages/product_search.py | 10 +++--- 4 files changed, 50 insertions(+), 23 deletions(-) diff --git a/erpnext/e_commerce/doctype/e_commerce_settings/e_commerce_settings.json b/erpnext/e_commerce/doctype/e_commerce_settings/e_commerce_settings.json index d5fb9697f89..62505e61db6 100644 --- a/erpnext/e_commerce/doctype/e_commerce_settings/e_commerce_settings.json +++ b/erpnext/e_commerce/doctype/e_commerce_settings/e_commerce_settings.json @@ -47,6 +47,7 @@ "item_search_settings_section", "redisearch_warning", "search_index_fields", + "is_redisearch_enabled", "show_categories_in_search_autocomplete", "is_redisearch_loaded", "shop_by_category_section", @@ -303,6 +304,7 @@ }, { "default": "1", + "depends_on": "is_redisearch_enabled", "fieldname": "show_categories_in_search_autocomplete", "fieldtype": "Check", "label": "Show Categories in Search Autocomplete", @@ -365,12 +367,19 @@ "fieldname": "show_price_in_quotation", "fieldtype": "Check", "label": "Show Price in Quotation" + }, + { + "default": "0", + "fieldname": "is_redisearch_enabled", + "fieldtype": "Check", + "label": "Enable Redisearch", + "read_only_depends_on": "eval:!doc.is_redisearch_loaded" } ], "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2021-09-02 14:02:44.785824", + "modified": "2022-03-31 16:01:46.308663", "modified_by": "Administrator", "module": "E-commerce", "name": "E Commerce Settings", @@ -389,5 +398,6 @@ ], "sort_field": "modified", "sort_order": "DESC", + "states": [], "track_changes": 1 } \ No newline at end of file diff --git a/erpnext/e_commerce/doctype/e_commerce_settings/e_commerce_settings.py b/erpnext/e_commerce/doctype/e_commerce_settings/e_commerce_settings.py index b5cd067e38f..2bb4ad69784 100644 --- a/erpnext/e_commerce/doctype/e_commerce_settings/e_commerce_settings.py +++ b/erpnext/e_commerce/doctype/e_commerce_settings/e_commerce_settings.py @@ -9,6 +9,7 @@ from frappe.utils import comma_and, flt, unique from erpnext.e_commerce.redisearch_utils import ( create_website_items_index, + define_autocomplete_dictionary, get_indexable_web_fields, is_search_module_loaded, ) @@ -21,6 +22,8 @@ class ShoppingCartSetupError(frappe.ValidationError): class ECommerceSettings(Document): def onload(self): self.get("__onload").quotation_series = frappe.get_meta("Quotation").get_options("naming_series") + + # flag >> if redisearch is installed and loaded self.is_redisearch_loaded = is_search_module_loaded() def validate(self): @@ -34,6 +37,20 @@ class ECommerceSettings(Document): frappe.clear_document_cache("E Commerce Settings", "E Commerce Settings") + self.is_redisearch_enabled_pre_save = frappe.db.get_single_value( + "E Commerce Settings", "is_redisearch_enabled" + ) + + def after_save(self): + self.create_redisearch_indexes() + + def create_redisearch_indexes(self): + # if redisearch is enabled (value changed) create indexes and dictionary + value_changed = self.is_redisearch_enabled != self.is_redisearch_enabled_pre_save + if self.is_redisearch_loaded and self.is_redisearch_enabled and value_changed: + define_autocomplete_dictionary() + create_website_items_index() + def validate_field_filters(self): if not (self.enable_field_filters and self.filter_fields): return diff --git a/erpnext/e_commerce/redisearch_utils.py b/erpnext/e_commerce/redisearch_utils.py index 82829bf1eff..78cc05af38b 100644 --- a/erpnext/e_commerce/redisearch_utils.py +++ b/erpnext/e_commerce/redisearch_utils.py @@ -22,6 +22,12 @@ def get_indexable_web_fields(): return [df.fieldname for df in valid_fields] +def is_redisearch_enabled(): + "Return True only if redisearch is loaded and enabled." + is_redisearch_enabled = frappe.db.get_single_value("E Commerce Settings", "is_redisearch_enabled") + return is_search_module_loaded() and is_redisearch_enabled + + def is_search_module_loaded(): try: cache = frappe.cache() @@ -35,11 +41,11 @@ def is_search_module_loaded(): return False -def if_redisearch_loaded(function): - "Decorator to check if Redisearch is loaded." +def if_redisearch_enabled(function): + "Decorator to check if Redisearch is enabled." def wrapper(*args, **kwargs): - if is_search_module_loaded(): + if is_redisearch_enabled(): func = function(*args, **kwargs) return func return @@ -51,7 +57,7 @@ def make_key(key): return "{0}|{1}".format(frappe.conf.db_name, key).encode("utf-8") -@if_redisearch_loaded +@if_redisearch_enabled def create_website_items_index(): "Creates Index Definition." @@ -91,7 +97,7 @@ def to_search_field(field): return TextField(field) -@if_redisearch_loaded +@if_redisearch_enabled def insert_item_to_index(website_item_doc): # Insert item to index key = get_cache_key(website_item_doc.name) @@ -104,7 +110,7 @@ def insert_item_to_index(website_item_doc): insert_to_name_ac(website_item_doc.web_item_name, website_item_doc.name) -@if_redisearch_loaded +@if_redisearch_enabled def insert_to_name_ac(web_name, doc_name): ac = AutoCompleter(make_key(WEBSITE_ITEM_NAME_AUTOCOMPLETE), conn=frappe.cache()) ac.add_suggestions(Suggestion(web_name, payload=doc_name)) @@ -120,14 +126,14 @@ def create_web_item_map(website_item_doc): return web_item -@if_redisearch_loaded +@if_redisearch_enabled def update_index_for_item(website_item_doc): # Reinsert to Cache insert_item_to_index(website_item_doc) define_autocomplete_dictionary() -@if_redisearch_loaded +@if_redisearch_enabled def delete_item_from_index(website_item_doc): cache = frappe.cache() key = get_cache_key(website_item_doc.name) @@ -141,7 +147,7 @@ def delete_item_from_index(website_item_doc): return True -@if_redisearch_loaded +@if_redisearch_enabled def delete_from_ac_dict(website_item_doc): """Removes this items's name from autocomplete dictionary""" cache = frappe.cache() @@ -149,7 +155,7 @@ def delete_from_ac_dict(website_item_doc): name_ac.delete(website_item_doc.web_item_name) -@if_redisearch_loaded +@if_redisearch_enabled def define_autocomplete_dictionary(): """Creates an autocomplete search dictionary for `name`. Also creats autocomplete dictionary for `categories` if @@ -182,7 +188,7 @@ def define_autocomplete_dictionary(): return True -@if_redisearch_loaded +@if_redisearch_enabled def reindex_all_web_items(): items = frappe.get_all("Website Item", fields=get_fields_indexed(), filters={"published": True}) @@ -208,9 +214,3 @@ def get_fields_indexed(): fields_to_index = fields_to_index + mandatory_fields return fields_to_index - - -# TODO: Remove later -# # Figure out a way to run this at startup -define_autocomplete_dictionary() -create_website_items_index() diff --git a/erpnext/templates/pages/product_search.py b/erpnext/templates/pages/product_search.py index 77a749ef9eb..ce04068cf64 100644 --- a/erpnext/templates/pages/product_search.py +++ b/erpnext/templates/pages/product_search.py @@ -9,7 +9,7 @@ from erpnext.e_commerce.redisearch_utils import ( WEBSITE_ITEM_CATEGORY_AUTOCOMPLETE, WEBSITE_ITEM_INDEX, WEBSITE_ITEM_NAME_AUTOCOMPLETE, - is_search_module_loaded, + is_redisearch_enabled, make_key, ) from erpnext.e_commerce.shopping_cart.product_info import set_product_info_for_website @@ -74,8 +74,8 @@ def search(query): def product_search(query, limit=10, fuzzy_search=True): search_results = {"from_redisearch": True, "results": []} - if not is_search_module_loaded(): - # Redisearch module not loaded + if not is_redisearch_enabled(): + # Redisearch module not enabled search_results["from_redisearch"] = False search_results["results"] = get_product_data(query, 0, limit) return search_results @@ -121,8 +121,8 @@ def convert_to_dict(redis_search_doc): def get_category_suggestions(query): search_results = {"results": []} - if not is_search_module_loaded(): - # Redisearch module not loaded, query db + if not is_redisearch_enabled(): + # Redisearch module not enabled, query db categories = frappe.db.get_all( "Item Group", filters={"name": ["like", "%{0}%".format(query)], "show_in_website": 1}, From 07f17453cd2b673311029764135c3e26128905b8 Mon Sep 17 00:00:00 2001 From: marination Date: Fri, 1 Apr 2022 18:47:01 +0530 Subject: [PATCH 2/7] fix: Use Payload in AutoCompleter (categories in search) and misc - Separate Item group and Item autocomplete dict definition - Add payload along with Item group, containing namke and route - Pass weightage while defining item group autocomplete dict (auto sort) - Use payload while getting results for categories in search - Remove check to show categories, always show - Search fields mandatory if reidsearch enabled - Code separation (rough) --- .../e_commerce_settings.json | 12 +---- erpnext/e_commerce/redisearch_utils.py | 46 +++++++++++++------ erpnext/templates/pages/product_search.py | 8 +++- 3 files changed, 41 insertions(+), 25 deletions(-) diff --git a/erpnext/e_commerce/doctype/e_commerce_settings/e_commerce_settings.json b/erpnext/e_commerce/doctype/e_commerce_settings/e_commerce_settings.json index 62505e61db6..e6f08f708a8 100644 --- a/erpnext/e_commerce/doctype/e_commerce_settings/e_commerce_settings.json +++ b/erpnext/e_commerce/doctype/e_commerce_settings/e_commerce_settings.json @@ -48,7 +48,6 @@ "redisearch_warning", "search_index_fields", "is_redisearch_enabled", - "show_categories_in_search_autocomplete", "is_redisearch_loaded", "shop_by_category_section", "slideshow", @@ -294,6 +293,7 @@ "fieldname": "search_index_fields", "fieldtype": "Small Text", "label": "Search Index Fields", + "mandatory_depends_on": "is_redisearch_enabled", "read_only_depends_on": "eval:!doc.is_redisearch_loaded" }, { @@ -302,14 +302,6 @@ "fieldtype": "Section Break", "label": "Item Search Settings" }, - { - "default": "1", - "depends_on": "is_redisearch_enabled", - "fieldname": "show_categories_in_search_autocomplete", - "fieldtype": "Check", - "label": "Show Categories in Search Autocomplete", - "read_only_depends_on": "eval:!doc.is_redisearch_loaded" - }, { "default": "0", "fieldname": "is_redisearch_loaded", @@ -379,7 +371,7 @@ "index_web_pages_for_search": 1, "issingle": 1, "links": [], - "modified": "2022-03-31 16:01:46.308663", + "modified": "2022-04-01 18:35:56.106756", "modified_by": "Administrator", "module": "E-commerce", "name": "E Commerce Settings", diff --git a/erpnext/e_commerce/redisearch_utils.py b/erpnext/e_commerce/redisearch_utils.py index 78cc05af38b..32b35db04ce 100644 --- a/erpnext/e_commerce/redisearch_utils.py +++ b/erpnext/e_commerce/redisearch_utils.py @@ -157,17 +157,14 @@ def delete_from_ac_dict(website_item_doc): @if_redisearch_enabled def define_autocomplete_dictionary(): - """Creates an autocomplete search dictionary for `name`. - Also creats autocomplete dictionary for `categories` if - checked in E Commerce Settings""" + """ + Defines/Redefines an autocomplete search dictionary for Website Item Name. + Also creats autocomplete dictionary for Published Item Groups. + """ cache = frappe.cache() - name_ac = AutoCompleter(make_key(WEBSITE_ITEM_NAME_AUTOCOMPLETE), conn=cache) - cat_ac = AutoCompleter(make_key(WEBSITE_ITEM_CATEGORY_AUTOCOMPLETE), conn=cache) - - ac_categories = frappe.db.get_single_value( - "E Commerce Settings", "show_categories_in_search_autocomplete" - ) + item_ac = AutoCompleter(make_key(WEBSITE_ITEM_NAME_AUTOCOMPLETE), conn=cache) + item_group_ac = AutoCompleter(make_key(WEBSITE_ITEM_CATEGORY_AUTOCOMPLETE), conn=cache) # Delete both autocomplete dicts try: @@ -176,16 +173,39 @@ def define_autocomplete_dictionary(): except Exception: return False + create_items_autocomplete_dict(autocompleter=item_ac) + create_item_groups_autocomplete_dict(autocompleter=item_group_ac) + + +@if_redisearch_enabled +def create_items_autocomplete_dict(autocompleter): + "Add items as suggestions in Autocompleter." items = frappe.get_all( "Website Item", fields=["web_item_name", "item_group"], filters={"published": 1} ) for item in items: - name_ac.add_suggestions(Suggestion(item.web_item_name)) - if ac_categories and item.item_group: - cat_ac.add_suggestions(Suggestion(item.item_group)) + autocompleter.add_suggestions(Suggestion(item.web_item_name)) - return True + +@if_redisearch_enabled +def create_item_groups_autocomplete_dict(autocompleter): + "Add item groups with weightage as suggestions in Autocompleter." + published_item_groups = frappe.get_all( + "Item Group", fields=["name", "route", "weightage"], filters={"show_in_website": 1} + ) + if not published_item_groups: + return + + for item_group in published_item_groups: + payload = {"name": item_group, "route": item_group.route} + autocompleter.add_suggestions( + Suggestion( + string=item_group.name, + score=item_group.weightage, + payload=payload, # additional info that can be retrieved later + ) + ) @if_redisearch_enabled diff --git a/erpnext/templates/pages/product_search.py b/erpnext/templates/pages/product_search.py index ce04068cf64..94e893e1369 100644 --- a/erpnext/templates/pages/product_search.py +++ b/erpnext/templates/pages/product_search.py @@ -1,6 +1,8 @@ # Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors # License: GNU General Public License v3. See license.txt +import json + import frappe from frappe.utils import cint, cstr from redisearch import AutoCompleter, Client, Query @@ -135,8 +137,10 @@ def get_category_suggestions(query): return search_results ac = AutoCompleter(make_key(WEBSITE_ITEM_CATEGORY_AUTOCOMPLETE), conn=frappe.cache()) - suggestions = ac.get_suggestions(query, num=10) + suggestions = ac.get_suggestions(query, num=10, with_payloads=True) - search_results["results"] = [s.string for s in suggestions] + results = [json.loads(s.payload) for s in suggestions] + + search_results["results"] = results return search_results From ea036e495854e0a65fc2c7fa2065078745e87987 Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 4 Apr 2022 11:07:53 +0530 Subject: [PATCH 3/7] fix: Better Exception Handling and vaeiabl naming - Function to handle RS exceptions (create log and raise error) - Handle `ResponseError` where it is anticipated - Misc: Better variables --- erpnext/e_commerce/redisearch_utils.py | 44 ++++++++++++++++++-------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/erpnext/e_commerce/redisearch_utils.py b/erpnext/e_commerce/redisearch_utils.py index 32b35db04ce..f9890cca1a8 100644 --- a/erpnext/e_commerce/redisearch_utils.py +++ b/erpnext/e_commerce/redisearch_utils.py @@ -1,8 +1,10 @@ -# Copyright (c) 2021, Frappe Technologies Pvt. Ltd. and Contributors +# Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors # License: GNU General Public License v3. See license.txt import frappe +from frappe import _ from frappe.utils.redis_wrapper import RedisWrapper +from redis import ResponseError from redisearch import AutoCompleter, Client, IndexDefinition, Suggestion, TagField, TextField WEBSITE_ITEM_INDEX = "website_items_index" @@ -38,7 +40,7 @@ def is_search_module_loaded(): ) return "search" in parsed_output except Exception: - return False + return False # handling older redis versions def if_redisearch_enabled(function): @@ -64,15 +66,18 @@ def create_website_items_index(): # CREATE index client = Client(make_key(WEBSITE_ITEM_INDEX), conn=frappe.cache()) - # DROP if already exists try: - client.drop_index() - except Exception: + client.drop_index() # drop if already exists + except ResponseError: + # will most likely raise a ResponseError if index does not exist + # ignore and create index pass + except Exception: + raise_redisearch_error() idx_def = IndexDefinition([make_key(WEBSITE_ITEM_KEY_PREFIX)]) - # Based on e-commerce settings + # Index fields mentioned in e-commerce settings idx_fields = frappe.db.get_single_value("E Commerce Settings", "search_index_fields") idx_fields = idx_fields.split(",") if idx_fields else [] @@ -104,8 +109,8 @@ def insert_item_to_index(website_item_doc): cache = frappe.cache() web_item = create_web_item_map(website_item_doc) - for k, v in web_item.items(): - super(RedisWrapper, cache).hset(make_key(key), k, v) + for field, value in web_item.items(): + super(RedisWrapper, cache).hset(make_key(key), field, value) insert_to_name_ac(website_item_doc.web_item_name, website_item_doc.name) @@ -120,8 +125,8 @@ def create_web_item_map(website_item_doc): fields_to_index = get_fields_indexed() web_item = {} - for f in fields_to_index: - web_item[f] = website_item_doc.get(f) or "" + for field in fields_to_index: + web_item[field] = website_item_doc.get(field) or "" return web_item @@ -141,7 +146,7 @@ def delete_item_from_index(website_item_doc): try: cache.delete(key) except Exception: - return False + raise_redisearch_error() delete_from_ac_dict(website_item_doc) return True @@ -171,7 +176,7 @@ def define_autocomplete_dictionary(): cache.delete(make_key(WEBSITE_ITEM_NAME_AUTOCOMPLETE)) cache.delete(make_key(WEBSITE_ITEM_CATEGORY_AUTOCOMPLETE)) except Exception: - return False + raise_redisearch_error() create_items_autocomplete_dict(autocompleter=item_ac) create_item_groups_autocomplete_dict(autocompleter=item_group_ac) @@ -217,8 +222,8 @@ def reindex_all_web_items(): web_item = create_web_item_map(item) key = make_key(get_cache_key(item.name)) - for k, v in web_item.items(): - super(RedisWrapper, cache).hset(key, k, v) + for field, value in web_item.items(): + super(RedisWrapper, cache).hset(key, field, value) def get_cache_key(name): @@ -234,3 +239,14 @@ def get_fields_indexed(): fields_to_index = fields_to_index + mandatory_fields return fields_to_index + + +def raise_redisearch_error(): + "Create an Error Log and raise error." + traceback = frappe.get_traceback() + log = frappe.log_error(traceback, frappe._("Redisearch Error")) + log_link = frappe.utils.get_link_to_form("Error Log", log.name) + + frappe.throw( + msg=_("Something went wrong. Check {0}").format(log_link), title=_("Redisearch Error") + ) From 97e3a855f7cb3f7efa4068e5480ad57e9158a37e Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 4 Apr 2022 11:32:49 +0530 Subject: [PATCH 4/7] fix: Convert payload to string before adding to autocompleter --- erpnext/e_commerce/redisearch_utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/erpnext/e_commerce/redisearch_utils.py b/erpnext/e_commerce/redisearch_utils.py index f9890cca1a8..95b74e0795c 100644 --- a/erpnext/e_commerce/redisearch_utils.py +++ b/erpnext/e_commerce/redisearch_utils.py @@ -1,6 +1,8 @@ # Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors # License: GNU General Public License v3. See license.txt +import json + import frappe from frappe import _ from frappe.utils.redis_wrapper import RedisWrapper @@ -203,7 +205,7 @@ def create_item_groups_autocomplete_dict(autocompleter): return for item_group in published_item_groups: - payload = {"name": item_group, "route": item_group.route} + payload = json.dumps({"name": item_group, "route": item_group.route}) autocompleter.add_suggestions( Suggestion( string=item_group.name, From 7ef1ccbe8489d38168517758a8badfde2c1dc6fb Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 4 Apr 2022 12:04:35 +0530 Subject: [PATCH 5/7] fix: Add default score of 1 to Item Group Autocompleter - If score 0 is inserted into suggestions, RS does not consider that suggestion --- erpnext/e_commerce/redisearch_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/e_commerce/redisearch_utils.py b/erpnext/e_commerce/redisearch_utils.py index 95b74e0795c..b2f97e308c4 100644 --- a/erpnext/e_commerce/redisearch_utils.py +++ b/erpnext/e_commerce/redisearch_utils.py @@ -209,7 +209,7 @@ def create_item_groups_autocomplete_dict(autocompleter): autocompleter.add_suggestions( Suggestion( string=item_group.name, - score=item_group.weightage, + score=frappe.utils.flt(item_group.weightage) or 1.0, payload=payload, # additional info that can be retrieved later ) ) From 3445682563374f247327e1242fddd346f81d6c15 Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 4 Apr 2022 12:33:25 +0530 Subject: [PATCH 6/7] fix: Payload incorrect data (pass item_group.name) --- erpnext/e_commerce/redisearch_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/erpnext/e_commerce/redisearch_utils.py b/erpnext/e_commerce/redisearch_utils.py index b2f97e308c4..f2dd796f2c5 100644 --- a/erpnext/e_commerce/redisearch_utils.py +++ b/erpnext/e_commerce/redisearch_utils.py @@ -205,7 +205,7 @@ def create_item_groups_autocomplete_dict(autocompleter): return for item_group in published_item_groups: - payload = json.dumps({"name": item_group, "route": item_group.route}) + payload = json.dumps({"name": item_group.name, "route": item_group.route}) autocompleter.add_suggestions( Suggestion( string=item_group.name, From 397b46a08b4b5b1dde1cd89ccbd0ad25304395bc Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 4 Apr 2022 13:24:56 +0530 Subject: [PATCH 7/7] chore: Add TODOs(perf) for Redisearch Query --- erpnext/templates/pages/product_search.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/erpnext/templates/pages/product_search.py b/erpnext/templates/pages/product_search.py index 94e893e1369..3ed056f55e7 100644 --- a/erpnext/templates/pages/product_search.py +++ b/erpnext/templates/pages/product_search.py @@ -88,6 +88,8 @@ def product_search(query, limit=10, fuzzy_search=True): red = frappe.cache() query = clean_up_query(query) + # TODO: Check perf/correctness with Suggestions & Query vs only Query + # TODO: Use Levenshtein Distance in Query (max=3) ac = AutoCompleter(make_key(WEBSITE_ITEM_NAME_AUTOCOMPLETE), conn=red) client = Client(make_key(WEBSITE_ITEM_INDEX), conn=red) suggestions = ac.get_suggestions(