fix: data-safety fixes from the project review

- autosave: pages a failed flush could not write no longer end up in the
  notebook that is opened next; they go to the recovery folder
- app: finish an open text edit and save before switching notebooks, and
  stay in the notebook when saving fails
- trash: purge by the deletion time in the entry name, not by the folder's
  modification time, which survives the move into .trash
- notebooks are identified by their folder name, so a copied folder is a
  notebook of its own instead of an alias of the original
- manifest: fall back to manifest.json.bak only for a damaged file, not for
  a newer format version or an I/O error
- strokes keep their width after scaling and a restart
- PDF export draws shapes with their stroke width instead of a fixed 1.5 pt
- a first page that fails to load shows the warning; a first page that was
  never saved is an empty page; make-space refuses read-only pages

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
This commit is contained in:
haraldandClaude Fable 5.2 committed 2026-11-14 15:39:54 +01:00
1 parent 7c69edab12
commit c2652ea05b
6 files changed
+540 -135

No files matched your search

+191 -113
View File
@@ -125,9 +125,16 @@ fn space_release(
return; return;
}; };
if let Some(run) = notebook.space_run(drag.page_index) { if let Some(run) = notebook.space_run(drag.page_index) {
for index in run { for index in run.clone() {
let _ = storage::ensure_page_loaded(storage_dir, notebook, index); let _ = storage::ensure_page_loaded(storage_dir, notebook, index);
} }
if notebook.pages[run].iter().any(|p| p.is_readonly()) {
ui_state.set_export_message(
t!("Platz schaffen nicht möglich: Eine der Seiten konnte nicht geladen werden")
.to_string(),
);
return;
}
} }
let dy = drag.clamped_dy(); let dy = drag.clamped_dy();
if let Some(changed) = notebook.make_space(drag.page_index, drag.y, dy) { if let Some(changed) = notebook.make_space(drag.page_index, drag.y, dy) {
@@ -1017,6 +1024,32 @@ fn finish_text_edit(
canvas.sync_page(notebook.current_page_mut()); canvas.sync_page(notebook.current_page_mut());
} }
/// Before another notebook takes the place of the open one: finishes a running text edit
/// (it belongs to this notebook, page ids repeat in every notebook) and saves everything.
/// Returns `false` and tells the user when saving failed; the notebook then has to stay open,
/// otherwise the unsaved changes would be lost.
fn save_before_switch(
canvas: &mut CanvasState,
notebook: &mut Notebook,
manifest: &mut NotebookManifest,
autosave: &mut AutosaveController,
ui_state: &mut UiState,
) -> bool {
finish_text_edit(canvas, notebook, manifest, autosave, ui_state);
match autosave.flush() {
Ok(()) => true,
Err(err) => {
let msg = err.messages.join(", ");
eprintln!("[Autosave] Notebook not switched, saving failed: {msg}");
ui_state.set_export_message(t!(
"Nicht gewechselt, Speichern fehlgeschlagen: {err}",
err = msg
));
false
}
}
}
/// Text-Werkzeug: Tippen auf das Blatt beendet eine laufende Bearbeitung und öffnet das /// Text-Werkzeug: Tippen auf das Blatt beendet eine laufende Bearbeitung und öffnet das
/// Textfeld unter `page_pt` zum Bearbeiten – oder ein neues an dieser Stelle. /// Textfeld unter `page_pt` zum Bearbeiten – oder ein neues an dieser Stelle.
fn text_tool_click( fn text_tool_click(
@@ -4853,11 +4886,18 @@ pub fn run_with(
if id == manifest.id { if id == manifest.id {
ui_state.is_document_menu_open = false; ui_state.is_document_menu_open = false;
window.request_redraw(); window.request_redraw();
} else if !save_before_switch(
&mut canvas,
&mut notebook,
&mut manifest,
&mut autosave,
&mut ui_state,
) {
window.request_redraw();
} else { } else {
let new_notebook_dir = storage_base_dir.join(&id); let new_notebook_dir = storage_base_dir.join(&id);
match storage::load_notebook(&new_notebook_dir) { match storage::load_notebook(&new_notebook_dir) {
Ok((loaded_nb, loaded_manifest)) => { Ok((loaded_nb, loaded_manifest)) => {
let _ = autosave.flush();
match storage::NotebookLock::acquire( match storage::NotebookLock::acquire(
&new_notebook_dir, &new_notebook_dir,
) { ) {
@@ -4920,71 +4960,78 @@ pub fn run_with(
} }
} }
UiAction::ImportFile(path) => { UiAction::ImportFile(path) => {
// Neben das gerade geöffnete Notizbuch if save_before_switch(
let folder = &mut canvas,
ui_state.library.folder_of(&manifest.id).to_string(); &mut notebook,
let title = path &mut manifest,
.file_stem() &mut autosave,
.map(|s| s.to_string_lossy().into_owned()) &mut ui_state,
.unwrap_or_else(|| "Import".to_string());
match crate::import::import_file(
&storage_base_dir,
&title,
&path,
) { ) {
Ok((new_nb, new_manifest, new_dir, report)) => { // Neben das gerade geöffnete Notizbuch
let id = new_manifest.id.clone(); let folder =
change_library( ui_state.library.folder_of(&manifest.id).to_string();
&storage_base_dir, let title = path
&mut ui_state, .file_stem()
|library| library.move_notebook(&id, &folder), .map(|s| s.to_string_lossy().into_owned())
); .unwrap_or_else(|| "Import".to_string());
let _ = autosave.flush(); match crate::import::import_file(
_notebook_lock = &storage_base_dir,
storage::NotebookLock::acquire(&new_dir).ok(); &title,
autosave.set_read_only(_notebook_lock.is_none()); &path,
autosave.switch_notebook_dir(new_dir.clone()); ) {
notebook = new_nb; Ok((new_nb, new_manifest, new_dir, report)) => {
manifest = new_manifest; let id = new_manifest.id.clone();
storage_dir = new_dir; change_library(
selection_state.clear(); &storage_base_dir,
ui_state.current_notebook_id = manifest.id.clone(); &mut ui_state,
ui_state.document_title = manifest.title.clone(); |library| library.move_notebook(&id, &folder),
refresh_notebooks(&storage_base_dir, &mut ui_state); );
canvas.reset_view(); _notebook_lock =
canvas.sync_page(notebook.current_page_mut()); storage::NotebookLock::acquire(&new_dir).ok();
update_window_title( autosave.set_read_only(_notebook_lock.is_none());
&window, autosave.switch_notebook_dir(new_dir.clone());
&notebook, notebook = new_nb;
&manifest, manifest = new_manifest;
ui_state.active_tool, storage_dir = new_dir;
autosave.status(), selection_state.clear();
); ui_state.current_notebook_id = manifest.id.clone();
let mut message = t!( ui_state.document_title = manifest.title.clone();
"'{title}' importiert ({count} Seiten)", refresh_notebooks(&storage_base_dir, &mut ui_state);
title = manifest.title, canvas.reset_view();
count = notebook.pages.len() canvas.sync_page(notebook.current_page_mut());
); update_window_title(
if let Some(pdf) = &report.missing_pdf { &window,
message.push_str(&t!( &notebook,
", PDF-Hintergrund nicht gefunden: {file}", &manifest,
file = pdf ui_state.active_tool,
autosave.status(),
);
let mut message = t!(
"'{title}' importiert ({count} Seiten)",
title = manifest.title,
count = notebook.pages.len()
);
if let Some(pdf) = &report.missing_pdf {
message.push_str(&t!(
", PDF-Hintergrund nicht gefunden: {file}",
file = pdf
));
}
if report.skipped > 0 {
message.push_str(&t!(
", {count} Elemente übersprungen",
count = report.skipped
));
}
ui_state.set_export_message(message);
}
Err(err) => {
eprintln!("[Import] '{}': {err}", path.display());
ui_state.set_export_message(t!(
"Nicht importiert: {err}",
err = err
)); ));
} }
if report.skipped > 0 {
message.push_str(&t!(
", {count} Elemente übersprungen",
count = report.skipped
));
}
ui_state.set_export_message(message);
}
Err(err) => {
eprintln!("[Import] '{}': {err}", path.display());
ui_state.set_export_message(t!(
"Nicht importiert: {err}",
err = err
));
} }
} }
window.request_redraw(); window.request_redraw();
@@ -4994,54 +5041,66 @@ pub fn run_with(
folder, folder,
layout, layout,
} => { } => {
let _ = autosave.flush(); if save_before_switch(
match storage::create_new_notebook(&storage_base_dir, &title) { &mut canvas,
Ok((mut new_nb, mut new_manifest, new_dir)) => { &mut notebook,
let id = new_manifest.id.clone(); &mut manifest,
new_manifest.layout = Some(layout); &mut autosave,
if layout == NotebookLayout::Endless { &mut ui_state,
new_nb.make_endless(); ) {
match storage::create_new_notebook(
&storage_base_dir,
&title,
) {
Ok((mut new_nb, mut new_manifest, new_dir)) => {
let id = new_manifest.id.clone();
new_manifest.layout = Some(layout);
if layout == NotebookLayout::Endless {
new_nb.make_endless();
}
change_library(
&storage_base_dir,
&mut ui_state,
|library| library.move_notebook(&id, &folder),
);
_notebook_lock =
storage::NotebookLock::acquire(&new_dir).ok();
autosave.set_read_only(_notebook_lock.is_none());
autosave.switch_notebook_dir(new_dir.clone());
notebook = new_nb;
manifest = new_manifest;
storage_dir = new_dir;
autosave.mark_dirty(&notebook, &mut manifest);
canvas.reset_view();
selection_state.clear();
ui_state.current_notebook_id = manifest.id.clone();
ui_state.document_title = manifest.title.clone();
refresh_notebooks(&storage_base_dir, &mut ui_state);
ui_state.is_document_menu_open = false;
canvas.sync_page(notebook.current_page_mut());
update_window_title(
&window,
&notebook,
&manifest,
ui_state.active_tool,
autosave.status(),
);
ui_state.set_export_message(t!(
"Dokument '{title}' erstellt",
title = manifest.title
));
window.request_redraw();
}
Err(err) => {
eprintln!("[Dokument-Fehler] Konnte Dokument '{title}' nicht erstellen: {err}");
ui_state.set_export_message(t!(
"Fehler beim Erstellen: {err}",
err = err
));
} }
change_library(
&storage_base_dir,
&mut ui_state,
|library| library.move_notebook(&id, &folder),
);
_notebook_lock =
storage::NotebookLock::acquire(&new_dir).ok();
autosave.set_read_only(_notebook_lock.is_none());
autosave.switch_notebook_dir(new_dir.clone());
notebook = new_nb;
manifest = new_manifest;
storage_dir = new_dir;
autosave.mark_dirty(&notebook, &mut manifest);
canvas.reset_view();
selection_state.clear();
ui_state.current_notebook_id = manifest.id.clone();
ui_state.document_title = manifest.title.clone();
refresh_notebooks(&storage_base_dir, &mut ui_state);
ui_state.is_document_menu_open = false;
canvas.sync_page(notebook.current_page_mut());
update_window_title(
&window,
&notebook,
&manifest,
ui_state.active_tool,
autosave.status(),
);
ui_state.set_export_message(t!(
"Dokument '{title}' erstellt",
title = manifest.title
));
window.request_redraw();
}
Err(err) => {
eprintln!("[Dokument-Fehler] Konnte Dokument '{title}' nicht erstellen: {err}");
ui_state.set_export_message(t!(
"Fehler beim Erstellen: {err}",
err = err
));
} }
} else {
window.request_redraw();
} }
} }
UiAction::RenameNotebook { id, new_title } => { UiAction::RenameNotebook { id, new_title } => {
@@ -5111,7 +5170,21 @@ pub fn run_with(
.unwrap_or_else(|| id.clone()); .unwrap_or_else(|| id.clone());
if is_current { if is_current {
let _ = autosave.flush(); finish_text_edit(
&mut canvas,
&mut notebook,
&mut manifest,
&mut autosave,
&mut ui_state,
);
if let Err(err) = autosave.flush() {
eprintln!(
"[Autosave] Not saved before deleting: {}",
err.messages.join(", ")
);
}
// Nothing may be written into the folder once it is in the trash
autosave.abandon_pending();
drop(_notebook_lock.take()); drop(_notebook_lock.take());
} }
@@ -5423,8 +5496,13 @@ pub fn run_with(
ui_state.set_export_message( ui_state.set_export_message(
t!("Die Notizbücher liegen bereits dort").to_string(), t!("Die Notizbücher liegen bereits dort").to_string(),
); );
} else { } else if save_before_switch(
let _ = autosave.flush(); &mut canvas,
&mut notebook,
&mut manifest,
&mut autosave,
&mut ui_state,
) {
match storage::copy_storage(&storage_base_dir, &new_base) { match storage::copy_storage(&storage_base_dir, &new_base) {
Ok(copied) => { Ok(copied) => {
// Release lock and open marker in the old location // Release lock and open marker in the old location
+66 -1
View File
@@ -618,11 +618,44 @@ impl AutosaveController {
} }
/// Wechselt das Zielverzeichnis für zukünftige Speicheroperationen (z. B. beim Dokumentenwechsel). /// Wechselt das Zielverzeichnis für zukünftige Speicheroperationen (z. B. beim Dokumentenwechsel).
///
/// Whatever the flush could not write belongs to the old notebook and must never be
/// written into the new folder, see `abandon_pending`.
pub fn switch_notebook_dir(&mut self, new_notebook_dir: PathBuf) { pub fn switch_notebook_dir(&mut self, new_notebook_dir: PathBuf) {
let _ = self.flush(); if let Err(err) = self.flush() {
eprintln!(
"[Autosave] Unsaved changes of '{}' dropped on notebook switch: {}",
self.notebook_dir.display(),
err.messages.join(", ")
);
}
self.abandon_pending();
self.notebook_dir = new_notebook_dir; self.notebook_dir = new_notebook_dir;
} }
/// Gives up on everything still waiting to be written: it goes into the recovery folder
/// and leaves the queue. For the moment the notebook folder stops being the target
/// (switch to another notebook, notebook moved to the trash).
pub fn abandon_pending(&mut self) {
if !self.pending_pages.is_empty() {
match self.export_recovery(None) {
Ok(files) => {
for file in files {
eprintln!("[Autosave] Recovery copy: {}", file.display());
}
}
Err(e) => eprintln!("[Autosave] Recovery copy failed: {e}"),
}
}
self.pending_pages.clear();
self.pending_manifest = None;
self.retries.clear();
self.last_errors.clear();
self.first_change_time = None;
self.last_change_time = None;
self.status = AutosaveStatus::Saved;
}
/// Beendet den Hintergrund-Worker ordnungsgemäß nach einem Flush. /// Beendet den Hintergrund-Worker ordnungsgemäß nach einem Flush.
pub fn shutdown(&mut self) -> Result<(), AutosaveError> { pub fn shutdown(&mut self) -> Result<(), AutosaveError> {
let res = self.flush(); let res = self.flush();
@@ -787,6 +820,38 @@ pub mod tests {
} }
} }
#[test]
fn test_switch_drops_pages_the_flush_could_not_write() {
let clock = Arc::new(MockClock::new(Instant::now()));
let store = Arc::new(FakePageStore::new());
*store.fail_page_writes.lock().unwrap() = 1;
let recovery = tempfile::tempdir().unwrap();
std::env::set_var("XDG_DATA_HOME", recovery.path());
let mut controller = AutosaveController::with_options(
PathBuf::from("/tmp/old_nb"),
Duration::from_millis(100),
Duration::from_secs(10),
Arc::clone(&store) as Arc<dyn PageStore>,
Arc::clone(&clock) as Arc<dyn Clock>,
);
let page = Page::new(7, PageBackground::Grid5mm);
let manifest = NotebookManifest::new("old_nb", "Old");
controller.mark_page_dirty(&page, &manifest);
controller.switch_notebook_dir(PathBuf::from("/tmp/new_nb"));
let manifests_before = store.manifest_writes.load(Ordering::SeqCst);
// Nothing of the old notebook may reach the new folder
assert!(controller.flush().is_ok());
assert_eq!(store.page_writes.load(Ordering::SeqCst), 0);
assert_eq!(
store.manifest_writes.load(Ordering::SeqCst),
manifests_before
);
assert_eq!(controller.status(), AutosaveStatus::Saved);
}
#[test] #[test]
fn test_debounce_bundles_changes() { fn test_debounce_bundles_changes() {
let clock = Arc::new(MockClock::new(Instant::now())); let clock = Arc::new(MockClock::new(Instant::now()));
+27 -1
View File
@@ -691,6 +691,15 @@ impl Stroke {
if let Some(ref mut shape) = self.shape { if let Some(ref mut shape) = self.shape {
shape.scale(origin, factors); shape.scale(origin, factors);
} }
// The line width grows with the stroke. It is stored and the mesh is rebuilt from it
// on load; without this a scaled stroke would be back at its old width after a restart.
// Images and text fields have no line width.
if !self.is_object() {
let factor = (factors.x * factors.y).abs().sqrt();
if factor.is_finite() && factor > 0.0 {
self.width *= factor;
}
}
self.recompute_aabb(); self.recompute_aabb();
} }
@@ -1574,7 +1583,12 @@ impl Notebook {
/// der Seite und gibt die Kennungen der geänderten Seiten zurück (`None`: nichts zu tun). /// der Seite und gibt die Kennungen der geänderten Seiten zurück (`None`: nichts zu tun).
pub fn make_space(&mut self, page_index: usize, y: f32, dy: f32) -> Option<Vec<u64>> { pub fn make_space(&mut self, page_index: usize, y: f32, dy: f32) -> Option<Vec<u64>> {
let run = self.space_run(page_index)?; let run = self.space_run(page_index)?;
if self.pages[run.clone()].iter().any(|p| !p.is_loaded) { // A page that could not be loaded is never saved: strokes pushed onto it would be gone
// after a restart, and its own content is unknown
if self.pages[run.clone()]
.iter()
.any(|p| !p.is_loaded || p.is_readonly())
{
return None; return None;
} }
let dy = if dy < 0.0 { let dy = if dy < 0.0 {
@@ -2680,6 +2694,18 @@ mod tests {
); );
} }
#[test]
fn test_make_space_refuses_pages_that_failed_to_load() {
let mut nb = Notebook::new();
nb.add_page(PageBackground::Blank);
nb.pages[0].strokes = vec![block(1, 1000.0, 100.0)];
nb.pages[1].persist = PersistState::ReadOnlyAfterLoadError;
assert!(nb.make_space(0, 900.0, 300.0).is_none());
assert_eq!(top_of(&nb, 0, 1), Some(1000.0));
nb.pages[1].persist = PersistState::Normal;
assert!(nb.make_space(0, 900.0, 300.0).is_some());
}
#[test] #[test]
fn test_remove_and_restore_current_page() { fn test_remove_and_restore_current_page() {
let mut nb = Notebook::new(); let mut nb = Notebook::new();
+27 -4
View File
@@ -553,7 +553,7 @@ fn render_ink_layer(
)); ));
} }
if let Some(ref shape) = stroke.shape { if let Some(ref shape) = stroke.shape {
render_recognized_shape(layer, shape, stroke.color); render_recognized_shape(layer, shape, stroke.color, stroke.width);
} else { } else {
match strategy { match strategy {
ExportStrategy::FilledPolygon => { ExportStrategy::FilledPolygon => {
@@ -831,20 +831,31 @@ fn render_pdf_raster(
draw_image_xobject(layer, xobject, &pdf.corners(page.size)); draw_image_xobject(layer, xobject, &pdf.corners(page.size));
} }
/// Line width of a shape in PDF points, as wide as its mesh on screen
/// (`geometry::shape_to_mesh`: the stroke width in page units, at least 2). Shapes without a
/// stored width get the former fixed 1.5 pt (about 0.5 mm).
fn shape_outline_pt(width_units: f32) -> f32 {
if width_units <= 0.0 || !width_units.is_finite() {
return 1.5;
}
width_units.max(2.0) * (A4_WIDTH_MM / PAGE_WIDTH) * (72.0 / 25.4)
}
/// Rendert eine mathematisch bereinigte Form (Gerade, Kreis, Rechteck, Koordinatensystem) /// Rendert eine mathematisch bereinigte Form (Gerade, Kreis, Rechteck, Koordinatensystem)
/// als verlustfreien Vektorpfad im PDF. /// als verlustfreien Vektorpfad im PDF.
fn render_recognized_shape( fn render_recognized_shape(
layer: &crate::pdf_writer::PdfLayerReference, layer: &crate::pdf_writer::PdfLayerReference,
shape: &crate::geometry::RecognizedShape, shape: &crate::geometry::RecognizedShape,
color: [f32; 4], color: [f32; 4],
width_units: f32,
) { ) {
use crate::geometry::RecognizedShape; use crate::geometry::RecognizedShape;
let stroke_color = Color::Rgb(Rgb::new(color[0], color[1], color[2], None)); let stroke_color = Color::Rgb(Rgb::new(color[0], color[1], color[2], None));
layer.set_outline_color(stroke_color); layer.set_outline_color(stroke_color);
layer.set_outline_thickness(1.5); // ca. 0.5 mm Standard-Liniendicke layer.set_outline_thickness(shape_outline_pt(width_units));
// Wie auf dem Bildschirm: spitze Ecken und stumpfe Enden (der Grafikzustand kann noch // Wie auf dem Bildschirm: spitze Ecken und stumpfe Enden (der Grafikzustand kann noch
// runde Verbindungen von einem vorherigen Freihandstrich enthalten) // runde Verbindungen von einem vorherigen Freihandstrich enthalten)
layer.set_line_cap_style(crate::pdf_writer::LineCapStyle::Butt); layer.set_line_cap_style(crate::pdf_writer::LineCapStyle::Butt);
layer.set_line_join_style(crate::pdf_writer::LineJoinStyle::Miter); layer.set_line_join_style(crate::pdf_writer::LineJoinStyle::Miter);
@@ -1588,6 +1599,18 @@ mod tests {
assert!(bytes.starts_with(b"%PDF-1.")); assert!(bytes.starts_with(b"%PDF-1."));
} }
#[test]
fn test_shape_outline_follows_stroke_width() {
let mm = |pt: f32| pt * 25.4 / 72.0;
// 5 page units are 0.5 mm, a 100 unit highlighter line is 10 mm wide
assert!((mm(shape_outline_pt(5.0)) - 0.5).abs() < 1e-3);
assert!((mm(shape_outline_pt(100.0)) - 10.0).abs() < 1e-3);
// The mesh is never thinner than 2 units
assert!((mm(shape_outline_pt(0.5)) - 0.2).abs() < 1e-3);
assert_eq!(shape_outline_pt(0.0), 1.5);
assert_eq!(shape_outline_pt(f32::NAN), 1.5);
}
#[test] #[test]
fn test_image_matrix_maps_unit_square_to_corners() { fn test_image_matrix_maps_unit_square_to_corners() {
// Bild über die volle Seitenbreite, oberes Viertel der Seite // Bild über die volle Seitenbreite, oberes Viertel der Seite
+8
View File
@@ -494,6 +494,14 @@ pub const EN: &[(&str, &str)] = &[
("🚀 Updates & Versionen", "🚀 Updates & versions"), ("🚀 Updates & Versionen", "🚀 Updates & versions"),
(", PDF-Hintergrund nicht gefunden: {file}", ", PDF background not found: {file}"), (", PDF-Hintergrund nicht gefunden: {file}", ", PDF background not found: {file}"),
(", {count} Elemente übersprungen", ", {count} elements skipped"), (", {count} Elemente übersprungen", ", {count} elements skipped"),
(
"Platz schaffen nicht möglich: Eine der Seiten konnte nicht geladen werden",
"Cannot make space: one of the pages could not be loaded",
),
(
"Nicht gewechselt, Speichern fehlgeschlagen: {err}",
"Not switched, saving failed: {err}",
),
("Nicht importiert: {err}", "Not imported: {err}"), ("Nicht importiert: {err}", "Not imported: {err}"),
("Importieren", "Import"), ("Importieren", "Import"),
("📄 Importieren", "📄 Import"), ("📄 Importieren", "📄 Import"),
+221 -16
View File
@@ -630,8 +630,29 @@ fn load_manifest_from_file(path: &Path) -> Result<NotebookManifest, StorageError
} }
/// Lädt das Manifest aus `manifest.json`. /// Lädt das Manifest aus `manifest.json`.
/// Fällt bei Fehlern/Beschädigung automatisch auf `manifest.json.bak` zurück. /// Fällt bei Beschädigung automatisch auf `manifest.json.bak` zurück.
///
/// The folder name is the identity of a notebook: everything opens, renames and deletes
/// `<base>/<id>`. A copied or renamed folder still carries the id of the original in its
/// manifest, so the id is always taken from the folder.
pub fn load_manifest(notebook_dir: &Path) -> Result<NotebookManifest, StorageError> { pub fn load_manifest(notebook_dir: &Path) -> Result<NotebookManifest, StorageError> {
let mut manifest = load_manifest_or_backup(notebook_dir)?;
// Canonical, so that a path like `.` still yields the folder name
let dir = notebook_dir.canonicalize();
let name = dir
.as_deref()
.unwrap_or(notebook_dir)
.file_name()
.and_then(|n| n.to_str());
if let Some(name) = name {
if manifest.id != name {
manifest.id = name.to_string();
}
}
Ok(manifest)
}
fn load_manifest_or_backup(notebook_dir: &Path) -> Result<NotebookManifest, StorageError> {
let path = notebook_dir.join("manifest.json"); let path = notebook_dir.join("manifest.json");
if !path.exists() { if !path.exists() {
let bak_path = notebook_dir.join("manifest.json.bak"); let bak_path = notebook_dir.join("manifest.json.bak");
@@ -647,6 +668,13 @@ pub fn load_manifest(notebook_dir: &Path) -> Result<NotebookManifest, StorageErr
match load_manifest_from_file(&path) { match load_manifest_from_file(&path) {
Ok(manifest) => Ok(manifest), Ok(manifest) => Ok(manifest),
// The backup only stands in for a damaged file. A manifest of a newer app version or
// one that cannot be read right now is intact: opening the older backup instead would
// let the next save overwrite it with stale content.
Err(e @ StorageError::UnsupportedVersion { .. }) => Err(e),
Err(StorageError::Io(e)) if e.kind() != io::ErrorKind::InvalidData => {
Err(StorageError::Io(e))
}
Err(e) => { Err(e) => {
let bak_path = notebook_dir.join("manifest.json.bak"); let bak_path = notebook_dir.join("manifest.json.bak");
if bak_path.exists() { if bak_path.exists() {
@@ -817,8 +845,22 @@ pub fn load_notebook(notebook_dir: &Path) -> Result<(Notebook, NotebookManifest)
page.persist = PersistState::ReadOnlyAfterLoadError; page.persist = PersistState::ReadOnlyAfterLoadError;
page page
} }
// Listed but never saved (still empty): an empty page, as with lazy loading. Nothing
// can be overwritten, there is no file.
Err(StorageError::PageNotFound(_)) => {
Page::new(first_id, first_bg).with_size(manifest.size_for_page(first_id))
}
Err(e) => { Err(e) => {
eprintln!("[Storage] Konnte erste Seite {first_id} nicht laden: {e}. Erstelle leere Ersatzseite."); eprintln!("[Storage] Konnte erste Seite {first_id} nicht laden: {e}. Erstelle leere Ersatzseite.");
// Without the warning the page looks writable while nothing on it is saved
load_warnings.insert(
first_id,
PageLoadWarning {
page_id: first_id,
message: e.to_string(),
quarantine_path: None,
},
);
let mut page = let mut page =
Page::new(first_id, first_bg).with_size(manifest.size_for_page(first_id)); Page::new(first_id, first_bg).with_size(manifest.size_for_page(first_id));
page.persist = PersistState::ReadOnlyAfterLoadError; page.persist = PersistState::ReadOnlyAfterLoadError;
@@ -931,6 +973,14 @@ pub fn ensure_page_loaded(
} }
Err(e) => { Err(e) => {
eprintln!("[Storage] Fehler beim Lazy Loading von Seite {page_id}: {e}"); eprintln!("[Storage] Fehler beim Lazy Loading von Seite {page_id}: {e}");
notebook.load_warnings.insert(
page_id,
PageLoadWarning {
page_id,
message: e.to_string(),
quarantine_path: None,
},
);
notebook.pages[page_idx].is_loaded = true; notebook.pages[page_idx].is_loaded = true;
notebook.pages[page_idx].persist = PersistState::ReadOnlyAfterLoadError; notebook.pages[page_idx].persist = PersistState::ReadOnlyAfterLoadError;
notebook.pages[page_idx].rebuild_cache_if_dirty(); notebook.pages[page_idx].rebuild_cache_if_dirty();
@@ -1502,7 +1552,23 @@ pub fn restore_manifest_from_backup(notebook_dir: &Path) -> Result<(), StorageEr
} }
} }
/// Time an entry was moved into `.trash`, read from the `_<unix seconds>` suffix of its name
/// (`<id>_<ts>`, `sync_<ts>`).
fn trashed_at(name: &str) -> Option<std::time::SystemTime> {
let (_, ts) = name.rsplit_once('_')?;
if ts.is_empty() || !ts.bytes().all(|b| b.is_ascii_digit()) {
return None;
}
let secs = ts.parse::<u64>().ok()?;
std::time::UNIX_EPOCH.checked_add(std::time::Duration::from_secs(secs))
}
/// Entfernt Einträge aus `.trash`, die älter als `max_age` sind. /// Entfernt Einträge aus `.trash`, die älter als `max_age` sind.
///
/// The age counts from the moment of deletion, which is the timestamp in the entry name.
/// The modification time of a folder survives the move into `.trash`, so it would purge a
/// notebook that had not been edited for a while right after it was deleted. It is only the
/// fallback for entries without a timestamp in the name.
pub fn cleanup_trash(base_dir: &Path, max_age: std::time::Duration) -> io::Result<usize> { pub fn cleanup_trash(base_dir: &Path, max_age: std::time::Duration) -> io::Result<usize> {
let trash_dir = base_dir.join(".trash"); let trash_dir = base_dir.join(".trash");
if !trash_dir.exists() { if !trash_dir.exists() {
@@ -1513,20 +1579,25 @@ pub fn cleanup_trash(base_dir: &Path, max_age: std::time::Duration) -> io::Resul
if let Ok(entries) = fs::read_dir(&trash_dir) { if let Ok(entries) = fs::read_dir(&trash_dir) {
for entry in entries.flatten() { for entry in entries.flatten() {
let path = entry.path(); let path = entry.path();
if let Ok(metadata) = entry.metadata() { let since = entry
if let Ok(modified) = metadata.modified() { .file_name()
if let Ok(age) = now.duration_since(modified) { .to_str()
if age >= max_age { .and_then(trashed_at)
if path.is_dir() { .or_else(|| entry.metadata().ok()?.modified().ok());
if fs::remove_dir_all(&path).is_ok() { let expired = since
removed += 1; .and_then(|t| now.duration_since(t).ok())
} .is_some_and(|age| age >= max_age);
} else if fs::remove_file(&path).is_ok() { if !expired {
removed += 1; continue;
} }
} let is_dir = entry.file_type().map(|t| t.is_dir()).unwrap_or(false);
} let gone = if is_dir {
} fs::remove_dir_all(&path)
} else {
fs::remove_file(&path)
};
if gone.is_ok() {
removed += 1;
} }
} }
} }
@@ -1566,6 +1637,77 @@ mod tests {
use std::sync::Arc; use std::sync::Arc;
use std::time::{Duration, Instant}; use std::time::{Duration, Instant};
#[test]
fn test_scaled_stroke_keeps_its_width_after_reload() {
let temp_dir = tempfile::tempdir().unwrap();
let color = [0.0, 0.0, 0.0, 1.0];
let samples: Vec<StylusSample> = (0..20)
.map(|i| StylusSample {
position: glam::Vec2::new(200.0 + 10.0 * i as f32, 500.0),
pressure: 0.5,
timestamp: i as f64 * 0.01,
})
.collect();
let tool = crate::document::StrokeTool::default();
let vertices = reconstruct_stroke_vertices(&samples, color, 5.0, tool, true);
let mut stroke = Stroke::new(1, samples, vertices, color);
stroke.width = 5.0;
stroke.finalized = true;
let thickness = |s: &Stroke| {
let ys = s.vertices.iter().map(|v| v.position[1]);
ys.clone().fold(f32::MIN, f32::max) - ys.fold(f32::MAX, f32::min)
};
let before = thickness(&stroke);
stroke.scale(glam::Vec2::new(200.0, 500.0), glam::Vec2::splat(3.0));
assert!((stroke.width - 15.0).abs() < 1e-4);
let scaled = thickness(&stroke);
assert!((scaled - 3.0 * before).abs() < 0.01);
let mut page = Page::new(1, PageBackground::Blank);
page.add_stroke(stroke);
save_page(temp_dir.path(), &PageData::from(&page)).unwrap();
let loaded = load_page(temp_dir.path(), 1).unwrap().to_page();
assert!((loaded.strokes[0].width - 15.0).abs() < 1e-4);
let reloaded = thickness(&loaded.strokes[0]);
assert!(
(reloaded - scaled).abs() < 0.05 * scaled,
"scaled {scaled}, after reload {reloaded}"
);
}
#[test]
fn test_cleanup_trash_counts_from_deletion_not_from_last_edit() {
let base = tempfile::tempdir().unwrap();
let trash = base.path().join(".trash");
let now = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap()
.as_secs();
let day = 24 * 3600;
let long_ago = std::time::SystemTime::now() - Duration::from_secs(90 * day);
// Deleted just now, last edited 90 days ago: stays
let fresh = trash.join(format!("nb_a_{now}"));
// Deleted 40 days ago: goes
let expired = trash.join(format!("nb_b_{}", now - 40 * day));
// No timestamp in the name: the modification time decides
let unnamed = trash.join("stray");
for dir in [&fresh, &expired, &unnamed] {
fs::create_dir_all(dir).unwrap();
fs::write(dir.join("manifest.json"), "{}").unwrap();
}
for dir in [&fresh, &unnamed] {
fs::File::open(dir).unwrap().set_modified(long_ago).unwrap();
}
let removed = cleanup_trash(base.path(), Duration::from_secs(30 * day)).unwrap();
assert_eq!(removed, 2);
assert!(fresh.exists());
assert!(!expired.exists());
assert!(!unnamed.exists());
}
#[test] #[test]
fn test_atomic_save_and_reload() { fn test_atomic_save_and_reload() {
let temp_dir = tempfile::tempdir().expect("Konnte temporäres Verzeichnis nicht anlegen"); let temp_dir = tempfile::tempdir().expect("Konnte temporäres Verzeichnis nicht anlegen");
@@ -2111,10 +2253,73 @@ mod tests {
fs::write(&broken_manifest_path, b"{ broken json ... ").unwrap(); fs::write(&broken_manifest_path, b"{ broken json ... ").unwrap();
let loaded = load_manifest(nb_dir).expect("Muss aus .bak geladen werden"); let loaded = load_manifest(nb_dir).expect("Muss aus .bak geladen werden");
assert_eq!(loaded.id, "nb_test");
assert_eq!(loaded.title, "Backup Test"); assert_eq!(loaded.title, "Backup Test");
} }
#[test]
fn test_first_page_that_fails_to_load_warns_and_a_missing_one_is_empty() {
let base = tempfile::tempdir().unwrap();
let (_, _, dir) = create_new_notebook(base.path(), "First page").unwrap();
let first = load_manifest(&dir).unwrap().page_ids()[0];
let file = dir.join(format!("page_{first}.json"));
// Never saved: an ordinary empty page
let _ = fs::remove_file(&file);
let (nb, _) = load_notebook(&dir).unwrap();
assert!(!nb.pages[0].is_readonly());
assert!(nb.load_warnings.is_empty());
// Written by a newer version: read-only, and the user is told
let mut data = PageData::from(&Page::new(first, PageBackground::Blank));
data.format_version = CURRENT_FORMAT_VERSION + 1;
fs::write(&file, serde_json::to_vec(&data).unwrap()).unwrap();
let (nb, _) = load_notebook(&dir).unwrap();
assert!(nb.pages[0].is_readonly());
assert!(nb.load_warnings.contains_key(&first));
}
#[test]
fn test_newer_manifest_is_not_replaced_by_its_backup() {
let temp_dir = tempfile::tempdir().unwrap();
let nb_dir = temp_dir.path();
let old = NotebookManifest::new("nb", "Old state");
atomic_write_json(&nb_dir.join("manifest.json.bak"), &old).unwrap();
let mut newer = NotebookManifest::new("nb", "New state");
newer.format_version = CURRENT_FORMAT_VERSION + 1;
atomic_write_json(&nb_dir.join("manifest.json"), &newer).unwrap();
assert!(matches!(
load_manifest(nb_dir),
Err(StorageError::UnsupportedVersion { .. })
));
}
#[test]
fn test_copied_notebook_folder_is_a_notebook_of_its_own() {
let base = tempfile::tempdir().unwrap();
let (_, manifest, dir) = create_new_notebook(base.path(), "Original").unwrap();
let copy = base.path().join("copy_of_it");
fs::create_dir(&copy).unwrap();
fs::copy(dir.join("manifest.json"), copy.join("manifest.json")).unwrap();
let mut ids: Vec<String> = list_notebooks(base.path())
.into_iter()
.map(|s| s.id)
.collect();
ids.sort();
let mut expected = vec!["copy_of_it".to_string(), manifest.id.clone()];
expected.sort();
assert_eq!(ids, expected);
// Opened by its folder name, and the id stays that way once saved
let (_, mut loaded) = load_notebook(&copy).unwrap();
assert_eq!(loaded.id, "copy_of_it");
loaded.title = "Copy".to_string();
save_manifest(&copy, &loaded).unwrap();
assert_eq!(load_manifest(&dir).unwrap().title, "Original");
assert_eq!(load_manifest(&copy).unwrap().title, "Copy");
}
#[test] #[test]
fn test_create_notebook_collision_generates_new_id() { fn test_create_notebook_collision_generates_new_id() {
let temp_dir = tempfile::tempdir().unwrap(); let temp_dir = tempfile::tempdir().unwrap();