From e09f6f0a6d9e119a8b6086ebac4f33cb5449bc8c Mon Sep 17 00:00:00 2001
From: Jana Cupova <root@localhost>
Date: Dec 20 2022 11:48:19 +0000
Subject: Next rewrite Select/Update queries


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

---

diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py
index ea1160b..c650e62 100644
--- a/kojihub/kojihub.py
+++ b/kojihub/kojihub.py
@@ -760,16 +760,13 @@ def eventCondition(event, table=None):
 
 
 def readInheritanceData(tag_id, event=None):
-    c = context.cnx.cursor()
-    fields = ('parent_id', 'name', 'priority', 'maxdepth', 'intransitive', 'noconfig',
-              'pkg_filter')
-    q = """SELECT %s FROM tag_inheritance JOIN tag ON parent_id = id
-    WHERE %s AND tag_id = %%(tag_id)i
-    ORDER BY priority
-    """ % (",".join(fields), eventCondition(event))
-    c.execute(q, locals())
-    # convert list of lists into a list of dictionaries
-    data = [dict(zip(fields, x)) for x in c.fetchall()]
+    columns = ['parent_id', 'name', 'priority', 'maxdepth', 'intransitive', 'noconfig',
+               'pkg_filter']
+    query = QueryProcessor(tables=['tag_inheritance'], columns=columns,
+                           joins=['tag ON parent_id = id'],
+                           clauses=[eventCondition(event), 'tag_id = %(tag_id)i'],
+                           values={'tag_id': tag_id}, opts={'order': 'priority'})
+    data = query.execute()
     # include the current tag_id as child_id, so we can retrace the inheritance chain later
     for datum in data:
         datum['child_id'] = tag_id
@@ -777,16 +774,13 @@ def readInheritanceData(tag_id, event=None):
 
 
 def readDescendantsData(tag_id, event=None):
-    c = context.cnx.cursor()
-    fields = ('tag_id', 'parent_id', 'name', 'priority', 'maxdepth', 'intransitive', 'noconfig',
-              'pkg_filter')
-    q = """SELECT %s FROM tag_inheritance JOIN tag ON tag_id = id
-    WHERE %s AND parent_id = %%(tag_id)i
-    ORDER BY priority
-    """ % (",".join(fields), eventCondition(event))
-    c.execute(q, locals())
-    # convert list of lists into a list of dictionaries
-    data = [dict(zip(fields, x)) for x in c.fetchall()]
+    columns = ['tag_id', 'parent_id', 'name', 'priority', 'maxdepth', 'intransitive', 'noconfig',
+               'pkg_filter']
+    query = QueryProcessor(tables=['tag_inheritance'], columns=columns,
+                           joins=['tag ON tag_id = id'],
+                           clauses=[eventCondition(event), 'parent_id = %(tag_id)i'],
+                           values={'tag_id': tag_id}, opts={'order': 'priority'})
+    data = query.execute()
     return data
 
 
@@ -13255,10 +13249,11 @@ class RootExports(object):
         koji.plugin.run_callbacks('preBuildStateChange',
                                   attribute='completion_ts', old=ts_old, new=ts, info=buildinfo)
         buildid = buildinfo['id']
-        q = """UPDATE build
-        SET completion_time=TIMESTAMP 'epoch' AT TIME ZONE 'utc' + '%(ts)f seconds'::interval
-        WHERE id=%%(buildid)i""" % locals()
-        _dml(q, locals())
+        update = UpdateProcessor('build',
+                                 clauses=['id = %(buildid)i'], values={'buildid': buildid})
+        update.rawset(completion_time=f"TIMESTAMP 'epoch' AT TIME ZONE 'utc' + "
+                                      f"'{ts:f} seconds'::interval")
+        update.execute()
         buildinfo = get_build(build, strict=True)
         koji.plugin.run_callbacks('postBuildStateChange',
                                   attribute='completion_ts', old=ts_old, new=ts, info=buildinfo)
