Merge pull request #158 from ral-facilities/feature/icat-order-filter…

…-#140

Implement Order Filter for Python ICAT Backend

common/database/filters.py

@@ -8,6 +8,8 @@
 )
 from common.exceptions import FilterError
 
+from sqlalchemy import asc, desc
+
 
 class DatabaseWhereFilter(WhereFilter):
     def __init__(self, field, value, operation):

common/database/helpers.py

@@ -3,7 +3,6 @@
 from abc import ABC, abstractmethod
 from functools import wraps
 
-from sqlalchemy import asc, desc
 from sqlalchemy.orm import aliased
 
 from common.exceptions import (
@@ -25,7 +24,7 @@
     SESSION,
 )
 from common.session_manager import session_manager
-from common.filters import FilterOrderHandler
+from common.filter_order_handler import FilterOrderHandler
 from common.config import config
 
 backend_type = config.get_backend_type()

common/filter_order_handler.py

@@ -0,0 +1,31 @@
+class FilterOrderHandler(object):
+    """
+    The FilterOrderHandler takes in filters, sorts them according to the order of
+    operations, then applies them.
+    """
+
+    def __init__(self):
+        self.filters = []
+
+    def add_filter(self, filter):
+        self.filters.append(filter)
+
+    def add_filters(self, filters):
+        self.filters.extend(filters)
+
+    def sort_filters(self):
+        """
+        Sorts the filters according to the order of operations
+        """
+        self.filters.sort(key=lambda x: x.precedence)
+
+    def apply_filters(self, query):
+        """
+        Given a query apply the filters the handler has in the correct order.
+
+        :param query: The query to have filters applied to
+        """
+        self.sort_filters()
+
+        for filter in self.filters:
+            filter.apply_filter(query)

common/filters.py

@@ -81,34 +81,3 @@ class IncludeFilter(QueryFilter):
 
     def __init__(self, included_filters):
         self.included_filters = included_filters["include"]
-
-
-class FilterOrderHandler(object):
-    """
-    The FilterOrderHandler takes in filters, sorts them according to the order of
-    operations, then applies them.
-    """
-
-    def __init__(self):
-        self.filters = []
-
-    def add_filter(self, filter):
-        self.filters.append(filter)
-
-    def add_filters(self, filters):
-        self.filters.extend(filters)
-
-    def sort_filters(self):
-        """
-        Sorts the filters according to the order of operations
-        """
-        self.filters.sort(key=lambda x: x.precedence)
-
-    def apply_filters(self, query):
-        """
-        Given a query apply the filters the handler has in the correct order.
-        :param query: The query to have filters applied to
-        """
-        self.sort_filters()
-        for filter in self.filters:
-            filter.apply_filter(query)

common/icat/filters.py

@@ -9,7 +9,6 @@
     IncludeFilter,
 )
 from common.exceptions import FilterError
-from common.icat.helpers import create_condition
 
 log = logging.getLogger()
 
@@ -19,31 +18,56 @@ def __init__(self, field, value, operation):
         super().__init__(field, value, operation)
 
     def apply_filter(self, query):
-
+        log.info("Creating condition for ICAT where filter")
         if self.operation == "eq":
-            where_filter = create_condition(self.field, "=", self.value)
+            where_filter = self.create_condition(self.field, "=", self.value)
         elif self.operation == "like":
-            where_filter = create_condition(self.field, "like", self.value)
+            where_filter = self.create_condition(self.field, "like", self.value)
         elif self.operation == "lt":
-            where_filter = create_condition(self.field, "<", self.value)
+            where_filter = self.create_condition(self.field, "<", self.value)
         elif self.operation == "lte":
-            where_filter = create_condition(self.field, "<=", self.value)
+            where_filter = self.create_condition(self.field, "<=", self.value)
         elif self.operation == "gt":
-            where_filter = create_condition(self.field, ">", self.value)
+            where_filter = self.create_condition(self.field, ">", self.value)
         elif self.operation == "gte":
-            where_filter = create_condition(self.field, ">=", self.value)
+            where_filter = self.create_condition(self.field, ">=", self.value)
         elif self.operation == "in":
-            where_filter = create_condition(self.field, "in", tuple(self.value))
+            where_filter = self.create_condition(self.field, "in", tuple(self.value))
         else:
             raise FilterError(f"Bad operation given to where filter: {self.operation}")
 
+        log.debug("ICAT Where Filter: %s", where_filter)
         try:
+            log.info("Adding ICAT where filter to query")
             query.addConditions(where_filter)
         except ValueError:
             raise FilterError(
                 "Something went wrong when adding WHERE filter to ICAT query"
             )
 
+    @staticmethod
+    def create_condition(attribute_name, operator, value):
+        """
+        Construct and return a Python dictionary containing conditions to be used in a
+        Query object
+
+        :param attribute_name: Attribute name to search
+        :type attribute_name: :class:`str`
+        :param operator: Operator to use when filtering the data
+        :type operator: :class:`str`
+        :param value: What ICAT will use to filter the data
+        :type value: :class:`str` or :class:`tuple` (when using an IN expression)
+        :return: Condition (of type :class:`dict`) ready to be added to a Python ICAT
+            Query object
+        """
+
+        conditions = {}
+        # Removing quote marks when doing conditions with IN expressions
+        jpql_value = f"{value}" if isinstance(value, tuple) else f"'{value}'"
+        conditions[attribute_name] = f"{operator} {jpql_value}"
+
+        return conditions
+
 
 class PythonICATDistinctFieldFilter(DistinctFieldFilter):
     def __init__(self, fields):
