From fc729a241436b2009c0c8dcc5cde4a4488d0889e Mon Sep 17 00:00:00 2001 From: Javanaut Date: Sat, 11 Apr 2026 16:04:54 +0200 Subject: [PATCH] Opt database bootstrapping --- SCRATCHPAD.md | 10 +---- src/ffx/database.py | 18 +++++++- tests/unit/test_database.py | 83 +++++++++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 11 deletions(-) create mode 100644 tests/unit/test_database.py diff --git a/SCRATCHPAD.md b/SCRATCHPAD.md index fa4d84b..a1ba86a 100644 --- a/SCRATCHPAD.md +++ b/SCRATCHPAD.md @@ -12,6 +12,7 @@ - The CLI root now lazy-loads heavy runtime dependencies so lightweight commands such as `version`, `help`, `configure_workstation`, and `upgrade` stay import-light. - Shared CLI defaults for container/output tokens now live outside [`src/ffx/ffx_controller.py`](/home/osgw/.local/src/codex/ffx/src/ffx/ffx_controller.py), and a focused unit test locks in the lazy-import contract. - `FileProperties` now uses one cached `ffprobe -show_format -show_streams -of json` call per source file, and the combined payload was confirmed against the Dragonball asset to satisfy both previous probe call sites fully. +- Database startup now bootstraps schema only when required tables are actually missing, while version enforcement still runs on ordinary DB-backed context creation. - FFX logger setup now reuses named handlers, and fallback logger access no longer mutates handlers in ordinary constructors and helpers. - The process wrapper now uses `subprocess.run(...)` with centralized command formatting plus stable timeout and missing-command error mapping. - Active ORM controllers now use single-query accessors instead of paired `count()` plus `first()` lookups. @@ -103,15 +104,6 @@ - Cleaner runtime output. - Less warning noise during dry-run maintenance commands. -9. Database startup always runs schema creation and version checks -- [`src/ffx/database.py`](/home/osgw/.local/src/codex/ffx/src/ffx/database.py) runs `Base.metadata.create_all(...)` and version checks every time a DB-backed context is created. -- Optimization: - - Measure startup cost and consider separating bootstrapping from ordinary command execution. - - Keep schema migration/version enforcement explicit. -- Expected value: - - Faster command startup. - - Clearer operational boundaries. - ## Open - Should optimization work focus first on operator-perceived latency, internal maintainability, or correctness-risk cleanup that also has performance upside? diff --git a/src/ffx/database.py b/src/ffx/database.py index 10430e3..3d5e551 100644 --- a/src/ffx/database.py +++ b/src/ffx/database.py @@ -1,6 +1,6 @@ import os, click -from sqlalchemy import create_engine +from sqlalchemy import create_engine, inspect from sqlalchemy.orm import sessionmaker # Import the full model package so SQLAlchemy registers every mapped class @@ -14,6 +14,7 @@ from ffx.constants import DATABASE_VERSION DATABASE_VERSION_KEY = 'database_version' +EXPECTED_TABLE_NAMES = set(Base.metadata.tables.keys()) class DatabaseVersionException(Exception): def __init__(self, errorMessage): @@ -37,7 +38,7 @@ def databaseContext(databasePath: str = ''): databaseContext['engine'] = create_engine(databaseContext['url']) databaseContext['session'] = sessionmaker(bind=databaseContext['engine']) - Base.metadata.create_all(databaseContext['engine']) + bootstrapDatabaseIfNeeded(databaseContext) # isSyncronuous = False # while not isSyncronuous: @@ -54,6 +55,19 @@ def databaseContext(databasePath: str = ''): return databaseContext + +def databaseNeedsBootstrap(databaseContext) -> bool: + inspector = inspect(databaseContext['engine']) + existingTableNames = set(inspector.get_table_names()) + return not EXPECTED_TABLE_NAMES.issubset(existingTableNames) + + +def bootstrapDatabaseIfNeeded(databaseContext): + if not databaseNeedsBootstrap(databaseContext): + return + + Base.metadata.create_all(databaseContext['engine']) + def ensureDatabaseVersion(databaseContext): currentDatabaseVersion = getDatabaseVersion(databaseContext) diff --git a/tests/unit/test_database.py b/tests/unit/test_database.py new file mode 100644 index 0000000..27fa2da --- /dev/null +++ b/tests/unit/test_database.py @@ -0,0 +1,83 @@ +from __future__ import annotations + +from pathlib import Path +import sys +import tempfile +import unittest +from unittest.mock import patch + + +SRC_ROOT = Path(__file__).resolve().parents[2] / "src" + +if str(SRC_ROOT) not in sys.path: + sys.path.insert(0, str(SRC_ROOT)) + + +from ffx.constants import DATABASE_VERSION # noqa: E402 +from ffx.database import DATABASE_VERSION_KEY, databaseContext, getDatabaseVersion # noqa: E402 +from ffx.model.property import Property # noqa: E402 +from ffx.model.show import Base # noqa: E402 + + +class DatabaseContextTests(unittest.TestCase): + def setUp(self): + self.tempdir = tempfile.TemporaryDirectory() + self.database_path = Path(self.tempdir.name) / "ffx-test.db" + + def tearDown(self): + self.tempdir.cleanup() + + def test_database_context_bootstraps_new_database_with_current_version(self): + with patch("ffx.database.Base.metadata.create_all", wraps=Base.metadata.create_all) as mocked_create_all: + context = databaseContext(str(self.database_path)) + try: + self.assertTrue(self.database_path.exists()) + self.assertEqual(DATABASE_VERSION, getDatabaseVersion(context)) + finally: + context["engine"].dispose() + + mocked_create_all.assert_called_once() + + def test_database_context_skips_create_all_when_schema_is_already_present(self): + initial_context = databaseContext(str(self.database_path)) + initial_context["engine"].dispose() + + with patch("ffx.database.Base.metadata.create_all") as mocked_create_all: + context = databaseContext(str(self.database_path)) + try: + self.assertEqual(DATABASE_VERSION, getDatabaseVersion(context)) + finally: + context["engine"].dispose() + + mocked_create_all.assert_not_called() + + def test_database_context_restores_missing_version_property_without_schema_bootstrap(self): + context = databaseContext(str(self.database_path)) + Session = context["session"] + try: + session = Session() + try: + version_row = ( + session.query(Property) + .filter(Property.key == DATABASE_VERSION_KEY) + .first() + ) + session.delete(version_row) + session.commit() + finally: + session.close() + finally: + context["engine"].dispose() + + with patch("ffx.database.Base.metadata.create_all") as mocked_create_all: + reopened_context = databaseContext(str(self.database_path)) + try: + self.assertEqual(DATABASE_VERSION, getDatabaseVersion(reopened_context)) + finally: + reopened_context["engine"].dispose() + + mocked_create_all.assert_not_called() + + +if __name__ == "__main__": + unittest.main()