PR#212: direct tag functions for hub - koji

koji

#212 direct tag functions for hub

Merged 7 years ago by mikem. Opened 7 years ago by mikem.

https://github.com/mikem23/koji-playground.git hub-direct-tag-functions into master

include tag_extra data when deleting a tag

Mike McLean • 7 years ago

8e1e72b

handle long type for tag ids

Mike McLean • 7 years ago

b48e3f2

rename test file

Mike McLean • 7 years ago

caf17b6

test for _untag_build

Mike McLean • 7 years ago

ce59d83

fix up test case

Mike McLean • 7 years ago

6f0685c

more direct tag calls, also a fix

Mike McLean • 7 years ago

434bbb9

_direct_pkglist_remove

Mike McLean • 7 years ago

63168e4

_direct_untag_build for consistency

Mike McLean • 7 years ago

3d0791f

stub test for _tag_build

Mike McLean • 7 years ago

38a42e9

unit test for create_tag()

Mike McLean • 7 years ago

4c6992a

more pkglist...

Mike McLean • 7 years ago

12d339d

also pkglist_add

Mike McLean • 7 years ago

06692a2

provide some lower-level functions sans access check

Mike McLean • 7 years ago

71f1b22

hub/kojihub.py

file modified

+121 -27

		`@@ -836,6 +836,13 @@`

		`def pkglist_add(taginfo, pkginfo, owner=None, block=None, extra_arches=None, force=False, update=False):`
		`"""Add to (or update) package list for tag"""`
		`+ return _direct_pkglist_add(taginfo, pkginfo, owner, block, extra_arches,`
		`+ force, update, policy=True)`
		`+`
		`+`
		`+ def _direct_pkglist_add(taginfo, pkginfo, owner, block, extra_arches, force,`
		`+ update, policy=False):`
		`+ """Like pkglist_add, but without policy or access check"""`
		`#access control comes a little later (via an assert_policy)`
		`#should not make any changes until after policy is checked`
		`tag = get_tag(taginfo, strict=True)`
		`@@ -851,11 +858,12 @@`
		`action = 'update'`
		`elif bool(block):`
		`action = 'block'`
		`- context.session.assertLogin()`
		`- policy_data = {'tag' : tag_id, 'action' : action, 'package' : pkginfo, 'force' : force}`
		`- #don't check policy for admins using force`
		`- if not (force and context.session.hasPerm('admin')):`
		`- assert_policy('package_list', policy_data)`
		`+ if policy:`
		`+ context.session.assertLogin()`
		`+ policy_data = {'tag' : tag_id, 'action' : action, 'package' : pkginfo, 'force' : force}`
		`+ #don't check policy for admins using force`
		`+ if not (force and context.session.hasPerm('admin')):`
		`+ assert_policy('package_list', policy_data)`
		`if not pkg:`
		`pkg = lookup_package(pkginfo, create=True)`
		`koji.plugin.run_callbacks('prePackageListChange', action=action, tag=tag, package=pkg, owner=owner,`
		`@@ -914,17 +922,24 @@`
		`The main reason to remove an entry like this is to remove an override so`
		`that the package data can be inherited from elsewhere.`
		`"""`
		`+ _direct_pkglist_remove(taginfo, pkginfo, force, policy=True)`
		`+`
		`+`
		`+ def _direct_pkglist_remove(taginfo, pkginfo, force=False, policy=False):`
		`+ """Like pkglist_remove, but without policy check"""`
		`tag = get_tag(taginfo, strict=True)`
		`pkg = lookup_package(pkginfo, strict=True)`
		`- context.session.assertLogin()`
		`- policy_data = {'tag' : tag['id'], 'action' : 'remove', 'package' : pkg['id'], 'force' : force}`
		`- #don't check policy for admins using force`
		`- if not (force and context.session.hasPerm('admin')):`
		`- assert_policy('package_list', policy_data)`
		`+ if policy:`
		`+ context.session.assertLogin()`
		`+ policy_data = {'tag' : tag['id'], 'action' : 'remove', 'package' : pkg['id'], 'force' : force}`
		`+ #don't check policy for admins using force`
		`+ if not (force and context.session.hasPerm('admin')):`
		`+ assert_policy('package_list', policy_data)`
		`koji.plugin.run_callbacks('prePackageListChange', action='remove', tag=tag, package=pkg)`
		`_pkglist_remove(tag['id'], pkg['id'])`
		`koji.plugin.run_callbacks('postPackageListChange', action='remove', tag=tag, package=pkg)`

		`+`
		`def pkglist_block(taginfo, pkginfo):`
		`"""Block the package in tag"""`
		`pkglist_add(taginfo, pkginfo, block=True)`
		`@@ -1461,16 +1476,22 @@`
		`else:`
		`# use the user associated with the current session`
		`user = get_user(context.session.user_id, strict=True)`
		`+ #access check`
		`+ assert_tag_access(tag['id'], user_id=user_id, force=force)`
		`+ return _direct_tag_build(tag, build, user, force)`
		`+`
		`+`
		`+ def _direct_tag_build(tag, build, user, force=False):`
		`+ """Directly tag a build. No access check or value lookup."""`
		`koji.plugin.run_callbacks('preTag', tag=tag, build=build, user=user, force=force)`
		`tag_id = tag['id']`
		`build_id = build['id']`
		`+ user_id = user['id']`
		`nvr = "%(name)s-%(version)s-%(release)s" % build`
		`if build['state'] != koji.BUILD_STATES['COMPLETE']:`
		`# incomplete builds may not be tagged, not even when forced`
		`state = koji.BUILD_STATES[build['state']]`
		`raise koji.TagError, "build %s not complete: state %s" % (nvr, state)`
		`- #access check`
		`- assert_tag_access(tag['id'], user_id=user_id, force=force)`
		`# see if it's already tagged`
		`retag = False`
		`table = 'tag_listing'`
		`@@ -1513,19 +1534,26 @@`
		`else:`
		`# use the user associated with the current session`
		`user = get_user(context.session.user_id, strict=True)`
		`- koji.plugin.run_callbacks('preUntag', tag=tag, build=build, user=user, force=force, strict=strict)`
		`tag_id = tag['id']`
		`build_id = build['id']`
		`assert_tag_access(tag_id, user_id=user_id, force=force)`
		`- update = UpdateProcessor('tag_listing', values=locals(),`
		`+ return _direct_untag_build(tag, build, user, strict, force)`
		`+`
		`+`
		`+ def _direct_untag_build(tag, build, user, strict=True, force=False):`
		`+ """Directly untag a build. No access check or value lookup."""`
		`+ koji.plugin.run_callbacks('preUntag', tag=tag, build=build, user=user, force=force, strict=strict)`
		`+ values = {'tag_id': tag['id'], 'build_id': build['id']}`
		`+ update = UpdateProcessor('tag_listing', values=values,`
		`clauses=['tag_id=%(tag_id)i', 'build_id=%(build_id)i'])`
		`- update.make_revoke(user_id=user_id)`
		`+ update.make_revoke(user_id=user['id'])`
		`count = update.execute()`
		`if count == 0 and strict:`
		`nvr = "%(name)s-%(version)s-%(release)s" % build`
		`raise koji.TagError, "build %s not in tag %s" % (nvr, tag['name'])`
		`koji.plugin.run_callbacks('postUntag', tag=tag, build=build, user=user, force=force, strict=strict)`

		`+`
		`# tag-group operations`
		`# add`
		`# remove`
		`@@ -1533,10 +1561,16 @@`
		`# unblock`
		`# list (readTagGroups)`

		`+`
		`def grplist_add(taginfo, grpinfo, block=False, force=False, **opts):`
		`"""Add to (or update) group list for tag"""`
		`#only admins....`
		`context.session.assertPerm('admin')`
		`+ _grplist_add(taginfo, grpinfo, block, force, **opts)`
		`+`
		`+`
		`+ def _grplist_add(taginfo, grpinfo, block, force, **opts):`
		`+ """grplist_add without permission check"""`
		`tag = get_tag(taginfo)`
		`group = lookup_group(grpinfo, create=True)`
		`block = bool(block)`
		`@@ -1582,6 +1616,7 @@`
		`insert.make_create()`
		`insert.execute()`

		`+`
		`def grplist_remove(taginfo, grpinfo, force=False):`
		`"""Remove group from the list for tag`

		`@@ -1590,6 +1625,11 @@`
		`"""`
		`#only admins....`
		`context.session.assertPerm('admin')`
		`+ _grplist_remove(taginfo, grpinfo, force)`
		`+`
		`+`
		`+ def _grplist_remove(taginfo, grpinfo, force):`
		`+ """grplist_remove without permssion check"""`
		`tag = get_tag(taginfo)`
		`group = lookup_group(grpinfo, strict=True)`
		`tag_id = tag['id']`
		`@@ -1599,10 +1639,12 @@`
		`update.make_revoke()`
		`update.execute()`

		`+`
		`def grplist_block(taginfo, grpinfo):`
		`"""Block the group in tag"""`
		`grplist_add(taginfo, grpinfo, block=True)`

		`+`
		`def grplist_unblock(taginfo, grpinfo):`
		`"""Unblock the group in tag`

		`@@ -1611,6 +1653,11 @@`
		`"""`
		`# only admins...`
		`context.session.assertPerm('admin')`
		`+ _grplist_unblock(taginfo, grpinfo)`
		`+`
		`+`
		`+ def _grplist_unblock(taginfo, grpinfo):`
		`+ """grplist_unblock without permssion check"""`
		`tag = lookup_tag(taginfo, strict=True)`
		`group = lookup_group(grpinfo, strict=True)`
		`tag_id = tag['id']`
		`@@ -1635,10 +1682,16 @@`
		`# unblock`
		`# list (readTagGroups)`

		`+`
		`def grp_pkg_add(taginfo, grpinfo, pkg_name, block=False, force=False, **opts):`
		`"""Add package to group for tag"""`
		`#only admins....`
		`context.session.assertPerm('admin')`
		`+ _grp_pkg_add(taginfo, grpinfo, pkg_name, block, force, **opts)`
		`+`
		`+`
		`+ def _grp_pkg_add(taginfo, grpinfo, pkg_name, block, force, **opts):`
		`+ """grp_pkg_add without permssion checks"""`
		`tag = lookup_tag(taginfo, strict=True)`
		`group = lookup_group(grpinfo, strict=True)`
		`block = bool(block)`
		`@@ -1689,6 +1742,7 @@`
		`insert.make_create()`
		`insert.execute()`

		`+`
		`def grp_pkg_remove(taginfo, grpinfo, pkg_name, force=False):`
		`"""Remove package from the list for group-tag`

		`@@ -1697,6 +1751,11 @@`
		`"""`
		`#only admins....`
		`context.session.assertPerm('admin')`
		`+ _grp_pkg_remove(taginfo, grpinfo, pkg_name, force)`
		`+`
		`+`
		`+ def _grp_pkg_remove(taginfo, grpinfo, pkg_name, force):`
		`+ """grp_pkg_remove without permssion checks"""`
		`tag_id = get_tag_id(taginfo, strict=True)`
		`grp_id = get_group_id(grpinfo, strict=True)`
		`update = UpdateProcessor('group_package_listing', values=locals(),`
		`@@ -1704,10 +1763,12 @@`
		`update.make_revoke()`
		`update.execute()`

		`+`
		`def grp_pkg_block(taginfo, grpinfo, pkg_name):`
		`"""Block the package in group-tag"""`
		`grp_pkg_add(taginfo, grpinfo, pkg_name, block=True)`

		`+`
		`def grp_pkg_unblock(taginfo, grpinfo, pkg_name):`
		`"""Unblock the package in group-tag`

		`@@ -1716,6 +1777,11 @@`
		`"""`
		`# only admins...`
		`context.session.assertPerm('admin')`
		`+ _grp_pkg_unblock(taginfo, grpinfo, pkg_name)`
		`+`
		`+`
		`+ def _grp_pkg_unblock(taginfo, grpinfo, pkg_name):`
		`+ """grp_pkg_unblock without permission checks"""`
		`table = 'group_package_listing'`
		`tag_id = get_tag_id(taginfo, strict=True)`
		`grp_id = get_group_id(grpinfo, strict=True)`
		`@@ -1731,6 +1797,7 @@`
		`update.make_revoke()`
		`update.execute()`

		`+`
		`# tag-group-req operations`
		`# add`
		`# remove`
		`@@ -1738,10 +1805,16 @@`
		`# unblock`
		`# list (readTagGroups)`

		`+`
		`def grp_req_add(taginfo, grpinfo, reqinfo, block=False, force=False, **opts):`
		`"""Add group requirement to group for tag"""`
		`#only admins....`
		`context.session.assertPerm('admin')`
		`+ _grp_req_add(taginfo, grpinfo, reqinfo, block, force, **opts)`
		`+`
		`+`
		`+ def _grp_req_add(taginfo, grpinfo, reqinfo, block, force, **opts):`
		`+ """grp_req_add without permssion checks"""`
		`tag = lookup_tag(taginfo, strict=True)`
		`group = lookup_group(grpinfo, strict=True, create=False)`
		`req = lookup_group(reqinfo, strict=True, create=False)`
		`@@ -1793,6 +1866,7 @@`
		`insert.make_create()`
		`insert.execute()`

		`+`
		`def grp_req_remove(taginfo, grpinfo, reqinfo, force=False):`
		`"""Remove group requirement from the list for group-tag`

		`@@ -1801,6 +1875,11 @@`
		`"""`
		`#only admins....`
		`context.session.assertPerm('admin')`
		`+ _grp_req_remove(taginfo, grpinfo, reqinfo, force)`
		`+`
		`+`
		`+ def _grp_req_remove(taginfo, grpinfo, reqinfo, force):`
		`+ """grp_req_remove without permission checks"""`
		`tag_id = get_tag_id(taginfo, strict=True)`
		`grp_id = get_group_id(grpinfo, strict=True)`
		`req_id = get_group_id(reqinfo, strict=True)`
		`@@ -1809,10 +1888,12 @@`
		`update.make_revoke()`
		`update.execute()`

		`+`
		`def grp_req_block(taginfo, grpinfo, reqinfo):`
		`"""Block the group requirement in group-tag"""`
		`grp_req_add(taginfo, grpinfo, reqinfo, block=True)`

		`+`
		`def grp_req_unblock(taginfo, grpinfo, reqinfo):`
		`"""Unblock the group requirement in group-tag`

		`@@ -1821,6 +1902,11 @@`
		`"""`
		`# only admins...`
		`context.session.assertPerm('admin')`
		`+ _grp_req_unblock(taginfo, grpinfo, reqinfo)`
		`+`
		`+`
		`+ def _grp_req_unblock(taginfo, grpinfo, reqinfo):`
		`+ """grp_req_unblock without permssion checks"""`
		`tag_id = get_tag_id(taginfo, strict=True)`
		`grp_id = get_group_id(grpinfo, strict=True)`
		`req_id = get_group_id(reqinfo, strict=True)`
		`@@ -1838,6 +1924,7 @@`
		`update.make_revoke()`
		`update.execute()`

		`+`
		`def get_tag_groups(tag, event=None, inherit=True, incl_pkgs=True, incl_reqs=True):`
		`"""Return group data for the tag`

		`@@ -2740,10 +2827,16 @@`
		`"""Get the id,name for build target"""`
		`return lookup_name('build_target', info, strict, create)`

		`+`
		`def create_tag(name, parent=None, arches=None, perm=None, locked=False, maven_support=False, maven_include_all=False, extra=None):`
		`"""Create a new tag"""`
		`-`
		`context.session.assertPerm('admin')`
		`+ return _create_tag(name, parent, arches, perm, locked, maven_support, maven_include_all, extra)`
		`+`
		`+`
		`+ def _create_tag(name, parent=None, arches=None, perm=None, locked=False, maven_support=False, maven_include_all=False, extra=None):`
		`+ """Create a new tag, without access check"""`
		`+`
		`if not context.opts.get('EnableMaven') and (maven_support or maven_include_all):`
		`raise koji.GenericError, "Maven support not enabled"`

		`@@ -2828,7 +2921,7 @@`
		`'tag_config.maven_include_all': 'maven_include_all'`
		`}`
		`clauses = [eventCondition(event, table='tag_config')]`
		`- if isinstance(tagInfo, int):`
		`+ if isinstance(tagInfo, (int, long)):`
		`clauses.append("tag.id = %(tagInfo)i")`
		`elif isinstance(tagInfo, basestring):`
		`clauses.append("tag.name = %(tagInfo)s")`
		`@@ -2974,6 +3067,7 @@`
		`_tagDelete('tag_config', tagID)`
		`#technically, to 'delete' the tag we only have to revoke the tag_config entry`
		`#these remaining revocations are more for cleanup.`
		`+ _tagDelete('tag_extra', tagID)`
		`_tagDelete('tag_inheritance', tagID)`
		`_tagDelete('tag_inheritance', tagID, 'parent_id')`
		`_tagDelete('build_target_config', tagID, 'build_tag')`
		`@@ -9219,7 +9313,7 @@`

		`def listTagged(self, tag, event=None, inherit=False, prefix=None, latest=False, package=None, owner=None, type=None):`
		`"""List builds tagged with tag"""`
		`- if not isinstance(tag, int):`
		`+ if not isinstance(tag, (int, long)):`
		`#lookup tag id`
		`tag = get_tag_id(tag, strict=True)`
		`results = readTaggedBuilds(tag, event, inherit=inherit, latest=latest, package=package, owner=owner, type=type)`
		`@@ -9230,14 +9324,14 @@`

		`def listTaggedRPMS(self, tag, event=None, inherit=False, latest=False, package=None, arch=None, rpmsigs=False, owner=None, type=None):`
		`"""List rpms and builds within tag"""`
		`- if not isinstance(tag, int):`
		`+ if not isinstance(tag, (int, long)):`
		`#lookup tag id`
		`tag = get_tag_id(tag, strict=True)`
		`return readTaggedRPMS(tag, event=event, inherit=inherit, latest=latest, package=package, arch=arch, rpmsigs=rpmsigs, owner=owner, type=type)`

		`def listTaggedArchives(self, tag, event=None, inherit=False, latest=False, package=None, type=None):`
		`"""List archives and builds within a tag"""`
		`- if not isinstance(tag, int):`
		`+ if not isinstance(tag, (int, long)):`
		`tag = get_tag_id(tag, strict=True)`
		`return readTaggedArchives(tag, event=event, inherit=inherit, latest=latest, package=package, type=type)`

		`@@ -9394,14 +9488,14 @@`

		`def getLatestBuilds(self, tag, event=None, package=None, type=None):`
		`"""List latest builds for tag (inheritance enabled)"""`
		`- if not isinstance(tag, int):`
		`+ if not isinstance(tag, (int, long)):`
		`#lookup tag id`
		`tag = get_tag_id(tag, strict=True)`
		`return readTaggedBuilds(tag, event, inherit=True, latest=True, package=package, type=type)`

		`def getLatestRPMS(self, tag, package=None, arch=None, event=None, rpmsigs=False, type=None):`
		`"""List latest RPMS for tag (inheritance enabled)"""`
		`- if not isinstance(tag, int):`
		`+ if not isinstance(tag, (int, long)):`
		`#lookup tag id`
		`tag = get_tag_id(tag, strict=True)`
		`return readTaggedRPMS(tag, package=package, arch=arch, event=event, inherit=True, latest=True, rpmsigs=rpmsigs, type=type)`
		`@@ -9461,13 +9555,13 @@`

		`def getInheritanceData(self, tag, event=None):`
		`"""Return inheritance data for tag"""`
		`- if not isinstance(tag, int):`
		`+ if not isinstance(tag, (int, long)):`
		`#lookup tag id`
		`tag = get_tag_id(tag, strict=True)`
		`return readInheritanceData(tag, event)`

		`def setInheritanceData(self, tag, data, clear=False):`
		`- if not isinstance(tag, int):`
		`+ if not isinstance(tag, (int, long)):`
		`#lookup tag id`
		`tag = get_tag_id(tag, strict=True)`
		`context.session.assertPerm('admin')`
		`@@ -9478,7 +9572,7 @@`
		`stops = {}`
		`if jumps is None:`
		`jumps = {}`
		`- if not isinstance(tag, int):`
		`+ if not isinstance(tag, (int, long)):`
		`#lookup tag id`
		`tag = get_tag_id(tag, strict=True)`
		`for mapping in [stops, jumps]:`
		`@@ -9505,7 +9599,7 @@`
		`- buildroot_id`

		`If no build has the given ID, or the build generated no RPMs, an empty list is returned."""`
		`- if not isinstance(build, int):`
		`+ if not isinstance(build, (int, long)):`
		`#lookup build id`
		`build = self.findBuildID(build, strict=True)`
		`return self.listRPMs(buildID=build)`
		`@@ -9870,7 +9964,7 @@`
		`return taginfo`

		`def getRepo(self, tag, state=None, event=None):`
		`- if isinstance(tag, int):`
		`+ if isinstance(tag, (int, long)):`
		`id = tag`
		`else:`
		`id = get_tag_id(tag, strict=True)`

