diff --git a/hub/kojihub.py b/hub/kojihub.py index 680f9d34..d1276727 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -77,6 +77,7 @@ from koji.util import ( ) from koji.db import ( BulkInsertProcessor, + DeleteProcessor, InsertProcessor, QueryProcessor, Savepoint, @@ -5805,8 +5806,8 @@ def remove_volume(volume): values=volinfo, columns=['id'], opts={'limit': 1}) if query.execute(): raise koji.GenericError('volume %(name)s has build references' % volinfo) - delete = """DELETE FROM volume WHERE id=%(id)i""" - _dml(delete, volinfo) + delete = DeleteProcessor(table='volume', clauses=['id=%(id)i'], values=volinfo) + delete.execute() def list_volumes(): @@ -6113,14 +6114,14 @@ def recycle_build(old, data): old=old['state'], new=data['state'], info=data) # If there is any old build type info, clear it - delete = """DELETE FROM maven_builds WHERE build_id = %(id)i""" - _dml(delete, old) - delete = """DELETE FROM win_builds WHERE build_id = %(id)i""" - _dml(delete, old) - delete = """DELETE FROM image_builds WHERE build_id = %(id)i""" - _dml(delete, old) - delete = """DELETE FROM build_types WHERE build_id = %(id)i""" - _dml(delete, old) + delete = DeleteProcessor(table='maven_builds', clauses=['build_id = %(id)i'], values=old) + delete.execute() + delete = DeleteProcessor(table='win_builds', clauses=['build_id = %(id)i'], values=old) + delete.execute() + delete = DeleteProcessor(table='image_builds', clauses=['build_id = %(id)i'], values=old) + delete.execute() + delete = DeleteProcessor(table='build_types', clauses=['build_id = %(id)i'], values=old) + delete.execute() data['id'] = old['id'] update = UpdateProcessor('build', clauses=['id=%(id)s'], values=data) @@ -6397,8 +6398,9 @@ def get_reservation_token(build_id): def clear_reservation(build_id): '''Remove reservation entry for build''' - delete = "DELETE FROM build_reservations WHERE build_id = %(build_id)i" - _dml(delete, {'build_id': build_id}) + delete = DeleteProcessor(table='build_reservations', clauses=['build_id = %(build_id)i'], + values={'build_id': build_id}) + delete.execute() def cg_init_build(cg, data): @@ -7811,10 +7813,9 @@ def delete_rpm_sig(rpminfo, sigkey=None, all_sigs=False): clauses = ["rpm_id=%(rpm_id)i"] if sigkey is not None: clauses.append("sigkey=%(sigkey)s") - clauses_str = " AND ".join(clauses) - delete = """DELETE FROM rpmsigs WHERE %s""" % clauses_str rpm_id = rinfo['id'] - _dml(delete, locals()) + delete = DeleteProcessor(table='rpmsigs', clauses=clauses, values=locals()) + delete.execute() binfo = get_build(rinfo['build_id']) builddir = koji.pathinfo.build(binfo) list_sigcaches = [] @@ -8569,8 +8570,9 @@ def _delete_build(binfo): query = QueryProcessor(tables=['rpminfo'], columns=['id'], clauses=['build_id=%(build_id)i'], values={'build_id': build_id}, opts={'asList': True}) for (rpm_id,) in query.execute(): - delete = """DELETE FROM rpmsigs WHERE rpm_id=%(rpm_id)i""" - _dml(delete, locals()) + delete = DeleteProcessor(table='rpmsigs', clauses=['rpm_id=%(rpm_id)i'], + values={'rpm_id': rpm_id}) + delete.execute() values = {'build_id': build_id} update = UpdateProcessor('tag_listing', clauses=["build_id=%(build_id)i"], values=values) update.make_revoke() @@ -8611,43 +8613,62 @@ def reset_build(build): query = QueryProcessor(tables=['rpminfo'], columns=['id'], clauses=['build_id=%(id)i'], values=binfo, opts={'asList': True}) for (rpm_id,) in query.execute(): - delete = """DELETE FROM rpmsigs WHERE rpm_id=%(rpm_id)i""" - _dml(delete, locals()) - delete = """DELETE FROM buildroot_listing WHERE rpm_id=%(rpm_id)i""" - _dml(delete, locals()) - delete = """DELETE FROM archive_rpm_components WHERE rpm_id=%(rpm_id)i""" - _dml(delete, locals()) - delete = """DELETE FROM rpminfo WHERE build_id=%(id)i""" - _dml(delete, binfo) + delete = DeleteProcessor(table='rpmsigs', clauses=['rpm_id=%(rpm_id)i'], + values={'rpm_id': rpm_id}) + delete.execute() + delete = DeleteProcessor(table='buildroot_listing', clauses=['rpm_id=%(rpm_id)i'], + values={'rpm_id': rpm_id}) + delete.execute() + delete = DeleteProcessor(table='archive_rpm_components', clauses=['rpm_id=%(rpm_id)i'], + values={'rpm_id': rpm_id}) + delete.execute() + delete = DeleteProcessor(table='rpminfo', clauses=['build_id=%(id)i'], + values=binfo) + delete.execute() query = QueryProcessor(tables=['archiveinfo'], columns=['id'], clauses=['build_id=%(id)i'], values=binfo, opts={'asList': True}) for (archive_id,) in query.execute(): - delete = """DELETE FROM maven_archives WHERE archive_id=%(archive_id)i""" - _dml(delete, locals()) - delete = """DELETE FROM win_archives WHERE archive_id=%(archive_id)i""" - _dml(delete, locals()) - delete = """DELETE FROM image_archives WHERE archive_id=%(archive_id)i""" - _dml(delete, locals()) - delete = """DELETE FROM buildroot_archives WHERE archive_id=%(archive_id)i""" - _dml(delete, locals()) - delete = """DELETE FROM archive_rpm_components WHERE archive_id=%(archive_id)i""" - _dml(delete, locals()) - delete = """DELETE FROM archive_components WHERE archive_id=%(archive_id)i""" - _dml(delete, locals()) - delete = """DELETE FROM archive_components WHERE component_id=%(archive_id)i""" - _dml(delete, locals()) - delete = """DELETE FROM archiveinfo WHERE build_id=%(id)i""" - _dml(delete, binfo) - delete = """DELETE FROM maven_builds WHERE build_id = %(id)i""" - _dml(delete, binfo) - delete = """DELETE FROM win_builds WHERE build_id = %(id)i""" - _dml(delete, binfo) - delete = """DELETE FROM image_builds WHERE build_id = %(id)i""" - _dml(delete, binfo) - delete = """DELETE FROM build_types WHERE build_id = %(id)i""" - _dml(delete, binfo) - delete = """DELETE FROM tag_listing WHERE build_id = %(id)i""" - _dml(delete, binfo) + delete = DeleteProcessor(table='maven_archives', clauses=['archive_id=%(archive_id)i'], + values={'archive_id': archive_id}) + delete.execute() + delete = DeleteProcessor(table='win_archives', clauses=['archive_id=%(archive_id)i'], + values={'archive_id': archive_id}) + delete.execute() + delete = DeleteProcessor(table='image_archives', clauses=['archive_id=%(archive_id)i'], + values={'archive_id': archive_id}) + delete.execute() + delete = DeleteProcessor(table='buildroot_archives', clauses=['archive_id=%(archive_id)i'], + values={'archive_id': archive_id}) + delete.execute() + delete = DeleteProcessor(table='archive_rpm_components', + clauses=['archive_id=%(archive_id)i'], + values={'archive_id': archive_id}) + delete.execute() + delete = DeleteProcessor(table='archive_components', clauses=['archive_id=%(archive_id)i'], + values={'archive_id': archive_id}) + delete.execute() + delete = DeleteProcessor(table='archive_components', + clauses=['component_id=%(archive_id)i'], + values={'archive_id': archive_id}) + delete.execute() + delete = DeleteProcessor(table='archiveinfo', clauses=['build_id=%(id)i'], + values=binfo) + delete.execute() + delete = DeleteProcessor(table='maven_builds', clauses=['build_id=%(id)i'], + values=binfo) + delete.execute() + delete = DeleteProcessor(table='win_builds', clauses=['build_id=%(id)i'], + values=binfo) + delete.execute() + delete = DeleteProcessor(table='image_builds', clauses=['build_id=%(id)i'], + values=binfo) + delete.execute() + delete = DeleteProcessor(table='build_types', clauses=['build_id=%(id)i'], + values=binfo) + delete.execute() + delete = DeleteProcessor(table='tag_listing', clauses=['build_id=%(id)i'], + values=binfo) + delete.execute() binfo['state'] = koji.BUILD_STATES['CANCELED'] update = UpdateProcessor('build', clauses=['id=%(id)s'], values=binfo, data={'state': binfo['state'], 'task_id': None, 'volume_id': 0}) @@ -8697,8 +8718,9 @@ def cancel_build(build_id, cancel_task=True): Task(task_id).cancelFull(strict=False) # remove possible CG reservations - delete = "DELETE FROM build_reservations WHERE build_id = %(build_id)i" - _dml(delete, {'build_id': build_id}) + delete = DeleteProcessor(table='build_reservations', clauses=['build_id = %(build_id)i'], + values={'build_id': build_id}) + delete.execute() build = get_build(build_id, strict=True) koji.plugin.run_callbacks('postBuildStateChange', @@ -13443,8 +13465,9 @@ class RootExports(object): self.hasPerm('admin')): raise koji.GenericError('user %i cannot delete notifications for user %i' % (currentUser['id'], notification['user_id'])) - delete = """DELETE FROM build_notifications WHERE id = %(id)i""" - _dml(delete, locals()) + delete = DeleteProcessor(table='build_notifications', clauses=['id=%(id)i'], + values={'id': id}) + delete.execute() def createNotificationBlock(self, user_id, package_id=None, tag_id=None): """Create notification block. If the user_id does not match the @@ -13490,8 +13513,9 @@ class RootExports(object): self.hasPerm('admin')): raise koji.GenericError('user %i cannot delete notification blocks for user %i' % (currentUser['id'], block['user_id'])) - delete = """DELETE FROM build_notifications_block WHERE id = %(id)i""" - _dml(delete, locals()) + delete = DeleteProcessor(table='build_notifications_block', clauses=['id=%(id)i'], + values={'id': id}) + delete.execute() def _prepareSearchTerms(self, terms, matchType): r"""Process the search terms before passing them to the database. diff --git a/koji/auth.py b/koji/auth.py index ddc84de3..c9a4aeed 100644 --- a/koji/auth.py +++ b/koji/auth.py @@ -33,7 +33,7 @@ import koji from .context import context from .util import to_list -from koji.db import InsertProcessor, QueryProcessor, UpdateProcessor, nextval +from koji.db import DeleteProcessor, InsertProcessor, QueryProcessor, UpdateProcessor, nextval # 1 - load session if provided @@ -670,10 +670,11 @@ class Session(object): 'cannot remove Kerberos Principal:' ' %(krb_principal)s with user %(name)s' % locals()) cursor = context.cnx.cursor() - delete = """DELETE FROM user_krb_principals - WHERE user_id = %(user_id)i - AND krb_principal = %(krb_principal)s""" - cursor.execute(delete, locals()) + delete = DeleteProcessor(table='user_krb_principals', + clauses=['user_id = %(user_id)i', + 'krb_principal = %(krb_principal)s'], + values={'user_id': user_id, 'krb_principal': krb_principal}) + delete.execute() context.cnx.commit() return user_id diff --git a/koji/db.py b/koji/db.py index 6e0b5497..2e08a04e 100644 --- a/koji/db.py +++ b/koji/db.py @@ -475,6 +475,43 @@ class UpdateProcessor(object): return _dml(str(self), self.get_values()) +class DeleteProcessor(object): + """Build an delete statement + + table - the table to delete + clauses - a list of where clauses which will be ANDed together + values - dict of values used in clauses + """ + + def __init__(self, table, clauses=None, values=None): + self.table = table + self.clauses = [] + if clauses: + self.clauses.extend(clauses) + self.values = {} + if values: + self.values.update(values) + + def __str__(self): + parts = ['DELETE FROM %s ' % self.table] + if self.clauses: + parts.append('\nWHERE ') + parts.append(' AND '.join(["( %s )" % c for c in sorted(self.clauses)])) + return ''.join(parts) + + def __repr__(self): + return "" % vars(self) + + def get_values(self): + """Returns unified values dict, including data""" + ret = {} + ret.update(self.values) + return ret + + def execute(self): + return _dml(str(self), self.get_values()) + + class QueryProcessor(object): """ Build a query from its components. diff --git a/plugins/hub/protonmsg.py b/plugins/hub/protonmsg.py index 6fcc57bf..5e4b09fa 100644 --- a/plugins/hub/protonmsg.py +++ b/plugins/hub/protonmsg.py @@ -18,7 +18,7 @@ import koji from koji.context import context from koji.plugin import callback, convert_datetime, ignore_error from kojihub import get_build_type -from koji.db import QueryProcessor, InsertProcessor +from koji.db import QueryProcessor, InsertProcessor, DeleteProcessor CONFIG_FILE = '/etc/koji-hub/plugins/protonmsg.conf' CONFIG = None @@ -371,8 +371,9 @@ def handle_db_msgs(urls, CONFIG): if not max_age: # age in config file is deprecated max_age = CONFIG.getint('queue', 'age', fallback=24) - c.execute("DELETE FROM proton_queue WHERE created_ts < NOW() -'%s hours'::interval" - % max_age) + delete = DeleteProcessor(table='proton_queue', + clauses=[f"created_ts < NOW() -'{max_age:d} hours'::interval"]) + delete.execute() query = QueryProcessor(tables=('proton_queue',), columns=('id', 'address', 'props', 'body::TEXT'), aliases=('id', 'address', 'props', 'body'), @@ -388,8 +389,10 @@ def handle_db_msgs(urls, CONFIG): unsent = {m['id'] for m in _send_msgs(urls, list(msgs), CONFIG)} sent = [m for m in msgs if m['id'] not in unsent] if sent: - c.execute('DELETE FROM proton_queue WHERE id IN %(ids)s', - {'ids': [msg['id'] for msg in sent]}) + ids = [msg['id'] for msg in sent] + delete = DeleteProcessor(table='proton_queue', clauses=['id IN %(ids)s'], + values={'ids': ids}) + delete.execute() finally: # make sure we free the lock try: diff --git a/tests/test_hub/test_delete_notification.py b/tests/test_hub/test_delete_notification.py index 90da1935..6cd82fc6 100644 --- a/tests/test_hub/test_delete_notification.py +++ b/tests/test_hub/test_delete_notification.py @@ -4,29 +4,15 @@ import unittest import koji import kojihub -QP = kojihub.QueryProcessor -IP = kojihub.InsertProcessor -UP = kojihub.UpdateProcessor +DP = kojihub.DeleteProcessor class TestDeleteNotifications(unittest.TestCase): - def getInsert(self, *args, **kwargs): - insert = IP(*args, **kwargs) - insert.execute = mock.MagicMock() - self.inserts.append(insert) - return insert - - def getQuery(self, *args, **kwargs): - query = QP(*args, **kwargs) - query.execute = mock.MagicMock() - self.queries.append(query) - return query - - def getUpdate(self, *args, **kwargs): - update = UP(*args, **kwargs) - update.execute = mock.MagicMock() - self.updates.append(update) - return update + def getDelete(self, *args, **kwargs): + delete = DP(*args, **kwargs) + delete.execute = mock.MagicMock() + self.deletes.append(delete) + return delete def setUp(self): self.context = mock.patch('kojihub.context').start() @@ -34,17 +20,9 @@ class TestDeleteNotifications(unittest.TestCase): 'EmailDomain': 'test.domain.com', 'NotifyOnSuccess': True, } - - self.QueryProcessor = mock.patch('kojihub.QueryProcessor', - side_effect=self.getQuery).start() - self.queries = [] - self.InsertProcessor = mock.patch('kojihub.InsertProcessor', - side_effect=self.getInsert).start() - self.inserts = [] - self.UpdateProcessor = mock.patch('kojihub.UpdateProcessor', - side_effect=self.getUpdate).start() - self.updates = [] - self._dml = mock.patch('kojihub._dml').start() + self.DeleteProcessor = mock.patch('kojihub.DeleteProcessor', + side_effect=self.getDelete).start() + self.deletes = [] self.exports = kojihub.RootExports() self.exports.getLoggedInUser = mock.MagicMock() @@ -60,16 +38,20 @@ class TestDeleteNotifications(unittest.TestCase): self.exports.getBuildNotification.return_value = {'user_id': self.user_id} self.exports.deleteNotification(self.n_id) + self.assertEqual(len(self.deletes), 1) + delete = self.deletes[0] + self.assertEqual(delete.table, 'build_notifications') + self.assertEqual(delete.clauses, ['id=%(id)i']) self.exports.getBuildNotification.assert_called_once_with(self.n_id, strict=True) self.exports.getLoggedInUser.assert_called_once_with() - self._dml.assert_called_once() def test_deleteNotification_missing(self): self.exports.getBuildNotification.side_effect = koji.GenericError with self.assertRaises(koji.GenericError): self.exports.deleteNotification(self.n_id) + self.assertEqual(len(self.deletes), 0) self.exports.getBuildNotification.assert_called_once_with(self.n_id, strict=True) @@ -83,11 +65,9 @@ class TestDeleteNotifications(unittest.TestCase): with self.assertRaises(koji.GenericError) as cm: self.exports.deleteNotification(self.n_id) self.assertEqual('Not logged-in', str(cm.exception)) + self.assertEqual(len(self.deletes), 0) self.exports.getBuildNotification.assert_called_once_with(self.n_id, strict=True) - self.assertEqual(len(self.inserts), 0) - self.assertEqual(len(self.updates), 0) - self.assertEqual(len(self.queries), 0) def test_deleteNotification_no_perm(self): self.exports.getBuildNotification.return_value = {'user_id': self.user_id} @@ -98,6 +78,6 @@ class TestDeleteNotifications(unittest.TestCase): self.exports.deleteNotification(self.n_id) self.assertEqual(f'user 1 cannot delete notifications for user {self.user_id}', str(cm.exception)) + self.assertEqual(len(self.deletes), 0) self.exports.getBuildNotification.assert_called_once_with(self.n_id, strict=True) - self._dml.assert_not_called() diff --git a/tests/test_hub/test_delete_notification_block.py b/tests/test_hub/test_delete_notification_block.py index 5f5379b0..d0887cbf 100644 --- a/tests/test_hub/test_delete_notification_block.py +++ b/tests/test_hub/test_delete_notification_block.py @@ -4,29 +4,15 @@ import unittest import koji import kojihub -QP = kojihub.QueryProcessor -IP = kojihub.InsertProcessor -UP = kojihub.UpdateProcessor +DP = kojihub.DeleteProcessor class TestDeleteNotificationsBlocks(unittest.TestCase): - def getInsert(self, *args, **kwargs): - insert = IP(*args, **kwargs) - insert.execute = mock.MagicMock() - self.inserts.append(insert) - return insert - - def getQuery(self, *args, **kwargs): - query = QP(*args, **kwargs) - query.execute = mock.MagicMock() - self.queries.append(query) - return query - - def getUpdate(self, *args, **kwargs): - update = UP(*args, **kwargs) - update.execute = mock.MagicMock() - self.updates.append(update) - return update + def getDelete(self, *args, **kwargs): + delete = DP(*args, **kwargs) + delete.execute = mock.MagicMock() + self.deletes.append(delete) + return delete def setUp(self): self.context = mock.patch('kojihub.context').start() @@ -35,17 +21,10 @@ class TestDeleteNotificationsBlocks(unittest.TestCase): 'NotifyOnSuccess': True, } - self.QueryProcessor = mock.patch('kojihub.QueryProcessor', - side_effect=self.getQuery).start() - self.queries = [] - self.InsertProcessor = mock.patch('kojihub.InsertProcessor', - side_effect=self.getInsert).start() - self.inserts = [] - self.UpdateProcessor = mock.patch('kojihub.UpdateProcessor', - side_effect=self.getUpdate).start() - self.updates = [] + self.DeleteProcessor = mock.patch('kojihub.DeleteProcessor', + side_effect=self.getDelete).start() + self.deletes = [] self.get_user = mock.patch('kojihub.get_user').start() - self._dml = mock.patch('kojihub._dml').start() self.exports = kojihub.RootExports() self.exports.getLoggedInUser = mock.MagicMock() @@ -61,16 +40,20 @@ class TestDeleteNotificationsBlocks(unittest.TestCase): self.exports.getBuildNotificationBlock.return_value = {'user_id': self.user_id} self.exports.deleteNotificationBlock(self.n_id) + self.assertEqual(len(self.deletes), 1) + delete = self.deletes[0] + self.assertEqual(delete.table, 'build_notifications_block') + self.assertEqual(delete.clauses, ['id=%(id)i']) self.exports.getBuildNotificationBlock.assert_called_once_with(self.n_id, strict=True) self.exports.getLoggedInUser.assert_called_once_with() - self._dml.assert_called_once() def test_deleteNotificationBlock_missing(self): self.exports.getBuildNotificationBlock.side_effect = koji.GenericError with self.assertRaises(koji.GenericError): self.exports.deleteNotificationBlock(self.n_id) + self.assertEqual(len(self.deletes), 0) self.exports.getBuildNotificationBlock.assert_called_once_with(self.n_id, strict=True) @@ -84,11 +67,9 @@ class TestDeleteNotificationsBlocks(unittest.TestCase): with self.assertRaises(koji.GenericError) as cm: self.exports.deleteNotificationBlock(self.n_id) self.assertEqual('Not logged-in', str(cm.exception)) + self.assertEqual(len(self.deletes), 0) self.exports.getBuildNotificationBlock.assert_called_once_with(self.n_id, strict=True) - self.assertEqual(len(self.inserts), 0) - self.assertEqual(len(self.updates), 0) - self.assertEqual(len(self.queries), 0) def test_deleteNotificationBlock_no_perm2(self): self.exports.getBuildNotificationBlock.return_value = {'user_id': self.user_id} @@ -99,6 +80,6 @@ class TestDeleteNotificationsBlocks(unittest.TestCase): self.exports.deleteNotificationBlock(self.n_id) self.assertEqual(f'user 1 cannot delete notification blocks for user {self.user_id}', str(cm.exception)) + self.assertEqual(len(self.deletes), 0) self.exports.getBuildNotificationBlock.assert_called_once_with(self.n_id, strict=True) - self._dml.assert_not_called() diff --git a/tests/test_hub/test_delete_rpm_sig.py b/tests/test_hub/test_delete_rpm_sig.py index ee986185..64fdf437 100644 --- a/tests/test_hub/test_delete_rpm_sig.py +++ b/tests/test_hub/test_delete_rpm_sig.py @@ -5,21 +5,21 @@ import mock import koji import kojihub -QP = kojihub.QueryProcessor +DP = kojihub.DeleteProcessor class TestDeleteRPMSig(unittest.TestCase): - def getQuery(self, *args, **kwargs): - query = QP(*args, **kwargs) - query.execute = mock.MagicMock() - self.queries.append(query) - return query + def getDelete(self, *args, **kwargs): + delete = DP(*args, **kwargs) + delete.execute = mock.MagicMock() + self.deletes.append(delete) + return delete def setUp(self): - self.QueryProcessor = mock.patch('kojihub.QueryProcessor', - side_effect=self.getQuery).start() - self.queries = [] + self.DeleteProcessor = mock.patch('kojihub.DeleteProcessor', + side_effect=self.getDelete).start() + self.deletes = [] self.get_rpm = mock.patch('kojihub.get_rpm').start() self.query_rpm_sigs = mock.patch('kojihub.query_rpm_sigs').start() self.get_build = mock.patch('kojihub.get_build').start() @@ -62,46 +62,39 @@ class TestDeleteRPMSig(unittest.TestCase): def tearDown(self): mock.patch.stopall() - @mock.patch('kojihub._dml') - def test_rpm_not_existing(self, dml): + def test_rpm_not_existing(self): rpm_id = 1234 expected_msg = 'No such rpm: %s' % rpm_id self.get_rpm.side_effect = koji.GenericError("No such rpm: %s" % rpm_id) with self.assertRaises(koji.GenericError) as ex: kojihub.delete_rpm_sig(rpm_id, all_sigs=True) - self.assertEqual(len(self.queries), 0) + self.assertEqual(len(self.deletes), 0) self.assertEqual(ex.exception.args[0], expected_msg) self.get_rpm.assert_called_once_with(rpm_id, strict=True) self.query_rpm_sigs.assert_not_called() - dml.assert_not_called() - @mock.patch('kojihub._dml') - def test_not_all_sig_and_not_sigkey(self, dml): + def test_not_all_sig_and_not_sigkey(self): expected_msg = 'No signature specified' with self.assertRaises(koji.GenericError) as ex: kojihub.delete_rpm_sig(1234) - self.assertEqual(len(self.queries), 0) + self.assertEqual(len(self.deletes), 0) self.assertEqual(ex.exception.args[0], expected_msg) self.get_rpm.assert_not_called() self.query_rpm_sigs.assert_not_called() - dml.assert_not_called() - @mock.patch('kojihub._dml') - def test_external_repo(self, dml): + def test_external_repo(self): rpminfo = 1234 rinfo = {'external_repo_id': 1, 'external_repo_name': 'INTERNAL'} self.get_rpm.return_value = rinfo with self.assertRaises(koji.GenericError) as ex: kojihub.delete_rpm_sig(rpminfo, all_sigs=True) - self.assertEqual(len(self.queries), 0) + self.assertEqual(len(self.deletes), 0) expected_msg = "Not an internal rpm: %s (from %s)" % (rpminfo, rinfo['external_repo_name']) self.assertEqual(ex.exception.args[0], expected_msg) self.get_rpm.assert_called_once_with(rpminfo, strict=True) self.query_rpm_sigs.assert_not_called() - dml.assert_not_called() - @mock.patch('kojihub._dml') - def test_empty_query_sign(self, dml): + def test_empty_query_sign(self): rpminfo = 1234 nvra = "%s-%s-%s.%s" % (self.rinfo['name'], self.rinfo['version'], self.rinfo['release'], self.rinfo['arch']) @@ -110,33 +103,32 @@ class TestDeleteRPMSig(unittest.TestCase): self.query_rpm_sigs.return_value = [] with self.assertRaises(koji.GenericError) as ex: kojihub.delete_rpm_sig(rpminfo, all_sigs=True) - self.assertEqual(len(self.queries), 0) + self.assertEqual(len(self.deletes), 0) self.assertEqual(ex.exception.args[0], expected_msg) self.get_rpm.assert_called_once_with(rpminfo, strict=True) self.query_rpm_sigs.assert_called_once_with(rpm_id=self.rinfo['id'], sigkey=None) - dml.assert_not_called() - @mock.patch('kojihub._dml') @mock.patch('koji.pathinfo.build', return_value='fakebuildpath') @mock.patch('os.remove') - def test_file_not_found_error(self, os_remove, pb, dml): + def test_file_not_found_error(self, os_remove, pb): rpminfo = 2 os_remove.side_effect = FileNotFoundError() self.get_rpm.return_value = self.rinfo self.get_build.return_value = self.buildinfo self.get_user.return_value = self.userinfo self.query_rpm_sigs.return_value = self.queryrpmsigs - r = kojihub.delete_rpm_sig(rpminfo, all_sigs=True) + r = kojihub.delete_rpm_sig(rpminfo, sigkey='testkey') self.assertEqual(r, None) - self.assertEqual(len(self.queries), 0) + delete = self.deletes[0] + self.assertEqual(delete.table, 'rpmsigs') + self.assertEqual(delete.clauses, ["rpm_id=%(rpm_id)i", "sigkey=%(sigkey)s"]) self.get_rpm.assert_called_once_with(rpminfo, strict=True) - self.query_rpm_sigs.assert_called_once_with(rpm_id=self.rinfo['id'], sigkey=None) + self.query_rpm_sigs.assert_called_once_with(rpm_id=self.rinfo['id'], sigkey='testkey') self.get_build.assert_called_once_with(self.rinfo['build_id']) - @mock.patch('kojihub._dml') @mock.patch('koji.pathinfo.build', return_value='fakebuildpath') @mock.patch('os.remove', side_effect=OSError) - def test_not_valid(self, os_remove, pb, dml): + def test_not_valid(self, os_remove, pb): rpminfo = 2 filepath = 'fakebuildpath/data/signed/x86_64/fs_mark-3.3-20.el8.x86_64.rpm' self.get_rpm.return_value = self.rinfo @@ -146,21 +138,26 @@ class TestDeleteRPMSig(unittest.TestCase): with self.assertRaises(koji.GenericError) as ex: kojihub.delete_rpm_sig(rpminfo, all_sigs=True) self.assertEqual(ex.exception.args[0], expected_msg) - self.assertEqual(len(self.queries), 0) + delete = self.deletes[0] + self.assertEqual(delete.table, 'rpmsigs') + self.assertEqual(delete.clauses, ["rpm_id=%(rpm_id)i"]) self.get_rpm.assert_called_once_with(rpminfo, strict=True) self.query_rpm_sigs.assert_called_once_with(rpm_id=self.rinfo['id'], sigkey=None) self.get_build.assert_called_once_with(self.rinfo['build_id']) - @mock.patch('kojihub._dml') @mock.patch('koji.pathinfo.build', return_value='fakebuildpath') @mock.patch('os.remove') - def test_valid(self, os_remove, pb, dml): + def test_valid(self, os_remove, pb): rpminfo = 2 self.get_rpm.return_value = self.rinfo self.get_build.return_value = self.buildinfo self.get_user.return_value = self.userinfo self.query_rpm_sigs.return_value = self.queryrpmsigs kojihub.delete_rpm_sig(rpminfo, all_sigs=True) + self.assertEqual(len(self.deletes), 1) + delete = self.deletes[0] + self.assertEqual(delete.table, 'rpmsigs') + self.assertEqual(delete.clauses, ["rpm_id=%(rpm_id)i"]) self.get_rpm.assert_called_once_with(rpminfo, strict=True) self.query_rpm_sigs.assert_called_once_with(rpm_id=self.rinfo['id'], sigkey=None) self.get_build.assert_called_once_with(self.rinfo['build_id']) diff --git a/util/koji-sweep-db b/util/koji-sweep-db index 919d6991..0c54e855 100755 --- a/util/koji-sweep-db +++ b/util/koji-sweep-db @@ -7,7 +7,7 @@ from koji.context import context import koji import koji.db -from koji.db import QueryProcessor, BulkInsertProcessor +from koji.db import DeleteProcessor, QueryProcessor, BulkInsertProcessor def clean_sessions(cursor, vacuum, test, age, absolute): @@ -21,23 +21,23 @@ def clean_sessions(cursor, vacuum, test, age, absolute): print(f"Deleting {rows} sessions") if not test: - cursor.execute(f"DELETE FROM sessions WHERE {clauses}") + delete = DeleteProcessor(table='sessions', clauses=[clauses]) + delete.execute() if vacuum: cursor.execute("VACUUM ANALYZE sessions") def clean_reservations(cursor, vacuum, test, age): + clauses = [f"created < NOW() - '{age} days'::interval"] if options.verbose: - query = QueryProcessor( - tables=['build_reservations'], - clauses=[f"created < NOW() - '{age} days'::interval"], - opts={'countOnly': True}) + query = QueryProcessor(tables=['build_reservations'], clauses=clauses, + opts={'countOnly': True}) rows = query.execute() print(f"Deleting {rows} build reservations") if not test: - cursor.execute( - f"DELETE FROM build_reservations WHERE created < NOW() - '{age} days'::interval") + delete = DeleteProcessor(table='build_reservations', clauses=clauses) + delete.execute() if vacuum: cursor.execute("VACUUM ANALYZE build_reservations") @@ -51,9 +51,9 @@ def clean_notification_tasks(cursor, vacuum, test, age): print(f"Deleting {rows} tagNotification tasks") if not test: - # cascade - cursor.execute("DELETE FROM task WHERE method = 'tagNotification' AND " - f"completion_time < NOW() - '{age} days'::interval") + # cascades + delete = DeleteProcessor(table='task', clauses=clauses) + delete.execute() if vacuum: cursor.execute("VACUUM ANALYZE task") @@ -112,11 +112,14 @@ def clean_scratch_tasks(cursor, vacuum, test, age): return # delete standard buildroots - cursor.execute( - "DELETE FROM standard_buildroot WHERE task_id IN (SELECT task_id FROM temp_scratch_tasks)") + delete = DeleteProcessor(table='standard_buildroot', + clauses=['task_id IN (SELECT task_id FROM temp_scratch_tasks)']) + delete.execute() # delete tasks finally - cursor.execute("DELETE FROM task WHERE id IN (SELECT task_id FROM temp_scratch_tasks)") + delete = DeleteProcessor(table='task', + clauses=['id IN (SELECT task_id FROM temp_scratch_tasks)']) + delete.execute() if vacuum: cursor.execute("VACUUM ANALYZE standard_buildroot") @@ -133,8 +136,12 @@ def clean_buildroots(cursor, vacuum, test): if not test: q = "FROM buildroot WHERE cg_id IS NULL AND id NOT IN " \ "(SELECT buildroot_id FROM standard_buildroot)" - cursor.execute(f"DELETE FROM buildroot_listing WHERE buildroot_id IN (SELECT id {q})") - cursor.execute(f"DELETE {q}") + delete = DeleteProcessor(table='buildroot_listing', + clauses=[f'buildroot_id IN (SELECT id {q})']) + delete.execute() + clauses = ['cg_id IS NULL AND id NOT IN (SELECT buildroot_id FROM standard_buildroot)'] + delete = DeleteProcessor(table='buildroot', clauses=clauses) + delete.execute() if vacuum: cursor.execute("VACUUM ANALYZE buildroot_listing") cursor.execute("VACUUM ANALYZE buildroot")