From 75d07f6d01d52d4f984e96c1f05e831a03ae4f5f Mon Sep 17 00:00:00 2001
From: Yuming Zhu <yzhu@redhat.com>
Date: Mar 16 2017 20:48:13 +0000
Subject: make cli and hub to support remove-extra for edit-tag


---

diff --git a/cli/koji b/cli/koji
index 40cc7c7..67fa358 100755
--- a/cli/koji
+++ b/cli/koji
@@ -4886,6 +4886,8 @@ def handle_edit_tag(options, session, args):
     parser.add_option("--no-include-all", action="store_true", help=_("Do not include all packages in this tag when generating Maven repos"))
     parser.add_option("-x", "--extra", action="append", default=[], metavar="key=value",
                       help=_("Set tag extra option"))
+    parser.add_option("-r", "--remove-extra", action="append", default=[], metavar="key",
+                      help=_("Remove tag extra option"))
     (options, args) = parser.parse_args(args)
     if len(args) != 1:
         parser.error(_("Please specify a name for the tag"))
@@ -4920,8 +4922,10 @@ def handle_edit_tag(options, session, args):
             value = arg_filter(value)
             extra[key] = value
         opts['extra'] = extra
+    if options.remove_extra:
+        opts['remove_extra'] = options.remove_extra
     #XXX change callname
-    session.editTag2(tag,**opts)
+    session.editTag2(tag, **opts)
 
 def handle_lock_tag(options, session, args):
     "[admin] Lock a tag"
diff --git a/hub/kojihub.py b/hub/kojihub.py
index 2be5d84..f820c17 100644
--- a/hub/kojihub.py
+++ b/hub/kojihub.py
@@ -2976,7 +2976,8 @@ def edit_tag(tagInfo, **kwargs):
         maven_support: whether Maven repos should be generated for the tag
         maven_include_all: include every build in this tag (including multiple
                            versions of the same package) in the Maven repo
-        extra: extra tag parameters (dictionary)
+        extra: add or update extra tag parameters (dictionary)
+        remove_extra: remove extra tag parameters (list)
     """
 
     context.session.assertPerm('admin')
@@ -3014,8 +3015,8 @@ def _edit_tag(tagInfo, **kwargs):
             #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"""
