From 028d6661ca75b06b85b51e8e37ad6189d277a83a Mon Sep 17 00:00:00 2001 From: Lucas Nogueira Date: Tue, 15 Oct 2024 09:52:20 -0300 Subject: [PATCH] reintroduce "get existing store" behavior for create_or_load --- plugins/store/guest-js/index.ts | 6 +++++- plugins/store/src/lib.rs | 10 ++++++---- plugins/store/src/store.rs | 13 ++++++++++++- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/plugins/store/guest-js/index.ts b/plugins/store/guest-js/index.ts index 1f2c0cff..ae14ddc9 100644 --- a/plugins/store/guest-js/index.ts +++ b/plugins/store/guest-js/index.ts @@ -46,7 +46,11 @@ export async function create( } /** - * Create a new Store or load the existing store with the path + * Create a new Store or load the existing store with the path. + * + * If the store at the given path is already loaded, + * its instance is returned regardless of the options object. + * If the settings to not match an error is returned. * * @param path Path to save the store in `app_data_dir` * @param options Store configuration options diff --git a/plugins/store/src/lib.rs b/plugins/store/src/lib.rs index f9cf5bc0..9251fa00 100644 --- a/plugins/store/src/lib.rs +++ b/plugins/store/src/lib.rs @@ -299,7 +299,9 @@ pub trait StoreExt { /// Get a handle of an already loaded store. /// /// If the store is not loaded or does not exist, it returns `None`. - /// In this case, you should initialize it with [`Self::store`]. + /// + /// Note that using this function can cause race conditions if you fallback to creating or loading the store, + /// so you should consider using [`Self::store`] if you are not sure if the store is loaded or not. /// /// # Examples /// @@ -312,6 +314,9 @@ pub trait StoreExt { /// let store = if let Some(s) = app.get_store("store.json") { /// s /// } else { + /// // this is not thread safe; if another thread is doing the same load/create, + /// // there will be a race condition; in this case we could remove the get_store + /// // and only run app.store() as it will return the existing store if it has been loaded /// app.store("store.json")? /// }; /// Ok(()) @@ -323,9 +328,6 @@ pub trait StoreExt { impl> StoreExt for T { fn store(&self, path: impl AsRef) -> Result>> { let path = path.as_ref(); - if let Some(store) = self.get_store(path) { - return Ok(store); - } StoreBuilder::new(self.app_handle(), path).create_or_load() } diff --git a/plugins/store/src/store.rs b/plugins/store/src/store.rs index 1d8d2dd1..8091f132 100644 --- a/plugins/store/src/store.rs +++ b/plugins/store/src/store.rs @@ -230,7 +230,11 @@ impl StoreBuilder { self.build_inner(false) } - /// Get the existing store with the same path or creates a new [`Store`], also see [`create`](Self::create). + /// Get the existing store with the same path or creates a new [`Store`]. + /// + /// If a store with the same path has already been loaded its instance is returned. + /// + /// See [`create`](Self::create) if you want to force create a new store in the path. /// /// # Examples /// ``` @@ -247,6 +251,13 @@ impl StoreBuilder { } pub(crate) fn create_or_load_inner(self) -> crate::Result<(Arc>, ResourceId)> { + { + let state = self.app.state::(); + let stores = state.stores.lock().unwrap(); + if let Some(rid) = stores.get(&self.path) { + return Ok((self.app.resources_table().get(*rid).unwrap(), *rid)); + } + } self.build_inner(true) } }