From 62783997dff5a80226c31124f0036cf00b6c48b2 Mon Sep 17 00:00:00 2001 From: Antioch Peverell Date: Fri, 25 Sep 2020 12:47:04 +0100 Subject: [PATCH] refactor get_ser to use get_with internally (#3451) less code duplication and a cleaner code path batch.exists() now aware of current write tx --- chain/src/store.rs | 4 +- store/src/lmdb.rs | 99 ++++++++++++++++++++++++---------------------- 2 files changed, 54 insertions(+), 49 deletions(-) diff --git a/chain/src/store.rs b/chain/src/store.rs index f4e3b897e..1a5a746d5 100644 --- a/chain/src/store.rs +++ b/chain/src/store.rs @@ -359,7 +359,9 @@ impl<'a> Batch<'a> { fn get_legacy_input_bitmap(&self, bh: &Hash) -> Result { option_to_not_found( self.db - .get_with(&to_key(BLOCK_INPUT_BITMAP_PREFIX, bh), Bitmap::deserialize), + .get_with(&to_key(BLOCK_INPUT_BITMAP_PREFIX, bh), |data| { + Ok(Bitmap::deserialize(data)) + }), || "legacy block input bitmap".to_string(), ) } diff --git a/store/src/lmdb.rs b/store/src/lmdb.rs index b22bb0c79..7e548a7d3 100644 --- a/store/src/lmdb.rs +++ b/store/src/lmdb.rs @@ -245,23 +245,27 @@ impl Store { Ok(()) } - /// Gets a value from the db, provided its key - pub fn get_with(&self, key: &[u8], f: F) -> Result, Error> + /// Gets a value from the db, provided its key. + /// Deserializes the retrieved data using the provided function. + pub fn get_with( + &self, + key: &[u8], + access: &lmdb::ConstAccessor<'_>, + db: &lmdb::Database<'_>, + deserialize: F, + ) -> Result, Error> where - F: Fn(&[u8]) -> T, + F: Fn(&[u8]) -> Result, { - let lock = self.db.read(); - let db = lock - .as_ref() - .ok_or_else(|| Error::NotFoundErr("chain db is None".to_string()))?; - let txn = lmdb::ReadTransaction::new(self.env.clone())?; - let access = txn.access(); - let res = access.get(db, key); - res.map(f).to_opt().map_err(From::from) + let res: Option<&[u8]> = access.get(db, key).to_opt()?; + match res { + None => Ok(None), + Some(res) => deserialize(res).map(|x| Some(x)), + } } - /// Gets a `Readable` value from the db, provided its key. Encapsulates - /// serialization. + /// Gets a `Readable` value from the db, provided its key. + /// Note: Creates a new read transaction so will *not* see any uncommitted data. pub fn get_ser(&self, key: &[u8]) -> Result, Error> { let lock = self.db.read(); let db = lock @@ -269,24 +273,11 @@ impl Store { .ok_or_else(|| Error::NotFoundErr("chain db is None".to_string()))?; let txn = lmdb::ReadTransaction::new(self.env.clone())?; let access = txn.access(); - self.get_ser_access(key, &access, db.clone()) - } - fn get_ser_access( - &self, - key: &[u8], - access: &lmdb::ConstAccessor<'_>, - db: Arc>, - ) -> Result, Error> { - let res: lmdb::error::Result<&[u8]> = access.get(&db, key); - match res.to_opt() { - Ok(Some(mut res)) => match ser::deserialize(&mut res, self.protocol_version()) { - Ok(res) => Ok(Some(res)), - Err(e) => Err(Error::SerErr(format!("{}", e))), - }, - Ok(None) => Ok(None), - Err(e) => Err(From::from(e)), - } + self.get_with(key, &access, &db, |mut data| { + ser::deserialize(&mut data, self.protocol_version()) + .map_err(|e| Error::SerErr(format!("{}", e))) + }) } /// Whether the provided key exists @@ -297,8 +288,9 @@ impl Store { .ok_or_else(|| Error::NotFoundErr("chain db is None".to_string()))?; let txn = lmdb::ReadTransaction::new(self.env.clone())?; let access = txn.access(); - let res: lmdb::error::Result<&lmdb::Ignore> = access.get(db, key); - res.to_opt().map(|r| r.is_some()).map_err(From::from) + + let res: Option<&lmdb::Ignore> = access.get(db, key).to_opt()?; + Ok(res.is_some()) } /// Produces an iterator of (key, value) pairs, where values are `Readable` types @@ -376,17 +368,31 @@ impl<'a> Batch<'a> { } } - /// gets a value from the db, provided its key - pub fn get_with(&self, key: &[u8], f: F) -> Result, Error> + /// Low-level access for retrieving data by key. + /// Takes a function for flexible deserialization. + pub fn get_with(&self, key: &[u8], deserialize: F) -> Result, Error> where - F: Fn(&[u8]) -> T, + F: Fn(&[u8]) -> Result, { - self.store.get_with(key, f) + let access = self.tx.access(); + let lock = self.store.db.read(); + let db = lock + .as_ref() + .ok_or_else(|| Error::NotFoundErr("chain db is None".to_string()))?; + + self.store.get_with(key, &access, &db, deserialize) } - /// Whether the provided key exists + /// Whether the provided key exists. + /// This is in the context of the current write transaction. pub fn exists(&self, key: &[u8]) -> Result { - self.store.exists(key) + let access = self.tx.access(); + let lock = self.store.db.read(); + let db = lock + .as_ref() + .ok_or_else(|| Error::NotFoundErr("chain db is None".to_string()))?; + let res: Option<&lmdb::Ignore> = access.get(db, key).to_opt()?; + Ok(res.is_some()) } /// Produces an iterator of `Readable` types moving forward from the @@ -395,17 +401,14 @@ impl<'a> Batch<'a> { self.store.iter(from) } - /// Gets a `Readable` value from the db, provided its key, taking the - /// content of the current batch into account. + /// Gets a `Readable` value from the db by provided key and default deserialization strategy. pub fn get_ser(&self, key: &[u8]) -> Result, Error> { - let access = self.tx.access(); - - let lock = self.store.db.read(); - let db = lock - .as_ref() - .ok_or_else(|| Error::NotFoundErr("chain db is None".to_string()))?; - - self.store.get_ser_access(key, &access, db.clone()) + self.get_with(key, |mut data| { + match ser::deserialize(&mut data, self.protocol_version()) { + Ok(res) => Ok(res), + Err(e) => Err(Error::SerErr(format!("{}", e))), + } + }) } /// Deletes a key/value pair from the db