Align in_flight_scripts_json to the array shape it actually carries#447
Merged
Conversation
The deployment checkpoint's in_flight_scripts_json column was seeded/defaulted
to the legacy object form "{}", but InFlightScriptMap serializes a List<Entry>
— a JSON array [{ "m", "s", "a", "t" }]. Parse("{}") throws (caught → treated
as empty), so "{}" only ever worked via that catch fallback, and a fresh
checkpoint's "{}" leaked past IsMachineBusyAsync's "!= []" pre-filter.
Align the empty form to "[]":
- Entity default (DeploymentExecutionCheckpoint) + doc-comment (now describes
the array shape).
- EF column default (DeploymentExecutionCheckpointConfiguration) → "[]".
- EnsureExistsAsync seed → "[]".
- DbUp migration: ALTER COLUMN ... SET DEFAULT '[]'::jsonb and rewrite existing
empty '{}' rows to '[]' (empty object == empty array in meaning, so a pure
shape normalization).
Non-breaking: InFlightScriptMap.Parse still tolerates any leftover legacy shape
(left untouched), and EF always supplies an explicit value on insert, so the DB
default is belt-and-braces. No new MachineUpgradeStatus enum / no upgrade-defer
logic change — purely a data-shape alignment.
Tests: new integration case pins EnsureExistsAsync seeds "[]" (not "{}");
existing InFlightScriptStore (16) + InFlightScriptMap (23, incl. legacy-{}
→ empty via the catch) suites green.
…t (review) Adversarial review of #447 surfaced two fix-optional items, both addressed: - The migration's UPDATE ('{}' -> '[]') was untested: the integration harness migrates a FRESH (empty) DB, so the UPDATE is a no-op there. Add an integration test that seeds BOTH legacy shapes (empty '{}' and a non-empty {machineId:ticket} object) then runs the same UPDATE, asserting the empty one normalizes to '[]' and the non-empty one is NOT clobbered (pins the WHERE = '{}' selectivity the migration comment promises). - Update the IsMachineBusyAsync comment, which still claimed '{}' was the seed / column default — it is now '[]'; '{}' is the legacy shape kept defensively until the 20260615 migration normalizes it. InFlightScriptStore (17) integration tests green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
in_flight_scripts_jsoncolumn was seeded/defaulted/documented as the legacy object form"{}", butInFlightScriptMapserializes aList<Entry>— a JSON array[{ "m": machineId, "s": stepId, "a": actionId, "t": scriptTicket }].Parse("{}")throws (caught → treated as empty), so"{}"only ever worked via that catch fallback, and a fresh checkpoint's"{}"leaked pastIsMachineBusyAsync's!= "[]"pre-filter."[]"across all four sources of truth: entity default (+ doc-comment), EF column default,EnsureExistsAsyncseed, and a DbUp migration.Changes
DeploymentExecutionCheckpoint.cs— default"[]"+ doc-comment now describes the array shape.DeploymentExecutionCheckpointConfiguration.cs—HasDefaultValue("[]").DeploymentCheckpointService.EnsureExistsAsync— seed"[]".20260615_align_inflight_scripts_default_to_array.sql—ALTER COLUMN ... SET DEFAULT '[]'::jsonb+ rewrite existing empty'{}'rows to'[]'(empty object ≡ empty array — pure shape normalization).Non-breaking
InFlightScriptMap.Parsestill tolerates any leftover legacy shape (left untouched) — existing data unaffected.MachineUpgradeStatusenum value; no change to the upgrade-defer logic. Purely a data-shape alignment.Test plan
InFlightScriptStoreTests): newEnsureExists_SeedsInFlightScriptsJson_AsEmptyArraypins the seed is"[]"(discriminating vs the prior"{}"); existing suite (16) green incl. the seeded-checkpoint busy-check.InFlightScriptMapTests, 23): legacy"{}"/ malformed /"[]"all → empty via the retainedParsecatch — confirms tolerance of pre-migration rows.