From 5194f3873ea04834672d708f9e93d66449e273f9 Mon Sep 17 00:00:00 2001 From: ashwini-orchestral Date: Thu, 2 Sep 2021 14:44:22 +0000 Subject: [PATCH 1/3] Bug fix in modified action delete API --- st2api/st2api/controllers/v1/actions.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/st2api/st2api/controllers/v1/actions.py b/st2api/st2api/controllers/v1/actions.py index 1e8bf7c7b5..368783aa44 100644 --- a/st2api/st2api/controllers/v1/actions.py +++ b/st2api/st2api/controllers/v1/actions.py @@ -264,6 +264,9 @@ def delete(self, ref_or_id, requester_user): "Exception was %s", e, ) + action = ActionAPI.from_model(action_db) + action_db = ActionAPI.to_model(action) + Action.add_or_update(action_db) abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) return From 3900f03269ed2832f1fe1cfa77c2f9af89d4df6a Mon Sep 17 00:00:00 2001 From: ashwini-orchestral Date: Fri, 3 Sep 2021 14:09:16 +0000 Subject: [PATCH 2/3] Adding unit tests --- st2api/st2api/controllers/v1/actions.py | 3 +-- .../tests/unit/controllers/v1/test_actions.py | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/st2api/st2api/controllers/v1/actions.py b/st2api/st2api/controllers/v1/actions.py index 368783aa44..f02d7e762f 100644 --- a/st2api/st2api/controllers/v1/actions.py +++ b/st2api/st2api/controllers/v1/actions.py @@ -264,8 +264,7 @@ def delete(self, ref_or_id, requester_user): "Exception was %s", e, ) - action = ActionAPI.from_model(action_db) - action_db = ActionAPI.to_model(action) + action_db.id = None Action.add_or_update(action_db) abort(http_client.INTERNAL_SERVER_ERROR, six.text_type(e)) return diff --git a/st2api/tests/unit/controllers/v1/test_actions.py b/st2api/tests/unit/controllers/v1/test_actions.py index cd1d8b555c..567a5d0df6 100644 --- a/st2api/tests/unit/controllers/v1/test_actions.py +++ b/st2api/tests/unit/controllers/v1/test_actions.py @@ -626,6 +626,23 @@ def test_delete(self): del_resp = self.__do_delete(self.__get_action_id(post_resp)) self.assertEqual(del_resp.status_int, 204) + @mock.patch("st2api.controllers.v1.actions.delete_action_files_from_pack") + @mock.patch.object( + action_validator, "validate_action", mock.MagicMock(return_value=True) + ) + def test_delete_exception_to_remove_action_files( + self, mock_delete_action_files_from_pack + ): + msg = "No permission to delete action files from disk" + mock_delete_action_files_from_pack.side_effect = PermissionError(msg) + post_resp = self.__do_post(ACTION_1) + with mock.patch("st2api.controllers.v1.actions.Action.add_or_update"): + del_resp = self.__do_delete( + self.__get_action_id(post_resp), expect_errors=True + ) + self.assertEqual(del_resp.status_int, 500) + self.assertEqual(del_resp.json["faultstring"], msg) + @mock.patch.object( action_validator, "validate_action", mock.MagicMock(return_value=True) ) From 65b4827f1d8cca7ee7fb0806ab3accc0b39c584b Mon Sep 17 00:00:00 2001 From: ashwini-orchestral Date: Mon, 6 Sep 2021 13:24:26 +0000 Subject: [PATCH 3/3] Updating unit tests and fixing error code incase of permission error to delete files --- st2api/st2api/controllers/v1/actions.py | 10 +++- .../tests/unit/controllers/v1/test_actions.py | 49 ++++++++++++++++--- 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/st2api/st2api/controllers/v1/actions.py b/st2api/st2api/controllers/v1/actions.py index f02d7e762f..d1ee3d88b8 100644 --- a/st2api/st2api/controllers/v1/actions.py +++ b/st2api/st2api/controllers/v1/actions.py @@ -258,9 +258,17 @@ def delete(self, ref_or_id, requester_user): entry_point=entry_point, metadata_file=metadata_file, ) + + except PermissionError as e: + LOG.error("No permission to delete resource files from disk.") + action_db.id = None + Action.add_or_update(action_db) + abort(http_client.FORBIDDEN, six.text_type(e)) + return + except Exception as e: LOG.error( - "Exception encountered during deleting resource files from disk." + "Exception encountered during deleting resource files from disk. " "Exception was %s", e, ) diff --git a/st2api/tests/unit/controllers/v1/test_actions.py b/st2api/tests/unit/controllers/v1/test_actions.py index 567a5d0df6..af6ad3d91d 100644 --- a/st2api/tests/unit/controllers/v1/test_actions.py +++ b/st2api/tests/unit/controllers/v1/test_actions.py @@ -626,23 +626,58 @@ def test_delete(self): del_resp = self.__do_delete(self.__get_action_id(post_resp)) self.assertEqual(del_resp.status_int, 204) - @mock.patch("st2api.controllers.v1.actions.delete_action_files_from_pack") @mock.patch.object( action_validator, "validate_action", mock.MagicMock(return_value=True) ) - def test_delete_exception_to_remove_action_files( - self, mock_delete_action_files_from_pack - ): - msg = "No permission to delete action files from disk" - mock_delete_action_files_from_pack.side_effect = PermissionError(msg) + def test_delete_permission_error_and_action_reregistered_to_database(self): post_resp = self.__do_post(ACTION_1) - with mock.patch("st2api.controllers.v1.actions.Action.add_or_update"): + + with mock.patch( + "st2api.controllers.v1.actions.delete_action_files_from_pack" + ) as mock_remove_files: + msg = "No permission to delete action files from disk" + mock_remove_files.side_effect = PermissionError(msg) + del_resp = self.__do_delete( + self.__get_action_id(post_resp), expect_errors=True + ) + self.assertEqual(del_resp.status_int, 403) + self.assertEqual(del_resp.json["faultstring"], msg) + + # retrieving reregistered action + get_resp = self.__do_get_actions_by_url_parameter("name", ACTION_1["name"]) + expected_uid = post_resp.json["uid"] + actual_uid = get_resp.json[0]["uid"] + self.assertEqual(actual_uid, expected_uid) + action_id = get_resp.json[0]["id"] + del_resp = self.__do_delete(action_id) + self.assertEqual(del_resp.status_int, 204) + + @mock.patch.object( + action_validator, "validate_action", mock.MagicMock(return_value=True) + ) + def test_delete_exception_and_action_reregistered_to_database(self): + post_resp = self.__do_post(ACTION_1) + + with mock.patch( + "st2api.controllers.v1.actions.delete_action_files_from_pack" + ) as mock_remove_files: + msg = "Exception encountered during removing files from disk" + mock_remove_files.side_effect = Exception(msg) del_resp = self.__do_delete( self.__get_action_id(post_resp), expect_errors=True ) self.assertEqual(del_resp.status_int, 500) self.assertEqual(del_resp.json["faultstring"], msg) + # retrieving reregistered action + get_resp = self.__do_get_actions_by_url_parameter("name", ACTION_1["name"]) + expected_uid = post_resp.json["uid"] + actual_uid = get_resp.json[0]["uid"] + self.assertEqual(actual_uid, expected_uid) + action_id = get_resp.json[0]["id"] + del_resp = self.__do_delete(action_id) + self.assertEqual(del_resp.status_int, 204) + @mock.patch.object( action_validator, "validate_action", mock.MagicMock(return_value=True) )