Release 0.8.0: Windows installer, twelve fixes from a real Server 2019 install, operator docs

tests/test_csv_import.py

@@ -0,0 +1,236 @@
+"""Bulk CSV import: name resolution, all-or-nothing safety, and template truth.
+
+The behaviour under test is what makes a spreadsheet import usable by someone
+who does not have the database in front of them. In particular: a foreign key
+column accepts the NAME of the thing it points at. Requiring numeric ids means
+importing a file, reading back generated ids, and pasting them into the next
+file - a workflow nobody finishes.
+"""
+import pytest
+
+from shopdb.core.services.csvimport import (
+    IMPORTABLE, ImportError_, Resolver, generate_template, import_csv,
+    importable_columns, required_columns,
+)
+from shopdb.extensions import db as _db
+
+
+REFERENCE = [
+    ('assettypes', 'assettype,description\nMachine,Shop floor\nMeasuring Tool,Gage lab\n'),
+    ('assetstatuses', 'status,description\nActive,In service\nRetired,Out\n'),
+    ('locationtypes', 'locationtype\nBay\nRoom\n'),
+]
+
+
+def seed_reference(resolver):
+    for table, text in REFERENCE:
+        result = import_csv(table, text, resolver=resolver, commit=True)
+        assert result.ok, [str(p) for p in result.problems]
+    result = import_csv(
+        'locations',
+        'locationname,building,locationtypeid\nBay 3,Building 1,Bay\nGage Lab,Building 2,Room\n',
+        resolver=resolver, commit=True)
+    assert result.ok, [str(p) for p in result.problems]
+
+
+def count(table):
+    return _db.session.execute(_db.text('select count(*) from %s' % table)).scalar()
+
+
+def test_foreign_keys_resolve_by_name(db):
+    """The headline behaviour: write 'Gage Lab', not locationid=2."""
+    resolver = Resolver()
+    seed_reference(resolver)
+    result = import_csv(
+        'assets',
+        'assetnumber,name,assettypeid,statusid,locationid\n'
+        'CMM-01,Zeiss,Measuring Tool,Active,Gage Lab\n',
+        resolver=resolver, commit=True)
+    assert result.ok, [str(p) for p in result.problems]
+
+    row = _db.session.execute(_db.text(
+        'select t.assettype, s.status, l.locationname from assets a '
+        'join assettypes t on t.assettypeid = a.assettypeid '
+        'join assetstatuses s on s.statusid = a.statusid '
+        'join locations l on l.locationid = a.locationid')).fetchone()
+    assert tuple(row) == ('Measuring Tool', 'Active', 'Gage Lab')
+
+
+def test_numeric_ids_still_work(db):
+    """Exports round-trip, and some sites genuinely know their ids."""
+    resolver = Resolver()
+    seed_reference(resolver)
+    typeid = _db.session.execute(_db.text(
+        "select assettypeid from assettypes where assettype = 'Machine'")).scalar()
+    result = import_csv(
+        'assets', 'assetnumber,assettypeid\nMILL-1,%d\n' % typeid,
+        resolver=resolver, commit=True)
+    assert result.ok, [str(p) for p in result.problems]
+    assert count('assets') == 1
+
+
+def test_unknown_name_names_the_column_and_the_value(db):
+    """The error has to be actionable by someone holding a spreadsheet."""
+    resolver = Resolver()
+    seed_reference(resolver)
+    result = import_csv(
+        'assets',
+        'assetnumber,assettypeid,locationid\nX-1,Machine,Bay 9\n',
+        resolver=resolver, commit=False)
+    assert not result.ok
+    message = str(result.problems[0])
+    assert "locationid" in message
+    assert "Bay 9" in message
+    assert "locations.csv" in message
+
+
+def test_reimport_updates_rather_than_duplicating(db):
+    """Sites correct their spreadsheet and run it again. That must not double up."""
+    resolver = Resolver()
+    seed_reference(resolver)
+    text = 'assetnumber,name,assettypeid\nCMM-01,Zeiss,Machine\n'
+    first = import_csv('assets', text, resolver=resolver, commit=True)
+    assert (first.created, first.updated) == (1, 0)
+
+    renamed = 'assetnumber,name,assettypeid\nCMM-01,Zeiss Contura,Machine\n'
+    second = import_csv('assets', renamed, resolver=resolver, commit=True)
+    assert (second.created, second.updated) == (0, 1)
+    assert count('assets') == 1
+    assert _db.session.execute(_db.text(
+        "select name from assets where assetnumber = 'CMM-01'")).scalar() == 'Zeiss Contura'
+
+
+def test_one_bad_row_writes_nothing(db):
+    """A typo on the last line must not leave the earlier lines imported."""
+    resolver = Resolver()
+    seed_reference(resolver)
+    result = import_csv(
+        'assets',
+        'assetnumber,assettypeid\n'
+        'GOOD-1,Machine\n'
+        'GOOD-2,Machine\n'
+        'BAD-3,NoSuchType\n',
+        resolver=resolver, commit=True)
+    assert not result.ok
+    assert count('assets') == 0, 'rows were written despite a failure'
+
+
+def test_unknown_column_is_rejected_with_the_accepted_list(db):
+    """The failure mode of a hand-written template: invented columns."""
+    resolver = Resolver()
+    seed_reference(resolver)
+    result = import_csv(
+        'vendors', 'vendor,zipcode\nDell,78682\n', resolver=resolver, commit=False)
+    assert not result.ok
+    message = str(result.problems[0])
+    assert 'zipcode' in message
+    assert 'this table accepts' in message
+
+
+def test_missing_required_column_is_rejected(db):
+    result = import_csv('assets', 'name\nNo asset number\n', commit=False)
+    assert not result.ok
+    assert 'assetnumber' in str(result.problems[0])
+
+
+def test_booleans_take_1_and_0_and_reject_prose(db):
+    resolver = Resolver()
+    seed_reference(resolver)
+    ok = import_csv('assets', 'assetnumber,assettypeid,isactive\nA-1,Machine,0\n',
+                    resolver=resolver, commit=True)
+    assert ok.ok, [str(p) for p in ok.problems]
+
+    bad = import_csv('assets', 'assetnumber,assettypeid,isactive\nA-2,Machine,maybe\n',
+                     resolver=resolver, commit=False)
+    assert not bad.ok
+    assert 'isactive' in str(bad.problems[0])
+
+
+def test_comment_and_blank_lines_are_ignored(db):
+    """The templates ship a commented example. Importing one unedited should be
+    a no-op, not a row of literal placeholder text."""
+    resolver = Resolver()
+    seed_reference(resolver)
+    result = import_csv(
+        'assets',
+        '# a comment\nassetnumber,assettypeid\n\n# <assetnumber>,<type>\nREAL-1,Machine\n',
+        resolver=resolver, commit=True)
+    assert result.ok, [str(p) for p in result.problems]
+    assert result.created == 1
+
+
+def test_passwords_are_never_importable():
+    """A CSV carrying password hashes gets mailed around; one carrying plaintext
+    is worse. Neither column may be set from a file."""
+    for table in IMPORTABLE:
+        names = {c.name for c in importable_columns(table)}
+        assert 'passwordhash' not in names
+        assert 'password' not in names
+
+
+def test_generated_templates_only_contain_real_columns(db):
+    """The regression that motivated all of this.
+
+    A hand-written template set had invented columns on 7 of 11 tables and named
+    a table that does not exist. Generating from the schema is what makes that
+    impossible - so assert the generator actually does it.
+    """
+    for table in IMPORTABLE:
+        text = generate_template(table)
+        header = [line for line in text.splitlines() if line and not line.startswith('#')][0]
+        real = {c.name for c in importable_columns(table)}
+        for column in header.split(','):
+            assert column in real, '%s.csv offers a column that does not exist: %s' % (table, column)
+
+
+def test_every_template_includes_its_required_columns(db):
+    """A template that omits a required column produces a file that cannot import."""
+    for table in IMPORTABLE:
+        header = [line for line in generate_template(table).splitlines()
+                  if line and not line.startswith('#')][0].split(',')
+        for column in required_columns(table):
+            assert column in header, '%s.csv is missing required column %s' % (table, column)
+
+
+def test_importable_tables_all_exist(db):
+    """A registry entry naming a table that is not in the schema would fail only
+    when someone tried to use it."""
+    for table in IMPORTABLE:
+        assert table in _db.metadata.tables, '%s is registered but not in the schema' % table
+
+
+def test_unimportable_table_is_refused(db):
+    with pytest.raises(ImportError_):
+        import_csv('auditlog', 'x\n1\n', commit=False)
+
+
+def test_dry_run_reports_counts_then_rolls_back(db):
+    """The number an operator needs before saying yes - and no trace afterwards.
+
+    Writes go into the transaction even on a dry run, which is what lets a later
+    file resolve names an earlier one created. The rollback is what makes it a
+    dry run.
+    """
+    resolver = Resolver()
+    seed_reference(resolver)
+    result = import_csv(
+        'assets', 'assetnumber,assettypeid\nD-1,Machine\nD-2,Machine\n',
+        resolver=resolver)
+    assert result.ok
+    assert result.created == 2
+    _db.session.rollback()
+    assert count('assets') == 0
+
+
+def test_a_dry_run_over_several_files_resolves_across_them(db):
+    """The whole-folder check: assets must resolve locations that only exist
+    because an earlier file in the same run created them."""
+    resolver = Resolver()
+    seed_reference(resolver)
+    result = import_csv(
+        'assets',
+        'assetnumber,assettypeid,locationid\nX-1,Machine,Bay 3\n',
+        resolver=resolver)
+    assert result.ok, [str(p) for p in result.problems]
+    _db.session.rollback()
+    assert count('assets') == 0

tests/test_installer_defaults.py

@@ -53,3 +53,54 @@ def test_every_wizard_exclusion_is_a_real_plugin():
     for name in wizard_excludes():
         assert (PLUGINS / name / 'manifest.json').exists(), (
             '%s is excluded by the wizard but has no manifest' % name)
+
+
+# The preflight blocks the wizard on any FAIL. That is correct for something the
+# operator must go and fix, and wrong for anything the installer carries in its
+# own bundle - there, blocking stops the wizard over something it was about to do
+# itself, with no way forward.
+BUNDLE_SUPPLIED = [
+    'HttpPlatformHandler',   # httpplatformhandler\*.msi, installed by stage 4
+    'URL Rewrite',           # urlrewrite\*.msi, installed by stage 4
+]
+
+PREFLIGHT = REPO / 'deploy' / 'windows' / 'installer' / 'shopdb-preflight.ps1'
+
+
+def test_nothing_the_bundle_supplies_is_a_blocker():
+    """Regression: HttpPlatformHandler was a FAIL, so a server without it could
+    not get past the preflight page - to install the very thing that was
+    missing."""
+    text = PREFLIGHT.read_text(encoding='utf-8', errors='replace')
+    offenders = []
+    for line in text.splitlines():
+        if "'FAIL'" not in line:
+            continue
+        for component in BUNDLE_SUPPLIED:
+            if component.lower() in line.lower():
+                offenders.append(line.strip())
+    assert not offenders, (
+        'the installer supplies these, so they must not block the wizard:\n  %s'
+        % '\n  '.join(offenders))
+
+
+def test_installer_controlled_paths_have_no_spaces():
+    """msiexec and bootstrapper command lines split on spaces.
+
+    The installer passes these as PROPERTY=value / TargetDir=value on a command
+    line. A value containing a space has to be quoted as PROPERTY="C:\\..." -
+    quoting the whole token instead makes the tool reject the line and print its
+    usage dialog, which is what 'the bundled MySQL database could not be
+    installed' looked like from the outside. The Python target was already
+    space-free for this exact reason; MySQL was not.
+    """
+    text = PREFLIGHT.parent.joinpath('shopdb-install.ps1').read_text(
+        encoding='utf-8', errors='replace')
+    offenders = []
+    for line in text.splitlines():
+        match = re.match(r"\s*\[string\]\s*\$(Mysql\w*|PyTarget)\s*=\s*'([^']+)'", line)
+        if match and ' ' in match.group(2):
+            offenders.append('%s = %s' % (match.group(1), match.group(2)))
+    assert not offenders, (
+        'these are passed on a command line and must not contain spaces:\n  %s'
+        % '\n  '.join(offenders))

