diff --git a/barbican/model/clean.py b/barbican/model/clean.py index 0ddc38e25..8f3f6acc2 100644 --- a/barbican/model/clean.py +++ b/barbican/model/clean.py @@ -29,11 +29,6 @@ log.setup(CONF, 'barbican') LOG = log.getLogger(__name__) -def _exception_is_successful_exit(thrown_exception): - return (isinstance(thrown_exception, SystemExit) and - (thrown_exception.code is None or thrown_exception.code == 0)) - - def cleanup_unassociated_projects(): """Clean up unassociated projects. @@ -311,8 +306,6 @@ def clean_command(sql_url, min_num_days, do_clean_unassociated_projects, :param verbose: If True, log and print more information :param log_file: If set, override the log_file configured """ - # TODO(edtubill) Make unit test for this method - if verbose: # The verbose flag prints out log events to the screen, otherwise # the log events will only go to the log file @@ -350,12 +343,10 @@ def clean_command(sql_url, min_num_days, do_clean_unassociated_projects, repo.commit() except Exception as ex: - if not _exception_is_successful_exit(ex): - LOG.exception('Failed to clean up soft deletions in database.') - LOG.exception(ex.message) - repo.rollback() - cleanup_total = 0 # rollback happened, no entries affected - raise ex + LOG.exception('Failed to clean up soft deletions in database.') + repo.rollback() + cleanup_total = 0 # rollback happened, no entries affected + raise ex finally: stop_watch.stop() elapsed_time = stop_watch.elapsed() diff --git a/barbican/tests/cmd/test_db_cleanup.py b/barbican/tests/cmd/test_db_cleanup.py index 1086cb22f..84dc61484 100644 --- a/barbican/tests/cmd/test_db_cleanup.py +++ b/barbican/tests/cmd/test_db_cleanup.py @@ -20,6 +20,7 @@ from barbican.tests import database_utils as utils from sqlalchemy.exc import IntegrityError import datetime +import mock def _create_project(project_name): @@ -340,6 +341,82 @@ class WhenTestingDBCleanUpCommand(utils.RepositoryTestCase): clean.cleanup_unassociated_projects() self.assertFalse(_entry_exists(project_with_children)) + @mock.patch('barbican.model.clean.cleanup_all') + @mock.patch('barbican.model.clean.soft_delete_expired_secrets') + @mock.patch('barbican.model.clean.cleanup_unassociated_projects') + @mock.patch('barbican.model.clean.repo') + @mock.patch('barbican.model.clean.log') + @mock.patch('barbican.model.clean.CONF') + def test_clean_up_command(self, mock_conf, mock_log, mock_repo, + mock_clean_unc_projects, + mock_soft_del_expire_secrets, mock_clean_all): + """Tests the clean command""" + test_sql_url = "mysql+pymysql://notrealuser:datab@127.0.0.1/barbican't" + min_num_days = 91 + do_clean_unassociated_projects = True + do_soft_delete_expired_secrets = True + verbose = True + test_log_file = "/tmp/sometempfile" + clean.clean_command(test_sql_url, min_num_days, + do_clean_unassociated_projects, + do_soft_delete_expired_secrets, verbose, + test_log_file) + set_calls = [mock.call('debug', True), + mock.call('log_file', test_log_file), + mock.call('sql_connection', test_sql_url)] + mock_conf.set_override.assert_has_calls(set_calls) + + clear_calls = [mock.call('debug'), mock.call('log_file'), + mock.call('sql_connection')] + mock_conf.clear_override.assert_has_calls(clear_calls) + self.assertTrue(mock_repo.setup_database_engine_and_factory.called) + self.assertTrue(mock_repo.commit.called) + self.assertTrue(mock_repo.clear.called) + self.assertTrue(mock_clean_unc_projects.called) + self.assertTrue(mock_soft_del_expire_secrets) + self.assertTrue(mock_clean_all) + + @mock.patch('barbican.model.clean.cleanup_all') + @mock.patch('barbican.model.clean.soft_delete_expired_secrets') + @mock.patch('barbican.model.clean.cleanup_unassociated_projects') + @mock.patch('barbican.model.clean.repo') + @mock.patch('barbican.model.clean.log') + @mock.patch('barbican.model.clean.CONF') + def test_clean_up_command_with_false_args( + self, mock_conf, mock_log, mock_repo, mock_clean_unc_projects, + mock_soft_del_expire_secrets, mock_clean_all): + """Tests the clean command with false args""" + test_sql_url = None + min_num_days = -1 + do_clean_unassociated_projects = False + do_soft_delete_expired_secrets = False + verbose = None + test_log_file = None + clean.clean_command(test_sql_url, min_num_days, + do_clean_unassociated_projects, + do_soft_delete_expired_secrets, verbose, + test_log_file) + mock_conf.set_override.assert_not_called() + mock_conf.clear_override.assert_not_called() + self.assertTrue(mock_repo.setup_database_engine_and_factory.called) + self.assertTrue(mock_repo.commit.called) + self.assertTrue(mock_repo.clear.called) + self.assertTrue(mock_clean_all) + self.assertFalse(mock_clean_unc_projects.called) + self.assertFalse(mock_soft_del_expire_secrets.called) + + @mock.patch('barbican.model.clean.cleanup_all', + side_effect=IntegrityError("", "", "", "")) + @mock.patch('barbican.model.clean.repo') + @mock.patch('barbican.model.clean.log') + @mock.patch('barbican.model.clean.CONF') + def test_clean_up_command_with_exception( + self, mock_conf, mock_log, mock_repo, mock_clean_all): + """Tests that the clean command throws exceptions""" + args = ("sql", 2, False, False, False, "/tmp/nope") + self.assertRaises(IntegrityError, clean.clean_command, *args) + self.assertTrue(mock_repo.rollback.called) + @_create_project("my integrity error keystone id") def test_db_cleanup_raise_integrity_error(self, project): """Test that an integrity error is thrown