From 04d92f76001b107fc36452057106a0a13124d517 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 4 Nov 2021 11:13:43 +0000 Subject: [PATCH 01/44] refactor: make `QueryFilterFactory` return a list of filters #259 - This is in preparation to add a `SearchAPIQueryFilterFactory` where a single query parameter will have multiple filters - This commit also fixes the tests which impact this change --- .../datagateway_api/query_filter_factory.py | 12 +++--- datagateway_api/common/helpers.py | 2 +- test/db/test_query_filter_factory.py | 39 +++++++++---------- 3 files changed, 26 insertions(+), 27 deletions(-) diff --git a/datagateway_api/common/datagateway_api/query_filter_factory.py b/datagateway_api/common/datagateway_api/query_filter_factory.py index f2e58c52..5d3830f8 100644 --- a/datagateway_api/common/datagateway_api/query_filter_factory.py +++ b/datagateway_api/common/datagateway_api/query_filter_factory.py @@ -57,18 +57,18 @@ def get_query_filter(request_filter): field = list(request_filter[filter_name].keys())[0] operation = list(request_filter[filter_name][field].keys())[0] value = request_filter[filter_name][field][operation] - return WhereFilter(field, value, operation) + return [WhereFilter(field, value, operation)] elif filter_name == "order": field = request_filter["order"].split(" ")[0] direction = request_filter["order"].split(" ")[1] - return OrderFilter(field, direction) + return [OrderFilter(field, direction)] elif filter_name == "skip": - return SkipFilter(request_filter["skip"]) + return [SkipFilter(request_filter["skip"])] elif filter_name == "limit": - return LimitFilter(request_filter["limit"]) + return [LimitFilter(request_filter["limit"])] elif filter_name == "include": - return IncludeFilter(request_filter["include"]) + return [IncludeFilter(request_filter["include"])] elif filter_name == "distinct": - return DistinctFieldFilter(request_filter["distinct"]) + return [DistinctFieldFilter(request_filter["distinct"])] else: raise FilterError(f" Bad filter: {request_filter}") diff --git a/datagateway_api/common/helpers.py b/datagateway_api/common/helpers.py index a474a3c4..74ed1ac4 100644 --- a/datagateway_api/common/helpers.py +++ b/datagateway_api/common/helpers.py @@ -103,7 +103,7 @@ def get_filters_from_query_string(): filters = [] for arg in request.args: for value in request.args.getlist(arg): - filters.append( + filters.extend( QueryFilterFactory.get_query_filter({arg: json.loads(value)}), ) return filters diff --git a/test/db/test_query_filter_factory.py b/test/db/test_query_filter_factory.py index 7f81d98e..4171174b 100644 --- a/test/db/test_query_filter_factory.py +++ b/test/db/test_query_filter_factory.py @@ -13,13 +13,13 @@ ) +# TODO - Move outside of db/ class TestQueryFilterFactory: @pytest.mark.usefixtures("flask_test_app_db") def test_valid_distinct_filter(self): - assert isinstance( - QueryFilterFactory.get_query_filter({"distinct": "TEST"}), - DatabaseDistinctFieldFilter, - ) + test_filter = QueryFilterFactory.get_query_filter({"distinct": "TEST"}) + assert isinstance(test_filter[0], DatabaseDistinctFieldFilter) + assert len(test_filter) == 1 @pytest.mark.usefixtures("flask_test_app_db") @pytest.mark.parametrize( @@ -34,28 +34,27 @@ def test_valid_distinct_filter(self): ], ) def test_valid_include_filter(self, filter_input): - assert isinstance( - QueryFilterFactory.get_query_filter(filter_input), DatabaseIncludeFilter, - ) + test_filter = QueryFilterFactory.get_query_filter(filter_input) + assert isinstance(test_filter[0], DatabaseIncludeFilter) + assert len(test_filter) == 1 @pytest.mark.usefixtures("flask_test_app_db") def test_valid_limit_filter(self): - assert isinstance( - QueryFilterFactory.get_query_filter({"limit": 10}), DatabaseLimitFilter, - ) + test_filter = QueryFilterFactory.get_query_filter({"limit": 10}) + assert isinstance(test_filter[0], DatabaseLimitFilter) + assert len(test_filter) == 1 @pytest.mark.usefixtures("flask_test_app_db") def test_valid_order_filter(self): - assert isinstance( - QueryFilterFactory.get_query_filter({"order": "id DESC"}), - DatabaseOrderFilter, - ) + test_filter = QueryFilterFactory.get_query_filter({"order": "id DESC"}) + assert isinstance(test_filter[0], DatabaseOrderFilter) + assert len(test_filter) == 1 @pytest.mark.usefixtures("flask_test_app_db") def test_valid_skip_filter(self): - assert isinstance( - QueryFilterFactory.get_query_filter({"skip": 10}), DatabaseSkipFilter, - ) + test_filter = QueryFilterFactory.get_query_filter({"skip": 10}) + assert isinstance(test_filter[0], DatabaseSkipFilter) + assert len(test_filter) == 1 @pytest.mark.usefixtures("flask_test_app_db") @pytest.mark.parametrize( @@ -72,6 +71,6 @@ def test_valid_skip_filter(self): ], ) def test_valid_where_filter(self, filter_input): - assert isinstance( - QueryFilterFactory.get_query_filter(filter_input), DatabaseWhereFilter, - ) + test_filter = QueryFilterFactory.get_query_filter(filter_input) + assert isinstance(test_filter[0], DatabaseWhereFilter) + assert len(test_filter) == 1 From bbc9412ef0d8e858381da202a753512090abbcf0 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 4 Nov 2021 11:24:13 +0000 Subject: [PATCH 02/44] refactor: add in `SearchAPIQueryFilterFactory` #259 - Added in a generic `QueryFilterFactory` object which inherits from the search API and DataGateway API versions - Also fixed imports of the DataGateway API specific implementation --- .../common/base_query_filter_factory.py | 14 ++++++++ .../datagateway_api/query_filter_factory.py | 3 +- datagateway_api/common/helpers.py | 6 ++-- .../common/search_api/query_filter_factory.py | 35 +++++++++++++++++++ test/db/test_query_filter_factory.py | 20 ++++++----- test/test_query_filter.py | 4 +-- 6 files changed, 69 insertions(+), 13 deletions(-) create mode 100644 datagateway_api/common/base_query_filter_factory.py create mode 100644 datagateway_api/common/search_api/query_filter_factory.py diff --git a/datagateway_api/common/base_query_filter_factory.py b/datagateway_api/common/base_query_filter_factory.py new file mode 100644 index 00000000..89405f78 --- /dev/null +++ b/datagateway_api/common/base_query_filter_factory.py @@ -0,0 +1,14 @@ +from abc import abstractstaticmethod + + +class QueryFilterFactory(object): + @abstractstaticmethod + def get_query_filter(request_filter): + """ + Given a filter, return a matching Query filter object + + :param request_filter: The filter to create the QueryFilter for + :type request_filter: :class:`dict` + :return: The QueryFilter object created + """ + pass diff --git a/datagateway_api/common/datagateway_api/query_filter_factory.py b/datagateway_api/common/datagateway_api/query_filter_factory.py index 5d3830f8..2ad97b0e 100644 --- a/datagateway_api/common/datagateway_api/query_filter_factory.py +++ b/datagateway_api/common/datagateway_api/query_filter_factory.py @@ -1,5 +1,6 @@ import logging +from datagateway_api.common.base_query_filter_factory import QueryFilterFactory from datagateway_api.common.config import APIConfigOptions, config from datagateway_api.common.exceptions import ( ApiError, @@ -9,7 +10,7 @@ log = logging.getLogger() -class QueryFilterFactory(object): +class DataGatewayAPIQueryFilterFactory(QueryFilterFactory): @staticmethod def get_query_filter(request_filter): """ diff --git a/datagateway_api/common/helpers.py b/datagateway_api/common/helpers.py index 74ed1ac4..64a126b4 100644 --- a/datagateway_api/common/helpers.py +++ b/datagateway_api/common/helpers.py @@ -10,7 +10,7 @@ from datagateway_api.common.datagateway_api.database import models from datagateway_api.common.datagateway_api.query_filter_factory import ( - QueryFilterFactory, + DataGatewayAPIQueryFilterFactory, ) from datagateway_api.common.date_handler import DateHandler from datagateway_api.common.exceptions import ( @@ -104,7 +104,9 @@ def get_filters_from_query_string(): for arg in request.args: for value in request.args.getlist(arg): filters.extend( - QueryFilterFactory.get_query_filter({arg: json.loads(value)}), + DataGatewayAPIQueryFilterFactory.get_query_filter( + {arg: json.loads(value)} + ), ) return filters except Exception as e: diff --git a/datagateway_api/common/search_api/query_filter_factory.py b/datagateway_api/common/search_api/query_filter_factory.py new file mode 100644 index 00000000..2bb312c5 --- /dev/null +++ b/datagateway_api/common/search_api/query_filter_factory.py @@ -0,0 +1,35 @@ +import logging + +from datagateway_api.common.base_query_filter_factory import QueryFilterFactory +from datagateway_api.common.exceptions import FilterError + +log = logging.getLogger() + + +class SearchAPIQueryFilterFactory(QueryFilterFactory): + @staticmethod + def get_query_filter(request_filter): + query_param_name = list(request_filter)[0].lower() + query_filters = [] + + if query_param_name == "filter": + log.debug( + f"Filter: {request_filter['filter']}, Type: {type(request_filter['filter'])})" + ) + for filter_name, filter_input in request_filter["filter"].items(): + if filter_name == "where": + pass + elif filter_name == "include": + pass + elif filter_name == "limit": + pass + elif filter_name == "skip": + pass + else: + raise FilterError( + "No valid filter name given within filter query param" + ) + + return query_filters + else: + raise FilterError(f"Bad filter, please check input: {request_filter}") diff --git a/test/db/test_query_filter_factory.py b/test/db/test_query_filter_factory.py index 4171174b..fe0a6113 100644 --- a/test/db/test_query_filter_factory.py +++ b/test/db/test_query_filter_factory.py @@ -9,15 +9,17 @@ DatabaseWhereFilter, ) from datagateway_api.common.datagateway_api.query_filter_factory import ( - QueryFilterFactory, + DataGatewayAPIQueryFilterFactory, ) # TODO - Move outside of db/ -class TestQueryFilterFactory: +class TestDataGatewayAPIQueryFilterFactory: @pytest.mark.usefixtures("flask_test_app_db") def test_valid_distinct_filter(self): - test_filter = QueryFilterFactory.get_query_filter({"distinct": "TEST"}) + test_filter = DataGatewayAPIQueryFilterFactory.get_query_filter( + {"distinct": "TEST"} + ) assert isinstance(test_filter[0], DatabaseDistinctFieldFilter) assert len(test_filter) == 1 @@ -34,25 +36,27 @@ def test_valid_distinct_filter(self): ], ) def test_valid_include_filter(self, filter_input): - test_filter = QueryFilterFactory.get_query_filter(filter_input) + test_filter = DataGatewayAPIQueryFilterFactory.get_query_filter(filter_input) assert isinstance(test_filter[0], DatabaseIncludeFilter) assert len(test_filter) == 1 @pytest.mark.usefixtures("flask_test_app_db") def test_valid_limit_filter(self): - test_filter = QueryFilterFactory.get_query_filter({"limit": 10}) + test_filter = DataGatewayAPIQueryFilterFactory.get_query_filter({"limit": 10}) assert isinstance(test_filter[0], DatabaseLimitFilter) assert len(test_filter) == 1 @pytest.mark.usefixtures("flask_test_app_db") def test_valid_order_filter(self): - test_filter = QueryFilterFactory.get_query_filter({"order": "id DESC"}) + test_filter = DataGatewayAPIQueryFilterFactory.get_query_filter( + {"order": "id DESC"} + ) assert isinstance(test_filter[0], DatabaseOrderFilter) assert len(test_filter) == 1 @pytest.mark.usefixtures("flask_test_app_db") def test_valid_skip_filter(self): - test_filter = QueryFilterFactory.get_query_filter({"skip": 10}) + test_filter = DataGatewayAPIQueryFilterFactory.get_query_filter({"skip": 10}) assert isinstance(test_filter[0], DatabaseSkipFilter) assert len(test_filter) == 1 @@ -71,6 +75,6 @@ def test_valid_skip_filter(self): ], ) def test_valid_where_filter(self, filter_input): - test_filter = QueryFilterFactory.get_query_filter(filter_input) + test_filter = DataGatewayAPIQueryFilterFactory.get_query_filter(filter_input) assert isinstance(test_filter[0], DatabaseWhereFilter) assert len(test_filter) == 1 diff --git a/test/test_query_filter.py b/test/test_query_filter.py index 3304fa40..3ce98717 100644 --- a/test/test_query_filter.py +++ b/test/test_query_filter.py @@ -3,7 +3,7 @@ import pytest from datagateway_api.common.datagateway_api.query_filter_factory import ( - QueryFilterFactory, + DataGatewayAPIQueryFilterFactory, ) from datagateway_api.common.exceptions import ApiError from datagateway_api.common.filters import QueryFilter @@ -31,4 +31,4 @@ def test_invalid_query_filter_getter(self): return_value="invalid_backend", ): with pytest.raises(ApiError): - QueryFilterFactory.get_query_filter({"order": "id DESC"}) + DataGatewayAPIQueryFilterFactory.get_query_filter({"order": "id DESC"}) From 600ddf7189edfda936a8bf0f466ef80c6cff4539 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 4 Nov 2021 12:22:13 +0000 Subject: [PATCH 03/44] refactor: make fetching filters from request more generic #259 - The function will now work for DataGateway API and Search API variants --- datagateway_api/common/helpers.py | 23 +++++++++++++++---- .../entities/entity_endpoint.py | 6 ++--- .../table_endpoints/table_endpoints.py | 8 +++---- test/test_get_filters_from_query.py | 8 +++---- 4 files changed, 30 insertions(+), 15 deletions(-) diff --git a/datagateway_api/common/helpers.py b/datagateway_api/common/helpers.py index 64a126b4..4f7e9456 100644 --- a/datagateway_api/common/helpers.py +++ b/datagateway_api/common/helpers.py @@ -91,22 +91,37 @@ def is_valid_json(string): return True -def get_filters_from_query_string(): +def get_filters_from_query_string(api_type): """ Gets a list of filters from the query_strings arg,value pairs, and returns a list of QueryFilter Objects + :param api_type: Type of API this function is being used for i.e. DataGateway API or + Search API + :type api_type: :class:`str` + :raises ApiError: If `api_type` isn't a valid value :return: The list of filters """ + if api_type == "search_api": + from datagateway_api.common.search_api.query_filter_factory import ( + SearchAPIQueryFilterFactory as QueryFilterFactory, + ) + elif api_type == "datagateway_api": + from datagateway_api.common.datagateway_api.query_filter_factory import ( + DataGatewayAPIQueryFilterFactory as QueryFilterFactory, + ) + else: + raise ApiError( + "Incorrect api_type passed into `get_filter_from_query_string(): " + f"{api_type}", + ) log.info(" Getting filters from query string") try: filters = [] for arg in request.args: for value in request.args.getlist(arg): filters.extend( - DataGatewayAPIQueryFilterFactory.get_query_filter( - {arg: json.loads(value)} - ), + QueryFilterFactory.get_query_filter({arg: json.loads(value)}), ) return filters except Exception as e: diff --git a/datagateway_api/src/resources/datagateway_api/entities/entity_endpoint.py b/datagateway_api/src/resources/datagateway_api/entities/entity_endpoint.py index 5a278e4b..adaa2f76 100644 --- a/datagateway_api/src/resources/datagateway_api/entities/entity_endpoint.py +++ b/datagateway_api/src/resources/datagateway_api/entities/entity_endpoint.py @@ -30,7 +30,7 @@ def get(self): backend.get_with_filters( get_session_id_from_auth_header(), entity_type, - get_filters_from_query_string(), + get_filters_from_query_string("datagateway_api"), **kwargs, ), 200, @@ -321,7 +321,7 @@ def get_count_endpoint(name, entity_type, backend, **kwargs): class CountEndpoint(Resource): def get(self): - filters = get_filters_from_query_string() + filters = get_filters_from_query_string("datagateway_api") return ( backend.count_with_filters( get_session_id_from_auth_header(), entity_type, filters, **kwargs, @@ -380,7 +380,7 @@ def get_find_one_endpoint(name, entity_type, backend, **kwargs): class FindOneEndpoint(Resource): def get(self): - filters = get_filters_from_query_string() + filters = get_filters_from_query_string("datagateway_api") return ( backend.get_one_with_filters( get_session_id_from_auth_header(), entity_type, filters, **kwargs, diff --git a/datagateway_api/src/resources/datagateway_api/table_endpoints/table_endpoints.py b/datagateway_api/src/resources/datagateway_api/table_endpoints/table_endpoints.py index ee5819b3..bcbcc39e 100644 --- a/datagateway_api/src/resources/datagateway_api/table_endpoints/table_endpoints.py +++ b/datagateway_api/src/resources/datagateway_api/table_endpoints/table_endpoints.py @@ -65,7 +65,7 @@ def get(self, id_): backend.get_facility_cycles_for_instrument_with_filters( get_session_id_from_auth_header(), id_, - get_filters_from_query_string(), + get_filters_from_query_string("datagateway_api"), **kwargs, ), 200, @@ -126,7 +126,7 @@ def get(self, id_): backend.get_facility_cycles_for_instrument_count_with_filters( get_session_id_from_auth_header(), id_, - get_filters_from_query_string(), + get_filters_from_query_string("datagateway_api"), **kwargs, ), 200, @@ -202,7 +202,7 @@ def get(self, instrument_id, cycle_id): get_session_id_from_auth_header(), instrument_id, cycle_id, - get_filters_from_query_string(), + get_filters_from_query_string("datagateway_api"), **kwargs, ), 200, @@ -272,7 +272,7 @@ def get(self, instrument_id, cycle_id): get_session_id_from_auth_header(), instrument_id, cycle_id, - get_filters_from_query_string(), + get_filters_from_query_string("datagateway_api"), **kwargs, ), 200, diff --git a/test/test_get_filters_from_query.py b/test/test_get_filters_from_query.py index 0c3cb3a4..a3e31659 100644 --- a/test/test_get_filters_from_query.py +++ b/test/test_get_filters_from_query.py @@ -16,14 +16,14 @@ def test_valid_no_filters(self, flask_test_app_db): with flask_test_app_db: flask_test_app_db.get("/") - assert [] == get_filters_from_query_string() + assert [] == get_filters_from_query_string("datagateway_api") def test_invalid_filter(self, flask_test_app_db): with flask_test_app_db: flask_test_app_db.get('/?test="test"') with pytest.raises(FilterError): - get_filters_from_query_string() + get_filters_from_query_string("datagateway_api") @pytest.mark.parametrize( "filter_input, filter_type", @@ -42,13 +42,13 @@ def test_invalid_filter(self, flask_test_app_db): def test_valid_filter(self, flask_test_app_db, filter_input, filter_type): with flask_test_app_db: flask_test_app_db.get(f"/?{filter_input}") - filters = get_filters_from_query_string() + filters = get_filters_from_query_string("datagateway_api") assert isinstance(filters[0], filter_type) def test_valid_multiple_filters(self, flask_test_app_db): with flask_test_app_db: flask_test_app_db.get("/?limit=10&skip=4") - filters = get_filters_from_query_string() + filters = get_filters_from_query_string("datagateway_api") assert len(filters) == 2 From 12c680fdcea546e594e463a26205a743399d0623 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Fri, 19 Nov 2021 09:30:57 +0000 Subject: [PATCH 04/44] add boolean operator to WHERE filter #259 --- datagateway_api/common/search_api/filters.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/datagateway_api/common/search_api/filters.py b/datagateway_api/common/search_api/filters.py index b3ca847d..7172f07b 100644 --- a/datagateway_api/common/search_api/filters.py +++ b/datagateway_api/common/search_api/filters.py @@ -10,8 +10,9 @@ class SearchAPIWhereFilter(PythonICATWhereFilter): - def __init__(self, field, value, operation): + def __init__(self, field, value, operation, boolean_operator="and"): super().__init__(field, value, operation) + self.boolean_operator = boolean_operator def apply_filter(self, query): return super().apply_filter(query) From 22e8a738385cb77f5cfa2cfff616590cfa571638 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Fri, 19 Nov 2021 09:31:49 +0000 Subject: [PATCH 05/44] add fetch filters to search API endpoints #259 --- .../src/resources/search_api/search_api_endpoints.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/datagateway_api/src/resources/search_api/search_api_endpoints.py b/datagateway_api/src/resources/search_api/search_api_endpoints.py index 07c2aa85..79d28c35 100644 --- a/datagateway_api/src/resources/search_api/search_api_endpoints.py +++ b/datagateway_api/src/resources/search_api/search_api_endpoints.py @@ -1,5 +1,7 @@ from flask_restful import Resource +from datagateway_api.common.helpers import get_filters_from_query_string + # TODO - Might need kwargs on get_search_endpoint(), get_single_endpoint(), # get_number_count_endpoint(), get_files_endpoint(), get_number_count_files_endpoint() @@ -11,6 +13,7 @@ def get_search_endpoint(name): class Endpoint(Resource): def get(self): + filters = get_filters_from_query_string("search_api") """ TODO - Need to return similar to return ( @@ -38,6 +41,7 @@ def get_single_endpoint(name): class EndpointWithID(Resource): def get(self, pid): + filters = get_filters_from_query_string("search_api") # TODO - Add return pass @@ -54,6 +58,8 @@ def get_number_count_endpoint(name): class CountEndpoint(Resource): def get(self): + # Only WHERE included on count endpoints + filters = get_filters_from_query_string("search_api") # TODO - Add return pass @@ -70,6 +76,7 @@ def get_files_endpoint(name): class FilesEndpoint(Resource): def get(self, pid): + filters = get_filters_from_query_string("search_api") # TODO - Add return pass @@ -86,6 +93,8 @@ def get_number_count_files_endpoint(name): class CountFilesEndpoint(Resource): def get(self, pid): + # Only WHERE included on count endpoints + filters = get_filters_from_query_string("search_api") # TODO - Add return pass From ee668e38cd43354851163616a93924ad84e14b90 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Fri, 19 Nov 2021 11:32:12 +0000 Subject: [PATCH 06/44] feat: add first pass of query param implementation #259 --- .../common/search_api/query_filter_factory.py | 117 ++++++++++++++++-- 1 file changed, 110 insertions(+), 7 deletions(-) diff --git a/datagateway_api/common/search_api/query_filter_factory.py b/datagateway_api/common/search_api/query_filter_factory.py index 2bb312c5..4926a3f2 100644 --- a/datagateway_api/common/search_api/query_filter_factory.py +++ b/datagateway_api/common/search_api/query_filter_factory.py @@ -2,6 +2,12 @@ from datagateway_api.common.base_query_filter_factory import QueryFilterFactory from datagateway_api.common.exceptions import FilterError +from datagateway_api.common.search_api.filters import ( + SearchAPIIncludeFilter, + SearchAPILimitFilter, + SearchAPISkipFilter, + SearchAPIWhereFilter, +) log = logging.getLogger() @@ -14,22 +20,119 @@ def get_query_filter(request_filter): if query_param_name == "filter": log.debug( - f"Filter: {request_filter['filter']}, Type: {type(request_filter['filter'])})" + f"Filter: {request_filter['filter']}, Type: {type(request_filter['filter'])})", ) for filter_name, filter_input in request_filter["filter"].items(): + log.debug(f"Name: {filter_name}, Type: {type(filter_name)}") + log.debug(f"Input: {filter_input}, Type: {type(filter_input)}") + # {"where": {"property": "value"}} + # {"where": {"property": {"operator": "value"}}} + # {"where": {"text": "value"}} + # {"where": {"and": [{"property": "value"}, {"property": "value"}]}} + # {"where": {"or": [{"property": "value"}, {"property": "value"}]}} if filter_name == "where": - pass + if list(filter_input.keys())[0] == "text": + # Multiple WHERE filters ORing - e.g. title or summary + pass + elif ( + list(filter_input.keys())[0] == "and" + or list(filter_input.keys())[0] == "or" + ): + boolean_operator = list(filter_input.keys())[0] + conditions = list(filter_input.values())[0] + for condition in conditions: + # Could be nested AND/OR + filter_data = SearchAPIQueryFilterFactory.get_condition_values( + condition, + ) + for condition in filter_data: + query_filters.append( + SearchAPIWhereFilter( + condition[0], condition[1], condition[2], + ), + ) + else: + filter_data = SearchAPIQueryFilterFactory.get_condition_values( + filter_input, + ) + for condition in filter_data: + + query_filters.append( + SearchAPIWhereFilter( + condition[0], condition[1], condition[2], + ), + ) + elif filter_name == "include": - pass + # {"include": [{"relation": "relatedModel"}]} + # + # {"include": [{"relation": "relatedModel1"}, + # {"relation": "relatedModel2"}]} + # + # {"include": [{"relation": "relatedModel", + # "scope": {"where": {"property": "value"}}}]} + + for related_model in filter_input: + included_entity = related_model["relation"] + query_filters.append(SearchAPIIncludeFilter(included_entity)) + + if "scope" in related_model: + try: + field_name = list( + related_model["scope"]["where"].keys(), + )[0] + full_field_path = f"{included_entity}.{field_name}" + related_model["scope"]["where"][ + full_field_path + ] = related_model["scope"]["where"].pop("name") + + query_filters.extend( + SearchAPIQueryFilterFactory.get_query_filter( + {"filter": related_model["scope"]}, + ), + ) + except KeyError: + raise FilterError("Error in scope for include filter") elif filter_name == "limit": - pass + query_filters.append(SearchAPILimitFilter(filter_input)) elif filter_name == "skip": - pass + query_filters.append(SearchAPISkipFilter(filter_input)) else: raise FilterError( - "No valid filter name given within filter query param" + "No valid filter name given within filter query param", ) return query_filters + elif query_param_name == "where": + # For the count endpoints + """ + query_filters.extend( + SearchAPIQueryFilterFactory.get_query_filter( + {"filter": related_model["scope"]}, + ), + ), + """ else: - raise FilterError(f"Bad filter, please check input: {request_filter}") + raise FilterError( + f"Bad filter, please check query parameters: {request_filter}", + ) + + @staticmethod + def get_condition_values(filter_input): + where_filter_data = [] + field = list(filter_input.keys())[0] + filter_data = list(filter_input.values())[0] + + if isinstance(filter_data, str): + # {"where": {"property": "value"}} + value = filter_input[field] + operation = "eq" + elif isinstance(filter_data, dict): + # {"where": {"property": {"operator": "value"}}} + print(f"filter data: {filter_data}") + value = list(filter_input[field].values())[0] + operation = list(filter_input[field].keys())[0] + + where_filter_data.append((field, value, operation)) + + return where_filter_data From 9a5e02bbeb5b20d7b16b45e33bc6244cba1543d9 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Fri, 19 Nov 2021 11:32:41 +0000 Subject: [PATCH 07/44] test: add some tests for search API query params #259 --- .../test_search_api_query_filter_factory.py | 299 ++++++++++++++++++ 1 file changed, 299 insertions(+) create mode 100644 test/search_api/test_search_api_query_filter_factory.py diff --git a/test/search_api/test_search_api_query_filter_factory.py b/test/search_api/test_search_api_query_filter_factory.py new file mode 100644 index 00000000..c6b36ce8 --- /dev/null +++ b/test/search_api/test_search_api_query_filter_factory.py @@ -0,0 +1,299 @@ +import pytest + +from datagateway_api.common.exceptions import FilterError +from datagateway_api.common.search_api.filters import ( + SearchAPIIncludeFilter, + SearchAPILimitFilter, + SearchAPISkipFilter, + SearchAPIWhereFilter, +) +from datagateway_api.common.search_api.query_filter_factory import ( + SearchAPIQueryFilterFactory, +) + + +class TestSearchAPIQueryFilterFactory: + @pytest.mark.parametrize( + "test_request_filter, expected_length, expected_fields, expected_operations" + ", expected_values", + [ + pytest.param( + {"filter": {"where": {"title": "My Title"}}}, + 1, + ["title"], + ["eq"], + ["My Title"], + id="Property value, no specified operator", + ), + pytest.param( + {"filter": {"where": {"summary": {"like": "My Test Summary"}}}}, + 1, + ["summary"], + ["like"], + ["My Test Summary"], + id="Property value with operator", + ), + pytest.param( + {"filter": {"where": {"and": [{"summary": "My Test Summary"}]}}}, + 1, + ["summary"], + ["eq"], + ["My Test Summary"], + id="AND with single condition, no specified operator", + ), + pytest.param( + { + "filter": { + "where": { + "and": [ + {"summary": "My Test Summary"}, + {"title": "Test title"}, + ], + }, + }, + }, + 2, + ["summary", "title"], + ["eq", "eq"], + ["My Test Summary", "Test title"], + id="AND with multiple conditions, no specified operator", + ), + pytest.param( + {"filter": {"where": {"and": [{"value": {"lt": 50}}]}}}, + 1, + ["value"], + ["lt"], + [50], + id="AND, single condition with operator", + ), + pytest.param( + { + "filter": { + "where": { + "and": [ + {"name": {"like": "Test name"}}, + {"value": {"gte": 275}}, + ], + }, + }, + }, + 1, + ["name", "value"], + ["like", "gte"], + ["Test name", 275], + id="AND, multiple conditions with operator", + ), + pytest.param( + { + "filter": { + "where": { + "and": [ + { + "and": [ + {"name": {"like": "Test name"}}, + {"value": {"gte": 275}}, + ], + }, + ], + }, + }, + }, + 1, + ["name", "value"], + ["like", "gte"], + ["Test name", 275], + id="Nested AND, multiple conditions with operator", + ), + ], + ) + def test_valid_where_filter( + self, + test_request_filter, + expected_length, + expected_fields, + expected_operations, + expected_values, + ): + filters = SearchAPIQueryFilterFactory.get_query_filter(test_request_filter) + + assert len(filters) == expected_length + for test_filter, field, operation, value in zip( + filters, expected_fields, expected_operations, expected_values, + ): + assert isinstance(test_filter, SearchAPIWhereFilter) + assert test_filter.field == field + assert test_filter.operation == operation + assert test_filter.value == value + + @pytest.mark.parametrize( + "test_request_filter, expected_length, expected_included_entities, expected_where_filter_data", + [ + pytest.param( + {"filter": {"include": [{"relation": "files"}]}}, + 1, + [["files"]], + [[]], + id="Single related model", + ), + pytest.param( + { + "filter": { + "include": [{"relation": "files"}, {"relation": "instrument"}], + }, + }, + 2, + [["files"], ["instrument"]], + [[], []], + id="Multiple related models", + ), + pytest.param( + { + "filter": { + "include": [ + { + "relation": "parameters", + "scope": {"where": {"name": "My parameter"}}, + }, + ], + }, + }, + 2, + [["parameters"], []], + [[], ["parameters.name", "eq", "My parameter"]], + id="Related model with scope", + ), + pytest.param( + { + "filter": { + "include": [ + { + "relation": "parameters", + "scope": {"where": {"name": {"ne": "My parameter"}}}, + }, + ], + }, + }, + 2, + [["parameters"], []], + [[], ["parameters.name", "ne", "My parameter"]], + id="Related model with scope (with operator)", + ), + pytest.param( + { + "filter": { + "include": [ + { + "relation": "parameters", + "scope": {"where": {"text": "My parameter"}}, + }, + ], + }, + }, + 2, + [["parameters"], []], + [[], ["parameters.name", "ne", "My parameter"]], + id="Related model with scope (text operator)", + ), + pytest.param( + { + "filter": { + "include": [ + { + "relation": "documents", + "scope": { + "where": { + "and": [ + {"summary": "My Test Summary"}, + {"title": "Test title"}, + ], + }, + }, + }, + ], + }, + }, + 3, + [["documents"], [], []], + [ + [], + ["documents.summary", "eq", "My Test Summary"], + ["documents.title", "eq", "Test title"], + ], + id="Related model with scope (boolean operator)", + ), + ], + ) + def test_valid_include_filter( + self, + test_request_filter, + expected_length, + expected_included_entities, + expected_where_filter_data, + ): + filters = SearchAPIQueryFilterFactory.get_query_filter(test_request_filter) + + assert len(filters) == expected_length + + for test_filter, included_entities, where_filter_data in zip( + filters, expected_included_entities, expected_where_filter_data, + ): + if isinstance(test_filter, SearchAPIIncludeFilter): + assert test_filter.included_filters == included_entities + if isinstance(test_filter, SearchAPIWhereFilter): + assert test_filter.field == where_filter_data[0] + assert test_filter.operation == where_filter_data[1] + assert test_filter.value == where_filter_data[2] + # TODO - Assert for boolean_operator + + @pytest.mark.parametrize( + "test_request_filter, expected_limit_value", + [ + pytest.param({"filter": {"limit": 0}}, 0, id="Limit 0 values"), + pytest.param({"filter": {"limit": 50}}, 50, id="Limit 50 values"), + ], + ) + def test_valid_limit_filter(self, test_request_filter, expected_limit_value): + filters = SearchAPIQueryFilterFactory.get_query_filter(test_request_filter) + + assert len(filters) == 1 + assert isinstance(filters[0], SearchAPILimitFilter) + assert filters[0].limit_value == expected_limit_value + + @pytest.mark.parametrize( + "test_request_filter, expected_skip_value", + [ + pytest.param({"filter": {"skip": 0}}, 0, id="Skip 0 values"), + pytest.param({"filter": {"skip": 50}}, 50, id="Skip 50 values"), + ], + ) + def test_valid_skip_filter( + self, test_request_filter, expected_skip_value, + ): + filters = SearchAPIQueryFilterFactory.get_query_filter(test_request_filter) + + assert len(filters) == 1 + assert isinstance(filters[0], SearchAPISkipFilter) + assert filters[0].skip_value == expected_skip_value + + @pytest.mark.parametrize( + "test_request_filter", + [ + pytest.param("invalid query filter input", id="Generally invalid input"), + pytest.param( + { + "filter": { + "include": [ + { + "relation": "parameters", + "scope": {"text": "My parameter"}, + }, + ], + }, + }, + id="Invalid scope syntax on include filter", + ), + ], + ) + def test_invalid_filter_input(self, test_request_filter): + with pytest.raises(FilterError): + SearchAPIQueryFilterFactory.get_query_filter(test_request_filter) From 7b3d039570c9b748cab9c2893078b712f4b7ada1 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Fri, 19 Nov 2021 13:53:48 +0000 Subject: [PATCH 08/44] test: add boolean operator assertion #259 --- .../test_search_api_query_filter_factory.py | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/test/search_api/test_search_api_query_filter_factory.py b/test/search_api/test_search_api_query_filter_factory.py index c6b36ce8..107caf6a 100644 --- a/test/search_api/test_search_api_query_filter_factory.py +++ b/test/search_api/test_search_api_query_filter_factory.py @@ -15,7 +15,7 @@ class TestSearchAPIQueryFilterFactory: @pytest.mark.parametrize( "test_request_filter, expected_length, expected_fields, expected_operations" - ", expected_values", + ", expected_values, expected_boolean_operators", [ pytest.param( {"filter": {"where": {"title": "My Title"}}}, @@ -23,6 +23,7 @@ class TestSearchAPIQueryFilterFactory: ["title"], ["eq"], ["My Title"], + ["and"], id="Property value, no specified operator", ), pytest.param( @@ -31,6 +32,7 @@ class TestSearchAPIQueryFilterFactory: ["summary"], ["like"], ["My Test Summary"], + ["and"], id="Property value with operator", ), pytest.param( @@ -39,6 +41,7 @@ class TestSearchAPIQueryFilterFactory: ["summary"], ["eq"], ["My Test Summary"], + ["and"], id="AND with single condition, no specified operator", ), pytest.param( @@ -56,6 +59,7 @@ class TestSearchAPIQueryFilterFactory: ["summary", "title"], ["eq", "eq"], ["My Test Summary", "Test title"], + ["and", "and"], id="AND with multiple conditions, no specified operator", ), pytest.param( @@ -64,6 +68,7 @@ class TestSearchAPIQueryFilterFactory: ["value"], ["lt"], [50], + ["and"], id="AND, single condition with operator", ), pytest.param( @@ -77,10 +82,11 @@ class TestSearchAPIQueryFilterFactory: }, }, }, - 1, + 2, ["name", "value"], ["like", "gte"], ["Test name", 275], + ["and", "and"], id="AND, multiple conditions with operator", ), pytest.param( @@ -98,10 +104,11 @@ class TestSearchAPIQueryFilterFactory: }, }, }, - 1, + 2, ["name", "value"], ["like", "gte"], ["Test name", 275], + ["and", "and"], id="Nested AND, multiple conditions with operator", ), ], @@ -113,17 +120,23 @@ def test_valid_where_filter( expected_fields, expected_operations, expected_values, + expected_boolean_operators, ): filters = SearchAPIQueryFilterFactory.get_query_filter(test_request_filter) assert len(filters) == expected_length - for test_filter, field, operation, value in zip( - filters, expected_fields, expected_operations, expected_values, + for test_filter, field, operation, value, boolean_operator in zip( + filters, + expected_fields, + expected_operations, + expected_values, + expected_boolean_operators, ): assert isinstance(test_filter, SearchAPIWhereFilter) assert test_filter.field == field assert test_filter.operation == operation assert test_filter.value == value + assert test_filter.boolean_operator == boolean_operator @pytest.mark.parametrize( "test_request_filter, expected_length, expected_included_entities, expected_where_filter_data", From 6882b9fa4c571ec7d6d20f7183f7c50e08879077 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Fri, 19 Nov 2021 15:22:14 +0000 Subject: [PATCH 09/44] add boolean operator to WHERE filter creation #259 --- datagateway_api/src/search_api/query_filter_factory.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/datagateway_api/src/search_api/query_filter_factory.py b/datagateway_api/src/search_api/query_filter_factory.py index 11fab59e..f0a690d8 100644 --- a/datagateway_api/src/search_api/query_filter_factory.py +++ b/datagateway_api/src/search_api/query_filter_factory.py @@ -48,7 +48,10 @@ def get_query_filter(request_filter): for condition in filter_data: query_filters.append( SearchAPIWhereFilter( - condition[0], condition[1], condition[2], + field=condition[0], + value=condition[1], + operation=condition[2], + boolean_operator=boolean_operator, ), ) else: From f56465c18443afbd8914d6f41cc3243ba03835de Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Fri, 19 Nov 2021 15:23:02 +0000 Subject: [PATCH 10/44] test: add test for OR operator on WHERE filter #259 --- .../test_search_api_query_filter_factory.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/search_api/test_search_api_query_filter_factory.py b/test/search_api/test_search_api_query_filter_factory.py index fada9f60..8c9f3738 100644 --- a/test/search_api/test_search_api_query_filter_factory.py +++ b/test/search_api/test_search_api_query_filter_factory.py @@ -111,6 +111,24 @@ class TestSearchAPIQueryFilterFactory: ["and", "and"], id="Nested AND, multiple conditions with operator", ), + pytest.param( + { + "filter": { + "where": { + "or": [ + {"name": {"like": "Test name"}}, + {"value": {"gte": 275}}, + ], + }, + }, + }, + 2, + ["name", "value"], + ["like", "gte"], + ["Test name", 275], + ["or", "or"], + id="OR, multiple conditions with operator", + ), ], ) def test_valid_where_filter( From f9d1e577848b0302bee570f65e0ed1eb00adbad0 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Fri, 19 Nov 2021 15:38:28 +0000 Subject: [PATCH 11/44] add support for WHERE filters coming from count endpoints #259 --- datagateway_api/src/search_api/query_filter_factory.py | 10 ++++------ .../search_api/test_search_api_query_filter_factory.py | 9 +++++++++ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/datagateway_api/src/search_api/query_filter_factory.py b/datagateway_api/src/search_api/query_filter_factory.py index f0a690d8..5bec920d 100644 --- a/datagateway_api/src/search_api/query_filter_factory.py +++ b/datagateway_api/src/search_api/query_filter_factory.py @@ -104,22 +104,20 @@ def get_query_filter(request_filter): raise FilterError( "No valid filter name given within filter query param", ) - - return query_filters elif query_param_name == "where": # For the count endpoints - """ query_filters.extend( SearchAPIQueryFilterFactory.get_query_filter( - {"filter": related_model["scope"]}, + {"filter": request_filter}, ), - ), - """ + ) else: raise FilterError( f"Bad filter, please check query parameters: {request_filter}", ) + return query_filters + @staticmethod def get_condition_values(filter_input): where_filter_data = [] diff --git a/test/search_api/test_search_api_query_filter_factory.py b/test/search_api/test_search_api_query_filter_factory.py index 8c9f3738..087a462c 100644 --- a/test/search_api/test_search_api_query_filter_factory.py +++ b/test/search_api/test_search_api_query_filter_factory.py @@ -129,6 +129,15 @@ class TestSearchAPIQueryFilterFactory: ["or", "or"], id="OR, multiple conditions with operator", ), + pytest.param( + {"where": {"summary": {"like": "My Test Summary"}}}, + 1, + ["summary"], + ["like"], + ["My Test Summary"], + ["and"], + id="WHERE filter in syntax for count endpoints", + ), ], ) def test_valid_where_filter( From 19ff8418d04104abcf92b9ba443b4449fd6c076c Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Fri, 19 Nov 2021 18:33:07 +0000 Subject: [PATCH 12/44] add support for text operator in query params #259 --- .../src/search_api/query_filter_factory.py | 60 ++++++++++++------- .../test_search_api_query_filter_factory.py | 31 ++++++++-- 2 files changed, 64 insertions(+), 27 deletions(-) diff --git a/datagateway_api/src/search_api/query_filter_factory.py b/datagateway_api/src/search_api/query_filter_factory.py index 5bec920d..574457ea 100644 --- a/datagateway_api/src/search_api/query_filter_factory.py +++ b/datagateway_api/src/search_api/query_filter_factory.py @@ -19,22 +19,15 @@ def get_query_filter(request_filter): query_filters = [] if query_param_name == "filter": - log.debug( - f"Filter: {request_filter['filter']}, Type: {type(request_filter['filter'])})", - ) + log.debug("Filter: %s", request_filter["filter"]) for filter_name, filter_input in request_filter["filter"].items(): - log.debug(f"Name: {filter_name}, Type: {type(filter_name)}") - log.debug(f"Input: {filter_input}, Type: {type(filter_input)}") # {"where": {"property": "value"}} # {"where": {"property": {"operator": "value"}}} # {"where": {"text": "value"}} # {"where": {"and": [{"property": "value"}, {"property": "value"}]}} # {"where": {"or": [{"property": "value"}, {"property": "value"}]}} if filter_name == "where": - if list(filter_input.keys())[0] == "text": - # Multiple WHERE filters ORing - e.g. title or summary - pass - elif ( + if ( list(filter_input.keys())[0] == "and" or list(filter_input.keys())[0] == "or" ): @@ -42,7 +35,7 @@ def get_query_filter(request_filter): conditions = list(filter_input.values())[0] for condition in conditions: # Could be nested AND/OR - filter_data = SearchAPIQueryFilterFactory.get_condition_values( + filter_data = SearchAPIQueryFilterFactory.get_condition_values( # noqa: B950 condition, ) for condition in filter_data: @@ -59,10 +52,11 @@ def get_query_filter(request_filter): filter_input, ) for condition in filter_data: - query_filters.append( SearchAPIWhereFilter( - condition[0], condition[1], condition[2], + field=condition[0], + value=condition[1], + operation=condition[2], ), ) @@ -81,19 +75,41 @@ def get_query_filter(request_filter): if "scope" in related_model: try: - field_name = list( + where_key = list( related_model["scope"]["where"].keys(), )[0] - full_field_path = f"{included_entity}.{field_name}" - related_model["scope"]["where"][ - full_field_path - ] = related_model["scope"]["where"].pop("name") - query_filters.extend( - SearchAPIQueryFilterFactory.get_query_filter( - {"filter": related_model["scope"]}, - ), - ) + if where_key == "text": + # TODO - we might want to move this to the data + # definitions at a later point + text_operator_fields = { + "datasets": ["title"], + "documents": ["title", "summary"], + "files": ["name"], + "instrument": ["name", "facility"], + "samples": ["name", "description"], + "techniques": ["name"], + } + field_names = text_operator_fields[included_entity] + else: + field_names = [where_key] + + for field_name in field_names: + full_field_path = f"{included_entity}.{field_name}" + where_filter = { + "filter": { + "where": { + full_field_path: related_model["scope"][ + "where" + ][where_key], + }, + }, + } + query_filters.extend( + SearchAPIQueryFilterFactory.get_query_filter( + where_filter, + ), + ) except KeyError: raise FilterError("Error in scope for include filter") elif filter_name == "limit": diff --git a/test/search_api/test_search_api_query_filter_factory.py b/test/search_api/test_search_api_query_filter_factory.py index 087a462c..c7d029fd 100644 --- a/test/search_api/test_search_api_query_filter_factory.py +++ b/test/search_api/test_search_api_query_filter_factory.py @@ -166,7 +166,8 @@ def test_valid_where_filter( assert test_filter.boolean_operator == boolean_operator @pytest.mark.parametrize( - "test_request_filter, expected_length, expected_included_entities, expected_where_filter_data", + "test_request_filter, expected_length, expected_included_entities" + ", expected_where_filter_data", [ pytest.param( {"filter": {"include": [{"relation": "files"}]}}, @@ -223,17 +224,37 @@ def test_valid_where_filter( "filter": { "include": [ { - "relation": "parameters", - "scope": {"where": {"text": "My parameter"}}, + "relation": "files", + "scope": {"where": {"text": "file1"}}, }, ], }, }, 2, - [["parameters"], []], - [[], ["parameters.name", "ne", "My parameter"]], + [["files"], []], + [[], ["files.name", "eq", "file1"]], id="Related model with scope (text operator)", ), + pytest.param( + { + "filter": { + "include": [ + { + "relation": "documents", + "scope": {"where": {"text": "document1"}}, + }, + ], + }, + }, + 3, + [["documents"], []], + [ + [], + ["documents.title", "eq", "document1"], + ["documents.summary", "eq", "document1"], + ], + id="Related model with scope (text operator, multiple fields)", + ), pytest.param( { "filter": { From d2112786cb96c731557d2a9eea585afea84b3d77 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Mon, 22 Nov 2021 12:04:20 +0000 Subject: [PATCH 13/44] test: add remaining test cases for query parameter inputs #259 --- .../test_search_api_query_filter_factory.py | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/test/search_api/test_search_api_query_filter_factory.py b/test/search_api/test_search_api_query_filter_factory.py index c7d029fd..c6e98ca8 100644 --- a/test/search_api/test_search_api_query_filter_factory.py +++ b/test/search_api/test_search_api_query_filter_factory.py @@ -35,6 +35,24 @@ class TestSearchAPIQueryFilterFactory: ["and"], id="Property value with operator", ), + pytest.param( + {"filter": {"where": {"text": "Dataset 1"}}}, + 1, + ["title"], + ["eq"], + ["Dataset 1"], + ["and"], + id="Text operator on dataset", + ), + pytest.param( + {"filter": {"where": {"text": "Instrument 1"}}}, + 2, + ["name", "facility"], + ["eq", "eq"], + ["Instrument 1", "Instrument 1"], + ["and", "and"], + id="Text operator on instrument", + ), pytest.param( {"filter": {"where": {"and": [{"summary": "My Test Summary"}]}}}, 1, @@ -203,6 +221,31 @@ def test_valid_where_filter( [[], ["parameters.name", "eq", "My parameter"]], id="Related model with scope", ), + pytest.param( + { + "filter": { + "include": [ + { + "relation": "parameters", + "scope": {"where": {"name": "My parameter"}}, + }, + { + "relation": "documents", + "scope": {"where": {"title": "Document title"}}, + }, + ], + }, + }, + 4, + [["parameters"], [], ["documents"], []], + [ + [], + ["parameters.name", "eq", "My parameter"], + [], + ["documents.title", "eq", "Document title"], + ], + id="Multiple related models with scopes", + ), pytest.param( { "filter": { From 94af185d4f8cd1bc270e99354f2eb4c3105b29ad Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Mon, 22 Nov 2021 12:24:37 +0000 Subject: [PATCH 14/44] style: clean up comments #259 --- .../src/search_api/query_filter_factory.py | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/datagateway_api/src/search_api/query_filter_factory.py b/datagateway_api/src/search_api/query_filter_factory.py index 574457ea..7295a8c8 100644 --- a/datagateway_api/src/search_api/query_filter_factory.py +++ b/datagateway_api/src/search_api/query_filter_factory.py @@ -21,11 +21,6 @@ def get_query_filter(request_filter): if query_param_name == "filter": log.debug("Filter: %s", request_filter["filter"]) for filter_name, filter_input in request_filter["filter"].items(): - # {"where": {"property": "value"}} - # {"where": {"property": {"operator": "value"}}} - # {"where": {"text": "value"}} - # {"where": {"and": [{"property": "value"}, {"property": "value"}]}} - # {"where": {"or": [{"property": "value"}, {"property": "value"}]}} if filter_name == "where": if ( list(filter_input.keys())[0] == "and" @@ -61,14 +56,6 @@ def get_query_filter(request_filter): ) elif filter_name == "include": - # {"include": [{"relation": "relatedModel"}]} - # - # {"include": [{"relation": "relatedModel1"}, - # {"relation": "relatedModel2"}]} - # - # {"include": [{"relation": "relatedModel", - # "scope": {"where": {"property": "value"}}}]} - for related_model in filter_input: included_entity = related_model["relation"] query_filters.append(SearchAPIIncludeFilter(included_entity)) @@ -141,12 +128,11 @@ def get_condition_values(filter_input): filter_data = list(filter_input.values())[0] if isinstance(filter_data, str): - # {"where": {"property": "value"}} + # Format: {"where": {"property": "value"}} value = filter_input[field] operation = "eq" elif isinstance(filter_data, dict): - # {"where": {"property": {"operator": "value"}}} - print(f"filter data: {filter_data}") + # Format: {"where": {"property": {"operator": "value"}}} value = list(filter_input[field].values())[0] operation = list(filter_input[field].keys())[0] From f7d98297c3b100dedffe0e68f3a3c40d65086042 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Mon, 22 Nov 2021 12:38:59 +0000 Subject: [PATCH 15/44] style: add logging to satisfy linter #259 --- datagateway_api/src/resources/search_api_endpoints.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/datagateway_api/src/resources/search_api_endpoints.py b/datagateway_api/src/resources/search_api_endpoints.py index ec0d9c69..0290b451 100644 --- a/datagateway_api/src/resources/search_api_endpoints.py +++ b/datagateway_api/src/resources/search_api_endpoints.py @@ -1,7 +1,11 @@ +import logging + from flask_restful import Resource from datagateway_api.src.common.helpers import get_filters_from_query_string +log = logging.getLogger() + # TODO - Might need kwargs on get_search_endpoint(), get_single_endpoint(), # get_number_count_endpoint(), get_files_endpoint(), get_number_count_files_endpoint() @@ -14,6 +18,7 @@ def get_search_endpoint(name): class Endpoint(Resource): def get(self): filters = get_filters_from_query_string("search_api") + log.debug("Filters: %s", filters) """ TODO - Need to return similar to return ( @@ -42,6 +47,7 @@ def get_single_endpoint(name): class EndpointWithID(Resource): def get(self, pid): filters = get_filters_from_query_string("search_api") + log.debug("Filters: %s", filters) # TODO - Add return pass @@ -60,6 +66,7 @@ class CountEndpoint(Resource): def get(self): # Only WHERE included on count endpoints filters = get_filters_from_query_string("search_api") + log.debug("Filters: %s", filters) # TODO - Add return pass @@ -77,6 +84,7 @@ def get_files_endpoint(name): class FilesEndpoint(Resource): def get(self, pid): filters = get_filters_from_query_string("search_api") + log.debug("Filters: %s", filters) # TODO - Add return pass @@ -95,6 +103,7 @@ class CountFilesEndpoint(Resource): def get(self, pid): # Only WHERE included on count endpoints filters = get_filters_from_query_string("search_api") + log.debug("Filters: %s", filters) # TODO - Add return pass From c09c982977dcb70a8ed8a82a21be607b47cca19c Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Mon, 22 Nov 2021 12:39:34 +0000 Subject: [PATCH 16/44] style: make `get_query_filter()` less complex #259 - I've moved most of the functionality into separate functions, so flake8 doesn't complain that `get_query_filter()` is too complex --- .../src/search_api/query_filter_factory.py | 162 ++++++++++-------- 1 file changed, 86 insertions(+), 76 deletions(-) diff --git a/datagateway_api/src/search_api/query_filter_factory.py b/datagateway_api/src/search_api/query_filter_factory.py index 7295a8c8..80337184 100644 --- a/datagateway_api/src/search_api/query_filter_factory.py +++ b/datagateway_api/src/search_api/query_filter_factory.py @@ -22,83 +22,13 @@ def get_query_filter(request_filter): log.debug("Filter: %s", request_filter["filter"]) for filter_name, filter_input in request_filter["filter"].items(): if filter_name == "where": - if ( - list(filter_input.keys())[0] == "and" - or list(filter_input.keys())[0] == "or" - ): - boolean_operator = list(filter_input.keys())[0] - conditions = list(filter_input.values())[0] - for condition in conditions: - # Could be nested AND/OR - filter_data = SearchAPIQueryFilterFactory.get_condition_values( # noqa: B950 - condition, - ) - for condition in filter_data: - query_filters.append( - SearchAPIWhereFilter( - field=condition[0], - value=condition[1], - operation=condition[2], - boolean_operator=boolean_operator, - ), - ) - else: - filter_data = SearchAPIQueryFilterFactory.get_condition_values( - filter_input, - ) - for condition in filter_data: - query_filters.append( - SearchAPIWhereFilter( - field=condition[0], - value=condition[1], - operation=condition[2], - ), - ) - + query_filters.extend( + SearchAPIQueryFilterFactory.get_where_filter(filter_input), + ) elif filter_name == "include": - for related_model in filter_input: - included_entity = related_model["relation"] - query_filters.append(SearchAPIIncludeFilter(included_entity)) - - if "scope" in related_model: - try: - where_key = list( - related_model["scope"]["where"].keys(), - )[0] - - if where_key == "text": - # TODO - we might want to move this to the data - # definitions at a later point - text_operator_fields = { - "datasets": ["title"], - "documents": ["title", "summary"], - "files": ["name"], - "instrument": ["name", "facility"], - "samples": ["name", "description"], - "techniques": ["name"], - } - field_names = text_operator_fields[included_entity] - else: - field_names = [where_key] - - for field_name in field_names: - full_field_path = f"{included_entity}.{field_name}" - where_filter = { - "filter": { - "where": { - full_field_path: related_model["scope"][ - "where" - ][where_key], - }, - }, - } - query_filters.extend( - SearchAPIQueryFilterFactory.get_query_filter( - where_filter, - ), - ) - except KeyError: - raise FilterError("Error in scope for include filter") + query_filters.extend( + SearchAPIQueryFilterFactory.get_include_filter(filter_input), + ) elif filter_name == "limit": query_filters.append(SearchAPILimitFilter(filter_input)) elif filter_name == "skip": @@ -121,6 +51,86 @@ def get_query_filter(request_filter): return query_filters + @staticmethod + def get_where_filter(filter_input): + where_filters = [] + if ( + list(filter_input.keys())[0] == "and" + or list(filter_input.keys())[0] == "or" + ): + boolean_operator = list(filter_input.keys())[0] + conditions = list(filter_input.values())[0] + for condition in conditions: + # Could be nested AND/OR + filter_data = SearchAPIQueryFilterFactory.get_condition_values( + condition, + ) + for condition in filter_data: + where_filters.append( + SearchAPIWhereFilter( + field=condition[0], + value=condition[1], + operation=condition[2], + boolean_operator=boolean_operator, + ), + ) + else: + filter_data = SearchAPIQueryFilterFactory.get_condition_values( + filter_input, + ) + for condition in filter_data: + where_filters.append( + SearchAPIWhereFilter( + field=condition[0], value=condition[1], operation=condition[2], + ), + ) + return where_filters + + @staticmethod + def get_include_filter(filter_input): + query_filters = [] + for related_model in filter_input: + included_entity = related_model["relation"] + query_filters.append(SearchAPIIncludeFilter(included_entity)) + + if "scope" in related_model: + try: + where_key = list(related_model["scope"]["where"].keys(),)[0] + + if where_key == "text": + # TODO - we might want to move this to the data + # definitions at a later point + text_operator_fields = { + "datasets": ["title"], + "documents": ["title", "summary"], + "files": ["name"], + "instrument": ["name", "facility"], + "samples": ["name", "description"], + "techniques": ["name"], + } + field_names = text_operator_fields[included_entity] + else: + field_names = [where_key] + + for field_name in field_names: + full_field_path = f"{included_entity}.{field_name}" + where_filter = { + "filter": { + "where": { + full_field_path: related_model["scope"]["where"][ + where_key + ], + }, + }, + } + query_filters.extend( + SearchAPIQueryFilterFactory.get_query_filter(where_filter,), + ) + except KeyError: + raise FilterError("Error in scope for include filter") + + return query_filters + @staticmethod def get_condition_values(filter_input): where_filter_data = [] From b851f969b0e5503d341c6251e7057c1cb7856c0c Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Mon, 22 Nov 2021 12:40:47 +0000 Subject: [PATCH 17/44] chore: add pattern to gitignore - I've got a few different config files as I'm switching between the old and new config style, so this change keeps them from Git worrying about them --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 6c34b475..887d4d11 100644 --- a/.gitignore +++ b/.gitignore @@ -2,7 +2,7 @@ venv/ .idea/ *.pyc logs.log* -config.json +config.json* .vscode/ .nox/ .python-version From cdb30ff0d4ebc8a0901e385d14bbd623f7c96fbb Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Fri, 26 Nov 2021 08:59:23 +0000 Subject: [PATCH 18/44] move text operator logic to WHERE filter #259 --- .../src/search_api/query_filter_factory.py | 81 ++++++++++--------- 1 file changed, 44 insertions(+), 37 deletions(-) diff --git a/datagateway_api/src/search_api/query_filter_factory.py b/datagateway_api/src/search_api/query_filter_factory.py index 80337184..6c8a1267 100644 --- a/datagateway_api/src/search_api/query_filter_factory.py +++ b/datagateway_api/src/search_api/query_filter_factory.py @@ -14,7 +14,7 @@ class SearchAPIQueryFilterFactory(QueryFilterFactory): @staticmethod - def get_query_filter(request_filter): + def get_query_filter(request_filter, entity_name=None): query_param_name = list(request_filter)[0].lower() query_filters = [] @@ -23,7 +23,9 @@ def get_query_filter(request_filter): for filter_name, filter_input in request_filter["filter"].items(): if filter_name == "where": query_filters.extend( - SearchAPIQueryFilterFactory.get_where_filter(filter_input), + SearchAPIQueryFilterFactory.get_where_filter( + filter_input, entity_name, + ), ) elif filter_name == "include": query_filters.extend( @@ -41,7 +43,7 @@ def get_query_filter(request_filter): # For the count endpoints query_filters.extend( SearchAPIQueryFilterFactory.get_query_filter( - {"filter": request_filter}, + {"filter": request_filter}, entity_name, ), ) else: @@ -52,7 +54,7 @@ def get_query_filter(request_filter): return query_filters @staticmethod - def get_where_filter(filter_input): + def get_where_filter(filter_input, entity_name): where_filters = [] if ( list(filter_input.keys())[0] == "and" @@ -74,6 +76,34 @@ def get_where_filter(filter_input): boolean_operator=boolean_operator, ), ) + elif list(filter_input.keys())[0] == "text": + # TODO - we might want to move this to the data + # definitions at a later point + text_operator_fields = { + "datasets": ["title"], + "documents": ["title", "summary"], + "files": ["name"], + "instrument": ["name", "facility"], + "samples": ["name", "description"], + "techniques": ["name"], + } + + try: + field_names = text_operator_fields[entity_name] + for field_name in field_names: + where_filter = { + "filter": {"where": {field_name: filter_input["text"]}}, + } + where_filters.extend( + SearchAPIQueryFilterFactory.get_query_filter( + where_filter, entity_name, + ), + ) + except KeyError: + # Do not raise FilterError nor attempt to create filters. Simply + # ignore text operator queries on fields that are not part of the + # text_operator_fields dict. + pass else: filter_data = SearchAPIQueryFilterFactory.get_condition_values( filter_input, @@ -94,40 +124,17 @@ def get_include_filter(filter_input): query_filters.append(SearchAPIIncludeFilter(included_entity)) if "scope" in related_model: - try: - where_key = list(related_model["scope"]["where"].keys(),)[0] - - if where_key == "text": - # TODO - we might want to move this to the data - # definitions at a later point - text_operator_fields = { - "datasets": ["title"], - "documents": ["title", "summary"], - "files": ["name"], - "instrument": ["name", "facility"], - "samples": ["name", "description"], - "techniques": ["name"], - } - field_names = text_operator_fields[included_entity] - else: - field_names = [where_key] - - for field_name in field_names: - full_field_path = f"{included_entity}.{field_name}" - where_filter = { - "filter": { - "where": { - full_field_path: related_model["scope"]["where"][ - where_key - ], - }, - }, - } - query_filters.extend( - SearchAPIQueryFilterFactory.get_query_filter(where_filter,), + # Scope filter can have WHERE, INCLUDE, LIMIT and SKIP filters + scope_query_filters = SearchAPIQueryFilterFactory.get_query_filter( + {"filter": related_model["scope"]}, included_entity, + ) + + for scope_query_filter in scope_query_filters: + if isinstance(scope_query_filter, SearchAPIWhereFilter): + scope_query_filter.field = ( + f"{included_entity}.{scope_query_filter.field}" ) - except KeyError: - raise FilterError("Error in scope for include filter") + query_filters.extend(scope_query_filters) return query_filters From 68b85efa8a9209219e8a64e0debe17a2005a4d08 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Fri, 26 Nov 2021 09:00:57 +0000 Subject: [PATCH 19/44] test: fix failing tests and add more text operator test cases #259 --- .../test_search_api_query_filter_factory.py | 88 +++++++++++++++++-- 1 file changed, 81 insertions(+), 7 deletions(-) diff --git a/test/search_api/test_search_api_query_filter_factory.py b/test/search_api/test_search_api_query_filter_factory.py index c6e98ca8..dd7ae45f 100644 --- a/test/search_api/test_search_api_query_filter_factory.py +++ b/test/search_api/test_search_api_query_filter_factory.py @@ -14,11 +14,12 @@ class TestSearchAPIQueryFilterFactory: @pytest.mark.parametrize( - "test_request_filter, expected_length, expected_fields, expected_operations" - ", expected_values, expected_boolean_operators", + "test_request_filter, test_entity_name, expected_length, expected_fields," + "expected_operations, expected_values, expected_boolean_operators", [ pytest.param( {"filter": {"where": {"title": "My Title"}}}, + "documents", 1, ["title"], ["eq"], @@ -28,6 +29,7 @@ class TestSearchAPIQueryFilterFactory: ), pytest.param( {"filter": {"where": {"summary": {"like": "My Test Summary"}}}}, + "documents", 1, ["summary"], ["like"], @@ -37,6 +39,7 @@ class TestSearchAPIQueryFilterFactory: ), pytest.param( {"filter": {"where": {"text": "Dataset 1"}}}, + "datasets", 1, ["title"], ["eq"], @@ -46,6 +49,7 @@ class TestSearchAPIQueryFilterFactory: ), pytest.param( {"filter": {"where": {"text": "Instrument 1"}}}, + "instrument", 2, ["name", "facility"], ["eq", "eq"], @@ -55,6 +59,7 @@ class TestSearchAPIQueryFilterFactory: ), pytest.param( {"filter": {"where": {"and": [{"summary": "My Test Summary"}]}}}, + "documents", 1, ["summary"], ["eq"], @@ -73,6 +78,7 @@ class TestSearchAPIQueryFilterFactory: }, }, }, + "documents", 2, ["summary", "title"], ["eq", "eq"], @@ -82,6 +88,7 @@ class TestSearchAPIQueryFilterFactory: ), pytest.param( {"filter": {"where": {"and": [{"value": {"lt": 50}}]}}}, + "parameters", 1, ["value"], ["lt"], @@ -100,6 +107,7 @@ class TestSearchAPIQueryFilterFactory: }, }, }, + "parameters", 2, ["name", "value"], ["like", "gte"], @@ -122,6 +130,7 @@ class TestSearchAPIQueryFilterFactory: }, }, }, + "parameters", 2, ["name", "value"], ["like", "gte"], @@ -140,6 +149,7 @@ class TestSearchAPIQueryFilterFactory: }, }, }, + "parameters", 2, ["name", "value"], ["like", "gte"], @@ -149,6 +159,7 @@ class TestSearchAPIQueryFilterFactory: ), pytest.param( {"where": {"summary": {"like": "My Test Summary"}}}, + "documents", 1, ["summary"], ["like"], @@ -161,13 +172,16 @@ class TestSearchAPIQueryFilterFactory: def test_valid_where_filter( self, test_request_filter, + test_entity_name, expected_length, expected_fields, expected_operations, expected_values, expected_boolean_operators, ): - filters = SearchAPIQueryFilterFactory.get_query_filter(test_request_filter) + filters = SearchAPIQueryFilterFactory.get_query_filter( + test_request_filter, test_entity_name, + ) assert len(filters) == expected_length for test_filter, field, operation, value, boolean_operator in zip( @@ -184,11 +198,12 @@ def test_valid_where_filter( assert test_filter.boolean_operator == boolean_operator @pytest.mark.parametrize( - "test_request_filter, expected_length, expected_included_entities" - ", expected_where_filter_data", + "test_request_filter, test_entity_name, expected_length" + ", expected_included_entities, expected_where_filter_data", [ pytest.param( {"filter": {"include": [{"relation": "files"}]}}, + "datasets", 1, [["files"]], [[]], @@ -200,6 +215,7 @@ def test_valid_where_filter( "include": [{"relation": "files"}, {"relation": "instrument"}], }, }, + "datasets", 2, [["files"], ["instrument"]], [[], []], @@ -216,6 +232,7 @@ def test_valid_where_filter( ], }, }, + "datasets", 2, [["parameters"], []], [[], ["parameters.name", "eq", "My parameter"]], @@ -236,6 +253,7 @@ def test_valid_where_filter( ], }, }, + "datasets", 4, [["parameters"], [], ["documents"], []], [ @@ -257,6 +275,7 @@ def test_valid_where_filter( ], }, }, + "datasets", 2, [["parameters"], []], [[], ["parameters.name", "ne", "My parameter"]], @@ -273,10 +292,28 @@ def test_valid_where_filter( ], }, }, + "datasets", 2, [["files"], []], [[], ["files.name", "eq", "file1"]], - id="Related model with scope (text operator)", + id="Related model with scope (text operator on defined field)", + ), + pytest.param( + { + "filter": { + "include": [ + { + "relation": "parameters", + "scope": {"where": {"text": "My parameter"}}, + }, + ], + }, + }, + "datasets", + 1, + [["parameters"], []], + [[], []], + id="Related model with scope (text operator on non-defined field)", ), pytest.param( { @@ -289,6 +326,7 @@ def test_valid_where_filter( ], }, }, + "datasets", 3, [["documents"], []], [ @@ -316,6 +354,7 @@ def test_valid_where_filter( ], }, }, + "datasets", 3, [["documents"], [], []], [ @@ -325,16 +364,51 @@ def test_valid_where_filter( ], id="Related model with scope (boolean operator)", ), + pytest.param( + { + "filter": { + "include": [ + { + "relation": "datasets", + "scope": { + "where": {"title": "Dataset 1"}, + "include": [ + { + "relation": "instrument", + "scope": { + "where": {"name": "Instrument 1"}, + }, + }, + ], + }, + }, + ], + }, + }, + "documents", + 4, + [["datasets"], [], ["instrument"], []], + [ + [], + ["datasets.title", "eq", "Dataset 1"], + [], + ["datasets.instrument.name", "eq", "Instrument 1"], + ], + id="Nested related models (with scope and where filters)", + ), ], ) def test_valid_include_filter( self, test_request_filter, + test_entity_name, expected_length, expected_included_entities, expected_where_filter_data, ): - filters = SearchAPIQueryFilterFactory.get_query_filter(test_request_filter) + filters = SearchAPIQueryFilterFactory.get_query_filter( + test_request_filter, test_entity_name, + ) assert len(filters) == expected_length From a5905148e524fec250c1d381440cca0dc3fab7a7 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Fri, 26 Nov 2021 10:47:19 +0000 Subject: [PATCH 20/44] add support for nested AND/ OR #259 --- .../src/search_api/query_filter_factory.py | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/datagateway_api/src/search_api/query_filter_factory.py b/datagateway_api/src/search_api/query_filter_factory.py index 6c8a1267..1366ab75 100644 --- a/datagateway_api/src/search_api/query_filter_factory.py +++ b/datagateway_api/src/search_api/query_filter_factory.py @@ -64,18 +64,16 @@ def get_where_filter(filter_input, entity_name): conditions = list(filter_input.values())[0] for condition in conditions: # Could be nested AND/OR - filter_data = SearchAPIQueryFilterFactory.get_condition_values( - condition, + where_filter = { + "filter": {"where": condition}, + } + conditional_where_filters = SearchAPIQueryFilterFactory.get_query_filter( + where_filter, entity_name, ) - for condition in filter_data: - where_filters.append( - SearchAPIWhereFilter( - field=condition[0], - value=condition[1], - operation=condition[2], - boolean_operator=boolean_operator, - ), - ) + + for conditional_where_filter in conditional_where_filters: + conditional_where_filter.boolean_operator = boolean_operator + where_filters.extend(conditional_where_filters) elif list(filter_input.keys())[0] == "text": # TODO - we might want to move this to the data # definitions at a later point From 865bf977623d93018a653aa13d0b9ea74c91041e Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Fri, 26 Nov 2021 10:52:19 +0000 Subject: [PATCH 21/44] use OR boolean operator for WHERE filters in text operator #259 --- .../src/search_api/query_filter_factory.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/datagateway_api/src/search_api/query_filter_factory.py b/datagateway_api/src/search_api/query_filter_factory.py index 1366ab75..c071abc6 100644 --- a/datagateway_api/src/search_api/query_filter_factory.py +++ b/datagateway_api/src/search_api/query_filter_factory.py @@ -87,16 +87,19 @@ def get_where_filter(filter_input, entity_name): } try: + or_conditional_filters = [] field_names = text_operator_fields[entity_name] for field_name in field_names: - where_filter = { - "filter": {"where": {field_name: filter_input["text"]}}, - } - where_filters.extend( - SearchAPIQueryFilterFactory.get_query_filter( - where_filter, entity_name, - ), - ) + or_conditional_filters.append({field_name: filter_input["text"]}) + + where_filter = { + "filter": {"where": {"or": or_conditional_filters}}, + } + where_filters.extend( + SearchAPIQueryFilterFactory.get_query_filter( + where_filter, entity_name, + ), + ) except KeyError: # Do not raise FilterError nor attempt to create filters. Simply # ignore text operator queries on fields that are not part of the From c39795c4b5e4b04403f8bd1270f20267e7cb4269 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Fri, 26 Nov 2021 10:53:11 +0000 Subject: [PATCH 22/44] test: correct unit tests #259 --- test/search_api/test_search_api_query_filter_factory.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/search_api/test_search_api_query_filter_factory.py b/test/search_api/test_search_api_query_filter_factory.py index dd7ae45f..9d019322 100644 --- a/test/search_api/test_search_api_query_filter_factory.py +++ b/test/search_api/test_search_api_query_filter_factory.py @@ -44,7 +44,7 @@ class TestSearchAPIQueryFilterFactory: ["title"], ["eq"], ["Dataset 1"], - ["and"], + ["or"], id="Text operator on dataset", ), pytest.param( @@ -54,7 +54,7 @@ class TestSearchAPIQueryFilterFactory: ["name", "facility"], ["eq", "eq"], ["Instrument 1", "Instrument 1"], - ["and", "and"], + ["or", "or"], id="Text operator on instrument", ), pytest.param( From 1d24021ce32fe7fa9843de7c438aa6713d4eb44e Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Mon, 29 Nov 2021 11:43:49 +0000 Subject: [PATCH 23/44] pass in entity name to `get_query_filter()` to be used by text operator #259 --- .../src/common/base_query_filter_factory.py | 5 ++++- datagateway_api/src/common/helpers.py | 9 +++++++-- .../src/datagateway_api/query_filter_factory.py | 7 ++++++- datagateway_api/src/resources/search_api_endpoints.py | 10 +++++----- 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/datagateway_api/src/common/base_query_filter_factory.py b/datagateway_api/src/common/base_query_filter_factory.py index 89405f78..d9a9c4a4 100644 --- a/datagateway_api/src/common/base_query_filter_factory.py +++ b/datagateway_api/src/common/base_query_filter_factory.py @@ -3,12 +3,15 @@ class QueryFilterFactory(object): @abstractstaticmethod - def get_query_filter(request_filter): + def get_query_filter(request_filter, entity_name=None): """ Given a filter, return a matching Query filter object :param request_filter: The filter to create the QueryFilter for :type request_filter: :class:`dict` + :param entity_name: Entity name of the endpoint, optional (only used for search + API, not DataGateway API) + :type entity_name: :class:`str` :return: The QueryFilter object created """ pass diff --git a/datagateway_api/src/common/helpers.py b/datagateway_api/src/common/helpers.py index f6b3c24c..79d8c7ec 100644 --- a/datagateway_api/src/common/helpers.py +++ b/datagateway_api/src/common/helpers.py @@ -87,7 +87,7 @@ def is_valid_json(string): return True -def get_filters_from_query_string(api_type): +def get_filters_from_query_string(api_type, entity_name=None): """ Gets a list of filters from the query_strings arg,value pairs, and returns a list of QueryFilter Objects @@ -95,6 +95,9 @@ def get_filters_from_query_string(api_type): :param api_type: Type of API this function is being used for i.e. DataGateway API or Search API :type api_type: :class:`str` + :param entity_name: Entity name of the endpoint, optional (only used for search + API, not DataGateway API) + :type entity_name: :class:`str` :raises ApiError: If `api_type` isn't a valid value :return: The list of filters """ @@ -117,7 +120,9 @@ def get_filters_from_query_string(api_type): for arg in request.args: for value in request.args.getlist(arg): filters.extend( - QueryFilterFactory.get_query_filter({arg: json.loads(value)}), + QueryFilterFactory.get_query_filter( + {arg: json.loads(value)}, entity_name, + ), ) return filters except Exception as e: diff --git a/datagateway_api/src/datagateway_api/query_filter_factory.py b/datagateway_api/src/datagateway_api/query_filter_factory.py index 7f5f838f..e50a3eb7 100644 --- a/datagateway_api/src/datagateway_api/query_filter_factory.py +++ b/datagateway_api/src/datagateway_api/query_filter_factory.py @@ -12,7 +12,7 @@ class DataGatewayAPIQueryFilterFactory(QueryFilterFactory): @staticmethod - def get_query_filter(request_filter): + def get_query_filter(request_filter, entity_name=None): """ Given a filter, return a matching Query filter object @@ -23,6 +23,11 @@ def get_query_filter(request_filter): :param request_filter: The filter to create the QueryFilter for :type request_filter: :class:`dict` + :param entity_name: Not utilised in DataGateway API implementation of this + static function, used in the search API. It is part of the method signature + as the same function call (called in `get_filters_from_query_string()`) is + used for both implementations + :type entity_name: :class:`str` :return: The QueryFilter object created :raises ApiError: If the backend type contains an invalid value :raises FilterError: If the filter name is not recognised diff --git a/datagateway_api/src/resources/search_api_endpoints.py b/datagateway_api/src/resources/search_api_endpoints.py index 0290b451..e9fd1353 100644 --- a/datagateway_api/src/resources/search_api_endpoints.py +++ b/datagateway_api/src/resources/search_api_endpoints.py @@ -17,7 +17,7 @@ def get_search_endpoint(name): class Endpoint(Resource): def get(self): - filters = get_filters_from_query_string("search_api") + filters = get_filters_from_query_string("search_api", name) log.debug("Filters: %s", filters) """ TODO - Need to return similar to @@ -46,7 +46,7 @@ def get_single_endpoint(name): class EndpointWithID(Resource): def get(self, pid): - filters = get_filters_from_query_string("search_api") + filters = get_filters_from_query_string("search_api", name) log.debug("Filters: %s", filters) # TODO - Add return pass @@ -65,7 +65,7 @@ def get_number_count_endpoint(name): class CountEndpoint(Resource): def get(self): # Only WHERE included on count endpoints - filters = get_filters_from_query_string("search_api") + filters = get_filters_from_query_string("search_api", name) log.debug("Filters: %s", filters) # TODO - Add return pass @@ -83,7 +83,7 @@ def get_files_endpoint(name): class FilesEndpoint(Resource): def get(self, pid): - filters = get_filters_from_query_string("search_api") + filters = get_filters_from_query_string("search_api", name) log.debug("Filters: %s", filters) # TODO - Add return pass @@ -102,7 +102,7 @@ def get_number_count_files_endpoint(name): class CountFilesEndpoint(Resource): def get(self, pid): # Only WHERE included on count endpoints - filters = get_filters_from_query_string("search_api") + filters = get_filters_from_query_string("search_api", name) log.debug("Filters: %s", filters) # TODO - Add return pass From 583cbf29744b72c020429b61ae7442b19acef231 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Tue, 30 Nov 2021 17:04:49 +0000 Subject: [PATCH 24/44] feat: add class to represent nested conditions #259 --- .../src/search_api/nested_where_filters.py | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 datagateway_api/src/search_api/nested_where_filters.py diff --git a/datagateway_api/src/search_api/nested_where_filters.py b/datagateway_api/src/search_api/nested_where_filters.py new file mode 100644 index 00000000..2c166e55 --- /dev/null +++ b/datagateway_api/src/search_api/nested_where_filters.py @@ -0,0 +1,30 @@ +class NestedWhereFilters: + def __init__(self, lhs, rhs, joining_operator): + """ + Class to represent nested conditions that use different boolean operators e.g. + `(A OR B) AND (C OR D)`. This works by joining the two conditions with a boolean + operator + + :param lhs: Left hand side of the condition - either a string condition, WHERE + filter or instance of this class + :type lhs: Any class that has `__str__()` implemented, but use cases will be for + :class:`str` or :class:`SearchAPIWhereFilter` or :class:`NestedWhereFilters` + :param rhs: Right hand side of the condition - either a string condition, WHERE + filter or instance of this class + :type rhs: Any class that has `__str__()` implemented, but use cases will be for + :class:`str` or :class:`SearchAPIWhereFilter` or :class:`NestedWhereFilters` + :param joining_operator: Boolean operator used to join the conditions of `lhs` + `rhs` (e.g. `AND` or `OR`) + :type joining_operator: :class:`str` + """ + + self.lhs = str(lhs) + self.rhs = str(rhs) + self.joining_operator = joining_operator + + def __str__(self): + """ + Join the condition on the left with the one on the right with the boolean + operator + """ + return f"({self.lhs} {self.joining_operator} {self.rhs})" From 530f0242ed61e14d5bd360f6a3bd5faf873a252b Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Tue, 30 Nov 2021 17:05:49 +0000 Subject: [PATCH 25/44] test: add initial tests for `NestedWhereFilters` #259 - I will likely add more tests to this class in the future, particularly to cover a mixture of `SearchAPIWhereFilter` and `NestedWhereFilters` --- test/search_api/test_nested_where_filters.py | 107 +++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 test/search_api/test_nested_where_filters.py diff --git a/test/search_api/test_nested_where_filters.py b/test/search_api/test_nested_where_filters.py new file mode 100644 index 00000000..37acbdfc --- /dev/null +++ b/test/search_api/test_nested_where_filters.py @@ -0,0 +1,107 @@ +import pytest + +from datagateway_api.src.search_api.filters import SearchAPIWhereFilter +from datagateway_api.src.search_api.nested_where_filters import NestedWhereFilters + + +class TestNestedWhereFilters: + @pytest.mark.parametrize( + "lhs, rhs, joining_operator, expected_where_clause", + [ + pytest.param("A", "B", "AND", "(A AND B)", id="(A AND B)"), + pytest.param("A", "B", "OR", "(A OR B)", id="(A OR B)"), + pytest.param( + "(A AND B)", + "(C AND D)", + "AND", + "((A AND B) AND (C AND D))", + id="((A AND B) AND (C AND D))", + ), + pytest.param( + "(A AND B)", + "(C AND D)", + "OR", + "((A AND B) OR (C AND D))", + id="((A AND B) OR (C AND D))", + ), + pytest.param( + "(A OR B)", + "(C OR D)", + "OR", + "((A OR B) OR (C OR D))", + id="((A OR B) OR (C OR D))", + ), + pytest.param( + "(A OR B)", + "(C OR D)", + "AND", + "((A OR B) AND (C OR D))", + id="((A OR B) AND (C OR D))", + ), + pytest.param( + "(A AND B AND C) OR (C AND D)", + "(E AND F)", + "OR", + "((A AND B AND C) OR (C AND D) OR (E AND F))", + id="((A AND B AND C) OR (C AND D) OR (E AND F))", + ), + pytest.param( + "(A AND B AND C) AND (C OR D)", + "(E AND F)", + "OR", + "((A AND B AND C) AND (C OR D) OR (E AND F))", + id="((A AND B AND C) AND (C OR D)) OR (E AND F))", + ), + pytest.param( + "((A AND B AND C) AND (C OR D))", + "(E AND F)", + "OR", + "(((A AND B AND C) AND (C OR D)) OR (E AND F))", + id="(((A AND B AND C) AND (C OR D))) OR (E AND F))", + ), + ], + ) + def test_str_filters(self, lhs, rhs, joining_operator, expected_where_clause): + test_nest = NestedWhereFilters(lhs, rhs, joining_operator) + + where_clause = str(test_nest) + + assert where_clause == expected_where_clause + + @pytest.mark.parametrize( + "lhs, rhs, joining_operator, expected_where_clause", + [ + pytest.param( + SearchAPIWhereFilter("name", "test name", "eq", "and"), + SearchAPIWhereFilter("id", 3, "eq", "and"), + "OR", + "(o.name = 'test name' OR o.id = '3')", + id="(o.name = 'test name' OR o.id = '3')", + ), + ], + ) + def test_search_api_filters( + self, lhs, rhs, joining_operator, expected_where_clause, + ): + test_nest = NestedWhereFilters(lhs, rhs, joining_operator) + where_clause = str(test_nest) + assert where_clause == expected_where_clause + + @pytest.mark.parametrize( + "lhs, rhs, joining_operator, expected_where_clause", + [ + pytest.param( + NestedWhereFilters("A", "B", "OR"), + NestedWhereFilters("C", "D", "AND"), + "OR", + "((A OR B) OR (C AND D))", + id="((A OR B) OR (C AND D))", + ), + ], + ) + def test_nested_classes( + self, lhs, rhs, joining_operator, expected_where_clause, + ): + test_nest = NestedWhereFilters(lhs, rhs, joining_operator) + where_clause = str(test_nest) + assert where_clause == expected_where_clause From 8f90ac5049ab9e7f009c894029d9a3bc1626b929 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Tue, 30 Nov 2021 18:10:56 +0000 Subject: [PATCH 26/44] test: add more test cases for where filters with boolean operators #259 --- .../test_search_api_query_filter_factory.py | 977 +++++++++++++++++- 1 file changed, 950 insertions(+), 27 deletions(-) diff --git a/test/search_api/test_search_api_query_filter_factory.py b/test/search_api/test_search_api_query_filter_factory.py index 9d019322..2c07a859 100644 --- a/test/search_api/test_search_api_query_filter_factory.py +++ b/test/search_api/test_search_api_query_filter_factory.py @@ -25,7 +25,7 @@ class TestSearchAPIQueryFilterFactory: ["eq"], ["My Title"], ["and"], - id="Property value, no specified operator", + id="Property value with no operator", ), pytest.param( {"filter": {"where": {"summary": {"like": "My Test Summary"}}}}, @@ -57,6 +57,50 @@ class TestSearchAPIQueryFilterFactory: ["or", "or"], id="Text operator on instrument", ), + pytest.param( + {"where": {"summary": {"like": "My Test Summary"}}}, + "documents", + 1, + ["summary"], + ["like"], + ["My Test Summary"], + ["and"], + id="WHERE filter in syntax for count endpoints", + ), + ], + ) + def test_valid_where_filter( + self, + test_request_filter, + test_entity_name, + expected_length, + expected_fields, + expected_operations, + expected_values, + expected_boolean_operators, + ): + filters = SearchAPIQueryFilterFactory.get_query_filter( + test_request_filter, test_entity_name, + ) + + assert len(filters) == expected_length + for test_filter, field, operation, value, boolean_operator in zip( + filters, + expected_fields, + expected_operations, + expected_values, + expected_boolean_operators, + ): + assert isinstance(test_filter, SearchAPIWhereFilter) + assert test_filter.field == field + assert test_filter.operation == operation + assert test_filter.value == value + assert test_filter.boolean_operator == boolean_operator + + @pytest.mark.parametrize( + "test_request_filter, test_entity_name, expected_length, expected_fields," + "expected_operations, expected_values, expected_boolean_operators", + [ pytest.param( {"filter": {"where": {"and": [{"summary": "My Test Summary"}]}}}, "documents", @@ -65,7 +109,7 @@ class TestSearchAPIQueryFilterFactory: ["eq"], ["My Test Summary"], ["and"], - id="AND with single condition, no specified operator", + id="Single condition, property value with no operator", ), pytest.param( { @@ -84,7 +128,27 @@ class TestSearchAPIQueryFilterFactory: ["eq", "eq"], ["My Test Summary", "Test title"], ["and", "and"], - id="AND with multiple conditions, no specified operator", + id="Multiple conditions (two), property values with no operator", + ), + pytest.param( + { + "filter": { + "where": { + "and": [ + {"summary": "My Test Summary"}, + {"title": "Test title"}, + {"type": "Test type"}, + ], + }, + }, + }, + "documents", + 3, + ["summary", "title", "type"], + ["eq", "eq", "eq"], + ["My Test Summary", "Test title", "Test type"], + ["and", "and", "and"], + id="Multiple conditions (three), property values with no operator", ), pytest.param( {"filter": {"where": {"and": [{"value": {"lt": 50}}]}}}, @@ -94,7 +158,7 @@ class TestSearchAPIQueryFilterFactory: ["lt"], [50], ["and"], - id="AND, single condition with operator", + id="Single condition, property value with operator", ), pytest.param( { @@ -113,30 +177,212 @@ class TestSearchAPIQueryFilterFactory: ["like", "gte"], ["Test name", 275], ["and", "and"], - id="AND, multiple conditions with operator", + id="Multiple conditions (two), property values with operator", ), pytest.param( { "filter": { "where": { "and": [ - { - "and": [ - {"name": {"like": "Test name"}}, - {"value": {"gte": 275}}, - ], - }, + {"name": {"like": "Test name"}}, + {"value": {"gte": 275}}, + {"unit": {"nlike": "Test unit"}}, ], }, }, }, "parameters", + 3, + ["name", "value", "unit"], + ["like", "gte", "nlike"], + ["Test name", 275, "Test unit"], + ["and", "and", "and"], + id="Multiple conditions (three), property values with operator", + ), + pytest.param( + {"filter": {"where": {"and": [{"text": "Dataset 1"}]}}}, + "datasets", + 1, + ["title"], + ["eq"], + ["Dataset 1"], + ["or"], + id="Single condition, text operator on dataset", + ), + pytest.param( + {"filter": {"where": {"and": [{"text": "Instrument 1"}]}}}, + "instrument", 2, - ["name", "value"], - ["like", "gte"], - ["Test name", 275], - ["and", "and"], - id="Nested AND, multiple conditions with operator", + ["name", "facility"], + ["eq", "eq"], + ["Instrument 1", "Instrument 1"], + ["or", "or"], + id="Single condition, text operator on instrument", + ), + pytest.param( + { + "filter": { + "where": {"and": [{"text": "Dataset 1"}, {"pid": "Test pid"}]}, + }, + }, + "datasets", + 2, + ["title", "pid"], + ["eq", "eq"], + ["Dataset 1", "Test pid"], + ["or", "and"], + id="Multiple conditions (two), text operator on dataset and " + "property value with no operator", + ), + pytest.param( + { + "filter": { + "where": { + "and": [{"text": "Instrument 1"}, {"pid": "Test pid"}], + }, + }, + }, + "instrument", + 3, + ["name", "facility", "pid"], + ["eq", "eq", "eq"], + ["Instrument 1", "Instrument 1", "Test pid"], + ["or", "or", "and"], + id="Multiple conditions (two), text operator on instrument and " + "property value with no operator", + ), + pytest.param( + { + "filter": { + "where": { + "and": [ + {"text": "Dataset 1"}, + {"pid": {"eq": "Test pid"}}, + ], + }, + }, + }, + "datasets", + 2, + ["title", "pid"], + ["eq", "eq"], + ["Dataset 1", "Test pid"], + ["or", "and"], + id="Multiple conditions (two), text operator on dataset and " + "property value with operator", + ), + pytest.param( + { + "filter": { + "where": { + "and": [ + {"text": "Instrument 1"}, + {"pid": {"eq": "Test pid"}}, + ], + }, + }, + }, + "instrument", + 3, + ["name", "facility", "pid"], + ["eq", "eq", "eq"], + ["Instrument 1", "Instrument 1", "Test pid"], + ["or", "or", "and"], + id="Multiple conditions (two), text operator on instrument and " + "property value with operator", + ), + ], + ) + def test_valid_where_filter_with_and_boolean_operator( + self, + test_request_filter, + test_entity_name, + expected_length, + expected_fields, + expected_operations, + expected_values, + expected_boolean_operators, + ): + filters = SearchAPIQueryFilterFactory.get_query_filter( + test_request_filter, test_entity_name, + ) + + assert len(filters) == expected_length + for test_filter, field, operation, value, boolean_operator in zip( + filters, + expected_fields, + expected_operations, + expected_values, + expected_boolean_operators, + ): + assert isinstance(test_filter, SearchAPIWhereFilter) + assert test_filter.field == field + assert test_filter.operation == operation + assert test_filter.value == value + assert test_filter.boolean_operator == boolean_operator + + @pytest.mark.parametrize( + "test_request_filter, test_entity_name, expected_length, expected_fields," + "expected_operations, expected_values, expected_boolean_operators", + [ + pytest.param( + {"filter": {"where": {"or": [{"summary": "My Test Summary"}]}}}, + "documents", + 1, + ["summary"], + ["eq"], + ["My Test Summary"], + ["or"], + id="Single condition, property value with no operator", + ), + pytest.param( + { + "filter": { + "where": { + "or": [ + {"summary": "My Test Summary"}, + {"title": "Test title"}, + ], + }, + }, + }, + "documents", + 2, + ["summary", "title"], + ["eq", "eq"], + ["My Test Summary", "Test title"], + ["or", "or"], + id="Multiple conditions (two), property values with no operator", + ), + pytest.param( + { + "filter": { + "where": { + "or": [ + {"summary": "My Test Summary"}, + {"title": "Test title"}, + {"type": "Test type"}, + ], + }, + }, + }, + "documents", + 3, + ["summary", "title", "type"], + ["eq", "eq", "eq"], + ["My Test Summary", "Test title", "Test type"], + ["or", "or", "or"], + id="Multiple conditions (three), property values with no operator", + ), + pytest.param( + {"filter": {"where": {"or": [{"value": {"lt": 50}}]}}}, + "parameters", + 1, + ["value"], + ["lt"], + [50], + ["or"], + id="Single condition, property value with operator", ), pytest.param( { @@ -155,21 +401,698 @@ class TestSearchAPIQueryFilterFactory: ["like", "gte"], ["Test name", 275], ["or", "or"], - id="OR, multiple conditions with operator", + id="Multiple conditions (two), property values with operator", ), pytest.param( - {"where": {"summary": {"like": "My Test Summary"}}}, - "documents", + { + "filter": { + "where": { + "or": [ + {"name": {"like": "Test name"}}, + {"value": {"gte": 275}}, + {"unit": {"nlike": "Test unit"}}, + ], + }, + }, + }, + "parameters", + 3, + ["name", "value", "unit"], + ["like", "gte", "nlike"], + ["Test name", 275, "Test unit"], + ["or", "or", "or"], + id="Multiple conditions (three), property values with operator", + ), + pytest.param( + {"filter": {"where": {"or": [{"text": "Dataset 1"}]}}}, + "datasets", 1, - ["summary"], - ["like"], - ["My Test Summary"], - ["and"], - id="WHERE filter in syntax for count endpoints", + ["title"], + ["eq"], + ["Dataset 1"], + ["or"], + id="Single condition, text operator on dataset", ), - ], - ) - def test_valid_where_filter( + pytest.param( + {"filter": {"where": {"or": [{"text": "Instrument 1"}]}}}, + "instrument", + 2, + ["name", "facility"], + ["eq", "eq"], + ["Instrument 1", "Instrument 1"], + ["or", "or"], + id="Single condition, text operator on instrument", + ), + pytest.param( + { + "filter": { + "where": {"or": [{"text": "Dataset 1"}, {"pid": "Test pid"}]}, + }, + }, + "datasets", + 2, + ["title", "pid"], + ["eq", "eq"], + ["Dataset 1", "Test pid"], + ["or", "or"], + id="Multiple conditions (two), text operator on dataset and " + "property value with no operator", + ), + pytest.param( + { + "filter": { + "where": { + "or": [{"text": "Instrument 1"}, {"pid": "Test pid"}], + }, + }, + }, + "instrument", + 3, + ["name", "facility", "pid"], + ["eq", "eq", "eq"], + ["Instrument 1", "Instrument 1", "Test pid"], + ["or", "or", "or"], + id="Multiple conditions (two), text operator on instrument and " + "property value with no operator", + ), + pytest.param( + { + "filter": { + "where": { + "or": [{"text": "Dataset 1"}, {"pid": {"eq": "Test pid"}}], + }, + }, + }, + "datasets", + 2, + ["title", "pid"], + ["eq", "eq"], + ["Dataset 1", "Test pid"], + ["or", "or"], + id="Multiple conditions (two), text operator on dataset and " + "property value with operator", + ), + pytest.param( + { + "filter": { + "where": { + "or": [ + {"text": "Instrument 1"}, + {"pid": {"eq": "Test pid"}}, + ], + }, + }, + }, + "instrument", + 3, + ["name", "facility", "pid"], + ["eq", "eq", "eq"], + ["Instrument 1", "Instrument 1", "Test pid"], + ["or", "or", "or"], + id="Multiple conditions (two), text operator on instrument and " + "property value with operator", + ), + ], + ) + def test_valid_where_filter_with_or_boolean_operator( + self, + test_request_filter, + test_entity_name, + expected_length, + expected_fields, + expected_operations, + expected_values, + expected_boolean_operators, + ): + filters = SearchAPIQueryFilterFactory.get_query_filter( + test_request_filter, test_entity_name, + ) + + assert len(filters) == expected_length + for test_filter, field, operation, value, boolean_operator in zip( + filters, + expected_fields, + expected_operations, + expected_values, + expected_boolean_operators, + ): + assert isinstance(test_filter, SearchAPIWhereFilter) + assert test_filter.field == field + assert test_filter.operation == operation + assert test_filter.value == value + assert test_filter.boolean_operator == boolean_operator + + @pytest.mark.parametrize( + "test_request_filter, test_entity_name, expected_length, expected_fields," + "expected_operations, expected_values, expected_boolean_operators", + [ + pytest.param( + { + "filter": { + "where": { + "and": [ + { + "and": [ + {"summary": "My Test Summary"}, + {"title": {"like": "Test title"}}, + ], + }, + { + "and": [ + {"pid": "Test pid"}, + {"type": {"eq": "Test type"}}, + ], + }, + ], + }, + }, + }, + "documents", + 4, + ["summary", "title", "pid", "type"], + ["eq", "like", "eq", "eq"], + ["My Test Summary", "Test title", "Test pid", "Test type"], + ["and", "and", "and", "and"], + id="With two AND boolean operators", + ), + pytest.param( + { + "filter": { + "where": { + "and": [ + { + "and": [ + {"summary": "My Test Summary"}, + {"title": {"like": "Test title"}}, + ], + }, + { + "or": [ + {"pid": "Test pid"}, + {"type": {"eq": "Test type"}}, + ], + }, + ], + }, + }, + }, + "documents", + 4, + ["summary", "title", "pid", "type"], + ["eq", "like", "eq", "eq"], + ["My Test Summary", "Test title", "Test pid", "Test type"], + ["and", "and", "or", "or"], + id="With AND and OR boolean operators", + ), + pytest.param( + { + "filter": { + "where": { + "and": [ + { + "or": [ + {"summary": "My Test Summary"}, + {"title": {"like": "Test title"}}, + ], + }, + { + "or": [ + {"pid": "Test pid"}, + {"type": {"eq": "Test type"}}, + ], + }, + ], + }, + }, + }, + "documents", + 4, + ["summary", "title", "pid", "type"], + ["eq", "like", "eq", "eq"], + ["My Test Summary", "Test title", "Test pid", "Test type"], + ["or", "or", "or", "or"], + id="With two OR boolean operators", + ), + pytest.param( + { + "filter": { + "where": { + "and": [ + { + "and": [ + {"summary": "My Test Summary"}, + {"title": {"like": "Test title"}}, + ], + }, + { + "and": [ + {"pid": "Test pid"}, + {"type": {"eq": "Test type"}}, + ], + }, + { + "and": [ + {"doi": "Test doi"}, + {"license": {"like": "Test license"}}, + ], + }, + ], + }, + }, + }, + "documents", + 6, + ["summary", "title", "pid", "type", "doi", "license"], + ["eq", "like", "eq", "eq", "eq", "like"], + [ + "My Test Summary", + "Test title", + "Test pid", + "Test type", + "Test doi", + "Test license", + ], + ["and", "and", "and", "and", "and", "and"], + id="With three AND boolean operators", + ), + pytest.param( + { + "filter": { + "where": { + "and": [ + { + "and": [ + {"summary": "My Test Summary"}, + {"title": {"like": "Test title"}}, + ], + }, + { + "and": [ + {"pid": "Test pid"}, + {"type": {"eq": "Test type"}}, + ], + }, + { + "or": [ + {"doi": "Test doi"}, + {"license": {"like": "Test license"}}, + ], + }, + ], + }, + }, + }, + "documents", + 6, + ["summary", "title", "pid", "type", "doi", "license"], + ["eq", "like", "eq", "eq", "eq", "like"], + [ + "My Test Summary", + "Test title", + "Test pid", + "Test type", + "Test doi", + "Test license", + ], + ["and", "and", "and", "and", "or", "or"], + id="With two AND and one OR boolean operators", + ), + pytest.param( + { + "filter": { + "where": { + "and": [ + { + "and": [ + {"summary": "My Test Summary"}, + {"title": {"like": "Test title"}}, + ], + }, + { + "or": [ + {"pid": "Test pid"}, + {"type": {"eq": "Test type"}}, + ], + }, + { + "or": [ + {"doi": "Test doi"}, + {"license": {"like": "Test license"}}, + ], + }, + ], + }, + }, + }, + "documents", + 6, + ["summary", "title", "pid", "type", "doi", "license"], + ["eq", "like", "eq", "eq", "eq", "like"], + [ + "My Test Summary", + "Test title", + "Test pid", + "Test type", + "Test doi", + "Test license", + ], + ["and", "and", "or", "or", "or", "or"], + id="With one AND and two OR boolean operators", + ), + pytest.param( + { + "filter": { + "where": { + "and": [ + { + "or": [ + {"summary": "My Test Summary"}, + {"title": {"like": "Test title"}}, + ], + }, + { + "or": [ + {"pid": "Test pid"}, + {"type": {"eq": "Test type"}}, + ], + }, + { + "or": [ + {"doi": "Test doi"}, + {"license": {"like": "Test license"}}, + ], + }, + ], + }, + }, + }, + "documents", + 6, + ["summary", "title", "pid", "type", "doi", "license"], + ["eq", "like", "eq", "eq", "eq", "like"], + [ + "My Test Summary", + "Test title", + "Test pid", + "Test type", + "Test doi", + "Test license", + ], + ["or", "or", "or", "or", "or", "or"], + id="With three OR boolean operators", + ), + ], + ) + def test_valid_where_filter_with_nested_and_boolean_operator( + self, + test_request_filter, + test_entity_name, + expected_length, + expected_fields, + expected_operations, + expected_values, + expected_boolean_operators, + ): + filters = SearchAPIQueryFilterFactory.get_query_filter( + test_request_filter, test_entity_name, + ) + + assert len(filters) == expected_length + for test_filter, field, operation, value, boolean_operator in zip( + filters, + expected_fields, + expected_operations, + expected_values, + expected_boolean_operators, + ): + assert isinstance(test_filter, SearchAPIWhereFilter) + assert test_filter.field == field + assert test_filter.operation == operation + assert test_filter.value == value + assert test_filter.boolean_operator == boolean_operator + + @pytest.mark.parametrize( + "test_request_filter, test_entity_name, expected_length, expected_fields," + "expected_operations, expected_values, expected_boolean_operators", + [ + pytest.param( + { + "filter": { + "where": { + "or": [ + { + "and": [ + {"summary": "My Test Summary"}, + {"title": {"like": "Test title"}}, + ], + }, + { + "and": [ + {"pid": "Test pid"}, + {"type": {"eq": "Test type"}}, + ], + }, + ], + }, + }, + }, + "documents", + 4, + ["summary", "title", "pid", "type"], + ["eq", "like", "eq", "eq"], + ["My Test Summary", "Test title", "Test pid", "Test type"], + ["and", "and", "and", "and"], + id="With two AND boolean operators", + ), + pytest.param( + { + "filter": { + "where": { + "or": [ + { + "and": [ + {"summary": "My Test Summary"}, + {"title": {"like": "Test title"}}, + ], + }, + { + "or": [ + {"pid": "Test pid"}, + {"type": {"eq": "Test type"}}, + ], + }, + ], + }, + }, + }, + "documents", + 4, + ["summary", "title", "pid", "type"], + ["eq", "like", "eq", "eq"], + ["My Test Summary", "Test title", "Test pid", "Test type"], + ["and", "and", "or", "or"], + id="With AND and OR boolean operators", + ), + pytest.param( + { + "filter": { + "where": { + "or": [ + { + "or": [ + {"summary": "My Test Summary"}, + {"title": {"like": "Test title"}}, + ], + }, + { + "or": [ + {"pid": "Test pid"}, + {"type": {"eq": "Test type"}}, + ], + }, + ], + }, + }, + }, + "documents", + 4, + ["summary", "title", "pid", "type"], + ["eq", "like", "eq", "eq"], + ["My Test Summary", "Test title", "Test pid", "Test type"], + ["or", "or", "or", "or"], + id="With two OR boolean operators", + ), + pytest.param( + { + "filter": { + "where": { + "or": [ + { + "and": [ + {"summary": "My Test Summary"}, + {"title": {"like": "Test title"}}, + ], + }, + { + "and": [ + {"pid": "Test pid"}, + {"type": {"eq": "Test type"}}, + ], + }, + { + "and": [ + {"doi": "Test doi"}, + {"license": {"like": "Test license"}}, + ], + }, + ], + }, + }, + }, + "documents", + 6, + ["summary", "title", "pid", "type", "doi", "license"], + ["eq", "like", "eq", "eq", "eq", "like"], + [ + "My Test Summary", + "Test title", + "Test pid", + "Test type", + "Test doi", + "Test license", + ], + ["and", "and", "and", "and", "and", "and"], + id="With three AND boolean operators", + ), + pytest.param( + { + "filter": { + "where": { + "or": [ + { + "and": [ + {"summary": "My Test Summary"}, + {"title": {"like": "Test title"}}, + ], + }, + { + "and": [ + {"pid": "Test pid"}, + {"type": {"eq": "Test type"}}, + ], + }, + { + "or": [ + {"doi": "Test doi"}, + {"license": {"like": "Test license"}}, + ], + }, + ], + }, + }, + }, + "documents", + 6, + ["summary", "title", "pid", "type", "doi", "license"], + ["eq", "like", "eq", "eq", "eq", "like"], + [ + "My Test Summary", + "Test title", + "Test pid", + "Test type", + "Test doi", + "Test license", + ], + ["and", "and", "and", "and", "or", "or"], + id="With two AND and one OR boolean operators", + ), + pytest.param( + { + "filter": { + "where": { + "or": [ + { + "and": [ + {"summary": "My Test Summary"}, + {"title": {"like": "Test title"}}, + ], + }, + { + "or": [ + {"pid": "Test pid"}, + {"type": {"eq": "Test type"}}, + ], + }, + { + "or": [ + {"doi": "Test doi"}, + {"license": {"like": "Test license"}}, + ], + }, + ], + }, + }, + }, + "documents", + 6, + ["summary", "title", "pid", "type", "doi", "license"], + ["eq", "like", "eq", "eq", "eq", "like"], + [ + "My Test Summary", + "Test title", + "Test pid", + "Test type", + "Test doi", + "Test license", + ], + ["and", "and", "or", "or", "or", "or"], + id="With one AND and two OR boolean operators", + ), + pytest.param( + { + "filter": { + "where": { + "or": [ + { + "or": [ + {"summary": "My Test Summary"}, + {"title": {"like": "Test title"}}, + ], + }, + { + "or": [ + {"pid": "Test pid"}, + {"type": {"eq": "Test type"}}, + ], + }, + { + "or": [ + {"doi": "Test doi"}, + {"license": {"like": "Test license"}}, + ], + }, + ], + }, + }, + }, + "documents", + 6, + ["summary", "title", "pid", "type", "doi", "license"], + ["eq", "like", "eq", "eq", "eq", "like"], + [ + "My Test Summary", + "Test title", + "Test pid", + "Test type", + "Test doi", + "Test license", + ], + ["or", "or", "or", "or", "or", "or"], + id="With three OR boolean operators", + ), + ], + ) + def test_valid_where_filter_with_nested_or_boolean_operator( self, test_request_filter, test_entity_name, From 0f0691782c10a2f805cf11d8ca8d90d0db4e6b36 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Wed, 1 Dec 2021 11:34:24 +0000 Subject: [PATCH 27/44] test: add more test cases for include filter with scope #259 --- .../test_search_api_query_filter_factory.py | 671 ++++++++++++++++-- 1 file changed, 623 insertions(+), 48 deletions(-) diff --git a/test/search_api/test_search_api_query_filter_factory.py b/test/search_api/test_search_api_query_filter_factory.py index 2c07a859..f4c832c0 100644 --- a/test/search_api/test_search_api_query_filter_factory.py +++ b/test/search_api/test_search_api_query_filter_factory.py @@ -1122,14 +1122,13 @@ def test_valid_where_filter_with_nested_or_boolean_operator( @pytest.mark.parametrize( "test_request_filter, test_entity_name, expected_length" - ", expected_included_entities, expected_where_filter_data", + ", expected_included_entities", [ pytest.param( {"filter": {"include": [{"relation": "files"}]}}, "datasets", 1, [["files"]], - [[]], id="Single related model", ), pytest.param( @@ -1141,9 +1140,31 @@ def test_valid_where_filter_with_nested_or_boolean_operator( "datasets", 2, [["files"], ["instrument"]], - [[], []], id="Multiple related models", ), + ], + ) + def test_valid_include_filter( + self, + test_request_filter, + test_entity_name, + expected_length, + expected_included_entities, + ): + filters = SearchAPIQueryFilterFactory.get_query_filter( + test_request_filter, test_entity_name, + ) + + assert len(filters) == expected_length + + for test_filter, included_entities in zip(filters, expected_included_entities): + if isinstance(test_filter, SearchAPIIncludeFilter): + assert test_filter.included_filters == included_entities + + @pytest.mark.parametrize( + "test_request_filter, test_entity_name, expected_length" + ", expected_included_entities, expected_where_filter_data", + [ pytest.param( { "filter": { @@ -1158,34 +1179,8 @@ def test_valid_where_filter_with_nested_or_boolean_operator( "datasets", 2, [["parameters"], []], - [[], ["parameters.name", "eq", "My parameter"]], - id="Related model with scope", - ), - pytest.param( - { - "filter": { - "include": [ - { - "relation": "parameters", - "scope": {"where": {"name": "My parameter"}}, - }, - { - "relation": "documents", - "scope": {"where": {"title": "Document title"}}, - }, - ], - }, - }, - "datasets", - 4, - [["parameters"], [], ["documents"], []], - [ - [], - ["parameters.name", "eq", "My parameter"], - [], - ["documents.title", "eq", "Document title"], - ], - id="Multiple related models with scopes", + [[], ["parameters.name", "eq", "My parameter", "and"]], + id="Property value with no operator", ), pytest.param( { @@ -1201,8 +1196,8 @@ def test_valid_where_filter_with_nested_or_boolean_operator( "datasets", 2, [["parameters"], []], - [[], ["parameters.name", "ne", "My parameter"]], - id="Related model with scope (with operator)", + [[], ["parameters.name", "ne", "My parameter", "and"]], + id="Property value with operator", ), pytest.param( { @@ -1218,8 +1213,8 @@ def test_valid_where_filter_with_nested_or_boolean_operator( "datasets", 2, [["files"], []], - [[], ["files.name", "eq", "file1"]], - id="Related model with scope (text operator on defined field)", + [[], ["files.name", "eq", "file1", "or"]], + id="Text operator on defined field mapping to single field", ), pytest.param( { @@ -1236,7 +1231,7 @@ def test_valid_where_filter_with_nested_or_boolean_operator( 1, [["parameters"], []], [[], []], - id="Related model with scope (text operator on non-defined field)", + id="Text operator on non-defined field", ), pytest.param( { @@ -1254,10 +1249,38 @@ def test_valid_where_filter_with_nested_or_boolean_operator( [["documents"], []], [ [], - ["documents.title", "eq", "document1"], - ["documents.summary", "eq", "document1"], + ["documents.title", "eq", "document1", "or"], + ["documents.summary", "eq", "document1", "or"], + ], + id="Text operator on defined field mapping to multiple field", + ), + pytest.param( + { + "filter": { + "include": [ + { + "relation": "documents", + "scope": { + "where": { + "and": [ + {"summary": "My Test Summary"}, + {"title": "Test title"}, + ], + }, + }, + }, + ], + }, + }, + "datasets", + 3, + [["documents"], [], []], + [ + [], + ["documents.summary", "eq", "My Test Summary", "and"], + ["documents.title", "eq", "Test title", "and"], ], - id="Related model with scope (text operator, multiple fields)", + id="AND boolean operator", ), pytest.param( { @@ -1282,10 +1305,140 @@ def test_valid_where_filter_with_nested_or_boolean_operator( [["documents"], [], []], [ [], - ["documents.summary", "eq", "My Test Summary"], - ["documents.title", "eq", "Test title"], + ["documents.summary", "eq", "My Test Summary", "and"], + ["documents.title", "eq", "Test title", "and"], ], - id="Related model with scope (boolean operator)", + id="OR boolean operator", + ), + pytest.param( + { + "filter": { + "include": [ + { + "relation": "documents", + "scope": { + "where": { + "and": [ + { + "and": [ + {"summary": "My Test Summary"}, + {"title": {"like": "Test title"}}, + ], + }, + { + "and": [ + {"pid": "Test pid"}, + {"type": {"eq": "Test type"}}, + ], + }, + { + "or": [ + {"doi": "Test doi"}, + { + "license": { + "like": "Test license", + }, + }, + ], + }, + ], + }, + }, + }, + ], + }, + }, + "datasets", + 7, + [["documents"], [], [], [], [], [], []], + [ + [], + ["documents.summary", "eq", "My Test Summary", "and"], + ["documents.title", "like", "Test title", "and"], + ["documents.pid", "eq", "Test pid", "and"], + ["documents.type", "eq", "Test type", "and"], + ["documents.doi", "eq", "Test doi", "or"], + ["documents.license", "like", "Test license", "or"], + ], + id="Nested AND boolean operator", + ), + pytest.param( + { + "filter": { + "include": [ + { + "relation": "documents", + "scope": { + "where": { + "or": [ + { + "and": [ + {"summary": "My Test Summary"}, + {"title": {"like": "Test title"}}, + ], + }, + { + "and": [ + {"pid": "Test pid"}, + {"type": {"eq": "Test type"}}, + ], + }, + { + "or": [ + {"doi": "Test doi"}, + { + "license": { + "like": "Test license", + }, + }, + ], + }, + ], + }, + }, + }, + ], + }, + }, + "datasets", + 7, + [["documents"], [], [], [], [], [], []], + [ + [], + ["documents.summary", "eq", "My Test Summary", "and"], + ["documents.title", "like", "Test title", "and"], + ["documents.pid", "eq", "Test pid", "and"], + ["documents.type", "eq", "Test type", "and"], + ["documents.doi", "eq", "Test doi", "or"], + ["documents.license", "like", "Test license", "or"], + ], + id="Nested OR boolean operator", + ), + pytest.param( + { + "filter": { + "include": [ + { + "relation": "parameters", + "scope": {"where": {"name": "My parameter"}}, + }, + { + "relation": "documents", + "scope": {"where": {"title": "Document title"}}, + }, + ], + }, + }, + "datasets", + 4, + [["parameters"], [], ["documents"], []], + [ + [], + ["parameters.name", "eq", "My parameter", "and"], + [], + ["documents.title", "eq", "Document title", "and"], + ], + id="Multiple related models", ), pytest.param( { @@ -1310,18 +1463,18 @@ def test_valid_where_filter_with_nested_or_boolean_operator( }, "documents", 4, - [["datasets"], [], ["instrument"], []], + [["datasets"], [], ["datasets.instrument"], []], [ [], - ["datasets.title", "eq", "Dataset 1"], + ["datasets.title", "eq", "Dataset 1", "and"], [], - ["datasets.instrument.name", "eq", "Instrument 1"], + ["datasets.instrument.name", "eq", "Instrument 1", "and"], ], - id="Nested related models (with scope and where filters)", + id="Nested related models", ), ], ) - def test_valid_include_filter( + def test_valid_include_filter_with_where_filter_in_scope( self, test_request_filter, test_entity_name, @@ -1344,7 +1497,429 @@ def test_valid_include_filter( assert test_filter.field == where_filter_data[0] assert test_filter.operation == where_filter_data[1] assert test_filter.value == where_filter_data[2] - # TODO - Assert for boolean_operator + assert test_filter.boolean_operator == where_filter_data[3] + + @pytest.mark.parametrize( + "test_request_filter, test_entity_name, expected_length" + ", expected_included_entities", + [ + pytest.param( + { + "filter": { + "include": [ + { + "relation": "datasets", + "scope": {"include": [{"relation": "parameters"}]}, + }, + ], + }, + }, + "documents", + 2, + [["datasets"], ["datasets.parameters"]], + id="Single related model", + ), + pytest.param( + { + "filter": { + "include": [ + { + "relation": "datasets", + "scope": { + "include": [ + {"relation": "parameters"}, + {"relation": "instrument"}, + ], + }, + }, + ], + }, + }, + "documents", + 3, + [["datasets"], ["datasets.parameters"], ["datasets.instrument"]], + id="Multiple related models", + ), + pytest.param( + { + "filter": { + "include": [ + { + "relation": "datasets", + "scope": { + "include": [ + { + "relation": "documents", + "scope": { + "include": [{"relation": "parameters"}], + }, + }, + ], + }, + }, + ], + }, + }, + "instruments", + 3, + [ + ["datasets"], + ["datasets.documents"], + ["datasets.documents.parameters"], + ], + id="Nested related models", + ), + ], + ) + def test_valid_include_filter_with_include_filter_in_scope( + self, + test_request_filter, + test_entity_name, + expected_length, + expected_included_entities, + ): + filters = SearchAPIQueryFilterFactory.get_query_filter( + test_request_filter, test_entity_name, + ) + + assert len(filters) == expected_length + + for test_filter, included_entities in zip(filters, expected_included_entities): + if isinstance(test_filter, SearchAPIIncludeFilter): + assert test_filter.included_filters == included_entities + + @pytest.mark.parametrize( + "test_request_filter, test_entity_name, expected_length" + ", expected_included_entities, expected_limit_values", + [ + pytest.param( + { + "filter": { + "include": [{"relation": "datasets", "scope": {"limit": 50}}], + }, + }, + "documents", + 2, + [["datasets"], []], + [None, 50], + id="Single related model", + ), + pytest.param( + { + "filter": { + "include": [ + {"relation": "datasets", "scope": {"limit": 50}}, + {"relation": "parameters", "scope": {"limit": 20}}, + ], + }, + }, + "documents", + 4, + [["datasets"], [], ["parameters"], []], + [None, 50, None, 20], + id="Multiple related model", + ), + pytest.param( + { + "filter": { + "include": [ + { + "relation": "datasets", + "scope": { + "include": [ + { + "relation": "documents", + "scope": { + "include": [{"relation": "parameters"}], + "limit": 20, + }, + }, + ], + "limit": 50, + }, + }, + ], + }, + }, + "documents", + 5, + [ + ["datasets"], + ["datasets.documents"], + ["datasets.documents.parameters"], + [], + [], + ], + [None, None, None, 20, 50], + id="Nested related models", + ), + ], + ) + def test_valid_include_filter_with_limit_filter_in_scope( + self, + test_request_filter, + test_entity_name, + expected_length, + expected_included_entities, + expected_limit_values, + ): + filters = SearchAPIQueryFilterFactory.get_query_filter( + test_request_filter, test_entity_name, + ) + + assert len(filters) == expected_length + + for test_filter, included_entities, limit_value in zip( + filters, expected_included_entities, expected_limit_values, + ): + if isinstance(test_filter, SearchAPIIncludeFilter): + assert test_filter.included_filters == included_entities + if isinstance(test_filter, SearchAPILimitFilter): + assert test_filter.limit_value == limit_value + + @pytest.mark.parametrize( + "test_request_filter, test_entity_name, expected_length" + ", expected_included_entities, expected_skip_values", + [ + pytest.param( + { + "filter": { + "include": [{"relation": "datasets", "scope": {"skip": 50}}], + }, + }, + "documents", + 2, + [["datasets"], []], + [None, 50], + id="Single related model", + ), + pytest.param( + { + "filter": { + "include": [ + {"relation": "datasets", "scope": {"skip": 50}}, + {"relation": "parameters", "scope": {"skip": 20}}, + ], + }, + }, + "documents", + 4, + [["datasets"], [], ["parameters"], []], + [None, 50, None, 20], + id="Multiple related model", + ), + pytest.param( + { + "filter": { + "include": [ + { + "relation": "datasets", + "scope": { + "include": [ + { + "relation": "documents", + "scope": { + "include": [{"relation": "parameters"}], + "skip": 20, + }, + }, + ], + "skip": 50, + }, + }, + ], + }, + }, + "documents", + 5, + [ + ["datasets"], + ["datasets.documents"], + ["datasets.documents.parameters"], + [], + [], + ], + [None, None, None, 20, 50], + id="Nested related models", + ), + ], + ) + def test_valid_include_filter_with_skip_filter_in_scope( + self, + test_request_filter, + test_entity_name, + expected_length, + expected_included_entities, + expected_skip_values, + ): + filters = SearchAPIQueryFilterFactory.get_query_filter( + test_request_filter, test_entity_name, + ) + + assert len(filters) == expected_length + + for test_filter, included_entities, skip_value in zip( + filters, expected_included_entities, expected_skip_values, + ): + if isinstance(test_filter, SearchAPIIncludeFilter): + assert test_filter.included_filters == included_entities + if isinstance(test_filter, SearchAPISkipFilter): + assert test_filter.skip_value == skip_value + + @pytest.mark.parametrize( + "test_request_filter, test_entity_name, expected_length" + ", expected_included_entities, expected_where_filter_data" + ", expected_limit_values, expected_skip_values", + [ + pytest.param( + { + "filter": { + "include": [ + { + "relation": "documents", + "scope": { + "where": {"title": "My Title"}, + "include": [{"relation": "instrument"}], + "limit": 50, + "skip": 20, + }, + }, + ], + }, + }, + "datasets", + 5, + [["documents"], [], ["documents.instrument"], [], []], + [[], ["documents.title", "eq", "My Title", "and"], [], [], []], + [None, None, None, 50, None], + [None, None, None, None, 20], + id="Simple case", + ), + pytest.param( + { + "filter": { + "include": [ + { + "relation": "documents", + "scope": { + "where": { + "and": [ + { + "and": [ + {"summary": "My Test Summary"}, + {"title": {"like": "Test title"}}, + ], + }, + { + "and": [ + {"pid": "Test pid"}, + {"type": {"eq": "Test type"}}, + ], + }, + { + "or": [ + {"doi": "Test doi"}, + { + "license": { + "like": "Test license", + }, + }, + ], + }, + ], + }, + "include": [ + { + "relation": "instrument", + "scope": { + "where": {"name": "Instrument 1"}, + "limit": 2, + }, + }, + ], + "limit": 50, + "skip": 20, + }, + }, + ], + }, + }, + "datasets", + 12, + [ + ["documents"], + [], + [], + [], + [], + [], + [], + ["documents.instrument"], + [], + [], + [], + [], + ], + [ + [], + ["documents.summary", "eq", "My Test Summary", "and"], + ["documents.title", "like", "Test title", "and"], + ["documents.pid", "eq", "Test pid", "and"], + ["documents.type", "eq", "Test type", "and"], + ["documents.doi", "eq", "Test doi", "or"], + ["documents.license", "like", "Test license", "or"], + [], + ["documents.instrument.name", "eq", "Instrument 1", "and"], + [], + [], + [], + ], + [None, None, None, None, None, None, None, None, None, 2, 50, None], + [None, None, None, None, None, None, None, None, None, None, None, 20], + id="Complex case", + ), + ], + ) + def test_valid_include_filter_with_all_filters_in_scope( + self, + test_request_filter, + test_entity_name, + expected_length, + expected_included_entities, + expected_where_filter_data, + expected_limit_values, + expected_skip_values, + ): + filters = SearchAPIQueryFilterFactory.get_query_filter( + test_request_filter, test_entity_name, + ) + + assert len(filters) == expected_length + + for ( + test_filter, + included_entities, + where_filter_data, + limit_value, + skip_value, + ) in zip( + filters, + expected_included_entities, + expected_where_filter_data, + expected_limit_values, + expected_skip_values, + ): + if isinstance(test_filter, SearchAPIIncludeFilter): + assert test_filter.included_filters == included_entities + if isinstance(test_filter, SearchAPIWhereFilter): + assert test_filter.field == where_filter_data[0] + assert test_filter.operation == where_filter_data[1] + assert test_filter.value == where_filter_data[2] + assert test_filter.boolean_operator == where_filter_data[3] + if isinstance(test_filter, SearchAPILimitFilter): + assert test_filter.limit_value == limit_value + if isinstance(test_filter, SearchAPISkipFilter): + assert test_filter.skip_value == skip_value @pytest.mark.parametrize( "test_request_filter, expected_limit_value", From 60c3bade2374a599720f300fa380e11ac4ca1209 Mon Sep 17 00:00:00 2001 From: Viktor Bozhinov Date: Wed, 1 Dec 2021 11:35:04 +0000 Subject: [PATCH 28/44] test: add test cases for filter input with all filters #259 --- .../test_search_api_query_filter_factory.py | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) diff --git a/test/search_api/test_search_api_query_filter_factory.py b/test/search_api/test_search_api_query_filter_factory.py index f4c832c0..3c0fa5bb 100644 --- a/test/search_api/test_search_api_query_filter_factory.py +++ b/test/search_api/test_search_api_query_filter_factory.py @@ -1951,6 +1951,129 @@ def test_valid_skip_filter( assert isinstance(filters[0], SearchAPISkipFilter) assert filters[0].skip_value == expected_skip_value + @pytest.mark.parametrize( + "test_request_filter, test_entity_name, expected_length" + ", expected_included_entities, expected_where_filter_data" + ", expected_limit_values, expected_skip_values", + [ + pytest.param( + { + "filter": { + "where": {"title": "My Title"}, + "include": [{"relation": "instrument"}], + "limit": 50, + "skip": 20, + }, + }, + "datasets", + 4, + [[], ["instrument"], [], []], + [["title", "eq", "My Title", "and"], [], [], []], + [None, None, 50, None], + [None, None, None, 20], + id="Simple case", + ), + pytest.param( + { + "filter": { + "where": { + "and": [ + { + "and": [ + {"summary": "My Test Summary"}, + {"title": {"like": "Test title"}}, + ], + }, + { + "and": [ + {"pid": "Test pid"}, + {"type": {"eq": "Test type"}}, + ], + }, + { + "or": [ + {"doi": "Test doi"}, + {"license": {"like": "Test license"}}, + ], + }, + ], + }, + "include": [ + { + "relation": "instrument", + "scope": { + "where": {"name": "Instrument 1"}, + "limit": 2, + }, + }, + ], + "limit": 50, + "skip": 20, + }, + }, + "datasets", + 11, + [[], [], [], [], [], [], ["instrument"], [], [], [], []], + [ + ["summary", "eq", "My Test Summary", "and"], + ["title", "like", "Test title", "and"], + ["pid", "eq", "Test pid", "and"], + ["type", "eq", "Test type", "and"], + ["doi", "eq", "Test doi", "or"], + ["license", "like", "Test license", "or"], + [], + ["instrument.name", "eq", "Instrument 1", "and"], + [], + [], + [], + ], + [None, None, None, None, None, None, None, None, 2, 50, None], + [None, None, None, None, None, None, None, None, None, None, 20], + id="Complex case", + ), + ], + ) + def test_valid_filter_input_with_all_filters( + self, + test_request_filter, + test_entity_name, + expected_length, + expected_included_entities, + expected_where_filter_data, + expected_limit_values, + expected_skip_values, + ): + filters = SearchAPIQueryFilterFactory.get_query_filter( + test_request_filter, test_entity_name, + ) + + assert len(filters) == expected_length + + for ( + test_filter, + included_entities, + where_filter_data, + limit_value, + skip_value, + ) in zip( + filters, + expected_included_entities, + expected_where_filter_data, + expected_limit_values, + expected_skip_values, + ): + if isinstance(test_filter, SearchAPIIncludeFilter): + assert test_filter.included_filters == included_entities + if isinstance(test_filter, SearchAPIWhereFilter): + assert test_filter.field == where_filter_data[0] + assert test_filter.operation == where_filter_data[1] + assert test_filter.value == where_filter_data[2] + assert test_filter.boolean_operator == where_filter_data[3] + if isinstance(test_filter, SearchAPILimitFilter): + assert test_filter.limit_value == limit_value + if isinstance(test_filter, SearchAPISkipFilter): + assert test_filter.skip_value == skip_value + @pytest.mark.parametrize( "test_request_filter", [ From 0c4f9ac194aea8b3393e203f6acf68de1ab53975 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 2 Dec 2021 19:12:29 +0000 Subject: [PATCH 29/44] move string conversion to `__str__()` #259 - This means when the input is of type `SearchAPIWhereFilter`, it won't execute Python ICAT code until the filter needs to be applied/processed --- datagateway_api/src/search_api/nested_where_filters.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/datagateway_api/src/search_api/nested_where_filters.py b/datagateway_api/src/search_api/nested_where_filters.py index 2c166e55..d31016f0 100644 --- a/datagateway_api/src/search_api/nested_where_filters.py +++ b/datagateway_api/src/search_api/nested_where_filters.py @@ -18,13 +18,12 @@ def __init__(self, lhs, rhs, joining_operator): :type joining_operator: :class:`str` """ - self.lhs = str(lhs) - self.rhs = str(rhs) - self.joining_operator = joining_operator + self.lhs = lhs + self.rhs = rhs def __str__(self): """ Join the condition on the left with the one on the right with the boolean operator """ - return f"({self.lhs} {self.joining_operator} {self.rhs})" + return f"({str(self.lhs)} {self.joining_operator} {str(self.rhs)})" From 211485817e91626d82a13002630572b04621db8d Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 2 Dec 2021 19:16:09 +0000 Subject: [PATCH 30/44] add initial usage of `NestedWhereFilters into query params factory class #259 --- .../src/search_api/query_filter_factory.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/datagateway_api/src/search_api/query_filter_factory.py b/datagateway_api/src/search_api/query_filter_factory.py index c071abc6..8549e064 100644 --- a/datagateway_api/src/search_api/query_filter_factory.py +++ b/datagateway_api/src/search_api/query_filter_factory.py @@ -8,6 +8,7 @@ SearchAPISkipFilter, SearchAPIWhereFilter, ) +from datagateway_api.src.search_api.nested_where_filters import NestedWhereFilters log = logging.getLogger() @@ -62,6 +63,8 @@ def get_where_filter(filter_input, entity_name): ): boolean_operator = list(filter_input.keys())[0] conditions = list(filter_input.values())[0] + test_where_filters = [] + for condition in conditions: # Could be nested AND/OR where_filter = { @@ -70,10 +73,19 @@ def get_where_filter(filter_input, entity_name): conditional_where_filters = SearchAPIQueryFilterFactory.get_query_filter( where_filter, entity_name, ) + # TODO - could just go to extend the function call? + test_where_filters.extend(conditional_where_filters) + + # for conditional_where_filter in conditional_where_filters: + # conditional_where_filter.boolean_operator = boolean_operator + # where_filters.extend(conditional_where_filters) - for conditional_where_filter in conditional_where_filters: - conditional_where_filter.boolean_operator = boolean_operator - where_filters.extend(conditional_where_filters) + print(f"test Cond where filters: {test_where_filters}") + + nested = NestedWhereFilters( + test_where_filters[:-1], test_where_filters[-1], boolean_operator, + ) + where_filters.append(nested) elif list(filter_input.keys())[0] == "text": # TODO - we might want to move this to the data # definitions at a later point From 490470e1a92c046d567600c30911ccad23ebe3e2 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 2 Dec 2021 19:19:57 +0000 Subject: [PATCH 31/44] add string representation function to `SearchAPIWhereFilter` #259 - To be used by `NestedWhereFilters` when LHS or RHS has an input of this type --- datagateway_api/src/search_api/filters.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/datagateway_api/src/search_api/filters.py b/datagateway_api/src/search_api/filters.py index 7601eb34..6fbed611 100644 --- a/datagateway_api/src/search_api/filters.py +++ b/datagateway_api/src/search_api/filters.py @@ -4,6 +4,8 @@ PythonICATSkipFilter, PythonICATWhereFilter, ) +from icat.query import Query +from icat.client import Client # TODO - Implement each of these filters for Search API, inheriting from the Python ICAT # versions @@ -17,6 +19,19 @@ def __init__(self, field, value, operation, boolean_operator="and"): def apply_filter(self, query): return super().apply_filter(query) + def __str__(self): + # TODO - replace with `SessionHandler.client` when that work is merged + client = Client("https://localhost.localdomain:8181", checkCert=False) + client.login("simple", {"username": "root", "password": "pw"}) + + # TODO - can't just hardcode investigation entity. Might need `icat_entity_name` + # to be passed into init + query = Query(client, "Investigation") + query.addConditions(self.create_filter()) + str_conds = query.get_conditions_as_str() + + return str_conds[0] + class SearchAPISkipFilter(PythonICATSkipFilter): def __init__(self, skip_value): From df323fecd78dc0d37c84176597401db882c9233f Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 2 Dec 2021 19:21:31 +0000 Subject: [PATCH 32/44] add `__repr__` functions for easier testing #259 --- datagateway_api/src/search_api/filters.py | 3 +++ datagateway_api/src/search_api/nested_where_filters.py | 3 +++ 2 files changed, 6 insertions(+) diff --git a/datagateway_api/src/search_api/filters.py b/datagateway_api/src/search_api/filters.py index 6fbed611..d1003378 100644 --- a/datagateway_api/src/search_api/filters.py +++ b/datagateway_api/src/search_api/filters.py @@ -32,6 +32,9 @@ def __str__(self): return str_conds[0] + def __repr__(self): + return f"Field: '{self.field}', Value: '{self.value}', Operation: '{self.operation}'" + class SearchAPISkipFilter(PythonICATSkipFilter): def __init__(self, skip_value): diff --git a/datagateway_api/src/search_api/nested_where_filters.py b/datagateway_api/src/search_api/nested_where_filters.py index d31016f0..1f1521a2 100644 --- a/datagateway_api/src/search_api/nested_where_filters.py +++ b/datagateway_api/src/search_api/nested_where_filters.py @@ -27,3 +27,6 @@ def __str__(self): operator """ return f"({str(self.lhs)} {self.joining_operator} {str(self.rhs)})" + + def __repr__(self): + return f"LHS: {repr(self.lhs)}, RHS: {repr(self.rhs)}, Operator: {repr(self.joining_operator)}" From ea5deb0bbeebe6955d9cf7a05d6fba2ff03d46fc Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 2 Dec 2021 19:26:42 +0000 Subject: [PATCH 33/44] add functionality to deal with an empty RHS #259 - Relevant when a filter input has a boolean operator but there's only one condition in that list. Makes the boolean operator redundant but is still an edge case we should support - Additional test cases have been added to cover this --- .../src/search_api/nested_where_filters.py | 17 ++++++++++++++++- test/search_api/test_nested_where_filters.py | 2 ++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/datagateway_api/src/search_api/nested_where_filters.py b/datagateway_api/src/search_api/nested_where_filters.py index 1f1521a2..fa822194 100644 --- a/datagateway_api/src/search_api/nested_where_filters.py +++ b/datagateway_api/src/search_api/nested_where_filters.py @@ -20,13 +20,28 @@ def __init__(self, lhs, rhs, joining_operator): self.lhs = lhs self.rhs = rhs + self.joining_operator = f" {joining_operator} " def __str__(self): """ Join the condition on the left with the one on the right with the boolean operator """ - return f"({str(self.lhs)} {self.joining_operator} {str(self.rhs)})" + boolean_algebra_list = [self.lhs, self.rhs] + try: + boolean_algebra_list.remove(None) + except ValueError: + # If neither side contains `None`, we should continue as normal + pass + + for i in range(len(boolean_algebra_list)): + # Convert inputs to JPQL-ready + boolean_algebra_list[i] = str(boolean_algebra_list[i]) + + # TODO - LHS and RHS need to be able to deal with a list of + # `SearchAPIWhereFilters` + + return f"({self.joining_operator.join(boolean_algebra_list)})" def __repr__(self): return f"LHS: {repr(self.lhs)}, RHS: {repr(self.rhs)}, Operator: {repr(self.joining_operator)}" diff --git a/test/search_api/test_nested_where_filters.py b/test/search_api/test_nested_where_filters.py index 37acbdfc..4e25ec06 100644 --- a/test/search_api/test_nested_where_filters.py +++ b/test/search_api/test_nested_where_filters.py @@ -8,6 +8,8 @@ class TestNestedWhereFilters: @pytest.mark.parametrize( "lhs, rhs, joining_operator, expected_where_clause", [ + pytest.param("A", None, "AND", "(A)", id="(A) w/ misc. AND"), + pytest.param("A", None, "OR", "(A)", id="(A) w/ misc. OR"), pytest.param("A", "B", "AND", "(A AND B)", id="(A AND B)"), pytest.param("A", "B", "OR", "(A OR B)", id="(A OR B)"), pytest.param( From 31c07abed5c35215f2e1a1de5d890270442a758f Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 2 Dec 2021 20:41:54 +0000 Subject: [PATCH 34/44] make LHS and RHS support lists of WHERE filters #259 - Relevant for request filter inputs that come with 3 or more - Test cases have been added to cover this as well as edge cases such as lists with a single filter in them --- .../src/search_api/nested_where_filters.py | 16 ++++---- test/search_api/test_nested_where_filters.py | 39 +++++++++++++++++++ 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/datagateway_api/src/search_api/nested_where_filters.py b/datagateway_api/src/search_api/nested_where_filters.py index fa822194..5fe1248e 100644 --- a/datagateway_api/src/search_api/nested_where_filters.py +++ b/datagateway_api/src/search_api/nested_where_filters.py @@ -18,6 +18,12 @@ def __init__(self, lhs, rhs, joining_operator): :type joining_operator: :class:`str` """ + # Ensure each side is in a list for consistency for string conversion + if not isinstance(lhs, list): + lhs = [lhs] + if not isinstance(rhs, list): + rhs = [rhs] + self.lhs = lhs self.rhs = rhs self.joining_operator = f" {joining_operator} " @@ -34,14 +40,10 @@ def __str__(self): # If neither side contains `None`, we should continue as normal pass - for i in range(len(boolean_algebra_list)): - # Convert inputs to JPQL-ready - boolean_algebra_list[i] = str(boolean_algebra_list[i]) - - # TODO - LHS and RHS need to be able to deal with a list of - # `SearchAPIWhereFilters` + # If either side contains a list of WHERE filter objects, flatten the conditions + conditions = [str(m) for n in (i for i in boolean_algebra_list) for m in n] - return f"({self.joining_operator.join(boolean_algebra_list)})" + return f"({self.joining_operator.join(conditions)})" def __repr__(self): return f"LHS: {repr(self.lhs)}, RHS: {repr(self.rhs)}, Operator: {repr(self.joining_operator)}" diff --git a/test/search_api/test_nested_where_filters.py b/test/search_api/test_nested_where_filters.py index 4e25ec06..c9ae6c64 100644 --- a/test/search_api/test_nested_where_filters.py +++ b/test/search_api/test_nested_where_filters.py @@ -80,11 +80,50 @@ def test_str_filters(self, lhs, rhs, joining_operator, expected_where_clause): "(o.name = 'test name' OR o.id = '3')", id="(o.name = 'test name' OR o.id = '3')", ), + pytest.param( + [SearchAPIWhereFilter("name", "test name", "eq", "and")], + SearchAPIWhereFilter("id", 3, "eq", "and"), + "OR", + "(o.name = 'test name' OR o.id = '3')", + id="Single filter list in LHS", + ), + pytest.param( + [SearchAPIWhereFilter("name", "test name", "eq", "and")], + [SearchAPIWhereFilter("id", 3, "eq", "and")], + "OR", + "(o.name = 'test name' OR o.id = '3')", + id="Single filter list in LHS and RHS", + ), + pytest.param( + [ + SearchAPIWhereFilter("name", "test name", "eq", "and"), + SearchAPIWhereFilter("id", 10, "lt"), + ], + [SearchAPIWhereFilter("id", 3, "gt", "and")], + "AND", + "(o.name = 'test name' AND o.id < '10' AND o.id > '3')", + id="Multiple filters on LHS", + ), + pytest.param( + [ + SearchAPIWhereFilter("name", "test name", "eq", "and"), + SearchAPIWhereFilter("id", 10, "lt"), + ], + [ + SearchAPIWhereFilter("id", 3, "gt", "and"), + SearchAPIWhereFilter("doi", "Test DOI", "like"), + ], + "AND", + "(o.name = 'test name' AND o.id < '10' AND o.id > '3')", + id="Multiple filters on LHS and RHS", + ), ], ) def test_search_api_filters( self, lhs, rhs, joining_operator, expected_where_clause, ): + # TODO - Is creating clients causing this to be slow? Test once session handler + # work merged test_nest = NestedWhereFilters(lhs, rhs, joining_operator) where_clause = str(test_nest) assert where_clause == expected_where_clause From 46cb5c3bd72cea6f34f9b62084d17275941d72a2 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Fri, 3 Dec 2021 10:43:05 +0000 Subject: [PATCH 35/44] test: fix tests for `NestedWhereFilters` #259 --- datagateway_api/src/search_api/nested_where_filters.py | 7 ++++--- test/search_api/test_nested_where_filters.py | 9 ++++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/datagateway_api/src/search_api/nested_where_filters.py b/datagateway_api/src/search_api/nested_where_filters.py index 5fe1248e..82b02f89 100644 --- a/datagateway_api/src/search_api/nested_where_filters.py +++ b/datagateway_api/src/search_api/nested_where_filters.py @@ -26,7 +26,7 @@ def __init__(self, lhs, rhs, joining_operator): self.lhs = lhs self.rhs = rhs - self.joining_operator = f" {joining_operator} " + self.joining_operator = joining_operator def __str__(self): """ @@ -35,15 +35,16 @@ def __str__(self): """ boolean_algebra_list = [self.lhs, self.rhs] try: - boolean_algebra_list.remove(None) + boolean_algebra_list.remove([None]) except ValueError: # If neither side contains `None`, we should continue as normal pass # If either side contains a list of WHERE filter objects, flatten the conditions conditions = [str(m) for n in (i for i in boolean_algebra_list) for m in n] + operator = f" {self.joining_operator} " - return f"({self.joining_operator.join(conditions)})" + return f"({operator.join(conditions)})" def __repr__(self): return f"LHS: {repr(self.lhs)}, RHS: {repr(self.rhs)}, Operator: {repr(self.joining_operator)}" diff --git a/test/search_api/test_nested_where_filters.py b/test/search_api/test_nested_where_filters.py index c9ae6c64..4746b389 100644 --- a/test/search_api/test_nested_where_filters.py +++ b/test/search_api/test_nested_where_filters.py @@ -8,8 +8,10 @@ class TestNestedWhereFilters: @pytest.mark.parametrize( "lhs, rhs, joining_operator, expected_where_clause", [ - pytest.param("A", None, "AND", "(A)", id="(A) w/ misc. AND"), - pytest.param("A", None, "OR", "(A)", id="(A) w/ misc. OR"), + pytest.param("A", None, "AND", "(A)", id="LHS (A) w/ misc. AND"), + pytest.param("A", None, "OR", "(A)", id="LHS (A) w/ misc. OR"), + pytest.param([], "A", "AND", "(A)", id="RHS (A) w/ misc. AND"), + pytest.param([], "A", "OR", "(A)", id="RHS (A) w/ misc. OR"), pytest.param("A", "B", "AND", "(A AND B)", id="(A AND B)"), pytest.param("A", "B", "OR", "(A OR B)", id="(A OR B)"), pytest.param( @@ -114,7 +116,8 @@ def test_str_filters(self, lhs, rhs, joining_operator, expected_where_clause): SearchAPIWhereFilter("doi", "Test DOI", "like"), ], "AND", - "(o.name = 'test name' AND o.id < '10' AND o.id > '3')", + "(o.name = 'test name' AND o.id < '10' AND o.id > '3' AND o.doi like" + " '%Test DOI%')", id="Multiple filters on LHS and RHS", ), ], From 2106e9960da40aca18a913e29d9c2c7c6f6512d9 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Fri, 3 Dec 2021 14:00:11 +0000 Subject: [PATCH 36/44] test: fix some query param tests #259 - It's a big ol' file this! --- .../test_search_api_query_filter_factory.py | 519 +++++++++++++++--- 1 file changed, 439 insertions(+), 80 deletions(-) diff --git a/test/search_api/test_search_api_query_filter_factory.py b/test/search_api/test_search_api_query_filter_factory.py index 3c0fa5bb..2e4f2923 100644 --- a/test/search_api/test_search_api_query_filter_factory.py +++ b/test/search_api/test_search_api_query_filter_factory.py @@ -7,6 +7,7 @@ SearchAPISkipFilter, SearchAPIWhereFilter, ) +from datagateway_api.src.search_api.nested_where_filters import NestedWhereFilters from datagateway_api.src.search_api.query_filter_factory import ( SearchAPIQueryFilterFactory, ) @@ -45,16 +46,18 @@ class TestSearchAPIQueryFilterFactory: ["eq"], ["Dataset 1"], ["or"], + # TODO id="Text operator on dataset", ), pytest.param( {"filter": {"where": {"text": "Instrument 1"}}}, "instrument", - 2, + 1, ["name", "facility"], ["eq", "eq"], ["Instrument 1", "Instrument 1"], ["or", "or"], + # TODO id="Text operator on instrument", ), pytest.param( @@ -99,7 +102,8 @@ def test_valid_where_filter( @pytest.mark.parametrize( "test_request_filter, test_entity_name, expected_length, expected_fields," - "expected_operations, expected_values, expected_boolean_operators", + "expected_operations, expected_values, expected_boolean_operators" + ", expected_lhs, expected_rhs, expected_joining_operator", [ pytest.param( {"filter": {"where": {"and": [{"summary": "My Test Summary"}]}}}, @@ -109,6 +113,9 @@ def test_valid_where_filter( ["eq"], ["My Test Summary"], ["and"], + [], + [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], + "and", id="Single condition, property value with no operator", ), pytest.param( @@ -123,11 +130,14 @@ def test_valid_where_filter( }, }, "documents", - 2, + 1, ["summary", "title"], ["eq", "eq"], ["My Test Summary", "Test title"], ["and", "and"], + [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], + [SearchAPIWhereFilter("title", "Test title", "eq")], + "and", id="Multiple conditions (two), property values with no operator", ), pytest.param( @@ -143,11 +153,17 @@ def test_valid_where_filter( }, }, "documents", - 3, + 1, ["summary", "title", "type"], ["eq", "eq", "eq"], ["My Test Summary", "Test title", "Test type"], ["and", "and", "and"], + [ + SearchAPIWhereFilter("summary", "My Test Summary", "eq"), + SearchAPIWhereFilter("title", "Test title", "eq"), + ], + [SearchAPIWhereFilter("type", "Test type", "eq")], + "and", id="Multiple conditions (three), property values with no operator", ), pytest.param( @@ -158,6 +174,9 @@ def test_valid_where_filter( ["lt"], [50], ["and"], + [], + [SearchAPIWhereFilter("value", 50, "lt")], + "and", id="Single condition, property value with operator", ), pytest.param( @@ -172,11 +191,14 @@ def test_valid_where_filter( }, }, "parameters", - 2, + 1, ["name", "value"], ["like", "gte"], ["Test name", 275], ["and", "and"], + [SearchAPIWhereFilter("name", "Test name", "like")], + [SearchAPIWhereFilter("value", 275, "gte")], + "and", id="Multiple conditions (two), property values with operator", ), pytest.param( @@ -192,11 +214,17 @@ def test_valid_where_filter( }, }, "parameters", - 3, + 1, ["name", "value", "unit"], ["like", "gte", "nlike"], ["Test name", 275, "Test unit"], ["and", "and", "and"], + [ + SearchAPIWhereFilter("name", "Test name", "like"), + SearchAPIWhereFilter("value", 275, "gte"), + ], + [SearchAPIWhereFilter("unit", "Test unit", "nlike")], + "and", id="Multiple conditions (three), property values with operator", ), pytest.param( @@ -207,6 +235,10 @@ def test_valid_where_filter( ["eq"], ["Dataset 1"], ["or"], + # TODO + "lhs", + "rhs", + "joining operator", id="Single condition, text operator on dataset", ), pytest.param( @@ -217,6 +249,10 @@ def test_valid_where_filter( ["eq", "eq"], ["Instrument 1", "Instrument 1"], ["or", "or"], + # TODO + "lhs", + "rhs", + "joining operator", id="Single condition, text operator on instrument", ), pytest.param( @@ -231,6 +267,10 @@ def test_valid_where_filter( ["eq", "eq"], ["Dataset 1", "Test pid"], ["or", "and"], + # TODO + "lhs", + "rhs", + "joining operator", id="Multiple conditions (two), text operator on dataset and " "property value with no operator", ), @@ -248,6 +288,10 @@ def test_valid_where_filter( ["eq", "eq", "eq"], ["Instrument 1", "Instrument 1", "Test pid"], ["or", "or", "and"], + # TODO + "lhs", + "rhs", + "joining operator", id="Multiple conditions (two), text operator on instrument and " "property value with no operator", ), @@ -268,6 +312,10 @@ def test_valid_where_filter( ["eq", "eq"], ["Dataset 1", "Test pid"], ["or", "and"], + # TODO + "lhs", + "rhs", + "joining operator", id="Multiple conditions (two), text operator on dataset and " "property value with operator", ), @@ -288,6 +336,10 @@ def test_valid_where_filter( ["eq", "eq", "eq"], ["Instrument 1", "Instrument 1", "Test pid"], ["or", "or", "and"], + # TODO + "lhs", + "rhs", + "joining operator", id="Multiple conditions (two), text operator on instrument and " "property value with operator", ), @@ -302,28 +354,29 @@ def test_valid_where_filter_with_and_boolean_operator( expected_operations, expected_values, expected_boolean_operators, + expected_lhs, + expected_rhs, + expected_joining_operator, ): + # TODO - Could test_entity_name just be hardcoded to the same entity? filters = SearchAPIQueryFilterFactory.get_query_filter( test_request_filter, test_entity_name, ) + # TODO - Will expected length always be 1? assert len(filters) == expected_length - for test_filter, field, operation, value, boolean_operator in zip( - filters, - expected_fields, - expected_operations, - expected_values, - expected_boolean_operators, - ): - assert isinstance(test_filter, SearchAPIWhereFilter) - assert test_filter.field == field - assert test_filter.operation == operation - assert test_filter.value == value - assert test_filter.boolean_operator == boolean_operator + assert isinstance(filters[0], NestedWhereFilters) + print(type(filters[0])) + print(f"LHS: {repr(filters[0].lhs)}, Type: {type(filters[0].lhs)}") + print(f"RHS: {repr(filters[0].rhs)}, Type: {type(filters[0].rhs)}") + assert repr(filters[0].lhs) == repr(expected_lhs) + assert repr(filters[0].rhs) == repr(expected_rhs) + assert filters[0].joining_operator == expected_joining_operator @pytest.mark.parametrize( "test_request_filter, test_entity_name, expected_length, expected_fields," - "expected_operations, expected_values, expected_boolean_operators", + "expected_operations, expected_values, expected_boolean_operators" + ", expected_lhs, expected_rhs, expected_joining_operator", [ pytest.param( {"filter": {"where": {"or": [{"summary": "My Test Summary"}]}}}, @@ -333,6 +386,9 @@ def test_valid_where_filter_with_and_boolean_operator( ["eq"], ["My Test Summary"], ["or"], + [], + [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], + "or", id="Single condition, property value with no operator", ), pytest.param( @@ -347,11 +403,15 @@ def test_valid_where_filter_with_and_boolean_operator( }, }, "documents", - 2, + 1, ["summary", "title"], ["eq", "eq"], ["My Test Summary", "Test title"], ["or", "or"], + # TODO + [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], + [SearchAPIWhereFilter("title", "Test title", "eq")], + "or", id="Multiple conditions (two), property values with no operator", ), pytest.param( @@ -367,11 +427,17 @@ def test_valid_where_filter_with_and_boolean_operator( }, }, "documents", - 3, + 1, ["summary", "title", "type"], ["eq", "eq", "eq"], ["My Test Summary", "Test title", "Test type"], ["or", "or", "or"], + [ + SearchAPIWhereFilter("summary", "My Test Summary", "eq"), + SearchAPIWhereFilter("title", "Test title", "eq"), + ], + [SearchAPIWhereFilter("type", "Test type", "eq")], + "or", id="Multiple conditions (three), property values with no operator", ), pytest.param( @@ -382,6 +448,9 @@ def test_valid_where_filter_with_and_boolean_operator( ["lt"], [50], ["or"], + [], + [SearchAPIWhereFilter("value", 50, "lt")], + "or", id="Single condition, property value with operator", ), pytest.param( @@ -396,11 +465,14 @@ def test_valid_where_filter_with_and_boolean_operator( }, }, "parameters", - 2, + 1, ["name", "value"], ["like", "gte"], ["Test name", 275], ["or", "or"], + [SearchAPIWhereFilter("name", "Test name", "like")], + [SearchAPIWhereFilter("value", 275, "gte")], + "or", id="Multiple conditions (two), property values with operator", ), pytest.param( @@ -416,11 +488,17 @@ def test_valid_where_filter_with_and_boolean_operator( }, }, "parameters", - 3, + 1, ["name", "value", "unit"], ["like", "gte", "nlike"], ["Test name", 275, "Test unit"], ["or", "or", "or"], + [ + SearchAPIWhereFilter("name", "Test name", "like"), + SearchAPIWhereFilter("value", 275, "gte"), + ], + [SearchAPIWhereFilter("unit", "Test unit", "nlike")], + "or", id="Multiple conditions (three), property values with operator", ), pytest.param( @@ -431,16 +509,24 @@ def test_valid_where_filter_with_and_boolean_operator( ["eq"], ["Dataset 1"], ["or"], + # TODO + "lhs", + "rhs", + "joining operator", id="Single condition, text operator on dataset", ), pytest.param( {"filter": {"where": {"or": [{"text": "Instrument 1"}]}}}, "instrument", - 2, + 1, ["name", "facility"], ["eq", "eq"], ["Instrument 1", "Instrument 1"], ["or", "or"], + # TODO + "lhs", + "rhs", + "joining operator", id="Single condition, text operator on instrument", ), pytest.param( @@ -450,11 +536,15 @@ def test_valid_where_filter_with_and_boolean_operator( }, }, "datasets", - 2, + 1, ["title", "pid"], ["eq", "eq"], ["Dataset 1", "Test pid"], ["or", "or"], + # TODO + "lhs", + "rhs", + "joining operator", id="Multiple conditions (two), text operator on dataset and " "property value with no operator", ), @@ -467,11 +557,15 @@ def test_valid_where_filter_with_and_boolean_operator( }, }, "instrument", - 3, + 1, ["name", "facility", "pid"], ["eq", "eq", "eq"], ["Instrument 1", "Instrument 1", "Test pid"], ["or", "or", "or"], + # TODO + "lhs", + "rhs", + "joining operator", id="Multiple conditions (two), text operator on instrument and " "property value with no operator", ), @@ -484,11 +578,15 @@ def test_valid_where_filter_with_and_boolean_operator( }, }, "datasets", - 2, + 1, ["title", "pid"], ["eq", "eq"], ["Dataset 1", "Test pid"], ["or", "or"], + # TODO + "lhs", + "rhs", + "joining operator", id="Multiple conditions (two), text operator on dataset and " "property value with operator", ), @@ -504,11 +602,15 @@ def test_valid_where_filter_with_and_boolean_operator( }, }, "instrument", - 3, + 1, ["name", "facility", "pid"], ["eq", "eq", "eq"], ["Instrument 1", "Instrument 1", "Test pid"], ["or", "or", "or"], + # TODO + "lhs", + "rhs", + "joining operator", id="Multiple conditions (two), text operator on instrument and " "property value with operator", ), @@ -523,28 +625,29 @@ def test_valid_where_filter_with_or_boolean_operator( expected_operations, expected_values, expected_boolean_operators, + expected_lhs, + expected_rhs, + expected_joining_operator, ): + # TODO - Could test_entity_name just be hardcoded to the same entity? filters = SearchAPIQueryFilterFactory.get_query_filter( test_request_filter, test_entity_name, ) + # TODO - Will expected length always be 1? assert len(filters) == expected_length - for test_filter, field, operation, value, boolean_operator in zip( - filters, - expected_fields, - expected_operations, - expected_values, - expected_boolean_operators, - ): - assert isinstance(test_filter, SearchAPIWhereFilter) - assert test_filter.field == field - assert test_filter.operation == operation - assert test_filter.value == value - assert test_filter.boolean_operator == boolean_operator + assert isinstance(filters[0], NestedWhereFilters) + print(type(filters[0])) + print(f"LHS: {repr(filters[0].lhs)}, Type: {type(filters[0].lhs)}") + print(f"RHS: {repr(filters[0].rhs)}, Type: {type(filters[0].rhs)}") + assert repr(filters[0].lhs) == repr(expected_lhs) + assert repr(filters[0].rhs) == repr(expected_rhs) + assert filters[0].joining_operator == expected_joining_operator @pytest.mark.parametrize( "test_request_filter, test_entity_name, expected_length, expected_fields," - "expected_operations, expected_values, expected_boolean_operators", + "expected_operations, expected_values, expected_boolean_operators" + ", expected_lhs, expected_rhs, expected_joining_operator", [ pytest.param( { @@ -568,11 +671,26 @@ def test_valid_where_filter_with_or_boolean_operator( }, }, "documents", - 4, + 1, ["summary", "title", "pid", "type"], ["eq", "like", "eq", "eq"], ["My Test Summary", "Test title", "Test pid", "Test type"], ["and", "and", "and", "and"], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], + [SearchAPIWhereFilter("title", "Test title", "like")], + "and", + ), + ], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + [SearchAPIWhereFilter("type", "Test type", "eq")], + "and", + ), + ], + "and", id="With two AND boolean operators", ), pytest.param( @@ -597,11 +715,26 @@ def test_valid_where_filter_with_or_boolean_operator( }, }, "documents", - 4, + 1, ["summary", "title", "pid", "type"], ["eq", "like", "eq", "eq"], ["My Test Summary", "Test title", "Test pid", "Test type"], ["and", "and", "or", "or"], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], + [SearchAPIWhereFilter("title", "Test title", "like")], + "and", + ), + ], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + [SearchAPIWhereFilter("type", "Test type", "eq")], + "or", + ), + ], + "and", id="With AND and OR boolean operators", ), pytest.param( @@ -626,11 +759,26 @@ def test_valid_where_filter_with_or_boolean_operator( }, }, "documents", - 4, + 1, ["summary", "title", "pid", "type"], ["eq", "like", "eq", "eq"], ["My Test Summary", "Test title", "Test pid", "Test type"], ["or", "or", "or", "or"], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], + [SearchAPIWhereFilter("title", "Test title", "like")], + "or", + ), + ], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + [SearchAPIWhereFilter("type", "Test type", "eq")], + "or", + ), + ], + "and", id="With two OR boolean operators", ), pytest.param( @@ -661,7 +809,7 @@ def test_valid_where_filter_with_or_boolean_operator( }, }, "documents", - 6, + 1, ["summary", "title", "pid", "type", "doi", "license"], ["eq", "like", "eq", "eq", "eq", "like"], [ @@ -673,6 +821,26 @@ def test_valid_where_filter_with_or_boolean_operator( "Test license", ], ["and", "and", "and", "and", "and", "and"], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], + [SearchAPIWhereFilter("title", "Test title", "like")], + "and", + ), + NestedWhereFilters( + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + [SearchAPIWhereFilter("type", "Test type", "eq")], + "and", + ), + ], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("doi", "Test doi", "eq")], + [SearchAPIWhereFilter("license", "Test license", "like")], + "and", + ), + ], + "and", id="With three AND boolean operators", ), pytest.param( @@ -703,7 +871,7 @@ def test_valid_where_filter_with_or_boolean_operator( }, }, "documents", - 6, + 1, ["summary", "title", "pid", "type", "doi", "license"], ["eq", "like", "eq", "eq", "eq", "like"], [ @@ -715,6 +883,26 @@ def test_valid_where_filter_with_or_boolean_operator( "Test license", ], ["and", "and", "and", "and", "or", "or"], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], + [SearchAPIWhereFilter("title", "Test title", "like")], + "and", + ), + NestedWhereFilters( + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + [SearchAPIWhereFilter("type", "Test type", "eq")], + "and", + ), + ], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("doi", "Test doi", "eq")], + [SearchAPIWhereFilter("license", "Test license", "like")], + "or", + ), + ], + "and", id="With two AND and one OR boolean operators", ), pytest.param( @@ -745,7 +933,7 @@ def test_valid_where_filter_with_or_boolean_operator( }, }, "documents", - 6, + 1, ["summary", "title", "pid", "type", "doi", "license"], ["eq", "like", "eq", "eq", "eq", "like"], [ @@ -757,6 +945,26 @@ def test_valid_where_filter_with_or_boolean_operator( "Test license", ], ["and", "and", "or", "or", "or", "or"], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], + [SearchAPIWhereFilter("title", "Test title", "like")], + "and", + ), + NestedWhereFilters( + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + [SearchAPIWhereFilter("type", "Test type", "eq")], + "or", + ), + ], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("doi", "Test doi", "eq")], + [SearchAPIWhereFilter("license", "Test license", "like")], + "or", + ), + ], + "and", id="With one AND and two OR boolean operators", ), pytest.param( @@ -787,7 +995,7 @@ def test_valid_where_filter_with_or_boolean_operator( }, }, "documents", - 6, + 1, ["summary", "title", "pid", "type", "doi", "license"], ["eq", "like", "eq", "eq", "eq", "like"], [ @@ -799,6 +1007,26 @@ def test_valid_where_filter_with_or_boolean_operator( "Test license", ], ["or", "or", "or", "or", "or", "or"], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], + [SearchAPIWhereFilter("title", "Test title", "like")], + "or", + ), + NestedWhereFilters( + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + [SearchAPIWhereFilter("type", "Test type", "eq")], + "or", + ), + ], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("doi", "Test doi", "eq")], + [SearchAPIWhereFilter("license", "Test license", "like")], + "or", + ), + ], + "and", id="With three OR boolean operators", ), ], @@ -812,28 +1040,29 @@ def test_valid_where_filter_with_nested_and_boolean_operator( expected_operations, expected_values, expected_boolean_operators, + expected_lhs, + expected_rhs, + expected_joining_operator, ): + # TODO - Could test_entity_name just be hardcoded to the same entity? filters = SearchAPIQueryFilterFactory.get_query_filter( test_request_filter, test_entity_name, ) + # TODO - Will expected length always be 1? assert len(filters) == expected_length - for test_filter, field, operation, value, boolean_operator in zip( - filters, - expected_fields, - expected_operations, - expected_values, - expected_boolean_operators, - ): - assert isinstance(test_filter, SearchAPIWhereFilter) - assert test_filter.field == field - assert test_filter.operation == operation - assert test_filter.value == value - assert test_filter.boolean_operator == boolean_operator + assert isinstance(filters[0], NestedWhereFilters) + print(type(filters[0])) + print(f"LHS: {repr(filters[0].lhs)}, Type: {type(filters[0].lhs)}") + print(f"RHS: {repr(filters[0].rhs)}, Type: {type(filters[0].rhs)}") + assert repr(filters[0].lhs) == repr(expected_lhs) + assert repr(filters[0].rhs) == repr(expected_rhs) + assert filters[0].joining_operator == expected_joining_operator @pytest.mark.parametrize( "test_request_filter, test_entity_name, expected_length, expected_fields," - "expected_operations, expected_values, expected_boolean_operators", + "expected_operations, expected_values, expected_boolean_operators" + ", expected_lhs, expected_rhs, expected_joining_operator", [ pytest.param( { @@ -857,11 +1086,26 @@ def test_valid_where_filter_with_nested_and_boolean_operator( }, }, "documents", - 4, + 1, ["summary", "title", "pid", "type"], ["eq", "like", "eq", "eq"], ["My Test Summary", "Test title", "Test pid", "Test type"], ["and", "and", "and", "and"], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], + [SearchAPIWhereFilter("title", "Test title", "like")], + "and", + ), + ], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + [SearchAPIWhereFilter("type", "Test type", "eq")], + "and", + ), + ], + "or", id="With two AND boolean operators", ), pytest.param( @@ -886,11 +1130,26 @@ def test_valid_where_filter_with_nested_and_boolean_operator( }, }, "documents", - 4, + 1, ["summary", "title", "pid", "type"], ["eq", "like", "eq", "eq"], ["My Test Summary", "Test title", "Test pid", "Test type"], ["and", "and", "or", "or"], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], + [SearchAPIWhereFilter("title", "Test title", "like")], + "and", + ), + ], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + [SearchAPIWhereFilter("type", "Test type", "eq")], + "or", + ), + ], + "or", id="With AND and OR boolean operators", ), pytest.param( @@ -915,11 +1174,26 @@ def test_valid_where_filter_with_nested_and_boolean_operator( }, }, "documents", - 4, + 1, ["summary", "title", "pid", "type"], ["eq", "like", "eq", "eq"], ["My Test Summary", "Test title", "Test pid", "Test type"], ["or", "or", "or", "or"], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], + [SearchAPIWhereFilter("title", "Test title", "like")], + "or", + ), + ], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + [SearchAPIWhereFilter("type", "Test type", "eq")], + "or", + ), + ], + "or", id="With two OR boolean operators", ), pytest.param( @@ -950,7 +1224,7 @@ def test_valid_where_filter_with_nested_and_boolean_operator( }, }, "documents", - 6, + 1, ["summary", "title", "pid", "type", "doi", "license"], ["eq", "like", "eq", "eq", "eq", "like"], [ @@ -962,6 +1236,26 @@ def test_valid_where_filter_with_nested_and_boolean_operator( "Test license", ], ["and", "and", "and", "and", "and", "and"], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], + [SearchAPIWhereFilter("title", "Test title", "like")], + "and", + ), + NestedWhereFilters( + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + [SearchAPIWhereFilter("type", "Test type", "eq")], + "and", + ), + ], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("doi", "Test doi", "eq")], + [SearchAPIWhereFilter("license", "Test license", "like")], + "and", + ), + ], + "or", id="With three AND boolean operators", ), pytest.param( @@ -992,7 +1286,7 @@ def test_valid_where_filter_with_nested_and_boolean_operator( }, }, "documents", - 6, + 1, ["summary", "title", "pid", "type", "doi", "license"], ["eq", "like", "eq", "eq", "eq", "like"], [ @@ -1004,6 +1298,26 @@ def test_valid_where_filter_with_nested_and_boolean_operator( "Test license", ], ["and", "and", "and", "and", "or", "or"], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], + [SearchAPIWhereFilter("title", "Test title", "like")], + "and", + ), + NestedWhereFilters( + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + [SearchAPIWhereFilter("type", "Test type", "eq")], + "and", + ), + ], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("doi", "Test doi", "eq")], + [SearchAPIWhereFilter("license", "Test license", "like")], + "or", + ), + ], + "or", id="With two AND and one OR boolean operators", ), pytest.param( @@ -1034,7 +1348,7 @@ def test_valid_where_filter_with_nested_and_boolean_operator( }, }, "documents", - 6, + 1, ["summary", "title", "pid", "type", "doi", "license"], ["eq", "like", "eq", "eq", "eq", "like"], [ @@ -1046,6 +1360,26 @@ def test_valid_where_filter_with_nested_and_boolean_operator( "Test license", ], ["and", "and", "or", "or", "or", "or"], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], + [SearchAPIWhereFilter("title", "Test title", "like")], + "and", + ), + NestedWhereFilters( + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + [SearchAPIWhereFilter("type", "Test type", "eq")], + "or", + ), + ], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("doi", "Test doi", "eq")], + [SearchAPIWhereFilter("license", "Test license", "like")], + "or", + ), + ], + "or", id="With one AND and two OR boolean operators", ), pytest.param( @@ -1076,7 +1410,7 @@ def test_valid_where_filter_with_nested_and_boolean_operator( }, }, "documents", - 6, + 1, ["summary", "title", "pid", "type", "doi", "license"], ["eq", "like", "eq", "eq", "eq", "like"], [ @@ -1088,6 +1422,26 @@ def test_valid_where_filter_with_nested_and_boolean_operator( "Test license", ], ["or", "or", "or", "or", "or", "or"], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], + [SearchAPIWhereFilter("title", "Test title", "like")], + "or", + ), + NestedWhereFilters( + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + [SearchAPIWhereFilter("type", "Test type", "eq")], + "or", + ), + ], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("doi", "Test doi", "eq")], + [SearchAPIWhereFilter("license", "Test license", "like")], + "or", + ), + ], + "or", id="With three OR boolean operators", ), ], @@ -1101,24 +1455,24 @@ def test_valid_where_filter_with_nested_or_boolean_operator( expected_operations, expected_values, expected_boolean_operators, + expected_lhs, + expected_rhs, + expected_joining_operator, ): + # TODO - Could test_entity_name just be hardcoded to the same entity? filters = SearchAPIQueryFilterFactory.get_query_filter( test_request_filter, test_entity_name, ) + # TODO - Will expected length always be 1? assert len(filters) == expected_length - for test_filter, field, operation, value, boolean_operator in zip( - filters, - expected_fields, - expected_operations, - expected_values, - expected_boolean_operators, - ): - assert isinstance(test_filter, SearchAPIWhereFilter) - assert test_filter.field == field - assert test_filter.operation == operation - assert test_filter.value == value - assert test_filter.boolean_operator == boolean_operator + assert isinstance(filters[0], NestedWhereFilters) + print(type(filters[0])) + print(f"LHS: {repr(filters[0].lhs)}, Type: {type(filters[0].lhs)}") + print(f"RHS: {repr(filters[0].rhs)}, Type: {type(filters[0].rhs)}") + assert repr(filters[0].lhs) == repr(expected_lhs) + assert repr(filters[0].rhs) == repr(expected_rhs) + assert filters[0].joining_operator == expected_joining_operator @pytest.mark.parametrize( "test_request_filter, test_entity_name, expected_length" @@ -1482,6 +1836,7 @@ def test_valid_include_filter_with_where_filter_in_scope( expected_included_entities, expected_where_filter_data, ): + # TODO filters = SearchAPIQueryFilterFactory.get_query_filter( test_request_filter, test_entity_name, ) @@ -1663,6 +2018,7 @@ def test_valid_include_filter_with_limit_filter_in_scope( expected_included_entities, expected_limit_values, ): + # TODO - do we need to support this?? filters = SearchAPIQueryFilterFactory.get_query_filter( test_request_filter, test_entity_name, ) @@ -1752,6 +2108,7 @@ def test_valid_include_filter_with_skip_filter_in_scope( expected_included_entities, expected_skip_values, ): + # TODO - do we need to support this?? filters = SearchAPIQueryFilterFactory.get_query_filter( test_request_filter, test_entity_name, ) @@ -1890,6 +2247,7 @@ def test_valid_include_filter_with_all_filters_in_scope( expected_limit_values, expected_skip_values, ): + # TODO - are we going to support limit in include? If not, probably remove test? filters = SearchAPIQueryFilterFactory.get_query_filter( test_request_filter, test_entity_name, ) @@ -2043,6 +2401,7 @@ def test_valid_filter_input_with_all_filters( expected_limit_values, expected_skip_values, ): + # TODO - remove limit from scope filters = SearchAPIQueryFilterFactory.get_query_filter( test_request_filter, test_entity_name, ) From 9e404ed15b5a0501e19406daf9b1fb606c33d867 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Fri, 3 Dec 2021 14:56:10 +0000 Subject: [PATCH 37/44] test: fix text operator tests for query params #259 --- .../test_search_api_query_filter_factory.py | 224 ++++++++++++------ 1 file changed, 148 insertions(+), 76 deletions(-) diff --git a/test/search_api/test_search_api_query_filter_factory.py b/test/search_api/test_search_api_query_filter_factory.py index 2e4f2923..e4d751c8 100644 --- a/test/search_api/test_search_api_query_filter_factory.py +++ b/test/search_api/test_search_api_query_filter_factory.py @@ -38,28 +38,6 @@ class TestSearchAPIQueryFilterFactory: ["and"], id="Property value with operator", ), - pytest.param( - {"filter": {"where": {"text": "Dataset 1"}}}, - "datasets", - 1, - ["title"], - ["eq"], - ["Dataset 1"], - ["or"], - # TODO - id="Text operator on dataset", - ), - pytest.param( - {"filter": {"where": {"text": "Instrument 1"}}}, - "instrument", - 1, - ["name", "facility"], - ["eq", "eq"], - ["Instrument 1", "Instrument 1"], - ["or", "or"], - # TODO - id="Text operator on instrument", - ), pytest.param( {"where": {"summary": {"like": "My Test Summary"}}}, "documents", @@ -100,6 +78,53 @@ def test_valid_where_filter( assert test_filter.value == value assert test_filter.boolean_operator == boolean_operator + @pytest.mark.parametrize( + "test_request_filter, test_entity_name, expected_length, expected_lhs" + ", expected_rhs, expected_joining_operator", + [ + pytest.param( + {"filter": {"where": {"text": "Dataset 1"}}}, + "datasets", + 1, + [], + [SearchAPIWhereFilter("title", "Dataset 1", "eq")], + "or", + id="Text operator on dataset", + ), + pytest.param( + {"filter": {"where": {"text": "Instrument 1"}}}, + "instrument", + 1, + [SearchAPIWhereFilter("name", "Instrument 1", "eq")], + [SearchAPIWhereFilter("facility", "Instrument 1", "eq")], + "or", + id="Text operator on instrument", + ), + ], + ) + def test_valid_where_filter_text_operator( + self, + test_request_filter, + test_entity_name, + expected_length, + expected_lhs, + expected_rhs, + expected_joining_operator, + ): + filters = SearchAPIQueryFilterFactory.get_query_filter( + test_request_filter, test_entity_name, + ) + + # TODO - Will expected length always be 1? + assert len(filters) == expected_length + assert isinstance(filters[0], NestedWhereFilters) + print(type(filters[0])) + print(f"LHS: {repr(filters[0].lhs)}, Type: {type(filters[0].lhs)}") + print(f"RHS: {repr(filters[0].rhs)}, Type: {type(filters[0].rhs)}") + assert repr(filters[0].lhs) == repr(expected_lhs) + assert repr(filters[0].rhs) == repr(expected_rhs) + assert filters[0].joining_operator == expected_joining_operator + @pytest.mark.parametrize( "test_request_filter, test_entity_name, expected_length, expected_fields," "expected_operations, expected_values, expected_boolean_operators" @@ -235,24 +260,32 @@ def test_valid_where_filter( ["eq"], ["Dataset 1"], ["or"], - # TODO - "lhs", - "rhs", - "joining operator", + [], + [ + NestedWhereFilters( + [], SearchAPIWhereFilter("title", "Dataset 1", "eq"), "or", + ), + ], + "and", id="Single condition, text operator on dataset", ), pytest.param( {"filter": {"where": {"and": [{"text": "Instrument 1"}]}}}, "instrument", - 2, + 1, ["name", "facility"], ["eq", "eq"], ["Instrument 1", "Instrument 1"], ["or", "or"], - # TODO - "lhs", - "rhs", - "joining operator", + [], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("name", "Instrument 1", "eq")], + [SearchAPIWhereFilter("facility", "Instrument 1", "eq")], + "or", + ), + ], + "and", id="Single condition, text operator on instrument", ), pytest.param( @@ -262,15 +295,18 @@ def test_valid_where_filter( }, }, "datasets", - 2, + 1, ["title", "pid"], ["eq", "eq"], ["Dataset 1", "Test pid"], ["or", "and"], - # TODO - "lhs", - "rhs", - "joining operator", + [ + NestedWhereFilters( + [], [SearchAPIWhereFilter("title", "Dataset 1", "eq")], "or", + ), + ], + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + "and", id="Multiple conditions (two), text operator on dataset and " "property value with no operator", ), @@ -283,15 +319,20 @@ def test_valid_where_filter( }, }, "instrument", - 3, + 1, ["name", "facility", "pid"], ["eq", "eq", "eq"], ["Instrument 1", "Instrument 1", "Test pid"], ["or", "or", "and"], - # TODO - "lhs", - "rhs", - "joining operator", + [ + NestedWhereFilters( + [SearchAPIWhereFilter("name", "Instrument 1", "eq")], + [SearchAPIWhereFilter("facility", "Instrument 1", "eq")], + "or", + ), + ], + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + "and", id="Multiple conditions (two), text operator on instrument and " "property value with no operator", ), @@ -307,15 +348,18 @@ def test_valid_where_filter( }, }, "datasets", - 2, + 1, ["title", "pid"], ["eq", "eq"], ["Dataset 1", "Test pid"], ["or", "and"], - # TODO - "lhs", - "rhs", - "joining operator", + [ + NestedWhereFilters( + [], [SearchAPIWhereFilter("title", "Dataset 1", "eq")], "or", + ), + ], + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + "and", id="Multiple conditions (two), text operator on dataset and " "property value with operator", ), @@ -331,15 +375,20 @@ def test_valid_where_filter( }, }, "instrument", - 3, + 1, ["name", "facility", "pid"], ["eq", "eq", "eq"], ["Instrument 1", "Instrument 1", "Test pid"], ["or", "or", "and"], - # TODO - "lhs", - "rhs", - "joining operator", + [ + NestedWhereFilters( + [SearchAPIWhereFilter("name", "Instrument 1", "eq")], + [SearchAPIWhereFilter("facility", "Instrument 1", "eq")], + "or", + ), + ], + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + "and", id="Multiple conditions (two), text operator on instrument and " "property value with operator", ), @@ -408,7 +457,6 @@ def test_valid_where_filter_with_and_boolean_operator( ["eq", "eq"], ["My Test Summary", "Test title"], ["or", "or"], - # TODO [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], [SearchAPIWhereFilter("title", "Test title", "eq")], "or", @@ -509,10 +557,13 @@ def test_valid_where_filter_with_and_boolean_operator( ["eq"], ["Dataset 1"], ["or"], - # TODO - "lhs", - "rhs", - "joining operator", + [], + [ + NestedWhereFilters( + [], SearchAPIWhereFilter("title", "Dataset 1", "eq"), "or", + ), + ], + "or", id="Single condition, text operator on dataset", ), pytest.param( @@ -523,10 +574,15 @@ def test_valid_where_filter_with_and_boolean_operator( ["eq", "eq"], ["Instrument 1", "Instrument 1"], ["or", "or"], - # TODO - "lhs", - "rhs", - "joining operator", + [], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("name", "Instrument 1", "eq")], + [SearchAPIWhereFilter("facility", "Instrument 1", "eq")], + "or", + ), + ], + "or", id="Single condition, text operator on instrument", ), pytest.param( @@ -541,10 +597,13 @@ def test_valid_where_filter_with_and_boolean_operator( ["eq", "eq"], ["Dataset 1", "Test pid"], ["or", "or"], - # TODO - "lhs", - "rhs", - "joining operator", + [ + NestedWhereFilters( + [], [SearchAPIWhereFilter("title", "Dataset 1", "eq")], "or", + ), + ], + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + "or", id="Multiple conditions (two), text operator on dataset and " "property value with no operator", ), @@ -562,10 +621,15 @@ def test_valid_where_filter_with_and_boolean_operator( ["eq", "eq", "eq"], ["Instrument 1", "Instrument 1", "Test pid"], ["or", "or", "or"], - # TODO - "lhs", - "rhs", - "joining operator", + [ + NestedWhereFilters( + [SearchAPIWhereFilter("name", "Instrument 1", "eq")], + [SearchAPIWhereFilter("facility", "Instrument 1", "eq")], + "or", + ), + ], + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + "or", id="Multiple conditions (two), text operator on instrument and " "property value with no operator", ), @@ -583,10 +647,13 @@ def test_valid_where_filter_with_and_boolean_operator( ["eq", "eq"], ["Dataset 1", "Test pid"], ["or", "or"], - # TODO - "lhs", - "rhs", - "joining operator", + [ + NestedWhereFilters( + [], [SearchAPIWhereFilter("title", "Dataset 1", "eq")], "or", + ), + ], + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + "or", id="Multiple conditions (two), text operator on dataset and " "property value with operator", ), @@ -607,10 +674,15 @@ def test_valid_where_filter_with_and_boolean_operator( ["eq", "eq", "eq"], ["Instrument 1", "Instrument 1", "Test pid"], ["or", "or", "or"], - # TODO - "lhs", - "rhs", - "joining operator", + [ + NestedWhereFilters( + [SearchAPIWhereFilter("name", "Instrument 1", "eq")], + [SearchAPIWhereFilter("facility", "Instrument 1", "eq")], + "or", + ), + ], + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + "or", id="Multiple conditions (two), text operator on instrument and " "property value with operator", ), From 1abaf6689e8f96af598f408d8106eeb3813390f0 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Fri, 3 Dec 2021 15:17:22 +0000 Subject: [PATCH 38/44] test: refactor `TestSearchAPIQueryFilterFactory` #259 --- .../test_search_api_query_filter_factory.py | 372 ++---------------- 1 file changed, 22 insertions(+), 350 deletions(-) diff --git a/test/search_api/test_search_api_query_filter_factory.py b/test/search_api/test_search_api_query_filter_factory.py index e4d751c8..fc9a872e 100644 --- a/test/search_api/test_search_api_query_filter_factory.py +++ b/test/search_api/test_search_api_query_filter_factory.py @@ -15,77 +15,45 @@ class TestSearchAPIQueryFilterFactory: @pytest.mark.parametrize( - "test_request_filter, test_entity_name, expected_length, expected_fields," - "expected_operations, expected_values, expected_boolean_operators", + "test_request_filter, test_entity_name, expected_where", [ pytest.param( {"filter": {"where": {"title": "My Title"}}}, "documents", - 1, - ["title"], - ["eq"], - ["My Title"], - ["and"], + SearchAPIWhereFilter("title", "My Title", "eq"), id="Property value with no operator", ), pytest.param( {"filter": {"where": {"summary": {"like": "My Test Summary"}}}}, "documents", - 1, - ["summary"], - ["like"], - ["My Test Summary"], - ["and"], + SearchAPIWhereFilter("summary", "My Test Summary", "like"), id="Property value with operator", ), pytest.param( {"where": {"summary": {"like": "My Test Summary"}}}, "documents", - 1, - ["summary"], - ["like"], - ["My Test Summary"], - ["and"], + SearchAPIWhereFilter("summary", "My Test Summary", "like"), id="WHERE filter in syntax for count endpoints", ), ], ) def test_valid_where_filter( - self, - test_request_filter, - test_entity_name, - expected_length, - expected_fields, - expected_operations, - expected_values, - expected_boolean_operators, + self, test_request_filter, test_entity_name, expected_where, ): filters = SearchAPIQueryFilterFactory.get_query_filter( test_request_filter, test_entity_name, ) - assert len(filters) == expected_length - for test_filter, field, operation, value, boolean_operator in zip( - filters, - expected_fields, - expected_operations, - expected_values, - expected_boolean_operators, - ): - assert isinstance(test_filter, SearchAPIWhereFilter) - assert test_filter.field == field - assert test_filter.operation == operation - assert test_filter.value == value - assert test_filter.boolean_operator == boolean_operator + assert len(filters) == 1 + assert repr(filters[0]) == repr(expected_where) @pytest.mark.parametrize( - "test_request_filter, test_entity_name, expected_length, expected_lhs" - ", expected_rhs, expected_joining_operator", + "test_request_filter, test_entity_name, expected_lhs, expected_rhs" + ", expected_joining_operator", [ pytest.param( {"filter": {"where": {"text": "Dataset 1"}}}, "datasets", - 1, [], [SearchAPIWhereFilter("title", "Dataset 1", "eq")], "or", @@ -94,7 +62,6 @@ def test_valid_where_filter( pytest.param( {"filter": {"where": {"text": "Instrument 1"}}}, "instrument", - 1, [SearchAPIWhereFilter("name", "Instrument 1", "eq")], [SearchAPIWhereFilter("facility", "Instrument 1", "eq")], "or", @@ -106,7 +73,6 @@ def test_valid_where_filter_text_operator( self, test_request_filter, test_entity_name, - expected_length, expected_lhs, expected_rhs, expected_joining_operator, @@ -115,29 +81,19 @@ def test_valid_where_filter_text_operator( test_request_filter, test_entity_name, ) - # TODO - Will expected length always be 1? - assert len(filters) == expected_length + assert len(filters) == 1 assert isinstance(filters[0], NestedWhereFilters) - print(type(filters[0])) - print(f"LHS: {repr(filters[0].lhs)}, Type: {type(filters[0].lhs)}") - print(f"RHS: {repr(filters[0].rhs)}, Type: {type(filters[0].rhs)}") assert repr(filters[0].lhs) == repr(expected_lhs) assert repr(filters[0].rhs) == repr(expected_rhs) assert filters[0].joining_operator == expected_joining_operator @pytest.mark.parametrize( - "test_request_filter, test_entity_name, expected_length, expected_fields," - "expected_operations, expected_values, expected_boolean_operators" - ", expected_lhs, expected_rhs, expected_joining_operator", + "test_request_filter, test_entity_name, expected_lhs, expected_rhs" + ", expected_joining_operator", [ pytest.param( {"filter": {"where": {"and": [{"summary": "My Test Summary"}]}}}, "documents", - 1, - ["summary"], - ["eq"], - ["My Test Summary"], - ["and"], [], [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], "and", @@ -155,11 +111,6 @@ def test_valid_where_filter_text_operator( }, }, "documents", - 1, - ["summary", "title"], - ["eq", "eq"], - ["My Test Summary", "Test title"], - ["and", "and"], [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], [SearchAPIWhereFilter("title", "Test title", "eq")], "and", @@ -178,11 +129,6 @@ def test_valid_where_filter_text_operator( }, }, "documents", - 1, - ["summary", "title", "type"], - ["eq", "eq", "eq"], - ["My Test Summary", "Test title", "Test type"], - ["and", "and", "and"], [ SearchAPIWhereFilter("summary", "My Test Summary", "eq"), SearchAPIWhereFilter("title", "Test title", "eq"), @@ -194,11 +140,6 @@ def test_valid_where_filter_text_operator( pytest.param( {"filter": {"where": {"and": [{"value": {"lt": 50}}]}}}, "parameters", - 1, - ["value"], - ["lt"], - [50], - ["and"], [], [SearchAPIWhereFilter("value", 50, "lt")], "and", @@ -216,11 +157,6 @@ def test_valid_where_filter_text_operator( }, }, "parameters", - 1, - ["name", "value"], - ["like", "gte"], - ["Test name", 275], - ["and", "and"], [SearchAPIWhereFilter("name", "Test name", "like")], [SearchAPIWhereFilter("value", 275, "gte")], "and", @@ -239,11 +175,6 @@ def test_valid_where_filter_text_operator( }, }, "parameters", - 1, - ["name", "value", "unit"], - ["like", "gte", "nlike"], - ["Test name", 275, "Test unit"], - ["and", "and", "and"], [ SearchAPIWhereFilter("name", "Test name", "like"), SearchAPIWhereFilter("value", 275, "gte"), @@ -255,11 +186,6 @@ def test_valid_where_filter_text_operator( pytest.param( {"filter": {"where": {"and": [{"text": "Dataset 1"}]}}}, "datasets", - 1, - ["title"], - ["eq"], - ["Dataset 1"], - ["or"], [], [ NestedWhereFilters( @@ -272,11 +198,6 @@ def test_valid_where_filter_text_operator( pytest.param( {"filter": {"where": {"and": [{"text": "Instrument 1"}]}}}, "instrument", - 1, - ["name", "facility"], - ["eq", "eq"], - ["Instrument 1", "Instrument 1"], - ["or", "or"], [], [ NestedWhereFilters( @@ -295,11 +216,6 @@ def test_valid_where_filter_text_operator( }, }, "datasets", - 1, - ["title", "pid"], - ["eq", "eq"], - ["Dataset 1", "Test pid"], - ["or", "and"], [ NestedWhereFilters( [], [SearchAPIWhereFilter("title", "Dataset 1", "eq")], "or", @@ -319,11 +235,6 @@ def test_valid_where_filter_text_operator( }, }, "instrument", - 1, - ["name", "facility", "pid"], - ["eq", "eq", "eq"], - ["Instrument 1", "Instrument 1", "Test pid"], - ["or", "or", "and"], [ NestedWhereFilters( [SearchAPIWhereFilter("name", "Instrument 1", "eq")], @@ -348,11 +259,6 @@ def test_valid_where_filter_text_operator( }, }, "datasets", - 1, - ["title", "pid"], - ["eq", "eq"], - ["Dataset 1", "Test pid"], - ["or", "and"], [ NestedWhereFilters( [], [SearchAPIWhereFilter("title", "Dataset 1", "eq")], "or", @@ -375,11 +281,6 @@ def test_valid_where_filter_text_operator( }, }, "instrument", - 1, - ["name", "facility", "pid"], - ["eq", "eq", "eq"], - ["Instrument 1", "Instrument 1", "Test pid"], - ["or", "or", "and"], [ NestedWhereFilters( [SearchAPIWhereFilter("name", "Instrument 1", "eq")], @@ -398,43 +299,27 @@ def test_valid_where_filter_with_and_boolean_operator( self, test_request_filter, test_entity_name, - expected_length, - expected_fields, - expected_operations, - expected_values, - expected_boolean_operators, expected_lhs, expected_rhs, expected_joining_operator, ): - # TODO - Could test_entity_name just be hardcoded to the same entity? filters = SearchAPIQueryFilterFactory.get_query_filter( test_request_filter, test_entity_name, ) - # TODO - Will expected length always be 1? - assert len(filters) == expected_length + assert len(filters) == 1 assert isinstance(filters[0], NestedWhereFilters) - print(type(filters[0])) - print(f"LHS: {repr(filters[0].lhs)}, Type: {type(filters[0].lhs)}") - print(f"RHS: {repr(filters[0].rhs)}, Type: {type(filters[0].rhs)}") assert repr(filters[0].lhs) == repr(expected_lhs) assert repr(filters[0].rhs) == repr(expected_rhs) assert filters[0].joining_operator == expected_joining_operator @pytest.mark.parametrize( - "test_request_filter, test_entity_name, expected_length, expected_fields," - "expected_operations, expected_values, expected_boolean_operators" - ", expected_lhs, expected_rhs, expected_joining_operator", + "test_request_filter, test_entity_name, expected_lhs, expected_rhs" + ", expected_joining_operator", [ pytest.param( {"filter": {"where": {"or": [{"summary": "My Test Summary"}]}}}, "documents", - 1, - ["summary"], - ["eq"], - ["My Test Summary"], - ["or"], [], [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], "or", @@ -452,11 +337,6 @@ def test_valid_where_filter_with_and_boolean_operator( }, }, "documents", - 1, - ["summary", "title"], - ["eq", "eq"], - ["My Test Summary", "Test title"], - ["or", "or"], [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], [SearchAPIWhereFilter("title", "Test title", "eq")], "or", @@ -475,11 +355,6 @@ def test_valid_where_filter_with_and_boolean_operator( }, }, "documents", - 1, - ["summary", "title", "type"], - ["eq", "eq", "eq"], - ["My Test Summary", "Test title", "Test type"], - ["or", "or", "or"], [ SearchAPIWhereFilter("summary", "My Test Summary", "eq"), SearchAPIWhereFilter("title", "Test title", "eq"), @@ -491,11 +366,6 @@ def test_valid_where_filter_with_and_boolean_operator( pytest.param( {"filter": {"where": {"or": [{"value": {"lt": 50}}]}}}, "parameters", - 1, - ["value"], - ["lt"], - [50], - ["or"], [], [SearchAPIWhereFilter("value", 50, "lt")], "or", @@ -513,11 +383,6 @@ def test_valid_where_filter_with_and_boolean_operator( }, }, "parameters", - 1, - ["name", "value"], - ["like", "gte"], - ["Test name", 275], - ["or", "or"], [SearchAPIWhereFilter("name", "Test name", "like")], [SearchAPIWhereFilter("value", 275, "gte")], "or", @@ -536,11 +401,6 @@ def test_valid_where_filter_with_and_boolean_operator( }, }, "parameters", - 1, - ["name", "value", "unit"], - ["like", "gte", "nlike"], - ["Test name", 275, "Test unit"], - ["or", "or", "or"], [ SearchAPIWhereFilter("name", "Test name", "like"), SearchAPIWhereFilter("value", 275, "gte"), @@ -552,11 +412,6 @@ def test_valid_where_filter_with_and_boolean_operator( pytest.param( {"filter": {"where": {"or": [{"text": "Dataset 1"}]}}}, "datasets", - 1, - ["title"], - ["eq"], - ["Dataset 1"], - ["or"], [], [ NestedWhereFilters( @@ -569,11 +424,6 @@ def test_valid_where_filter_with_and_boolean_operator( pytest.param( {"filter": {"where": {"or": [{"text": "Instrument 1"}]}}}, "instrument", - 1, - ["name", "facility"], - ["eq", "eq"], - ["Instrument 1", "Instrument 1"], - ["or", "or"], [], [ NestedWhereFilters( @@ -592,11 +442,6 @@ def test_valid_where_filter_with_and_boolean_operator( }, }, "datasets", - 1, - ["title", "pid"], - ["eq", "eq"], - ["Dataset 1", "Test pid"], - ["or", "or"], [ NestedWhereFilters( [], [SearchAPIWhereFilter("title", "Dataset 1", "eq")], "or", @@ -616,11 +461,6 @@ def test_valid_where_filter_with_and_boolean_operator( }, }, "instrument", - 1, - ["name", "facility", "pid"], - ["eq", "eq", "eq"], - ["Instrument 1", "Instrument 1", "Test pid"], - ["or", "or", "or"], [ NestedWhereFilters( [SearchAPIWhereFilter("name", "Instrument 1", "eq")], @@ -642,11 +482,6 @@ def test_valid_where_filter_with_and_boolean_operator( }, }, "datasets", - 1, - ["title", "pid"], - ["eq", "eq"], - ["Dataset 1", "Test pid"], - ["or", "or"], [ NestedWhereFilters( [], [SearchAPIWhereFilter("title", "Dataset 1", "eq")], "or", @@ -669,11 +504,6 @@ def test_valid_where_filter_with_and_boolean_operator( }, }, "instrument", - 1, - ["name", "facility", "pid"], - ["eq", "eq", "eq"], - ["Instrument 1", "Instrument 1", "Test pid"], - ["or", "or", "or"], [ NestedWhereFilters( [SearchAPIWhereFilter("name", "Instrument 1", "eq")], @@ -692,34 +522,23 @@ def test_valid_where_filter_with_or_boolean_operator( self, test_request_filter, test_entity_name, - expected_length, - expected_fields, - expected_operations, - expected_values, - expected_boolean_operators, expected_lhs, expected_rhs, expected_joining_operator, ): - # TODO - Could test_entity_name just be hardcoded to the same entity? filters = SearchAPIQueryFilterFactory.get_query_filter( test_request_filter, test_entity_name, ) - # TODO - Will expected length always be 1? - assert len(filters) == expected_length + assert len(filters) == 1 assert isinstance(filters[0], NestedWhereFilters) - print(type(filters[0])) - print(f"LHS: {repr(filters[0].lhs)}, Type: {type(filters[0].lhs)}") - print(f"RHS: {repr(filters[0].rhs)}, Type: {type(filters[0].rhs)}") assert repr(filters[0].lhs) == repr(expected_lhs) assert repr(filters[0].rhs) == repr(expected_rhs) assert filters[0].joining_operator == expected_joining_operator @pytest.mark.parametrize( - "test_request_filter, test_entity_name, expected_length, expected_fields," - "expected_operations, expected_values, expected_boolean_operators" - ", expected_lhs, expected_rhs, expected_joining_operator", + "test_request_filter, test_entity_name, expected_lhs, expected_rhs" + ", expected_joining_operator", [ pytest.param( { @@ -743,11 +562,6 @@ def test_valid_where_filter_with_or_boolean_operator( }, }, "documents", - 1, - ["summary", "title", "pid", "type"], - ["eq", "like", "eq", "eq"], - ["My Test Summary", "Test title", "Test pid", "Test type"], - ["and", "and", "and", "and"], [ NestedWhereFilters( [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], @@ -787,11 +601,6 @@ def test_valid_where_filter_with_or_boolean_operator( }, }, "documents", - 1, - ["summary", "title", "pid", "type"], - ["eq", "like", "eq", "eq"], - ["My Test Summary", "Test title", "Test pid", "Test type"], - ["and", "and", "or", "or"], [ NestedWhereFilters( [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], @@ -831,11 +640,6 @@ def test_valid_where_filter_with_or_boolean_operator( }, }, "documents", - 1, - ["summary", "title", "pid", "type"], - ["eq", "like", "eq", "eq"], - ["My Test Summary", "Test title", "Test pid", "Test type"], - ["or", "or", "or", "or"], [ NestedWhereFilters( [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], @@ -881,18 +685,6 @@ def test_valid_where_filter_with_or_boolean_operator( }, }, "documents", - 1, - ["summary", "title", "pid", "type", "doi", "license"], - ["eq", "like", "eq", "eq", "eq", "like"], - [ - "My Test Summary", - "Test title", - "Test pid", - "Test type", - "Test doi", - "Test license", - ], - ["and", "and", "and", "and", "and", "and"], [ NestedWhereFilters( [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], @@ -943,18 +735,6 @@ def test_valid_where_filter_with_or_boolean_operator( }, }, "documents", - 1, - ["summary", "title", "pid", "type", "doi", "license"], - ["eq", "like", "eq", "eq", "eq", "like"], - [ - "My Test Summary", - "Test title", - "Test pid", - "Test type", - "Test doi", - "Test license", - ], - ["and", "and", "and", "and", "or", "or"], [ NestedWhereFilters( [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], @@ -1005,18 +785,6 @@ def test_valid_where_filter_with_or_boolean_operator( }, }, "documents", - 1, - ["summary", "title", "pid", "type", "doi", "license"], - ["eq", "like", "eq", "eq", "eq", "like"], - [ - "My Test Summary", - "Test title", - "Test pid", - "Test type", - "Test doi", - "Test license", - ], - ["and", "and", "or", "or", "or", "or"], [ NestedWhereFilters( [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], @@ -1067,18 +835,6 @@ def test_valid_where_filter_with_or_boolean_operator( }, }, "documents", - 1, - ["summary", "title", "pid", "type", "doi", "license"], - ["eq", "like", "eq", "eq", "eq", "like"], - [ - "My Test Summary", - "Test title", - "Test pid", - "Test type", - "Test doi", - "Test license", - ], - ["or", "or", "or", "or", "or", "or"], [ NestedWhereFilters( [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], @@ -1107,34 +863,23 @@ def test_valid_where_filter_with_nested_and_boolean_operator( self, test_request_filter, test_entity_name, - expected_length, - expected_fields, - expected_operations, - expected_values, - expected_boolean_operators, expected_lhs, expected_rhs, expected_joining_operator, ): - # TODO - Could test_entity_name just be hardcoded to the same entity? filters = SearchAPIQueryFilterFactory.get_query_filter( test_request_filter, test_entity_name, ) - # TODO - Will expected length always be 1? - assert len(filters) == expected_length + assert len(filters) == 1 assert isinstance(filters[0], NestedWhereFilters) - print(type(filters[0])) - print(f"LHS: {repr(filters[0].lhs)}, Type: {type(filters[0].lhs)}") - print(f"RHS: {repr(filters[0].rhs)}, Type: {type(filters[0].rhs)}") assert repr(filters[0].lhs) == repr(expected_lhs) assert repr(filters[0].rhs) == repr(expected_rhs) assert filters[0].joining_operator == expected_joining_operator @pytest.mark.parametrize( - "test_request_filter, test_entity_name, expected_length, expected_fields," - "expected_operations, expected_values, expected_boolean_operators" - ", expected_lhs, expected_rhs, expected_joining_operator", + "test_request_filter, test_entity_name, expected_lhs, expected_rhs" + ", expected_joining_operator", [ pytest.param( { @@ -1158,11 +903,6 @@ def test_valid_where_filter_with_nested_and_boolean_operator( }, }, "documents", - 1, - ["summary", "title", "pid", "type"], - ["eq", "like", "eq", "eq"], - ["My Test Summary", "Test title", "Test pid", "Test type"], - ["and", "and", "and", "and"], [ NestedWhereFilters( [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], @@ -1202,11 +942,6 @@ def test_valid_where_filter_with_nested_and_boolean_operator( }, }, "documents", - 1, - ["summary", "title", "pid", "type"], - ["eq", "like", "eq", "eq"], - ["My Test Summary", "Test title", "Test pid", "Test type"], - ["and", "and", "or", "or"], [ NestedWhereFilters( [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], @@ -1246,11 +981,6 @@ def test_valid_where_filter_with_nested_and_boolean_operator( }, }, "documents", - 1, - ["summary", "title", "pid", "type"], - ["eq", "like", "eq", "eq"], - ["My Test Summary", "Test title", "Test pid", "Test type"], - ["or", "or", "or", "or"], [ NestedWhereFilters( [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], @@ -1296,18 +1026,6 @@ def test_valid_where_filter_with_nested_and_boolean_operator( }, }, "documents", - 1, - ["summary", "title", "pid", "type", "doi", "license"], - ["eq", "like", "eq", "eq", "eq", "like"], - [ - "My Test Summary", - "Test title", - "Test pid", - "Test type", - "Test doi", - "Test license", - ], - ["and", "and", "and", "and", "and", "and"], [ NestedWhereFilters( [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], @@ -1358,18 +1076,6 @@ def test_valid_where_filter_with_nested_and_boolean_operator( }, }, "documents", - 1, - ["summary", "title", "pid", "type", "doi", "license"], - ["eq", "like", "eq", "eq", "eq", "like"], - [ - "My Test Summary", - "Test title", - "Test pid", - "Test type", - "Test doi", - "Test license", - ], - ["and", "and", "and", "and", "or", "or"], [ NestedWhereFilters( [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], @@ -1420,18 +1126,6 @@ def test_valid_where_filter_with_nested_and_boolean_operator( }, }, "documents", - 1, - ["summary", "title", "pid", "type", "doi", "license"], - ["eq", "like", "eq", "eq", "eq", "like"], - [ - "My Test Summary", - "Test title", - "Test pid", - "Test type", - "Test doi", - "Test license", - ], - ["and", "and", "or", "or", "or", "or"], [ NestedWhereFilters( [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], @@ -1482,18 +1176,6 @@ def test_valid_where_filter_with_nested_and_boolean_operator( }, }, "documents", - 1, - ["summary", "title", "pid", "type", "doi", "license"], - ["eq", "like", "eq", "eq", "eq", "like"], - [ - "My Test Summary", - "Test title", - "Test pid", - "Test type", - "Test doi", - "Test license", - ], - ["or", "or", "or", "or", "or", "or"], [ NestedWhereFilters( [SearchAPIWhereFilter("summary", "My Test Summary", "eq")], @@ -1522,26 +1204,16 @@ def test_valid_where_filter_with_nested_or_boolean_operator( self, test_request_filter, test_entity_name, - expected_length, - expected_fields, - expected_operations, - expected_values, - expected_boolean_operators, expected_lhs, expected_rhs, expected_joining_operator, ): - # TODO - Could test_entity_name just be hardcoded to the same entity? filters = SearchAPIQueryFilterFactory.get_query_filter( test_request_filter, test_entity_name, ) - # TODO - Will expected length always be 1? - assert len(filters) == expected_length + assert len(filters) == 1 assert isinstance(filters[0], NestedWhereFilters) - print(type(filters[0])) - print(f"LHS: {repr(filters[0].lhs)}, Type: {type(filters[0].lhs)}") - print(f"RHS: {repr(filters[0].rhs)}, Type: {type(filters[0].rhs)}") assert repr(filters[0].lhs) == repr(expected_lhs) assert repr(filters[0].rhs) == repr(expected_rhs) assert filters[0].joining_operator == expected_joining_operator From 38d17a53cd7e58e72dbe1d4c1b9804331e2f5c51 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Fri, 3 Dec 2021 15:35:22 +0000 Subject: [PATCH 39/44] test: remove test cases on things we're not going to support #259 --- .../test_search_api_query_filter_factory.py | 341 +----------------- 1 file changed, 1 insertion(+), 340 deletions(-) diff --git a/test/search_api/test_search_api_query_filter_factory.py b/test/search_api/test_search_api_query_filter_factory.py index fc9a872e..3f0daf3a 100644 --- a/test/search_api/test_search_api_query_filter_factory.py +++ b/test/search_api/test_search_api_query_filter_factory.py @@ -1687,342 +1687,6 @@ def test_valid_include_filter_with_include_filter_in_scope( if isinstance(test_filter, SearchAPIIncludeFilter): assert test_filter.included_filters == included_entities - @pytest.mark.parametrize( - "test_request_filter, test_entity_name, expected_length" - ", expected_included_entities, expected_limit_values", - [ - pytest.param( - { - "filter": { - "include": [{"relation": "datasets", "scope": {"limit": 50}}], - }, - }, - "documents", - 2, - [["datasets"], []], - [None, 50], - id="Single related model", - ), - pytest.param( - { - "filter": { - "include": [ - {"relation": "datasets", "scope": {"limit": 50}}, - {"relation": "parameters", "scope": {"limit": 20}}, - ], - }, - }, - "documents", - 4, - [["datasets"], [], ["parameters"], []], - [None, 50, None, 20], - id="Multiple related model", - ), - pytest.param( - { - "filter": { - "include": [ - { - "relation": "datasets", - "scope": { - "include": [ - { - "relation": "documents", - "scope": { - "include": [{"relation": "parameters"}], - "limit": 20, - }, - }, - ], - "limit": 50, - }, - }, - ], - }, - }, - "documents", - 5, - [ - ["datasets"], - ["datasets.documents"], - ["datasets.documents.parameters"], - [], - [], - ], - [None, None, None, 20, 50], - id="Nested related models", - ), - ], - ) - def test_valid_include_filter_with_limit_filter_in_scope( - self, - test_request_filter, - test_entity_name, - expected_length, - expected_included_entities, - expected_limit_values, - ): - # TODO - do we need to support this?? - filters = SearchAPIQueryFilterFactory.get_query_filter( - test_request_filter, test_entity_name, - ) - - assert len(filters) == expected_length - - for test_filter, included_entities, limit_value in zip( - filters, expected_included_entities, expected_limit_values, - ): - if isinstance(test_filter, SearchAPIIncludeFilter): - assert test_filter.included_filters == included_entities - if isinstance(test_filter, SearchAPILimitFilter): - assert test_filter.limit_value == limit_value - - @pytest.mark.parametrize( - "test_request_filter, test_entity_name, expected_length" - ", expected_included_entities, expected_skip_values", - [ - pytest.param( - { - "filter": { - "include": [{"relation": "datasets", "scope": {"skip": 50}}], - }, - }, - "documents", - 2, - [["datasets"], []], - [None, 50], - id="Single related model", - ), - pytest.param( - { - "filter": { - "include": [ - {"relation": "datasets", "scope": {"skip": 50}}, - {"relation": "parameters", "scope": {"skip": 20}}, - ], - }, - }, - "documents", - 4, - [["datasets"], [], ["parameters"], []], - [None, 50, None, 20], - id="Multiple related model", - ), - pytest.param( - { - "filter": { - "include": [ - { - "relation": "datasets", - "scope": { - "include": [ - { - "relation": "documents", - "scope": { - "include": [{"relation": "parameters"}], - "skip": 20, - }, - }, - ], - "skip": 50, - }, - }, - ], - }, - }, - "documents", - 5, - [ - ["datasets"], - ["datasets.documents"], - ["datasets.documents.parameters"], - [], - [], - ], - [None, None, None, 20, 50], - id="Nested related models", - ), - ], - ) - def test_valid_include_filter_with_skip_filter_in_scope( - self, - test_request_filter, - test_entity_name, - expected_length, - expected_included_entities, - expected_skip_values, - ): - # TODO - do we need to support this?? - filters = SearchAPIQueryFilterFactory.get_query_filter( - test_request_filter, test_entity_name, - ) - - assert len(filters) == expected_length - - for test_filter, included_entities, skip_value in zip( - filters, expected_included_entities, expected_skip_values, - ): - if isinstance(test_filter, SearchAPIIncludeFilter): - assert test_filter.included_filters == included_entities - if isinstance(test_filter, SearchAPISkipFilter): - assert test_filter.skip_value == skip_value - - @pytest.mark.parametrize( - "test_request_filter, test_entity_name, expected_length" - ", expected_included_entities, expected_where_filter_data" - ", expected_limit_values, expected_skip_values", - [ - pytest.param( - { - "filter": { - "include": [ - { - "relation": "documents", - "scope": { - "where": {"title": "My Title"}, - "include": [{"relation": "instrument"}], - "limit": 50, - "skip": 20, - }, - }, - ], - }, - }, - "datasets", - 5, - [["documents"], [], ["documents.instrument"], [], []], - [[], ["documents.title", "eq", "My Title", "and"], [], [], []], - [None, None, None, 50, None], - [None, None, None, None, 20], - id="Simple case", - ), - pytest.param( - { - "filter": { - "include": [ - { - "relation": "documents", - "scope": { - "where": { - "and": [ - { - "and": [ - {"summary": "My Test Summary"}, - {"title": {"like": "Test title"}}, - ], - }, - { - "and": [ - {"pid": "Test pid"}, - {"type": {"eq": "Test type"}}, - ], - }, - { - "or": [ - {"doi": "Test doi"}, - { - "license": { - "like": "Test license", - }, - }, - ], - }, - ], - }, - "include": [ - { - "relation": "instrument", - "scope": { - "where": {"name": "Instrument 1"}, - "limit": 2, - }, - }, - ], - "limit": 50, - "skip": 20, - }, - }, - ], - }, - }, - "datasets", - 12, - [ - ["documents"], - [], - [], - [], - [], - [], - [], - ["documents.instrument"], - [], - [], - [], - [], - ], - [ - [], - ["documents.summary", "eq", "My Test Summary", "and"], - ["documents.title", "like", "Test title", "and"], - ["documents.pid", "eq", "Test pid", "and"], - ["documents.type", "eq", "Test type", "and"], - ["documents.doi", "eq", "Test doi", "or"], - ["documents.license", "like", "Test license", "or"], - [], - ["documents.instrument.name", "eq", "Instrument 1", "and"], - [], - [], - [], - ], - [None, None, None, None, None, None, None, None, None, 2, 50, None], - [None, None, None, None, None, None, None, None, None, None, None, 20], - id="Complex case", - ), - ], - ) - def test_valid_include_filter_with_all_filters_in_scope( - self, - test_request_filter, - test_entity_name, - expected_length, - expected_included_entities, - expected_where_filter_data, - expected_limit_values, - expected_skip_values, - ): - # TODO - are we going to support limit in include? If not, probably remove test? - filters = SearchAPIQueryFilterFactory.get_query_filter( - test_request_filter, test_entity_name, - ) - - assert len(filters) == expected_length - - for ( - test_filter, - included_entities, - where_filter_data, - limit_value, - skip_value, - ) in zip( - filters, - expected_included_entities, - expected_where_filter_data, - expected_limit_values, - expected_skip_values, - ): - if isinstance(test_filter, SearchAPIIncludeFilter): - assert test_filter.included_filters == included_entities - if isinstance(test_filter, SearchAPIWhereFilter): - assert test_filter.field == where_filter_data[0] - assert test_filter.operation == where_filter_data[1] - assert test_filter.value == where_filter_data[2] - assert test_filter.boolean_operator == where_filter_data[3] - if isinstance(test_filter, SearchAPILimitFilter): - assert test_filter.limit_value == limit_value - if isinstance(test_filter, SearchAPISkipFilter): - assert test_filter.skip_value == skip_value - @pytest.mark.parametrize( "test_request_filter, expected_limit_value", [ @@ -2103,10 +1767,7 @@ def test_valid_skip_filter( "include": [ { "relation": "instrument", - "scope": { - "where": {"name": "Instrument 1"}, - "limit": 2, - }, + "scope": {"where": {"name": "Instrument 1"}}, }, ], "limit": 50, From d6c62ea64443eb8ee0f2c248d873d13ef8d4a9d1 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Fri, 3 Dec 2021 17:04:28 +0000 Subject: [PATCH 40/44] test: refactor tests for where filter in include #259 --- .../src/search_api/query_filter_factory.py | 2 + .../test_search_api_query_filter_factory.py | 234 +++++++++++++----- 2 files changed, 173 insertions(+), 63 deletions(-) diff --git a/datagateway_api/src/search_api/query_filter_factory.py b/datagateway_api/src/search_api/query_filter_factory.py index 8549e064..5d14ec98 100644 --- a/datagateway_api/src/search_api/query_filter_factory.py +++ b/datagateway_api/src/search_api/query_filter_factory.py @@ -143,6 +143,8 @@ def get_include_filter(filter_input): ) for scope_query_filter in scope_query_filters: + # TODO - fix for AND boolean operator test needs to go in here + # Search through scope_query_filter to find where filters via nested if isinstance(scope_query_filter, SearchAPIWhereFilter): scope_query_filter.field = ( f"{included_entity}.{scope_query_filter.field}" diff --git a/test/search_api/test_search_api_query_filter_factory.py b/test/search_api/test_search_api_query_filter_factory.py index 3f0daf3a..0567332c 100644 --- a/test/search_api/test_search_api_query_filter_factory.py +++ b/test/search_api/test_search_api_query_filter_factory.py @@ -1261,7 +1261,8 @@ def test_valid_include_filter( @pytest.mark.parametrize( "test_request_filter, test_entity_name, expected_length" - ", expected_included_entities, expected_where_filter_data", + ", expected_included_entities, expected_where_filter_data" + ", expected_nested_wheres", [ pytest.param( { @@ -1276,8 +1277,9 @@ def test_valid_include_filter( }, "datasets", 2, - [["parameters"], []], - [[], ["parameters.name", "eq", "My parameter", "and"]], + [["parameters"]], + [SearchAPIWhereFilter("parameters.name", "My parameter", "eq")], + "", id="Property value with no operator", ), pytest.param( @@ -1293,8 +1295,9 @@ def test_valid_include_filter( }, "datasets", 2, - [["parameters"], []], - [[], ["parameters.name", "ne", "My parameter", "and"]], + [["parameters"]], + [SearchAPIWhereFilter("parameters.name", "My parameter", "ne")], + "", id="Property value with operator", ), pytest.param( @@ -1310,8 +1313,13 @@ def test_valid_include_filter( }, "datasets", 2, - [["files"], []], - [[], ["files.name", "eq", "file1", "or"]], + [["files"]], + [], + [ + NestedWhereFilters( + [], [SearchAPIWhereFilter("name", "file1", "eq")], "or", + ), + ], id="Text operator on defined field mapping to single field", ), pytest.param( @@ -1327,8 +1335,9 @@ def test_valid_include_filter( }, "datasets", 1, - [["parameters"], []], - [[], []], + [["parameters"]], + [], + [], id="Text operator on non-defined field", ), pytest.param( @@ -1343,12 +1352,15 @@ def test_valid_include_filter( }, }, "datasets", - 3, - [["documents"], []], + 2, + [["documents"]], + [], [ - [], - ["documents.title", "eq", "document1", "or"], - ["documents.summary", "eq", "document1", "or"], + NestedWhereFilters( + [SearchAPIWhereFilter("documents.title", "document1", "eq")], + [SearchAPIWhereFilter("documents.summary", "document1", "eq")], + "or", + ), ], id="Text operator on defined field mapping to multiple field", ), @@ -1371,12 +1383,19 @@ def test_valid_include_filter( }, }, "datasets", - 3, - [["documents"], [], []], + 2, + [["documents"]], + [], [ - [], - ["documents.summary", "eq", "My Test Summary", "and"], - ["documents.title", "eq", "Test title", "and"], + NestedWhereFilters( + [ + SearchAPIWhereFilter( + "documents.summary", "My Test Summary", "eq" + ) + ], + [SearchAPIWhereFilter("documents.title", "Test title", "eq")], + "and", + ), ], id="AND boolean operator", ), @@ -1388,7 +1407,7 @@ def test_valid_include_filter( "relation": "documents", "scope": { "where": { - "and": [ + "or": [ {"summary": "My Test Summary"}, {"title": "Test title"}, ], @@ -1399,12 +1418,19 @@ def test_valid_include_filter( }, }, "datasets", - 3, - [["documents"], [], []], + 2, + [["documents"]], + [], [ - [], - ["documents.summary", "eq", "My Test Summary", "and"], - ["documents.title", "eq", "Test title", "and"], + NestedWhereFilters( + [ + SearchAPIWhereFilter( + "documents.summary", "My Test Summary", "eq" + ) + ], + [SearchAPIWhereFilter("documents.title", "Test title", "eq")], + "or", + ), ], id="OR boolean operator", ), @@ -1447,16 +1473,56 @@ def test_valid_include_filter( }, }, "datasets", - 7, - [["documents"], [], [], [], [], [], []], + 2, + [["documents"]], + [], [ - [], - ["documents.summary", "eq", "My Test Summary", "and"], - ["documents.title", "like", "Test title", "and"], - ["documents.pid", "eq", "Test pid", "and"], - ["documents.type", "eq", "Test type", "and"], - ["documents.doi", "eq", "Test doi", "or"], - ["documents.license", "like", "Test license", "or"], + NestedWhereFilters( + [ + NestedWhereFilters( + [ + SearchAPIWhereFilter( + "documents.summary", "My Test Summary", "eq" + ) + ], + [ + SearchAPIWhereFilter( + "documents.title", "Test title", "like" + ) + ], + "and", + ), + NestedWhereFilters( + [ + SearchAPIWhereFilter( + "documents.pid", "Test pid", "eq" + ) + ], + [ + SearchAPIWhereFilter( + "documents.type", "Test type", "eq" + ) + ], + "and", + ), + ], + [ + NestedWhereFilters( + [ + SearchAPIWhereFilter( + "documents.doi", "Test doi", "eq" + ) + ], + [ + SearchAPIWhereFilter( + "documents.license", "Test license", "like" + ) + ], + "or", + ), + ], + "and", + ), ], id="Nested AND boolean operator", ), @@ -1499,16 +1565,56 @@ def test_valid_include_filter( }, }, "datasets", - 7, - [["documents"], [], [], [], [], [], []], + 2, + [["documents"]], + [], [ - [], - ["documents.summary", "eq", "My Test Summary", "and"], - ["documents.title", "like", "Test title", "and"], - ["documents.pid", "eq", "Test pid", "and"], - ["documents.type", "eq", "Test type", "and"], - ["documents.doi", "eq", "Test doi", "or"], - ["documents.license", "like", "Test license", "or"], + NestedWhereFilters( + [ + NestedWhereFilters( + [ + SearchAPIWhereFilter( + "documents.summary", "My Test Summary", "eq" + ) + ], + [ + SearchAPIWhereFilter( + "documents.title", "Test title", "like" + ) + ], + "and", + ), + NestedWhereFilters( + [ + SearchAPIWhereFilter( + "documents.pid", "Test pid", "eq" + ) + ], + [ + SearchAPIWhereFilter( + "documents.type", "Test type", "eq" + ) + ], + "and", + ), + ], + [ + NestedWhereFilters( + [ + SearchAPIWhereFilter( + "documents.doi", "Test doi", "eq" + ) + ], + [ + SearchAPIWhereFilter( + "documents.license", "Test license", "like" + ) + ], + "or", + ), + ], + "or", + ), ], id="Nested OR boolean operator", ), @@ -1529,13 +1635,12 @@ def test_valid_include_filter( }, "datasets", 4, - [["parameters"], [], ["documents"], []], + [["parameters"], ["documents"]], [ - [], - ["parameters.name", "eq", "My parameter", "and"], - [], - ["documents.title", "eq", "Document title", "and"], + SearchAPIWhereFilter("parameters.name", "My parameter", "eq"), + SearchAPIWhereFilter("documents.title", "Document title", "eq"), ], + [], id="Multiple related models", ), pytest.param( @@ -1561,13 +1666,14 @@ def test_valid_include_filter( }, "documents", 4, - [["datasets"], [], ["datasets.instrument"], []], + [["datasets"], ["datasets.instrument"]], [ - [], - ["datasets.title", "eq", "Dataset 1", "and"], - [], - ["datasets.instrument.name", "eq", "Instrument 1", "and"], + SearchAPIWhereFilter("datasets.title", "Dataset 1", "eq"), + SearchAPIWhereFilter( + "datasets.instrument.name", "Instrument 1", "eq" + ), ], + [], id="Nested related models", ), ], @@ -1579,24 +1685,26 @@ def test_valid_include_filter_with_where_filter_in_scope( expected_length, expected_included_entities, expected_where_filter_data, + expected_nested_wheres, ): - # TODO filters = SearchAPIQueryFilterFactory.get_query_filter( test_request_filter, test_entity_name, ) assert len(filters) == expected_length - - for test_filter, included_entities, where_filter_data in zip( - filters, expected_included_entities, expected_where_filter_data, - ): + for test_filter in filters: if isinstance(test_filter, SearchAPIIncludeFilter): - assert test_filter.included_filters == included_entities + for expected_include in expected_included_entities: + assert test_filter.included_filters == expected_include + expected_included_entities.remove(expected_include) + if isinstance(test_filter, NestedWhereFilters): + for expected_nested in expected_nested_wheres: + assert repr(test_filter) == repr(expected_nested) + expected_nested_wheres.remove(expected_nested) if isinstance(test_filter, SearchAPIWhereFilter): - assert test_filter.field == where_filter_data[0] - assert test_filter.operation == where_filter_data[1] - assert test_filter.value == where_filter_data[2] - assert test_filter.boolean_operator == where_filter_data[3] + for expected_where in expected_where_filter_data: + assert repr(test_filter) == repr(expected_where) + expected_where_filter_data.remove(expected_where) @pytest.mark.parametrize( "test_request_filter, test_entity_name, expected_length" From 605315c64cc3f8c445ca0bf83b73813bada785fd Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Fri, 3 Dec 2021 17:31:53 +0000 Subject: [PATCH 41/44] test: refactor more query params tests #259 --- .../test_search_api_query_filter_factory.py | 108 ++++++++++-------- 1 file changed, 63 insertions(+), 45 deletions(-) diff --git a/test/search_api/test_search_api_query_filter_factory.py b/test/search_api/test_search_api_query_filter_factory.py index 0567332c..43d257c2 100644 --- a/test/search_api/test_search_api_query_filter_factory.py +++ b/test/search_api/test_search_api_query_filter_factory.py @@ -1790,10 +1790,11 @@ def test_valid_include_filter_with_include_filter_in_scope( ) assert len(filters) == expected_length - - for test_filter, included_entities in zip(filters, expected_included_entities): + for test_filter in filters: if isinstance(test_filter, SearchAPIIncludeFilter): - assert test_filter.included_filters == included_entities + for expected_include in expected_included_entities: + assert test_filter.included_filters == expected_include + expected_included_entities.remove(expected_include) @pytest.mark.parametrize( "test_request_filter, expected_limit_value", @@ -1828,7 +1829,7 @@ def test_valid_skip_filter( @pytest.mark.parametrize( "test_request_filter, test_entity_name, expected_length" ", expected_included_entities, expected_where_filter_data" - ", expected_limit_values, expected_skip_values", + ", expected_nested_wheres, expected_limit_values, expected_skip_values", [ pytest.param( { @@ -1841,10 +1842,11 @@ def test_valid_skip_filter( }, "datasets", 4, - [[], ["instrument"], [], []], - [["title", "eq", "My Title", "and"], [], [], []], - [None, None, 50, None], - [None, None, None, 20], + [["instrument"]], + [SearchAPIWhereFilter("title", "My Title", "eq")], + [], + [50], + [20], id="Simple case", ), pytest.param( @@ -1883,23 +1885,43 @@ def test_valid_skip_filter( }, }, "datasets", - 11, - [[], [], [], [], [], [], ["instrument"], [], [], [], []], + 5, + [["instrument"]], + [SearchAPIWhereFilter("instrument.name", "Instrument 1", "eq")], [ - ["summary", "eq", "My Test Summary", "and"], - ["title", "like", "Test title", "and"], - ["pid", "eq", "Test pid", "and"], - ["type", "eq", "Test type", "and"], - ["doi", "eq", "Test doi", "or"], - ["license", "like", "Test license", "or"], - [], - ["instrument.name", "eq", "Instrument 1", "and"], - [], - [], - [], + NestedWhereFilters( + [ + NestedWhereFilters( + [ + SearchAPIWhereFilter( + "summary", "My Test Summary", "eq" + ) + ], + [SearchAPIWhereFilter("title", "Test title", "like")], + "and", + ), + NestedWhereFilters( + [SearchAPIWhereFilter("pid", "Test pid", "eq")], + [SearchAPIWhereFilter("type", "Test type", "eq")], + "and", + ), + ], + [ + NestedWhereFilters( + [SearchAPIWhereFilter("doi", "Test doi", "eq")], + [ + SearchAPIWhereFilter( + "license", "Test license", "like" + ) + ], + "or", + ), + ], + "and", + ) ], - [None, None, None, None, None, None, None, None, 2, 50, None], - [None, None, None, None, None, None, None, None, None, None, 20], + [50], + [20], id="Complex case", ), ], @@ -1911,40 +1933,36 @@ def test_valid_filter_input_with_all_filters( expected_length, expected_included_entities, expected_where_filter_data, + expected_nested_wheres, expected_limit_values, expected_skip_values, ): - # TODO - remove limit from scope filters = SearchAPIQueryFilterFactory.get_query_filter( test_request_filter, test_entity_name, ) assert len(filters) == expected_length - - for ( - test_filter, - included_entities, - where_filter_data, - limit_value, - skip_value, - ) in zip( - filters, - expected_included_entities, - expected_where_filter_data, - expected_limit_values, - expected_skip_values, - ): + for test_filter in filters: if isinstance(test_filter, SearchAPIIncludeFilter): - assert test_filter.included_filters == included_entities + for expected_include in expected_included_entities: + assert test_filter.included_filters == expected_include + expected_included_entities.remove(expected_include) + if isinstance(test_filter, NestedWhereFilters): + for expected_nested in expected_nested_wheres: + assert repr(test_filter) == repr(expected_nested) + expected_nested_wheres.remove(expected_nested) if isinstance(test_filter, SearchAPIWhereFilter): - assert test_filter.field == where_filter_data[0] - assert test_filter.operation == where_filter_data[1] - assert test_filter.value == where_filter_data[2] - assert test_filter.boolean_operator == where_filter_data[3] + for expected_where in expected_where_filter_data: + assert repr(test_filter) == repr(expected_where) + expected_where_filter_data.remove(expected_where) if isinstance(test_filter, SearchAPILimitFilter): - assert test_filter.limit_value == limit_value + for expected_limit in expected_limit_values: + assert test_filter.limit_value == expected_limit + expected_limit_values.remove(expected_limit) if isinstance(test_filter, SearchAPISkipFilter): - assert test_filter.skip_value == skip_value + for expected_skip in expected_skip_values: + assert test_filter.skip_value == expected_skip + expected_skip_values.remove(expected_skip) @pytest.mark.parametrize( "test_request_filter", From 4134a1673a4cf239f32fe9dfce03fed903b4693d Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Mon, 6 Dec 2021 12:10:20 +0000 Subject: [PATCH 42/44] refactor: clean up `SearchAPIQueryFilterFactory` #259 --- .../src/search_api/query_filter_factory.py | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/datagateway_api/src/search_api/query_filter_factory.py b/datagateway_api/src/search_api/query_filter_factory.py index 5d14ec98..689fc715 100644 --- a/datagateway_api/src/search_api/query_filter_factory.py +++ b/datagateway_api/src/search_api/query_filter_factory.py @@ -63,27 +63,23 @@ def get_where_filter(filter_input, entity_name): ): boolean_operator = list(filter_input.keys())[0] conditions = list(filter_input.values())[0] - test_where_filters = [] + conditional_where_filters = [] for condition in conditions: # Could be nested AND/OR where_filter = { "filter": {"where": condition}, } - conditional_where_filters = SearchAPIQueryFilterFactory.get_query_filter( - where_filter, entity_name, + conditional_where_filters.extend( + SearchAPIQueryFilterFactory.get_query_filter( + where_filter, entity_name, + ), ) - # TODO - could just go to extend the function call? - test_where_filters.extend(conditional_where_filters) - - # for conditional_where_filter in conditional_where_filters: - # conditional_where_filter.boolean_operator = boolean_operator - # where_filters.extend(conditional_where_filters) - - print(f"test Cond where filters: {test_where_filters}") nested = NestedWhereFilters( - test_where_filters[:-1], test_where_filters[-1], boolean_operator, + conditional_where_filters[:-1], + conditional_where_filters[-1], + boolean_operator, ) where_filters.append(nested) elif list(filter_input.keys())[0] == "text": From 60cc5d1006e6e9f0591531014bf36c4c4b44509e Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Mon, 6 Dec 2021 12:18:21 +0000 Subject: [PATCH 43/44] refactor: remove `boolean_operator` from `SearchAPIWhereFilter` #259 - This is no longer used, boolean operators are managed by `NestedWhereFilters` --- datagateway_api/src/search_api/filters.py | 3 +-- test/search_api/test_nested_where_filters.py | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/datagateway_api/src/search_api/filters.py b/datagateway_api/src/search_api/filters.py index d1003378..4405ccef 100644 --- a/datagateway_api/src/search_api/filters.py +++ b/datagateway_api/src/search_api/filters.py @@ -12,9 +12,8 @@ class SearchAPIWhereFilter(PythonICATWhereFilter): - def __init__(self, field, value, operation, boolean_operator="and"): + def __init__(self, field, value, operation): super().__init__(field, value, operation) - self.boolean_operator = boolean_operator def apply_filter(self, query): return super().apply_filter(query) diff --git a/test/search_api/test_nested_where_filters.py b/test/search_api/test_nested_where_filters.py index 4746b389..f4c851f9 100644 --- a/test/search_api/test_nested_where_filters.py +++ b/test/search_api/test_nested_where_filters.py @@ -76,43 +76,43 @@ def test_str_filters(self, lhs, rhs, joining_operator, expected_where_clause): "lhs, rhs, joining_operator, expected_where_clause", [ pytest.param( - SearchAPIWhereFilter("name", "test name", "eq", "and"), - SearchAPIWhereFilter("id", 3, "eq", "and"), + SearchAPIWhereFilter("name", "test name", "eq"), + SearchAPIWhereFilter("id", 3, "eq"), "OR", "(o.name = 'test name' OR o.id = '3')", id="(o.name = 'test name' OR o.id = '3')", ), pytest.param( - [SearchAPIWhereFilter("name", "test name", "eq", "and")], - SearchAPIWhereFilter("id", 3, "eq", "and"), + [SearchAPIWhereFilter("name", "test name", "eq")], + SearchAPIWhereFilter("id", 3, "eq"), "OR", "(o.name = 'test name' OR o.id = '3')", id="Single filter list in LHS", ), pytest.param( - [SearchAPIWhereFilter("name", "test name", "eq", "and")], - [SearchAPIWhereFilter("id", 3, "eq", "and")], + [SearchAPIWhereFilter("name", "test name", "eq")], + [SearchAPIWhereFilter("id", 3, "eq")], "OR", "(o.name = 'test name' OR o.id = '3')", id="Single filter list in LHS and RHS", ), pytest.param( [ - SearchAPIWhereFilter("name", "test name", "eq", "and"), + SearchAPIWhereFilter("name", "test name", "eq"), SearchAPIWhereFilter("id", 10, "lt"), ], - [SearchAPIWhereFilter("id", 3, "gt", "and")], + [SearchAPIWhereFilter("id", 3, "gt")], "AND", "(o.name = 'test name' AND o.id < '10' AND o.id > '3')", id="Multiple filters on LHS", ), pytest.param( [ - SearchAPIWhereFilter("name", "test name", "eq", "and"), + SearchAPIWhereFilter("name", "test name", "eq"), SearchAPIWhereFilter("id", 10, "lt"), ], [ - SearchAPIWhereFilter("id", 3, "gt", "and"), + SearchAPIWhereFilter("id", 3, "gt"), SearchAPIWhereFilter("doi", "Test DOI", "like"), ], "AND", From b0a4c47416da78cbc093c4137f3f50c034abc518 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Mon, 6 Dec 2021 12:29:03 +0000 Subject: [PATCH 44/44] style: fix linting issues #259 --- .../src/common/base_query_filter_factory.py | 2 +- datagateway_api/src/search_api/filters.py | 10 ++- .../src/search_api/nested_where_filters.py | 5 +- .../test_search_api_query_filter_factory.py | 68 +++++++++---------- 4 files changed, 46 insertions(+), 39 deletions(-) diff --git a/datagateway_api/src/common/base_query_filter_factory.py b/datagateway_api/src/common/base_query_filter_factory.py index d9a9c4a4..e4272f93 100644 --- a/datagateway_api/src/common/base_query_filter_factory.py +++ b/datagateway_api/src/common/base_query_filter_factory.py @@ -3,7 +3,7 @@ class QueryFilterFactory(object): @abstractstaticmethod - def get_query_filter(request_filter, entity_name=None): + def get_query_filter(request_filter, entity_name=None): # noqa: B902, N805 """ Given a filter, return a matching Query filter object diff --git a/datagateway_api/src/search_api/filters.py b/datagateway_api/src/search_api/filters.py index 4405ccef..7aa78b61 100644 --- a/datagateway_api/src/search_api/filters.py +++ b/datagateway_api/src/search_api/filters.py @@ -1,11 +1,12 @@ +from icat.client import Client +from icat.query import Query + from datagateway_api.src.datagateway_api.icat.filters import ( PythonICATIncludeFilter, PythonICATLimitFilter, PythonICATSkipFilter, PythonICATWhereFilter, ) -from icat.query import Query -from icat.client import Client # TODO - Implement each of these filters for Search API, inheriting from the Python ICAT # versions @@ -32,7 +33,10 @@ def __str__(self): return str_conds[0] def __repr__(self): - return f"Field: '{self.field}', Value: '{self.value}', Operation: '{self.operation}'" + return ( + f"Field: '{self.field}', Value: '{self.value}', Operation:" + f" '{self.operation}'" + ) class SearchAPISkipFilter(PythonICATSkipFilter): diff --git a/datagateway_api/src/search_api/nested_where_filters.py b/datagateway_api/src/search_api/nested_where_filters.py index 82b02f89..a3a38a25 100644 --- a/datagateway_api/src/search_api/nested_where_filters.py +++ b/datagateway_api/src/search_api/nested_where_filters.py @@ -47,4 +47,7 @@ def __str__(self): return f"({operator.join(conditions)})" def __repr__(self): - return f"LHS: {repr(self.lhs)}, RHS: {repr(self.rhs)}, Operator: {repr(self.joining_operator)}" + return ( + f"LHS: {repr(self.lhs)}, RHS: {repr(self.rhs)}, Operator:" + f" {repr(self.joining_operator)}" + ) diff --git a/test/search_api/test_search_api_query_filter_factory.py b/test/search_api/test_search_api_query_filter_factory.py index 43d257c2..6176b313 100644 --- a/test/search_api/test_search_api_query_filter_factory.py +++ b/test/search_api/test_search_api_query_filter_factory.py @@ -1390,8 +1390,8 @@ def test_valid_include_filter( NestedWhereFilters( [ SearchAPIWhereFilter( - "documents.summary", "My Test Summary", "eq" - ) + "documents.summary", "My Test Summary", "eq", + ), ], [SearchAPIWhereFilter("documents.title", "Test title", "eq")], "and", @@ -1425,8 +1425,8 @@ def test_valid_include_filter( NestedWhereFilters( [ SearchAPIWhereFilter( - "documents.summary", "My Test Summary", "eq" - ) + "documents.summary", "My Test Summary", "eq", + ), ], [SearchAPIWhereFilter("documents.title", "Test title", "eq")], "or", @@ -1482,26 +1482,26 @@ def test_valid_include_filter( NestedWhereFilters( [ SearchAPIWhereFilter( - "documents.summary", "My Test Summary", "eq" - ) + "documents.summary", "My Test Summary", "eq", + ), ], [ SearchAPIWhereFilter( - "documents.title", "Test title", "like" - ) + "documents.title", "Test title", "like", + ), ], "and", ), NestedWhereFilters( [ SearchAPIWhereFilter( - "documents.pid", "Test pid", "eq" - ) + "documents.pid", "Test pid", "eq", + ), ], [ SearchAPIWhereFilter( - "documents.type", "Test type", "eq" - ) + "documents.type", "Test type", "eq", + ), ], "and", ), @@ -1510,13 +1510,13 @@ def test_valid_include_filter( NestedWhereFilters( [ SearchAPIWhereFilter( - "documents.doi", "Test doi", "eq" - ) + "documents.doi", "Test doi", "eq", + ), ], [ SearchAPIWhereFilter( - "documents.license", "Test license", "like" - ) + "documents.license", "Test license", "like", + ), ], "or", ), @@ -1574,26 +1574,26 @@ def test_valid_include_filter( NestedWhereFilters( [ SearchAPIWhereFilter( - "documents.summary", "My Test Summary", "eq" - ) + "documents.summary", "My Test Summary", "eq", + ), ], [ SearchAPIWhereFilter( - "documents.title", "Test title", "like" - ) + "documents.title", "Test title", "like", + ), ], "and", ), NestedWhereFilters( [ SearchAPIWhereFilter( - "documents.pid", "Test pid", "eq" - ) + "documents.pid", "Test pid", "eq", + ), ], [ SearchAPIWhereFilter( - "documents.type", "Test type", "eq" - ) + "documents.type", "Test type", "eq", + ), ], "and", ), @@ -1602,13 +1602,13 @@ def test_valid_include_filter( NestedWhereFilters( [ SearchAPIWhereFilter( - "documents.doi", "Test doi", "eq" - ) + "documents.doi", "Test doi", "eq", + ), ], [ SearchAPIWhereFilter( - "documents.license", "Test license", "like" - ) + "documents.license", "Test license", "like", + ), ], "or", ), @@ -1670,7 +1670,7 @@ def test_valid_include_filter( [ SearchAPIWhereFilter("datasets.title", "Dataset 1", "eq"), SearchAPIWhereFilter( - "datasets.instrument.name", "Instrument 1", "eq" + "datasets.instrument.name", "Instrument 1", "eq", ), ], [], @@ -1894,8 +1894,8 @@ def test_valid_skip_filter( NestedWhereFilters( [ SearchAPIWhereFilter( - "summary", "My Test Summary", "eq" - ) + "summary", "My Test Summary", "eq", + ), ], [SearchAPIWhereFilter("title", "Test title", "like")], "and", @@ -1911,14 +1911,14 @@ def test_valid_skip_filter( [SearchAPIWhereFilter("doi", "Test doi", "eq")], [ SearchAPIWhereFilter( - "license", "Test license", "like" - ) + "license", "Test license", "like", + ), ], "or", ), ], "and", - ) + ), ], [50], [20],