diff --git a/CHANGELOG.md b/CHANGELOG.md index 6986983..9a3272d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.8.1] - 2026-08-11 + +### Fixed + +- `bcli post`, `bcli patch`, and `bcli action` no longer crash with a raw + `json.JSONDecodeError` traceback when `--data`/`-d` fails to parse. All + three commands had their own copy of the same unguarded `json.loads()` + call: a shell stripping the quotes out of an inline JSON literal (routine + in PowerShell — `{"a": 1}` arrives as `{a: 1}`) or a bare file path passed + without the `@` prefix both surfaced a ~25-line Python traceback instead + of an actionable error. The three copies are now one helper + (`bcli_cli._data_arg.parse_data_argument`), and every failure raises + `typer.BadParameter` instead: the message names the JSON error's + line/column/reason plus a short excerpt of what was received (never the + whole payload), and — when the input looks like a filesystem path or a + shell-mangled literal — adds a one-line hint pointing at `-d @file.json`. + A malformed `@file` now names the file in its error; a missing `@file` + still reports `File not found` unchanged. + ## [0.8.0] - 2026-08-10 ### Added diff --git a/docs/command-reference.md b/docs/command-reference.md index dc1b659..8aa1e67 100644 --- a/docs/command-reference.md +++ b/docs/command-reference.md @@ -328,6 +328,14 @@ Create a new record. bcli post --data [--publisher ...] [--group ...] [--version ...] ``` +`--data`/`-d` takes a JSON literal or `@` to read the body from a +file — the `@` is required; a bare path (`--data payload.json`) is parsed +as JSON and rejected, not silently treated as a file. On PowerShell, +inline JSON often arrives with its quotes stripped by the shell before +bcli ever sees it, so prefer `--data @payload.json` over an inline literal +there. A malformed `--data` value fails with a usage error naming the +problem, not a Python traceback. + --- ## patch @@ -338,6 +346,8 @@ Update an existing record. bcli patch --data [--etag ] [--publisher ...] [--group ...] [--version ...] ``` +Same `--data`/`-d` rules as `post` above. + --- ## delete diff --git a/docs/write-operations.md b/docs/write-operations.md index 2d57256..c63b953 100644 --- a/docs/write-operations.md +++ b/docs/write-operations.md @@ -14,6 +14,13 @@ bcli post customers --data @customer.json The `@` prefix reads JSON from a file. The response shows the created record. +The `@` is required — a bare path (`--data customer.json`, no `@`) is parsed +as inline JSON and rejected, it isn't silently treated as a file. If your +shell strips quotes from inline JSON (PowerShell does this routinely), pass +`--data @customer.json` instead of fighting the quoting. Either way, a +malformed `--data` value fails with a usage error naming the problem, not a +Python traceback. + ## PATCH (Update) Update an existing record by ID: diff --git a/pyproject.toml b/pyproject.toml index 33fb0e5..b60a129 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,7 +9,7 @@ build-backend = "hatchling.build" # installed CLI binary (`bcli`) are unaffected — only `pip install` / # `uv tool install` use this name. name = "bc-cli" -version = "0.8.0" +version = "0.8.1" description = "Python SDK and CLI for Microsoft Dynamics 365 Business Central APIs" readme = "README.md" license = "Apache-2.0" diff --git a/src/bcli_cli/_data_arg.py b/src/bcli_cli/_data_arg.py new file mode 100644 index 0000000..1e6593b --- /dev/null +++ b/src/bcli_cli/_data_arg.py @@ -0,0 +1,127 @@ +"""Shared ``--data``/``-d`` parsing for ``post``, ``patch``, and ``action``. + +All three verbs accept a request body as either a literal JSON string or +an ``@filename`` reference. Both had bare ``json.loads()`` calls with +nothing catching a decode failure, so a mangled inline literal (a shell +stripping quotes is the common case) or a bare file path passed without +the ``@`` prefix surfaced as a raw ``json.JSONDecodeError`` traceback +instead of a usage error naming the fix. +""" + +from __future__ import annotations + +import json +import re +from pathlib import Path + +import typer + +_EXCERPT_LIMIT = 80 + +_WINDOWS_DRIVE_RE = re.compile(r"^[A-Za-z]:[\\/]") +_UNQUOTED_KEY_RE = re.compile(r'[{,]\s*[A-Za-z_][\w-]*\s*:') + + +def _excerpt(value: str, limit: int = _EXCERPT_LIMIT) -> str: + """Truncate ``value`` for safe inclusion in an error message.""" + value = value.strip() + if len(value) <= limit: + return value + return value[:limit] + "…" + + +def _looks_like_path(data: str) -> bool: + """True if ``data`` looks like a filesystem path rather than JSON. + + Only called after ``json.loads`` has already failed, so this doesn't + need to be precise — just catch the shapes that mean "this needed an + ``@`` prefix": a real file on disk (the strongest signal), a bare + Windows drive path, or a path with a separator ending in ``.json``. + """ + if data.startswith("{") or data.startswith("["): + return False + # A malformed payload is arbitrary text, not a path: on Linux anything + # over 255 bytes raises ENAMETOOLONG here, and an embedded NUL raises + # ValueError. Letting either escape would put back exactly the raw + # traceback this module exists to prevent — for the *long* payloads that + # are the most likely to be malformed in the first place. + try: + if Path(data).is_file(): + return True + except (OSError, ValueError): + return False + if _WINDOWS_DRIVE_RE.match(data): + return True + if ("/" in data or "\\" in data) and data.lower().endswith(".json"): + return True + return False + + +def _looks_shell_mangled(data: str) -> bool: + """True if ``data`` looks like a shell stripped the quotes out of an + inline JSON literal — e.g. PowerShell turning + ``{"bladeType": "HPT BLADE"}`` into ``{bladeType: HPT BLADE}``. + """ + if not data.startswith("{"): + return False + if '"' not in data: + return True + return bool(_UNQUOTED_KEY_RE.search(data)) + + +def _mangling_hint(data: str) -> str | None: + if _looks_like_path(data): + return ( + f"'{data}' looks like a file path — pass '-d @{data}' " + "to load it from a file." + ) + if _looks_shell_mangled(data): + return ( + "Some shells (e.g. PowerShell) strip quotes from inline JSON — " + "pass the payload as a file instead: -d @payload.json." + ) + return None + + +def parse_data_argument(data: str) -> dict: + """Parse a ``--data``/``-d`` argument: a JSON literal or ``@filename``. + + Raises ``typer.BadParameter`` — never a raw ``json.JSONDecodeError`` — + with a message that names the fix: a missing ``@`` prefix, likely + shell quote-stripping, or (for a malformed file) the file's path. + """ + if data.startswith("@"): + path = Path(data[1:]) + # Same guard as _looks_like_path: an over-long or NUL-bearing name + # raises here rather than returning False, and an unreadable file + # raises on read. Report them as bad input, not as a traceback. + try: + is_file = path.is_file() + except (OSError, ValueError): + is_file = False + if not is_file: + raise typer.BadParameter(f"File not found: {path}") + try: + return json.loads(path.read_text(encoding="utf-8")) + except OSError as e: + raise typer.BadParameter(f"Could not read {path}: {e.strerror or e}") from e + except json.JSONDecodeError as e: + raise typer.BadParameter( + f"{path} is not valid JSON: {e.msg} " + f"(line {e.lineno}, column {e.colno})." + ) from e + + try: + return json.loads(data) + except json.JSONDecodeError as e: + message = ( + f"--data is not valid JSON: {e.msg} " + f"(line {e.lineno}, column {e.colno}). Received: {_excerpt(data)!r}" + ) + hint = _mangling_hint(data) + if hint: + message += "\n " + hint + raise typer.BadParameter(message) from e + + +__all__ = ["parse_data_argument"] diff --git a/src/bcli_cli/commands/action_cmd.py b/src/bcli_cli/commands/action_cmd.py index 33be037..c156f8a 100644 --- a/src/bcli_cli/commands/action_cmd.py +++ b/src/bcli_cli/commands/action_cmd.py @@ -27,7 +27,6 @@ from __future__ import annotations import asyncio -import json from pathlib import Path from typing import Optional @@ -35,6 +34,7 @@ from rich.console import Console from bcli.errors import BCLIError +from bcli_cli._data_arg import parse_data_argument from bcli_cli._envelope_wrap import capture, validate_flags from bcli_cli._out_path import atomic_write_bytes, prepare_out_path from bcli_cli._state import state @@ -142,7 +142,7 @@ def action_command( "Pass one or the other (or neither — empty body is the default).", ) - body: dict = {} if data is None else _parse_data(data) + body: dict = {} if data is None else parse_data_argument(data) ns = namespace or DEFAULT_NAMESPACE # Compose the synthetic bound-action string. The registry validator @@ -291,13 +291,3 @@ def _write_decoded_payload(result: dict | str, dest: Path, *, overwrite: bool) - raw = _decode_base64_payload(result) atomic_write_bytes(dest, raw, overwrite=overwrite) return len(raw) - - -def _parse_data(data: str) -> dict: - """Parse --data argument: JSON string or @filename.""" - if data.startswith("@"): - path = Path(data[1:]) - if not path.is_file(): - raise typer.BadParameter(f"File not found: {path}") - return json.loads(path.read_text(encoding="utf-8")) - return json.loads(data) diff --git a/src/bcli_cli/commands/patch_cmd.py b/src/bcli_cli/commands/patch_cmd.py index 31339c3..c8ef0e2 100644 --- a/src/bcli_cli/commands/patch_cmd.py +++ b/src/bcli_cli/commands/patch_cmd.py @@ -3,13 +3,13 @@ from __future__ import annotations import asyncio -import json from pathlib import Path from typing import Optional import typer from rich.console import Console +from bcli_cli._data_arg import parse_data_argument from bcli_cli._envelope_wrap import capture, validate_flags from bcli_cli._state import state from bcli_cli.output import format_output, print_context_banner @@ -53,7 +53,7 @@ def patch_command( print_context_banner() - body = _parse_data(data) + body = parse_data_argument(data) with capture( method="PATCH", @@ -121,12 +121,3 @@ async def _audited_patch(endpoint, record_id, body, **kwargs): async def _execute_patch(endpoint, record_id, body, **kwargs): async with state.make_async_client() as client: return await client.patch(endpoint, record_id, body, **kwargs) - - -def _parse_data(data: str) -> dict: - if data.startswith("@"): - path = Path(data[1:]) - if not path.is_file(): - raise typer.BadParameter(f"File not found: {path}") - return json.loads(path.read_text(encoding="utf-8")) - return json.loads(data) diff --git a/src/bcli_cli/commands/post_cmd.py b/src/bcli_cli/commands/post_cmd.py index 7901020..6fae9b2 100644 --- a/src/bcli_cli/commands/post_cmd.py +++ b/src/bcli_cli/commands/post_cmd.py @@ -3,13 +3,13 @@ from __future__ import annotations import asyncio -import json from pathlib import Path from typing import Optional import typer from rich.console import Console +from bcli_cli._data_arg import parse_data_argument from bcli_cli._envelope_wrap import capture, validate_flags from bcli_cli._state import state from bcli_cli.output import format_output, print_context_banner @@ -51,7 +51,7 @@ def post_command( print_context_banner() - body = _parse_data(data) + body = parse_data_argument(data) with capture( method="POST", @@ -119,13 +119,3 @@ async def _execute_post(endpoint, body, **kwargs): async with state.make_async_client() as client: # ``client.post`` accepts ``idempotency_key``; passthrough. return await client.post(endpoint, body, **kwargs) - - -def _parse_data(data: str) -> dict: - """Parse --data argument: JSON string or @filename.""" - if data.startswith("@"): - path = Path(data[1:]) - if not path.is_file(): - raise typer.BadParameter(f"File not found: {path}") - return json.loads(path.read_text(encoding="utf-8")) - return json.loads(data) diff --git a/tests/test_cli/test_action_cmd.py b/tests/test_cli/test_action_cmd.py index b01f691..fea7196 100644 --- a/tests/test_cli/test_action_cmd.py +++ b/tests/test_cli/test_action_cmd.py @@ -162,6 +162,37 @@ def test_data_and_no_data_mutually_exclusive(self, cli_state, fake_client): with pytest.raises(typer.BadParameter): _run(data='{"x": 1}', no_data=True) + def test_malformed_inline_json_raises_bad_parameter_not_traceback( + self, cli_state, fake_client, + ): + """Regression: a mangled inline literal used to surface a raw + json.JSONDecodeError. It must come back as a usage error, and + the call must never reach the network.""" + with pytest.raises(typer.BadParameter): + _run(data="{not valid json") + fake_client.post.assert_not_called() + + def test_bare_file_path_without_at_prefix_hints_the_fix( + self, cli_state, fake_client, tmp_path: Path, + ): + f = tmp_path / "payload.json" + f.write_text('{"x": 1}', encoding="utf-8") + with pytest.raises(typer.BadParameter) as exc_info: + _run(data=str(f)) + assert "looks like a file path" in str(exc_info.value) + assert f"-d @{f}" in str(exc_info.value) + fake_client.post.assert_not_called() + + def test_malformed_at_file_names_the_file( + self, cli_state, fake_client, tmp_path: Path, + ): + f = tmp_path / "bad.json" + f.write_text("{not valid json", encoding="utf-8") + with pytest.raises(typer.BadParameter) as exc_info: + _run(data=f"@{f}") + assert str(f) in str(exc_info.value) + fake_client.post.assert_not_called() + class TestProfileOverrides: def test_publisher_group_version_forwarded(self, cli_state, fake_client): diff --git a/tests/test_cli/test_data_arg.py b/tests/test_cli/test_data_arg.py new file mode 100644 index 0000000..b9bfd3e --- /dev/null +++ b/tests/test_cli/test_data_arg.py @@ -0,0 +1,196 @@ +"""Tests for ``bcli_cli._data_arg`` — the shared ``--data``/``-d`` parser +used by ``post``, ``patch``, and ``action``. + +Before this module existed, each command had its own unguarded +``json.loads()`` call: a malformed inline literal or a bare file path +passed without ``@`` surfaced as a raw ``json.JSONDecodeError`` +traceback. Every case here must raise ``typer.BadParameter`` instead. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest +import typer + +from bcli_cli._data_arg import parse_data_argument + + +class TestValidInput: + def test_valid_inline_json(self): + assert parse_data_argument('{"a": 1, "b": "two"}') == {"a": 1, "b": "two"} + + def test_valid_file(self, tmp_path: Path): + f = tmp_path / "payload.json" + f.write_text('{"loaded": "from-file"}', encoding="utf-8") + assert parse_data_argument(f"@{f}") == {"loaded": "from-file"} + + +class TestMissingFile: + def test_missing_at_file_names_the_path(self): + with pytest.raises(typer.BadParameter) as exc_info: + parse_data_argument("@/no/such/file.json") + assert "File not found" in str(exc_info.value) + assert "/no/such/file.json" in str(exc_info.value) + + +class TestMalformedInlineJson: + def test_raises_bad_parameter_not_json_decode_error(self): + """The core bug: a decode failure must never escape as a raw + json.JSONDecodeError — it has to become a usage error.""" + with pytest.raises(typer.BadParameter): + parse_data_argument("{not valid json") + # And specifically NOT a bare JSONDecodeError bubbling past us. + try: + parse_data_argument("{not valid json") + except typer.BadParameter: + pass + except json.JSONDecodeError: + pytest.fail("json.JSONDecodeError leaked instead of BadParameter") + + def test_message_includes_line_col_and_reason(self): + with pytest.raises(typer.BadParameter) as exc_info: + parse_data_argument("{not valid json") + msg = str(exc_info.value) + assert "line 1" in msg + assert "column" in msg + + def test_message_includes_truncated_excerpt(self): + long_garbage = "x" * 300 + with pytest.raises(typer.BadParameter) as exc_info: + parse_data_argument(long_garbage) + msg = str(exc_info.value) + # Never dump the whole payload. + assert long_garbage not in msg + assert "…" in msg + + def test_generic_garbage_gets_no_spurious_hint(self): + """Plain nonsense that isn't path-like or shell-mangled should + get the base message only — no misleading hint appended.""" + with pytest.raises(typer.BadParameter) as exc_info: + parse_data_argument("definitely not json") + msg = str(exc_info.value) + assert "@" not in msg + assert "shell" not in msg.lower() + + +class TestBareFilePathTrap: + """The exact trap from the bug report: a user passes a real file + path to --data without the @ prefix.""" + + def test_existing_file_path_suggests_at_form(self, tmp_path: Path): + f = tmp_path / "payload.json" + f.write_text('{"x": 1}', encoding="utf-8") + with pytest.raises(typer.BadParameter) as exc_info: + parse_data_argument(str(f)) + msg = str(exc_info.value) + assert "looks like a file path" in msg + assert f"-d @{f}" in msg + + def test_windows_drive_path_suggests_at_form_even_if_missing(self): + # Backslash path with a drive letter — classic Windows/PowerShell + # shape. Doesn't exist on this (or any) filesystem, but the shape + # alone is a strong enough signal to hint. + windows_path = r"C:\Users\test\payload.json" + with pytest.raises(typer.BadParameter) as exc_info: + parse_data_argument(windows_path) + msg = str(exc_info.value) + assert "looks like a file path" in msg + assert f"-d @{windows_path}" in msg + + def test_nonexistent_but_path_shaped_string_still_hints(self): + # Has a separator and a .json suffix but doesn't exist — weaker + # signal than an existing file, but still worth a hint. + path_like = "some/dir/payload.json" + with pytest.raises(typer.BadParameter) as exc_info: + parse_data_argument(path_like) + msg = str(exc_info.value) + assert "looks like a file path" in msg + + def test_bare_word_without_json_suffix_gets_no_path_hint(self): + # No separator, no .json suffix, not a real file — nothing here + # actually looks like a path, so no path hint should fire. + with pytest.raises(typer.BadParameter) as exc_info: + parse_data_argument("archive") + msg = str(exc_info.value) + assert "looks like a file path" not in msg + + +class TestShellManglingTrap: + """PowerShell (and other shells) can strip the quotes out of an + inline JSON literal before bcli ever sees it.""" + + def test_no_quotes_at_all_hints_shell_mangling(self): + with pytest.raises(typer.BadParameter) as exc_info: + parse_data_argument("{bladeType: HPT BLADE}") + msg = str(exc_info.value) + assert "shell" in msg.lower() + assert "-d @payload.json" in msg + + def test_unquoted_keys_with_some_quotes_hints_shell_mangling(self): + with pytest.raises(typer.BadParameter) as exc_info: + parse_data_argument('{bladeType: "HPT BLADE", qty: 5}') + msg = str(exc_info.value) + assert "shell" in msg.lower() + + def test_hint_is_a_single_sentence(self): + with pytest.raises(typer.BadParameter) as exc_info: + parse_data_argument("{bladeType: HPT BLADE}") + hint_line = str(exc_info.value).splitlines()[-1].strip() + # One sentence: exactly one terminal period, not a lecture. (The + # "e.g." abbreviation has its own internal periods, so strip it + # before counting sentence-ending ". ".) + assert hint_line.replace("e.g.", "eg").count(". ") == 0 + assert hint_line.endswith(".") + + +class TestMalformedFile: + def test_malformed_at_file_names_the_file_path(self, tmp_path: Path): + f = tmp_path / "bad.json" + f.write_text("{not valid json", encoding="utf-8") + with pytest.raises(typer.BadParameter) as exc_info: + parse_data_argument(f"@{f}") + msg = str(exc_info.value) + assert str(f) in msg + assert "line 1" in msg + + def test_malformed_at_file_not_a_json_decode_error(self, tmp_path: Path): + f = tmp_path / "bad.json" + f.write_text("{not valid json", encoding="utf-8") + try: + parse_data_argument(f"@{f}") + except typer.BadParameter: + pass + except json.JSONDecodeError: + pytest.fail("json.JSONDecodeError leaked instead of BadParameter") + + +class TestPathProbeIsNeverFatal: + """The path heuristic must not let filesystem errors escape. + + Regression: `Path(data).is_file()` raises OSError(ENAMETOOLONG) on Linux + for any argument over 255 bytes, and ValueError for an embedded NUL — so + a long malformed payload (exactly the kind most likely to be malformed) + produced a raw traceback, the very thing this module prevents. macOS does + not raise, so this only reproduces on Linux/CI. + """ + + @pytest.mark.parametrize( + "payload", + [ + "x" * 300, # ENAMETOOLONG on Linux + "{" + "a" * 300, # long *and* JSON-ish + "/tmp/" + "y" * 300 + ".json", # long, path-shaped + "bad\x00json", # embedded NUL -> ValueError + ], + ids=["long", "long-brace", "long-path-shaped", "nul-byte"], + ) + def test_long_or_nul_payload_still_raises_bad_parameter(self, payload): + with pytest.raises(typer.BadParameter): + parse_data_argument(payload) + + def test_long_at_path_reports_not_found_not_oserror(self): + with pytest.raises(typer.BadParameter, match="File not found"): + parse_data_argument("@" + "z" * 300) diff --git a/tests/test_cli/test_patch_cmd.py b/tests/test_cli/test_patch_cmd.py new file mode 100644 index 0000000..bbb47fa --- /dev/null +++ b/tests/test_cli/test_patch_cmd.py @@ -0,0 +1,145 @@ +"""Tests for the ``bcli patch`` verb, focused on ``--data``/``-d`` handling. + +``patch`` was the other command whose bare ``json.loads()`` call on +``--data`` surfaced a raw traceback on malformed input; see +``bcli_cli._data_arg`` for the fix and ``test_data_arg.py`` for the full +heuristic matrix. These tests confirm the command wires the shared +helper in correctly — bad ``--data`` must fail before any network call. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from unittest.mock import AsyncMock + +import pytest +import typer + +from bcli.config._model import BCConfig, BCDefaults, BCProfile +from bcli_cli._state import state +from bcli_cli.commands import patch_cmd + + +@pytest.fixture +def cli_state(): + cfg = BCConfig( + defaults=BCDefaults(profile="dev"), + profiles={ + "dev": BCProfile( + tenant_id="t1", + environment="Sandbox", + company_id="c-123", + disable_writes=False, + ), + }, + ) + state._config = cfg + state._registry = None + state.profile_name = None + state.env_override = None + state.company_override = None + state.format = "table" + state.dry_run = False + state.quiet = True + yield state + state._config = None + state._registry = None + state.profile_name = None + state.dry_run = False + + +@pytest.fixture +def fake_client(monkeypatch): + c = AsyncMock() + c.__aenter__ = AsyncMock(return_value=c) + c.__aexit__ = AsyncMock(return_value=False) + c.patch = AsyncMock(return_value={"result": "ok"}) + c._resolve_url = lambda entity, **kw: f"https://example.test/{entity}" + monkeypatch.setattr(state, "make_async_client", lambda **_: c) + return c + + +@pytest.fixture(autouse=True) +def non_interactive(monkeypatch): + import sys + monkeypatch.setattr(sys.stdin, "isatty", lambda: False) + + +def _run( + *, + endpoint="vendors", + record_id="vnd-1", + data='{"displayName": "Renamed"}', + etag="*", + yes=True, + **kwargs, +): + kwargs.setdefault("format", None) + kwargs.setdefault("publisher", None) + kwargs.setdefault("group", None) + kwargs.setdefault("version", None) + kwargs.setdefault("result_out", None) + kwargs.setdefault("result_fd", None) + kwargs.setdefault("idempotency_key", None) + return patch_cmd.patch_command( + endpoint=endpoint, record_id=record_id, data=data, etag=etag, yes=yes, **kwargs, + ) + + +class TestValidData: + def test_inline_json_literal_parsed(self, cli_state, fake_client): + _run(data='{"a": 1, "b": "two"}') + body_arg = fake_client.patch.await_args.args[2] + assert body_arg == {"a": 1, "b": "two"} + + def test_data_from_file(self, cli_state, fake_client, tmp_path: Path): + f = tmp_path / "payload.json" + f.write_text('{"loaded": "from-file"}', encoding="utf-8") + _run(data=f"@{f}") + body_arg = fake_client.patch.await_args.args[2] + assert body_arg == {"loaded": "from-file"} + + +class TestMalformedData: + def test_malformed_inline_json_raises_bad_parameter(self, cli_state, fake_client): + with pytest.raises(typer.BadParameter): + _run(data="{not valid json") + fake_client.patch.assert_not_called() + + def test_bare_file_path_without_at_prefix_hints_the_fix( + self, cli_state, fake_client, tmp_path: Path, + ): + f = tmp_path / "payload.json" + f.write_text('{"x": 1}', encoding="utf-8") + with pytest.raises(typer.BadParameter) as exc_info: + _run(data=str(f)) + assert "looks like a file path" in str(exc_info.value) + assert f"-d @{f}" in str(exc_info.value) + fake_client.patch.assert_not_called() + + def test_missing_at_file_keeps_file_not_found(self, cli_state, fake_client): + with pytest.raises(typer.BadParameter) as exc_info: + _run(data="@/no/such/file.json") + assert "File not found" in str(exc_info.value) + fake_client.patch.assert_not_called() + + def test_malformed_at_file_names_the_file( + self, cli_state, fake_client, tmp_path: Path, + ): + f = tmp_path / "bad.json" + f.write_text("{not valid json", encoding="utf-8") + with pytest.raises(typer.BadParameter) as exc_info: + _run(data=f"@{f}") + assert str(f) in str(exc_info.value) + fake_client.patch.assert_not_called() + + +class TestEnvelope: + def test_envelope_written_on_success(self, cli_state, fake_client, tmp_path: Path): + out = tmp_path / "env.json" + _run(result_out=out) + env = json.loads(out.read_text()) + assert env["status"] == "succeeded" + assert env["method"] == "PATCH" + assert env["endpoint"] == "vendors" diff --git a/tests/test_cli/test_post_cmd.py b/tests/test_cli/test_post_cmd.py new file mode 100644 index 0000000..33f4421 --- /dev/null +++ b/tests/test_cli/test_post_cmd.py @@ -0,0 +1,136 @@ +"""Tests for the ``bcli post`` verb, focused on ``--data``/``-d`` handling. + +``post`` was one of two commands whose bare ``json.loads()`` call on +``--data`` surfaced a raw traceback on malformed input; see +``bcli_cli._data_arg`` for the fix and ``test_data_arg.py`` for the full +heuristic matrix. These tests confirm the command wires the shared +helper in correctly — bad ``--data`` must fail before any network call. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from unittest.mock import AsyncMock + +import pytest +import typer + +from bcli.config._model import BCConfig, BCDefaults, BCProfile +from bcli_cli._state import state +from bcli_cli.commands import post_cmd + + +@pytest.fixture +def cli_state(): + cfg = BCConfig( + defaults=BCDefaults(profile="dev"), + profiles={ + "dev": BCProfile( + tenant_id="t1", + environment="Sandbox", + company_id="c-123", + disable_writes=False, + ), + }, + ) + state._config = cfg + state._registry = None + state.profile_name = None + state.env_override = None + state.company_override = None + state.format = "table" + state.dry_run = False + state.quiet = True + yield state + state._config = None + state._registry = None + state.profile_name = None + state.dry_run = False + + +@pytest.fixture +def fake_client(monkeypatch): + c = AsyncMock() + c.__aenter__ = AsyncMock(return_value=c) + c.__aexit__ = AsyncMock(return_value=False) + c.post = AsyncMock(return_value={"result": "ok"}) + c._resolve_url = lambda entity, **kw: f"https://example.test/{entity}" + monkeypatch.setattr(state, "make_async_client", lambda **_: c) + return c + + +@pytest.fixture(autouse=True) +def non_interactive(monkeypatch): + import sys + monkeypatch.setattr(sys.stdin, "isatty", lambda: False) + + +def _run(*, endpoint="vendors", data='{"displayName": "Acme"}', yes=True, **kwargs): + kwargs.setdefault("format", None) + kwargs.setdefault("publisher", None) + kwargs.setdefault("group", None) + kwargs.setdefault("version", None) + kwargs.setdefault("result_out", None) + kwargs.setdefault("result_fd", None) + kwargs.setdefault("idempotency_key", None) + return post_cmd.post_command(endpoint=endpoint, data=data, yes=yes, **kwargs) + + +class TestValidData: + def test_inline_json_literal_parsed(self, cli_state, fake_client): + _run(data='{"a": 1, "b": "two"}') + body_arg = fake_client.post.await_args.args[1] + assert body_arg == {"a": 1, "b": "two"} + + def test_data_from_file(self, cli_state, fake_client, tmp_path: Path): + f = tmp_path / "payload.json" + f.write_text('{"loaded": "from-file"}', encoding="utf-8") + _run(data=f"@{f}") + body_arg = fake_client.post.await_args.args[1] + assert body_arg == {"loaded": "from-file"} + + +class TestMalformedData: + def test_malformed_inline_json_raises_bad_parameter(self, cli_state, fake_client): + with pytest.raises(typer.BadParameter): + _run(data="{not valid json") + # Must fail before any network call goes out. + fake_client.post.assert_not_called() + + def test_bare_file_path_without_at_prefix_hints_the_fix( + self, cli_state, fake_client, tmp_path: Path, + ): + f = tmp_path / "payload.json" + f.write_text('{"x": 1}', encoding="utf-8") + with pytest.raises(typer.BadParameter) as exc_info: + _run(data=str(f)) + assert "looks like a file path" in str(exc_info.value) + assert f"-d @{f}" in str(exc_info.value) + fake_client.post.assert_not_called() + + def test_missing_at_file_keeps_file_not_found(self, cli_state, fake_client): + with pytest.raises(typer.BadParameter) as exc_info: + _run(data="@/no/such/file.json") + assert "File not found" in str(exc_info.value) + fake_client.post.assert_not_called() + + def test_malformed_at_file_names_the_file( + self, cli_state, fake_client, tmp_path: Path, + ): + f = tmp_path / "bad.json" + f.write_text("{not valid json", encoding="utf-8") + with pytest.raises(typer.BadParameter) as exc_info: + _run(data=f"@{f}") + assert str(f) in str(exc_info.value) + fake_client.post.assert_not_called() + + +class TestEnvelope: + def test_envelope_written_on_success(self, cli_state, fake_client, tmp_path: Path): + out = tmp_path / "env.json" + _run(result_out=out) + env = json.loads(out.read_text()) + assert env["status"] == "succeeded" + assert env["method"] == "POST" + assert env["endpoint"] == "vendors" diff --git a/uv.lock b/uv.lock index b57d4a2..82086d7 100644 --- a/uv.lock +++ b/uv.lock @@ -321,7 +321,7 @@ wheels = [ [[package]] name = "bc-cli" -version = "0.8.0" +version = "0.8.1" source = { editable = "." } dependencies = [ { name = "httpx" },