tests/test_hub/test_create_tag.py

file added

+66

		`@@ -0,0 +1,66 @@`
		`+ import copy`
		`+ import mock`
		`+ import shutil`
		`+ import tempfile`
		`+ import unittest`
		`+`
		`+ import koji`
		`+ import kojihub`
		`+`
		`+ IP = kojihub.InsertProcessor`
		`+`
		`+`
		`+ class TestCreateTag(unittest.TestCase):`
		`+`
		`+ def getInsert(self, args, *kwargs):`
		`+ insert = IP(args, *kwargs)`
		`+ insert.execute = mock.MagicMock()`
		`+ self.inserts.append(insert)`
		`+ return insert`
		`+`
		`+ def setUp(self):`
		`+ self.InsertProcessor = mock.patch('kojihub.InsertProcessor',`
		`+ side_effect=self.getInsert).start()`
		`+ self.inserts = []`
		`+ self._dml = mock.patch('kojihub._dml').start()`
		`+ self.get_tag = mock.patch('kojihub.get_tag').start()`
		`+ self.get_tag_id = mock.patch('kojihub.get_tag_id').start()`
		`+ self.writeInheritanceData = mock.patch('kojihub.writeInheritanceData').start()`
		`+ self.context = mock.patch('kojihub.context').start()`
		`+ # It seems MagicMock will not automatically handle attributes that`
		`+ # start with "assert"`
		`+ self.context.session.assertPerm = mock.MagicMock()`
		`+ self.context.session.assertLogin = mock.MagicMock()`
		`+`
		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`+ def test_duplicate(self):`
		`+ self.get_tag.return_value = {'name': 'duptag'}`
		`+ with self.assertRaises(koji.GenericError):`
		`+ kojihub.create_tag('duptag')`
		`+`
		`+ def test_simple_create(self):`
		`+ self.get_tag.return_value = None`
		`+ self.get_tag_id.return_value = 99`
		`+ self.context.event_id = 42`
		`+ self.context.session.user_id = 23`
		`+ kojihub.create_tag('newtag')`
		`+`
		`+ # check the insert`
		`+ self.assertEqual(len(self.inserts), 1)`
		`+ insert = self.inserts[0]`
		`+ self.assertEqual(insert.table, 'tag_config')`
		`+ values = {`
		`+ 'arches': None,`
		`+ 'create_event': 42,`
		`+ 'creator_id': 23,`
		`+ 'locked': False,`
		`+ 'maven_include_all': False,`
		`+ 'maven_support': False,`
		`+ 'perm_id': None,`
		`+ 'tag_id': 99,`
		`+ }`
		`+ self.assertEqual(insert.data, values)`
		`+ self.assertEqual(insert.rawdata, {})`
		`+ insert = self.inserts[0]`

