PR#1256: add strict option to getTaskChildren - koji

koji

#1256 add strict option to getTaskChildren

Merged 3 years ago by mikem. Opened 4 years ago by tkopecek.

tkopecek/koji issue1199 into master

fix tests for updated query

Tomas Kopecek • 4 years ago

2cf8313

use correct QueryProcessor methods; add strict opt to executeOne

Mike McLean • 4 years ago

d501719

add strict option to getTaskChildren

Tomas Kopecek • 4 years ago

42541bd

Move some Task methods to QueryProcessor

Tomas Kopecek • 4 years ago

f855b53

hub/kojihub.py

file modified

+32 -10

		`@@ -105,6 +105,15 @@`
		`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 @@`
		`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.singleValue()`
		`if xml_request.find('<?xml', 0, 10) == -1:`
		`# handle older base64 encoded data`
		`xml_request = base64.b64decode(xml_request)`
		`@@ -440,8 +449,11 @@`
		`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.executeOne(strict=strict)`
		`if result and request:`
		`result['request'] = self.getRequest()`
		`return result`
		`@@ -452,12 +464,15 @@`
		`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]`
		`@@ -8082,11 +8097,15 @@`
		`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`
		`@@ -10591,10 +10610,13 @@`
		`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):`

tests/test_hub/test_get_task_children.py

file added

+57

		`@@ -0,0 +1,57 @@`
		`+ 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.executeOne = 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.executeOne.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)`