Fix overrides and updates n stuff

This commit is contained in:
Julian Chang 2026-06-22 07:17:23 +07:00
parent 1635ca38ce
commit 723d1e43ae
No known key found for this signature in database
7 changed files with 351 additions and 88 deletions

View file

@ -251,6 +251,12 @@ impl OneClientApi for OneClientApiImpl {
self, self,
cluster_id: ClusterId, cluster_id: ClusterId,
) -> LauncherResult<ApplyBundleUpdatesResult> { ) -> LauncherResult<ApplyBundleUpdatesResult> {
// A manual update check must re-fetch the manifest and re-validate bundles rather than
// serving the process-cached copy, otherwise updates published after launch are missed.
crate::oneclient::bundles::BundlesManager::get()
.await
.refresh()
.await;
crate::oneclient::bundle_updates::apply_bundle_updates(cluster_id).await crate::oneclient::bundle_updates::apply_bundle_updates(cluster_id).await
} }

View file

@ -1370,82 +1370,16 @@ pub async fn apply_bundle_updates(
} }
} }
// Re-extract overrides from any bundle that had changes applied. // Always re-extract overrides for every subscribed bundle, regardless of whether any
// This keeps config files, resource packs, and other overridden assets in sync. // package was added/updated/removed this run.
let has_changes = !updates_applied.is_empty() //
|| !removals_applied.is_empty() // Overrides (config files, compatibility JSONs, resource packs, etc.) can change in a
|| !additions_applied.is_empty(); // bundle WITHOUT any package version changing — e.g. publishing a JSON that makes an
// already-installed mod compatible with a new MC version. Gating extraction on
if has_changes { // package-level changes (the previous behaviour) silently skipped these override-only
let mut affected_bundles: std::collections::HashSet<String> = // updates, so users who already had the mods installed never received the new override.
std::collections::HashSet::new(); if let Err(e) = extract_overrides_for_subscribed_bundles(cluster_id).await {
send_error!("Failed to extract bundle overrides for cluster {cluster_id}: {e}");
for u in &updates_applied {
affected_bundles.insert(u.bundle_name.clone());
}
for r in &removals_applied {
affected_bundles.insert(r.bundle_name.clone());
}
for a in &additions_applied {
affected_bundles.insert(a.bundle_name.clone());
}
if !affected_bundles.is_empty() {
match api::cluster::dao::get_cluster_by_id(cluster_id).await {
Ok(Some(cluster)) => {
match BundlesManager::get()
.await
.get_bundles_for(&cluster.mc_version, cluster.mc_loader)
.await
{
Ok(bundles) => {
for bundle in &bundles {
if affected_bundles.contains(&bundle.manifest.name) {
tracing::info!(
bundle_name = %bundle.manifest.name,
cluster_id = %cluster_id,
"Re-extracting overrides from updated bundle"
);
if let Err(e) =
onelauncher_core::api::packages::modpack::mrpack::copy_overrides_folder_no_overwrite(
&cluster,
&bundle.path,
)
.await
{
send_error!(
"Failed to extract overrides from bundle '{}': {}",
bundle.manifest.name,
e
);
}
}
}
}
Err(e) => {
tracing::error!(
cluster_id = %cluster_id,
"Failed to retrieve bundles for override extraction after update: {}",
e
);
}
}
}
Ok(None) => {
tracing::error!(
cluster_id = %cluster_id,
"Cluster not found during post-update override extraction"
);
}
Err(e) => {
tracing::error!(
cluster_id = %cluster_id,
"Failed to retrieve cluster for override extraction after update: {}",
e
);
}
}
}
} }
let package_operation_failed = let package_operation_failed =
@ -1470,6 +1404,65 @@ pub async fn apply_bundle_updates(
}) })
} }
/// Re-extracts override files (configs, compatibility JSONs, resource packs, etc.) for
/// every bundle the cluster is subscribed to.
///
/// This runs on every bundle sync — overrides can change independently of package
/// versions, so it must NOT be gated behind package-level changes. Existing on-disk files
/// are preserved (see [`copy_overrides_folder_no_overwrite`]) so user customizations and
/// already-correct files are not clobbered; only missing override files are written.
async fn extract_overrides_for_subscribed_bundles(cluster_id: ClusterId) -> LauncherResult<()> {
let Some(cluster) = api::cluster::dao::get_cluster_by_id(cluster_id).await? else {
tracing::warn!(
cluster_id = %cluster_id,
"Skipping override extraction because cluster no longer exists"
);
return Ok(());
};
let bundle_packages = bundle_dao::get_bundle_packages_for_cluster(cluster_id).await?;
let all_linked_packages = api::packages::dao::get_linked_packages(&cluster).await?;
let bundles = BundlesManager::get()
.await
.get_bundles_for(&cluster.mc_version, cluster.mc_loader)
.await?;
let all_installed_managed_keys = collect_all_installed_managed_keys(&all_linked_packages);
let subscribed_bundles = infer_subscribed_bundles_for_loader_sync(
&bundle_packages,
&bundles,
&all_installed_managed_keys,
);
for bundle in &bundles {
if !subscribed_bundles.contains(&bundle.manifest.name) {
continue;
}
tracing::debug!(
bundle_name = %bundle.manifest.name,
cluster_id = %cluster_id,
"Re-extracting overrides for subscribed bundle"
);
if let Err(e) =
onelauncher_core::api::packages::modpack::mrpack::copy_overrides_folder_no_overwrite(
&cluster,
&bundle.path,
)
.await
{
send_error!(
"Failed to extract overrides from bundle '{}': {}",
bundle.manifest.name,
e
);
}
}
Ok(())
}
/// Migrates non-bundle (custom) packages from `source_cluster_id` to `target_cluster_id`. /// Migrates non-bundle (custom) packages from `source_cluster_id` to `target_cluster_id`.
/// ///
/// For packages tracked on Modrinth or `CurseForge`, attempts to find and install a version /// For packages tracked on Modrinth or `CurseForge`, attempts to find and install a version

