#1059 Emit user in PackageListChange messages
Merged 2 years ago by tkopecek. Opened 3 years ago by tkopecek.
tkopecek/koji issue1035  into  master

file modified
+15 -8
@@ -966,8 +966,11 @@ 

          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 @@ 

          _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 @@ 

          #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):
@@ -1058,7 +1064,8 @@ 

      #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)
@@ -1078,7 +1085,7 @@ 

          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"""

file modified
+2 -1
@@ -158,7 +158,8 @@ 

      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

@@ -89,6 +89,7 @@ 

      @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 @@ 

  

  

      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

file modified
+70 -21
@@ -8,12 +8,26 @@ 

  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):
@@ -59,8 +73,8 @@ 

          _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')
@@ -178,10 +192,14 @@ 

          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 = {}

  

  
@@ -191,17 +209,22 @@ 

  

          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)
@@ -282,10 +305,14 @@ 

          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 = {}

  

  
@@ -299,17 +326,22 @@ 

              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)
@@ -330,10 +362,14 @@ 

          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,
@@ -347,13 +383,17 @@ 

  

          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')
@@ -372,10 +412,14 @@ 

          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,
@@ -388,7 +432,10 @@ 

  

          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()

  
@@ -396,10 +443,12 @@ 

          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)

@@ -44,9 +44,10 @@ 

                                             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 @@ 

                                             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 @@ 

                                             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,