Merge remote-tracking branch 'origin/main' into wilm

This commit is contained in:
benjamin.baker
2026-08-25 16:48:23 -04:00
51 changed files with 2806 additions and 608 deletions

View File

@@ -0,0 +1,239 @@
"""Catalog hygiene: merging vendor/model/OS rows that are the same name twice.
Legacy data arrived carrying space-stripped twins - 'DellInc.' beside
'Dell Inc.', 'Windows10' beside 'Windows 10'. They rode into the shipped seed
catalog and out to every site that ran `seed catalog`, and stay invisible until
somebody opens a vendor picker and finds the same company listed twice.
The distinction these tests exist to protect: a twin is the same name TYPED two
ways and may be merged mechanically, while 'Dell' and 'Dell Inc.' are two
different names and merging them is a judgment only a person can make.
"""
import pytest
from shopdb.extensions import db
from shopdb.core.models import Vendor, Model
from shopdb.cli import _catalog_norm
def _run(app, args):
return app.test_cli_runner().invoke(args=args)
# --- the normaliser draws the line ------------------------------------------
def test_whitespace_and_punctuation_twins_normalise_together():
assert _catalog_norm('Dell Inc.') == _catalog_norm('DellInc.')
assert _catalog_norm('Windows 10') == _catalog_norm('Windows10')
assert _catalog_norm('OptiPlex 7080') == _catalog_norm('OptiPlex7080')
def test_a_different_name_does_not_normalise_together():
"""'Dell' is not a typo of 'Dell Inc.', it is a shorter name.
If these collapsed, `dedupe` would silently pick one and delete a name the
site may have chosen deliberately.
"""
assert _catalog_norm('Dell') != _catalog_norm('Dell Inc.')
assert _catalog_norm('Precision 5560') != _catalog_norm('Precision 5570')
# --- dedupe ------------------------------------------------------------------
@pytest.fixture
def dell_twins(db):
"""The real shape found in production: a twin, plus a genuine third name."""
canonical = Vendor(vendor='Dell Inc.')
twin = Vendor(vendor='DellInc.')
shortname = Vendor(vendor='Dell')
db.session.add_all([canonical, twin, shortname])
db.session.flush()
# the twin owns a model, so the merge has a reference to repoint
db.session.add(Model(modelnumber='OptiPlex7080', vendorid=twin.vendorid))
db.session.add(Model(modelnumber='OptiPlex 7080', vendorid=canonical.vendorid))
db.session.commit()
return {'canonical': canonical.vendorid, 'twin': twin.vendorid,
'shortname': shortname.vendorid}
def test_dedupe_reports_without_changing_anything(app, dell_twins):
"""The default is a dry run: a cleanup that writes by surprise is a trap."""
result = _run(app, ['catalog', 'dedupe'])
assert result.exit_code == 0
assert 'DellInc.' in result.output
assert 'Dry run' in result.output
# every row still stands
assert db.session.get(Vendor, dell_twins['twin']) is not None
assert Vendor.query.filter_by(vendor='DellInc.').count() == 1
def test_dedupe_merges_the_twin_and_repoints_its_model(app, dell_twins):
"""The twin disappears and its model moves, rather than being orphaned."""
result = _run(app, ['catalog', 'dedupe', '--apply'])
assert result.exit_code == 0
assert Vendor.query.filter_by(vendor='DellInc.').count() == 0
assert Vendor.query.filter_by(vendor='Dell Inc.').count() == 1
# nothing points at a vendor that no longer exists
vendorids = {v.vendorid for v in Vendor.query.all()}
for model in Model.query.all():
assert model.vendorid is None or model.vendorid in vendorids
def test_dedupe_leaves_a_genuinely_different_name_alone(app, dell_twins):
"""'Dell' survives. It is a naming choice, not a typo."""
_run(app, ['catalog', 'dedupe', '--apply'])
assert Vendor.query.filter_by(vendor='Dell').count() == 1
def test_dedupe_keeps_the_spelling_a_person_typed(app, db):
"""With no references either way, the spaced spelling wins.
Whitespace is evidence of intent: somebody pressed the space bar.
"""
db.session.add_all([Vendor(vendor='AcmeCorp'), Vendor(vendor='Acme Corp')])
db.session.commit()
_run(app, ['catalog', 'dedupe', '--apply'])
assert Vendor.query.filter_by(vendor='Acme Corp').count() == 1
assert Vendor.query.filter_by(vendor='AcmeCorp').count() == 0
def test_dedupe_keeps_the_most_referenced_row(app, db):
"""References outrank spelling: the row in use is the one that survives."""
bare = Vendor(vendor='ZZTop')
spaced = Vendor(vendor='ZZ Top')
db.session.add_all([bare, spaced])
db.session.flush()
for i in range(3):
db.session.add(Model(modelnumber=f'ZZ-{i}', vendorid=bare.vendorid))
db.session.commit()
_run(app, ['catalog', 'dedupe', '--apply'])
assert Vendor.query.filter_by(vendor='ZZTop').count() == 1
assert Vendor.query.filter_by(vendor='ZZ Top').count() == 0
def test_dedupe_is_idempotent(app, dell_twins):
"""Running it twice is a no-op, so it is safe in a deploy script."""
_run(app, ['catalog', 'dedupe', '--apply'])
second = _run(app, ['catalog', 'dedupe', '--apply'])
assert second.exit_code == 0
assert 'No catalog twins found' in second.output
def test_dedupe_on_a_clean_catalog_says_so(app, db):
db.session.add(Vendor(vendor='Solo Vendor'))
db.session.commit()
result = _run(app, ['catalog', 'dedupe'])
assert result.exit_code == 0
assert 'No catalog twins found' in result.output
# --- explicit merge ----------------------------------------------------------
def test_merge_absorbs_a_deliberately_different_name(app, dell_twins):
"""The judgment call dedupe refuses: 'Dell' means 'Dell Inc.' at this site."""
result = _run(app, ['catalog', 'merge', 'vendors', 'Dell', 'Dell Inc.',
'--apply'])
assert result.exit_code == 0
assert Vendor.query.filter_by(vendor='Dell').count() == 0
assert Vendor.query.filter_by(vendor='Dell Inc.').count() == 1
def test_merge_dry_runs_by_default(app, dell_twins):
result = _run(app, ['catalog', 'merge', 'vendors', 'Dell', 'Dell Inc.'])
assert 'Dry run' in result.output
assert Vendor.query.filter_by(vendor='Dell').count() == 1
def test_merge_refuses_to_invent_the_target(app, dell_twins):
"""A typo in the target must not create a new vendor and move rows into it."""
result = _run(app, ['catalog', 'merge', 'vendors', 'Dell', 'Dell Incorporated',
'--apply'])
assert result.exit_code == 1
assert 'Refusing to invent' in result.output
assert Vendor.query.filter_by(vendor='Dell').count() == 1
def test_merge_reports_a_missing_source_without_failing(app, dell_twins):
"""Already merged is not an error: the deploy script can just run again."""
result = _run(app, ['catalog', 'merge', 'vendors', 'Nonexistent',
'Dell Inc.', '--apply'])
assert result.exit_code == 0
assert 'No vendors row named' in result.output
# --- rename ------------------------------------------------------------------
def test_rename_fixes_a_stripped_name_with_no_twin(app, db):
"""'OptiPlex5000' has nothing to merge into, so dedupe cannot reach it."""
vendor = Vendor(vendor='Dell Inc.')
db.session.add(vendor)
db.session.flush()
db.session.add(Model(modelnumber='OptiPlex5000', vendorid=vendor.vendorid))
db.session.commit()
_run(app, ['catalog', 'dedupe', '--apply'])
assert Model.query.filter_by(modelnumber='OptiPlex5000').count() == 1, \
'dedupe should not touch a name with no twin'
result = _run(app, ['catalog', 'rename', 'models', 'OptiPlex5000',
'OptiPlex 5000', '--apply'])
assert result.exit_code == 0
assert Model.query.filter_by(modelnumber='OptiPlex 5000').count() == 1
assert Model.query.filter_by(modelnumber='OptiPlex5000').count() == 0
def test_rename_keeps_the_row_and_its_references(app, db):
"""A rename is in place: the id does not change, so nothing is repointed."""
vendor = Vendor(vendor='Acme')
db.session.add(vendor)
db.session.flush()
model = Model(modelnumber='Widget100', vendorid=vendor.vendorid)
db.session.add(model)
db.session.commit()
original = model.modelnumberid
_run(app, ['catalog', 'rename', 'models', 'Widget100', 'Widget 100', '--apply'])
renamed = Model.query.filter_by(modelnumber='Widget 100').one()
assert renamed.modelnumberid == original
assert renamed.vendorid == vendor.vendorid
def test_rename_refuses_when_the_target_already_exists(app, db):
"""Renaming onto an existing name would need a merge, not an UPDATE."""
db.session.add_all([Vendor(vendor='Acme'), Vendor(vendor='Acme Corp')])
db.session.commit()
result = _run(app, ['catalog', 'rename', 'vendors', 'Acme', 'Acme Corp',
'--apply'])
assert result.exit_code == 1
assert 'use `catalog merge`' in result.output
assert Vendor.query.filter_by(vendor='Acme').count() == 1
def test_rename_dry_runs_by_default(app, db):
db.session.add(Vendor(vendor='Acme'))
db.session.commit()
result = _run(app, ['catalog', 'rename', 'vendors', 'Acme', 'Acme Corp'])
assert 'Dry run' in result.output
assert Vendor.query.filter_by(vendor='Acme').count() == 1

