From ef762ea8e474cc45bc92c9df2cc1897054b94ca2 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Sun, 28 Jun 2026 23:11:57 +0000 Subject: [PATCH] gui: make consumers_delete guards DB-independent + add rendered-HTML tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Builds on the consumers_info coroutine fix: - consumers_delete: acquire the DB pool only when actually writing the audit (after the CSRF / archive-guard / NATS-unavailable early exits) and use a local `get_js` import. Previously `pool = get_pool()` ran at the top, so the CSRF-reject, archive-refuse and NATS-down paths all needed an initialized DB pool, and the module-level get_js bound at import time ignored test patches of central.gui.nats.get_js. Mirrors the local-import pattern the streams routes already use. - tests: add TestConsumersListHtmlRender — renders consumers_list.html through the real Jinja2 environment and asserts the consumer NAME reaches the HTML body, the central-owned label gates on `protected`, and None counts render an em dash rather than the literal "None". Stronger than the context-dict checks; the coroutine/None regressions cannot return. All 4 delete-route tests now pass (were failing on an uninitialized-pool RuntimeError). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/central/gui/routes.py | 4 +- tests/test_consumers.py | 99 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 1 deletion(-) diff --git a/src/central/gui/routes.py b/src/central/gui/routes.py index 4b25fe2..8dff9c5 100644 --- a/src/central/gui/routes.py +++ b/src/central/gui/routes.py @@ -2258,7 +2258,8 @@ async def consumers_list(request: Request) -> HTMLResponse: @router.post("/consumers/{stream}/{consumer}/delete", response_class=HTMLResponse) async def consumers_delete(request: Request, stream: str, consumer: str) -> Response: """Delete a JetStream consumer.""" - pool = get_pool() + from central.gui.nats import get_js + operator = request.state.operator form = await request.form() @@ -2296,6 +2297,7 @@ async def consumers_delete(request: Request, stream: str, consumer: str) -> Resp return RedirectResponse("/consumers", status_code=302) # Write audit log + pool = get_pool() async with pool.acquire() as conn: await write_audit( conn, diff --git a/tests/test_consumers.py b/tests/test_consumers.py index b947b8f..743dcb8 100644 --- a/tests/test_consumers.py +++ b/tests/test_consumers.py @@ -169,6 +169,105 @@ class TestConsumersListWithConsumers: assert fire["consumers"] == [] +class TestConsumersListHtmlRender: + """Render consumers_list.html through the real Jinja2 environment. + + Stronger than the context-dict checks above: these prove the values + actually reach the rendered HTML body. Guards two regressions: + - the consumer NAME must appear in the rendered HTML (proves the + ``await js.consumers_info(...)`` list reaches the template, not the + coroutine/async-iterator bug) + - Optional[int] count fields that are None must not render the literal + string ``None`` (they are guarded to an em dash). + """ + + PROTECTED_LABEL = 'central-owned' + + def _render(self, streams): + from central.gui import templates as templates_mod + template = templates_mod.env.get_template("consumers_list.html") + return template.render( + operator=MagicMock(username="testop"), + csrf_token="test_csrf", + streams=streams, + ) + + def test_consumer_name_appears_in_html(self): + streams = [ + { + "stream": "CENTRAL_WX", + "error": None, + "consumers": [ + { + "name": "meshai-wx", + "num_pending": 1000, + "num_ack_pending": 0, + "num_redelivered": 0, + "num_waiting": 0, + "created": datetime(2026, 5, 17, 12, 0, 0, tzinfo=timezone.utc), + "protected": False, + }, + ], + }, + ] + html = self._render(streams) + assert "meshai-wx" in html + # Non-protected consumer renders a delete form + assert "/consumers/CENTRAL_WX/meshai-wx/delete" in html + # ...and not the central-owned label span (which only the legend prose + # mentions, so we match the exact span markup, not the bare phrase) + assert self.PROTECTED_LABEL not in html + + def test_protected_consumer_renders_label_not_button(self): + streams = [ + { + "stream": "CENTRAL_WX", + "error": None, + "consumers": [ + { + "name": "archive-CENTRAL_WX", + "num_pending": 5, + "num_ack_pending": 0, + "num_redelivered": 0, + "num_waiting": 1, + "created": datetime(2026, 5, 17, 12, 0, 0, tzinfo=timezone.utc), + "protected": True, + }, + ], + }, + ] + html = self._render(streams) + assert "archive-CENTRAL_WX" in html + assert self.PROTECTED_LABEL in html + # No delete form for the protected consumer + assert "/consumers/CENTRAL_WX/archive-CENTRAL_WX/delete" not in html + + def test_none_counts_render_dash_not_literal_none(self): + streams = [ + { + "stream": "CENTRAL_WX", + "error": None, + "consumers": [ + { + "name": "meshai-wx", + "num_pending": None, + "num_ack_pending": None, + "num_redelivered": None, + "num_waiting": None, + "created": None, + "protected": False, + }, + ], + }, + ] + html = self._render(streams) + assert "meshai-wx" in html + # The literal "None" must never leak into a rendered table cell + assert ">None<" not in html + # The guarded fallback em dash is rendered instead + assert "—" in html + + class TestConsumersDeleteArchiveGuard: """POST /consumers/{stream}/archive-*/delete must be refused."""