#3601 Create DeleteProcessor class and use it
Merged a year ago by tkopecek. Opened a year ago by jcupova.
jcupova/koji issue-3580  into  master

file modified
+81 -57
@@ -77,6 +77,7 @@ 

  )

  from koji.db import (

      BulkInsertProcessor,

+     DeleteProcessor,

      InsertProcessor,

      QueryProcessor,

      Savepoint,
@@ -5805,8 +5806,8 @@ 

                             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 @@ 

                                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 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 @@ 

      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 @@ 

      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 @@ 

      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 @@ 

              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 @@ 

                  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 @@ 

                  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.

file modified
+6 -5
@@ -33,7 +33,7 @@ 

  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 @@ 

                  '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

  

file modified
+37
@@ -475,6 +475,43 @@ 

          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.

file modified
+8 -5
@@ -18,7 +18,7 @@ 

  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 @@ 

          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 @@ 

              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:

@@ -4,29 +4,15 @@ 

  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 @@ 

              '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 @@ 

          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 @@ 

          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 @@ 

              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()

@@ -4,29 +4,15 @@ 

  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 @@ 

              '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 @@ 

          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 @@ 

          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 @@ 

              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()

@@ -5,21 +5,21 @@ 

  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 @@ 

      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 @@ 

          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 @@ 

          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'])

file modified
+23 -16
@@ -7,7 +7,7 @@ 

  

  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 @@ 

          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 @@ 

          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 @@ 

          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 @@ 

      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")

^ it is not used anymore

It is duplication of clauses in test/exec. Let's define them just once. Query and Delete should be 1:1 (also in following changes)

rebased onto fc05ae8968131cd5268b75e6afd3b95938311115

a year ago

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

a year ago

rebased onto 6ea0189

a year ago

Metadata Update from @jcupova:
- Pull-request tagged with: testing-done

a year ago

Commit 769ac0e fixes this pull-request

Pull-Request has been merged by tkopecek

a year ago