@@ -54,11 +78,24 @@ def apply_filter(self, query):
 
 
 class PythonICATOrderFilter(OrderFilter):
+    # Used to append the order tuples across all filters in a single request
+    result_order = []
+
     def __init__(self, field, direction):
-        super().__init__(field, direction)
+        # Python ICAT doesn't automatically uppercase the direction, errors otherwise
+        super().__init__(field, direction.upper())
 
     def apply_filter(self, query):
-        pass
+        PythonICATOrderFilter.result_order.append((self.field, self.direction))
+        log.debug("Result Order: %s", PythonICATOrderFilter.result_order)
+
+        try:
+            log.info("Adding order filter")
+            query.setOrder(PythonICATOrderFilter.result_order)
+        except ValueError as e:
+            # Typically either invalid attribute(s) or attribute(s) contains 1-many
+            # relationship
+            raise FilterError(e)
 
 
 class PythonICATSkipFilter(SkipFilter):

common/icat/helpers.py

@@ -10,8 +10,9 @@
     MissingRecordError,
     PythonICATError,
 )
-from common.filters import FilterOrderHandler
+from common.filter_order_handler import FilterOrderHandler
 from common.constants import Constants
+from common.icat.filters import PythonICATOrderFilter, PythonICATWhereFilter
 
 
 log = logging.getLogger()
@@ -206,29 +207,6 @@ def get_python_icat_entity_name(client, database_table_name):
     return python_icat_entity_name
 
 
-def create_condition(attribute_name, operator, value):
-    """
-    Construct and return a Python dictionary containing conditions to be used in a
-    Query object
-
-    :param attribute_name: Attribute name to search
-    :type attribute_name: :class:`str`
-    :param operator: Operator to use when filtering the data
-    :type operator: :class:`str`
-    :param value: What ICAT will use to filter the data
-    :type value: :class:`str` or :class:`tuple` (when using an IN expression)
-    :return: Condition (of type :class:`dict`) ready to be added to a Python ICAT Query
-        object
-    """
-
-    conditions = {}
-    # Removing quote marks when doing conditions with IN expressions
-    jpql_value = f"{value}" if isinstance(value, tuple) else f"'{value}'"
-    conditions[attribute_name] = f"{operator} {jpql_value}"
-
-    return conditions
-
-
 def str_to_datetime_object(icat_attribute, data):
     """
     Where data is stored as dates in ICAT (which this function determines), convert 
@@ -321,7 +299,7 @@ def get_entity_by_id(client, table_name, id_, return_json_formattable_data):
     """
 
     # Set query condition for the selected ID
-    id_condition = create_condition("id", "=", id_)
+    id_condition = PythonICATWhereFilter.create_condition("id", "=", id_)
 
     selected_entity_name = get_python_icat_entity_name(client, table_name)
 
@@ -382,10 +360,25 @@ def update_entity_by_id(client, table_name, id_, new_data):
 
 
 def get_entity_with_filters(client, table_name, filters):
+    """
+    Gets all the records of a given entity, based on the filters provided in the request
+
+    :param client: ICAT client containing an authenticated user
+    :type client: :class:`icat.client.Client`
+    :param table_name: Table name to extract which entity to use
+    :type table_name: :class:`str`
+    :param filters: The list of filters to be applied to the request
+    :type filters: List of specific implementations :class:`QueryFilter`
+    :return: The list of records of the given entity, using the filters to restrict the
+        result of the query
+    """
+
     selected_entity_name = get_python_icat_entity_name(client, table_name)
     query = construct_icat_query(client, selected_entity_name)
+
     filter_handler = FilterOrderHandler()
     filter_handler.add_filters(filters)
+    clear_order_filters(filter_handler.filters)
     filter_handler.apply_filters(query)
 
     data = execute_icat_query(client, query, True)
@@ -394,3 +387,19 @@ def get_entity_with_filters(client, table_name, filters):
         raise MissingRecordError("No results found")
     else:
         return data
+
+
+def clear_order_filters(filters):
+    """
+    Checks if any order filters have been added to the request and resets the variable
+    used to manage which attribute(s) to use for sorting results.
+    
+    A reset is required because Python ICAT overwrites (as opposed to appending to it)
+    the query's order list every time one is added to the query.
+
+    :param filters: The list of filters to be applied to the request
+    :type filters: List of specific implementations :class:`QueryFilter`
+    """
+
+    if any(isinstance(filter, PythonICATOrderFilter) for filter in filters):
+        PythonICATOrderFilter.result_order = []

src/resources/entities/entity_endpoint.py

@@ -1,16 +1,6 @@
 from flask import request
 from flask_restful import Resource
 
-from common.database.helpers import (
-    get_rows_by_filter,
-    create_rows_from_json,
-    patch_entities,
-    get_row_by_id,
-    delete_row_by_id,
-    update_row_from_id,
-    get_filtered_row_count,
-    get_first_filtered_row,
-)
 from common.helpers import (
     get_session_id_from_auth_header,
     get_filters_from_query_string,