fix(cli): allow exclusive arguments as optional (#2770)

Sjord · web-flow · commit 7ec3189d6eac · 2024-01-29T08:12:34.000-08:00
* fix(cli): allow exclusive arguments as optional The CLI takes its arguments from the RequiredOptional, which has three fields: required, optional, and exclusive. In practice, the exclusive options are not defined as either required or optional, and would not be allowed in the CLI. This changes that, so that exclusive options are also added to the argument parser. * fix(cli): inform argument parser that options are mutually exclusive * fix(cli): use correct exclusive options, add unit test Closes #2769
diff --git a/gitlab/v4/cli.py b/gitlab/v4/cli.py
@@ -262,6 +262,10 @@ def _populate_sub_parser_by_class(
                 sub_parser_action.add_argument(
                     f"--{x.replace('_', '-')}", required=False
                 )
+            if mgr_cls._create_attrs.exclusive:
+                group = sub_parser_action.add_mutually_exclusive_group()
+                for x in mgr_cls._create_attrs.exclusive:
+                    group.add_argument(f"--{x.replace('_', '-')}")
 
         if action_name == "update":
             if cls._id_attr is not None:
@@ -280,6 +284,11 @@ def _populate_sub_parser_by_class(
                         f"--{x.replace('_', '-')}", required=False
                     )
 
+            if mgr_cls._update_attrs.exclusive:
+                group = sub_parser_action.add_mutually_exclusive_group()
+                for x in mgr_cls._update_attrs.exclusive:
+                    group.add_argument(f"--{x.replace('_', '-')}")
+
     if cls.__name__ in cli.custom_actions:
         name = cls.__name__
         for action_name in cli.custom_actions[name]:
diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py
@@ -11,6 +11,8 @@
 import gitlab.base
 from gitlab import cli
 from gitlab.exceptions import GitlabError
+from gitlab.mixins import CreateMixin, UpdateMixin
+from gitlab.types import RequiredOptional
 from gitlab.v4 import cli as v4_cli
 
 
@@ -157,6 +159,65 @@ def test_v4_parser():
     assert actions["--name"].required
 
 
+def test_extend_parser():
+    class ExceptionArgParser(argparse.ArgumentParser):
+        def error(self, message):
+            "Raise error instead of exiting on invalid arguments, to make testing easier"
+            raise ValueError(message)
+
+    class Fake:
+        _id_attr = None
+
+    class FakeManager(gitlab.base.RESTManager, CreateMixin, UpdateMixin):
+        _obj_cls = Fake
+        _create_attrs = RequiredOptional(
+            required=("create",),
+            optional=("opt_create",),
+            exclusive=("create_a", "create_b"),
+        )
+        _update_attrs = RequiredOptional(
+            required=("update",),
+            optional=("opt_update",),
+            exclusive=("update_a", "update_b"),
+        )
+
+    parser = ExceptionArgParser()
+    with mock.patch.dict(
+        "gitlab.v4.objects.__dict__", {"FakeManager": FakeManager}, clear=True
+    ):
+        v4_cli.extend_parser(parser)
+
+    assert parser.parse_args(["fake", "create", "--create", "1"])
+    assert parser.parse_args(["fake", "create", "--create", "1", "--opt-create", "1"])
+    assert parser.parse_args(["fake", "create", "--create", "1", "--create-a", "1"])
+    assert parser.parse_args(["fake", "create", "--create", "1", "--create-b", "1"])
+
+    with pytest.raises(ValueError):
+        # missing required "create"
+        parser.parse_args(["fake", "create", "--opt_create", "1"])
+
+    with pytest.raises(ValueError):
+        # both exclusive options
+        parser.parse_args(
+            ["fake", "create", "--create", "1", "--create-a", "1", "--create-b", "1"]
+        )
+
+    assert parser.parse_args(["fake", "update", "--update", "1"])
+    assert parser.parse_args(["fake", "update", "--update", "1", "--opt-update", "1"])
+    assert parser.parse_args(["fake", "update", "--update", "1", "--update-a", "1"])
+    assert parser.parse_args(["fake", "update", "--update", "1", "--update-b", "1"])
+
+    with pytest.raises(ValueError):
+        # missing required "update"
+        parser.parse_args(["fake", "update", "--opt_update", "1"])
+
+    with pytest.raises(ValueError):
+        # both exclusive options
+        parser.parse_args(
+            ["fake", "update", "--update", "1", "--update-a", "1", "--update-b", "1"]
+        )
+
+
 @pytest.mark.skipif(sys.version_info < (3, 8), reason="added in 3.8")
 def test_legacy_display_without_fields_warns(fake_object_no_id):
     printer = v4_cli.LegacyPrinter()
