diff --git a/docs/plans/001-search-engine-api.md b/docs/plans/001-search-engine-api.md index 2ca95ed..92bb7df 100644 --- a/docs/plans/001-search-engine-api.md +++ b/docs/plans/001-search-engine-api.md @@ -190,7 +190,7 @@ git commit -m "chore: scaffold Python service" - Consumes: `Settings.database_path` - Produces: `Database(path: Path)`, `Database.connect()`, `Database.transaction()` and `Database.migrate()` -- [ ] **Step 1: Write failing migration and constraint tests** +- [x] **Step 1: Write failing migration and constraint tests** ```python def test_migrate_creates_all_tables(database: Database) -> None: @@ -209,13 +209,13 @@ def test_one_canonical_path_per_document(database: Database) -> None: `EXPECTED_TABLES` names all thirteen spec tables. Add tests for foreign keys, unique normalized collection names, unique directory rules, and cascade behavior. -- [ ] **Step 2: Run the focused tests and observe failure** +- [x] **Step 2: Run the focused tests and observe failure** Run: `uv run pytest tests/storage/test_database.py tests/storage/test_migrations.py -v` Expected: FAIL because `Database` and migration 001 do not exist. -- [ ] **Step 3: Implement connection and transaction policy** +- [x] **Step 3: Implement connection and transaction policy** Every connection sets `row_factory=sqlite3.Row`, `PRAGMA foreign_keys=ON`, `PRAGMA journal_mode=WAL`, and `PRAGMA busy_timeout=5000`. `transaction()` begins `IMMEDIATE`, commits on success, rolls back on exception, and closes the connection. @@ -260,7 +260,7 @@ CREATE TABLE sessions(token_hash TEXT PRIMARY KEY, csrf_hash TEXT NOT NULL, Migration application is ordered, transactional, records each version, and refuses duplicate version numbers. -- [ ] **Step 4: Run tests, lint, typecheck, and commit** +- [x] **Step 4: Run tests, lint, typecheck, and commit** Run: `uv run pytest tests/storage -v && uv run ruff check . && uv run mypy src tests` diff --git a/src/recoll_modern/storage/__init__.py b/src/recoll_modern/storage/__init__.py new file mode 100644 index 0000000..b2270d7 --- /dev/null +++ b/src/recoll_modern/storage/__init__.py @@ -0,0 +1,5 @@ +"""SQLite persistence primitives and repositories.""" + +from recoll_modern.storage.database import Database + +__all__ = ["Database"] diff --git a/src/recoll_modern/storage/database.py b/src/recoll_modern/storage/database.py new file mode 100644 index 0000000..9db0bbd --- /dev/null +++ b/src/recoll_modern/storage/database.py @@ -0,0 +1,100 @@ +"""SQLite connection, transaction, and migration support.""" + +import sqlite3 +from collections.abc import Generator +from contextlib import contextmanager +from datetime import UTC, datetime +from importlib.resources import files +from pathlib import Path + + +class Database: + """Provide consistently configured SQLite connections for one database file.""" + + def __init__(self, path: Path) -> None: + self.path = path + + def connect(self) -> sqlite3.Connection: + """Open a connection with the service's SQLite safety policy.""" + connection = sqlite3.connect(self.path) + connection.row_factory = sqlite3.Row + connection.execute("PRAGMA foreign_keys = ON") + connection.execute("PRAGMA journal_mode = WAL") + connection.execute("PRAGMA busy_timeout = 5000") + return connection + + @contextmanager + def transaction(self) -> Generator[sqlite3.Connection]: + """Yield an immediate transaction that commits or rolls back atomically.""" + connection = self.connect() + try: + connection.execute("BEGIN IMMEDIATE") + yield connection + except BaseException: + connection.rollback() + raise + else: + connection.commit() + finally: + connection.close() + + def migrate(self) -> None: + """Apply every pending migration as one immediate transaction.""" + migrations = sorted(_migration_scripts(), key=lambda migration: migration[0]) + _ensure_unique_versions(migrations) + + connection = self.connect() + try: + applied_versions = _applied_versions(connection) + pending = [ + migration for migration in migrations if migration[0] not in applied_versions + ] + if not pending: + return + + applied_at = datetime.now(UTC).isoformat() + script = ["BEGIN IMMEDIATE;"] + for version, migration_sql in pending: + script.append(migration_sql) + script.append( + "INSERT INTO schema_migrations(version, applied_at) " + f"VALUES ({version}, '{applied_at}');" + ) + script.append("COMMIT;") + connection.executescript("\n".join(script)) + except BaseException: + connection.rollback() + raise + finally: + connection.close() + + +def _migration_scripts() -> list[tuple[int, str]]: + """Read numbered SQL migrations bundled with the storage package.""" + migrations: list[tuple[int, str]] = [] + for resource in files("recoll_modern.storage.migrations").iterdir(): + if not resource.name.endswith(".sql"): + continue + version_text, separator, _ = resource.name.partition("_") + if not separator or not version_text.isdigit(): + raise ValueError(f"invalid migration filename: {resource.name}") + migrations.append((int(version_text), resource.read_text(encoding="utf-8"))) + return migrations + + +def _ensure_unique_versions(migrations: list[tuple[int, str]]) -> None: + """Reject ambiguous migration histories before modifying the database.""" + versions = [version for version, _ in migrations] + for version in versions: + if versions.count(version) > 1: + raise ValueError(f"duplicate migration version: {version}") + + +def _applied_versions(connection: sqlite3.Connection) -> set[int]: + """Return applied versions, treating a new database as having none.""" + try: + return {int(row[0]) for row in connection.execute("SELECT version FROM schema_migrations")} + except sqlite3.OperationalError as error: + if "no such table: schema_migrations" not in str(error): + raise + return set() diff --git a/src/recoll_modern/storage/migrations/001_initial.sql b/src/recoll_modern/storage/migrations/001_initial.sql new file mode 100644 index 0000000..5df03ea --- /dev/null +++ b/src/recoll_modern/storage/migrations/001_initial.sql @@ -0,0 +1,34 @@ +CREATE TABLE schema_migrations(version INTEGER PRIMARY KEY, applied_at TEXT NOT NULL); +CREATE TABLE collections(id TEXT PRIMARY KEY, name TEXT NOT NULL, name_key TEXT NOT NULL UNIQUE, + description TEXT, created_at TEXT NOT NULL, updated_at TEXT NOT NULL); +CREATE TABLE documents(content_hash TEXT PRIMARY KEY, byte_size INTEGER NOT NULL CHECK(byte_size >= 0), + state TEXT NOT NULL CHECK(state IN ('present','missing')), created_at TEXT NOT NULL, updated_at TEXT NOT NULL); +CREATE TABLE document_paths(id INTEGER PRIMARY KEY, document_hash TEXT NOT NULL REFERENCES documents(content_hash) ON DELETE CASCADE, + relative_path TEXT NOT NULL UNIQUE, is_canonical INTEGER NOT NULL CHECK(is_canonical IN (0,1)), + last_seen_at TEXT NOT NULL, missing_at TEXT); +CREATE UNIQUE INDEX one_canonical_path ON document_paths(document_hash) WHERE is_canonical = 1; +CREATE TABLE direct_memberships(collection_id TEXT NOT NULL REFERENCES collections(id) ON DELETE CASCADE, + document_hash TEXT NOT NULL REFERENCES documents(content_hash) ON DELETE CASCADE, created_at TEXT NOT NULL, + PRIMARY KEY(collection_id, document_hash)); +CREATE TABLE directory_rules(id TEXT PRIMARY KEY, collection_id TEXT NOT NULL REFERENCES collections(id) ON DELETE CASCADE, + relative_path TEXT NOT NULL, created_at TEXT NOT NULL, updated_at TEXT NOT NULL, UNIQUE(collection_id, relative_path)); +CREATE TABLE directory_rule_exclusions(rule_id TEXT NOT NULL REFERENCES directory_rules(id) ON DELETE CASCADE, + relative_path TEXT NOT NULL, PRIMARY KEY(rule_id, relative_path)); +CREATE TABLE rule_memberships(rule_id TEXT NOT NULL REFERENCES directory_rules(id) ON DELETE CASCADE, + document_hash TEXT NOT NULL REFERENCES documents(content_hash) ON DELETE CASCADE, last_seen_at TEXT NOT NULL, + PRIMARY KEY(rule_id, document_hash)); +CREATE TABLE membership_exclusions(collection_id TEXT NOT NULL REFERENCES collections(id) ON DELETE CASCADE, + document_hash TEXT NOT NULL REFERENCES documents(content_hash) ON DELETE CASCADE, created_at TEXT NOT NULL, + PRIMARY KEY(collection_id, document_hash)); +CREATE TABLE jobs(id TEXT PRIMARY KEY, kind TEXT NOT NULL, + state TEXT NOT NULL CHECK(state IN ('queued','running','completed','completed_with_errors','failed')), + request_json TEXT NOT NULL, fail_on_file_error INTEGER NOT NULL CHECK(fail_on_file_error IN (0,1)), + progress INTEGER NOT NULL DEFAULT 0, total INTEGER NOT NULL DEFAULT 0, + error_code TEXT, error_message TEXT, created_at TEXT NOT NULL, started_at TEXT, finished_at TEXT); +CREATE TABLE job_file_errors(job_id TEXT NOT NULL REFERENCES jobs(id) ON DELETE CASCADE, + relative_path TEXT NOT NULL, code TEXT NOT NULL, message TEXT NOT NULL, PRIMARY KEY(job_id, relative_path)); +CREATE TABLE users(id TEXT PRIMARY KEY, username TEXT NOT NULL, username_key TEXT NOT NULL UNIQUE, + password_hash TEXT NOT NULL, created_at TEXT NOT NULL, disabled_at TEXT); +CREATE TABLE sessions(token_hash TEXT PRIMARY KEY, csrf_hash TEXT NOT NULL, + user_id TEXT NOT NULL REFERENCES users(id) ON DELETE CASCADE, expires_at TEXT NOT NULL, + created_at TEXT NOT NULL, last_seen_at TEXT NOT NULL); diff --git a/src/recoll_modern/storage/migrations/__init__.py b/src/recoll_modern/storage/migrations/__init__.py new file mode 100644 index 0000000..2b9d5ce --- /dev/null +++ b/src/recoll_modern/storage/migrations/__init__.py @@ -0,0 +1 @@ +"""Bundled forward-only SQLite migrations.""" diff --git a/tests/storage/test_database.py b/tests/storage/test_database.py new file mode 100644 index 0000000..5badc94 --- /dev/null +++ b/tests/storage/test_database.py @@ -0,0 +1,71 @@ +"""Tests for SQLite connection and transaction behavior.""" + +import sqlite3 +from pathlib import Path + +import pytest + +from recoll_modern.storage.database import Database + + +@pytest.fixture +def database(tmp_path: Path) -> Database: + """Provide an isolated database for each test.""" + return Database(tmp_path / "recoll-modern.sqlite3") + + +def test_connect_configures_sqlite_policy(database: Database) -> None: + """Fail if a connection stops enforcing the required SQLite safeguards.""" + connection = database.connect() + try: + assert connection.row_factory is sqlite3.Row + assert connection.execute("PRAGMA foreign_keys").fetchone()[0] == 1 + assert connection.execute("PRAGMA journal_mode").fetchone()[0] == "wal" + assert connection.execute("PRAGMA busy_timeout").fetchone()[0] == 5000 + finally: + connection.close() + + +def test_transaction_commits_and_closes_connection(database: Database) -> None: + """Fail if a successful transaction loses data or leaks its connection.""" + database.migrate() + with database.transaction() as transaction_connection: + transaction_connection.execute( + "INSERT INTO users VALUES (?, ?, ?, ?, ?, ?)", + ("u1", "Ada", "ada", "hash", "2026-09-18T00:00:00+00:00", None), + ) + + check = database.connect() + try: + assert check.execute("SELECT username FROM users").fetchone()[0] == "Ada" + finally: + check.close() + + assert transaction_connection is not None + with pytest.raises(sqlite3.ProgrammingError): + transaction_connection.execute("SELECT 1") + + +def test_transaction_rolls_back_and_closes_connection(database: Database) -> None: + """Fail if an exception leaves a partial write or an open connection behind.""" + database.migrate() + transaction_connection: sqlite3.Connection | None = None + + with pytest.raises(RuntimeError, match="abort"): + with database.transaction() as current_connection: + transaction_connection = current_connection + current_connection.execute( + "INSERT INTO users VALUES (?, ?, ?, ?, ?, ?)", + ("u1", "Ada", "ada", "hash", "2026-09-18T00:00:00+00:00", None), + ) + raise RuntimeError("abort") + + check = database.connect() + try: + assert check.execute("SELECT COUNT(*) FROM users").fetchone()[0] == 0 + finally: + check.close() + + assert transaction_connection is not None + with pytest.raises(sqlite3.ProgrammingError): + transaction_connection.execute("SELECT 1") diff --git a/tests/storage/test_migrations.py b/tests/storage/test_migrations.py new file mode 100644 index 0000000..5390c39 --- /dev/null +++ b/tests/storage/test_migrations.py @@ -0,0 +1,267 @@ +"""Tests for the initial SQLite schema migration.""" + +import sqlite3 +from pathlib import Path + +import pytest + +from recoll_modern.storage import database as database_module +from recoll_modern.storage.database import Database + +EXPECTED_TABLES = { + "schema_migrations", + "collections", + "documents", + "document_paths", + "direct_memberships", + "directory_rules", + "directory_rule_exclusions", + "rule_memberships", + "membership_exclusions", + "jobs", + "job_file_errors", + "users", + "sessions", +} + + +@pytest.fixture +def database(tmp_path: Path) -> Database: + """Provide an isolated database for each test.""" + return Database(tmp_path / "recoll-modern.sqlite3") + + +def test_migrate_creates_all_tables_and_records_version(database: Database) -> None: + """Fail if initial migration omits schema or is not recorded exactly once.""" + database.migrate() + database.migrate() + + connection = database.connect() + try: + names = { + row[0] + for row in connection.execute("SELECT name FROM sqlite_master WHERE type = 'table'") + } + versions = [row[0] for row in connection.execute("SELECT version FROM schema_migrations")] + finally: + connection.close() + + assert EXPECTED_TABLES <= names + assert versions == [1] + + +def test_one_canonical_path_per_document(database: Database) -> None: + """Fail if the partial unique index allows two canonical paths per document.""" + database.migrate() + _seed_document_with_two_paths(database) + + with pytest.raises(sqlite3.IntegrityError): + with database.transaction() as connection: + connection.execute("UPDATE document_paths SET is_canonical = 1 WHERE id = 1") + connection.execute("UPDATE document_paths SET is_canonical = 1 WHERE id = 2") + + connection = database.connect() + try: + canonical_count = connection.execute( + "SELECT COUNT(*) FROM document_paths WHERE is_canonical = 1" + ).fetchone()[0] + finally: + connection.close() + assert canonical_count == 0 + + +def test_foreign_keys_reject_orphaned_document_path(database: Database) -> None: + """Fail if a document path can refer to a non-existent document.""" + database.migrate() + + with pytest.raises(sqlite3.IntegrityError): + with database.transaction() as connection: + connection.execute( + "INSERT INTO document_paths VALUES (?, ?, ?, ?, ?, ?)", + (1, "missing", "books/a.pdf", 1, "2026-09-18T00:00:00+00:00", None), + ) + + +def test_collection_name_keys_are_unique(database: Database) -> None: + """Fail if two application-normalized collection names can coexist.""" + database.migrate() + + with pytest.raises(sqlite3.IntegrityError): + with database.transaction() as connection: + _insert_collection(connection, "c1", "Café", "café") + _insert_collection(connection, "c2", "CAFÉ", "café") + + +def test_directory_rules_are_unique_within_a_collection(database: Database) -> None: + """Fail if a collection can contain the same directory rule twice.""" + database.migrate() + + with pytest.raises(sqlite3.IntegrityError): + with database.transaction() as connection: + _insert_collection(connection, "c1", "Books", "books") + _insert_directory_rule(connection, "r1", "c1", "books") + _insert_directory_rule(connection, "r2", "c1", "books") + + +def test_deletes_cascade_to_related_rows(database: Database) -> None: + """Fail if removing a collection leaves membership or rule records behind.""" + database.migrate() + + with database.transaction() as connection: + _insert_collection(connection, "c1", "Books", "books") + _insert_document(connection, "hash") + connection.execute( + "INSERT INTO direct_memberships VALUES (?, ?, ?)", + ("c1", "hash", "2026-09-18T00:00:00+00:00"), + ) + _insert_directory_rule(connection, "r1", "c1", "books") + connection.execute( + "INSERT INTO directory_rule_exclusions VALUES (?, ?)", ("r1", "books/private") + ) + connection.execute( + "INSERT INTO rule_memberships VALUES (?, ?, ?)", + ("r1", "hash", "2026-09-18T00:00:00+00:00"), + ) + connection.execute( + "INSERT INTO membership_exclusions VALUES (?, ?, ?)", + ("c1", "hash", "2026-09-18T00:00:00+00:00"), + ) + connection.execute("DELETE FROM collections WHERE id = ?", ("c1",)) + + connection = database.connect() + try: + for table in ( + "direct_memberships", + "directory_rules", + "directory_rule_exclusions", + "rule_memberships", + "membership_exclusions", + ): + assert connection.execute(f"SELECT COUNT(*) FROM {table}").fetchone()[0] == 0 + assert connection.execute("SELECT COUNT(*) FROM documents").fetchone()[0] == 1 + finally: + connection.close() + + +def test_migrations_are_atomic(database: Database, monkeypatch: pytest.MonkeyPatch) -> None: + """Fail if a bad later migration leaves earlier schema changes committed.""" + monkeypatch.setattr( + database_module, + "_migration_scripts", + lambda: [ + ( + 1, + "CREATE TABLE schema_migrations(" + "version INTEGER PRIMARY KEY, applied_at TEXT NOT NULL);" + "CREATE TABLE first_change(id INTEGER PRIMARY KEY);", + ), + (2, "CREATE TABLE broken_change(id INTEGER PRIMARY KEY); INVALID SQL;"), + ], + ) + + with pytest.raises(sqlite3.OperationalError): + database.migrate() + + connection = database.connect() + try: + names = { + row[0] + for row in connection.execute("SELECT name FROM sqlite_master WHERE type = 'table'") + } + finally: + connection.close() + assert "first_change" not in names + assert "broken_change" not in names + + +def test_migrations_apply_in_version_order( + database: Database, monkeypatch: pytest.MonkeyPatch +) -> None: + """Fail if an out-of-order resource list cannot establish its prerequisite schema.""" + monkeypatch.setattr( + database_module, + "_migration_scripts", + lambda: [ + (2, "CREATE TABLE second_change(id INTEGER PRIMARY KEY);"), + ( + 1, + "CREATE TABLE schema_migrations(" + "version INTEGER PRIMARY KEY, applied_at TEXT NOT NULL);" + "CREATE TABLE first_change(id INTEGER PRIMARY KEY);", + ), + ], + ) + + database.migrate() + + connection = database.connect() + try: + versions = [row[0] for row in connection.execute("SELECT version FROM schema_migrations")] + finally: + connection.close() + assert versions == [1, 2] + + +def test_migrations_refuse_duplicate_versions( + database: Database, monkeypatch: pytest.MonkeyPatch +) -> None: + """Fail if two scripts with one version could be applied ambiguously.""" + monkeypatch.setattr( + database_module, + "_migration_scripts", + lambda: [(1, "SELECT 1;"), (1, "SELECT 1;")], + ) + + with pytest.raises(ValueError, match="duplicate migration version: 1"): + database.migrate() + + +def _seed_document_with_two_paths(database: Database) -> None: + with database.transaction() as connection: + _insert_document(connection, "hash") + connection.execute( + "INSERT INTO document_paths VALUES (?, ?, ?, ?, ?, ?)", + (1, "hash", "books/a.pdf", 0, "2026-09-18T00:00:00+00:00", None), + ) + connection.execute( + "INSERT INTO document_paths VALUES (?, ?, ?, ?, ?, ?)", + (2, "hash", "books/b.pdf", 0, "2026-09-18T00:00:00+00:00", None), + ) + + +def _insert_collection( + connection: sqlite3.Connection, collection_id: str, name: str, name_key: str +) -> None: + connection.execute( + "INSERT INTO collections VALUES (?, ?, ?, ?, ?, ?)", + ( + collection_id, + name, + name_key, + None, + "2026-09-18T00:00:00+00:00", + "2026-09-18T00:00:00+00:00", + ), + ) + + +def _insert_document(connection: sqlite3.Connection, content_hash: str) -> None: + connection.execute( + "INSERT INTO documents VALUES (?, ?, ?, ?, ?)", + (content_hash, 1, "present", "2026-09-18T00:00:00+00:00", "2026-09-18T00:00:00+00:00"), + ) + + +def _insert_directory_rule( + connection: sqlite3.Connection, rule_id: str, collection_id: str, relative_path: str +) -> None: + connection.execute( + "INSERT INTO directory_rules VALUES (?, ?, ?, ?, ?)", + ( + rule_id, + collection_id, + relative_path, + "2026-09-18T00:00:00+00:00", + "2026-09-18T00:00:00+00:00", + ), + )