Skip to content

Commit 067b37c

Browse files
committed
refactor(cli): version the update cache instead of migrating it
The cache now records its schema and a foreign value -- including an absent one, which reads as 0 -- means the file is discarded and the next check rewrites it. The alternative was keeping every field optional forever so older shapes load, which for a CACHE buys one saved request in exchange for Option fields that only describe versions nobody runs and that nothing removes. Bumping CACHE_SCHEMA is now the entire migration story. Check sources lose their aliases: github, npm-registry and github-packages are gone, leaving gh-releases, npm, gh-registry and custom. An accepted-alias set is a surface to document and test forever to save one look at the docs. An unknown source still falls back to the default, but because an update check is the wrong place to turn a config typo into a hard failure -- not for compatibility.
1 parent 15b9873 commit 067b37c

3 files changed

Lines changed: 110 additions & 27 deletions

File tree

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
### Changed
2+
3+
**The update cache carries its shape, and a foreign shape is discarded rather
4+
than migrated.** `~/.perry/update-check.json` now records a `schema` number; a
5+
value this build does not recognize — including its absence, which reads as `0`
6+
means the file is thrown away and the next check rewrites it.
7+
8+
This replaces the alternative, which was to keep every field optional forever so
9+
that older shapes still load. That trade is a bad one for a cache: it buys one
10+
saved network request in exchange for a set of `Option` fields that only exist
11+
to describe versions nobody runs, and that nothing ever removes. Bumping
12+
`CACHE_SCHEMA` is now the whole migration story.
13+
14+
**One spelling per check source.** `github`, `npm-registry` and
15+
`github-packages` are gone; the names are `gh-releases`, `npm`, `gh-registry`
16+
and `custom`. A set of accepted aliases is a surface to document and test
17+
forever in exchange for saving one look at the docs.
18+
19+
An unknown `source` still falls back to the default rather than failing, but for
20+
a different reason than compatibility: an update check is the wrong place to
21+
turn a config typo into a hard error.
22+
23+
<details>
24+
<summary><b>Tests</b></summary>
25+
26+
The test that asserted a pre-throttle cache still loads is replaced by one
27+
asserting the opposite — that a foreign schema, and an absent one, are both
28+
recognized as not-ours. Verified at runtime as well: a planted cache with no
29+
schema, claiming version `99.0.0` and a `last_check` in 2099, was ignored, a
30+
real check ran, and the file came back stamped `"schema": 1`.
31+
32+
`cargo test -p perry`: 930 passed, 0 failed.
33+
</details>

