fix: avoid panic during scan

vincent-herlemont · Aug 24, 2024 · 93771b0 · 93771b0
1 parent 488a620
commit 93771b0
Show file tree

Hide file tree

Showing 9 changed files with 70 additions and 71 deletions.
diff --git a/README.md b/README.md
@@ -95,7 +95,7 @@ fn main() -> Result<(), db_type::Error> {
     let retrieve_data: Item = r.get().primary(3_u32)?.unwrap();
     println!("data id='3': {:?}", retrieve_data);
     // Iterate items with name starting with "red"
-    for item in r.scan().secondary::<Item>(ItemKey::name)?.start_with("red") {
+    for item in r.scan().secondary::<Item>(ItemKey::name)?.start_with("red")? {
         println!("data name=\"red\": {:?}", item);
     }
 

diff --git a/benches/overhead_data_size.rs b/benches/overhead_data_size.rs
@@ -134,7 +134,7 @@ fn use_native_db_insert(db: &Database, data: Data) {
 
 fn use_native_db_scan(db: &Database) -> Vec<Data> {
     let r = db.r_transaction().unwrap();
-    let out = r.scan().primary().unwrap().all().try_collect().unwrap();
+    let out = r.scan().primary().unwrap().all().unwrap().try_collect().unwrap();
     out
 }
 

diff --git a/src/transaction/query/scan/primary_scan.rs b/src/transaction/query/scan/primary_scan.rs
@@ -49,16 +49,16 @@ where
     ///     let r = db.r_transaction()?;
     ///     
     ///     // Get all values
-    ///     let _values: Vec<Data> = r.scan().primary()?.all().try_collect()?;
+    ///     let _values: Vec<Data> = r.scan().primary()?.all().unwrap().try_collect()?;
     ///     Ok(())
     /// }
     /// ```
-    pub fn all(&self) -> PrimaryScanIterator<T> {
-        let range = self.primary_table.range::<Key>(..).unwrap();
-        PrimaryScanIterator {
+    pub fn all(&self) -> Result<PrimaryScanIterator<T>> {
+        let range = self.primary_table.range::<Key>(..)?;
+        Ok(PrimaryScanIterator {
             range,
             _marker: PhantomData::default(),
-        }
+        })
     }
 
     /// Iterate over all values in a range.
@@ -87,20 +87,19 @@ where
     ///     let r = db.r_transaction()?;
     ///     
     ///     // Get the values from 5 to the end
-    ///     let _values: Vec<Data> = r.scan().primary()?.range(5u64..).try_collect()?;
+    ///     let _values: Vec<Data> = r.scan().primary()?.range(5u64..)?.try_collect()?;
     ///     Ok(())
     /// }
     /// ```
-    pub fn range<TR: ToKey, R: RangeBounds<TR>>(&self, range: R) -> PrimaryScanIterator<T> {
+    pub fn range<TR: ToKey, R: RangeBounds<TR>>(&self, range: R) -> Result<PrimaryScanIterator<T>> {
         let database_inner_key_value_range = KeyRange::new(range);
         let range = self
             .primary_table
-            .range::<Key>(database_inner_key_value_range)
-            .unwrap();
-        PrimaryScanIterator {
+            .range::<Key>(database_inner_key_value_range)?;
+        Ok(PrimaryScanIterator {
             range,
             _marker: PhantomData::default(),
-        }
+        })
     }
 
     /// Iterate over all values starting with a prefix.
@@ -129,24 +128,24 @@ where
     ///     let r = db.r_transaction()?;
     ///     
     ///     // Get the values starting with "victor"
-    ///     let _values: Vec<Data> = r.scan().primary()?.start_with("victor").try_collect()?;
+    ///     let _values: Vec<Data> = r.scan().primary()?.start_with("victor")?.try_collect()?;
     ///     Ok(())
     /// }
     /// ```
     pub fn start_with<'a>(
         &'a self,
         start_with: impl ToKey + 'a,
-    ) -> PrimaryScanIteratorStartWith<'a, T> {
+    ) -> Result<PrimaryScanIteratorStartWith<'a, T>> {
         let start_with = start_with.to_key();
         let range = self
             .primary_table
-            .range::<Key>(start_with.clone()..)
-            .unwrap();
-        PrimaryScanIteratorStartWith {
-            start_with,
+            .range::<Key>(start_with.clone()..)?;
+
+        Ok(PrimaryScanIteratorStartWith {
             range,
+            start_with,
             _marker: PhantomData::default(),
-        }
+        })
     }
 }
 

diff --git a/src/transaction/query/scan/secondary_scan.rs b/src/transaction/query/scan/secondary_scan.rs
@@ -61,31 +61,25 @@ where
     ///     let r = db.r_transaction()?;
     ///     
     ///     // Get only values that have the secondary key set (name is not None)
-    ///     let _values: Vec<Data> = r.scan().secondary(DataKey::name)?.all().try_collect()?;
+    ///     let _values: Vec<Data> = r.scan().secondary(DataKey::name)?.all().unwrap().try_collect()?;
     ///     Ok(())
     /// }
     /// ```
-    /// TODO: remove unwrap and return Result
-    pub fn all(&self) -> SecondaryScanIterator<PrimaryTable, T> {
+    pub fn all(&self) -> Result<SecondaryScanIterator<PrimaryTable, T>> {
         let mut primary_keys = vec![];
-        for keys in self.secondary_table.iter().unwrap() {
-            let (l_secondary_key, l_primary_keys) = keys.unwrap();
-            dbg!(&l_secondary_key.value());
+        for keys in self.secondary_table.iter()? {
+            let (_, l_primary_keys) = keys?;
             for primary_key in l_primary_keys {
-                let primary_key = primary_key.unwrap();
+                let primary_key = primary_key?;
                 primary_keys.push(primary_key);
             }
         }
 
-        for primary_key in primary_keys.iter() {
-            dbg!(&primary_key.value());
-        }
-
-        SecondaryScanIterator {
+        Ok(SecondaryScanIterator {
             primary_table: &self.primary_table,
             primary_keys: primary_keys.into_iter(),
             _marker: PhantomData::default(),
-        }
+        })
     }
 
     /// Iterate over all values by secondary key.
@@ -118,20 +112,19 @@ where
     ///     let r = db.r_transaction()?;
     ///     
     ///     // Get only values that have the secondary key name from C to the end
-    ///     let _values: Vec<Data> = r.scan().secondary(DataKey::name)?.range("C"..).try_collect()?;
+    ///     let _values: Vec<Data> = r.scan().secondary(DataKey::name)?.range("C"..).unwrap().try_collect()?;
     ///     Ok(())
     /// }
     /// ```
     pub fn range<TR: ToKey, R: RangeBounds<TR>>(
         &self,
         range: R,
-    ) -> SecondaryScanIterator<PrimaryTable, T> {
+    ) -> Result<SecondaryScanIterator<PrimaryTable, T>> {
         let mut primary_keys = vec![];
         let database_inner_key_value_range = KeyRange::new(range);
         for keys in self
             .secondary_table
-            .range::<Key>(database_inner_key_value_range)
-            .unwrap()
+            .range::<Key>(database_inner_key_value_range)?
         {
             let (_, l_primary_keys) = keys.unwrap();
             for primary_key in l_primary_keys {
@@ -140,11 +133,11 @@ where
             }
         }
 
-        SecondaryScanIterator {
+        Ok(SecondaryScanIterator {
             primary_table: &self.primary_table,
             primary_keys: primary_keys.into_iter(),
             _marker: PhantomData::default(),
-        }
+        })
     }
 
     /// Iterate over all values by secondary key.
@@ -177,20 +170,19 @@ where
     ///     let r = db.r_transaction()?;
     ///     
     ///     // Get only values that have the secondary key name starting with "hello"
-    ///     let _values: Vec<Data> = r.scan().secondary(DataKey::name)?.start_with("hello").try_collect()?;
+    ///     let _values: Vec<Data> = r.scan().secondary(DataKey::name)?.start_with("hello").unwrap().try_collect()?;
     ///     Ok(())
     /// }
     /// ```
     pub fn start_with<'a>(
         &'a self,
         start_with: impl ToKey + 'a,
-    ) -> SecondaryScanIterator<'a, PrimaryTable, T> {
+    ) -> Result<SecondaryScanIterator<'a, PrimaryTable, T>> {
         let start_with = start_with.to_key();
         let mut primary_keys = vec![];
         for keys in self
             .secondary_table
-            .range::<Key>(start_with.clone()..)
-            .unwrap()
+            .range::<Key>(start_with.clone()..)?
         {
             let (l_secondary_key, l_primary_keys) = keys.unwrap();
             if !l_secondary_key
@@ -206,11 +198,11 @@ where
             }
         }
 
