I think I have the optimistic update thing down?

Author: davidatsurge

shared/src/sync_engine/optimistic/db/object_store.rs

@@ -62,17 +62,20 @@ where
         if self.optimistic_changes.deletes.latest::<S>(id).is_some() {
             return Ok(None);
         }
-        let optimistically_updated = self.optimistic_changes.updates.latest_downcasted::<S>(id);
+        let optimistically_updated = self.optimistic_changes.updates.latest_downcasted::<S>(&id);
         if let Some(o) = optimistically_updated {
             return Ok(Some(MaybeOptimistic::new(o, true)));
         }
-        let optimistically_created = self.optimistic_changes.creations.latest_downcasted::<S>(id);
+        let optimistically_created = self
+            .optimistic_changes
+            .creations
+            .latest_downcasted::<S>(id);
         if let Some(o) = optimistically_created {
             return Ok(Some(MaybeOptimistic::new(o, true)));
         };
 
         self.inner
-            .get(id)
+            .get(&id)
             .await
             .map_err(|e| Error::new(e, self.location))
             .map(|o| o.map(|o| MaybeOptimistic::new(o, false)))
@@ -152,25 +155,28 @@ where
     S: Store + 'static,
     Mode: TxnMode<SupportsReadWrite = Present>,
 {
-    pub async fn delete(&self, id: &S::Id) -> Result<(), Error> {
-        self.inner
-            .delete(id)
-            .await
-            .map_err(|e| Error::new(e, self.location))?;
-        self.optimistic_changes.remove_successful_for_id::<S>(id);
-
+    pub fn add_to_reactivity_during_write(&self, id: &S::Id) {
         let serialized_id = SerializedId::new_from_id::<S>(id);
         let optimistic_id = self
             .optimistic_changes
-            .get_realistic_to_optimistic_for_creations()
-            .get(&serialized_id)
-            .map(|i| i.clone());
+            .get_realistic_to_optimistic_for_creations::<S>(id)
+            .map(|i| SerializedId::new_from_id::<S>(&i));
 
-        let mut reactivity_trackers = self.reactivity_trackers.borrow_mut();
-        reactivity_trackers.add_modification(S::NAME, serialized_id);
         if let Some(optimistic_id) = optimistic_id {
+            let mut reactivity_trackers = self.reactivity_trackers.borrow_mut();
+            reactivity_trackers.add_modification(S::NAME, serialized_id);
             reactivity_trackers.add_modification(S::NAME, optimistic_id);
         }
+    }
+
+    pub async fn delete(&self, id: &S::Id) -> Result<(), Error> {
+        self.inner
+            .delete(id)
+            .await
+            .map_err(|e| Error::new(e, self.location))?;
+        self.optimistic_changes.remove_successful_for_id::<S>(id);
+
+        self.add_to_reactivity_during_write(id);
 
         Ok(())
     }
@@ -183,18 +189,7 @@ where
         self.optimistic_changes
             .remove_successful_for_id::<S>(item.id());
 
-        let serialized_id = SerializedId::new_from_row(item);
-        let optimistic_id = self
-            .optimistic_changes
-            .get_realistic_to_optimistic_for_creations()
-            .get(&serialized_id)
-            .map(|i| i.clone());
-
-        let mut reactivity_trackers = self.reactivity_trackers.borrow_mut();
-        reactivity_trackers.add_modification(S::NAME, serialized_id);
-        if let Some(optimistic_id) = optimistic_id {
-            reactivity_trackers.add_modification(S::NAME, optimistic_id);
-        }
+        self.add_to_reactivity_during_write(item.id());
 
         Ok(())
     }

shared/src/sync_engine/optimistic/db/reactivity_trackers.rs

@@ -24,6 +24,10 @@ impl SerializedId {
     pub fn new_from_id<S: Store>(id: &S::Id) -> Self {
         Self(serde_json::to_string(&id).expect("did not expect ids not to be json serializable?"))
     }
+
+    pub fn to_unserialized_id<S: Store>(&self) -> S::Id {
+        serde_json::from_str(&self.0).expect("did not expect ids not to be json de-serializable?")
+    }
 }
 
 #[derive(Debug, Clone, Default)]

shared/src/sync_engine/optimistic/optimistic_change_map.rs

@@ -68,14 +68,16 @@ mod status {
 struct OptimisticChangeMapInner<T, RealisticId> {
     changes:
         HashMap<StoreName, HashMap<SerializedId, BTreeMap<MonotonicTime, Status<T, RealisticId>>>>,
-    realistic_to_optimistic: HashMap<RealisticId, SerializedId>,
+    optimistic_to_realistic: HashMap<RealisticId, SerializedId>,
 }
 
-impl<T, RealisticId> Default for OptimisticChangeMapInner<T, RealisticId> {
+impl<T, RealisticId: std::cmp::Eq + std::hash::Hash> Default
+    for OptimisticChangeMapInner<T, RealisticId>
+{
     fn default() -> Self {
         Self {
             changes: Default::default(),
-            realistic_to_optimistic: Default::default(),
+            optimistic_to_realistic: HashMap::new(),
         }
     }
 }
@@ -92,7 +94,7 @@ impl<T, S> Clone for OptimisticChangeMap<T, S> {
     }
 }
 
-impl<T, S> Default for OptimisticChangeMap<T, S> {
+impl<T, S: std::cmp::Eq + std::hash::Hash + std::fmt::Debug> Default for OptimisticChangeMap<T, S> {
     fn default() -> Self {
         Self {
             inner: Default::default(),
@@ -145,45 +147,44 @@ impl<T, RealisticId: Eq + std::fmt::Debug> OptimisticChangeMap<T, RealisticId> {
         }
     }
 
-    pub fn remove_all_realistic<S: Store>(
-        &self,
-        optimistic_id: &S::Id,
-        realistic_id: &RealisticId,
-    ) {
-        let optimistic_id = SerializedId::new_from_id::<S>(optimistic_id);
+    pub fn remove_all_realistic<S: Store>(&self, realistic_id: &RealisticId) {
         let mut by_id_len = None;
         let mut inner = self.inner.write();
         if let Some(by_id) = inner.changes.get_mut(&S::NAME) {
-            let mut by_time_len = None;
-            if let Some(by_time) = by_id.get_mut(&optimistic_id) {
-                let to_remove_keys = by_time
-                    .iter()
-                    .filter_map(|(time, status)| match status {
-                        Status::Realistic {
-                            realistic_id: realistic_id_candidate,
-                            ..
-                        } if realistic_id_candidate == realistic_id => Some(time),
-                        _ => None,
-                    })
-                    .cloned()
-                    .collect::<Vec<_>>();
-                for key in to_remove_keys {
-                    by_time.remove(&key);
+            for optimistic_id in by_id.keys().cloned().collect::<Vec<_>>().into_iter() {
+                let mut by_time_len = None;
+                if let Some(by_time) = by_id.get_mut(&optimistic_id) {
+                    let to_remove_keys = by_time
+                        .iter()
+                        .filter_map(|(time, status)| match status {
+                            Status::Realistic {
+                                realistic_id: realistic_id_candidate,
+                                ..
+                            } if realistic_id_candidate == realistic_id => Some(time),
+                            _ => None,
+                        })
+                        .cloned()
+                        .collect::<Vec<_>>();
+                    for key in to_remove_keys {
+                        by_time.remove(&key);
+                    }
+                    by_time_len = Some(by_time.len());
                 }
-                by_time_len = Some(by_time.len());
-            }
-            if by_time_len == Some(0) {
-                by_id.remove(&optimistic_id);
+                if by_time_len == Some(0) {
+                    by_id.remove(&optimistic_id);
+                }
+                by_id_len = Some(by_id.len());
             }
-            by_id_len = Some(by_id.len());
         }
         if by_id_len == Some(0) {
             inner.changes.remove(&S::NAME);
         }
     }
 }
 
-impl<T, RealisticId: Clone + Eq + std::hash::Hash> OptimisticChangeMap<T, RealisticId> {
+impl<T, RealisticId: Clone + Eq + std::hash::Hash + std::fmt::Debug>
+    OptimisticChangeMap<T, RealisticId>
+{
     pub fn mark_realistic<S: Store>(
         &self,
         optimistic_id: &S::Id,
@@ -203,12 +204,16 @@ impl<T, RealisticId: Clone + Eq + std::hash::Hash> OptimisticChangeMap<T, Realis
             .mark_realistic(realistic_id.clone());
 
         inner
-            .realistic_to_optimistic
+            .optimistic_to_realistic
             .insert(realistic_id, optimistic_id);
     }
 
-    pub fn get_realistic_to_optimistic(&self) -> HashMap<RealisticId, SerializedId> {
-        self.inner.read().realistic_to_optimistic.clone()
+    pub fn get_realistic_to_optimistic(&self, realistic_id: &RealisticId) -> Option<SerializedId> {
+        self.inner
+            .read()
+            .optimistic_to_realistic
+            .get(realistic_id)
+            .cloned()
     }
 }
 

shared/src/sync_engine/optimistic/optimistic_changes.rs

@@ -1,7 +1,7 @@
 #![allow(dead_code)]
 #![allow(clippy::type_complexity)]
 
-use std::{any::Any, collections::HashMap, future::Future, rc::Rc};
+use std::{any::Any, future::Future, rc::Rc};
 
 use any_spawner::Executor;
 use typesafe_idb::Store;
@@ -87,15 +87,21 @@ impl OptimisticChanges {
         });
     }
 
+    /// This can be refactored (along with mark_realistic).
     pub fn remove_successful_for_id<S: Store>(&self, id: &S::Id) {
         tracing::info!("Called remove successful for id: {id:?}");
-        self.deletes.remove_all_realistic::<S>(id, &());
-        self.updates.remove_all_realistic::<S>(id, &());
+        self.deletes.remove_all_realistic::<S>(&());
+        self.updates.remove_all_realistic::<S>(&());
         self.creations
-            .remove_all_realistic::<S>(id, &SerializedId::new_from_id::<S>(id));
+            .remove_all_realistic::<S>(&SerializedId::new_from_id::<S>(id));
     }
 
-    pub fn get_realistic_to_optimistic_for_creations(&self) -> HashMap<SerializedId, SerializedId> {
-        self.creations.get_realistic_to_optimistic()
+    pub fn get_realistic_to_optimistic_for_creations<S: Store>(
+        &self,
+        realistic_id: &S::Id,
+    ) -> Option<S::Id> {
+        self.creations
+            .get_realistic_to_optimistic(&SerializedId::new_from_id::<S>(realistic_id))
+            .map(|id| id.to_unserialized_id::<S>())
     }
 }