crates/perry/src/release_source.rs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,14 @@ pub(crate) enum CheckSource {
7373

7474
/// Parse a configured `source` name into a source, given the other keys.
7575
///
76-
/// Returns `None` for a name this build does not know, so the caller can fall
77-
/// back rather than fail — a config written by a newer Perry must not break an
78-
/// older one's update check.
76+
/// One spelling per source, deliberately: a set of accepted aliases is a
77+
/// surface to keep documented and tested forever in exchange for saving one
78+
/// lookup in the docs.
79+
///
80+
/// Returns `None` for a name this build does not know, so the caller falls back
81+
/// to the default rather than refusing to check at all. That is not a
82+
/// compatibility affordance — it is that an update check is the wrong place to
83+
/// turn a config typo into a hard failure.
7984
pub(crate) fn from_config(
8085
source: Option<&str>,
8186
package: Option<&str>,
@@ -84,16 +89,16 @@ pub(crate) fn from_config(
8489
) -> Option<CheckSource> {
8590
let package = || package.unwrap_or(PERRY_NPM_PACKAGE).to_string();
8691
match source?.trim().to_ascii_lowercase().as_str() {
87-
"gh-releases" | "github" => Some(CheckSource::GhReleases {
92+
"gh-releases" => Some(CheckSource::GhReleases {
8893
url: server
8994
.unwrap_or(super::update_checker::GITHUB_URL)
9095
.to_string(),
9196
}),
92-
"npm" | "npm-registry" => Some(CheckSource::Npm {
97+
"npm" => Some(CheckSource::Npm {
9398
package: package(),
9499
registry: registry.unwrap_or(NPM_REGISTRY).to_string(),
95100
}),
96-
"gh-registry" | "github-packages" => Some(CheckSource::GhRegistry {
101+
"gh-registry" => Some(CheckSource::GhRegistry {
97102
package: package(),
98103
registry: registry.unwrap_or(GH_REGISTRY).to_string(),
99104
}),

crates/perry/src/update_checker.rs

Lines changed: 66 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,26 @@ const CACHE_MAX_AGE: Duration = Duration::from_secs(24 * 60 * 60);
2121
const CONNECT_TIMEOUT: Duration = Duration::from_secs(5);
2222
const REQUEST_TIMEOUT: Duration = Duration::from_secs(10);
2323

24+
/// The shape of `~/.perry/update-check.json` this build writes and reads.
25+
///
26+
/// Bump it whenever the meaning of a field changes. There is deliberately no
27+
/// migration path: this file is a CACHE, rebuilt by the next check, so reading
28+
/// an older shape buys nothing and costs a growing set of optional fields that
29+
/// exist only to describe versions nobody runs. A mismatch is discarded.
30+
const CACHE_SCHEMA: u32 = 1;
31+
2432
#[derive(Debug, Serialize, Deserialize, Clone, PartialEq)]
2533
pub struct UpdateCache {
34+
/// See [`CACHE_SCHEMA`]. Absent or different means "throw this away".
35+
#[serde(default)]
36+
pub schema: u32,
2637
pub last_check: String,
2738
pub latest_version: String,
2839
pub release_url: String,
2940
/// When the user was last told about this update, if ever.
3041
///
31-
/// `default` + `skip_serializing_if` so a cache written by an older Perry
32-
/// still loads, and a cache that has never notified stays the shape it
33-
/// always was.
42+
/// Optional because "never notified" is a real state, not because an older
43+
/// shape has to load — see [`CACHE_SCHEMA`].
3444
#[serde(default, skip_serializing_if = "Option::is_none")]
3545
pub last_notification: Option<String>,
3646
/// Which version that notice was about.
@@ -95,7 +105,10 @@ fn cache_path() -> PathBuf {
95105
pub fn load_cache() -> Option<UpdateCache> {
96106
let path = cache_path();
97107
let content = fs::read_to_string(&path).ok()?;
98-
serde_json::from_str(&content).ok()
108+
let cache: UpdateCache = serde_json::from_str(&content).ok()?;
109+
// A different shape is thrown away, not migrated. The next check rewrites
110+
// it, so the only cost is one extra request.
111+
(cache.schema == CACHE_SCHEMA).then_some(cache)
99112
}
100113

101114
fn save_cache(cache: &UpdateCache) {
@@ -205,8 +218,7 @@ pub fn is_cache_stale_with(max_age: Duration) -> bool {
205218
};
206219

207220
// An invalid cached release must be refreshed rather than suppressing a
208-
// check for up to 24 hours. `parse_version` also accepts the abbreviated
209-
// versions written by older Perry releases.
221+
// check for up to 24 hours.
210222
if parse_version(&cache.latest_version).is_err() {
211223
return true;
212224
}
@@ -347,7 +359,6 @@ fn fetch_latest_version() -> Result<UpdateCache> {
347359
.context("Failed to create HTTP client")?;
348360

349361
let mut last_err = None;
350-
let prior_notification = load_cache().and_then(|c| c.last_notification);
351362

352363
// A configured source answers on its own. Nothing falls back to the ladder
353364
// after it: a user who said "ask npm" and got an error wants to hear that,
@@ -384,6 +395,7 @@ fn fetch_latest_version() -> Result<UpdateCache> {
384395
let _guard = lock_cache();
385396
let prior = load_cache();
386397
let cache = UpdateCache {
398+
schema: CACHE_SCHEMA,
387399
last_check: now_rfc3339(),
388400
latest_version: probe.latest_version,
389401
release_url: probe.release_url,
@@ -422,6 +434,7 @@ fn fetch_latest_version() -> Result<UpdateCache> {
422434
let _guard = lock_cache();
423435
let prior = load_cache();
424436
let cache = UpdateCache {
437+
schema: CACHE_SCHEMA,
425438
last_check: now_rfc3339(),
426439
latest_version: version,
427440
release_url: info.html_url,
@@ -1626,6 +1639,7 @@ mod tests {
16261639
#[test]
16271640
fn test_cache_roundtrip() {
16281641
let cache = UpdateCache {
1642+
schema: CACHE_SCHEMA,
16291643
last_check: "2025-01-15T10:30:00Z".to_string(),
16301644
latest_version: "0.2.171".to_string(),
16311645
release_url: "https://github.com/PerryTS/perry/releases/tag/v0.2.171".to_string(),
@@ -1640,29 +1654,60 @@ mod tests {
16401654
assert_eq!(cache, parsed);
16411655
}
16421656

1643-
/// A cache file written by a Perry that predates the notify throttle must
1644-
/// still load. Without `serde(default)` it would fail to parse, `load_cache`
1645-
/// would return `None`, and every user's first run on the new build would
1646-
/// re-check the network for no reason.
1657+
/// A cache whose shape this build does not recognize is DISCARDED, not
1658+
/// migrated. The file is a cache — the next check rewrites it — so reading
1659+
/// an older shape would buy one saved request in exchange for a growing set
1660+
/// of optional fields describing versions nobody runs.
16471661
#[test]
1648-
fn a_cache_without_the_notification_field_still_loads() {
1649-
let legacy = r#"{
1662+
fn a_cache_of_another_schema_is_discarded() {
1663+
let foreign = r#"{
1664+
"schema": 999,
1665+
"last_check": "2025-01-15T10:30:00Z",
1666+
"latest_version": "0.2.171",
1667+
"release_url": "https://example.test/v0.2.171"
1668+
}"#;
1669+
let parsed: UpdateCache = serde_json::from_str(foreign).expect("it still parses");
1670+
assert_ne!(
1671+
parsed.schema, CACHE_SCHEMA,
1672+
"test premise: this fixture is a foreign shape"
1673+
);
1674+
1675+
// A file with no schema at all reads as 0, which is equally foreign —
1676+
// that is what makes every pre-versioning cache fall out on its own
1677+
// without a compatibility branch.
1678+
let unversioned = r#"{
16501679
"last_check": "2025-01-15T10:30:00Z",
16511680
"latest_version": "0.2.171",
16521681
"release_url": "https://example.test/v0.2.171"
16531682
}"#;
1654-
let parsed: UpdateCache =
1655-
serde_json::from_str(legacy).expect("a pre-throttle cache must still parse");
1656-
assert_eq!(parsed.last_notification, None);
1657-
assert_eq!(parsed.latest_version, "0.2.171");
1658-
1659-
// ...and a cache that has never notified round-trips to the same shape
1660-
// it always had, rather than growing a null field.
1661-
let written = serde_json::to_string(&parsed).unwrap();
1683+
let parsed: UpdateCache = serde_json::from_str(unversioned).expect("parses");
1684+
assert_eq!(parsed.schema, 0);
1685+
assert_ne!(parsed.schema, CACHE_SCHEMA);
1686+
}
1687+
1688+
/// A cache that has never notified is written without the field, because
1689+
/// absence is the state — not because anything else has to read it.
1690+
#[test]
1691+
fn an_unset_optional_field_is_not_written() {
1692+
let cache = UpdateCache {
1693+
schema: CACHE_SCHEMA,
1694+
last_check: "2025-01-15T10:30:00Z".to_string(),
1695+
latest_version: "0.2.171".to_string(),
1696+
release_url: "https://example.test/v0.2.171".to_string(),
1697+
last_notification: None,
1698+
last_notified_version: None,
1699+
published_at: None,
1700+
headline: None,
1701+
};
1702+
let written = serde_json::to_string(&cache).unwrap();
16621703
assert!(
16631704
!written.contains("last_notification"),
16641705
"an unset field must not be written: {written}"
16651706
);
1707+
assert!(
1708+
written.contains("\"schema\":1"),
1709+
"the shape is stamped: {written}"
1710+
);
16661711
}
16671712

16681713
#[test]

0 commit comments

Comments
 (0)