fix: update checking version match reliability (#6411)

apps/app-frontend/src/pages/instance/Mods.vue

@@ -538,6 +538,7 @@ async function updateProject(mod: ContentItem) {
 		})
 	} catch (err) {
 		handleError(err as Error)
+		throw err
 	} finally {
 		await refreshContentState('must_revalidate')
 		finishContentOperation(mod, operation)
@@ -942,13 +943,15 @@ async function handleModalUpdate(
 	} else if (updatingProject.value) {
 		const mod = updatingProject.value
 
-		if (mod.has_update && mod.update_version_id === selectedVersion.id) {
+		try {
-			await updateProject(mod)
+			if (mod.has_update && mod.update_version_id === selectedVersion.id) {
-		} else {
+				await updateProject(mod)
-			await switchProjectVersion(mod, selectedVersion)
+			} else {
+				await switchProjectVersion(mod, selectedVersion)
+			}
+		} finally {
+			resetUpdateState()
 		}
-
-		resetUpdateState()
 	}
 }
 

packages/app-lib/src/api/pack/install_mrpack.rs

@@ -456,6 +456,7 @@ pub async fn install_zipped_mrpack_files(
                     project.path.as_str(),
                     project.hashes.get(&PackFileHash::Sha1).map(|x| &**x),
                     ProjectType::get_from_parent_folder(&path),
+                    None,
                     &state.pool,
                 )
                 .await?;
@@ -514,6 +515,7 @@ pub async fn install_zipped_mrpack_files(
             ProjectType::get_from_parent_folder(
                 relative_override_file_path.as_str(),
             ),
+            None,
             &state.pool,
         )
         .await?;

packages/app-lib/src/api/profile/mod.rs

@@ -547,6 +547,7 @@ pub async fn add_project_from_path(
         bytes::Bytes::from(file),
         None,
         project_type,
+        None,
         &state.io_semaphore,
         &state.pool,
     )

packages/app-lib/src/state/cache.rs

@@ -372,6 +372,16 @@ pub struct CachedFileHash {
     pub size: u64,
     pub hash: String,
     pub project_type: Option<ProjectType>,
+    #[serde(default)]
+    pub project_id: Option<String>,
+    #[serde(default)]
+    pub version_id: Option<String>,
+}
+
+#[derive(Clone, Copy, Debug)]
+pub struct KnownModrinthFile<'a> {
+    pub project_id: &'a str,
+    pub version_id: &'a str,
 }
 
 #[derive(Serialize, Deserialize, Clone, Debug)]
@@ -1503,6 +1513,8 @@ impl CachedEntry {
                             project_type: ProjectType::get_from_parent_folder(
                                 &full_path,
                             ),
+                            project_id: None,
+                            version_id: None,
                         })
                         .get_entry(),
                         true,
