diff --git a/deploy/windows/installer/shopdb-install.ps1 b/deploy/windows/installer/shopdb-install.ps1 index bf57df2..d7c0485 100644 --- a/deploy/windows/installer/shopdb-install.ps1 +++ b/deploy/windows/installer/shopdb-install.ps1 @@ -183,7 +183,146 @@ function Fail { throw $Message } -function Track { param([string] $Kind, [string] $Id) $null = $script:Created.Add(@{Kind=$Kind; Id=$Id}) } +# ----------------------------------------------------------------------------- +# DURABLE INSTALL RECORD +# +# "Is this a re-run of MY install?" used to be answered from whatever the machine +# happened to look like: a MySQL service exists, the database has tables, the +# site exists, the venv exists. None of those record WHO created them, so a retry +# after a failed first attempt was repeatedly mistaken for an upgrade of somebody +# else's working system. That single confusion produced several separate dead +# ends, the worst being a failed first install demanding a mandatory backup of +# its own half-written database and then refusing to prune tables it had created +# itself minutes earlier. +# +# This file answers the question instead. It is written when provisioning +# STARTS, not when it finishes, so a run that dies halfway still leaves a record +# the next run can read - which is the whole point, because the run that dies +# halfway is the one whose retry needs to know. +# ----------------------------------------------------------------------------- +$script:StateFile = Join-Path $env:ProgramData 'ShopDB-Flask\install-state.json' +$script:InstallState = $null + +function Get-StateValue { + # Set-StrictMode makes reading a missing property a terminating error, so + # every read of a file that may be older, hand-edited or truncated goes + # through here rather than dotting into it directly. + param($Object, [string] $Name) + if ($null -eq $Object) { return $null } + $prop = $Object.PSObject.Properties[$Name] + if ($null -eq $prop) { return $null } + return $prop.Value +} + +function Save-InstallState { + param($State) + # Never fatal. The record is an improvement on guessing; failing to write it + # only returns the installer to the behaviour it had before, so a permissions + # problem here must not stop an install that is otherwise fine. + try { + $dir = Split-Path $script:StateFile -Parent + if (-not (Test-Path $dir)) { New-Item -ItemType Directory -Path $dir -Force | Out-Null } + ($State | ConvertTo-Json -Depth 6) | Set-Content -Path $script:StateFile -Encoding UTF8 + Protect-File $script:StateFile + } catch { + Write-Log "could not write the install record: $($_.Exception.Message)" 'WARN' + } +} + +function Get-InstallState { + if (-not (Test-Path $script:StateFile)) { return $null } + try { + $raw = Get-Content $script:StateFile -Raw -ErrorAction Stop + if (-not $raw) { return $null } + $obj = $raw | ConvertFrom-Json -ErrorAction Stop + } catch { + Write-Log 'the install record is unreadable; falling back to inspecting the server' 'WARN' + return $null + } + # A record describing a DIFFERENT directory says nothing about this one. + $recorded = Get-StateValue $obj 'approot' + if ($recorded -and -not (Test-SamePath $recorded $AppRoot)) { return $null } + return $obj +} + +function Initialize-InstallState { + $state = Get-InstallState + if ($null -eq $state) { + # No record at all. Either this is a genuinely new server, or an install + # that predates this file. Distinguish by the version stamp, and when in + # doubt choose the SAFE side: an established install, which keeps the + # mandatory pre-upgrade backup. Guessing "first run" for a server holding + # real data would let prune --force loose on it. + $stamped = Test-Path (Join-Path $AppRoot '.installed-version') + $state = [PSCustomObject]@{ + schema = 1 + approot = $AppRoot + firstrunstarted = (Get-Date).ToUniversalTime().ToString('yyyy-MM-ddTHH:mm:ssZ') + firstruncompleted = $(if ($stamped) { 'predates-this-record' } else { $null }) + created = [PSCustomObject]@{} + } + if ($stamped) { + Write-Log 'no install record found, but this server carries a version stamp' 'WARN' + Write-Log ' treating it as an established install, so the pre-upgrade backup stays mandatory' 'WARN' + } + Save-InstallState $state + } + $script:InstallState = $state +} + +function Test-FirstProvisioning { + # TRUE only while OUR first provisioning of this directory is unfinished - + # including every retry of it. Whatever is in the database at that point was + # put there by an earlier attempt of this same install, not by a running site. + if ($null -eq $script:InstallState) { return $false } + return ($null -eq (Get-StateValue $script:InstallState 'firstruncompleted')) +} + +function Complete-InstallState { + if ($null -eq $script:InstallState) { return } + if ($null -ne (Get-StateValue $script:InstallState 'firstruncompleted')) { return } + $script:InstallState.firstruncompleted = + (Get-Date).ToUniversalTime().ToString('yyyy-MM-ddTHH:mm:ssZ') + Save-InstallState $script:InstallState +} + +function Track { + param([string] $Kind, [string] $Id) + $null = $script:Created.Add(@{Kind=$Kind; Id=$Id}) + # Persist as well. $script:Created lives only in memory, so a run that died + # left nothing behind saying what it had already made, and the next run was + # back to inferring it from the machine. + try { + if ($null -ne $script:InstallState) { + $created = Get-StateValue $script:InstallState 'created' + if ($null -eq $created) { + $created = [PSCustomObject]@{} + $script:InstallState | Add-Member -NotePropertyName 'created' -NotePropertyValue $created -Force + } + $list = @(Get-StateValue $created $Kind | Where-Object { $_ }) + if ($list -notcontains $Id) { + $created | Add-Member -NotePropertyName $Kind -NotePropertyValue (@($list) + $Id) -Force + Save-InstallState $script:InstallState + } + } + } catch { } +} + +function Get-CreatedItems { + # What THIS installer recorded creating, by kind. Empty when there is no + # record, which callers must treat as "unknown", never as "nothing". + # + # The leading comma is load-bearing. Returning a zero-length array from a + # PowerShell function UNROLLS it to $null, so the caller gets $null and + # $null.Count is a terminating error under Set-StrictMode - the exact fault + # that made bundle verification fail on every install earlier. Comma-wrapping + # emits the array itself as a single object, so an empty result stays an + # empty array. + param([string] $Kind) + if ($null -eq $script:InstallState) { return ,@() } + $created = Get-StateValue $script:InstallState 'created' + return ,@(Get-StateValue $created $Kind | Where-Object { $_ }) +} function Invoke-Native { # Runs an external command and fails loudly. @@ -1543,9 +1682,22 @@ See deploy/site-profile.example.json for the format. # separately runnable ("re-run stage 3"), and under Set-StrictMode # reading an unset $script:IsUpgrade is a terminating error. $script:DbWasEmpty = Test-DatabaseEmpty + # Tables present does NOT mean upgrade. It meant that for a long time, + # and it is why the retry after a failed first install dead-ended: stage 3 + # had already created the schema before dying, so the retry found tables, + # called itself an upgrade of a live site, and then demanded a mandatory + # backup and refused to prune - of a database it had written itself. + # The install record distinguishes the two, which nothing on the machine + # can. + $firstRun = Test-FirstProvisioning if (-not $script:DbWasEmpty) { - $script:IsUpgrade = $true - Write-Log 'existing data found in the target database - treating this as an upgrade' 'WARN' + if ($firstRun) { + Write-Log 'the database already has tables, but the install record shows first provisioning is still unfinished' 'WARN' + Write-Log ' they were created by an earlier attempt of THIS install, so this is not an upgrade' 'WARN' + } else { + $script:IsUpgrade = $true + Write-Log 'existing data found in the target database - treating this as an upgrade' 'WARN' + } } # Back up BEFORE Alembic touches anything. Skipped only when the database @@ -1555,10 +1707,19 @@ See deploy/site-profile.example.json for the format. if (-not $script:DbWasEmpty) { $script:PreUpgradeBackup = Backup-Database 'pre-upgrade' if (-not $script:PreUpgradeBackup) { - Fail 'could not back up the database before upgrading' @' + if ($firstRun) { + # Nothing of anyone's to lose: every row was written by an + # earlier attempt of this same install. Refusing here turned a + # missing mysqldump into a hard dead end on exactly the retry + # the wizard tells the operator is safe to run. + Write-Log 'no pre-migration backup was taken' 'WARN' + Write-Log ' continuing anyway: this is still first provisioning, so the only data present is what an earlier attempt wrote' 'WARN' + } else { + Fail 'could not back up the database before upgrading' @' Refusing to migrate without a backup. Fix mysqldump access (or take a backup manually with shopdb-admin.ps1 backup) and run this again. '@ + } } } @@ -1636,7 +1797,12 @@ build to get a matching pair, then send the install log to support. # data for any plugin that is not installed - so never force there. # Without --force, prune-schema refuses non-empty tables and says so, # which is the behaviour we want when data exists. - if (-not $script:DbWasEmpty) { + # --force is safe during first provisioning for the same reason the backup + # is not required: every row present was written by an earlier attempt of + # this install. Core migration 7d05 seeds access protocols owned by the + # computers plugin, so any profile omitting computers hit the refusal on + # every single retry and could never get past stage 3. + if ((-not $script:DbWasEmpty) -and (-not $firstRun)) { Write-Log 'flask plugin prune-schema --yes (no --force: refuses to drop tables holding data)' # -OkExit 0,1: refusing is the DESIGNED outcome here, and the CLI # signals it with SystemExit(1). Without this, Invoke-Native failed @@ -1657,9 +1823,15 @@ build to get a matching pair, then send the install log to support. Fail 'the plugin manager did not initialise' 'Check the application log; the app could not start.' } } else { - # Provably-empty database only: core migrations seed a few plugin - # reference tables, so prune would otherwise refuse. - Write-Log 'flask plugin prune-schema --yes --force (database was empty at start)' + # Reached when the database was provably empty, OR when the install + # record says first provisioning is still unfinished. Both mean the + # only rows present are ones this install put there - core migrations + # seed a few plugin reference tables, so prune would otherwise refuse. + if ($script:DbWasEmpty) { + Write-Log 'flask plugin prune-schema --yes --force (database was empty at start)' + } else { + Write-Log 'flask plugin prune-schema --yes --force (still first provisioning: these tables are ours)' + } Invoke-Native $Flask @('plugin','prune-schema','--yes','--force') 'prune not-installed plugin tables' } } @@ -2492,6 +2664,11 @@ Write-Log "ShopDB-Flask installer, stage=$Stage, approot=$AppRoot, bundle=$Bundl if ($WhatIfOnly) { Write-Log 'WhatIfOnly: no changes will be made' 'WARN' } try { + # Before ANY stage, so that a stage which dies still leaves a record saying + # provisioning had started. Skipped for uninstall, which must not create a + # record of an install it is removing. + if ($Stage -ne 'uninstall') { Initialize-InstallState } + switch ($Stage) { '0' { Invoke-Stage0 } '1' { & (Join-Path $PSScriptRoot 'shopdb-preflight.ps1') -SitePort $SitePort -AppRoot $AppRoot } @@ -2504,6 +2681,11 @@ try { 'all' { Invoke-Stage2; Invoke-Stage3; Invoke-Stage4; Invoke-Stage5 } 'uninstall' { Invoke-Uninstall } } + # Provisioning is finished only when a run that INCLUDED the later stages + # completed. A successful stage 2 on its own leaves first-run open, because + # the schema and the site are still ahead of it, and the retry that follows + # must still be treated as part of the first install. + if ($Stage -eq 'all' -or $Stage -eq '5') { Complete-InstallState } Write-Log "done. log: $script:LogPath" 'OK' exit 0 } diff --git a/deploy/windows/installer/tests/test-install-state.ps1 b/deploy/windows/installer/tests/test-install-state.ps1 new file mode 100644 index 0000000..c5e19dc --- /dev/null +++ b/deploy/windows/installer/tests/test-install-state.ps1 @@ -0,0 +1,130 @@ +# Tests the installer's durable install record. +# +# The record answers one question: "is this a re-run of MY install?". Getting it +# wrong is destructive in one direction - answering "first provisioning" for a +# server holding real data lets prune-schema --force loose on it - and a dead +# end in the other, which is what it was built to fix. +# +# Run directly: pwsh -File test-install-state.ps1 +# pytest runs it through tests/test_installer_state.py wherever pwsh exists. + +Set-StrictMode -Version 2.0 +$ErrorActionPreference = 'Stop' + +$Here = Split-Path -Parent $MyInvocation.MyCommand.Path +$Installer = Join-Path (Split-Path -Parent $Here) 'shopdb-install.ps1' +if (-not (Test-Path $Installer)) { throw "installer not found at $Installer" } + +$Sandbox = Join-Path ([System.IO.Path]::GetTempPath()) ('shopdb-state-' + [System.Guid]::NewGuid().ToString('N')) + +# Load ONLY the state block, so the rest of the installer does not run. Bounded +# by the same markers the installer uses, and it fails loudly if either moves +# rather than silently testing nothing. +$lines = Get-Content $Installer +$start = ($lines | Select-String -Pattern '^\$script:StateFile ' | Select-Object -First 1) +$end = ($lines | Select-String -Pattern '^function Invoke-Native' | Select-Object -First 1) +if (-not $start -or -not $end) { throw 'could not locate the install-record block in shopdb-install.ps1' } +$block = $lines[($start.LineNumber - 1)..($end.LineNumber - 2)] -join "`n" +if ($block -notmatch 'function Test-FirstProvisioning') { throw 'extracted block does not contain the state functions' } + +# Stand-ins for the surrounding script. +$script:Created = New-Object System.Collections.ArrayList +function Write-Log { param($m, $l = 'INFO') } +function Protect-File { param([string] $Path, [switch] $Directory) } +function Test-SamePath { + param([string] $A, [string] $B) + if (-not $A) { return -not $B } + return ($A.TrimEnd('\', '/').ToLowerInvariant() -eq $B.TrimEnd('\', '/').ToLowerInvariant()) +} +if (-not $env:ProgramData) { $env:ProgramData = [System.IO.Path]::GetTempPath() } + +Invoke-Expression $block + +$script:Failures = 0 +function Check { + param([string] $Name, $Expected, $Actual) + if ($Expected -eq $Actual) { + Write-Host (" PASS " + $Name) + } else { + Write-Host (" FAIL {0}: expected [{1}], got [{2}]" -f $Name, $Expected, $Actual) + $script:Failures++ + } +} + +function Reset-Case { + param([switch] $Stamped) + if (Test-Path $Sandbox) { Remove-Item $Sandbox -Recurse -Force } + New-Item -ItemType Directory -Path $Sandbox -Force | Out-Null + $script:AppRoot = $Sandbox + $script:StateFile = Join-Path $Sandbox 'install-state.json' + $script:InstallState = $null + if ($Stamped) { Set-Content (Join-Path $Sandbox '.installed-version') '0.7.0' } +} + +Write-Host 'brand new server' +Reset-Case +Initialize-InstallState +Check 'first provisioning' $true (Test-FirstProvisioning) +Check 'record written' $true (Test-Path $script:StateFile) + +Write-Host 'retry of a failed first install' +$script:InstallState = $null +Initialize-InstallState +Check 'still first provisioning' $true (Test-FirstProvisioning) + +Write-Host 'genuine upgrade after a completed install' +Complete-InstallState +$script:InstallState = $null +Initialize-InstallState +Check 'no longer first provisioning' $false (Test-FirstProvisioning) + +Write-Host 'install predating the record - must take the safe side' +Reset-Case -Stamped +Initialize-InstallState +Check 'treated as established' $false (Test-FirstProvisioning) + +Write-Host 'record naming a different directory' +Reset-Case +Initialize-InstallState +$other = Get-Content $script:StateFile -Raw | ConvertFrom-Json +$other.approot = 'C:\somewhere-else' +($other | ConvertTo-Json -Depth 6) | Set-Content $script:StateFile +$script:InstallState = $null +Check 'foreign record rejected' $null (Get-InstallState) + +Write-Host 'corrupt record' +Reset-Case +Set-Content $script:StateFile '{ this is not json' +$script:InstallState = $null +Check 'corrupt record rejected' $null (Get-InstallState) +Reset-Case +Set-Content $script:StateFile '{ this is not json' +Initialize-InstallState +Check 'recovers and rewrites' $true ($null -ne $script:InstallState) + +Write-Host 'created items survive a crash' +Reset-Case +Initialize-InstallState +Track 'site' 'shopdb-flask' +Track 'apppool' 'shopdbflask' +Track 'site' 'shopdb-flask' +$script:InstallState = $null +Initialize-InstallState +Check 'site remembered' 'shopdb-flask' ((Get-CreatedItems 'site') -join ',') +Check 'apppool remembered' 'shopdbflask' ((Get-CreatedItems 'apppool') -join ',') +# A zero-length array returned from a function unrolls to $null, and $null.Count +# is fatal under StrictMode. This is the check that catches losing the comma. +Check 'unknown kind is an empty array' 0 ((Get-CreatedItems 'firewall')).Count + +Write-Host 'no record at all' +$script:InstallState = $null +Check 'empty list, not null' 0 ((Get-CreatedItems 'site')).Count +Check 'not first provisioning' $false (Test-FirstProvisioning) + +if (Test-Path $Sandbox) { Remove-Item $Sandbox -Recurse -Force } +if ($script:Failures -gt 0) { + Write-Host ("{0} check(s) failed" -f $script:Failures) + exit 1 +} +Write-Host 'all checks passed' +exit 0 diff --git a/pytest.ini b/pytest.ini new file mode 100644 index 0000000..7a4df7b --- /dev/null +++ b/pytest.ini @@ -0,0 +1,28 @@ +[pytest] +# Tests live in tests/. Saying so is not cosmetic. +# +# deploy/windows/installer/bundle/ is BUILD OUTPUT: build-installer.sh stages a +# complete copy of the application there, plugins included. Without this, +# collection walked into it and imported every plugin a second time under a +# second path, so SQLAlchemy raised "Table 'printeditems' is already defined for +# this MetaData instance" and the whole suite failed to collect - on a tree with +# nothing wrong in it, purely because somebody had built an installer first. +# +# It only started biting when the bundle grew from four plugins to thirteen, +# which is the sort of delay that makes this look like a code fault rather than +# a stale artefact. +testpaths = tests + +# Belt and braces: even an explicit path or an IDE run must not descend into +# build output or a staged copy of the tree. +norecursedirs = + .git + venv + .venv + node_modules + bundle + dist + build + frontend/dist + frontend/dist-subpath + *.egg-info diff --git a/tests/test_installer_state.py b/tests/test_installer_state.py new file mode 100644 index 0000000..6a2f797 --- /dev/null +++ b/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