mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-17 19:36:28 +02:00
fix(backup): reconnect the MQTT relay after restoring mqtt_* settings (#2656)
The relay reads its broker config once, when configure() is called — which is
why the settings PUT handler reconfigures it after writing those rows
(api/routes/settings.py:246). The restore wrote the rows and stopped there, so
the relay stayed on the pre-restore broker until the next backend restart while
the UI showed the restored values: the one way a settings restore could look
applied without being applied.
_restore_settings now reports the keys it actually wrote, and run_restore
reconfigures the relay from the committed rows when any of them is an mqtt_ one.
Three details worth keeping:
* it runs after the commit, because configure() drops the connection and
rebuilds it — not something to do on values a later failure could roll back;
* it is keyed on written, not merely present: a key skipped for overwrite=off
or by the credential blocklist must not trigger a reconnect;
* mqtt_password is never restorable, so configure() gets the row already in the
database and an unchanged broker keeps working.
A broker that refuses the new config is noted on the settings tally ("restart
Bambuddy") rather than failing the restore, matching the PUT handler's
best-effort handling of the same call.
This commit is contained in:
@@ -1090,6 +1090,98 @@ class TestMutex:
|
||||
assert "restore is currently running" in result["message"]
|
||||
|
||||
|
||||
class TestMqttRelayReconfigure:
|
||||
"""Restoring mqtt_* rows has to reach the live relay, not just the table."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reconfigures_from_the_committed_rows(self, db_session):
|
||||
db_session.add(Settings(key="mqtt_enabled", value="true"))
|
||||
db_session.add(Settings(key="mqtt_broker", value="restored.local"))
|
||||
db_session.add(Settings(key="mqtt_port", value="8883"))
|
||||
db_session.add(Settings(key="mqtt_use_tls", value="true"))
|
||||
# Never restorable (credential blocklist), so it comes from the row that
|
||||
# was already there.
|
||||
db_session.add(Settings(key="mqtt_password", value="kept"))
|
||||
await db_session.commit()
|
||||
tally = _CategoryTally()
|
||||
relay = MagicMock()
|
||||
relay.configure = AsyncMock(return_value=True)
|
||||
|
||||
with patch("backend.app.services.mqtt_relay.mqtt_relay", relay):
|
||||
await _service()._reconfigure_mqtt_relay(db_session, {"mqtt_broker"}, tally)
|
||||
|
||||
relay.configure.assert_awaited_once()
|
||||
sent = relay.configure.await_args.args[0]
|
||||
assert sent["mqtt_enabled"] is True
|
||||
assert sent["mqtt_broker"] == "restored.local"
|
||||
assert sent["mqtt_port"] == 8883
|
||||
assert sent["mqtt_use_tls"] is True
|
||||
assert sent["mqtt_password"] == "kept"
|
||||
assert sent["mqtt_topic_prefix"] == "bambuddy"
|
||||
assert tally.notes == []
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_no_reconnect_when_no_mqtt_key_was_written(self, db_session):
|
||||
"""configure() tears the connection down, so don't call it for a theme change."""
|
||||
tally = _CategoryTally()
|
||||
relay = MagicMock()
|
||||
relay.configure = AsyncMock()
|
||||
|
||||
with patch("backend.app.services.mqtt_relay.mqtt_relay", relay):
|
||||
await _service()._reconfigure_mqtt_relay(db_session, {"currency", "theme"}, tally)
|
||||
|
||||
relay.configure.assert_not_awaited()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_broker_failure_is_noted_not_fatal(self, db_session):
|
||||
tally = _CategoryTally()
|
||||
relay = MagicMock()
|
||||
relay.configure = AsyncMock(side_effect=OSError("no route to broker"))
|
||||
|
||||
with patch("backend.app.services.mqtt_relay.mqtt_relay", relay):
|
||||
await _service()._reconfigure_mqtt_relay(db_session, {"mqtt_enabled"}, tally)
|
||||
|
||||
assert any("restart Bambuddy" in note for note in tally.notes)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_restore_settings_reports_the_keys_it_wrote(self, db_session):
|
||||
db_session.add(Settings(key="mqtt_broker", value="old.local"))
|
||||
await db_session.commit()
|
||||
written: set[str] = set()
|
||||
payload = {
|
||||
"settings": {
|
||||
"mqtt_broker": "new.local",
|
||||
"currency": "EUR",
|
||||
"mqtt_password": "leaked",
|
||||
"auth_enabled": "false",
|
||||
}
|
||||
}
|
||||
|
||||
await _service()._restore_settings(
|
||||
db_session, payload, overwrite=True, tally=_CategoryTally(), keys_written=written
|
||||
)
|
||||
|
||||
# Skipped keys are not "written", or a blocked mqtt_password would
|
||||
# trigger a pointless reconnect.
|
||||
assert written == {"mqtt_broker", "currency"}
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_keys_skipped_for_overwrite_off_are_not_reported(self, db_session):
|
||||
db_session.add(Settings(key="mqtt_broker", value="old.local"))
|
||||
await db_session.commit()
|
||||
written: set[str] = set()
|
||||
|
||||
await _service()._restore_settings(
|
||||
db_session,
|
||||
{"settings": {"mqtt_broker": "new.local"}},
|
||||
overwrite=False,
|
||||
tally=_CategoryTally(),
|
||||
keys_written=written,
|
||||
)
|
||||
|
||||
assert written == set()
|
||||
|
||||
|
||||
class TestApplyOrdering:
|
||||
"""_apply must not hold SQLite's write transaction across the MQTT phase."""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user