diff --git a/datagateway_api/src/search_api/helpers.py b/datagateway_api/src/search_api/helpers.py index 43513174..27399324 100644 --- a/datagateway_api/src/search_api/helpers.py +++ b/datagateway_api/src/search_api/helpers.py @@ -146,6 +146,9 @@ def get_files(entity_name, pid, filters): "Entity Name: %s, Filters: %s", entity_name, filters, ) + # Check if dataset with such pid exists before proceeding + get_with_pid("Dataset", pid, []) + filters.append(SearchAPIWhereFilter("dataset.pid", pid, "eq")) return get_search(entity_name, filters) @@ -171,5 +174,8 @@ def get_files_count(entity_name, filters, pid): "Entity Name: %s, Filters: %s", entity_name, filters, ) + # Check if dataset with such pid exists before proceeding + get_with_pid("Dataset", pid, []) + filters.append(SearchAPIWhereFilter("dataset.pid", pid, "eq")) return get_count(entity_name, filters) diff --git a/datagateway_api/src/search_api/query_filter_factory.py b/datagateway_api/src/search_api/query_filter_factory.py index 0086b7d1..e199bc75 100644 --- a/datagateway_api/src/search_api/query_filter_factory.py +++ b/datagateway_api/src/search_api/query_filter_factory.py @@ -104,87 +104,98 @@ def get_where_filter(where_filter_input, entity_name): :type entity_name: :class:`str` :return: The list of `NestedWhereFilters` and/ or `SearchAPIWhereFilter` objects created + :raises FilterError: If the where filter input is not provided as an object """ + if not isinstance(where_filter_input, dict): + raise FilterError( + "Bad where filter input, please ensure that it is provided as an " + "object", + ) where_filters = [] - if ( - list(where_filter_input.keys())[0] == "and" - or list(where_filter_input.keys())[0] == "or" - ): - log.debug("and/or operators found: %s", list(where_filter_input.keys())[0]) - boolean_operator = list(where_filter_input.keys())[0] - conditions = list(where_filter_input.values())[0] - conditional_where_filters = [] - - for condition in conditions: - # Could be nested AND/OR - where_filter = { - "filter": {"where": condition}, - } - conditional_where_filters.extend( - SearchAPIQueryFilterFactory.get_query_filter( - where_filter, entity_name, - ), + if where_filter_input: + if ( + list(where_filter_input.keys())[0] == "and" + or list(where_filter_input.keys())[0] == "or" + ): + log.debug( + "and/or operators found: %s", list(where_filter_input.keys())[0], ) + boolean_operator = list(where_filter_input.keys())[0] + conditions = list(where_filter_input.values())[0] + conditional_where_filters = [] + + for condition in conditions: + # Could be nested AND/OR + where_filter = { + "filter": {"where": condition}, + } + conditional_where_filters.extend( + SearchAPIQueryFilterFactory.get_query_filter( + where_filter, entity_name, + ), + ) - nested = NestedWhereFilters( - conditional_where_filters[:-1], - conditional_where_filters[-1], - boolean_operator, - SearchAPIQuery(entity_name), - ) - where_filters.append(nested) - elif list(where_filter_input.keys())[0] == "text": - log.debug("Text operator found within JSON where object") - try: - entity_class = getattr(search_api_models, entity_name) - except AttributeError as e: - raise SearchAPIError( - f"No text operator fields have been defined for {entity_name}" - f", {e.args}", + nested = NestedWhereFilters( + conditional_where_filters[:-1], + conditional_where_filters[-1], + boolean_operator, + SearchAPIQuery(entity_name), ) + where_filters.append(nested) + elif list(where_filter_input.keys())[0] == "text": + log.debug("Text operator found within JSON where object") + try: + entity_class = getattr(search_api_models, entity_name) + except AttributeError: + raise SearchAPIError( + f"No model for {entity_name} could be found, a different entity" + f"name should be used", + ) - or_conditional_filters = [] - field_names = entity_class._text_operator_fields - log.debug( - "Text operators found for PaNOSC %s: %s", entity_name, field_names, - ) - if not field_names: - # No text operator fields present, simply log and move on, we should - # ignore text operator queries on entities where `_text_operator_fields` - # is empty (meaning they are not present in the origina PaNOSC data - # model) - log.info( - "No text operator fields found for PaNOSC entity %s, will" - " ignore", - entity_name, + or_conditional_filters = [] + field_names = entity_class._text_operator_fields + log.debug( + "Text operators found for PaNOSC %s: %s", entity_name, field_names, ) - else: - for field_name in field_names: - or_conditional_filters.append( - {field_name: {"like": where_filter_input["text"]}}, + if not field_names: + # No text operator fields present, simply log and move on, we should + # ignore text operator queries on entities where + # `_text_operator_fields` is empty (meaning they are not present in + # the origina PaNOSC data model) + log.info( + "No text operator fields found for PaNOSC entity %s, will" + " ignore", + entity_name, ) + else: + for field_name in field_names: + or_conditional_filters.append( + {field_name: {"like": where_filter_input["text"]}}, + ) - where_filter = { - "filter": {"where": {"or": or_conditional_filters}}, - } - where_filters.extend( - SearchAPIQueryFilterFactory.get_query_filter( - where_filter, entity_name, + where_filter = { + "filter": {"where": {"or": or_conditional_filters}}, + } + where_filters.extend( + SearchAPIQueryFilterFactory.get_query_filter( + where_filter, entity_name, + ), + ) + else: + log.info( + "Basic where filter found, extracting field, value and operation", + ) + filter_data = SearchAPIQueryFilterFactory.get_condition_values( + where_filter_input, + ) + where_filters.append( + SearchAPIWhereFilter( + field=filter_data[0], + value=filter_data[1], + operation=filter_data[2], ), ) - else: - log.info("Basic where filter found, extracting field, value and operation") - filter_data = SearchAPIQueryFilterFactory.get_condition_values( - where_filter_input, - ) - where_filters.append( - SearchAPIWhereFilter( - field=filter_data[0], - value=filter_data[1], - operation=filter_data[2], - ), - ) return where_filters diff --git a/test/search_api/endpoints/test_count_dataset_files.py b/test/search_api/endpoints/test_count_dataset_files.py index 32749a1f..b48e1c20 100644 --- a/test/search_api/endpoints/test_count_dataset_files.py +++ b/test/search_api/endpoints/test_count_dataset_files.py @@ -12,8 +12,6 @@ class TestSearchAPICountDatasetFilesEndpoint: "{}", {"count": 56}, id="Basic /datasets/{pid}/files/count request", - # Skipped because empty dict for filter doesn't work on where - marks=pytest.mark.skip, ), pytest.param( "0-8401-1070-7", @@ -39,14 +37,6 @@ class TestSearchAPICountDatasetFilesEndpoint: {"count": 0}, id="Count dataset files with filter to return zero count", ), - pytest.param( - "unknown pid", - "{}", - {"count": 0}, - id="Non-existent dataset pid", - # Skipped because empty dict for filter doesn't work on where - marks=pytest.mark.skip, - ), ], ) def test_valid_count_dataset_files_endpoint( @@ -61,22 +51,24 @@ def test_valid_count_dataset_files_endpoint( assert test_response.json == expected_json @pytest.mark.parametrize( - "pid, request_filter", + "pid, request_filter, expected_status_code", [ - pytest.param("0-8401-1070-7", '{"bad filter"}', id="Bad filter"), + pytest.param("0-8401-1070-7", '{"bad filter"}', 400, id="Bad filter"), pytest.param( "0-8401-1070-7", '{"where": {"name": "FILE 4"}}', + 400, id="Where filter inside where query param", ), + pytest.param("my 404 test pid", "{}", 404, id="Non-existent dataset pid"), ], ) def test_invalid_count_dataset_files_endpoint( - self, flask_test_app_search_api, pid, request_filter, + self, flask_test_app_search_api, pid, request_filter, expected_status_code, ): test_response = flask_test_app_search_api.get( f"{Config.config.search_api.extension}/datasets/{pid}/files/count" f"?where={request_filter}", ) - assert test_response.status_code == 400 + assert test_response.status_code == expected_status_code diff --git a/test/search_api/endpoints/test_count_endpoint.py b/test/search_api/endpoints/test_count_endpoint.py index 5942c122..fbd55cc1 100644 --- a/test/search_api/endpoints/test_count_endpoint.py +++ b/test/search_api/endpoints/test_count_endpoint.py @@ -11,28 +11,16 @@ class TestSearchAPICountEndpoint: "endpoint_name, request_filter, expected_json", [ pytest.param( - "datasets", - "{}", - {"count": 479}, - id="Basic /datasets/count request", - # Skipped because empty dict for filter doesn't work on where - marks=pytest.mark.skip, + "datasets", "{}", {"count": 479}, id="Basic /datasets/count request", ), pytest.param( - "documents", - "{}", - {"count": 239}, - id="Basic /documents/count request", - # Skipped because empty dict for filter doesn't work on where - marks=pytest.mark.skip, + "documents", "{}", {"count": 239}, id="Basic /documents/count request", ), pytest.param( "instruments", "{}", {"count": 14}, id="Basic /instruments/count request", - # Skipped because empty dict for filter doesn't work on where - marks=pytest.mark.skip, ), pytest.param( "datasets", diff --git a/test/search_api/endpoints/test_get_dataset_files.py b/test/search_api/endpoints/test_get_dataset_files.py index e0d47f6a..99b75e82 100644 --- a/test/search_api/endpoints/test_get_dataset_files.py +++ b/test/search_api/endpoints/test_get_dataset_files.py @@ -141,14 +141,7 @@ def test_valid_get_dataset_files_endpoint( pytest.param( "0-8401-1070-7", '{"include": ""}', 400, id="Bad include filter", ), - pytest.param( - "my 404 test pid", - "{}", - 404, - id="Non-existent dataset pid", - # Skipped because this actually returns 200 - marks=pytest.mark.skip, - ), + pytest.param("my 404 test pid", "{}", 404, id="Non-existent dataset pid"), ], ) def test_invalid_get_dataset_files_endpoint( 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 9744c1c5..9bcc9863 100644 --- a/test/search_api/test_search_api_query_filter_factory.py +++ b/test/search_api/test_search_api_query_filter_factory.py @@ -2063,6 +2063,7 @@ def test_valid_filter_input_with_all_filters( }, id="Unsupported skip filter in scope of include filter", ), + pytest.param({"filter": {"where": []}}, id="Bad where filter input"), pytest.param( {"filter": {"where": {"isPublic": {"lt": True}}}}, id="Unsupported operator in where filter with boolean value",