From 4513f1e04417eff56db212f54b0b59fbcccc5f79 Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Mar 04 2020 13:56:20 +0000 Subject: [PATCH 1/3] hub: editTagExternalRepo is able to set merge_mode fixes: #1857 --- diff --git a/docs/schema-upgrade-1.20-1.21.sql b/docs/schema-upgrade-1.20-1.21.sql new file mode 100644 index 0000000..82a40d1 --- /dev/null +++ b/docs/schema-upgrade-1.20-1.21.sql @@ -0,0 +1,11 @@ +-- upgrade script to migrate the Koji database schema +-- from version 1.20 to 1.21 + + +BEGIN; + +-- merge_mode can not be null +UPDATE tag_external_repos SET merge_mode = 'koji' WHERE merge_mode is NULL; +ALTER TABLE tag_external_repos ALTER COLUMN merge_mode SET NOT NULL; + +COMMIT; diff --git a/docs/schema.sql b/docs/schema.sql index 4966015..e97e1ec 100644 --- a/docs/schema.sql +++ b/docs/schema.sql @@ -484,7 +484,7 @@ create table tag_external_repos ( tag_id INTEGER NOT NULL REFERENCES tag(id), external_repo_id INTEGER NOT NULL REFERENCES external_repo(id), priority INTEGER NOT NULL, - merge_mode TEXT DEFAULT 'koji', + merge_mode TEXT NOT NULL DEFAULT 'koji', -- versioned - see earlier description of versioning create_event INTEGER NOT NULL REFERENCES events(id) DEFAULT get_event(), revoke_event INTEGER REFERENCES events(id), diff --git a/hub/kojihub.py b/hub/kojihub.py index 46db0c5..4350b6b 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -3667,6 +3667,9 @@ def add_external_repo_to_tag(tag_info, repo_info, priority, merge_mode='koji'): context.session.assertPerm('tag') + # sanity check for None value, which may happen if DB schema isn't updated to 1.21+ + if merge_mode is None: + merge_mode = 'koji' if merge_mode not in koji.REPO_MERGE_MODES: raise koji.GenericError('Invalid merge mode: %s' % merge_mode) @@ -3710,9 +3713,12 @@ def remove_external_repo_from_tag(tag_info, repo_info): update.execute() -def edit_tag_external_repo(tag_info, repo_info, priority): +def edit_tag_external_repo(tag_info, repo_info, priority=None, merge_mode=None): """Edit a tag<->external repo association - This allows you to update the priority without removing/adding the repo.""" + This allows you to update the priority and merge_mode without removing/adding the repo. + + Note that None value of priority and merge_mode means no change on it + """ context.session.assertPerm('tag') @@ -3727,9 +3733,20 @@ def edit_tag_external_repo(tag_info, repo_info, priority): (repo['name'], tag['name'])) tag_repo = tag_repos[0] - if priority != tag_repo['priority']: + data = {} + for k in ('priority', 'merge_mode'): + val = locals().get(k) + # None value means no change + if val is not None and val != tag_repo[k]: + data[k] = val + if not data: + return False + else: + for k in ('priority', 'merge_mode'): + data.setdefault(k, tag_repo[k]) remove_external_repo_from_tag(tag_id, repo_id) - add_external_repo_to_tag(tag_id, repo_id, priority) + add_external_repo_to_tag(tag_id, repo_id, **data) + return True def get_tag_external_repos(tag_info=None, repo_info=None, event=None): From 64c7505bbaaf9faacc0bfdd678d4e5a93c6c8c22 Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Mar 04 2020 13:58:24 +0000 Subject: [PATCH 2/3] cli: extend edit-external-repo to support change of priority and merge mode --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index 11353be..8e9aa05 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -5470,22 +5470,48 @@ def handle_add_external_repo(goptions, session, args): def handle_edit_external_repo(goptions, session, args): "[admin] Edit data for an external repo" - usage = _("usage: %prog edit-external-repo ") + usage = _("usage: %prog edit-external-repo [options] ") parser = OptionParser(usage=get_usage_str(usage)) parser.add_option("--url", help=_("Change the url")) parser.add_option("--name", help=_("Change the name")) + parser.add_option("-t", "--tag", metavar="TAG", + help=_("Edit the repo properties for the tag.")) + parser.add_option("-p", "--priority", metavar="PRIORITY", type='int', + help=_("Edit the priority of the repo for the tag specified by --tag.")) + parser.add_option("-m", "--mode", metavar="MODE", + help=_("Edit the merge mode of the repo for the tag specified by --tag. " + "Options: %s.") % ", ".join(koji.REPO_MERGE_MODES)) (options, args) = parser.parse_args(args) if len(args) != 1: parser.error(_("Incorrect number of arguments")) - opts = {} + repo_opts = {} if options.url: - opts['url'] = options.url + repo_opts['url'] = options.url if options.name: - opts['name'] = options.name - if not opts: + repo_opts['name'] = options.name + tag_repo_opts = {} + if options.tag: + if options.priority is not None: + tag_repo_opts['priority'] = options.priority + if options.mode: + tag_repo_opts['merge_mode'] = options.mode + if not tag_repo_opts: + parser.error(_("At least, one of priority and merge mode should be specified")) + tag_repo_opts['tag_info'] = options.tag + tag_repo_opts['repo_info'] = args[0] + else: + for k in ('priority', 'mode'): + if getattr(options, k) is not None: + parser.error(_("If %s is specified, --tag must be specified as well") % k) + + if not (repo_opts or tag_repo_opts): parser.error(_("No changes specified")) + activate_session(session, goptions) - session.editExternalRepo(args[0], **opts) + if repo_opts: + session.editExternalRepo(args[0], **repo_opts) + if tag_repo_opts: + session.editTagExternalRepo(**tag_repo_opts) def handle_remove_external_repo(goptions, session, args): From ae22245ae58553f4346f754d2f8cd0a9f874a247 Mon Sep 17 00:00:00 2001 From: Yuming Zhu Date: Mar 04 2020 13:58:24 +0000 Subject: [PATCH 3/3] unittests: for edit-external-repo cmd and editTagExternalRepo API --- diff --git a/tests/test_cli/test_edit_external_repo.py b/tests/test_cli/test_edit_external_repo.py new file mode 100644 index 0000000..dda7bdd --- /dev/null +++ b/tests/test_cli/test_edit_external_repo.py @@ -0,0 +1,101 @@ +# coding=utf-8 +from __future__ import absolute_import +import mock +import six +try: + import unittest2 as unittest +except ImportError: + import unittest + +import koji +from koji_cli.commands import handle_edit_external_repo +from . import utils + + +class TestEditExternalRepo(utils.CliTestCase): + + # Show long diffs in error output... + maxDiff = None + + def setUp(self): + self.options = mock.MagicMock() + self.session = mock.MagicMock() + self.error_format = """Usage: %s edit-external-repo [options] +(Specify the --help global option for a list of other help options) + +%s: error: {message} +""" % (self.progname, self.progname) + + @mock.patch('sys.stdout', new_callable=six.StringIO) + @mock.patch('sys.stderr', new_callable=six.StringIO) + @mock.patch('koji_cli.commands.activate_session') + def test_handle_edit_external_repo_error( + self, + activate_session_mock, + stderr, + stdout): + """Test handle_edit_external_repo function""" + # [(expected, args),...] + items = [ + ("Incorrect number of arguments", []), + ("Incorrect number of arguments", ['arg1', 'arg2']), + ("No changes specified", ['ext_repo']), + ("At least, one of priority and merge mode should be specified", + ['ext_repo', '-t', 'tag']), + ("If priority is specified, --tag must be specified as well", + ['ext_repo', '-p', '0']), + ("If mode is specified, --tag must be specified as well", + ['ext_repo', '-m', 'koji']) + + ] + for expected, cmd_args in items: + self.assert_system_exit( + handle_edit_external_repo, + self.options, + self.session, + cmd_args, + stderr=self.format_error_message(expected), + activate_session=None) + + # edit ext-repo only + handle_edit_external_repo(self.options, self.session, + ['ext_repo','--name', 'newname', '--url', 'https://newurl']) + self.assert_console_message(stdout, "") + self.assert_console_message(stderr, "") + self.session.editExternalRepo.assert_called_once_with('ext_repo', + name='newname', url='https://newurl') + self.session.editTagExternalRepo.assert_not_called() + + # edit tag-repo only + self.session.reset_mock() + handle_edit_external_repo(self.options, self.session, + ['ext_repo','-t', 'tag', '-p', '0', '-m', 'koji']) + self.assert_console_message(stdout, "") + self.assert_console_message(stderr, "") + self.session.editExternalRepo.assert_not_called() + self.session.editTagExternalRepo.assert_called_once_with(repo_info='ext_repo', + tag_info='tag', + priority=0, + merge_mode='koji') + + def test_handle_edit_external_repo_help(self): + self.assert_help( + handle_edit_external_repo, + """Usage: %s edit-external-repo [options] +(Specify the --help global option for a list of other help options) + +Options: + -h, --help show this help message and exit + --url=URL Change the url + --name=NAME Change the name + -t TAG, --tag=TAG Edit the repo properties for the tag. + -p PRIORITY, --priority=PRIORITY + Edit the priority of the repo for the tag specified by + --tag. + -m MODE, --mode=MODE Edit the merge mode of the repo for the tag specified + by --tag. Options: %s. +""" % (self.progname, ', '.join(koji.REPO_MERGE_MODES))) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/test_hub/test_edit_tag_external_repo.py b/tests/test_hub/test_edit_tag_external_repo.py new file mode 100644 index 0000000..f7dd3b4 --- /dev/null +++ b/tests/test_hub/test_edit_tag_external_repo.py @@ -0,0 +1,85 @@ +import mock +try: + import unittest2 as unittest +except ImportError: + import unittest + +import koji +import kojihub + + +class TestEditTagExternalRepo(unittest.TestCase): + + def setUp(self): + self.context = mock.patch('kojihub.context').start() + self.context.session.assertPerm = mock.MagicMock() + self.get_tag = mock.patch('kojihub.get_tag').start() + self.get_external_repo = mock.patch('kojihub.get_external_repo').start() + self.get_tag_external_repos = mock.patch('kojihub.get_tag_external_repos').start() + self.get_tag.return_value = {'id': 1, 'name': 'tag'} + self.get_external_repo.return_value = {'id': 11, 'name': 'ext_repo'} + self.get_tag_external_repos.return_value = [{'external_repo_id': 11, + 'tag_id': 1, + 'priority': 5, + 'merge_mode': 'simple'}] + + self.remove_external_repo_from_tag = mock.patch( + 'kojihub.remove_external_repo_from_tag').start() + self.add_external_repo_to_tag = mock.patch('kojihub.add_external_repo_to_tag').start() + + def tearDown(self): + mock.patch.stopall() + + def test_edit(self): + rv = kojihub.edit_tag_external_repo('tag', 'ext_repo', priority=6, merge_mode='bare') + self.get_tag.assert_called_once_with('tag', strict=True) + self.get_external_repo.assert_called_once_with('ext_repo', strict=True) + self.get_tag_external_repos.assert_called_once_with(tag_info=1, repo_info=11) + self.remove_external_repo_from_tag.assert_called_once_with(1, 11) + self.add_external_repo_to_tag.assert_called_once_with(1, 11, priority=6, merge_mode='bare') + self.assertTrue(rv) + + def test_edit_no_tag_repo(self): + self.get_tag_external_repos.return_value = [] + with self.assertRaises(koji.GenericError) as cm: + kojihub.edit_tag_external_repo('tag', 'ext_repo', priority=6, merge_mode='bare') + self.assertEqual(cm.exception.args[0], + 'external repo ext_repo not associated with tag tag') + self.get_tag.assert_called_once_with('tag', strict=True) + self.get_external_repo.assert_called_once_with('ext_repo', strict=True) + self.get_tag_external_repos.assert_called_once_with(tag_info=1, repo_info=11) + self.remove_external_repo_from_tag.assert_not_called() + self.add_external_repo_to_tag.assert_not_called() + + def test_edit_no_changes_2(self): + rv = kojihub.edit_tag_external_repo('tag', 'ext_repo', priority=5, merge_mode='simple') + self.remove_external_repo_from_tag.assert_not_called() + self.add_external_repo_to_tag.assert_not_called() + self.assertFalse(rv) + + def test_edit_all_none(self): + self.get_tag_external_repos.return_value = [{'external_repo_id': 11, + 'tag_id': 1, + 'priority': None, + 'merge_mode': None}] + rv = kojihub.edit_tag_external_repo('tag', 'ext_repo', priority=None, merge_mode=None) + self.remove_external_repo_from_tag.assert_not_called() + self.add_external_repo_to_tag.assert_not_called() + self.assertFalse(rv) + + def test_edit_none_new(self): + rv = kojihub.edit_tag_external_repo('tag', 'ext_repo', priority=None, merge_mode=None) + self.remove_external_repo_from_tag.assert_not_called() + self.add_external_repo_to_tag.assert_not_called() + self.assertFalse(rv) + + def test_edit_none_old(self): + self.get_tag_external_repos.return_value = [{'external_repo_id': 11, + 'tag_id': 1, + 'priority': 5, + 'merge_mode': None}] + rv = kojihub.edit_tag_external_repo('tag', 'ext_repo', priority=None, merge_mode='simple') + self.remove_external_repo_from_tag.assert_called_once_with(1, 11) + self.add_external_repo_to_tag.assert_called_once_with(1, 11, + priority=5, merge_mode='simple') + self.assertTrue(rv)