#139: Allow a request to have both limit and skip filters

- The skip filter will be merged into the limit filter, with the skip filter being removed from the filter handler
- There is a bug when using a skip filter on it's own, as it'll always return a 404

common/database/filters.py

@@ -10,6 +10,7 @@
 
 from sqlalchemy import asc, desc
 
+
 class DatabaseWhereFilter(WhereFilter):
     def __init__(self, field, value, operation):
         super().__init__(field, value, operation)

common/filter_order_handler.py

@@ -13,6 +13,9 @@ def add_filter(self, filter):
     def add_filters(self, filters):
         self.filters.extend(filters)
 
+    def remove_filter(self, filter):
+        self.filters.remove(filter)
+
     def sort_filters(self):
         """
         Sorts the filters according to the order of operations

common/icat/filters.py

@@ -96,21 +96,41 @@ def apply_filter(self, query):
             )
 
 
-
 class PythonICATSkipFilter(SkipFilter):
     def __init__(self, skip_value):
         super().__init__(skip_value)
 
     def apply_filter(self, query):
-        pass
+        icat_set_limit(query, self.skip_value, False)
 
 
 class PythonICATLimitFilter(LimitFilter):
     def __init__(self, limit_value):
         super().__init__(limit_value)
+        self.skip_value = 0
 
     def apply_filter(self, query):
-        pass
+        icat_set_limit(query, self.skip_value, self.limit_value)
+
+
+def icat_set_limit(query, skip_number, limit_number):
+    """
+    Add limit (utilising skip and count) to an ICAT query
+
+    :param query: ICAT Query object to execute within Python ICAT
+    :type query: :class:`icat.query.Query`
+    :param skip_number:
+    :type skip_number: :class:`int`
+    :param limit_number:
+    :type limit_number: :class:`int
+    :raises FilterError: If the tuple is not of two elements, or the elements aren't of
+        the valid type
+    """
+    try:
+        query.setLimit((skip_number, limit_number))
+    except TypeError as e:
+        # Not a two element tuple as managed by Python ICAT's setLimit()
+        raise FilterError(e)
 
 
 class PythonICATIncludeFilter(IncludeFilter):

common/icat/helpers.py

@@ -12,7 +12,11 @@
 )
 from common.filter_order_handler import FilterOrderHandler
 from common.constants import Constants
-from common.icat.filters import PythonICATLimitFilter, PythonICATWhereFilter
+from common.icat.filters import (
+    PythonICATLimitFilter,
+    PythonICATWhereFilter,
+    PythonICATSkipFilter,
+)
 
 
 log = logging.getLogger()
@@ -364,11 +368,44 @@ def get_entity_with_filters(client, table_name, filters):
     query = construct_icat_query(client, selected_entity_name)
     filter_handler = FilterOrderHandler()
     filter_handler.add_filters(filters)
+    merge_limit_skip_filters(filter_handler)
     filter_handler.apply_filters(query)
+
 
     data = execute_icat_query(client, query, True)
 
     if not data:
         raise MissingRecordError("No results found")
     else:
         return data
+
+
+def merge_limit_skip_filters(filter_handler):
+    """
+    When there are both limit and skip filters in a request, merge them into the limit
+    filter and remove the skip filter from `filter_handler`
+
+    :param filter_handler: The filter handler to apply the filters
+    :param filters: The filters to be applied
+    """
+
+    if any(
+        isinstance(filter, PythonICATSkipFilter) for filter in filter_handler.filters
+    ) and any(
+        isinstance(filter, PythonICATLimitFilter) for filter in filter_handler.filters
+    ):
+        # Merge skip and limit filter into a single limit filter
+        for filter in filter_handler.filters:
+            if isinstance(filter, PythonICATSkipFilter):
+                skip_filter = filter
+                request_skip_value = filter.skip_value
+
+            if isinstance(filter, PythonICATLimitFilter):
+                limit_filter = filter
+
+        if skip_filter and limit_filter:
+            log.info("Merging skip filter with limit filter")
+            limit_filter.skip_value = skip_filter.skip_value
+            log.info("Removing skip filter from list of filters")
+            filter_handler.remove_filter(skip_filter)
+            log.debug("Filters: %s", filter_handler.filters)