Return Option<T> instead of erroring with api::Error:::KeyNotFound (#171

)
ava-labs · Aug 1, 2023 · 63b2022 · 63b2022
1 parent f08367a
commit 63b2022
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 33 deletions.
diff --git a/firewood/src/v2/api.rs b/firewood/src/v2/api.rs
@@ -62,8 +62,6 @@ pub enum Error {
         provided: HashKey,
         current: HashKey,
     },
-    /// Key not found
-    KeyNotFound,
     IO(std::io::Error),
     InvalidProposal,
 }
@@ -131,10 +129,13 @@ pub trait DbView {
     async fn root_hash(&self) -> Result<HashKey, Error>;
 
     /// Get the value of a specific key
-    async fn val<K: KeyType>(&self, key: K) -> Result<Vec<u8>, Error>;
+    async fn val<K: KeyType>(&self, key: K) -> Result<Option<Vec<u8>>, Error>;
 
     /// Obtain a proof for a single key
-    async fn single_key_proof<K: KeyType, V: ValueType>(&self, key: K) -> Result<Proof<V>, Error>;
+    async fn single_key_proof<K: KeyType, V: ValueType>(
+        &self,
+        key: K,
+    ) -> Result<Option<Proof<V>>, Error>;
 
     /// Obtain a range proof over a set of keys
     ///
@@ -149,7 +150,7 @@ pub trait DbView {
         first_key: Option<K>,
         last_key: Option<K>,
         limit: usize,
-    ) -> Result<RangeProof<K, V>, Error>;
+    ) -> Result<Option<RangeProof<K, V>>, Error>;
 }
 
 /// A proposal for a new revision of the database.

diff --git a/firewood/src/v2/db.rs b/firewood/src/v2/db.rs
@@ -72,14 +72,14 @@ impl api::DbView for DbView {
         todo!()
     }
 
-    async fn val<K: KeyType>(&self, _key: K) -> Result<Vec<u8>, api::Error> {
+    async fn val<K: KeyType>(&self, _key: K) -> Result<Option<Vec<u8>>, api::Error> {
         todo!()
     }
 
     async fn single_key_proof<K: KeyType, V: ValueType>(
         &self,
         _key: K,
-    ) -> Result<api::Proof<V>, api::Error> {
+    ) -> Result<Option<api::Proof<V>>, api::Error> {
         todo!()
     }
 
@@ -88,7 +88,7 @@ impl api::DbView for DbView {
         _first_key: Option<K>,
         _last_key: Option<K>,
         _limit: usize,
-    ) -> Result<api::RangeProof<K, V>, api::Error> {
+    ) -> Result<Option<api::RangeProof<K, V>>, api::Error> {
         todo!()
     }
 }
diff --git a/firewood/src/v2/emptydb.rs b/firewood/src/v2/emptydb.rs
@@ -54,21 +54,24 @@ impl DbView for HistoricalImpl {
         Ok(ROOT_HASH)
     }
 
