Refactor persistence to combine document handling and workspace layout (#4031)

* Unify editor state persistence

* Review

* Fix

* Remove redundant DocumentDetails

* LoadDocumentContent indirection

Author: timon-schelling

desktop/src/app.rs

@@ -17,11 +17,10 @@ use crate::cef;
 use crate::cli::Cli;
 use crate::consts::CEF_MESSAGE_LOOP_MAX_ITERATIONS;
 use crate::event::{AppEvent, AppEventScheduler};
-use crate::persist::PersistentData;
+use crate::persist;
 use crate::preferences;
 use crate::render::{RenderError, RenderState};
 use crate::window::Window;
-use crate::workspace_layout;
 use crate::wrapper::messages::{DesktopFrontendMessage, DesktopWrapperMessage, InputMessage, MouseKeys, MouseState, Preferences};
 use crate::wrapper::{DesktopWrapper, NodeGraphExecutionResult, WgpuContext, serialize_frontend_messages};
 
@@ -46,7 +45,6 @@ pub(crate) struct App {
 	start_render_sender: SyncSender<()>,
 	web_communication_initialized: bool,
 	web_communication_startup_buffer: Vec<Vec<u8>>,
-	persistent_data: PersistentData,
 	#[cfg_attr(not(target_os = "macos"), expect(unused))]
 	preferences: Preferences,
 	cli: Cli,
@@ -93,9 +91,6 @@ impl App {
 			}
 		});
 
