diff --git a/packages/oneclient_content/src/bundles/updates.rs b/packages/oneclient_content/src/bundles/updates.rs index 632722b1..cf25db9b 100644 --- a/packages/oneclient_content/src/bundles/updates.rs +++ b/packages/oneclient_content/src/bundles/updates.rs @@ -295,6 +295,15 @@ async fn check_bundle_updates_inner( ) }); + let addition_eligible_bundles = addition_eligible_bundles( + ctx, + &bundle_packages, + &all_linked, + &candidate_keys_by_bundle, + overrides, + ) + .await?; + let mut additions_available = Vec::new(); let mut planned_addition_keys = all_installed_managed_keys.clone(); for hash in all_installed_external_hashes { @@ -302,7 +311,7 @@ async fn check_bundle_updates_inner( } for archive in &archives { - if !subscribed_bundles.contains(&archive.manifest.name) { + if !addition_eligible_bundles.contains(&archive.manifest.name) { continue; } @@ -771,10 +780,55 @@ fn bundle_package_key( } } +/// Bundles that may still take on files the catalog has newly added. +/// +/// Subscription is deliberately looser than this: a bundle whose mods are all +/// installed-but-disabled still wants version updates and still owns its files. +/// New content is different. Removing or disabling everything a bundle ships is +/// how a user opts out of that bundle, and an absent per-file override is not +/// consent for a file that did not exist when they opted out — so gating +/// additions on subscription alone let one catalog addition quietly reinstate a +/// bundle the user had emptied. +/// +/// A bundle qualifies while any of it is still live here: a tracked artifact +/// that is enabled, an enabled artifact that can only have come from this bundle +/// (the untracked/migrated case), or an explicit opt-in override. #[tracing::instrument(level = "debug", skip_all)] -async fn installed_bundle_keys( +async fn addition_eligible_bundles( ctx: &ContentCtx, - linked: &[LinkedArtifactInfo], + bundle_packages: &[BundleTrackedArtifactRow], + all_linked: &[LinkedArtifactInfo], + candidate_keys_by_bundle: &HashMap>, + overrides: &[ClusterBundleOverrideRow], +) -> ContentResult> { + let (live_managed_keys, _) = + installed_bundle_keys(ctx, all_linked.iter().filter(|item| item.enabled)).await?; + + let mut eligible: HashSet = bundle_packages + .iter() + .filter(|bp| bp.enabled != 0) + .filter_map(|bp| bp.bundle_name.clone()) + .collect(); + + eligible.extend(infer_bundle_names_from_unique_installed_keys( + candidate_keys_by_bundle, + &live_managed_keys, + )); + + eligible.extend( + overrides + .iter() + .filter(|o| OverrideType::parse(&o.override_type) == Some(OverrideType::Enabled)) + .map(|o| o.bundle_name.clone()), + ); + + Ok(eligible) +} + +#[tracing::instrument(level = "debug", skip_all)] +async fn installed_bundle_keys<'a>( + ctx: &ContentCtx, + linked: impl IntoIterator, ) -> ContentResult<(HashSet, HashSet)> { let mut managed = HashSet::new(); let mut external = HashSet::new(); diff --git a/packages/oneclient_core/tests/bundle_updates.rs b/packages/oneclient_core/tests/bundle_updates.rs index 41de3b2d..b4dc3d49 100644 --- a/packages/oneclient_core/tests/bundle_updates.rs +++ b/packages/oneclient_core/tests/bundle_updates.rs @@ -26,6 +26,23 @@ fn managed_file(enabled: bool) -> BundleFile { } } +/// A file the catalog added after the cluster was last synced: nothing on the +/// cluster tracks it, so it has no override either. +fn newly_shipped_file() -> BundleFile { + BundleFile { + enabled: true, + hidden: false, + path: "mods/newcomer.jar".to_string(), + size: 1, + kind: BundleFileKind::Managed { + provider: ProviderId::Modrinth, + project_id: "newcomer".to_string(), + version_id: "v1".to_string(), + sha1: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(), + }, + } +} + fn manifest(files: Vec) -> BundleManifest { BundleManifest { name: BUNDLE.to_string(), @@ -196,3 +213,150 @@ async fn user_disabled_mod_is_not_treated_as_a_removal() { check.removals_available ); } + +#[tokio::test] +async fn live_bundle_takes_on_new_catalog_files() { + let state = oneclient_core::dev::ephemeral_state().await.unwrap(); + oneclient_core::dev::seed_bundle_archive( + &state, + manifest(vec![managed_file(true), newly_shipped_file()]), + ) + .await + .unwrap(); + let cluster_id = cluster_with_tracked_mod(&state).await; + + let check = check_bundle_updates(cluster_id, state.bundles.as_ref(), &state.services.content()) + .await + .unwrap(); + + assert_eq!( + check.additions_available.len(), + 1, + "a bundle the user still has enabled should pick up newly shipped files" + ); + assert_eq!( + check.additions_available[0].new_file.kind.package_id(), + "newcomer" + ); +} + +#[tokio::test] +async fn emptied_bundle_does_not_take_on_new_catalog_files() { + let state = oneclient_core::dev::ephemeral_state().await.unwrap(); + oneclient_core::dev::seed_bundle_archive( + &state, + manifest(vec![managed_file(true), newly_shipped_file()]), + ) + .await + .unwrap(); + let cluster_id = cluster_with_tracked_mod(&state).await; + + // The user disabled everything this bundle shipped, which is how you opt out + // of it. The newcomer has no override of its own precisely because it did not + // exist yet, and that absence must not read as consent. + artifact_dao::update_cluster_artifact(&state.services.db, cluster_id, HASH, "sodium.jar", 0) + .await + .unwrap(); + bundle_dao::save_override( + &state.services.db, + cluster_id, + BUNDLE, + PROJECT_ID, + OverrideType::Disabled, + ) + .await + .unwrap(); + + let check = check_bundle_updates(cluster_id, state.bundles.as_ref(), &state.services.content()) + .await + .unwrap(); + + assert!( + check.additions_available.is_empty(), + "a bundle whose content the user disabled must not reinstate itself: {:?}", + check + .additions_available + .iter() + .map(|a| a.new_file.kind.package_id()) + .collect::>() + ); +} + +#[tokio::test] +async fn removed_bundle_content_does_not_take_on_new_catalog_files() { + let state = oneclient_core::dev::ephemeral_state().await.unwrap(); + oneclient_core::dev::seed_bundle_archive( + &state, + manifest(vec![managed_file(true), newly_shipped_file()]), + ) + .await + .unwrap(); + let cluster_id = cluster_with_tracked_mod(&state).await; + + oneclient_content::bundles::remove_artifact_from_cluster( + cluster_id, + HASH, + true, + &state.services.content(), + ) + .await + .unwrap(); + + let check = check_bundle_updates(cluster_id, state.bundles.as_ref(), &state.services.content()) + .await + .unwrap(); + + assert!( + check.additions_available.is_empty(), + "a bundle the user emptied by removal must not reinstate itself: {:?}", + check + .additions_available + .iter() + .map(|a| a.new_file.kind.package_id()) + .collect::>() + ); +} + +#[tokio::test] +async fn opting_a_single_file_in_keeps_the_bundle_live() { + let state = oneclient_core::dev::ephemeral_state().await.unwrap(); + oneclient_core::dev::seed_bundle_archive( + &state, + manifest(vec![managed_file(true), newly_shipped_file()]), + ) + .await + .unwrap(); + let cluster_id = cluster_with_tracked_mod(&state).await; + + artifact_dao::update_cluster_artifact(&state.services.db, cluster_id, HASH, "sodium.jar", 0) + .await + .unwrap(); + bundle_dao::save_override( + &state.services.db, + cluster_id, + BUNDLE, + PROJECT_ID, + OverrideType::Disabled, + ) + .await + .unwrap(); + bundle_dao::save_override( + &state.services.db, + cluster_id, + BUNDLE, + "newcomer", + OverrideType::Enabled, + ) + .await + .unwrap(); + + let check = check_bundle_updates(cluster_id, state.bundles.as_ref(), &state.services.content()) + .await + .unwrap(); + + assert_eq!( + check.additions_available.len(), + 1, + "an explicit opt-in is consent, even with the rest of the bundle disabled" + ); +}