diff --git a/src/hubblenetwork/cli.py b/src/hubblenetwork/cli.py index 340dbd6..5498545 100644 --- a/src/hubblenetwork/cli.py +++ b/src/hubblenetwork/cli.py @@ -2816,7 +2816,46 @@ def log_step(msg: str) -> None: pass_orgcfg = click.make_pass_decorator(Organization, ensure=True) -@cli.group() +class _CredAnywhereGroup(click.Group): + """Group that accepts its credential options anywhere in the arguments. + + Click normally requires group-level options to precede the subcommand + name, so ``org -o ORG -t TOKEN register-device`` works but + ``org register-device ... -o ORG -t TOKEN`` does not. This subclass hoists + the ``--org-id``/``-o`` and ``--token``/``-t`` options (and their values) + to the front of the argument list before parsing, so the user may place + them anywhere. Subcommands therefore must not define their own ``-o``/``-t`` + short options. + """ + + _CRED_OPTS = ("--org-id", "-o", "--token", "-t") + + def parse_args(self, ctx, args): + hoisted: list[str] = [] + rest: list[str] = [] + i = 0 + while i < len(args): + arg = args[i] + if arg == "--": + # Everything after the end-of-options marker is passed through. + rest.extend(args[i:]) + break + if arg in self._CRED_OPTS: + # Space-separated value: take the following token too. + hoisted.append(arg) + if i + 1 < len(args): + i += 1 + hoisted.append(args[i]) + elif arg.startswith(("--org-id=", "--token=")) or (arg[:2] in ("-o", "-t") and len(arg) > 2): + # Attached value, e.g. --org-id=ORG or -oORG. + hoisted.append(arg) + else: + rest.append(arg) + i += 1 + return super().parse_args(ctx, hoisted + rest) + + +@cli.group(cls=_CredAnywhereGroup) @click.option( "--org-id", "-o", @@ -2970,12 +3009,12 @@ def set_device_name(org: Organization, device_id: str, name: str) -> None: @click.argument("device-id", type=str) @click.option( "--format", - "-o", + "-f", "output_format", type=click.Choice(["tabular", "csv", "json"], case_sensitive=False), default="tabular", show_default=True, - help="Output format for packets", + help="Output format for packets (-o is reserved for --org-id)", ) @click.option( "--days", @@ -3002,7 +3041,7 @@ def get_packets( Example: hubblenetwork org get-packets DEVICE_ID - hubblenetwork org get-packets DEVICE_ID -o json + hubblenetwork org get-packets DEVICE_ID -f json hubblenetwork org get-packets DEVICE_ID --format csv --days 30 """ device = Device(id=device_id) diff --git a/tests/test_org_option_position.py b/tests/test_org_option_position.py new file mode 100644 index 0000000..637e87b --- /dev/null +++ b/tests/test_org_option_position.py @@ -0,0 +1,105 @@ +"""Tests that org-level --org-id/-o and --token/-t can appear anywhere. + +Click normally requires group-level options to precede the subcommand name. +The org group uses a custom Group class that hoists the credential options +(and their values) out of the argument list wherever the user placed them, so +e.g. `org register-device -e ... -o ORG -t TOKEN` works. +""" +from unittest.mock import patch, MagicMock + +from click.testing import CliRunner + +from hubblenetwork.org import Organization +from hubblenetwork.cli import cli + + +def _run(args): + runner = CliRunner() + with patch("hubblenetwork.cli.Organization") as mock_org_cls: + mock_org_cls.return_value = MagicMock(spec=Organization) + result = runner.invoke(cli, args) + return result, mock_org_cls + + +class TestCredentialOptionPositioning: + def test_credentials_after_subcommand(self): + """The motivating example: -o/-t trail the subcommand and its options.""" + result, mock_org_cls = _run( + ["org", "register-device", "-e", "AES-128-EAX", + "-c", "DEVICE_UPTIME", "-o", "MY_ORG_ID", "-t", "MY_TOKEN"] + ) + assert result.exit_code == 0, result.output + mock_org_cls.assert_called_once_with(org_id="MY_ORG_ID", api_token="MY_TOKEN") + + def test_credentials_before_subcommand_still_works(self): + """Backward compatibility: the old position must still parse.""" + result, mock_org_cls = _run( + ["org", "-o", "MY_ORG_ID", "-t", "MY_TOKEN", + "register-device", "-e", "AES-128-EAX", "-c", "DEVICE_UPTIME"] + ) + assert result.exit_code == 0, result.output + mock_org_cls.assert_called_once_with(org_id="MY_ORG_ID", api_token="MY_TOKEN") + + def test_credentials_interspersed(self): + """Credentials split around the subcommand and its options.""" + result, mock_org_cls = _run( + ["org", "-o", "MY_ORG_ID", "register-device", + "-e", "AES-128-EAX", "-t", "MY_TOKEN"] + ) + assert result.exit_code == 0, result.output + mock_org_cls.assert_called_once_with(org_id="MY_ORG_ID", api_token="MY_TOKEN") + + def test_long_form_with_equals_after_subcommand(self): + result, mock_org_cls = _run( + ["org", "register-device", "--org-id=MY_ORG_ID", "--token=MY_TOKEN"] + ) + assert result.exit_code == 0, result.output + mock_org_cls.assert_called_once_with(org_id="MY_ORG_ID", api_token="MY_TOKEN") + + def test_long_form_space_separated_after_subcommand(self): + result, mock_org_cls = _run( + ["org", "list-devices", "--org-id", "MY_ORG_ID", "--token", "MY_TOKEN"] + ) + assert result.exit_code == 0, result.output + mock_org_cls.assert_called_once_with(org_id="MY_ORG_ID", api_token="MY_TOKEN") + + +class TestGetPacketsFormatRebind: + """get-packets used to use -o for output format; -o now means org-id.""" + + def test_dash_o_is_org_id_on_get_packets(self): + result, mock_org_cls = _run( + ["org", "get-packets", "dev-abc", "-o", "MY_ORG_ID", "-t", "MY_TOKEN"] + ) + assert result.exit_code == 0, result.output + mock_org_cls.assert_called_once_with(org_id="MY_ORG_ID", api_token="MY_TOKEN") + + def test_dash_f_controls_format(self): + runner = CliRunner() + with patch("hubblenetwork.cli.Organization") as mock_org_cls: + mock_org_cls.return_value = MagicMock(spec=Organization) + mock_org = mock_org_cls.return_value + mock_org.retrieve_packets.return_value = [] + result = runner.invoke( + cli, + ["org", "-o", "MY_ORG_ID", "-t", "MY_TOKEN", + "get-packets", "dev-abc", "-f", "json"], + ) + assert result.exit_code == 0, result.output + assert result.output.strip() == "[]" + + def test_org_id_and_format_together(self): + """-o as org-id and -f as format coexist on get-packets.""" + runner = CliRunner() + with patch("hubblenetwork.cli.Organization") as mock_org_cls: + mock_org_cls.return_value = MagicMock(spec=Organization) + mock_org = mock_org_cls.return_value + mock_org.retrieve_packets.return_value = [] + result = runner.invoke( + cli, + ["org", "get-packets", "dev-abc", + "-o", "MY_ORG_ID", "-t", "MY_TOKEN", "-f", "json"], + ) + assert result.exit_code == 0, result.output + assert result.output.strip() == "[]" + mock_org_cls.assert_called_once_with(org_id="MY_ORG_ID", api_token="MY_TOKEN")