From e7dd4b36f9f7e24b8de2f50198e1d9a7f6efaf04 Mon Sep 17 00:00:00 2001
From: Mike McLean <mikem@redhat.com>
Date: Jun 21 2017 15:03:50 +0000
Subject: PR#465 Don't allow not-null empty arch/userID in listHosts


Merges #465
https://pagure.io/koji/pull-request/465
Fixes #463
https://pagure.io/koji/issue/463

---

diff --git a/hub/kojihub.py b/hub/kojihub.py
index 9581f97..4fce02b 100644
--- a/hub/kojihub.py
+++ b/hub/kojihub.py
@@ -10532,27 +10532,31 @@ class RootExports(object):
 
         clauses = []
         joins = []
-        if arches != None:
+        if arches is not None:
+            if not arches:
+                raise koji.GenericError('arches option cannot be empty')
             # include the regex constraints below so we can match 'ppc' without
             # matching 'ppc64'
             if not (isinstance(arches, list) or isinstance(arches, tuple)):
                 arches = [arches]
             archClause = [r"""arches ~ E'\\m%s\\M'""" % arch for arch in arches]
             clauses.append('(' + ' OR '.join(archClause) + ')')
-        if channelID != None:
+        if channelID is not None:
+            channelID = get_channel_id(channelID, strict=True)
             joins.append('host_channels on host.id = host_channels.host_id')
             clauses.append('host_channels.channel_id = %(channelID)i')
-        if ready != None:
+        if ready is not None:
             if ready:
                 clauses.append('ready is true')
             else:
                 clauses.append('ready is false')
-        if enabled != None:
+        if enabled is not None:
             if enabled:
                 clauses.append('enabled is true')
             else:
                 clauses.append('enabled is false')
-        if userID != None:
+        if userID is not None:
+            userID = get_user(userID, strict=True)['id']
             clauses.append('user_id = %(userID)i')
 
         query = QueryProcessor(columns=fields, tables=['host'],
diff --git a/tests/test_hub/test_list_hosts.py b/tests/test_hub/test_list_hosts.py
new file mode 100644
index 0000000..62f2002
--- /dev/null
+++ b/tests/test_hub/test_list_hosts.py
@@ -0,0 +1,114 @@
+import unittest
+import mock
+
+import koji
+import kojihub
+
+QP = kojihub.QueryProcessor
+
+
+class TestListHosts(unittest.TestCase):
+
+    def setUp(self):
+        self.QueryProcessor = mock.patch('kojihub.QueryProcessor',
+                side_effect=self.get_query).start()
+        self.queries = []
+        self.exports = kojihub.RootExports()
+
+    def tearDown(self):
+        mock.patch.stopall()
+
+    def get_query(self, *args, **kwargs):
+        query = QP(*args, **kwargs)
+        query.execute = mock.MagicMock()
+        self.queries.append(query)
+        return query
+
+    def test_list_hosts_simple(self):
+        self.exports.listHosts()
+
+        self.assertEqual(len(self.queries), 1)
+        query = self.queries[0]
+        self.assertEqual(query.tables, ['host'])
+        self.assertEqual(query.joins, [])
+        self.assertEqual(query.clauses, [])
+
+    @mock.patch('kojihub.get_user')
+    def test_list_hosts_user_id(self, get_user):
+        get_user.return_value = {'id': 99}
+        self.exports.listHosts(userID=99)
+
+        self.assertEqual(len(self.queries), 1)
+        query = self.queries[0]
+        self.assertEqual(query.tables, ['host'])
+        self.assertEqual(query.joins, [])
+        self.assertEqual(query.clauses, ['user_id = %(userID)i'])
+
+    @mock.patch('kojihub.get_channel_id')
+    def test_list_hosts_channel_id(self, get_channel_id):
+        get_channel_id.return_value = 2
+        self.exports.listHosts(channelID=2)
+
+        self.assertEqual(len(self.queries), 1)
+        query = self.queries[0]
+        self.assertEqual(query.tables, ['host'])
+        self.assertEqual(query.joins, ['host_channels on host.id = host_channels.host_id'])
+        self.assertEqual(query.clauses, ['host_channels.channel_id = %(channelID)i'])
+
+    def test_list_hosts_single_arch(self):
+        self.exports.listHosts(arches='x86_64')
+
+        self.assertEqual(len(self.queries), 1)
+        query = self.queries[0]
+        self.assertEqual(query.tables, ['host'])
+        self.assertEqual(query.joins, [])
+        self.assertEqual(query.clauses, [r"""(arches ~ E'\\mx86_64\\M')"""])
+
+    def test_list_hosts_multi_arch(self):
+        self.exports.listHosts(arches=['x86_64', 's390'])
+
+        self.assertEqual(len(self.queries), 1)
+        query = self.queries[0]
+        self.assertEqual(query.tables, ['host'])
+        self.assertEqual(query.joins, [])
+        self.assertEqual(query.clauses, [r"""(arches ~ E'\\mx86_64\\M' OR arches ~ E'\\ms390\\M')"""])
+
+    def test_list_hosts_bad_arch(self):
+        with self.assertRaises(koji.GenericError):
+            self.exports.listHosts(arches='')
+
+    def test_list_hosts_ready(self):
+        self.exports.listHosts(ready=1)
+
+        self.assertEqual(len(self.queries), 1)
+        query = self.queries[0]
+        self.assertEqual(query.tables, ['host'])
+        self.assertEqual(query.joins, [])
+        self.assertEqual(query.clauses, ['ready is true'])
+
+    def test_list_hosts_nonready(self):
+        self.exports.listHosts(ready=0)
+
+        self.assertEqual(len(self.queries), 1)
+        query = self.queries[0]
+        self.assertEqual(query.tables, ['host'])
+        self.assertEqual(query.joins, [])
+        self.assertEqual(query.clauses, ['ready is false'])
+
+    def test_list_hosts_enabled(self):
+        self.exports.listHosts(enabled=1)
+
+        self.assertEqual(len(self.queries), 1)
+        query = self.queries[0]
+        self.assertEqual(query.tables, ['host'])
+        self.assertEqual(query.joins, [])
+        self.assertEqual(query.clauses, ['enabled is true'])
+
+    def test_list_hosts_disabled(self):
+        self.exports.listHosts(enabled=0)
+
+        self.assertEqual(len(self.queries), 1)
+        query = self.queries[0]
+        self.assertEqual(query.tables, ['host'])
+        self.assertEqual(query.joins, [])
+        self.assertEqual(query.clauses, ['enabled is false'])