From 44c6e6fd8a5755cedc34f67c1685250256026b35 Mon Sep 17 00:00:00 2001 From: matthewberry Date: Thu, 22 Oct 2026 20:30:10 -0500 Subject: [PATCH 1/2] feat: record API requests in Matomo Each request is sent to Matomo as a page view after the response, so API usage shows up next to the sites' visit numbers. Disabled unless MATOMO_SITE_ID is set; MATOMO_TOKEN lets Matomo record the caller's IP. --- .env.example | 4 +++ app/core/config.py | 8 ++++- app/core/matomo.py | 65 ++++++++++++++++++++++++++++++++++ app/main.py | 4 +++ tests/test_matomo.py | 83 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 163 insertions(+), 1 deletion(-) create mode 100644 app/core/matomo.py create mode 100644 tests/test_matomo.py diff --git a/.env.example b/.env.example index 2131d11..edbddc5 100644 --- a/.env.example +++ b/.env.example @@ -7,3 +7,7 @@ CLEAN_DB_PASSWORD=mmli CLEAN_DB_HOST=host.docker.internal CLEAN_DB_PORT=5432 CLEAN_DB_NAME=mmlidb + +# Matomo usage tracking (optional; disabled unless MATOMO_SITE_ID is set) +# MATOMO_SITE_ID= +# MATOMO_TOKEN= diff --git a/app/core/config.py b/app/core/config.py index 8c0415c..022375b 100644 --- a/app/core/config.py +++ b/app/core/config.py @@ -1,4 +1,4 @@ -from typing import List +from typing import List, Optional from pydantic import ConfigDict from pydantic_settings import BaseSettings @@ -16,6 +16,12 @@ class Settings(BaseSettings): # CORS configuration CORS_ORIGINS: List[str] = ["*"] + # Matomo usage tracking; disabled unless MATOMO_SITE_ID is set + MATOMO_URL: str = "https://matomo.mmli2.ncsa.illinois.edu" + MATOMO_SITE_ID: Optional[int] = None + # Token of a user with write access to the site; lets Matomo record the caller's IP + MATOMO_TOKEN: Optional[str] = None + # Database configuration CLEAN_DB_USER: str CLEAN_DB_PASSWORD: str diff --git a/app/core/matomo.py b/app/core/matomo.py new file mode 100644 index 0000000..7e010e5 --- /dev/null +++ b/app/core/matomo.py @@ -0,0 +1,65 @@ +import asyncio +import hashlib + +import httpx +from loguru import logger +from starlette.requests import Request + +from app.core.config import settings + +# Hits still being sent; holding a reference keeps the tasks from being garbage collected. +_pending: set[asyncio.Task] = set() + + +def build_hit(request: Request) -> dict: + """Build the Matomo tracking parameters for one API request.""" + ip = request.client.host if request.client else "" + ua = request.headers.get("user-agent", "") + params = { + "idsite": settings.MATOMO_SITE_ID, + "rec": 1, + "apiv": 1, + "send_image": 0, + "url": str(request.url), + "action_name": request.url.path, + "ua": ua, + "urlref": request.headers.get("referer", ""), + "lang": request.headers.get("accept-language", ""), + # API clients send no Matomo cookie, so derive a stable visitor id from IP + user agent. + "cid": hashlib.sha256(f"{ip}|{ua}".encode()).hexdigest()[:16], + } + # Matomo ignores cip (the caller's IP) unless the request carries a token with write access. + if settings.MATOMO_TOKEN: + params.update(token_auth=settings.MATOMO_TOKEN, cip=ip) + return params + + +async def send_hit(params: dict) -> None: + try: + async with httpx.AsyncClient(timeout=5) as client: + response = await client.post(f"{settings.MATOMO_URL}/matomo.php", data=params) + response.raise_for_status() + except httpx.HTTPError as e: + logger.warning(f"Matomo tracking failed: {e!r}") + + +class MatomoTrackingMiddleware: + """Record each API request in Matomo, after the response, without delaying it. + + Disabled unless MATOMO_SITE_ID is set. CORS preflight (OPTIONS) requests are skipped so a + browser call is counted once. + """ + + def __init__(self, app): + self.app = app + + async def __call__(self, scope, receive, send): + if scope["type"] != "http" or settings.MATOMO_SITE_ID is None or scope["method"] == "OPTIONS": + await self.app(scope, receive, send) + return + try: + await self.app(scope, receive, send) + finally: + task = asyncio.create_task(send_hit(build_hit(Request(scope)))) + _pending.add(task) + task.add_done_callback(_pending.discard) diff --git a/app/main.py b/app/main.py index b6eca5e..a9987ca 100644 --- a/app/main.py +++ b/app/main.py @@ -5,6 +5,7 @@ from fastapi.middleware.gzip import GZipMiddleware from app.core.config import settings +from app.core.matomo import MatomoTrackingMiddleware from app.db.database import _db from app.routers import search @@ -100,5 +101,8 @@ async def lifespan(app: FastAPI): # Add GZip compression middleware to compress large responses app.add_middleware(GZipMiddleware, minimum_size=1000) +# Record API usage in Matomo +app.add_middleware(MatomoTrackingMiddleware) + # Include API routers app.include_router(search.router, prefix="/api/v1") diff --git a/tests/test_matomo.py b/tests/test_matomo.py new file mode 100644 index 0000000..0e67d65 --- /dev/null +++ b/tests/test_matomo.py @@ -0,0 +1,83 @@ +import asyncio +from unittest.mock import AsyncMock, patch + +import httpx +import pytest + +import app.core.matomo as matomo +from app.core.config import settings +from app.main import app + + +@pytest.fixture +def tracking(monkeypatch): + monkeypatch.setattr(settings, "MATOMO_SITE_ID", 7) + monkeypatch.setattr(settings, "MATOMO_TOKEN", None) + with patch.object(matomo, "send_hit", AsyncMock()) as send_hit: + yield send_hit + + +async def request(method, path, **kwargs): + transport = httpx.ASGITransport(app=app, client=("203.0.113.5", 4321)) + async with httpx.AsyncClient(transport=transport, base_url="https://cleandb-api.test") as client: + response = await client.request(method, path, **kwargs) + await asyncio.gather(*matomo._pending) + return response + + +async def test_records_request(tracking): + response = await request("GET", "/api/v1/openapi.json?x=1", + headers={"user-agent": "curl/8", "referer": "https://example.org/"}) + assert response.status_code == 200 + params = tracking.await_args.args[0] + assert params["idsite"] == 7 + assert params["rec"] == 1 + assert params["url"] == "https://cleandb-api.test/api/v1/openapi.json?x=1" + assert params["action_name"] == "/api/v1/openapi.json" + assert params["ua"] == "curl/8" + assert params["urlref"] == "https://example.org/" + assert len(params["cid"]) == 16 + assert "cip" not in params and "token_auth" not in params + + +async def test_visitor_id_depends_on_ip_and_user_agent(tracking): + await request("GET", "/api/v1/openapi.json", headers={"user-agent": "a"}) + await request("GET", "/api/v1/openapi.json", headers={"user-agent": "a"}) + await request("GET", "/api/v1/openapi.json", headers={"user-agent": "b"}) + cids = [c.args[0]["cid"] for c in tracking.await_args_list] + assert cids[0] == cids[1] != cids[2] + + +async def test_token_sends_caller_ip(tracking, monkeypatch): + monkeypatch.setattr(settings, "MATOMO_TOKEN", "secret") + await request("GET", "/api/v1/openapi.json") + params = tracking.await_args.args[0] + assert params["token_auth"] == "secret" + assert params["cip"] == "203.0.113.5" + + +async def test_records_failed_requests(tracking): + response = await request("GET", "/api/v1/no-such-endpoint") + assert response.status_code == 404 + tracking.assert_awaited_once() + + +async def test_skips_cors_preflight(tracking): + await request("OPTIONS", "/api/v1/search", + headers={"origin": "https://example.org", "access-control-request-method": "GET"}) + tracking.assert_not_awaited() + + +async def test_disabled_without_site_id(tracking, monkeypatch): + monkeypatch.setattr(settings, "MATOMO_SITE_ID", None) + response = await request("GET", "/api/v1/openapi.json") + assert response.status_code == 200 + tracking.assert_not_awaited() + + +async def test_send_failure_is_logged_not_raised(monkeypatch): + # Nothing listens on port 9, so the POST fails to connect. + monkeypatch.setattr(settings, "MATOMO_URL", "http://127.0.0.1:9") + with patch.object(matomo.logger, "warning") as warning: + await matomo.send_hit({"idsite": 7, "rec": 1}) + warning.assert_called_once() From 216400a209368be167b3d66ffe2d422ba07f3be1 Mon Sep 17 00:00:00 2001 From: matthewberry Date: Thu, 22 Oct 2026 21:52:57 -0500 Subject: [PATCH 2/2] fix: address review of Matomo tracking - Treat empty MATOMO_SITE_ID/MATOMO_TOKEN as unset instead of failing to start - Catch every exception when sending a hit (httpx.InvalidURL is not an HTTPError) - Share one client with a connection limit, created and closed in lifespan; shutdown waits up to 5 s for in-flight hits - Send the path without the query string - Key the visitor id with the token so it can't be reversed into an IP; send cid/cip only with a token and a known caller address - Force tracking off in tests regardless of a local .env --- app/core/config.py | 8 +++- app/core/matomo.py | 55 ++++++++++++++++++++------- app/main.py | 6 ++- tests/conftest.py | 2 + tests/test_matomo.py | 90 +++++++++++++++++++++++++++++++++----------- 5 files changed, 124 insertions(+), 37 deletions(-) diff --git a/app/core/config.py b/app/core/config.py index 022375b..6140bac 100644 --- a/app/core/config.py +++ b/app/core/config.py @@ -1,6 +1,6 @@ from typing import List, Optional -from pydantic import ConfigDict +from pydantic import ConfigDict, field_validator from pydantic_settings import BaseSettings @@ -22,6 +22,12 @@ class Settings(BaseSettings): # Token of a user with write access to the site; lets Matomo record the caller's IP MATOMO_TOKEN: Optional[str] = None + @field_validator("MATOMO_SITE_ID", "MATOMO_TOKEN", mode="before") + @classmethod + def empty_is_unset(cls, value): + """Treat MATOMO_SITE_ID= (as in .env.example) as unset instead of failing to start.""" + return None if value == "" else value + # Database configuration CLEAN_DB_USER: str CLEAN_DB_PASSWORD: str diff --git a/app/core/matomo.py b/app/core/matomo.py index 7e010e5..49c1069 100644 --- a/app/core/matomo.py +++ b/app/core/matomo.py @@ -1,5 +1,6 @@ import asyncio import hashlib +import hmac import httpx from loguru import logger @@ -7,54 +8,82 @@ from app.core.config import settings +# Shared client, created in start() only when tracking is configured; None means tracking is off. +_client: httpx.AsyncClient | None = None # Hits still being sent; holding a reference keeps the tasks from being garbage collected. _pending: set[asyncio.Task] = set() +def start() -> None: + global _client + if settings.MATOMO_SITE_ID is not None: + # The connection limit caps concurrent hits; hits waiting for a connection give up + # after the pool timeout, so a slow Matomo can't pile up open connections. + _client = httpx.AsyncClient(timeout=5, limits=httpx.Limits(max_connections=10)) + + +async def stop() -> None: + """Give in-flight hits a few seconds to finish, then close the client.""" + global _client + if _client is None: + return + if _pending: + await asyncio.wait(_pending, timeout=5) + await _client.aclose() + _client = None + + def build_hit(request: Request) -> dict: """Build the Matomo tracking parameters for one API request.""" - ip = request.client.host if request.client else "" ua = request.headers.get("user-agent", "") params = { "idsite": settings.MATOMO_SITE_ID, "rec": 1, "apiv": 1, "send_image": 0, - "url": str(request.url), + # Path only: query strings would make every search and typeahead keystroke its own page. + "url": str(request.url.replace(query="")), "action_name": request.url.path, "ua": ua, "urlref": request.headers.get("referer", ""), "lang": request.headers.get("accept-language", ""), - # API clients send no Matomo cookie, so derive a stable visitor id from IP + user agent. - "cid": hashlib.sha256(f"{ip}|{ua}".encode()).hexdigest()[:16], } + ip = request.client.host if request.client else None # Matomo ignores cip (the caller's IP) unless the request carries a token with write access. - if settings.MATOMO_TOKEN: - params.update(token_auth=settings.MATOMO_TOKEN, cip=ip) + if settings.MATOMO_TOKEN and ip: + params.update( + token_auth=settings.MATOMO_TOKEN, + cip=ip, + # API clients send no Matomo cookie, so derive a stable visitor id from IP + user + # agent. Keyed with the token so the id can't be reversed into the caller's IP; + # rotating the token resets visitor ids. + cid=hmac.new(settings.MATOMO_TOKEN.encode(), f"{ip}|{ua}".encode(), + hashlib.sha256).hexdigest()[:16], + ) return params async def send_hit(params: dict) -> None: try: - async with httpx.AsyncClient(timeout=5) as client: - response = await client.post(f"{settings.MATOMO_URL}/matomo.php", data=params) - response.raise_for_status() - except httpx.HTTPError as e: + response = await _client.post(f"{settings.MATOMO_URL}/matomo.php", data=params) + response.raise_for_status() + except Exception as e: + # Tracking must never affect the API, whatever goes wrong. logger.warning(f"Matomo tracking failed: {e!r}") class MatomoTrackingMiddleware: """Record each API request in Matomo, after the response, without delaying it. - Disabled unless MATOMO_SITE_ID is set. CORS preflight (OPTIONS) requests are skipped so a - browser call is counted once. + Disabled unless MATOMO_SITE_ID is set (see start()). CORS preflight (OPTIONS) requests are + skipped so a browser call is counted once. """ def __init__(self, app): self.app = app async def __call__(self, scope, receive, send): - if scope["type"] != "http" or settings.MATOMO_SITE_ID is None or scope["method"] == "OPTIONS": + if scope["type"] != "http" or _client is None or scope["method"] == "OPTIONS": await self.app(scope, receive, send) return try: diff --git a/app/main.py b/app/main.py index a9987ca..c206409 100644 --- a/app/main.py +++ b/app/main.py @@ -4,8 +4,8 @@ from fastapi.middleware.cors import CORSMiddleware from fastapi.middleware.gzip import GZipMiddleware +from app.core import matomo from app.core.config import settings -from app.core.matomo import MatomoTrackingMiddleware from app.db.database import _db from app.routers import search @@ -15,9 +15,11 @@ async def lifespan(app: FastAPI): """Lifespan context manager for database connection handling.""" # Connect to database on startup await _db.connect() + matomo.start() yield # Disconnect from database on shutdown await _db.disconnect() + await matomo.stop() # Initialize FastAPI application @@ -102,7 +104,7 @@ async def lifespan(app: FastAPI): app.add_middleware(GZipMiddleware, minimum_size=1000) # Record API usage in Matomo -app.add_middleware(MatomoTrackingMiddleware) +app.add_middleware(matomo.MatomoTrackingMiddleware) # Include API routers app.include_router(search.router, prefix="/api/v1") diff --git a/tests/conftest.py b/tests/conftest.py index 82a4eb2..9bb538b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -6,6 +6,8 @@ # Settings require DB credentials at import time; tests mock the DB calls. for key in ("CLEAN_DB_HOST", "CLEAN_DB_USER", "CLEAN_DB_PASSWORD", "CLEAN_DB_NAME"): os.environ.setdefault(key, "test") +# Keep a developer's .env from sending test requests to the real Matomo site. +os.environ["MATOMO_SITE_ID"] = "" @pytest.fixture diff --git a/tests/test_matomo.py b/tests/test_matomo.py index 0e67d65..2df5361 100644 --- a/tests/test_matomo.py +++ b/tests/test_matomo.py @@ -1,20 +1,24 @@ import asyncio +import hashlib from unittest.mock import AsyncMock, patch import httpx import pytest +from starlette.requests import Request import app.core.matomo as matomo -from app.core.config import settings +from app.core.config import Settings, settings from app.main import app @pytest.fixture -def tracking(monkeypatch): +async def tracking(monkeypatch): monkeypatch.setattr(settings, "MATOMO_SITE_ID", 7) monkeypatch.setattr(settings, "MATOMO_TOKEN", None) + matomo.start() with patch.object(matomo, "send_hit", AsyncMock()) as send_hit: yield send_hit + await matomo.stop() async def request(method, path, **kwargs): @@ -25,35 +29,44 @@ async def request(method, path, **kwargs): return response -async def test_records_request(tracking): - response = await request("GET", "/api/v1/openapi.json?x=1", +async def test_records_request_path_without_query(tracking): + response = await request("GET", "/api/v1/openapi.json?accession=P51451", headers={"user-agent": "curl/8", "referer": "https://example.org/"}) assert response.status_code == 200 params = tracking.await_args.args[0] assert params["idsite"] == 7 assert params["rec"] == 1 - assert params["url"] == "https://cleandb-api.test/api/v1/openapi.json?x=1" + assert params["url"] == "https://cleandb-api.test/api/v1/openapi.json" assert params["action_name"] == "/api/v1/openapi.json" assert params["ua"] == "curl/8" assert params["urlref"] == "https://example.org/" - assert len(params["cid"]) == 16 - assert "cip" not in params and "token_auth" not in params + assert not {"cip", "cid", "token_auth"} & params.keys() -async def test_visitor_id_depends_on_ip_and_user_agent(tracking): +async def test_token_sends_caller_ip_and_keyed_visitor_id(tracking, monkeypatch): + monkeypatch.setattr(settings, "MATOMO_TOKEN", "secret") await request("GET", "/api/v1/openapi.json", headers={"user-agent": "a"}) await request("GET", "/api/v1/openapi.json", headers={"user-agent": "a"}) await request("GET", "/api/v1/openapi.json", headers={"user-agent": "b"}) - cids = [c.args[0]["cid"] for c in tracking.await_args_list] - assert cids[0] == cids[1] != cids[2] + first, again, other_ua = (c.args[0] for c in tracking.await_args_list) + assert first["token_auth"] == "secret" + assert first["cip"] == "203.0.113.5" + assert len(first["cid"]) == 16 + assert first["cid"] == again["cid"] != other_ua["cid"] + # Not recoverable by hashing candidate IPs and user agents without the token. + assert first["cid"] != hashlib.sha256(b"203.0.113.5|a").hexdigest()[:16] + + monkeypatch.setattr(settings, "MATOMO_TOKEN", "other") + await request("GET", "/api/v1/openapi.json", headers={"user-agent": "a"}) + assert tracking.await_args.args[0]["cid"] != first["cid"] -async def test_token_sends_caller_ip(tracking, monkeypatch): +def test_no_caller_address_sends_no_ip_or_visitor_id(monkeypatch): monkeypatch.setattr(settings, "MATOMO_TOKEN", "secret") - await request("GET", "/api/v1/openapi.json") - params = tracking.await_args.args[0] - assert params["token_auth"] == "secret" - assert params["cip"] == "203.0.113.5" + scope = {"type": "http", "method": "GET", "scheme": "https", "server": ("api.test", 443), + "path": "/api/v1/search", "query_string": b"", "headers": [], "client": None} + params = matomo.build_hit(Request(scope)) + assert not {"cip", "cid", "token_auth"} & params.keys() async def test_records_failed_requests(tracking): @@ -68,16 +81,51 @@ async def test_skips_cors_preflight(tracking): tracking.assert_not_awaited() -async def test_disabled_without_site_id(tracking, monkeypatch): +async def test_disabled_without_site_id(monkeypatch): monkeypatch.setattr(settings, "MATOMO_SITE_ID", None) - response = await request("GET", "/api/v1/openapi.json") + matomo.start() + with patch.object(matomo, "send_hit", AsyncMock()) as send_hit: + response = await request("GET", "/api/v1/openapi.json") assert response.status_code == 200 - tracking.assert_not_awaited() + send_hit.assert_not_awaited() + +def test_empty_settings_mean_unset(monkeypatch): + monkeypatch.setenv("MATOMO_SITE_ID", "") + monkeypatch.setenv("MATOMO_TOKEN", "") + loaded = Settings(_env_file=None) + assert loaded.MATOMO_SITE_ID is None + assert loaded.MATOMO_TOKEN is None -async def test_send_failure_is_logged_not_raised(monkeypatch): - # Nothing listens on port 9, so the POST fails to connect. - monkeypatch.setattr(settings, "MATOMO_URL", "http://127.0.0.1:9") + +@pytest.mark.parametrize("url", [ + "http://127.0.0.1:9", # nothing listens on port 9, so the POST fails to connect + "http://matomo:not-a-port", # httpx.InvalidURL, which is not an httpx.HTTPError +]) +async def test_send_failure_is_logged_not_raised(monkeypatch, url): + monkeypatch.setattr(settings, "MATOMO_SITE_ID", 7) + monkeypatch.setattr(settings, "MATOMO_URL", url) + matomo.start() with patch.object(matomo.logger, "warning") as warning: await matomo.send_hit({"idsite": 7, "rec": 1}) + await matomo.stop() warning.assert_called_once() + + +async def test_stop_waits_for_in_flight_hits(monkeypatch): + monkeypatch.setattr(settings, "MATOMO_SITE_ID", 7) + matomo.start() + + async def slow_post(*args, **kwargs): + await asyncio.sleep(0.1) + return httpx.Response(204, request=httpx.Request("POST", "https://matomo.test")) + + with patch.object(matomo._client, "post", side_effect=slow_post) as post: + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="https://cleandb-api.test") as client: + await client.get("/api/v1/openapi.json") + assert matomo._pending + await matomo.stop() + post.assert_awaited_once() + assert not matomo._pending + assert matomo._client is None