View File

@@ -1,11 +1,19 @@
"""Tests for the computers collector PC->printer relationship sync.
"""The computers collector must NEVER write a printer assignment.
A collector payload carrying defaultprinter/printers creates AssetRelationship
rows linking the PC asset to the resolved printer assets ('defaultprinter' for
the default, 'connectedto' for the rest). Stale collector links archive on
re-report; manual links survive; unresolved identifiers warn without failing.
This file used to pin the opposite: `_sync_printer_links` turned a payload's
`defaultprinter`/`printers` into AssetRelationship rows. That contradicted the
rule the printer feature rests on - ASSIGNED and OBSERVED never mix - because a
bay that installed the wrong printer would have made itself right just by
reporting it.
It was not dormant. The collector schema advertised both keys and
COLLECTOR-INTEGRATION.md's example client set `defaultprinter`, while another
page of the same document promised nothing on this path writes an assignment
row. Retired 2026-08-24; these tests are the guard that it stays retired.
Observed queues have their own path (POST /api/collector/printers) and become
assignments only when a person asks, via seed-from-observed.
"""
import pytest
from shopdb.core.models import (
@@ -81,119 +89,75 @@ def _active_links(db, pcid):
sourceassetid=pcid, isactive=True).all()
def test_creates_default_and_connected_links(client, db, collector_key, scene):
"""Default -> defaultprinter, others -> connectedto, resolved by three id forms."""
resp = _post(client, {
'hostname': 'WJPC-P1',
'defaultprinter': 'HP-Materials', # windowsname
'printers': ['prn-front.wjs.local', '10.1.2.3'], # hostname, IP
})
assert resp.status_code == 200, resp.get_json()
data = resp.get_json()['data']
assert data['printerlinkcount'] == 3
assert not data['warnings']
pc = _pc_asset(db)
links = {(r.targetassetid, r.relationshiptype.relationshiptype)
for r in _active_links(db, pc.assetid)}
assert (scene['a'].assetid, 'defaultprinter') in links
assert (scene['b'].assetid, 'connectedto') in links
assert (scene['c'].assetid, 'connectedto') in links
def test_unresolved_identifier_warns_not_fails(client, db, collector_key, scene):
"""An unknown printer identifier is a warning, the rest still link."""
resp = _post(client, {
'hostname': 'WJPC-P2',
def test_printer_keys_create_no_relationships(client, db, collector_key, scene):
"""The reported case, inverted: sending them must change nothing."""
response = _post(client, {
'hostname': 'BAY-3105-PC',
'defaultprinter': 'HP-Materials',
'printers': ['does-not-exist'],
'printers': ['HP-Materials', 'prn-front.wjs.local', '10.1.2.3'],
})
assert resp.status_code == 200
data = resp.get_json()['data']
assert any('unresolved printer: does-not-exist' in w
for w in data['warnings'])
pc = _pc_asset(db)
assert data['printerlinkcount'] == 1
assert len(_active_links(db, pc.assetid)) == 1
assert response.status_code in (200, 201), response.get_json()
pcid = _pc_asset(db).assetid
assert _active_links(db, pcid) == [], (
'a collector report wrote printer relationships')
def test_idempotent_no_duplicates(client, db, collector_key, scene):
"""Re-report the same set: no new rows, still one link per printer."""
payload = {'hostname': 'WJPC-P3', 'defaultprinter': 'HP-Materials',
'printers': ['Zebra-Ship']}
assert _post(client, payload).status_code == 200
assert _post(client, payload).status_code == 200
def test_the_push_still_succeeds_and_says_why(client, db, collector_key, scene):
"""Warn, do not reject.
pc = _pc_asset(db)
all_rows = AssetRelationship.query.filter_by(sourceassetid=pc.assetid).all()
assert len(all_rows) == 2 # one default + one connected, no dupes
assert all(r.isactive for r in all_rows)
Unknown keys are not validated, so a client written against the old docs
keeps pushing successfully - only its printer keys stop doing anything.
Silently dropping them would leave an integrator believing assignments were
still being written, which is worse than the original bug.
"""
response = _post(client, {
'hostname': 'BAY-3105-PC',
'defaultprinter': 'HP-Materials',
})
assert response.status_code in (200, 201)
warnings = ' '.join(response.get_json()['data']['warnings']).lower()
assert 'defaultprinter' in warnings
assert 'seed-from-observed' in warnings, (
'the warning should say where assignment actually comes from')
def test_dropped_printer_is_archived(client, db, collector_key, scene):
"""A printer that stops being reported loses its collector link."""
_post(client, {'hostname': 'WJPC-P4', 'defaultprinter': 'HP-Materials',
'printers': ['Zebra-Ship']})
pc = _pc_asset(db)
assert len(_active_links(db, pc.assetid)) == 2
# Re-report without Zebra-Ship.
_post(client, {'hostname': 'WJPC-P4', 'defaultprinter': 'HP-Materials',
'printers': []})
active = _active_links(db, pc.assetid)
assert len(active) == 1
assert active[0].targetassetid == scene['a'].assetid
# the dropped row still exists but is inactive
zebra = AssetRelationship.query.filter_by(
sourceassetid=pc.assetid, targetassetid=scene['c'].assetid).first()
assert zebra is not None and zebra.isactive is False
def test_a_payload_without_printer_keys_warns_about_nothing(
client, db, collector_key, scene):
"""The normal case must stay quiet - this is every push in the fleet."""
response = _post(client, {'hostname': 'BAY-3105-PC'})
assert response.status_code in (200, 201)
warnings = ' '.join(response.get_json()['data']['warnings']).lower()
assert 'printer' not in warnings
def test_default_change_archives_old_default(client, db, collector_key, scene):
"""Changing the default printer archives the previous defaultprinter link."""
_post(client, {'hostname': 'WJPC-P5', 'defaultprinter': 'HP-Materials'})
pc = _pc_asset(db)
_post(client, {'hostname': 'WJPC-P5', 'defaultprinter': 'prn-front.wjs.local'})
active = _active_links(db, pc.assetid)
assert len(active) == 1
assert active[0].targetassetid == scene['b'].assetid
assert active[0].relationshiptype.relationshiptype == 'defaultprinter'
def test_manual_link_survives_collector_sync(client, db, collector_key, scene):
"""A manually-created link (no collector label) is never archived."""
# PC created first via a bare report.
_post(client, {'hostname': 'WJPC-P6', 'defaultprinter': 'HP-Materials',
'printers': ['Zebra-Ship']})
pc = _pc_asset(db)
# Manual connectedto link PC -> printer B (label stays NULL).
def test_a_manual_assignment_survives_a_report_that_names_printers(
client, db, collector_key, scene):
"""The row a person made is the one that matters, and it stays untouched."""
_post(client, {'hostname': 'BAY-3105-PC'})
pcasset = _pc_asset(db)
manual = AssetRelationship(
sourceassetid=pc.assetid, targetassetid=scene['b'].assetid,
relationshiptypeid=scene['ct_type'].relationshiptypeid, notes='by hand')
sourceassetid=pcasset.assetid, targetassetid=scene['a'].assetid,
relationshiptypeid=scene['dp_type'].relationshiptypeid)
db.session.add(manual)
db.session.commit()
manual_id = manual.relationshipid
# Re-report dropping Zebra-Ship and never mentioning printer B.
_post(client, {'hostname': 'WJPC-P6', 'defaultprinter': 'HP-Materials',
'printers': []})
_post(client, {
'hostname': 'BAY-3105-PC',
'defaultprinter': 'prn-front.wjs.local', # a DIFFERENT printer
'printers': ['prn-front.wjs.local'],
})
surviving = db.session.get(AssetRelationship, manual_id)
assert surviving.isactive is True # manual link untouched
# Zebra (collector) archived.
zebra = AssetRelationship.query.filter_by(
sourceassetid=pc.assetid, targetassetid=scene['c'].assetid).first()
assert zebra.isactive is False
links = _active_links(db, pcasset.assetid)
assert len(links) == 1
assert links[0].targetassetid == scene['a'].assetid, (
'the report retargeted a human-made default printer')
def test_no_printer_keys_leaves_links_untouched(client, db, collector_key, scene):
"""A report with no printer keys does not archive existing links."""
_post(client, {'hostname': 'WJPC-P7', 'defaultprinter': 'HP-Materials'})
pc = _pc_asset(db)
assert len(_active_links(db, pc.assetid)) == 1
def test_the_retired_keys_are_gone_from_the_collector_schema(app):
"""The schema advertised them, which is how integrators found them."""
from plugins.computers.plugin import ComputersPlugin
# A plain update (no defaultprinter, no printers key) must not archive.
_post(client, {'hostname': 'WJPC-P7', 'currentuser': 'alice'})
assert len(_active_links(db, pc.assetid)) == 1
fields = ComputersPlugin().get_collector_schema()['fields']
assert 'defaultprinter' not in fields
assert 'printers' not in fields

View File

@@ -199,3 +199,70 @@ def test_a_machine_is_found_by_its_maintenance_reference(client, taggedmachine):
type - a field the UI collects and search could not find."""
rows = [r for r in search(client, 'ZZMAINT888') if r['title'] == 'ZZMACH01']
assert rows, 'a maintenance reference matched nothing'
# --- a search result must land on the record, not its list -------------------
#
# A result carries an id, so sending the reader to the plugin's list page makes
# them find by hand the row search had already identified. Both cases below
# shipped that way: the id was in the payload and the URL ignored it.
@pytest.fixture
def noticerow(db):
"""One notification with a distinctive body and its own type."""
from plugins.notifications.models import Notification, NotificationType
notice = NotificationType(typename='Notice', typecolor='#17a2b8',
isactive=True)
db.session.add(notice)
db.session.flush()
row = Notification(
notificationtypeid=notice.notificationtypeid,
notification='ZZNOTICE7788 press brake down for repair',
ticketnumber='INC7788',
isactive=True,
)
db.session.add(row)
db.session.commit()
return row.notificationid
def test_a_notice_result_links_to_the_notice(client, noticerow):
"""Clicking a NOTICE opens that notification, not /notifications.
`/notifications/<id>` is the `notification-detail` route; the list page
cannot show which of its rows the search matched.
"""
rows = [r for r in search(client, 'ZZNOTICE7788')
if r['type'] == 'notification']
assert len(rows) == 1
assert rows[0]['id'] == noticerow
assert rows[0]['url'] == f'/notifications/{noticerow}'
def test_a_notice_found_by_ticket_number_links_the_same_way(client, noticerow):
"""The ticket number is the strongest match and must land as precisely."""
rows = [r for r in search(client, 'INC7788')
if r['type'] == 'notification']
assert len(rows) == 1
assert rows[0]['url'] == f'/notifications/{noticerow}'
@pytest.fixture
def subnetrow(db):
"""A subnet an IP search can fall inside."""
from plugins.network.models import Subnet
row = Subnet(cidr='10.77.88.0/24', name='ZZ Test subnet', isactive=True)
db.session.add(row)
db.session.commit()
return row.subnetid
def test_an_ip_search_links_to_the_matched_subnet(client, subnetrow):
"""Searching an IP already picked WHICH subnet; land on it.
The detail route is `/networks/<id>` (plural) - `/network` is the hub.
"""
rows = [r for r in search(client, '10.77.88.42') if r['type'] == 'subnet']
assert len(rows) == 1
assert rows[0]['id'] == subnetrow
assert rows[0]['url'] == f'/networks/{subnetrow}'