diff --git a/tests/test_hub/test_group_operations.py b/tests/test_hub/test_group_operations.py
index 56e5dcb..393d627 100644
--- a/tests/test_hub/test_group_operations.py
+++ b/tests/test_hub/test_group_operations.py
@@ -183,11 +183,10 @@ class TestGrouplist(unittest.TestCase):
         self.assertEqual(len(self.inserts), 0)
         self.assertEqual(len(self.updates), 1)
         query = self.queries[0]
-        self.assertEqual(' '.join(str(query).split()),
-                         'SELECT active, group_id, tag_id FROM group_config'
-                         ' WHERE ((active = TRUE))'
-                         ' AND (group_id=%(grp_id)s)'
-                         ' AND (tag_id=%(tag_id)s)')
+        self.assertEqual(query.tables, ['group_config'])
+        self.assertEqual(query.clauses, ['(active = TRUE)', 'group_id=%(grp_id)s',
+                                         'tag_id=%(tag_id)s'])
+        self.assertEqual(query.columns, ['active', 'group_id', 'tag_id'])
         update = self.updates[0]
         self.assertEqual(update.table, 'group_config')
         self.assertEqual(update.data, {'revoke_event': 42, 'revoker_id': 24})
diff --git a/tests/test_hub/test_set_build_timestamp.py b/tests/test_hub/test_set_build_timestamp.py
new file mode 100644
index 0000000..8a2857d
--- /dev/null
+++ b/tests/test_hub/test_set_build_timestamp.py
@@ -0,0 +1,56 @@
+import mock
+import unittest
+
+import kojihub
+import koji
+
+UP = kojihub.UpdateProcessor
+
+
+class TestSetBuildTimestamp(unittest.TestCase):
+
+    def getUpdate(self, *args, **kwargs):
+        update = UP(*args, **kwargs)
+        update.execute = mock.MagicMock()
+        self.updates.append(update)
+        return update
+
+    def setUp(self):
+        self.UpdateProcessor = mock.patch('kojihub.kojihub.UpdateProcessor',
+                                          side_effect=self.getUpdate).start()
+        self.updates = []
+        self.context = mock.patch('kojihub.kojihub.context').start()
+        # It seems MagicMock will not automatically handle attributes that
+        # start with "assert"
+        self.context.session.assertLogin = mock.MagicMock()
+        self.context.session.assertPerm = mock.MagicMock()
+        self.exports = kojihub.RootExports()
+        self.get_build = mock.patch('kojihub.kojihub.get_build').start()
+        self.run_callbacks = mock.patch('koji.plugin.run_callbacks').start()
+        self.buildinfo_old = {'completion_ts': 1671466000.613543, 'id': 111}
+        self.buildinfo_new = {'completion_ts': 1671468684.613543, 'id': 111}
+
+    def tearDown(self):
+        mock.patch.stopall()
+
+    def test_ts_not_correct_type(self):
+        build_id = 111
+        ts = 'wrong-type-ts'
+        self.get_build.return_value = self.buildinfo_old
+        with self.assertRaises(koji.GenericError) as cm:
+            self.exports.setBuildTimestamp(build_id, ts)
+        self.assertEqual("Invalid type for timestamp", str(cm.exception))
+
+    def test_valid(self):
+        build_id = 111
+        ts = 1671468684.613543
+        self.get_build.side_effect = [self.buildinfo_old, self.buildinfo_new]
+        self.exports.setBuildTimestamp(build_id, ts)
+        update = self.updates[0]
+        self.assertEqual(update.table, 'build')
+        self.assertEqual(update.data, {})
+        self.assertEqual(update.rawdata, {
+            'completion_time':
+                f"TIMESTAMP 'epoch' AT TIME ZONE 'utc' + '{ts:f} seconds'::interval"})
+        self.assertEqual(update.clauses, ['id = %(buildid)i'])
+        self.assertEqual(update.values, {'buildid': build_id})

