DSMR hardening: restart-free config + decouple from telegram id
Config hot-reload (no restart): - MqttManager.unsubscribe(); apply_dsmr_subscription(settings) re-applies the DSMR subscription (subscribe/unsubscribe/topic-change, fresh snapshot so the sample interval also takes effect). - Called from lifespan AND PUT /api/config, so toggling DSMR ingest in the UI takes effect immediately without restarting the app. Decouple ingest from the DSMR telegram id (overflows / resets to zero): - Migration 20260624_12: drop UNIQUE(source_id), make recorded_at UNIQUE. - Idempotency now keys on recorded_at (telegram timestamp); the table's own autoincrement PK is the stable identity. source_id kept only as a reference. - Regression test: colliding telegram ids no longer drop new data.
This commit is contained in:
@@ -187,6 +187,28 @@ def test_put_config_with_csrf_header_updates_app_name(
|
||||
assert app_name_field["value"] == "Updated via API"
|
||||
|
||||
|
||||
def test_put_config_reapplies_dsmr_subscription(
|
||||
client: TestClient, test_database_urls
|
||||
) -> None:
|
||||
"""Saving config must re-apply the DSMR subscription so enabling DSMR ingest
|
||||
takes effect without an app restart (the route calls apply_dsmr_subscription
|
||||
with the refreshed settings)."""
|
||||
_login(client)
|
||||
|
||||
payload = _full_config_payload({"DSMR_INGEST_ENABLED": "true"})
|
||||
with patch("app.services.dsmr_ingest.apply_dsmr_subscription") as spy:
|
||||
response = client.put(
|
||||
"/api/config",
|
||||
json={"updates": payload},
|
||||
headers={"X-CSRF-Token": "any-non-empty-value"},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
spy.assert_called_once()
|
||||
applied_settings = spy.call_args.args[0]
|
||||
assert applied_settings.dsmr_ingest_enabled is True
|
||||
|
||||
|
||||
def test_put_config_blank_secret_keeps_existing_value(
|
||||
client: TestClient, test_database_urls
|
||||
) -> None:
|
||||
|
||||
@@ -214,12 +214,12 @@ def test_off_interval_not_multiple_of_10(dsmr_db):
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 3. Idempotency — same source_id not re-inserted
|
||||
# 3. Idempotency — keyed on recorded_at (telegram timestamp), NOT the telegram id
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_same_source_id_not_reinserted(dsmr_db):
|
||||
"""Feeding the same telegram twice (same id) must result in exactly one row."""
|
||||
def test_same_timestamp_not_reinserted(dsmr_db):
|
||||
"""Feeding the same telegram twice (same timestamp) must result in one row."""
|
||||
_, SessionLocal = dsmr_db
|
||||
settings = _make_settings(dsmr_sample_interval_s=10)
|
||||
|
||||
@@ -229,8 +229,8 @@ def test_same_source_id_not_reinserted(dsmr_db):
|
||||
assert _count_readings(SessionLocal) == 1
|
||||
|
||||
|
||||
def test_different_source_ids_are_independent(dsmr_db):
|
||||
"""Different id values from different telegrams produce separate rows."""
|
||||
def test_different_timestamps_are_independent(dsmr_db):
|
||||
"""Telegrams with different timestamps produce separate rows."""
|
||||
_, SessionLocal = dsmr_db
|
||||
settings = _make_settings(dsmr_sample_interval_s=10)
|
||||
|
||||
@@ -240,6 +240,24 @@ def test_different_source_ids_are_independent(dsmr_db):
|
||||
assert _count_readings(SessionLocal) == 2
|
||||
|
||||
|
||||
def test_telegram_id_collision_does_not_drop_new_data(dsmr_db):
|
||||
"""Regression: the telegram id overflows / gets reset to zero in DSMR firmware.
|
||||
Two DISTINCT telegrams (different timestamps) that happen to share the SAME
|
||||
telegram id must BOTH be stored — dedup must not depend on the telegram id."""
|
||||
_, SessionLocal = dsmr_db
|
||||
settings = _make_settings(dsmr_sample_interval_s=10)
|
||||
|
||||
first = {**_SAMPLE_TELEGRAM, "id": 0, "timestamp": "2026-06-23T12:16:00Z"}
|
||||
# Later telegram, id reset back to the same value after an overflow.
|
||||
second = {**_SAMPLE_TELEGRAM, "id": 0, "timestamp": "2026-06-23T12:16:10Z"}
|
||||
|
||||
_call_handle_message(first, settings, SessionLocal)
|
||||
_call_handle_message(second, settings, SessionLocal)
|
||||
|
||||
# Both must persist — the colliding telegram id must not cause a drop.
|
||||
assert _count_readings(SessionLocal) == 2
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 4. Missing source_id — still persisted with source_id=None
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -0,0 +1,92 @@
|
||||
"""Tests for restart-free DSMR subscription management (apply_dsmr_subscription).
|
||||
|
||||
These verify that toggling DSMR ingest / changing its topic / changing its sample
|
||||
interval via the config UI is reflected in the live MQTT subscription without an
|
||||
app restart. A fake MQTT manager is injected so no real broker is touched.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from app.services import dsmr_ingest
|
||||
|
||||
|
||||
class _FakeMqtt:
|
||||
def __init__(self) -> None:
|
||||
self.subscribe_calls: list[tuple[str, object]] = []
|
||||
self.unsubscribe_calls: list[str] = []
|
||||
|
||||
def subscribe(self, topic: str, handler) -> None:
|
||||
self.subscribe_calls.append((topic, handler))
|
||||
|
||||
def unsubscribe(self, topic: str) -> None:
|
||||
self.unsubscribe_calls.append(topic)
|
||||
|
||||
|
||||
def _settings(*, enabled: bool = True, topic: str = "dsmr/json", interval: int = 10):
|
||||
s = MagicMock()
|
||||
s.dsmr_ingest_enabled = enabled
|
||||
s.dsmr_mqtt_topic = topic
|
||||
s.dsmr_sample_interval_s = interval
|
||||
return s
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def fake_mqtt(monkeypatch):
|
||||
fake = _FakeMqtt()
|
||||
# apply_dsmr_subscription does `from app.integrations.mqtt import mqtt_manager`
|
||||
# at call time, so patching the module attribute is picked up.
|
||||
monkeypatch.setattr("app.integrations.mqtt.mqtt_manager", fake)
|
||||
# Reset (and auto-restore) the module-level "currently subscribed topic".
|
||||
monkeypatch.setattr(dsmr_ingest, "_current_dsmr_topic", None)
|
||||
return fake
|
||||
|
||||
|
||||
def test_enabled_subscribes_to_topic(fake_mqtt):
|
||||
dsmr_ingest.apply_dsmr_subscription(_settings(enabled=True, topic="dsmr/json"))
|
||||
|
||||
assert len(fake_mqtt.subscribe_calls) == 1
|
||||
assert fake_mqtt.subscribe_calls[0][0] == "dsmr/json"
|
||||
assert fake_mqtt.unsubscribe_calls == []
|
||||
assert dsmr_ingest._current_dsmr_topic == "dsmr/json"
|
||||
|
||||
|
||||
def test_disabled_after_enabled_unsubscribes(fake_mqtt):
|
||||
dsmr_ingest.apply_dsmr_subscription(_settings(enabled=True, topic="dsmr/json"))
|
||||
dsmr_ingest.apply_dsmr_subscription(_settings(enabled=False))
|
||||
|
||||
assert fake_mqtt.unsubscribe_calls == ["dsmr/json"]
|
||||
assert dsmr_ingest._current_dsmr_topic is None
|
||||
|
||||
|
||||
def test_topic_change_unsubscribes_old_subscribes_new(fake_mqtt):
|
||||
dsmr_ingest.apply_dsmr_subscription(_settings(topic="dsmr/json"))
|
||||
dsmr_ingest.apply_dsmr_subscription(_settings(topic="meter/dsmr"))
|
||||
|
||||
assert fake_mqtt.unsubscribe_calls == ["dsmr/json"]
|
||||
assert [t for t, _ in fake_mqtt.subscribe_calls] == ["dsmr/json", "meter/dsmr"]
|
||||
assert dsmr_ingest._current_dsmr_topic == "meter/dsmr"
|
||||
|
||||
|
||||
def test_reapply_same_topic_resubscribes_fresh_handler(fake_mqtt):
|
||||
# A changed sample interval must take effect — the handler is re-bound to a
|
||||
# fresh settings snapshot, so re-applying the same topic re-subscribes.
|
||||
dsmr_ingest.apply_dsmr_subscription(_settings(topic="dsmr/json", interval=10))
|
||||
dsmr_ingest.apply_dsmr_subscription(_settings(topic="dsmr/json", interval=20))
|
||||
|
||||
assert len(fake_mqtt.subscribe_calls) == 2
|
||||
assert fake_mqtt.unsubscribe_calls == [] # same topic, no churn
|
||||
handler1 = fake_mqtt.subscribe_calls[0][1]
|
||||
handler2 = fake_mqtt.subscribe_calls[1][1]
|
||||
assert handler1 is not handler2 # fresh closure carrying the new settings
|
||||
|
||||
|
||||
def test_disabled_when_never_enabled_is_noop(fake_mqtt):
|
||||
dsmr_ingest.apply_dsmr_subscription(_settings(enabled=False))
|
||||
|
||||
assert fake_mqtt.subscribe_calls == []
|
||||
assert fake_mqtt.unsubscribe_calls == []
|
||||
assert dsmr_ingest._current_dsmr_topic is None
|
||||
+48
-13
@@ -117,21 +117,21 @@ def test_dsmr_reading_columns(energy_db):
|
||||
assert "payload" in columns and not columns["payload"]["nullable"]
|
||||
|
||||
|
||||
def test_dsmr_reading_source_id_unique(energy_db):
|
||||
"""dsmr_reading.source_id must have a unique constraint."""
|
||||
def test_dsmr_reading_recorded_at_unique(energy_db):
|
||||
"""dsmr_reading.recorded_at is the UNIQUE de-dup key (telegram-id-independent)."""
|
||||
inspector = inspect(energy_db)
|
||||
unique_constraints = inspector.get_unique_constraints("dsmr_reading")
|
||||
unique_cols = [col for uc in unique_constraints for col in uc["column_names"]]
|
||||
assert "source_id" in unique_cols, "source_id must have a unique constraint"
|
||||
assert "recorded_at" in unique_cols, "recorded_at must have a unique constraint"
|
||||
|
||||
|
||||
def test_dsmr_reading_recorded_at_index(energy_db):
|
||||
"""dsmr_reading.recorded_at must have an index."""
|
||||
def test_dsmr_reading_source_id_not_unique(energy_db):
|
||||
"""dsmr_reading.source_id (telegram id) must NOT be unique — it overflows/resets,
|
||||
so it is kept only as a reference value and never relied on for dedup."""
|
||||
inspector = inspect(energy_db)
|
||||
indexes = {idx["name"]: idx for idx in inspector.get_indexes("dsmr_reading")}
|
||||
assert "ix_dsmr_reading_recorded_at" in indexes, (
|
||||
"ix_dsmr_reading_recorded_at missing from dsmr_reading"
|
||||
)
|
||||
unique_constraints = inspector.get_unique_constraints("dsmr_reading")
|
||||
unique_cols = [col for uc in unique_constraints for col in uc["column_names"]]
|
||||
assert "source_id" not in unique_cols, "source_id must NOT have a unique constraint"
|
||||
|
||||
|
||||
def test_energy_contract_columns(energy_db):
|
||||
@@ -268,6 +268,40 @@ def test_downgrade_removes_energy_tables(tmp_path: Path):
|
||||
engine.dispose()
|
||||
|
||||
|
||||
def test_dsmr_decouple_migration_downgrade_restores_source_id_unique(tmp_path: Path):
|
||||
"""Downgrading the telegram-id-decouple migration to 20260623_11 must restore the
|
||||
original constraints: source_id UNIQUE, recorded_at non-unique index."""
|
||||
db_path = tmp_path / "dsmr_decouple_downgrade.db"
|
||||
db_url = f"sqlite:///{db_path}"
|
||||
alembic_cfg = _make_app_alembic_config(db_url)
|
||||
|
||||
command.upgrade(alembic_cfg, "head")
|
||||
|
||||
# At head: recorded_at is unique, source_id is not.
|
||||
engine = create_engine(db_url, connect_args={"check_same_thread": False})
|
||||
inspector = inspect(engine)
|
||||
head_unique = [
|
||||
c for uc in inspector.get_unique_constraints("dsmr_reading") for c in uc["column_names"]
|
||||
]
|
||||
assert "recorded_at" in head_unique
|
||||
assert "source_id" not in head_unique
|
||||
engine.dispose()
|
||||
|
||||
# Downgrade just this migration.
|
||||
command.downgrade(alembic_cfg, "20260623_11_energy_tables")
|
||||
|
||||
engine = create_engine(db_url, connect_args={"check_same_thread": False})
|
||||
inspector = inspect(engine)
|
||||
pre_unique = [
|
||||
c for uc in inspector.get_unique_constraints("dsmr_reading") for c in uc["column_names"]
|
||||
]
|
||||
assert "source_id" in pre_unique, "source_id unique must be restored on downgrade"
|
||||
assert "recorded_at" not in pre_unique
|
||||
index_names = {idx["name"] for idx in inspector.get_indexes("dsmr_reading")}
|
||||
assert "ix_dsmr_reading_recorded_at" in index_names
|
||||
engine.dispose()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 2. ORM metadata checks (Base.metadata)
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -332,11 +366,12 @@ def test_energy_cost_period_fk_ondelete_restrict():
|
||||
)
|
||||
|
||||
|
||||
def test_dsmr_reading_source_id_unique_in_metadata():
|
||||
"""DsmrReading.source_id must be declared unique in ORM metadata."""
|
||||
def test_dsmr_reading_recorded_at_unique_in_metadata():
|
||||
"""DsmrReading.recorded_at must be the unique de-dup key in ORM metadata,
|
||||
and source_id must NOT be unique (decoupled from the telegram id)."""
|
||||
table = Base.metadata.tables["dsmr_reading"]
|
||||
col = table.columns["source_id"]
|
||||
assert col.unique, "source_id must be declared unique"
|
||||
assert table.columns["recorded_at"].unique, "recorded_at must be declared unique"
|
||||
assert not table.columns["source_id"].unique, "source_id must NOT be unique"
|
||||
|
||||
|
||||
def test_tibber_price_starts_at_unique_in_metadata():
|
||||
|
||||
@@ -107,6 +107,36 @@ def test_subscribe_when_connected_calls_paho_subscribe_immediately() -> None:
|
||||
mock_client.subscribe.assert_called_once_with("dsmr/json")
|
||||
|
||||
|
||||
def test_unsubscribe_removes_handler_from_registry() -> None:
|
||||
"""unsubscribe() must drop the handler so it is not re-subscribed on reconnect."""
|
||||
manager = MqttManager()
|
||||
manager.subscribe("dsmr/json", MagicMock())
|
||||
|
||||
manager.unsubscribe("dsmr/json")
|
||||
|
||||
assert "dsmr/json" not in manager._subscriptions
|
||||
|
||||
|
||||
def test_unsubscribe_when_connected_calls_paho_unsubscribe() -> None:
|
||||
"""unsubscribe() while connected must call client.unsubscribe(topic)."""
|
||||
manager = MqttManager()
|
||||
mock_client = MagicMock()
|
||||
manager._client = mock_client
|
||||
manager._connected = True
|
||||
manager.subscribe("dsmr/json", MagicMock())
|
||||
|
||||
manager.unsubscribe("dsmr/json")
|
||||
|
||||
mock_client.unsubscribe.assert_called_once_with("dsmr/json")
|
||||
|
||||
|
||||
def test_unsubscribe_unknown_topic_is_noop() -> None:
|
||||
"""unsubscribe() on a topic that was never registered must not raise."""
|
||||
manager = MqttManager()
|
||||
manager.unsubscribe("never/registered") # must not raise
|
||||
assert "never/registered" not in manager._subscriptions
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# on_message dispatch
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user