tests/test_installer_state.py

@@ -0,0 +1,106 @@
+"""Guards on the installer's durable install record.
+
+The record replaced a set of guesses. "Is this a re-run of MY install?" used to
+be answered from whatever the machine looked like - a MySQL service exists, the
+database has tables, the site exists - and none of those say who created them.
+A retry after a failed first install was therefore taken for an upgrade of a
+working system, which dead-ended the retry the wizard invites the operator to
+run.
+
+Getting it wrong is asymmetric. Answering "still first provisioning" for a
+server holding real data would let `prune-schema --force` drop live tables. The
+behavioural checks live in the PowerShell harness next to the installer, run
+here when pwsh is available; the text assertions below hold everywhere, because
+CI does not necessarily have pwsh and these are the invariants worth failing a
+build over.
+"""
+
+import shutil
+import subprocess
+from pathlib import Path
+
+import pytest
+
+REPO = Path(__file__).resolve().parent.parent
+INSTALLER = REPO / 'deploy' / 'windows' / 'installer' / 'shopdb-install.ps1'
+HARNESS = REPO / 'deploy' / 'windows' / 'installer' / 'tests' / 'test-install-state.ps1'
+
+pytestmark = pytest.mark.skipif(
+    not INSTALLER.is_file(),
+    reason='installer not in this checkout')
+
+
+def installer_text():
+    return INSTALLER.read_text(encoding='utf-8', errors='replace')
+
+
+def test_prune_force_is_gated_on_first_provisioning():
+    """--force drops tables that contain rows.
+
+    It is correct on a first install, where core migrations seed a few plugin
+    reference tables, and destructive on an upgrade, where those rows are the
+    site's data. The gate must therefore test BOTH "database was empty" and the
+    install record, never the database alone.
+    """
+    text = installer_text()
+    assert "'prune-schema','--yes','--force'" in text, 'the forcing call moved or was renamed'
+    guard = "if ((-not $script:DbWasEmpty) -and (-not $firstRun)) {"
+    assert guard in text, (
+        'the prune gate no longer consults the install record. Forcing on the '
+        'strength of an empty database alone is what made every retry of a '
+        'failed first install fail at stage 3.')
+
+
+def test_first_provisioning_comes_from_the_record_not_the_database():
+    text = installer_text()
+    assert 'function Test-FirstProvisioning' in text
+    assert '$firstRun = Test-FirstProvisioning' in text, (
+        'stage 3 must ask the record, not infer from table counts')
+
+
+def test_missing_record_takes_the_safe_side():
+    """An install predating the record has a version stamp and probably data.
+
+    Treating that as a first run would arm prune --force against it.
+    """
+    text = installer_text()
+    assert "'predates-this-record'" in text, (
+        'the fallback for an unrecorded but stamped install is gone; without it '
+        'an established server can be classified as first provisioning')
+
+
+def test_created_items_returns_an_array_when_empty():
+    """A zero-length array returned from a PowerShell function unrolls to $null.
+
+    $null.Count is then a terminating error under Set-StrictMode -Version 2.0 -
+    the same fault that made bundle verification fail on every install. The
+    leading comma is what prevents it.
+    """
+    text = installer_text()
+    assert 'return ,@()' in text, 'Get-CreatedItems lost its comma protection'
+    assert 'return ,@(Get-StateValue $created $Kind' in text, (
+        'the populated return path lost its comma protection')
+
+
+def test_record_is_written_before_any_stage_runs():
+    """Written at the START of provisioning, not the end.
+
+    A run that dies halfway is exactly the run whose retry needs the record.
+    """
+    text = installer_text()
+    assert "if ($Stage -ne 'uninstall') { Initialize-InstallState }" in text
+    start = text.index('Initialize-InstallState }')
+    switch = text.index('switch ($Stage)', start - 400)
+    assert start < switch, 'the record must be initialised before the stage switch'
+
+
+@pytest.mark.skipif(shutil.which('pwsh') is None, reason='pwsh not installed')
+def test_install_state_behaviour():
+    """Run the PowerShell harness: retries, upgrades, corrupt and foreign records."""
+    assert HARNESS.is_file(), 'the PowerShell harness is missing'
+    result = subprocess.run(
+        ['pwsh', '-NoProfile', '-File', str(HARNESS)],
+        capture_output=True, text=True, timeout=300)
+    assert result.returncode == 0, (
+        'install-record harness failed:\n' + result.stdout + result.stderr)
+    assert 'all checks passed' in result.stdout

