fix(app/s-inst): preserve multiple shared versions of a project (#7770)

* fix(app): preserve multiple shared versions of a project

* fix: fmt

* fix: ci

Author: IMB11

packages/app-lib/src/api/instance/shared/diff.rs

@@ -2,6 +2,7 @@ use super::client::*;
 use super::publish::*;
 use super::types::*;
 use super::*;
+use std::collections::BTreeMap;
 
 struct SharedContentSnapshot {
     version_ids: Vec<String>,
@@ -36,6 +37,7 @@ pub(super) async fn shared_instance_update_diffs(
         &before,
         &after,
         &HashSet::new(),
+        &HashSet::new(),
         &BTreeSet::new(),
         CommonExternalFilePolicy::AssumeUpdated,
         state,
@@ -53,22 +55,8 @@ pub(super) async fn shared_instance_publish_diffs(
     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,
-                true,
-                state,
-            )
-            .await
-        }
-    };
-    let ((version_ids, external_files), disabled_versions) = tokio::try_join!(
-        remote_publish_content(version, modpack_unlinked, state),
-        disabled_versions,
-    )?;
+    let (version_ids, external_files) =
+        remote_publish_content(version, modpack_unlinked, state).await?;
     let before = SharedContentSnapshot {
         version_ids,
         external_files,
@@ -93,14 +81,11 @@ pub(super) async fn shared_instance_publish_diffs(
             .collect::<crate::Result<_>>()?,
         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_project_ids,
+        &snapshot.disabled_version_ids.iter().cloned().collect(),
         &snapshot.disabled_external_files,
         CommonExternalFilePolicy::AssumeUnchanged,
         state,
@@ -295,30 +280,24 @@ async fn shared_content_diffs(
     before: &SharedContentSnapshot,
     after: &SharedContentSnapshot,
     removed_disabled_project_ids: &HashSet<String>,
+    removed_disabled_version_ids: &HashSet<String>,
     removed_disabled_external_files: &BTreeSet<ExternalFileKey>,
     common_external_files: CommonExternalFilePolicy,
     state: &State,
 ) -> crate::Result<Vec<SharedInstanceUpdateDiff>> {
     let (before_versions, after_versions) = tokio::try_join!(
-        shared_versions_by_project(&before.version_ids, true, state),
-        shared_versions_by_project(&after.version_ids, false, state),
+        shared_versions_by_id(&before.version_ids, true, state),
+        shared_versions_by_id(&after.version_ids, false, state),
     )?;
-    let to_snapshot =
-        |source: &SharedContentSnapshot,
-         versions: &HashMap<String, crate::state::Version>| {
-            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),
+    let mut diff = diff_content_sets(
+        &ContentSetSnapshot {
+            projects: BTreeMap::default(),
+            external_files: before.external_files.clone(),
+        },
+        &ContentSetSnapshot {
+            projects: BTreeMap::default(),
+            external_files: after.external_files.clone(),
+        },
         &ContentSetDiffOptions {
             common_external_files,
         },
@@ -327,6 +306,52 @@ async fn shared_content_diffs(
         &before.configuration,
         &after.configuration,
     ));
+    let project_ids = before_versions
+        .values()
+        .chain(after_versions.values())
+        .map(|version| version.project_id.clone())
+        .collect::<BTreeSet<_>>();
+    for project_id in project_ids {
+        let removed = before_versions
+            .values()
+            .filter(|version| {
+                version.project_id == project_id
+                    && !after_versions.contains_key(&version.id)
+            })
+            .map(|version| version.id.clone())
+            .collect::<BTreeSet<_>>();
+        let added = after_versions
+            .values()
+            .filter(|version| {
+                version.project_id == project_id
+                    && !before_versions.contains_key(&version.id)
+            })
+            .map(|version| version.id.clone())
+            .collect::<BTreeSet<_>>();
+        // Only pair an unambiguous replacement; preserve every other version change.
+        if removed.len() == 1 && added.len() == 1 {
+            diff.content.push(ContentSetDiffEntry::Project {
+                project_id,
+                change: Change::Updated {
+                    before: removed.into_iter().next().unwrap(),
+                    after: added.into_iter().next().unwrap(),
+                },
+            });
+        } else {
+            for before in removed {
+                diff.content.push(ContentSetDiffEntry::Project {
+                    project_id: project_id.clone(),
+                    change: Change::Removed { before },
+                });
+            }
+            for after in added {
+                diff.content.push(ContentSetDiffEntry::Project {
+                    project_id: project_id.clone(),
+                    change: Change::Added { after },
+                });
+            }
+        }
+    }
     let resolved_before_ids = before_versions
         .values()
         .map(|version| version.id.as_str())
@@ -367,7 +392,10 @@ async fn shared_content_diffs(
         match entry {
             ContentSetDiffEntry::Project { project_id, change } => {
                 let disabled = change.kind() == ContentSetDiffKind::Removed
-                    && removed_disabled_project_ids.contains(&project_id);
+                    && (removed_disabled_project_ids.contains(&project_id)
+                        || change.before().is_some_and(|id| {
+                            removed_disabled_version_ids.contains(id)
+                        }));
                 content_diffs.push(SharedInstanceUpdateDiff {
                     type_: shared_update_diff_type(change.kind()),
                     project_name: Some(
@@ -378,13 +406,13 @@ async fn shared_content_diffs(
                     ),
                     current_version_name: change.before().map(|id| {
                         before_versions
-                            .get(&project_id)
+                            .get(id)
                             .map(|version| version.version_number.clone())
                             .unwrap_or_else(|| id.clone())
                     }),
                     new_version_name: change.after().map(|id| {
                         after_versions
-                            .get(&project_id)
+                            .get(id)
                             .map(|version| version.version_number.clone())
                             .unwrap_or_else(|| id.clone())
                     }),

packages/app-lib/src/api/instance/shared/publish.rs

@@ -237,13 +237,17 @@ async fn extend_shared_modpack_dependencies(
     let dependency_ids =
         modpack_dependency_version_ids(modpack_id, state).await?;
     let (explicit, inherited) = tokio::try_join!(
-        shared_versions_by_project(version_ids, false, state),
-        shared_versions_by_project(&dependency_ids, false, state),
+        shared_versions_by_id(version_ids, false, state),
+        shared_versions_by_id(&dependency_ids, false, state),
     )?;
     version_ids.extend(
         inherited
             .into_values()
-            .filter(|version| !explicit.contains_key(&version.project_id))
+            .filter(|version| {
+                !explicit
+                    .values()
+                    .any(|installed| installed.project_id == version.project_id)
+            })
             .map(|version| version.id),
     );
     Ok(())
@@ -460,7 +464,9 @@ pub(super) async fn collect_publish_snapshot(
             continue;
         }
 
-        if let Some(project) = item.project.as_ref() {
+        if item.version.is_none()
+            && let Some(project) = item.project.as_ref()
+        {
             disabled_project_ids.insert(project.id.clone());
         }
 
@@ -491,7 +497,7 @@ pub(super) async fn collect_publish_snapshot(
     })
 }
 
-pub(super) async fn shared_versions_by_project(
+pub(super) async fn shared_versions_by_id(
     version_ids: &[String],
     allow_missing: bool,
     state: &State,
@@ -520,15 +526,10 @@ pub(super) async fn shared_versions_by_project(
         ))
         .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)
+    Ok(versions
+        .into_iter()
+        .map(|version| (version.id.clone(), version))
+        .collect())
 }
 
 pub(super) async fn shared_project_names(

packages/app-lib/src/install/shared_instance.rs

@@ -155,7 +155,7 @@ impl SharedInstanceApplyPlan {
             .projects
             .values()
             .filter(|current| {
-                !desired.projects.contains_key(&current.project_id)
+                !desired.projects.contains_key(&current.version_id)
             })
             .cloned()
             .collect();
@@ -172,16 +172,35 @@ impl SharedInstanceApplyPlan {
             ..Default::default()
         };
 
-        for desired in desired.projects.into_values() {
-            match current.projects.get(&desired.project_id) {
-                Some(current) if current.version_id != desired.version_id => {
-                    plan.project_updates.push(SharedInstanceProjectUpdate {
-                        current: current.clone(),
-                        desired,
-                    });
-                }
-                None => plan.project_additions.push(desired),
-                Some(_) => {}
+        let mut additions = desired
+            .projects
+            .into_values()
+            .filter(|desired| {
+                !current.projects.contains_key(&desired.version_id)
+            })
+            .collect::<Vec<_>>();
+        while let Some(desired) = additions.pop() {
+            let replacements = plan
+                .project_removals
+                .iter()
+                .enumerate()
+                .filter(|(_, current)| current.project_id == desired.project_id)
+                .map(|(index, _)| index)
+                .collect::<Vec<_>>();
+            let unambiguous = !additions
+                .iter()
+                .any(|other| other.project_id == desired.project_id)
+                && !plan
+                    .project_additions
+                    .iter()
+                    .any(|other| other.project_id == desired.project_id);
+            if replacements.len() == 1 && unambiguous {
+                plan.project_updates.push(SharedInstanceProjectUpdate {
+                    current: plan.project_removals.remove(replacements[0]),
+                    desired,
+                });
+            } else {
+                plan.project_additions.push(desired);
             }
         }
 
@@ -474,7 +493,7 @@ async fn current_shared_instance_content(
             };
 
             content.projects.insert(
-                project_id.clone(),
+                version_id.clone(),
                 CurrentSharedInstanceProject {
                     project_id,
                     version_id,
@@ -509,7 +528,7 @@ async fn desired_shared_instance_content(
             ))
         })?;
         content.projects.insert(
-            version.project_id.clone(),
+            version.id.clone(),
             DesiredSharedInstanceProject {
                 project_id: version.project_id.clone(),
                 version_id: version.id.clone(),