From 63a6f43d8af67c516c627228c600aa64a85d5311 Mon Sep 17 00:00:00 2001 From: Jana Cupova Date: Mon, 29 Jan 2024 18:11:48 +0100 Subject: [PATCH] Fix remove-tag-inheritance with priority Fix remote-tag-inheritance with priority Fixes: https://pagure.io/koji/issue/3985 --- cli/koji_cli/commands.py | 20 +-- tests/test_cli/test_edit_tag_inheritance.py | 9 +- tests/test_cli/test_remove_tag_inheritance.py | 10 +- tests/test_hub/test_write_inheritance_data.py | 128 ++++++++++++++++++ 4 files changed, 150 insertions(+), 17 deletions(-) create mode 100644 tests/test_hub/test_write_inheritance_data.py diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index 95189a2c..7553890e 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -5227,7 +5227,7 @@ def handle_add_tag_inheritance(goptions, session, args): def handle_edit_tag_inheritance(goptions, session, args): """[admin] Edit tag inheritance""" - usage = "usage: %prog edit-tag-inheritance [options] " + usage = "usage: %prog edit-tag-inheritance [options] []" parser = OptionParser(usage=get_usage_str(usage)) parser.add_option("--priority", help="Specify a new priority") parser.add_option("--maxdepth", help="Specify max depth") @@ -5256,13 +5256,13 @@ def handle_edit_tag_inheritance(goptions, session, args): if not parent: parser.error("No such tag: %s" % args[1]) if len(args) > 2: - priority = args[2] + priority = int(args[2]) data = session.getInheritanceData(tag['id']) if parent and data: data = [datum for datum in data if datum['parent_id'] == parent['id']] if priority and data: - data = [datum for datum in data if datum['priority'] == priority] + data = [datum for datum in data if int(datum['priority']) == priority] if len(data) == 0: error("No inheritance link found to remove. Please check your arguments") @@ -5278,10 +5278,12 @@ def handle_edit_tag_inheritance(goptions, session, args): data = data[0] inheritanceData = session.getInheritanceData(tag['id']) - samePriority = [datum for datum in inheritanceData if datum['priority'] == options.priority] - if samePriority: - error("Error: There is already an active inheritance with that priority on %s, " - "please specify a different priority with --priority." % tag['name']) + if options.priority is not None: + samePriority = [datum for datum in inheritanceData + if datum['priority'] == int(options.priority)] + if samePriority: + error("Error: There is already an active inheritance with that priority on %s, " + "please specify a different priority with --priority." % tag['name']) new_data = data.copy() if options.priority is not None and options.priority.isdigit(): @@ -5308,7 +5310,7 @@ def handle_edit_tag_inheritance(goptions, session, args): def handle_remove_tag_inheritance(goptions, session, args): """[admin] Remove a tag inheritance link""" - usage = "usage: %prog remove-tag-inheritance " + usage = "usage: %prog remove-tag-inheritance []" parser = OptionParser(usage=get_usage_str(usage)) (options, args) = parser.parse_args(args) @@ -5332,7 +5334,7 @@ def handle_remove_tag_inheritance(goptions, session, args): if not parent: parser.error("No such tag: %s" % args[1]) if len(args) > 2: - priority = args[2] + priority = int(args[2]) data = session.getInheritanceData(tag['id']) if parent and data: diff --git a/tests/test_cli/test_edit_tag_inheritance.py b/tests/test_cli/test_edit_tag_inheritance.py index fe31adbd..87eec6ef 100644 --- a/tests/test_cli/test_edit_tag_inheritance.py +++ b/tests/test_cli/test_edit_tag_inheritance.py @@ -15,7 +15,7 @@ class TestEditTagInheritance(utils.CliTestCase): self.session = mock.MagicMock() self.session.getAPIVersion.return_value = koji.API_VERSION self.activate_session_mock = mock.patch('koji_cli.commands.activate_session').start() - self.error_format = """Usage: %s edit-tag-inheritance [options] + self.error_format = """Usage: %s edit-tag-inheritance [options] [] (Specify the --help global option for a list of other help options) %s: error: {message} @@ -31,7 +31,7 @@ class TestEditTagInheritance(utils.CliTestCase): 'noconfig': False, 'parent_id': 2, 'pkg_filter': '', - 'priority': self.priority} + 'priority': int(self.priority)} self.child_tag_info = {'arches': 'x86_64', 'extra': {}, 'id': 1, @@ -51,6 +51,9 @@ class TestEditTagInheritance(utils.CliTestCase): 'perm': None, 'perm_id': None} + def tearDown(self): + mock.patch.stopall() + def test_edit_tag_inheritance_without_option(self): expected = self.format_error_message( "This command takes at least one argument: a tag name or ID") @@ -271,7 +274,7 @@ class TestEditTagInheritance(utils.CliTestCase): def test_edit_tag_inheritance_help(self): self.assert_help( handle_edit_tag_inheritance, - """Usage: %s edit-tag-inheritance [options] + """Usage: %s edit-tag-inheritance [options] [] (Specify the --help global option for a list of other help options) Options: diff --git a/tests/test_cli/test_remove_tag_inheritance.py b/tests/test_cli/test_remove_tag_inheritance.py index f4b88f5f..dd02d265 100644 --- a/tests/test_cli/test_remove_tag_inheritance.py +++ b/tests/test_cli/test_remove_tag_inheritance.py @@ -15,7 +15,7 @@ class TestRemoveTagInheritance(utils.CliTestCase): self.session = mock.MagicMock() self.session.getAPIVersion.return_value = koji.API_VERSION self.activate_session_mock = mock.patch('koji_cli.commands.activate_session').start() - self.error_format = """Usage: %s remove-tag-inheritance + self.error_format = """Usage: %s remove-tag-inheritance [] (Specify the --help global option for a list of other help options) %s: error: {message} @@ -30,7 +30,7 @@ class TestRemoveTagInheritance(utils.CliTestCase): 'noconfig': False, 'parent_id': 2, 'pkg_filter': '', - 'priority': self.priority} + 'priority': int(self.priority)} self.child_tag_info = {'arches': 'x86_64', 'extra': {}, 'id': 1, @@ -206,13 +206,13 @@ class TestRemoveTagInheritance(utils.CliTestCase): self.session.getInheritanceData.assert_has_calls([call(1), call(1)]) self.session.setInheritanceData.assert_called_once_with( 1, [{'child_id': 1, 'intransitive': False, 'maxdepth': None, 'name': self.tag, - 'noconfig': False, 'parent_id': 2, 'pkg_filter': '', 'priority': self.priority, - 'delete link': True}]) + 'noconfig': False, 'parent_id': 2, 'pkg_filter': '', + 'priority': int(self.priority), 'delete link': True}]) def test_remove_tag_inheritance_help(self): self.assert_help( handle_remove_tag_inheritance, - """Usage: %s remove-tag-inheritance + """Usage: %s remove-tag-inheritance [] (Specify the --help global option for a list of other help options) Options: diff --git a/tests/test_hub/test_write_inheritance_data.py b/tests/test_hub/test_write_inheritance_data.py new file mode 100644 index 00000000..f5b3de1a --- /dev/null +++ b/tests/test_hub/test_write_inheritance_data.py @@ -0,0 +1,128 @@ +# coding: utf-8 +import unittest + +import mock + +import koji +import kojihub + +UP = kojihub.UpdateProcessor +IP = kojihub.InsertProcessor + + +class TestWriteInheritanceData(unittest.TestCase): + + def getInsert(self, *args, **kwargs): + insert = IP(*args, **kwargs) + insert.execute = mock.MagicMock() + insert.make_create = mock.MagicMock() + self.inserts.append(insert) + return insert + + def getUpdate(self, *args, **kwargs): + update = UP(*args, **kwargs) + update.execute = mock.MagicMock() + update.make_revoke = mock.MagicMock() + self.updates.append(update) + return update + + def setUp(self): + self.maxDiff = None + self.InsertProcessor = mock.patch('kojihub.kojihub.InsertProcessor', + side_effect=self.getInsert).start() + self.inserts = [] + self.UpdateProcessor = mock.patch('kojihub.kojihub.UpdateProcessor', + side_effect=self.getUpdate).start() + self.updates = [] + self.read_inheritance_data = mock.patch('kojihub.kojihub.readInheritanceData').start() + self.get_tag = mock.patch('kojihub.kojihub.get_tag').start() + self.context = mock.patch('kojihub.kojihub.context').start() + self.context.session.assertPerm = mock.MagicMock() + self.tag_id = 5 + self.changes = {'parent_id': 10, 'priority': 7, 'maxdepth': None, 'intransitive': False, + 'noconfig': False, 'pkg_filter': '', 'child_id': 5, 'is_update': True} + + def tearDown(self): + mock.patch.stopall() + + def test_no_value_check_fields(self): + changes = self.changes.copy() + del changes['parent_id'] + with self.assertRaises(koji.GenericError) as cm: + kojihub.writeInheritanceData(self.tag_id, changes) + self.assertEqual("No value for parent_id", str(cm.exception)) + self.read_inheritance_data.assert_not_called() + self.get_tag.assert_not_called() + self.assertEqual(len(self.inserts), 0) + self.assertEqual(len(self.updates), 0) + + def test_valid(self): + self.read_inheritance_data.return_value = [ + {'intransitive': False, 'maxdepth': None, 'name': 'test-tag', 'noconfig': False, + 'parent_id': 10, 'pkg_filter': '', 'priority': 4, 'child_id': 5}] + self.get_tag.return_value = {'id': 10, 'name': 'parent_tag'} + rv = kojihub.writeInheritanceData(self.tag_id, self.changes) + self.assertEqual(rv, None) + self.read_inheritance_data.assert_called_once_with(5) + self.get_tag.assert_called_once_with(10, strict=True) + self.assertEqual(len(self.inserts), 1) + self.assertEqual(len(self.updates), 1) + update = self.updates[0] + self.assertEqual(update.table, 'tag_inheritance') + self.assertEqual(update.clauses, ['tag_id=%(tag_id)s', 'parent_id = %(parent_id)s']) + + insert = self.inserts[0] + self.assertEqual(insert.table, 'tag_inheritance') + self.assertEqual(insert.data, {'parent_id': 10, 'priority': 7, 'maxdepth': None, + 'intransitive': False, 'noconfig': False, + 'pkg_filter': '', 'tag_id': 5}) + self.assertEqual(insert.rawdata, {}) + + def test_delete_link(self): + changes = self.changes.copy() + changes['delete link'] = True + self.read_inheritance_data.return_value = [ + {'intransitive': False, 'maxdepth': None, 'name': 'test-tag', 'noconfig': False, + 'parent_id': 10, 'pkg_filter': '', 'priority': 4, 'child_id': 5}] + self.get_tag.return_value = {'id': 10, 'name': 'parent_tag'} + rv = kojihub.writeInheritanceData(self.tag_id, changes) + self.assertEqual(rv, None) + self.read_inheritance_data.assert_called_once_with(5) + self.get_tag.assert_called_once_with(10, strict=True) + self.assertEqual(len(self.inserts), 0) + self.assertEqual(len(self.updates), 1) + update = self.updates[0] + self.assertEqual(update.table, 'tag_inheritance') + self.assertEqual(update.clauses, ['tag_id=%(tag_id)s', 'parent_id = %(parent_id)s']) + + def test_multiple_parent_with_the_same_priority(self): + changes = self.changes.copy() + changes = [changes, {'parent_id': 8, 'priority': 7, 'maxdepth': None, + 'intransitive': False, 'noconfig': False, 'pkg_filter': '', + 'child_id': 5, 'is_update': True}] + self.read_inheritance_data.return_value = [ + {'intransitive': False, 'maxdepth': None, 'name': 'test-tag', 'noconfig': False, + 'parent_id': 10, 'pkg_filter': '', 'priority': 4, 'child_id': 5}] + self.get_tag.return_value = {'id': 10, 'name': 'parent_tag'} + with self.assertRaises(koji.GenericError) as cm: + kojihub.writeInheritanceData(self.tag_id, changes, clear=True) + self.assertEqual(f"Multiple parent tags ([{changes[0]['parent_id']}, " + f"{changes[1]['parent_id']}]) cannot have the same priority value " + f"({changes[0]['priority']}) on child tag {changes[0]['child_id']}", + str(cm.exception)) + self.read_inheritance_data.assert_called_once_with(5) + self.get_tag.assert_has_calls([mock.call(10, strict=True)], [mock.call(7, strict=True)]) + self.assertEqual(len(self.inserts), 0) + self.assertEqual(len(self.updates), 0) + + def test_no_inheritance_changes(self): + self.read_inheritance_data.return_value = [ + {'intransitive': False, 'maxdepth': None, 'name': 'test-tag', 'noconfig': False, + 'parent_id': 10, 'pkg_filter': '', 'priority': 7, 'child_id': 5}] + self.get_tag.return_value = {'id': 10, 'name': 'parent_tag'} + rv = kojihub.writeInheritanceData(self.tag_id, self.changes) + self.assertEqual(rv, None) + self.read_inheritance_data.assert_called_once_with(5) + self.get_tag.assert_called_once_with(10, strict=True) + self.assertEqual(len(self.inserts), 0) + self.assertEqual(len(self.updates), 0)