View file

@ -424,4 +424,161 @@ mod tests {
let _ = dao::delete_cluster_by_id(cluster.id).await; let _ = dao::delete_cluster_by_id(cluster.id).await;
let _ = fs::remove_dir_all(cluster.path().await.unwrap()).await; let _ = fs::remove_dir_all(cluster.path().await.unwrap()).await;
} }
/// Regression test for the bundle-override bug: override files (config JSONs, etc.) must
/// be re-extracted on a sync EVEN WHEN no package was added/updated/removed.
///
/// Previously override extraction was gated behind package-level changes, so an
/// override-only update (e.g. a JSON making an already-installed mod compatible with a
/// new MC version) was silently skipped for users who already had the mods installed.
#[tokio::test]
#[ignore = "touches true files and takes long"]
#[allow(clippy::too_many_lines, clippy::items_after_statements)]
async fn test_overrides_reextracted_without_package_changes() {
use sea_orm::{ActiveModelTrait, Set};
Core::initialize(CoreOptions::default()).await.unwrap();
let cluster_name = format!(
"Bundle Override-Only Test {}",
std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap()
.as_nanos()
);
let cluster = create_cluster(&cluster_name, "1.21.1", GameLoader::Fabric, None, None)
.await
.unwrap();
let cluster_dir = cluster.path().await.unwrap();
let bundles = BundlesManager::get()
.await
.get_bundles_for(&cluster.mc_version, cluster.mc_loader)
.await
.unwrap();
let target_bundle = bundles
.into_iter()
.find(|b| b.manifest.name.contains("Performance"))
.expect("Expected to find a Performance bundle");
// Find an override file shipped in the bundle archive so we have something to assert on.
let override_rel_path = first_override_entry(&target_bundle.path)
.await
.expect("bundle has no override files to test against; cannot run this regression");
// Subscribe the cluster to the bundle.
let dummy_hash = uuid::Uuid::new_v4().to_string();
onelauncher_core::entity::packages::ActiveModel {
hash: Set(dummy_hash.clone()),
file_name: Set("dummy.jar".to_string()),
version_id: Set("dummy_version".to_string()),
display_name: Set("Dummy".to_string()),
display_version: Set("1.0.0".to_string()),
package_type: Set(onelauncher_core::entity::package::PackageType::Mod),
provider: Set(onelauncher_core::entity::package::Provider::Modrinth),
package_id: Set("dummy_id".to_string()),
mc_versions: Set(onelauncher_core::entity::utility::DbVec(vec![
"1.21.1".to_string(),
])),
mc_loader: Set(onelauncher_core::entity::utility::DbVec(vec![
GameLoader::Fabric,
])),
published_at: Set(chrono::Utc::now()),
..Default::default()
}
.insert(&onelauncher_core::store::State::get().await.unwrap().db)
.await
.unwrap();
onelauncher_core::entity::cluster_packages::ActiveModel {
cluster_id: Set(cluster.id),
package_hash: Set(dummy_hash.clone()),
bundle_name: Set(Some(target_bundle.manifest.name.clone())),
..Default::default()
}
.insert(&onelauncher_core::store::State::get().await.unwrap().db)
.await
.unwrap();
// First apply: installs packages AND extracts overrides (has_changes == true).
let applied1 = apply_bundle_updates(cluster.id).await.unwrap();
assert!(
!applied1.additions_applied.is_empty(),
"Expected initial additions from bundle"
);
let extracted = cluster_dir.join(&override_rel_path);
assert!(
extracted.exists(),
"override file should be extracted on first apply"
);
// Simulate the override file being missing (e.g. it was only just added to the bundle,
// so existing users never had it). Delete it from disk.
fs::remove_file(&extracted).await.unwrap();
assert!(!extracted.exists());
// Second apply: nothing about the packages changed, so has_changes == false.
// The override file MUST still be re-extracted (this is the bug being regression-tested).
let applied2 = apply_bundle_updates(cluster.id).await.unwrap();
assert!(
applied2.updates_applied.is_empty()
&& applied2.additions_applied.is_empty()
&& applied2.removals_applied.is_empty(),
"second apply should have no package-level changes"
);
assert!(
extracted.exists(),
"override file must be re-extracted even when no package changed (Bug 2 regression)"
);
// Clean up
let _ = onelauncher_core::api::packages::dao::delete_package_by_id(dummy_hash).await;
let _ = dao::delete_cluster_by_id(cluster.id).await;
let _ = fs::remove_dir_all(cluster_dir).await;
}
/// Returns the first file path under `overrides/` in a `.mrpack`/zip archive, relative to
/// the cluster root (i.e. with the `overrides/` prefix stripped).
///
/// Extracts the archive's overrides into a throwaway temp dir using the same public helper
/// the real sync uses, then walks it — avoiding a direct zip-crate dependency in this test.
async fn first_override_entry(archive_path: &std::path::Path) -> Option<String> {
let probe_dir = std::env::temp_dir().join(format!(
"oneclient_override_probe_{}",
std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap()
.as_nanos()
));
fs::create_dir_all(&probe_dir).await.ok()?;
onelauncher_core::api::packages::modpack::mrpack::extract_overrides_no_overwrite(
&archive_path.to_path_buf(),
&probe_dir,
)
.await
.ok()?;
let relative = first_file_relative(&probe_dir, &probe_dir);
let _ = fs::remove_dir_all(&probe_dir).await;
relative
}
/// Recursively finds the first regular file under `dir`, returning its path relative to
/// `root` with forward slashes.
fn first_file_relative(dir: &std::path::Path, root: &std::path::Path) -> Option<String> {
let mut entries: Vec<_> = std::fs::read_dir(dir).ok()?.flatten().collect();
entries.sort_by_key(std::fs::DirEntry::path);
for entry in entries {
let path = entry.path();
if path.is_dir() {
if let Some(found) = first_file_relative(&path, root) {
return Some(found);
}
} else if let Ok(rel) = path.strip_prefix(root) {
return Some(rel.to_string_lossy().replace('\\', "/"));
}
}
None
}
} }