View File

@@ -0,0 +1,94 @@
"""Standalone maintenance scripts must load .env before reading config.
`wsgi.py` calls `load_dotenv()` for the served app, and the `flask` CLI loads
.env for itself, so both reach the site's real database. A plain
`python scripts/<name>.py` run gets neither. Without an explicit load,
DATABASE_URL is unset, FLASK_ENV defaults to 'development', and config.py falls
back to a hardcoded `root@localhost` URL.
On a real deployment that surfaced as a bare 1045 "Access denied for user
'root'@'localhost'" from `export_catalog.py`, naming a user the site does not
use. The read-only case is merely confusing. The dangerous case is a box where
those root credentials DO resolve: six of these scripts write, and they would
have silently rewritten the wrong database.
This went unnoticed for so long because the dev .env happens to point at root,
so the fallback matched what the developer expected.
"""
import ast
from pathlib import Path
import pytest
SCRIPTS_DIR = Path(__file__).resolve().parent.parent / 'scripts'
def _scripts_that_build_the_app():
"""Every script that calls create_app, so it resolves a database URL."""
found = []
for path in sorted(SCRIPTS_DIR.glob('*.py')):
if 'create_app' in path.read_text(encoding='utf-8'):
found.append(path)
return found
def test_there_are_scripts_to_check():
"""Guard the guard: a glob that matches nothing passes every test below."""
assert _scripts_that_build_the_app(), \
'no scripts call create_app - has the layout moved?'
@pytest.mark.parametrize('path', _scripts_that_build_the_app(),
ids=lambda p: p.name)
def test_script_loads_dotenv_before_building_the_app(path):
"""The script calls load_dotenv, and does so before create_app runs.
Order matters as much as presence: config is read when create_app builds
the app, so a load_dotenv underneath it changes nothing.
"""
source = path.read_text(encoding='utf-8')
assert 'load_dotenv' in source, (
f'{path.name} builds the app but never loads .env, so a plain '
f'python run falls back to the hardcoded root@localhost URL'
)
tree = ast.parse(source)
load_lines = [
node.lineno for node in ast.walk(tree)
if isinstance(node, ast.Call)
and getattr(node.func, 'id', None) == 'load_dotenv'
]
assert load_lines, f'{path.name} imports load_dotenv but never calls it'
create_lines = [
node.lineno for node in ast.walk(tree)
if isinstance(node, ast.Call)
and getattr(node.func, 'id', None) == 'create_app'
]
assert create_lines, f'{path.name} references create_app but never calls it'
assert min(load_lines) < min(create_lines), (
f'{path.name} calls load_dotenv at line {min(load_lines)}, after '
f'create_app at line {min(create_lines)} - config is already read'
)
@pytest.mark.parametrize('path', _scripts_that_build_the_app(),
ids=lambda p: p.name)
def test_dotenv_is_located_from_the_script_not_the_cwd(path):
"""load_dotenv is given an explicit repo-root path.
A bare `load_dotenv()` searches upward from the CURRENT DIRECTORY, so the
same command works from the repo root and silently finds nothing when run
from anywhere else - the failure these scripts are most likely to hit,
since they are run by hand during a deployment.
"""
tree = ast.parse(path.read_text(encoding='utf-8'))
for node in ast.walk(tree):
if (isinstance(node, ast.Call)
and getattr(node.func, 'id', None) == 'load_dotenv'):
assert node.args or node.keywords, (
f'{path.name} calls load_dotenv() with no path, so it resolves '
f'.env relative to the working directory'
)

