refactor: per-patch lifecycle state machine (#352)
* refactor: introduce per-patch lifecycle state machine (types + persistence)
First slice of shorebirdtech/shorebird#3737 — replace the scattered
storage of per-patch state across download_state.rs sidecars, bare
files in downloads/, and PatchesState fields (next_boot_patch,
last_booted_patch, known_bad_patches) with a single per-patch
state.json driven by an explicit state machine.
This commit only adds the types and the storage layer; nothing wires
into update_internal or report_launch_* yet. Subsequent commits will
build out transitions and migrate the call sites.
States:
- Downloading { url, hash, signature, partial_size }
- Downloaded { url, hash, signature, size }
- Installed { hash, signature, size }
- Bad { reason, hash?, signature?, size? } // tombstone
Operations:
- mark_bad(n, reason) — sugar over write Bad{} + cleanup
- cleanup(n) — state-aware: keeps Bad tombstone, else forgets dir
Per-release pointers (next_boot, last_booted, currently_booting,
boot_started_at) move to a separate pointers.json holding patch
numbers; metadata lives once per patch in state.json instead of
duplicated across pointers.
Persistence rides on the existing atomic disk_io::write (sibling-
write + rename) so partial writes can't leave torn files.
* refactor: add download/install/boot transitions to lifecycle
Extends the lifecycle module with the methods update_internal and
report_launch_* will need:
- decide_start(n, url, hash) → DownloadAction (Fresh, Resume, Complete, Skip)
- record_download_started / _complete
- record_install_complete (transitions Downloaded → Installed,
removes the now-unneeded compressed download file)
- promote_to_next_boot (transitions a freshly Installed patch into
pointers.next_boot, retiring any unbooted predecessor)
- record_boot_start / _success / _failure
- detect_boot_crash_on_init (handles the breadcrumb left when a
prior process crashed during boot)
- validate_next_boot_patch (size + signature checks; marks
Bad{ValidationFailed} on failure)
- recompute_next_boot
Nothing wires into the existing updater.rs / report_launch_* yet —
those changes follow in the cutover commit.
* refactor: replace UpdaterState's PatchManager backend with PatchLifecycle
Drops the patch_manager dependency from updater_state.rs. UpdaterState
now owns a PatchLifecycle directly, with all patch-related methods
delegating to it:
install_patch → write Installed state + promote to next_boot
next_boot_patch → pointers.next_boot_patch + installed_artifact_path
is_known_bad_patch → read_state matches Bad
uninstall_patch → cleanup + recompute_next_boot
record_boot_* → lifecycle's boot transitions
validate_next_boot_patch → lifecycle's signature/size check
UpdaterState's own state.json shrinks to {client_id, release_version,
queued_events}. Per-release per-patch state lives entirely in the
lifecycle (pointers.json + patches/{N}/state.json).
Other notable behavior changes:
- record_boot_failure_for_patch no longer requires
currently_booting_patch to match. Matches the prior PatchManager
semantics: clear breadcrumb, mark Bad{BootCrash}, recompute.
- recompute_next_boot leaves a valid Installed next_boot_patch alone
instead of promoting last_booted over it. Without this, processing
server rollbacks would clobber a freshly-installed newer patch.
Tests in updater.rs that asserted file paths (patches_state.json) update
to the new layout (pointers.json). The MockManagePatches-backed unit
tests in updater_state.rs are gone — replaced by direct end-to-end tests
that exercise PatchLifecycle through UpdaterState.
patch_manager.rs is still on disk and compiled (nothing references it
from cache/mod.rs anymore) — deletion happens after the updater.rs
cutover.
* refactor: cut update_internal over to PatchLifecycle
Replaces the download_state.rs sidecar machinery and the bespoke
DownloadStartState / should_install_patch helpers in update_internal
with calls into PatchLifecycle.
Concrete changes:
- Drops compute_resume_offset / determine_download_start_state /
DownloadStartState — replaced by PatchLifecycle::decide_start
returning DownloadAction.
- Drops should_install_patch / ShouldInstallPatchCheckResult —
folded into the same DownloadAction match. KnownBad → UpdateIsBadPatch,
AlreadyInstalled → NoUpdate, the rest proceed.
- Drops install_downloaded_patch's bespoke "stage in downloads/, rename
on install" choreography. The lifecycle owns the per-patch directory;
inflate writes directly into patches/{N}/dlc.vmcode and the
Downloaded → Installed transition removes the now-unneeded compressed
download file.
- Drops cleanup_download_artifacts and clean_download_dir. mark_bad
handles tombstone-aware cleanup for failures, and the lifecycle owns
its directory.
- Marks the patch Bad{InvalidPatchBytes} when inflate fails and
Bad{InstallHashMismatch} when check_hash fails. Subsequent attempts
short-circuit at decide_start (Skip(KnownBad)) instead of
re-downloading and re-failing on every cycle. This is the marks-bad-
on-install-failure behavior we deferred from #351.
- Preserves the explicit "server-side Content-Length vs total_bytes"
mismatch check — surfaces the contract violation directly instead of
obliquely via inflate failure.
decide_start consults the on-disk download file as the source of truth
for "how many bytes do we have so far." The denormalized partial_size
in PatchState::Downloading is kept for diagnostics/serde stability but
not consulted for the resume offset; this matches the prior
file-size-on-disk behavior and avoids needing to update state.json
mid-stream from the network layer.
For Downloading / Downloaded states where the OS evicted the artifact
file out from under us (e.g. iOS code-cache eviction), decide_start
falls through to Fresh so the next attempt re-downloads from scratch.
The failing-test fixes update file paths and pre-staged state from the
old layout (downloads/{N} + downloads/{N}.download.json) to the new
layout (patches/{N}/download + patches/{N}/state.json). Several tests
that targeted the now-deleted helpers directly are removed; their
behavior is covered by the new lifecycle unit tests.
* refactor: delete patch_manager.rs and download_state.rs
Both modules are now subsumed by PatchLifecycle:
- patch_manager.rs (~1600 lines) managed the per-release `patches/`
tree, the `next_boot_patch` / `last_booted_patch` /
`currently_booting_patch` / `known_bad_patches` fields of
`PatchesState`, and the fall-back-from-bad-patch logic. All of
this is now in lifecycle.rs split between `PatchLifecycle`
(per-patch state.json) and `ReleasePointers` (a single
pointers.json).
- download_state.rs (~125 lines) wrote the per-download sidecar
JSON. Now folded into PatchState::Downloading /
PatchState::Downloaded variants in state.json, owned by the
lifecycle module.
The MockManagePatches / ManagePatches trait machinery in
updater_state.rs's tests goes away with patch_manager. The lifecycle
operations are tested directly in cache::lifecycle::tests against a
real on-disk filesystem under TempDir, which catches issues the
mocks couldn't (e.g. file-existence cross-checks in `decide_start`).
The cargo workspace now has 212 passing tests vs. 257 before, the
delta being the patch_manager unit tests whose subjects no longer
exist. End-to-end coverage in `updater.rs::tests` is unchanged and
all green.
* refactor: address self-review feedback on lifecycle PR
Nine fixes from the self-review pass on shorebirdtech/updater#352:
- Drop `partial_size` from PatchState::Downloading. The field was
misleading (decide_start reads from disk, not from the recorded
value) and unused. record_download_started loses its 5th arg.
- Gate `UpdaterState::install_patch` to `#[cfg(test)]`. Production
no longer routes through it; only test_utils and the updater_state
tests do. The gate makes the divergence intentional and prevents
future production callers.
- install_patch defensively removes any prior `dlc.vmcode` before
rename so behavior is OS-agnostic (POSIX rename overwrites silently;
Windows fails). Also mirrors record_install_complete's cleanup of
a stale `download` file in the patch dir.
- Document on `lifecycle()` / `lifecycle_mut()` that the direct
accessors are intentional — wrapping every transition would be
churn for no reader benefit.
- recompute_next_boot now clears `last_booted_patch` when its
on-disk record is gone (Unknown), so pointers.json doesn't
accumulate references to nothing. A `Bad` last_booted is left
alone — that's a useful breadcrumb and recompute simply doesn't
promote it.
- Promote `download_artifact_path` and `installed_artifact_path` to
methods on PatchLifecycle for symmetry with state_path /
pointers_path. Updates all call sites.
- Document mark_bad-on-Bad merge semantics: latest reason wins,
other fields preserved. Hypothetical in practice but no longer
silent.
- Add tests for recompute_next_boot's stale-pointer clearing
(Unknown → cleared, Bad → kept). Brings the suite to 214 tests.
The mark_bad-cleanup-stale-bytes recovery path I flagged was already
covered by `cleanup_on_bad_patch_keeps_tombstone` — no new test needed.
* refactor: undo two over-corrections from the prior self-review fix
Two issues introduced by 07ea84a that the second self-review caught:
- update_internal computed download_path / installed_path via
`with_state(...).lifecycle().download_artifact_path(n)`. That
routed a pure-function path computation through a state read,
serializing it against other state operations and adding a disk
read per call. Restore the free `download_artifact_path` /
`installed_artifact_path` functions as the canonical entry point;
the methods on PatchLifecycle are kept as thin wrappers for
callers that already hold a lifecycle handle.
- install_patch's "defensive remove_file before rename" added a
TOCTOU window between the exists() check and the rename for no
real benefit — install_patch is now `#[cfg(test)]`-only, the
Windows-rename concern was theoretical for tests, and POSIX rename
atomically overwrites. Drop the explicit remove_file.
* ci: fix warnings-as-errors and cspell
Three CI failures from the prior push:
- `Context` and `bail` imports in updater_state.rs are only used
inside the `#[cfg(test)] install_patch` helper. Moved them under
`#[cfg(test)]` so non-test builds don't see unused imports.
- `PathBuf` import in updater.rs became unused after switching the
download/install paths to take `Path::new(&config.storage_dir)`
directly. Dropped.
- `FileOperation::DeleteDir` is no longer constructed by production
code (was used by the deleted patch_manager.rs). Mirrored the
existing `#[allow(dead_code)]` annotation already on `DeleteFile`
rather than removing the variant — the format/handling code for
it is still useful for any future code that needs to surface a
"delete dir failed" error.
CSpell additions: `unparseable`, `tombstoned`, `roundtrips` — words
introduced by the lifecycle module's docs and test names.
* ci: gate PathBuf import to platforms that actually use it
The previous fix removed `PathBuf` from the top-level use, which
broke the lib build on non-android non-test platforms (macOS,
Linux, Windows) where `libapp_path_from_settings` uses `PathBuf`.
That function is itself gated to `cfg(not(any(target_os = \"android\",
test)))`, so the PathBuf import needs the same gate.
Restoring it as a cfg-gated separate import — keeps the lib build
green on all three runners and keeps the lib-test build clean
under -D warnings.
* test: cover the gaps surfaced by the coverage report
Adds 7 tests targeting branches that were uncovered:
- mark_bad_from_downloading_records_partial_file_size: the
Downloading source state in mark_bad reads bytes-on-disk for
the recorded `size` field; the existing tests only covered
Installed → Bad.
- mark_bad_overwrites_reason_when_already_bad: documented merge
semantics (latest reason wins, other fields preserved) now
has a test backing the doc.
- validate_next_boot_patch_marks_bad_when_artifact_missing: the
"Installed state but dlc.vmcode is gone" path. Existing test
covered size mismatch; this covers the missing-file branch.
- validate_next_boot_patch_marks_bad_when_pointer_targets_non_installed:
the case where the next_boot pointer references a state other
than Installed (state.json + pointers can diverge through
corruption).
- install_patch_install_only_{accepts_valid,rejects_missing,rejects_bad}_signature:
cover the InstallOnly verification path in
UpdaterState::install_patch using the existing test keypair
from signing.rs.
Coverage improvements:
- cache/lifecycle.rs: 94.65% → 96.06% lines, 98.72% → 100% fns
- cache/updater_state.rs: 94.34% → 96.94% lines, 95.65% → 98% fns
- total: 94.70% → 95.11% lines
214 → 221 tests, all passing under -D warnings.
* ci: add 'keypair' to cspell dictionary
* test: audit deleted patch_manager tests; restore strict checks
Goes through every test deleted with patch_manager.rs (~40) and either
maps it to a new test (with a `Ports patch_manager.rs::mod::name`
comment) or adds a port. Three behavioral regressions caught and fixed:
- record_boot_start now requires next_boot_patch to match the arg.
Old PatchManager had this defensive check; my refactor dropped it.
Carries forward the engine-vs-state agreement guard.
- Added strict-mode signature tests for validate_next_boot_patch
(valid, missing, invalid, no public key, bad public key). Ports
five `validate_next_boot_patch_tests::strict_mode_*` tests.
- Added InstallOnly + no public_key test (ports
add_patch_tests::install_only_succeeds_with_any_signature_if_no_public_key).
Plus two scenario tests that didn't have direct coverage:
- rolled_back_patch_not_resurrected_when_replacement_fails: ports
fall_back_tests::rollback_then_failed_replacement_does_not_resurrect_rolled_back_patch
against the new state-based fallback (cleanup forgets, recompute
won't promote a None state).
- validate_then_promote_catches_corrupted_last_booted: ports
validate_next_boot_patch_tests::does_not_fall_back_to_last_booted_patch_if_corrupted
with a note: new code catches the corruption at the next validate
pass instead of at fall-back time, but the user-visible outcome
(boot the base release) is the same.
Test helper split: `install_state` (writes Installed without touching
pointers) vs callers explicitly setting pointers. Avoids the prior
helper's auto-promote masking pointer-management behavior the tests
were trying to exercise.
Tests deliberately not ported (with reasoning):
- debug_tests::manage_patches_is_debug, patch_manager_is_debug:
Trait-Debug tests for types that no longer exist. Replaced
implicitly by `#[derive(Debug)]` on PatchLifecycle.
- fall_back_tests::succeeds_if_deleting_artifacts_fails: needs
filesystem fault injection that wasn't worth the test
infrastructure to bring back.
- record_boot_success_for_patch_tests::deletes_unrecognized_directories_in_patches_dir:
behavior change — new cleanup_older_than skips non-numeric
directory names rather than deleting them. Defensive; the prior
`rm -rf` was overly aggressive.
Coverage: 222 → 230 tests, all green under -D warnings.
* test: port the two patch_manager tests I'd handwaved
Both turned out trivial to port — neither was actually doing fault
injection, just exploiting graceful-failure paths.
- record_boot_success_deletes_unrecognized_directories_in_patches_dir:
pre-creates a junk dir and a stray file in patches/ before the
install/boot, asserts they're gone after record_boot_success.
Restores the prior `cleanup_older_than` behavior of removing
non-numeric entries — we own patches/, so anything not named like
a patch number is corruption to sweep up.
- record_boot_failure_succeeds_if_artifact_dirs_are_already_gone:
pre-deletes both patch dirs and verifies record_boot_failure still
completes correctly (mark_bad recreates the dir for the tombstone;
cleanup paths are graceful when the dir is missing).
Reverts the prior behavior change in cleanup_older_than that skipped
non-numeric entries instead of deleting them. The "defensive" framing
was wrong — we own patches/, anything unexpected there came from a
different version of our code or actual corruption, and leaving it
behind is the wrong default.
* test: port the remaining patch_manager tests with real coverage gaps
- validate_next_boot_strict_mode_succeeds_with_valid_signature: ports
`patch_manager.rs::validate_next_boot_patch_tests::strict_mode_succeeds_with_valid_signature_at_boot_time`.
Required reusing the prior test fixture's INFLATED_PATCH_HASH /
INFLATED_PATCH_SIGNATURE constants (a real signature for the
1-byte file content `b"1"`, matching TEST_PUBLIC_KEY's private
half).
- record_boot_failure_keeps_last_booted_pointer_when_failed_patch_was_last_booted:
ports
`patch_manager.rs::record_boot_failure_for_patch_tests::preserves_last_booted_patch_on_failure_but_marks_bad`.
The new behavior is similar but more explicit: last_booted's
*pointer* is preserved (with the patch now in Bad state); the
underlying intent — "we still know what last booted, even after
that patch failed" — survives.
Test helper split: `install_signed` (failure paths, mismatched hash
OK) vs `install_with_valid_signature` (happy path, real fixture).
Now every behavioral test from the deleted patch_manager.rs has a
corresponding new test with a porting comment. Two trivial ones not
ported: the `Debug` impl tests for traits/types that no longer exist.
236 tests, all green under -D warnings.
* refactor: restore separate download directory under OS cache root
Reverts an unintended policy change in the cutover: my refactor moved
compressed download bytes from `{code_cache_dir}/downloads/{N}` (the
OS-managed cache dir, evictable under storage pressure) into
`{storage_dir}/patches/{N}/download` (persistent app storage). That
made downloads count against persistent storage and survive in iCloud
backups — neither of which we want for transient bytes.
Restoring the original split:
- `{state_root}/patches/{N}/{state.json,dlc.vmcode}` — persistent
state and the installed artifact.
- `{download_root}/{N}` — flat, in the OS cache dir.
`PatchLifecycle::load_or_default` now takes both roots. The two
artifact-path helpers each route to their respective root, the
free-function variants used by `update_internal` now take the right
root, and `mark_bad`/`cleanup`/`record_install_complete` clean both
locations as appropriate.
`UpdaterState::create_new_and_save` also wipes `download_dir` on
release-version change, in addition to the persistent patches/ tree.
`update_internal` and tests updated to use the right root for each
file type. Test fixtures pass `tmp.path().join("downloads")` as the
download root (separate subdir of the same TempDir).
Net change in test surface: moved a handful of `patch_dir/download`
references to `downloads_dir/N`, no behavioral changes to tests
themselves. 236 tests pass under -D warnings.
* test: cover the new download_root cleanup paths
Self-review caught two gaps in the download-root split:
- release_version_change_wipes_download_dir: the cache-rooted
download dir should be wiped along with the persistent patches/
tree on a release-version mismatch. Adds explicit coverage —
the code path was already in create_new_and_save but not
directly tested.
- record_boot_success_promotes_and_cleans_older: extends the
existing test to drop a stale download in download_root
alongside an older patch's state, asserts that
cleanup_older_than's chain (cleanup → forget_dir) deletes both
roots' artifacts.
Also adds a comment on cleanup_older_than noting that it only
walks the persistent patches/ tree — orphan downloads (no
state.json) would persist until the OS evicts them. Noted as a
known limitation, not blocking.
* refactor: targeted wipe + legacy patches_state.json + orphan-download walk
Two fixes to the release-version-change wipe and one to the boot-
success cleanup walk.
- The "wipe everything in cache_dir" approach broke 27 tests whose
setup uses cache_dir as the test temp dir (with base.apk and
libapp.so as siblings). Production gives shorebird a dedicated
subdir but the API doesn't enforce that. Switched to a targeted
wipe with a documented allowlist of paths shorebird has ever
written under cache_dir: `SHOREBIRD_OWNED_PATHS`.
- Added `patches_state.json` to the wipe list — that's the
legacy file from the prior `PatchManager` implementation, and
devices upgrading through this PR would otherwise orphan it.
New test `release_version_change_wipes_legacy_patches_state_json`
covers it.
- `cleanup_older_than` now also calls `cleanup_orphan_downloads`,
which walks `download_root/` and removes any file that doesn't
correspond to a patch in `Downloading`/`Downloaded` state. We own
the cache root and shouldn't rely on OS eviction. Catches the
"state.json gone but download lingered" case the prior comment
handwaved.
Adding a new file under cache_dir means adding it to
SHOREBIRD_OWNED_PATHS — small bookkeeping cost, but it lets us
keep cache_dir co-tenant with embedder-provided files.
* test: cover all four branches of cleanup_orphan_downloads
The new orphan-download sweep distinguishes four cases but only the
"older patch via cleanup chain" branch was being exercised. Single
test now drops one of each kind of file into download_root before a
boot success and asserts which survive:
- orphan (numeric, no state.json): removed
- stale (numeric, state is Installed): removed
- non-numeric name: removed
- live (numeric, state is Downloading): preserved
* ci: add 'embedder' to cspell dictionary
* refactor: drop Installed.hash, fix mark_bad ordering, address bdero review
bdero's review on #352 surfaced six items worth landing inline:
1. record_boot_failure used to clear `currently_booting_patch` before
`mark_bad`. A crash between the two left the patch `Installed` with
no breadcrumb, so `detect_boot_crash_on_init` wouldn't fire next
boot — the patch silently retried. Flipped: mark_bad first, then
clear. mark_bad on Bad is idempotent, so the worst case is a
redundant tombstone-rewrite on next init.
2. Drop the `hash` field from `PatchState::Installed`. It was recorded
at install but never read at boot — Strict-mode validation
recomputes the hash from the artifact's bytes and feeds it into
`check_signature`. A hash that lives only on disk can't be trusted
as a security input; deleting the field removes the temptation.
`Downloading.hash`/`Downloaded.hash` stay as comparators against
the server's freshly-delivered hash (a tampered on-disk hash there
just causes a redownload).
3. Port the customer-scenario test from #356 into `c_api/mod.rs`:
server rolls patch 1 back, then forward again with the same number
and hash. Pre-lifecycle this was permanently dropped via
`known_bad_patches`. Post-lifecycle `cleanup` is state-aware and
forgets the patch entirely on server-driven rollback, so the
rollforward installs cleanly. End-to-end coverage of the regression
#356 hotfixed.
4. Add a TODO + target version on the `patches_state.json` entry in
`SHOREBIRD_OWNED_PATHS` so the legacy wipe doesn't outlive its
usefulness.
5. Document that `running_patch` lives in `config.rs` (session-scoped),
not in this module — saves a future grep.
6. Clarify `recompute_next_boot`'s fallback policy: it's
fall-back-to-`last_booted_patch`-only, not fall-back-to-anything-
bootable. A fresh release whose first Installed patch fails
validation goes to base, even if older patches sit in `patches/`.
cargo test: 242 passed (was 241).
This commit is contained in:
Eric Seidel
+7
-1
@@ -39,8 +39,9 @@ words:
|
||||
- dllexport
|
||||
- dlopen
|
||||
- downloadables
|
||||
- EDQUOT
|
||||
- EACCES
|
||||
- EDQUOT
|
||||
- embedder
|
||||
- embedders
|
||||
- EOCD
|
||||
- endtemplate
|
||||
@@ -51,6 +52,7 @@ words:
|
||||
- gclient
|
||||
- hdpi
|
||||
- ifdef
|
||||
- keypair
|
||||
- libapp
|
||||
- libc
|
||||
- libflutter
|
||||
@@ -68,7 +70,9 @@ words:
|
||||
- repr
|
||||
- reqwest
|
||||
- rlib
|
||||
- roundtrips
|
||||
- rsplit
|
||||
- rollforward
|
||||
- rollouts
|
||||
- RTLD
|
||||
- rustflags
|
||||
@@ -82,9 +86,11 @@ words:
|
||||
- subosito
|
||||
- swiftpm
|
||||
- symbolication
|
||||
- tombstoned
|
||||
- ureq
|
||||
- unbootable
|
||||
- unbooted
|
||||
- unparseable
|
||||
- unreviewable
|
||||
- unwritable
|
||||
- usize
|
||||
|
||||
@@ -876,6 +876,105 @@ mod test {
|
||||
assert_eq!(shorebird_current_boot_patch_number(), 0);
|
||||
}
|
||||
|
||||
/// Server rolls back patch 1, then later rolls it forward again with
|
||||
/// the same number and same hash. The pre-lifecycle code added the
|
||||
/// rolled-back number to `known_bad_patches` permanently for the
|
||||
/// release, so the rollforward was silently dropped on the device.
|
||||
/// The lifecycle's `cleanup` is state-aware: a server-driven
|
||||
/// rollback on a non-`Bad` patch forgets the patch entirely (no
|
||||
/// tombstone), leaving the number free to be reinstalled.
|
||||
/// shorebirdtech/shorebird#3728.
|
||||
#[serial]
|
||||
#[test]
|
||||
fn rollforward_after_server_rollback_reinstalls_patch() {
|
||||
testing_reset_config();
|
||||
let tmp_dir = TempDir::new().unwrap();
|
||||
|
||||
let apk_path = tmp_dir.path().join("base.apk");
|
||||
write_fake_apk(
|
||||
apk_path.to_str().unwrap(),
|
||||
HELLO_TESTS_PATCH.base.as_bytes(),
|
||||
);
|
||||
let fake_libapp_path = tmp_dir.path().join("lib/arch/ignored.so");
|
||||
let c_params = parameters(&tmp_dir, fake_libapp_path.to_str().unwrap());
|
||||
let c_yaml = c_string("app_id: foo");
|
||||
assert!(shorebird_init(&c_params, FileCallbacks::new(), c_yaml));
|
||||
free_c_string(c_yaml);
|
||||
free_parameters(c_params);
|
||||
|
||||
// Phase 1: install patch 1 and report a successful launch.
|
||||
testing_set_network_hooks(
|
||||
|_url, _request| {
|
||||
Ok(PatchCheckResponse {
|
||||
patch_available: true,
|
||||
patch: Some(crate::Patch {
|
||||
number: 1,
|
||||
hash: HELLO_TESTS_PATCH.hash.to_owned(),
|
||||
download_url: "ignored".to_owned(),
|
||||
hash_signature: None,
|
||||
}),
|
||||
rolled_back_patch_numbers: None,
|
||||
})
|
||||
},
|
||||
|_url, dest: &Path, _resume_from: u64| HELLO_TESTS_PATCH.write_to(dest),
|
||||
|_url, _event| Ok(()),
|
||||
);
|
||||
assert!(shorebird_check_for_downloadable_update(std::ptr::null()));
|
||||
run_update_expecting(std::ptr::null(), SHOREBIRD_UPDATE_INSTALLED);
|
||||
shorebird_report_launch_start();
|
||||
shorebird_report_launch_success();
|
||||
assert_eq!(shorebird_current_boot_patch_number(), 1);
|
||||
|
||||
// Phase 2: server rolls patch 1 back with no replacement.
|
||||
// Phase-1 spawned threads (PatchDownload, PatchInstallSuccess)
|
||||
// hold a clone of the config from when they were spawned, so they
|
||||
// hit the old report hook above. Nothing in phase 2 should report
|
||||
// or download — only a patch-check happens.
|
||||
testing_set_network_hooks(
|
||||
|_url, _request| {
|
||||
Ok(PatchCheckResponse {
|
||||
patch_available: false,
|
||||
patch: None,
|
||||
rolled_back_patch_numbers: Some(vec![1]),
|
||||
})
|
||||
},
|
||||
UNEXPECTED_DOWNLOAD,
|
||||
UNEXPECTED_REPORT,
|
||||
);
|
||||
assert!(!shorebird_check_for_downloadable_update(std::ptr::null()));
|
||||
assert_eq!(shorebird_next_boot_patch_number(), 0);
|
||||
|
||||
// Phase 3: server rolls patch 1 forward — same number, same hash,
|
||||
// empty rolled_back list (the row's `is_rolled_back` flipped back
|
||||
// to false on the server). The device must accept this as a normal
|
||||
// "patch available" response and reinstall.
|
||||
testing_set_network_hooks(
|
||||
|_url, _request| {
|
||||
Ok(PatchCheckResponse {
|
||||
patch_available: true,
|
||||
patch: Some(crate::Patch {
|
||||
number: 1,
|
||||
hash: HELLO_TESTS_PATCH.hash.to_owned(),
|
||||
download_url: "ignored".to_owned(),
|
||||
hash_signature: None,
|
||||
}),
|
||||
rolled_back_patch_numbers: Some(vec![]),
|
||||
})
|
||||
},
|
||||
|_url, dest: &Path, _resume_from: u64| HELLO_TESTS_PATCH.write_to(dest),
|
||||
|_url, _event| Ok(()),
|
||||
);
|
||||
|
||||
// Pre-lifecycle: returns false because patch 1 sat in
|
||||
// `known_bad_patches` from phase 2's `remove_patch` call.
|
||||
// Post-lifecycle: phase 2's cleanup forgot patch 1 entirely
|
||||
// (no Bad tombstone for server-driven rollbacks), so this
|
||||
// installs cleanly.
|
||||
assert!(shorebird_check_for_downloadable_update(std::ptr::null()));
|
||||
run_update_expecting(std::ptr::null(), SHOREBIRD_UPDATE_INSTALLED);
|
||||
assert_eq!(shorebird_next_boot_patch_number(), 1);
|
||||
}
|
||||
|
||||
/// Patch-to-patch rollback: device on patch 2, server rolls back to
|
||||
/// patch 1 (sends rollback signal AND a downloadable replacement).
|
||||
/// `check_for_downloadable_update` returns true (replacement available),
|
||||
|
||||
Vendored
+2178
File diff suppressed because it is too large
Load Diff
Vendored
+2
-2
@@ -1,5 +1,5 @@
|
||||
mod disk_io;
|
||||
mod patch_manager;
|
||||
pub(crate) mod disk_io;
|
||||
pub mod lifecycle;
|
||||
mod signing;
|
||||
pub mod updater_state;
|
||||
|
||||
|
||||
Vendored
-1615
File diff suppressed because it is too large
Load Diff
Vendored
+524
-364
File diff suppressed because it is too large
Load Diff
@@ -1,126 +0,0 @@
|
||||
/// Tracks metadata about an in-progress patch download so that it can be
|
||||
/// resumed after a failure or app restart.
|
||||
///
|
||||
/// Stored as a sidecar JSON file alongside the partial download:
|
||||
/// {download_dir}/{patch_number}.download.json
|
||||
use serde::{Deserialize, Serialize};
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
use crate::file_errors::{FileOperation, IoResultExt};
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
|
||||
pub struct DownloadState {
|
||||
/// The URL this download was started from. Used to decide whether a
|
||||
/// partial file on disk matches the current server response — if the URL
|
||||
/// changed, we discard and start fresh.
|
||||
pub url: String,
|
||||
/// The patch number being downloaded.
|
||||
pub patch_number: usize,
|
||||
/// Expected total file size from Content-Length/Content-Range (if known
|
||||
/// from a prior download attempt). Used for post-download validation.
|
||||
pub expected_size: Option<u64>,
|
||||
/// Hash of the inflated patch from the server response. Checked on resume
|
||||
/// to catch the case where a patch is deleted and re-added with the same
|
||||
/// number but different content (URL may stay the same but hash changes).
|
||||
pub expected_hash: String,
|
||||
}
|
||||
|
||||
/// Returns the sidecar path for a given download path.
|
||||
/// e.g. "{download_dir}/1" -> "{download_dir}/1.download.json"
|
||||
pub fn sidecar_path(download_path: &Path) -> PathBuf {
|
||||
let mut p = download_path.as_os_str().to_owned();
|
||||
p.push(".download.json");
|
||||
PathBuf::from(p)
|
||||
}
|
||||
|
||||
/// Write a DownloadState to its sidecar file.
|
||||
pub fn write_download_state(download_path: &Path, state: &DownloadState) -> anyhow::Result<()> {
|
||||
let path = sidecar_path(download_path);
|
||||
let json = serde_json::to_string(state)?;
|
||||
std::fs::write(&path, json).with_file_context(FileOperation::WriteFile, &path)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Read a DownloadState from its sidecar file, if it exists.
|
||||
pub fn read_download_state(download_path: &Path) -> anyhow::Result<Option<DownloadState>> {
|
||||
let path = sidecar_path(download_path);
|
||||
if !path.exists() {
|
||||
return Ok(None);
|
||||
}
|
||||
let json = std::fs::read_to_string(&path).with_file_context(FileOperation::ReadFile, &path)?;
|
||||
let state: DownloadState = serde_json::from_str(&json)?;
|
||||
Ok(Some(state))
|
||||
}
|
||||
|
||||
/// Delete the sidecar file for a download, if it exists.
|
||||
pub fn delete_download_state(download_path: &Path) -> anyhow::Result<()> {
|
||||
let path = sidecar_path(download_path);
|
||||
if path.exists() {
|
||||
std::fs::remove_file(&path).with_file_context(FileOperation::DeleteFile, &path)?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use tempfile::TempDir;
|
||||
|
||||
#[test]
|
||||
fn round_trip_download_state() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let download_path = tmp.path().join("1");
|
||||
|
||||
let state = DownloadState {
|
||||
url: "https://example.com/patch/1".to_string(),
|
||||
patch_number: 1,
|
||||
expected_size: Some(12345),
|
||||
expected_hash: "abc123".to_string(),
|
||||
};
|
||||
|
||||
write_download_state(&download_path, &state).unwrap();
|
||||
let loaded = read_download_state(&download_path).unwrap();
|
||||
assert_eq!(loaded, Some(state));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn read_missing_returns_none() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let download_path = tmp.path().join("99");
|
||||
let loaded = read_download_state(&download_path).unwrap();
|
||||
assert_eq!(loaded, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn delete_removes_sidecar() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let download_path = tmp.path().join("1");
|
||||
|
||||
let state = DownloadState {
|
||||
url: "https://example.com/patch/1".to_string(),
|
||||
patch_number: 1,
|
||||
expected_size: None,
|
||||
expected_hash: "abc".to_string(),
|
||||
};
|
||||
|
||||
write_download_state(&download_path, &state).unwrap();
|
||||
assert!(sidecar_path(&download_path).exists());
|
||||
|
||||
delete_download_state(&download_path).unwrap();
|
||||
assert!(!sidecar_path(&download_path).exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn delete_missing_is_ok() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let download_path = tmp.path().join("99");
|
||||
// Should not error.
|
||||
delete_download_state(&download_path).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sidecar_path_is_correct() {
|
||||
let p = sidecar_path(Path::new("/cache/downloads/1"));
|
||||
assert_eq!(p, PathBuf::from("/cache/downloads/1.download.json"));
|
||||
}
|
||||
}
|
||||
@@ -13,6 +13,7 @@ pub enum FileOperation {
|
||||
ReadFile,
|
||||
#[allow(dead_code)] // Included for completeness; not yet used outside tests.
|
||||
DeleteFile,
|
||||
#[allow(dead_code)] // Included for completeness; not yet used outside tests.
|
||||
DeleteDir,
|
||||
RenameFile,
|
||||
GetMetadata,
|
||||
|
||||
@@ -11,7 +11,6 @@ pub mod c_api;
|
||||
// Declare other .rs file/module exists, but make them private.
|
||||
mod cache;
|
||||
mod config;
|
||||
mod download_state;
|
||||
mod events;
|
||||
mod file_errors;
|
||||
mod logging;
|
||||
|
||||
@@ -16,6 +16,7 @@ pub fn install_fake_patch(patch_number: usize) -> anyhow::Result<()> {
|
||||
|
||||
let mut state = UpdaterState::load_or_new_on_error(
|
||||
&config.storage_dir,
|
||||
&config.download_dir,
|
||||
&config.release_version,
|
||||
config.patch_public_key.as_deref(),
|
||||
config.patch_verification,
|
||||
|
||||
+278
-664
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user