diff --git a/src/seriousdb/cache.py b/src/seriousdb/cache.py index bca4180..9c1d9c6 100644 --- a/src/seriousdb/cache.py +++ b/src/seriousdb/cache.py @@ -299,15 +299,13 @@ def load(self, filename: str) -> None: "Database file %s does not exist; creating a new database", filename, ) - self.db = _write_default(filename) + db = _write_default(filename) else: try: with open(filename, "rb") as f: - self.db = json.loads(f.read().decode()) - if not isinstance(self.db, dict): - raise TypeError( - f"expected dict, got {type(self.db).__name__}" - ) + db = json.loads(f.read().decode()) + if not isinstance(db, dict): + raise TypeError(f"expected dict, got {type(db).__name__}") logger.info("Loaded database from %s", filename) except (json.JSONDecodeError, UnicodeDecodeError, TypeError) as e: @@ -319,13 +317,18 @@ def load(self, filename: str) -> None: e, backup, ) - self.db = _write_default(filename) + db = _write_default(filename) + + wal = WriteAheadLog(f"{filename}.wal") + replayed = wal.replay() + + for entry in replayed: + entry.apply(db) + + self.db = db self.filename = filename - self.wal = WriteAheadLog(f"{filename}.wal") - replayed = self.wal.replay() + self.wal = wal self._writes_since_compact = len(replayed) - for entry in replayed: - entry.apply(self._require_db()) def flush(self) -> None: """No-op, kept for backward compatibility. diff --git a/tests/test_cache.py b/tests/test_cache.py index 2ca8ebb..6c4e71d 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -9,7 +9,7 @@ from seriousdb.cache import COMPACTION_THRESHOLD, Cache from seriousdb.exceptions import ResourceNotFoundError, ServiceUnavailableError -from seriousdb.wal import SetEntry +from seriousdb.wal import SetEntry, WalEntry, WriteAheadLog @pytest.fixture @@ -144,6 +144,89 @@ def test_load_rejects_non_object_json(db_path): assert list(db_path.parent.glob(".sdb.corrupt-*")) +def test_load_failure_does_not_leave_cache_partially_initialized(db_path, monkeypatch): + """A transient WAL replay failure must leave load retryable without data loss.""" + original = Cache() + original.load(str(db_path)) + original.insert("committed", "important") + + cache = Cache() + replay = WriteAheadLog.replay + calls = 0 + + def fail_once(wal): + nonlocal calls + calls += 1 + if calls == 1: + raise OSError("simulated transient WAL read failure") + return replay(wal) + + monkeypatch.setattr(WriteAheadLog, "replay", fail_once) + + with pytest.raises(OSError, match="transient WAL read failure"): + cache.load(str(db_path)) + + # A failed load must not look like successful initialization. + assert cache.db is None + assert cache.filename is None + assert cache.wal is None + assert cache._writes_since_compact == 0 + + # Retry must recover the previously committed write. + cache.load(str(db_path)) + assert cache.db == {"committed": "important"} + assert cache._writes_since_compact == 1 + + +def test_load_failure_preserves_committed_wal_entries(tmp_path, monkeypatch): + """A failed load must not lose committed WAL entries.""" + filename = str(tmp_path / "test.db") + wal_filename = f"{filename}.wal" + + # Create a snapshot without the committed WAL entry. + with open(filename, "w", encoding="utf-8") as f: + json.dump({"existing": "snapshot data"}, f) + + # Append a durable WAL entry using the actual WAL API. + wal = WriteAheadLog(wal_filename) + wal.append(SetEntry("committed", "important")) + + original_replay = WriteAheadLog.replay + calls = 0 + + def fail_once(wal) -> list[WalEntry]: + nonlocal calls + calls += 1 + if calls == 1: + raise OSError("simulated transient WAL read failure") + return original_replay(wal) + + monkeypatch.setattr(WriteAheadLog, "replay", fail_once) + + cache = Cache() + + with pytest.raises(OSError, match="transient WAL read failure"): + cache.load(filename) + + # The failed load must not leave the cache partially initialized. + assert cache.db is None + assert cache.filename is None + assert cache.wal is None + + # Retrying must recover the committed entry. + cache.load(filename) + assert cache.select("committed") == "important" + + # Force compaction with subsequent writes. + for i in range(COMPACTION_THRESHOLD): + cache.insert(f"extra-{i}", str(i)) + + # A fresh instance must still recover the committed value. + recovered = Cache() + recovered.load(filename) + assert recovered.select("committed") == "important" + + def test_require_db_returns_loaded_database(cache): assert cache._require_db() is cache.db