From 6eedca5251d8458089e099cd6f6880ae035547a9 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Tue, 6 Apr 2021 17:35:02 +0000 Subject: [PATCH 01/11] #217: Improve comments --- datagateway_api/common/backends.py | 1 - test/test_backends.py | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/datagateway_api/common/backends.py b/datagateway_api/common/backends.py index 1cb92aa0..84993b5f 100644 --- a/datagateway_api/common/backends.py +++ b/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 diff --git a/test/test_backends.py b/test/test_backends.py index dc79e7c9..358ada68 100644 --- a/test/test_backends.py +++ b/test/test_backends.py @@ -20,7 +20,9 @@ def test_backend_creation(self, backend_name, backend_type): assert type(test_backend) == backend_type 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): From 20e4b688ed0d9871dc48df7774d17fbb2bb794ec Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Tue, 6 Apr 2021 17:36:15 +0000 Subject: [PATCH 02/11] #217: Correct config test function call --- test/test_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_config.py b/test/test_config.py index 248764cb..b50f638a 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -121,7 +121,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: From fb57cee963fecdd7829ff185a06addc2825bbb9d Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Tue, 6 Apr 2021 17:44:07 +0000 Subject: [PATCH 03/11] #217: Add tests for QueryFilter abstract class - Similar testing principle to testing the abstract backend class --- test/test_query_filter.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 test/test_query_filter.py diff --git a/test/test_query_filter.py b/test/test_query_filter.py new file mode 100644 index 00000000..ec865390 --- /dev/null +++ b/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 From 7e723be753d263d9b9ed0840a618e6cd3bc0ee4d Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Tue, 6 Apr 2021 18:10:25 +0000 Subject: [PATCH 04/11] #217: Add test case for empty list where filter --- test/icat/filters/test_where_filter.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/icat/filters/test_where_filter.py b/test/icat/filters/test_where_filter.py index 6c101926..6fde3665 100644 --- a/test/icat/filters/test_where_filter.py +++ b/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( From 550bd41b6560c4efc81b131c70cc0dd1294f943a Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Tue, 6 Apr 2021 18:37:47 +0000 Subject: [PATCH 05/11] #217: Update a date attribute on update by ID endpoint --- test/icat/endpoints/test_update_by_id_icat.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/icat/endpoints/test_update_by_id_icat.py b/test/icat/endpoints/test_update_by_id_icat.py index af97f54e..d1c8227b 100644 --- a/test/icat/endpoints/test_update_by_id_icat.py +++ b/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) From b9fc4548b714461e8e7f72c708652bbd932dfc6a Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Tue, 6 Apr 2021 19:04:05 +0000 Subject: [PATCH 06/11] #217: Add tests for get_entity_object_from_name() --- test/test_get_entity_object.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 test/test_get_entity_object.py diff --git a/test/test_get_entity_object.py b/test/test_get_entity_object.py new file mode 100644 index 00000000..19834a2f --- /dev/null +++ b/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") From 285362ae539c3ee24752bb3521c698e18c88ec69 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Wed, 7 Apr 2021 08:29:57 +0000 Subject: [PATCH 07/11] #217: Add tests for camelCase ICAT entity name function --- test/icat/test_helpers.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 test/icat/test_helpers.py diff --git a/test/icat/test_helpers.py b/test/icat/test_helpers.py new file mode 100644 index 00000000..dde50c69 --- /dev/null +++ b/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") From 0aec6da8af043c981f68676c40b585a03455db63 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Wed, 7 Apr 2021 13:11:25 +0000 Subject: [PATCH 08/11] #217: Add tests for custom API exceptions --- test/test_exceptions.py | 81 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 test/test_exceptions.py diff --git a/test/test_exceptions.py b/test/test_exceptions.py new file mode 100644 index 00000000..e9b04b6d --- /dev/null +++ b/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() From af49cd181546759aa1e5b58111387327c4718129 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 8 Apr 2021 08:49:25 +0000 Subject: [PATCH 09/11] #217: Add test case for invalid backend name --- test/test_backends.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/test_backends.py b/test/test_backends.py index 358ada68..36637365 100644 --- a/test/test_backends.py +++ b/test/test_backends.py @@ -14,11 +14,15 @@ 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 required abstract methods for the API From 63b3537df9409705d1a95adf29176671888698dd Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 8 Apr 2021 08:50:59 +0000 Subject: [PATCH 10/11] #217: Add tests for rollback functionality on update and create endpoints --- test/icat/endpoints/test_create_icat.py | 36 ++++++++++++++++ .../endpoints/test_update_multiple_icat.py | 41 +++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/test/icat/endpoints/test_create_icat.py b/test/icat/endpoints/test_create_icat.py index c20aeaba..031eaf3f 100644 --- a/test/icat/endpoints/test_create_icat.py +++ b/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 == [] diff --git a/test/icat/endpoints/test_update_multiple_icat.py b/test/icat/endpoints/test_update_multiple_icat.py index 0cd02415..135bb032 100644 --- a/test/icat/endpoints/test_update_multiple_icat.py +++ b/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]] From e6aee902f2a1ec437f150804bc2a8415011634c3 Mon Sep 17 00:00:00 2001 From: Matthew Richards Date: Thu, 8 Apr 2021 10:46:47 +0000 Subject: [PATCH 11/11] #217: Fix linting issue --- test/icat/endpoints/test_update_multiple_icat.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/icat/endpoints/test_update_multiple_icat.py b/test/icat/endpoints/test_update_multiple_icat.py index 135bb032..050dd88e 100644 --- a/test/icat/endpoints/test_update_multiple_icat.py +++ b/test/icat/endpoints/test_update_multiple_icat.py @@ -133,7 +133,7 @@ def test_valid_rollback_behaviour( "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,}, + {"id": multiple_investigation_test_data[1]["id"], "doi": "_" * 256}, ] update_response = flask_test_app_icat.patch(