From f855b53824c588946024442cf454f2e5ee149cc3 Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Mar 13 2019 12:57:03 +0000
Subject: [PATCH 1/4] Move some Task methods to QueryProcessor


Related: https://pagure.io/koji/issue/1199

---

diff --git a/hub/kojihub.py b/hub/kojihub.py
index 51255f2..a21e3e2 100644
--- a/hub/kojihub.py
+++ b/hub/kojihub.py
@@ -105,6 +105,15 @@ class Task(object):
         self.id = id
         self.logger = logging.getLogger("koji.hub.Task")
 
+    def _split_fields(self, fields=None):
+        """Helper function for split fields to QueryProcessor's
+       columns/aliases options"""
+        if fields is None:
+            fields = self.fields
+        columns = [f[0] for f in fields]
+        aliases = [f[1] for f in fields]
+        return columns, aliases
+
     def verifyHost(self, host_id=None):
         """Verify that host owns task"""
         if host_id is None:
@@ -401,9 +410,9 @@ class Task(object):
                 tasklist.append(child_id)
 
     def getRequest(self):
-        id = self.id
-        query = """SELECT request FROM task WHERE id = %(id)i"""
-        xml_request = _singleValue(query, locals())
+        query = QueryProcessor(columns=['request'], tables=['task'],
+                               clauses=['id = %(id)i'], values={'id': self.id})
+        xml_request = query.executeOne()
         if xml_request.find('<?xml', 0, 10) == -1:
             # handle older base64 encoded data
             xml_request = base64.b64decode(xml_request)
@@ -440,8 +449,11 @@ class Task(object):
     def getInfo(self, strict=True, request=False):
         """Return information about the task in a dictionary.  If "request" is True,
         the request will be decoded and included in the dictionary."""
-        q = """SELECT %s FROM task WHERE id = %%(id)i""" % ','.join([f[0] for f in self.fields])
-        result = _singleRow(q, vars(self), [f[1] for f in self.fields], strict)
+        columns, aliases = self._split_fields()
+        query = QueryProcessor(columns=columns, aliases=aliases,
+                               tables=['task'], clauses=['id = %(id)i'],
+                               values={'id': self.id})
+        result = query.singleValue(strict=strict)
         if result and request:
             result['request'] = self.getRequest()
         return result
@@ -452,12 +464,15 @@ class Task(object):
         fields = self.fields
         if request:
             fields = fields + (('request', 'request'),)
-        query = """SELECT %s FROM task WHERE parent = %%(id)i""" % ', '.join([f[0] for f in fields])
-        results = _multiRow(query, vars(self), [f[1] for f in fields])
+        columns, aliases = self._split_fields(fields)
+        query = QueryProcessor(columns=columns, aliases=aliases,
+                               tables=['task'], clauses=['parent = %(id)i'],
+                               values={'id': self.id})
+        results = query.execute()
         if request:
             for task in results:
                 if task['request'].find('<?xml', 0, 10) == -1:
-                    #handle older base64 encoded data
+                    # handle older base64 encoded data
                     task['request'] = base64.b64decode(task['request'])
                 # note: loads accepts either bytes or string
                 task['request'] = six.moves.xmlrpc_client.loads(task['request'])[0]

From 42541bd09b117ad2c0693fb7aac62a3407a20c24 Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Mar 13 2019 12:57:03 +0000
Subject: [PATCH 2/4] add strict option to getTaskChildren


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

---

diff --git a/hub/kojihub.py b/hub/kojihub.py
index a21e3e2..2f15033 100644
--- a/hub/kojihub.py
+++ b/hub/kojihub.py
@@ -10606,10 +10606,13 @@ class RootExports(object):
         else:
             return ret
 
-    def getTaskChildren(self, task_id, request=False):
+    def getTaskChildren(self, task_id, request=False, strict=False):
         """Return a list of the children
         of the Task with the given ID."""
         task = Task(task_id)
+        if strict:
+            # check, that task_id is real
+            task.getInfo(strict=True)
         return task.getChildren(request=request)
 
     def getTaskDescendents(self, task_id, request=False):
