Files
LEDMatrix/test/web_interface/integration/test_config_flows.py
T
ChuckandClaude Opus 5 9ad7528c9b fix(config): stop same-second backups overwriting each other (#564)
* fix(config): stop same-second backups overwriting each other

A backup's version is its identity. save_config_atomic() hands the path
back, rollback_config(backup_version=...) looks that version up, and the
paired secrets backup is found by reusing the same string.

The version was stamped at second granularity, so two saves inside the
same second produced the same filename and the second shutil.copy2()
silently overwrote the first backup. The path a caller was still holding
then pointed at different content, and rolling back to it restored the
wrong config. A user saving twice in quick succession lost a restore
point with no error.

list_backups() made it worse. It parsed the version off Path.stem, which
drops only the last dot-component, so for config.json.backup.20240101_120000
parts was ['config', 'json', 'backup'] and parts[-2] was 'json' -- never
'backup'. The filename branch was unreachable: every backup fell through
to the mtime fallback and reported a second-granularity restamp of its
mtime rather than the name on disk, so a unique filename alone would not
have been enough for rollback to find the right version.

Stamp microseconds, and never overwrite an existing backup -- on a
collision bump a -N suffix rather than lose a restore point. Parse the
version off the exact glob prefix so it round-trips with the filename,
still reading the legacy second-granularity format so restore points that
predate this keep working.

Two tests had encoded the bug:

  - test_multiple_config_changes asserted a rollback produced plugin1=45
    with plugin2=15, a state no single backup ever held -- 45 was only in
    the second backup, 15 only in the first. It passed because the two
    saves collided onto one file, so the first version resolved to the
    second's content. Corrected to the state that backup actually holds.

  - test_backup_rotation asserted against a hardcoded max of 3 while
    setUp configured 5, and still passed: every save in its loop collapsed
    onto a single filename, so there was only ever one backup to count and
    rotation was never exercised. It now asks the manager for its limit
    and overshoots it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(config): fold collision suffix into ordering, close backup-path race

_parse_backup_version() stripped any trailing "-segment" unconditionally,
so a collision-suffixed backup parsed to the exact same timestamp as its
sibling and list_backups() had no deterministic way to order them. Only
strip the suffix when it's numeric, and fold it back in as extra
microseconds so same-tick collisions sort newest-first reliably.

_create_backup() also checked backup_path.exists() before shutil.copy2(),
which two concurrent callers can both pass for the same path -- the second
copy2() then silently destroys the first call's restore point. Reserve
each path (config and, when configured, secrets) with exclusive file
creation instead of a check-then-copy, retrying on a real conflict.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-09-12 16:13:50 -04:00

176 lines
7.6 KiB
Python

"""
Integration tests for configuration save/rollback flows.
"""
import unittest
import tempfile
import shutil
import json
from pathlib import Path
from src.config_manager_atomic import AtomicConfigManager, SaveResultStatus
from src.config_manager import ConfigManager
class TestConfigFlowsIntegration(unittest.TestCase):
"""Integration tests for configuration flows."""
def setUp(self):
"""Set up test fixtures."""
self.temp_dir = Path(tempfile.mkdtemp())
self.config_path = self.temp_dir / "config.json"
self.secrets_path = self.temp_dir / "secrets.json"
self.backup_dir = self.temp_dir / "backups"
# Create initial config
initial_config = {
"plugin1": {"enabled": True, "display_duration": 30},
"plugin2": {"enabled": False, "display_duration": 15}
}
with open(self.config_path, 'w') as f:
json.dump(initial_config, f)
# Initialize atomic config manager
self.atomic_manager = AtomicConfigManager(
config_path=str(self.config_path),
secrets_path=str(self.secrets_path),
backup_dir=str(self.backup_dir),
max_backups=5
)
# Initialize regular config manager
self.config_manager = ConfigManager()
# Override paths for testing
self.config_manager.config_path = self.config_path
self.config_manager.secrets_path = self.secrets_path
def tearDown(self):
"""Clean up test fixtures."""
shutil.rmtree(self.temp_dir)
def test_save_and_rollback_flow(self):
"""Test saving config and rolling back."""
# Load initial config
initial_config = self.config_manager.load_config()
self.assertIn("plugin1", initial_config)
# Make changes
new_config = initial_config.copy()
new_config["plugin1"]["display_duration"] = 60
new_config["plugin3"] = {"enabled": True, "display_duration": 20}
# Save with atomic manager
result = self.atomic_manager.save_config_atomic(new_config, create_backup=True)
self.assertEqual(result.status, SaveResultStatus.SUCCESS)
self.assertIsNotNone(result.backup_path)
# Verify config was saved
saved_config = self.config_manager.load_config()
self.assertEqual(saved_config["plugin1"]["display_duration"], 60)
self.assertIn("plugin3", saved_config)
# Rollback - extract version from backup path or use most recent
# The backup_path is a full path, but rollback_config expects a version string
# So we'll use None to get the most recent backup
rollback_success = self.atomic_manager.rollback_config(backup_version=None)
self.assertTrue(rollback_success)
# Verify config was rolled back
rolled_back_config = self.config_manager.load_config()
self.assertEqual(rolled_back_config["plugin1"]["display_duration"], 30)
self.assertNotIn("plugin3", rolled_back_config)
def test_backup_rotation(self):
"""Test that backup rotation works correctly."""
# setUp built the manager with max_backups=5; ask it rather than
# restating the number, which is how this drifted in the first place.
max_backups = self.atomic_manager.max_backups
# Overshoot the limit so rotation actually has something to remove.
for i in range(max_backups + 3):
config = {"test": f"value_{i}"}
result = self.atomic_manager.save_config_atomic(config, create_backup=True)
self.assertEqual(result.status, SaveResultStatus.SUCCESS)
# Rotation should have trimmed the excess, and kept exactly the limit.
#
# This used to assert against a hardcoded 3 while setUp configured 5,
# and still passed -- because every save in the loop landed in the same
# second and collapsed onto a single backup filename, so there was only
# ever one backup to count and rotation was never exercised at all.
backups = self.atomic_manager.list_backups()
self.assertEqual(max_backups, len(backups))
def test_validation_failure_triggers_rollback(self):
"""Test that validation failure triggers automatic rollback."""
# Create invalid config (this would fail validation in real scenario)
# For this test, we'll simulate by making save fail after write
initial_config = self.config_manager.load_config()
# Try to save (in real scenario, validation would fail)
# Here we'll just verify the atomic save mechanism works
new_config = initial_config.copy()
new_config["plugin1"]["display_duration"] = 60
result = self.atomic_manager.save_config_atomic(new_config, create_backup=True)
# If validation fails, the atomic save should rollback automatically
# (This would be handled by the validation step in the atomic save process)
self.assertEqual(result.status, SaveResultStatus.SUCCESS)
def test_multiple_config_changes(self):
"""Test multiple sequential config changes."""
config = self.config_manager.load_config()
# Make first change
config["plugin1"]["display_duration"] = 45
result1 = self.atomic_manager.save_config_atomic(config, create_backup=True)
self.assertEqual(result1.status, SaveResultStatus.SUCCESS)
# Make second change
config = self.config_manager.load_config()
config["plugin2"]["display_duration"] = 20
result2 = self.atomic_manager.save_config_atomic(config, create_backup=True)
self.assertEqual(result2.status, SaveResultStatus.SUCCESS)
# Verify both changes persisted
final_config = self.config_manager.load_config()
self.assertEqual(final_config["plugin1"]["display_duration"], 45)
self.assertEqual(final_config["plugin2"]["display_duration"], 20)
# Rollback to first change - get the backup version from the backup path
# Extract version from backup path (format: config.json.backup.YYYYMMDD_HHMMSS)
import os
backup_filename = os.path.basename(result1.backup_path)
# Extract timestamp part
if '.backup.' in backup_filename:
version = backup_filename.split('.backup.')[-1]
rollback_success = self.atomic_manager.rollback_config(backup_version=version)
else:
# Fallback: use most recent backup
rollback_success = self.atomic_manager.rollback_config(backup_version=None)
self.assertTrue(rollback_success)
# Verify rollback.
#
# result1's backup was taken *before* the first save, so it holds the
# config setUp wrote -- plugin1=30, plugin2=15 -- and neither change.
#
# This used to assert plugin1 == 45, a state no single backup ever held:
# 45 was only ever in result2's backup, and 15 only in result1's. It
# passed because both saves landed in the same second, so both backups
# were written to one filename and "result1's version" resolved to
# result2's content. Once the backup id became unique the rollback
# started returning the version actually asked for, and the assertion
# -- not the rollback -- was what had to change.
rolled_back_config = self.config_manager.load_config()
self.assertEqual(rolled_back_config["plugin1"]["display_duration"], 30)
self.assertEqual(rolled_back_config["plugin2"]["display_duration"], 15)
if __name__ == '__main__':
unittest.main()