+SET name = %(name)s
+WHERE id = %(tagID)i"""
         _dml(update, values)
 
     #check for changes
@@ -3038,13 +3039,18 @@ def _edit_tag(tagInfo, **kwargs):
 
     # handle extra data
     if 'extra' in kwargs:
+        # check whether one key is both in extra and remove_extra
+        if 'remove_extra' in kwargs:
+            for removed in kwargs['remove_extra']:
+                if removed in kwargs['extra']:
+                    raise koji.GenericError("Can not both add/update and remove tag-extra: '%s'" % removed)
         for key in kwargs['extra']:
             value = kwargs['extra'][key]
-            if key not in tag['extra'] or tag['extra'] != value:
+            if key not in tag['extra'] or tag['extra'][key] != value:
                 data = {
-                    'tag_id' : tag['id'],
-                    'key' : key,
-                    'value' : json.dumps(kwargs['extra'][key]),
+                    'tag_id': tag['id'],
+                    'key': key,
+                    'value': json.dumps(kwargs['extra'][key]),
                 }
                 # revoke old entry, if any
                 update = UpdateProcessor('tag_extra', values=data, clauses=['tag_id = %(tag_id)i', 'key=%(key)s'])
@@ -3055,6 +3061,21 @@ def _edit_tag(tagInfo, **kwargs):
                 insert.make_create()
                 insert.execute()
 
+    # handle remove_extra data
+    if 'remove_extra' in kwargs:
+        ne = [e for e in kwargs['remove_extra'] if e not in tag['extra']]
+        if ne:
+            raise koji.GenericError("Tag: %s doesn't have extra: %s" % (tag['name'], ', '.join(ne)))
+        for key in kwargs['remove_extra']:
+            data = {
+                'tag_id': tag['id'],
+                'key': key,
+            }
+            # revoke old entry
+            update = UpdateProcessor('tag_extra', values=data, clauses=['tag_id = %(tag_id)i', 'key=%(key)s'])
+            update.make_revoke()
+            update.execute()
+
 
 def old_edit_tag(tagInfo, name, arches, locked, permissionID, extra=None):
     """Edit information for an existing tag."""
diff --git a/tests/test_cli/test_edit_tag.py b/tests/test_cli/test_edit_tag.py
new file mode 100644
index 0000000..3b7cbf2
--- /dev/null
+++ b/tests/test_cli/test_edit_tag.py
@@ -0,0 +1,180 @@
+import unittest
+
+import StringIO as stringio
+
+import os
+
+import sys
+
+import mock
+
+from mock import call
+
+import loadcli
+
+cli = loadcli.cli
+
+progname = os.path.basename(sys.argv[0]) or 'koji'
+
+
+class TestEditTag(unittest.TestCase):
+    # Show long diffs in error output...
+    maxDiff = None
+
+    @mock.patch('sys.stdout', new_callable=stringio.StringIO)
+    @mock.patch('koji_cli.activate_session')
+    def test_handle_edit_tag(self, activate_session_mock, stdout):
+        tag = 'tag'
+        arches = 'arch1 arch2'
+        perm = 'perm'
+        locked = True
+        rename = 'tag2'
+        maven_support = True
+        maven_include_all = True
+        extra = {'extraA': 'A', 'extraB': True}
+        remove_extra = ['extraC', 'extraD']
+        args = [tag]
+        args.append('--arches=' + arches)
+        args.append('--perm=' + perm)
+        args.append('--lock')
+        args.append('--rename=' + rename)
+        args.append('--maven-support')
+        args.append('--include-all')
+        for k, x in extra.iteritems():
+            args.append('-x')
+            args.append(k + '=' + str(x))
+        for r in remove_extra:
+            args.append('-r')
+            args.append(r)
+        opts = {'arches': arches,
+                'perm': perm,
+                'locked': locked,
+                'name': rename,
+                'maven_support': maven_support,
+                'maven_include_all': maven_include_all,
+                'extra': extra,
+                'remove_extra': remove_extra}
+        options = mock.MagicMock()
+
+        # Mock out the xmlrpc server
+        session = mock.MagicMock()
+
+        # Run it and check immediate output
+        # args: tag --arches='arch1 arch2' --perm --lock
+        # --rename=tag2 --maven-support --include-all
+        # -x extraA=A -x extraB=True -r extraC -r extraD
+        # expected: success
+        rv = cli.handle_edit_tag(options, session, args)
+        actual = stdout.getvalue()
+        expected = ''
+        self.assertMultiLineEqual(actual, expected)
+        # Finally, assert that things were called as we expected.
+        activate_session_mock.assert_called_once_with(session)
+        session.editTag2.assert_called_once_with(tag, **opts)
+        self.assertEqual(rv, None)
+
+        stdout.seek(0)
+        stdout.truncate()
+        session.reset_mock()
+        activate_session_mock.reset_mock()
+        args = [tag]
+        args.append('--no-perm')
+        args.append('--unlock')
+        args.append('--no-maven-support')
+        args.append('--no-include-all')
+        opts = {'perm_id': None,
+                'locked': not locked,
+                'maven_support': not maven_support,
+                'maven_include_all': not maven_include_all}
+        # Run it and check immediate output
+        # args: tag --no-perm --unlock --no-maven-support --no-include-all
+        # expected: success
+        rv = cli.handle_edit_tag(options, session, args)
+        actual = stdout.getvalue()
+        expected = ''
+        self.assertMultiLineEqual(actual, expected)
+        # Finally, assert that things were called as we expected.
+        activate_session_mock.assert_called_once_with(session)
+        session.editTag2.assert_called_once_with(tag, **opts)
+        self.assertEqual(rv, None)
+
+    @mock.patch('sys.stdout', new_callable=stringio.StringIO)
+    @mock.patch('sys.stderr', new_callable=stringio.StringIO)
+    @mock.patch('koji_cli.activate_session')
+    def test_handle_edit_tag_help(self, activate_session_mock, stderr, stdout):
+        args = ['--help']
+        options = mock.MagicMock()
+
+        # Mock out the xmlrpc server
+        session = mock.MagicMock()
+
+        # Run it and check immediate output
+        # args: --help
+        # expected: failed, help info shows
+        with self.assertRaises(SystemExit) as cm:
+            cli.handle_edit_tag(options, session, args)
+        actual_stdout = stdout.getvalue()
+        actual_stderr = stderr.getvalue()
+        expected_stdout = """Usage: %s edit-tag [options] name
+(Specify the --help global option for a list of other help options)
+
+Options:
+  -h, --help            show this help message and exit
+  --arches=ARCHES       Specify arches
+  --perm=PERM           Specify permission requirement
+  --no-perm             Remove permission requirement
+  --lock                Lock the tag
+  --unlock              Unlock the tag
+  --rename=RENAME       Rename the tag
+  --maven-support       Enable creation of Maven repos for this tag
+  --no-maven-support    Disable creation of Maven repos for this tag
+  --include-all         Include all packages in this tag when generating Maven
+                        repos
+  --no-include-all      Do not include all packages in this tag when
+                        generating Maven repos
+  -x key=value, --extra=key=value
+                        Set tag extra option
+  -r key, --remove-extra=key
+                        Remove tag extra option
+""" % progname
+        expected_stderr = ''
+        self.assertMultiLineEqual(actual_stdout, expected_stdout)
+        self.assertMultiLineEqual(actual_stderr, expected_stderr)
+        # Finally, assert that things were called as we expected.
+        activate_session_mock.assert_not_called()
+        session.editTag2.assert_not_called()
+        self.assertEqual(cm.exception.code, 0)
+
+    @mock.patch('sys.stdout', new_callable=stringio.StringIO)
+    @mock.patch('sys.stderr', new_callable=stringio.StringIO)
+    @mock.patch('koji_cli.activate_session')
+    def test_handle_edit_tag_no_arg(self, activate_session_mock, stderr, stdout):
+        args = []
+        options = mock.MagicMock()
+
+        # Mock out the xmlrpc server
+        session = mock.MagicMock()
+
+        # Run it and check immediate output
+        # args: --help
+        # expected: failed, help info shows
+        with self.assertRaises(SystemExit) as cm:
+            cli.handle_edit_tag(options, session, args)
+        actual_stdout = stdout.getvalue()
+        actual_stderr = stderr.getvalue()
+        expected_stdout = ''
+        expected_stderr = """Usage: %(progname)s edit-tag [options] name
+(Specify the --help global option for a list of other help options)
+
+%(progname)s: error: Please specify a name for the tag
+""" % {'progname': progname}
+        self.assertMultiLineEqual(actual_stdout, expected_stdout)
+        self.assertMultiLineEqual(actual_stderr, expected_stderr)
+        # Finally, assert that things were called as we expected.
+        activate_session_mock.assert_not_called()
+        session.editTag2.assert_not_called()
+        self.assertEqual(cm.exception.code, 2)
+
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/tests/test_hub/test_edit_tag.py b/tests/test_hub/test_edit_tag.py
new file mode 100644
index 0000000..65d6dc6
--- /dev/null
+++ b/tests/test_hub/test_edit_tag.py
@@ -0,0 +1,196 @@
+import mock
+import unittest
+
+import koji
+import kojihub
+
+UP = kojihub.UpdateProcessor
+IP = kojihub.InsertProcessor
+
+
+class TestEditTag(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 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._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()
+        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()
+
+    def tearDown(self):
+        mock.patch.stopall()
+
+    def test_maven_disable(self):
+        self.context.opts.get.return_value = False
+        with self.assertRaises(koji.GenericError):
+            kojihub._edit_tag('tag', maven_support=True)
+        with self.assertRaises(koji.GenericError):
+            kojihub._edit_tag('tag', maven_include_all=False)
+        self.context.opts.get.return_value = True
+        kojihub._edit_tag('tag', maven_support=True)
+        self.get_tag.assert_called_once()
+
+    def test_edit(self):
+        self.get_tag.return_value = {'id': 333,
+                                     'perm_id': 1,
+                                     'name': 'tag',
+                                     'arches': 'arch1 arch3',
+                                     'locked': False,
+                                     'maven_support': False,
+                                     'maven_include_all': False,
+                                     'extra': {'exA': 1,
+                                               'exC': 3,
+                                               'exD': 4}}
+        self._singleValue.return_value = None
+        self.context.event_id = 42
+        self.context.session.user_id = 23
+        # no1 invoke
+        kwargs = {
+            'perm': None,
+            'name': 'newtag',
+            'arches': 'arch1 arch2',
+            'locked': True,
+            'maven_support': True,
+            'maven_include_all': True,
+            'extra': {'exB': [None, 1, 'text', {'dict': 'val'}, (1, 2)]},
+            'remove_extra': ['exC']
+        }
+        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.inserts), 2)
+        values = {
+            'arches': 'arch1 arch2',
+            'locked': True,
+            'maven_include_all': True,
+            'maven_support': True,
+            'perm_id': None,
+            'id': 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]
+        self.assertEqual(update.table, 'tag_config')
+        self.assertEqual(update.values, values)
+        self.assertEqual(update.data, revoke_data)
+        self.assertEqual(update.rawdata, revoke_rawdata)
+        self.assertEqual(update.clauses, ['tag_id = %(id)i', 'active = TRUE'])
+
+        data = {
+            'create_event': 42,
+            'creator_id': 23,
+            'arches': 'arch1 arch2',
+            'locked': True,
+            'maven_include_all': True,
+            'maven_support': True,
+            'perm_id': None,
+            'tag_id': 333,
+        }
+        insert = self.inserts[0]
+        self.assertEqual(insert.table, 'tag_config')
+        self.assertEqual(insert.data, data)
+        self.assertEqual(insert.rawdata, {})
+
+        values = {
+            'key': 'exB',
+            'value': '[null, 1, "text", {"dict": "val"}, [1, 2]]',
+            'tag_id': 333,
+        }
+
+        update = self.updates[1]
+        self.assertEqual(update.table, 'tag_extra')
+        self.assertEqual(update.values, values)
+        self.assertEqual(update.data, revoke_data)
+        self.assertEqual(update.rawdata, revoke_rawdata)
+        self.assertEqual(update.clauses, ['tag_id = %(tag_id)i', 'key=%(key)s', 'active = TRUE'])
+
+        data = {
+            'create_event': 42,
+            'creator_id': 23,
+            'key': 'exB',
+            'value': '[null, 1, "text", {"dict": "val"}, [1, 2]]',
+            'tag_id': 333,
+        }
+
+        insert = self.inserts[1]
+        self.assertEqual(insert.table, 'tag_extra')
+        self.assertEqual(insert.data, data)
+        self.assertEqual(insert.rawdata, {})
+
+        values = {
+            'key': 'exC',
+            'tag_id': 333,
+        }
+
+        update = self.updates[2]
+        self.assertEqual(update.table, 'tag_extra')
+        self.assertEqual(update.values, values)
+        self.assertEqual(update.data, revoke_data)
+        self.assertEqual(update.rawdata, revoke_rawdata)
+        self.assertEqual(update.clauses, ['tag_id = %(tag_id)i', 'key=%(key)s', 'active = TRUE'])
+
+        # no2 invoke
+        kwargs = {
+            'extra': {'exC': 'text'},
+            'remove_extra': ['exC']
+        }
+
+        with self.assertRaises(koji.GenericError) as cm:
+            kojihub._edit_tag('tag', **kwargs)
+        self.assertEqual(cm.exception.args[0], 'Can not both add/update and remove tag-extra: \'exC\'')
+
+        # no3 invoke
+        kwargs = {
+            'remove_extra': ['exE']
+        }
+
+        with self.assertRaises(koji.GenericError) as cm:
+            kojihub._edit_tag('tag', **kwargs)
+        self.assertEqual(cm.exception.args[0], "Tag: tag doesn't have extra: exE")
+
+        # no4 invoke
+        self.get_perm_id.reset_mock()
+        self.get_perm_id.return_value = 99
+        self._singleValue.reset_mock()
+        self._singleValue.return_value = 2
+
+        kwargs = {
+            'perm': 'admin',
+            'name': 'newtag',
+        }
+        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')