From 1b5ad1c7335dbec77a24fa1ae3b41a24bb557f61 Mon Sep 17 00:00:00 2001 From: khaledk2 Date: Thu, 1 Jun 2023 21:46:16 +0100 Subject: [PATCH 1/3] Fix issue of different resources combined with OR clauses returns no results --- .../api/v1/resources/query_handler.py | 32 +-- .../v1/resources/swagger_docs/submitquery.yml | 5 - .../submitquery_returncontainers.yml | 5 - omero_search_engine/api/v1/resources/utils.py | 216 ++++++++++-------- .../validation/results_validator.py | 7 +- 5 files changed, 138 insertions(+), 127 deletions(-) diff --git a/omero_search_engine/api/v1/resources/query_handler.py b/omero_search_engine/api/v1/resources/query_handler.py index 822b6b7f..e76c6f78 100644 --- a/omero_search_engine/api/v1/resources/query_handler.py +++ b/omero_search_engine/api/v1/resources/query_handler.py @@ -70,6 +70,8 @@ def __init__(self, filter, adjust_res=True): """ self.resource = filter.get("resource") self.name = filter.get("name") + if self.name and self.name.lower() == "name": + self.name = "name" self.value = filter.get("value") self.operator = filter.get("operator") if filter.get("set_query_type") and filter.get("query_type"): @@ -238,10 +240,13 @@ def get_image_non_image_query(self): for or_it in self.or_query_group: checked_list = [] main_or_attribute_ = {} + ss = [] + image_or_queries.append(ss) for resource, or_query in or_it.resources_query.items(): checked_list.append(resource) if resource == "image": - image_or_queries.append(or_query) + ss += or_query + # image_or_queries.append(or_query) else: # non image or filters should be inside the or main # attributes filters @@ -252,29 +257,15 @@ def get_image_non_image_query(self): res = self.run_query(query, resource) new_cond = get_ids(res, resource) if new_cond: - if not main_or_attribute_.get(resource): - main_or_attribute_[resource] = new_cond - else: - main_or_attribute_[resource] = ( - main_or_attribute_[resource] + new_cond - ) - - # main_or_attribute.append(new_cond) - # self.additional_image_conds.append(new_cond) + for cond in new_cond: + cond.resource = "image" + ss.append(cond) else: # check if all the conditions have been checked if len(main_or_attribute_.keys()) == 0 and len( checked_list ) == len(or_it.resources_query): return {"Error": "Your query returns no results"} - for res, items_ in main_or_attribute_.items(): - if res not in main_or_attribute: - main_or_attribute[res] = items_ - else: - main_or_attribute[res] = combine_conds( - main_or_attribute[res], items_, res - ) - if len(self.or_query_group) > 0 and len(image_or_queries) == 0: no_conds = 0 for res, item in main_or_attribute.items(): @@ -650,7 +641,7 @@ def determine_search_results_(query_, return_columns=False, return_containers=Fa "This release does not support search by description." ) if q_item.query_type == "main_attribute" and ( - filter["name"] == "name" # or filter["name"] == "description" + filter["name"].lower() == "name" # or filter["name"] == "description" ): if isinstance(q_item.value, list): new_or_filter = [] @@ -688,7 +679,8 @@ def determine_search_results_(query_, return_columns=False, return_containers=Fa "This release does not support search by description." ) if q_item.query_type == "main_attribute" and ( - filter["name"] == "name" # or filter["name"] == "description" + filter["name"].lower() + == "name" # or filter["name"] == "description" ): if isinstance(q_item.value, list): for val in q_item.value: diff --git a/omero_search_engine/api/v1/resources/swagger_docs/submitquery.yml b/omero_search_engine/api/v1/resources/swagger_docs/submitquery.yml index efd57a92..aab371f2 100644 --- a/omero_search_engine/api/v1/resources/swagger_docs/submitquery.yml +++ b/omero_search_engine/api/v1/resources/swagger_docs/submitquery.yml @@ -96,11 +96,6 @@ tags: - Mixed Complex query parameters: - - name: return_columns - description: return additional columns to help display the results in a table - in: query - type: boolean - required: false - name: data in: body required: true diff --git a/omero_search_engine/api/v1/resources/swagger_docs/submitquery_returncontainers.yml b/omero_search_engine/api/v1/resources/swagger_docs/submitquery_returncontainers.yml index 5aab3e29..d16efac0 100644 --- a/omero_search_engine/api/v1/resources/swagger_docs/submitquery_returncontainers.yml +++ b/omero_search_engine/api/v1/resources/swagger_docs/submitquery_returncontainers.yml @@ -53,11 +53,6 @@ tags: - Mixed Complex query parameters: - - name: return_columns - description: return additional columns to help display the results in a table - in: query - type: boolean - required: false - name: data in: body required: true diff --git a/omero_search_engine/api/v1/resources/utils.py b/omero_search_engine/api/v1/resources/utils.py index 41d7d889..d82b165f 100644 --- a/omero_search_engine/api/v1/resources/utils.py +++ b/omero_search_engine/api/v1/resources/utils.py @@ -159,7 +159,6 @@ def get_resource_annotation_table(resource_table): query_template = Template("""{"query": {"bool": {$query}}}""") - # This template is added to the query to return the count of an attribute count_attr_template = Template( """{"key_count": {"terms": {"field": "$field","size": 10000}}} @@ -477,142 +476,163 @@ def elasticsearch_query_builder( for or_filter in or_filters_: should_values = [] shoud_not_value = [] - # should_names = [] + main_should_values = [] + main_shoud_not_value = [] try: key = or_filter["name"].strip() - value = or_filter["value"].strip() + value = str(or_filter["value"]).strip() operator = or_filter["operator"].strip() - except Exception: - return build_error_message( + + except Exception as e: + e_message = build_error_message( "Each Filter needs to have,\ - name, value and operator keywords." + name, value and operator keywords. Error message: %s" + % str(e) + ) + search_omero_app.logger.info(e_message) + + return e_message + # searching using recourse id, e.g. project id + if key.endswith("_id") or key == "id": + main_clause = main_attribute_query_template_id.substitute( # noqa + attribute=key.strip(), + value=value.strip(), ) - if key not in added_keys: - added_keys.append(key) + if or_filter["operator"].strip() == "equals": + main_should_values.append(main_clause) + elif or_filter["operator"].strip() == "not_equals": + main_shoud_not_value.append(main_clause) + else: + if key not in added_keys: + added_keys.append(key) - if operator == "equals": - if case_sensitive: - should_values.append( - case_sensitive_must_value_condition_template.substitute( # noqa - value=value - ) - ) - should_values.append( - case_sensitive_must_name_condition_template.substitute( - name=key - ) - ) - else: - should_values.append( - case_insensitive_must_value_condition_template.substitute( # noqa - value=value - ) - ) - should_values.append( - case_insensitive_must_name_condition_template.substitute( - name=key - ) - ) - elif operator == "contains": - value = "*{value}*".format(value=value) - if case_sensitive: - should_values.append( - case_sensitive_wildcard_value_condition_template.substitute( # noqa - wild_card_value=value + if operator == "equals": + if case_sensitive: + should_values.append( + case_sensitive_must_value_condition_template.substitute( # noqa + value=value + ) ) - ) - should_values.append( - case_sensitive_must_name_condition_template.substitute( - name=key + should_values.append( + case_sensitive_must_name_condition_template.substitute( + name=key + ) ) - ) - else: - should_values.append( - case_insensitive_wildcard_value_condition_template.substitute( # noqa - wild_card_value=value + else: + should_values.append( + case_insensitive_must_value_condition_template.substitute( # noqa + value=value + ) ) - ) - should_values.append( - case_insensitive_must_name_condition_template.substitute( - name=key + should_values.append( + case_insensitive_must_name_condition_template.substitute( # noqa + name=key + ) ) - ) - elif operator in ["not_equals", "not_contains"]: - if operator == "not_contains": + elif operator == "contains": value = "*{value}*".format(value=value) if case_sensitive: - shoud_not_value.append( + should_values.append( case_sensitive_wildcard_value_condition_template.substitute( # noqa wild_card_value=value ) ) - shoud_not_value.append( + should_values.append( case_sensitive_must_name_condition_template.substitute( name=key ) ) else: - shoud_not_value.append( + should_values.append( case_insensitive_wildcard_value_condition_template.substitute( # noqa wild_card_value=value ) ) - shoud_not_value.append( + should_values.append( case_insensitive_must_name_condition_template.substitute( # noqa name=key ) ) - else: - if case_sensitive: - shoud_not_value.append( - case_sensitive_must_value_condition_template.substitute( # noqa - value=value + elif operator in ["not_equals", "not_contains"]: + if operator == "not_contains": + value = "*{value}*".format(value=value) + if case_sensitive: + shoud_not_value.append( + case_sensitive_wildcard_value_condition_template.substitute( # noqa + wild_card_value=value + ) ) - ) - shoud_not_value.append( - case_sensitive_must_name_condition_template.substitute( - name=key + shoud_not_value.append( + case_sensitive_must_name_condition_template.substitute( # noqa + name=key + ) + ) + else: + shoud_not_value.append( + case_insensitive_wildcard_value_condition_template.substitute( # noqa + wild_card_value=value + ) + ) + shoud_not_value.append( + case_insensitive_must_name_condition_template.substitute( # noqa + name=key + ) ) - ) else: - shoud_not_value.append( - case_insensitive_must_value_condition_template.substitute( # noqa - value=value + if case_sensitive: + shoud_not_value.append( + case_sensitive_must_value_condition_template.substitute( # noqa + value=value + ) + ) + shoud_not_value.append( + case_sensitive_must_name_condition_template.substitute( # noqa + name=key + ) + ) + else: + shoud_not_value.append( + case_insensitive_must_value_condition_template.substitute( # noqa + value=value + ) + ) + shoud_not_value.append( + case_insensitive_must_name_condition_template.substitute( # noqa + name=key + ) + ) + elif operator in ["lt", "lte", "gt", "gte"]: + if case_sensitive: + should_values.append( + case_sensitive_range_value_condition_template.substitute( # noqa + operator=operator, value=value ) ) - shoud_not_value.append( - case_insensitive_must_name_condition_template.substitute( # noqa + should_values.append( + case_sensitive_must_name_condition_template.substitute( name=key ) ) - elif operator in ["lt", "lte", "gt", "gte"]: - if case_sensitive: + else: should_values.append( - case_sensitive_range_value_condition_template.substitute( # noqa + case_insensitive_range_value_condition_template.substitute( # noqa operator=operator, value=value ) ) should_values.append( - case_sensitive_must_name_condition_template.substitute( + case_insensitive_must_name_condition_template.substitute( name=key ) ) - else: - should_values.append( - case_insensitive_range_value_condition_template.substitute( # noqa - operator=operator, value=value - ) - ) - should_values.append( - case_insensitive_must_name_condition_template.substitute( - name=key - ) - ) - # must_value_condition - ss = ",".join(should_values) - ff = nested_keyvalue_pair_query_template.substitute(nested=ss) - should_part_list_or.append(ff) + # must_value_condition + if len(main_should_values) > 0: + ss_ = ",".join(main_should_values) + should_part_list_or.append(ss_) + if len(should_values) > 0: + ss = ",".join(should_values) + ff = nested_keyvalue_pair_query_template.substitute(nested=ss) + should_part_list_or.append(ff) if len(shoud_not_value) > 0: ss = ",".join(shoud_not_value) ff = nested_query_template_must_not.substitute(must_not_value=ss) @@ -1072,17 +1092,23 @@ def adjust_query_for_container(query): if or_filters: for filter in or_filters: if isinstance(filter, list): + del_list = [] + new_or = [] for filter_ in filter: if filter_.get("resource") == "container": - new_or_filters.append(get_filter_list(filter_)) - to_delete_or_filter.append(filter_) + new_or += get_filter_list(filter_) + del_list.append(filter_) + for ff in del_list: + filter.remove(ff) + filter += new_or else: if filter.get("resource") == "container": - new_or_filters.append(get_filter_list(filter)) - to_delete_or_filter.append(filter) + filter = get_filter_list(filter) + # to_delete_or_filter.append(filter) else: or_filters = [] query_details["or_filters"] = or_filters + for filter in to_delete_or_filter: if filter in or_filters: or_filters.remove(filter) diff --git a/omero_search_engine/validation/results_validator.py b/omero_search_engine/validation/results_validator.py index eabb2130..64339e40 100644 --- a/omero_search_engine/validation/results_validator.py +++ b/omero_search_engine/validation/results_validator.py @@ -730,10 +730,13 @@ def get_no_images_sql_containers(): ) search_omero_app.logger.info(message2) messages.append(message2) - sql = query_methods["%s_name" % resource].substitute(name=res_name) + sql = query_methods["%s_name" % resource].substitute( + name=res_name, operator="=" + ) results = conn.execute_query(sql) postgres_results = len(results) - message3 = "No of images returned from postgresql: %s" % seachengine_results + message3 = "No of images returned from postgresql: %s" % postgres_results + messages.append(message3) search_omero_app.logger.info(message3) if seachengine_results != postgres_results: From 6ebb45ccae66997be05bd960e0b967015ce7e5a9 Mon Sep 17 00:00:00 2001 From: khaledk2 Date: Sat, 3 Jun 2023 00:03:31 +0100 Subject: [PATCH 2/3] check key is not none --- omero_search_engine/api/v1/resources/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/omero_search_engine/api/v1/resources/utils.py b/omero_search_engine/api/v1/resources/utils.py index d82b165f..c9858ce0 100644 --- a/omero_search_engine/api/v1/resources/utils.py +++ b/omero_search_engine/api/v1/resources/utils.py @@ -493,7 +493,7 @@ def elasticsearch_query_builder( return e_message # searching using recourse id, e.g. project id - if key.endswith("_id") or key == "id": + if key and key.endswith("_id") or key == "id": main_clause = main_attribute_query_template_id.substitute( # noqa attribute=key.strip(), value=value.strip(), From efae6b18e333d94ec3958ab1f1fc1fd733fbff43 Mon Sep 17 00:00:00 2001 From: khaledk2 Date: Sat, 3 Jun 2023 00:06:05 +0100 Subject: [PATCH 3/3] check key and or --- omero_search_engine/api/v1/resources/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/omero_search_engine/api/v1/resources/utils.py b/omero_search_engine/api/v1/resources/utils.py index c9858ce0..7dff250c 100644 --- a/omero_search_engine/api/v1/resources/utils.py +++ b/omero_search_engine/api/v1/resources/utils.py @@ -493,7 +493,7 @@ def elasticsearch_query_builder( return e_message # searching using recourse id, e.g. project id - if key and key.endswith("_id") or key == "id": + if key and (key.endswith("_id") or key == "id"): main_clause = main_attribute_query_template_id.substitute( # noqa attribute=key.strip(), value=value.strip(),