From cfafb31541665607f85fc7077a00be40957780ae Mon Sep 17 00:00:00 2001
From: Jana Cupova <root@localhost>
Date: Mar 25 2024 13:07:27 +0000
Subject: Fix remove-tag-inheritance with priority


Fix remote-tag-inheritance with priority

Fixes: https://pagure.io/koji/issue/3985

---

diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py
index a7db87d..f0c9435 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] <tag> <parent> <priority>"
+    usage = "usage: %prog edit-tag-inheritance [options] <tag> <parent> [<priority>]"
     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 <tag> <parent> <priority>"
+    usage = "usage: %prog remove-tag-inheritance <tag> <parent> [<priority>]"
     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 fe31adb..87eec6e 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] <tag> <parent> <priority>
+        self.error_format = """Usage: %s edit-tag-inheritance [options] <tag> <parent> [<priority>]
 (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] <tag> <parent> <priority>
+            """Usage: %s edit-tag-inheritance [options] <tag> <parent> [<priority>]
 (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 f4b88f5..dd02d26 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 <tag> <parent> <priority>
+        self.error_format = """Usage: %s remove-tag-inheritance <tag> <parent> [<priority>]
 (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 <tag> <parent> <priority>
+            """Usage: %s remove-tag-inheritance <tag> <parent> [<priority>]
 (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 0000000..f5b3de1
--- /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)