View File

@@ -276,3 +276,89 @@ def test_upgrade_all_on_fresh_db_is_clean_and_idempotent(tmp_path, monkeypatch):
(plugin_manager._app, plugin_manager._db, plugin_manager.registry,
plugin_manager.loader, plugin_manager.migration_manager,
plugin_manager._registered_prefixes) = saved
# Plugins whose whole chain is a single baseline. Their tables existing already
# is the ONLY divergence possible between schema and stamp, so upgrade-all must
# handle it end to end. A longer chain can also meet an ALTER whose column is
# already present, which no amount of guarding a create_table fixes - see the
# test below for why that is a different problem.
SINGLE_BASELINE_PLUGINS = tuple(
plugin for plugin in POST_CUTOVER_PLUGINS
if len(list((PLUGINS_DIR / plugin / 'migrations' / 'versions').glob('0*.py'))) == 1
)
@pytest.mark.parametrize('plugin', SINGLE_BASELINE_PLUGINS)
def test_baseline_skips_tables_that_already_exist(plugin, tmp_path, monkeypatch):
"""A baseline must stamp, not fail, when its table is already there.
Reported: `flask plugin upgrade-all` failed the tools plugin with "table
securesendsecrets already exists". Every post-cutover baseline was a plain
op.create_table, correct only while a table's existence and its plugin's
Alembic stamp can never disagree. They can - anything that builds tables
from the models (db.create_all) stamps nothing, and a run that creates a
table then fails before its stamp commits leaves the same split.
The other upgrade-all test DROPS these tables before running, so it only
ever covered the state where they are absent. This is the other state.
"""
from sqlalchemy import inspect, text
from shopdb.config import TestingConfig
from shopdb import create_app
from shopdb.extensions import db
from shopdb.plugins import plugin_manager
monkeypatch.setattr(TestingConfig, 'SQLALCHEMY_DATABASE_URI',
f'sqlite:///{tmp_path / "preexisting.db"}')
saved = (plugin_manager._app, plugin_manager._db, plugin_manager.registry,
plugin_manager.loader, plugin_manager.migration_manager,
plugin_manager._registered_prefixes)
try:
app = create_app('testing')
with app.app_context():
db.create_all()
# Leave THIS plugin's tables in place - that is the whole point -
# and drop the other post-cutover ones so their own (longer) chains
# run the way the sibling test already covers.
insp0 = inspect(db.engine)
for other in POST_CUTOVER_PLUGINS:
if other == plugin:
continue
for tablename in PLUGIN_TABLE_OWNERS[other]:
if insp0.has_table(tablename):
db.session.execute(text(f'DROP TABLE {tablename}'))
db.session.commit()
for tablename in PLUGIN_TABLE_OWNERS[plugin]:
assert inspect(db.engine).has_table(tablename), (
f'{tablename} should have been built by create_all; '
f'this test is meaningless without it')
first = app.extensions['plugin_manager'].upgrade_all_plugins()
assert first[plugin] == 'ok', first[plugin]
second = app.extensions['plugin_manager'].upgrade_all_plugins()
assert second[plugin] == 'ok', second[plugin]
insp = inspect(db.engine)
row = db.session.execute(
text(f'SELECT version_num FROM alembic_version_{plugin}')
).fetchone()
assert row and row[0] == EXPECTED_HEAD_REVISION[plugin]
# No second index over the same columns: create_all names indexes by
# SQLAlchemy's convention and the baseline by this repo's, so a
# name-keyed guard would have added a duplicate rather than skipped.
for tablename in PLUGIN_TABLE_OWNERS[plugin]:
seen = [tuple(ix.get('column_names') or ())
for ix in insp.get_indexes(tablename)]
assert len(seen) == len(set(seen)), (
f'{tablename} has two indexes over the same columns: {seen}')
finally:
(plugin_manager._app, plugin_manager._db, plugin_manager.registry,
plugin_manager.loader, plugin_manager.migration_manager,
plugin_manager._registered_prefixes) = saved