-		let mut persistent_data = PersistentData::default();
-		persistent_data.load_from_disk();
-
 		let desktop_wrapper = DesktopWrapper::new(rand::rng().random());
 
 		Self {
@@ -119,7 +114,6 @@ impl App {
 			start_render_sender,
 			web_communication_initialized: false,
 			web_communication_startup_buffer: Vec::new(),
-			persistent_data,
 			preferences,
 			cli,
 			startup_time: None,
@@ -285,17 +279,24 @@ impl App {
 					window.request_redraw();
 				}
 			}
-			DesktopFrontendMessage::PersistenceWriteDocument { id, document } => {
-				self.persistent_data.write_document(id, document);
+			DesktopFrontendMessage::PersistenceWriteState { state } => {
+				persist::write_state(state);
 			}
-			DesktopFrontendMessage::PersistenceDeleteDocument { id } => {
-				self.persistent_data.delete_document(&id);
+			DesktopFrontendMessage::PersistenceReadState => {
+				responses.push(DesktopWrapperMessage::LoadPersistedState { state: persist::read_state() });
 			}
-			DesktopFrontendMessage::PersistenceUpdateCurrentDocument { id } => {
-				self.persistent_data.set_current_document(id);
+			DesktopFrontendMessage::PersistenceReadDocument { id } => {
+				if let Some(document) = persist::read_document_content(&id) {
+					responses.push(DesktopWrapperMessage::LoadDocumentContent { id, document });
+				} else {
+					tracing::error!("Failed to read document content for {id:?}");
+				}
 			}
-			DesktopFrontendMessage::PersistenceUpdateDocumentsList { ids } => {
-				self.persistent_data.force_document_order(ids);
+			DesktopFrontendMessage::PersistenceWriteDocument { id, document_serialized_content } => {
+				persist::write_document_content(id, document_serialized_content);
+			}
+			DesktopFrontendMessage::PersistenceDeleteDocument { id } => {
+				persist::delete_document(&id);
 			}
 			DesktopFrontendMessage::PersistenceWritePreferences { preferences } => {
 				preferences::write(preferences);
@@ -305,30 +306,6 @@ impl App {
 				let message = DesktopWrapperMessage::LoadPreferences { preferences };
 				responses.push(message);
 			}
-			DesktopFrontendMessage::PersistenceWriteWorkspaceLayout { workspace_layout: layout } => {
-				workspace_layout::write(&layout);
-			}
-			DesktopFrontendMessage::PersistenceLoadWorkspaceLayout => {
-				if let Some(workspace_layout) = workspace_layout::read() {
-					let message = DesktopWrapperMessage::LoadWorkspaceLayout { workspace_layout };
-					responses.push(message);
-				}
-			}
-			DesktopFrontendMessage::PersistenceLoadDocuments => {
-				// Open all documents in persisted tab order, then select the current one
-				for (id, document) in self.persistent_data.documents() {
-					responses.push(DesktopWrapperMessage::LoadDocument {
-						id,
-						document,
-						to_front: false,
-						select_after_open: false,
-					});
-				}
-
-				if let Some(id) = self.persistent_data.current_document_id() {
-					responses.push(DesktopWrapperMessage::SelectDocument { id });
-				}
-			}
 			DesktopFrontendMessage::OpenLaunchDocuments => {
 				if self.cli.files.is_empty() {
 					return;

desktop/src/consts.rs

@@ -9,7 +9,6 @@ pub(crate) const APP_DIRECTORY_NAME: &str = "Graphite";
 pub(crate) const APP_LOCK_FILE_NAME: &str = "instance.lock";
 pub(crate) const APP_STATE_FILE_NAME: &str = "state.ron";
 pub(crate) const APP_PREFERENCES_FILE_NAME: &str = "preferences.ron";
-pub(crate) const APP_WORKSPACE_LAYOUT_FILE_NAME: &str = "workspace_layout.ron";
 pub(crate) const APP_DOCUMENTS_DIRECTORY_NAME: &str = "documents";
 
 // CEF configuration constants

desktop/src/dirs.rs

@@ -84,3 +84,11 @@ impl AsRef<Path> for TempDir {
 		&self.path
 	}
 }
+
+// TODO: Eventually remove this cleanup code for the old "browser" CEF directory
+pub(crate) fn delete_old_cef_browser_directory() {
+	let old_browser_dir = crate::dirs::app_data_dir().join("browser");
+	if old_browser_dir.is_dir() {
+		let _ = std::fs::remove_dir_all(&old_browser_dir);
+	}
+}

desktop/src/lib.rs

@@ -20,7 +20,6 @@ mod persist;
 mod preferences;
 mod render;
 mod window;
-mod workspace_layout;
 
 pub(crate) mod consts;
 
@@ -65,6 +64,9 @@ pub fn start() {
 
 	dirs::app_tmp_dir_cleanup();
 
+	// TODO: Eventually remove this cleanup code for the old "browser" CEF directory
+	dirs::delete_old_cef_browser_directory();
+
 	let prefs = preferences::read();
 
 	// Must be called before event loop initialization or native window integrations will break

desktop/src/persist.rs

@@ -1,169 +1,92 @@
-use crate::wrapper::messages::{Document, DocumentId, PersistedDocumentInfo};
-
-#[derive(Default, serde::Serialize, serde::Deserialize)]
-pub(crate) struct PersistentData {
-	documents: Vec<PersistedDocumentInfo>,
-	current_document: Option<DocumentId>,
-	#[serde(skip)]
-	document_order: Option<Vec<DocumentId>>,
-}
-
-impl PersistentData {
-	pub(crate) fn write_document(&mut self, id: DocumentId, document: Document) {
-		let info = PersistedDocumentInfo {
-			id,
-			name: document.name.clone(),
-			path: document.path.clone(),
-			is_saved: document.is_saved,
-		};
-		if let Some(existing) = self.documents.iter_mut().find(|doc| doc.id == id) {
-			*existing = info;
-		} else {
-			self.documents.push(info);
+use crate::wrapper::messages::{DocumentId, PersistedState};
+
+pub(crate) fn read_state() -> PersistedState {
+	let path = state_file_path();
+	let data = match std::fs::read_to_string(&path) {
+		Ok(d) => d,
+		Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
+			tracing::info!("No persistent data file found at {path:?}, starting fresh");
+			return PersistedState::default();
 		}
-
-		if let Err(e) = std::fs::write(Self::document_content_path(&id), document.content) {
-			tracing::error!("Failed to write document {id:?} to disk: {e}");
+		Err(e) => {
+			tracing::error!("Failed to read persistent data from disk: {e}");
+			return PersistedState::default();
 		}
-
-		self.flush();
-	}
-
-	pub(crate) fn delete_document(&mut self, id: &DocumentId) {
-		if Some(*id) == self.current_document {
-			self.current_document = None;
-		}
-
-		self.documents.retain(|doc| doc.id != *id);
-		if let Err(e) = std::fs::remove_file(Self::document_content_path(id)) {
-			tracing::error!("Failed to delete document {id:?} from disk: {e}");
+	};
+	let loaded = match ron::from_str(&data) {
+		Ok(d) => d,
+		Err(e) => {
+			tracing::error!("Failed to deserialize persistent data: {e}");
+			return PersistedState::default();
 		}
+	};
 
-		self.flush();
-	}
+	garbage_collect_document_files(&loaded);
+	loaded
+}
 
-	pub(crate) fn current_document_id(&self) -> Option<DocumentId> {
-		match self.current_document {
-			Some(id) => Some(id),
-			None => Some(self.documents.first()?.id),
+pub(crate) fn write_state(state: PersistedState) {
+	let state: &PersistedState = &state;
+	let data = match ron::ser::to_string_pretty(state, Default::default()) {
+		Ok(d) => d,
+		Err(e) => {
+			tracing::error!("Failed to serialize persistent data: {e}");
+			return;
 		}
+	};
+	if let Err(e) = std::fs::write(state_file_path(), data) {
+		tracing::error!("Failed to write persistent data to disk: {e}");
 	}
+	garbage_collect_document_files(&state);
+}
 
-	pub(crate) fn documents(&self) -> Vec<(DocumentId, Document)> {
-		self.documents.iter().filter_map(|doc| Some((doc.id, self.read_document(&doc.id)?))).collect()
+pub(crate) fn write_document_content(id: DocumentId, document_content: String) {
+	if let Err(e) = std::fs::write(document_content_path(&id), document_content) {
+		tracing::error!("Failed to write document {id:?} to disk: {e}");
 	}
+}
 
-	pub(crate) fn set_current_document(&mut self, id: DocumentId) {
-		self.current_document = Some(id);
-		self.flush();
-	}
+pub(crate) fn read_document_content(id: &DocumentId) -> Option<String> {
+	std::fs::read_to_string(document_content_path(id)).ok()
+}
 
-	pub(crate) fn force_document_order(&mut self, order: Vec<DocumentId>) {
-		let mut ordered_prefix_length = 0;
-		for id in &order {
-			if let Some(offset) = self.documents[ordered_prefix_length..].iter().position(|doc| doc.id == *id) {
-				let found_index = ordered_prefix_length + offset;
-				if found_index != ordered_prefix_length {
-					self.documents[ordered_prefix_length..=found_index].rotate_right(1);
-				}
-				ordered_prefix_length += 1;
-			}
-		}
-		self.document_order = Some(order);
-		self.flush();
+pub(crate) fn delete_document(id: &DocumentId) {
+	if let Err(e) = std::fs::remove_file(document_content_path(id)) {
+		tracing::error!("Failed to delete document {id:?} from disk: {e}");
 	}
+}
 
-	fn read_document(&self, id: &DocumentId) -> Option<Document> {
-		let info = self.documents.iter().find(|doc| doc.id == *id)?;
-		let content = std::fs::read_to_string(Self::document_content_path(id)).ok()?;
-		Some(Document {
-			content,
-			name: info.name.clone(),
-			path: info.path.clone(),
-			is_saved: info.is_saved,
-		})
-	}
+fn garbage_collect_document_files(state: &PersistedState) {
+	let valid_paths: std::collections::HashSet<_> = state.documents.iter().map(|doc| document_content_path(&doc.id)).collect();
 
-	fn flush(&self) {
-		let data = match ron::ser::to_string_pretty(self, Default::default()) {
-			Ok(d) => d,
-			Err(e) => {
-				tracing::error!("Failed to serialize persistent data: {e}");
-				return;
-			}
-		};
-		if let Err(e) = std::fs::write(Self::state_file_path(), data) {
-			tracing::error!("Failed to write persistent data to disk: {e}");
+	let directory = crate::dirs::app_autosave_documents_dir();
+	let entries = match std::fs::read_dir(&directory) {
+		Ok(entries) => entries,
+		Err(e) if e.kind() == std::io::ErrorKind::NotFound => return,
+		Err(e) => {
+			tracing::error!("Failed to read autosave documents directory: {e}");
+			return;
 		}
-	}
-
-	pub(crate) fn load_from_disk(&mut self) {
-		delete_old_cef_browser_directory();
-
-		let path = Self::state_file_path();
-		let data = match std::fs::read_to_string(&path) {
-			Ok(d) => d,
-			Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
-				tracing::info!("No persistent data file found at {path:?}, starting fresh");
-				return;
-			}
-			Err(e) => {
-				tracing::error!("Failed to read persistent data from disk: {e}");
-				return;
-			}
-		};
-		let loaded = match ron::from_str(&data) {
-			Ok(d) => d,
-			Err(e) => {
-				tracing::error!("Failed to deserialize persistent data: {e}");
-				return;
-			}
-		};
-		*self = loaded;
-
-		self.garbage_collect_document_files();
-	}
-
-	fn garbage_collect_document_files(&self) {
-		let valid_paths: std::collections::HashSet<_> = self.documents.iter().map(|doc| Self::document_content_path(&doc.id)).collect();
+	};
 
-		let directory = crate::dirs::app_autosave_documents_dir();
-		let entries = match std::fs::read_dir(&directory) {
-			Ok(entries) => entries,
-			Err(e) if e.kind() == std::io::ErrorKind::NotFound => return,
-			Err(e) => {
-				tracing::error!("Failed to read autosave documents directory: {e}");
-				return;
-			}
-		};
-
-		for entry in entries.flatten() {
-			let path = entry.path();
-			if path.is_file() && !valid_paths.contains(&path) {
-				if let Err(e) = std::fs::remove_file(&path) {
-					tracing::error!("Failed to remove orphaned document file {path:?}: {e}");
-				}
+	for entry in entries.flatten() {
+		let path = entry.path();
+		if path.is_file() && !valid_paths.contains(&path) {
+			if let Err(e) = std::fs::remove_file(&path) {
+				tracing::error!("Failed to remove orphaned document file {path:?}: {e}");
 			}
 		}
 	}
+}
 
-	fn state_file_path() -> std::path::PathBuf {
-		let mut path = crate::dirs::app_data_dir();
-		path.push(crate::consts::APP_STATE_FILE_NAME);
-		path
-	}
-
-	fn document_content_path(id: &DocumentId) -> std::path::PathBuf {
-		let mut path = crate::dirs::app_autosave_documents_dir();
-		path.push(format!("{:x}.{}", id.0, graphite_desktop_wrapper::FILE_EXTENSION));
-		path
-	}
+fn state_file_path() -> std::path::PathBuf {
+	let mut path = crate::dirs::app_data_dir();
+	path.push(crate::consts::APP_STATE_FILE_NAME);
+	path
 }
 
-// TODO: Eventually remove this cleanup code for the old "browser" CEF directory
-fn delete_old_cef_browser_directory() {
-	let old_browser_dir = crate::dirs::app_data_dir().join("browser");
-	if old_browser_dir.is_dir() {
-		let _ = std::fs::remove_dir_all(&old_browser_dir);
-	}
+fn document_content_path(id: &DocumentId) -> std::path::PathBuf {
+	let mut path = crate::dirs::app_autosave_documents_dir();
+	path.push(format!("{:x}.{}", id.0, graphite_desktop_wrapper::FILE_EXTENSION));
+	path
 }