View file

@ -128,6 +128,22 @@ impl BundlesManager {
Ok(found.clone()) Ok(found.clone())
} }
/// Re-fetches the bundles manifest from remote and clears the in-memory parsed-bundle
/// cache so the next [`get_bundles_for`](Self::get_bundles_for) call reloads everything.
///
/// The on-disk `.mrpack` cache is still consulted (and validated via ETag) by
/// `download_and_load_bundle`, so this only forces re-validation, not blind re-downloads.
///
/// Without this, the manifest is fetched exactly once per process at startup and the
/// parsed bundles are cached for the process lifetime — meaning a long-running session
/// (or a manual "check for updates") would never observe a bundle the publisher updated
/// after launch. Mods would silently stop updating until a full restart.
pub async fn refresh(&self) {
let manifest = Self::fetch_cached().await;
*self.manifest.write().await = manifest;
self.bundles.write().await.clear();
}
/// Fetches the bundles manifest from remote, falling back to a saved copy on disk if available /// Fetches the bundles manifest from remote, falling back to a saved copy on disk if available
#[tracing::instrument] #[tracing::instrument]
pub async fn fetch_cached() -> BundleManifest { pub async fn fetch_cached() -> BundleManifest {

View file

@ -35,6 +35,11 @@ async fn check_and_apply_all_bundle_updates() {
BUNDLE_SYNCING.store(true, Ordering::Relaxed); BUNDLE_SYNCING.store(true, Ordering::Relaxed);
tracing::info!("checking for bundle updates..."); tracing::info!("checking for bundle updates...");
// Force a fresh manifest fetch + cache clear so we observe bundles the publisher updated
// since this process started (or since the last sync). The on-disk .mrpack cache is still
// ETag-validated, so unchanged bundles are not re-downloaded.
bundles::BundlesManager::get().await.refresh().await;
let clusters = match get_all_clusters().await { let clusters = match get_all_clusters().await {
Ok(clusters) => clusters, Ok(clusters) => clusters,
Err(err) => { Err(err) => {

View file

@ -75,6 +75,28 @@ export function getOnlineEntryForVersion(
export interface ParsedMcVersion { export interface ParsedMcVersion {
major: number; major: number;
minor: number | undefined; minor: number | undefined;
/**
* Patch component of the new (year-based) scheme, e.g. the `2` in `26.1.2`.
* `undefined` for two-part new versions (`26.1`) and for all old `1.X.Y` versions
* (whose third component is the `minor`, not a patch).
*/
patch?: number | undefined;
}
/**
* The major version at and above which Minecraft uses the new year-based scheme
* (`YY.N[.P]`, e.g. `26.1` / `26.1.2`). Below it, the legacy `1.X[.Y]` scheme is used.
*
* Kept as a single named constant so every formatter/parser agrees on the boundary —
* an inconsistent boundary is how malformed strings like `1.26` get produced.
*/
export const NEW_SCHEME_MAJOR = 26;
function toNumberOrUndefined(value: string | undefined): number | undefined {
if (value === undefined)
return undefined;
const parsed = Number.parseInt(value, 10);
return Number.isNaN(parsed) ? undefined : parsed;
} }
export function parseMcVersion(version: string | null | undefined): ParsedMcVersion | undefined { export function parseMcVersion(version: string | null | undefined): ParsedMcVersion | undefined {
@ -83,15 +105,19 @@ export function parseMcVersion(version: string | null | undefined): ParsedMcVers
const parts = version.split('.'); const parts = version.split('.');
// New format: YY.N[.P] (e.g. "26.1" or "26.1.1") — no "1." prefix // New format: YY.N[.P] (e.g. "26.1", "26.1.1", "26.1.2") — no "1." prefix.
if (parts[0] !== '1') { if (parts[0] !== '1') {
if (parts.length < 2) if (parts.length < 2)
return undefined; return undefined;
const major = Number.parseInt(parts[0], 10); const major = toNumberOrUndefined(parts[0]);
const minor = Number.parseInt(parts[1], 10); if (major === undefined)
return undefined;
return { return {
major, major,
minor: Number.isNaN(minor) ? undefined : minor, minor: toNumberOrUndefined(parts[1]),
// Preserve the patch component instead of silently dropping it, so a
// "26.1.2" cluster is parsed in full rather than collapsing to "26.1".
patch: toNumberOrUndefined(parts[2]),
}; };
} }
@ -99,21 +125,28 @@ export function parseMcVersion(version: string | null | undefined): ParsedMcVers
if (parts.length <= 1) if (parts.length <= 1)
return undefined; return undefined;
const major = Number.parseInt(parts[1], 10); const major = toNumberOrUndefined(parts[1]);
const minor = parts.length > 2 ? Number.parseInt(parts[2], 10) : undefined; if (major === undefined)
return undefined;
return { return {
major, major,
minor: Number.isNaN(minor) ? undefined : minor, minor: toNumberOrUndefined(parts[2]),
patch: undefined,
}; };
} }
/** /**
* Constructs a Minecraft version string from a major+minor pair. * Constructs a Minecraft version string from a major+minor[+patch] tuple.
* Versions from 2026 onward use the new YY.N format; older versions use 1.X.Y. * Versions from {@link NEW_SCHEME_MAJOR} onward use the new `YY.N[.P]` format;
* older versions use `1.X.Y`.
*
* The `major >= NEW_SCHEME_MAJOR` check MUST stay in sync with {@link parseMcVersion}'s
* `parts[0] !== '1'` rule: a `major` of 26 must never be formatted with a `1.` prefix
* (which would yield bogus strings like `1.26`).
*/ */
export function formatMcVersion(major: number, minor: number): string { export function formatMcVersion(major: number, minor: number, patch?: number): string {
if (major >= 26) if (major >= NEW_SCHEME_MAJOR)
return `${major}.${minor}`; return patch === undefined ? `${major}.${minor}` : `${major}.${minor}.${patch}`;
return `1.${major}.${minor}`; return `1.${major}.${minor}`;
} }

View file

@ -0,0 +1,53 @@
/* eslint-disable no-console, node/prefer-global/process -- standalone dev verification script, not app code */
/**
* Standalone verification for {@link ./versionMap.ts} parsing/formatting.
*
* No test runner is wired into this app, so this is a runnable assertion script:
* npx tsx src/utils/versionMap.verify.mts
*
* Confirms (Bug 3):
* 1. OneClient parses the new year-based version "26.1.2" (and "26.1") correctly.
* 2. No code path emits the malformed "1.26" string for a 2026-era version.
*/
import { formatMcVersion, parseMcVersion } from './versionMap.ts';
let failures = 0;
function check(label: string, actual: unknown, expected: unknown): void {
const a = JSON.stringify(actual);
const e = JSON.stringify(expected);
if (a === e) {
console.log(` ok ${label}`);
}
else {
failures++;
console.error(` FAIL ${label}\n expected ${e}\n actual ${a}`);
}
}
console.log('parseMcVersion:');
check('26.1.2 -> full parse with patch', parseMcVersion('26.1.2'), { major: 26, minor: 1, patch: 2 });
check('26.1 -> no patch', parseMcVersion('26.1'), { major: 26, minor: 1, patch: undefined });
check('26.1.1 -> patch 1', parseMcVersion('26.1.1'), { major: 26, minor: 1, patch: 1 });
check('1.21.5 -> old scheme', parseMcVersion('1.21.5'), { major: 21, minor: 5, patch: undefined });
check('1.21.1 -> old scheme', parseMcVersion('1.21.1'), { major: 21, minor: 1, patch: undefined });
check('null -> undefined', parseMcVersion(null), undefined);
check('"26" -> undefined (no minor)', parseMcVersion('26'), undefined);
console.log('formatMcVersion:');
check('(26,1,2) -> 26.1.2', formatMcVersion(26, 1, 2), '26.1.2');
check('(26,1) -> 26.1', formatMcVersion(26, 1), '26.1');
check('(21,5) -> 1.21.5', formatMcVersion(21, 5), '1.21.5');
console.log('round-trip (must never produce "1.26"):');
for (const v of ['26.1', '26.1.2', '26.1.1']) {
const p = parseMcVersion(v)!;
const out = formatMcVersion(p.major, p.minor!, p.patch);
check(`${v} round-trips to ${out}`, out, v);
check(`${v} never yields a "1." prefix`, out.startsWith('1.'), false);
}
if (failures > 0) {
console.error(`\n${failures} check(s) FAILED`);
process.exit(1);
}
console.log('\nAll version parsing checks passed.');