Merge branch 'master' into bugfix/fix-distinct-filter-#141

.github/workflows/ci-build.yml

@@ -50,6 +50,10 @@ jobs:
       run: |
         sed -i -e "s/^payara_user: \"glassfish\"/payara_user: \"runner\"/" icat-ansible/group_vars/all/vars.yml
 
+    # Force hostname to localhost - bug fix for previous ICAT Ansible issues on Actions
+    - name: Change hostname to localhost
+      run: sudo hostname -b localhost
+
     # Create local instance of ICAT
     - name: Run ICAT Ansible Playbook
       run: |

README.md

@@ -334,6 +334,13 @@ PowerShell:
 > flask run
 ```
 
+The Flask app can be configured so that code changes are monitored and the server will
+reload itself when a change is detected. This setting can be toggled using
+`flask_reloader` in `config.json`. This is useful for development purposes. It should be
+noted that when this setting is enabled, the API will go through the startup process
+twice. In the case of the ICAT backend, this could dramatically increase startup time if
+the API is configured with a large initial client pool size.
+
 
 ## Authentication
 Each request requires a valid session ID to be provided in the Authorization header.

datagateway_api/common/backends.py

@@ -24,7 +24,6 @@ def create_backend(backend_type):
     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

@@ -40,6 +40,12 @@ def get_db_url(self):
         except KeyError:
             sys.exit("Missing config value, DB_URL")
 
+    def is_flask_reloader(self):
+        try:
+            return self.config["flask_reloader"]
+        except KeyError:
+            sys.exit("Missing config value, flask_reloader")
+
     def get_icat_url(self):
         try:
             return self.config["ICAT_URL"]

datagateway_api/config.json.example

@@ -1,6 +1,7 @@
 {
   "backend": "db",
   "DB_URL": "mysql+pymysql://icatdbuser:icatdbuserpw@localhost:3306/icatdb",
+  "flask_reloader": false,
   "ICAT_URL": "https://localhost:8181",
   "icat_check_cert": false,
   "log_level": "WARN",

datagateway_api/src/main.py

@@ -23,5 +23,8 @@
 
 if __name__ == "__main__":
     app.run(
-        host=config.get_host(), port=config.get_port(), debug=config.is_debug_mode(),
+        host=config.get_host(),
+        port=config.get_port(),
+        debug=config.is_debug_mode(),
+        use_reloader=config.is_flask_reloader(),
     )

test/icat/endpoints/test_create_icat.py

@@ -118,3 +118,39 @@ def test_invalid_existing_data_create(
         )
 
         assert test_response.status_code == 400
+
+    def test_valid_rollback_behaviour(
+        self, flask_test_app_icat, valid_icat_credentials_header,
+    ):
+        request_body = [
+            {
+                "name": "Test Investigation DG API Testing Name Test",
+                "title": "My New Investigation with Title",
+                "visitId": "Visit ID for Testing",
+                "facility": 1,
+                "type": 1,
+            },
+            {
+                "name": "Invalid Investigation for testing",
+                "title": "My New Investigation with Title",
+                "visitId": "Visit ID for Testing",
+                "doi": "_" * 256,
+                "facility": 1,
+                "type": 1,
+            },
+        ]
+
+        create_response = flask_test_app_icat.post(
+            "/investigations", headers=valid_icat_credentials_header, json=request_body,
+        )
+
+        get_response = flask_test_app_icat.get(
+            '/investigations?where={"title": {"eq": "'
+            f'{request_body[0]["title"]}'
+            '"}}',
+            headers=valid_icat_credentials_header,
+        )
+        get_response_json = prepare_icat_data_for_assertion(get_response.json)
+
+        assert create_response.status_code == 400
+        assert get_response_json == []

test/icat/endpoints/test_update_by_id_icat.py

@@ -11,6 +11,7 @@ def test_valid_update_with_id(
         update_data_json = {
             "doi": "Test Data Identifier",
             "summary": "Test Summary",
+            "startDate": "2019-01-04 01:01:01+00:00",
         }
         single_investigation_test_data[0].update(update_data_json)
 

test/icat/endpoints/test_update_multiple_icat.py

@@ -111,3 +111,44 @@ def test_invalid_attribute_update(
         )
 
         assert test_response.status_code == 400
+
+    def test_valid_rollback_behaviour(
+        self,
+        flask_test_app_icat,
+        valid_icat_credentials_header,
+        multiple_investigation_test_data,
+    ):
+        """
+        Testing the rollback functionality when an `ICATValidationError` is thrown when
+        trying to update data as per the request body.
+
+        In this test, the first dictionary in the request body contains valid data. This
+        record should be successfully updated. The second dictionary contains data that
+        will throw an ICAT related exception. At this point, the rollback behaviour
+        should execute, restoring the state of the first record (i.e. un-updating it)
+        """
+
+        request_body = [
+            {
+                "id": multiple_investigation_test_data[0]["id"],
+                "summary": "An example summary for an investigation used for testing.",
+            },
+            {"id": multiple_investigation_test_data[1]["id"], "doi": "_" * 256},
+        ]
+
+        update_response = flask_test_app_icat.patch(
+            "/investigations", headers=valid_icat_credentials_header, json=request_body,
+        )
+
+        # Get first entity that would've been successfully updated to ensure the changes
+        # were rolled back when the ICATValidationError occurred for the second entity
+        # in the request body
+        get_response = flask_test_app_icat.get(
+            f"/investigations/{multiple_investigation_test_data[0]['id']}",
+            headers=valid_icat_credentials_header,
+        )
+        get_response_json = prepare_icat_data_for_assertion([get_response.json])
+
+        assert update_response.status_code == 500
+        # RHS encased in a list as prepare_icat_data_for_assertion() always returns list
+        assert get_response_json == [multiple_investigation_test_data[0]]

test/icat/filters/test_where_filter.py

@@ -18,6 +18,7 @@ class TestICATWhereFilter:
             pytest.param("gt", 5, "> '5'", id="greater than"),
             pytest.param("gte", 5, ">= '5'", id="greater than or equal"),
             pytest.param("in", [1, 2, 3, 4], "in (1, 2, 3, 4)", id="in a list"),
+            pytest.param("in", [], "in (NULL)", id="empty list"),
         ],
     )
     def test_valid_operations(

test/icat/test_helpers.py

@@ -0,0 +1,34 @@
+import pytest
+
+from datagateway_api.common.exceptions import BadRequestError
+from datagateway_api.common.icat.helpers import get_icat_entity_name_as_camel_case
+
+
+class TestICATHelpers:
+    """Testing the helper functions which aren't covered in the endpoint tests"""
+
+    @pytest.mark.parametrize(
+        "input_entity_name, expected_entity_name",
+        [
+            pytest.param("User", "user", id="singular single word entity name"),
+            pytest.param(
+                "PublicStep", "publicStep", id="singular two word entity name",
+            ),
+            pytest.param(
+                "PermissibleStringValue",
+                "permissibleStringValue",
+                id="singular multi-word entity name",
+            ),
+        ],
+    )
+    def test_valid_get_icat_entity_name_as_camel_case(
+        self, icat_client, input_entity_name, expected_entity_name,
+    ):
+        camel_case_entity_name = get_icat_entity_name_as_camel_case(
+            icat_client, input_entity_name,
+        )
+        assert camel_case_entity_name == expected_entity_name
+
+    def test_invalid_get_icat_entity_name_as_camel_case(self, icat_client):
+        with pytest.raises(BadRequestError):
+            get_icat_entity_name_as_camel_case(icat_client, "UnknownEntityName")

test/test_backends.py

@@ -14,13 +14,19 @@ class TestBackends:
             pytest.param("python_icat", PythonICATBackend, id="Python ICAT Backend"),
         ],
     )
-    def test_backend_creation(self, backend_name, backend_type):
+    def test_valid_backend_creation(self, backend_name, backend_type):
         test_backend = create_backend(backend_name)
 
         assert type(test_backend) == backend_type
 
+    def test_invalid_backend_creation(self):
+        with pytest.raises(SystemExit):
+            create_backend("invalid_backend_name")
+
     def test_abstract_class(self):
-        """Test the `Backend` abstract class has all the required classes for the API"""
+        """
+        Test the `Backend` abstract class has all required abstract methods for the API
+        """
         Backend.__abstractmethods__ = set()
 
         class DummyBackend(Backend):

test/test_config.py

@@ -42,6 +42,16 @@ def test_invalid_db_url(self, invalid_config):
             invalid_config.get_db_url()
 
 
+class TestIsFlaskReloader:
+    def test_valid_flask_reloader(self, valid_config):
+        flask_reloader = valid_config.is_flask_reloader()
+        assert flask_reloader is False
+
+    def test_invalid_flask_reloader(self, invalid_config):
+        with pytest.raises(SystemExit):
+            invalid_config.is_flask_reloader()
+
+
 class TestICATURL:
     def test_valid_icat_url(self, valid_config):
         icat_url = valid_config.get_icat_url()
@@ -121,7 +131,7 @@ def test_valid_port(self, valid_config):
 
     def test_invalid_port(self, invalid_config):
         with pytest.raises(SystemExit):
-            invalid_config.get_icat_url()
+            invalid_config.get_port()
 
 
 class TestGetTestUserCredentials:

test/test_exceptions.py

@@ -0,0 +1,81 @@
+import pytest
+
+from datagateway_api.common.exceptions import (
+    ApiError,
+    AuthenticationError,
+    BadRequestError,
+    DatabaseError,
+    FilterError,
+    MissingCredentialsError,
+    MissingRecordError,
+    MultipleIncludeError,
+    PythonICATError,
+)
+
+
+class TestExceptions:
+    @pytest.mark.parametrize(
+        "exception_class, expected_message",
+        [
+            pytest.param(
+                AuthenticationError, "Authentication error", id="AuthenticationError",
+            ),
+            pytest.param(BadRequestError, "Bad request", id="BadRequestError"),
+            pytest.param(DatabaseError, "Database error", id="DatabaseError"),
+            pytest.param(FilterError, "Invalid filter requested", id="FilterError"),
+            pytest.param(
+                MissingCredentialsError,
+                "No credentials provided in auth header",
+                id="MissingCredentialsError",
+            ),
+            pytest.param(
+                MissingRecordError, "No such record in table", id="MissingRecordError",
+            ),
+            pytest.param(
+                MultipleIncludeError,
+                "Bad request, only one include filter may be given per request",
+                id="MultipleIncludeError",
+            ),
+            pytest.param(PythonICATError, "Python ICAT error", id="PythonICATError"),
+        ],
+    )
+    def test_valid_exception_message(self, exception_class, expected_message):
+        assert exception_class().args[0] == expected_message
+
+    @pytest.mark.parametrize(
+        "exception_class, expected_status_code",
+        [
+            pytest.param(ApiError, 500, id="ApiError"),
+            pytest.param(AuthenticationError, 403, id="AuthenticationError"),
+            pytest.param(BadRequestError, 400, id="BadRequestError"),
+            pytest.param(DatabaseError, 500, id="DatabaseError"),
+            pytest.param(FilterError, 400, id="FilterError"),
+            pytest.param(MissingCredentialsError, 401, id="MissingCredentialsError"),
+            pytest.param(MissingRecordError, 404, id="MissingRecordError"),
+            pytest.param(MultipleIncludeError, 400, id="MultipleIncludeError"),
+            pytest.param(PythonICATError, 500, id="PythonICATError"),
+        ],
+    )
+    def test_valid_exception_status_code(self, exception_class, expected_status_code):
+        assert exception_class().status_code == expected_status_code
+
+    @pytest.mark.parametrize(
+        "exception_class",
+        [
+            pytest.param(ApiError, id="ApiError"),
+            pytest.param(AuthenticationError, id="AuthenticationError"),
+            pytest.param(BadRequestError, id="BadRequestError"),
+            pytest.param(DatabaseError, id="DatabaseError"),
+            pytest.param(FilterError, id="FilterError"),
+            pytest.param(MissingCredentialsError, id="MissingCredentialsError"),
+            pytest.param(MissingRecordError, id="MissingRecordError"),
+            pytest.param(MultipleIncludeError, id="MultipleIncludeError"),
+            pytest.param(PythonICATError, id="PythonICATError"),
+        ],
+    )
+    def test_valid_raise_exception(self, exception_class):
+        def raise_exception():
+            raise exception_class()
+
+        with pytest.raises(exception_class):
+            raise_exception()

test/test_get_entity_object.py

@@ -0,0 +1,28 @@
+import pytest
+
+from datagateway_api.common.database.models import FACILITY, INVESTIGATION, JOB
+from datagateway_api.common.exceptions import ApiError
+from datagateway_api.common.helpers import get_entity_object_from_name
+
+
+class TestGetEntityObject:
+    @pytest.mark.parametrize(
+        "entity_name, expected_object_type",
+        [
+            pytest.param(
+                "investigation", type(INVESTIGATION), id="singular entity name",
+            ),
+            pytest.param("jobs", type(JOB), id="plural entity name, 's' added"),
+            pytest.param(
+                "facilities", type(FACILITY), id="plural entity name, 'y' to 'ies'",
+            ),
+        ],
+    )
+    def test_valid_get_entity_object_from_name(self, entity_name, expected_object_type):
+        database_entity = get_entity_object_from_name(entity_name)
+
+        assert type(database_entity) == expected_object_type
+
+    def test_invalid_get_entity_object_from_name(self):
+        with pytest.raises(ApiError):
+            get_entity_object_from_name("Application1234s")

test/test_query_filter.py

@@ -0,0 +1,18 @@
+from datagateway_api.common.filters import QueryFilter
+
+
+class TestQueryFilter:
+    def test_abstract_class(self):
+        """Test the `QueryFilter` class has all required abstract methods"""
+
+        QueryFilter.__abstractmethods__ = set()
+
+        class DummyQueryFilter(QueryFilter):
+            pass
+
+        qf = DummyQueryFilter()
+
+        apply_filter = "apply_filter"
+
+        assert qf.precedence is None
+        assert qf.apply_filter(apply_filter) is None