tests/test_hub/test_tag_operations.py

file added

+151

		`@@ -0,0 +1,151 @@`
		`+ import copy`
		`+ import mock`
		`+ import shutil`
		`+ import tempfile`
		`+ import unittest`
		`+`
		`+ import koji`
		`+ import kojihub`
		`+`
		`+ QP = kojihub.QueryProcessor`
		`+ UP = kojihub.UpdateProcessor`
		`+ IP = kojihub.InsertProcessor`
		`+`
		`+`
		`+ class TestTagBuild(unittest.TestCase):`
		`+`
		`+ def getInsert(self, args, *kwargs):`
		`+ insert = IP(args, *kwargs)`
		`+ insert.execute = mock.MagicMock()`
		`+ self.inserts.append(insert)`
		`+ return insert`
		`+`
		`+ def getUpdate(self, args, *kwargs):`
		`+ update = UP(args, *kwargs)`
		`+ update.execute = mock.MagicMock()`
		`+ self.updates.append(update)`
		`+ return update`
		`+`
		`+ def getQuery(self, args, *kwargs):`
		`+ query = QP(args, *kwargs)`
		`+ query.execute = mock.MagicMock()`
		`+ query.executeOne = self.query_executeOne`
		`+ query.iterate = mock.MagicMock()`
		`+ self.queries.append(query)`
		`+ return query`
		`+`
		`+ def setUp(self):`
		`+ self.InsertProcessor = mock.patch('kojihub.InsertProcessor',`
		`+ side_effect=self.getInsert).start()`
		`+ self.inserts = []`
		`+ self.UpdateProcessor = mock.patch('kojihub.UpdateProcessor',`
		`+ side_effect=self.getUpdate).start()`
		`+ self.updates = []`
		`+ self.query_executeOne = mock.MagicMock()`
		`+ self.QueryProcessor = mock.patch('kojihub.QueryProcessor',`
		`+ side_effect=self.getQuery).start()`
		`+ self.queries = []`
		`+ self._dml = mock.patch('kojihub._dml').start()`
		`+ self.get_tag = mock.patch('kojihub.get_tag').start()`
		`+ self.get_build = mock.patch('kojihub.get_build').start()`
		`+ self.get_user = mock.patch('kojihub.get_user').start()`
		`+ self.get_tag_id = mock.patch('kojihub.get_tag_id').start()`
		`+ self.check_tag_access = mock.patch('kojihub.check_tag_access').start()`
		`+ self.writeInheritanceData = mock.patch('kojihub.writeInheritanceData').start()`
		`+ self.context = mock.patch('kojihub.context').start()`
		`+ # It seems MagicMock will not automatically handle attributes that`
		`+ # start with "assert"`
		`+ self.context.session.assertPerm = mock.MagicMock()`
		`+ self.context.session.assertLogin = mock.MagicMock()`
		`+`
		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`+ def test_simple_tag(self):`
		`+ self.check_tag_access.return_value = (True, False, "")`
		`+ self.get_build.return_value = {`
		`+ 'id': 1,`
		`+ 'name': 'name',`
		`+ 'version': 'version',`
		`+ 'release': 'release',`
		`+ 'state': koji.BUILD_STATES['COMPLETE'],`
		`+ }`
		`+ self.get_tag.return_value = {`
		`+ 'id': 777,`
		`+ 'name': 'tag',`
		`+ }`
		`+ self.get_user.return_value = {`
		`+ 'id': 999,`
		`+ 'name': 'user',`
		`+ }`
		`+ self.context.event_id = 42`
		`+ # set return for the already tagged check`
		`+ self.query_executeOne.return_value = None`
		`+`
		`+ # call it`
		`+ kojihub._tag_build('sometag', 'name-version-release')`
		`+`
		`+ self.get_tag.called_once_with('sometag', strict=True)`
		`+ self.get_build.called_once_with('name-version-release', strict=True)`
		`+ self.context.session.assertPerm.called_with('admin')`
		`+`
		`+ # check the insert`
		`+ self.assertEqual(len(self.inserts), 1)`
		`+ insert = self.inserts[0]`
		`+ self.assertEqual(insert.table, 'tag_listing')`
		`+ values = {`
		`+ 'build_id': 1,`
		`+ 'create_event': 42,`
		`+ 'creator_id': 999,`
		`+ 'tag_id': 777`
		`+ }`
		`+ self.assertEqual(insert.data, values)`
		`+ self.assertEqual(insert.rawdata, {})`
		`+ insert = self.inserts[0]`
		`+`
		`+`
		`+ def test_simple_untag(self):`
		`+ self.check_tag_access.return_value = (True, False, "")`
		`+ self.get_build.return_value = {`
		`+ 'id': 1,`
		`+ 'name': 'name',`
		`+ 'version': 'version',`
		`+ 'release': 'release',`
		`+ 'state': koji.BUILD_STATES['COMPLETE'],`
		`+ }`
		`+ self.get_tag.return_value = {`
		`+ 'id': 777,`
		`+ 'name': 'tag',`
		`+ }`
		`+ self.get_user.return_value = {`
		`+ 'id': 999,`
		`+ 'name': 'user',`
		`+ }`
		`+ self.context.event_id = 42`
		`+ # set return for the already tagged check`
		`+ self.query_executeOne.return_value = None`
		`+`
		`+ # call it`
		`+ kojihub._untag_build('sometag', 'name-version-release')`
		`+`
		`+ self.get_tag.called_once_with('sometag', strict=True)`
		`+ self.get_build.called_once_with('name-version-release', strict=True)`
		`+ self.context.session.assertPerm.called_with('admin')`
		`+ self.assertEqual(len(self.inserts), 0)`
		`+`
		`+ # check the update`
		`+ self.assertEqual(len(self.updates), 1)`
		`+ update = self.updates[0]`
		`+ self.assertEqual(update.table, 'tag_listing')`
		`+ values = {`
		`+ 'build_id': 1,`
		`+ 'tag_id': 777`
		`+ }`
		`+ data = {`
		`+ 'revoke_event': 42,`
		`+ 'revoker_id': 999,`
		`+ }`
		`+ self.assertEqual(update.rawdata, {'active': 'NULL'})`
		`+ self.assertEqual(update.data, data)`
		`+ self.assertEqual(update.values, values)`
		`+ update = self.updates[0]`

no initial comment

mikem commented 7 years ago

These changes slightly refactor some tag manipulation functions on the hub to provide a lower level (nonpublic) version that does not perform access checks. This enables a plugin to perform such actions and enforce its own rules about who may do so.

Also added some relevant unit tests.