diff --git a/services/capability/capability/views/capability.py b/services/capability/capability/views/capability.py
index 9dd94bbc4b416e762e9b1d636a03b85911604abd..aef1c7f9015985f54f24e4bb5d5e7d0d499691d1 100644
--- a/services/capability/capability/views/capability.py
+++ b/services/capability/capability/views/capability.py
@@ -160,7 +160,8 @@ def edit_capability(request: Request) -> Response:
if success:
return Response(body="Capability successfully edited!")
else:
- return HTTPExpectationFailed(detail=f"Unable to edit capability {capability_name}.")
+ # Need to raise on failure when a transaction is in-progress so that it rolls back instead of committing
+ raise HTTPExpectationFailed(detail=f"Unable to edit capability {capability_name}.")
@view_config(route_name="enable_capability", renderer="json")
@@ -184,7 +185,8 @@ def enable_capability(request: Request) -> Response:
if success:
return Response(body="Capability successfully enabled!")
else:
- return HTTPExpectationFailed(detail=f"Unable to enable capability {capability_name}.")
+ # Need to raise on failure when a transaction is in-progress so that it rolls back instead of committing
+ raise HTTPExpectationFailed(detail=f"Unable to enable capability {capability_name}.")
@view_config(route_name="disable_capability", renderer="json")
@@ -208,7 +210,8 @@ def disable_capability(request: Request) -> Response:
if success:
return Response(body="Capability successfully disabled!")
else:
- return HTTPExpectationFailed(detail=f"Unable to disable capability {capability_name}.")
+ # Need to raise on failure when a transaction is in-progress so that it rolls back instead of committing
+ raise HTTPExpectationFailed(detail=f"Unable to disable capability {capability_name}.")
@view_config(route_name="pause_capability", renderer="json")
diff --git a/services/capability/capability/views/capability_request.py b/services/capability/capability/views/capability_request.py
index dc4e7f859f1c58b3f817edb787681c6c06246936..30878378f3a1681743a3ba66def00c13d8c4cb3b 100644
--- a/services/capability/capability/views/capability_request.py
+++ b/services/capability/capability/views/capability_request.py
@@ -237,7 +237,8 @@ def close_capability_request(request: Request) -> Response:
request.capability_info.save_entity(capability_request)
except Exception as exc:
detail = f"sealing of {capability_request.id} failed: {exc}"
- return HTTPBadRequest(detail=detail)
+ # Need to raise on failure when a transaction is in-progress so that it rolls back instead of committing
+ raise HTTPBadRequest(detail=detail)
return Response(body=f"Capability request with ID {request_id} successfully closed.")
else:
@@ -277,7 +278,8 @@ def delete_capability_request(request: Request) -> Response:
f"Capability request for {capability_name} with ID {request_id} cannot be deleted;",
"it has existing executions.",
)
- return HTTPBadRequest(detail=bad_request_msg)
+ # Need to raise on failure when a transaction is in-progress so that it rolls back instead of committing
+ raise HTTPBadRequest(detail=bad_request_msg)
return Response(body=f"Capability request for {capability_name} with ID {request_id} successfully deleted.")
@@ -335,7 +337,8 @@ def assign_capability_request(request: Request) -> Response:
request.capability_info.save_entity(capability_request)
except Exception as exc:
detail = f"assignment of {user_name} as {group} to request {request_id} failed: {exc}"
- return HTTPBadRequest(detail=detail)
+ # Need to raise on failure when a transaction is in-progress so that it rolls back instead of committing
+ raise HTTPBadRequest(detail=detail)
return Response(json_body={"resp": response_body})
@@ -409,6 +412,7 @@ def change_srdp_compatibility_version_metadata(request: Request) -> Response:
r = requests.post(change_srdp_url, json=request.json_body)
if r.status_code != requests.codes.ok:
srdp_change_failed_msg = f"SRDP-Compatibility change failed: {r.status_code}"
- return HTTPInternalServerError(detail=srdp_change_failed_msg)
+ # Need to raise on failure when a transaction is in-progress so that it rolls back instead of committing
+ raise HTTPInternalServerError(detail=srdp_change_failed_msg)
return request.json_body
diff --git a/services/capability/capability/views/capability_version.py b/services/capability/capability/views/capability_version.py
index 803231f0983364a2607b48b30a1390db426b1319..6348ba3f6265b28045e8578b0f79a92a646130f9 100644
--- a/services/capability/capability/views/capability_version.py
+++ b/services/capability/capability/views/capability_version.py
@@ -177,7 +177,8 @@ def update_internal_notes(request: Request) -> Response:
)
else:
no_versions_msg = f"Capability request with ID {capability_request_id} has no version {version_id}"
- return HTTPPreconditionFailed(no_versions_msg)
+ # Need to raise on failure when a transaction is in-progress so that it rolls back instead of committing
+ raise HTTPPreconditionFailed(no_versions_msg)
else:
not_found_msg = f"Capability request with ID {capability_request_id} not found."
return HTTPNotFound(detail=not_found_msg)
diff --git a/services/capability/test/test_capability_request_views.py b/services/capability/test/test_capability_request_views.py
index 6ead542bb523d1d28b6869e1bd8b511489e5fe09..6421eec6354d902701f893f4612a9a98c756c15e 100644
--- a/services/capability/test/test_capability_request_views.py
+++ b/services/capability/test/test_capability_request_views.py
@@ -23,6 +23,7 @@ The logic can be found in `capability/views/capability_request.py`.
import http
+import pytest
from capability.views.capability_request import (
create_capability_request,
delete_capability_request,
@@ -210,9 +211,9 @@ def test_delete_capability_request(request_null_capability: DummyRequest):
assert response.status_code == http.HTTPStatus.OK
-def test_delete_capability_request_error(request_null_capability: DummyRequest):
+def test_delete_capability_request_error_undeletable_request(request_null_capability: DummyRequest):
"""
- Tests that the delete_capability_request view correctly executes its logic
+ Tests that the delete_capability_request view correctly executes its logic for a request that can't be deleted
:param request_null_capability: Dummy Pyramid request object set up with mocked DB access
supporting the null capability
@@ -224,11 +225,20 @@ def test_delete_capability_request_error(request_null_capability: DummyRequest):
request_null_capability.matchdict["capability_name"] = "null"
request_null_capability.capability_service.run_capability(capability_request)
- response = delete_capability_request(request_null_capability)
- assert response.status_code == http.HTTPStatus.BAD_REQUEST
- assert isinstance(response, HTTPBadRequest)
+ with pytest.raises(HTTPBadRequest):
+ response = delete_capability_request(request_null_capability)
+ assert response.status_code == http.HTTPStatus.BAD_REQUEST
+ assert isinstance(response, HTTPBadRequest)
+
+
+def test_delete_capability_request_error_nonexistent_request(request_null_capability: DummyRequest):
+ """
+ Tests that the delete_capability_request view correctly executes its logic when a request does not exist
- # Request does not exist
+ :param request_null_capability: Dummy Pyramid request object set up with mocked DB access
+ supporting the null capability
+ """
+ request_null_capability.matchdict["capability_name"] = "null"
request_null_capability.matchdict["request_id"] = -1
response = delete_capability_request(request_null_capability)
assert response.status_code == http.HTTPStatus.PRECONDITION_FAILED
diff --git a/services/capability/test/test_capability_views.py b/services/capability/test/test_capability_views.py
index b6a741dacd031b2e6e3ae8ed31ba1fa11c75771e..a635db87a9e544e9be25bc39b0785f16bf4f3040 100644
--- a/services/capability/test/test_capability_views.py
+++ b/services/capability/test/test_capability_views.py
@@ -20,11 +20,13 @@ Tests that test the view functionality of our capability REST API.
The logic can be found in `capability/views/capability.py`.
"""
import http
+from typing import Optional
import pytest
from pyramid.config import Configurator
from pyramid.httpexceptions import (
HTTPBadRequest,
+ HTTPException,
HTTPExpectationFailed,
HTTPNotFound,
HTTPPreconditionFailed,
@@ -182,7 +184,22 @@ def test_edit_capability(test_config: Configurator, request_null_capability: Dum
assert response_jobs_only.status_code == http.HTTPStatus.OK
-def test_edit_capability_error(test_config: Configurator, request_null_capability: DummyRequest):
+@pytest.mark.parametrize(
+ ["capability_name", "max_jobs", "response_type", "is_raised"],
+ [
+ ("not_found", 3, HTTPPreconditionFailed, False),
+ ("null", None, HTTPBadRequest, False),
+ ("error", 3, HTTPExpectationFailed, True),
+ ],
+)
+def test_edit_capability_error(
+ test_config: Configurator,
+ request_null_capability: DummyRequest,
+ capability_name: str,
+ max_jobs: int | None,
+ response_type: HTTPException,
+ is_raised: bool,
+):
"""
Tests that the edit_capability view correctly responds with exceptions
@@ -192,23 +209,20 @@ def test_edit_capability_error(test_config: Configurator, request_null_capabilit
"""
from capability.views.capability import edit_capability
- request_null_capability.matchdict["capability_name"] = "not_found"
- request_null_capability.json_body = {"max_jobs": 3}
- response_cap_not_exists = edit_capability(request_null_capability)
- assert response_cap_not_exists.status_code == http.HTTPStatus.PRECONDITION_FAILED
- assert type(response_cap_not_exists) is HTTPPreconditionFailed
-
- request_null_capability.matchdict["capability_name"] = "null"
- request_null_capability.json_body = {}
- response_empty = edit_capability(request_null_capability)
- assert response_empty.status_code == http.HTTPStatus.BAD_REQUEST
- assert type(response_empty) is HTTPBadRequest
-
- request_null_capability.matchdict["capability_name"] = "error"
- request_null_capability.json_body = {"max_jobs": 3}
- response_edit_failed = edit_capability(request_null_capability)
- assert response_edit_failed.status_code == http.HTTPStatus.EXPECTATION_FAILED
- assert type(response_edit_failed) is HTTPExpectationFailed
+ request_null_capability.matchdict["capability_name"] = capability_name
+ if max_jobs is not None:
+ request_null_capability.json_body = {"max_jobs": max_jobs}
+ else:
+ request_null_capability.json_body = {}
+ if is_raised:
+ with pytest.raises(response_type):
+ response = edit_capability(request_null_capability)
+ assert response.status_code == response_type.code
+ assert type(response) is response_type
+ else:
+ response = edit_capability(request_null_capability)
+ assert response.status_code == response_type.code
+ assert type(response) is response_type
def test_enable_capability(test_config: Configurator, request_null_capability: DummyRequest):
@@ -226,7 +240,17 @@ def test_enable_capability(test_config: Configurator, request_null_capability: D
assert response.status_code == http.HTTPStatus.OK
-def test_enable_capability_error(test_config: Configurator, request_null_capability: DummyRequest):
+@pytest.mark.parametrize(
+ ["capability_name", "response_type", "is_raised"],
+ [("does_not_exist", HTTPPreconditionFailed, False), ("error", HTTPExpectationFailed, True)],
+)
+def test_enable_capability_error(
+ test_config: Configurator,
+ request_null_capability: DummyRequest,
+ capability_name: str,
+ response_type: HTTPException,
+ is_raised: bool,
+):
"""
Tests that enable_capability view properly responds with HTTP exceptions given bad input
@@ -236,14 +260,16 @@ def test_enable_capability_error(test_config: Configurator, request_null_capabil
"""
from capability.views.capability import enable_capability
- request_null_capability.matchdict["capability_name"] = "does_not_exist"
- response_does_not_exist = enable_capability(request_null_capability)
- assert response_does_not_exist.status_code == http.HTTPStatus.PRECONDITION_FAILED
- assert type(response_does_not_exist) is HTTPPreconditionFailed
- request_null_capability.matchdict["capability_name"] = "error"
- response_enable_failed = enable_capability(request_null_capability)
- assert response_enable_failed.status_code == http.HTTPStatus.EXPECTATION_FAILED
- assert type(response_enable_failed) is HTTPExpectationFailed
+ request_null_capability.matchdict["capability_name"] = capability_name
+ if is_raised:
+ with pytest.raises(response_type):
+ response_enable_failed = enable_capability(request_null_capability)
+ assert response_enable_failed.status_code == response_type.code
+ assert type(response_enable_failed) is response_type
+ else:
+ response_enable_failed = enable_capability(request_null_capability)
+ assert response_enable_failed.status_code == response_type.code
+ assert type(response_enable_failed) is response_type
def test_disable_capability(test_config: Configurator, request_null_capability: DummyRequest):
@@ -261,7 +287,17 @@ def test_disable_capability(test_config: Configurator, request_null_capability:
assert response.status_code == http.HTTPStatus.OK
-def test_disable_capability_error(test_config: Configurator, request_null_capability: DummyRequest):
+@pytest.mark.parametrize(
+ ["capability_name", "response_type", "is_raised"],
+ [("does_not_exist", HTTPPreconditionFailed, False), ("error", HTTPExpectationFailed, True)],
+)
+def test_disable_capability_error(
+ test_config: Configurator,
+ request_null_capability: DummyRequest,
+ capability_name: str,
+ response_type: HTTPException,
+ is_raised: bool,
+):
"""
Tests that disable_capability view properly responds with HTTP exceptions given bad input
@@ -271,11 +307,13 @@ def test_disable_capability_error(test_config: Configurator, request_null_capabi
"""
from capability.views.capability import disable_capability
- request_null_capability.matchdict["capability_name"] = "does_not_exist"
- response_does_not_exist = disable_capability(request_null_capability)
- assert response_does_not_exist.status_code == http.HTTPStatus.PRECONDITION_FAILED
- assert type(response_does_not_exist) is HTTPPreconditionFailed
- request_null_capability.matchdict["capability_name"] = "error"
- response_disable_failed = disable_capability(request_null_capability)
- assert response_disable_failed.status_code == http.HTTPStatus.EXPECTATION_FAILED
- assert type(response_disable_failed) is HTTPExpectationFailed
+ request_null_capability.matchdict["capability_name"] = capability_name
+ if is_raised:
+ with pytest.raises(response_type):
+ response_enable_failed = disable_capability(request_null_capability)
+ assert response_enable_failed.status_code == response_type.code
+ assert type(response_enable_failed) is response_type
+ else:
+ response_enable_failed = disable_capability(request_null_capability)
+ assert response_enable_failed.status_code == response_type.code
+ assert type(response_enable_failed) is response_type