Merge pull request #191 from ral-facilities/feature/test-multiple-bac…

…kends-#150

Add Tests for ICAT Backend

.gitignore

@@ -6,3 +6,4 @@ config.json
 .vscode/
 .nox/
 .python-version
+.coverage

config.json.example

@@ -8,5 +8,7 @@
   "debug_mode": false,
   "generate_swagger": false,
   "host": "127.0.0.1",
-  "port": "5000"
+  "port": "5000",
+  "test_user_credentials": {"username": "root", "password": "pw"},
+  "test_mechanism": "simple"
 }

datagateway_api/common/backend.py

@@ -172,7 +172,7 @@ def get_facility_cycles_for_instrument_count_with_filters(
         pass
 
     @abstractmethod
-    def get_investigations_for_instrument_in_facility_cycle_with_filters(
+    def get_investigations_for_instrument_facility_cycle_with_filters(
         self, session_id, instrument_id, facilitycycle_id, filters,
     ):
         """
@@ -188,7 +188,7 @@ def get_investigations_for_instrument_in_facility_cycle_with_filters(
         pass
 
     @abstractmethod
-    def get_investigation_count_for_instrument_facility_cycle_with_filters(
+    def get_investigation_count_instrument_facility_cycle_with_filters(
         self, session_id, instrument_id, facilitycycle_id, filters,
     ):
         """

datagateway_api/common/backends.py

@@ -1,17 +1,30 @@
 import sys
 
-from datagateway_api.common.backend import Backend
-from datagateway_api.common.config import config
 from datagateway_api.common.database.backend import DatabaseBackend
 from datagateway_api.common.icat.backend import PythonICATBackend
 
 
-backend_type = config.get_backend_type()
+def create_backend(backend_type):
+    """
+    Create an instance of a backend dependent on the value parsed into the function. The
+    value will typically be from the contents of `config.json`, however when creating a
+    backend during automated tests the value will be from the Flask app's config (which
+    will be set in the API's config at `common.config`
 
-if backend_type == "db":
-    backend = DatabaseBackend()
-elif backend_type == "python_icat":
-    backend = PythonICATBackend()
-else:
-    sys.exit(f"Invalid config value '{backend_type}' for config option backend")
-    backend = Backend()
+    The API will exit if a valid value isn't given.
+
+    :param backend_type: The type of backend that should be created and used for the API
+    :type backend_type: :class:`str`
+    :return: Either an instance of `common.database.backend.DatabaseBackend` or
+        `common.icat.backend.PythonICATBackend`
+    """
+
+    if backend_type == "db":
+        backend = DatabaseBackend()
+    elif backend_type == "python_icat":
+        backend = PythonICATBackend()
+    else:
+        # Might turn to a warning so the abstract class can be tested?
+        sys.exit(f"Invalid config value '{backend_type}' for config option backend")
+
+    return backend

datagateway_api/common/config.py

@@ -10,18 +10,30 @@
 
 
 class Config(object):
-    def __init__(self):
-        config_path = Path(__file__).parent.parent.parent / "config.json"
-        with open(config_path) as target:
+    def __init__(self, path=Path(__file__).parent.parent.parent / "config.json"):
+        self.path = path
+        with open(self.path) as target:
             self.config = json.load(target)
-        target.close()
 
     def get_backend_type(self):
         try:
             return self.config["backend"]
         except KeyError:
             sys.exit("Missing config value, backend")
 
+    def set_backend_type(self, backend_type):
+        """
+        This setter is used as a way for automated tests to set the backend type. The
+        API can detect if the Flask app setup is from an automated test by checking the
+        app's config for a `TEST_BACKEND`. If this value exists (a KeyError will be
+        raised when the API is run normally, which will then grab the backend type from
+        `config.json`), it needs to be set using this function. This is required because
+        creating filters in the `QueryFilterFactory` is backend-specific so the backend
+        type must be fetched. This must be done using this module (rather than directly
+        importing and checking the Flask app's config) to avoid circular import issues.
+        """
+        self.config["backend"] = backend_type
+
     def get_db_url(self):
         try:
             return self.config["DB_URL"]
@@ -76,6 +88,18 @@ def get_port(self):
         except KeyError:
             sys.exit("Missing config value, port")
 
+    def get_test_user_credentials(self):
+        try:
+            return self.config["test_user_credentials"]
+        except KeyError:
+            sys.exit("Missing config value, test_user_credentials")
+
+    def get_test_mechanism(self):
+        try:
+            return self.config["test_mechanism"]
+        except KeyError:
+            sys.exit("Missing config value, test_mechanism")
+
     def get_icat_properties(self):
         """
         ICAT properties can be retrieved using Python ICAT's client object, however this

datagateway_api/common/constants.py

@@ -1,3 +1,5 @@
+from datetime import datetime
+
 from datagateway_api.common.config import config
 
 
@@ -6,3 +8,4 @@ class Constants:
     ACCEPTED_DATE_FORMAT = "%Y-%m-%d %H:%M:%S"
     PYTHON_ICAT_DISTNCT_CONDITION = "!= null"
     ICAT_PROPERTIES = config.get_icat_properties()
+    TEST_MOD_CREATE_DATETIME = datetime(2000, 1, 1)

datagateway_api/common/database/backend.py

@@ -124,7 +124,7 @@ def get_facility_cycles_for_instrument_count_with_filters(
 
     @requires_session_id
     @queries_records
-    def get_investigations_for_instrument_in_facility_cycle_with_filters(
+    def get_investigations_for_instrument_facility_cycle_with_filters(
         self, session_id, instrument_id, facilitycycle_id, filters,
     ):
         return get_investigations_for_instrument_in_facility_cycle(
@@ -133,7 +133,7 @@ def get_investigations_for_instrument_in_facility_cycle_with_filters(
 
     @requires_session_id
     @queries_records
-    def get_investigation_count_for_instrument_facility_cycle_with_filters(
+    def get_investigation_count_instrument_facility_cycle_with_filters(
         self, session_id, instrument_id, facilitycycle_id, filters,
     ):
         return get_investigations_for_instrument_in_facility_cycle_count(

datagateway_api/common/database/helpers.py

@@ -5,7 +5,10 @@
 
 from sqlalchemy.orm import aliased
 
-from datagateway_api.common.config import config
+from datagateway_api.common.database.filters import (
+    DatabaseIncludeFilter as IncludeFilter,
+    DatabaseWhereFilter as WhereFilter,
+)
 from datagateway_api.common.database.models import (
     FACILITY,
     FACILITYCYCLE,
@@ -16,40 +19,13 @@
 )
 from datagateway_api.common.database.session_manager import session_manager
 from datagateway_api.common.exceptions import (
-    ApiError,
     AuthenticationError,
     BadRequestError,
-    FilterError,
     MissingRecordError,
 )
 from datagateway_api.common.filter_order_handler import FilterOrderHandler
 
 
-backend_type = config.get_backend_type()
-if backend_type == "db":
-    from datagateway_api.common.database.filters import (
-        DatabaseDistinctFieldFilter as DistinctFieldFilter,
-        DatabaseIncludeFilter as IncludeFilter,
-        DatabaseLimitFilter as LimitFilter,
-        DatabaseOrderFilter as OrderFilter,
-        DatabaseSkipFilter as SkipFilter,
-        DatabaseWhereFilter as WhereFilter,
-    )
-elif backend_type == "python_icat":
-    from datagateway_api.common.icat.filters import (
-        PythonICATDistinctFieldFilter as DistinctFieldFilter,
-        PythonICATIncludeFilter as IncludeFilter,
-        PythonICATLimitFilter as LimitFilter,
-        PythonICATOrderFilter as OrderFilter,
-        PythonICATSkipFilter as SkipFilter,
-        PythonICATWhereFilter as WhereFilter,
-    )
-else:
-    raise ApiError(
-        "Cannot select which implementation of filters to import, check the config file"
-        " has a valid backend type",
-    )
-
 log = logging.getLogger()
 
 
@@ -202,42 +178,6 @@ def execute_query(self):
         self.commit_changes()
 
 
-class QueryFilterFactory(object):
-    @staticmethod
-    def get_query_filter(request_filter):
-        """
-        Given a filter return a matching Query filter object
-
-        This factory is not in common.filters so the created filter can be for the
-        correct backend. Moving the factory into that file would mean the filters would
-        be based off the abstract classes (because they're in the same file) which won't
-        enable filters to be unique to the backend
-
-        :param request_filter: dict - The filter to create the QueryFilter for
-        :return: The QueryFilter object created
-        """
-        filter_name = list(request_filter)[0].lower()
-        if filter_name == "where":
-            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)
-        elif filter_name == "order":
-            field = request_filter["order"].split(" ")[0]
-            direction = request_filter["order"].split(" ")[1]
-            return OrderFilter(field, direction)
-        elif filter_name == "skip":
-            return SkipFilter(request_filter["skip"])
-        elif filter_name == "limit":
-            return LimitFilter(request_filter["limit"])
-        elif filter_name == "include":
-            return IncludeFilter(request_filter["include"])
-        elif filter_name == "distinct":
-            return DistinctFieldFilter(request_filter["distinct"])
-        else:
-            raise FilterError(f" Bad filter: {request_filter}")
-
-
 def insert_row_into_table(table, row):
     """
     Insert the given row into its table
@@ -285,7 +225,7 @@ def get_row_by_id(table, id_):
     :return: the record retrieved
     """
     with ReadQuery(table) as read_query:
-        log.info(" Querying %s for record with ID: %d", table.__tablename__, id_)
+        log.info(" Querying %s for record with ID: %s", table.__tablename__, id_)
         where_filter = WhereFilter("ID", id_, "eq")
         where_filter.apply_filter(read_query)
         return read_query.get_single_result()
@@ -299,7 +239,7 @@ def delete_row_by_id(table, id_):
     :param table: the table to be searched
     :param id_: the id of the record to delete
     """
-    log.info(" Deleting row from %s with ID: %d", table.__tablename__, id_)
+    log.info(" Deleting row from %s with ID: %s", table.__tablename__, id_)
     row = get_row_by_id(table, id_)
     with DeleteQuery(table, row) as delete_query:
         delete_query.execute_query()
@@ -389,7 +329,11 @@ def get_first_filtered_row(table, filters):
     :return: the first row matching the filter
     """
     log.info(" Getting first filtered row for %s", table.__tablename__)
-    return get_rows_by_filter(table, filters)[0]
+    try:
+        result = get_rows_by_filter(table, filters)[0]
+    except IndexError:
+        raise MissingRecordError()
+    return result
 
 
 def get_filtered_row_count(table, filters):

datagateway_api/common/filters.py

@@ -1,7 +1,7 @@
 from abc import ABC, abstractmethod
 import logging
 
-from datagateway_api.common.exceptions import BadRequestError
+from datagateway_api.common.exceptions import BadRequestError, FilterError
 
 log = logging.getLogger()
 
@@ -55,14 +55,20 @@ class SkipFilter(QueryFilter):
     precedence = 3
 
     def __init__(self, skip_value):
-        self.skip_value = skip_value
+        if skip_value >= 0:
+            self.skip_value = skip_value
+        else:
+            raise FilterError("The value of the skip filter must be positive")
 
 
 class LimitFilter(QueryFilter):
     precedence = 4
 
     def __init__(self, limit_value):
-        self.limit_value = limit_value
+        if limit_value >= 0:
+            self.limit_value = limit_value
+        else:
+            raise FilterError("The value of the limit filter must be positive")
 
 
 class IncludeFilter(QueryFilter):

datagateway_api/common/helpers.py

@@ -6,14 +6,14 @@
 from flask_restful import reqparse
 from sqlalchemy.exc import IntegrityError
 
-from datagateway_api.common.database.helpers import QueryFilterFactory
 from datagateway_api.common.exceptions import (
     ApiError,
     AuthenticationError,
     BadRequestError,
     FilterError,
     MissingCredentialsError,
 )
+from datagateway_api.common.query_filter_factory import QueryFilterFactory
 
 log = logging.getLogger()
 

datagateway_api/common/icat/backend.py

@@ -137,7 +137,7 @@ def get_facility_cycles_for_instrument_count_with_filters(
 
     @requires_session_id
     @queries_records
-    def get_investigations_for_instrument_in_facility_cycle_with_filters(
+    def get_investigations_for_instrument_facility_cycle_with_filters(
         self, session_id, instrument_id, facilitycycle_id, filters, **kwargs,
     ):
         client = kwargs["client"] if kwargs["client"] else create_client()
@@ -147,7 +147,7 @@ def get_investigations_for_instrument_in_facility_cycle_with_filters(
 
     @requires_session_id
     @queries_records
-    def get_investigation_count_for_instrument_facility_cycle_with_filters(
+    def get_investigation_count_instrument_facility_cycle_with_filters(
         self, session_id, instrument_id, facilitycycle_id, filters, **kwargs,
     ):
         client = kwargs["client"] if kwargs["client"] else create_client()

datagateway_api/common/icat/helpers.py

@@ -169,6 +169,7 @@ def update_attributes(old_entity, new_entity):
         - typically if Python ICAT doesn't allow an attribute to be edited (e.g. modId &
         modTime)
     """
+    log.debug("Updating entity attributes: %s", list(new_entity.keys()))
     for key in new_entity:
         try:
             original_data_attribute = getattr(old_entity, key)
@@ -194,8 +195,10 @@ def update_attributes(old_entity, new_entity):
 def push_data_updates_to_icat(entity):
     try:
         entity.update()
-    except (ICATValidationError, ICATInternalError) as e:
+    except ICATInternalError as e:
         raise PythonICATError(e)
+    except ICATValidationError as e:
+        raise BadRequestError(e)
 
 
 def get_entity_by_id(
@@ -517,13 +520,13 @@ def create_entities(client, entity_type, data):
     for entity in created_icat_data:
         try:
             entity.create()
-        except (ICATValidationError, ICATInternalError) as e:
+        except ICATInternalError as e:
             for entity_json in created_data:
                 # Delete any data that has been pushed to ICAT before the exception
                 delete_entity_by_id(client, entity_type, entity_json["id"])
 
             raise PythonICATError(e)
-        except (ICATObjectExistsError, ICATParameterError) as e:
+        except (ICATObjectExistsError, ICATParameterError, ICATValidationError) as e:
             for entity_json in created_data:
                 delete_entity_by_id(client, entity_type, entity_json["id"])
 

datagateway_api/common/icat/query.py

@@ -325,4 +325,4 @@ def flatten_query_included_fields(self, includes):
             ICAT query
         """
 
-        return [m for n in (field.split(".") for field in includes) for m in n]
+        return [m for n in (field.split(".") for field in sorted(includes)) for m in n]