View File

@@ -1,16 +1,21 @@
"""Canary tests for the plugin scaffolder's frontend generation.
`flask plugin new <name>` generates the backend skeleton and, when a frontend
tree is present, a matching Vue frontend starting point: list, detail, and form
views, a router route file, and a paste-in api-client snippet. These tests
guard the file names, the substitutions, the ADR-009 plugin gating on the
routes, the use of global CSS classes over per-page input styling, graceful
skipping when there is no frontend tree, and the no-clobber behaviour.
`flask plugin new <name>` generates the backend skeleton and a matching Vue
frontend starting point: list, detail, and form views, a route file, and a
paste-in api-client snippet. All of it lands INSIDE the plugin, under
`plugins/<name>/frontend/`, which is what lets stage-frontend.mjs stage it and
a lean build prune it (ADR-013).
These tests guard the file names and their location, the substitutions, the
ADR-009 plugin gating on the routes, the alias-vs-relative import split that
has to survive staging, the use of global CSS classes over per-page input
styling, and the no-clobber behaviour.
All generation runs against tmp_path. The real frontend/src tree is never
touched.
"""
import re
from pathlib import Path
import pytest
@@ -18,178 +23,172 @@ import pytest
from shopdb.plugins.scaffolder import scaffold_plugin, ScaffoldError
def _make_dirs(tmp_path: Path):
"""Create a tmp plugins dir and a tmp frontend/src tree.
Returns (plugins_dir, frontend_dir).
"""
def _make_plugins_dir(tmp_path: Path) -> Path:
"""Create a tmp plugins dir."""
plugins_dir = tmp_path / 'plugins'
plugins_dir.mkdir()
frontend_dir = tmp_path / 'frontend' / 'src'
frontend_dir.mkdir(parents=True)
return plugins_dir, frontend_dir
return plugins_dir
def test_frontend_files_generated_with_correct_names(tmp_path):
"""Views, route file, and api snippet appear with the expected names."""
plugins_dir, frontend_dir = _make_dirs(tmp_path)
def test_frontend_files_generated_inside_the_plugin(tmp_path):
"""Views, route file, and api snippet appear with the expected names.
Location is the point of the test: the frontend belongs to the plugin, not
to the core tree.
"""
plugins_dir = _make_plugins_dir(tmp_path)
target = scaffold_plugin(
name='widgets',
description='Test widgets plugin',
plugins_dir=plugins_dir,
frontend_dir=frontend_dir,
)
assert (frontend_dir / 'views' / 'widgets' / 'WidgetsList.vue').exists()
assert (frontend_dir / 'views' / 'widgets' / 'WidgetsDetail.vue').exists()
assert (frontend_dir / 'views' / 'widgets' / 'WidgetsForm.vue').exists()
assert (frontend_dir / 'router' / 'routes' / 'widgets.js').exists()
# paste-in snippet lands in the plugin dir, not the frontend tree
assert (target / 'frontend' / 'views' / 'WidgetsList.vue').exists()
assert (target / 'frontend' / 'views' / 'WidgetsDetail.vue').exists()
assert (target / 'frontend' / 'views' / 'WidgetsForm.vue').exists()
assert (target / 'frontend' / 'routes.js').exists()
assert (target / 'frontend-api-snippet.js').exists()
def test_nothing_is_written_into_the_core_frontend_tree(tmp_path):
"""The scaffolder never touches frontend/src.
This is the regression that prompted the move. Writing views to
`frontend/src/views/<name>/` and a route to
`frontend/src/router/routes/<name>.js` still *renders* - the router globs
that directory - but the result is invisible to stage-frontend.mjs, so the
plugin cannot be pruned from a lean build and deleting it strands its
views in the core tree.
"""
plugins_dir = _make_plugins_dir(tmp_path)
core_frontend = tmp_path / 'frontend' / 'src'
core_frontend.mkdir(parents=True)
scaffold_plugin('widgets', 'Test', plugins_dir)
assert not (core_frontend / 'views' / 'widgets').exists()
assert not (core_frontend / 'router' / 'routes' / 'widgets.js').exists()
assert list(core_frontend.iterdir()) == []
def test_frontend_generated_with_no_core_tree_at_all(tmp_path):
"""A plugin developed in its own repo gets the same frontend.
There is no core frontend tree to depend on, and none is fabricated. The
plugin's frontend starts working the moment the directory is dropped into
a host repo's `plugins/`.
"""
plugins_dir = _make_plugins_dir(tmp_path)
target = scaffold_plugin('widgets', 'Test', plugins_dir)
assert (target / 'frontend' / 'routes.js').exists()
assert (target / 'frontend' / 'views' / 'WidgetsList.vue').exists()
assert not (tmp_path / 'frontend').exists()
def test_frontend_substitutions_applied(tmp_path):
"""Name and Name placeholders are substituted, none left raw."""
plugins_dir, frontend_dir = _make_dirs(tmp_path)
scaffold_plugin('widgets', 'Test', plugins_dir, frontend_dir=frontend_dir)
"""Name and Name substitutions land in the route file and the views."""
plugins_dir = _make_plugins_dir(tmp_path)
target = scaffold_plugin('widgets', 'Test', plugins_dir)
route = (frontend_dir / 'router' / 'routes' / 'widgets.js').read_text()
assert "import('../../views/widgets/WidgetsList.vue')" in route
assert "import('../../views/widgets/WidgetsForm.vue')" in route
assert "import('../../views/widgets/WidgetsDetail.vue')" in route
route = (target / 'frontend' / 'routes.js').read_text()
assert "import('./views/WidgetsList.vue')" in route
assert "import('./views/WidgetsForm.vue')" in route
assert "import('./views/WidgetsDetail.vue')" in route
list_view = (frontend_dir / 'views' / 'widgets' / 'WidgetsList.vue').read_text()
list_view = (target / 'frontend' / 'views' / 'WidgetsList.vue').read_text()
assert 'widgetsApi' in list_view
assert "api.get('/widgets'" in list_view
# no unresolved template placeholders for our known substitution keys
# No unresolved placeholders for our known substitution keys. Checked by
# key, not for a bare '${': the views carry real JS template literals
# (`/widgets/${item.assetid}`) that must survive.
assert '${name}' not in list_view
assert '${Name}' not in list_view
def test_route_file_has_plugin_gating_and_requiresauth(tmp_path):
"""Every route carries meta.plugin; form routes add requiresAuth."""
plugins_dir, frontend_dir = _make_dirs(tmp_path)
scaffold_plugin('widgets', 'Test', plugins_dir, frontend_dir=frontend_dir)
"""ADR-009 gating on every route; requiresAuth on the form routes."""
plugins_dir = _make_plugins_dir(tmp_path)
target = scaffold_plugin('widgets', 'Test', plugins_dir)
route = (frontend_dir / 'router' / 'routes' / 'widgets.js').read_text()
# ADR-009 gating on every route
route = (target / 'frontend' / 'routes.js').read_text()
# every route is gated on the plugin being enabled
assert route.count("plugin: 'widgets'") == 4
# form routes (new + edit) require auth; list + detail do not
# create and edit are behind auth
assert route.count('requiresAuth: true') == 2
assert "path: 'widgets/new'" in route
assert "path: 'widgets/:id/edit'" in route
def test_route_file_exports_toplevel(tmp_path):
"""`toplevel` is exported even when empty.
routes.gen.js reads `.toplevel` off every staged route module; a module
without the export is a Rollup warning under strict ESM.
"""
plugins_dir = _make_plugins_dir(tmp_path)
target = scaffold_plugin('widgets', 'Test', plugins_dir)
route = (target / 'frontend' / 'routes.js').read_text()
assert 'export const toplevel' in route
def test_view_imports_survive_staging(tmp_path):
"""Core imports use the @ alias; sibling views are relative.
stage-frontend.mjs copies `plugins/<name>/frontend/` to
`frontend/src/.plugins-staged/<name>/`, so a `../../api` written for the
old location resolves somewhere else entirely once staged. `@/api` is
correct in both places, and `./views/...` travels with the route file.
"""
plugins_dir = _make_plugins_dir(tmp_path)
target = scaffold_plugin('widgets', 'Test', plugins_dir)
views = target / 'frontend' / 'views'
for view in views.iterdir():
text = view.read_text()
assert '../../api' not in text
assert '../../components' not in text
assert '../../stores' not in text
route = (target / 'frontend' / 'routes.js').read_text()
assert './views/' in route
assert '../../views' not in route
def test_views_use_global_classes_not_custom_input_css(tmp_path):
"""Views lean on global .filters / .form-control / .card classes and never
redefine input styling in a scoped block."""
plugins_dir, frontend_dir = _make_dirs(tmp_path)
scaffold_plugin('widgets', 'Test', plugins_dir, frontend_dir=frontend_dir)
"""Views lean on the global form/filter styles, not per-page input CSS."""
plugins_dir = _make_plugins_dir(tmp_path)
target = scaffold_plugin('widgets', 'Test', plugins_dir)
views = frontend_dir / 'views' / 'widgets'
views = target / 'frontend' / 'views'
list_view = (views / 'WidgetsList.vue').read_text()
form_view = (views / 'WidgetsForm.vue').read_text()
assert 'class="filters"' in list_view
assert 'class="form-control"' in list_view
assert 'class="card"' in list_view
assert 'class="form-control"' in form_view
assert 'form-control' in form_view
# per-page input styling is forbidden: no scoped rule targets .form-control
# on its own (a descendant layout helper like `.filters .form-control` is ok)
import re
for view in (list_view, form_view):
assert not re.search(r'(?m)^\s*\.form-control\s*\{', view)
def test_default_frontend_dir_resolves_relative_to_plugins(tmp_path):
"""With no frontend_dir arg, views land in <plugins_dir>/../frontend/src."""
plugins_dir, frontend_dir = _make_dirs(tmp_path)
scaffold_plugin('widgets', 'Test', plugins_dir)
assert (frontend_dir / 'views' / 'widgets' / 'WidgetsList.vue').exists()
assert (frontend_dir / 'router' / 'routes' / 'widgets.js').exists()
def test_skip_when_no_frontend_dir(tmp_path):
"""No frontend tree: views and route are skipped, backend still generated.
The api snippet is still emitted because it is a plugin-directory artifact.
"""
plugins_dir = tmp_path / 'plugins'
plugins_dir.mkdir()
missing_frontend = tmp_path / 'nowhere' / 'src'
target = scaffold_plugin(
name='widgets',
description='Test',
plugins_dir=plugins_dir,
frontend_dir=missing_frontend,
)
# backend skeleton is intact
assert (target / 'manifest.json').exists()
assert (target / 'plugin.py').exists()
# frontend views and route were skipped
assert not missing_frontend.exists()
# snippet still lands in the plugin dir
assert (target / 'frontend-api-snippet.js').exists()
def test_frontend_can_be_disabled(tmp_path):
"""frontend=False generates neither views, route, nor snippet."""
plugins_dir, frontend_dir = _make_dirs(tmp_path)
plugins_dir = _make_plugins_dir(tmp_path)
target = scaffold_plugin(
name='widgets',
description='Test',
plugins_dir=plugins_dir,
frontend_dir=frontend_dir,
frontend=False,
)
assert not (frontend_dir / 'views' / 'widgets').exists()
assert not (frontend_dir / 'router' / 'routes' / 'widgets.js').exists()
assert not (target / 'frontend').exists()
assert not (target / 'frontend-api-snippet.js').exists()
def test_no_clobber_without_overwrite(tmp_path):
"""An existing frontend view is left untouched unless overwrite is set."""
plugins_dir, frontend_dir = _make_dirs(tmp_path)
# author already hand-edited a view before re-scaffolding
views_dir = frontend_dir / 'views' / 'widgets'
views_dir.mkdir(parents=True)
existing = views_dir / 'WidgetsList.vue'
existing.write_text('SENTINEL do not clobber')
scaffold_plugin('widgets', 'Test', plugins_dir, frontend_dir=frontend_dir)
# untouched without overwrite
assert existing.read_text() == 'SENTINEL do not clobber'
def test_overwrite_replaces_existing_frontend(tmp_path):
"""overwrite=True regenerates an existing frontend view."""
plugins_dir, frontend_dir = _make_dirs(tmp_path)
views_dir = frontend_dir / 'views' / 'widgets'
views_dir.mkdir(parents=True)
existing = views_dir / 'WidgetsList.vue'
existing.write_text('SENTINEL do not clobber')
# plugin dir must also exist to reach overwrite path
scaffold_plugin('widgets', 'Test', plugins_dir, frontend_dir=frontend_dir)
scaffold_plugin(
'widgets', 'Test', plugins_dir,
frontend_dir=frontend_dir, overwrite=True,
)
assert existing.read_text() != 'SENTINEL do not clobber'
assert 'widgetsApi' in existing.read_text()
# backend skeleton is still intact
assert (target / 'manifest.json').exists()
assert (target / 'plugin.py').exists()
def test_route_references_only_existing_views(tmp_path):
@@ -198,55 +197,54 @@ def test_route_references_only_existing_views(tmp_path):
Guards the write-views-before-route ordering: a route pointing at a missing
view crashes the Vite dev server.
"""
plugins_dir, frontend_dir = _make_dirs(tmp_path)
scaffold_plugin('widgets', 'Test', plugins_dir, frontend_dir=frontend_dir)
plugins_dir = _make_plugins_dir(tmp_path)
target = scaffold_plugin('widgets', 'Test', plugins_dir)
route = (frontend_dir / 'router' / 'routes' / 'widgets.js').read_text()
import re
imported = re.findall(r"import\('(\.\./\.\./views/[^']+)'\)", route)
route_dir = target / 'frontend'
route = (route_dir / 'routes.js').read_text()
imported = re.findall(r"import\('(\./views/[^']+)'\)", route)
assert imported
for rel in imported:
# rel is relative to router/routes/; resolve against that dir
resolved = (frontend_dir / 'router' / 'routes' / rel).resolve()
assert resolved.exists(), f'route imports missing view: {rel}'
# rel is relative to the route file, which sits at frontend/routes.js
assert (route_dir / rel).resolve().exists(), \
f'route imports missing view: {rel}'
def test_generated_frontend_is_ascii(tmp_path):
"""Generated frontend files contain plain ASCII only."""
plugins_dir, frontend_dir = _make_dirs(tmp_path)
target = scaffold_plugin(
'widgets', 'Test', plugins_dir, frontend_dir=frontend_dir,
)
plugins_dir = _make_plugins_dir(tmp_path)
target = scaffold_plugin('widgets', 'Test', plugins_dir)
generated = [
frontend_dir / 'views' / 'widgets' / 'WidgetsList.vue',
frontend_dir / 'views' / 'widgets' / 'WidgetsDetail.vue',
frontend_dir / 'views' / 'widgets' / 'WidgetsForm.vue',
frontend_dir / 'router' / 'routes' / 'widgets.js',
target / 'frontend' / 'views' / 'WidgetsList.vue',
target / 'frontend' / 'views' / 'WidgetsDetail.vue',
target / 'frontend' / 'views' / 'WidgetsForm.vue',
target / 'frontend' / 'routes.js',
target / 'frontend-api-snippet.js',
]
for path in generated:
text = path.read_text()
text.encode('ascii') # raises if any non-ASCII slipped in
def test_frontend_dir_not_created_when_absent(tmp_path):
"""Scaffolder does not fabricate a frontend tree that was not there."""
plugins_dir = tmp_path / 'plugins'
plugins_dir.mkdir()
# no frontend dir exists; default resolves to <tmp>/frontend/src (absent)
scaffold_plugin('widgets', 'Test', plugins_dir)
assert not (tmp_path / 'frontend').exists()
path.read_text().encode('ascii') # raises if any non-ASCII slipped in
def test_scaffold_still_raises_on_existing_plugin(tmp_path):
"""Frontend generation does not weaken the plugin-dir overwrite guard."""
plugins_dir, frontend_dir = _make_dirs(tmp_path)
scaffold_plugin('widgets', 'first', plugins_dir, frontend_dir=frontend_dir)
plugins_dir = _make_plugins_dir(tmp_path)
scaffold_plugin('widgets', 'first', plugins_dir)
with pytest.raises(ScaffoldError, match='already exists'):
scaffold_plugin(
'widgets', 'second', plugins_dir, frontend_dir=frontend_dir,
)
scaffold_plugin('widgets', 'second', plugins_dir)
def test_overwrite_replaces_existing_frontend(tmp_path):
"""--overwrite regenerates the plugin's frontend files."""
plugins_dir = _make_plugins_dir(tmp_path)
target = scaffold_plugin('widgets', 'Test', plugins_dir)
list_view = target / 'frontend' / 'views' / 'WidgetsList.vue'
list_view.write_text('CLOBBER ME')
scaffold_plugin('widgets', 'Test', plugins_dir, overwrite=True)
regenerated = list_view.read_text()
assert regenerated != 'CLOBBER ME'
assert 'widgetsApi' in regenerated