@@ -1653,13 +1665,32 @@ impl CachedEntry {
                         let versions = variation.remove(hash);
 
                         if let Some(versions) = versions {
+                            let mut emitted_update = false;
+
                             for version in versions {
                                 let version_id = version.id.clone();
+                                let target_hash = version
+                                    .files
+                                    .iter()
+                                    .find(|file| file.primary)
+                                    .or_else(|| version.files.first())
+                                    .and_then(|file| file.hashes.get("sha1"))
+                                    .map(String::as_str);
+
+                                // Some update responses point at a different version ID for the exact installed file.
+                                let same_file =
+                                    target_hash == Some(hash.as_str());
+
                                 vals.push((
                                     CacheValue::Version(version).get_entry(),
                                     false,
                                 ));
 
+                                if same_file {
+                                    continue;
+                                }
+
+                                emitted_update = true;
                                 vals.push((
                                     CacheValue::FileUpdate(CachedFileUpdate {
                                         hash: hash.clone(),
@@ -1676,6 +1707,16 @@ impl CachedEntry {
                                     true,
                                 ));
                             }
+
+                            if !emitted_update {
+                                vals.push((
+                                    CacheValueType::FileUpdate
+                                        .get_empty_entry(format!(
+                                            "{hash}-{loaders_key}-{channel_policy_key}-{game_version}"
+                                        )),
+                                    true,
+                                ));
+                            }
                         } else {
                             vals.push((
                                 CacheValueType::FileUpdate.get_empty_entry(
@@ -2061,6 +2102,7 @@ pub async fn cache_file_hash(
     path: &str,
     known_hash: Option<&str>,
     project_type: Option<ProjectType>,
+    known_modrinth_file: Option<KnownModrinthFile<'_>>,
     exec: impl sqlx::Executor<'_, Database = sqlx::Sqlite>,
 ) -> crate::Result<()> {
     let size = bytes.len();
@@ -2077,6 +2119,7 @@ pub async fn cache_file_hash(
         size as u64,
         hash,
         project_type,
+        known_modrinth_file,
         exec,
     )
     .await
@@ -2088,8 +2131,17 @@ pub async fn cache_file_hash_metadata(
     size: u64,
     hash: String,
     project_type: Option<ProjectType>,
+    known_modrinth_file: Option<KnownModrinthFile<'_>>,
     exec: impl sqlx::Executor<'_, Database = sqlx::Sqlite>,
 ) -> crate::Result<()> {
+    let (project_id, version_id) =
+        known_modrinth_file.map_or((None, None), |metadata| {
+            (
+                Some(metadata.project_id.to_string()),
+                Some(metadata.version_id.to_string()),
+            )
+        });
+
     // Streamed extraction already computed these values, so avoid buffering the file just to cache them.
     CachedEntry::upsert_many(
         &[CacheValue::FileHash(CachedFileHash {
@@ -2097,6 +2149,8 @@ pub async fn cache_file_hash_metadata(
             size,
             hash,
             project_type,
+            project_id,
+            version_id,
         })
         .get_entry()],
         exec,

packages/app-lib/src/state/legacy_converter.rs

@@ -225,6 +225,10 @@ where
                                         ProjectType::get_from_parent_folder(
                                             &full_path,
                                         ),
+                                    project_id: Some(
+                                        version.project_id.clone(),
+                                    ),
+                                    version_id: Some(version.id.clone()),
                                 },
                             ));
                         }

packages/app-lib/src/state/profiles.rs

@@ -2,8 +2,8 @@ use super::settings::{Hooks, MemorySettings, WindowSize};
 use crate::profile::get_full_path;
 use crate::state::server_join_log::JoinLogEntry;
 use crate::state::{
-    CacheBehaviour, CachedEntry, CachedFile, CachedFileHash, ReleaseChannel,
+    CacheBehaviour, CachedEntry, CachedFile, CachedFileHash, KnownModrinthFile,
-    cache_file_hash,
+    ReleaseChannel, Version, cache_file_hash,
 };
 use crate::util;
 use crate::util::fetch::{FetchSemaphore, IoSemaphore, write_cached_icon};
@@ -1022,6 +1022,16 @@ impl Profile {
                     .into_iter()
                     .map(|f| (f.hash.clone(), f))
                     .collect();
+                let file_info_by_hash = Self::resolve_installed_file_metadata(
+                    &file_hashes,
+                    &keys,
+                    file_info_by_hash,
+                    self,
+                    cache_behaviour,
+                    pool,
+                    fetch_semaphore,
+                )
+                .await?;
 
                 let file_hashes = file_hashes
                     .into_iter()
@@ -1080,6 +1090,16 @@ impl Profile {
                     .into_iter()
                     .map(|f| (f.hash.clone(), f))
                     .collect();
+                let file_info_by_hash = Self::resolve_installed_file_metadata(
+                    &file_hashes,
+                    &keys,
+                    file_info_by_hash,
+                    self,
+                    cache_behaviour,
+                    pool,
+                    fetch_semaphore,
+                )
+                .await?;
 
                 let installed_channels = Self::get_installed_update_channels(
                     &file_info_by_hash,
@@ -1331,6 +1351,192 @@ impl Profile {
         )
     }
 
+    async fn resolve_installed_file_metadata(
+        file_hashes: &[CachedFileHash],
+        scan_files: &[InitialScanFile],
+        mut file_info_by_hash: HashMap<String, CachedFile>,
+        profile: &Profile,
+        cache_behaviour: Option<CacheBehaviour>,
+        pool: &SqlitePool,
+        fetch_semaphore: &FetchSemaphore,
+    ) -> crate::Result<HashMap<String, CachedFile>> {
+        let scan_files_by_path: HashMap<&str, &InitialScanFile> = scan_files
+            .iter()
+            .map(|file| (file.path.as_str(), file))
+            .collect();
+        struct MetadataCandidate {
+            hash: String,
+            project_id: String,
+            version_id: String,
+            file_name: String,
+            project_type: ProjectType,
+        }
+
+        let mut candidates = Vec::new();
+        let mut version_ids = HashSet::new();
+
+        for file_hash in file_hashes {
+            if let (Some(project_id), Some(version_id)) =
+                (&file_hash.project_id, &file_hash.version_id)
+            {
+                file_info_by_hash.insert(
+                    file_hash.hash.clone(),
+                    CachedFile {
+                        hash: file_hash.hash.clone(),
+                        project_id: project_id.clone(),
+                        version_id: version_id.clone(),
+                    },
+                );
+                continue;
+            }
+
+            let Some(file_info) = file_info_by_hash.get(&file_hash.hash) else {
+                continue;
+            };
+
+            let Some(scan_file) = scan_files_by_path
+                .get(file_hash.path.trim_end_matches(".disabled"))
+            else {
+                continue;
+            };
+
+            version_ids.insert(file_info.version_id.clone());
+            candidates.push(MetadataCandidate {
+                hash: file_hash.hash.clone(),
+                project_id: file_info.project_id.clone(),
+                version_id: file_info.version_id.clone(),
+                file_name: scan_file.file_name.clone(),
+                project_type: scan_file.project_type,
+            });
+        }
+
+        let version_ids_ref =
+            version_ids.iter().map(|id| id.as_str()).collect::<Vec<_>>();
+        let versions_by_id: HashMap<String, Version> =
+            CachedEntry::get_version_many(
+                &version_ids_ref,
+                None,
+                pool,
+                fetch_semaphore,
+            )
+            .await?
+            .into_iter()
+            .map(|version| (version.id.clone(), version))
+            .collect();
+
+        let mut project_versions_by_id: HashMap<String, Option<Vec<Version>>> =
+            HashMap::new();
+
+        for candidate in candidates {
+            if versions_by_id.get(&candidate.version_id).is_some_and(
+                |version| {
+                    Self::version_matches_file(
+                        version,
+                        &candidate.hash,
+                        &candidate.file_name,
+                        candidate.project_type,
+                        profile,
+                    )
+                },
+            ) {
+                continue;
+            }
+
+            if !project_versions_by_id.contains_key(&candidate.project_id) {
+                let versions = CachedEntry::get_project_versions(
+                    &candidate.project_id,
+                    cache_behaviour,
+                    pool,
+                    fetch_semaphore,
+                )
+                .await?;
+                project_versions_by_id
+                    .insert(candidate.project_id.clone(), versions);
+            }
+
+            let Some(Some(versions)) =
+                project_versions_by_id.get(&candidate.project_id)
+            else {
+                continue;
+            };
+
+            if let Some(version) = Self::find_matching_file_version(
+                versions,
+                &candidate.hash,
+                &candidate.file_name,
+                candidate.project_type,
+                profile,
+            ) {
+                file_info_by_hash.insert(
+                    candidate.hash.clone(),
+                    CachedFile {
+                        hash: candidate.hash,
+                        project_id: version.project_id.clone(),
+                        version_id: version.id.clone(),
+                    },
+                );
+            }
+        }
+
+        Ok(file_info_by_hash)
+    }
+
+    fn find_matching_file_version<'a>(
+        versions: &'a [Version],
+        hash: &str,
+        file_name: &str,
+        project_type: ProjectType,
+        profile: &Profile,
+    ) -> Option<&'a Version> {
+        versions.iter().find(|version| {
+            Self::version_matches_file(
+                version,
+                hash,
+                file_name,
+                project_type,
+                profile,
+            )
+        })
+    }
+
+    fn version_matches_file(
+        version: &Version,
+        hash: &str,
+        file_name: &str,
+        project_type: ProjectType,
+        profile: &Profile,
+    ) -> bool {
+        version.game_versions.contains(&profile.game_version)
+            && Self::version_loaders_match_profile(
+                version,
+                project_type,
+                profile,
+            )
+            && version.files.iter().any(|file| {
+                file.hashes.get("sha1").is_some_and(|sha1| sha1 == hash)
+                    && (file.primary
+                        || file.filename
+                            == file_name.trim_end_matches(".disabled"))
+            })
+    }
+
+    fn version_loaders_match_profile(
+        version: &Version,
+        project_type: ProjectType,
+        profile: &Profile,
+    ) -> bool {
+        if project_type == ProjectType::Mod {
+            version
+                .loaders
+                .iter()
+                .any(|loader| loader == profile.loader.as_str())
+        } else {
+            version.loaders.iter().any(|loader| {
+                project_type.get_loaders().contains(&loader.as_str())
+            })
+        }
+    }
+
     #[tracing::instrument(skip(pool))]
     pub async fn add_project_version(
         profile_path: &str,
@@ -1393,6 +1599,10 @@ impl Profile {
             bytes,
             file.hashes.get("sha1").map(|x| &**x),
             ProjectType::get_from_loaders(version.loaders.clone()),
+            Some(KnownModrinthFile {
+                project_id: &version.project_id,
+                version_id: &version.id,
+            }),
             io_semaphore,
             pool,
         )
@@ -1408,6 +1618,7 @@ impl Profile {
         bytes: bytes::Bytes,
         hash: Option<&str>,
         project_type: Option<ProjectType>,
+        known_modrinth_file: Option<KnownModrinthFile<'_>>,
         io_semaphore: &IoSemaphore,
         exec: impl sqlx::Executor<'_, Database = sqlx::Sqlite>,
     ) -> crate::Result<String> {
@@ -1455,6 +1666,7 @@ impl Profile {
             &project_path,
             hash,
             Some(project_type),
+            known_modrinth_file,
             exec,
         )
         .await?;