From bc91f6b9c0028fa4234b5c4068381b0ff6decbc8 Mon Sep 17 00:00:00 2001 From: nhertz <nhertz@nrao.edu> Date: Thu, 25 Feb 2021 15:11:32 -0700 Subject: [PATCH] Added new REST endpoint for deleting capability requests; requests with existing executions cannot be deleted (they must be disabled instead, which will have to be implemented) --- .../capability/views/capability_request.py | 44 ++++++++++++++--- services/capability/test/conftest.py | 19 +++++++ .../test/test_capability_request_views.py | 49 +++++++++++++++++++ .../workspaces/test/test_capability_info.py | 22 +++++++++ .../capability/services/capability_info.py | 46 +++++++++++++++++ 5 files changed, 174 insertions(+), 6 deletions(-) diff --git a/services/capability/capability/views/capability_request.py b/services/capability/capability/views/capability_request.py index 3ed61379d..3757f0cb6 100644 --- a/services/capability/capability/views/capability_request.py +++ b/services/capability/capability/views/capability_request.py @@ -4,13 +4,12 @@ File containing definitions for the other half of the capability side of the Workspaces REST API, concerning capability requests """ + from pyramid.httpexceptions import HTTPBadRequest, HTTPNotFound, HTTPPreconditionFailed from pyramid.request import Request from pyramid.response import Response from pyramid.view import view_config -from workspaces.capability.schema import CapabilityRequest - @view_config(route_name="view_capability_request", renderer="json") def view_capability_request(request: Request) -> Response: @@ -19,7 +18,7 @@ def view_capability_request(request: Request) -> Response: URL: capability/{capability_name}/request/{request_id} :param request: GET request - :return: Response with JSON-formatted capability request info if found + :return: 200 OK response with JSON-formatted capability request info if found or 404 response (HTTPNotFound) """ capability_request = request.capability_info.lookup_capability_request( @@ -42,7 +41,7 @@ def create_capability_request(request: Request) -> Response: URL: capability/{capability_name}/request/create :param request: POST request, expecting JSON parameter "parameters" - :return: Response with JSON-formatted info of newly created capability request + :return: 200 OK response with JSON-formatted info of newly created capability request or 400 response (HTTPBadRequest) if expected parameters not given or 412 response (HTTPPreconditionFailed) if capability with given name does not exist and thus cannot be requested @@ -92,7 +91,7 @@ def submit_capability_request(request: Request) -> Response: URL: capability/{capability_name}/request/{request_id}/submit :param request: POST request - :return: Response with + :return: 200 OK response or 412 response (HTTPPreconditionFailed) if capability request with given ID does not exist and thus cannot be submitted """ @@ -102,8 +101,41 @@ def submit_capability_request(request: Request) -> Response: if not capability_request: # Capability request not found - does_not_exist_msg = f"Capability request for {capability_name} with ID {request_id} does not exist. Cannot submit request." + does_not_exist_msg = f"Capability request for {capability_name} with ID {request_id} does not exist. Cannot submit." return HTTPPreconditionFailed(detail=does_not_exist_msg) else: execution = request.capability_service.run_capability(capability_request) return Response(json_body=execution.__json__()) + + +@view_config(route_name="delete_capability_request", renderer="json") +def delete_capability_request(request: Request) -> Response: + """ + Pyramid view that accepts a request to delete a capability request from the face of the archive + URL: capability/{capability_name}/request/{request_id} + + :param request: DELETE request + :return: 200 OK response + or 412 response (HTTPPreconditionFailed) if capability request with given ID does not exist and thus cannot be + deleted + or 400 response (HTTPBadRequest) if capability request with given ID cannot be deleted + """ + capability_name = request.matchdict["capability_name"] + request_id = request.matchdict["request_id"] + capability_request = request.capability_info.lookup_capability_request(request_id) + + if not capability_request: + # Capability request not found + does_not_exist_msg = f"Capability request for {capability_name} with ID {request_id} does not exist. Cannot delete." + return HTTPPreconditionFailed(detail=does_not_exist_msg) + else: + rows_deleted = request.capability_info.delete_capability_request(request_id) + if rows_deleted == -1: + bad_request_msg = ( + f"Capability request for {capability_name} with ID {request_id} cannot be deleted;", + f"it has existing executions.", + ) + return HTTPBadRequest(detail=bad_request_msg) + return Response( + body=f"Capability request for {capability_name} with ID {request_id} successfully deleted." + ) diff --git a/services/capability/test/conftest.py b/services/capability/test/conftest.py index fb86f616e..0a944eef8 100644 --- a/services/capability/test/conftest.py +++ b/services/capability/test/conftest.py @@ -90,6 +90,24 @@ class MockCapabilityInfo(MagicMock): return capability_request return None + def delete_capability_request(self, request_id: int) -> int: + for capability_request in self.capability_requests: + if request_id == capability_request.id: + if self.can_delete_capability_request(request_id): + self.capability_requests.remove(capability_request) + return 1 + else: + return -1 + return 0 + + def can_delete_capability_request(self, request_id) -> bool: + for execution in self.capability_executions: + if execution.capability_request_id == request_id: + print(execution) + print(execution.capability_request_id) + return False + return True + def save_entity(self, entity: Any): if type(entity) is Capability: self.capabilities.append(entity) @@ -150,6 +168,7 @@ class MockExecutionManager: """ execution = CapabilityExecution( state=ExecutionState.Ready.name, + capability_request_id=capability_request.id, version=CapabilityVersion( capability_request_id=capability_request.id, version_number=1, diff --git a/services/capability/test/test_capability_request_views.py b/services/capability/test/test_capability_request_views.py index 8565d6d41..935de60d7 100644 --- a/services/capability/test/test_capability_request_views.py +++ b/services/capability/test/test_capability_request_views.py @@ -135,3 +135,52 @@ def test_submit_capability_request_error( response = submit_capability_request(request_null_capability) assert response.status_code == 412 assert type(response) is HTTPPreconditionFailed + + +def test_delete_capability_request( + test_config: Configurator, request_null_capability: DummyRequest +): + """ + Tests that the delete_capability_request view correctly executes its logic + + :param test_config: Dummy Pyramid Configurator object set up for testing + :param request_null_capability: Dummy Pyramid request object set up with mocked DB access + supporting the null capability + """ + from capability.views.capability_request import delete_capability_request + + request_null_capability.capability_info.capability_executions = [] + request_null_capability.matchdict["capability_name"] = "null" + request_null_capability.matchdict["request_id"] = 0 + response = delete_capability_request(request_null_capability) + assert response.status_code == 200 + + +def test_delete_capability_request_error( + test_config: Configurator, request_null_capability: DummyRequest +): + """ + Tests that the delete_capability_request view correctly executes its logic + + :param test_config: Dummy Pyramid Configurator object set up for testing + :param request_null_capability: Dummy Pyramid request object set up with mocked DB access + supporting the null capability + """ + from capability.views.capability_request import delete_capability_request + + # Request cannot be deleted + request_id = request_null_capability.matchdict["request_id"] = 1 + capability_request = request_null_capability.capability_info.lookup_capability_request( + request_id + ) + 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 == 400 + assert type(response) is HTTPBadRequest + + # Request does not exist + request_null_capability.matchdict["request_id"] = -1 + response = delete_capability_request(request_null_capability) + assert response.status_code == 412 + assert type(response) is HTTPPreconditionFailed diff --git a/shared/workspaces/test/test_capability_info.py b/shared/workspaces/test/test_capability_info.py index 329d386b3..37552bc83 100644 --- a/shared/workspaces/test/test_capability_info.py +++ b/shared/workspaces/test/test_capability_info.py @@ -1,3 +1,4 @@ +from workspaces.capability.schema import CapabilityVersion from workspaces.capability.services.capability_info import CapabilityInfo pytest_plugins = ["testing.utils.conftest"] @@ -24,3 +25,24 @@ def test_edit_capability(mock_capability_info: CapabilityInfo): mock_capability_info.session.update.return_value = 0 assert not mock_capability_info.edit_capability("null", "test", 2) assert mock_capability_info.session.flush.call_count == 4 + + +# def test_delete_request_versions(mock_capability_info: CapabilityInfo): +# """ +# Tests that delete_request_versions correctly deletes capability versions associated with a capability request +# TODO: Implement once mocked capability_info is complete with mocked values for capability requests +# """ +# pass + + +def test_can_delete_capability_request(mock_capability_info: CapabilityInfo): + """ + Tests that can_delete_capability_request correctly determines if a capability request is safe to delete + """ + request = mock_capability_info.create_capability_request("null") + request.capability = mock_capability_info.lookup_capability("null") + # Request has no executions; can be deleted + assert mock_capability_info.can_delete_capability_request(request.id) is True + mock_capability_info.create_execution(request) + # Request not has associated execution; can no longer be deleted + assert mock_capability_info.can_delete_capability_request(request.id) is False diff --git a/shared/workspaces/workspaces/capability/services/capability_info.py b/shared/workspaces/workspaces/capability/services/capability_info.py index 366b5c039..ef68432a2 100644 --- a/shared/workspaces/workspaces/capability/services/capability_info.py +++ b/shared/workspaces/workspaces/capability/services/capability_info.py @@ -177,3 +177,49 @@ class CapabilityInfo(CapabilityInfoIF): def save_execution(self, execution: CapabilityExecutionIF): self.session.add(execution) transaction.commit() + + def delete_request_versions(self, request_id: int) -> int: + """ + Deleting all versions of a capability request. This is executed as preparation for deleting the request itself + + :param request_id: ID of request that's versions will be deleted + :return: Number of rows deleted + """ + rows_changed = ( + self.session.query(CapabilityVersion) + .filter_by(capability_request_id=request_id) + .delete() + ) + self.session.flush() + return rows_changed + + def delete_capability_request(self, request_id: int) -> int: + """ + Delete a specified capability request + + :param request_id: ID of capability request to delete + :return: Number of rows deleted + """ + if self.can_delete_capability_request(request_id): + self.delete_request_versions(request_id) + rows_changed = self.session.query(CapabilityRequest).filter_by(id=request_id).delete() + self.session.flush() + return rows_changed + else: + return -1 + + def can_delete_capability_request(self, request_id: int) -> bool: + """ + Method that checks if a capability request has executions associated with it. If it does, it cannot be deleted. + + :param request_id: ID of capability request to be checked + :return: True if there are no executions associated with request; else False + """ + if ( + self.session.query(CapabilityExecution) + .filter_by(capability_request_id=request_id) + .all() + ): + return False + else: + return True -- GitLab