From d05f8c354c56b1b953582f942770fb61604eb7ab Mon Sep 17 00:00:00 2001 From: Jana Cupova Date: Oct 11 2022 12:32:31 +0000 Subject: [PATCH 1/4] Rewrite DB query to Procesors Fixes: https://pagure.io/koji/issue/3511 Fixes: https://pagure.io/koji/issue/3493 --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 5c7a42c..706188a 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -184,12 +184,13 @@ class Task(object): task_id = self.id # getting a row lock on this task to ensure task assignment sanity # no other concurrent transaction should be altering this row - q = """SELECT state,host_id FROM task WHERE id=%(task_id)s FOR UPDATE""" - r = _fetchSingle(q, locals()) + query = QueryProcessor(columns=['state', 'host_id'], tables=['task'], + clauses=['id=%(task_id)s'], values={'task_id': task_id}, + opts={'rowlock': True}) + r = query.executeOne() if not r: raise koji.GenericError("No such task: %i" % task_id) - state, otherhost = r - return (state == koji.TASK_STATES['OPEN'] and otherhost == host_id) + return (r['state'] == koji.TASK_STATES['OPEN'] and r['host_id'] == host_id) def assertHost(self, host_id): if not self.verifyHost(host_id): @@ -197,8 +198,9 @@ class Task(object): def getOwner(self): """Return the owner (user_id) for this task""" - q = """SELECT owner FROM task WHERE id=%(id)i""" - return _singleValue(q, vars(self)) + query = QueryProcessor(tables=['task'], columns=['owner'], clauses=['id=%(id)i'], + values=vars(self)) + return query.singleValue() def verifyOwner(self, user_id=None): """Verify that user owns task""" @@ -208,11 +210,11 @@ class Task(object): return False task_id = self.id # getting a row lock on this task to ensure task state sanity - q = """SELECT owner FROM task WHERE id=%(task_id)s FOR UPDATE""" - r = _fetchSingle(q, locals()) - if not r: + query = QueryProcessor(columns=['owner'], tables=['task'], clauses=['id=%(task_id)s'], + values={'task_id': task_id}, opts={'rowlock': True}) + owner = query.singleValue(strict=False) + if not owner: raise koji.GenericError("No such task: %i" % task_id) - (owner,) = r return (owner == user_id) def assertOwner(self, user_id=None): @@ -227,14 +229,17 @@ class Task(object): self.runCallbacks('preTaskStateChange', info, 'state', koji.TASK_STATES[newstate]) self.runCallbacks('preTaskStateChange', info, 'host_id', host_id) # we use row-level locks to keep things sane - # note the SELECT...FOR UPDATE + # note the QueryProcessor...opts={'rowlock': True} task_id = self.id if not force: - q = """SELECT state,host_id FROM task WHERE id=%(task_id)i FOR UPDATE""" - r = _fetchSingle(q, locals()) + query = QueryProcessor(columns=['state', 'host_id'], tables=['task'], + clauses=['id=%(task_id)s'], values={'task_id': task_id}, + opts={'rowlock': True}) + r = query.executeOne() if not r: raise koji.GenericError("No such task: %i" % task_id) - state, otherhost = r + state = r['state'] + otherhost = r['host_id'] if state == koji.TASK_STATES['FREE']: if otherhost is not None: log_error(f"Error: task {task_id} is both free " @@ -313,58 +318,58 @@ class Task(object): info = self.getInfo(request=True) self.runCallbacks('preTaskStateChange', info, 'state', koji.TASK_STATES['FREE']) self.runCallbacks('preTaskStateChange', info, 'host_id', None) - task_id = self.id # access checks should be performed by calling function - query = """SELECT state FROM task WHERE id = %(id)i FOR UPDATE""" - row = _fetchSingle(query, vars(self)) - if not row: + query = QueryProcessor(columns=['state'], tables=['task'], clauses=['id = %(id)i'], + values=vars(self), opts={'rowlock': True}) + oldstate = query.singleValue(strict=False) + if not oldstate: raise koji.GenericError("No such task: %i" % self.id) - oldstate = row[0] if koji.TASK_STATES[oldstate] in ['CLOSED', 'CANCELED', 'FAILED']: raise koji.GenericError("Cannot free task %i, state is %s" % (self.id, koji.TASK_STATES[oldstate])) newstate = koji.TASK_STATES['FREE'] newhost = None - q = """UPDATE task SET state=%(newstate)s,host_id=%(newhost)s - WHERE id=%(task_id)s""" - _dml(q, locals()) + update = UpdateProcessor('task', values={'task_id': self.id}, clauses=['id=%(task_id)s'], + data={'state': newstate, 'host_id': newhost}) + update.execute() self.runCallbacks('postTaskStateChange', info, 'state', koji.TASK_STATES['FREE']) self.runCallbacks('postTaskStateChange', info, 'host_id', None) return True def setWeight(self, weight): """Set weight for task""" - task_id = self.id weight = convert_value(weight, cast=float) info = self.getInfo(request=True) self.runCallbacks('preTaskStateChange', info, 'weight', weight) # access checks should be performed by calling function - q = """UPDATE task SET weight=%(weight)s WHERE id = %(task_id)s""" - _dml(q, locals()) + update = UpdateProcessor('task', values={'task_id': self.id}, clauses=['id=%(task_id)s'], + data={'weight': weight}) + update.execute() self.runCallbacks('postTaskStateChange', info, 'weight', weight) def setPriority(self, priority, recurse=False): """Set priority for task""" - task_id = self.id priority = convert_value(priority, cast=int) info = self.getInfo(request=True) self.runCallbacks('preTaskStateChange', info, 'priority', priority) - # access checks should be performed by calling function - q = """UPDATE task SET priority=%(priority)s WHERE id = %(task_id)s""" - _dml(q, locals()) + task_id = self.id + update = UpdateProcessor('task', values={'task_id': task_id}, clauses=['id=%(task_id)s'], + data={'priority': priority}) + update.execute() self.runCallbacks('postTaskStateChange', info, 'priority', priority) if recurse: # Change priority of child tasks - q = """SELECT id FROM task WHERE parent = %(task_id)s""" - for (child_id,) in _fetchMulti(q, locals()): + query = QueryProcessor(columns=['id'], tables=['task'], + clauses=['parent = %(task_id)s'], values={'task_id': task_id}, + opts={'asList': True}) + for (child_id,) in query.execute(): Task(child_id).setPriority(priority, recurse=True) def _close(self, result, state): """Mark task closed and set response Returns True if successful, False if not""" - task_id = self.id # access checks should be performed by calling function # this is an approximation, and will be different than what is in the database # the actual value should be retrieved from the 'new' value of the post callback @@ -373,11 +378,11 @@ class Task(object): info['result'] = result self.runCallbacks('preTaskStateChange', info, 'state', state) self.runCallbacks('preTaskStateChange', info, 'completion_ts', now) - update = """UPDATE task SET result = %(result)s, state = %(state)s, completion_time = NOW() - WHERE id = %(task_id)d - """ # get the result from the info dict, so callbacks have a chance to modify it - _dml(update, {'result': info['result'], 'state': state, 'task_id': task_id}) + update = UpdateProcessor('task', values={'task_id': self.id}, clauses=['id = %(task_id)d'], + data={'result': info['result'], 'state': state}, + rawdata={'completion_time': 'NOW()'}) + update.execute() self.runCallbacks('postTaskStateChange', info, 'state', state) self.runCallbacks('postTaskStateChange', info, 'completion_ts', now) @@ -390,8 +395,9 @@ class Task(object): self._close(result, koji.TASK_STATES['FAILED']) def getState(self): - query = """SELECT state FROM task WHERE id = %(id)i""" - return _singleValue(query, vars(self)) + query = QueryProcessor(columns=['state'], tables=['task'], clauses=['id = %(id)i'], + values=vars(self)) + return query.singleValue() def isFinished(self): return (koji.TASK_STATES[self.getState()] in ['CLOSED', 'CANCELED', 'FAILED']) @@ -415,8 +421,9 @@ class Task(object): self.runCallbacks('preTaskStateChange', info, 'state', koji.TASK_STATES['CANCELED']) self.runCallbacks('preTaskStateChange', info, 'completion_ts', now) task_id = self.id - q = """SELECT state FROM task WHERE id = %(task_id)s FOR UPDATE""" - state = _singleValue(q, locals()) + query = QueryProcessor(columns=['state'], tables=['task'], clauses=['id = %(task_id)s'], + values={'task_id': task_id}, opts={'rowlock': True}) + state = query.singleValue() st_canceled = koji.TASK_STATES['CANCELED'] st_closed = koji.TASK_STATES['CLOSED'] st_failed = koji.TASK_STATES['FAILED'] @@ -424,18 +431,19 @@ class Task(object): return True elif state in [st_closed, st_failed]: return False - update = """UPDATE task SET state = %(st_canceled)i, completion_time = NOW() - WHERE id = %(task_id)i""" - _dml(update, locals()) + update = UpdateProcessor('task', values={'task_id': task_id}, clauses=['id = %(task_id)i'], + data={'state': st_canceled}, rawdata={'completion_time': 'NOW()'}) + update.execute() self.runCallbacks('postTaskStateChange', info, 'state', koji.TASK_STATES['CANCELED']) self.runCallbacks('postTaskStateChange', info, 'completion_ts', now) # cancel associated builds (only if state is 'BUILDING') # since we check build state, we avoid loops with cancel_build on our end b_building = koji.BUILD_STATES['BUILDING'] - q = """SELECT id FROM build WHERE task_id = %(task_id)i - AND state = %(b_building)i - FOR UPDATE""" - for (build_id,) in _fetchMulti(q, locals()): + query = QueryProcessor(columns=['id'], tables=['build'], + clauses=['task_id = %(task_id)i', 'state = %(b_building)i'], + values={'task_id': task_id, 'b_building': b_building}, + opts={'rowlock': True, 'asList': True}) + for (build_id,) in query.execute(): cancel_build(build_id, cancel_task=False) if recurse: # also cancel child tasks @@ -444,9 +452,9 @@ class Task(object): def cancelChildren(self): """Cancel child tasks""" - task_id = self.id - q = """SELECT id FROM task WHERE parent = %(task_id)i""" - for (id,) in _fetchMulti(q, locals()): + query = QueryProcessor(columns=['id'], tables=['task'], clauses=['parent = %(task_id)i'], + values={'task_id': self.id}, opts={'asList': True}) + for (id, ) in query.execute(): Task(id).cancel(recurse=True) def cancelFull(self, strict=True): @@ -456,8 +464,9 @@ class Task(object): Otherwise we will follow up the chain to find the top-level task """ task_id = self.id - q = """SELECT parent FROM task WHERE id = %(task_id)i FOR UPDATE""" - parent = _singleValue(q, locals()) + query = QueryProcessor(columns=['parent'], tables=['task'], clauses=['id = %(task_id)i'], + values={'task_id': task_id}, opts={'rowlock': True}) + parent = query.singleValue(strict=False) if parent is not None: if strict: raise koji.GenericError("Task %d is not top-level (parent=%d)" % (task_id, parent)) @@ -468,21 +477,27 @@ class Task(object): raise koji.GenericError("Task LOOP at task %i" % task_id) task_id = parent seen[task_id] = 1 - parent = _singleValue(q, locals()) + query = QueryProcessor(columns=['parent'], tables=['task'], + clauses=['id = %(task_id)i'], + values={'task_id': task_id}, opts={'rowlock': True}) + parent = query.singleValue() return Task(task_id).cancelFull(strict=True) # We handle the recursion ourselves, since self.cancel will stop at # canceled or closed tasks. tasklist = [task_id] seen = {} # query for use in loop - q_children = """SELECT id FROM task WHERE parent = %(task_id)i""" for task_id in tasklist: if task_id in seen: # shouldn't happen raise koji.GenericError("Task LOOP at task %i" % task_id) seen[task_id] = 1 Task(task_id).cancel(recurse=False) - for (child_id,) in _fetchMulti(q_children, locals()): + query = QueryProcessor(columns=['id'], tables=['task'], + clauses=['parent = %(task_id)i'], + values={'task_id': task_id}, opts={'asList': True}) + result = query.execute() + for (child_id,) in result: tasklist.append(child_id) def getRequest(self): @@ -497,11 +512,13 @@ class Task(object): return params def getResult(self, raise_fault=True): - query = """SELECT state,result FROM task WHERE id = %(id)i""" - r = _fetchSingle(query, vars(self)) + query = QueryProcessor(columns=['state', 'result'], tables=['task'], + clauses=['id = %(id)i'], values=vars(self)) + r = query.executeOne() if not r: raise koji.GenericError("No such task") - state, xml_result = r + state = r['state'] + xml_result = r['result'] if koji.TASK_STATES[state] == 'CANCELED': raise koji.GenericError("Task %i is canceled" % self.id) elif koji.TASK_STATES[state] not in ['CLOSED', 'FAILED']: @@ -605,12 +622,12 @@ def make_task(method, arglist, **opts): opts['assign'] = get_host(opts['assign'], strict=True)['id'] if 'parent' in opts: # for subtasks, we use some of the parent's options as defaults - fields = ('state', 'owner', 'channel_id', 'priority', 'arch') - q = """SELECT %s FROM task WHERE id = %%(parent)i""" % ','.join(fields) - r = _fetchSingle(q, opts) - if not r: + query = QueryProcessor(columns=['state', 'owner', 'channel_id', 'priority', 'arch'], + tables=['task'], clauses=['id = %(parent)i'], + values={'parent': opts['parent']}) + pdata = query.executeOne() + if not pdata: raise koji.GenericError("Invalid parent task: %(parent)s" % opts) - pdata = dict(zip(fields, r)) if pdata['state'] != koji.TASK_STATES['OPEN']: raise koji.GenericError("Parent task (id %(parent)s) is not open" % opts) # default to a higher priority than parent @@ -963,15 +980,15 @@ def readFullInheritanceRecurse(tag_id, event, order, top, hist, currdepth, maxde def _pkglist_remove(tag_id, pkg_id): - clauses = ('package_id=%(pkg_id)i', 'tag_id=%(tag_id)i') - update = UpdateProcessor('tag_packages', values=locals(), clauses=clauses) + update = UpdateProcessor('tag_packages', values={'pkg_id': pkg_id, 'tag_id': tag_id}, + clauses=['package_id=%(pkg_id)i', 'tag_id=%(tag_id)i']) update.make_revoke() # XXX user_id? update.execute() def _pkglist_owner_remove(tag_id, pkg_id): - clauses = ('package_id=%(pkg_id)i', 'tag_id=%(tag_id)i') - update = UpdateProcessor('tag_package_owners', values=locals(), clauses=clauses) + update = UpdateProcessor('tag_package_owners', values={'pkg_id': pkg_id, 'tag_id': tag_id}, + clauses=['package_id=%(pkg_id)i', 'tag_id=%(tag_id)i']) update.make_revoke() # XXX user_id? update.execute() @@ -1787,9 +1804,9 @@ def _direct_tag_build(tag, build, user, force=False): # see if it's already tagged retag = False table = 'tag_listing' - clauses = ('tag_id=%(tag_id)i', 'build_id=%(build_id)i') + clauses = ['tag_id=%(tag_id)i', 'build_id=%(build_id)i'] query = QueryProcessor(columns=['build_id'], tables=[table], - clauses=('active = TRUE',) + clauses, + clauses=['active = TRUE'] + clauses, values=locals(), opts={'rowlock': True}) # note: tag_listing is unique on (build_id, tag_id, active) if query.executeOne(): @@ -1974,9 +1991,9 @@ def _grplist_unblock(taginfo, grpinfo): tag_id = tag['id'] grp_id = group['id'] table = 'group_config' - clauses = ('group_id=%(grp_id)s', 'tag_id=%(tag_id)s') + clauses = ['group_id=%(grp_id)s', 'tag_id=%(tag_id)s'] query = QueryProcessor(columns=['blocked'], tables=[table], - clauses=('active = TRUE',) + clauses, + clauses=['active = TRUE'] + clauses, values=locals(), opts={'rowlock': True}) blocked = query.singleValue(strict=False) if not blocked: @@ -2100,9 +2117,9 @@ def _grp_pkg_unblock(taginfo, grpinfo, pkg_name): table = 'group_package_listing' tag_id = get_tag_id(taginfo, strict=True) grp_id = get_group_id(grpinfo, strict=True) - clauses = ('group_id=%(grp_id)s', 'tag_id=%(tag_id)s', 'package = %(pkg_name)s') + clauses = ['group_id=%(grp_id)s', 'tag_id=%(tag_id)s', 'package = %(pkg_name)s'] query = QueryProcessor(columns=['blocked'], tables=[table], - clauses=('active = TRUE',) + clauses, + clauses=['active = TRUE'] + clauses, values=locals(), opts={'rowlock': True}) blocked = query.singleValue(strict=False) if not blocked: @@ -2233,9 +2250,9 @@ def _grp_req_unblock(taginfo, grpinfo, reqinfo): req_id = get_group_id(reqinfo, strict=True) table = 'group_req_listing' - clauses = ('group_id=%(grp_id)s', 'tag_id=%(tag_id)s', 'req_id = %(req_id)s') + clauses = ['group_id=%(grp_id)s', 'tag_id=%(tag_id)s', 'req_id = %(req_id)s'] query = QueryProcessor(columns=['blocked'], tables=[table], - clauses=('active = TRUE',) + clauses, + clauses=['active = TRUE'] + clauses, values=locals(), opts={'rowlock': True}) blocked = query.singleValue(strict=False) if not blocked: @@ -2524,35 +2541,30 @@ def get_ready_hosts(): Note: We ignore hosts that are late checking in (even if a host is busy with tasks, it should be checking in quite often). """ - c = context.cnx.cursor() - fields = ('host.id', 'name', 'arches', 'task_load', 'capacity') - aliases = ('id', 'name', 'arches', 'task_load', 'capacity') - q = """ - SELECT %s FROM host - JOIN sessions USING (user_id) - JOIN host_config ON host.id = host_config.host_id - WHERE enabled = TRUE AND ready = TRUE - AND expired = FALSE - AND master IS NULL - AND update_time > NOW() - '5 minutes'::interval - AND active IS TRUE - """ % ','.join(fields) - # XXX - magic number in query - c.execute(q) - hosts = [dict(zip(aliases, row)) for row in c.fetchall()] + query = QueryProcessor(columns=['host.id', 'name', 'arches', 'task_load', 'capacity'], + tables=['host'], + clauses=['enabled = TRUE', 'ready = TRUE', 'expired = FALSE', + 'master IS NULL', 'active IS TRUE', + "update_time > NOW() - '5 minutes'::interval"], + joins=['sessions USING (user_id)', + 'host_config ON host.id = host_config.host_id'], + aliases=['id', 'name', 'arches', 'task_load', 'capacity']) + hosts = query.execute() for host in hosts: - q = """SELECT channel_id FROM host_channels - JOIN channels ON host_channels.channel_id = channels.id - WHERE host_id=%(id)s AND active IS TRUE AND enabled IS TRUE""" - c.execute(q, host) - host['channels'] = [row[0] for row in c.fetchall()] + query = QueryProcessor(columns=['channel_id'], tables=['host_channels'], values=host, + clauses=['host_id=%(id)s', 'active IS TRUE', 'enabled IS TRUE'], + joins=['channels ON host_channels.channel_id = channels.id']) + rows = query.execute() + host['channels'] = [row['channel_id'] for row in rows] return hosts def get_all_arches(): """Return a list of all (canonical) arches available from hosts""" ret = {} - for (arches,) in _fetchMulti('SELECT arches FROM host_config WHERE active IS TRUE', {}): + query = QueryProcessor(columns=['arches'], tables=['host_config'], clauses=['active IS TRUE'], + opts={'asList': True}) + for (arches,) in query.execute(): if arches is None: continue for arch in arches.split(): @@ -2699,7 +2711,6 @@ def repo_init(tag, task_id=None, with_src=False, with_debuginfo=False, event=Non repo_id, event_id """ task_id = convert_value(task_id, cast=int, none_allowed=True) - logger = logging.getLogger("koji.hub.repo_init") state = koji.REPO_INIT tinfo = get_tag(tag, strict=True, event=event) koji.plugin.run_callbacks('preRepoInit', tag=tinfo, with_src=with_src, @@ -2720,8 +2731,9 @@ def repo_init(tag, task_id=None, with_src=False, with_debuginfo=False, event=Non event_id = _singleValue("SELECT get_event()") else: # make sure event is valid - q = "SELECT time FROM events WHERE id=%(event)s" - event_time = _singleValue(q, locals(), strict=True) + query = QueryProcessor(tables=['events'], columns=['time'], + clauses=['id=%(event)s'], values={'event': event}) + query.singleValue() event_id = event insert = InsertProcessor('repo') insert.set(id=repo_id, create_event=event_id, tag_id=tag_id, state=state, task_id=task_id) @@ -2943,13 +2955,15 @@ def repo_set_state(repo_id, state, check=True): repo_id = convert_value(repo_id, cast=int) if check: # The repo states are sequential, going backwards makes no sense - q = """SELECT state FROM repo WHERE id = %(repo_id)i FOR UPDATE""" - oldstate = _singleValue(q, locals()) + query = QueryProcessor(columns=['state'], tables=['repo'], clauses=['id = %(repo_id)i'], + values={'repo_id': repo_id}, opts={'rowlock': True}) + oldstate = query.singleValue() if oldstate > state: raise koji.GenericError("Invalid repo state transition %s->%s" % (oldstate, state)) - q = """UPDATE repo SET state=%(state)s WHERE id = %(repo_id)s""" - _dml(q, locals()) + update = UpdateProcessor('repo', values={'repo_id': repo_id}, clauses=['id=%(repo_id)s'], + data={'state': state}) + update.execute() def repo_info(repo_id, strict=False): @@ -3000,8 +3014,9 @@ def repo_delete(repo_id): If the number of references is nonzero, no change is made""" repo_id = convert_value(repo_id, cast=int) # get a row lock on the repo - q = """SELECT state FROM repo WHERE id = %(repo_id)i FOR UPDATE""" - _singleValue(q, locals()) + query = QueryProcessor(columns=['state'], tables=['repo'], clauses=['id = %(repo_id)i'], + values={'repo_id': repo_id}, opts={'rowlock': True}) + query.singleValue() references = repo_references(repo_id) if not references: repo_set_state(repo_id, koji.REPO_DELETED) @@ -3033,10 +3048,9 @@ def repo_references(repo_id): 'create_event': 'create_event', 'state': 'state'} fields, aliases = zip(*fields.items()) - values = {'repo_id': repo_id} - clauses = ['repo_id=%(repo_id)s', 'retire_event IS NULL'] query = QueryProcessor(columns=fields, aliases=aliases, tables=['standard_buildroot'], - clauses=clauses, values=values) + clauses=['repo_id=%(repo_id)s', 'retire_event IS NULL'], + values={'repo_id': repo_id}) # check results for bad states ret = [] for data in query.execute(): @@ -3084,14 +3098,13 @@ def tag_changed_since_event(event, taglist): """ data = locals().copy() # first check the tag_updates table - clauses = ['update_event > %(event)i', 'tag_id IN %(taglist)s'] query = QueryProcessor(tables=['tag_updates'], columns=['id'], - clauses=clauses, values=data, - opts={'limit': 1}) + clauses=['update_event > %(event)i', 'tag_id IN %(taglist)s'], + values=data, opts={'limit': 1}) if query.execute(): return True # also check these versioned tables - tables = ( + tables = [ 'tag_listing', 'tag_inheritance', 'tag_config', @@ -3101,12 +3114,11 @@ def tag_changed_since_event(event, taglist): 'group_package_listing', 'group_req_listing', 'group_config', - ) - clauses = ['create_event > %(event)i OR revoke_event > %(event)i', - 'tag_id IN %(taglist)s'] + ] for table in tables: - query = QueryProcessor(tables=[table], columns=['tag_id'], clauses=clauses, - values=data, opts={'limit': 1}) + query = QueryProcessor(tables=[table], columns=['tag_id'], values=data, + clauses=['create_event > %(event)i OR revoke_event > %(event)i', + 'tag_id IN %(taglist)s'], opts={'limit': 1}) if query.execute(): return True return False @@ -3191,20 +3203,20 @@ def _edit_build_target(buildTargetInfo, name, build_tag, dest_tag): raise koji.GenericError("destination tag '%s' does not exist" % dest_tag) destTagID = dest_tag_object['id'] + values = {'buildTargetID': buildTargetID} if target['name'] != name: # Allow renaming, for parity with tags - id = _singleValue("""SELECT id from build_target where name = %(name)s""", - locals(), strict=False) + query = QueryProcessor(tables=['build_target'], columns=['id'], + clauses=['name = %(name)s'], values={'name': name}) + id = query.singleValue(strict=False) if id is not None: raise koji.GenericError('name "%s" is already taken by build target %i' % (name, id)) - rename = """UPDATE build_target - SET name = %(name)s - WHERE id = %(buildTargetID)i""" - - _dml(rename, locals()) + update = UpdateProcessor('build_target', values=values, clauses=['id = %(buildTargetID)i'], + data={'name': name}) + update.execute() - update = UpdateProcessor('build_target_config', values=locals(), + update = UpdateProcessor('build_target_config', values=values, clauses=["build_target_id = %(buildTargetID)i"]) update.make_revoke() @@ -3335,9 +3347,8 @@ def lookup_name(table, info, strict=False, create=False): Any other fields should have default values, otherwise the create option will fail. """ - fields = ('id', 'name') clause, values = name_or_id_clause(table, info) - query = QueryProcessor(columns=fields, tables=[table], + query = QueryProcessor(columns=['id', 'name'], tables=[table], clauses=[clause], values=values) ret = query.executeOne() if ret is not None: @@ -3568,14 +3579,12 @@ def get_tag(tagInfo, strict=False, event=None, blocked=False): def get_tag_extra(tagInfo, event=None, blocked=False): """ Get tag extra info (no inheritance) """ - tables = ['tag_extra'] fields = ['key', 'value', 'CASE WHEN value IS NULL THEN TRUE ELSE FALSE END'] - aliases = ['key', 'value', 'blocked'] clauses = [eventCondition(event, table='tag_extra'), "tag_id = %(id)i"] if not blocked: clauses.append("value IS NOT NULL") - query = QueryProcessor(columns=fields, tables=tables, clauses=clauses, values=tagInfo, - aliases=aliases) + query = QueryProcessor(columns=fields, tables=['tag_extra'], values=tagInfo, + clauses=clauses, aliases=['key', 'value', 'blocked']) result = {} for h in query.execute(): if h['value'] is not None: @@ -3643,19 +3652,15 @@ def _edit_tag(tagInfo, **kwargs): # a cosmetic one). The more versioning-friendly way would be to create # a new tag with duplicate data and revoke the old tag. This is more # of a pain of course :-/ -mikem - values = { - 'name': name, - 'tagID': tag['id'] - } - q = """SELECT id FROM tag WHERE name=%(name)s""" - id = _singleValue(q, values, strict=False) + query = QueryProcessor(tables=['tag'], columns=['id'], + clauses=['name = %(name)s'], values={'name': name}) + id = query.singleValue(strict=False) if id is not None: # new name is taken raise koji.GenericError("Name %s already taken by tag %s" % (name, id)) - update = """UPDATE tag -SET name = %(name)s -WHERE id = %(tagID)i""" - _dml(update, values) + update = UpdateProcessor('tag', values={'tagID': tag['id']}, clauses=['id = %(tagID)i'], + data={'name': name}) + update.execute() # sanitize architecture names (space-separated string) arches = kwargs.get('arches') @@ -3881,14 +3886,16 @@ def edit_external_repo(info, name=None, url=None): if name and name != repo['name']: verify_name_internal(name) - existing_id = _singleValue("""SELECT id FROM external_repo WHERE name = %(name)s""", - locals(), strict=False) + query = QueryProcessor(tables=['external_repo'], columns=['id'], + clauses=['name = %(name)s'], values={'name': name}) + existing_id = query.singleValue(strict=False) if existing_id is not None: raise koji.GenericError('name "%s" is already taken by external repo %i' % (name, existing_id)) - rename = """UPDATE external_repo SET name = %(name)s WHERE id = %(repo_id)i""" - _dml(rename, locals()) + update = UpdateProcessor('external_repo', values={'repo_id': repo_id}, + clauses=['id = %(repo_id)i'], data={'name': name}) + update.execute() if url and url != repo['url']: if not url.endswith('/'): @@ -4224,15 +4231,14 @@ def _edit_user(userInfo, name=None, krb_principal_mappings=None): 'name': name, 'userID': user['id'] } - q = """SELECT id FROM users WHERE name=%(name)s""" - id = _singleValue(q, values, strict=False) + query = QueryProcessor(tables=['users'], columns=['id'], + clauses=['name = %(name)s'], values=values) + id = query.singleValue(strict=False) if id is not None: # new name is taken raise koji.GenericError("Name %s already taken by user %s" % (name, id)) - update = UpdateProcessor('users', - values={'userID': user['id']}, - clauses=['id = %(userID)i']) - update.set(name=name) + update = UpdateProcessor('users', values=values, clauses=['id = %(userID)i'], + data={'name': name}) update.execute() if krb_principal_mappings: added = set() @@ -4336,22 +4342,18 @@ def find_build_id(X, strict=False): if not ('name' in data and 'version' in data and 'release' in data): raise koji.GenericError('did not provide name, version, and release') - c = context.cnx.cursor() - q = """SELECT build.id FROM build JOIN package ON build.pkg_id=package.id - WHERE package.name=%(name)s AND build.version=%(version)s - AND build.release=%(release)s - """ - # contraints should ensure this is unique - # log_error(koji.db._quoteparams(q,data)) - c.execute(q, data) - r = c.fetchone() + query = QueryProcessor(columns=['build.id'], values=data, + tables=['build'], joins=['package ON build.pkg_id=package.id'], + clauses=['package.name=%(name)s', 'build.version=%(version)s', + 'build.release=%(release)s']) + r = query.singleValue(strict=False) # log_error("%r" % r ) if not r: if strict: raise koji.GenericError('No such build: %r' % X) else: return None - return r[0] + return r def get_build(buildInfo, strict=False): @@ -4782,15 +4784,14 @@ def get_maven_build(buildInfo, strict=False): artifact_id: Maven artifact_Id (string) version: Maven version (string) """ - fields = ('build_id', 'group_id', 'artifact_id', 'version') build_id = find_build_id(buildInfo, strict=strict) if not build_id: return None - query = """SELECT %s - FROM maven_builds - WHERE build_id = %%(build_id)i""" % ', '.join(fields) - return _singleRow(query, locals(), fields, strict) + query = QueryProcessor(columns=['build_id', 'group_id', 'artifact_id', 'version'], + tables=['maven_builds'], clauses=['build_id = %(build_id)i'], + values={'build_id': build_id}) + return query.executeOne(strict=strict) def get_win_build(buildInfo, strict=False): @@ -4803,14 +4804,12 @@ def get_win_build(buildInfo, strict=False): build_id: id of the build (integer) platform: the platform the build was performed on (string) """ - fields = ('build_id', 'platform') build_id = find_build_id(buildInfo, strict=strict) if not build_id: return None - query = QueryProcessor(tables=('win_builds',), columns=fields, - clauses=('build_id = %(build_id)i',), - values={'build_id': build_id}) + query = QueryProcessor(tables=['win_builds'], columns=['build_id', 'platform'], + clauses=['build_id = %(build_id)i'], values={'build_id': build_id}) result = query.executeOne() if strict and not result: raise koji.GenericError('no such Windows build: %s' % buildInfo) @@ -4830,8 +4829,8 @@ def get_image_build(buildInfo, strict=False): build_id = find_build_id(buildInfo, strict=strict) if not build_id: return None - query = QueryProcessor(tables=('image_builds',), columns=('build_id',), - clauses=('build_id = %(build_id)i',), + query = QueryProcessor(tables=['image_builds'], columns=['build_id'], + clauses=['build_id = %(build_id)i'], values={'build_id': build_id}) result = query.executeOne() if strict and not result: @@ -5180,10 +5179,10 @@ def get_maven_archive(archive_id, strict=False): artifact_id: Maven artifact_Id (string) version: Maven version (string) """ - fields = ('archive_id', 'group_id', 'artifact_id', 'version') - select = """SELECT %s FROM maven_archives - WHERE archive_id = %%(archive_id)i""" % ', '.join(fields) - return _singleRow(select, locals(), fields, strict=strict) + query = QueryProcessor(columns=['archive_id', 'group_id', 'artifact_id', 'version'], + tables=['maven_archives'], clauses=['archive_id = %(archive_id)i'], + values={'archive_id': archive_id}) + return query.executeOne(strict=strict) def get_win_archive(archive_id, strict=False): @@ -5196,10 +5195,10 @@ def get_win_archive(archive_id, strict=False): platforms: space-separated list of platforms the file is suitable for use on (string) flags: space-separated list of flags used when building the file (fre, chk) (string) """ - fields = ('archive_id', 'relpath', 'platforms', 'flags') - select = """SELECT %s FROM win_archives - WHERE archive_id = %%(archive_id)i""" % ', '.join(fields) - return _singleRow(select, locals(), fields, strict=strict) + query = QueryProcessor(columns=['archive_id', 'relpath', 'platforms', 'flags'], + tables=['win_archives'], clauses=['archive_id = %(archive_id)i'], + values={'archive_id': archive_id}) + return query.executeOne(strict=strict) def get_image_archive(archive_id, strict=False): @@ -5211,17 +5210,16 @@ def get_image_archive(archive_id, strict=False): arch: the architecture of the image rootid: True if this image has the root '/' partition """ - fields = ('archive_id', 'arch') - select = """SELECT %s FROM image_archives - WHERE archive_id = %%(archive_id)i""" % ', '.join(fields) - results = _singleRow(select, locals(), fields, strict=strict) + query = QueryProcessor(columns=['archive_id', 'arch'], tables=['image_archives'], + clauses=['archive_id = %(archive_id)i'], + values={'archive_id': archive_id}) + results = query.executeOne(strict=strict) if not results: return None results['rootid'] = False - fields = ['rpm_id'] - select = """SELECT %s FROM archive_rpm_components - WHERE archive_id = %%(archive_id)i""" % ', '.join(fields) - rpms = _singleRow(select, locals(), fields) + query = QueryProcessor(columns=['rpm_id'], clauses=['archive_id = %(archive_id)i'], + tables=['archive_rpm_components'], values={'archive_id': archive_id}) + rpms = query.executeOne() if rpms: results['rootid'] = True return results @@ -5542,10 +5540,9 @@ def get_channel(channelInfo, strict=False): :returns: dict of the channel ID and name, or None. For example, {'id': 20, 'name': 'container'} """ - fields = ('id', 'name', 'description', 'enabled', 'comment') clause, values = name_or_id_clause('channels', channelInfo) - query = QueryProcessor(columns=fields, tables=['channels'], - clauses=[clause], values=values) + query = QueryProcessor(columns=['id', 'name', 'description', 'enabled', 'comment'], + tables=['channels'], clauses=[clause], values=values) return query.executeOne(strict=strict) @@ -5732,21 +5729,20 @@ def list_channels(hostID=None, event=None, enabled=None): def new_package(name, strict=True): verify_name_internal(name) - c = context.cnx.cursor() # TODO - table lock? # check for existing - q = """SELECT id FROM package WHERE name=%(name)s""" - c.execute(q, locals()) - row = c.fetchone() - if row: - (pkg_id,) = row + query = QueryProcessor(columns=['id'], values={'name': name}, + tables=['package'], clauses=['name=%(name)s']) + pkg_id = query.singleValue(strict=False) + if pkg_id: if strict: raise koji.GenericError("Package already exists [id %d]" % pkg_id) else: pkg_id = nextval('package_id_seq') - q = """INSERT INTO package (id,name) VALUES (%(pkg_id)s,%(name)s)""" + insert = InsertProcessor('package') + insert.set(id=pkg_id, name=name) + insert.execute() context.commit_pending = True - c.execute(q, locals()) return pkg_id @@ -7293,23 +7289,29 @@ def merge_scratch(task_id): def get_archive_types(): """Return a list of all supported archive types.""" - select = """SELECT id, name, description, extensions, compression_type FROM archivetypes - ORDER BY id""" - return _multiRow(select, {}, ('id', 'name', 'description', 'extensions', 'compression_type')) + query = QueryProcessor(columns=['id', 'name', 'description', 'extensions', 'compression_type'], + tables=['archivetypes'], opts={'order': '-id'}) + return query.execute() def _get_archive_type_by_name(name, strict=True): - select = """SELECT id, name, description, extensions, compression_type FROM archivetypes - WHERE name = %(name)s""" - return _singleRow(select, locals(), - ('id', 'name', 'description', 'extensions', 'compression_type'), strict) + query = QueryProcessor(columns=['id', 'name', 'description', 'extensions', 'compression_type'], + tables=['archivetypes'], clauses=['name = %(name)s'], + values={'name': name}) + result = query.executeOne() + if strict and not result: + raise koji.GenericError("query returned no rows") + return result def _get_archive_type_by_id(type_id, strict=False): - select = """SELECT id, name, description, extensions, compression_type FROM archivetypes - WHERE id = %(type_id)i""" - return _singleRow(select, locals(), - ('id', 'name', 'description', 'extensions', 'compression_type'), strict) + query = QueryProcessor(columns=['id', 'name', 'description', 'extensions', 'compression_type'], + tables=['archivetypes'], clauses=['id = %(type_id)i'], + values={'type_id': type_id}) + result = query.executeOne() + if strict and not result: + raise koji.GenericError("query returned no rows") + return result def get_archive_type(filename=None, type_name=None, type_id=None, strict=False): @@ -7453,8 +7455,8 @@ def new_image_build(build_info): # We don't have to worry about updating an image build because the id is # the only thing we care about, and that should never change if a build # fails first and succeeds later on a resubmission. - query = QueryProcessor(tables=('image_builds',), columns=('build_id',), - clauses=('build_id = %(build_id)i',), + query = QueryProcessor(tables=['image_builds'], columns=['build_id'], + clauses=['build_id = %(build_id)i'], values={'build_id': build_info['id']}) result = query.executeOne() if not result: @@ -7469,9 +7471,9 @@ def new_typed_build(build_info, btype): """Mark build as a given btype""" btype_id = lookup_name('btype', btype, strict=True)['id'] - query = QueryProcessor(tables=('build_types',), columns=('build_id',), - clauses=('build_id = %(build_id)i', - 'btype_id = %(btype_id)i',), + query = QueryProcessor(tables=['build_types'], columns=['build_id'], + clauses=['build_id = %(build_id)i', + 'btype_id = %(btype_id)i'], values={'build_id': build_info['id'], 'btype_id': btype_id}) result = query.executeOne() @@ -7911,7 +7913,6 @@ def query_rpm_sigs(rpm_id=None, sigkey=None, queryOpts=None): :returns: list of dicts (rpm_id, sigkey, sighash) """ - fields = ('rpm_id', 'sigkey', 'sighash') clauses = [] if rpm_id is not None and not isinstance(rpm_id, int): rpminfo = get_rpm(rpm_id) @@ -7924,8 +7925,9 @@ def query_rpm_sigs(rpm_id=None, sigkey=None, queryOpts=None): if sigkey is not None: sigkey = sigkey.lower() clauses.append("sigkey=%(sigkey)s") - query = QueryProcessor(columns=fields, tables=('rpmsigs',), clauses=clauses, - values=locals(), opts=queryOpts) + query = QueryProcessor(columns=['rpm_id', 'sigkey', 'sighash'], tables=['rpmsigs'], + clauses=clauses, values={'rpm_id': rpm_id, 'sigkey': sigkey}, + opts=queryOpts) return query.execute() @@ -7946,11 +7948,12 @@ def write_signed_rpm(an_rpm, sigkey, force=False): raise koji.GenericError("Not a regular file: %s" % rpm_path) # make sure we have it in the db rpm_id = rinfo['id'] - q = """SELECT sighash FROM rpmsigs WHERE rpm_id=%(rpm_id)i AND sigkey=%(sigkey)s""" - row = _fetchSingle(q, locals()) - if not row: + query = QueryProcessor(columns=['sighash'], tables=['rpmsigs'], + clauses=['rpm_id=%(rpm_id)i', 'sigkey=%(sigkey)s'], + values={'rpm_id': rpm_id, 'sigkey': sigkey}) + sighash = query.singleValue(strict=False) + if not sighash: raise koji.GenericError("No cached signature for package %s, key %s" % (nvra, sigkey)) - (sighash,) = row signedpath = "%s/%s" % (builddir, koji.pathinfo.signed(rinfo, sigkey)) if os.path.exists(signedpath): if not force: @@ -8260,13 +8263,10 @@ def query_history(tables=None, **kwargs): def untagged_builds(name=None, queryOpts=None): """Returns the list of untagged builds""" - fields = ('build.id', 'package.name', 'build.version', 'build.release') - aliases = ('id', 'name', 'version', 'release') st_complete = koji.BUILD_STATES['COMPLETE'] # following can be achieved with simple query but with # linear complexity while this one will be parallelized to # full number of workers giving at least 2x speedup - tables = ('build', 'package') clauses = [ """NOT EXISTS (SELECT 1 FROM tag_listing @@ -8278,8 +8278,9 @@ def untagged_builds(name=None, queryOpts=None): if name is not None: clauses.append('package.name = %(name)s') - query = QueryProcessor(columns=fields, aliases=aliases, tables=tables, - clauses=clauses, values=locals(), + query = QueryProcessor(columns=['build.id', 'package.name', 'build.version', 'build.release'], + aliases=['id', 'name', 'version', 'release'], + tables=['build', 'package'], clauses=clauses, values=locals(), opts=queryOpts) return query.iterate() @@ -8307,10 +8308,15 @@ def build_references(build_id, limit=None, lazy=False): return ret # we'll need the component rpm and archive ids for the rest - q = """SELECT id FROM rpminfo WHERE build_id=%(build_id)i""" - build_rpm_ids = _fetchMulti(q, locals()) - q = """SELECT id FROM archiveinfo WHERE build_id=%(build_id)i""" - build_archive_ids = _fetchMulti(q, locals()) + query = QueryProcessor(columns=['id'], tables=['rpminfo'], clauses=['build_id=%(build_id)i'], + values={'build_id': build_id}, opts={'asList': True}) + build_rpm_ids = query.execute() + query = QueryProcessor(columns=['id'], tables=['archiveinfo'], values={'build_id': build_id}, + clauses=['build_id=%(build_id)i'], opts={'asList': True}, + aliases=['id']) + build_archive_ids = query.execute() + if not build_archive_ids: + build_archive_ids = [] # find rpms whose buildroots we were in st_complete = koji.BUILD_STATES['COMPLETE'] @@ -8328,7 +8334,7 @@ def build_references(build_id, limit=None, lazy=False): AND build.state = %(st_complete)i""" if limit is not None: q += "\nLIMIT %(limit)i" - for (rpm_id,) in build_rpm_ids: + for (rpm_id, ) in build_rpm_ids: for row in _multiRow(q, locals(), fields): idx.setdefault(row['id'], row) if limit is not None and len(idx) > limit: @@ -8523,16 +8529,18 @@ def _delete_build(binfo): koji.plugin.run_callbacks('preBuildStateChange', attribute='state', old=st_old, new=st_deleted, info=binfo) build_id = binfo['id'] - q = """SELECT id FROM rpminfo WHERE build_id=%(build_id)i""" - rpm_ids = _fetchMulti(q, locals()) - for (rpm_id,) in rpm_ids: + query = QueryProcessor(columns=['id'], tables=['rpminfo'], 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()) - update = UpdateProcessor('tag_listing', clauses=["build_id=%(build_id)i"], values=locals()) + values = {'build_id': build_id} + update = UpdateProcessor('tag_listing', clauses=["build_id=%(build_id)i"], values=values) update.make_revoke() update.execute() - update = """UPDATE build SET state=%(st_deleted)i WHERE id=%(build_id)i""" - _dml(update, locals()) + update = UpdateProcessor('build', values=values, clauses=['id=%(build_id)i'], + data={'state': st_deleted}) + update.execute() # now clear the build dir builddir = koji.pathinfo.build(binfo) if os.path.exists(builddir): @@ -8563,9 +8571,9 @@ def reset_build(build): koji.plugin.run_callbacks('preBuildStateChange', attribute='state', old=st_old, new=koji.BUILD_STATES['CANCELED'], info=binfo) - q = """SELECT id FROM rpminfo WHERE build_id=%(id)i""" - ids = _fetchMulti(q, binfo) - for (rpm_id,) in ids: + query = QueryProcessor(columns=['id'], tables=['rpminfo'], clauses=['build_id=%(id)i'], + values={'id': binfo['id']}, 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""" @@ -8574,9 +8582,9 @@ def reset_build(build): _dml(delete, locals()) delete = """DELETE FROM rpminfo WHERE build_id=%(id)i""" _dml(delete, binfo) - q = """SELECT id FROM archiveinfo WHERE build_id=%(id)i""" - ids = _fetchMulti(q, binfo) - for (archive_id,) in ids: + query = QueryProcessor(columns=['id'], tables=['archiveinfo'], clauses=['build_id=%(id)i'], + values={'id': binfo['id']}, 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""" @@ -8604,8 +8612,9 @@ def reset_build(build): delete = """DELETE FROM tag_listing WHERE build_id = %(id)i""" _dml(delete, binfo) binfo['state'] = koji.BUILD_STATES['CANCELED'] - update = """UPDATE build SET state=%(state)s, task_id=NULL, volume_id=0 WHERE id=%(id)s""" - _dml(update, binfo) + update = UpdateProcessor('build', values={'id': binfo['id']}, clauses=['id=%(id)s'], + data={'state': binfo['state'], 'task_id': None, 'volume_id': 0}) + update.execute() # now clear the build dir builddir = koji.pathinfo.build(binfo) if os.path.exists(builddir): @@ -8636,10 +8645,10 @@ def cancel_build(build_id, cancel_task=True): st_old = build['state'] koji.plugin.run_callbacks('preBuildStateChange', attribute='state', old=st_old, new=st_canceled, info=build) - update = """UPDATE build - SET state = %(st_canceled)i, completion_time = NOW() - WHERE id = %(build_id)i AND state = %(st_building)i""" - _dml(update, locals()) + update = UpdateProcessor('build', values={'build_id': build_id, 'st_building': st_building}, + clauses=['id = %(build_id)i', 'state = %(st_building)i'], + data={'state': st_canceled}, rawdata={'completion_time': 'NOW()'}) + update.execute() build = get_build(build_id) if build['state'] != st_canceled: return False @@ -8714,7 +8723,7 @@ def get_notification_recipients(build, tag_id, state): if state != koji.BUILD_STATES['COMPLETE']: clauses.append('success_only = FALSE') - query = QueryProcessor(columns=('user_id', 'email'), tables=['build_notifications'], + query = QueryProcessor(columns=['user_id', 'email'], tables=['build_notifications'], joins=joins, clauses=clauses, values=locals()) recipients = query.execute() @@ -8829,10 +8838,10 @@ def build_notification(task_id, build_id): def get_build_notifications(user_id): query = QueryProcessor(tables=['build_notifications'], - columns=('id', 'user_id', 'package_id', 'tag_id', - 'success_only', 'email'), + columns=['id', 'user_id', 'package_id', 'tag_id', + 'success_only', 'email'], clauses=['user_id = %(user_id)i'], - values=locals()) + values={'user_id': user_id}) return query.execute() @@ -8840,7 +8849,7 @@ def get_build_notification_blocks(user_id): query = QueryProcessor(tables=['build_notifications_block'], columns=['id', 'user_id', 'package_id', 'tag_id'], clauses=['user_id = %(user_id)i'], - values=locals()) + values={'user_id': user_id}) return query.execute() @@ -8867,9 +8876,9 @@ def add_group_member(group, user, strict=True): # check to see if user is already a member data = {'user_id': uinfo['id'], 'group_id': ginfo['id']} table = 'user_groups' - clauses = ('user_id = %(user_id)i', 'group_id = %(group_id)s') + clauses = ['user_id = %(user_id)i', 'group_id = %(group_id)s'] query = QueryProcessor(columns=['user_id'], tables=[table], - clauses=('active = TRUE',) + clauses, + clauses=['active = TRUE'] + clauses, values=data, opts={'rowlock': True}) row = query.executeOne() if row: @@ -8904,18 +8913,13 @@ def get_group_members(group): if not ginfo or ginfo['usertype'] != koji.USERTYPES['GROUP']: raise koji.GenericError("No such group: %s" % group) group_id = ginfo['id'] - columns = ('id', 'name', 'usertype', 'array_agg(krb_principal)') - aliases = ('id', 'name', 'usertype', 'krb_principals') - joins = ['JOIN users ON user_groups.user_id = users.id', - 'LEFT JOIN user_krb_principals' - ' ON users.id = user_krb_principals.user_id'] - clauses = [eventCondition(None), 'group_id = %(group_id)i'] - query = QueryProcessor(tables=['user_groups'], - columns=columns, - aliases=aliases, - joins=joins, - clauses=clauses, + columns=['id', 'name', 'usertype', 'array_agg(krb_principal)'], + aliases=['id', 'name', 'usertype', 'krb_principals'], + joins=['JOIN users ON user_groups.user_id = users.id', + 'LEFT JOIN user_krb_principals' + ' ON users.id = user_krb_principals.user_id'], + clauses=[eventCondition(None), 'group_id = %(group_id)i'], values=locals(), opts={'group': 'users.id'}, enable_group=True, @@ -8930,9 +8934,10 @@ def set_user_status(user, status): if user['status'] == status: # nothing to do return - update = """UPDATE users SET status = %(status)i WHERE id = %(user_id)i""" user_id = user['id'] - rows = _dml(update, locals()) + update = UpdateProcessor('users', values={'user_id': user_id}, clauses=['id = %(user_id)i'], + data={'status': status}) + rows = update.execute() # sanity check if rows == 0: raise koji.GenericError('No such user ID: %i' % user_id) @@ -12427,11 +12432,9 @@ class RootExports(object): If no users of the specified type exist, return an empty list.""" - fields = ('id', 'name', 'status', 'usertype', - 'array_agg(krb_principal)') - aliases = ('id', 'name', 'status', 'usertype', 'krb_principals') - joins = ('LEFT JOIN user_krb_principals' - ' ON users.id = user_krb_principals.user_id',) + fields = ['id', 'name', 'status', 'usertype', 'array_agg(krb_principal)'] + aliases = ['id', 'name', 'status', 'usertype', 'krb_principals'] + joins = ['LEFT JOIN user_krb_principals ON users.id = user_krb_principals.user_id'] clauses = ['usertype = %(userType)i'] if prefix: clauses.append("name ilike %(prefix)s || '%%'") @@ -12442,7 +12445,7 @@ class RootExports(object): else: raise koji.GenericError('queryOpts.group is not available for this API') query = QueryProcessor(columns=fields, aliases=aliases, - tables=('users',), joins=joins, clauses=clauses, + tables=['users'], joins=joins, clauses=clauses, values=locals(), opts=queryOpts, enable_group=True, transform=xform_user_krb) return query.execute() @@ -12844,8 +12847,8 @@ class RootExports(object): arches = koji.parse_arches(arches, strict=True) if get_host(hostname): raise koji.GenericError('host already exists: %s' % hostname) - q = """SELECT id FROM channels WHERE name = 'default'""" - default_channel = _singleValue(q) + query = QueryProcessor(columns=['id'], tables=['channels'], clauses=["name = 'default'"]) + default_channel = query.singleValue(strict=False) # builder user can already exist, if host tried to log in before adding into db userinfo = {'name': hostname} if krb_principal: @@ -12874,9 +12877,9 @@ class RootExports(object): krb_principal=krb_principal) # host entry hostID = nextval('host_id_seq') - insert = "INSERT INTO host (id, user_id, name) VALUES (%(hostID)i, %(userID)i, " \ - "%(hostname)s)" - _dml(insert, dslice(locals(), ('hostID', 'userID', 'hostname'))) + insert = InsertProcessor('host') + insert.set(id=hostID, user_id=userID, name=hostname) + insert.execute() insert = InsertProcessor('host_config') insert.set(host_id=hostID, arches=arches) @@ -13045,10 +13048,9 @@ class RootExports(object): - name - description """ - query = """SELECT id, name, description FROM permissions - ORDER BY id""" - - return _multiRow(query, {}, ['id', 'name', 'description']) + query = QueryProcessor(columns=['id', 'name', 'description'], tables=['permissions'], + opts={'order': 'id'}) + return query.execute() def getLoggedInUser(self): """Return information about the currently logged-in user. Returns data @@ -13082,14 +13084,13 @@ class RootExports(object): buildinfo = get_build(build, strict=True) userinfo = get_user(user, strict=True) userid = userinfo['id'] - buildid = buildinfo['id'] owner_id_old = buildinfo['owner_id'] koji.plugin.run_callbacks('preBuildStateChange', attribute='owner_id', old=owner_id_old, new=userid, info=buildinfo) - q = """UPDATE build SET owner=%(userid)i WHERE id=%(buildid)i""" - _dml(q, locals()) - buildinfo = get_build(build, strict=True) + update = UpdateProcessor('build', values={'buildid': buildinfo['id']}, + clauses=['id=%(buildid)i'], data={'owner': userid}) + update.execute() koji.plugin.run_callbacks('postBuildStateChange', attribute='owner_id', old=owner_id_old, new=userid, info=buildinfo) @@ -13248,10 +13249,10 @@ class RootExports(object): raise GenericError, else return None. """ query = QueryProcessor(tables=['build_notifications'], - columns=('id', 'user_id', 'package_id', 'tag_id', - 'success_only', 'email'), + columns=['id', 'user_id', 'package_id', 'tag_id', + 'success_only', 'email'], clauses=['id = %(id)i'], - values=locals()) + values={'id': id}) result = query.executeOne() if strict and not result: raise koji.GenericError("No notification with ID %i found" % id) @@ -13271,9 +13272,9 @@ class RootExports(object): raise GenericError, else return None. """ query = QueryProcessor(tables=['build_notifications_block'], - columns=('id', 'user_id', 'package_id', 'tag_id'), + columns=['id', 'user_id', 'package_id', 'tag_id'], clauses=['id = %(id)i'], - values=locals()) + values={'id': id}) result = query.executeOne() if strict and not result: raise koji.GenericError("No notification block with ID %i found" % id) @@ -13508,7 +13509,7 @@ class RootExports(object): clause = 'name %s %%(terms)s' % oper query = QueryProcessor(columns=cols, - aliases=aliases, tables=(table,), + aliases=aliases, tables=[table], joins=joins, clauses=(clause,), values=locals(), opts=queryOpts) return query.iterate() @@ -13652,7 +13653,6 @@ class BuildRoot(object): row = query.executeOne() if not row: raise koji.GenericError("Unable to get state for buildroot %s" % self.id) - lstate, retire_event = row if koji.BR_STATES[row['state']] == 'EXPIRED': # we will quietly ignore a request to expire an expired buildroot # otherwise this is an error @@ -13689,7 +13689,7 @@ class BuildRoot(object): joins=["rpminfo ON rpm_id = rpminfo.id", "external_repo ON external_repo_id = external_repo.id"], clauses=["buildroot_listing.buildroot_id = %(brootid)i"], - values=locals()) + values={'brootid': brootid}) return query.execute() def _setList(self, rpmlist, update=False): @@ -13738,9 +13738,6 @@ class BuildRoot(object): def getArchiveList(self, queryOpts=None): """Get the list of archives in the buildroot""" - tables = ('archiveinfo',) - joins = ('buildroot_archives ON archiveinfo.id = buildroot_archives.archive_id',) - clauses = ('buildroot_archives.buildroot_id = %(id)i',) fields = [('id', 'id'), ('type_id', 'type_id'), ('build_id', 'build_id'), @@ -13752,8 +13749,10 @@ class BuildRoot(object): ('project_dep', 'project_dep'), ] columns, aliases = zip(*fields) - query = QueryProcessor(tables=tables, columns=columns, - joins=joins, clauses=clauses, + query = QueryProcessor(tables=['archiveinfo'], columns=columns, + joins=['buildroot_archives ON archiveinfo.id = ' + 'buildroot_archives.archive_id'], + clauses=['buildroot_archives.buildroot_id = %(id)i'], values=self.data, opts=queryOpts) return query.execute() @@ -13864,22 +13863,20 @@ class Host(object): The return value is [finished, unfinished] where each entry is a list of task ids.""" # check to see if any of the tasks have finished - c = context.cnx.cursor() - q = """ - SELECT id,state FROM task - WHERE parent=%(parent)s AND awaited = TRUE - FOR UPDATE""" - c.execute(q, locals()) + query = QueryProcessor(columns=['id', 'state'], tables=['task'], values={'parent': parent}, + clauses=['parent=%(parent)s', 'awaited = TRUE'], + opts={'rowlock': True}) + result = query.execute() canceled = koji.TASK_STATES['CANCELED'] closed = koji.TASK_STATES['CLOSED'] failed = koji.TASK_STATES['FAILED'] finished = [] unfinished = [] - for id, state in c.fetchall(): - if state in (canceled, closed, failed): - finished.append(id) + for r in result: + if r['state'] in (canceled, closed, failed): + finished.append(r['id']) else: - unfinished.append(id) + unfinished.append(r['id']) return finished, unfinished def taskWait(self, parent): @@ -13889,9 +13886,9 @@ class Host(object): if finished: context.commit_pending = True for id in finished: - c = context.cnx.cursor() - q = """UPDATE task SET awaited='false' WHERE id=%(id)s""" - c.execute(q, locals()) + update = UpdateProcessor('task', values={'id': id}, clauses=['id=%(id)s'], + rawdata={'awaited': 'false'}) + update.execute() return [finished, unfinished] def taskWaitResults(self, parent, tasks, canfail=None): @@ -13927,21 +13924,17 @@ class Host(object): def getHostTasks(self): """get status of open tasks assigned to host""" - c = context.cnx.cursor() host_id = self.id # query tasks - fields = ['id', 'waiting', 'weight'] st_open = koji.TASK_STATES['OPEN'] - q = """ - SELECT %s FROM task - WHERE host_id = %%(host_id)s AND state = %%(st_open)s - """ % (",".join(fields)) - c.execute(q, locals()) - tasks = [dict(zip(fields, x)) for x in c.fetchall()] + query = QueryProcessor(columns=['id', 'waiting', 'weight'], tables=['task'], + clauses=['host_id = %(host_id)s', 'state = %(st_open)s'], + values={'host_id': host_id, 'st_open': st_open}) + tasks = query.execute() for task in tasks: id = task['id'] if task['waiting']: - finished, unfinished = self.taskWaitCheck(id) + finished, _ = self.taskWaitCheck(id) if finished: task['alert'] = True return tasks @@ -13950,10 +13943,9 @@ class Host(object): host_data = get_host(self.id) task_load = float(task_load) if task_load != host_data['task_load'] or ready != host_data['ready']: - c = context.cnx.cursor() - id = self.id - q = "UPDATE host SET task_load=%(task_load)f,ready=%(ready)s WHERE id=%(id)i" - c.execute(q, locals()) + update = UpdateProcessor('host', values={'id': self.id}, clauses=['id=%(id)i'], + data={'task_load': task_load, 'ready': ready}) + update.execute() context.commit_pending = True def getLoadData(self): @@ -13977,16 +13969,14 @@ class Host(object): c = context.cnx.cursor() id = self.id # get arch and channel info for host - q = """ - SELECT arches FROM host_config WHERE host_id = %(id)s AND active IS TRUE - """ - c.execute(q, locals()) - arches = c.fetchone()[0].split() - q = """ - SELECT channel_id FROM host_channels WHERE host_id = %(id)s AND active is TRUE - """ - c.execute(q, locals()) - channels = [x[0] for x in c.fetchall()] + values = {'id': id} + query = QueryProcessor(columns=['arches'], tables=['host_config'], + clauses=['host_id = %(id)s', 'active IS TRUE'], values=values) + arches = query.singleValue() + query = QueryProcessor(columns=['channel_id'], tables=['host_channels'], + clauses=['host_id = %(id)s', 'active IS TRUE'], values=values, + opts={'asList': True}) + channels = query.execute() # query tasks fields = ['id', 'state', 'method', 'request', 'channel_id', 'arch', 'parent'] @@ -13998,7 +13988,7 @@ class Host(object): OR (state = %%(st_assigned)s AND host_id = %%(id)s) ORDER BY priority,create_time """ % (",".join(fields)) - c.execute(q, locals()) + c.execute(q, {'st_free': st_free, 'st_assigned': st_assigned, 'id': id}) for data in c.fetchall(): data = dict(zip(fields, data)) # XXX - we should do some pruning here, but for now... @@ -14020,8 +14010,9 @@ class Host(object): def isEnabled(self): """Return whether this host is enabled or not.""" - query = """SELECT enabled FROM host_config WHERE host_id = %(id)i AND active IS TRUE""" - return _singleValue(query, {'id': self.id}, strict=True) + query = QueryProcessor(columns=['enabled'], tables=['host_config'], values={'id': self.id}, + clauses=['host_id = %(id)i', 'active IS TRUE']) + return query.singleValue(strict=True) class HostExports(object): @@ -14116,12 +14107,13 @@ class HostExports(object): opts['parent'] = parent if 'label' in opts: # first check for existing task with this parent/label - q = """SELECT id FROM task - WHERE parent=%(parent)s AND label=%(label)s""" - row = _fetchSingle(q, opts) - if row: + query = QueryProcessor(columns=['id'], tables=['task'], + clauses=['parent=%(parent)s', 'label=%(label)s'], + values={'parent': opts['parent'], 'label': opts['label']}) + task_id = query.singleValue(strict=False) + if task_id: # return task id - return row[0] + return task_id if 'kwargs' in opts: arglist = koji.encode_args(*arglist, **opts['kwargs']) del opts['kwargs'] @@ -14636,12 +14628,10 @@ class HostExports(object): koji.plugin.run_callbacks('preBuildStateChange', attribute='state', old=st_old, new=st_failed, info=buildinfo) - query = """SELECT state, completion_time - FROM build - WHERE id = %(build_id)i - FOR UPDATE""" - result = _singleRow(query, locals(), ('state', 'completion_time')) - + query = QueryProcessor(columns=['state', 'completion_time'], tables=['build'], + clauses=['id = %(build_id)i'], values={'build_id': build_id}, + opts={'rowlock': True}) + result = query.executeOne() if result['state'] != koji.BUILD_STATES['BUILDING']: raise koji.GenericError('cannot update build %i, state: %s' % (build_id, koji.BUILD_STATES[result['state']])) @@ -14649,11 +14639,10 @@ class HostExports(object): raise koji.GenericError('cannot update build %i, completed at %s' % (build_id, result['completion_time'])) - update = """UPDATE build - SET state = %(st_failed)i, - completion_time = NOW() - WHERE id = %(build_id)i""" - _dml(update, locals()) + update = UpdateProcessor('build', values={'build_id': build_id}, + clauses=['id = %(build_id)i'], data={'state': st_failed}, + rawdata={'completion_time': 'NOW()'}) + update.execute() buildinfo = get_build(build_id, strict=True) koji.plugin.run_callbacks('postBuildStateChange', attribute='state', old=st_old, new=st_failed, info=buildinfo) diff --git a/tests/test_hub/test_add_host.py b/tests/test_hub/test_add_host.py index caad3ab..36a5782 100644 --- a/tests/test_hub/test_add_host.py +++ b/tests/test_hub/test_add_host.py @@ -7,6 +7,7 @@ import kojihub UP = kojihub.UpdateProcessor IP = kojihub.InsertProcessor +QP = kojihub.QueryProcessor class TestAddHost(unittest.TestCase): @@ -22,6 +23,13 @@ class TestAddHost(unittest.TestCase): self.updates.append(update) return update + def getQuery(self, *args, **kwargs): + query = QP(*args, **kwargs) + query.execute = mock.MagicMock() + query.executeOne = mock.MagicMock() + self.queries.append(query) + return query + def setUp(self): self.InsertProcessor = mock.patch('kojihub.InsertProcessor', side_effect=self.getInsert).start() @@ -29,6 +37,9 @@ class TestAddHost(unittest.TestCase): self.UpdateProcessor = mock.patch('kojihub.UpdateProcessor', side_effect=self.getUpdate).start() self.updates = [] + self.QueryProcessor = mock.patch('kojihub.QueryProcessor', + side_effect=self.getQuery).start() + self.queries = [] self.context = mock.patch('kojihub.context').start() self.context_db = mock.patch('koji.db.context').start() # It seems MagicMock will not automatically handle attributes that @@ -39,7 +50,6 @@ class TestAddHost(unittest.TestCase): self.exports = kojihub.RootExports() self.verify_host_name = mock.patch('kojihub.verify_host_name').start() self.verify_name_user = mock.patch('kojihub.verify_name_user').start() - self._dml = mock.patch('kojihub._dml').start() self.get_host = mock.patch('kojihub.get_host').start() self._singleValue = mock.patch('kojihub._singleValue').start() self.nextval = mock.patch('kojihub.nextval').start() @@ -53,9 +63,9 @@ class TestAddHost(unittest.TestCase): self.get_host.return_value = {'id': 123} with self.assertRaises(koji.GenericError): self.exports.addHost('hostname', ['i386', 'x86_64']) - self._dml.assert_not_called() self.get_host.assert_called_once_with('hostname') - self._singleValue.assert_not_called() + self.nextval.assert_not_called() + self.assertEqual(len(self.queries), 0) def test_add_host_valid(self): self.verify_host_name.return_value = None @@ -72,12 +82,13 @@ class TestAddHost(unittest.TestCase): kojihub.get_host.assert_called_once_with('hostname') self.context.session.createUser.assert_called_once_with( 'hostname', usertype=koji.USERTYPES['HOST'], krb_principal='-hostname-') - self._singleValue.assert_called_once_with("SELECT id FROM channels WHERE name = 'default'") self.nextval.assert_called_once_with('host_id_seq') - self.assertEqual(self._dml.call_count, 1) - self._dml.assert_called_once_with("INSERT INTO host (id, user_id, name) " - "VALUES (%(hostID)i, %(userID)i, %(hostname)s)", - {'hostID': 12, 'userID': 456, 'hostname': 'hostname'}) + self.assertEqual(len(self.queries), 1) + query = self.queries[0] + self.assertEqual(query.tables, ['channels']) + self.assertEqual(query.joins, None) + self.assertEqual(set(query.columns), set(['id'])) + self.assertEqual(set(query.clauses), set(["name = 'default'"])) def test_add_host_wrong_user(self): self.verify_host_name.return_value = None @@ -89,17 +100,24 @@ class TestAddHost(unittest.TestCase): self.get_host.return_value = {} with self.assertRaises(koji.GenericError): self.exports.addHost('hostname', ['i386', 'x86_64']) - self._dml.assert_not_called() self.get_user.assert_called_once_with(userInfo={'name': 'hostname'}) self.get_host.assert_called_once_with('hostname') - self._singleValue.assert_called_once() + self.nextval.assert_not_called() self.assertEqual(len(self.inserts), 0) self.assertEqual(len(self.updates), 0) + self.assertEqual(len(self.queries), 1) + query = self.queries[0] + self.assertEqual(query.tables, ['channels']) + self.assertEqual(query.joins, None) + self.assertEqual(set(query.columns), set(['id'])) + self.assertEqual(set(query.clauses), set(["name = 'default'"])) def test_add_host_wrong_user_forced(self): self.verify_host_name.return_value = None + user_id = 123 + self.nextval.return_value = user_id self.get_user.return_value = { - 'id': 123, + 'id': user_id, 'name': 'hostname', 'usertype': koji.USERTYPES['NORMAL'] } @@ -107,17 +125,22 @@ class TestAddHost(unittest.TestCase): self.exports.addHost('hostname', ['i386', 'x86_64'], force=True) - self._dml.assert_called_once() self.get_user.assert_called_once_with(userInfo={'name': 'hostname'}) self.get_host.assert_called_once_with('hostname') - self._singleValue.assert_called() - self.assertEqual(len(self.inserts), 2) + self.nextval.assert_called_once_with('host_id_seq') + self.assertEqual(len(self.inserts), 3) self.assertEqual(len(self.updates), 1) update = self.updates[0] - self.assertEqual(update.values, {'userID': 123}) + self.assertEqual(update.values, {'userID': user_id}) self.assertEqual(update.table, 'users') self.assertEqual(update.clauses, ['id = %(userID)i']) self.assertEqual(update.data, {'usertype': koji.USERTYPES['HOST']}) + self.assertEqual(len(self.queries), 1) + query = self.queries[0] + self.assertEqual(query.tables, ['channels']) + self.assertEqual(query.joins, None) + self.assertEqual(set(query.columns), set(['id'])) + self.assertEqual(set(query.clauses), set(["name = 'default'"])) def test_add_host_superwrong_user_forced(self): self.verify_host_name.return_value = None @@ -131,12 +154,17 @@ class TestAddHost(unittest.TestCase): with self.assertRaises(koji.GenericError): self.exports.addHost('hostname', ['i386', 'x86_64'], force=True) - self._dml.assert_not_called() self.get_user.assert_called_once_with(userInfo={'name': 'hostname'}) self.get_host.assert_called_once_with('hostname') - self._singleValue.assert_called() + self.nextval.assert_not_called() self.assertEqual(len(self.inserts), 0) self.assertEqual(len(self.updates), 0) + self.assertEqual(len(self.queries), 1) + query = self.queries[0] + self.assertEqual(query.tables, ['channels']) + self.assertEqual(query.joins, None) + self.assertEqual(set(query.columns), set(['id'])) + self.assertEqual(set(query.clauses), set(["name = 'default'"])) def test_add_host_wrong_format(self): # name is longer as expected @@ -154,7 +182,7 @@ class TestAddHost(unittest.TestCase): krb_principal = ['test-krb'] self.verify_host_name.return_value = None self.get_host.return_value = {} - self._singleValue.side_effect = [333, 12] + self.QueryProcessor.return_value = 333 self.verify_name_user.side_effect = koji.GenericError with self.assertRaises(koji.GenericError): self.exports.addHost('hostname', ['i386', 'x86_64'], krb_principal=krb_principal) @@ -162,6 +190,11 @@ class TestAddHost(unittest.TestCase): self.context.session.assertPerm.assert_called_once_with('host') kojihub.get_host.assert_called_once_with('hostname') self.context.session.createUser.assert_not_called() - self.assertEqual(self._singleValue.call_count, 1) - self._singleValue.assert_called_once_with("SELECT id FROM channels WHERE name = 'default'") self.verify_host_name.assert_called_once_with('hostname') + self.nextval.assert_not_called() + self.assertEqual(len(self.queries), 1) + query = self.queries[0] + self.assertEqual(query.tables, ['channels']) + self.assertEqual(query.joins, None) + self.assertEqual(set(query.columns), set(['id'])) + self.assertEqual(set(query.clauses), set(["name = 'default'"])) diff --git a/tests/test_hub/test_edit_tag.py b/tests/test_hub/test_edit_tag.py index 8098822..192b8b7 100644 --- a/tests/test_hub/test_edit_tag.py +++ b/tests/test_hub/test_edit_tag.py @@ -30,7 +30,6 @@ class TestEditTag(unittest.TestCase): self.UpdateProcessor = mock.patch('kojihub.UpdateProcessor', side_effect=self.getUpdate).start() self.updates = [] - self._dml = mock.patch('kojihub._dml').start() self._singleValue = mock.patch('kojihub._singleValue').start() self.get_tag = mock.patch('kojihub.get_tag').start() self.get_perm_id = mock.patch('kojihub.get_perm_id').start() @@ -83,13 +82,22 @@ class TestEditTag(unittest.TestCase): kojihub._edit_tag('tag', **kwargs) self.get_perm_id.assert_not_called() - self._dml.assert_called_with("""UPDATE tag -SET name = %(name)s -WHERE id = %(tagID)i""", {'name': 'newtag', 'tagID': 333}) # check the insert/update - self.assertEqual(len(self.updates), 3) + self.assertEqual(len(self.updates), 4) self.assertEqual(len(self.inserts), 2) + + revoke_data = {'name': 'newtag'} + + values = {'tagID': 333} + + update = self.updates[0] + self.assertEqual(update.table, 'tag') + self.assertEqual(update.values, values) + self.assertEqual(update.data, revoke_data) + self.assertEqual(update.rawdata, {}) + self.assertEqual(update.clauses, ['id = %(tagID)i']) + values = { 'arches': 'arch1 arch2', 'locked': True, @@ -100,12 +108,14 @@ WHERE id = %(tagID)i""", {'name': 'newtag', 'tagID': 333}) 'name': 'tag', 'extra': {'exA': 1, 'exC': 3, 'exD': 4} } + revoke_data = { 'revoke_event': 42, 'revoker_id': 23 } revoke_rawdata = {'active': 'NULL'} - update = self.updates[0] + + update = self.updates[1] self.assertEqual(update.table, 'tag_config') self.assertEqual(update.values, values) self.assertEqual(update.data, revoke_data) @@ -133,7 +143,7 @@ WHERE id = %(tagID)i""", {'name': 'newtag', 'tagID': 333}) 'tag_id': 333, } - update = self.updates[1] + update = self.updates[2] self.assertEqual(update.table, 'tag_extra') self.assertEqual(update.values, values) self.assertEqual(update.data, revoke_data) @@ -158,7 +168,7 @@ WHERE id = %(tagID)i""", {'name': 'newtag', 'tagID': 333}) 'tag_id': 333, } - update = self.updates[2] + update = self.updates[3] self.assertEqual(update.table, 'tag_extra') self.assertEqual(update.values, values) self.assertEqual(update.data, revoke_data) diff --git a/tests/test_hub/test_edit_user.py b/tests/test_hub/test_edit_user.py index 0452c86..5fc34ae 100644 --- a/tests/test_hub/test_edit_user.py +++ b/tests/test_hub/test_edit_user.py @@ -44,7 +44,7 @@ class TestEditUser(unittest.TestCase): update = self.updates[0] self.assertEqual(update.table, 'users') self.assertEqual(update.data, {'name': 'newuser'}) - self.assertEqual(update.values, {'userID': 333}) + self.assertEqual(update.values, {'name': 'newuser', 'userID': 333}) self.assertEqual(update.clauses, ['id = %(userID)i']) kojihub._edit_user('user', krb_principal_mappings=[{'old': 'krb', 'new': 'newkrb'}]) diff --git a/tests/test_hub/test_models/test_host.py b/tests/test_hub/test_models/test_host.py index db9ea56..9df50cd 100644 --- a/tests/test_hub/test_models/test_host.py +++ b/tests/test_hub/test_models/test_host.py @@ -5,9 +5,22 @@ import unittest import koji import kojihub +UP = kojihub.UpdateProcessor + class TestHost(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.UpdateProcessor', + side_effect=self.getUpdate).start() + self.updates = [] + @mock.patch('kojihub.context') def test_instantiation_not_a_host(self, context): context.session.getHostId.return_value = None @@ -125,12 +138,35 @@ class TestHost(unittest.TestCase): def test_task_wait(self, context): cursor = mock.MagicMock() context.cnx.cursor.return_value = cursor + context.session.assertLogin = mock.MagicMock() cursor.fetchall.return_value = [ (1, 1), (2, 2), (3, 3), (4, 4), ] + context.event_id = 42 + context.session.user_id = 23 + kojihub.Host.return_value = 1234 host = kojihub.Host(id=1234) host.taskWait(parent=123) - self.assertEqual(len(cursor.execute.mock_calls), 3) + self.assertEqual(len(self.updates), 2) + self.assertEqual(len(cursor.execute.mock_calls), 1) + + rawdata = {'awaited': 'false'} + + update = self.updates[0] + values = {'id': 2} + self.assertEqual(update.table, 'task') + self.assertEqual(update.values, values) + self.assertEqual(update.data, {}) + self.assertEqual(update.rawdata, rawdata) + self.assertEqual(update.clauses, ['id=%(id)s']) + + update = self.updates[1] + values = {'id': 3} + self.assertEqual(update.table, 'task') + self.assertEqual(update.values, values) + self.assertEqual(update.data, {}) + self.assertEqual(update.rawdata, rawdata) + self.assertEqual(update.clauses, ['id=%(id)s']) diff --git a/tests/test_hub/test_set_build_owner.py b/tests/test_hub/test_set_build_owner.py new file mode 100644 index 0000000..46039f7 --- /dev/null +++ b/tests/test_hub/test_set_build_owner.py @@ -0,0 +1,48 @@ +import mock +import unittest + +import kojihub + +UP = kojihub.UpdateProcessor + + +class TestSetBuildOwner(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.UpdateProcessor', + side_effect=self.getUpdate).start() + self.updates = [] + self.context = mock.patch('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.get_build').start() + self.get_user = mock.patch('kojihub.get_user').start() + self.run_callbacks = mock.patch('koji.plugin.run_callbacks').start() + + def tearDown(self): + mock.patch.stopall() + + def test_set_build_owner(self): + self.get_build.return_value = {'id': 123, 'owner_id': 1} + self.get_user.return_value = {'id': 2} + self.context.event_id = 42 + self.context.session.user_id = 23 + self.exports.setBuildOwner('test-build', 'test-user') + clauses = ['id=%(buildid)i'] + data = {'owner': 2} + values = {'buildid': 123} + update = self.updates[0] + self.assertEqual(update.table, 'build') + self.assertEqual(update.data, data) + self.assertEqual(update.rawdata, {}) + self.assertEqual(update.clauses, clauses) + self.assertEqual(update.values, values) From 0ae29a9eaa3f5be3900a18c131837a1731bc661b Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Oct 11 2022 12:32:31 +0000 Subject: [PATCH 2/4] Fix few typos, reordering according to defintion --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 706188a..5074ca9 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -184,7 +184,7 @@ class Task(object): task_id = self.id # getting a row lock on this task to ensure task assignment sanity # no other concurrent transaction should be altering this row - query = QueryProcessor(columns=['state', 'host_id'], tables=['task'], + query = QueryProcessor(tables=['task'], columns=['state', 'host_id'], clauses=['id=%(task_id)s'], values={'task_id': task_id}, opts={'rowlock': True}) r = query.executeOne() @@ -198,8 +198,8 @@ class Task(object): def getOwner(self): """Return the owner (user_id) for this task""" - query = QueryProcessor(tables=['task'], columns=['owner'], clauses=['id=%(id)i'], - values=vars(self)) + query = QueryProcessor(tables=['task'], columns=['owner'], + clauses=['id=%(id)i'], values=vars(self)) return query.singleValue() def verifyOwner(self, user_id=None): @@ -210,8 +210,9 @@ class Task(object): return False task_id = self.id # getting a row lock on this task to ensure task state sanity - query = QueryProcessor(columns=['owner'], tables=['task'], clauses=['id=%(task_id)s'], - values={'task_id': task_id}, opts={'rowlock': True}) + query = QueryProcessor(tables=['task'], columns=['owner'], + clauses=['id=%(task_id)s'], values={'task_id': task_id}, + opts={'rowlock': True}) owner = query.singleValue(strict=False) if not owner: raise koji.GenericError("No such task: %i" % task_id) @@ -319,7 +320,7 @@ class Task(object): self.runCallbacks('preTaskStateChange', info, 'state', koji.TASK_STATES['FREE']) self.runCallbacks('preTaskStateChange', info, 'host_id', None) # access checks should be performed by calling function - query = QueryProcessor(columns=['state'], tables=['task'], clauses=['id = %(id)i'], + query = QueryProcessor(tables=['task'], columns=['state'], clauses=['id = %(id)i'], values=vars(self), opts={'rowlock': True}) oldstate = query.singleValue(strict=False) if not oldstate: @@ -329,7 +330,7 @@ class Task(object): (self.id, koji.TASK_STATES[oldstate])) newstate = koji.TASK_STATES['FREE'] newhost = None - update = UpdateProcessor('task', values={'task_id': self.id}, clauses=['id=%(task_id)s'], + update = UpdateProcessor('task', clauses=['id=%(task_id)s'], values={'task_id': self.id}, data={'state': newstate, 'host_id': newhost}) update.execute() self.runCallbacks('postTaskStateChange', info, 'state', koji.TASK_STATES['FREE']) @@ -342,7 +343,7 @@ class Task(object): info = self.getInfo(request=True) self.runCallbacks('preTaskStateChange', info, 'weight', weight) # access checks should be performed by calling function - update = UpdateProcessor('task', values={'task_id': self.id}, clauses=['id=%(task_id)s'], + update = UpdateProcessor('task', clauses=['id=%(task_id)s'], values={'task_id': self.id}, data={'weight': weight}) update.execute() self.runCallbacks('postTaskStateChange', info, 'weight', weight) @@ -352,16 +353,17 @@ class Task(object): priority = convert_value(priority, cast=int) info = self.getInfo(request=True) self.runCallbacks('preTaskStateChange', info, 'priority', priority) - task_id = self.id - update = UpdateProcessor('task', values={'task_id': task_id}, clauses=['id=%(task_id)s'], + # access checks should be performed by calling function + update = UpdateProcessor('task', clauses=['id=%(task_id)s'], values={'task_id': self.id}, data={'priority': priority}) update.execute() self.runCallbacks('postTaskStateChange', info, 'priority', priority) if recurse: # Change priority of child tasks - query = QueryProcessor(columns=['id'], tables=['task'], - clauses=['parent = %(task_id)s'], values={'task_id': task_id}, + query = QueryProcessor(tables=['task'], columns=['id'], + clauses=['parent = %(task_id)s'], + values={'task_id': self.id}, opts={'asList': True}) for (child_id,) in query.execute(): Task(child_id).setPriority(priority, recurse=True) @@ -379,7 +381,8 @@ class Task(object): self.runCallbacks('preTaskStateChange', info, 'state', state) self.runCallbacks('preTaskStateChange', info, 'completion_ts', now) # get the result from the info dict, so callbacks have a chance to modify it - update = UpdateProcessor('task', values={'task_id': self.id}, clauses=['id = %(task_id)d'], + update = UpdateProcessor('task', clauses=['id = %(task_id)d'], + values={'task_id': self.id}, data={'result': info['result'], 'state': state}, rawdata={'completion_time': 'NOW()'}) update.execute() @@ -395,7 +398,7 @@ class Task(object): self._close(result, koji.TASK_STATES['FAILED']) def getState(self): - query = QueryProcessor(columns=['state'], tables=['task'], clauses=['id = %(id)i'], + query = QueryProcessor(tables=['task'], columns=['state'], clauses=['id = %(id)i'], values=vars(self)) return query.singleValue() @@ -420,9 +423,8 @@ class Task(object): info = self.getInfo(request=True) self.runCallbacks('preTaskStateChange', info, 'state', koji.TASK_STATES['CANCELED']) self.runCallbacks('preTaskStateChange', info, 'completion_ts', now) - task_id = self.id - query = QueryProcessor(columns=['state'], tables=['task'], clauses=['id = %(task_id)s'], - values={'task_id': task_id}, opts={'rowlock': True}) + query = QueryProcessor(tables=['task'], columns=['state'], clauses=['id = %(task_id)s'], + values={'task_id': self.id}, opts={'rowlock': True}) state = query.singleValue() st_canceled = koji.TASK_STATES['CANCELED'] st_closed = koji.TASK_STATES['CLOSED'] @@ -431,7 +433,7 @@ class Task(object): return True elif state in [st_closed, st_failed]: return False - update = UpdateProcessor('task', values={'task_id': task_id}, clauses=['id = %(task_id)i'], + update = UpdateProcessor('task', clauses=['id = %(task_id)i'], values={'task_id': self.id}, data={'state': st_canceled}, rawdata={'completion_time': 'NOW()'}) update.execute() self.runCallbacks('postTaskStateChange', info, 'state', koji.TASK_STATES['CANCELED']) @@ -439,9 +441,9 @@ class Task(object): # cancel associated builds (only if state is 'BUILDING') # since we check build state, we avoid loops with cancel_build on our end b_building = koji.BUILD_STATES['BUILDING'] - query = QueryProcessor(columns=['id'], tables=['build'], + query = QueryProcessor(tables=['build'], columns=['id'], clauses=['task_id = %(task_id)i', 'state = %(b_building)i'], - values={'task_id': task_id, 'b_building': b_building}, + values={'task_id': self.id, 'b_building': b_building}, opts={'rowlock': True, 'asList': True}) for (build_id,) in query.execute(): cancel_build(build_id, cancel_task=False) @@ -452,7 +454,7 @@ class Task(object): def cancelChildren(self): """Cancel child tasks""" - query = QueryProcessor(columns=['id'], tables=['task'], clauses=['parent = %(task_id)i'], + query = QueryProcessor(tables=['task'], columns=['id'], clauses=['parent = %(task_id)i'], values={'task_id': self.id}, opts={'asList': True}) for (id, ) in query.execute(): Task(id).cancel(recurse=True) @@ -464,7 +466,8 @@ class Task(object): Otherwise we will follow up the chain to find the top-level task """ task_id = self.id - query = QueryProcessor(columns=['parent'], tables=['task'], clauses=['id = %(task_id)i'], + query = QueryProcessor(tables=['task'], columns=['parent'], + clauses=['id = %(task_id)i'], values={'task_id': task_id}, opts={'rowlock': True}) parent = query.singleValue(strict=False) if parent is not None: @@ -477,9 +480,7 @@ class Task(object): raise koji.GenericError("Task LOOP at task %i" % task_id) task_id = parent seen[task_id] = 1 - query = QueryProcessor(columns=['parent'], tables=['task'], - clauses=['id = %(task_id)i'], - values={'task_id': task_id}, opts={'rowlock': True}) + query.values = values parent = query.singleValue() return Task(task_id).cancelFull(strict=True) # We handle the recursion ourselves, since self.cancel will stop at @@ -493,7 +494,7 @@ class Task(object): raise koji.GenericError("Task LOOP at task %i" % task_id) seen[task_id] = 1 Task(task_id).cancel(recurse=False) - query = QueryProcessor(columns=['id'], tables=['task'], + query = QueryProcessor(tables=['task'], columns=['id'], clauses=['parent = %(task_id)i'], values={'task_id': task_id}, opts={'asList': True}) result = query.execute() @@ -512,8 +513,8 @@ class Task(object): return params def getResult(self, raise_fault=True): - query = QueryProcessor(columns=['state', 'result'], tables=['task'], - clauses=['id = %(id)i'], values=vars(self)) + query = QueryProcessor(tables=['task'], columns=['state', 'result'], + clauses=['id = %(id)i'], values={'id': self.id}) r = query.executeOne() if not r: raise koji.GenericError("No such task") @@ -622,9 +623,11 @@ def make_task(method, arglist, **opts): opts['assign'] = get_host(opts['assign'], strict=True)['id'] if 'parent' in opts: # for subtasks, we use some of the parent's options as defaults - query = QueryProcessor(columns=['state', 'owner', 'channel_id', 'priority', 'arch'], - tables=['task'], clauses=['id = %(parent)i'], - values={'parent': opts['parent']}) + query = QueryProcessor( + tables=['task'], + columns=['state', 'owner', 'channel_id', 'priority', 'arch'], + clauses=['id = %(parent)i'], + values={'parent': opts['parent']}) pdata = query.executeOne() if not pdata: raise koji.GenericError("Invalid parent task: %(parent)s" % opts) @@ -980,15 +983,15 @@ def readFullInheritanceRecurse(tag_id, event, order, top, hist, currdepth, maxde def _pkglist_remove(tag_id, pkg_id): - update = UpdateProcessor('tag_packages', values={'pkg_id': pkg_id, 'tag_id': tag_id}, - clauses=['package_id=%(pkg_id)i', 'tag_id=%(tag_id)i']) + clauses = ('package_id=%(pkg_id)i', 'tag_id=%(tag_id)i') + update = UpdateProcessor('tag_packages', values=locals(), clauses=clauses) update.make_revoke() # XXX user_id? update.execute() def _pkglist_owner_remove(tag_id, pkg_id): - update = UpdateProcessor('tag_package_owners', values={'pkg_id': pkg_id, 'tag_id': tag_id}, - clauses=['package_id=%(pkg_id)i', 'tag_id=%(tag_id)i']) + clauses = ('package_id=%(pkg_id)i', 'tag_id=%(tag_id)i') + update = UpdateProcessor('tag_package_owners', values=locals(), clauses=clauses) update.make_revoke() # XXX user_id? update.execute() @@ -1804,9 +1807,9 @@ def _direct_tag_build(tag, build, user, force=False): # see if it's already tagged retag = False table = 'tag_listing' - clauses = ['tag_id=%(tag_id)i', 'build_id=%(build_id)i'] + clauses = ('tag_id=%(tag_id)i', 'build_id=%(build_id)i') query = QueryProcessor(columns=['build_id'], tables=[table], - clauses=['active = TRUE'] + clauses, + clauses=('active = TRUE',) + clauses, values=locals(), opts={'rowlock': True}) # note: tag_listing is unique on (build_id, tag_id, active) if query.executeOne(): @@ -1991,9 +1994,9 @@ def _grplist_unblock(taginfo, grpinfo): tag_id = tag['id'] grp_id = group['id'] table = 'group_config' - clauses = ['group_id=%(grp_id)s', 'tag_id=%(tag_id)s'] + clauses = ('group_id=%(grp_id)s', 'tag_id=%(tag_id)s') query = QueryProcessor(columns=['blocked'], tables=[table], - clauses=['active = TRUE'] + clauses, + clauses=('active = TRUE',) + clauses, values=locals(), opts={'rowlock': True}) blocked = query.singleValue(strict=False) if not blocked: @@ -2117,9 +2120,9 @@ def _grp_pkg_unblock(taginfo, grpinfo, pkg_name): table = 'group_package_listing' tag_id = get_tag_id(taginfo, strict=True) grp_id = get_group_id(grpinfo, strict=True) - clauses = ['group_id=%(grp_id)s', 'tag_id=%(tag_id)s', 'package = %(pkg_name)s'] + clauses = ('group_id=%(grp_id)s', 'tag_id=%(tag_id)s', 'package = %(pkg_name)s') query = QueryProcessor(columns=['blocked'], tables=[table], - clauses=['active = TRUE'] + clauses, + clauses=('active = TRUE',) + clauses, values=locals(), opts={'rowlock': True}) blocked = query.singleValue(strict=False) if not blocked: @@ -2250,9 +2253,9 @@ def _grp_req_unblock(taginfo, grpinfo, reqinfo): req_id = get_group_id(reqinfo, strict=True) table = 'group_req_listing' - clauses = ['group_id=%(grp_id)s', 'tag_id=%(tag_id)s', 'req_id = %(req_id)s'] + clauses = ('group_id=%(grp_id)s', 'tag_id=%(tag_id)s', 'req_id = %(req_id)s') query = QueryProcessor(columns=['blocked'], tables=[table], - clauses=['active = TRUE'] + clauses, + clauses=('active = TRUE',) + clauses, values=locals(), opts={'rowlock': True}) blocked = query.singleValue(strict=False) if not blocked: @@ -2541,19 +2544,32 @@ def get_ready_hosts(): Note: We ignore hosts that are late checking in (even if a host is busy with tasks, it should be checking in quite often). """ - query = QueryProcessor(columns=['host.id', 'name', 'arches', 'task_load', 'capacity'], - tables=['host'], - clauses=['enabled = TRUE', 'ready = TRUE', 'expired = FALSE', - 'master IS NULL', 'active IS TRUE', - "update_time > NOW() - '5 minutes'::interval"], - joins=['sessions USING (user_id)', - 'host_config ON host.id = host_config.host_id'], - aliases=['id', 'name', 'arches', 'task_load', 'capacity']) + query = QueryProcessor( + tables=['host'], + columns=['host.id', 'name', 'arches', 'task_load', 'capacity'], + aliases=['id', 'name', 'arches', 'task_load', 'capacity'], + clauses=[ + 'enabled = TRUE', + 'ready = TRUE', + 'expired = FALSE', + 'master IS NULL', + 'active IS TRUE', + "update_time > NOW() - '5 minutes'::interval" + ], + joins=[ + 'sessions USING (user_id)', + 'host_config ON host.id = host_config.host_id' + ] + ) hosts = query.execute() for host in hosts: - query = QueryProcessor(columns=['channel_id'], tables=['host_channels'], values=host, - clauses=['host_id=%(id)s', 'active IS TRUE', 'enabled IS TRUE'], - joins=['channels ON host_channels.channel_id = channels.id']) + query = QueryProcessor( + tables=['host_channels'], + columns=['channel_id'], + clauses=['host_id=%(id)s', 'active IS TRUE', 'enabled IS TRUE'], + joins=['channels ON host_channels.channel_id = channels.id'], + values=host + ) rows = query.execute() host['channels'] = [row['channel_id'] for row in rows] return hosts @@ -2562,7 +2578,7 @@ def get_ready_hosts(): def get_all_arches(): """Return a list of all (canonical) arches available from hosts""" ret = {} - query = QueryProcessor(columns=['arches'], tables=['host_config'], clauses=['active IS TRUE'], + query = QueryProcessor(tables=['host_config'], columns=['arches'], clauses=['active IS TRUE'], opts={'asList': True}) for (arches,) in query.execute(): if arches is None: @@ -2955,13 +2971,15 @@ def repo_set_state(repo_id, state, check=True): repo_id = convert_value(repo_id, cast=int) if check: # The repo states are sequential, going backwards makes no sense - query = QueryProcessor(columns=['state'], tables=['repo'], clauses=['id = %(repo_id)i'], - values={'repo_id': repo_id}, opts={'rowlock': True}) + query = QueryProcessor( + tables=['repo'], columns=['state'], clauses=['id = %(repo_id)i'], + values={'repo_id': repo_id}, opts={'rowlock': True}) oldstate = query.singleValue() if oldstate > state: raise koji.GenericError("Invalid repo state transition %s->%s" % (oldstate, state)) - update = UpdateProcessor('repo', values={'repo_id': repo_id}, clauses=['id=%(repo_id)s'], + update = UpdateProcessor('repo', clauses=['id=%(repo_id)s'], + values={'repo_id': repo_id}, data={'state': state}) update.execute() @@ -3014,9 +3032,9 @@ def repo_delete(repo_id): If the number of references is nonzero, no change is made""" repo_id = convert_value(repo_id, cast=int) # get a row lock on the repo - query = QueryProcessor(columns=['state'], tables=['repo'], clauses=['id = %(repo_id)i'], + query = QueryProcessor(tables=['repo'], columns=['state'], clauses=['id = %(repo_id)i'], values={'repo_id': repo_id}, opts={'rowlock': True}) - query.singleValue() + query.execute() references = repo_references(repo_id) if not references: repo_set_state(repo_id, koji.REPO_DELETED) @@ -3048,9 +3066,11 @@ def repo_references(repo_id): 'create_event': 'create_event', 'state': 'state'} fields, aliases = zip(*fields.items()) - query = QueryProcessor(columns=fields, aliases=aliases, tables=['standard_buildroot'], - clauses=['repo_id=%(repo_id)s', 'retire_event IS NULL'], - values={'repo_id': repo_id}) + query = QueryProcessor( + tables=['standard_buildroot'], + columns=fields, aliases=aliases, + clauses=['repo_id=%(repo_id)s', 'retire_event IS NULL'], + values={'repo_id': repo_id}) # check results for bad states ret = [] for data in query.execute(): @@ -3104,7 +3124,7 @@ def tag_changed_since_event(event, taglist): if query.execute(): return True # also check these versioned tables - tables = [ + tables = ( 'tag_listing', 'tag_inheritance', 'tag_config', @@ -3114,7 +3134,7 @@ def tag_changed_since_event(event, taglist): 'group_package_listing', 'group_req_listing', 'group_config', - ] + ) for table in tables: query = QueryProcessor(tables=[table], columns=['tag_id'], values=data, clauses=['create_event > %(event)i OR revoke_event > %(event)i', @@ -3212,8 +3232,8 @@ def _edit_build_target(buildTargetInfo, name, build_tag, dest_tag): if id is not None: raise koji.GenericError('name "%s" is already taken by build target %i' % (name, id)) - update = UpdateProcessor('build_target', values=values, clauses=['id = %(buildTargetID)i'], - data={'name': name}) + update = UpdateProcessor('build_target', clauses=['id = %(buildTargetID)i'], + values=values, data={'name': name}) update.execute() update = UpdateProcessor('build_target_config', values=values, @@ -3580,11 +3600,12 @@ def get_tag(tagInfo, strict=False, event=None, blocked=False): def get_tag_extra(tagInfo, event=None, blocked=False): """ Get tag extra info (no inheritance) """ fields = ['key', 'value', 'CASE WHEN value IS NULL THEN TRUE ELSE FALSE END'] + aliases = ['key', 'value', 'blocked'] clauses = [eventCondition(event, table='tag_extra'), "tag_id = %(id)i"] if not blocked: clauses.append("value IS NOT NULL") - query = QueryProcessor(columns=fields, tables=['tag_extra'], values=tagInfo, - clauses=clauses, aliases=['key', 'value', 'blocked']) + query = QueryProcessor(tables=['tag_extra'], columns=fields, clauses=clauses, values=tagInfo, + aliases=aliases) result = {} for h in query.execute(): if h['value'] is not None: @@ -3893,8 +3914,8 @@ def edit_external_repo(info, name=None, url=None): raise koji.GenericError('name "%s" is already taken by external repo %i' % (name, existing_id)) - update = UpdateProcessor('external_repo', values={'repo_id': repo_id}, - clauses=['id = %(repo_id)i'], data={'name': name}) + update = UpdateProcessor('external_repo', clauses=['id = %(repo_id)i'], + values={'repo_id': repo_id}, data={'name': name}) update.execute() if url and url != repo['url']: @@ -4342,10 +4363,12 @@ def find_build_id(X, strict=False): if not ('name' in data and 'version' in data and 'release' in data): raise koji.GenericError('did not provide name, version, and release') - query = QueryProcessor(columns=['build.id'], values=data, - tables=['build'], joins=['package ON build.pkg_id=package.id'], - clauses=['package.name=%(name)s', 'build.version=%(version)s', - 'build.release=%(release)s']) + query = QueryProcessor(tables=['build'], columns=['build.id'], + clauses=['package.name=%(name)s', + 'build.version=%(version)s', + 'build.release=%(release)s'], + joins=['package ON build.pkg_id=package.id'], + values=data) r = query.singleValue(strict=False) # log_error("%r" % r ) if not r: @@ -4788,8 +4811,9 @@ def get_maven_build(buildInfo, strict=False): build_id = find_build_id(buildInfo, strict=strict) if not build_id: return None - query = QueryProcessor(columns=['build_id', 'group_id', 'artifact_id', 'version'], - tables=['maven_builds'], clauses=['build_id = %(build_id)i'], + query = QueryProcessor(tables=['maven_builds'], + columns=['build_id', 'group_id', 'artifact_id', 'version'], + clauses=['build_id = %(build_id)i'], values={'build_id': build_id}) return query.executeOne(strict=strict) @@ -4829,8 +4853,8 @@ def get_image_build(buildInfo, strict=False): build_id = find_build_id(buildInfo, strict=strict) if not build_id: return None - query = QueryProcessor(tables=['image_builds'], columns=['build_id'], - clauses=['build_id = %(build_id)i'], + query = QueryProcessor(tables=('image_builds',), columns=('build_id',), + clauses=('build_id = %(build_id)i',), values={'build_id': build_id}) result = query.executeOne() if strict and not result: @@ -5179,8 +5203,9 @@ def get_maven_archive(archive_id, strict=False): artifact_id: Maven artifact_Id (string) version: Maven version (string) """ - query = QueryProcessor(columns=['archive_id', 'group_id', 'artifact_id', 'version'], - tables=['maven_archives'], clauses=['archive_id = %(archive_id)i'], + query = QueryProcessor(tables=['maven_archives'], + columns=['archive_id', 'group_id', 'artifact_id', 'version'], + clauses=['archive_id = %(archive_id)i'], values={'archive_id': archive_id}) return query.executeOne(strict=strict) @@ -5195,8 +5220,9 @@ def get_win_archive(archive_id, strict=False): platforms: space-separated list of platforms the file is suitable for use on (string) flags: space-separated list of flags used when building the file (fre, chk) (string) """ - query = QueryProcessor(columns=['archive_id', 'relpath', 'platforms', 'flags'], - tables=['win_archives'], clauses=['archive_id = %(archive_id)i'], + query = QueryProcessor(tables=['win_archives'], + columns=['archive_id', 'relpath', 'platforms', 'flags'], + clauses=['archive_id = %(archive_id)i'], values={'archive_id': archive_id}) return query.executeOne(strict=strict) @@ -5210,15 +5236,18 @@ def get_image_archive(archive_id, strict=False): arch: the architecture of the image rootid: True if this image has the root '/' partition """ - query = QueryProcessor(columns=['archive_id', 'arch'], tables=['image_archives'], + query = QueryProcessor(tables=['image_archives'], + columns=['archive_id', 'arch'], clauses=['archive_id = %(archive_id)i'], values={'archive_id': archive_id}) results = query.executeOne(strict=strict) if not results: return None results['rootid'] = False - query = QueryProcessor(columns=['rpm_id'], clauses=['archive_id = %(archive_id)i'], - tables=['archive_rpm_components'], values={'archive_id': archive_id}) + query = QueryProcessor(tables=['archive_rpm_components'], + columns=['rpm_id'], + clauses=['archive_id = %(archive_id)i'], + values={'archive_id': archive_id}) rpms = query.executeOne() if rpms: results['rootid'] = True @@ -5541,8 +5570,9 @@ def get_channel(channelInfo, strict=False): For example, {'id': 20, 'name': 'container'} """ clause, values = name_or_id_clause('channels', channelInfo) - query = QueryProcessor(columns=['id', 'name', 'description', 'enabled', 'comment'], - tables=['channels'], clauses=[clause], values=values) + query = QueryProcessor(tables=['channels'], + columns=['id', 'name', 'description', 'enabled', 'comment'], + clauses=[clause], values=values) return query.executeOne(strict=strict) @@ -5731,16 +5761,15 @@ def new_package(name, strict=True): verify_name_internal(name) # TODO - table lock? # check for existing - query = QueryProcessor(columns=['id'], values={'name': name}, - tables=['package'], clauses=['name=%(name)s']) + query = QueryProcessor(tables=['package'], columns=['id'], + clauses=['name=%(name)s'], values={'name': name}) pkg_id = query.singleValue(strict=False) if pkg_id: if strict: raise koji.GenericError("Package already exists [id %d]" % pkg_id) else: pkg_id = nextval('package_id_seq') - insert = InsertProcessor('package') - insert.set(id=pkg_id, name=name) + insert = InsertProcessor('package', data={'id': pkg_id, 'name': name}) insert.execute() context.commit_pending = True return pkg_id @@ -7289,29 +7318,26 @@ def merge_scratch(task_id): def get_archive_types(): """Return a list of all supported archive types.""" - query = QueryProcessor(columns=['id', 'name', 'description', 'extensions', 'compression_type'], - tables=['archivetypes'], opts={'order': '-id'}) + query = QueryProcessor(tables=['archivetypes'], + columns=['id', 'name', 'description', 'extensions', 'compression_type'], + opts={'order': 'id'}) return query.execute() def _get_archive_type_by_name(name, strict=True): - query = QueryProcessor(columns=['id', 'name', 'description', 'extensions', 'compression_type'], - tables=['archivetypes'], clauses=['name = %(name)s'], + query = QueryProcessor(tables=['archivetypes'], + columns=['id', 'name', 'description', 'extensions', 'compression_type'], + clauses=['name = %(name)s'], values={'name': name}) - result = query.executeOne() - if strict and not result: - raise koji.GenericError("query returned no rows") - return result + return query.executeOne(strict=strict) def _get_archive_type_by_id(type_id, strict=False): - query = QueryProcessor(columns=['id', 'name', 'description', 'extensions', 'compression_type'], - tables=['archivetypes'], clauses=['id = %(type_id)i'], + query = QueryProcessor(tables=['archivetypes'], + columns=['id', 'name', 'description', 'extensions', 'compression_type'], + clauses=['id = %(type_id)i'], values={'type_id': type_id}) - result = query.executeOne() - if strict and not result: - raise koji.GenericError("query returned no rows") - return result + return query.executeOne(strict=strict) def get_archive_type(filename=None, type_name=None, type_id=None, strict=False): @@ -7455,8 +7481,8 @@ def new_image_build(build_info): # We don't have to worry about updating an image build because the id is # the only thing we care about, and that should never change if a build # fails first and succeeds later on a resubmission. - query = QueryProcessor(tables=['image_builds'], columns=['build_id'], - clauses=['build_id = %(build_id)i'], + query = QueryProcessor(tables=('image_builds',), columns=('build_id',), + clauses=('build_id = %(build_id)i',), values={'build_id': build_info['id']}) result = query.executeOne() if not result: @@ -7471,9 +7497,9 @@ def new_typed_build(build_info, btype): """Mark build as a given btype""" btype_id = lookup_name('btype', btype, strict=True)['id'] - query = QueryProcessor(tables=['build_types'], columns=['build_id'], - clauses=['build_id = %(build_id)i', - 'btype_id = %(btype_id)i'], + query = QueryProcessor(tables=('build_types',), columns=('build_id',), + clauses=('build_id = %(build_id)i', + 'btype_id = %(btype_id)i'), values={'build_id': build_info['id'], 'btype_id': btype_id}) result = query.executeOne() @@ -7925,8 +7951,10 @@ def query_rpm_sigs(rpm_id=None, sigkey=None, queryOpts=None): if sigkey is not None: sigkey = sigkey.lower() clauses.append("sigkey=%(sigkey)s") - query = QueryProcessor(columns=['rpm_id', 'sigkey', 'sighash'], tables=['rpmsigs'], - clauses=clauses, values={'rpm_id': rpm_id, 'sigkey': sigkey}, + query = QueryProcessor(tables=['rpmsigs'], + columns=['rpm_id', 'sigkey', 'sighash'], + clauses=clauses, + values={'rpm_id': rpm_id, 'sigkey': sigkey}, opts=queryOpts) return query.execute() @@ -7948,7 +7976,7 @@ def write_signed_rpm(an_rpm, sigkey, force=False): raise koji.GenericError("Not a regular file: %s" % rpm_path) # make sure we have it in the db rpm_id = rinfo['id'] - query = QueryProcessor(columns=['sighash'], tables=['rpmsigs'], + query = QueryProcessor(tables=['rpmsigs'], columns=['sighash'], clauses=['rpm_id=%(rpm_id)i', 'sigkey=%(sigkey)s'], values={'rpm_id': rpm_id, 'sigkey': sigkey}) sighash = query.singleValue(strict=False) @@ -8278,9 +8306,10 @@ def untagged_builds(name=None, queryOpts=None): if name is not None: clauses.append('package.name = %(name)s') - query = QueryProcessor(columns=['build.id', 'package.name', 'build.version', 'build.release'], + query = QueryProcessor(tables=['build', 'package'], + columns=['build.id', 'package.name', 'build.version', 'build.release'], aliases=['id', 'name', 'version', 'release'], - tables=['build', 'package'], clauses=clauses, values=locals(), + clauses=clauses, values=locals(), opts=queryOpts) return query.iterate() @@ -8308,12 +8337,13 @@ def build_references(build_id, limit=None, lazy=False): return ret # we'll need the component rpm and archive ids for the rest - query = QueryProcessor(columns=['id'], tables=['rpminfo'], clauses=['build_id=%(build_id)i'], + query = QueryProcessor(tables=['rpminfo'], columns=['id'], + clauses=['build_id=%(build_id)i'], values={'build_id': build_id}, opts={'asList': True}) build_rpm_ids = query.execute() - query = QueryProcessor(columns=['id'], tables=['archiveinfo'], values={'build_id': build_id}, - clauses=['build_id=%(build_id)i'], opts={'asList': True}, - aliases=['id']) + query = QueryProcessor(tables=['archiveinfo'], columns=['id'], + clauses=['build_id=%(build_id)i'], + values={'build_id': build_id}, opts={'asList': True}) build_archive_ids = query.execute() if not build_archive_ids: build_archive_ids = [] @@ -8529,7 +8559,7 @@ def _delete_build(binfo): koji.plugin.run_callbacks('preBuildStateChange', attribute='state', old=st_old, new=st_deleted, info=binfo) build_id = binfo['id'] - query = QueryProcessor(columns=['id'], tables=['rpminfo'], clauses=['build_id=%(build_id)i'], + 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""" @@ -8571,8 +8601,8 @@ def reset_build(build): koji.plugin.run_callbacks('preBuildStateChange', attribute='state', old=st_old, new=koji.BUILD_STATES['CANCELED'], info=binfo) - query = QueryProcessor(columns=['id'], tables=['rpminfo'], clauses=['build_id=%(id)i'], - values={'id': binfo['id']}, opts={'asList': True}) + query = QueryProcessor(tables=['rpminfo'], columns=['id'], clauses=['build_id=%(id)i'], + values=binfo['id'], opts={'asList': True}) for (rpm_id,) in query.execute(): delete = """DELETE FROM rpmsigs WHERE rpm_id=%(rpm_id)i""" _dml(delete, locals()) @@ -8582,8 +8612,8 @@ def reset_build(build): _dml(delete, locals()) delete = """DELETE FROM rpminfo WHERE build_id=%(id)i""" _dml(delete, binfo) - query = QueryProcessor(columns=['id'], tables=['archiveinfo'], clauses=['build_id=%(id)i'], - values={'id': binfo['id']}, opts={'asList': True}) + 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()) @@ -8612,7 +8642,7 @@ def reset_build(build): delete = """DELETE FROM tag_listing WHERE build_id = %(id)i""" _dml(delete, binfo) binfo['state'] = koji.BUILD_STATES['CANCELED'] - update = UpdateProcessor('build', values={'id': binfo['id']}, clauses=['id=%(id)s'], + update = UpdateProcessor('build', clauses=['id=%(id)s'], values=binfo, data={'state': binfo['state'], 'task_id': None, 'volume_id': 0}) update.execute() # now clear the build dir @@ -8645,8 +8675,9 @@ def cancel_build(build_id, cancel_task=True): st_old = build['state'] koji.plugin.run_callbacks('preBuildStateChange', attribute='state', old=st_old, new=st_canceled, info=build) - update = UpdateProcessor('build', values={'build_id': build_id, 'st_building': st_building}, + update = UpdateProcessor('build', clauses=['id = %(build_id)i', 'state = %(st_building)i'], + values={'build_id': build_id, 'st_building': st_building}, data={'state': st_canceled}, rawdata={'completion_time': 'NOW()'}) update.execute() build = get_build(build_id) @@ -8723,7 +8754,7 @@ def get_notification_recipients(build, tag_id, state): if state != koji.BUILD_STATES['COMPLETE']: clauses.append('success_only = FALSE') - query = QueryProcessor(columns=['user_id', 'email'], tables=['build_notifications'], + query = QueryProcessor(columns=('user_id', 'email'), tables=['build_notifications'], joins=joins, clauses=clauses, values=locals()) recipients = query.execute() @@ -8838,8 +8869,8 @@ def build_notification(task_id, build_id): def get_build_notifications(user_id): query = QueryProcessor(tables=['build_notifications'], - columns=['id', 'user_id', 'package_id', 'tag_id', - 'success_only', 'email'], + columns=('id', 'user_id', 'package_id', 'tag_id', + 'success_only', 'email'), clauses=['user_id = %(user_id)i'], values={'user_id': user_id}) return query.execute() @@ -8876,9 +8907,9 @@ def add_group_member(group, user, strict=True): # check to see if user is already a member data = {'user_id': uinfo['id'], 'group_id': ginfo['id']} table = 'user_groups' - clauses = ['user_id = %(user_id)i', 'group_id = %(group_id)s'] + clauses = ('user_id = %(user_id)i', 'group_id = %(group_id)s') query = QueryProcessor(columns=['user_id'], tables=[table], - clauses=['active = TRUE'] + clauses, + clauses=('active = TRUE',) + clauses, values=data, opts={'rowlock': True}) row = query.executeOne() if row: @@ -8912,7 +8943,6 @@ def get_group_members(group): ginfo = get_user(group) if not ginfo or ginfo['usertype'] != koji.USERTYPES['GROUP']: raise koji.GenericError("No such group: %s" % group) - group_id = ginfo['id'] query = QueryProcessor(tables=['user_groups'], columns=['id', 'name', 'usertype', 'array_agg(krb_principal)'], aliases=['id', 'name', 'usertype', 'krb_principals'], @@ -8920,7 +8950,7 @@ def get_group_members(group): 'LEFT JOIN user_krb_principals' ' ON users.id = user_krb_principals.user_id'], clauses=[eventCondition(None), 'group_id = %(group_id)i'], - values=locals(), + values={'group_id': ginfo['id']}, opts={'group': 'users.id'}, enable_group=True, transform=xform_user_krb) @@ -8935,8 +8965,8 @@ def set_user_status(user, status): # nothing to do return user_id = user['id'] - update = UpdateProcessor('users', values={'user_id': user_id}, clauses=['id = %(user_id)i'], - data={'status': status}) + update = UpdateProcessor('users', clauses=['id = %(user_id)i'], + values={'user_id': user_id}, data={'status': status}) rows = update.execute() # sanity check if rows == 0: @@ -12432,8 +12462,8 @@ class RootExports(object): If no users of the specified type exist, return an empty list.""" - fields = ['id', 'name', 'status', 'usertype', 'array_agg(krb_principal)'] - aliases = ['id', 'name', 'status', 'usertype', 'krb_principals'] + fields = ('id', 'name', 'status', 'usertype', 'array_agg(krb_principal)') + aliases = ('id', 'name', 'status', 'usertype', 'krb_principals') joins = ['LEFT JOIN user_krb_principals ON users.id = user_krb_principals.user_id'] clauses = ['usertype = %(userType)i'] if prefix: @@ -12445,7 +12475,7 @@ class RootExports(object): else: raise koji.GenericError('queryOpts.group is not available for this API') query = QueryProcessor(columns=fields, aliases=aliases, - tables=['users'], joins=joins, clauses=clauses, + tables=('users',), joins=joins, clauses=clauses, values=locals(), opts=queryOpts, enable_group=True, transform=xform_user_krb) return query.execute() @@ -12847,8 +12877,8 @@ class RootExports(object): arches = koji.parse_arches(arches, strict=True) if get_host(hostname): raise koji.GenericError('host already exists: %s' % hostname) - query = QueryProcessor(columns=['id'], tables=['channels'], clauses=["name = 'default'"]) - default_channel = query.singleValue(strict=False) + query = QueryProcessor(tables=['channels'], columns=['id'], clauses=["name = 'default'"]) + default_channel = query.singleValue(strict=True) # builder user can already exist, if host tried to log in before adding into db userinfo = {'name': hostname} if krb_principal: @@ -12877,8 +12907,7 @@ class RootExports(object): krb_principal=krb_principal) # host entry hostID = nextval('host_id_seq') - insert = InsertProcessor('host') - insert.set(id=hostID, user_id=userID, name=hostname) + insert = InsertProcessor('host', data={'id': hostID, 'user_id': userID, 'name': hostname}) insert.execute() insert = InsertProcessor('host_config') @@ -13048,7 +13077,8 @@ class RootExports(object): - name - description """ - query = QueryProcessor(columns=['id', 'name', 'description'], tables=['permissions'], + query = QueryProcessor(tables=['permissions'], + columns=['id', 'name', 'description'], opts={'order': 'id'}) return query.execute() @@ -13088,8 +13118,10 @@ class RootExports(object): koji.plugin.run_callbacks('preBuildStateChange', attribute='owner_id', old=owner_id_old, new=userid, info=buildinfo) - update = UpdateProcessor('build', values={'buildid': buildinfo['id']}, - clauses=['id=%(buildid)i'], data={'owner': userid}) + update = UpdateProcessor('build', + clauses=['id=%(buildid)i'], + values={'buildid': buildinfo['id']}, + data={'owner': userid}) update.execute() koji.plugin.run_callbacks('postBuildStateChange', attribute='owner_id', old=owner_id_old, new=userid, @@ -13249,8 +13281,8 @@ class RootExports(object): raise GenericError, else return None. """ query = QueryProcessor(tables=['build_notifications'], - columns=['id', 'user_id', 'package_id', 'tag_id', - 'success_only', 'email'], + columns=('id', 'user_id', 'package_id', 'tag_id', + 'success_only', 'email'), clauses=['id = %(id)i'], values={'id': id}) result = query.executeOne() @@ -13272,7 +13304,7 @@ class RootExports(object): raise GenericError, else return None. """ query = QueryProcessor(tables=['build_notifications_block'], - columns=['id', 'user_id', 'package_id', 'tag_id'], + columns=('id', 'user_id', 'package_id', 'tag_id'), clauses=['id = %(id)i'], values={'id': id}) result = query.executeOne() @@ -13509,7 +13541,7 @@ class RootExports(object): clause = 'name %s %%(terms)s' % oper query = QueryProcessor(columns=cols, - aliases=aliases, tables=[table], + aliases=aliases, tables=(table,), joins=joins, clauses=(clause,), values=locals(), opts=queryOpts) return query.iterate() @@ -13863,8 +13895,9 @@ class Host(object): The return value is [finished, unfinished] where each entry is a list of task ids.""" # check to see if any of the tasks have finished - query = QueryProcessor(columns=['id', 'state'], tables=['task'], values={'parent': parent}, + query = QueryProcessor(tables=['task'], columns=['id', 'state'], clauses=['parent=%(parent)s', 'awaited = TRUE'], + values={'parent': parent}, opts={'rowlock': True}) result = query.execute() canceled = koji.TASK_STATES['CANCELED'] @@ -13886,8 +13919,8 @@ class Host(object): if finished: context.commit_pending = True for id in finished: - update = UpdateProcessor('task', values={'id': id}, clauses=['id=%(id)s'], - rawdata={'awaited': 'false'}) + update = UpdateProcessor('task', clauses=['id=%(id)s'], + values={'id': id}, rawdata={'awaited': 'false'}) update.execute() return [finished, unfinished] @@ -13927,7 +13960,7 @@ class Host(object): host_id = self.id # query tasks st_open = koji.TASK_STATES['OPEN'] - query = QueryProcessor(columns=['id', 'waiting', 'weight'], tables=['task'], + query = QueryProcessor(tables=['task'], columns=['id', 'waiting', 'weight'], clauses=['host_id = %(host_id)s', 'state = %(st_open)s'], values={'host_id': host_id, 'st_open': st_open}) tasks = query.execute() @@ -13943,7 +13976,7 @@ class Host(object): host_data = get_host(self.id) task_load = float(task_load) if task_load != host_data['task_load'] or ready != host_data['ready']: - update = UpdateProcessor('host', values={'id': self.id}, clauses=['id=%(id)i'], + update = UpdateProcessor('host', clauses=['id=%(id)i'], values={'id': self.id}, data={'task_load': task_load, 'ready': ready}) update.execute() context.commit_pending = True @@ -13970,25 +14003,26 @@ class Host(object): id = self.id # get arch and channel info for host values = {'id': id} - query = QueryProcessor(columns=['arches'], tables=['host_config'], + query = QueryProcessor(tables=['host_config'], columns=['arches'], clauses=['host_id = %(id)s', 'active IS TRUE'], values=values) - arches = query.singleValue() - query = QueryProcessor(columns=['channel_id'], tables=['host_channels'], + arches = query.singleValue().split() + query = QueryProcessor(tables=['host_channels'], columns=['channel_id'], clauses=['host_id = %(id)s', 'active IS TRUE'], values=values, opts={'asList': True}) - channels = query.execute() + channels = [x[0] for x in query.execute()] # query tasks - fields = ['id', 'state', 'method', 'request', 'channel_id', 'arch', 'parent'] - st_free = koji.TASK_STATES['FREE'] - st_assigned = koji.TASK_STATES['ASSIGNED'] - q = """ - SELECT %s FROM task - WHERE (state = %%(st_free)s) - OR (state = %%(st_assigned)s AND host_id = %%(id)s) - ORDER BY priority,create_time - """ % (",".join(fields)) - c.execute(q, {'st_free': st_free, 'st_assigned': st_assigned, 'id': id}) + query = QueryProcessor(tables=['task'], + columns=['id', 'state', 'method', 'request', + 'channel_id', 'arch', 'parent'], + clauses=['(state = %(st_free)s) OR ' + '(state = %(st_assigned)s AND host_id = %(id)s)'], + values={ + 'st_free': koji.TASK_STATES['FREE'], + 'st_assigned': koji.TASK_STATES['ASSIGNED'], + 'id': id + }, + queryOpts={'order': 'priority,create_time'}) for data in c.fetchall(): data = dict(zip(fields, data)) # XXX - we should do some pruning here, but for now... @@ -14010,8 +14044,9 @@ class Host(object): def isEnabled(self): """Return whether this host is enabled or not.""" - query = QueryProcessor(columns=['enabled'], tables=['host_config'], values={'id': self.id}, - clauses=['host_id = %(id)i', 'active IS TRUE']) + query = QueryProcessor(tables=['host_config'], columns=['enabled'], + clauses=['host_id = %(id)i', 'active IS TRUE'], + values={'id': self.id}) return query.singleValue(strict=True) @@ -14107,9 +14142,9 @@ class HostExports(object): opts['parent'] = parent if 'label' in opts: # first check for existing task with this parent/label - query = QueryProcessor(columns=['id'], tables=['task'], - clauses=['parent=%(parent)s', 'label=%(label)s'], - values={'parent': opts['parent'], 'label': opts['label']}) + query = QueryProcessor(tables=['task'], columns=['id'], + clauses=['parent = %(parent)s', 'label = %(label)s'], + values=opts) task_id = query.singleValue(strict=False) if task_id: # return task id @@ -14628,7 +14663,7 @@ class HostExports(object): koji.plugin.run_callbacks('preBuildStateChange', attribute='state', old=st_old, new=st_failed, info=buildinfo) - query = QueryProcessor(columns=['state', 'completion_time'], tables=['build'], + query = QueryProcessor(tables=['build'], columns=['state', 'completion_time'], clauses=['id = %(build_id)i'], values={'build_id': build_id}, opts={'rowlock': True}) result = query.executeOne() @@ -14640,7 +14675,8 @@ class HostExports(object): (build_id, result['completion_time'])) update = UpdateProcessor('build', values={'build_id': build_id}, - clauses=['id = %(build_id)i'], data={'state': st_failed}, + clauses=['id = %(build_id)i'], + data={'state': st_failed}, rawdata={'completion_time': 'NOW()'}) update.execute() buildinfo = get_build(build_id, strict=True) From bf6d72069fba6747e1ee0a244599b0cb84d1586a Mon Sep 17 00:00:00 2001 From: Jana Cupova Date: Oct 11 2022 12:32:31 +0000 Subject: [PATCH 3/4] Fixes --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 5074ca9..2b57157 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -480,7 +480,7 @@ class Task(object): raise koji.GenericError("Task LOOP at task %i" % task_id) task_id = parent seen[task_id] = 1 - query.values = values + query.values = {'task_id': task_id} parent = query.singleValue() return Task(task_id).cancelFull(strict=True) # We handle the recursion ourselves, since self.cancel will stop at @@ -3067,10 +3067,10 @@ def repo_references(repo_id): 'state': 'state'} fields, aliases = zip(*fields.items()) query = QueryProcessor( - tables=['standard_buildroot'], - columns=fields, aliases=aliases, - clauses=['repo_id=%(repo_id)s', 'retire_event IS NULL'], - values={'repo_id': repo_id}) + tables=['standard_buildroot'], + columns=fields, aliases=aliases, + clauses=['repo_id=%(repo_id)s', 'retire_event IS NULL'], + values={'repo_id': repo_id}) # check results for bad states ret = [] for data in query.execute(): @@ -3233,7 +3233,7 @@ def _edit_build_target(buildTargetInfo, name, build_tag, dest_tag): raise koji.GenericError('name "%s" is already taken by build target %i' % (name, id)) update = UpdateProcessor('build_target', clauses=['id = %(buildTargetID)i'], - values=values, data={'name': name}) + values=values, data={'name': name}) update.execute() update = UpdateProcessor('build_target_config', values=values, @@ -8602,7 +8602,7 @@ def reset_build(build): attribute='state', old=st_old, new=koji.BUILD_STATES['CANCELED'], info=binfo) query = QueryProcessor(tables=['rpminfo'], columns=['id'], clauses=['build_id=%(id)i'], - values=binfo['id'], opts={'asList': True}) + values=binfo, opts={'asList': True}) for (rpm_id,) in query.execute(): delete = """DELETE FROM rpmsigs WHERE rpm_id=%(rpm_id)i""" _dml(delete, locals()) @@ -13999,7 +13999,6 @@ class Host(object): def getTask(self): """Open next available task and return it""" - c = context.cnx.cursor() id = self.id # get arch and channel info for host values = {'id': id} @@ -14020,17 +14019,17 @@ class Host(object): values={ 'st_free': koji.TASK_STATES['FREE'], 'st_assigned': koji.TASK_STATES['ASSIGNED'], - 'id': id - }, - queryOpts={'order': 'priority,create_time'}) - for data in c.fetchall(): - data = dict(zip(fields, data)) + 'id': id, }, + queryOpts={'order': 'priority,create_time'} + ) + for data in query.execute(): # XXX - we should do some pruning here, but for now... # check arch if data['arch'] not in arches: continue # NOTE: channels ignored for explicit assignments - if data['state'] != st_assigned and data['channel_id'] not in channels: + if data['state'] != koji.TASK_STATES['ASSIGNED'] and \ + data['channel_id'] not in channels: continue task = Task(data['id']) ret = task.open(self.id) From 9888e6916d4a8950ded2a9b3ecf4b2fda88a78d1 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Nov 04 2022 11:28:16 +0000 Subject: [PATCH 4/4] readability fixes --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 2b57157..a3a0343 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -83,7 +83,6 @@ from koji.db import ( UpdateProcessor, _applyQueryOpts, _dml, - _fetchMulti, _fetchSingle, _multiRow, _singleRow, @@ -456,7 +455,7 @@ class Task(object): """Cancel child tasks""" query = QueryProcessor(tables=['task'], columns=['id'], clauses=['parent = %(task_id)i'], values={'task_id': self.id}, opts={'asList': True}) - for (id, ) in query.execute(): + for (id,) in query.execute(): Task(id).cancel(recurse=True) def cancelFull(self, strict=True): @@ -497,8 +496,7 @@ class Task(object): query = QueryProcessor(tables=['task'], columns=['id'], clauses=['parent = %(task_id)i'], values={'task_id': task_id}, opts={'asList': True}) - result = query.execute() - for (child_id,) in result: + for (child_id,) in query.execute(): tasklist.append(child_id) def getRequest(self): @@ -2549,9 +2547,9 @@ def get_ready_hosts(): columns=['host.id', 'name', 'arches', 'task_load', 'capacity'], aliases=['id', 'name', 'arches', 'task_load', 'capacity'], clauses=[ - 'enabled = TRUE', - 'ready = TRUE', - 'expired = FALSE', + 'enabled IS TRUE', + 'ready IS TRUE', + 'expired IS FALSE', 'master IS NULL', 'active IS TRUE', "update_time > NOW() - '5 minutes'::interval" @@ -2749,7 +2747,7 @@ def repo_init(tag, task_id=None, with_src=False, with_debuginfo=False, event=Non # make sure event is valid query = QueryProcessor(tables=['events'], columns=['time'], clauses=['id=%(event)s'], values={'event': event}) - query.singleValue() + query.singleValue(strict=True) event_id = event insert = InsertProcessor('repo') insert.set(id=repo_id, create_event=event_id, tag_id=tag_id, state=state, task_id=task_id) @@ -8364,7 +8362,7 @@ def build_references(build_id, limit=None, lazy=False): AND build.state = %(st_complete)i""" if limit is not None: q += "\nLIMIT %(limit)i" - for (rpm_id, ) in build_rpm_ids: + for (rpm_id,) in build_rpm_ids: for row in _multiRow(q, locals(), fields): idx.setdefault(row['id'], row) if limit is not None and len(idx) > limit: @@ -13896,7 +13894,7 @@ class Host(object): is a list of task ids.""" # check to see if any of the tasks have finished query = QueryProcessor(tables=['task'], columns=['id', 'state'], - clauses=['parent=%(parent)s', 'awaited = TRUE'], + clauses=['parent=%(parent)s', 'awaited IS TRUE'], values={'parent': parent}, opts={'rowlock': True}) result = query.execute() @@ -13920,7 +13918,7 @@ class Host(object): context.commit_pending = True for id in finished: update = UpdateProcessor('task', clauses=['id=%(id)s'], - values={'id': id}, rawdata={'awaited': 'false'}) + values={'id': id}, data={'awaited': False}) update.execute() return [finished, unfinished] diff --git a/tests/test_hub/test_add_host.py b/tests/test_hub/test_add_host.py index 36a5782..4b85192 100644 --- a/tests/test_hub/test_add_host.py +++ b/tests/test_hub/test_add_host.py @@ -27,6 +27,7 @@ class TestAddHost(unittest.TestCase): query = QP(*args, **kwargs) query.execute = mock.MagicMock() query.executeOne = mock.MagicMock() + query.singleValue = self.query_singleValue self.queries.append(query) return query @@ -51,9 +52,9 @@ class TestAddHost(unittest.TestCase): self.verify_host_name = mock.patch('kojihub.verify_host_name').start() self.verify_name_user = mock.patch('kojihub.verify_name_user').start() self.get_host = mock.patch('kojihub.get_host').start() - self._singleValue = mock.patch('kojihub._singleValue').start() self.nextval = mock.patch('kojihub.nextval').start() self.get_user = mock.patch('kojihub.get_user').start() + self.query_singleValue = mock.MagicMock() def tearDown(self): mock.patch.stopall() @@ -70,10 +71,10 @@ class TestAddHost(unittest.TestCase): def test_add_host_valid(self): self.verify_host_name.return_value = None self.get_host.return_value = {} - self._singleValue.return_value = 333 self.nextval.return_value = 12 self.context.session.createUser.return_value = 456 self.get_user.return_value = None + self.query_singleValue.return_value = 333 r = self.exports.addHost('hostname', ['i386', 'x86_64']) self.assertEqual(r, 12) @@ -150,9 +151,11 @@ class TestAddHost(unittest.TestCase): 'usertype': koji.USERTYPES['GROUP'] } self.get_host.return_value = {} + self.query_singleValue.return_value = 333 - with self.assertRaises(koji.GenericError): + with self.assertRaises(koji.GenericError) as ex: self.exports.addHost('hostname', ['i386', 'x86_64'], force=True) + self.assertEqual("user hostname already exists and it is not a host", str(ex.exception)) self.get_user.assert_called_once_with(userInfo={'name': 'hostname'}) self.get_host.assert_called_once_with('hostname') diff --git a/tests/test_hub/test_add_rpm_sig.py b/tests/test_hub/test_add_rpm_sig.py index 40d1325..7d0e7ea 100644 --- a/tests/test_hub/test_add_rpm_sig.py +++ b/tests/test_hub/test_add_rpm_sig.py @@ -7,6 +7,7 @@ import koji import kojihub IP = kojihub.InsertProcessor +QP = kojihub.QueryProcessor class TestAddRPMSig(unittest.TestCase): @@ -16,10 +17,20 @@ class TestAddRPMSig(unittest.TestCase): self.inserts.append(insert) return insert + def getQuery(self, *args, **kwargs): + query = QP(*args, **kwargs) + query.execute = self.query_execute + self.queries.append(query) + return query + def setUp(self): self.InsertProcessor = mock.patch('kojihub.InsertProcessor', side_effect=self.getInsert).start() self.inserts = [] + self.QueryProcessor = mock.patch('kojihub.QueryProcessor', + side_effect=self.getQuery).start() + self.queries = [] + self.query_execute = mock.MagicMock() self.context = mock.patch('kojihub.context').start() # It seems MagicMock will not automatically handle attributes that # start with "assert" @@ -32,7 +43,6 @@ class TestAddRPMSig(unittest.TestCase): def tearDown(self): mock.patch.stopall() - @mock.patch('kojihub._fetchMulti') @mock.patch('koji.plugin.run_callbacks') @mock.patch('kojihub.get_rpm') @mock.patch('kojihub.get_build') @@ -46,10 +56,9 @@ class TestAddRPMSig(unittest.TestCase): isdir, get_build, get_rpm, - run_callbacks, - _fetchMulti): + run_callbacks): """Test addRPMSig with header-only signed RPM""" - _fetchMulti.side_effect = [[]] + self.query_execute.side_effect = [[]] isdir.side_effect = [True] get_rpm.side_effect = [{ 'id': 1, diff --git a/tests/test_hub/test_edit_build_target.py b/tests/test_hub/test_edit_build_target.py index 68f9e9c..6d351d4 100644 --- a/tests/test_hub/test_edit_build_target.py +++ b/tests/test_hub/test_edit_build_target.py @@ -5,14 +5,23 @@ import mock import koji import kojihub +QP = kojihub.QueryProcessor + class TestEditBuildTarget(unittest.TestCase): + def getQuery(self, *args, **kwargs): + query = QP(*args, **kwargs) + query.execute = mock.MagicMock() + query.executeOne = mock.MagicMock() + query.singleValue = self.query_singleValue + self.queries.append(query) + return query + def setUp(self): self.lookup_build_target = mock.patch('kojihub.lookup_build_target').start() self.verify_name_internal = mock.patch('kojihub.verify_name_internal').start() self.get_tag = mock.patch('kojihub.get_tag').start() - self._singleValue = mock.patch('kojihub._singleValue').start() self.exports = kojihub.RootExports() self.target_name = 'build-target' self.name = 'build-target-rename' @@ -23,6 +32,10 @@ class TestEditBuildTarget(unittest.TestCase): self.dest_tag_info = {'id': 112, 'name': self.dest_tag} self.session = kojihub.context.session = mock.MagicMock() self.session.assertPerm = mock.MagicMock() + self.QueryProcessor = mock.patch('kojihub.QueryProcessor', + side_effect=self.getQuery).start() + self.queries = [] + self.query_singleValue = mock.MagicMock() def tearDown(self): mock.patch.stopall() @@ -81,7 +94,7 @@ class TestEditBuildTarget(unittest.TestCase): self.verify_name_internal.return_value = None self.lookup_build_target.return_value = self.target_info self.get_tag.side_effect = [self.build_tag_info, self.dest_tag_info] - self._singleValue.return_value = 2 + self.query_singleValue.return_value = 2 with self.assertRaises(koji.GenericError) as cm: self.exports.editBuildTarget(self.target_name, self.name, self.build_tag, self.dest_tag) @@ -91,4 +104,3 @@ class TestEditBuildTarget(unittest.TestCase): self.verify_name_internal.called_once_with(name=self.name) self.lookup_build_target.called_once_with(self.target_name) self.get_tag.has_calls([mock.call(self.build_tag), mock.call(self.dest_tag)]) - self._singleValue.called_once_with(self.name) diff --git a/tests/test_hub/test_edit_tag.py b/tests/test_hub/test_edit_tag.py index 192b8b7..4295ca4 100644 --- a/tests/test_hub/test_edit_tag.py +++ b/tests/test_hub/test_edit_tag.py @@ -8,6 +8,7 @@ import kojihub UP = kojihub.UpdateProcessor IP = kojihub.InsertProcessor +QP = kojihub.QueryProcessor class TestEditTag(unittest.TestCase): @@ -23,6 +24,14 @@ class TestEditTag(unittest.TestCase): self.updates.append(update) return update + def getQuery(self, *args, **kwargs): + query = QP(*args, **kwargs) + query.execute = mock.MagicMock() + query.executeOne = mock.MagicMock() + query.singleValue = self.query_singleValue + self.queries.append(query) + return query + def setUp(self): self.InsertProcessor = mock.patch('kojihub.InsertProcessor', side_effect=self.getInsert).start() @@ -30,7 +39,6 @@ class TestEditTag(unittest.TestCase): self.UpdateProcessor = mock.patch('kojihub.UpdateProcessor', side_effect=self.getUpdate).start() self.updates = [] - self._singleValue = mock.patch('kojihub._singleValue').start() self.get_tag = mock.patch('kojihub.get_tag').start() self.get_perm_id = mock.patch('kojihub.get_perm_id').start() self.verify_name_internal = mock.patch('kojihub.verify_name_internal').start() @@ -39,6 +47,10 @@ class TestEditTag(unittest.TestCase): # It seems MagicMock will not automatically handle attributes that # start with "assert" self.context_db.session.assertLogin = mock.MagicMock() + self.QueryProcessor = mock.patch('kojihub.QueryProcessor', + side_effect=self.getQuery).start() + self.queries = [] + self.query_singleValue = mock.MagicMock() def tearDown(self): mock.patch.stopall() @@ -64,7 +76,7 @@ class TestEditTag(unittest.TestCase): 'extra': {'exA': 1, 'exC': 3, 'exD': 4}} - self._singleValue.return_value = None + self.query_singleValue.return_value = None self.verify_name_internal.return_value = None self.context_db.event_id = 42 self.context_db.session.user_id = 23 @@ -198,8 +210,7 @@ class TestEditTag(unittest.TestCase): # no4 invoke self.get_perm_id.reset_mock() self.get_perm_id.return_value = 99 - self._singleValue.reset_mock() - self._singleValue.return_value = 2 + self.query_singleValue.return_value = 2 kwargs = { 'perm': 'admin', @@ -208,7 +219,6 @@ class TestEditTag(unittest.TestCase): with self.assertRaises(koji.GenericError) as cm: kojihub._edit_tag('tag', **kwargs) self.get_perm_id.assert_called_once() - self._singleValue.assert_called_once() self.assertEqual(cm.exception.args[0], 'Name newtag already taken by tag 2') def test_invalid_archs(self): diff --git a/tests/test_hub/test_edit_user.py b/tests/test_hub/test_edit_user.py index 5fc34ae..b7b7486 100644 --- a/tests/test_hub/test_edit_user.py +++ b/tests/test_hub/test_edit_user.py @@ -6,6 +6,7 @@ import koji import kojihub UP = kojihub.UpdateProcessor +QP = kojihub.QueryProcessor class TestEditUser(unittest.TestCase): @@ -16,9 +17,16 @@ class TestEditUser(unittest.TestCase): self.updates.append(update) return update + def getQuery(self, *args, **kwargs): + query = QP(*args, **kwargs) + query.execute = mock.MagicMock() + query.executeOne = mock.MagicMock() + query.singleValue = self.query_singleValue + self.queries.append(query) + return query + def setUp(self): self.updates = [] - self._singleValue = mock.patch('kojihub._singleValue').start() self.get_user = mock.patch('kojihub.get_user').start() self.verify_name_user = mock.patch('kojihub.verify_name_user').start() self.context = mock.patch('kojihub.context').start() @@ -27,6 +35,10 @@ class TestEditUser(unittest.TestCase): # It seems MagicMock will not automatically handle attributes that # start with "assert" self.context.session.assertLogin = mock.MagicMock() + self.QueryProcessor = mock.patch('kojihub.QueryProcessor', + side_effect=self.getQuery).start() + self.queries = [] + self.query_singleValue = mock.MagicMock() def tearDown(self): mock.patch.stopall() @@ -35,7 +47,7 @@ class TestEditUser(unittest.TestCase): self.get_user.return_value = {'id': 333, 'name': 'user', 'krb_principals': ['krb']} - self._singleValue.return_value = None + self.query_singleValue.return_value = None self.verify_name_user.return_value = None kojihub._edit_user('user', name='newuser') @@ -88,11 +100,10 @@ class TestEditUser(unittest.TestCase): self.context.session.removeKrbPrincipal.assert_not_called() self.context.session.setKrbPrincipal.assert_not_called() - self._singleValue.reset_mock() - self._singleValue.return_value = 2 + self.query_singleValue.reset_mock() + self.query_singleValue.return_value = 2 with self.assertRaises(koji.GenericError) as cm: kojihub._edit_user('user', name='newuser') - self._singleValue.assert_called_once() self.assertEqual(cm.exception.args[0], 'Name newuser already taken by user 2') diff --git a/tests/test_hub/test_find_build_id.py b/tests/test_hub/test_find_build_id.py index dd29172..4f94797 100644 --- a/tests/test_hub/test_find_build_id.py +++ b/tests/test_hub/test_find_build_id.py @@ -5,12 +5,24 @@ import mock import koji import kojihub +QP = kojihub.QueryProcessor + class TestFindBuildId(unittest.TestCase): + def getQuery(self, *args, **kwargs): + query = QP(*args, **kwargs) + query.execute = mock.MagicMock() + query.executeOne = mock.MagicMock() + query.singleValue = self.query_singleValue + self.queries.append(query) + return query + def setUp(self): - self.context = mock.patch('kojihub.context').start() - self.cursor = mock.MagicMock() + self.QueryProcessor = mock.patch('kojihub.QueryProcessor', + side_effect=self.getQuery).start() + self.queries = [] + self.query_singleValue = mock.MagicMock() def test_non_exist_build_dict(self): build = { @@ -18,16 +30,13 @@ class TestFindBuildId(unittest.TestCase): 'version': 'test_version', 'release': 'test_release', } - self.cursor.fetchone.return_value = None - self.context.cnx.cursor.return_value = self.cursor + self.query_singleValue.return_value = None with self.assertRaises(koji.GenericError) as cm: kojihub.find_build_id(build, strict=True) self.assertEqual("No such build: %s" % build, str(cm.exception)) def test_invalid_argument(self): build = ['test-build'] - self.cursor.fetchone.return_value = None - self.context.cnx.cursor.return_value = self.cursor with self.assertRaises(koji.GenericError) as cm: kojihub.find_build_id(build) self.assertEqual("Invalid type for argument: %s" % type(build), str(cm.exception)) @@ -40,8 +49,6 @@ class TestFindBuildId(unittest.TestCase): 'owner': 'test_owner', 'extra': {'extra_key': 'extra_value'}, } - self.cursor.fetchone.return_value = None - self.context.cnx.cursor.return_value = self.cursor with self.assertRaises(koji.GenericError) as cm: kojihub.find_build_id(build, strict=True) self.assertEqual("did not provide name, version, and release", str(cm.exception)) diff --git a/tests/test_hub/test_models/test_host.py b/tests/test_hub/test_models/test_host.py index 9df50cd..ef01052 100644 --- a/tests/test_hub/test_models/test_host.py +++ b/tests/test_hub/test_models/test_host.py @@ -6,6 +6,7 @@ import koji import kojihub UP = kojihub.UpdateProcessor +QP = kojihub.QueryProcessor class TestHost(unittest.TestCase): @@ -16,10 +17,20 @@ class TestHost(unittest.TestCase): self.updates.append(update) return update + def getQuery(self, *args, **kwargs): + query = QP(*args, **kwargs) + query.execute = self.query_execute + self.queries.append(query) + return query + def setUp(self): self.UpdateProcessor = mock.patch('kojihub.UpdateProcessor', side_effect=self.getUpdate).start() self.updates = [] + self.QueryProcessor = mock.patch('kojihub.QueryProcessor', + side_effect=self.getQuery).start() + self.queries = [] + self.query_execute = mock.MagicMock() @mock.patch('kojihub.context') def test_instantiation_not_a_host(self, context): @@ -118,55 +129,41 @@ class TestHost(unittest.TestCase): ) self.assertEqual(processor.call_args_list[2], update3) - @mock.patch('kojihub.context') - def test_task_wait_check(self, context): - cursor = mock.MagicMock() - context.cnx.cursor.return_value = cursor - cursor.fetchall.return_value = [ - (1, 1), - (2, 2), - (3, 3), - (4, 4), - ] + def test_task_wait_check(self): + self.query_execute.return_value = [{'id': 1, 'state': 1}, + {'id': 2, 'state': 2}, + {'id': 3, 'state': 3}, + {'id': 4, 'state': 4}, ] host = kojihub.Host(id=1234) finished, unfinished = host.taskWaitCheck(parent=123) - cursor.execute.assert_called_once() self.assertEqual(finished, [2, 3]) self.assertEqual(unfinished, [1, 4]) @mock.patch('kojihub.context') def test_task_wait(self, context): - cursor = mock.MagicMock() - context.cnx.cursor.return_value = cursor - context.session.assertLogin = mock.MagicMock() - cursor.fetchall.return_value = [ - (1, 1), - (2, 2), - (3, 3), - (4, 4), - ] - context.event_id = 42 - context.session.user_id = 23 + self.query_execute.return_value = [{'id': 1, 'state': 1}, + {'id': 2, 'state': 2}, + {'id': 3, 'state': 3}, + {'id': 4, 'state': 4}, ] kojihub.Host.return_value = 1234 host = kojihub.Host(id=1234) host.taskWait(parent=123) self.assertEqual(len(self.updates), 2) - self.assertEqual(len(cursor.execute.mock_calls), 1) - rawdata = {'awaited': 'false'} + data = {'awaited': False} update = self.updates[0] values = {'id': 2} self.assertEqual(update.table, 'task') self.assertEqual(update.values, values) - self.assertEqual(update.data, {}) - self.assertEqual(update.rawdata, rawdata) + self.assertEqual(update.data, data) + self.assertEqual(update.rawdata, {}) self.assertEqual(update.clauses, ['id=%(id)s']) update = self.updates[1] values = {'id': 3} self.assertEqual(update.table, 'task') self.assertEqual(update.values, values) - self.assertEqual(update.data, {}) - self.assertEqual(update.rawdata, rawdata) + self.assertEqual(update.data, data) + self.assertEqual(update.rawdata, {}) self.assertEqual(update.clauses, ['id=%(id)s'])