Nora
d81b6ed4fc
Before this fix, the list endpoints computed `total` via
`len(list(store.iter_*(**common)))`. Both `iter_claims` and
`iter_remittances` default to `limit=100`, so the reported total
silently capped at 100 even when the DB held 60k claims or 835
remits. The frontend rendered a `data.total` of 100 in the KPI
tile and a 100-row table, the page looked complete, and the bug
stayed hidden — exactly the Dashboard silent-failure pattern that
59c3275 (server-aggregated KPIs) was meant to retire.
The Remittances page symptom reported on Jun 29 ('REMITS 100',
empty CLAIM column on 100 rows) was this bug. The empty CLAIM is
a separate matching concern (those remits have `claim_id = NULL`
in the DB); the count fix addresses the population-size lie.
Fix:
* `CycloneStore.count_claims` + `count_remittances` reuse the
iter's filter pipeline (DB filters + in-memory payer/date
filters) with an effectively-unbounded limit so the count
reflects the true DB population.
* `list_claims` + `list_remittances` call the new count
helpers instead of slicing `iter_*` twice.
* 13 new tests in `test_list_endpoint_counts.py` cover the
empty/filter/per-dimension/cardinality cases plus the HTTP
regression (seed 150/120, assert total matches).
Bonus fix:
* `conftest._reset_rate_limit_buckets` walks the middleware
stack and clears `RateLimitMiddleware._buckets` between
tests. Without this, the full suite tripped the 300 req/60s
rate limiter mid-run and 9 later tests (4 of mine, 5
pre-existing in test_inbox_endpoints / test_payer_summary)
got 429s. All 1167 tests now pass.
Follow-ups noted but out of scope:
* `/api/admin/audit-log`, `/api/admin/backup/list`,
`/api/admin/scheduler/processed-files` use the same
`len(rows)`-after-`.limit(limit)` pattern. Admin-only,
no `has_more` consumption, so impact is bounded.
* `count_*` could short-circuit to `func.count()` to skip
the UI-dict build, but iter already calls `q.all()` so
the win is modest.
92 lines
4.0 KiB
Python
92 lines
4.0 KiB
Python
"""Per-test database setup for the Cyclone test suite.
|
|
|
|
Every test gets a fresh SQLite DB at ``tmp_path/test.db`` so the suite
|
|
can run in parallel and never touches the user's ``~/.local/share/cyclone``
|
|
database. ``db.init_db()`` is idempotent — tests that already call it
|
|
explicitly are unaffected (the second call short-circuits).
|
|
|
|
This fixture exists because the SQLAlchemy-backed ``CycloneStore``
|
|
requires the engine to be initialized before any DB access, whereas the
|
|
old in-memory store did not. Adding the init here keeps existing test
|
|
modules (test_api.py, test_api_835.py, test_api_gets.py,
|
|
test_api_parse_persists.py) working unchanged.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import pytest
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def _auto_init_db(tmp_path, monkeypatch):
|
|
"""Point CYCLONE_DB_URL at a per-test SQLite file and init the schema.
|
|
|
|
Also wires a fresh ``EventBus`` onto ``app.state`` because ``TestClient``
|
|
does not invoke the FastAPI lifespan handler unless used as a context
|
|
manager. The bus is reset between tests so subscribers don't leak.
|
|
|
|
Auth gating is enabled at the router/endpoint level via the
|
|
``Depends(matrix_gate)`` wiring in ``cyclone.api``. The auth tests
|
|
(``test_auth_*``) explicitly flip ``AUTH_DISABLED = False`` and
|
|
authenticate via the public login route to exercise the real
|
|
auth path. Every other test — the original test suite predates
|
|
auth — gets ``AUTH_DISABLED = True`` here so the existing tests
|
|
keep working without each one having to login first.
|
|
"""
|
|
monkeypatch.setenv("CYCLONE_DB_URL", f"sqlite:///{tmp_path}/test.db")
|
|
from cyclone import db
|
|
from cyclone.pubsub import EventBus
|
|
from cyclone.auth import deps
|
|
|
|
db._reset_for_tests()
|
|
db.init_db()
|
|
# Re-resolve `app` each fixture invocation because some tests
|
|
# (test_cors_extra_origins_via_env) call ``importlib.reload`` on
|
|
# ``cyclone.api`` to mutate the CORS allow-list. If we cached
|
|
# the app reference at conftest module load, we'd be setting
|
|
# ``event_bus`` on a stale instance that no test client is
|
|
# actually using.
|
|
from cyclone import api as _api_mod
|
|
_api_mod.app.state.event_bus = EventBus()
|
|
deps.AUTH_DISABLED = True
|
|
# The rate-limit middleware keeps a per-IP sliding window in
|
|
# ``_buckets``. Without a reset between tests, later tests in a
|
|
# full-suite run get ``429 Too Many Requests`` once the testclient
|
|
# IP exhausts its 300 req/60s budget. Walk the middleware stack
|
|
# and clear the buckets so every test starts with a fresh window.
|
|
# Trigger the stack build with a cheap health probe (the only
|
|
# request exempt from the limiter — see RateLimitMiddleware.EXEMPT_PATHS).
|
|
_reset_rate_limit_buckets(_api_mod.app)
|
|
try:
|
|
yield
|
|
finally:
|
|
deps.AUTH_DISABLED = False
|
|
_api_mod.app.state.event_bus = None
|
|
db._reset_for_tests()
|
|
|
|
|
|
def _reset_rate_limit_buckets(app) -> None:
|
|
"""Clear the rate-limit middleware's per-IP sliding-window buckets.
|
|
|
|
SP27 Task 13b follow-up: ``RateLimitMiddleware._buckets`` is shared
|
|
across all tests in a process (TestClient reuses the same ``app``
|
|
instance), so without a reset between tests the full suite trips
|
|
the limiter after ~300 requests and later tests get 429s.
|
|
|
|
The middleware stack is only built on the first request, so we
|
|
prime it with an exempt health probe before walking to the
|
|
RateLimit layer. If the stack ever stops being a single chain
|
|
of ``.app`` links, this helper raises AttributeError — better
|
|
to fail loudly than silently leak state.
|
|
"""
|
|
from fastapi.testclient import TestClient
|
|
TestClient(app).get("/api/health")
|
|
cur = app.middleware_stack
|
|
while cur is not None:
|
|
if hasattr(cur, "_buckets"):
|
|
cur._buckets.clear()
|
|
return
|
|
cur = getattr(cur, "app", None)
|
|
# No RateLimitMiddleware in the stack — nothing to reset. Should
|
|
# not happen in this codebase (security.py registers it at boot)
|
|
# but we don't want a missing reset to crash unrelated tests. |