Create DeleteProcessor class and use it

Fixes: https://pagure.io/koji/issue/3580

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"""
+    delete = DeleteProcessor(table='volume', clauses=['id=%(id)i'], values=volinfo)
-    _dml(delete, 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"""
+    delete = DeleteProcessor(table='maven_builds', clauses=['build_id = %(id)i'], values=old)
-    _dml(delete, old)
+    delete.execute()
-    delete = """DELETE FROM win_builds WHERE build_id = %(id)i"""
+    delete = DeleteProcessor(table='win_builds', clauses=['build_id = %(id)i'], values=old)
-    _dml(delete, old)
+    delete.execute()
-    delete = """DELETE FROM image_builds WHERE build_id = %(id)i"""
+    delete = DeleteProcessor(table='image_builds', clauses=['build_id = %(id)i'], values=old)
-    _dml(delete, old)
+    delete.execute()
-    delete = """DELETE FROM build_types WHERE build_id = %(id)i"""
+    delete = DeleteProcessor(table='build_types', clauses=['build_id = %(id)i'], values=old)
-    _dml(delete, 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"
+    delete = DeleteProcessor(table='build_reservations', clauses=['build_id = %(build_id)i'],
-    _dml(delete, {'build_id': build_id})
+                             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"""
+        delete = DeleteProcessor(table='rpmsigs', clauses=['rpm_id=%(rpm_id)i'],
-        _dml(delete, locals())
+                                 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"""
+        delete = DeleteProcessor(table='rpmsigs', clauses=['rpm_id=%(rpm_id)i'],
-        _dml(delete, locals())
+                                 values={'rpm_id': rpm_id})
-        delete = """DELETE FROM buildroot_listing WHERE rpm_id=%(rpm_id)i"""
+        delete.execute()
-        _dml(delete, locals())
+        delete = DeleteProcessor(table='buildroot_listing', clauses=['rpm_id=%(rpm_id)i'],
-        delete = """DELETE FROM archive_rpm_components WHERE rpm_id=%(rpm_id)i"""
+                                 values={'rpm_id': rpm_id})
-        _dml(delete, locals())
+        delete.execute()
-    delete = """DELETE FROM rpminfo WHERE build_id=%(id)i"""
+        delete = DeleteProcessor(table='archive_rpm_components', clauses=['rpm_id=%(rpm_id)i'],
-    _dml(delete, binfo)
+                                 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"""
+        delete = DeleteProcessor(table='maven_archives', clauses=['archive_id=%(archive_id)i'],
-        _dml(delete, locals())
+                                 values={'archive_id': archive_id})
-        delete = """DELETE FROM win_archives WHERE archive_id=%(archive_id)i"""
+        delete.execute()
-        _dml(delete, locals())
+        delete = DeleteProcessor(table='win_archives', clauses=['archive_id=%(archive_id)i'],
-        delete = """DELETE FROM image_archives WHERE archive_id=%(archive_id)i"""
+                                 values={'archive_id': archive_id})
-        _dml(delete, locals())
+        delete.execute()
-        delete = """DELETE FROM buildroot_archives WHERE archive_id=%(archive_id)i"""
+        delete = DeleteProcessor(table='image_archives', clauses=['archive_id=%(archive_id)i'],
-        _dml(delete, locals())
+                                 values={'archive_id': archive_id})
-        delete = """DELETE FROM archive_rpm_components WHERE archive_id=%(archive_id)i"""
+        delete.execute()
-        _dml(delete, locals())
+        delete = DeleteProcessor(table='buildroot_archives', clauses=['archive_id=%(archive_id)i'],
-        delete = """DELETE FROM archive_components WHERE archive_id=%(archive_id)i"""
+                                 values={'archive_id': archive_id})
-        _dml(delete, locals())
+        delete.execute()
-        delete = """DELETE FROM archive_components WHERE component_id=%(archive_id)i"""
+        delete = DeleteProcessor(table='archive_rpm_components',
-        _dml(delete, locals())
+                                 clauses=['archive_id=%(archive_id)i'],
-    delete = """DELETE FROM archiveinfo WHERE build_id=%(id)i"""
+                                 values={'archive_id': archive_id})
-    _dml(delete, binfo)
+        delete.execute()
-    delete = """DELETE FROM maven_builds WHERE build_id = %(id)i"""
+        delete = DeleteProcessor(table='archive_components', clauses=['archive_id=%(archive_id)i'],
-    _dml(delete, binfo)
+                                 values={'archive_id': archive_id})
-    delete = """DELETE FROM win_builds WHERE build_id = %(id)i"""
+        delete.execute()
-    _dml(delete, binfo)
+        delete = DeleteProcessor(table='archive_components',
-    delete = """DELETE FROM image_builds WHERE build_id = %(id)i"""
+                                 clauses=['component_id=%(archive_id)i'],
-    _dml(delete, binfo)
+                                 values={'archive_id': archive_id})
-    delete = """DELETE FROM build_types WHERE build_id = %(id)i"""
+        delete.execute()
-    _dml(delete, binfo)
+    delete = DeleteProcessor(table='archiveinfo', clauses=['build_id=%(id)i'],
-    delete = """DELETE FROM tag_listing WHERE build_id = %(id)i"""
+                             values=binfo)
-    _dml(delete, 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"
+    delete = DeleteProcessor(table='build_reservations', clauses=['build_id = %(build_id)i'],
-    _dml(delete, {'build_id': build_id})
+                             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"""
+        delete = DeleteProcessor(table='build_notifications', clauses=['id=%(id)i'],
-        _dml(delete, locals())
+                                 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"""
+        delete = DeleteProcessor(table='build_notifications_block', clauses=['id=%(id)i'],
-        _dml(delete, locals())
+                                 values={'id': id})
+        delete.execute()
 
     def _prepareSearchTerms(self, terms, matchType):
         r"""Process the search terms before passing them to the database.

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
+        delete = DeleteProcessor(table='user_krb_principals',
-                    WHERE user_id = %(user_id)i
+                                 clauses=['user_id = %(user_id)i',
-                    AND krb_principal = %(krb_principal)s"""
+                                          'krb_principal = %(krb_principal)s'],
-        cursor.execute(delete, locals())
+                                 values={'user_id': user_id, 'krb_principal': krb_principal})
+        delete.execute()
         context.cnx.commit()
         return user_id
 

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 "<DeleteProcessor: %r>" % 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.

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"
+        delete = DeleteProcessor(table='proton_queue',
-                  % max_age)
+                                 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:

tests/test_hub/test_delete_notification.py

@@ -4,29 +4,15 @@ import unittest
 import koji
 import kojihub
 
-QP = kojihub.QueryProcessor
+DP = kojihub.DeleteProcessor
-IP = kojihub.InsertProcessor
-UP = kojihub.UpdateProcessor
 
 
 class TestDeleteNotifications(unittest.TestCase):
-    def getInsert(self, *args, **kwargs):
+    def getDelete(self, *args, **kwargs):
-        insert = IP(*args, **kwargs)
+        delete = DP(*args, **kwargs)
-        insert.execute = mock.MagicMock()
+        delete.execute = mock.MagicMock()
-        self.inserts.append(insert)
+        self.deletes.append(delete)
-        return insert
+        return delete
-
-    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 setUp(self):
         self.context = mock.patch('kojihub.context').start()
@@ -34,17 +20,9 @@ class TestDeleteNotifications(unittest.TestCase):
             'EmailDomain': 'test.domain.com',
             'NotifyOnSuccess': True,
         }
-
+        self.DeleteProcessor = mock.patch('kojihub.DeleteProcessor',
-        self.QueryProcessor = mock.patch('kojihub.QueryProcessor',
+                                          side_effect=self.getDelete).start()
-                                         side_effect=self.getQuery).start()
+        self.deletes = []
-        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.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()

tests/test_hub/test_delete_notification_block.py

@@ -4,29 +4,15 @@ import unittest
 import koji
 import kojihub
 
-QP = kojihub.QueryProcessor
+DP = kojihub.DeleteProcessor
-IP = kojihub.InsertProcessor
-UP = kojihub.UpdateProcessor
 
 
 class TestDeleteNotificationsBlocks(unittest.TestCase):
-    def getInsert(self, *args, **kwargs):
+    def getDelete(self, *args, **kwargs):
-        insert = IP(*args, **kwargs)
+        delete = DP(*args, **kwargs)
-        insert.execute = mock.MagicMock()
+        delete.execute = mock.MagicMock()
-        self.inserts.append(insert)
+        self.deletes.append(delete)
-        return insert
+        return delete
-
-    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 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',
+        self.DeleteProcessor = mock.patch('kojihub.DeleteProcessor',
-                                         side_effect=self.getQuery).start()
+                                          side_effect=self.getDelete).start()
-        self.queries = []
+        self.deletes = []
-        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.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()

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):
+    def getDelete(self, *args, **kwargs):
-        query = QP(*args, **kwargs)
+        delete = DP(*args, **kwargs)
-        query.execute = mock.MagicMock()
+        delete.execute = mock.MagicMock()
-        self.queries.append(query)
+        self.deletes.append(delete)
-        return query
+        return delete
 
     def setUp(self):
-        self.QueryProcessor = mock.patch('kojihub.QueryProcessor',
+        self.DeleteProcessor = mock.patch('kojihub.DeleteProcessor',
-                                         side_effect=self.getQuery).start()
+                                          side_effect=self.getDelete).start()
-        self.queries = []
+        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):
-    def test_rpm_not_existing(self, dml):
         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):
-    def test_not_all_sig_and_not_sigkey(self, dml):
         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):
-    def test_external_repo(self, dml):
         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):
-    def test_empty_query_sign(self, dml):
         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'])

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(
+        query = QueryProcessor(tables=['build_reservations'], clauses=clauses,
-            tables=['build_reservations'],
+                               opts={'countOnly': True})
-            clauses=[f"created < NOW() - '{age} days'::interval"],
-            opts={'countOnly': True})
         rows = query.execute()
         print(f"Deleting {rows} build reservations")
 
     if not test:
-        cursor.execute(
+        delete = DeleteProcessor(table='build_reservations', clauses=clauses)
-            f"DELETE FROM build_reservations WHERE created < NOW() - '{age} days'::interval")
+        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
+        # cascades
-        cursor.execute("DELETE FROM task WHERE method = 'tagNotification' AND "
+        delete = DeleteProcessor(table='task', clauses=clauses)
-                       f"completion_time < NOW() - '{age} days'::interval")
+        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 = DeleteProcessor(table='standard_buildroot',
-        "DELETE FROM standard_buildroot WHERE task_id IN (SELECT task_id FROM temp_scratch_tasks)")
+                             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})")
+        delete = DeleteProcessor(table='buildroot_listing',
-        cursor.execute(f"DELETE {q}")
+                                 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")