diff --git a/src/dualentry_cli/client.py b/src/dualentry_cli/client.py index 280f9c9..5aa99b2 100644 --- a/src/dualentry_cli/client.py +++ b/src/dualentry_cli/client.py @@ -19,7 +19,8 @@ # way every time, so retrying only delays the error the user needs to see. _RETRYABLE_EXCEPTIONS = (httpx.TimeoutException, httpx.NetworkError, httpx.RemoteProtocolError) _RETRY_DELAYS = [1, 2, 4] # Exponential backoff: 1s, 2s, 4s -_MAX_RETRIES = len(_RETRY_DELAYS) +# One initial request plus one retry per backoff delay. +_MAX_ATTEMPTS = len(_RETRY_DELAYS) + 1 # The API replays the original response for a repeated Idempotency-Key instead of # running the operation again, so a retried write cannot create a duplicate record. @@ -196,7 +197,7 @@ def _request(self, method: str, path: str, **kwargs) -> dict: # every retry waits, including the one after the loop delay = retry_after if retry_after is not None else backoff - print(f"\033[33mRetrying in {delay}s... (attempt {attempt + 2}/{_MAX_RETRIES + 1})\033[0m", file=sys.stderr) + print(f"\033[33mRetrying in {delay}s... (attempt {attempt + 2}/{_MAX_ATTEMPTS})\033[0m", file=sys.stderr) time.sleep(delay) # Final attempt diff --git a/src/dualentry_cli/commands/__init__.py b/src/dualentry_cli/commands/__init__.py index 41f6db1..f429e99 100644 --- a/src/dualentry_cli/commands/__init__.py +++ b/src/dualentry_cli/commands/__init__.py @@ -121,6 +121,7 @@ def make_resource_app( resource: str, path: str, *, + has_get: bool = True, has_create: bool = True, has_update: bool = True, has_delete: bool = False, @@ -180,7 +181,7 @@ def list_cmd( sig = inspect.signature(list_cmd) list_cmd.__signature__ = sig.replace(parameters=[p for p in sig.parameters.values() if p.name not in remove]) - if has_number: + if has_get and has_number: @app.command("get") def get_cmd_auto( @@ -234,7 +235,8 @@ def get_cmd_by_id( format_output(data, resource=resource, fmt=output) get_cmd_by_id.__doc__ = f"Get a {resource} by ID." - else: + + elif has_get: @app.command("get") def get_cmd( diff --git a/src/dualentry_cli/main.py b/src/dualentry_cli/main.py index f50302f..30d6dae 100644 --- a/src/dualentry_cli/main.py +++ b/src/dualentry_cli/main.py @@ -53,13 +53,13 @@ app.add_typer(make_resource_app("journal entries", "journal-entry", "journal-entries", has_number=True), name="journal-entries") app.add_typer(make_resource_app("bank transfers", "bank-transfer", "bank-transfers", has_number=True), name="bank-transfers") app.add_typer(make_resource_app("fixed assets", "fixed-asset", "fixed-assets", has_number=True), name="fixed-assets") -app.add_typer(make_resource_app("depreciation books", "depreciation-book", "depreciation-books"), name="depreciation-books") +app.add_typer(make_resource_app("depreciation books", "depreciation-book", "depreciation-books", has_create=False, has_update=False), name="depreciation-books") # Entities app.add_typer(make_resource_app("customers", "customer", "customers"), name="customers") app.add_typer(make_resource_app("vendors", "vendor", "vendors"), name="vendors") app.add_typer(make_resource_app("items", "item", "items"), name="items") -app.add_typer(make_resource_app("companies", "company", "companies"), name="companies") +app.add_typer(make_resource_app("companies", "company", "companies", has_create=False, has_update=False), name="companies") app.add_typer(make_resource_app("classifications", "classification", "classifications"), name="classifications") # Recurring @@ -71,7 +71,7 @@ # Other app.add_typer(make_resource_app("contracts", "contract", "contracts"), name="contracts") -app.add_typer(make_resource_app("budgets", "budget", "budgets"), name="budgets") +app.add_typer(make_resource_app("budgets", "budget", "budgets", has_create=False, has_update=False), name="budgets") app.add_typer(make_resource_app("workflows", "workflow", "workflows", has_create=False, has_update=False), name="workflows") app.add_typer( make_resource_app( @@ -87,8 +87,8 @@ ), name="intercompany-journal-entries", ) -app.add_typer(make_resource_app("paper checks", "paper-check", "paper-checks", has_number=True), name="paper-checks") -app.add_typer(make_resource_app("inbox items", "inbox-item", "inbox", has_create=False, has_update=False), name="inbox") +app.add_typer(make_resource_app("paper checks", "paper-check", "paper-checks", has_create=False, has_update=False), name="paper-checks") +app.add_typer(make_resource_app("inbox items", "inbox-item", "inbox", has_get=False, has_create=False, has_update=False), name="inbox") def version_callback(value: bool): diff --git a/tests/test_client.py b/tests/test_client.py index 18974a7..5ffbc78 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -5,6 +5,10 @@ import pytest import respx +# Retry-After values int() cannot use; every status that follows the header +# must fall back to the exponential backoff instead. +_UNREADABLE_RETRY_AFTER = ["next tuesday", "inf", "Infinity", "1e9", "2.5", "-5", ""] + class TestDualEntryClient: def test_sets_api_key_header(self): @@ -200,9 +204,7 @@ def test_every_retry_attempt_carries_the_key(self): with pytest.raises(APIError): self._client(retry=True).post("/invoices/", json={"customer_id": 1}) - # 4, not 3: the loop runs _MAX_RETRIES times and then issues one more - # request after it. That off-by-one is tracked separately; it is harmless - # here precisely because every attempt replays the same key. + # 4 = _MAX_ATTEMPTS: the initial request plus one retry per backoff delay. assert route.call_count == 4 keys = {c.request.headers["Idempotency-Key"] for c in route.calls} assert len(keys) == 1, f"every attempt must reuse one key, got {keys}" @@ -238,7 +240,8 @@ def test_key_is_sent_even_when_retry_is_disabled(self): class TestRetryAfterAndConflicts: """ - Retry timing follows the server, and the two meanings of 409 are separated. + Retry timing follows the server (capped at _MAX_RETRY_AFTER), and the two + meanings of 409 are separated. https://docs.dualentry.com/developers/guides/rate-limiting https://docs.dualentry.com/developers/guides/idempotency-and-write-validation @@ -290,7 +293,8 @@ def test_conflict_without_retry_after_is_not_retried(self, sleeps): assert route.call_count == 1 assert sleeps == [] assert exc.value.status_code == 409 - assert "256 KB" in exc.value.detail + assert "256 KB" in exc.value.detail, "the write carried a key, so the replay guidance applies" + assert "original response cannot be replayed" in exc.value.detail, "and the server's own words survive" @respx.mock def test_rate_limit_waits_for_retry_after_not_the_hardcoded_backoff(self, sleeps): @@ -331,7 +335,7 @@ def test_the_last_attempt_also_waits_for_retry_after(self, sleeps): assert sleeps == [3, 3, 3], "the request after the loop must wait too" assert route.call_count == len(sleeps) + 1 - @pytest.mark.parametrize("bad_value", ["next tuesday", "inf", "Infinity", "1e9", "2.5", "-5", ""]) + @pytest.mark.parametrize("bad_value", _UNREADABLE_RETRY_AFTER) @respx.mock def test_unparsable_retry_after_falls_back_to_backoff(self, sleeps, bad_value): """Values int() cannot use fall back to the backoff; "inf" must never reach time.sleep().""" @@ -409,23 +413,7 @@ def test_storage_unavailable_is_retried_with_the_same_key(self): keys = {c.request.headers["Idempotency-Key"] for c in route.calls} assert len(keys) == 1 - -class TestRetryAfterCeilingAndConflictDetail: - BASE = "https://api.dualentry.com/public/v2" - - @pytest.fixture - def sleeps(self, monkeypatch): - recorded = [] - monkeypatch.setattr("dualentry_cli.client.time", SimpleNamespace(sleep=recorded.append)) - return recorded - - @staticmethod - def _client(): - from dualentry_cli.client import DualEntryClient - - return DualEntryClient(api_url="https://api.dualentry.com", api_key="test_key", retry=True) - - @pytest.mark.parametrize("unreadable", ["soon", "2.5", "-5", ""]) + @pytest.mark.parametrize("unreadable", _UNREADABLE_RETRY_AFTER) @respx.mock def test_conflict_with_unreadable_retry_after_still_retries(self, sleeps, unreadable): route = respx.post(f"{self.BASE}/invoices/").mock( @@ -485,19 +473,7 @@ def test_conflict_without_a_key_keeps_the_server_message(self, sleeps): assert "256 KB" not in exc.value.detail assert sleeps == [] - @respx.mock - def test_conflict_on_a_write_keeps_both_the_guidance_and_the_server_message(self): - from dualentry_cli.client import APIError - - respx.post(f"{self.BASE}/invoices/").mock(return_value=httpx.Response(409, json={"errors": {"__all__": ["response cannot be replayed"]}})) - - with pytest.raises(APIError) as exc: - self._client().post("/invoices/", json={}) - - assert "256 KB" in exc.value.detail, "the write did carry a key, so the guidance applies" - assert "response cannot be replayed" in exc.value.detail, "and the server's own words survive" - - def test_backoff_table_and_retry_count_cannot_drift(self): - from dualentry_cli.client import _MAX_RETRIES, _RETRY_DELAYS + def test_backoff_table_and_attempt_count_cannot_drift(self): + from dualentry_cli.client import _MAX_ATTEMPTS, _RETRY_DELAYS - assert len(_RETRY_DELAYS) == _MAX_RETRIES + assert len(_RETRY_DELAYS) + 1 == _MAX_ATTEMPTS diff --git a/tests/test_stale_commands.py b/tests/test_stale_commands.py new file mode 100644 index 0000000..5ccbb2f --- /dev/null +++ b/tests/test_stale_commands.py @@ -0,0 +1,46 @@ +"""Commands for operations public API v2 does not expose must stay unregistered.""" + +from __future__ import annotations + +import pytest + +from dualentry_cli.main import app + +STALE_COMMANDS = [ + ("companies", "create"), + ("companies", "update"), + ("budgets", "create"), + ("budgets", "update"), + ("depreciation-books", "create"), + ("depreciation-books", "update"), + ("paper-checks", "create"), + ("paper-checks", "update"), + ("inbox", "get"), +] + + +def _commands(resource: str) -> set[str]: + group = next(g for g in app.registered_groups if g.name == resource) + return {c.name for c in group.typer_instance.registered_commands} + + +@pytest.mark.parametrize(("resource", "command"), STALE_COMMANDS) +def test_stale_command_is_not_registered(resource: str, command: str): + assert command not in _commands(resource), f"'dualentry {resource} {command}' has no v2 route and must not be registered" + + +@pytest.mark.parametrize("resource", ["companies", "budgets", "depreciation-books", "paper-checks"]) +def test_read_only_resource_keeps_its_read_commands(resource: str): + assert _commands(resource) == {"list", "get"} + + +def test_inbox_keeps_only_list(): + assert _commands("inbox") == {"list"} + + +def test_paper_checks_has_no_number_lookups(): + assert not _commands("paper-checks") & {"get-number", "get-id"} + + +def test_writable_resource_is_untouched(): + assert {"create", "update"} <= _commands("invoices")