From 31e72dab8cb94d45aaf39c26c33c3c6717cbd159 Mon Sep 17 00:00:00 2001 From: "Calum H. (IMB11)" Date: Fri, 4 Sep 2026 17:31:47 +0100 Subject: [PATCH 1/3] feat(modrinth-content-management): content set diff api --- packages/app-lib/src/api/instance.rs | 1 - .../src/api/instance/content_set_diff.rs | 152 ---- .../app-lib/src/api/instance/shared/diff.rs | 676 ++++++++---------- .../app-lib/src/api/instance/shared/mod.rs | 9 +- .../src/api/instance/shared/publish.rs | 64 +- .../src/diff/configuration.rs | 54 ++ .../src/diff/mod.rs | 47 ++ .../src/diff/model.rs | 168 +++++ .../src/{install.rs => install/mod.rs} | 17 +- .../src/{ => install}/model.rs | 28 +- .../{provider/mod.rs => install/provider.rs} | 3 +- .../modrinth-content-management/src/lib.rs | 22 +- .../src/shared/mod.rs | 70 ++ .../modrinth-content-management/tests/diff.rs | 399 +++++++++++ 14 files changed, 1109 insertions(+), 601 deletions(-) delete mode 100644 packages/app-lib/src/api/instance/content_set_diff.rs create mode 100644 packages/modrinth-content-management/src/diff/configuration.rs create mode 100644 packages/modrinth-content-management/src/diff/mod.rs create mode 100644 packages/modrinth-content-management/src/diff/model.rs rename packages/modrinth-content-management/src/{install.rs => install/mod.rs} (96%) rename packages/modrinth-content-management/src/{ => install}/model.rs (76%) rename packages/modrinth-content-management/src/{provider/mod.rs => install/provider.rs} (85%) create mode 100644 packages/modrinth-content-management/src/shared/mod.rs create mode 100644 packages/modrinth-content-management/tests/diff.rs diff --git a/packages/app-lib/src/api/instance.rs b/packages/app-lib/src/api/instance.rs index 1b5f1baf68..d4155b21df 100644 --- a/packages/app-lib/src/api/instance.rs +++ b/packages/app-lib/src/api/instance.rs @@ -1,7 +1,6 @@ //! Theseus instance management interface mod content; -mod content_set_diff; mod export_mrpack; mod get; mod groups; diff --git a/packages/app-lib/src/api/instance/content_set_diff.rs b/packages/app-lib/src/api/instance/content_set_diff.rs deleted file mode 100644 index 57d1e9c178..0000000000 --- a/packages/app-lib/src/api/instance/content_set_diff.rs +++ /dev/null @@ -1,152 +0,0 @@ -use crate::state::Version; -use std::collections::{HashMap, HashSet}; - -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub(crate) enum ContentSetDiffKind { - Added, - Removed, - Updated, -} - -#[derive(Clone, Debug)] -pub(crate) enum ContentSetDiffEntry { - Project { - kind: ContentSetDiffKind, - project_id: String, - current_version_name: Option, - new_version_name: Option, - disabled: bool, - }, - ExternalFile { - kind: ContentSetDiffKind, - file_name: String, - disabled: bool, - }, -} - -#[derive(Clone, Debug, Default)] -pub(crate) struct ContentSetDiffOptions { - pub removed_disabled_project_ids: HashSet, - pub removed_disabled_external_files: HashSet, - pub common_external_files_are_updated: bool, -} - -#[derive(Clone, Debug, Default)] -pub(crate) struct ContentSetSnapshot { - pub versions: Vec, - pub external_files: HashSet, -} - -#[derive(Clone, Debug)] -pub(crate) struct ContentSetSnapshotVersion { - pub project_id: String, - pub version_id: String, - pub version_name: String, -} - -impl From for ContentSetSnapshotVersion { - fn from(version: Version) -> Self { - Self { - project_id: version.project_id, - version_id: version.id, - version_name: version.version_number, - } - } -} - -pub(crate) fn diff_content_sets( - current: &ContentSetSnapshot, - latest: &ContentSetSnapshot, - options: &ContentSetDiffOptions, -) -> Vec { - let current_versions = versions_by_project(current); - let latest_versions = versions_by_project(latest); - let project_ids = current_versions - .keys() - .chain(latest_versions.keys()) - .copied() - .collect::>(); - - let mut diffs = Vec::new(); - for project_id in project_ids { - let current = current_versions.get(project_id); - let latest = latest_versions.get(project_id); - - match (current, latest) { - (None, Some(latest)) => { - diffs.push(ContentSetDiffEntry::Project { - kind: ContentSetDiffKind::Added, - project_id: project_id.to_string(), - current_version_name: None, - new_version_name: Some(latest.version_name.clone()), - disabled: false, - }); - } - (Some(current), None) => { - diffs.push(ContentSetDiffEntry::Project { - kind: ContentSetDiffKind::Removed, - project_id: project_id.to_string(), - current_version_name: Some(current.version_name.clone()), - new_version_name: None, - disabled: options - .removed_disabled_project_ids - .contains(project_id), - }); - } - (Some(current), Some(latest)) - if current.version_id != latest.version_id => - { - diffs.push(ContentSetDiffEntry::Project { - kind: ContentSetDiffKind::Updated, - project_id: project_id.to_string(), - current_version_name: Some(current.version_name.clone()), - new_version_name: Some(latest.version_name.clone()), - disabled: false, - }); - } - _ => {} - } - } - - for file_name in latest.external_files.difference(¤t.external_files) { - diffs.push(ContentSetDiffEntry::ExternalFile { - kind: ContentSetDiffKind::Added, - file_name: file_name.clone(), - disabled: false, - }); - } - - if options.common_external_files_are_updated { - for file_name in - latest.external_files.intersection(¤t.external_files) - { - diffs.push(ContentSetDiffEntry::ExternalFile { - kind: ContentSetDiffKind::Updated, - file_name: file_name.clone(), - disabled: false, - }); - } - } - - for file_name in current.external_files.difference(&latest.external_files) { - diffs.push(ContentSetDiffEntry::ExternalFile { - kind: ContentSetDiffKind::Removed, - file_name: file_name.clone(), - disabled: options - .removed_disabled_external_files - .contains(file_name), - }); - } - - diffs -} - -fn versions_by_project( - snapshot: &ContentSetSnapshot, -) -> HashMap<&str, &ContentSetSnapshotVersion> { - snapshot - .versions - .iter() - .map(|version| (version.project_id.as_str(), version)) - .collect() -} diff --git a/packages/app-lib/src/api/instance/shared/diff.rs b/packages/app-lib/src/api/instance/shared/diff.rs index 072c9cb661..c8366755f9 100644 --- a/packages/app-lib/src/api/instance/shared/diff.rs +++ b/packages/app-lib/src/api/instance/shared/diff.rs @@ -3,430 +3,334 @@ use super::publish::*; use super::types::*; use super::*; +struct SharedContentSnapshot { + version_ids: Vec, + external_files: BTreeSet, + configuration: ContentSetConfiguration, +} + pub(super) async fn shared_instance_update_diffs( - metadata: &crate::state::InstanceMetadata, - version: &InstanceVersionResponse, - state: &State, + metadata: &crate::state::InstanceMetadata, + version: &InstanceVersionResponse, + state: &State, ) -> crate::Result> { - let remote_modpack_id = - version.modpack_id.as_deref().filter(|id| !id.is_empty()); - let current_modpack_id = shared_modpack_id(&metadata.link); - let modpack_unlinked = - current_modpack_id.is_some() && remote_modpack_id.is_none(); - let (current_version_ids, current_external_files) = - current_shared_content(metadata, modpack_unlinked, state).await?; - let (latest_version_ids, latest_external_files) = - remote_shared_content(version); - let removed_disabled_project_ids = HashSet::new(); - let removed_disabled_external_files = HashSet::new(); - let mut diffs = shared_content_diffs( - ¤t_version_ids, - ¤t_external_files, - &latest_version_ids, - &latest_external_files, - &removed_disabled_project_ids, - &removed_disabled_external_files, - true, - state, - ) - .await?; - let mut configuration_diffs = shared_instance_configuration_diffs( - current_modpack_id.as_deref(), - remote_modpack_id, - &metadata.applied_content_set.game_version, - &version.game_version, - metadata.applied_content_set.loader, - version.loader, - metadata.applied_content_set.loader_version.as_deref(), - Some(version.loader_version.as_str()), - state, - ) - .await?; - configuration_diffs.append(&mut diffs); + let before_configuration = local_configuration(metadata); + let after_configuration = remote_configuration(version); + let modpack_unlinked = before_configuration.modpack_version_id.is_some() + && after_configuration.modpack_version_id.is_none(); + let (version_ids, external_files) = current_shared_content(metadata, modpack_unlinked, state).await?; + let before = SharedContentSnapshot { version_ids, external_files, configuration: before_configuration }; + let (version_ids, external_files) = remote_shared_content(version)?; + let after = SharedContentSnapshot { version_ids, external_files, configuration: after_configuration }; - Ok(configuration_diffs) + shared_content_diffs( + &before, + &after, + &HashSet::new(), + &BTreeSet::new(), + CommonExternalFilePolicy::AssumeUpdated, + state, + ).await } pub(super) async fn shared_instance_publish_diffs( - metadata: &crate::state::InstanceMetadata, - version: &InstanceVersionResponse, - snapshot: &CurrentPublishSnapshot, - state: &State, + metadata: &crate::state::InstanceMetadata, + version: &InstanceVersionResponse, + snapshot: &CurrentPublishSnapshot, + state: &State, ) -> crate::Result> { - let remote_modpack_id = - version.modpack_id.as_deref().filter(|id| !id.is_empty()); - let current_modpack_id = shared_modpack_id(&metadata.link); - let modpack_unlinked = - remote_modpack_id.is_some() && current_modpack_id.is_none(); - let disabled_versions = async { - if snapshot.disabled_version_ids.is_empty() { - Ok(HashMap::new()) - } else { - shared_versions_by_project(&snapshot.disabled_version_ids, state) - .await - } - }; - let ((latest_version_ids, latest_external_files), disabled_versions) = tokio::try_join!( - remote_publish_content(version, modpack_unlinked, state), - disabled_versions, - )?; - let current_external_files = snapshot - .external_files - .iter() - .map(|file| file.file_name.clone()) - .collect::>(); - let current_version_ids = snapshot - .version_ids - .iter() - .filter(|id| current_modpack_id.as_deref() != Some(id.as_str())) - .cloned() - .collect::>(); - let mut removed_disabled_project_ids = - snapshot.disabled_project_ids.clone(); - removed_disabled_project_ids.extend(disabled_versions.into_keys()); - let mut diffs = shared_content_diffs( - &latest_version_ids, - &latest_external_files, - ¤t_version_ids, - ¤t_external_files, - &removed_disabled_project_ids, - &snapshot.disabled_external_files, - false, - state, - ) - .await?; - let mut configuration_diffs = shared_instance_configuration_diffs( - remote_modpack_id, - current_modpack_id.as_deref(), - &version.game_version, - &metadata.applied_content_set.game_version, - version.loader, - metadata.applied_content_set.loader, - Some(version.loader_version.as_str()), - metadata.applied_content_set.loader_version.as_deref(), - state, - ) - .await?; - configuration_diffs.append(&mut diffs); + let before_configuration = remote_configuration(version); + let after_configuration = local_configuration(metadata); + let modpack_unlinked = before_configuration.modpack_version_id.is_some() + && after_configuration.modpack_version_id.is_none(); + let disabled_versions = async { + if snapshot.disabled_version_ids.is_empty() { + Ok(HashMap::new()) + } else { + shared_versions_by_project(&snapshot.disabled_version_ids, state).await + } + }; + let ((version_ids, external_files), disabled_versions) = tokio::try_join!( + remote_publish_content(version, modpack_unlinked, state), + disabled_versions, + )?; + let before = SharedContentSnapshot { version_ids, external_files, configuration: before_configuration }; + let after = SharedContentSnapshot { + version_ids: snapshot.version_ids.iter() + .filter(|id| after_configuration.modpack_version_id.as_deref() != Some(id.as_str())) + .cloned().collect(), + external_files: snapshot.external_files.iter() + .map(|file| shared_external_file_key(&file.file_type, &file.file_name)) + .collect::>()?, + configuration: after_configuration, + }; + let mut removed_disabled_project_ids = snapshot.disabled_project_ids.clone(); + removed_disabled_project_ids.extend(disabled_versions.into_keys()); - Ok(configuration_diffs) + shared_content_diffs( + &before, + &after, + &removed_disabled_project_ids, + &snapshot.disabled_external_files, + CommonExternalFilePolicy::AssumeUnchanged, + state, + ).await } -pub(super) async fn shared_instance_configuration_diffs( - current_modpack_id: Option<&str>, - new_modpack_id: Option<&str>, - current_game_version: &str, - new_game_version: &str, - current_loader: ModLoader, - new_loader: ModLoader, - current_loader_version: Option<&str>, - new_loader_version: Option<&str>, - state: &State, -) -> crate::Result> { - let mut diffs = Vec::new(); - - if current_modpack_id != new_modpack_id { - match (current_modpack_id, new_modpack_id) { - (None, Some(_)) => diffs.push(configuration_diff( - SharedInstanceUpdateDiffType::ModpackLinked, - None, - shared_modpack_version_label(new_modpack_id, state).await, - )), - (Some(_), None) => diffs.push(configuration_diff( - SharedInstanceUpdateDiffType::ModpackUnlinked, - shared_modpack_version_label(current_modpack_id, state).await, - None, - )), - (Some(current_modpack_id), Some(new_modpack_id)) => { - let current = - shared_modpack_version_details(current_modpack_id, state) - .await; - let new = - shared_modpack_version_details(new_modpack_id, state).await; - let project_name = new - .as_ref() - .and_then(|details| details.project_name.clone()) - .or_else(|| { - current - .as_ref() - .and_then(|details| details.project_name.clone()) - }); - - diffs.push(SharedInstanceUpdateDiff { - type_: SharedInstanceUpdateDiffType::ModpackUpdated, - project_id: None, - project_name, - file_name: None, - current_version_name: current - .map(|details| details.version_name), - new_version_name: new.map(|details| details.version_name), - config_file_count: None, - disabled: false, - }); - } - (None, None) => unreachable!(), - } - } - - if current_game_version != new_game_version { - diffs.push(configuration_diff( - SharedInstanceUpdateDiffType::GameVersionUpdated, - Some(current_game_version.to_string()), - Some(new_game_version.to_string()), - )); - } +fn local_configuration(metadata: &crate::state::InstanceMetadata) -> ContentSetConfiguration { + ContentSetConfiguration { + modpack_version_id: shared_modpack_id(&metadata.link).filter(|id| !id.is_empty()), + game_version: metadata.applied_content_set.game_version.clone(), + loader: LoaderReference { + name: metadata.applied_content_set.loader.as_str().to_string(), + version: metadata.applied_content_set.loader_version.clone(), + }, + } +} - let current_loader_version = - normalized_loader_version(current_loader_version); - let new_loader_version = normalized_loader_version(new_loader_version); - if current_loader != new_loader - || current_loader_version != new_loader_version - { - diffs.push(configuration_diff( - SharedInstanceUpdateDiffType::LoaderUpdated, - Some(shared_loader_label(current_loader, current_loader_version)), - Some(shared_loader_label(new_loader, new_loader_version)), - )); - } +fn remote_configuration(version: &InstanceVersionResponse) -> ContentSetConfiguration { + ContentSetConfiguration { + modpack_version_id: version.modpack_id.clone().filter(|id| !id.is_empty()), + game_version: version.game_version.clone(), + loader: LoaderReference { + name: version.loader.as_str().to_string(), + version: Some(version.loader_version.clone()), + }, + } +} - Ok(diffs) +async fn shared_configuration_diffs( + changes: Vec, + state: &State, +) -> Vec { + let mut diffs = Vec::new(); + for change in changes { + match change { + ConfigurationDiff::Modpack(Change::Added { after }) => diffs.push(configuration_diff( + SharedInstanceUpdateDiffType::ModpackLinked, + None, + shared_modpack_version_label(Some(&after), state).await, + )), + ConfigurationDiff::Modpack(Change::Removed { before }) => diffs.push(configuration_diff( + SharedInstanceUpdateDiffType::ModpackUnlinked, + shared_modpack_version_label(Some(&before), state).await, + None, + )), + ConfigurationDiff::Modpack(Change::Updated { before, after }) => { + let current = shared_modpack_version_details(&before, state).await; + let new = shared_modpack_version_details(&after, state).await; + let project_name = new.as_ref().and_then(|details| details.project_name.clone()) + .or_else(|| current.as_ref().and_then(|details| details.project_name.clone())); + diffs.push(SharedInstanceUpdateDiff { + type_: SharedInstanceUpdateDiffType::ModpackUpdated, + project_id: None, + project_name, + file_name: None, + current_version_name: current.map(|details| details.version_name), + new_version_name: new.map(|details| details.version_name), + config_file_count: None, + disabled: false, + }); + } + ConfigurationDiff::GameVersion(change) => diffs.push(configuration_diff( + SharedInstanceUpdateDiffType::GameVersionUpdated, + change.before().cloned(), + change.after().cloned(), + )), + ConfigurationDiff::Loader(change) => diffs.push(configuration_diff( + SharedInstanceUpdateDiffType::LoaderUpdated, + change.before().map(shared_loader_label), + change.after().map(shared_loader_label), + )), + } + } + diffs } pub(super) fn configuration_diff( - type_: SharedInstanceUpdateDiffType, - current_version_name: Option, - new_version_name: Option, + type_: SharedInstanceUpdateDiffType, + current_version_name: Option, + new_version_name: Option, ) -> SharedInstanceUpdateDiff { - SharedInstanceUpdateDiff { - type_, - project_id: None, - project_name: None, - file_name: None, - current_version_name, - new_version_name, - config_file_count: None, - disabled: false, - } + SharedInstanceUpdateDiff { + type_, + project_id: None, + project_name: None, + file_name: None, + current_version_name, + new_version_name, + config_file_count: None, + disabled: false, + } } pub(super) async fn shared_modpack_version_label( - version_id: Option<&str>, - state: &State, + version_id: Option<&str>, + state: &State, ) -> Option { - let version_id = version_id?; - let details = shared_modpack_version_details(version_id, state).await?; + let version_id = version_id?; + let details = shared_modpack_version_details(version_id, state).await?; - Some(match details.project_name { - Some(project_name) => { - format!("{project_name} {}", details.version_name) - } - None => details.version_name, - }) + Some(match details.project_name { + Some(project_name) => { + format!("{project_name} {}", details.version_name) + } + None => details.version_name, + }) } struct SharedModpackVersionDetails { - project_name: Option, - version_name: String, + project_name: Option, + version_name: String, } async fn shared_modpack_version_details( - version_id: &str, - state: &State, + version_id: &str, + state: &State, ) -> Option { - let Some(version) = CachedEntry::get_version( - version_id, - Some(CacheBehaviour::Bypass), - &state.pool, - &state.api_semaphore, - ) - .await - .ok() - .flatten() else { - return Some(SharedModpackVersionDetails { - project_name: None, - version_name: version_id.to_string(), - }); - }; - let project = CachedEntry::get_project( - &version.project_id, - Some(CacheBehaviour::Bypass), - &state.pool, - &state.api_semaphore, - ) - .await - .ok() - .flatten(); + let Some(version) = CachedEntry::get_version( + version_id, + Some(CacheBehaviour::Bypass), + &state.pool, + &state.api_semaphore, + ) + .await + .ok() + .flatten() else { + return Some(SharedModpackVersionDetails { + project_name: None, + version_name: version_id.to_string(), + }); + }; + let project = CachedEntry::get_project( + &version.project_id, + Some(CacheBehaviour::Bypass), + &state.pool, + &state.api_semaphore, + ) + .await + .ok() + .flatten(); - Some(SharedModpackVersionDetails { - project_name: Some( - project.map(|project| project.title).unwrap_or(version.name), - ), - version_name: version.version_number, - }) + Some(SharedModpackVersionDetails { + project_name: Some( + project.map(|project| project.title).unwrap_or(version.name), + ), + version_name: version.version_number, + }) } -pub(super) fn normalized_loader_version( - loader_version: Option<&str>, -) -> Option<&str> { - loader_version.filter(|version| !version.is_empty()) +fn shared_loader_label(loader: &LoaderReference) -> String { + let loader_name = match loader.name.as_str() { + "vanilla" => "Vanilla", + "forge" => "Forge", + "fabric" => "Fabric", + "quilt" => "Quilt", + "neoforge" => "NeoForge", + name => name, + }; + match loader.version.as_deref() { + Some(version) => format!("{loader_name} {version}"), + None => loader_name.to_string(), + } } -pub(super) fn shared_loader_label( - loader: ModLoader, - loader_version: Option<&str>, -) -> String { - let loader_name = match loader { - ModLoader::Vanilla => "Vanilla", - ModLoader::Forge => "Forge", - ModLoader::Fabric => "Fabric", - ModLoader::Quilt => "Quilt", - ModLoader::NeoForge => "NeoForge", - }; - - match loader_version { - Some(version) => format!("{loader_name} {version}"), - None => loader_name.to_string(), - } -} - -pub(super) async fn shared_content_diffs( - current_version_ids: &[String], - current_external_files: &HashSet, - latest_version_ids: &[String], - latest_external_files: &HashSet, - removed_disabled_project_ids: &HashSet, - removed_disabled_external_files: &HashSet, - common_external_files_are_updated: bool, - state: &State, +async fn shared_content_diffs( + before: &SharedContentSnapshot, + after: &SharedContentSnapshot, + removed_disabled_project_ids: &HashSet, + removed_disabled_external_files: &BTreeSet, + common_external_files: CommonExternalFilePolicy, + state: &State, ) -> crate::Result> { - let current = shared_content_snapshot( - current_version_ids, - current_external_files, - state, - ) - .await?; - let latest = shared_content_snapshot( - latest_version_ids, - latest_external_files, - state, - ) - .await?; - let options = ContentSetDiffOptions { - removed_disabled_project_ids: removed_disabled_project_ids.clone(), - removed_disabled_external_files: removed_disabled_external_files - .clone(), - common_external_files_are_updated, - }; - let content_diffs = diff_content_sets(¤t, &latest, &options); - let project_ids = content_diffs - .iter() - .filter_map(|diff| match diff { - ContentSetDiffEntry::Project { project_id, .. } => { - Some(project_id.clone()) - } - ContentSetDiffEntry::ExternalFile { .. } => None, - }) - .collect::>(); - let project_names = shared_project_names(&project_ids, state).await?; - - let mut diffs = Vec::new(); - for diff in content_diffs { - match diff { - ContentSetDiffEntry::Project { - kind, - project_id, - current_version_name, - new_version_name, - disabled, - } => { - let project_name = Some( - project_names - .get(&project_id) - .cloned() - .unwrap_or_else(|| project_id.clone()), - ); - diffs.push(SharedInstanceUpdateDiff { - type_: shared_update_diff_type(kind), - project_id: Some(project_id), - project_name, - file_name: None, - current_version_name, - new_version_name, - config_file_count: None, - disabled, - }); - } - ContentSetDiffEntry::ExternalFile { - kind, - file_name, - disabled, - } => { - diffs.push(SharedInstanceUpdateDiff { - type_: shared_update_diff_type(kind), - project_id: None, - project_name: None, - file_name: Some(file_name), - current_version_name: None, - new_version_name: None, - config_file_count: None, - disabled, - }); - } - } - } - - diffs.sort_by(|a, b| { - a.project_name - .as_deref() - .or(a.file_name.as_deref()) - .cmp(&b.project_name.as_deref().or(b.file_name.as_deref())) - }); - Ok(diffs) + let (before_versions, after_versions) = tokio::try_join!( + shared_versions_by_project(&before.version_ids, state), + shared_versions_by_project(&after.version_ids, state), + )?; + let to_snapshot = |source: &SharedContentSnapshot, versions: &HashMap| ContentSetSnapshot { + projects: versions.iter().map(|(project_id, version)| (project_id.clone(), version.id.clone())).collect(), + external_files: source.external_files.clone(), + }; + let diff = diff_content_sets( + &to_snapshot(before, &before_versions), + &to_snapshot(after, &after_versions), + &ContentSetDiffOptions { common_external_files }, + ).with_additional(diff_configuration(&before.configuration, &after.configuration)); + if !diff.has_changes() { + return Ok(Vec::new()); + } + let project_ids = diff.content.iter().filter_map(|entry| match entry { + ContentSetDiffEntry::Project { project_id, .. } => Some(project_id.clone()), + ContentSetDiffEntry::ExternalFile { .. } => None, + }).collect::>(); + let project_names = shared_project_names(&project_ids, state).await?; + let mut content_diffs = Vec::new(); + for entry in diff.content { + match entry { + ContentSetDiffEntry::Project { project_id, change } => { + let disabled = change.kind() == ContentSetDiffKind::Removed + && removed_disabled_project_ids.contains(&project_id); + content_diffs.push(SharedInstanceUpdateDiff { + type_: shared_update_diff_type(change.kind()), + project_name: Some(project_names.get(&project_id).cloned().unwrap_or_else(|| project_id.clone())), + current_version_name: change.before().map(|id| before_versions.get(&project_id) + .map(|version| version.version_number.clone()).unwrap_or_else(|| id.clone())), + new_version_name: change.after().map(|id| after_versions.get(&project_id) + .map(|version| version.version_number.clone()).unwrap_or_else(|| id.clone())), + project_id: Some(project_id), + file_name: None, + config_file_count: None, + disabled, + }); + } + ContentSetDiffEntry::ExternalFile { file, kind } => { + let disabled = kind == ContentSetDiffKind::Removed && removed_disabled_external_files.contains(&file); + content_diffs.push(SharedInstanceUpdateDiff { + type_: shared_update_diff_type(kind), + project_id: None, + project_name: None, + file_name: Some(file.path), + current_version_name: None, + new_version_name: None, + config_file_count: None, + disabled, + }); + } + } + } + content_diffs.sort_by(|a, b| a.project_name.as_deref().or(a.file_name.as_deref()) + .cmp(&b.project_name.as_deref().or(b.file_name.as_deref()))); + let mut diffs = shared_configuration_diffs(diff.additional, state).await; + diffs.extend(content_diffs); + Ok(diffs) } -pub(super) fn shared_update_diff_type( - kind: ContentSetDiffKind, -) -> SharedInstanceUpdateDiffType { - match kind { - ContentSetDiffKind::Added => SharedInstanceUpdateDiffType::Added, - ContentSetDiffKind::Removed => SharedInstanceUpdateDiffType::Removed, - ContentSetDiffKind::Updated => SharedInstanceUpdateDiffType::Updated, - } +fn shared_update_diff_type(kind: ContentSetDiffKind) -> SharedInstanceUpdateDiffType { + match kind { + ContentSetDiffKind::Added => SharedInstanceUpdateDiffType::Added, + ContentSetDiffKind::Removed => SharedInstanceUpdateDiffType::Removed, + ContentSetDiffKind::Updated => SharedInstanceUpdateDiffType::Updated, + } } -pub(super) async fn shared_content_snapshot( - version_ids: &[String], - external_files: &HashSet, - state: &State, -) -> crate::Result { - let versions = shared_versions_by_project(version_ids, state) - .await? - .into_values() - .map(ContentSetSnapshotVersion::from) - .collect(); - - Ok(ContentSetSnapshot { - versions, - external_files: external_files.clone(), - }) +pub(super) fn shared_external_file_key(file_type: &str, path: &str) -> crate::Result { + Ok(ExternalFileKey { + content_type: file_type.parse().map_err(|error: modrinth_content_management::Error| { + crate::ErrorKind::InputError(error.to_string()) + })?, + path: path.to_string(), + }) } -pub(super) fn remote_shared_content( - version: &InstanceVersionResponse, -) -> (Vec, HashSet) { - let mut version_ids = version.modrinth_ids.clone(); - if let Some(modpack_id) = version.modpack_id.as_deref() { - version_ids.retain(|id| id != modpack_id); - } - dedupe_strings(&mut version_ids); - - ( - version_ids, - version - .external_files - .iter() - .filter(|file| file.file_type != CONFIG_BUNDLE_FILE_TYPE) - .map(|file| file.file_name.clone()) - .collect(), - ) +fn remote_shared_content(version: &InstanceVersionResponse) -> crate::Result<(Vec, BTreeSet)> { + let mut version_ids = version.modrinth_ids.clone(); + if let Some(modpack_id) = version.modpack_id.as_deref() { + version_ids.retain(|id| id != modpack_id); + } + dedupe_strings(&mut version_ids); + let external_files = version.external_files.iter() + .filter(|file| file.file_type != CONFIG_BUNDLE_FILE_TYPE) + .map(|file| shared_external_file_key(&file.file_type, &file.file_name)) + .collect::>()?; + Ok((version_ids, external_files)) } diff --git a/packages/app-lib/src/api/instance/shared/mod.rs b/packages/app-lib/src/api/instance/shared/mod.rs index 7706a4f841..16e3f92adc 100644 --- a/packages/app-lib/src/api/instance/shared/mod.rs +++ b/packages/app-lib/src/api/instance/shared/mod.rs @@ -1,7 +1,10 @@ -use super::content_set_diff::{ - ContentSetDiffEntry, ContentSetDiffKind, ContentSetDiffOptions, - ContentSetSnapshot, ContentSetSnapshotVersion, diff_content_sets, +use modrinth_content_management::{ + Change, CommonExternalFilePolicy, ConfigurationDiff, ContentSetConfiguration, + ContentSetDiffEntry, ContentSetDiffKind, ContentSetDiffOptions, + ContentSetSnapshot, ExternalFileKey, LoaderReference, diff_configuration, + diff_content_sets, }; +use std::collections::BTreeSet; use crate::SharedInstanceUnavailableReason; use crate::event::InstancePayloadType; use crate::event::emit::emit_instance; diff --git a/packages/app-lib/src/api/instance/shared/publish.rs b/packages/app-lib/src/api/instance/shared/publish.rs index 74b737bd26..6575497e29 100644 --- a/packages/app-lib/src/api/instance/shared/publish.rs +++ b/packages/app-lib/src/api/instance/shared/publish.rs @@ -171,7 +171,7 @@ pub(super) async fn remote_publish_content( version: &InstanceVersionResponse, include_modpack_dependencies: bool, state: &State, -) -> crate::Result<(Vec, HashSet)> { +) -> crate::Result<(Vec, BTreeSet)> { let mut version_ids = version.modrinth_ids.clone(); if let Some(modpack_id) = version.modpack_id.as_deref().filter(|id| !id.is_empty()) @@ -179,9 +179,7 @@ pub(super) async fn remote_publish_content( version_ids.retain(|id| id != modpack_id); if include_modpack_dependencies { - version_ids.extend( - modpack_dependency_version_ids(modpack_id, state).await?, - ); + extend_shared_modpack_dependencies(&mut version_ids, modpack_id, state).await?; } } dedupe_strings(&mut version_ids); @@ -192,8 +190,8 @@ pub(super) async fn remote_publish_content( .external_files .iter() .filter(|file| file.file_type != CONFIG_BUNDLE_FILE_TYPE) - .map(|file| file.file_name.clone()) - .collect(), + .map(|file| shared_external_file_key(&file.file_type, &file.file_name)) + .collect::>()?, )) } @@ -221,6 +219,23 @@ pub(super) async fn modpack_dependency_version_ids( .collect()) } +/// Adds inherited pack versions without replacing explicitly selected projects. +async fn extend_shared_modpack_dependencies( + version_ids: &mut Vec, + modpack_id: &str, + state: &State, +) -> crate::Result<()> { + let dependency_ids = modpack_dependency_version_ids(modpack_id, state).await?; + let (explicit, inherited) = tokio::try_join!( + shared_versions_by_project(version_ids, state), + shared_versions_by_project(&dependency_ids, state), + )?; + version_ids.extend(inherited.into_values() + .filter(|version| !explicit.contains_key(&version.project_id)) + .map(|version| version.id)); + Ok(()) +} + pub(super) async fn shared_instance_install_modpack( version: &InstanceVersionResponse, state: &State, @@ -272,7 +287,7 @@ pub(super) async fn current_shared_content( metadata: &crate::state::InstanceMetadata, include_linked_modpack_content: bool, state: &State, -) -> crate::Result<(Vec, HashSet)> { +) -> crate::Result<(Vec, BTreeSet)> { let entries = crate::state::instances::adapters::sqlite::content_rows::get_content_entries( &metadata.applied_content_set.id, @@ -288,7 +303,7 @@ pub(super) async fn current_shared_content( .map(|file| (file.id.clone(), file)) .collect::>(); let mut version_ids = Vec::new(); - let mut external_files = HashSet::new(); + let mut external_files = BTreeSet::new(); for entry in entries { let include_entry = entry.source_kind @@ -309,14 +324,16 @@ pub(super) async fn current_shared_content( continue; }; if let Some(file) = files.get(&file_id) { - external_files.insert(file.file_name.clone()); + external_files.insert(ExternalFileKey { + content_type: entry.project_type.into(), + path: file.file_name.clone(), + }); } } if include_linked_modpack_content && let Some(modpack_id) = shared_modpack_id(&metadata.link) { - version_ids - .extend(modpack_dependency_version_ids(&modpack_id, state).await?); + extend_shared_modpack_dependencies(&mut version_ids, &modpack_id, state).await?; } dedupe_strings(&mut version_ids); @@ -328,7 +345,7 @@ pub(super) struct CurrentPublishSnapshot { pub(super) external_files: Vec, pub(super) disabled_project_ids: HashSet, pub(super) disabled_version_ids: Vec, - pub(super) disabled_external_files: HashSet, + pub(super) disabled_external_files: BTreeSet, pub(super) config_files: Vec, } @@ -370,7 +387,7 @@ pub(super) async fn collect_publish_snapshot( let mut disabled_project_ids = HashSet::new(); let mut disabled_version_ids = Vec::new(); let mut seen_disabled_version_ids = HashSet::new(); - let mut disabled_external_files = HashSet::new(); + let mut disabled_external_files = BTreeSet::new(); for item in items { if item.enabled { @@ -419,7 +436,10 @@ pub(super) async fn collect_publish_snapshot( continue; } - disabled_external_files.insert(enabled_file_name(&item.file_name)); + disabled_external_files.insert(ExternalFileKey { + content_type: item.project_type.into(), + path: enabled_file_name(&item.file_name), + }); } Ok(CurrentPublishSnapshot { @@ -446,10 +466,18 @@ pub(super) async fn shared_versions_by_project( ) .await?; - Ok(versions - .into_iter() - .map(|version| (version.project_id.clone(), version)) - .collect()) + let fetched_ids = versions.iter().map(|version| version.id.as_str()).collect::>(); + if let Some(missing) = version_ids.iter().find(|id| !fetched_ids.contains(id.as_str())) { + return Err(crate::ErrorKind::InputError(format!("Shared content version {missing} was not found")).into()); + } + let mut snapshot = ContentSetSnapshot::default(); + let mut by_project = HashMap::new(); + for version in versions { + snapshot.insert_project(version.project_id.clone(), version.id.clone()) + .map_err(|error| crate::ErrorKind::InputError(error.to_string()))?; + by_project.insert(version.project_id.clone(), version); + } + Ok(by_project) } pub(super) async fn shared_project_names( diff --git a/packages/modrinth-content-management/src/diff/configuration.rs b/packages/modrinth-content-management/src/diff/configuration.rs new file mode 100644 index 0000000000..583a72cc6e --- /dev/null +++ b/packages/modrinth-content-management/src/diff/configuration.rs @@ -0,0 +1,54 @@ +use serde::{Deserialize, Serialize}; + +use crate::diff::Change; + +/// Shared configuration independent of any application's loader or link model. +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +pub struct ContentSetConfiguration { + pub modpack_version_id: Option, + pub game_version: String, + pub loader: LoaderReference, +} + +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +pub struct LoaderReference { + pub name: String, + pub version: Option, +} + +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +#[serde(tag = "type", content = "change", rename_all = "snake_case")] +pub enum ConfigurationDiff { + Modpack(Change), + GameVersion(Change), + Loader(Change), +} + +/// Compares modpack, Minecraft and loader configuration in that order. +/// Empty optional version IDs are equivalent to absent IDs. +pub fn diff_configuration( + before: &ContentSetConfiguration, + after: &ContentSetConfiguration, +) -> Vec { + let mut changes = Vec::new(); + if let Some(change) = Change::between( + before.modpack_version_id.as_ref().filter(|id| !id.is_empty()), + after.modpack_version_id.as_ref().filter(|id| !id.is_empty()), + ) { + changes.push(ConfigurationDiff::Modpack(change)); + } + if let Some(change) = Change::between(Some(&before.game_version), Some(&after.game_version)) { + changes.push(ConfigurationDiff::GameVersion(change)); + } + let normalize_loader = |loader: &LoaderReference| LoaderReference { + name: loader.name.to_lowercase(), + version: loader.version.clone().filter(|version| !version.is_empty()), + }; + if let Some(change) = Change::between( + Some(&normalize_loader(&before.loader)), + Some(&normalize_loader(&after.loader)), + ) { + changes.push(ConfigurationDiff::Loader(change)); + } + changes +} diff --git a/packages/modrinth-content-management/src/diff/mod.rs b/packages/modrinth-content-management/src/diff/mod.rs new file mode 100644 index 0000000000..b819edd928 --- /dev/null +++ b/packages/modrinth-content-management/src/diff/mod.rs @@ -0,0 +1,47 @@ +//! Compares prepared content sets and composes caller-specific changes. + +use std::collections::BTreeSet; + +pub use configuration::{ + ConfigurationDiff, ContentSetConfiguration, LoaderReference, + diff_configuration, +}; +pub use model::{ + Change, CommonExternalFilePolicy, ContentSetDiff, ContentSetDiffEntry, + ContentSetDiffKind, ContentSetDiffOptions, ContentSetSnapshot, + ExternalFileKey, +}; + +/// Computes changes from `before` to `after`, ordered by project and file identity. +/// +/// Callers select the publishing or installation scope before comparison. +/// External files are compared by identity, using the supplied policy for common files. +pub fn diff_content_sets( + before: &ContentSetSnapshot, + after: &ContentSetSnapshot, + options: &ContentSetDiffOptions, +) -> ContentSetDiff { + let mut content = Vec::new(); + let project_ids = before.projects.keys().chain(after.projects.keys()).collect::>(); + for project_id in project_ids { + if let Some(change) = Change::between(before.projects.get(project_id), after.projects.get(project_id)) { + content.push(ContentSetDiffEntry::Project { + project_id: project_id.clone(), + change, + }); + } + } + for file in before.external_files.union(&after.external_files) { + let kind = match (before.external_files.contains(file), after.external_files.contains(file)) { + (false, true) => ContentSetDiffKind::Added, + (true, false) => ContentSetDiffKind::Removed, + (true, true) if options.common_external_files == CommonExternalFilePolicy::AssumeUpdated => ContentSetDiffKind::Updated, + _ => continue, + }; + content.push(ContentSetDiffEntry::ExternalFile { file: file.clone(), kind }); + } + ContentSetDiff { content, additional: Vec::new() } +} + +mod configuration; +mod model; diff --git a/packages/modrinth-content-management/src/diff/model.rs b/packages/modrinth-content-management/src/diff/model.rs new file mode 100644 index 0000000000..9f7e10687b --- /dev/null +++ b/packages/modrinth-content-management/src/diff/model.rs @@ -0,0 +1,168 @@ +use std::collections::{BTreeMap, BTreeSet}; +use std::convert::Infallible; + +use serde::{Deserialize, Serialize}; + +use crate::shared::{ContentType, Error}; + +/// A caller-prepared set of content in the scope being compared. +#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] +pub struct ContentSetSnapshot { + /// Modrinth project IDs mapped to version IDs. + pub projects: BTreeMap, + pub external_files: BTreeSet, +} + +impl ContentSetSnapshot { + /// Adds a project, rejecting conflicting versions without replacing it. + pub fn insert_project( + &mut self, + project_id: String, + version_id: String, + ) -> Result<(), Error> { + if let Some(existing) = self.projects.get(&project_id) { + if existing != &version_id { + return Err(Error::ConflictingProjectVersions { + project_id, + before: existing.clone(), + after: version_id, + }); + } + } else { + self.projects.insert(project_id, version_id); + } + Ok(()) + } +} + +/// Identifies a file by its content type and path within that type's directory. +#[derive(Clone, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] +pub struct ExternalFileKey { + pub content_type: ContentType, + pub path: String, +} + +#[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum ContentSetDiffKind { + Added, + Removed, + Updated, +} + +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +#[serde(tag = "kind", rename_all = "snake_case")] +pub enum Change { + Added { after: T }, + Removed { before: T }, + Updated { before: T, after: T }, +} + +impl Change { + /// Transforms the values while preserving the classification of the change. + pub fn map(self, mut map: impl FnMut(T) -> U) -> Change { + match self { + Self::Added { after } => Change::Added { after: map(after) }, + Self::Removed { before } => Change::Removed { before: map(before) }, + Self::Updated { before, after } => Change::Updated { + before: map(before), + after: map(after), + }, + } + } + + /// Returns the classification of this change. + pub fn kind(&self) -> ContentSetDiffKind { + match self { + Self::Added { .. } => ContentSetDiffKind::Added, + Self::Removed { .. } => ContentSetDiffKind::Removed, + Self::Updated { .. } => ContentSetDiffKind::Updated, + } + } + + /// Returns the value before the change, if it existed. + pub fn before(&self) -> Option<&T> { + match self { + Self::Removed { before } | Self::Updated { before, .. } => Some(before), + Self::Added { .. } => None, + } + } + + /// Returns the value after the change, if it exists. + pub fn after(&self) -> Option<&T> { + match self { + Self::Added { after } | Self::Updated { after, .. } => Some(after), + Self::Removed { .. } => None, + } + } +} + +impl Change { + /// Compares optional values, returning no entry when they are equal. + pub fn between(before: Option<&T>, after: Option<&T>) -> Option { + match (before, after) { + (None, Some(after)) => Some(Self::Added { after: after.clone() }), + (Some(before), None) => Some(Self::Removed { before: before.clone() }), + (Some(before), Some(after)) if before != after => Some(Self::Updated { + before: before.clone(), + after: after.clone(), + }), + _ => None, + } + } +} + +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +#[serde(tag = "type", rename_all = "snake_case")] +pub enum ContentSetDiffEntry { + Project { + project_id: String, + change: Change, + }, + ExternalFile { + file: ExternalFileKey, + kind: ContentSetDiffKind, + }, +} + +/// Policy for files present on both sides when their bytes are not compared. +#[derive(Clone, Copy, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum CommonExternalFilePolicy { + #[default] + AssumeUnchanged, + AssumeUpdated, +} + +#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] +pub struct ContentSetDiffOptions { + pub common_external_files: CommonExternalFilePolicy, +} + +/// Content changes and typed entries contributed by the caller. +#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] +pub struct ContentSetDiff { + pub content: Vec, + pub additional: Vec, +} + +impl ContentSetDiff { + /// Includes both content changes and caller-provided changes. + pub fn has_changes(&self) -> bool { + !self.content.is_empty() || !self.additional.is_empty() + } +} + +impl ContentSetDiff { + /// Attaches typed changes without coupling the comparator to the caller. + pub fn with_additional( + self, + entries: impl IntoIterator, + ) -> ContentSetDiff { + ContentSetDiff { + content: self.content, + additional: entries.into_iter().collect(), + } + } +} + diff --git a/packages/modrinth-content-management/src/install.rs b/packages/modrinth-content-management/src/install/mod.rs similarity index 96% rename from packages/modrinth-content-management/src/install.rs rename to packages/modrinth-content-management/src/install/mod.rs index d6b97c07e2..a259dce472 100644 --- a/packages/modrinth-content-management/src/install.rs +++ b/packages/modrinth-content-management/src/install/mod.rs @@ -1,12 +1,16 @@ +//! Resolves requested content and its required dependencies through a metadata provider. + use std::cmp::Reverse; use std::collections::{HashMap, HashSet}; -use crate::model::{ - ContentType, Dependency, DependencyType, Error, ResolutionPreferences, - ResolveContentPlan, ResolveContentRequest, ResolvedContent, SkippedContent, - SkippedReason, Version, +use crate::shared::{ContentType, Error}; + +pub use model::{ + Dependency, DependencyType, ResolutionPreferences, ResolveContentPlan, + ResolveContentRequest, ResolvedContent, SkippedContent, SkippedReason, + Version, }; -use crate::provider::ContentMetadataProvider; +pub use provider::ContentMetadataProvider; // Skip Fabric API if you're installing a fabric project onto a quilt instance. const QUILT_FABRIC_API_EXCEPTION_PROJECT_ID: &str = "P7dR8mSH"; @@ -352,3 +356,6 @@ fn should_skip_quilt_fabric_api( .iter() .any(|loader| loaders_match(loader, "quilt")) } + +mod model; +mod provider; diff --git a/packages/modrinth-content-management/src/model.rs b/packages/modrinth-content-management/src/install/model.rs similarity index 76% rename from packages/modrinth-content-management/src/model.rs rename to packages/modrinth-content-management/src/install/model.rs index 10f54d991d..ef27d5f394 100644 --- a/packages/modrinth-content-management/src/model.rs +++ b/packages/modrinth-content-management/src/install/model.rs @@ -1,33 +1,7 @@ use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; -#[derive(thiserror::Error, Debug)] -pub enum Error { - #[error("metadata provider error: {0}")] - Provider(String), - #[error("project `{0}` was not found")] - ProjectNotFound(String), - #[error("version `{0}` was not found")] - VersionNotFound(String), - #[error("version `{version_id}` does not belong to project `{project_id}`")] - VersionProjectMismatch { - version_id: String, - project_id: String, - }, - #[error("no compatible version was found for project `{0}`")] - NoCompatibleVersion(String), -} - -#[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)] -#[serde(rename_all = "lowercase")] -pub enum ContentType { - Mod, - Plugin, - DataPack, - ResourcePack, - Shader, - ModPack, -} +use crate::shared::ContentType; #[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] pub struct ResolutionPreferences { diff --git a/packages/modrinth-content-management/src/provider/mod.rs b/packages/modrinth-content-management/src/install/provider.rs similarity index 85% rename from packages/modrinth-content-management/src/provider/mod.rs rename to packages/modrinth-content-management/src/install/provider.rs index 410fdb677a..145e888955 100644 --- a/packages/modrinth-content-management/src/provider/mod.rs +++ b/packages/modrinth-content-management/src/install/provider.rs @@ -1,6 +1,7 @@ use async_trait::async_trait; -use crate::model::{Error, Version}; +use crate::install::Version; +use crate::shared::Error; #[async_trait] pub trait ContentMetadataProvider: Send + Sync { diff --git a/packages/modrinth-content-management/src/lib.rs b/packages/modrinth-content-management/src/lib.rs index 0e25aa73b3..f7dadc4c60 100644 --- a/packages/modrinth-content-management/src/lib.rs +++ b/packages/modrinth-content-management/src/lib.rs @@ -1,11 +1,17 @@ +pub mod diff; pub mod install; -pub mod model; -pub mod provider; -pub use install::resolve_content; -pub use model::{ - ContentType, Dependency, DependencyType, Error, ResolutionPreferences, - ResolveContentPlan, ResolveContentRequest, ResolvedContent, SkippedContent, - SkippedReason, Version, +pub use diff::{ + Change, CommonExternalFilePolicy, ConfigurationDiff, ContentSetConfiguration, + ContentSetDiff, ContentSetDiffEntry, ContentSetDiffKind, ContentSetDiffOptions, + ContentSetSnapshot, ExternalFileKey, LoaderReference, diff_configuration, + diff_content_sets, }; -pub use provider::ContentMetadataProvider; +pub use install::{ + ContentMetadataProvider, Dependency, DependencyType, ResolutionPreferences, + ResolveContentPlan, ResolveContentRequest, ResolvedContent, SkippedContent, + SkippedReason, Version, resolve_content, +}; +pub use shared::{ContentType, Error}; + +mod shared; diff --git a/packages/modrinth-content-management/src/shared/mod.rs b/packages/modrinth-content-management/src/shared/mod.rs new file mode 100644 index 0000000000..aaa774f73a --- /dev/null +++ b/packages/modrinth-content-management/src/shared/mod.rs @@ -0,0 +1,70 @@ +//! Content identifiers and errors shared by content-management features. + +use serde::{Deserialize, Serialize}; + +#[derive(thiserror::Error, Debug)] +pub enum Error { + #[error("unknown content type `{0}`")] + UnknownContentType(String), + #[error("project `{project_id}` has conflicting versions `{before}` and `{after}`")] + ConflictingProjectVersions { + project_id: String, + before: String, + after: String, + }, + #[error("metadata provider error: {0}")] + Provider(String), + #[error("project `{0}` was not found")] + ProjectNotFound(String), + #[error("version `{0}` was not found")] + VersionNotFound(String), + #[error("version `{version_id}` does not belong to project `{project_id}`")] + VersionProjectMismatch { + version_id: String, + project_id: String, + }, + #[error("no compatible version was found for project `{0}`")] + NoCompatibleVersion(String), +} + +#[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] +#[serde(rename_all = "lowercase")] +pub enum ContentType { + Mod, + Plugin, + DataPack, + ResourcePack, + Shader, + ModPack, +} + +impl std::str::FromStr for ContentType { + type Err = Error; + + fn from_str(value: &str) -> Result { + match value { + "mod" => Ok(Self::Mod), + "plugin" => Ok(Self::Plugin), + "datapack" => Ok(Self::DataPack), + "resourcepack" => Ok(Self::ResourcePack), + "shader" => Ok(Self::Shader), + "modpack" => Ok(Self::ModPack), + _ => Err(Error::UnknownContentType(value.to_owned())), + } + } +} + +impl ContentType { + /// Returns the canonical content type used by shared-instance manifests. + pub fn as_str(self) -> &'static str { + match self { + Self::Mod => "mod", + Self::Plugin => "plugin", + Self::DataPack => "datapack", + Self::ResourcePack => "resourcepack", + Self::Shader => "shader", + Self::ModPack => "modpack", + } + } +} + diff --git a/packages/modrinth-content-management/tests/diff.rs b/packages/modrinth-content-management/tests/diff.rs new file mode 100644 index 0000000000..60c5301b8a --- /dev/null +++ b/packages/modrinth-content-management/tests/diff.rs @@ -0,0 +1,399 @@ +use modrinth_content_management::{ + Change, CommonExternalFilePolicy, ConfigurationDiff, ContentSetConfiguration, + ContentSetDiffEntry, ContentSetDiffKind, ContentSetDiffOptions, + ContentSetSnapshot, ContentType, Error, ExternalFileKey, LoaderReference, diff_configuration, + diff_content_sets, +}; + +fn snapshot( + projects: &[(&str, &str)], + files: &[(ContentType, &str)], +) -> ContentSetSnapshot { + ContentSetSnapshot { + projects: projects.iter() + .map(|(project, version)| (project.to_string(), version.to_string())) + .collect(), + external_files: files.iter() + .map(|(content_type, path)| file(*content_type, path)) + .collect(), + } +} + +fn file(content_type: ContentType, path: &str) -> ExternalFileKey { + ExternalFileKey { content_type, path: path.to_string() } +} + +fn configuration( + modpack_version_id: Option<&str>, + game_version: &str, + loader: &str, + loader_version: Option<&str>, +) -> ContentSetConfiguration { + ContentSetConfiguration { + modpack_version_id: modpack_version_id.map(str::to_string), + game_version: game_version.to_string(), + loader: LoaderReference { + name: loader.to_string(), + version: loader_version.map(str::to_string), + }, + } +} + +#[test] +fn empty_and_identical_snapshots_have_no_changes() { + for content in [ + ContentSetSnapshot::default(), + snapshot(&[("sodium", "v1")], &[(ContentType::Mod, "custom.jar")]), + ] { + let diff = diff_content_sets(&content, &content, &ContentSetDiffOptions::default()); + assert!(!diff.has_changes()); + assert!(diff.content.is_empty()); + assert!(diff.additional.is_empty()); + } +} + +#[test] +fn project_changes_preserve_identity_and_both_version_ids() { + let before = snapshot( + &[("unchanged", "v1"), ("updated", "old"), ("removed", "v2")], + &[], + ); + let after = snapshot( + &[("updated", "new"), ("added", "v3"), ("unchanged", "v1")], + &[], + ); + + let diff = diff_content_sets(&before, &after, &ContentSetDiffOptions::default()); + + assert!(diff.has_changes()); + assert_eq!(diff.content, vec![ + ContentSetDiffEntry::Project { + project_id: "added".to_string(), + change: Change::Added { after: "v3".to_string() }, + }, + ContentSetDiffEntry::Project { + project_id: "removed".to_string(), + change: Change::Removed { before: "v2".to_string() }, + }, + ContentSetDiffEntry::Project { + project_id: "updated".to_string(), + change: Change::Updated { + before: "old".to_string(), + after: "new".to_string(), + }, + }, + ]); +} + +#[test] +fn reversing_snapshots_reverses_project_and_file_changes() { + let before = snapshot( + &[("updated", "old"), ("removed", "v1")], + &[(ContentType::Mod, "removed.jar")], + ); + let after = snapshot( + &[("updated", "new"), ("added", "v2")], + &[(ContentType::Mod, "added.jar")], + ); + + let reverse = diff_content_sets(&after, &before, &ContentSetDiffOptions::default()); + + assert_eq!(reverse.content, vec![ + ContentSetDiffEntry::Project { + project_id: "added".to_string(), + change: Change::Removed { before: "v2".to_string() }, + }, + ContentSetDiffEntry::Project { + project_id: "removed".to_string(), + change: Change::Added { after: "v1".to_string() }, + }, + ContentSetDiffEntry::Project { + project_id: "updated".to_string(), + change: Change::Updated { + before: "new".to_string(), + after: "old".to_string(), + }, + }, + ContentSetDiffEntry::ExternalFile { + file: file(ContentType::Mod, "added.jar"), + kind: ContentSetDiffKind::Removed, + }, + ContentSetDiffEntry::ExternalFile { + file: file(ContentType::Mod, "removed.jar"), + kind: ContentSetDiffKind::Added, + }, + ]); +} + +#[test] +fn common_file_policy_only_affects_files_present_on_both_sides() { + let before = snapshot(&[], &[ + (ContentType::Mod, "common.jar"), + (ContentType::Mod, "removed.jar"), + ]); + let after = snapshot(&[], &[ + (ContentType::Mod, "added.jar"), + (ContentType::Mod, "common.jar"), + ]); + let added = ContentSetDiffEntry::ExternalFile { + file: file(ContentType::Mod, "added.jar"), + kind: ContentSetDiffKind::Added, + }; + let removed = ContentSetDiffEntry::ExternalFile { + file: file(ContentType::Mod, "removed.jar"), + kind: ContentSetDiffKind::Removed, + }; + let common = ContentSetDiffEntry::ExternalFile { + file: file(ContentType::Mod, "common.jar"), + kind: ContentSetDiffKind::Updated, + }; + for (policy, expected) in [ + (CommonExternalFilePolicy::AssumeUnchanged, vec![added.clone(), removed.clone()]), + (CommonExternalFilePolicy::AssumeUpdated, vec![added, common, removed]), + ] { + let diff = diff_content_sets(&before, &after, &ContentSetDiffOptions { + common_external_files: policy, + }); + assert_eq!(diff.content, expected, "{policy:?}"); + } +} + +#[test] +fn assumed_common_file_update_counts_as_a_change() { + let content = snapshot(&[], &[(ContentType::ResourcePack, "textures.zip")]); + let diff = diff_content_sets(&content, &content, &ContentSetDiffOptions { + common_external_files: CommonExternalFilePolicy::AssumeUpdated, + }); + + assert!(diff.has_changes()); + assert_eq!(diff.content, vec![ContentSetDiffEntry::ExternalFile { + file: file(ContentType::ResourcePack, "textures.zip"), + kind: ContentSetDiffKind::Updated, + }]); +} + +#[test] +fn file_identity_includes_content_type_and_relative_path() { + let before = snapshot(&[], &[ + (ContentType::Mod, "shared.jar"), + (ContentType::DataPack, "first/data.zip"), + ]); + let after = snapshot(&[], &[ + (ContentType::Plugin, "shared.jar"), + (ContentType::DataPack, "second/data.zip"), + ]); + + let diff = diff_content_sets(&before, &after, &ContentSetDiffOptions::default()); + + assert_eq!(diff.content, vec![ + ContentSetDiffEntry::ExternalFile { + file: file(ContentType::Mod, "shared.jar"), + kind: ContentSetDiffKind::Removed, + }, + ContentSetDiffEntry::ExternalFile { + file: file(ContentType::Plugin, "shared.jar"), + kind: ContentSetDiffKind::Added, + }, + ContentSetDiffEntry::ExternalFile { + file: file(ContentType::DataPack, "first/data.zip"), + kind: ContentSetDiffKind::Removed, + }, + ContentSetDiffEntry::ExternalFile { + file: file(ContentType::DataPack, "second/data.zip"), + kind: ContentSetDiffKind::Added, + }, + ]); +} + +#[test] +fn diff_order_is_independent_of_snapshot_insertion_order() { + let first = snapshot( + &[("z", "v2"), ("a", "v1")], + &[(ContentType::Mod, "z.jar"), (ContentType::Mod, "a.jar")], + ); + let second = snapshot( + &[("a", "v1"), ("z", "v2")], + &[(ContentType::Mod, "a.jar"), (ContentType::Mod, "z.jar")], + ); + let empty = ContentSetSnapshot::default(); + let options = ContentSetDiffOptions::default(); + + assert_eq!( + diff_content_sets(&empty, &first, &options), + diff_content_sets(&empty, &second, &options), + ); +} + +#[test] +fn inserting_the_same_project_version_is_idempotent() { + let mut content = ContentSetSnapshot::default(); + content.insert_project("sodium".to_string(), "v1".to_string()).unwrap(); + let original = content.clone(); + + content.insert_project("sodium".to_string(), "v1".to_string()).unwrap(); + + assert_eq!(content, original); +} + +#[test] +fn conflicting_project_version_is_rejected_without_changing_the_snapshot() { + let mut content = snapshot(&[("sodium", "v1")], &[]); + let original = content.clone(); + + let error = content.insert_project("sodium".to_string(), "v2".to_string()).unwrap_err(); + + assert!(matches!(error, Error::ConflictingProjectVersions { project_id, before, after } + if project_id == "sodium" && before == "v1" && after == "v2")); + assert_eq!(content, original); +} + +#[derive(Debug, PartialEq)] +enum AdditionalChange { + ConfigFilesUpdated { file_count: usize }, +} + +#[test] +fn caller_entries_can_trigger_changes_without_content_changes() { + let content = ContentSetSnapshot::default(); + let base = diff_content_sets(&content, &content, &ContentSetDiffOptions::default()); + let mut diff = base.with_additional(Vec::::new()); + assert!(!diff.has_changes()); + + diff.additional.push(AdditionalChange::ConfigFilesUpdated { file_count: 2 }); + + assert!(diff.content.is_empty()); + assert!(diff.has_changes()); + assert_eq!(diff.additional, vec![AdditionalChange::ConfigFilesUpdated { file_count: 2 }]); +} + +#[test] +fn attaching_caller_entries_preserves_content_changes() { + let before = ContentSetSnapshot::default(); + let after = snapshot(&[("sodium", "v1")], &[]); + let base = diff_content_sets(&before, &after, &ContentSetDiffOptions::default()); + let content = base.content.clone(); + let mut diff = base.with_additional([ + AdditionalChange::ConfigFilesUpdated { file_count: 1 }, + ]); + + assert_eq!(diff.content, content); + assert_eq!(diff.additional.len(), 1); + diff.additional.clear(); + assert!(diff.has_changes()); +} + +#[test] +fn modpack_link_unlink_and_update_have_directional_version_ids() { + for (before_id, after_id, expected) in [ + (None, Some("pack-v1"), Change::Added { after: "pack-v1".to_string() }), + (Some("pack-v1"), None, Change::Removed { before: "pack-v1".to_string() }), + (Some("pack-v1"), Some("pack-v2"), Change::Updated { + before: "pack-v1".to_string(), + after: "pack-v2".to_string(), + }), + ] { + let before = configuration(before_id, "1.21.1", "fabric", Some("0.16.0")); + let after = configuration(after_id, "1.21.1", "fabric", Some("0.16.0")); + + assert_eq!(diff_configuration(&before, &after), vec![ConfigurationDiff::Modpack(expected)]); + } +} + +#[test] +fn configuration_normalization_does_not_create_false_changes() { + let empty = configuration(None, "1.21.1", "fabric", None); + let explicit_empty = configuration(Some(""), "1.21.1", "Fabric", Some("")); + let linked = configuration(Some("pack-v1"), "1.21.1", "NeoForge", Some("21.1.1")); + let canonical = configuration(Some("pack-v1"), "1.21.1", "neoforge", Some("21.1.1")); + + for (before, after) in [(&empty, &explicit_empty), (&linked, &canonical)] { + assert!(diff_configuration(before, after).is_empty()); + assert!(diff_configuration(after, before).is_empty()); + assert!(diff_configuration(before, before).is_empty()); + } +} + +#[test] +fn changing_only_the_game_version_produces_one_configuration_entry() { + let before = configuration(None, "1.21.1", "fabric", Some("0.16.0")); + let after = configuration(None, "1.21.2", "fabric", Some("0.16.0")); + + assert_eq!(diff_configuration(&before, &after), vec![ + ConfigurationDiff::GameVersion(Change::Updated { + before: "1.21.1".to_string(), + after: "1.21.2".to_string(), + }), + ]); +} + +#[test] +fn loader_version_changes_include_added_and_removed_optional_versions() { + for (before_version, after_version) in [ + (Some("0.16.0"), Some("0.16.1")), + (None, Some("0.16.0")), + (Some("0.16.0"), None), + ] { + let before = configuration(None, "1.21.1", "fabric", before_version); + let after = configuration(None, "1.21.1", "fabric", after_version); + + assert_eq!(diff_configuration(&before, &after), vec![ + ConfigurationDiff::Loader(Change::Updated { + before: before.loader.clone(), + after: after.loader.clone(), + }), + ]); + } +} + +#[test] +fn compatible_loader_names_still_represent_different_installed_loaders() { + let before = configuration(None, "1.21.1", "paper", Some("123")); + let after = configuration(None, "1.21.1", "purpur", Some("123")); + + assert_eq!(diff_configuration(&before, &after), vec![ + ConfigurationDiff::Loader(Change::Updated { + before: before.loader.clone(), + after: after.loader.clone(), + }), + ]); +} + +#[test] +fn simultaneous_configuration_changes_are_composed_in_stable_order() { + let before = configuration(Some("pack-v1"), "1.21.1", "fabric", Some("0.16.0")); + let after = configuration(Some("pack-v2"), "1.21.2", "neoforge", Some("21.2.1")); + let content = ContentSetSnapshot::default(); + let diff = diff_content_sets(&content, &content, &ContentSetDiffOptions::default()) + .with_additional(diff_configuration(&before, &after)); + + assert!(diff.content.is_empty()); + assert!(diff.has_changes()); + assert_eq!(diff.additional, vec![ + ConfigurationDiff::Modpack(Change::Updated { + before: "pack-v1".to_string(), + after: "pack-v2".to_string(), + }), + ConfigurationDiff::GameVersion(Change::Updated { + before: "1.21.1".to_string(), + after: "1.21.2".to_string(), + }), + ConfigurationDiff::Loader(Change::Updated { + before: before.loader, + after: after.loader, + }), + ]); +} + +#[test] +fn mapping_changes_preserves_their_kind_and_before_after_values() { + for (change, kind, before, after) in [ + (Change::Added { after: 2 }, ContentSetDiffKind::Added, None, Some("v2")), + (Change::Removed { before: 1 }, ContentSetDiffKind::Removed, Some("v1"), None), + (Change::Updated { before: 1, after: 2 }, ContentSetDiffKind::Updated, Some("v1"), Some("v2")), + ] { + let mapped = change.map(|version| format!("v{version}")); + assert_eq!(mapped.kind(), kind); + assert_eq!(mapped.before().map(String::as_str), before); + assert_eq!(mapped.after().map(String::as_str), after); + } +} From 075a78c41c2a0f5178a01838eaaa539fd3c25a7a Mon Sep 17 00:00:00 2001 From: "Calum H. (IMB11)" Date: Fri, 4 Sep 2026 17:31:58 +0100 Subject: [PATCH 2/3] fix: fmt --- .../app-lib/src/api/instance/shared/diff.rs | 662 +++++++++------- .../app-lib/src/api/instance/shared/mod.rs | 14 +- .../src/api/instance/shared/publish.rs | 92 ++- .../src/diff/configuration.rs | 70 +- .../src/diff/mod.rs | 77 +- .../src/diff/model.rs | 231 +++--- .../src/install/mod.rs | 6 +- .../modrinth-content-management/src/lib.rs | 14 +- .../src/shared/mod.rs | 78 +- .../modrinth-content-management/tests/diff.rs | 746 ++++++++++-------- 10 files changed, 1150 insertions(+), 840 deletions(-) diff --git a/packages/app-lib/src/api/instance/shared/diff.rs b/packages/app-lib/src/api/instance/shared/diff.rs index c8366755f9..0d67205da5 100644 --- a/packages/app-lib/src/api/instance/shared/diff.rs +++ b/packages/app-lib/src/api/instance/shared/diff.rs @@ -4,333 +4,433 @@ use super::types::*; use super::*; struct SharedContentSnapshot { - version_ids: Vec, - external_files: BTreeSet, - configuration: ContentSetConfiguration, + version_ids: Vec, + external_files: BTreeSet, + configuration: ContentSetConfiguration, } pub(super) async fn shared_instance_update_diffs( - metadata: &crate::state::InstanceMetadata, - version: &InstanceVersionResponse, - state: &State, + metadata: &crate::state::InstanceMetadata, + version: &InstanceVersionResponse, + state: &State, ) -> crate::Result> { - let before_configuration = local_configuration(metadata); - let after_configuration = remote_configuration(version); - let modpack_unlinked = before_configuration.modpack_version_id.is_some() - && after_configuration.modpack_version_id.is_none(); - let (version_ids, external_files) = current_shared_content(metadata, modpack_unlinked, state).await?; - let before = SharedContentSnapshot { version_ids, external_files, configuration: before_configuration }; - let (version_ids, external_files) = remote_shared_content(version)?; - let after = SharedContentSnapshot { version_ids, external_files, configuration: after_configuration }; + let before_configuration = local_configuration(metadata); + let after_configuration = remote_configuration(version); + let modpack_unlinked = before_configuration.modpack_version_id.is_some() + && after_configuration.modpack_version_id.is_none(); + let (version_ids, external_files) = + current_shared_content(metadata, modpack_unlinked, state).await?; + let before = SharedContentSnapshot { + version_ids, + external_files, + configuration: before_configuration, + }; + let (version_ids, external_files) = remote_shared_content(version)?; + let after = SharedContentSnapshot { + version_ids, + external_files, + configuration: after_configuration, + }; - shared_content_diffs( - &before, - &after, - &HashSet::new(), - &BTreeSet::new(), - CommonExternalFilePolicy::AssumeUpdated, - state, - ).await + shared_content_diffs( + &before, + &after, + &HashSet::new(), + &BTreeSet::new(), + CommonExternalFilePolicy::AssumeUpdated, + state, + ) + .await } pub(super) async fn shared_instance_publish_diffs( - metadata: &crate::state::InstanceMetadata, - version: &InstanceVersionResponse, - snapshot: &CurrentPublishSnapshot, - state: &State, + metadata: &crate::state::InstanceMetadata, + version: &InstanceVersionResponse, + snapshot: &CurrentPublishSnapshot, + state: &State, ) -> crate::Result> { - let before_configuration = remote_configuration(version); - let after_configuration = local_configuration(metadata); - let modpack_unlinked = before_configuration.modpack_version_id.is_some() - && after_configuration.modpack_version_id.is_none(); - let disabled_versions = async { - if snapshot.disabled_version_ids.is_empty() { - Ok(HashMap::new()) - } else { - shared_versions_by_project(&snapshot.disabled_version_ids, state).await - } - }; - let ((version_ids, external_files), disabled_versions) = tokio::try_join!( - remote_publish_content(version, modpack_unlinked, state), - disabled_versions, - )?; - let before = SharedContentSnapshot { version_ids, external_files, configuration: before_configuration }; - let after = SharedContentSnapshot { - version_ids: snapshot.version_ids.iter() - .filter(|id| after_configuration.modpack_version_id.as_deref() != Some(id.as_str())) - .cloned().collect(), - external_files: snapshot.external_files.iter() - .map(|file| shared_external_file_key(&file.file_type, &file.file_name)) - .collect::>()?, - configuration: after_configuration, - }; - let mut removed_disabled_project_ids = snapshot.disabled_project_ids.clone(); - removed_disabled_project_ids.extend(disabled_versions.into_keys()); + let before_configuration = remote_configuration(version); + let after_configuration = local_configuration(metadata); + let modpack_unlinked = before_configuration.modpack_version_id.is_some() + && after_configuration.modpack_version_id.is_none(); + let disabled_versions = async { + if snapshot.disabled_version_ids.is_empty() { + Ok(HashMap::new()) + } else { + shared_versions_by_project(&snapshot.disabled_version_ids, state) + .await + } + }; + let ((version_ids, external_files), disabled_versions) = tokio::try_join!( + remote_publish_content(version, modpack_unlinked, state), + disabled_versions, + )?; + let before = SharedContentSnapshot { + version_ids, + external_files, + configuration: before_configuration, + }; + let after = SharedContentSnapshot { + version_ids: snapshot + .version_ids + .iter() + .filter(|id| { + after_configuration.modpack_version_id.as_deref() + != Some(id.as_str()) + }) + .cloned() + .collect(), + external_files: snapshot + .external_files + .iter() + .map(|file| { + shared_external_file_key(&file.file_type, &file.file_name) + }) + .collect::>()?, + configuration: after_configuration, + }; + let mut removed_disabled_project_ids = + snapshot.disabled_project_ids.clone(); + removed_disabled_project_ids.extend(disabled_versions.into_keys()); - shared_content_diffs( - &before, - &after, - &removed_disabled_project_ids, - &snapshot.disabled_external_files, - CommonExternalFilePolicy::AssumeUnchanged, - state, - ).await + shared_content_diffs( + &before, + &after, + &removed_disabled_project_ids, + &snapshot.disabled_external_files, + CommonExternalFilePolicy::AssumeUnchanged, + state, + ) + .await } -fn local_configuration(metadata: &crate::state::InstanceMetadata) -> ContentSetConfiguration { - ContentSetConfiguration { - modpack_version_id: shared_modpack_id(&metadata.link).filter(|id| !id.is_empty()), - game_version: metadata.applied_content_set.game_version.clone(), - loader: LoaderReference { - name: metadata.applied_content_set.loader.as_str().to_string(), - version: metadata.applied_content_set.loader_version.clone(), - }, - } +fn local_configuration( + metadata: &crate::state::InstanceMetadata, +) -> ContentSetConfiguration { + ContentSetConfiguration { + modpack_version_id: shared_modpack_id(&metadata.link) + .filter(|id| !id.is_empty()), + game_version: metadata.applied_content_set.game_version.clone(), + loader: LoaderReference { + name: metadata.applied_content_set.loader.as_str().to_string(), + version: metadata.applied_content_set.loader_version.clone(), + }, + } } -fn remote_configuration(version: &InstanceVersionResponse) -> ContentSetConfiguration { - ContentSetConfiguration { - modpack_version_id: version.modpack_id.clone().filter(|id| !id.is_empty()), - game_version: version.game_version.clone(), - loader: LoaderReference { - name: version.loader.as_str().to_string(), - version: Some(version.loader_version.clone()), - }, - } +fn remote_configuration( + version: &InstanceVersionResponse, +) -> ContentSetConfiguration { + ContentSetConfiguration { + modpack_version_id: version + .modpack_id + .clone() + .filter(|id| !id.is_empty()), + game_version: version.game_version.clone(), + loader: LoaderReference { + name: version.loader.as_str().to_string(), + version: Some(version.loader_version.clone()), + }, + } } async fn shared_configuration_diffs( - changes: Vec, - state: &State, + changes: Vec, + state: &State, ) -> Vec { - let mut diffs = Vec::new(); - for change in changes { - match change { - ConfigurationDiff::Modpack(Change::Added { after }) => diffs.push(configuration_diff( - SharedInstanceUpdateDiffType::ModpackLinked, - None, - shared_modpack_version_label(Some(&after), state).await, - )), - ConfigurationDiff::Modpack(Change::Removed { before }) => diffs.push(configuration_diff( - SharedInstanceUpdateDiffType::ModpackUnlinked, - shared_modpack_version_label(Some(&before), state).await, - None, - )), - ConfigurationDiff::Modpack(Change::Updated { before, after }) => { - let current = shared_modpack_version_details(&before, state).await; - let new = shared_modpack_version_details(&after, state).await; - let project_name = new.as_ref().and_then(|details| details.project_name.clone()) - .or_else(|| current.as_ref().and_then(|details| details.project_name.clone())); - diffs.push(SharedInstanceUpdateDiff { - type_: SharedInstanceUpdateDiffType::ModpackUpdated, - project_id: None, - project_name, - file_name: None, - current_version_name: current.map(|details| details.version_name), - new_version_name: new.map(|details| details.version_name), - config_file_count: None, - disabled: false, - }); - } - ConfigurationDiff::GameVersion(change) => diffs.push(configuration_diff( - SharedInstanceUpdateDiffType::GameVersionUpdated, - change.before().cloned(), - change.after().cloned(), - )), - ConfigurationDiff::Loader(change) => diffs.push(configuration_diff( - SharedInstanceUpdateDiffType::LoaderUpdated, - change.before().map(shared_loader_label), - change.after().map(shared_loader_label), - )), - } - } - diffs + let mut diffs = Vec::new(); + for change in changes { + match change { + ConfigurationDiff::Modpack(Change::Added { after }) => { + diffs.push(configuration_diff( + SharedInstanceUpdateDiffType::ModpackLinked, + None, + shared_modpack_version_label(Some(&after), state).await, + )) + } + ConfigurationDiff::Modpack(Change::Removed { before }) => diffs + .push(configuration_diff( + SharedInstanceUpdateDiffType::ModpackUnlinked, + shared_modpack_version_label(Some(&before), state).await, + None, + )), + ConfigurationDiff::Modpack(Change::Updated { before, after }) => { + let current = + shared_modpack_version_details(&before, state).await; + let new = shared_modpack_version_details(&after, state).await; + let project_name = new + .as_ref() + .and_then(|details| details.project_name.clone()) + .or_else(|| { + current + .as_ref() + .and_then(|details| details.project_name.clone()) + }); + diffs.push(SharedInstanceUpdateDiff { + type_: SharedInstanceUpdateDiffType::ModpackUpdated, + project_id: None, + project_name, + file_name: None, + current_version_name: current + .map(|details| details.version_name), + new_version_name: new.map(|details| details.version_name), + config_file_count: None, + disabled: false, + }); + } + ConfigurationDiff::GameVersion(change) => { + diffs.push(configuration_diff( + SharedInstanceUpdateDiffType::GameVersionUpdated, + change.before().cloned(), + change.after().cloned(), + )) + } + ConfigurationDiff::Loader(change) => { + diffs.push(configuration_diff( + SharedInstanceUpdateDiffType::LoaderUpdated, + change.before().map(shared_loader_label), + change.after().map(shared_loader_label), + )) + } + } + } + diffs } pub(super) fn configuration_diff( - type_: SharedInstanceUpdateDiffType, - current_version_name: Option, - new_version_name: Option, + type_: SharedInstanceUpdateDiffType, + current_version_name: Option, + new_version_name: Option, ) -> SharedInstanceUpdateDiff { - SharedInstanceUpdateDiff { - type_, - project_id: None, - project_name: None, - file_name: None, - current_version_name, - new_version_name, - config_file_count: None, - disabled: false, - } + SharedInstanceUpdateDiff { + type_, + project_id: None, + project_name: None, + file_name: None, + current_version_name, + new_version_name, + config_file_count: None, + disabled: false, + } } pub(super) async fn shared_modpack_version_label( - version_id: Option<&str>, - state: &State, + version_id: Option<&str>, + state: &State, ) -> Option { - let version_id = version_id?; - let details = shared_modpack_version_details(version_id, state).await?; + let version_id = version_id?; + let details = shared_modpack_version_details(version_id, state).await?; - Some(match details.project_name { - Some(project_name) => { - format!("{project_name} {}", details.version_name) - } - None => details.version_name, - }) + Some(match details.project_name { + Some(project_name) => { + format!("{project_name} {}", details.version_name) + } + None => details.version_name, + }) } struct SharedModpackVersionDetails { - project_name: Option, - version_name: String, + project_name: Option, + version_name: String, } async fn shared_modpack_version_details( - version_id: &str, - state: &State, + version_id: &str, + state: &State, ) -> Option { - let Some(version) = CachedEntry::get_version( - version_id, - Some(CacheBehaviour::Bypass), - &state.pool, - &state.api_semaphore, - ) - .await - .ok() - .flatten() else { - return Some(SharedModpackVersionDetails { - project_name: None, - version_name: version_id.to_string(), - }); - }; - let project = CachedEntry::get_project( - &version.project_id, - Some(CacheBehaviour::Bypass), - &state.pool, - &state.api_semaphore, - ) - .await - .ok() - .flatten(); + let Some(version) = CachedEntry::get_version( + version_id, + Some(CacheBehaviour::Bypass), + &state.pool, + &state.api_semaphore, + ) + .await + .ok() + .flatten() else { + return Some(SharedModpackVersionDetails { + project_name: None, + version_name: version_id.to_string(), + }); + }; + let project = CachedEntry::get_project( + &version.project_id, + Some(CacheBehaviour::Bypass), + &state.pool, + &state.api_semaphore, + ) + .await + .ok() + .flatten(); - Some(SharedModpackVersionDetails { - project_name: Some( - project.map(|project| project.title).unwrap_or(version.name), - ), - version_name: version.version_number, - }) + Some(SharedModpackVersionDetails { + project_name: Some( + project.map(|project| project.title).unwrap_or(version.name), + ), + version_name: version.version_number, + }) } fn shared_loader_label(loader: &LoaderReference) -> String { - let loader_name = match loader.name.as_str() { - "vanilla" => "Vanilla", - "forge" => "Forge", - "fabric" => "Fabric", - "quilt" => "Quilt", - "neoforge" => "NeoForge", - name => name, - }; - match loader.version.as_deref() { - Some(version) => format!("{loader_name} {version}"), - None => loader_name.to_string(), - } + let loader_name = match loader.name.as_str() { + "vanilla" => "Vanilla", + "forge" => "Forge", + "fabric" => "Fabric", + "quilt" => "Quilt", + "neoforge" => "NeoForge", + name => name, + }; + match loader.version.as_deref() { + Some(version) => format!("{loader_name} {version}"), + None => loader_name.to_string(), + } } async fn shared_content_diffs( - before: &SharedContentSnapshot, - after: &SharedContentSnapshot, - removed_disabled_project_ids: &HashSet, - removed_disabled_external_files: &BTreeSet, - common_external_files: CommonExternalFilePolicy, - state: &State, + before: &SharedContentSnapshot, + after: &SharedContentSnapshot, + removed_disabled_project_ids: &HashSet, + removed_disabled_external_files: &BTreeSet, + common_external_files: CommonExternalFilePolicy, + state: &State, ) -> crate::Result> { - let (before_versions, after_versions) = tokio::try_join!( - shared_versions_by_project(&before.version_ids, state), - shared_versions_by_project(&after.version_ids, state), - )?; - let to_snapshot = |source: &SharedContentSnapshot, versions: &HashMap| ContentSetSnapshot { - projects: versions.iter().map(|(project_id, version)| (project_id.clone(), version.id.clone())).collect(), - external_files: source.external_files.clone(), - }; - let diff = diff_content_sets( - &to_snapshot(before, &before_versions), - &to_snapshot(after, &after_versions), - &ContentSetDiffOptions { common_external_files }, - ).with_additional(diff_configuration(&before.configuration, &after.configuration)); - if !diff.has_changes() { - return Ok(Vec::new()); - } - let project_ids = diff.content.iter().filter_map(|entry| match entry { - ContentSetDiffEntry::Project { project_id, .. } => Some(project_id.clone()), - ContentSetDiffEntry::ExternalFile { .. } => None, - }).collect::>(); - let project_names = shared_project_names(&project_ids, state).await?; - let mut content_diffs = Vec::new(); - for entry in diff.content { - match entry { - ContentSetDiffEntry::Project { project_id, change } => { - let disabled = change.kind() == ContentSetDiffKind::Removed - && removed_disabled_project_ids.contains(&project_id); - content_diffs.push(SharedInstanceUpdateDiff { - type_: shared_update_diff_type(change.kind()), - project_name: Some(project_names.get(&project_id).cloned().unwrap_or_else(|| project_id.clone())), - current_version_name: change.before().map(|id| before_versions.get(&project_id) - .map(|version| version.version_number.clone()).unwrap_or_else(|| id.clone())), - new_version_name: change.after().map(|id| after_versions.get(&project_id) - .map(|version| version.version_number.clone()).unwrap_or_else(|| id.clone())), - project_id: Some(project_id), - file_name: None, - config_file_count: None, - disabled, - }); - } - ContentSetDiffEntry::ExternalFile { file, kind } => { - let disabled = kind == ContentSetDiffKind::Removed && removed_disabled_external_files.contains(&file); - content_diffs.push(SharedInstanceUpdateDiff { - type_: shared_update_diff_type(kind), - project_id: None, - project_name: None, - file_name: Some(file.path), - current_version_name: None, - new_version_name: None, - config_file_count: None, - disabled, - }); - } - } - } - content_diffs.sort_by(|a, b| a.project_name.as_deref().or(a.file_name.as_deref()) - .cmp(&b.project_name.as_deref().or(b.file_name.as_deref()))); - let mut diffs = shared_configuration_diffs(diff.additional, state).await; - diffs.extend(content_diffs); - Ok(diffs) + let (before_versions, after_versions) = tokio::try_join!( + shared_versions_by_project(&before.version_ids, state), + shared_versions_by_project(&after.version_ids, state), + )?; + let to_snapshot = + |source: &SharedContentSnapshot, + versions: &HashMap| { + ContentSetSnapshot { + projects: versions + .iter() + .map(|(project_id, version)| { + (project_id.clone(), version.id.clone()) + }) + .collect(), + external_files: source.external_files.clone(), + } + }; + let diff = diff_content_sets( + &to_snapshot(before, &before_versions), + &to_snapshot(after, &after_versions), + &ContentSetDiffOptions { + common_external_files, + }, + ) + .with_additional(diff_configuration( + &before.configuration, + &after.configuration, + )); + if !diff.has_changes() { + return Ok(Vec::new()); + } + let project_ids = diff + .content + .iter() + .filter_map(|entry| match entry { + ContentSetDiffEntry::Project { project_id, .. } => { + Some(project_id.clone()) + } + ContentSetDiffEntry::ExternalFile { .. } => None, + }) + .collect::>(); + let project_names = shared_project_names(&project_ids, state).await?; + let mut content_diffs = Vec::new(); + for entry in diff.content { + match entry { + ContentSetDiffEntry::Project { project_id, change } => { + let disabled = change.kind() == ContentSetDiffKind::Removed + && removed_disabled_project_ids.contains(&project_id); + content_diffs.push(SharedInstanceUpdateDiff { + type_: shared_update_diff_type(change.kind()), + project_name: Some( + project_names + .get(&project_id) + .cloned() + .unwrap_or_else(|| project_id.clone()), + ), + current_version_name: change.before().map(|id| { + before_versions + .get(&project_id) + .map(|version| version.version_number.clone()) + .unwrap_or_else(|| id.clone()) + }), + new_version_name: change.after().map(|id| { + after_versions + .get(&project_id) + .map(|version| version.version_number.clone()) + .unwrap_or_else(|| id.clone()) + }), + project_id: Some(project_id), + file_name: None, + config_file_count: None, + disabled, + }); + } + ContentSetDiffEntry::ExternalFile { file, kind } => { + let disabled = kind == ContentSetDiffKind::Removed + && removed_disabled_external_files.contains(&file); + content_diffs.push(SharedInstanceUpdateDiff { + type_: shared_update_diff_type(kind), + project_id: None, + project_name: None, + file_name: Some(file.path), + current_version_name: None, + new_version_name: None, + config_file_count: None, + disabled, + }); + } + } + } + content_diffs.sort_by(|a, b| { + a.project_name + .as_deref() + .or(a.file_name.as_deref()) + .cmp(&b.project_name.as_deref().or(b.file_name.as_deref())) + }); + let mut diffs = shared_configuration_diffs(diff.additional, state).await; + diffs.extend(content_diffs); + Ok(diffs) } -fn shared_update_diff_type(kind: ContentSetDiffKind) -> SharedInstanceUpdateDiffType { - match kind { - ContentSetDiffKind::Added => SharedInstanceUpdateDiffType::Added, - ContentSetDiffKind::Removed => SharedInstanceUpdateDiffType::Removed, - ContentSetDiffKind::Updated => SharedInstanceUpdateDiffType::Updated, - } +fn shared_update_diff_type( + kind: ContentSetDiffKind, +) -> SharedInstanceUpdateDiffType { + match kind { + ContentSetDiffKind::Added => SharedInstanceUpdateDiffType::Added, + ContentSetDiffKind::Removed => SharedInstanceUpdateDiffType::Removed, + ContentSetDiffKind::Updated => SharedInstanceUpdateDiffType::Updated, + } } -pub(super) fn shared_external_file_key(file_type: &str, path: &str) -> crate::Result { - Ok(ExternalFileKey { - content_type: file_type.parse().map_err(|error: modrinth_content_management::Error| { - crate::ErrorKind::InputError(error.to_string()) - })?, - path: path.to_string(), - }) +pub(super) fn shared_external_file_key( + file_type: &str, + path: &str, +) -> crate::Result { + Ok(ExternalFileKey { + content_type: file_type.parse().map_err( + |error: modrinth_content_management::Error| { + crate::ErrorKind::InputError(error.to_string()) + }, + )?, + path: path.to_string(), + }) } -fn remote_shared_content(version: &InstanceVersionResponse) -> crate::Result<(Vec, BTreeSet)> { - let mut version_ids = version.modrinth_ids.clone(); - if let Some(modpack_id) = version.modpack_id.as_deref() { - version_ids.retain(|id| id != modpack_id); - } - dedupe_strings(&mut version_ids); - let external_files = version.external_files.iter() - .filter(|file| file.file_type != CONFIG_BUNDLE_FILE_TYPE) - .map(|file| shared_external_file_key(&file.file_type, &file.file_name)) - .collect::>()?; - Ok((version_ids, external_files)) +fn remote_shared_content( + version: &InstanceVersionResponse, +) -> crate::Result<(Vec, BTreeSet)> { + let mut version_ids = version.modrinth_ids.clone(); + if let Some(modpack_id) = version.modpack_id.as_deref() { + version_ids.retain(|id| id != modpack_id); + } + dedupe_strings(&mut version_ids); + let external_files = version + .external_files + .iter() + .filter(|file| file.file_type != CONFIG_BUNDLE_FILE_TYPE) + .map(|file| shared_external_file_key(&file.file_type, &file.file_name)) + .collect::>()?; + Ok((version_ids, external_files)) } diff --git a/packages/app-lib/src/api/instance/shared/mod.rs b/packages/app-lib/src/api/instance/shared/mod.rs index 16e3f92adc..4fdce76f1c 100644 --- a/packages/app-lib/src/api/instance/shared/mod.rs +++ b/packages/app-lib/src/api/instance/shared/mod.rs @@ -1,10 +1,3 @@ -use modrinth_content_management::{ - Change, CommonExternalFilePolicy, ConfigurationDiff, ContentSetConfiguration, - ContentSetDiffEntry, ContentSetDiffKind, ContentSetDiffOptions, - ContentSetSnapshot, ExternalFileKey, LoaderReference, diff_configuration, - diff_content_sets, -}; -use std::collections::BTreeSet; use crate::SharedInstanceUnavailableReason; use crate::event::InstancePayloadType; use crate::event::emit::emit_instance; @@ -23,10 +16,17 @@ use crate::util::fetch::{ NO_TIMEOUT_REQWEST_CLIENT, REQWEST_CLIENT, }; use chrono::{DateTime, Utc}; +use modrinth_content_management::{ + Change, CommonExternalFilePolicy, ConfigurationDiff, + ContentSetConfiguration, ContentSetDiffEntry, ContentSetDiffKind, + ContentSetDiffOptions, ContentSetSnapshot, ExternalFileKey, + LoaderReference, diff_configuration, diff_content_sets, +}; use reqwest::{Method, StatusCode}; use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; use serde_json::json; +use std::collections::BTreeSet; use std::collections::{HashMap, HashSet}; use std::io::Read; diff --git a/packages/app-lib/src/api/instance/shared/publish.rs b/packages/app-lib/src/api/instance/shared/publish.rs index 6575497e29..e50a3e7c2e 100644 --- a/packages/app-lib/src/api/instance/shared/publish.rs +++ b/packages/app-lib/src/api/instance/shared/publish.rs @@ -179,7 +179,12 @@ pub(super) async fn remote_publish_content( version_ids.retain(|id| id != modpack_id); if include_modpack_dependencies { - extend_shared_modpack_dependencies(&mut version_ids, modpack_id, state).await?; + extend_shared_modpack_dependencies( + &mut version_ids, + modpack_id, + state, + ) + .await?; } } dedupe_strings(&mut version_ids); @@ -190,7 +195,9 @@ pub(super) async fn remote_publish_content( .external_files .iter() .filter(|file| file.file_type != CONFIG_BUNDLE_FILE_TYPE) - .map(|file| shared_external_file_key(&file.file_type, &file.file_name)) + .map(|file| { + shared_external_file_key(&file.file_type, &file.file_name) + }) .collect::>()?, )) } @@ -221,19 +228,23 @@ pub(super) async fn modpack_dependency_version_ids( /// Adds inherited pack versions without replacing explicitly selected projects. async fn extend_shared_modpack_dependencies( - version_ids: &mut Vec, - modpack_id: &str, - state: &State, + version_ids: &mut Vec, + modpack_id: &str, + state: &State, ) -> crate::Result<()> { - let dependency_ids = modpack_dependency_version_ids(modpack_id, state).await?; - let (explicit, inherited) = tokio::try_join!( - shared_versions_by_project(version_ids, state), - shared_versions_by_project(&dependency_ids, state), - )?; - version_ids.extend(inherited.into_values() - .filter(|version| !explicit.contains_key(&version.project_id)) - .map(|version| version.id)); - Ok(()) + let dependency_ids = + modpack_dependency_version_ids(modpack_id, state).await?; + let (explicit, inherited) = tokio::try_join!( + shared_versions_by_project(version_ids, state), + shared_versions_by_project(&dependency_ids, state), + )?; + version_ids.extend( + inherited + .into_values() + .filter(|version| !explicit.contains_key(&version.project_id)) + .map(|version| version.id), + ); + Ok(()) } pub(super) async fn shared_instance_install_modpack( @@ -325,15 +336,20 @@ pub(super) async fn current_shared_content( }; if let Some(file) = files.get(&file_id) { external_files.insert(ExternalFileKey { - content_type: entry.project_type.into(), - path: file.file_name.clone(), - }); + content_type: entry.project_type.into(), + path: file.file_name.clone(), + }); } } if include_linked_modpack_content && let Some(modpack_id) = shared_modpack_id(&metadata.link) { - extend_shared_modpack_dependencies(&mut version_ids, &modpack_id, state).await?; + extend_shared_modpack_dependencies( + &mut version_ids, + &modpack_id, + state, + ) + .await?; } dedupe_strings(&mut version_ids); @@ -437,9 +453,9 @@ pub(super) async fn collect_publish_snapshot( } disabled_external_files.insert(ExternalFileKey { - content_type: item.project_type.into(), - path: enabled_file_name(&item.file_name), - }); + content_type: item.project_type.into(), + path: enabled_file_name(&item.file_name), + }); } Ok(CurrentPublishSnapshot { @@ -466,18 +482,28 @@ pub(super) async fn shared_versions_by_project( ) .await?; - let fetched_ids = versions.iter().map(|version| version.id.as_str()).collect::>(); - if let Some(missing) = version_ids.iter().find(|id| !fetched_ids.contains(id.as_str())) { - return Err(crate::ErrorKind::InputError(format!("Shared content version {missing} was not found")).into()); - } - let mut snapshot = ContentSetSnapshot::default(); - let mut by_project = HashMap::new(); - for version in versions { - snapshot.insert_project(version.project_id.clone(), version.id.clone()) - .map_err(|error| crate::ErrorKind::InputError(error.to_string()))?; - by_project.insert(version.project_id.clone(), version); - } - Ok(by_project) + let fetched_ids = versions + .iter() + .map(|version| version.id.as_str()) + .collect::>(); + if let Some(missing) = version_ids + .iter() + .find(|id| !fetched_ids.contains(id.as_str())) + { + return Err(crate::ErrorKind::InputError(format!( + "Shared content version {missing} was not found" + )) + .into()); + } + let mut snapshot = ContentSetSnapshot::default(); + let mut by_project = HashMap::new(); + for version in versions { + snapshot + .insert_project(version.project_id.clone(), version.id.clone()) + .map_err(|error| crate::ErrorKind::InputError(error.to_string()))?; + by_project.insert(version.project_id.clone(), version); + } + Ok(by_project) } pub(super) async fn shared_project_names( diff --git a/packages/modrinth-content-management/src/diff/configuration.rs b/packages/modrinth-content-management/src/diff/configuration.rs index 583a72cc6e..7e1fa51774 100644 --- a/packages/modrinth-content-management/src/diff/configuration.rs +++ b/packages/modrinth-content-management/src/diff/configuration.rs @@ -5,50 +5,58 @@ use crate::diff::Change; /// Shared configuration independent of any application's loader or link model. #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] pub struct ContentSetConfiguration { - pub modpack_version_id: Option, - pub game_version: String, - pub loader: LoaderReference, + pub modpack_version_id: Option, + pub game_version: String, + pub loader: LoaderReference, } #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] pub struct LoaderReference { - pub name: String, - pub version: Option, + pub name: String, + pub version: Option, } #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] #[serde(tag = "type", content = "change", rename_all = "snake_case")] pub enum ConfigurationDiff { - Modpack(Change), - GameVersion(Change), - Loader(Change), + Modpack(Change), + GameVersion(Change), + Loader(Change), } /// Compares modpack, Minecraft and loader configuration in that order. /// Empty optional version IDs are equivalent to absent IDs. pub fn diff_configuration( - before: &ContentSetConfiguration, - after: &ContentSetConfiguration, + before: &ContentSetConfiguration, + after: &ContentSetConfiguration, ) -> Vec { - let mut changes = Vec::new(); - if let Some(change) = Change::between( - before.modpack_version_id.as_ref().filter(|id| !id.is_empty()), - after.modpack_version_id.as_ref().filter(|id| !id.is_empty()), - ) { - changes.push(ConfigurationDiff::Modpack(change)); - } - if let Some(change) = Change::between(Some(&before.game_version), Some(&after.game_version)) { - changes.push(ConfigurationDiff::GameVersion(change)); - } - let normalize_loader = |loader: &LoaderReference| LoaderReference { - name: loader.name.to_lowercase(), - version: loader.version.clone().filter(|version| !version.is_empty()), - }; - if let Some(change) = Change::between( - Some(&normalize_loader(&before.loader)), - Some(&normalize_loader(&after.loader)), - ) { - changes.push(ConfigurationDiff::Loader(change)); - } - changes + let mut changes = Vec::new(); + if let Some(change) = Change::between( + before + .modpack_version_id + .as_ref() + .filter(|id| !id.is_empty()), + after + .modpack_version_id + .as_ref() + .filter(|id| !id.is_empty()), + ) { + changes.push(ConfigurationDiff::Modpack(change)); + } + if let Some(change) = + Change::between(Some(&before.game_version), Some(&after.game_version)) + { + changes.push(ConfigurationDiff::GameVersion(change)); + } + let normalize_loader = |loader: &LoaderReference| LoaderReference { + name: loader.name.to_lowercase(), + version: loader.version.clone().filter(|version| !version.is_empty()), + }; + if let Some(change) = Change::between( + Some(&normalize_loader(&before.loader)), + Some(&normalize_loader(&after.loader)), + ) { + changes.push(ConfigurationDiff::Loader(change)); + } + changes } diff --git a/packages/modrinth-content-management/src/diff/mod.rs b/packages/modrinth-content-management/src/diff/mod.rs index b819edd928..645b5a585c 100644 --- a/packages/modrinth-content-management/src/diff/mod.rs +++ b/packages/modrinth-content-management/src/diff/mod.rs @@ -3,13 +3,13 @@ use std::collections::BTreeSet; pub use configuration::{ - ConfigurationDiff, ContentSetConfiguration, LoaderReference, - diff_configuration, + ConfigurationDiff, ContentSetConfiguration, LoaderReference, + diff_configuration, }; pub use model::{ - Change, CommonExternalFilePolicy, ContentSetDiff, ContentSetDiffEntry, - ContentSetDiffKind, ContentSetDiffOptions, ContentSetSnapshot, - ExternalFileKey, + Change, CommonExternalFilePolicy, ContentSetDiff, ContentSetDiffEntry, + ContentSetDiffKind, ContentSetDiffOptions, ContentSetSnapshot, + ExternalFileKey, }; /// Computes changes from `before` to `after`, ordered by project and file identity. @@ -17,30 +17,51 @@ pub use model::{ /// Callers select the publishing or installation scope before comparison. /// External files are compared by identity, using the supplied policy for common files. pub fn diff_content_sets( - before: &ContentSetSnapshot, - after: &ContentSetSnapshot, - options: &ContentSetDiffOptions, + before: &ContentSetSnapshot, + after: &ContentSetSnapshot, + options: &ContentSetDiffOptions, ) -> ContentSetDiff { - let mut content = Vec::new(); - let project_ids = before.projects.keys().chain(after.projects.keys()).collect::>(); - for project_id in project_ids { - if let Some(change) = Change::between(before.projects.get(project_id), after.projects.get(project_id)) { - content.push(ContentSetDiffEntry::Project { - project_id: project_id.clone(), - change, - }); - } - } - for file in before.external_files.union(&after.external_files) { - let kind = match (before.external_files.contains(file), after.external_files.contains(file)) { - (false, true) => ContentSetDiffKind::Added, - (true, false) => ContentSetDiffKind::Removed, - (true, true) if options.common_external_files == CommonExternalFilePolicy::AssumeUpdated => ContentSetDiffKind::Updated, - _ => continue, - }; - content.push(ContentSetDiffEntry::ExternalFile { file: file.clone(), kind }); - } - ContentSetDiff { content, additional: Vec::new() } + let mut content = Vec::new(); + let project_ids = before + .projects + .keys() + .chain(after.projects.keys()) + .collect::>(); + for project_id in project_ids { + if let Some(change) = Change::between( + before.projects.get(project_id), + after.projects.get(project_id), + ) { + content.push(ContentSetDiffEntry::Project { + project_id: project_id.clone(), + change, + }); + } + } + for file in before.external_files.union(&after.external_files) { + let kind = match ( + before.external_files.contains(file), + after.external_files.contains(file), + ) { + (false, true) => ContentSetDiffKind::Added, + (true, false) => ContentSetDiffKind::Removed, + (true, true) + if options.common_external_files + == CommonExternalFilePolicy::AssumeUpdated => + { + ContentSetDiffKind::Updated + } + _ => continue, + }; + content.push(ContentSetDiffEntry::ExternalFile { + file: file.clone(), + kind, + }); + } + ContentSetDiff { + content, + additional: Vec::new(), + } } mod configuration; diff --git a/packages/modrinth-content-management/src/diff/model.rs b/packages/modrinth-content-management/src/diff/model.rs index 9f7e10687b..90395b71a5 100644 --- a/packages/modrinth-content-management/src/diff/model.rs +++ b/packages/modrinth-content-management/src/diff/model.rs @@ -8,161 +8,174 @@ use crate::shared::{ContentType, Error}; /// A caller-prepared set of content in the scope being compared. #[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] pub struct ContentSetSnapshot { - /// Modrinth project IDs mapped to version IDs. - pub projects: BTreeMap, - pub external_files: BTreeSet, + /// Modrinth project IDs mapped to version IDs. + pub projects: BTreeMap, + pub external_files: BTreeSet, } impl ContentSetSnapshot { - /// Adds a project, rejecting conflicting versions without replacing it. - pub fn insert_project( - &mut self, - project_id: String, - version_id: String, - ) -> Result<(), Error> { - if let Some(existing) = self.projects.get(&project_id) { - if existing != &version_id { - return Err(Error::ConflictingProjectVersions { - project_id, - before: existing.clone(), - after: version_id, - }); - } - } else { - self.projects.insert(project_id, version_id); - } - Ok(()) - } + /// Adds a project, rejecting conflicting versions without replacing it. + pub fn insert_project( + &mut self, + project_id: String, + version_id: String, + ) -> Result<(), Error> { + if let Some(existing) = self.projects.get(&project_id) { + if existing != &version_id { + return Err(Error::ConflictingProjectVersions { + project_id, + before: existing.clone(), + after: version_id, + }); + } + } else { + self.projects.insert(project_id, version_id); + } + Ok(()) + } } /// Identifies a file by its content type and path within that type's directory. -#[derive(Clone, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] +#[derive( + Clone, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize, +)] pub struct ExternalFileKey { - pub content_type: ContentType, - pub path: String, + pub content_type: ContentType, + pub path: String, } #[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)] #[serde(rename_all = "snake_case")] pub enum ContentSetDiffKind { - Added, - Removed, - Updated, + Added, + Removed, + Updated, } #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] #[serde(tag = "kind", rename_all = "snake_case")] pub enum Change { - Added { after: T }, - Removed { before: T }, - Updated { before: T, after: T }, + Added { after: T }, + Removed { before: T }, + Updated { before: T, after: T }, } impl Change { - /// Transforms the values while preserving the classification of the change. - pub fn map(self, mut map: impl FnMut(T) -> U) -> Change { - match self { - Self::Added { after } => Change::Added { after: map(after) }, - Self::Removed { before } => Change::Removed { before: map(before) }, - Self::Updated { before, after } => Change::Updated { - before: map(before), - after: map(after), - }, - } - } - - /// Returns the classification of this change. - pub fn kind(&self) -> ContentSetDiffKind { - match self { - Self::Added { .. } => ContentSetDiffKind::Added, - Self::Removed { .. } => ContentSetDiffKind::Removed, - Self::Updated { .. } => ContentSetDiffKind::Updated, - } - } - - /// Returns the value before the change, if it existed. - pub fn before(&self) -> Option<&T> { - match self { - Self::Removed { before } | Self::Updated { before, .. } => Some(before), - Self::Added { .. } => None, - } - } - - /// Returns the value after the change, if it exists. - pub fn after(&self) -> Option<&T> { - match self { - Self::Added { after } | Self::Updated { after, .. } => Some(after), - Self::Removed { .. } => None, - } - } + /// Transforms the values while preserving the classification of the change. + pub fn map(self, mut map: impl FnMut(T) -> U) -> Change { + match self { + Self::Added { after } => Change::Added { after: map(after) }, + Self::Removed { before } => Change::Removed { + before: map(before), + }, + Self::Updated { before, after } => Change::Updated { + before: map(before), + after: map(after), + }, + } + } + + /// Returns the classification of this change. + pub fn kind(&self) -> ContentSetDiffKind { + match self { + Self::Added { .. } => ContentSetDiffKind::Added, + Self::Removed { .. } => ContentSetDiffKind::Removed, + Self::Updated { .. } => ContentSetDiffKind::Updated, + } + } + + /// Returns the value before the change, if it existed. + pub fn before(&self) -> Option<&T> { + match self { + Self::Removed { before } | Self::Updated { before, .. } => { + Some(before) + } + Self::Added { .. } => None, + } + } + + /// Returns the value after the change, if it exists. + pub fn after(&self) -> Option<&T> { + match self { + Self::Added { after } | Self::Updated { after, .. } => Some(after), + Self::Removed { .. } => None, + } + } } impl Change { - /// Compares optional values, returning no entry when they are equal. - pub fn between(before: Option<&T>, after: Option<&T>) -> Option { - match (before, after) { - (None, Some(after)) => Some(Self::Added { after: after.clone() }), - (Some(before), None) => Some(Self::Removed { before: before.clone() }), - (Some(before), Some(after)) if before != after => Some(Self::Updated { - before: before.clone(), - after: after.clone(), - }), - _ => None, - } - } + /// Compares optional values, returning no entry when they are equal. + pub fn between(before: Option<&T>, after: Option<&T>) -> Option { + match (before, after) { + (None, Some(after)) => Some(Self::Added { + after: after.clone(), + }), + (Some(before), None) => Some(Self::Removed { + before: before.clone(), + }), + (Some(before), Some(after)) if before != after => { + Some(Self::Updated { + before: before.clone(), + after: after.clone(), + }) + } + _ => None, + } + } } #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] #[serde(tag = "type", rename_all = "snake_case")] pub enum ContentSetDiffEntry { - Project { - project_id: String, - change: Change, - }, - ExternalFile { - file: ExternalFileKey, - kind: ContentSetDiffKind, - }, + Project { + project_id: String, + change: Change, + }, + ExternalFile { + file: ExternalFileKey, + kind: ContentSetDiffKind, + }, } /// Policy for files present on both sides when their bytes are not compared. -#[derive(Clone, Copy, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] +#[derive( + Clone, Copy, Debug, Default, Deserialize, Eq, PartialEq, Serialize, +)] #[serde(rename_all = "snake_case")] pub enum CommonExternalFilePolicy { - #[default] - AssumeUnchanged, - AssumeUpdated, + #[default] + AssumeUnchanged, + AssumeUpdated, } #[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] pub struct ContentSetDiffOptions { - pub common_external_files: CommonExternalFilePolicy, + pub common_external_files: CommonExternalFilePolicy, } /// Content changes and typed entries contributed by the caller. #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] pub struct ContentSetDiff { - pub content: Vec, - pub additional: Vec, + pub content: Vec, + pub additional: Vec, } impl ContentSetDiff { - /// Includes both content changes and caller-provided changes. - pub fn has_changes(&self) -> bool { - !self.content.is_empty() || !self.additional.is_empty() - } + /// Includes both content changes and caller-provided changes. + pub fn has_changes(&self) -> bool { + !self.content.is_empty() || !self.additional.is_empty() + } } impl ContentSetDiff { - /// Attaches typed changes without coupling the comparator to the caller. - pub fn with_additional( - self, - entries: impl IntoIterator, - ) -> ContentSetDiff { - ContentSetDiff { - content: self.content, - additional: entries.into_iter().collect(), - } - } + /// Attaches typed changes without coupling the comparator to the caller. + pub fn with_additional( + self, + entries: impl IntoIterator, + ) -> ContentSetDiff { + ContentSetDiff { + content: self.content, + additional: entries.into_iter().collect(), + } + } } - diff --git a/packages/modrinth-content-management/src/install/mod.rs b/packages/modrinth-content-management/src/install/mod.rs index a259dce472..feed3a4003 100644 --- a/packages/modrinth-content-management/src/install/mod.rs +++ b/packages/modrinth-content-management/src/install/mod.rs @@ -6,9 +6,9 @@ use std::collections::{HashMap, HashSet}; use crate::shared::{ContentType, Error}; pub use model::{ - Dependency, DependencyType, ResolutionPreferences, ResolveContentPlan, - ResolveContentRequest, ResolvedContent, SkippedContent, SkippedReason, - Version, + Dependency, DependencyType, ResolutionPreferences, ResolveContentPlan, + ResolveContentRequest, ResolvedContent, SkippedContent, SkippedReason, + Version, }; pub use provider::ContentMetadataProvider; diff --git a/packages/modrinth-content-management/src/lib.rs b/packages/modrinth-content-management/src/lib.rs index f7dadc4c60..b61576ef0c 100644 --- a/packages/modrinth-content-management/src/lib.rs +++ b/packages/modrinth-content-management/src/lib.rs @@ -2,15 +2,15 @@ pub mod diff; pub mod install; pub use diff::{ - Change, CommonExternalFilePolicy, ConfigurationDiff, ContentSetConfiguration, - ContentSetDiff, ContentSetDiffEntry, ContentSetDiffKind, ContentSetDiffOptions, - ContentSetSnapshot, ExternalFileKey, LoaderReference, diff_configuration, - diff_content_sets, + Change, CommonExternalFilePolicy, ConfigurationDiff, + ContentSetConfiguration, ContentSetDiff, ContentSetDiffEntry, + ContentSetDiffKind, ContentSetDiffOptions, ContentSetSnapshot, + ExternalFileKey, LoaderReference, diff_configuration, diff_content_sets, }; pub use install::{ - ContentMetadataProvider, Dependency, DependencyType, ResolutionPreferences, - ResolveContentPlan, ResolveContentRequest, ResolvedContent, SkippedContent, - SkippedReason, Version, resolve_content, + ContentMetadataProvider, Dependency, DependencyType, ResolutionPreferences, + ResolveContentPlan, ResolveContentRequest, ResolvedContent, SkippedContent, + SkippedReason, Version, resolve_content, }; pub use shared::{ContentType, Error}; diff --git a/packages/modrinth-content-management/src/shared/mod.rs b/packages/modrinth-content-management/src/shared/mod.rs index aaa774f73a..4e87c3e9d4 100644 --- a/packages/modrinth-content-management/src/shared/mod.rs +++ b/packages/modrinth-content-management/src/shared/mod.rs @@ -4,14 +4,16 @@ use serde::{Deserialize, Serialize}; #[derive(thiserror::Error, Debug)] pub enum Error { - #[error("unknown content type `{0}`")] - UnknownContentType(String), - #[error("project `{project_id}` has conflicting versions `{before}` and `{after}`")] - ConflictingProjectVersions { - project_id: String, - before: String, - after: String, - }, + #[error("unknown content type `{0}`")] + UnknownContentType(String), + #[error( + "project `{project_id}` has conflicting versions `{before}` and `{after}`" + )] + ConflictingProjectVersions { + project_id: String, + before: String, + after: String, + }, #[error("metadata provider error: {0}")] Provider(String), #[error("project `{0}` was not found")] @@ -27,7 +29,18 @@ pub enum Error { NoCompatibleVersion(String), } -#[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize)] +#[derive( + Clone, + Copy, + Debug, + Deserialize, + Eq, + Hash, + Ord, + PartialEq, + PartialOrd, + Serialize, +)] #[serde(rename_all = "lowercase")] pub enum ContentType { Mod, @@ -39,32 +52,31 @@ pub enum ContentType { } impl std::str::FromStr for ContentType { - type Err = Error; + type Err = Error; - fn from_str(value: &str) -> Result { - match value { - "mod" => Ok(Self::Mod), - "plugin" => Ok(Self::Plugin), - "datapack" => Ok(Self::DataPack), - "resourcepack" => Ok(Self::ResourcePack), - "shader" => Ok(Self::Shader), - "modpack" => Ok(Self::ModPack), - _ => Err(Error::UnknownContentType(value.to_owned())), - } - } + fn from_str(value: &str) -> Result { + match value { + "mod" => Ok(Self::Mod), + "plugin" => Ok(Self::Plugin), + "datapack" => Ok(Self::DataPack), + "resourcepack" => Ok(Self::ResourcePack), + "shader" => Ok(Self::Shader), + "modpack" => Ok(Self::ModPack), + _ => Err(Error::UnknownContentType(value.to_owned())), + } + } } impl ContentType { - /// Returns the canonical content type used by shared-instance manifests. - pub fn as_str(self) -> &'static str { - match self { - Self::Mod => "mod", - Self::Plugin => "plugin", - Self::DataPack => "datapack", - Self::ResourcePack => "resourcepack", - Self::Shader => "shader", - Self::ModPack => "modpack", - } - } + /// Returns the canonical content type used by shared-instance manifests. + pub fn as_str(self) -> &'static str { + match self { + Self::Mod => "mod", + Self::Plugin => "plugin", + Self::DataPack => "datapack", + Self::ResourcePack => "resourcepack", + Self::Shader => "shader", + Self::ModPack => "modpack", + } + } } - diff --git a/packages/modrinth-content-management/tests/diff.rs b/packages/modrinth-content-management/tests/diff.rs index 60c5301b8a..61255d0043 100644 --- a/packages/modrinth-content-management/tests/diff.rs +++ b/packages/modrinth-content-management/tests/diff.rs @@ -1,399 +1,529 @@ use modrinth_content_management::{ - Change, CommonExternalFilePolicy, ConfigurationDiff, ContentSetConfiguration, - ContentSetDiffEntry, ContentSetDiffKind, ContentSetDiffOptions, - ContentSetSnapshot, ContentType, Error, ExternalFileKey, LoaderReference, diff_configuration, - diff_content_sets, + Change, CommonExternalFilePolicy, ConfigurationDiff, + ContentSetConfiguration, ContentSetDiffEntry, ContentSetDiffKind, + ContentSetDiffOptions, ContentSetSnapshot, ContentType, Error, + ExternalFileKey, LoaderReference, diff_configuration, diff_content_sets, }; fn snapshot( - projects: &[(&str, &str)], - files: &[(ContentType, &str)], + projects: &[(&str, &str)], + files: &[(ContentType, &str)], ) -> ContentSetSnapshot { - ContentSetSnapshot { - projects: projects.iter() - .map(|(project, version)| (project.to_string(), version.to_string())) - .collect(), - external_files: files.iter() - .map(|(content_type, path)| file(*content_type, path)) - .collect(), - } + ContentSetSnapshot { + projects: projects + .iter() + .map(|(project, version)| { + (project.to_string(), version.to_string()) + }) + .collect(), + external_files: files + .iter() + .map(|(content_type, path)| file(*content_type, path)) + .collect(), + } } fn file(content_type: ContentType, path: &str) -> ExternalFileKey { - ExternalFileKey { content_type, path: path.to_string() } + ExternalFileKey { + content_type, + path: path.to_string(), + } } fn configuration( - modpack_version_id: Option<&str>, - game_version: &str, - loader: &str, - loader_version: Option<&str>, + modpack_version_id: Option<&str>, + game_version: &str, + loader: &str, + loader_version: Option<&str>, ) -> ContentSetConfiguration { - ContentSetConfiguration { - modpack_version_id: modpack_version_id.map(str::to_string), - game_version: game_version.to_string(), - loader: LoaderReference { - name: loader.to_string(), - version: loader_version.map(str::to_string), - }, - } + ContentSetConfiguration { + modpack_version_id: modpack_version_id.map(str::to_string), + game_version: game_version.to_string(), + loader: LoaderReference { + name: loader.to_string(), + version: loader_version.map(str::to_string), + }, + } } #[test] fn empty_and_identical_snapshots_have_no_changes() { - for content in [ - ContentSetSnapshot::default(), - snapshot(&[("sodium", "v1")], &[(ContentType::Mod, "custom.jar")]), - ] { - let diff = diff_content_sets(&content, &content, &ContentSetDiffOptions::default()); - assert!(!diff.has_changes()); - assert!(diff.content.is_empty()); - assert!(diff.additional.is_empty()); - } + for content in [ + ContentSetSnapshot::default(), + snapshot(&[("sodium", "v1")], &[(ContentType::Mod, "custom.jar")]), + ] { + let diff = diff_content_sets( + &content, + &content, + &ContentSetDiffOptions::default(), + ); + assert!(!diff.has_changes()); + assert!(diff.content.is_empty()); + assert!(diff.additional.is_empty()); + } } #[test] fn project_changes_preserve_identity_and_both_version_ids() { - let before = snapshot( - &[("unchanged", "v1"), ("updated", "old"), ("removed", "v2")], - &[], - ); - let after = snapshot( - &[("updated", "new"), ("added", "v3"), ("unchanged", "v1")], - &[], - ); - - let diff = diff_content_sets(&before, &after, &ContentSetDiffOptions::default()); - - assert!(diff.has_changes()); - assert_eq!(diff.content, vec![ - ContentSetDiffEntry::Project { - project_id: "added".to_string(), - change: Change::Added { after: "v3".to_string() }, - }, - ContentSetDiffEntry::Project { - project_id: "removed".to_string(), - change: Change::Removed { before: "v2".to_string() }, - }, - ContentSetDiffEntry::Project { - project_id: "updated".to_string(), - change: Change::Updated { - before: "old".to_string(), - after: "new".to_string(), - }, - }, - ]); + let before = snapshot( + &[("unchanged", "v1"), ("updated", "old"), ("removed", "v2")], + &[], + ); + let after = snapshot( + &[("updated", "new"), ("added", "v3"), ("unchanged", "v1")], + &[], + ); + + let diff = + diff_content_sets(&before, &after, &ContentSetDiffOptions::default()); + + assert!(diff.has_changes()); + assert_eq!( + diff.content, + vec![ + ContentSetDiffEntry::Project { + project_id: "added".to_string(), + change: Change::Added { + after: "v3".to_string() + }, + }, + ContentSetDiffEntry::Project { + project_id: "removed".to_string(), + change: Change::Removed { + before: "v2".to_string() + }, + }, + ContentSetDiffEntry::Project { + project_id: "updated".to_string(), + change: Change::Updated { + before: "old".to_string(), + after: "new".to_string(), + }, + }, + ] + ); } #[test] fn reversing_snapshots_reverses_project_and_file_changes() { - let before = snapshot( - &[("updated", "old"), ("removed", "v1")], - &[(ContentType::Mod, "removed.jar")], - ); - let after = snapshot( - &[("updated", "new"), ("added", "v2")], - &[(ContentType::Mod, "added.jar")], - ); - - let reverse = diff_content_sets(&after, &before, &ContentSetDiffOptions::default()); - - assert_eq!(reverse.content, vec![ - ContentSetDiffEntry::Project { - project_id: "added".to_string(), - change: Change::Removed { before: "v2".to_string() }, - }, - ContentSetDiffEntry::Project { - project_id: "removed".to_string(), - change: Change::Added { after: "v1".to_string() }, - }, - ContentSetDiffEntry::Project { - project_id: "updated".to_string(), - change: Change::Updated { - before: "new".to_string(), - after: "old".to_string(), - }, - }, - ContentSetDiffEntry::ExternalFile { - file: file(ContentType::Mod, "added.jar"), - kind: ContentSetDiffKind::Removed, - }, - ContentSetDiffEntry::ExternalFile { - file: file(ContentType::Mod, "removed.jar"), - kind: ContentSetDiffKind::Added, - }, - ]); + let before = snapshot( + &[("updated", "old"), ("removed", "v1")], + &[(ContentType::Mod, "removed.jar")], + ); + let after = snapshot( + &[("updated", "new"), ("added", "v2")], + &[(ContentType::Mod, "added.jar")], + ); + + let reverse = + diff_content_sets(&after, &before, &ContentSetDiffOptions::default()); + + assert_eq!( + reverse.content, + vec![ + ContentSetDiffEntry::Project { + project_id: "added".to_string(), + change: Change::Removed { + before: "v2".to_string() + }, + }, + ContentSetDiffEntry::Project { + project_id: "removed".to_string(), + change: Change::Added { + after: "v1".to_string() + }, + }, + ContentSetDiffEntry::Project { + project_id: "updated".to_string(), + change: Change::Updated { + before: "new".to_string(), + after: "old".to_string(), + }, + }, + ContentSetDiffEntry::ExternalFile { + file: file(ContentType::Mod, "added.jar"), + kind: ContentSetDiffKind::Removed, + }, + ContentSetDiffEntry::ExternalFile { + file: file(ContentType::Mod, "removed.jar"), + kind: ContentSetDiffKind::Added, + }, + ] + ); } #[test] fn common_file_policy_only_affects_files_present_on_both_sides() { - let before = snapshot(&[], &[ - (ContentType::Mod, "common.jar"), - (ContentType::Mod, "removed.jar"), - ]); - let after = snapshot(&[], &[ - (ContentType::Mod, "added.jar"), - (ContentType::Mod, "common.jar"), - ]); - let added = ContentSetDiffEntry::ExternalFile { - file: file(ContentType::Mod, "added.jar"), - kind: ContentSetDiffKind::Added, - }; - let removed = ContentSetDiffEntry::ExternalFile { - file: file(ContentType::Mod, "removed.jar"), - kind: ContentSetDiffKind::Removed, - }; - let common = ContentSetDiffEntry::ExternalFile { - file: file(ContentType::Mod, "common.jar"), - kind: ContentSetDiffKind::Updated, - }; - for (policy, expected) in [ - (CommonExternalFilePolicy::AssumeUnchanged, vec![added.clone(), removed.clone()]), - (CommonExternalFilePolicy::AssumeUpdated, vec![added, common, removed]), - ] { - let diff = diff_content_sets(&before, &after, &ContentSetDiffOptions { - common_external_files: policy, - }); - assert_eq!(diff.content, expected, "{policy:?}"); - } + let before = snapshot( + &[], + &[ + (ContentType::Mod, "common.jar"), + (ContentType::Mod, "removed.jar"), + ], + ); + let after = snapshot( + &[], + &[ + (ContentType::Mod, "added.jar"), + (ContentType::Mod, "common.jar"), + ], + ); + let added = ContentSetDiffEntry::ExternalFile { + file: file(ContentType::Mod, "added.jar"), + kind: ContentSetDiffKind::Added, + }; + let removed = ContentSetDiffEntry::ExternalFile { + file: file(ContentType::Mod, "removed.jar"), + kind: ContentSetDiffKind::Removed, + }; + let common = ContentSetDiffEntry::ExternalFile { + file: file(ContentType::Mod, "common.jar"), + kind: ContentSetDiffKind::Updated, + }; + for (policy, expected) in [ + ( + CommonExternalFilePolicy::AssumeUnchanged, + vec![added.clone(), removed.clone()], + ), + ( + CommonExternalFilePolicy::AssumeUpdated, + vec![added, common, removed], + ), + ] { + let diff = diff_content_sets( + &before, + &after, + &ContentSetDiffOptions { + common_external_files: policy, + }, + ); + assert_eq!(diff.content, expected, "{policy:?}"); + } } #[test] fn assumed_common_file_update_counts_as_a_change() { - let content = snapshot(&[], &[(ContentType::ResourcePack, "textures.zip")]); - let diff = diff_content_sets(&content, &content, &ContentSetDiffOptions { - common_external_files: CommonExternalFilePolicy::AssumeUpdated, - }); - - assert!(diff.has_changes()); - assert_eq!(diff.content, vec![ContentSetDiffEntry::ExternalFile { - file: file(ContentType::ResourcePack, "textures.zip"), - kind: ContentSetDiffKind::Updated, - }]); + let content = snapshot(&[], &[(ContentType::ResourcePack, "textures.zip")]); + let diff = diff_content_sets( + &content, + &content, + &ContentSetDiffOptions { + common_external_files: CommonExternalFilePolicy::AssumeUpdated, + }, + ); + + assert!(diff.has_changes()); + assert_eq!( + diff.content, + vec![ContentSetDiffEntry::ExternalFile { + file: file(ContentType::ResourcePack, "textures.zip"), + kind: ContentSetDiffKind::Updated, + }] + ); } #[test] fn file_identity_includes_content_type_and_relative_path() { - let before = snapshot(&[], &[ - (ContentType::Mod, "shared.jar"), - (ContentType::DataPack, "first/data.zip"), - ]); - let after = snapshot(&[], &[ - (ContentType::Plugin, "shared.jar"), - (ContentType::DataPack, "second/data.zip"), - ]); - - let diff = diff_content_sets(&before, &after, &ContentSetDiffOptions::default()); - - assert_eq!(diff.content, vec![ - ContentSetDiffEntry::ExternalFile { - file: file(ContentType::Mod, "shared.jar"), - kind: ContentSetDiffKind::Removed, - }, - ContentSetDiffEntry::ExternalFile { - file: file(ContentType::Plugin, "shared.jar"), - kind: ContentSetDiffKind::Added, - }, - ContentSetDiffEntry::ExternalFile { - file: file(ContentType::DataPack, "first/data.zip"), - kind: ContentSetDiffKind::Removed, - }, - ContentSetDiffEntry::ExternalFile { - file: file(ContentType::DataPack, "second/data.zip"), - kind: ContentSetDiffKind::Added, - }, - ]); + let before = snapshot( + &[], + &[ + (ContentType::Mod, "shared.jar"), + (ContentType::DataPack, "first/data.zip"), + ], + ); + let after = snapshot( + &[], + &[ + (ContentType::Plugin, "shared.jar"), + (ContentType::DataPack, "second/data.zip"), + ], + ); + + let diff = + diff_content_sets(&before, &after, &ContentSetDiffOptions::default()); + + assert_eq!( + diff.content, + vec![ + ContentSetDiffEntry::ExternalFile { + file: file(ContentType::Mod, "shared.jar"), + kind: ContentSetDiffKind::Removed, + }, + ContentSetDiffEntry::ExternalFile { + file: file(ContentType::Plugin, "shared.jar"), + kind: ContentSetDiffKind::Added, + }, + ContentSetDiffEntry::ExternalFile { + file: file(ContentType::DataPack, "first/data.zip"), + kind: ContentSetDiffKind::Removed, + }, + ContentSetDiffEntry::ExternalFile { + file: file(ContentType::DataPack, "second/data.zip"), + kind: ContentSetDiffKind::Added, + }, + ] + ); } #[test] fn diff_order_is_independent_of_snapshot_insertion_order() { - let first = snapshot( - &[("z", "v2"), ("a", "v1")], - &[(ContentType::Mod, "z.jar"), (ContentType::Mod, "a.jar")], - ); - let second = snapshot( - &[("a", "v1"), ("z", "v2")], - &[(ContentType::Mod, "a.jar"), (ContentType::Mod, "z.jar")], - ); - let empty = ContentSetSnapshot::default(); - let options = ContentSetDiffOptions::default(); - - assert_eq!( - diff_content_sets(&empty, &first, &options), - diff_content_sets(&empty, &second, &options), - ); + let first = snapshot( + &[("z", "v2"), ("a", "v1")], + &[(ContentType::Mod, "z.jar"), (ContentType::Mod, "a.jar")], + ); + let second = snapshot( + &[("a", "v1"), ("z", "v2")], + &[(ContentType::Mod, "a.jar"), (ContentType::Mod, "z.jar")], + ); + let empty = ContentSetSnapshot::default(); + let options = ContentSetDiffOptions::default(); + + assert_eq!( + diff_content_sets(&empty, &first, &options), + diff_content_sets(&empty, &second, &options), + ); } #[test] fn inserting_the_same_project_version_is_idempotent() { - let mut content = ContentSetSnapshot::default(); - content.insert_project("sodium".to_string(), "v1".to_string()).unwrap(); - let original = content.clone(); + let mut content = ContentSetSnapshot::default(); + content + .insert_project("sodium".to_string(), "v1".to_string()) + .unwrap(); + let original = content.clone(); - content.insert_project("sodium".to_string(), "v1".to_string()).unwrap(); + content + .insert_project("sodium".to_string(), "v1".to_string()) + .unwrap(); - assert_eq!(content, original); + assert_eq!(content, original); } #[test] fn conflicting_project_version_is_rejected_without_changing_the_snapshot() { - let mut content = snapshot(&[("sodium", "v1")], &[]); - let original = content.clone(); - - let error = content.insert_project("sodium".to_string(), "v2".to_string()).unwrap_err(); - - assert!(matches!(error, Error::ConflictingProjectVersions { project_id, before, after } - if project_id == "sodium" && before == "v1" && after == "v2")); - assert_eq!(content, original); + let mut content = snapshot(&[("sodium", "v1")], &[]); + let original = content.clone(); + + let error = content + .insert_project("sodium".to_string(), "v2".to_string()) + .unwrap_err(); + + assert!( + matches!(error, Error::ConflictingProjectVersions { project_id, before, after } + if project_id == "sodium" && before == "v1" && after == "v2") + ); + assert_eq!(content, original); } #[derive(Debug, PartialEq)] enum AdditionalChange { - ConfigFilesUpdated { file_count: usize }, + ConfigFilesUpdated { file_count: usize }, } #[test] fn caller_entries_can_trigger_changes_without_content_changes() { - let content = ContentSetSnapshot::default(); - let base = diff_content_sets(&content, &content, &ContentSetDiffOptions::default()); - let mut diff = base.with_additional(Vec::::new()); - assert!(!diff.has_changes()); - - diff.additional.push(AdditionalChange::ConfigFilesUpdated { file_count: 2 }); - - assert!(diff.content.is_empty()); - assert!(diff.has_changes()); - assert_eq!(diff.additional, vec![AdditionalChange::ConfigFilesUpdated { file_count: 2 }]); + let content = ContentSetSnapshot::default(); + let base = diff_content_sets( + &content, + &content, + &ContentSetDiffOptions::default(), + ); + let mut diff = base.with_additional(Vec::::new()); + assert!(!diff.has_changes()); + + diff.additional + .push(AdditionalChange::ConfigFilesUpdated { file_count: 2 }); + + assert!(diff.content.is_empty()); + assert!(diff.has_changes()); + assert_eq!( + diff.additional, + vec![AdditionalChange::ConfigFilesUpdated { file_count: 2 }] + ); } #[test] fn attaching_caller_entries_preserves_content_changes() { - let before = ContentSetSnapshot::default(); - let after = snapshot(&[("sodium", "v1")], &[]); - let base = diff_content_sets(&before, &after, &ContentSetDiffOptions::default()); - let content = base.content.clone(); - let mut diff = base.with_additional([ - AdditionalChange::ConfigFilesUpdated { file_count: 1 }, - ]); - - assert_eq!(diff.content, content); - assert_eq!(diff.additional.len(), 1); - diff.additional.clear(); - assert!(diff.has_changes()); + let before = ContentSetSnapshot::default(); + let after = snapshot(&[("sodium", "v1")], &[]); + let base = + diff_content_sets(&before, &after, &ContentSetDiffOptions::default()); + let content = base.content.clone(); + let mut diff = + base.with_additional([AdditionalChange::ConfigFilesUpdated { + file_count: 1, + }]); + + assert_eq!(diff.content, content); + assert_eq!(diff.additional.len(), 1); + diff.additional.clear(); + assert!(diff.has_changes()); } #[test] fn modpack_link_unlink_and_update_have_directional_version_ids() { - for (before_id, after_id, expected) in [ - (None, Some("pack-v1"), Change::Added { after: "pack-v1".to_string() }), - (Some("pack-v1"), None, Change::Removed { before: "pack-v1".to_string() }), - (Some("pack-v1"), Some("pack-v2"), Change::Updated { - before: "pack-v1".to_string(), - after: "pack-v2".to_string(), - }), - ] { - let before = configuration(before_id, "1.21.1", "fabric", Some("0.16.0")); - let after = configuration(after_id, "1.21.1", "fabric", Some("0.16.0")); - - assert_eq!(diff_configuration(&before, &after), vec![ConfigurationDiff::Modpack(expected)]); - } + for (before_id, after_id, expected) in [ + ( + None, + Some("pack-v1"), + Change::Added { + after: "pack-v1".to_string(), + }, + ), + ( + Some("pack-v1"), + None, + Change::Removed { + before: "pack-v1".to_string(), + }, + ), + ( + Some("pack-v1"), + Some("pack-v2"), + Change::Updated { + before: "pack-v1".to_string(), + after: "pack-v2".to_string(), + }, + ), + ] { + let before = + configuration(before_id, "1.21.1", "fabric", Some("0.16.0")); + let after = configuration(after_id, "1.21.1", "fabric", Some("0.16.0")); + + assert_eq!( + diff_configuration(&before, &after), + vec![ConfigurationDiff::Modpack(expected)] + ); + } } #[test] fn configuration_normalization_does_not_create_false_changes() { - let empty = configuration(None, "1.21.1", "fabric", None); - let explicit_empty = configuration(Some(""), "1.21.1", "Fabric", Some("")); - let linked = configuration(Some("pack-v1"), "1.21.1", "NeoForge", Some("21.1.1")); - let canonical = configuration(Some("pack-v1"), "1.21.1", "neoforge", Some("21.1.1")); - - for (before, after) in [(&empty, &explicit_empty), (&linked, &canonical)] { - assert!(diff_configuration(before, after).is_empty()); - assert!(diff_configuration(after, before).is_empty()); - assert!(diff_configuration(before, before).is_empty()); - } + let empty = configuration(None, "1.21.1", "fabric", None); + let explicit_empty = configuration(Some(""), "1.21.1", "Fabric", Some("")); + let linked = + configuration(Some("pack-v1"), "1.21.1", "NeoForge", Some("21.1.1")); + let canonical = + configuration(Some("pack-v1"), "1.21.1", "neoforge", Some("21.1.1")); + + for (before, after) in [(&empty, &explicit_empty), (&linked, &canonical)] { + assert!(diff_configuration(before, after).is_empty()); + assert!(diff_configuration(after, before).is_empty()); + assert!(diff_configuration(before, before).is_empty()); + } } #[test] fn changing_only_the_game_version_produces_one_configuration_entry() { - let before = configuration(None, "1.21.1", "fabric", Some("0.16.0")); - let after = configuration(None, "1.21.2", "fabric", Some("0.16.0")); - - assert_eq!(diff_configuration(&before, &after), vec![ - ConfigurationDiff::GameVersion(Change::Updated { - before: "1.21.1".to_string(), - after: "1.21.2".to_string(), - }), - ]); + let before = configuration(None, "1.21.1", "fabric", Some("0.16.0")); + let after = configuration(None, "1.21.2", "fabric", Some("0.16.0")); + + assert_eq!( + diff_configuration(&before, &after), + vec![ConfigurationDiff::GameVersion(Change::Updated { + before: "1.21.1".to_string(), + after: "1.21.2".to_string(), + }),] + ); } #[test] fn loader_version_changes_include_added_and_removed_optional_versions() { - for (before_version, after_version) in [ - (Some("0.16.0"), Some("0.16.1")), - (None, Some("0.16.0")), - (Some("0.16.0"), None), - ] { - let before = configuration(None, "1.21.1", "fabric", before_version); - let after = configuration(None, "1.21.1", "fabric", after_version); - - assert_eq!(diff_configuration(&before, &after), vec![ - ConfigurationDiff::Loader(Change::Updated { - before: before.loader.clone(), - after: after.loader.clone(), - }), - ]); - } + for (before_version, after_version) in [ + (Some("0.16.0"), Some("0.16.1")), + (None, Some("0.16.0")), + (Some("0.16.0"), None), + ] { + let before = configuration(None, "1.21.1", "fabric", before_version); + let after = configuration(None, "1.21.1", "fabric", after_version); + + assert_eq!( + diff_configuration(&before, &after), + vec![ConfigurationDiff::Loader(Change::Updated { + before: before.loader.clone(), + after: after.loader.clone(), + }),] + ); + } } #[test] fn compatible_loader_names_still_represent_different_installed_loaders() { - let before = configuration(None, "1.21.1", "paper", Some("123")); - let after = configuration(None, "1.21.1", "purpur", Some("123")); - - assert_eq!(diff_configuration(&before, &after), vec![ - ConfigurationDiff::Loader(Change::Updated { - before: before.loader.clone(), - after: after.loader.clone(), - }), - ]); + let before = configuration(None, "1.21.1", "paper", Some("123")); + let after = configuration(None, "1.21.1", "purpur", Some("123")); + + assert_eq!( + diff_configuration(&before, &after), + vec![ConfigurationDiff::Loader(Change::Updated { + before: before.loader.clone(), + after: after.loader.clone(), + }),] + ); } #[test] fn simultaneous_configuration_changes_are_composed_in_stable_order() { - let before = configuration(Some("pack-v1"), "1.21.1", "fabric", Some("0.16.0")); - let after = configuration(Some("pack-v2"), "1.21.2", "neoforge", Some("21.2.1")); - let content = ContentSetSnapshot::default(); - let diff = diff_content_sets(&content, &content, &ContentSetDiffOptions::default()) - .with_additional(diff_configuration(&before, &after)); - - assert!(diff.content.is_empty()); - assert!(diff.has_changes()); - assert_eq!(diff.additional, vec![ - ConfigurationDiff::Modpack(Change::Updated { - before: "pack-v1".to_string(), - after: "pack-v2".to_string(), - }), - ConfigurationDiff::GameVersion(Change::Updated { - before: "1.21.1".to_string(), - after: "1.21.2".to_string(), - }), - ConfigurationDiff::Loader(Change::Updated { - before: before.loader, - after: after.loader, - }), - ]); + let before = + configuration(Some("pack-v1"), "1.21.1", "fabric", Some("0.16.0")); + let after = + configuration(Some("pack-v2"), "1.21.2", "neoforge", Some("21.2.1")); + let content = ContentSetSnapshot::default(); + let diff = diff_content_sets( + &content, + &content, + &ContentSetDiffOptions::default(), + ) + .with_additional(diff_configuration(&before, &after)); + + assert!(diff.content.is_empty()); + assert!(diff.has_changes()); + assert_eq!( + diff.additional, + vec![ + ConfigurationDiff::Modpack(Change::Updated { + before: "pack-v1".to_string(), + after: "pack-v2".to_string(), + }), + ConfigurationDiff::GameVersion(Change::Updated { + before: "1.21.1".to_string(), + after: "1.21.2".to_string(), + }), + ConfigurationDiff::Loader(Change::Updated { + before: before.loader, + after: after.loader, + }), + ] + ); } #[test] fn mapping_changes_preserves_their_kind_and_before_after_values() { - for (change, kind, before, after) in [ - (Change::Added { after: 2 }, ContentSetDiffKind::Added, None, Some("v2")), - (Change::Removed { before: 1 }, ContentSetDiffKind::Removed, Some("v1"), None), - (Change::Updated { before: 1, after: 2 }, ContentSetDiffKind::Updated, Some("v1"), Some("v2")), - ] { - let mapped = change.map(|version| format!("v{version}")); - assert_eq!(mapped.kind(), kind); - assert_eq!(mapped.before().map(String::as_str), before); - assert_eq!(mapped.after().map(String::as_str), after); - } + for (change, kind, before, after) in [ + ( + Change::Added { after: 2 }, + ContentSetDiffKind::Added, + None, + Some("v2"), + ), + ( + Change::Removed { before: 1 }, + ContentSetDiffKind::Removed, + Some("v1"), + None, + ), + ( + Change::Updated { + before: 1, + after: 2, + }, + ContentSetDiffKind::Updated, + Some("v1"), + Some("v2"), + ), + ] { + let mapped = change.map(|version| format!("v{version}")); + assert_eq!(mapped.kind(), kind); + assert_eq!(mapped.before().map(String::as_str), before); + assert_eq!(mapped.after().map(String::as_str), after); + } } From c32984831b305f0ad30011e9b51b9e5128f63c9c Mon Sep 17 00:00:00 2001 From: "Calum H. (IMB11)" Date: Fri, 4 Sep 2026 17:39:24 +0100 Subject: [PATCH 3/3] chore: cleanup --- .../src/api/instance/shared/publish.rs | 3 +- .../src/diff/configuration.rs | 9 +++-- .../src/diff/mod.rs | 12 ++++--- .../src/diff/model.rs | 35 ++++++++++++------- .../src/install/mod.rs | 3 +- .../src/shared/mod.rs | 4 +-- 6 files changed, 41 insertions(+), 25 deletions(-) diff --git a/packages/app-lib/src/api/instance/shared/publish.rs b/packages/app-lib/src/api/instance/shared/publish.rs index e50a3e7c2e..c4c7af1d12 100644 --- a/packages/app-lib/src/api/instance/shared/publish.rs +++ b/packages/app-lib/src/api/instance/shared/publish.rs @@ -226,7 +226,8 @@ pub(super) async fn modpack_dependency_version_ids( .collect()) } -/// Adds inherited pack versions without replacing explicitly selected projects. +/// Adds the modpack's dependencies to the list. If a project is already in the +/// list, keep that version instead of the one bundled with the modpack. async fn extend_shared_modpack_dependencies( version_ids: &mut Vec, modpack_id: &str, diff --git a/packages/modrinth-content-management/src/diff/configuration.rs b/packages/modrinth-content-management/src/diff/configuration.rs index 7e1fa51774..5a70a9e2ce 100644 --- a/packages/modrinth-content-management/src/diff/configuration.rs +++ b/packages/modrinth-content-management/src/diff/configuration.rs @@ -2,7 +2,7 @@ use serde::{Deserialize, Serialize}; use crate::diff::Change; -/// Shared configuration independent of any application's loader or link model. +/// The linked modpack, Minecraft version, and loader settings to compare. #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] pub struct ContentSetConfiguration { pub modpack_version_id: Option, @@ -24,8 +24,11 @@ pub enum ConfigurationDiff { Loader(Change), } -/// Compares modpack, Minecraft and loader configuration in that order. -/// Empty optional version IDs are equivalent to absent IDs. +/// Lists changes to the linked modpack, Minecraft version, and mod loader. +/// +/// Some sources use an empty string where others use `None` for an unset +/// modpack ID or loader version. Treat them the same so this difference alone +/// does not show an update. pub fn diff_configuration( before: &ContentSetConfiguration, after: &ContentSetConfiguration, diff --git a/packages/modrinth-content-management/src/diff/mod.rs b/packages/modrinth-content-management/src/diff/mod.rs index 645b5a585c..b503c7b5be 100644 --- a/packages/modrinth-content-management/src/diff/mod.rs +++ b/packages/modrinth-content-management/src/diff/mod.rs @@ -1,4 +1,5 @@ -//! Compares prepared content sets and composes caller-specific changes. +//! Finds added, removed, and updated content, with room for extra changes +//! such as linking a modpack. use std::collections::BTreeSet; @@ -12,10 +13,13 @@ pub use model::{ ExternalFileKey, }; -/// Computes changes from `before` to `after`, ordered by project and file identity. +/// Lists what would change when replacing `before` with `after`. /// -/// Callers select the publishing or installation scope before comparison. -/// External files are compared by identity, using the supplied policy for common files. +/// Projects are matched by project ID and updated when their version ID changes. +/// Files are matched by content type and path. For matching files, `options` +/// decides whether to show an update; file contents are not checked. +/// +/// Results are sorted by project ID, followed by files sorted by type and path. pub fn diff_content_sets( before: &ContentSetSnapshot, after: &ContentSetSnapshot, diff --git a/packages/modrinth-content-management/src/diff/model.rs b/packages/modrinth-content-management/src/diff/model.rs index 90395b71a5..692435a1bb 100644 --- a/packages/modrinth-content-management/src/diff/model.rs +++ b/packages/modrinth-content-management/src/diff/model.rs @@ -5,16 +5,20 @@ use serde::{Deserialize, Serialize}; use crate::shared::{ContentType, Error}; -/// A caller-prepared set of content in the scope being compared. +/// The projects and files to compare. +/// +/// Include only content relevant to the operation. For example, when receiving +/// a shared-instance update, leave out mods the player added themselves. #[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)] pub struct ContentSetSnapshot { - /// Modrinth project IDs mapped to version IDs. + /// Each project has one version. Keys are project IDs; values are version IDs. pub projects: BTreeMap, pub external_files: BTreeSet, } impl ContentSetSnapshot { - /// Adds a project, rejecting conflicting versions without replacing it. + /// Adds a project's version. Adding the same version again does nothing. + /// Adding a different version for that project returns an error and keeps the original. pub fn insert_project( &mut self, project_id: String, @@ -35,7 +39,9 @@ impl ContentSetSnapshot { } } -/// Identifies a file by its content type and path within that type's directory. +/// A file's type and path, so a mod and a plugin with the same filename are +/// treated as separate files. For example, `mods/example.jar` uses type `Mod` +/// and path `example.jar`. #[derive( Clone, Debug, Deserialize, Eq, Hash, Ord, PartialEq, PartialOrd, Serialize, )] @@ -61,7 +67,7 @@ pub enum Change { } impl Change { - /// Transforms the values while preserving the classification of the change. + /// Converts the before/after values, for example from version IDs to display names. pub fn map(self, mut map: impl FnMut(T) -> U) -> Change { match self { Self::Added { after } => Change::Added { after: map(after) }, @@ -75,7 +81,6 @@ impl Change { } } - /// Returns the classification of this change. pub fn kind(&self) -> ContentSetDiffKind { match self { Self::Added { .. } => ContentSetDiffKind::Added, @@ -84,7 +89,7 @@ impl Change { } } - /// Returns the value before the change, if it existed. + /// Returns `None` for an added item because it had no previous value. pub fn before(&self) -> Option<&T> { match self { Self::Removed { before } | Self::Updated { before, .. } => { @@ -94,7 +99,7 @@ impl Change { } } - /// Returns the value after the change, if it exists. + /// Returns `None` for a removed item because it no longer has a value. pub fn after(&self) -> Option<&T> { match self { Self::Added { after } | Self::Updated { after, .. } => Some(after), @@ -104,7 +109,8 @@ impl Change { } impl Change { - /// Compares optional values, returning no entry when they are equal. + /// Compares the old and new values. `None` means the item is absent. + /// Returns no change if the values are equal or both absent. pub fn between(before: Option<&T>, after: Option<&T>) -> Option { match (before, after) { (None, Some(after)) => Some(Self::Added { @@ -137,7 +143,8 @@ pub enum ContentSetDiffEntry { }, } -/// Policy for files present on both sides when their bytes are not compared. +/// Decides whether to show an update when a file has the same type and path +/// in both sets. The comparison does not read the files or compare their contents. #[derive( Clone, Copy, Debug, Default, Deserialize, Eq, PartialEq, Serialize, )] @@ -153,7 +160,8 @@ pub struct ContentSetDiffOptions { pub common_external_files: CommonExternalFilePolicy, } -/// Content changes and typed entries contributed by the caller. +/// Changes to projects and files, plus any extra changes the app or server adds, +/// such as a linked modpack or selected config files. #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] pub struct ContentSetDiff { pub content: Vec, @@ -161,14 +169,15 @@ pub struct ContentSetDiff { } impl ContentSetDiff { - /// Includes both content changes and caller-provided changes. + /// Returns true for extra changes too, even when no projects or files changed. pub fn has_changes(&self) -> bool { !self.content.is_empty() || !self.additional.is_empty() } } impl ContentSetDiff { - /// Attaches typed changes without coupling the comparator to the caller. + /// Adds other changes to the result, such as a modpack change or selected config files. + /// These count towards `has_changes()` too. pub fn with_additional( self, entries: impl IntoIterator, diff --git a/packages/modrinth-content-management/src/install/mod.rs b/packages/modrinth-content-management/src/install/mod.rs index feed3a4003..fa109437d3 100644 --- a/packages/modrinth-content-management/src/install/mod.rs +++ b/packages/modrinth-content-management/src/install/mod.rs @@ -1,4 +1,4 @@ -//! Resolves requested content and its required dependencies through a metadata provider. +//! Chooses which version of a project to install and finds the dependencies it needs. use std::cmp::Reverse; use std::collections::{HashMap, HashSet}; @@ -12,7 +12,6 @@ pub use model::{ }; pub use provider::ContentMetadataProvider; -// Skip Fabric API if you're installing a fabric project onto a quilt instance. const QUILT_FABRIC_API_EXCEPTION_PROJECT_ID: &str = "P7dR8mSH"; pub async fn resolve_content( diff --git a/packages/modrinth-content-management/src/shared/mod.rs b/packages/modrinth-content-management/src/shared/mod.rs index 4e87c3e9d4..96938d1ed9 100644 --- a/packages/modrinth-content-management/src/shared/mod.rs +++ b/packages/modrinth-content-management/src/shared/mod.rs @@ -1,4 +1,4 @@ -//! Content identifiers and errors shared by content-management features. +//! Common content types and errors used when installing or comparing content. use serde::{Deserialize, Serialize}; @@ -68,7 +68,7 @@ impl std::str::FromStr for ContentType { } impl ContentType { - /// Returns the canonical content type used by shared-instance manifests. + /// Returns the API name, such as `mod` or `resourcepack`. pub fn as_str(self) -> &'static str { match self { Self::Mod => "mod",