diff --git a/tests/test_hub/test_get_task_children.py b/tests/test_hub/test_get_task_children.py
new file mode 100644
index 0000000..9ceb0b9
--- /dev/null
+++ b/tests/test_hub/test_get_task_children.py
@@ -0,0 +1,56 @@
+from __future__ import absolute_import
+import mock
+try:
+    import unittest2 as unittest
+except ImportError:
+    import unittest
+
+import koji
+import kojihub
+
+QP = kojihub.QueryProcessor
+
+class TestGetTaskChildren(unittest.TestCase):
+    def setUp(self):
+        self.exports = kojihub.RootExports()
+        self.QueryProcessor = mock.patch('kojihub.QueryProcessor',
+                side_effect=self.getQuery).start()
+        self.queries = []
+
+    def getQuery(self, *args, **kwargs):
+        query = QP(*args, **kwargs)
+        query.execute = mock.MagicMock()
+        query.singleValue = mock.MagicMock()
+        self.queries.append(query)
+        return query
+
+    def tearDown(self):
+        mock.patch.stopall()
+
+    def test_get_task_children_non_existing(self):
+        q = self.getQuery()
+        q.execute.return_value = []
+        self.QueryProcessor.side_effect = [q]
+
+        r = self.exports.getTaskChildren("bogus_item")
+
+        self.assertEqual(r, [])
+
+    def test_get_task_children_non_existing_strict(self):
+        # get task info
+        q = self.getQuery()
+        q.singleValue.side_effect = koji.GenericError
+        self.QueryProcessor.side_effect = [q]
+
+        with self.assertRaises(koji.GenericError):
+            self.exports.getTaskChildren("bogus_item", strict=True)
+
+    def test_get_task_children(self):
+        children = [{'id': 1}]
+        q = self.getQuery()
+        q.execute.return_value = children
+        self.QueryProcessor.side_effect = [q]
+
+        r = self.exports.getTaskChildren("bogus_item")
+
+        self.assertEqual(r, children)

From d501719265f31ed66188b95eb15bf24f97284991 Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
Date: Mar 13 2019 12:57:03 +0000
Subject: [PATCH 3/4] use correct QueryProcessor methods; add strict opt to executeOne


---

diff --git a/hub/kojihub.py b/hub/kojihub.py
index 2f15033..4bdbb76 100644
--- a/hub/kojihub.py
+++ b/hub/kojihub.py
@@ -412,7 +412,7 @@ class Task(object):
     def getRequest(self):
         query = QueryProcessor(columns=['request'], tables=['task'],
                                clauses=['id = %(id)i'], values={'id': self.id})
-        xml_request = query.executeOne()
+        xml_request = query.singleValue()
         if xml_request.find('<?xml', 0, 10) == -1:
             # handle older base64 encoded data
             xml_request = base64.b64decode(xml_request)
@@ -453,7 +453,7 @@ class Task(object):
         query = QueryProcessor(columns=columns, aliases=aliases,
                                tables=['task'], clauses=['id = %(id)i'],
                                values={'id': self.id})
-        result = query.singleValue(strict=strict)
+        result = query.executeOne(strict=strict)
         if result and request:
             result['request'] = self.getRequest()
         return result
@@ -8097,11 +8097,15 @@ SELECT %(col_str)s
         c.execute("CLOSE %s" % cname)
         c.close()
 
-    def executeOne(self):
+    def executeOne(self, strict=False):
         results = self.execute()
         if isinstance(results, list):
             if len(results) > 0:
+                if strict and len(results) > 1:
+                    raise koji.GenericError('multiple rows returned for a single row query')
                 return results[0]
+            elif strict:
+                raise koji.GenericError('query returned no rows')
             else:
                 return None
         return results

From 2cf8313931b2384e498de94cf971242ac8301277 Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Mar 13 2019 12:57:03 +0000
Subject: [PATCH 4/4] fix tests for updated query


---

diff --git a/tests/test_hub/test_get_task_children.py b/tests/test_hub/test_get_task_children.py
index 9ceb0b9..eab40f6 100644
--- a/tests/test_hub/test_get_task_children.py
+++ b/tests/test_hub/test_get_task_children.py
@@ -20,6 +20,7 @@ class TestGetTaskChildren(unittest.TestCase):
     def getQuery(self, *args, **kwargs):
         query = QP(*args, **kwargs)
         query.execute = mock.MagicMock()
+        query.executeOne = mock.MagicMock()
         query.singleValue = mock.MagicMock()
         self.queries.append(query)
         return query
@@ -39,7 +40,7 @@ class TestGetTaskChildren(unittest.TestCase):
     def test_get_task_children_non_existing_strict(self):
         # get task info
         q = self.getQuery()
-        q.singleValue.side_effect = koji.GenericError
+        q.executeOne.side_effect = koji.GenericError
         self.QueryProcessor.side_effect = [q]
 
         with self.assertRaises(koji.GenericError):