From 07a2d535f5d5943bfbbe89832ad461871599e616 Mon Sep 17 00:00:00 2001 From: Puschie Date: Sat, 24 Oct 2026 21:27:40 +0200 Subject: [PATCH 1/3] Group addon modules by required parent metadata Addons with multiple folders could appear as separate entries because Group metadata was ignored and module names without separators missed the name-stem fallback. - Read TOC Group metadata and group standalone modules when it names an installed required dependency, combining related folders into one entry. - Preserve catalogue, project and recorded installation ownership, including managed packages with folder: keys. - Share the ownership check and flatten the parent walk while retaining its ownership boundaries and four-step limit. - Add regression coverage for addon suites, unrelated addons, missing parents, optional dependencies and independently owned packages. --- src-tauri/src/lib.rs | 170 ++++++++++++++++++++++++++++++++++++++++--- src-tauri/src/wow.rs | 30 ++++++++ 2 files changed, 188 insertions(+), 12 deletions(-) diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index fdb190d..cb5286e 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -195,14 +195,23 @@ fn build_packages(st: &AppState, cat: &Catalog, folders: &[AddonFolder]) -> Vec< (key, cat_entry) }; - // Does `f` look like a module of `parent` (e.g. BigWigs_Sporefall -> BigWigs, - // DBM-GUI -> DBM-Core)? Needs a declared dependency and a shared name prefix. + // Prefer an explicit parent, then a required dependency backed by Group + // or a shared name stem (e.g. DBM-GUI -> DBM-Core). fn module_of<'a>(f: &AddonFolder, on_disk: &HashMap) -> Option<&'a AddonFolder> { if let Some(p) = &f.part_of { if let Some(parent) = on_disk.get(&p.to_ascii_lowercase()) { return Some(parent); } } + if let Some(group) = &f.group { + if f.dependencies.iter().any(|d| d.eq_ignore_ascii_case(group)) { + if let Some(parent) = on_disk.get(&group.to_ascii_lowercase()) { + if !parent.folder.eq_ignore_ascii_case(&f.folder) { + return Some(parent); + } + } + } + } let stem = |s: &str| s.split(['_', '-']).next().unwrap_or(s).to_ascii_lowercase(); let my_stem = stem(&f.folder); for d in &f.dependencies { @@ -225,22 +234,26 @@ fn build_packages(st: &AppState, cat: &Catalog, folders: &[AddonFolder]) -> Vec< .iter() .map(|(k, e, f)| (f.folder.to_ascii_lowercase(), (k.clone(), *e))) .collect(); + let has_package_owner = |key: &str, folder: &AddonFolder| { + !key.starts_with("folder:") + || managed_folder.contains_key(&folder.folder.to_ascii_lowercase()) + }; for item in keyed.iter_mut() { - if !item.0.starts_with("folder:") { + if has_package_owner(&item.0, item.2) { continue; } let mut cur = item.2; for _ in 0..4 { - match module_of(cur, &on_disk) { - Some(parent) => { - if let Some((k, e)) = key_of.get(&parent.folder.to_ascii_lowercase()) { - item.0 = k.clone(); - item.1 = *e; - } - cur = parent; - } - None => break, + let Some(parent) = module_of(cur, &on_disk) else { break }; + let Some((k, e)) = key_of.get(&parent.folder.to_ascii_lowercase()) else { break }; + item.0 = k.clone(); + item.1 = *e; + // An identified package owns this module even if it + // depends on another package in the same group. + if has_package_owner(k, parent) { + break; } + cur = parent; } } @@ -1460,6 +1473,139 @@ mod tests { } } + fn grouped_folder(name: &str, deps: &[&str], group: &str) -> AddonFolder { + AddonFolder { group: Some(group.into()), ..folder(name, deps, None) } + } + + #[test] + fn group_and_required_dependency_group_ellesmere_modules() { + let mut core = folder("EllesmereUI", &[], None); + core.wago = Some("ZKbxbRN1".into()); + core.version = Some("9.6.7".into()); + let folders = vec![ + core, + grouped_folder("EllesmereUIActionBars", &["EllesmereUI"], "EllesmereUI"), + grouped_folder("EllesmereUILocales", &["ELLESMEREUI"], "ellesmereui"), + grouped_folder("EllesmereUIOptions", &["EllesmereUI"], "EllesmereUI"), + ]; + let pkgs = build_packages(&AppState::default(), &Catalog::default(), &folders); + assert_eq!(pkgs.len(), 1); + assert_eq!(pkgs[0].key, "wago:ZKbxbRN1"); + assert_eq!(pkgs[0].name, "EllesmereUI"); + assert_eq!(pkgs[0].source, Some(Source::Wago("ZKbxbRN1".into()))); + assert_eq!(pkgs[0].installed_version.as_deref(), Some("9.6.7")); + assert_eq!(pkgs[0].folders, vec![ + "EllesmereUI", "EllesmereUIActionBars", "EllesmereUILocales", "EllesmereUIOptions", + ]); + } + + #[test] + fn group_requires_an_installed_required_parent() { + let folders = vec![ + folder("EllesmereUI", &[], None), + folder("LibStub", &[], None), + // A group alone does not establish folder ownership. + grouped_folder("EllesmereUIExtra", &[], "EllesmereUI"), + // A required dependency alone can be an independent addon/library. + folder("EllesmereUIPlugin", &["EllesmereUI"], None), + grouped_folder("OtherAddon", &["LibStub"], "EllesmereUI"), + // Shared category labels and missing parents are not packages. + grouped_folder("FirstTool", &["LibStub"], "Tools"), + grouped_folder("SecondTool", &["LibStub"], "Tools"), + grouped_folder("OrphanModule", &["MissingCore"], "MissingCore"), + ]; + let pkgs = build_packages(&AppState::default(), &Catalog::default(), &folders); + assert_eq!(pkgs.len(), folders.len()); + for f in &folders { + let pkg = pkgs.iter().find(|p| p.key == format!("folder:{}", f.folder)).unwrap(); + assert_eq!(pkg.folders, vec![f.folder.clone()]); + } + let orphan = pkgs.iter().find(|p| p.name == "OrphanModule").unwrap(); + assert_eq!(orphan.missing_deps.len(), 1); + assert_eq!(orphan.missing_deps[0].folder, "MissingCore"); + } + + #[test] + fn group_preserves_explicit_package_ownership() { + let mut st = AppState::default(); + st.installed.insert( + "github:author/Plugin".into(), + Installed { + source: "github".into(), source_id: "author/Plugin".into(), version: "2.0".into(), + folders: vec!["ManagedPlugin".into()], installed_at: 0, + }, + ); + let cat: Catalog = serde_json::from_str(r#"{"addons":[ + {"id":"dbm","name":"Deadly Boss Mods","folders":["DBM-Core"]}, + {"id":"dbm-dungeons","name":"DBM Dungeons","folders":["DBM-Party-Forever"]} + ]}"#).unwrap(); + let mut wago = grouped_folder("WagoPlugin", &["DBM-Core"], "DBM-Core"); + wago.wago = Some("another-project".into()); + let mut curse = grouped_folder("CursePlugin", &["DBM-Core"], "DBM-Core"); + curse.curse = Some(12345); + let mut wowi = grouped_folder("WowiPlugin", &["DBM-Core"], "DBM-Core"); + wowi.wowi = Some(67890); + let folders = vec![ + folder("DBM-Core", &[], None), + grouped_folder("DBM-Party-Forever", &["DBM-Core"], "DBM-Core"), + grouped_folder("ManagedPlugin", &["DBM-Core"], "DBM-Core"), + wago, curse, wowi, + ]; + let pkgs = build_packages(&st, &cat, &folders); + assert_eq!(pkgs.len(), folders.len()); + for key in ["cat:dbm", "cat:dbm-dungeons", "github:author/Plugin", + "wago:another-project", "curse:12345", "wowi:67890"] { + let pkg = pkgs.iter().find(|p| p.key == key).unwrap(); + assert_eq!(pkg.folders.len(), 1, "{key}"); + } + let managed = pkgs.iter().find(|p| p.key == "github:author/Plugin").unwrap(); + assert!(managed.managed); + assert_eq!(managed.source, Some(Source::Github("author/Plugin".into()))); + } + + #[test] + fn group_stops_at_an_identified_parent() { + let mut st = AppState::default(); + st.installed.insert( + // Updating an originally standalone addon retains its folder key. + "folder:ManagedAddon".into(), + Installed { + source: "github".into(), source_id: "author/Managed".into(), version: "1.0".into(), + folders: vec!["ManagedAddon".into()], installed_at: 0, + }, + ); + let cat: Catalog = serde_json::from_str(r#"{"addons":[ + {"id":"catalogue-addon","name":"Catalogue addon","folders":["CatalogueAddon"]} + ]}"#).unwrap(); + let mut project = grouped_folder("ProjectAddon", &["SharedFramework"], "SharedFramework"); + project.wago = Some("project-id".into()); + let folders = vec![ + folder("SharedFramework", &[], None), + grouped_folder("CatalogueAddon", &["SharedFramework"], "SharedFramework"), + grouped_folder("CatalogueOptions", &["CatalogueAddon"], "CatalogueAddon"), + grouped_folder("ManagedAddon", &["SharedFramework"], "SharedFramework"), + grouped_folder("ManagedOptions", &["ManagedAddon"], "ManagedAddon"), + project, + grouped_folder("ProjectOptions", &["ProjectAddon"], "ProjectAddon"), + ]; + let pkgs = build_packages(&st, &cat, &folders); + assert_eq!(pkgs.len(), 4); + let framework = pkgs.iter().find(|p| p.key == "folder:SharedFramework").unwrap(); + assert_eq!(framework.folders, vec!["SharedFramework"]); + for (key, parent, child) in [ + ("cat:catalogue-addon", "CatalogueAddon", "CatalogueOptions"), + ("folder:ManagedAddon", "ManagedAddon", "ManagedOptions"), + ("wago:project-id", "ProjectAddon", "ProjectOptions"), + ] { + let pkg = pkgs.iter().find(|p| p.key == key).unwrap(); + assert_eq!(pkg.folders, vec![parent, child], "{key}"); + } + let managed = pkgs.iter().find(|p| p.key == "folder:ManagedAddon").unwrap(); + assert!(managed.managed); + assert_eq!(managed.source, Some(Source::Github("author/Managed".into()))); + assert_eq!(managed.installed_version.as_deref(), Some("1.0")); + } + #[test] fn modules_group_under_their_core_without_catalogue() { let st = AppState::default(); diff --git a/src-tauri/src/wow.rs b/src-tauri/src/wow.rs index b92edf3..cb486d7 100644 --- a/src-tauri/src/wow.rs +++ b/src-tauri/src/wow.rs @@ -187,6 +187,8 @@ pub struct AddonFolder { pub dependencies: Vec, /// `## X-Part-Of: Parent` — this folder is a module of another addon. pub part_of: Option, + /// `## Group: Parent` — a grouping hint, used only with a required dependency. + pub group: Option, /// `## IconTexture:` — a file path under Interface\AddOns\ or a game-internal icon name. pub icon_texture: Option, } @@ -309,6 +311,11 @@ fn parse_toc(path: &Path, into: &mut AddonFolder) -> bool { into.part_of = Some(val.to_string()); } } + "group" => { + if into.group.is_none() { + into.group = Some(val.to_string()); + } + } "icontexture" => { if into.icon_texture.is_none() { into.icon_texture = Some(val.to_string()); @@ -380,6 +387,29 @@ pub fn scan_addons(install: &Path) -> Vec { mod tests { use super::*; + #[test] + fn scans_group_with_required_dependencies_and_prefers_forever_metadata() { + let root = std::env::temp_dir().join(format!("af-group-toc-{}", std::process::id())); + let addon = root.join("Interface").join("AddOns").join("EllesmereUIActionBars"); + fs::create_dir_all(&addon).unwrap(); + fs::write( + addon.join("EllesmereUIActionBars_Camelot.toc"), + "## Interface: 16001\n## Group: \n## gRoUp: EllesmereUI \n\ + ## RequiredDeps: EllesmereUI\n## OptionalDeps: OptionalAddon\n", + ).unwrap(); + fs::write( + addon.join("EllesmereUIActionBars.toc"), + "## Group: OtherSuite\n## Dependencies: LibStub\n", + ).unwrap(); + + let folders = scan_addons(&root); + assert_eq!(folders.len(), 1); + assert_eq!(folders[0].group.as_deref(), Some("EllesmereUI")); + assert_eq!(folders[0].dependencies, vec!["EllesmereUI", "LibStub"]); + assert!(folders[0].part_of.is_none()); + fs::remove_dir_all(&root).unwrap(); + } + #[test] fn live_forever_folder_is_recognised() { assert_eq!(product_flavor("wow_classic_beta").0, "forever"); From f053040368130ef9538e8dda5cb354372b436e62 Mon Sep 17 00:00:00 2001 From: Puschie Date: Sat, 24 Oct 2026 23:42:38 +0200 Subject: [PATCH 2/3] Add catalogue opt-in for required dependency grouping - Group direct required dependencies when the catalogue opts in, retaining existing module membership and separate package ownership. - Reuse existing installation keys and recognise catalogue membership by ID in Browse and Starter packs. - Reject conflicting archive targets before replacing installed folders. - Document the opt-in and add grouping, ownership and bundle-version regression tests. --- CONTRIBUTING.md | 11 + scripts/validate-catalog.js | 3 +- src-tauri/src/catalog.rs | 3 + src-tauri/src/install.rs | 126 ++++++-- src-tauri/src/lib.rs | 538 ++++++++++++++++++++++++++++---- src-tauri/src/sources/github.rs | 71 +++++ 6 files changed, 662 insertions(+), 90 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 63414a4..582a0ee 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -31,6 +31,7 @@ An entry looks like this: | `id` | yes | kebab-case, unique | | `name` | yes | display name | | `folders` | yes | top-level folders the zip installs into `Interface\AddOns`. Used to recognise what's already on disk. | +| `group_required_dependencies` | no | group the first folder's direct required dependencies into this addon; defaults to `false`. See below. | | `github` | one source | `owner/repo` that publishes GitHub Releases. Preferred: no key, no rate limit when the release has a `release.json` (the BigWigs packager makes one). | | `wago` | one source | Wago Addons project id. Users need their own free Wago key. | | `wowi` | one source | WoWInterface file id. | @@ -42,6 +43,16 @@ An entry looks like this: | `notice` | no | one short line (160 chars) shown in amber on the installed row. For "remove this once X is fixed" situations. | | `added` | no | `YYYY-MM-DD` the entry was added. Feeds "New in the catalogue" when nothing is featured. | +Set `group_required_dependencies: true` only when the download bundles the +first folder's required dependencies, as Questie does with QuestieDB. Keep those +dependencies out of `folders` so they do not identify the addon on their own. +When the first folder is present, its direct `Dependencies` / `RequiredDeps` +join the package unless another managed or identified package owns them. +This opt-in skips optional dependencies, does not follow dependency chains, and +leaves dependencies claimed by multiple opted-in packages unassigned. Existing +module grouping still applies. +This controls grouping only; update checks still use the source's package version. + Rules: - Only the author's own publishing location. Unofficial mirrors (for example `curseforge-mirror/*` or `hippuli/*`) are refused; several authors have DMCA'd them. diff --git a/scripts/validate-catalog.js b/scripts/validate-catalog.js index 6bcbdf6..8207878 100644 --- a/scripts/validate-catalog.js +++ b/scripts/validate-catalog.js @@ -24,7 +24,7 @@ const folderOwner = new Map(); const idRe = /^[a-z0-9]+(-[a-z0-9]+)*$/; const repoRe = /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/; const dateRe = /^\d{4}-\d{2}-\d{2}$/; -const knownKeys = new Set(["id", "name", "desc", "category", "github", "wago", "wowi", "tukui", "curse", "asset_hint", "folders", "url", "forever", "icon", "notice", "added"]); +const knownKeys = new Set(["id", "name", "desc", "category", "github", "wago", "wowi", "tukui", "curse", "asset_hint", "folders", "group_required_dependencies", "url", "forever", "icon", "notice", "added"]); for (const [i, a] of (cat.addons || []).entries()) { const where = `addons[${i}]${a && a.id ? ` (${a.id})` : ""}`; @@ -49,6 +49,7 @@ for (const [i, a] of (cat.addons || []).entries()) { if (a.notice !== undefined && (typeof a.notice !== "string" || a.notice.length > 160)) problems.push(`${where}: "notice" must be a short string (160 chars max)`); if (a.added !== undefined && !dateRe.test(a.added)) problems.push(`${where}: "added" must be YYYY-MM-DD`); if (a.forever !== undefined && a.forever !== null && typeof a.forever !== "boolean") problems.push(`${where}: "forever" must be true, false or null`); + if (a.group_required_dependencies !== undefined && typeof a.group_required_dependencies !== "boolean") problems.push(`${where}: "group_required_dependencies" must be true or false`); if (!Array.isArray(a.folders) || a.folders.length === 0 || !a.folders.every((f) => typeof f === "string" && f.trim() && !/[\\/]/.test(f))) { problems.push(`${where}: "folders" must list at least one AddOns folder name (no slashes)`); } else { diff --git a/src-tauri/src/catalog.rs b/src-tauri/src/catalog.rs index 3c740d7..16f2133 100644 --- a/src-tauri/src/catalog.rs +++ b/src-tauri/src/catalog.rs @@ -44,6 +44,9 @@ pub struct CatalogEntry { /// already on disk to this entry). #[serde(default)] pub folders: Vec, + /// Group otherwise unowned direct required dependencies of the first folder. + #[serde(default)] + pub group_required_dependencies: bool, #[serde(default)] pub url: Option, /// Does the author publish a Forever build? true / false / null=unknown. diff --git a/src-tauri/src/install.rs b/src-tauri/src/install.rs index 3cafc25..faf86a4 100644 --- a/src-tauri/src/install.rs +++ b/src-tauri/src/install.rs @@ -11,6 +11,7 @@ use anyhow::{anyhow, Context}; use futures_util::StreamExt; +use std::collections::HashMap; use std::fs; use std::io::Write; use std::path::{Path, PathBuf}; @@ -241,7 +242,12 @@ pub fn safety_scan(root: &Path) -> anyhow::Result { /// Extract `zip_path` and move its addon folders into `addons_dir`, /// replacing same-named folders. Returns the folder names installed. -pub fn place(zip_path: &Path, addons_dir: &Path) -> anyhow::Result> { +/// `protected_folders` maps lowercase folder names to their owning package. +pub fn place( + zip_path: &Path, + addons_dir: &Path, + protected_folders: &HashMap, +) -> anyhow::Result> { fs::create_dir_all(addons_dir)?; let work = addons_dir.join(TMP_DIR).join(unique()); let extract_dir = work.join("x"); @@ -256,7 +262,22 @@ pub fn place(zip_path: &Path, addons_dir: &Path) -> anyhow::Result> let files = safety_scan(&extract_dir)?; crate::logi!("zip ok: {} files, no executables", files); - let sources = addon_dirs(&extract_dir)?; + // Resolve every final target before moving any installed folder aside. + let sources: Vec<(PathBuf, String)> = addon_dirs(&extract_dir)? + .into_iter() + .map(|src| { + let original = src.file_name().ok_or_else(|| anyhow!("bad folder name"))? + .to_string_lossy().to_string(); + let name = canonical_name(&src).unwrap_or(original); + if name.starts_with('.') || name.contains("..") { + anyhow::bail!("refusing suspicious folder name {name}"); + } + if let Some(owner) = protected_folders.get(&name.to_ascii_lowercase()) { + anyhow::bail!("cannot replace addon folder {name}: it belongs to {owner}"); + } + Ok((src, name)) + }) + .collect::>()?; let mut moved: Vec<(PathBuf, PathBuf)> = Vec::new(); // (target, backup) let mut installed = Vec::new(); @@ -267,24 +288,7 @@ pub fn place(zip_path: &Path, addons_dir: &Path) -> anyhow::Result> } }; - for mut src in sources { - let mut name = src - .file_name() - .ok_or_else(|| anyhow!("bad folder name"))? - .to_string_lossy() - .to_string(); - if let Some(wanted) = canonical_name(&src) { - if wanted != name { - let renamed = src.with_file_name(&wanted); - fs::rename(&src, &renamed).with_context(|| format!("rename {name} to {wanted}"))?; - crate::logi!("folder {name} renamed to {wanted} (its .toc name)"); - src = renamed; - name = wanted; - } - } - if name.starts_with('.') || name.contains("..") { - anyhow::bail!("refusing suspicious folder name {name}"); - } + for (src, name) in sources { let target = addons_dir.join(&name); let backup = backup_dir.join(&name); if target.exists() { @@ -332,6 +336,88 @@ pub fn remove_folders(addons_dir: &Path, folders: &[String]) -> anyhow::Result PathBuf { + fs::create_dir_all(root).unwrap(); + let path = root.join("release.zip"); + let mut archive = zip::ZipWriter::new(fs::File::create(&path).unwrap()); + for (name, contents) in files { + archive.start_file(*name, zip::write::SimpleFileOptions::default()).unwrap(); + archive.write_all(contents.as_bytes()).unwrap(); + } + archive.finish().unwrap(); + path + } + + #[test] + fn protected_folder_prevents_changes_to_every_installed_folder() { + let root = std::env::temp_dir().join(format!("af-owned-{}", unique())); + let addons = root.join("AddOns"); + for name in ["AAddon", "ZDatabase"] { + fs::create_dir_all(addons.join(name)).unwrap(); + fs::write(addons.join(name).join("keep.lua"), format!("original {name}")).unwrap(); + } + let zip = addon_zip(&root, &[ + ("AAddon/AAddon.toc", "## Version: 2\n"), + ("AAddon/new.lua", "new primary"), + ("ZDATABASE/ZDatabase.toc", "## Version: 2\n"), + ("ZDATABASE/new.lua", "new database"), + ]); + let protected = HashMap::from([("zdatabase".into(), "github:author/Database".into())]); + let error = place(&zip, &addons, &protected).unwrap_err().to_string(); + assert!(error.contains("ZDATABASE"), "{error}"); + assert!(error.contains("github:author/Database"), "{error}"); + for name in ["AAddon", "ZDatabase"] { + assert_eq!(fs::read_to_string(addons.join(name).join("keep.lua")).unwrap(), format!("original {name}")); + assert!(!addons.join(name).join("new.lua").exists()); + assert!(!addons.join(name).join(format!("{name}.toc")).exists()); + } + fs::remove_dir_all(&root).unwrap(); + } + + #[test] + fn ownership_guard_checks_the_canonical_source_archive_name() { + let root = std::env::temp_dir().join(format!("af-owned-canon-{}", unique())); + let addons = root.join("AddOns"); + fs::create_dir_all(addons.join("SharedData")).unwrap(); + fs::write(addons.join("SharedData").join("keep.lua"), "keep database").unwrap(); + let zip = addon_zip(&root, &[ + ("Project-1.2/SharedData_Forever.toc", "## Version: 2\n"), + ]); + let protected = HashMap::from([("shareddata".into(), "folder:SharedData".into())]); + let error = place(&zip, &addons, &protected).unwrap_err().to_string(); + assert!(error.contains("SharedData"), "{error}"); + assert!(error.contains("folder:SharedData"), "{error}"); + assert_eq!(fs::read_to_string(addons.join("SharedData").join("keep.lua")).unwrap(), "keep database"); + assert!(!addons.join("SharedData").join("SharedData_Forever.toc").exists()); + assert!(!addons.join("Project-1.2").exists()); + fs::remove_dir_all(&root).unwrap(); + } + + #[test] + fn addon_folders_are_installed_under_canonical_names() { + let root = std::env::temp_dir().join(format!("af-folders-{}", unique())); + let addons = root.join("AddOns"); + fs::create_dir_all(addons.join("Core")).unwrap(); + fs::write(addons.join("Core").join("old.lua"), "old primary").unwrap(); + let zip = addon_zip(&root, &[ + ("Bundle-1.2/Core-2.0/Core_Forever.toc", "## Version: 2\n"), + ("Bundle-1.2/Core-2.0/new.lua", "new primary"), + ("Bundle-1.2/Data/Data.toc", "## Version: 3\n"), + ("Bundle-1.2/Data/new.lua", "new database"), + ]); + let mut installed = place(&zip, &addons, &HashMap::new()).unwrap(); + installed.sort(); + assert_eq!(installed, vec!["Core", "Data"]); + assert_eq!(fs::read_to_string(addons.join("Core").join("Core_Forever.toc")).unwrap(), "## Version: 2\n"); + assert_eq!(fs::read_to_string(addons.join("Data").join("Data.toc")).unwrap(), "## Version: 3\n"); + assert_eq!(fs::read_to_string(addons.join("Core").join("new.lua")).unwrap(), "new primary"); + assert_eq!(fs::read_to_string(addons.join("Data").join("new.lua")).unwrap(), "new database"); + assert!(!addons.join("Core").join("old.lua").exists()); + assert!(!addons.join("Core-2.0").exists()); + assert!(!addons.join("Bundle-1.2").exists()); + fs::remove_dir_all(&root).unwrap(); + } + /// ForeverSVFix puts a directory link inside addon folders that points at /// the user's SavedVariables. Replacing or cleaning up an addon folder must /// remove the link, never what it points to. diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index cb5286e..f390195 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -11,7 +11,7 @@ mod wow; use catalog::{Catalog, CatalogEntry, Stats}; use serde::{Deserialize, Serialize}; -use sources::{normalize_version, ResolveOpts, Source}; +use sources::{normalize_version, Remote, ResolveOpts, Source}; use state::{AppState, Installed}; use std::collections::HashMap; use std::path::{Path, PathBuf}; @@ -229,15 +229,15 @@ fn build_packages(st: &AppState, cat: &Catalog, folders: &[AddonFolder]) -> Vec< let (k, e) = direct_key(f); keyed.push((k, e, f)); } - // Second pass: standalone folders that are modules of another addon inherit its key. - let key_of: HashMap)> = keyed - .iter() - .map(|(k, e, f)| (f.folder.to_ascii_lowercase(), (k.clone(), *e))) - .collect(); let has_package_owner = |key: &str, folder: &AddonFolder| { !key.starts_with("folder:") || managed_folder.contains_key(&folder.folder.to_ascii_lowercase()) }; + // Resolve existing module ownership before adopting required dependencies. + let key_of: HashMap)> = keyed + .iter() + .map(|(k, e, f)| (f.folder.to_ascii_lowercase(), (k.clone(), *e))) + .collect(); for item in keyed.iter_mut() { if has_package_owner(&item.0, item.2) { continue; @@ -257,6 +257,34 @@ fn build_packages(st: &AppState, cat: &Catalog, folders: &[AddonFolder]) -> Vec< } } + // Adopt unowned dependency groups only after their module membership is known. + let required_roots: Vec<_> = keyed.iter().filter_map(|(key, entry, f)| { + let e = entry.filter(|e| e.group_required_dependencies)?; + let primary = e.folders.first()?; + primary.eq_ignore_ascii_case(&f.folder).then(|| (key.clone(), e, *f)) + }).collect(); + let mut adopted = HashMap::new(); + for (key, _, f) in &keyed { + if has_package_owner(key, f) || key != &format!("folder:{}", f.folder) + || f.folder.to_ascii_lowercase().starts_with("blizzard_") + { + continue; + } + let mut roots = required_roots.iter().filter(|(_, _, root)| { + root.dependencies.iter().any(|name| name.eq_ignore_ascii_case(&f.folder)) + }); + let Some((owner, entry, _)) = roots.next() else { continue }; + if roots.next().is_none() { + adopted.insert(key.clone(), (owner.clone(), *entry)); + } + } + for item in &mut keyed { + if let Some((key, entry)) = adopted.get(&item.0) { + item.0 = key.clone(); + item.1 = Some(*entry); + } + } + // key -> (entry, folders) let mut groups: Vec<(String, Option<&CatalogEntry>, Vec<&AddonFolder>)> = Vec::new(); let mut index: HashMap = HashMap::new(); @@ -282,17 +310,20 @@ fn build_packages(st: &AppState, cat: &Catalog, folders: &[AddonFolder]) -> Vec< !f.dependencies.iter().any(|d| in_group.contains(&d.to_ascii_lowercase())) && f.part_of.as_ref().map(|p| !in_group.contains(&p.to_ascii_lowercase())).unwrap_or(true) }; - let primary = fs - .iter() - .min_by_key(|f| (!is_root(f), f.folder.len(), f.folder.clone())) - .copied() + let primary = entry + .filter(|e| e.group_required_dependencies) + .and_then(|e| e.folders.first()) + .and_then(|name| fs.iter().copied().find(|f| f.folder.eq_ignore_ascii_case(name))) + .or_else(|| fs.iter() + .min_by_key(|f| (!is_root(f), f.folder.len(), f.folder.clone())) + .copied()) .unwrap(); let managed = st.installed.get(&key); let name = entry .map(|e| e.name.clone()) .unwrap_or_else(|| primary.title.clone()); - let installed_version = managed - .map(|m| m.version.clone()) + let installed_version = managed.map(|m| m.version.clone()) + .or_else(|| entry.filter(|e| e.group_required_dependencies).and_then(|_| primary.version.clone())) .or_else(|| fs.iter().find_map(|f| f.version.clone())) .filter(|v| !v.is_empty() && !v.starts_with('@')); let source = pick_source(entry, primary, has_wago, managed) @@ -409,6 +440,75 @@ fn entry_hint(cat: &Catalog, pkg: &Package) -> (Option, Option) { (entry.and_then(|e| e.asset_hint.clone()), entry.and_then(|e| e.forever)) } +fn entry_for_source<'a>(cat: &'a Catalog, key: &str, src: &Source) -> Option<&'a CatalogEntry> { + key.strip_prefix("cat:").and_then(|id| cat.addons.iter().find(|e| e.id == id)) + .or_else(|| cat.addons.iter().find(|e| match src { + Source::Github(repo) => e.github.as_deref().map(|g| g.eq_ignore_ascii_case(repo)).unwrap_or(false), + Source::Wago(id) => e.wago.as_ref() == Some(id), + Source::Wowi(id) => e.wowi == Some(*id), + Source::Tukui(id) => e.tukui.as_ref() == Some(id), + })) +} + +fn key_for_source(cat: &Catalog, st: &AppState, src: &Source, requested: Option<&str>) -> String { + let entry = entry_for_source(cat, "", src); + if let Some(key) = requested { + if st.installed.contains_key(key) || entry.is_none() { + return key.to_string(); + } + } + let canonical = entry.map(|e| format!("cat:{}", e.id)) + .unwrap_or_else(|| format!("{}:{}", src.kind(), src.id())); + if st.installed.contains_key(&canonical) { + return canonical; + } + // Reuse an existing installation of this source. + st.installed.iter().filter(|(_, rec)| { + match (Source::from_parts(&rec.source, &rec.source_id), src) { + (Some(Source::Github(a)), Source::Github(b)) => a.eq_ignore_ascii_case(b), + (Some(a), b) => &a == b, + _ => false, + } + }).map(|(key, _)| key).min().cloned().unwrap_or(canonical) +} + +fn apply_remote(pkg: &mut Package, mut remote: Remote, vouched: Option) { + if remote.forever == Some(false) && vouched == Some(true) { + remote.forever = None; + } + let same = pkg.installed_version.as_deref() + .map(|v| normalize_version(v) == normalize_version(&remote.version)).unwrap_or(false); + pkg.update_available = !same; + pkg.status = if same { "ok".into() } else { "update".into() }; + pkg.note = match remote.forever { + Some(false) => "Latest release does not list a Forever build".into(), + _ => String::new(), + }; + pkg.remote_forever = remote.forever; + pkg.remote_prerelease = remote.prerelease; + pkg.remote_version = Some(remote.version); +} + +fn protected_package_folders(st: &AppState, packages: &[Package], key: &str) -> HashMap { + let mut protected = HashMap::new(); + // Keep recorded ownership even if a separately managed folder is missing. + for (owner, rec) in &st.installed { + if owner != key { + for folder in &rec.folders { + protected.insert(folder.to_ascii_lowercase(), owner.clone()); + } + } + } + for package in packages { + if package.key != key { + for folder in &package.folders { + protected.insert(folder.to_ascii_lowercase(), package.key.clone()); + } + } + } + protected +} + async fn resolve_into(app: &App, pkg: &mut Package, cat: &Catalog) { let Some(src) = pkg.source.clone() else { return }; let st = app.state.lock().await.clone(); @@ -422,27 +522,7 @@ async fn resolve_into(app: &App, pkg: &mut Package, cat: &Catalog) { prerelease: st.allow_prerelease, }; match sources::resolve(&app.client, &src, opts).await { - Ok(mut r) => { - // Some authors list 16001 in the TOC but never added "forever" to - // release.json. If the catalogue vouches for it, don't scare people. - if r.forever == Some(false) && vouched == Some(true) { - r.forever = None; - } - let same = pkg - .installed_version - .as_deref() - .map(|v| normalize_version(v) == normalize_version(&r.version)) - .unwrap_or(false); - pkg.update_available = !same; - pkg.status = if same { "ok".into() } else { "update".into() }; - pkg.note = match r.forever { - Some(false) => "Latest release does not list a Forever build".into(), - _ => String::new(), - }; - pkg.remote_forever = r.forever; - pkg.remote_prerelease = r.prerelease; - pkg.remote_version = Some(r.version); - } + Ok(r) => apply_remote(pkg, r, vouched), Err(e) => { loge!("check {}: {e}", pkg.key); pkg.status = "error".into(); @@ -633,27 +713,30 @@ async fn install_from( let install = current_install(app).await?; let addons = addons_dir(&install); let st = app.state.lock().await.clone(); + let cat = catalog_cached(app).await; + let entry = entry_for_source(&cat, key, src); let opts = ResolveOpts { asset_hint, wago_key: st.wago_key.as_deref(), prerelease: st.allow_prerelease, }; let remote = sources::resolve(&app.client, src, opts).await.map_err(err)?; - let vouched = match key.strip_prefix("cat:") { - Some(id) => catalog_cached(app) - .await - .addons - .iter() - .find(|e| e.id == id) - .and_then(|e| e.forever), - None => None, - }; + let vouched = entry.and_then(|e| e.forever); // A source archive says nothing about flavours (forever = None); then the // catalogue's own verdict stands in for it. let not_forever = remote.forever == Some(false) || (remote.forever.is_none() && vouched == Some(false)); if not_forever && !allow_non_forever && vouched != Some(true) { return Err("NOT_FOREVER".into()); } + let protected = if entry.map(|e| e.group_required_dependencies).unwrap_or(false) { + let scan_path = install.clone(); + let folders = tokio::task::spawn_blocking(move || wow::scan_addons(Path::new(&scan_path))) + .await.map_err(err)?; + let packages = build_packages(&st, &cat, &folders); + protected_package_folders(&st, &packages, key) + } else { + protected_package_folders(&st, &[], key) + }; logi!("install {key} from {} {} -> {}", src.label(), remote.version, remote.download_url); let tmp = addons.join(install::TMP_DIR).join("dl"); let zip = install::download(&app.client, &remote.download_url, &tmp, &remote.filename) @@ -663,7 +746,7 @@ async fn install_from( e.to_string() })?; let addons2 = addons.clone(); - let folders = tokio::task::spawn_blocking(move || install::place(&zip, &addons2)) + let folders = tokio::task::spawn_blocking(move || install::place(&zip, &addons2, &protected)) .await .map_err(err)? .map_err(|e| { @@ -724,6 +807,10 @@ struct CatalogRow { stat_source: Option, } +fn installed_catalog_ids(packages: Vec) -> Vec { + packages.into_iter().filter_map(|p| p.catalog_id).collect() +} + #[tauri::command] async fn catalog_list(app: State<'_, Shared>, refresh: Option) -> Result, String> { if refresh.unwrap_or(false) { @@ -732,10 +819,7 @@ async fn catalog_list(app: State<'_, Shared>, refresh: Option) -> Result = match scan_packages(&app).await { - Ok(p) => p.into_iter().map(|p| p.key).collect(), - Err(_) => Vec::new(), - }; + let installed_ids = scan_packages(&app).await.map(installed_catalog_ids).unwrap_or_default(); Ok(cat .addons .into_iter() @@ -743,7 +827,7 @@ async fn catalog_list(app: State<'_, Shared>, refresh: Option) -> Result) -> Result) -> Result, String> { let cat = catalog_cached(&app).await; let has_wago = app.state.lock().await.has_wago_key(); - let installed_keys: Vec = match scan_packages(&app).await { - Ok(p) => p.into_iter().map(|p| p.key).collect(), - Err(_) => Vec::new(), - }; + let installed_ids = scan_packages(&app).await.map(installed_catalog_ids).unwrap_or_default(); Ok(cat .bundles .iter() @@ -808,7 +889,7 @@ async fn bundles(app: State<'_, Shared>) -> Result, String> { desc: e.desc.clone(), installable: direct || (e.wago.is_some() && has_wago), needs_key: !direct && e.wago.is_some() && !has_wago, - installed: installed_keys.contains(&format!("cat:{}", e.id)), + installed: installed_ids.contains(&e.id), forever: e.forever, } }) @@ -899,7 +980,7 @@ async fn install_catalog( if matches!(src, Source::Wago(_)) && !has_wago { return Err("This addon is on Wago: add your free Wago API key in Settings first".into()); } - let key = format!("cat:{}", e.id); + let key = key_for_source(&cat, &st, &src, None); install_from(&app, &key, &src, e.asset_hint.as_deref(), allow_non_forever.unwrap_or(false)).await?; refreshed(&app, &key).await } @@ -912,7 +993,9 @@ async fn install_source( key: Option, allow_non_forever: Option, ) -> Result { - let key = key.unwrap_or_else(|| format!("{}:{}", source.kind(), source.id())); + let cat = catalog_cached(&app).await; + let st = app.state.lock().await.clone(); + let key = key_for_source(&cat, &st, &source, key.as_deref()); install_from(&app, &key, &source, None, allow_non_forever.unwrap_or(false)).await?; refreshed(&app, &key).await } @@ -925,15 +1008,11 @@ async fn install_github( ) -> Result { let repo = sources::parse_github_repo(&repo) .ok_or_else(|| "Enter a GitHub repo as owner/name or paste its URL".to_string())?; - // If the catalogue already knows this repo, use its key so it groups properly. + // If the catalogue knows this source, retain its package identity. let cat = catalog_cached(&app).await; - let key = cat - .addons - .iter() - .find(|e| e.github.as_deref().map(|g| g.eq_ignore_ascii_case(&repo)).unwrap_or(false)) - .map(|e| format!("cat:{}", e.id)) - .unwrap_or_else(|| format!("github:{repo}")); let src = Source::Github(repo); + let st = app.state.lock().await.clone(); + let key = key_for_source(&cat, &st, &src, None); install_from(&app, &key, &src, None, allow_non_forever.unwrap_or(false)).await?; refreshed(&app, &key).await } @@ -1045,18 +1124,22 @@ async fn import_list(app: State<'_, Shared>, path: String) -> Result = scan_packages(&app).await?.into_iter().map(|p| p.key).collect(); - let has_wago = app.state.lock().await.has_wago_key(); + let cat = catalog_cached(&app).await; + let st = app.state.lock().await.clone(); + let has_wago = st.has_wago_key(); Ok(file .addons .into_iter() .map(|it| { - let installed = have.contains(&it.key); + let key = it.source.as_ref().map(|src| key_for_source(&cat, &st, src, Some(&it.key))) + .unwrap_or(it.key); + let installed = have.contains(&key); let (installable, why) = match &it.source { None => (false, "no source (probably CurseForge-only or a personal addon)".into()), Some(Source::Wago(_)) if !has_wago => (false, "needs a Wago key".into()), Some(_) => (true, String::new()), }; - ImportRow { name: it.name, key: it.key, source: it.source, installed, installable, why } + ImportRow { name: it.name, key, source: it.source, installed, installable, why } }) .collect()) } @@ -1324,17 +1407,22 @@ pub fn cli(args: &[String]) -> i32 { let id = args.get(1).ok_or("usage: install ")?; let cat = catalog_cached(&app).await; let e = cat.addons.iter().find(|e| &e.id == id).cloned().ok_or("not in catalogue")?; - let has_wago = app.state.lock().await.has_wago_key(); + let st = app.state.lock().await.clone(); + let has_wago = st.has_wago_key(); let src = pick_source(Some(&e), &AddonFolder::default(), has_wago, None).ok_or("no source")?; let force = args.iter().any(|a| a == "--force"); - let rec = install_from(&app, &format!("cat:{}", e.id), &src, e.asset_hint.as_deref(), force).await?; + let key = key_for_source(&cat, &st, &src, None); + let rec = install_from(&app, &key, &src, e.asset_hint.as_deref(), force).await?; Ok(serde_json::to_value(rec).unwrap()) } "github" => { let repo = args.get(1).and_then(|r| sources::parse_github_repo(r)).ok_or("usage: github ")?; let force = args.iter().any(|a| a == "--force"); - let key = format!("github:{repo}"); - let rec = install_from(&app, &key, &Source::Github(repo), None, force).await?; + let src = Source::Github(repo); + let cat = catalog_cached(&app).await; + let st = app.state.lock().await.clone(); + let key = key_for_source(&cat, &st, &src, None); + let rec = install_from(&app, &key, &src, None, force).await?; Ok(serde_json::to_value(rec).unwrap()) } "remove" => { @@ -1477,6 +1565,318 @@ mod tests { AddonFolder { group: Some(group.into()), ..folder(name, deps, None) } } + fn dependency_catalog() -> Catalog { + serde_json::from_str(r#"{"addons":[{ + "id":"atlas","name":"Atlas suite","github":"author/Atlas","folders":["Atlas"], + "group_required_dependencies":true + }]}"#).unwrap() + } + + fn dependency_folders() -> Vec { + let mut client = folder("Atlas", &["AtlasData"], None); + client.version = Some("2.4".into()); + client.author = Some("Client author".into()); + let mut data = folder("AtlasData", &[], None); + data.version = Some("7.1".into()); + data.author = Some("Data author".into()); + vec![client, data] + } + + fn remote(version: &str) -> Remote { + Remote { + version: version.into(), + download_url: "https://example.test/Atlas.zip".into(), filename: "Atlas.zip".into(), + forever: Some(true), prerelease: false, + } + } + + + #[test] + fn source_installs_use_catalogue_identity_when_known() { + let catalog = dependency_catalog(); + let mut state = AppState::default(); + let source = Source::Github("AUTHOR/aTlAs".into()); + assert_eq!(key_for_source(&catalog, &state, &source, None), "cat:atlas"); + assert_eq!(key_for_source(&catalog, &state, &source, Some("github:author/Atlas")), "cat:atlas"); + assert_eq!(key_for_source(&catalog, &state, &Source::Github("other/Atlas".into()), None), "github:other/Atlas"); + assert_eq!(key_for_source(&catalog, &state, &Source::Github("other/Atlas".into()), Some("custom:Atlas")), "custom:Atlas"); + assert_eq!(key_for_source(&catalog, &state, &Source::Wowi(123), None), "wowi:123"); + + let recorded = Installed { + source: "github".into(), source_id: "Author/ATLAS".into(), + folders: vec!["Atlas".into()], ..Default::default() + }; + state.installed.insert("github:author/Atlas".into(), recorded.clone()); + assert_eq!(key_for_source(&catalog, &state, &source, None), "github:author/Atlas"); + state.installed.insert("cat:atlas".into(), recorded.clone()); + assert_eq!(key_for_source(&catalog, &state, &source, None), "cat:atlas"); + assert_eq!(key_for_source(&catalog, &state, &source, Some("github:author/Atlas")), "github:author/Atlas"); + state.installed.insert("folder:Atlas".into(), recorded); + assert_eq!(key_for_source(&catalog, &state, &source, Some("folder:Atlas")), "folder:Atlas"); + } + + #[test] + fn catalogue_opt_in_groups_required_dependencies_under_the_primary() { + let mut folders = dependency_folders(); + folders.reverse(); + folders[1].dependencies = vec!["ATLASDATA".into()]; + let packages = build_packages(&AppState::default(), &dependency_catalog(), &folders); + assert_eq!(packages.len(), 1); + let package = &packages[0]; + assert_eq!(package.key, "cat:atlas"); + assert_eq!(package.name, "Atlas suite"); + assert_eq!(package.author.as_deref(), Some("Client author")); + assert_eq!(package.installed_version.as_deref(), Some("2.4")); + assert_eq!(package.source, Some(Source::Github("author/Atlas".into()))); + assert_eq!(package.folders, vec!["Atlas", "AtlasData"]); + assert_eq!(installed_catalog_ids(packages), vec!["atlas"]); + } + + #[test] + fn required_dependency_grouping_is_disabled_by_default() { + let catalog: Catalog = serde_json::from_str(r#"{"addons":[{ + "id":"atlas","name":"Atlas","folders":["Atlas"] + }]}"#).unwrap(); + assert!(!catalog.addons[0].group_required_dependencies); + let packages = build_packages(&AppState::default(), &catalog, &dependency_folders()); + assert_eq!(packages.len(), 2); + assert_eq!(packages.iter().find(|p| p.key == "cat:atlas").unwrap().folders, vec!["Atlas"]); + } + + #[test] + fn opt_in_groups_only_direct_non_blizzard_dependencies() { + let mut folders = dependency_folders(); + folders[0].dependencies.extend(["Blizzard_Shared".into(), "MissingData".into()]); + folders[1].dependencies.push("SharedStorage".into()); + for name in ["Blizzard_Shared", "OptionalAddon", "SharedStorage"] { + folders.push(folder(name, &[], None)); + } + folders.push(folder("IndependentTool", &["AtlasData"], None)); + let packages = build_packages(&AppState::default(), &dependency_catalog(), &folders); + assert_eq!(packages.len(), 5); + let bundle = packages.iter().find(|p| p.key == "cat:atlas").unwrap(); + assert_eq!(bundle.folders, vec!["Atlas", "AtlasData"]); + assert_eq!(bundle.missing_deps.len(), 1); + assert_eq!(bundle.missing_deps[0].folder, "MissingData"); + assert_eq!(packages.iter().find(|p| p.key == "folder:IndependentTool").unwrap().folders, vec!["IndependentTool"]); + } + + #[test] + fn managed_package_keys_preserve_catalogue_membership() { + for owner in ["cat:atlas", "github:author/Atlas", "folder:Atlas"] { + let mut state = AppState::default(); + state.installed.insert(owner.into(), Installed { + source: "github".into(), source_id: "author/Atlas".into(), + folders: vec!["Atlas".into()], version: "recorded-release".into(), ..Default::default() + }); + let packages = build_packages(&state, &dependency_catalog(), &dependency_folders()); + assert_eq!(packages.len(), 1); + assert_eq!(packages[0].key, owner); + assert_eq!(packages[0].folders, vec!["Atlas", "AtlasData"]); + assert_eq!(packages[0].installed_version.as_deref(), Some("recorded-release")); + let installed_ids = installed_catalog_ids(packages); + assert_eq!(installed_ids, vec!["atlas"]); + } + } + + #[test] + fn dependency_modules_use_existing_grouping() { + let mut folders = dependency_folders(); + folders.push(grouped_folder("AtlasOptions", &["Atlas"], "Atlas")); + folders.push(grouped_folder("AtlasDataOptions", &["AtlasData"], "AtlasData")); + folders.push(grouped_folder("AtlasDataLayout", &["AtlasDataOptions"], "AtlasDataOptions")); + folders.reverse(); + let packages = build_packages(&AppState::default(), &dependency_catalog(), &folders); + assert_eq!(packages.len(), 1); + assert_eq!(packages[0].folders, vec!["Atlas", "AtlasData", "AtlasDataLayout", "AtlasDataOptions", "AtlasOptions"]); + assert_eq!(packages[0].installed_version.as_deref(), Some("2.4")); + } + + #[test] + fn required_dependency_grouping_preserves_existing_module_ownership() { + for owner in ["catalogue", "managed", "standalone"] { + for hint in ["part_of", "group", "stem"] { + let mut catalog = dependency_catalog(); + let mut state = AppState::default(); + let expected_key = match owner { + "catalogue" => { + catalog.addons.push(serde_json::from_str(r#"{ + "id":"other","name":"Other addon","folders":["OtherRoot"] + }"#).unwrap()); + "cat:other" + } + "managed" => { + state.installed.insert("github:provider/Other".into(), Installed { + source: "github".into(), source_id: "provider/Other".into(), + folders: vec!["OtherRoot".into()], ..Default::default() + }); + "github:provider/Other" + } + _ => "folder:OtherRoot", + }; + let module_name = if hint == "stem" { "OtherRoot_Data" } else { "SharedData" }; + let options_name = format!("{module_name}Options"); + let mut module = folder(module_name, &["OtherRoot"], None); + match hint { + "part_of" => module.part_of = Some("OtherRoot".into()), + "group" => module.group = Some("OtherRoot".into()), + _ => {} + } + let folders = vec![ + folder("Atlas", &[module_name], None), + module, + grouped_folder(&options_name, &[module_name], module_name), + folder("OtherRoot", &[], None), + ]; + let packages = build_packages(&state, &catalog, &folders); + assert_eq!(packages.len(), 2, "{owner}: {hint}"); + let bundle = packages.iter().find(|p| p.key == "cat:atlas").unwrap(); + assert_eq!(bundle.folders, vec!["Atlas"], "{owner}: {hint}"); + let other = packages.iter().find(|p| p.key == expected_key).unwrap(); + assert_eq!(other.folders, vec!["OtherRoot".to_string(), module_name.into(), options_name]); + let protected = protected_package_folders(&state, &packages, &bundle.key); + assert_eq!(protected.get(&module_name.to_ascii_lowercase()).map(String::as_str), Some(expected_key)); + } + } + } + + #[test] + fn dependency_alone_does_not_identify_a_bundle() { + let mut catalog = dependency_catalog(); + let mut folders = dependency_folders()[1..].to_vec(); + let packages = build_packages(&AppState::default(), &catalog, &folders); + assert_eq!(packages.len(), 1); + assert_eq!(packages[0].key, "folder:AtlasData"); + assert_eq!(packages[0].name, "AtlasData"); + assert_eq!(packages[0].installed_version.as_deref(), Some("7.1")); + assert!(packages[0].catalog_id.is_none()); + assert!(packages[0].source.is_none()); + assert!(installed_catalog_ids(packages).is_empty()); + + // Another identity folder cannot activate grouping without the primary. + catalog.addons[0].folders.push("AtlasOptions".into()); + folders.push(folder("AtlasOptions", &["AtlasData"], None)); + let packages = build_packages(&AppState::default(), &catalog, &folders); + assert_eq!(packages.len(), 2); + assert_eq!(packages.iter().find(|p| p.key == "folder:AtlasData").unwrap().folders, vec!["AtlasData"]); + } + + #[test] + fn identified_dependencies_remain_independent() { + for catalogued in [false, true] { + let mut catalog = dependency_catalog(); + let mut folders = dependency_folders(); + let expected = if catalogued { + catalog.addons.push(serde_json::from_str(r#"{ + "id":"atlas-data","name":"Atlas Data","folders":["AtlasData"] + }"#).unwrap()); + "cat:atlas-data" + } else { + folders[1].wago = Some("data-project".into()); + "wago:data-project" + }; + let packages = build_packages(&AppState::default(), &catalog, &folders); + assert_eq!(packages.len(), 2); + assert_eq!(packages.iter().find(|p| p.key == "cat:atlas").unwrap().folders, vec!["Atlas"]); + assert_eq!(packages.iter().find(|p| p.key == expected).unwrap().folders, vec!["AtlasData"]); + } + } + + #[test] + fn ambiguous_required_dependencies_stay_independent() { + let mut catalog = dependency_catalog(); + catalog.addons.push(serde_json::from_str(r#"{ + "id":"survey","name":"Survey","folders":["Survey"],"group_required_dependencies":true + }"#).unwrap()); + let mut folders = dependency_folders(); + folders.push(folder("Survey", &["AtlasData"], None)); + for _ in 0..2 { + let packages = build_packages(&AppState::default(), &catalog, &folders); + assert_eq!(packages.len(), 3); + assert_eq!(packages.iter().find(|p| p.key == "folder:AtlasData").unwrap().folders, vec!["AtlasData"]); + catalog.addons.reverse(); + } + } + + #[test] + fn grouped_packages_use_recorded_provider_versions_for_updates() { + for (kind, id, installed, available) in [ + ("github", "author/Atlas", "Atlas-v2.4+v7.1", "Atlas-v2.4+v7.2"), + ("wago", "atlas-project", "v2.4+v7.1", "v2.4+v7.2"), + ] { + let mut state = AppState::default(); + state.installed.insert("cat:atlas".into(), Installed { + source: kind.into(), source_id: id.into(), version: installed.into(), + folders: vec!["Atlas".into(), "AtlasData".into()], ..Default::default() + }); + let package = build_packages(&state, &dependency_catalog(), &dependency_folders()).pop().unwrap(); + assert_eq!(package.installed_version.as_deref(), Some(installed)); + assert_eq!(package.source, Source::from_parts(kind, id)); + for (version, update) in [(installed, false), (available, true)] { + let mut checked = package.clone(); + apply_remote(&mut checked, remote(version), None); + assert_eq!(checked.update_available, update, "{kind}: {version}"); + assert_eq!(checked.status, if update { "update" } else { "ok" }); + assert_eq!(checked.remote_version.as_deref(), Some(version)); + } + } + } + + #[test] + fn independently_managed_dependencies_keep_their_source_version_and_folders() { + for owner in ["github:provider/AtlasData", "folder:AtlasData"] { + let mut state = AppState::default(); + state.installed.insert(owner.into(), Installed { + source: "github".into(), source_id: "provider/AtlasData".into(), + version: "provider-8".into(), folders: vec!["AtlasData".into()], + ..Default::default() + }); + let mut folders = dependency_folders(); + folders[1].group = Some("Atlas".into()); + folders[1].dependencies = vec!["Atlas".into()]; + folders.push(grouped_folder("AtlasDataOptions", &["AtlasData"], "AtlasData")); + let packages = build_packages(&state, &dependency_catalog(), &folders); + assert_eq!(packages.len(), 2, "{owner}"); + let bundle = packages.iter().find(|p| p.key == "cat:atlas").unwrap(); + assert_eq!(bundle.folders, vec!["Atlas"]); + let provider = packages.iter().find(|p| p.key == owner).unwrap(); + assert_eq!(provider.folders, vec!["AtlasData", "AtlasDataOptions"]); + assert_eq!(provider.installed_version.as_deref(), Some("provider-8")); + assert_eq!(provider.source, Some(Source::Github("provider/AtlasData".into()))); + assert!(provider.managed); + let protected = protected_package_folders(&state, &packages, &bundle.key); + assert_eq!(protected.get("atlasdata").map(String::as_str), Some(owner)); + assert!(!protected.contains_key("atlas")); + } + } + + #[test] + fn absent_separately_owned_dependency_still_protects_its_folder() { + let mut state = AppState::default(); + state.installed.insert("folder:AtlasData".into(), Installed { + source: "github".into(), source_id: "provider/AtlasData".into(), + version: "7.1".into(), folders: vec!["AtlasData".into()], + ..Default::default() + }); + let packages = build_packages(&state, &dependency_catalog(), &dependency_folders()[..1]); + assert_eq!(packages.len(), 1); + assert_eq!(packages[0].missing_deps[0].folder, "AtlasData"); + let protected = protected_package_folders(&state, &packages, "cat:atlas"); + assert_eq!(protected.get("atlasdata").map(String::as_str), Some("folder:AtlasData")); + assert!(!protected.contains_key("atlas")); + } + + #[test] + fn single_addon_updates_keep_legacy_version_matching() { + let mut package = build_packages(&AppState::default(), &Catalog::default(), &[folder("Single", &[], None)]).pop().unwrap(); + apply_remote(&mut package, remote("v1.0"), None); + assert_eq!(package.status, "ok"); + assert!(!package.update_available); + apply_remote(&mut package, remote("1.1"), None); + assert_eq!(package.status, "update"); + assert!(package.update_available); + } + #[test] fn group_and_required_dependency_group_ellesmere_modules() { let mut core = folder("EllesmereUI", &[], None); diff --git a/src-tauri/src/sources/github.rs b/src-tauri/src/sources/github.rs index ae9787c..2020b1c 100644 --- a/src-tauri/src/sources/github.rs +++ b/src-tauri/src/sources/github.rs @@ -241,3 +241,74 @@ pub async fn resolve( let rel: ApiRelease = resp.error_for_status()?.json().await?; from_api_release(client, repo, rel, asset_hint).await } + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::{json, Value}; + + fn bundle_manifest() -> Value { + json!({ + "releases": [{ + "filename": "Questie-v12.2.0+v1.0.7.zip", + "nolib": false, + "metadata": [{"flavor": "forever", "interface": 16001}] + }], + "questie": {"version": "12.2.0"}, + "questiedb": {"version": "1.0.7"} + }) + } + + fn pick(manifest: Value, prerelease: bool) -> anyhow::Result { + pick_from_release_json( + serde_json::from_value(manifest)?, + |name| format!("https://example.test/download/{name}"), + prerelease, + ) + } + + #[test] + fn bundle_uses_whole_package_filename_as_version() { + let remote = pick(bundle_manifest(), false).unwrap(); + assert_eq!(remote.version, "Questie-v12.2.0+v1.0.7"); + assert_eq!(remote.filename, "Questie-v12.2.0+v1.0.7.zip"); + assert_eq!(remote.download_url, "https://example.test/download/Questie-v12.2.0+v1.0.7.zip"); + assert_eq!(remote.forever, Some(true)); + assert!(!remote.prerelease); + } + + #[test] + fn database_only_bundle_release_changes_package_version() { + let current = pick(bundle_manifest(), false).unwrap(); + let mut next_manifest = bundle_manifest(); + next_manifest["releases"][0]["filename"] = json!("Questie-v12.2.0+v1.0.8.zip"); + next_manifest["questiedb"]["version"] = json!("1.0.8"); + let next = pick(next_manifest, false).unwrap(); + assert_eq!(next.version, "Questie-v12.2.0+v1.0.8"); + assert_ne!(next.version, current.version); + assert_eq!(next.download_url, "https://example.test/download/Questie-v12.2.0+v1.0.8.zip"); + } + + #[test] + fn explicit_release_version_takes_precedence_over_filename() { + let legacy = json!({"releases": [{"version": "v2.4", "filename": "Addon.zip"}]}); + let remote = pick(legacy, false).unwrap(); + assert_eq!(remote.version, "v2.4"); + } + + #[test] + fn bundle_preserves_flavor_and_prerelease_selection() { + let mut manifest = bundle_manifest(); + manifest["releases"] = json!([ + {"filename": "retail.zip", "metadata": [{"flavor": "mainline", "interface": 120000}]}, + {"filename": "nolib.zip", "nolib": true, "metadata": [{"flavor": "forever", "interface": 16001}]}, + {"filename": "bundle-beta.zip", "metadata": [{"flavor": "camelot", "interface": 16001}]} + ]); + let remote = pick(manifest, true).unwrap(); + assert_eq!(remote.version, "bundle-beta"); + assert_eq!(remote.filename, "bundle-beta.zip"); + assert_eq!(remote.download_url, "https://example.test/download/bundle-beta.zip"); + assert_eq!(remote.forever, Some(true)); + assert!(remote.prerelease); + } +} From 7ac233d7211b7c88d108cc5358385b2841c8ec41 Mon Sep 17 00:00:00 2001 From: Puschie Date: Mon, 26 Oct 2026 22:54:13 +0100 Subject: [PATCH 3/3] Fix stale folder ownership blocking addon installs Apply ownership protection only to folders present on disk. After successful installation, transfer folder claims to the new owner, preserving unrelated folders and removing emptied records. Add regression coverage for stale claims, existing-folder protection, ownership cleanup, and subsequent updates. --- src-tauri/src/install.rs | 51 +++++++++++++++++-- src-tauri/src/lib.rs | 107 ++++++++++++++++++++++++++++++++++----- 2 files changed, 141 insertions(+), 17 deletions(-) diff --git a/src-tauri/src/install.rs b/src-tauri/src/install.rs index faf86a4..f815a2b 100644 --- a/src-tauri/src/install.rs +++ b/src-tauri/src/install.rs @@ -11,7 +11,7 @@ use anyhow::{anyhow, Context}; use futures_util::StreamExt; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::fs; use std::io::Write; use std::path::{Path, PathBuf}; @@ -242,7 +242,8 @@ pub fn safety_scan(root: &Path) -> anyhow::Result { /// Extract `zip_path` and move its addon folders into `addons_dir`, /// replacing same-named folders. Returns the folder names installed. -/// `protected_folders` maps lowercase folder names to their owning package. +/// `protected_folders` maps lowercase folder names to their owning package; +/// ownership only prevents replacing folders that are still present on disk. pub fn place( zip_path: &Path, addons_dir: &Path, @@ -262,6 +263,10 @@ pub fn place( let files = safety_scan(&extract_dir)?; crate::logi!("zip ok: {} files, no executables", files); + // Compare actual names case-insensitively, including on case-sensitive volumes. + let on_disk: HashSet = fs::read_dir(addons_dir)? + .map(|entry| entry.map(|e| e.file_name().to_string_lossy().to_ascii_lowercase())) + .collect::>()?; // Resolve every final target before moving any installed folder aside. let sources: Vec<(PathBuf, String)> = addon_dirs(&extract_dir)? .into_iter() @@ -272,8 +277,11 @@ pub fn place( if name.starts_with('.') || name.contains("..") { anyhow::bail!("refusing suspicious folder name {name}"); } - if let Some(owner) = protected_folders.get(&name.to_ascii_lowercase()) { - anyhow::bail!("cannot replace addon folder {name}: it belongs to {owner}"); + let lower = name.to_ascii_lowercase(); + if on_disk.contains(&lower) { + if let Some(owner) = protected_folders.get(&lower) { + anyhow::bail!("cannot replace addon folder {name}: it belongs to {owner}"); + } } Ok((src, name)) }) @@ -393,6 +401,41 @@ mod tests { fs::remove_dir_all(&root).unwrap(); } + #[test] + fn missing_protected_folder_can_be_installed_but_is_protected_once_present() { + let root = std::env::temp_dir().join(format!("af-owned-missing-{}", unique())); + let addons = root.join("AddOns"); + let protected = HashMap::from([("shareddata".into(), "github:author/Database".into())]); + let zip = addon_zip(&root, &[ + ("Project-1.2/SharedData_Forever.toc", "## Version: 1\n"), + ("Project-1.2/data.lua", "original database"), + ]); + let installed = place(&zip, &addons, &protected).unwrap(); + assert_eq!(installed, vec!["SharedData"]); + assert_eq!(fs::read_to_string(addons.join("SharedData").join("SharedData_Forever.toc")).unwrap(), "## Version: 1\n"); + assert_eq!(fs::read_to_string(addons.join("SharedData").join("data.lua")).unwrap(), "original database"); + assert!(!addons.join("Project-1.2").exists()); + + fs::create_dir_all(addons.join("AAddon")).unwrap(); + fs::write(addons.join("AAddon").join("keep.lua"), "original primary").unwrap(); + let zip = addon_zip(&root, &[ + ("AAddon/AAddon.toc", "## Version: 2\n"), + ("AAddon/new.lua", "new primary"), + ("Project-2.0/SharedData_Forever.toc", "## Version: 2\n"), + ("Project-2.0/data.lua", "new database"), + ]); + let error = place(&zip, &addons, &protected).unwrap_err().to_string(); + assert!(error.contains("SharedData"), "{error}"); + assert!(error.contains("github:author/Database"), "{error}"); + assert_eq!(fs::read_to_string(addons.join("AAddon").join("keep.lua")).unwrap(), "original primary"); + assert!(!addons.join("AAddon").join("new.lua").exists()); + assert!(!addons.join("AAddon").join("AAddon.toc").exists()); + assert_eq!(fs::read_to_string(addons.join("SharedData").join("SharedData_Forever.toc")).unwrap(), "## Version: 1\n"); + assert_eq!(fs::read_to_string(addons.join("SharedData").join("data.lua")).unwrap(), "original database"); + assert!(!addons.join("Project-2.0").exists()); + fs::remove_dir_all(&root).unwrap(); + } + #[test] fn addon_folders_are_installed_under_canonical_names() { let root = std::env::temp_dir().join(format!("af-folders-{}", unique())); diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index f390195..ad9e3f9 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -13,7 +13,7 @@ use catalog::{Catalog, CatalogEntry, Stats}; use serde::{Deserialize, Serialize}; use sources::{normalize_version, Remote, ResolveOpts, Source}; use state::{AppState, Installed}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; use std::sync::Arc; use tauri::State; @@ -491,7 +491,7 @@ fn apply_remote(pkg: &mut Package, mut remote: Remote, vouched: Option) { fn protected_package_folders(st: &AppState, packages: &[Package], key: &str) -> HashMap { let mut protected = HashMap::new(); - // Keep recorded ownership even if a separately managed folder is missing. + // Include recorded claims; the installer only enforces them for existing folders. for (owner, rec) in &st.installed { if owner != key { for folder in &rec.folders { @@ -509,6 +509,19 @@ fn protected_package_folders(st: &AppState, packages: &[Package], key: &str) -> protected } +fn record_install(st: &mut AppState, key: &str, rec: &Installed) { + let folders: HashSet = rec.folders.iter().map(|f| f.to_ascii_lowercase()).collect(); + // Successful placement transfers any stale claims to the new owner. + st.installed.retain(|owner, previous| { + if owner == key || previous.folders.is_empty() { + return true; + } + previous.folders.retain(|f| !folders.contains(&f.to_ascii_lowercase())); + !previous.folders.is_empty() + }); + st.installed.insert(key.to_string(), rec.clone()); +} + async fn resolve_into(app: &App, pkg: &mut Package, cat: &Catalog) { let Some(src) = pkg.source.clone() else { return }; let st = app.state.lock().await.clone(); @@ -761,7 +774,7 @@ async fn install_from( installed_at: state::now_secs(), }; let mut st = app.state.lock().await; - st.installed.insert(key.to_string(), rec.clone()); + record_install(&mut st, key, &rec); st.save().map_err(err)?; logi!("installed {key} {} ({} folders)", rec.version, rec.folders.len()); Ok(rec) @@ -1851,19 +1864,87 @@ mod tests { } #[test] - fn absent_separately_owned_dependency_still_protects_its_folder() { + fn installing_missing_dependencies_transfers_ownership_and_allows_later_updates() { + use std::fs; + use std::io::Write; + + let root = std::env::temp_dir().join(format!("af-transfer-{}-{}", std::process::id(), state::now_secs())); + let addons = root.join("Interface").join("AddOns"); + fs::create_dir_all(addons.join("OtherData")).unwrap(); + fs::write(addons.join("OtherData").join("OtherData.toc"), "## Version: 7.1\n").unwrap(); + fs::write(addons.join("OtherData").join("keep.lua"), "keep other data").unwrap(); + let partial_owner = "github:provider/AtlasData"; + let stale_owner = "folder:AtlasLayout"; let mut state = AppState::default(); - state.installed.insert("folder:AtlasData".into(), Installed { + state.installed.insert(partial_owner.into(), Installed { source: "github".into(), source_id: "provider/AtlasData".into(), - version: "7.1".into(), folders: vec!["AtlasData".into()], - ..Default::default() + version: "provider-7.1".into(), folders: vec!["AtlasData".into(), "OtherData".into()], + installed_at: 11, }); - let packages = build_packages(&state, &dependency_catalog(), &dependency_folders()[..1]); - assert_eq!(packages.len(), 1); - assert_eq!(packages[0].missing_deps[0].folder, "AtlasData"); - let protected = protected_package_folders(&state, &packages, "cat:atlas"); - assert_eq!(protected.get("atlasdata").map(String::as_str), Some("folder:AtlasData")); - assert!(!protected.contains_key("atlas")); + state.installed.insert(stale_owner.into(), Installed { + folders: vec!["aTlAsLaYoUt".into()], ..Default::default() + }); + state.installed.insert("folder:Absent".into(), Installed { + folders: vec!["Absent".into()], ..Default::default() + }); + state.pinned.insert(stale_owner.into()); + state.pinned.insert(partial_owner.into()); + state.ignored.insert(stale_owner.into()); + state.ignored.insert("folder:Absent".into()); + let pinned = state.pinned.clone(); + let ignored = state.ignored.clone(); + let catalog = dependency_catalog(); + + for (release, data_version) in [("Atlas-v2.4+v7.1", "7.1"), ("Atlas-v2.4+v7.2", "7.2")] { + let packages = build_packages(&state, &catalog, &wow::scan_addons(&root)); + let protected = protected_package_folders(&state, &packages, "cat:atlas"); + let zip_path = root.join("release.zip"); + let mut archive = zip::ZipWriter::new(fs::File::create(&zip_path).unwrap()); + for (name, contents) in [ + ("Atlas/Atlas.toc", "## Version: 2.4\n## RequiredDeps: AtlasData, AtlasLayout\n".into()), + ("AtlasData/AtlasData.toc", format!("## Version: {data_version}\n")), + ("AtlasData/data.lua", format!("database {data_version}")), + ("AtlasLayout/AtlasLayout.toc", "## Version: 1\n".into()), + ] { + archive.start_file(name, zip::write::SimpleFileOptions::default()).unwrap(); + archive.write_all(contents.as_bytes()).unwrap(); + } + archive.finish().unwrap(); + let mut folders = install::place(&zip_path, &addons, &protected).unwrap(); + folders.sort(); + assert_eq!(folders, vec!["Atlas", "AtlasData", "AtlasLayout"]); + assert_eq!(fs::read_to_string(addons.join("AtlasData").join("data.lua")).unwrap(), format!("database {data_version}")); + let installed = Installed { + source: "github".into(), source_id: "author/Atlas".into(), + version: release.into(), folders, installed_at: 22, + }; + record_install(&mut state, "cat:atlas", &installed); + + let remaining = &state.installed[partial_owner]; + assert_eq!(remaining.folders, vec!["OtherData"]); + assert_eq!(remaining.source, "github"); + assert_eq!(remaining.source_id, "provider/AtlasData"); + assert_eq!(remaining.version, "provider-7.1"); + assert_eq!(remaining.installed_at, 11); + assert!(!state.installed.contains_key(stale_owner)); + assert_eq!(state.installed["folder:Absent"].folders, vec!["Absent"]); + assert_eq!(state.installed.len(), 3); + assert_eq!(state.pinned, pinned); + assert_eq!(state.ignored, ignored); + assert_eq!(fs::read_to_string(addons.join("OtherData").join("keep.lua")).unwrap(), "keep other data"); + + let packages = build_packages(&state, &catalog, &wow::scan_addons(&root)); + assert_eq!(packages.len(), 2); + let mut bundle = packages.into_iter().find(|p| p.key == "cat:atlas").unwrap(); + assert_eq!(bundle.folders, vec!["Atlas", "AtlasData", "AtlasLayout"]); + assert_eq!(bundle.source, Some(Source::Github("author/Atlas".into()))); + assert_eq!(bundle.installed_version.as_deref(), Some(release)); + assert!(bundle.missing_deps.is_empty()); + apply_remote(&mut bundle, remote(release), None); + assert!(!bundle.update_available); + assert_eq!(bundle.status, "ok"); + } + fs::remove_dir_all(&root).unwrap(); } #[test]