fix(core): bundle overrides being problematic
This commit is contained in:
LynithDev
parent
93aa1b5900
commit
ef60855f16
16 changed files with 441 additions and 116 deletions
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"db_name": "SQLite",
|
||||
"query": "\n SELECT 1 FROM cluster_artifacts\n WHERE cluster_id = ? AND bundle_name = ? AND package_id = ?\n LIMIT 1\n ",
|
||||
"query": "\n SELECT 1 FROM cluster_artifacts\n WHERE cluster_id = ? AND bundle_name IS NOT NULL AND package_id = ?\n LIMIT 1\n ",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
|
|
@ -11,11 +11,11 @@
|
|||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Right": 3
|
||||
"Right": 2
|
||||
},
|
||||
"nullable": [
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "dc93cdd57e23e48f8e3c333db4ec396b0ee319adec619e5a7d147805573ae2b2"
|
||||
"hash": "ab6dcea84a9502d5f8396a207ae613a4c41eb729ca22249f15eb45e6c6030094"
|
||||
}
|
||||
|
|
@ -797,7 +797,7 @@ impl Actions {
|
|||
// had leaves the newest live and the rest switched off. Done
|
||||
// before the refresh below so the version list never renders the
|
||||
// moment where both copies read as active.
|
||||
if let Err(err) = oneclient_content::packages::reconcile_duplicate_activity(
|
||||
if let Err(err) = oneclient_content::bundles::reconcile_duplicate_activity(
|
||||
cluster_id,
|
||||
&state.services.content(),
|
||||
)
|
||||
|
|
@ -806,13 +806,6 @@ impl Actions {
|
|||
tracing::warn!(%err, "failed to resolve duplicate package versions");
|
||||
}
|
||||
|
||||
// What the install button turns into is read straight off the
|
||||
// cluster's content, so that is refreshed here and waited for
|
||||
// before the busy flag drops. Left to the `ClustersChanged`
|
||||
// signal alone, the button came back as a live "Install" the
|
||||
// moment the download finished and only grew its "Installed"
|
||||
// badge once an event-pump round trip and a full cluster-query
|
||||
// sweep — network calls included — had gone by.
|
||||
super::invalidate_cluster_content_queries().await;
|
||||
|
||||
// Everything else the install touched is nobody's blocker and
|
||||
|
|
|
|||
|
|
@ -188,16 +188,6 @@ pub(crate) fn installed_badge_overlay(installed: InstallSource) -> impl IntoElem
|
|||
}
|
||||
|
||||
/// Which of several installed versions the game actually loads.
|
||||
///
|
||||
/// Only worth showing when a package has more than one copy in the cluster —
|
||||
/// with a single one "active" says nothing "installed" hasn't.
|
||||
///
|
||||
/// Reads the artifact's `enabled` flag rather than working the answer out
|
||||
/// itself, because
|
||||
/// [`reconcile_duplicate_activity`](oneclient_content::packages::reconcile_duplicate_activity)
|
||||
/// has already resolved duplicates to the newest copy. Exactly one row in a
|
||||
/// duplicated set should read Active; more than one means that pass has not run
|
||||
/// since the copies appeared.
|
||||
pub(crate) fn activity_badge(active: bool) -> impl IntoElement {
|
||||
let (text, color) = if active {
|
||||
("Active", colors::brand())
|
||||
|
|
|
|||
|
|
@ -507,9 +507,15 @@ fn build_plans(
|
|||
items
|
||||
.iter()
|
||||
.map(|cb| {
|
||||
let kept = kept_package_ids(cb.cluster.id, &cb.archives, selected);
|
||||
let mut overrides = Vec::new();
|
||||
for archive in &cb.archives {
|
||||
overrides.extend(archive_overrides(cb.cluster.id, archive, selected));
|
||||
overrides.extend(archive_overrides(
|
||||
cb.cluster.id,
|
||||
archive,
|
||||
selected,
|
||||
&kept,
|
||||
));
|
||||
}
|
||||
ClusterPlan {
|
||||
cluster_id: cb.cluster.id,
|
||||
|
|
@ -535,10 +541,62 @@ fn cluster_predownloads(versions: &[VersionMetadata], cluster: &Cluster) -> bool
|
|||
})
|
||||
}
|
||||
|
||||
/// Whether a bundle keeps its hidden files: true once anything visible in it was
|
||||
/// taken, including a single opted-in extra that would otherwise lose its
|
||||
/// dependencies.
|
||||
fn bundle_taken(
|
||||
cluster_id: i64,
|
||||
archive: &BundleArchive,
|
||||
selected: &std::collections::HashSet<String>,
|
||||
) -> bool {
|
||||
archive.manifest.files.iter().any(|file| {
|
||||
!file.hidden
|
||||
&& selected.contains(&pkg_key(
|
||||
cluster_id,
|
||||
&archive.manifest.name,
|
||||
&file.kind.package_id(),
|
||||
))
|
||||
})
|
||||
}
|
||||
|
||||
/// Every package id this cluster is keeping, across all of its bundles.
|
||||
///
|
||||
/// Bundles overlap: the library one pack needs is often the library another
|
||||
/// needs too, shipped hidden by both. Deciding each archive on its own meant a
|
||||
/// declined bundle recorded its hidden files as removed even when an accepted
|
||||
/// bundle right beside it was about to install the very same package — leaving a
|
||||
/// `removed` row for something the cluster demonstrably has, which then answered
|
||||
/// for it in anything that looks a package up by id.
|
||||
fn kept_package_ids(
|
||||
cluster_id: i64,
|
||||
archives: &[BundleArchive],
|
||||
selected: &std::collections::HashSet<String>,
|
||||
) -> std::collections::HashSet<String> {
|
||||
let mut kept = std::collections::HashSet::new();
|
||||
|
||||
for archive in archives {
|
||||
let takes_hidden = bundle_taken(cluster_id, archive, selected);
|
||||
for file in &archive.manifest.files {
|
||||
let package_id = file.kind.package_id();
|
||||
let wanted = if file.hidden {
|
||||
takes_hidden
|
||||
} else {
|
||||
selected.contains(&pkg_key(cluster_id, &archive.manifest.name, &package_id))
|
||||
};
|
||||
if wanted {
|
||||
kept.insert(package_id);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
kept
|
||||
}
|
||||
|
||||
fn archive_overrides(
|
||||
cluster_id: i64,
|
||||
archive: &BundleArchive,
|
||||
selected: &std::collections::HashSet<String>,
|
||||
kept_elsewhere: &std::collections::HashSet<String>,
|
||||
) -> Vec<(String, String, OverrideType)> {
|
||||
let bundle_name = &archive.manifest.name;
|
||||
let wants = |file: &BundleFile| {
|
||||
|
|
@ -546,19 +604,13 @@ fn archive_overrides(
|
|||
};
|
||||
|
||||
// Hidden files are dependencies the user never sees, so they follow the
|
||||
// bundle: kept if anything from it was taken (including a single opted-in
|
||||
// extra, which would otherwise lose its dependencies), dropped if not.
|
||||
let bundle_taken = archive
|
||||
.manifest
|
||||
.files
|
||||
.iter()
|
||||
.filter(|f| !f.hidden)
|
||||
.any(wants);
|
||||
// bundle: kept if anything from it was taken, dropped if not.
|
||||
let takes_hidden = bundle_taken(cluster_id, archive, selected);
|
||||
|
||||
let mut overrides = Vec::new();
|
||||
for file in &archive.manifest.files {
|
||||
let wanted = if file.hidden {
|
||||
bundle_taken
|
||||
takes_hidden
|
||||
} else {
|
||||
wants(file)
|
||||
};
|
||||
|
|
@ -568,8 +620,15 @@ fn archive_overrides(
|
|||
(true, true) | (false, false) => continue,
|
||||
// Opting into a mod the bundle ships turned off.
|
||||
(true, false) => OverrideType::Enabled,
|
||||
// Declined: don't fetch it at all.
|
||||
(false, true) => OverrideType::Removed,
|
||||
// Declined — but only really declined if no bundle the user did take
|
||||
// ships the same package. Recording a removal for something that is
|
||||
// being installed anyway states the opposite of what happens.
|
||||
(false, true) => {
|
||||
if kept_elsewhere.contains(&file.kind.package_id()) {
|
||||
continue;
|
||||
}
|
||||
OverrideType::Removed
|
||||
}
|
||||
};
|
||||
|
||||
overrides.push((bundle_name.clone(), file.kind.package_id(), override_type));
|
||||
|
|
@ -590,30 +649,17 @@ fn rough_download_estimate(
|
|||
if !cluster_predownloads(versions, &cb.cluster) {
|
||||
continue;
|
||||
}
|
||||
// What the cluster is keeping is already the answer to "what gets
|
||||
// downloaded", so the estimate reads it directly rather than
|
||||
// reconstructing it from the overrides. Counted once per package: two
|
||||
// bundles shipping the same library is one download, which is what the
|
||||
// install path does with it too.
|
||||
let kept = kept_package_ids(cb.cluster.id, &cb.archives, selected);
|
||||
let mut counted: std::collections::HashSet<String> = std::collections::HashSet::new();
|
||||
for archive in &cb.archives {
|
||||
let dropped: std::collections::HashSet<String> =
|
||||
archive_overrides(cb.cluster.id, archive, selected)
|
||||
.into_iter()
|
||||
.filter(|(_, _, ty)| *ty == OverrideType::Removed)
|
||||
.map(|(_, pid, _)| pid)
|
||||
.collect();
|
||||
let opted_in: std::collections::HashSet<String> =
|
||||
archive_overrides(cb.cluster.id, archive, selected)
|
||||
.into_iter()
|
||||
.filter(|(_, _, ty)| *ty == OverrideType::Enabled)
|
||||
.map(|(_, pid, _)| pid)
|
||||
.collect();
|
||||
|
||||
for file in &archive.manifest.files {
|
||||
let pid = file.kind.package_id();
|
||||
let installing = if opted_in.contains(&pid) {
|
||||
true
|
||||
} else if dropped.contains(&pid) {
|
||||
false
|
||||
} else {
|
||||
file.enabled
|
||||
};
|
||||
if installing {
|
||||
let package_id = file.kind.package_id();
|
||||
if kept.contains(&package_id) && counted.insert(package_id) {
|
||||
total += file.size;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -173,6 +173,88 @@ pub async fn install_bundle(
|
|||
install_enabled_bundle_files(&archive, cluster_id, skip_compatibility, None, ctx).await
|
||||
}
|
||||
|
||||
/// Whether a bundle file that is switched off was switched off on purpose.
|
||||
fn disable_was_deliberate(hidden: bool, user_override: Option<OverrideType>) -> bool {
|
||||
match user_override {
|
||||
Some(OverrideType::Removed) => true,
|
||||
// Hidden files are dependencies the bundle needs and the user was never
|
||||
// shown, so they follow the bundle rather than standing on their own.
|
||||
// Only an outright removal keeps one off.
|
||||
Some(OverrideType::Disabled) => !hidden,
|
||||
Some(OverrideType::Enabled) | None => false,
|
||||
}
|
||||
}
|
||||
|
||||
/// Switches bundle content back on where it was left off without saving the choice
|
||||
#[tracing::instrument(level = "debug", skip(archives, ctx))]
|
||||
pub async fn heal_bundle_activity(
|
||||
cluster_id: i64,
|
||||
archives: &[BundleArchive],
|
||||
ctx: &ContentCtx,
|
||||
) -> ContentResult<()> {
|
||||
let tracked = bundle_dao::list_bundle_tracked(&ctx.db, cluster_id).await?;
|
||||
if tracked.iter().all(|row| row.enabled != 0) {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let mut hidden: std::collections::HashMap<(&str, String), bool> =
|
||||
std::collections::HashMap::new();
|
||||
for archive in archives {
|
||||
for file in &archive.manifest.files {
|
||||
hidden.insert(
|
||||
(archive.manifest.name.as_str(), file.kind.package_id()),
|
||||
file.hidden,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
let overrides = bundle_dao::list_overrides(&ctx.db, cluster_id).await?;
|
||||
|
||||
for row in tracked.iter().filter(|row| row.enabled == 0) {
|
||||
let (Some(bundle_name), Some(package_id)) = (&row.bundle_name, &row.package_id) else {
|
||||
continue;
|
||||
};
|
||||
let Some(is_hidden) = hidden
|
||||
.get(&(bundle_name.as_str(), package_id.clone()))
|
||||
.copied()
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
|
||||
let user_override = find_override(&overrides, bundle_name, package_id);
|
||||
if disable_was_deliberate(is_hidden, user_override) {
|
||||
continue;
|
||||
}
|
||||
|
||||
tracing::info!(
|
||||
cluster_id,
|
||||
bundle = %bundle_name,
|
||||
package_id,
|
||||
hidden = is_hidden,
|
||||
?user_override,
|
||||
"re-enabling bundle content that was switched off with nothing recording the choice"
|
||||
);
|
||||
|
||||
// One unrepairable row is not worth failing an install over; the rest of
|
||||
// the cluster still wants fixing.
|
||||
if let Err(err) =
|
||||
PackageStore::set_artifact_enabled_to(cluster_id, &row.hash, true, ctx).await
|
||||
{
|
||||
tracing::warn!(hash = %row.hash, error = %err, "failed to re-enable bundle content");
|
||||
continue;
|
||||
}
|
||||
|
||||
// A hidden file reaching here with a `disabled` row is the one case where
|
||||
// the override is the wrong half. Drop it, or the next pass reads the
|
||||
// same disagreement and the two records never settle.
|
||||
if user_override == Some(OverrideType::Disabled) {
|
||||
bundle_dao::remove_override(&ctx.db, cluster_id, bundle_name, package_id).await?;
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tracing::instrument(level = "debug", skip(archive, progress, ctx), fields(bundle = %archive.manifest.name))]
|
||||
pub async fn install_enabled_bundle_files(
|
||||
archive: &BundleArchive,
|
||||
|
|
@ -182,6 +264,7 @@ pub async fn install_enabled_bundle_files(
|
|||
ctx: &ContentCtx,
|
||||
) -> ContentResult<Vec<String>> {
|
||||
extract_bundle_overrides_for_cluster(archive, cluster_id, ctx).await?;
|
||||
heal_bundle_activity(cluster_id, std::slice::from_ref(archive), ctx).await?;
|
||||
|
||||
let overrides = bundle_dao::list_overrides(&ctx.db, cluster_id).await?;
|
||||
let bundle_name = archive.manifest.name.clone();
|
||||
|
|
@ -475,6 +558,51 @@ pub async fn toggle_artifact_enabled(
|
|||
Ok(enabled)
|
||||
}
|
||||
|
||||
/// Puts an artifact into a known enabled state and updates the bundle override
|
||||
/// records to match.
|
||||
///
|
||||
/// The set counterpart to [`toggle_artifact_enabled`], and the one to reach for
|
||||
/// whenever the caller already knows the state it wants. Flipping only lands on
|
||||
/// the right answer when the current value is known to be the opposite, which is
|
||||
/// true of a user pressing a switch and false of everything else: an artifact
|
||||
/// that is relinked keeps whatever `enabled` it already had, so a flip applied
|
||||
/// to a row that was already correct puts it wrong.
|
||||
#[tracing::instrument(level = "debug", skip(ctx))]
|
||||
pub async fn set_artifact_enabled_to(
|
||||
cluster_id: i64,
|
||||
hash: &str,
|
||||
enabled: bool,
|
||||
ctx: &ContentCtx,
|
||||
) -> ContentResult<()> {
|
||||
PackageStore::set_artifact_enabled_to(cluster_id, hash, enabled, ctx).await?;
|
||||
|
||||
if enabled {
|
||||
on_user_enable_artifact(cluster_id, hash, ctx).await
|
||||
} else {
|
||||
on_user_disable_artifact(cluster_id, hash, ctx).await
|
||||
}
|
||||
}
|
||||
|
||||
/// [`crate::packages::reconcile_duplicate_activity`] with the bundle half added.
|
||||
///
|
||||
/// Same split as [`toggle_artifact_enabled`]: `packages` owns the flag and knows
|
||||
/// nothing about bundles, and the override that has to move with it is written
|
||||
/// here. A bundle copy that loses to a newer one used to be switched off with
|
||||
/// nothing recording why, which read as a state nobody chose — and once this
|
||||
/// module started repairing exactly that state, the two passes would have spent
|
||||
/// every launch undoing each other.
|
||||
#[tracing::instrument(level = "debug", skip(ctx))]
|
||||
pub async fn reconcile_duplicate_activity(
|
||||
cluster_id: i64,
|
||||
ctx: &ContentCtx,
|
||||
) -> ContentResult<()> {
|
||||
for hash in crate::packages::reconcile_duplicate_activity(cluster_id, ctx).await? {
|
||||
on_user_disable_artifact(cluster_id, &hash, ctx).await?;
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub async fn on_user_disable_artifact(
|
||||
cluster_id: i64,
|
||||
hash: &str,
|
||||
|
|
@ -580,10 +708,9 @@ pub async fn remove_artifact_from_cluster(
|
|||
)
|
||||
.await?;
|
||||
} else {
|
||||
let replacement_exists = bundle_dao::has_bundle_mapping(
|
||||
let replacement_exists = bundle_dao::has_bundle_mapping_for_package(
|
||||
&ctx.db,
|
||||
cluster_id,
|
||||
&bundle_name,
|
||||
&package_id,
|
||||
)
|
||||
.await?;
|
||||
|
|
@ -663,6 +790,36 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_disable_with_no_override_behind_it_is_an_accident() {
|
||||
assert!(!disable_was_deliberate(false, None));
|
||||
assert!(!disable_was_deliberate(true, None));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_users_own_disable_is_respected() {
|
||||
assert!(disable_was_deliberate(
|
||||
false,
|
||||
Some(OverrideType::Disabled)
|
||||
));
|
||||
assert!(disable_was_deliberate(false, Some(OverrideType::Removed)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_hidden_dependency_cannot_be_disabled_on_its_own() {
|
||||
assert!(
|
||||
!disable_was_deliberate(true, Some(OverrideType::Disabled)),
|
||||
"a hidden file follows its bundle; only removing it keeps it off"
|
||||
);
|
||||
assert!(disable_was_deliberate(true, Some(OverrideType::Removed)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_opt_in_override_never_reads_as_a_disable() {
|
||||
assert!(!disable_was_deliberate(false, Some(OverrideType::Enabled)));
|
||||
assert!(!disable_was_deliberate(true, Some(OverrideType::Enabled)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn find_override_matches_on_bundle_and_package() {
|
||||
let rows = vec![
|
||||
|
|
|
|||
|
|
@ -10,11 +10,14 @@ mod updates;
|
|||
|
||||
pub use error::BundleError;
|
||||
pub use install::{
|
||||
effective_enabled, enabled_bundle_bytes, extract_bundle_overrides_for_cluster, install_bundle,
|
||||
effective_enabled, enabled_bundle_bytes, extract_bundle_overrides_for_cluster,
|
||||
heal_bundle_activity, install_bundle,
|
||||
install_cluster_bundles, install_enabled_bundle_files, install_package_from_bundle,
|
||||
list_cluster_bundle_overrides,
|
||||
on_user_disable_artifact, on_user_enable_artifact, on_user_remove_artifact,
|
||||
remove_artifact_from_cluster, set_bundle_package_enabled, set_bundle_package_opt_in,
|
||||
reconcile_duplicate_activity,
|
||||
remove_artifact_from_cluster, set_artifact_enabled_to, set_bundle_package_enabled,
|
||||
set_bundle_package_opt_in,
|
||||
toggle_artifact_enabled,
|
||||
set_bundle_package_override, set_bundle_package_overrides,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -12,8 +12,8 @@ use tokio::sync::Mutex as AsyncMutex;
|
|||
use futures_util::StreamExt;
|
||||
|
||||
use crate::bundles::install::{
|
||||
BUNDLE_INSTALL_CONCURRENCY, install_package_from_bundle, remove_artifact_from_cluster,
|
||||
toggle_artifact_enabled,
|
||||
BUNDLE_INSTALL_CONCURRENCY, heal_bundle_activity, install_package_from_bundle,
|
||||
remove_artifact_from_cluster, set_artifact_enabled_to,
|
||||
};
|
||||
use crate::bundles::manager::BundlesManager;
|
||||
use crate::bundles::overrides;
|
||||
|
|
@ -382,6 +382,19 @@ pub async fn apply_bundle_updates_with(
|
|||
) -> ContentResult<ApplyBundleUpdatesResult> {
|
||||
let lock = cluster_lock(cluster_id);
|
||||
let _guard = lock.lock().await;
|
||||
|
||||
// Before anything is decided. The check below works from the override table
|
||||
// and the manifest defaults and never consults `enabled`, so a cluster
|
||||
// holding content that was switched off with nothing recording the choice
|
||||
// would be read as healthy and left exactly as it is.
|
||||
{
|
||||
let cluster = PackageStore::get_cluster(cluster_id, ctx).await?;
|
||||
let loader = GameLoader::from_repr(cluster.mc_loader as u8).unwrap_or(GameLoader::Fabric);
|
||||
if let Ok(archives) = bundles.archives_for(ctx, &cluster.mc_version, loader).await {
|
||||
heal_bundle_activity(cluster_id, &archives, ctx).await?;
|
||||
}
|
||||
}
|
||||
|
||||
let overrides = bundle_dao::list_overrides(&ctx.db, cluster_id).await?;
|
||||
let check = check_bundle_updates_inner(cluster_id, bundles, ctx, &overrides).await?;
|
||||
|
||||
|
|
@ -547,6 +560,11 @@ pub async fn apply_bundle_updates_with(
|
|||
/// Local bookkeeping for an update whose new file is already downloaded and
|
||||
/// linked: apply the user's enable/disable choice, then drop the artifact the
|
||||
/// update supersedes.
|
||||
///
|
||||
/// The choice is *set*, not flipped. Linking an artifact leaves an existing row's
|
||||
/// `enabled` alone, so re-applying an update to a row that already read correctly
|
||||
/// used to turn it into its opposite — which is the "my mods turn themselves off
|
||||
/// at random" report.
|
||||
#[tracing::instrument(level = "debug", skip_all, fields(cluster_id = update.cluster_id, bundle = %update.bundle_name, new_version = %update.new_version_id))]
|
||||
async fn reconcile_update(
|
||||
update: &BundlePackageUpdate,
|
||||
|
|
@ -555,9 +573,8 @@ async fn reconcile_update(
|
|||
ctx: &ContentCtx,
|
||||
) -> ContentResult<()> {
|
||||
let file_id = update.new_file.kind.package_id();
|
||||
if should_be_disabled(&update.bundle_name, &file_id, overrides) {
|
||||
toggle_artifact_enabled(update.cluster_id, hash, ctx).await?;
|
||||
}
|
||||
let enabled = !should_be_disabled(&update.bundle_name, &file_id, overrides);
|
||||
set_artifact_enabled_to(update.cluster_id, hash, enabled, ctx).await?;
|
||||
|
||||
if hash == update.installed_hash {
|
||||
return Ok(());
|
||||
|
|
@ -574,11 +591,8 @@ async fn reconcile_addition(
|
|||
ctx: &ContentCtx,
|
||||
) -> ContentResult<()> {
|
||||
let file_id = addition.new_file.kind.package_id();
|
||||
if should_be_disabled(&addition.bundle_name, &file_id, overrides) {
|
||||
toggle_artifact_enabled(addition.cluster_id, hash, ctx).await?;
|
||||
}
|
||||
|
||||
Ok(())
|
||||
let enabled = !should_be_disabled(&addition.bundle_name, &file_id, overrides);
|
||||
set_artifact_enabled_to(addition.cluster_id, hash, enabled, ctx).await
|
||||
}
|
||||
|
||||
fn should_be_disabled(
|
||||
|
|
|
|||
|
|
@ -29,7 +29,7 @@ struct Copy {
|
|||
published_at: Option<String>,
|
||||
}
|
||||
|
||||
/// Ensures each project in the cluster has exactly one enabled copy: the newest.
|
||||
/// Leaves each project in the cluster with the newest copy.
|
||||
///
|
||||
/// A no-op on a cluster with no duplicates, which is every healthy one, so it is
|
||||
/// cheap enough to run before a launch and after an install.
|
||||
|
|
@ -37,40 +37,45 @@ struct Copy {
|
|||
pub async fn reconcile_duplicate_activity(
|
||||
cluster_id: i64,
|
||||
ctx: &ContentCtx,
|
||||
) -> ContentResult<()> {
|
||||
) -> ContentResult<Vec<String>> {
|
||||
let linked = PackageStore::list_linked_artifacts(cluster_id, ctx).await?;
|
||||
let mut switched_off = Vec::new();
|
||||
|
||||
for ((provider, project_id), copies) in group_duplicates(&linked) {
|
||||
let Some(winner) = newest(&copies) else {
|
||||
for hash in duplicates_to_disable(&linked) {
|
||||
tracing::info!(
|
||||
cluster_id,
|
||||
%hash,
|
||||
"resolving duplicate versions of a package to the newest live one"
|
||||
);
|
||||
|
||||
PackageStore::set_artifact_enabled_to(cluster_id, &hash, false, ctx).await?;
|
||||
switched_off.push(hash);
|
||||
}
|
||||
|
||||
Ok(switched_off)
|
||||
}
|
||||
|
||||
fn duplicates_to_disable(linked: &[LinkedArtifactInfo]) -> Vec<String> {
|
||||
let mut out = Vec::new();
|
||||
|
||||
for (_, copies) in group_duplicates(linked) {
|
||||
let live: Vec<&Copy> = copies.iter().filter(|copy| copy.enabled).collect();
|
||||
if live.len() < 2 {
|
||||
continue;
|
||||
}
|
||||
|
||||
let Some(winner) = newest(live.iter().copied()) else {
|
||||
continue;
|
||||
};
|
||||
|
||||
for copy in &copies {
|
||||
let should_be_enabled = copy.hash == winner;
|
||||
if copy.enabled == should_be_enabled {
|
||||
continue;
|
||||
}
|
||||
|
||||
tracing::info!(
|
||||
cluster_id,
|
||||
?provider,
|
||||
project_id,
|
||||
hash = %copy.hash,
|
||||
enabled = should_be_enabled,
|
||||
"resolving duplicate versions of a package to the newest one"
|
||||
);
|
||||
|
||||
PackageStore::set_artifact_enabled_to(
|
||||
cluster_id,
|
||||
©.hash,
|
||||
should_be_enabled,
|
||||
ctx,
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
out.extend(
|
||||
live.iter()
|
||||
.filter(|copy| copy.hash != winner)
|
||||
.map(|copy| copy.hash.clone()),
|
||||
);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
out
|
||||
}
|
||||
|
||||
/// Groups the cluster's content by project, keeping only the projects it has
|
||||
|
|
@ -110,9 +115,9 @@ fn group_duplicates(
|
|||
/// loses to any copy that has one — it is usually an older row written before
|
||||
/// the field was recorded — and an all-undated group falls back to the first,
|
||||
/// which at least makes the choice stable across runs rather than arbitrary.
|
||||
fn newest(copies: &[Copy]) -> Option<String> {
|
||||
fn newest<'a>(copies: impl IntoIterator<Item = &'a Copy>) -> Option<String> {
|
||||
copies
|
||||
.iter()
|
||||
.into_iter()
|
||||
.max_by(|a, b| a.published_at.cmp(&b.published_at))
|
||||
.map(|copy| copy.hash.clone())
|
||||
}
|
||||
|
|
@ -195,6 +200,54 @@ mod tests {
|
|||
assert_eq!(groups[0].1.len(), 2);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_newest_live_copy_survives_and_the_rest_go_quiet() {
|
||||
let mut disable = duplicates_to_disable(&[
|
||||
info(Some("sodium"), "old", true, Some("2026-01-01T00:00:00Z")),
|
||||
info(Some("sodium"), "new", true, Some("2026-06-01T00:00:00Z")),
|
||||
info(Some("sodium"), "middle", true, Some("2026-03-01T00:00:00Z")),
|
||||
]);
|
||||
disable.sort();
|
||||
assert_eq!(disable, vec!["middle".to_string(), "old".to_string()]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_copy_that_is_already_off_is_not_counted_as_a_conflict() {
|
||||
let disable = duplicates_to_disable(&[
|
||||
info(Some("sodium"), "live", true, Some("2026-01-01T00:00:00Z")),
|
||||
info(Some("sodium"), "off", false, Some("2026-06-01T00:00:00Z")),
|
||||
]);
|
||||
|
||||
assert!(
|
||||
disable.is_empty(),
|
||||
"one live copy is not a duplicate, however many switched-off copies sit beside it"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_user_disabling_the_newest_copy_is_left_standing() {
|
||||
let disable = duplicates_to_disable(&[
|
||||
info(Some("sodium"), "chosen", true, Some("2026-01-01T00:00:00Z")),
|
||||
info(Some("sodium"), "newest", false, Some("2026-06-01T00:00:00Z")),
|
||||
]);
|
||||
|
||||
assert!(
|
||||
disable.is_empty(),
|
||||
"picking the older version on purpose must not be undone, and the newest must not be \
|
||||
switched back on"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_project_with_no_live_copy_at_all_is_left_alone() {
|
||||
let disable = duplicates_to_disable(&[
|
||||
info(Some("sodium"), "a", false, Some("2026-01-01T00:00:00Z")),
|
||||
info(Some("sodium"), "b", false, Some("2026-06-01T00:00:00Z")),
|
||||
]);
|
||||
|
||||
assert!(disable.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn content_with_no_project_is_left_alone() {
|
||||
let groups = group_duplicates(&[
|
||||
|
|
|
|||
|
|
@ -62,7 +62,7 @@ pub async fn materialize_content(
|
|||
// of one package would otherwise hand the game every enabled copy, and for
|
||||
// mods that is a classloader conflict rather than a choice. Resolving to the
|
||||
// newest here means the rest are simply not desired.
|
||||
if let Err(err) = oneclient_content::packages::reconcile_duplicate_activity(
|
||||
if let Err(err) = oneclient_content::bundles::reconcile_duplicate_activity(
|
||||
cluster.id,
|
||||
&services.content(),
|
||||
)
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"db_name": "SQLite",
|
||||
"query": "\n\t\tINSERT INTO java_versions (absolute_path, major, version, vendor, os_arch)\n\t\tVALUES (?, ?, ?, ?, ?)\n\t\tON CONFLICT(absolute_path) DO UPDATE SET\n\t\t\tmajor = excluded.major,\n\t\t\tversion = excluded.version,\n\t\t\tvendor = excluded.vendor,\n\t\t\tos_arch = excluded.os_arch\n\t\tRETURNING absolute_path, major, version, vendor, os_arch\n\t\t",
|
||||
"query": "\n\t\tINSERT INTO java_versions (absolute_path, major, version, vendor, os_arch, is_jdk)\n\t\tVALUES (?, ?, ?, ?, ?, ?)\n\t\tON CONFLICT(absolute_path) DO UPDATE SET\n\t\t\tmajor = excluded.major,\n\t\t\tversion = excluded.version,\n\t\t\tvendor = excluded.vendor,\n\t\t\tos_arch = excluded.os_arch,\n\t\t\tis_jdk = excluded.is_jdk\n\t\tRETURNING absolute_path, major, version, vendor, os_arch, is_jdk as \"is_jdk: bool\"\n\t\t",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
|
|
@ -57,18 +57,30 @@
|
|||
"name": "os_arch"
|
||||
}
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "is_jdk: bool",
|
||||
"ordinal": 5,
|
||||
"type_info": "Integer",
|
||||
"origin": {
|
||||
"Table": {
|
||||
"table": "java_versions",
|
||||
"name": "is_jdk"
|
||||
}
|
||||
}
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Right": 5
|
||||
"Right": 6
|
||||
},
|
||||
"nullable": [
|
||||
false,
|
||||
false,
|
||||
false,
|
||||
false,
|
||||
false,
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "a3bd6cf3048a6a5ed9d29e6bbb7f517676fe715c8af45eb5c58d69073e2d0304"
|
||||
"hash": "010f5dcda59ec32eaad1b9c3e2e9807f547a1376372c8b201653a36d6dd14cbc"
|
||||
}
|
||||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"db_name": "SQLite",
|
||||
"query": "\n SELECT absolute_path, major, version, vendor, os_arch FROM java_versions \n ORDER BY major DESC, version DESC\n ",
|
||||
"query": "\n SELECT absolute_path, major, version, vendor, os_arch, is_jdk as \"is_jdk: bool\"\n FROM java_versions\n ORDER BY major DESC, version DESC\n ",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
|
|
@ -57,6 +57,17 @@
|
|||
"name": "os_arch"
|
||||
}
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "is_jdk: bool",
|
||||
"ordinal": 5,
|
||||
"type_info": "Integer",
|
||||
"origin": {
|
||||
"Table": {
|
||||
"table": "java_versions",
|
||||
"name": "is_jdk"
|
||||
}
|
||||
}
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
|
|
@ -67,8 +78,9 @@
|
|||
false,
|
||||
false,
|
||||
false,
|
||||
false,
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "c1d3cf930d5f38e8f1046da1a8f895ee1de2d3fe9a40ecb47731618962d2a4ad"
|
||||
"hash": "0d9fb5d2fa8a788770e946241ea2f62f4b794c0fdda3ccbde49384689911aeba"
|
||||
}
|
||||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"db_name": "SQLite",
|
||||
"query": "\n\t\tSELECT absolute_path, major, version, vendor, os_arch\n\t\tFROM java_versions\n\t\tWHERE major = ?\n\t\tORDER BY version DESC\n\t\tLIMIT 1\n\t\t",
|
||||
"query": "\n\t\tSELECT absolute_path, major, version, vendor, os_arch, is_jdk as \"is_jdk: bool\"\n\t\tFROM java_versions\n\t\tWHERE major = ?\n\t\tORDER BY version DESC\n\t\tLIMIT 1\n\t\t",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
|
|
@ -57,6 +57,17 @@
|
|||
"name": "os_arch"
|
||||
}
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "is_jdk: bool",
|
||||
"ordinal": 5,
|
||||
"type_info": "Integer",
|
||||
"origin": {
|
||||
"Table": {
|
||||
"table": "java_versions",
|
||||
"name": "is_jdk"
|
||||
}
|
||||
}
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
|
|
@ -67,8 +78,9 @@
|
|||
false,
|
||||
false,
|
||||
false,
|
||||
false,
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "b153f762849649456dd97b6a5cd35fb7dafc3d5539441328e03442cea195483a"
|
||||
"hash": "33771b7003e64c8f19d15d663de773afa568d626c60a37bbc32cc932746ad49b"
|
||||
}
|
||||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"db_name": "SQLite",
|
||||
"query": "\n SELECT 1 FROM cluster_artifacts\n WHERE cluster_id = ? AND bundle_name = ? AND package_id = ?\n LIMIT 1\n ",
|
||||
"query": "\n SELECT 1 FROM cluster_artifacts\n WHERE cluster_id = ? AND bundle_name IS NOT NULL AND package_id = ?\n LIMIT 1\n ",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
|
|
@ -11,11 +11,11 @@
|
|||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Right": 3
|
||||
"Right": 2
|
||||
},
|
||||
"nullable": [
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "dc93cdd57e23e48f8e3c333db4ec396b0ee319adec619e5a7d147805573ae2b2"
|
||||
"hash": "ab6dcea84a9502d5f8396a207ae613a4c41eb729ca22249f15eb45e6c6030094"
|
||||
}
|
||||
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"db_name": "SQLite",
|
||||
"query": "\n SELECT absolute_path, major, version, vendor, os_arch FROM java_versions \n WHERE absolute_path = ?\n ",
|
||||
"query": "\n SELECT absolute_path, major, version, vendor, os_arch, is_jdk as \"is_jdk: bool\"\n FROM java_versions\n WHERE absolute_path = ?\n ",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
|
|
@ -57,6 +57,17 @@
|
|||
"name": "os_arch"
|
||||
}
|
||||
}
|
||||
},
|
||||
{
|
||||
"name": "is_jdk: bool",
|
||||
"ordinal": 5,
|
||||
"type_info": "Integer",
|
||||
"origin": {
|
||||
"Table": {
|
||||
"table": "java_versions",
|
||||
"name": "is_jdk"
|
||||
}
|
||||
}
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
|
|
@ -67,8 +78,9 @@
|
|||
false,
|
||||
false,
|
||||
false,
|
||||
false,
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "c570e309195b6063cf0fbee866d68972de05107769dab983524fd2e74a7ae31a"
|
||||
"hash": "f5992c9cec6a6d7015cda4c04b463bd5fd9c42ac38b434fa8a0c0fe4f7517cf2"
|
||||
}
|
||||
|
|
@ -0,0 +1,23 @@
|
|||
-- Switches bundle content back on where it was left off with nothing recording
|
||||
-- the choice.
|
||||
--
|
||||
-- `cluster_artifacts.enabled` and `cluster_bundle_overrides` are two records of
|
||||
-- one decision and they were able to come apart: duplicate resolution wrote the
|
||||
-- flag without the override, and re-resolving a package to a different bundle
|
||||
-- deleted the override without the flag. Either way the artifact ends up off
|
||||
-- with no override, which nothing could then repair -- the update flow decides
|
||||
-- from the override and the manifest default, never from `enabled`, so it read
|
||||
-- the package as installed and enabled and never touched it again. For a
|
||||
-- bundle's hidden dependencies that meant a library the pack needs never
|
||||
-- reaching `mods/`.
|
||||
--
|
||||
-- A genuine user disable always leaves a `disabled` or `removed` row for the
|
||||
-- same bundle, so those are matched here and stay off.
|
||||
UPDATE cluster_artifacts SET enabled = 1
|
||||
WHERE bundle_name IS NOT NULL
|
||||
AND package_id IS NOT NULL
|
||||
AND enabled = 0
|
||||
AND NOT EXISTS (SELECT 1 FROM cluster_bundle_overrides o
|
||||
WHERE o.cluster_id = cluster_artifacts.cluster_id
|
||||
AND o.bundle_name = cluster_artifacts.bundle_name
|
||||
AND o.package_id = cluster_artifacts.package_id);
|
||||
|
|
@ -96,20 +96,18 @@ pub async fn get_bundle_tracked(
|
|||
.await
|
||||
}
|
||||
|
||||
pub async fn has_bundle_mapping(
|
||||
pub async fn has_bundle_mapping_for_package(
|
||||
pool: &SqlitePool,
|
||||
cluster_id: i64,
|
||||
bundle_name: &str,
|
||||
package_id: &str,
|
||||
) -> Result<bool, sqlx::Error> {
|
||||
let row = sqlx::query_scalar!(
|
||||
r#"
|
||||
SELECT 1 FROM cluster_artifacts
|
||||
WHERE cluster_id = ? AND bundle_name = ? AND package_id = ?
|
||||
WHERE cluster_id = ? AND bundle_name IS NOT NULL AND package_id = ?
|
||||
LIMIT 1
|
||||
"#,
|
||||
cluster_id,
|
||||
bundle_name,
|
||||
package_id
|
||||
)
|
||||
.fetch_optional(pool)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue