From 384b8f766878e95c45cdf6b6a41b662ae621d618 Mon Sep 17 00:00:00 2001 From: Cam Proudlock Date: Sun, 19 Jul 2026 11:39:51 -0400 Subject: [PATCH] ADR-014 Phase 2: flask plugin prune-schema for lean per-site DBs --- shopdb/plugins/cli.py | 139 ++++++++++++++++++++++++++++++ tests/test_plugin_prune_schema.py | 87 +++++++++++++++++++ 2 files changed, 226 insertions(+) create mode 100644 tests/test_plugin_prune_schema.py diff --git a/shopdb/plugins/cli.py b/shopdb/plugins/cli.py index 40083f87..ca6709bf 100644 --- a/shopdb/plugins/cli.py +++ b/shopdb/plugins/cli.py @@ -886,3 +886,142 @@ def upgrade_all_plugins(): click.echo(click.style(f" {name:20} no migrations", fg='yellow')) else: click.echo(click.style(f" {name:20} {status}", fg='red')) + + +@plugin_cli.command('prune-schema') +@click.option('--yes', is_flag=True, + help='Actually drop the tables (default is a dry-run preview)') +@click.option('--force', is_flag=True, + help='Drop even tables that hold rows (DATA LOSS); default refuses') +@with_appcontext +def prune_schema(yes: bool, force: bool): + """Drop tables owned by plugins this site did NOT install (ADR-014). + + Schema-lean per-site DBs: the shared core Alembic baseline creates every + plugin's tables, so a lean site that omits a plugin still carries that + plugin's (empty) tables. This drops the tables of every plugin in + PLUGIN_TABLE_OWNERS that is not installed here, leaving core + chosen-plugin + tables only. Run once at deploy AFTER `flask db upgrade` and + `flask plugin upgrade-all`. + + Dry-run by default; pass --yes to execute. Drops by table name (no plugin + code import) so it works on a lean image where the omitted plugin's + directory is absent. Refuses to drop a non-empty table unless --force, so a + misfire on a populated full site cannot silently delete data. + """ + from sqlalchemy import create_engine, inspect, text + + from shopdb.extensions import db + from shopdb.plugins.alembic_template import PLUGIN_TABLE_OWNERS + + pm = current_app.extensions.get('plugin_manager') + if not pm: + click.echo(click.style("Plugin manager not initialized", fg='red')) + raise SystemExit(1) + + installed = {p['name'] for p in pm.discover_available() if p['installed']} + engine = db.engine + existing = set(inspect(engine).get_table_names()) + + # A table is a prune candidate when its owning plugin is not installed here + # AND the table actually exists in this database. + victims = [] + for plugin, tables in PLUGIN_TABLE_OWNERS.items(): + if plugin in installed: + continue + for tablename in tables: + if tablename in existing: + victims.append((plugin, tablename)) + + if not victims: + click.echo("Nothing to prune: every plugin-owned table belongs to an " + "installed plugin.") + return + + # On MySQL, use a SEPARATE engine in AUTOCOMMIT, not db.engine. Two reasons, + # both of which burned real debugging time: + # 1. db.engine's pool keeps connections idle-in-transaction (Flask- + # SQLAlchemy has no request-teardown in a CLI context); a DROP sharing + # that pool waits on their locks. + # 2. Without engine-level AUTOCOMMIT, SQLAlchemy runs the COUNT probes in an + # open transaction (SET AUTOCOMMIT=0). Those reads hold shared metadata + # locks on every victim table, so the later DROP blocks on the metadata + # lock forever. Setting isolation_level on the CONNECTION (not the + # engine) silently did NOT take effect - it must be on the engine. + # With engine-level AUTOCOMMIT every statement commits on its own, so no read + # holds a lock into the DROP phase. lock_wait_timeout makes any residual + # contention fail fast. SQLite has none of this (no metadata locks), and a + # separate engine to an in-memory database would be a different, empty DB, so + # there we just use db.engine. db.session is only used above, for pluginstate. + dialect = engine.dialect.name + db.session.remove() + if dialect == 'mysql': + prune_engine = create_engine( + current_app.config['SQLALCHEMY_DATABASE_URI'], + isolation_level='AUTOCOMMIT') + own_engine = True + else: + prune_engine = engine + own_engine = False + + try: + with prune_engine.connect() as conn: + if dialect == 'mysql': + conn.execute(text('SET SESSION lock_wait_timeout=15')) + + # Row counts so a non-empty table is never dropped by accident. + nonempty = [] + for plugin, tablename in victims: + count = conn.execute( + text(f'SELECT COUNT(*) FROM `{tablename}`')).scalar() + if count: + nonempty.append((plugin, tablename, count)) + + click.echo(click.style( + f"Plugins not installed here: " + f"{', '.join(sorted({p for p, _ in victims}))}", fg='cyan')) + click.echo(f"Tables to prune ({len(victims)}):") + for plugin, tablename in victims: + click.echo(f" {tablename:32} ({plugin})") + + if nonempty and not force: + click.echo("") + click.echo(click.style( + "REFUSING: the following tables hold rows. Re-run with " + "--force to drop them anyway (this deletes data), or install " + "the owning plugin instead.", fg='red')) + for plugin, tablename, count in nonempty: + click.echo(click.style(f" {tablename:32} {count} rows", + fg='red')) + raise SystemExit(1) + + if not yes: + click.echo("") + click.echo("Dry-run. Re-run with --yes to drop the tables above.") + return + + # Intra-plugin foreign keys mean drop order matters; disable the + # checks for the batch rather than topologically sorting the tables + # without the models. Restore the PRIOR foreign-key setting after, + # not a hard ON: on SQLite the same connection is reused (StaticPool), + # so forcing ON would leak into whatever ran next. + fk_prev = None + if dialect == 'mysql': + conn.execute(text('SET FOREIGN_KEY_CHECKS=0')) + elif dialect == 'sqlite': + fk_prev = conn.execute(text('PRAGMA foreign_keys')).scalar() + conn.execute(text('PRAGMA foreign_keys=OFF')) + for _plugin, tablename in victims: + conn.execute(text(f'DROP TABLE IF EXISTS `{tablename}`')) + if dialect == 'mysql': + conn.execute(text('SET FOREIGN_KEY_CHECKS=1')) + elif dialect == 'sqlite': + conn.execute(text(f'PRAGMA foreign_keys={int(fk_prev or 0)}')) + conn.commit() + finally: + if own_engine: + prune_engine.dispose() + + click.echo(click.style(f"Pruned {len(victims)} table(s). This database now " + f"carries core + installed-plugin tables only.", + fg='green')) diff --git a/tests/test_plugin_prune_schema.py b/tests/test_plugin_prune_schema.py new file mode 100644 index 00000000..92de627d --- /dev/null +++ b/tests/test_plugin_prune_schema.py @@ -0,0 +1,87 @@ +"""Tests for `flask plugin prune-schema` (ADR-014 schema-lean per-site). + +The shared core Alembic baseline creates every plugin's tables, so a lean site +that omits a plugin still carries that plugin's (empty) tables. prune-schema +drops the tables of every not-installed plugin, leaving core + chosen-plugin +tables. These tests pin the three behaviors that matter: only not-installed +plugins are dropped, the non-empty guard refuses without --force, and a full +site (every plugin installed) is a no-op. +""" + +import pytest +from sqlalchemy import inspect, text + +from shopdb.extensions import db as _db +from shopdb.plugins.cli import plugin_cli +from shopdb.plugins.alembic_template import PLUGIN_TABLE_OWNERS + + +def _install_only(app, monkeypatch, installed_names): + """Force discover_available to report exactly `installed_names` installed.""" + pm = app.extensions['plugin_manager'] + listing = [{'name': name, 'installed': name in installed_names} + for name in PLUGIN_TABLE_OWNERS] + monkeypatch.setattr(pm, 'discover_available', lambda: listing) + + +def test_prune_drops_only_not_installed(app, db, runner, monkeypatch): + """A lean site keeps its plugins' tables and drops every other plugin's.""" + _install_only(app, monkeypatch, {'machines', 'printers'}) + before = set(inspect(_db.engine).get_table_names()) + assert 'usbdevices' in before and 'machines' in before # baseline sanity + + result = runner.invoke(plugin_cli, ['prune-schema', '--yes']) + assert result.exit_code == 0, result.output + + after = set(inspect(_db.engine).get_table_names()) + # Kept: machines + printers owned tables. + for keep in PLUGIN_TABLE_OWNERS['machines'] + PLUGIN_TABLE_OWNERS['printers']: + assert keep in after, f'{keep} should have been kept' + # Dropped: every other plugin's tables. + for plugin, tables in PLUGIN_TABLE_OWNERS.items(): + if plugin in ('machines', 'printers'): + continue + for tablename in tables: + assert tablename not in after, f'{tablename} should have been dropped' + # Core tables untouched. + for core in ('assets', 'users', 'settings'): + assert core in after + + +def test_prune_full_site_is_noop(app, db, runner, monkeypatch): + """Every plugin installed -> nothing to prune, schema unchanged.""" + _install_only(app, monkeypatch, set(PLUGIN_TABLE_OWNERS)) + before = set(inspect(_db.engine).get_table_names()) + + result = runner.invoke(plugin_cli, ['prune-schema', '--yes', '--force']) + assert result.exit_code == 0, result.output + assert 'Nothing to prune' in result.output + assert set(inspect(_db.engine).get_table_names()) == before + + +def test_prune_refuses_non_empty_without_force(app, db, runner, monkeypatch): + """A not-installed table that holds rows is not dropped without --force.""" + _install_only(app, monkeypatch, {'machines'}) + _db.session.execute(text( + "INSERT INTO usbdevicetypes (typename, createddate, modifieddate, " + "isactive) VALUES ('kept-by-guard', '2026-01-01', '2026-01-01', 1)")) + _db.session.commit() + + result = runner.invoke(plugin_cli, ['prune-schema', '--yes']) + assert result.exit_code == 1 + assert 'REFUSING' in result.output + # The row-bearing table survives the refusal. + assert 'usbdevicetypes' in set(inspect(_db.engine).get_table_names()) + + +def test_prune_force_drops_non_empty(app, db, runner, monkeypatch): + """--force drops even a table that holds rows.""" + _install_only(app, monkeypatch, {'machines'}) + _db.session.execute(text( + "INSERT INTO usbdevicetypes (typename, createddate, modifieddate, " + "isactive) VALUES ('dropped-by-force', '2026-01-01', '2026-01-01', 1)")) + _db.session.commit() + + result = runner.invoke(plugin_cli, ['prune-schema', '--yes', '--force']) + assert result.exit_code == 0, result.output + assert 'usbdevicetypes' not in set(inspect(_db.engine).get_table_names())