Merge pull request #364 from antolinos/issue_363

This fixes the problem with limit when it is not a number
ral-facilities · May 25, 2022 · 076e805 · 076e805
2 parents bb2e158 + ab3839a
commit 076e805
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 9 deletions.
diff --git a/datagateway_api/src/search_api/filters.py b/datagateway_api/src/search_api/filters.py
@@ -148,15 +148,15 @@ def __repr__(self):
 
 class SearchAPISkipFilter(PythonICATSkipFilter):
     def __init__(self, skip_value):
-        super().__init__(skip_value, filter_use="search_api")
+        super().__init__(int(skip_value), filter_use="search_api")
 
     def apply_filter(self, query):
         return super().apply_filter(query.icat_query.query)
 
 
 class SearchAPILimitFilter(PythonICATLimitFilter):
     def __init__(self, limit_value):
-        super().__init__(limit_value)
+        super().__init__(int(limit_value))
 
     def apply_filter(self, query):
         return super().apply_filter(query.icat_query.query)

diff --git a/test/search_api/filters/test_search_api_limit_filter.py b/test/search_api/filters/test_search_api_limit_filter.py
@@ -11,18 +11,25 @@ class TestSearchAPILimitFilter:
             pytest.param(10, id="typical"),
             pytest.param(0, id="low boundary"),
             pytest.param(9999, id="high boundary"),
+            pytest.param("10", id="string parameter typical"),
+            pytest.param("0", id="string parameter low boundary"),
+            pytest.param("9999", id="string parameter high boundary"),
         ],
     )
     def test_valid_limit_value(self, limit_value, search_api_query_document):
         test_filter = SearchAPILimitFilter(limit_value)
         test_filter.apply_filter(search_api_query_document)
 
-        assert search_api_query_document.icat_query.query.limit == (0, limit_value)
+        assert search_api_query_document.icat_query.query.limit == (0, int(limit_value))
 
     @pytest.mark.parametrize(
         "limit_value",
-        [pytest.param(-50, id="extreme invalid"), pytest.param(-1, id="boundary")],
+        [
+            pytest.param(-50, id="extreme invalid"),
+            pytest.param(-1, id="boundary"),
+            pytest.param("Nan", id="Nan string"),
+        ],
     )
     def test_invalid_limit_value(self, limit_value):
-        with pytest.raises(FilterError):
+        with pytest.raises((FilterError, ValueError)):
             SearchAPILimitFilter(limit_value)
diff --git a/test/search_api/filters/test_search_api_skip_filter.py b/test/search_api/filters/test_search_api_skip_filter.py
@@ -8,14 +8,20 @@
 
 class TestSearchAPISkipFilter:
     @pytest.mark.parametrize(
-        "skip_value", [pytest.param(10, id="typical"), pytest.param(0, id="boundary")],
+        "skip_value",
+        [
+            pytest.param(10, id="typical"),
+            pytest.param(0, id="boundary"),
+            pytest.param("10", id="string format typical"),
+            pytest.param("0", id="string format boundary"),
+        ],
     )
     def test_valid_skip_value(self, search_api_query_document, skip_value):
         test_filter = SearchAPISkipFilter(skip_value)
         test_filter.apply_filter(search_api_query_document)
 
         assert search_api_query_document.icat_query.query.limit == (
-            skip_value,
+            int(skip_value),
             get_icat_properties(
                 Config.config.search_api.icat_url,
                 Config.config.search_api.icat_check_cert,
@@ -24,8 +30,12 @@ def test_valid_skip_value(self, search_api_query_document, skip_value):
 
     @pytest.mark.parametrize(
         "skip_value",
-        [pytest.param(-375, id="extreme invalid"), pytest.param(-1, id="boundary")],
+        [
+            pytest.param(-375, id="extreme invalid"),
+            pytest.param(-1, id="boundary"),
+            pytest.param("Nan", id="Nan string"),
+        ],
     )
     def test_invalid_skip_value(self, skip_value):
-        with pytest.raises(FilterError):
+        with pytest.raises((FilterError, ValueError)):
             SearchAPISkipFilter(skip_value)
diff --git a/test/search_api/test_search_api_query_filter_factory.py b/test/search_api/test_search_api_query_filter_factory.py
@@ -1879,6 +1879,7 @@ def test_valid_include_filter_with_include_filter_in_scope(
         [
             pytest.param({"filter": {"limit": 0}}, 0, id="Limit 0 values"),
             pytest.param({"filter": {"limit": 50}}, 50, id="Limit 50 values"),
+            pytest.param({"filter": {"limit": "25"}}, 25, id="Limit 25 values"),
         ],
     )
     def test_valid_limit_filter(self, test_request_filter, expected_limit_value):