tests/test_runtime_dependencies.py

@@ -0,0 +1,159 @@
+"""Every third-party module the app imports at runtime must be a DECLARED dependency.
+
+The bug this exists to prevent: `shopdb/plugins/loader.py` imports `packaging`,
+which was never listed in requirements.in. Locally and in CI that import
+resolved anyway, because pytest depends on packaging -- so all 1159 tests
+passed. The Windows installer builds its venv from requirements.txt alone, with
+no test dependencies, so `import shopdb` raised ModuleNotFoundError and the
+whole application failed to start on a customer server.
+
+Test dependencies are exactly the blind spot: they are present everywhere the
+test suite runs and absent everywhere it does not.
+
+The check is static (AST) rather than a real import, so it covers plugins that
+are not installed at this site and code paths no test exercises.
+"""
+
+import ast
+import sys
+from pathlib import Path
+
+import pytest
+
+try:
+    from importlib.metadata import packages_distributions
+except ImportError:  # pragma: no cover - Python < 3.10
+    packages_distributions = None
+
+REPO = Path(__file__).resolve().parent.parent
+SCANNED = ('shopdb', 'plugins', 'scripts')
+
+# First-party top-level names. Imports of these resolve from the source tree.
+FIRST_PARTY = {'shopdb', 'plugins', 'scripts', 'migrations', 'tests', 'wsgi', 'conftest'}
+
+# Modules deliberately imported without being declared. Each needs a reason.
+ALLOWED_UNDECLARED = {
+    # Provided by the interpreter's own bundled installer tooling, and only
+    # touched by developer scripts that never run on a deployed site.
+    'pip',
+    'setuptools',
+    'pkg_resources',
+}
+
+
+def _normalise(name):
+    """PEP 503 normalisation, so Flask_Migrate and flask-migrate compare equal."""
+    return name.lower().replace('_', '-').replace('.', '-')
+
+
+def _declared_distributions():
+    """Distribution names pinned in requirements.txt."""
+    text = (REPO / 'requirements.txt').read_text(encoding='utf-8')
+    declared = set()
+    for line in text.splitlines():
+        line = line.strip()
+        if not line or line.startswith('#') or line.startswith('--'):
+            continue
+        # "name==1.2.3 ; marker \" -> "name"
+        head = line.split('==', 1)[0].split(';', 1)[0].strip()
+        if head:
+            declared.add(_normalise(head))
+    return declared
+
+
+def _iter_source_files():
+    for top in SCANNED:
+        root = REPO / top
+        if not root.is_dir():
+            continue
+        for path in root.rglob('*.py'):
+            parts = set(path.parts)
+            if {'node_modules', 'venv', '.venv', 'build'} & parts:
+                continue
+            yield path
+
+
+def _imported_top_level_modules(path):
+    """Top-level module names imported unconditionally by `path`.
+
+    Imports nested inside a `try:` are skipped: that is how this codebase spells
+    an optional dependency, and a missing one is handled rather than fatal.
+    Imports inside functions are still counted -- a deferred import of an
+    undeclared package fails just as hard, only later.
+    """
+    try:
+        tree = ast.parse(path.read_text(encoding='utf-8'), filename=str(path))
+    except SyntaxError:
+        pytest.fail('could not parse {}'.format(path.relative_to(REPO)))
+
+    guarded = set()
+    for node in ast.walk(tree):
+        if isinstance(node, ast.Try):
+            for child in ast.walk(node):
+                guarded.add(id(child))
+
+    found = set()
+    for node in ast.walk(tree):
+        if id(node) in guarded:
+            continue
+        if isinstance(node, ast.Import):
+            for alias in node.names:
+                found.add(alias.name.split('.')[0])
+        elif isinstance(node, ast.ImportFrom):
+            # level > 0 is a relative (first-party) import.
+            if node.level == 0 and node.module:
+                found.add(node.module.split('.')[0])
+    return found
+
+
+@pytest.mark.skipif(packages_distributions is None, reason='needs Python 3.10+')
+def test_runtime_imports_are_declared_dependencies():
+    declared = _declared_distributions()
+    assert 'flask' in declared, 'requirements.txt did not parse as expected'
+
+    module_to_dists = packages_distributions()
+    stdlib = getattr(sys, 'stdlib_module_names', frozenset())
+
+    offenders = {}
+    for path in _iter_source_files():
+        for module in _imported_top_level_modules(path):
+            if module in FIRST_PARTY or module in stdlib or module in ALLOWED_UNDECLARED:
+                continue
+            if module.startswith('_'):
+                continue
+            dists = module_to_dists.get(module)
+            if not dists:
+                # Not installed in this environment, so its distribution cannot
+                # be identified. Absence here is not evidence of a problem.
+                continue
+            if not any(_normalise(d) in declared for d in dists):
+                rel = str(path.relative_to(REPO))
+                offenders.setdefault(
+                    '{} (from {})'.format(module, '/'.join(sorted(dists))), []
+                ).append(rel)
+
+    if offenders:
+        lines = []
+        for module, files in sorted(offenders.items()):
+            shown = sorted(files)[:3]
+            more = '' if len(files) <= 3 else ' (+{} more)'.format(len(files) - 3)
+            lines.append('  {}\n      {}{}'.format(module, ', '.join(shown), more))
+        pytest.fail(
+            'Imported at runtime but not declared in requirements.txt.\n'
+            'These resolve here only because a test dependency supplies them; a\n'
+            'production venv has no test dependencies and will fail to import.\n'
+            'Add them to requirements.in and recompile.\n\n' + '\n'.join(lines)
+        )
+
+
+def test_packaging_is_declared():
+    """Regression: the specific import that broke the Windows install.
+
+    Kept separate from the sweep above because that one can only flag a module
+    it can attribute to an installed distribution -- an environment without
+    packaging would skip it silently, which is precisely the shipping case.
+    """
+    assert 'packaging' in _declared_distributions(), (
+        'shopdb/plugins/loader.py imports packaging at module scope. Without it '
+        'declared, `import shopdb` fails on any venv built from requirements.txt.'
+    )