Emit user in PackageListChange messages

Fixes: https://pagure.io/koji/issue/1035
This commit is contained in:
Tomas Kopecek 2018-08-22 17:58:11 +02:00
parent b96fd708ec
commit e016e6624d
5 changed files with 105 additions and 41 deletions

View file

@ -966,8 +966,11 @@ def _direct_pkglist_add(taginfo, pkginfo, owner, block, extra_arches, force,
pkg = lookup_package(pkginfo, create=True)
# validate arches before running callbacks
extra_arches = koji.parse_arches(extra_arches, strict=True, allow_none=True)
koji.plugin.run_callbacks('prePackageListChange', action=action, tag=tag, package=pkg, owner=owner,
block=block, extra_arches=extra_arches, force=force, update=update)
user = get_user(context.session.user_id)
koji.plugin.run_callbacks('prePackageListChange', action=action,
tag=tag, package=pkg, owner=owner,
block=block, extra_arches=extra_arches,
force=force, update=update, user=user)
# first check to see if package is:
# already present (via inheritance)
# blocked
@ -1012,8 +1015,10 @@ def _direct_pkglist_add(taginfo, pkginfo, owner, block, extra_arches, force,
_pkglist_add(tag_id, pkg['id'], owner, block, extra_arches)
elif changed_owner:
_pkglist_owner_add(tag_id, pkg['id'], owner)
koji.plugin.run_callbacks('postPackageListChange', action=action, tag=tag, package=pkg, owner=owner,
block=block, extra_arches=extra_arches, force=force, update=update)
koji.plugin.run_callbacks('postPackageListChange', action=action,
tag=tag, package=pkg, owner=owner,
block=block, extra_arches=extra_arches,
force=force, update=update, user=user)
def pkglist_remove(taginfo, pkginfo, force=False):
"""Remove package from the list for tag
@ -1036,9 +1041,10 @@ def _direct_pkglist_remove(taginfo, pkginfo, force=False, policy=False):
#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)
user = get_user(context.session.user_id)
koji.plugin.run_callbacks('prePackageListChange', action='remove', tag=tag, package=pkg, user=user)
_pkglist_remove(tag['id'], pkg['id'])
koji.plugin.run_callbacks('postPackageListChange', action='remove', tag=tag, package=pkg)
koji.plugin.run_callbacks('postPackageListChange', action='remove', tag=tag, package=pkg, user=user)
def pkglist_block(taginfo, pkginfo, force=False):
@ -1064,7 +1070,8 @@ def pkglist_unblock(taginfo, pkginfo, force=False):
#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='unblock', tag=tag, package=pkg)
user = get_user(context.session.user_id)
koji.plugin.run_callbacks('prePackageListChange', action='unblock', tag=tag, package=pkg, user=user)
tag_id = tag['id']
pkg_id = pkg['id']
pkglist = readPackageList(tag_id, pkgID=pkg_id, inherit=True)
@ -1084,7 +1091,7 @@ def pkglist_unblock(taginfo, pkginfo, force=False):
pkglist = readPackageList(tag_id, pkgID=pkg_id, inherit=True)
if pkg_id not in pkglist or pkglist[pkg_id]['blocked']:
_pkglist_add(tag_id, pkg_id, previous['owner_id'], False, previous['extra_arches'])
koji.plugin.run_callbacks('postPackageListChange', action='unblock', tag=tag, package=pkg)
koji.plugin.run_callbacks('postPackageListChange', action='unblock', tag=tag, package=pkg, user=user)
def pkglist_setowner(taginfo, pkginfo, owner, force=False):
"""Set the owner for package in tag"""

View file

@ -158,7 +158,8 @@ def prep_package_list_change(cbtype, *args, **kws):
props = {'type': cbtype[4:],
'tag': kws['tag']['name'],
'package': kws['package']['name'],
'action': kws['action']}
'action': kws['action'],
'user': kws['user']['name']}
queue_msg(address, props, kws)
@convert_datetime

View file

@ -89,6 +89,7 @@ class TestDistRepo(unittest.TestCase):
@mock.patch('kojihub.make_task')
def test_DistRepo(self, make_task, dist_repo_init):
session = kojihub.context.session = mock.MagicMock()
session.user_id = 123
# It seems MagicMock will not automatically handle attributes that
# start with "assert"
session.assertPerm = mock.MagicMock()
@ -210,7 +211,8 @@ class TestDistRepoMove(unittest.TestCase):
def test_distRepoMove(self):
kojihub.context.session = mock.MagicMock()
session = kojihub.context.session = mock.MagicMock()
session.user_id = 123
exports = kojihub.HostExports()
exports.distRepoMove(self.rinfo['id'], self.uploadpath, self.arch)
# check result

View file

@ -8,12 +8,26 @@ except ImportError:
import koji
import kojihub
def get_user_factory(data):
def get_user(userInfo, strict=False):
if isinstance(userInfo, int):
key = 'id'
else:
key = 'name'
for ui in data:
if ui[key] == userInfo:
return ui
if strict:
raise koji.GenericError(user_id)
return get_user
class TestPkglistBlock(unittest.TestCase):
def setUp(self):
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.user_id = 112233
self.run_callbacks = mock.patch('koji.plugin.run_callbacks').start()
def tearDown(self):
@ -68,8 +82,8 @@ class TestPkglistBlock(unittest.TestCase):
_pkglist_remove.assert_called_once_with(tag['id'], pkg['id'])
self.assertEqual(self.run_callbacks.call_count, 2)
self.run_callbacks.assert_has_calls([
mock.call('prePackageListChange', action='unblock', tag=tag, package=pkg),
mock.call('postPackageListChange', action='unblock', tag=tag, package=pkg),
mock.call('prePackageListChange', action='unblock', tag=tag, package=pkg, user=None),
mock.call('postPackageListChange', action='unblock', tag=tag, package=pkg, user=None),
])
@mock.patch('kojihub._pkglist_remove')
@ -187,10 +201,14 @@ class TestPkglistBlock(unittest.TestCase):
policy=True
tag = {'id': 1, 'name': 'tag'}
pkg = {'id': 2, 'name': 'pkg', 'owner_id': 3}
user = {'id': 3, 'name': 'user'}
users = [
{'id': 3, 'name': 'user'},
{'id': 112233, 'name': 'user'},
]
user = users[0]
get_tag.return_value = tag
lookup_package.return_value = pkg
get_user.return_value = user
get_user.side_effect = get_user_factory(users)
readPackageList.return_value = {}
@ -200,17 +218,22 @@ class TestPkglistBlock(unittest.TestCase):
get_tag.assert_called_once_with(tag['name'], strict=True)
lookup_package.assert_called_once_with(pkg['name'], strict=False)
get_user.assert_called_once_with(user['name'], strict=True)
get_user.assert_has_calls([
mock.call(user['name'], strict=True),
mock.call(112233),
])
assert_policy.assert_called_once_with('package_list', {'tag': tag['id'],
'action': 'add', 'package': pkg['name'], 'force': False})
self.assertEqual(self.run_callbacks.call_count, 2)
self.run_callbacks.assert_has_calls([
mock.call('prePackageListChange', action='add', tag=tag,
package=pkg, owner=user['id'], block=block,
extra_arches=extra_arches, force=force, update=update),
extra_arches=extra_arches, force=force, update=update,
user=users[1]),
mock.call('postPackageListChange', action='add', tag=tag,
package=pkg, owner=user['id'], block=block,
extra_arches=extra_arches, force=force, update=update),
extra_arches=extra_arches, force=force, update=update,
user=users[1]),
])
_pkglist_add.assert_called_once_with(tag['id'], pkg['id'],
user['id'], block, extra_arches)
@ -291,10 +314,14 @@ class TestPkglistBlock(unittest.TestCase):
policy=True
tag = {'id': 1, 'name': 'tag'}
pkg = {'id': 2, 'name': 'pkg', 'owner_id': 3}
user = {'id': 3, 'name': 'user'}
users = [
{'id': 3, 'name': 'user'},
{'id': 112233, 'name': 'user1'},
]
user = users[0]
get_tag.return_value = tag
lookup_package.side_effect = [None, pkg]
get_user.return_value = user
get_user.side_effect = get_user_factory(users)
readPackageList.return_value = {}
@ -308,17 +335,22 @@ class TestPkglistBlock(unittest.TestCase):
mock.call(pkg['name'], strict=False),
mock.call(pkg['name'], create=True),
)
get_user.assert_called_once_with(user['name'], strict=True)
get_user.assert_has_calls([
mock.call(user['name'], strict=True),
mock.call(112233),
])
assert_policy.assert_called_once_with('package_list', {'tag': tag['id'],
'action': 'add', 'package': pkg['name'], 'force': False})
self.assertEqual(self.run_callbacks.call_count, 2)
self.run_callbacks.assert_has_calls([
mock.call('prePackageListChange', action='add', tag=tag,
package=pkg, owner=user['id'], block=block,
extra_arches=extra_arches, force=force, update=update),
extra_arches=extra_arches, force=force, update=update,
user=users[1]),
mock.call('postPackageListChange', action='add', tag=tag,
package=pkg, owner=user['id'], block=block,
extra_arches=extra_arches, force=force, update=update),
extra_arches=extra_arches, force=force, update=update,
user=users[1]),
])
_pkglist_add.assert_called_once_with(tag['id'], pkg['id'],
user['id'], block, extra_arches)
@ -339,10 +371,14 @@ class TestPkglistBlock(unittest.TestCase):
policy=True
tag = {'id': 1, 'name': 'tag'}
pkg = {'id': 2, 'name': 'pkg', 'owner_id': 3}
user = {'id': 3, 'name': 'user'}
users = [
{'id': 3, 'name': 'user',},
{'id': 112233, 'name': 'user1'},
]
user = users[0]
get_tag.return_value = tag
lookup_package.return_value = pkg
get_user.return_value = user
get_user.side_effect = get_user_factory(users)
readPackageList.return_value = {pkg['id']: {
'owner_id': pkg['owner_id'],
'blocked': True,
@ -356,13 +392,17 @@ class TestPkglistBlock(unittest.TestCase):
get_tag.assert_called_once_with(tag['name'], strict=True)
lookup_package.assert_called_once_with(pkg['name'], strict=False)
get_user.assert_called_once_with(user['name'], strict=True)
get_user.assert_has_calls([
mock.call(user['name'], strict=True),
mock.call(112233),
])
assert_policy.assert_called_once_with('package_list', {'tag': tag['id'],
'action': 'add', 'package': pkg['name'], 'force': False})
self.run_callbacks.assert_called_once_with(
'prePackageListChange', action='add', tag=tag,
package=pkg, owner=user['id'], block=block,
extra_arches=extra_arches, force=force, update=update)
extra_arches=extra_arches, force=force, update=update,
user=users[1])
_pkglist_add.assert_not_called()
@mock.patch('kojihub._pkglist_add')
@ -381,10 +421,14 @@ class TestPkglistBlock(unittest.TestCase):
policy=True
tag = {'id': 1, 'name': 'tag'}
pkg = {'id': 2, 'name': 'pkg', 'owner_id': 3}
user = {'id': 3, 'name': 'user'}
users = [
{'id': 3, 'name': 'user',},
{'id': 112233, 'name': 'user1'},
]
user = users[0]
get_tag.return_value = tag
lookup_package.return_value = pkg
get_user.return_value = user
get_user.side_effect = get_user_factory(users)
readPackageList.return_value = {pkg['id']: {
'owner_id': pkg['owner_id'],
'blocked': True,
@ -397,7 +441,10 @@ class TestPkglistBlock(unittest.TestCase):
get_tag.assert_called_once_with(tag['name'], strict=True)
lookup_package.assert_called_once_with(pkg['name'], strict=False)
get_user.assert_called_once_with(user['name'], strict=True)
get_user.assert_has_calls([
mock.call(user['name'], strict=True),
mock.call(112233),
])
# force + admin
assert_policy.assert_not_called()
@ -405,10 +452,12 @@ class TestPkglistBlock(unittest.TestCase):
self.run_callbacks.assert_has_calls([
mock.call('prePackageListChange', action='add', tag=tag,
package=pkg, owner=user['id'], block=block,
extra_arches=extra_arches, force=force, update=update),
extra_arches=extra_arches, force=force, update=update,
user=users[1]),
mock.call('postPackageListChange', action='add', tag=tag,
package=pkg, owner=user['id'], block=block,
extra_arches=extra_arches, force=force, update=update),
extra_arches=extra_arches, force=force, update=update,
user=users[1]),
])
_pkglist_add.assert_called_once_with(tag['id'], pkg['id'],
user['id'], block, extra_arches)

View file

@ -44,9 +44,10 @@ class TestProtonMsg(unittest.TestCase):
package={'name': 'test-pkg'},
owner=1,
block=False, extra_arches='i386 x86_64',
force=False, update=False)
force=False, update=False,
user={'name': 'username'})
self.assertMsg('package.add', type='PackageListChange', tag='test-tag',
package='test-pkg', action='add')
package='test-pkg', action='add', user='username')
def test_prep_package_list_change_update(self):
protonmsg.prep_package_list_change('postPackageListChange',
@ -54,9 +55,10 @@ class TestProtonMsg(unittest.TestCase):
package={'name': 'test-pkg'},
owner=1,
block=False, extra_arches='i386 x86_64',
force=False, update=False)
force=False, update=False,
user={'name': 'username'})
self.assertMsg('package.update', type='PackageListChange', tag='test-tag',
package='test-pkg', action='update')
package='test-pkg', action='update', user='username')
def test_prep_package_list_change_block(self):
protonmsg.prep_package_list_change('postPackageListChange',
@ -64,23 +66,26 @@ class TestProtonMsg(unittest.TestCase):
package={'name': 'test-pkg'},
owner=1,
block=False, extra_arches='i386 x86_64',
force=False, update=False)
force=False, update=False,
user={'name': 'username'})
self.assertMsg('package.block', type='PackageListChange', tag='test-tag',
package='test-pkg', action='block')
package='test-pkg', action='block', user='username')
def test_prep_package_list_change_unblock(self):
protonmsg.prep_package_list_change('postPackageListChange',
action='unblock', tag={'name': 'test-tag'},
package={'name': 'test-pkg'})
package={'name': 'test-pkg'},
user={'name': 'username'})
self.assertMsg('package.unblock', type='PackageListChange', tag='test-tag',
package='test-pkg', action='unblock')
package='test-pkg', action='unblock', user='username')
def test_prep_package_list_change_remove(self):
protonmsg.prep_package_list_change('postPackageListChange',
action='remove', tag={'name': 'test-tag'},
package={'name': 'test-pkg'})
package={'name': 'test-pkg'},
user={'name': 'username'})
self.assertMsg('package.remove', type='PackageListChange', tag='test-tag',
package='test-pkg', action='remove')
package='test-pkg', action='remove', user='username')
def test_prep_task_state_change(self):
info = {'id': 5678,