diff --git a/app/api/routes/api/config.py b/app/api/routes/api/config.py index aeea72f..9f4dfd6 100644 --- a/app/api/routes/api/config.py +++ b/app/api/routes/api/config.py @@ -22,6 +22,7 @@ from app.schemas.config import ( from app.services.auth import AuthenticatedSession from app.services.config_page import ConfigSaveError, build_config_sections, save_config_updates from app.services.email import EmailConfigurationError, EmailDeliveryError, send_smtp_test_email +from app.services.tibber_prices import active_tibber_contract_exists, trigger_tibber_refresh logger = logging.getLogger(__name__) @@ -65,6 +66,10 @@ def put_config( # Detect whether any MQTT-related key is being submitted (non-secret change # or non-blank secret change) so we know to reconnect after saving. mqtt_keys_submitted = any(k.lower() in MQTT_SETTINGS_KEYS for k in body.updates) + tibber_values_before = ( + settings.tibber_api_token, + settings.tibber_home_id, + ) try: save_config_updates(db, body.updates, settings) @@ -92,6 +97,13 @@ def put_config( from app.services.dsmr_ingest import apply_dsmr_subscription apply_dsmr_subscription(refreshed_settings) + tibber_values_changed = tibber_values_before != ( + refreshed_settings.tibber_api_token, + refreshed_settings.tibber_home_id, + ) + if tibber_values_changed and active_tibber_contract_exists(db): + trigger_tibber_refresh() + sections_raw = build_config_sections(db, refreshed_settings) return ConfigUpdateResponse(sections=_sections_from_raw(sections_raw)) diff --git a/app/api/routes/api/energy_contracts.py b/app/api/routes/api/energy_contracts.py index 7a4630d..e65cf35 100644 --- a/app/api/routes/api/energy_contracts.py +++ b/app/api/routes/api/energy_contracts.py @@ -56,6 +56,7 @@ from app.services.contracts import ( get_contract_or_none, list_contracts, ) +from app.services.tibber_prices import trigger_tibber_refresh logger = logging.getLogger(__name__) @@ -273,6 +274,7 @@ def patch_energy_contract( scope-local mutual exclusion. """ contract = _get_contract_or_404(db, contract_id) + was_active = contract.active if body.name is not None: contract.name = body.name @@ -285,6 +287,8 @@ def patch_energy_contract( db.commit() db.refresh(contract) + if body.active is True and not was_active and contract.kind == "tibber": + trigger_tibber_refresh() return _contract_detail(db, contract) diff --git a/app/main.py b/app/main.py index bd16fe3..9bfee89 100644 --- a/app/main.py +++ b/app/main.py @@ -1,6 +1,7 @@ import logging import os from contextlib import asynccontextmanager +from datetime import UTC, datetime from pathlib import Path from typing import Callable @@ -38,7 +39,7 @@ from app.services.dsmr_ingest import apply_dsmr_subscription from app.services.public_ip import check_public_ipv4_and_notify from app.services.modbus_poll import poll_all_enabled_devices, BASE_POLL_TICK_SECONDS from app.services.ha_discovery import publish_discovery, publish_states -from app.services.tibber_prices import refresh_prices +from app.services.tibber_prices import run_tibber_refresh_best_effort from app.services.energy_cost import compute_closed_periods from app.services.meter_cost import compute_closed_periods as compute_closed_meter_cost_periods from app.services.warmtelink_worker import warmtelink_worker_manager @@ -91,15 +92,7 @@ def _run_scheduled_tibber_refresh() -> None: so that a single failed fetch does not crash the scheduler or affect the other background jobs. """ - session_local = get_session_local() - session: Session = session_local() - try: - runtime_settings = build_runtime_settings(session, get_settings()) - refresh_prices(session, runtime_settings) - except Exception: - logger.exception("_run_scheduled_tibber_refresh: unexpected error") - finally: - session.close() + run_tibber_refresh_best_effort() def _run_scheduled_energy_cost() -> None: @@ -264,6 +257,10 @@ async def lifespan(_: FastAPI): replace_existing=True, max_instances=1, coalesce=True, + # APScheduler otherwise waits one full interval before its first run. + # This preserves the hourly cadence while requesting a non-blocking + # startup fetch as soon as the scheduler starts. + next_run_time=datetime.now(UTC), ) # Energy cost billing: compute uncalculated closed 15-minute periods every minute. # The job is a no-op when no active contract or DSMR data is present, so it is diff --git a/app/services/tibber_prices.py b/app/services/tibber_prices.py index 98bef19..27b20de 100644 --- a/app/services/tibber_prices.py +++ b/app/services/tibber_prices.py @@ -30,6 +30,7 @@ Design decisions from __future__ import annotations import logging +from threading import Lock, Thread from datetime import UTC, datetime from sqlalchemy import select @@ -41,7 +42,10 @@ from app.models.energy import EnergyContract, TibberPrice logger = logging.getLogger(__name__) -def _active_tibber_contract_exists(session: Session) -> bool: +_background_refresh_lock = Lock() + + +def active_tibber_contract_exists(session: Session) -> bool: """Return True if there is an active contract with kind='tibber'.""" row = session.execute( select(EnergyContract).where( @@ -84,14 +88,17 @@ def refresh_prices(session: Session, settings: object) -> int: logger.debug("refresh_prices: tibber_api_token is empty — no-op") return 0 - if not _active_tibber_contract_exists(session): + if not active_tibber_contract_exists(session): logger.debug("refresh_prices: no active tibber contract — no-op") return 0 home_id: str = getattr(settings, "tibber_home_id", "") or "" home_id_or_none: str | None = home_id.strip() or None - logger.info("refresh_prices: fetching Tibber price range (home_id=%r)", home_id_or_none) + # Neither the API token nor the selected home identifier is safe to emit in + # diagnostics. The fetch client receives them, but logs only describe the + # operation itself. + logger.info("refresh_prices: fetching Tibber price range") # May raise TibberError or TibberAuthError — let them propagate. price_points = fetch_price_range(token, home_id_or_none) @@ -137,3 +144,59 @@ def refresh_prices(session: Session, settings: object) -> int: logger.info("refresh_prices: upserted %d price points", upserted) return upserted + + +def run_tibber_refresh_best_effort() -> bool: + """Run one refresh with an isolated session, skipping concurrent requests. + + This is shared by the hourly scheduler and immediate post-commit triggers. + It intentionally catches all failures: refresh is advisory and must never + make app startup or a successfully committed configuration/contract update + appear to have failed. The boolean reports whether this invocation owned + the work; it is primarily useful for tests and diagnostics. + """ + if not _background_refresh_lock.acquire(blocking=False): + logger.debug("Tibber refresh already running; skipping duplicate request") + return False + + session: Session | None = None + try: + # Local imports keep the pure refresh service free of app startup import + # cycles, while every background invocation gets a fresh DB session. + from app.config import get_settings + from app.db import get_session_local + from app.services.config_page import build_runtime_settings + + session = get_session_local()() + refresh_prices(session, build_runtime_settings(session, get_settings())) + except Exception as exc: + # Exception text can contain remote request details. Keep diagnostics + # useful without allowing a token or home id to escape through logging. + logger.warning("Tibber price refresh failed (%s)", type(exc).__name__) + if session is not None: + try: + session.rollback() + except Exception: + logger.warning("Tibber price refresh rollback failed") + finally: + if session is not None: + try: + session.close() + except Exception: + logger.warning("Tibber price refresh session close failed") + _background_refresh_lock.release() + return True + + +def trigger_tibber_refresh() -> None: + """Request a non-blocking, best-effort Tibber refresh after a DB commit.""" + try: + Thread( + target=run_tibber_refresh_best_effort, + name="tibber-price-refresh", + daemon=True, + ).start() + except Exception as exc: + # Starting the optional worker must not turn an already committed API + # operation into a failure; avoid logging exception text for secrecy. + logger.warning("Unable to start Tibber price refresh (%s)", type(exc).__name__) diff --git a/tests/test_api_config.py b/tests/test_api_config.py index fa08453..193c2b0 100644 --- a/tests/test_api_config.py +++ b/tests/test_api_config.py @@ -946,6 +946,55 @@ def test_get_config_tibber_api_token_value_masked_after_save( assert "some-tibber-token" not in resp_after.text +def test_put_tibber_config_triggers_refresh_only_for_active_tibber_contract( + client: TestClient, test_database_urls +) -> None: + """Saved Tibber credentials request a refresh only when their contract is active.""" + _login(client) + conn = sqlite3.connect(test_database_urls["app_path"]) + try: + conn.execute( + "INSERT INTO energy_contract (name, kind, scope, active, currency, created_at, updated_at) " + "VALUES (?, ?, ?, ?, ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)", + ("Tibber", "tibber", "electricity", True, "EUR"), + ) + conn.commit() + finally: + conn.close() + + with patch("app.api.routes.api.config.trigger_tibber_refresh") as trigger: + response = client.put( + "/api/config", + json={"updates": _full_config_payload({"TIBBER_API_TOKEN": "new-secret-token"})}, + headers={"X-CSRF-Token": "token"}, + ) + assert response.status_code == 200 + trigger.assert_called_once() + + with patch("app.api.routes.api.config.trigger_tibber_refresh") as trigger: + response = client.put( + "/api/config", + json={"updates": _full_config_payload({"TIBBER_API_TOKEN": ""})}, + headers={"X-CSRF-Token": "token"}, + ) + assert response.status_code == 200 + trigger.assert_not_called() + + +def test_put_tibber_config_does_not_refresh_without_active_tibber_contract(client: TestClient) -> None: + _login(client) + + with patch("app.api.routes.api.config.trigger_tibber_refresh") as trigger: + response = client.put( + "/api/config", + json={"updates": _full_config_payload({"TIBBER_HOME_ID": "new-home-id"})}, + headers={"X-CSRF-Token": "token"}, + ) + + assert response.status_code == 200 + trigger.assert_not_called() + + def test_post_mqtt_test_uses_db_broker_host( client: TestClient, test_database_urls ) -> None: diff --git a/tests/test_api_energy_contracts.py b/tests/test_api_energy_contracts.py index fe13cc9..9580679 100644 --- a/tests/test_api_energy_contracts.py +++ b/tests/test_api_energy_contracts.py @@ -48,6 +48,7 @@ from __future__ import annotations from datetime import UTC, datetime, timedelta from typing import Any +from unittest.mock import patch import pytest from fastapi.testclient import TestClient @@ -614,6 +615,52 @@ def test_activate_contract_mutual_exclusion(contracts_client): assert active_contracts[0].id == id_b +def test_activating_tibber_contract_triggers_refresh_only_after_commit(contracts_client): + """Inactive→active Tibber is the only contract transition that requests refresh.""" + client, engine = contracts_client + _login(client) + created = client.post( + "/api/energy/contracts", + json=_tibber_payload(), + headers={"X-CSRF-Token": _CSRF}, + ) + contract_id = created.json()["id"] + + with patch("app.api.routes.api.energy_contracts.trigger_tibber_refresh") as trigger: + response = client.patch( + f"/api/energy/contracts/{contract_id}", + json={"active": True}, + headers={"X-CSRF-Token": _CSRF}, + ) + assert response.status_code == 200 + assert trigger.call_count == 1 + with Session(engine) as session: + assert session.get(EnergyContract, contract_id).active is True + + with patch("app.api.routes.api.energy_contracts.trigger_tibber_refresh") as trigger: + response = client.patch( + f"/api/energy/contracts/{contract_id}", + json={"active": True}, + headers={"X-CSRF-Token": _CSRF}, + ) + assert response.status_code == 200 + trigger.assert_not_called() + + manual = client.post( + "/api/energy/contracts", + json=_manual_payload(), + headers={"X-CSRF-Token": _CSRF}, + ).json() + with patch("app.api.routes.api.energy_contracts.trigger_tibber_refresh") as trigger: + response = client.patch( + f"/api/energy/contracts/{manual['id']}", + json={"active": True}, + headers={"X-CSRF-Token": _CSRF}, + ) + assert response.status_code == 200 + trigger.assert_not_called() + + def test_deactivate_contract(contracts_client): """PATCH active=false deactivates the contract without touching others.""" client, _ = contracts_client diff --git a/tests/test_app.py b/tests/test_app.py index 256a219..aaac8d1 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -152,3 +152,40 @@ def test_app_start_syncs_app_hostname_from_env_even_when_db_has_old_value( get_settings.cache_clear() reset_db_caches() + + +def test_lifespan_schedules_immediate_and_hourly_tibber_refresh(tmp_path, monkeypatch: pytest.MonkeyPatch) -> None: + """The single Tibber interval job starts immediately while retaining its hourly trigger.""" + import app.main as main + + app_database_url = _prepare_app_db(tmp_path) + added_jobs = [] + + class _Scheduler: + def __init__(self, **_kwargs): pass + def add_job(self, func, **kwargs): added_jobs.append((func, kwargs)) + def start(self): pass + def shutdown(self, **_kwargs): pass + + monkeypatch.setenv("APP_DATABASE_URL", app_database_url) + monkeypatch.setenv("AUTH_BOOTSTRAP_USERNAME", "admin") + monkeypatch.setenv("AUTH_BOOTSTRAP_PASSWORD", "test-password") + monkeypatch.setattr(main, "BackgroundScheduler", _Scheduler) + monkeypatch.setattr(main.mqtt_manager, "connect", lambda _settings: None) + monkeypatch.setattr(main.mqtt_manager, "disconnect", lambda: None) + monkeypatch.setattr(main, "apply_dsmr_subscription", lambda _settings: None) + monkeypatch.setattr(main.warmtelink_worker_manager, "start", lambda: None) + monkeypatch.setattr(main.warmtelink_worker_manager, "shutdown", lambda: None) + get_settings.cache_clear() + reset_db_caches() + + anyio.run(_run_lifespan, create_app()) + + tibber_jobs = [kwargs for func, kwargs in added_jobs if func is main._run_scheduled_tibber_refresh] + assert len(tibber_jobs) == 1 + assert tibber_jobs[0]["id"] == "tibber-refresh" + assert tibber_jobs[0]["max_instances"] == 1 + assert tibber_jobs[0]["next_run_time"] is not None + + get_settings.cache_clear() + reset_db_caches() diff --git a/tests/test_tibber_prices.py b/tests/test_tibber_prices.py index 5d65def..8748151 100644 --- a/tests/test_tibber_prices.py +++ b/tests/test_tibber_prices.py @@ -27,7 +27,7 @@ from sqlalchemy.orm import Session from app.integrations.tibber.client import PricePoint, TibberError from app.models.energy import EnergyContract, EnergyContractVersion, TibberPrice -from app.services.tibber_prices import refresh_prices +from app.services.tibber_prices import refresh_prices, run_tibber_refresh_best_effort, trigger_tibber_refresh # --------------------------------------------------------------------------- @@ -392,3 +392,55 @@ def test_refresh_prices_propagates_client_error(energy_db, monkeypatch): # No rows should have been written. assert _count_tibber_price_rows(session) == 0 + + +def test_best_effort_refresh_uses_isolated_session_and_sanitises_failure(monkeypatch, caplog): + """Background failures close their own session and never log supplied secrets.""" + import app.db as db_module + import app.services.config_page as config_page + import app.services.tibber_prices as tibber_prices + + token = "secret-token-must-not-appear" + home_id = "secret-home-must-not-appear" + settings = _FakeSettings(token=token, home_id=home_id) + + class _Session: + rolled_back = False + closed = False + + def rollback(self): + self.rolled_back = True + + def close(self): + self.closed = True + + session = _Session() + monkeypatch.setattr(db_module, "get_session_local", lambda: lambda: session) + monkeypatch.setattr(config_page, "build_runtime_settings", lambda *_args: settings) + monkeypatch.setattr(tibber_prices, "refresh_prices", lambda *_args: (_ for _ in ()).throw(RuntimeError(token))) + + assert run_tibber_refresh_best_effort() is True + assert session.rolled_back is True + assert session.closed is True + assert token not in caplog.text + assert home_id not in caplog.text + + +def test_trigger_tibber_refresh_starts_daemon_without_running_inline(monkeypatch): + """API triggers are non-blocking; the worker owns the eventual refresh.""" + import app.services.tibber_prices as tibber_prices + + captured = {} + + class _Thread: + def __init__(self, **kwargs): + captured.update(kwargs) + + def start(self): + captured["started"] = True + + monkeypatch.setattr(tibber_prices, "Thread", _Thread) + trigger_tibber_refresh() + assert captured["started"] is True + assert captured["daemon"] is True + assert captured["target"] is run_tibber_refresh_best_effort