-        SecondaryScanIterator {
+        Ok(SecondaryScanIterator {
             primary_table: &self.primary_table,
             primary_keys: primary_keys.into_iter(),
             _marker: PhantomData::default(),
-        }
+        })
     }
 }
 

diff --git a/src/transaction/rw_transaction.rs b/src/transaction/rw_transaction.rs
@@ -283,7 +283,7 @@ impl<'db, 'txn> RwTransaction<'db> {
         OldType: ToInput + Clone,
         NewType: ToInput + From<OldType>,
     {
-        let find_all_old: Result<Vec<OldType>> = self.scan().primary()?.all().collect();
+        let find_all_old: Result<Vec<OldType>> = self.scan().primary()?.all()?.collect();
         let find_all_old = find_all_old?;
         for old in find_all_old {
             let new: NewType = old.clone().into();

diff --git a/tests/deserialization_error.rs b/tests/deserialization_error.rs
@@ -33,7 +33,7 @@ fn create_local_database_for_tests() {
     models.define::<Item1>().unwrap();
     let db = Builder::new().open(&models, &database_path).unwrap();
     let r = db.r_transaction().unwrap();
-    let result: Result<Vec<Item1>> = r.scan().primary().unwrap().all().try_collect();
+    let result: Result<Vec<Item1>> = r.scan().primary().unwrap().all().unwrap().try_collect();
     assert!(matches!(
         result,
         Err(Error::ModelError(ModelError::DecodeBodyError(_)))

diff --git a/tests/migrate/with_secondary_keys.rs b/tests/migrate/with_secondary_keys.rs
@@ -165,7 +165,7 @@ fn test_migrate() {
         .secondary(ItemV2Key::first_name_key)
         .unwrap()
         .start_with("Alexandre")
-        .try_collect()
+        .unwrap().try_collect()
         .unwrap();
     assert_eq!(
         item,
@@ -182,7 +182,7 @@ fn test_migrate() {
         .secondary(ItemV2Key::last_name_key)
         .unwrap()
         .start_with("Verne")
-        .try_collect()
+        .unwrap().try_collect()
         .unwrap();
     assert_eq!(
         item,