From 7d71558bc5664682a978467367a308a95f20361b Mon Sep 17 00:00:00 2001 From: Jana Cupova Date: Jun 15 2022 08:56:35 +0000 Subject: Consistence pre/postPackageListChange sequence Fixes: https://pagure.io/koji/issue/1472 --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 4acd6b6..2728204 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -1016,10 +1016,6 @@ def _direct_pkglist_add(taginfo, pkginfo, owner, block, extra_arches, force, # validate arches before running callbacks extra_arches = koji.parse_arches(extra_arches, strict=True, allow_none=True) 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 @@ -1060,6 +1056,10 @@ def _direct_pkglist_add(taginfo, pkginfo, owner, block, extra_arches, force, owner = context.session.user_id else: raise koji.GenericError("owner not specified") + 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) if not previous or changed: _pkglist_add(tag_id, pkg['id'], owner, block, extra_arches) elif changed_owner: @@ -1136,8 +1136,6 @@ def pkglist_unblock(taginfo, pkginfo, force=False): # don't check policy for admins using force assert_policy('package_list', policy_data, force=force) 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) @@ -1147,6 +1145,8 @@ def pkglist_unblock(taginfo, pkginfo, force=False): % (pkg['name'], tag['name'])) if not previous['blocked']: raise koji.GenericError("package %s NOT blocked in tag %s" % (pkg['name'], tag['name'])) + koji.plugin.run_callbacks( + 'prePackageListChange', action='unblock', tag=tag, package=pkg, user=user) if previous['tag_id'] != tag_id: _pkglist_add(tag_id, pkg_id, previous['owner_id'], False, previous['extra_arches']) else: diff --git a/tests/test_hub/test_pkglist.py b/tests/test_hub/test_pkglist.py index f203671..6903950 100644 --- a/tests/test_hub/test_pkglist.py +++ b/tests/test_hub/test_pkglist.py @@ -20,7 +20,7 @@ def get_user_factory(data): return get_user -class TestPkglistBlock(unittest.TestCase): +class TestPkglist(unittest.TestCase): def setUp(self): self.context = mock.patch('kojihub.context').start() # It seems MagicMock will not automatically handle attributes that @@ -29,101 +29,91 @@ class TestPkglistBlock(unittest.TestCase): self.context.session.user_id = 112233 self.context.session.user_data = {'name': 'username'} self.run_callbacks = mock.patch('koji.plugin.run_callbacks').start() + self.read_package_list = mock.patch('kojihub.readPackageList').start() + self.lookup_package = mock.patch('kojihub.lookup_package').start() + self._pkglist_add = mock.patch('kojihub._pkglist_add').start() + self.get_tag = mock.patch('kojihub.get_tag').start() + self._pkglist_remove = mock.patch('kojihub._pkglist_remove').start() + self.assert_policy = mock.patch('kojihub.assert_policy').start() + self.get_user = mock.patch('kojihub.get_user').start() def tearDown(self): mock.patch.stopall() - @mock.patch('kojihub.readPackageList') - @mock.patch('kojihub.lookup_package') - @mock.patch('kojihub.get_tag') @mock.patch('kojihub.pkglist_add') - def test_pkglist_block(self, pkglist_add, get_tag, lookup_package, readPackageList): + def test_pkglist_block(self, pkglist_add): force = mock.MagicMock() - get_tag.return_value = {'name': 'tag', 'id': 123} - lookup_package.return_value = {'name': 'pkg', 'id': 321} - readPackageList.return_value = ['pkg'] + self.get_tag.return_value = {'name': 'tag', 'id': 123} + self.lookup_package.return_value = {'name': 'pkg', 'id': 321} + self.read_package_list.return_value = ['pkg'] kojihub.pkglist_block('tag', 'pkg', force=force) - get_tag.assert_called_once_with('tag', strict=True) - lookup_package.assert_called_once_with('pkg', strict=True) + self.get_tag.assert_called_once_with('tag', strict=True) + self.lookup_package.assert_called_once_with('pkg', strict=True) pkglist_add.assert_called_once_with('tag', 'pkg', block=True, force=force) - @mock.patch('kojihub.readPackageList') - @mock.patch('kojihub.lookup_package') - @mock.patch('kojihub.get_tag') @mock.patch('kojihub.pkglist_add') - def test_pkglist_block_package_error( - self, pkglist_add, get_tag, lookup_package, readPackageList): + def test_pkglist_block_package_error(self, pkglist_add): pkg_name = 'pkg' tag_name = 'tag' force = mock.MagicMock() - get_tag.return_value = {'name': tag_name, 'id': 123} - lookup_package.return_value = {'name': pkg_name, 'id': 321} - readPackageList.return_value = [] + self.get_tag.return_value = {'name': tag_name, 'id': 123} + self.lookup_package.return_value = {'name': pkg_name, 'id': 321} + self.read_package_list.return_value = [] with self.assertRaises(koji.GenericError) as ex: kojihub.pkglist_block('tag', 'pkg', force=force) self.assertEqual("Package %s is not in tag listing for %s" % (pkg_name, tag_name), str(ex.exception)) - get_tag.assert_called_once_with('tag', strict=True) - lookup_package.assert_called_once_with('pkg', strict=True) + self.get_tag.assert_called_once_with('tag', strict=True) + self.lookup_package.assert_called_once_with('pkg', strict=True) pkglist_add.assert_not_called() - @mock.patch('kojihub._pkglist_remove') - @mock.patch('kojihub._pkglist_add') - @mock.patch('kojihub.readPackageList') - @mock.patch('kojihub.assert_policy') - @mock.patch('kojihub.get_tag') - @mock.patch('kojihub.lookup_package') - def test_pkglist_unblock( - self, lookup_package, get_tag, assert_policy, readPackageList, _pkglist_add, - _pkglist_remove): + def test_pkglist_unblock(self,): tag = {'id': 1, 'name': 'tag'} pkg = {'id': 2, 'name': 'package', 'owner_id': 3} - get_tag.return_value = tag - lookup_package.return_value = pkg - readPackageList.return_value = {pkg['id']: { + self.get_tag.return_value = tag + self.lookup_package.return_value = pkg + self.read_package_list.return_value = {pkg['id']: { 'blocked': True, 'tag_id': tag['id'], 'owner_id': pkg['owner_id'], 'extra_arches': ''}} + users = [ + {'id': 3, 'name': 'user'}, + {'id': 112233, 'name': 'user'}, + ] + user = users[1] + self.get_user.side_effect = get_user_factory(users) kojihub.pkglist_unblock('tag', 'pkg', force=False) - get_tag.assert_called_once_with('tag', strict=True) - lookup_package.assert_called_once_with('pkg', strict=True) - assert_policy.assert_called_once_with( + self.get_tag.assert_called_once_with('tag', strict=True) + self.lookup_package.assert_called_once_with('pkg', strict=True) + self.assert_policy.assert_called_once_with( 'package_list', {'tag': tag['id'], 'action': 'unblock', 'package': pkg['id'], 'force': False}, force=False) - self.assertEqual(readPackageList.call_count, 2) - readPackageList.assert_has_calls([ + self.assertEqual(self.read_package_list.call_count, 2) + self.read_package_list.assert_has_calls([ mock.call(tag['id'], pkgID=pkg['id'], inherit=True), mock.call(tag['id'], pkgID=pkg['id'], inherit=True), ]) - _pkglist_add.assert_called_once_with(tag['id'], pkg['id'], pkg['owner_id'], False, '') - _pkglist_remove.assert_called_once_with(tag['id'], pkg['id']) + self._pkglist_add.assert_called_once_with(tag['id'], pkg['id'], pkg['owner_id'], False, '') + self._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, user=None), - mock.call('postPackageListChange', action='unblock', tag=tag, package=pkg, user=None), + mock.call('prePackageListChange', action='unblock', tag=tag, package=pkg, user=user), + mock.call('postPackageListChange', action='unblock', tag=tag, package=pkg, user=user), ]) - @mock.patch('kojihub._pkglist_remove') - @mock.patch('kojihub._pkglist_add') - @mock.patch('kojihub.readPackageList') - @mock.patch('kojihub.assert_policy') - @mock.patch('kojihub.get_tag') - @mock.patch('kojihub.lookup_package') - def test_pkglist_unblock_inherited( - self, lookup_package, get_tag, assert_policy, readPackageList, _pkglist_add, - _pkglist_remove): + def test_pkglist_unblock_inherited(self): tag_id, pkg_id, owner_id = 1, 2, 3 - get_tag.return_value = {'id': tag_id, 'name': 'tag'} - lookup_package.return_value = {'id': pkg_id, 'name': 'pkg'} - readPackageList.return_value = {pkg_id: { + self.get_tag.return_value = {'id': tag_id, 'name': 'tag'} + self.lookup_package.return_value = {'id': pkg_id, 'name': 'pkg'} + self.read_package_list.return_value = {pkg_id: { 'blocked': True, 'tag_id': 4, 'owner_id': owner_id, @@ -131,56 +121,40 @@ class TestPkglistBlock(unittest.TestCase): kojihub.pkglist_unblock('tag', 'pkg', force=False) - get_tag.assert_called_once_with('tag', strict=True) - lookup_package.assert_called_once_with('pkg', strict=True) - assert_policy.assert_called_once_with( + self.get_tag.assert_called_once_with('tag', strict=True) + self.lookup_package.assert_called_once_with('pkg', strict=True) + self.assert_policy.assert_called_once_with( 'package_list', {'tag': tag_id, 'action': 'unblock', 'package': pkg_id, 'force': False}, force=False) - readPackageList.assert_called_once_with(tag_id, pkgID=pkg_id, inherit=True) - _pkglist_add.assert_called_once_with(tag_id, pkg_id, owner_id, False, '') - _pkglist_remove.assert_not_called() - - @mock.patch('kojihub._pkglist_remove') - @mock.patch('kojihub._pkglist_add') - @mock.patch('kojihub.readPackageList') - @mock.patch('kojihub.assert_policy') - @mock.patch('kojihub.get_tag') - @mock.patch('kojihub.lookup_package') - def test_pkglist_unblock_not_present( - self, lookup_package, get_tag, assert_policy, readPackageList, _pkglist_add, - _pkglist_remove): + self.read_package_list.assert_called_once_with(tag_id, pkgID=pkg_id, inherit=True) + self._pkglist_add.assert_called_once_with(tag_id, pkg_id, owner_id, False, '') + self._pkglist_remove.assert_not_called() + + def test_pkglist_unblock_not_present(self): tag_id, pkg_id = 1, 2 - get_tag.return_value = {'id': tag_id, 'name': 'tag'} - lookup_package.return_value = {'id': pkg_id, 'name': 'pkg'} - readPackageList.return_value = {} + self.get_tag.return_value = {'id': tag_id, 'name': 'tag'} + self.lookup_package.return_value = {'id': pkg_id, 'name': 'pkg'} + self.read_package_list.return_value = {} with self.assertRaises(koji.GenericError): kojihub.pkglist_unblock('tag', 'pkg', force=False) - get_tag.assert_called_once_with('tag', strict=True) - lookup_package.assert_called_once_with('pkg', strict=True) - assert_policy.assert_called_once_with( + self.get_tag.assert_called_once_with('tag', strict=True) + self.lookup_package.assert_called_once_with('pkg', strict=True) + self.assert_policy.assert_called_once_with( 'package_list', {'tag': tag_id, 'action': 'unblock', 'package': pkg_id, 'force': False}, force=False) - readPackageList.assert_called_once_with(tag_id, pkgID=pkg_id, inherit=True) - _pkglist_add.assert_not_called() - _pkglist_remove.assert_not_called() - - @mock.patch('kojihub._pkglist_remove') - @mock.patch('kojihub._pkglist_add') - @mock.patch('kojihub.readPackageList') - @mock.patch('kojihub.assert_policy') - @mock.patch('kojihub.get_tag') - @mock.patch('kojihub.lookup_package') - def test_pkglist_unblock_not_blocked( - self, lookup_package, get_tag, assert_policy, readPackageList, _pkglist_add, - _pkglist_remove): + self.read_package_list.assert_called_once_with(tag_id, pkgID=pkg_id, inherit=True) + self._pkglist_add.assert_not_called() + self._pkglist_remove.assert_not_called() + + def test_pkglist_unblock_not_blocked(self): tag_id, pkg_id, owner_id = 1, 2, 3 - get_tag.return_value = {'id': tag_id, 'name': 'tag'} - lookup_package.return_value = {'id': pkg_id, 'name': 'pkg'} - readPackageList.return_value = {pkg_id: { + self.get_tag.return_value = {'id': tag_id, 'name': 'tag'} + self.lookup_package.return_value = {'id': pkg_id, 'name': 'pkg'} + self.read_package_list.return_value = {pkg_id: { 'blocked': False, 'tag_id': tag_id, 'owner_id': owner_id, @@ -189,15 +163,15 @@ class TestPkglistBlock(unittest.TestCase): with self.assertRaises(koji.GenericError): kojihub.pkglist_unblock('tag', 'pkg', force=False) - get_tag.assert_called_once_with('tag', strict=True) - lookup_package.assert_called_once_with('pkg', strict=True) - assert_policy.assert_called_once_with( + self.get_tag.assert_called_once_with('tag', strict=True) + self.lookup_package.assert_called_once_with('pkg', strict=True) + self.assert_policy.assert_called_once_with( 'package_list', {'tag': tag_id, 'action': 'unblock', 'package': pkg_id, 'force': False}, force=False) - readPackageList.assert_called_once_with(tag_id, pkgID=pkg_id, inherit=True) - _pkglist_add.assert_not_called() - _pkglist_remove.assert_not_called() + self.read_package_list.assert_called_once_with(tag_id, pkgID=pkg_id, inherit=True) + self._pkglist_add.assert_not_called() + self._pkglist_remove.assert_not_called() @mock.patch('kojihub.pkglist_add') def test_pkglist_setowner(self, pkglist_add): @@ -220,14 +194,7 @@ class TestPkglistBlock(unittest.TestCase): _direct_pkglist_add.assert_called_once_with('tag', 'pkg', 'owner', 'block', 'extra_arches', 'force', 'update', policy=True) - @mock.patch('kojihub._pkglist_add') - @mock.patch('kojihub.readPackageList') - @mock.patch('kojihub.assert_policy') - @mock.patch('kojihub.get_user') - @mock.patch('kojihub.get_tag') - @mock.patch('kojihub.lookup_package') - def test_direct_pkglist_add( - self, lookup_package, get_tag, get_user, assert_policy, readPackageList, _pkglist_add): + def test_direct_pkglist_add(self): block = False extra_arches = 'arch123' force = False @@ -240,22 +207,22 @@ class TestPkglistBlock(unittest.TestCase): {'id': 112233, 'name': 'user'}, ] user = users[0] - get_tag.return_value = tag - lookup_package.return_value = pkg - get_user.side_effect = get_user_factory(users) - readPackageList.return_value = {} + self.get_tag.return_value = tag + self.lookup_package.return_value = pkg + self.get_user.side_effect = get_user_factory(users) + self.read_package_list.return_value = {} kojihub._direct_pkglist_add(tag['name'], pkg['name'], user['name'], block=block, extra_arches=extra_arches, force=force, update=update, policy=policy) - get_tag.assert_called_once_with(tag['name'], strict=True) - lookup_package.assert_called_once_with(pkg['name'], strict=False) - get_user.assert_has_calls([ + self.get_tag.assert_called_once_with(tag['name'], strict=True) + self.lookup_package.assert_called_once_with(pkg['name'], strict=False) + self.get_user.assert_has_calls([ mock.call(user['name'], strict=True), mock.call(112233), ]) - assert_policy.assert_called_once_with( + self.assert_policy.assert_called_once_with( 'package_list', {'tag': tag['id'], 'action': 'add', 'package': pkg['name'], 'force': False}, force=False) @@ -268,16 +235,10 @@ class TestPkglistBlock(unittest.TestCase): owner=user['id'], block=block, 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) - - @mock.patch('kojihub._pkglist_add') - @mock.patch('kojihub.readPackageList') - @mock.patch('kojihub.assert_policy') - @mock.patch('kojihub.get_user') - @mock.patch('kojihub.get_tag') - @mock.patch('kojihub.lookup_package') - def test_direct_pkglist_add_no_package( - self, lookup_package, get_tag, get_user, assert_policy, readPackageList, _pkglist_add): + self._pkglist_add.assert_called_once_with( + tag['id'], pkg['id'], user['id'], block, extra_arches) + + def test_direct_pkglist_add_no_package(self): block = False extra_arches = 'arch123' force = False @@ -286,10 +247,10 @@ class TestPkglistBlock(unittest.TestCase): tag = {'id': 1, 'name': 'tag'} pkg = {'id': 2, 'name': 'pkg', 'owner_id': 3} user = {'id': 3, 'name': 'user'} - get_tag.return_value = tag - lookup_package.return_value = None - get_user.return_value = user - readPackageList.return_value = {} + self.get_tag.return_value = tag + self.lookup_package.return_value = None + self.get_user.return_value = user + self.read_package_list.return_value = {} # package needs to be name, not dict with self.assertRaises(koji.GenericError) as ex: @@ -298,29 +259,20 @@ class TestPkglistBlock(unittest.TestCase): policy=policy) self.assertEqual("No such package: %s" % pkg, str(ex.exception)) - @mock.patch('kojihub.get_tag') - @mock.patch('kojihub.lookup_package') - def test_direct_pkglist_add_pkginfo_dict(self, lookup_package, get_tag): + def test_direct_pkglist_add_pkginfo_dict(self): pkg = {'id': 2, 'name': 'pkg', 'owner_id': 3} user = 'user' tag = {'id': 1, 'name': 'tag'} expected = "Invalid type for id lookup: %s" % pkg - get_tag.return_value = tag - lookup_package.side_effect = koji.GenericError(expected) + self.get_tag.return_value = tag + self.lookup_package.side_effect = koji.GenericError(expected) with self.assertRaises(koji.GenericError) as ex: kojihub._direct_pkglist_add(tag['name'], pkg, user, block=False, extra_arches='arch', force=False, update=True) self.assertEqual(expected, str(ex.exception)) - @mock.patch('kojihub._pkglist_add') - @mock.patch('kojihub.readPackageList') - @mock.patch('kojihub.assert_policy') - @mock.patch('kojihub.get_user') - @mock.patch('kojihub.get_tag') - @mock.patch('kojihub.lookup_package') - def test_direct_pkglist_add_no_user( - self, lookup_package, get_tag, get_user, assert_policy, readPackageList, _pkglist_add): + def test_direct_pkglist_add_no_user(self): block = False extra_arches = 'arch123' force = False @@ -329,28 +281,21 @@ class TestPkglistBlock(unittest.TestCase): tag = {'id': 1, 'name': 'tag'} pkg = {'id': 2, 'name': 'pkg', 'owner_id': 3} user = {'id': 3, 'name': 'user'} - get_tag.return_value = tag - lookup_package.return_value = None - get_user.side_effect = koji.GenericError - readPackageList.return_value = {} + self.get_tag.return_value = tag + self.lookup_package.return_value = None + self.get_user.side_effect = koji.GenericError + self.read_package_list.return_value = {} with self.assertRaises(koji.GenericError): kojihub._direct_pkglist_add(tag['name'], pkg, user['name'], block=block, extra_arches=extra_arches, force=force, update=update, policy=policy) - lookup_package.assert_called_once_with(pkg, strict=False) + self.lookup_package.assert_called_once_with(pkg, strict=False) self.assertEqual(self.run_callbacks.call_count, 0) - _pkglist_add.assert_not_called() - - @mock.patch('kojihub._pkglist_add') - @mock.patch('kojihub.readPackageList') - @mock.patch('kojihub.assert_policy') - @mock.patch('kojihub.get_user') - @mock.patch('kojihub.get_tag') - @mock.patch('kojihub.lookup_package') - def test_direct_pkglist_add_new_package( - self, lookup_package, get_tag, get_user, assert_policy, readPackageList, _pkglist_add): + self._pkglist_add.assert_not_called() + + def test_direct_pkglist_add_new_package(self): block = False extra_arches = 'arch123' force = False @@ -363,26 +308,26 @@ class TestPkglistBlock(unittest.TestCase): {'id': 112233, 'name': 'user1'}, ] user = users[0] - get_tag.return_value = tag - lookup_package.side_effect = [None, pkg] - get_user.side_effect = get_user_factory(users) - readPackageList.return_value = {} + self.get_tag.return_value = tag + self.lookup_package.side_effect = [None, pkg] + self.get_user.side_effect = get_user_factory(users) + self.read_package_list.return_value = {} kojihub._direct_pkglist_add(tag['name'], pkg['name'], user['name'], block=block, extra_arches=extra_arches, force=force, update=update, policy=policy) - get_tag.assert_called_once_with(tag['name'], strict=True) - self.assertEqual(lookup_package.call_count, 2) - lookup_package.has_calls( + self.get_tag.assert_called_once_with(tag['name'], strict=True) + self.assertEqual(self.lookup_package.call_count, 2) + self.lookup_package.has_calls( mock.call(pkg['name'], strict=False), mock.call(pkg['name'], create=True), ) - get_user.assert_has_calls([ + self.get_user.assert_has_calls([ mock.call(user['name'], strict=True), mock.call(112233), ]) - assert_policy.assert_called_once_with( + self.assert_policy.assert_called_once_with( 'package_list', {'tag': tag['id'], 'action': 'add', 'package': pkg['name'], 'force': False}, force=False) @@ -395,16 +340,10 @@ class TestPkglistBlock(unittest.TestCase): owner=user['id'], block=block, 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) - - @mock.patch('kojihub._pkglist_add') - @mock.patch('kojihub.readPackageList') - @mock.patch('kojihub.assert_policy') - @mock.patch('kojihub.get_user') - @mock.patch('kojihub.get_tag') - @mock.patch('kojihub.lookup_package') - def test_direct_pkglist_add_blocked_previously( - self, lookup_package, get_tag, get_user, assert_policy, readPackageList, _pkglist_add): + self._pkglist_add.assert_called_once_with( + tag['id'], pkg['id'], user['id'], block, extra_arches) + + def test_direct_pkglist_add_blocked_previously(self): block = False extra_arches = 'arch123' force = False @@ -417,10 +356,10 @@ class TestPkglistBlock(unittest.TestCase): {'id': 112233, 'name': 'user1'}, ] user = users[0] - get_tag.return_value = tag - lookup_package.return_value = pkg - get_user.side_effect = get_user_factory(users) - readPackageList.return_value = {pkg['id']: { + self.get_tag.return_value = tag + self.lookup_package.return_value = pkg + self.get_user.side_effect = get_user_factory(users) + self.read_package_list.return_value = {pkg['id']: { 'owner_id': pkg['owner_id'], 'blocked': True, 'extra_arches': ''} @@ -431,29 +370,20 @@ class TestPkglistBlock(unittest.TestCase): extra_arches=extra_arches, force=force, update=update, policy=policy) - get_tag.assert_called_once_with(tag['name'], strict=True) - lookup_package.assert_called_once_with(pkg['name'], strict=False) - get_user.assert_has_calls([ + self.get_tag.assert_called_once_with(tag['name'], strict=True) + self.lookup_package.assert_called_once_with(pkg['name'], strict=False) + self.get_user.assert_has_calls([ mock.call(user['name'], strict=True), mock.call(112233), ]) - assert_policy.assert_called_once_with( + self.assert_policy.assert_called_once_with( 'package_list', {'tag': tag['id'], 'action': 'add', 'package': pkg['name'], 'force': False}, 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, user=users[1]) - _pkglist_add.assert_not_called() - - @mock.patch('kojihub._pkglist_add') - @mock.patch('kojihub.readPackageList') - @mock.patch('kojihub.assert_policy') - @mock.patch('kojihub.get_user') - @mock.patch('kojihub.get_tag') - @mock.patch('kojihub.lookup_package') - def test_direct_pkglist_add_blocked_previously_force( - self, lookup_package, get_tag, get_user, assert_policy, readPackageList, _pkglist_add): + self.run_callbacks.assert_not_called() + self._pkglist_add.assert_not_called() + + def test_direct_pkglist_add_blocked_previously_force(self): block = False extra_arches = 'arch123' force = True @@ -466,10 +396,10 @@ class TestPkglistBlock(unittest.TestCase): {'id': 112233, 'name': 'user1'}, ] user = users[0] - get_tag.return_value = tag - lookup_package.return_value = pkg - get_user.side_effect = get_user_factory(users) - readPackageList.return_value = {pkg['id']: { + self.get_tag.return_value = tag + self.lookup_package.return_value = pkg + self.get_user.side_effect = get_user_factory(users) + self.read_package_list.return_value = {pkg['id']: { 'owner_id': pkg['owner_id'], 'blocked': True, 'extra_arches': ''} @@ -479,14 +409,14 @@ class TestPkglistBlock(unittest.TestCase): extra_arches=extra_arches, force=force, update=update, policy=policy) - get_tag.assert_called_once_with(tag['name'], strict=True) - lookup_package.assert_called_once_with(pkg['name'], strict=False) - get_user.assert_has_calls([ + self.get_tag.assert_called_once_with(tag['name'], strict=True) + self.lookup_package.assert_called_once_with(pkg['name'], strict=False) + self.get_user.assert_has_calls([ mock.call(user['name'], strict=True), mock.call(112233), ]) # force + admin - assert_policy.assert_called_once_with( + self.assert_policy.assert_called_once_with( 'package_list', {'tag': 1, 'action': 'add', 'package': 'pkg', 'force': True}, force=True) @@ -499,4 +429,5 @@ class TestPkglistBlock(unittest.TestCase): owner=user['id'], block=block, 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) + self._pkglist_add.assert_called_once_with( + tag['id'], pkg['id'], user['id'], block, extra_arches)