-    async fn val<K: KeyType>(&self, _key: K) -> Result<Vec<u8>, Error> {
-        Err(Error::KeyNotFound)
+    async fn val<K: KeyType>(&self, _key: K) -> Result<Option<Vec<u8>>, Error> {
+        Ok(None)
     }
 
-    async fn single_key_proof<K: KeyType, V: ValueType>(&self, _key: K) -> Result<Proof<V>, Error> {
-        Err(Error::KeyNotFound)
+    async fn single_key_proof<K: KeyType, V: ValueType>(
+        &self,
+        _key: K,
+    ) -> Result<Option<Proof<V>>, Error> {
+        Ok(None)
     }
 
     async fn range_proof<K: KeyType, V: ValueType>(
         &self,
         _first_key: Option<K>,
         _last_key: Option<K>,
         _limit: usize,
-    ) -> Result<RangeProof<K, V>, Error> {
-        Err(Error::KeyNotFound)
+    ) -> Result<Option<RangeProof<K, V>>, Error> {
+        Ok(None)
     }
 }
 
@@ -91,12 +94,9 @@ mod tests {
 
         let proposal = db.propose(batch).await?;
 
-        assert_eq!(proposal.val(b"k").await.unwrap(), b"v");
+        assert_eq!(proposal.val(b"k").await.unwrap().unwrap(), b"v");
 
-        assert!(matches!(
-            proposal.val(b"z").await.unwrap_err(),
-            Error::KeyNotFound
-        ));
+        assert!(matches!(proposal.val(b"z").await.unwrap(), None));
 
         Ok(())
     }
@@ -126,20 +126,17 @@ mod tests {
             .await?;
         let proposal2 = Arc::new(proposal2);
         // both proposals still have (k,v)
-        assert_eq!(proposal1.val(b"k").await.unwrap(), b"v");
-        assert_eq!(proposal2.val(b"k").await.unwrap(), b"v");
+        assert_eq!(proposal1.val(b"k").await.unwrap().unwrap(), b"v");
+        assert_eq!(proposal2.val(b"k").await.unwrap().unwrap(), b"v");
         // only proposal1 doesn't have z
-        assert!(matches!(
-            proposal1.val(b"z").await.unwrap_err(),
-            Error::KeyNotFound
-        ));
+        assert!(matches!(proposal1.val(b"z").await.unwrap(), None));
         // proposal2 has z with value "undo"
-        assert_eq!(proposal2.val(b"z").await.unwrap(), b"undo");
+        assert_eq!(proposal2.val(b"z").await.unwrap().unwrap(), b"undo");
 
         // create a proposal3 by adding the two proposals together, keeping the originals
         let proposal3 = proposal1.as_ref() + proposal2.as_ref();
-        assert_eq!(proposal3.val(b"k").await.unwrap(), b"v");
-        assert_eq!(proposal3.val(b"z").await.unwrap(), b"undo");
+        assert_eq!(proposal3.val(b"k").await.unwrap().unwrap(), b"v");
+        assert_eq!(proposal3.val(b"z").await.unwrap().unwrap(), b"undo");
 
         // now consume proposal1 and proposal2
         proposal2.commit().await?;

diff --git a/firewood/src/v2/propose.rs b/firewood/src/v2/propose.rs
@@ -102,13 +102,13 @@ impl<T: api::DbView + Send + Sync> api::DbView for Proposal<T> {
         todo!()
     }
 
-    async fn val<K: KeyType>(&self, key: K) -> Result<Vec<u8>, api::Error> {
+    async fn val<K: KeyType>(&self, key: K) -> Result<Option<Vec<u8>>, api::Error> {
         // see if this key is in this proposal
         match self.delta.get(key.as_ref()) {
             Some(change) => match change {
                 // key in proposal, check for Put or Delete
-                KeyOp::Put(val) => Ok(val.clone()),
-                KeyOp::Delete => Err(api::Error::KeyNotFound), // key was deleted in this proposal
+                KeyOp::Put(val) => Ok(Some(val.clone())),
+                KeyOp::Delete => Ok(None), // key was deleted in this proposal
             },
             None => match &self.base {
                 // key not in this proposal, so delegate to base
@@ -121,7 +121,7 @@ impl<T: api::DbView + Send + Sync> api::DbView for Proposal<T> {
     async fn single_key_proof<K: KeyType, V: ValueType>(
         &self,
         _key: K,
-    ) -> Result<api::Proof<V>, api::Error> {
+    ) -> Result<Option<api::Proof<V>>, api::Error> {
         todo!()
     }
 
@@ -130,7 +130,7 @@ impl<T: api::DbView + Send + Sync> api::DbView for Proposal<T> {
         _first_key: Option<KT>,
         _last_key: Option<KT>,
         _limit: usize,
-    ) -> Result<api::RangeProof<KT, VT>, api::Error> {
+    ) -> Result<Option<api::RangeProof<KT, VT>>, api::Error> {
         todo!()
     }
 }