shifathosn
/
photocraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(presets): never overwrite preset or actions files that failed to load (#3346) 

* fix(presets): never overwrite a brush group file that failed to load

A group file skipped at load (corrupt, newer version, missing tips) was replaced on the next sync with only the new presets. Remember skipped files and leave them unchanged with a warning.

* fix(presets): never overwrite an actions.json that failed to load

A corrupt, oversized or newer-version actions.json loaded as an empty list, and the next action change rewrote it with only the new actions. Remember the skip and leave the file unchanged with a warning.
This commit is contained in:
Thomas Han 2026-10-12 10:25:31 +08:00 committed by GitHub
parent 60ef9d836f
commit fb36a3fdc4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 62 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
crates/engine/src/preset_store.rs
View File

@ -538,11 +538,15 @@ pub struct PresetStore {
synced_rev: Option<u64>,
/// Group file name → its last written content.
groups: HashMap<String, Synced>,
/// Group files that failed to load: left on disk untouched, so a sync never writes them.
skipped: HashSet<String>,
/// Tip hash → file size, for every tip file the store owns.
tips: HashMap<String, u64>,
index: Option<IndexFile>,
/// [`crate::actions_cmds::ActionState::rev`] last written (or loaded).
actions_rev: u64,
/// `actions.json` exists but failed to load: left on disk untouched, so it is never written.
actions_skipped: bool,
warnings: Vec<String>,
/// Full tips loaded for brushes in use (behind a lock so `&Session` paths can load).
cache: Mutex<TipCache>,
@ -597,6 +601,7 @@ pub fn open(backend: Box<dyn PresetBackend>) -> Opened {
// Parse the group files.
let mut parsed: Vec<(String, u64, GroupFile)> = Vec::new();
let mut skipped = HashSet::new();
let mut failed = false;
for (name, size) in group_files {
let r = backend.read(&name, MAX_GROUP_BYTES).and_then(|b| serde_json::from_slice::<GroupFile>(&b).map_err(|e| e.to_string())).and_then(|g| {
@ -615,6 +620,7 @@ pub fn open(backend: Box<dyn PresetBackend>) -> Opened {
Err(e) => {
failed = true;
warnings.push(format!("brush presets: {name} skipped: {e}"));
skipped.insert(name);
}
}
}
@ -632,14 +638,18 @@ pub fn open(backend: Box<dyn PresetBackend>) -> Opened {
}
let (tiles, regenerated) = load_tiles(backend.as_ref(), &wanted, &tip_sizes, &info, &mut warnings);
let actions = load_actions(backend.as_ref(), &mut warnings);
let loaded_actions = load_actions(backend.as_ref(), &mut warnings);
let actions_skipped = loaded_actions.is_none();
let actions = loaded_actions.unwrap_or_default();
let mut store = PresetStore {
backend,
synced_rev: None,
groups: HashMap::new(),
skipped: HashSet::new(),
tips: HashMap::new(),
index: None,
actions_rev: 0,
actions_skipped,
warnings: Vec::new(),
cache: Mutex::new(TipCache { limit: TIP_CACHE_BYTES, ..Default::default() }),
};
@ -668,6 +678,7 @@ pub fn open(backend: Box<dyn PresetBackend>) -> Opened {
// Left on disk untouched (a sync never deletes a file it did not load).
failed = true;
warnings.push(format!("brush presets: {file} skipped: {missing} preset(s) have missing or unreadable tips"));
skipped.insert(file);
continue;
}
for h in &hashes {
@ -678,6 +689,7 @@ pub fn open(backend: Box<dyn PresetBackend>) -> Opened {
presets.extend(items.iter().cloned());
store.groups.insert(file, Synced { presets: items, tips: hashes, bytes: size, stale });
}
store.skipped = skipped;
// Tip files nothing references are garbage (e.g. left by a crash), unless a group failed to
// load: its tips must survive so a fixed file still works.
if !failed {
@ -692,24 +704,25 @@ pub fn open(backend: Box<dyn PresetBackend>) -> Opened {
}
/// `actions.json`: a missing file is an empty list. Anything else unreadable is a warning.
fn load_actions(backend: &dyn PresetBackend, warnings: &mut Vec<String>) -> Vec<crate::actions_cmds::Action> {
/// The saved actions; `None` when `actions.json` exists but could not be loaded.
fn load_actions(backend: &dyn PresetBackend, warnings: &mut Vec<String>) -> Option<Vec<crate::actions_cmds::Action>> {
let bytes = match backend.read(ACTIONS_FILE, MAX_ACTIONS_BYTES) {
Ok(b) => b,
Err(e) if missing_file(&e) => return Vec::new(),
Err(e) if missing_file(&e) => return Some(Vec::new()),
Err(e) => {
warnings.push(format!("actions: {ACTIONS_FILE} skipped: {e}"));
return Vec::new();
return None;
}
};
match serde_json::from_slice::<crate::actions_cmds::ActionsFile>(&bytes) {
Ok(file) if file.version == 1 => file.actions,
Ok(file) if file.version == 1 => Some(file.actions),
Ok(file) => {
warnings.push(format!("actions: {ACTIONS_FILE} skipped: unsupported version {}", file.version));
Vec::new()
None
}
Err(e) => {
warnings.push(format!("actions: {ACTIONS_FILE} skipped: {e}"));
Vec::new()
None
}
}
}
@ -873,6 +886,13 @@ impl PresetStore {
if unchanged {
continue;
}
if self.skipped.contains(&file) {
self.warnings.push(format!(
"Couldn't save brush presets `{}`: {file} could not be read, so it was left unchanged",
if group.is_empty() { "(ungrouped)" } else { group }
));
continue;
}
let mut items: Vec<&mut BrushPreset> = presets.iter_mut().filter(|p| !p.builtin && p.group == *group).collect();
if let Err(e) = self.write_group(&file, group, &mut items) {
self.warnings.push(format!("Couldn't save brush presets `{}`: {e}", if group.is_empty() { "(ungrouped)" } else { group }));
@ -1013,6 +1033,11 @@ impl PresetStore {
if self.actions_rev == rev {
return;
}
if self.actions_skipped {
self.warnings.push(format!("Couldn't save actions: {ACTIONS_FILE} could not be read, so it was left unchanged"));
self.actions_rev = rev;
return;
}
let file = crate::actions_cmds::ActionsFile { version: 1, actions: list.to_vec() };
let bytes = match serde_json::to_vec_pretty(&file) {
Ok(b) => b,

30
crates/engine/src/preset_store/tests.rs
View File

@ -623,3 +623,33 @@ fn imported_abr_folders_persist_and_old_stores_load_flat() {
assert_eq!(photocraft_paint::presets::find(&d.tools.presets, "Deep").unwrap().folder.len(), crate::brush_preset_cmds::MAX_FOLDER_DEPTH);
assert!(photocraft_paint::presets::find(&d.tools.presets, "Blank").unwrap().folder.is_empty());
}
#[test]
fn a_group_file_that_failed_to_load_is_not_overwritten() {
// e.g. written by a newer PhotoCraft: skipped on load, so a save must not replace it.
let mem = MemBackend::default();
let file = group_file_name("");
let newer = br#"{"format":"photocraft-brush-group","version":2,"group":"","presets":[]}"#.to_vec();
mem.files.lock().unwrap().insert(file.clone(), newer.clone());
let mut s = Session::new();
s.attach_preset_store(open(Box::new(mem.clone())));
s.tools.brush.tip = TipShape::Sampled(tip8(4, 4, 0));
s.execute("brush.presets.save", json!({"name": "New"})).unwrap();
assert_eq!(mem.files.lock().unwrap().get(&file), Some(&newer), "a file the store could not read was overwritten");
assert!(!s.preset_store.as_mut().unwrap().take_warnings().is_empty());
}
#[test]
fn an_actions_file_that_failed_to_load_is_not_overwritten() {
// e.g. written by a newer PhotoCraft: skipped on load, so recording an action must not replace it.
let mem = MemBackend::default();
let newer = br#"{"version":2,"actions":[{"name":"Keep","steps":[]}]}"#.to_vec();
mem.files.lock().unwrap().insert(ACTIONS_FILE.into(), newer.clone());
let mut s = Session::new();
s.attach_preset_store(open(Box::new(mem.clone())));
s.execute("actions.record", json!({"name": "New"})).unwrap();
s.execute("actions.stop", json!({})).unwrap();
s.sync_preset_store();
assert_eq!(mem.files.lock().unwrap().get(ACTIONS_FILE), Some(&newer), "an actions file the store could not read was overwritten");
assert!(!s.preset_store.as_mut().unwrap().take_warnings().is_empty());
}