Skip to content

Commit

Permalink
chore: remove panicky unwrap()s, use `Result<(), ..> (#261)
Browse files Browse the repository at this point in the history 
* chore(cache): remove panicky `unwrap()`s, use `Result<(), ..>

* chore: remove panicky `unwrap()`s, use `Result<(), ..>

* fix(doctests): cache doctests
  • Loading branch information
@Jon-Becker
Jon-Becker committed Jan 1, 2024
1 parent ba4a0f2 commit fadfd04
Show file tree
Hide file tree
Showing 13 changed files with 275 additions and 199 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions cache/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ clap = { version = "3.1.18", features = ["derive"] }
serde = { version = "1.0", features = ["derive"] }
bincode = "1.3.3"
serde_json = "1.0.108"
thiserror = "1.0.50"
7 changes: 7 additions & 0 deletions cache/src/error.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#[derive(Debug, thiserror::Error)]
pub enum Error {
#[error("Error: {0}")]
Generic(String),
#[error("IO error: {0}")]
IOError(String),
}
177 changes: 107 additions & 70 deletions cache/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ use serde::{de::DeserializeOwned, Deserialize, Serialize};
#[allow(deprecated)]
use std::env::home_dir;

use error::Error;
use util::*;

pub mod error;
pub mod util;

/// Clap argument parser for the cache subcommand
Expand Down Expand Up @@ -59,24 +61,36 @@ pub struct Cache<T> {
/// store_cache("clear_cache_key", "value", None);
///
/// /// assert that the cache contains the key
/// assert!(keys("*").contains(&"clear_cache_key".to_string()));
/// assert!(keys("*").expect("!").contains(&"clear_cache_key".to_string()));
///
/// /// clear the cache
/// clear_cache();
///
/// /// assert that the cache no longer contains the key
/// assert!(!keys("*").contains(&"clear_cache_key".to_string()));
/// assert!(!keys("*").expect("!").contains(&"clear_cache_key".to_string()));
/// ```
#[allow(deprecated)]
pub fn clear_cache() {
let home = home_dir().unwrap();
pub fn clear_cache() -> Result<(), Error> {
let home = home_dir().ok_or(Error::Generic(
"failed to get home directory. does your os support `std::env::home_dir()`?".to_string(),
))?;
let cache_dir = home.join(".bifrost").join("cache");

for entry in cache_dir.read_dir().unwrap() {
let entry = entry.unwrap();
let path = entry.path();
delete_path(path.to_str().unwrap());
for entry in cache_dir
.read_dir()
.map_err(|e| Error::Generic(format!("failed to read cache directory: {:?}", e)))?
{
let entry =
entry.map_err(|e| Error::Generic(format!("failed to read cache entry: {:?}", e)))?;
delete_path(
entry
.path()
.to_str()
.ok_or(Error::Generic("failed to convert path to string".to_string()))?,
);
}

Ok(())
}

/// Check if a cached object exists
Expand All @@ -88,18 +102,20 @@ pub fn clear_cache() {
/// store_cache("exists_key", "value", None);
///
/// /// assert that the cache contains the key
/// assert!(exists("exists_key"));
/// assert!(exists("exists_key").expect("!"));
///
/// /// assert that the cache does not contain a non-existent key
/// assert!(!exists("non_existent_key"));
/// assert!(!exists("non_existent_key").expect("!"));
/// ```
#[allow(deprecated)]
pub fn exists(key: &str) -> bool {
let home = home_dir().unwrap();
pub fn exists(key: &str) -> Result<bool, Error> {
let home = home_dir().ok_or(Error::Generic(
"failed to get home directory. does your os support `std::env::home_dir()`?".to_string(),
))?;
let cache_dir = home.join(".bifrost").join("cache");
let cache_file = cache_dir.join(format!("{key}.bin"));

cache_file.exists()
Ok(cache_file.exists())
}

/// List all cached objects
Expand All @@ -111,27 +127,38 @@ pub fn exists(key: &str) -> bool {
/// store_cache("keys_key", "value", None);
///
/// /// assert that the cache contains the key
/// assert!(keys("*").contains(&"keys_key".to_string()));
/// assert!(keys("*").expect("!").contains(&"keys_key".to_string()));
///
/// /// assert that the cache does not contain a non-existent key
/// assert!(!keys("*").contains(&"non_existent_key".to_string()));
/// assert!(!keys("*").expect("!").contains(&"non_existent_key".to_string()));
///
/// /// assert that the cache contains the key
/// assert!(keys("keys_*").contains(&"keys_key".to_string()));
/// assert!(keys("keys_*").expect("!").contains(&"keys_key".to_string()));
/// ```
#[allow(deprecated)]
pub fn keys(pattern: &str) -> Vec<String> {
let home = home_dir().unwrap();
pub fn keys(pattern: &str) -> Result<Vec<String>, Error> {
let home = home_dir().ok_or(Error::Generic(
"failed to get home directory. does your os support `std::env::home_dir()`?".to_string(),
))?;
let cache_dir = home.join(".bifrost").join("cache");
let mut keys = Vec::new();

// remove wildcard
let pattern = pattern.replace('*', "");

for entry in cache_dir.read_dir().unwrap() {
let entry = entry.unwrap();
let path = entry.path();
let key = path.file_name().unwrap().to_str().unwrap().to_string();
for entry in cache_dir
.read_dir()
.map_err(|e| Error::Generic(format!("failed to read cache directory: {:?}", e)))?
{
let entry =
entry.map_err(|e| Error::Generic(format!("failed to read cache entry: {:?}", e)))?;
let key = entry
.path()
.file_name()
.ok_or(Error::Generic("failed to get file name".to_string()))?
.to_str()
.ok_or(Error::Generic("failed to convert path to string".to_string()))?
.to_string();
if pattern.is_empty() || key.contains(&pattern) {
keys.push(key.replace(".bin", ""));
}
Expand All @@ -140,7 +167,7 @@ pub fn keys(pattern: &str) -> Vec<String> {
// sort keys alphabetically
keys.sort();

keys
Ok(keys)
}

/// Delete a cached object
Expand All @@ -151,23 +178,28 @@ pub fn keys(pattern: &str) -> Vec<String> {
/// store_cache("delete_cache_key", "value", None);
///
/// /// assert that the cache contains the key
/// assert!(keys("*").contains(&"delete_cache_key".to_string()));
/// assert!(keys("*").expect("!").contains(&"delete_cache_key".to_string()));
///
/// /// delete the cached object
/// delete_cache("delete_cache_key");
///
/// /// assert that the cache does not contain the key
/// assert!(!keys("*").contains(&"delete_cache_key".to_string()));
/// assert!(!keys("*").expect("!").contains(&"delete_cache_key".to_string()));
/// ```
#[allow(deprecated)]
pub fn delete_cache(key: &str) {
let home = home_dir().unwrap();
pub fn delete_cache(key: &str) -> Result<(), Error> {
let home = home_dir().ok_or(Error::Generic(
"failed to get home directory. does your os support `std::env::home_dir()`?".to_string(),
))?;
let cache_dir = home.join(".bifrost").join("cache");
let cache_file = cache_dir.join(format!("{key}.bin"));

if cache_file.exists() {
std::fs::remove_file(cache_file).unwrap();
std::fs::remove_file(cache_file)
.map_err(|e| Error::IOError(format!("failed to delete cache file: {:?}", e)))?;
}

Ok(())
}

/// Read a cached object
Expand All @@ -179,41 +211,45 @@ pub fn delete_cache(key: &str) {
/// store_cache("read_cache_key", "value", None);
///
/// /// read the cached object
/// assert_eq!(read_cache::<String>("read_cache_key").unwrap(), "value");
/// assert_eq!(read_cache::<String>("read_cache_key").expect("!").expect("!"), "value");
/// ```
#[allow(deprecated)]
pub fn read_cache<T>(key: &str) -> Option<T>
pub fn read_cache<T>(key: &str) -> Result<Option<T>, Error>
where
T: 'static + DeserializeOwned, {
let home = home_dir().unwrap();
let home = home_dir().ok_or(Error::Generic(
"failed to get home directory. does your os support `std::env::home_dir()`?".to_string(),
))?;
let cache_dir = home.join(".bifrost").join("cache");
let cache_file = cache_dir.join(format!("{key}.bin"));

let binary_string = match read_file(cache_file.to_str().unwrap()) {
let binary_string = match read_file(
cache_file
.to_str()
.ok_or(Error::Generic("failed to convert path to string".to_string()))?,
) {
Some(s) => s,
None => return None,
None => return Ok(None),
};

let binary_vec = decode_hex(&binary_string).ok()?;

let cache: Cache<T> = match bincode::deserialize::<Cache<T>>(&binary_vec) {
Ok(c) => {
// check if the cache has expired, if so, delete it and return None
if c.expiry <
std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap()
.as_secs()
{
delete_cache(key);
return None
}
let binary_vec = decode_hex(&binary_string)
.map_err(|e| Error::Generic(format!("failed to decode hex: {:?}", e)))?;

let cache: Cache<T> = bincode::deserialize::<Cache<T>>(&binary_vec)
.map_err(|e| Error::Generic(format!("failed to deserialize cache object: {:?}", e)))?;

// check if the cache has expired, if so, delete it and return None
if cache.expiry <
std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.map_err(|e| Error::Generic(format!("failed to get current time: {:?}", e)))?
.as_secs()
{
delete_cache(key)?;
return Ok(None);
}

c
}
Err(_) => return None,
};
Some(*Box::new(cache.value))
Ok(Some(*Box::new(cache.value)))
}

/// Store a value in the cache, with an optional expiry time \
Expand All @@ -229,26 +265,27 @@ where
/// store_cache("store_cache_key2", "value", Some(60 * 60 * 24));
/// ```
#[allow(deprecated)]
pub fn store_cache<T>(
key: &str,
value: T,
expiry: Option<u64>,
) -> Result<(), Box<dyn std::error::Error>>
pub fn store_cache<T>(key: &str, value: T, expiry: Option<u64>) -> Result<(), Error>
where
T: Serialize, {
let home = home_dir().unwrap();
let home = home_dir().ok_or(Error::Generic(
"failed to get home directory. does your os support `std::env::home_dir()`?".to_string(),
))?;
let cache_dir = home.join(".bifrost").join("cache");
let cache_file = cache_dir.join(format!("{key}.bin"));

// expire in 90 days
let expiry = expiry.unwrap_or(
std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_secs() +
std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.map_err(|e| Error::Generic(format!("failed to get current time: {:?}", e)))?
.as_secs() +
60 * 60 * 24 * 90,
);

let cache = Cache { value, expiry };
let encoded: Vec<u8> = bincode::serialize(&cache)
.map_err(|e| format!("Failed to serialize cache object: {:?}", e))?;
.map_err(|e| Error::Generic(format!("failed to serialize cache object: {:?}", e)))?;
let binary_string = encode_hex(encoded);
write_file(cache_file.to_str().unwrap(), &binary_string);

Expand All @@ -257,14 +294,14 @@ where

/// Cache subcommand handler
#[allow(deprecated)]
pub fn cache(args: CacheArgs) -> Result<(), Box<dyn std::error::Error>> {
pub fn cache(args: CacheArgs) -> Result<(), Error> {
match args.sub {
Subcommands::Clean(_) => {
clear_cache();
clear_cache()?;
println!("Cache cleared.")
}
Subcommands::Ls(_) => {
let keys = keys("*");
let keys = keys("*")?;
println!("Displaying {} cached objects:", keys.len());

for (i, key) in keys.iter().enumerate() {
Expand All @@ -283,7 +320,7 @@ pub fn cache(args: CacheArgs) -> Result<(), Box<dyn std::error::Error>> {
size += metadata.len();
}

println!("Cached objects: {}", keys("*").len());
println!("Cached objects: {}", keys("*")?.len());
println!("Cache size: {}", prettify_bytes(size));
}
}
Expand Down Expand Up @@ -314,7 +351,7 @@ mod tests {
fn test_get_cache() {
store_cache("key3", "value".to_string(), None);
let value = read_cache("key3");
let value: String = value.unwrap();
let value: String = value.unwrap().unwrap();

// assert stored value matches
assert_eq!(value, "value");
Expand Down Expand Up @@ -351,7 +388,7 @@ mod tests {

store_cache("struct2", test_struct, None);
let value = read_cache("struct2");
let value: TestStruct = value.unwrap();
let value: TestStruct = value.unwrap().unwrap();

// assert stored value matches
assert_eq!(value.name, "test");
Expand All @@ -364,7 +401,7 @@ mod tests {
store_cache("some_other_key", "some_value", None);
store_cache("not_a_key", "some_value", None);

assert_eq!(keys("some_"), vec!["some_key", "some_other_key"]);
assert_eq!(keys("some_").unwrap(), vec!["some_key", "some_other_key"]);
}

#[test]
Expand All @@ -378,14 +415,14 @@ mod tests {

assert!(["a", "b", "c", "d", "e", "f"]
.iter()
.all(|key| { keys("*").contains(&key.to_string()) }));
.all(|key| { keys("*").unwrap().contains(&key.to_string()) }));
}

#[test]
fn test_exists() {
assert!(!exists("does_not_exist"));
assert!(!exists("does_not_exist").unwrap());
store_cache("does_not_exist", "some_value", None);
assert!(exists("does_not_exist"));
assert!(exists("does_not_exist").unwrap());
delete_cache("does_not_exist");
}
}
1 change: 1 addition & 0 deletions common/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@ async-recursion = "1.0.5"
async-trait = "0.1.51"
chrono = "0.4.31"
backoff = {version = "0.4.0", features = ["tokio"]}
thiserror = "1.0.50"
7 changes: 7 additions & 0 deletions common/src/error.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#[derive(Debug, thiserror::Error)]
pub enum Error {
#[error("Error: {0}")]
Generic(String),
#[error("IO error: {0}")]
IOError(String),
}
Loading

0 comments on commit fadfd04

Please sign in to comment.