View File

@@ -159,3 +159,86 @@ def test_an_unknown_token_answers_the_same_as_a_spent_one(client, auth_headers):
assert spent.status_code == never.status_code == 404
assert (spent.get_json()['data']['error']['message']
== never.get_json()['data']['error']['message'])
# ------------------------------------------------- guests, when a site allows
def _allow_guests(db, allowed=True):
from shopdb.core.models import Setting
row = Setting.query.filter_by(key='securesend_allow_guests').first()
if row is None:
row = Setting(key='securesend_allow_guests', valuetype='boolean',
category='tools')
db.session.add(row)
row.value = 'true' if allowed else 'false'
db.session.commit()
def test_a_guest_is_refused_by_default(client, db):
"""Default OFF. An unauthenticated write endpoint is not something to get
by accident."""
_allow_guests(db, False)
resp = client.post('/api/tools/secrets', json={'ciphertext': CIPHER, 'iv': IV})
assert resp.status_code in (401, 403, 422)
def test_a_guest_can_create_once_a_site_allows_it(client, db):
_allow_guests(db, True)
resp = client.post('/api/tools/secrets', json={'ciphertext': CIPHER, 'iv': IV})
assert resp.status_code in (200, 201), resp.get_data(as_text=True)[:300]
assert resp.get_json()['data']['token']
def test_a_guest_secret_has_no_creator(client, db):
"""The cost of the setting, pinned: the audit trail can say when and from
where, but never who."""
_allow_guests(db, True)
token = client.post('/api/tools/secrets',
json={'ciphertext': CIPHER, 'iv': IV}
).get_json()['data']['token']
secret = SecureSecret.query.filter_by(token=token).first()
assert secret.createdbyuserid is None
def test_guest_creation_is_rate_limited(client, db):
"""An open endpoint must not be an easy place to park data."""
from plugins.tools.api.routes import GUEST_CREATE_MAX
_allow_guests(db, True)
codes = [client.post('/api/tools/secrets',
json={'ciphertext': CIPHER, 'iv': IV}).status_code
for _ in range(GUEST_CREATE_MAX + 2)]
assert 429 in codes, codes
def test_a_signed_in_user_still_needs_the_permission(client, db, member_headers):
"""Opening the door to guests is a decision about the DOOR, not about
revoking what a role holds."""
_allow_guests(db, True)
resp = client.post('/api/tools/secrets',
json={'ciphertext': CIPHER, 'iv': IV},
headers=member_headers)
assert resp.status_code == 403
# ----------------------------------------------------------------- retention
def test_an_expired_secret_nobody_opened_is_swept(client, db, auth_headers):
"""Nothing removed a secret that was never clicked: the recipient goes on
holiday and the ciphertext sits there for good."""
from datetime import datetime, timedelta, timezone
token = _create(client, auth_headers).get_json()['data']['token']
secret = SecureSecret.query.filter_by(token=token).first()
secret.expiresat = (datetime.now(timezone.utc).replace(tzinfo=None)
- timedelta(hours=1))
db.session.commit()
# Any later create sweeps it, without a scheduler existing anywhere.
_create(client, auth_headers)
assert SecureSecret.query.filter_by(token=token).first() is None
def test_a_custom_view_count_is_allowed(client, auth_headers):
from plugins.tools.api.routes import MAX_VIEWS
assert _create(client, auth_headers, views=25).status_code in (200, 201)
assert _create(client, auth_headers, views=MAX_VIEWS).status_code in (200, 201)
assert _create(client, auth_headers, views=MAX_VIEWS + 1).status_code == 400