diff --git a/Cargo.toml b/Cargo.toml index 3be7ac1..74c1c7c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,6 +29,7 @@ uuid = { version = "1.8.0", features = ["v4"] } strum = "0.26.2" strum_macros = "0.26.2" clap = { version = "4.5.7", features = ["derive", "env"] } +num-traits = "0.2.19" [dev-dependencies] rand = "0.8.5" diff --git a/Makefile b/Makefile index 8c02017..097f4d4 100644 --- a/Makefile +++ b/Makefile @@ -3,3 +3,8 @@ test: cargo test -- \ --nocapture \ --color=always + + +.PHONY: run +run: + cargo run diff --git a/README.md b/README.md index 94762d8..b433d93 100644 --- a/README.md +++ b/README.md @@ -2,19 +2,25 @@ Welcome to rustdis, a partial Redis server implementation written in Rust. -This project came to life out of pure curiosity, and because we wanted to learn more about Rust and Redis. So doing this project seemed like a good idea. -The primary goal of rustdis is to offer a straightforward and comprehensible implementation, with no optimization techniques to ensure the code remains accessible and easy to understand. -As of now, rustdis focuses exclusively on implementing Redis' String data type and its associated methods. You can find more about Redis strings here: [Redis Strings](https://redis.io/docs/data-types/strings/). +This project came to life out of pure curiosity, and because we wanted to learn +more about Rust and Redis. So doing this project seemed like a good idea. The +primary goal of rustdis is to offer a straightforward and comprehensible +implementation, with no optimization techniques to ensure the code remains +accessible and easy to understand. As of now, rustdis focuses exclusively on +implementing Redis' String data type and its associated methods. You can find +more about Redis strings here: [Redis +Strings](https://redis.io/docs/data-types/strings/). -This server is not production-ready; it is intended purely for educational purposes. +This server is not production-ready; it is intended purely for educational +purposes. ### Run ```shell -cargo run +make run ``` ### Test ```shell -cargo test +make test ``` ### Architecture diff --git a/src/commands/incrbyfloat.rs b/src/commands/incrbyfloat.rs index 9f45682..fff12f5 100644 --- a/src/commands/incrbyfloat.rs +++ b/src/commands/incrbyfloat.rs @@ -27,9 +27,9 @@ pub struct IncrByFloat { impl Executable for IncrByFloat { fn exec(self, store: Store) -> Result { let mut store = store.lock(); - let res = store.incr_by(&self.key, self.increment); + let res = store.incr_by::(&self.key, self.increment); match res { - Ok(res) => Ok(Frame::Simple(res.to_string())), + Ok(res) => Ok(Frame::Bulk(res.into())), Err(msg) => Ok(Frame::Error(msg.to_string())), } } @@ -56,6 +56,7 @@ mod tests { #[tokio::test] async fn existing_key() { let store = Store::new(); + store.lock().set(String::from("key1"), Bytes::from("10.50")); let frame = Frame::Array(vec![ Frame::Bulk(Bytes::from("INCRBYFLOAT")), @@ -72,12 +73,13 @@ mod tests { }) ); - store.lock().set(String::from("key1"), Bytes::from("10.50")); - let result = cmd.exec(store.clone()).unwrap(); - assert_eq!(result, Frame::Simple("10.6".to_string())); - assert_eq!(store.lock().get("key1"), Some(Bytes::from("10.6"))); + assert_eq!(result, Frame::Bulk(Bytes::from("10.59999999999999964"))); + assert_eq!( + store.lock().get("key1"), + Some(Bytes::from("10.59999999999999964")) + ); } #[tokio::test] @@ -101,7 +103,7 @@ mod tests { let result = cmd.exec(store.clone()).unwrap(); - assert_eq!(result, Frame::Simple("10".to_string())); + assert_eq!(result, Frame::Bulk(Bytes::from("10"))); assert_eq!(store.lock().get("key1"), Some(Bytes::from("10"))); } diff --git a/src/store.rs b/src/store.rs index e06ce25..6ba9371 100644 --- a/src/store.rs +++ b/src/store.rs @@ -1,5 +1,7 @@ use bytes::Bytes; +use num_traits::{ToPrimitive, Zero}; use std::collections::{BTreeSet, HashMap}; +use std::fmt::Display; use std::ops::AddAssign; use std::ops::Deref; use std::str::FromStr; @@ -147,9 +149,10 @@ impl<'a> InnerStoreLocked<'a> { .map(|(key, value)| (key, &value.data)) } - pub fn incr_by(&mut self, key: &str, increment: T) -> Result + pub fn incr_by(&mut self, key: &str, increment: T) -> Result where - T: FromStr + ToString + AddAssign + Default, + T: AddAssign + FromStr + Display + Zero + ToPrimitive, + R: FromStr, { let err = "value is not an integer or out of range"; @@ -158,16 +161,27 @@ impl<'a> InnerStoreLocked<'a> { .map_err(|_| err.to_string()) .and_then(|s| s.parse::().map_err(|_| err.to_string())) { - Ok(value) => value, + Ok(v) => v, Err(e) => return Err(e), }, - None => T::default(), + None => T::zero(), }; value += increment; - self.set(key.to_string(), value.to_string().into()); - Ok(value) + let value = match value.to_f64() { + Some(v) if v.fract() == 0.0 => format!("{:.0}", v), // Format as an integer if no fractional part. + Some(v) => format!("{:.17}", v), // Format as a float with up to 17 digits of precision. + // This shouldn't happen since we're only using ints and floats, but ideally, a trait + // would enforce this at compile time. + None => return Err(err.to_string()), + }; + + self.set(key.to_string(), value.clone().into()); + + value.parse::().map_err(|_| err.to_string()) + + // Ok(value) } fn remove_expired_keys(&mut self) -> Option { @@ -244,6 +258,14 @@ async fn remove_expired_keys(store: Arc) { } } +fn is_float(value: T) -> bool { + if let Some(f) = value.to_f64() { + f.fract() != 0.0 + } else { + false + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/tests/integration.rs b/tests/integration.rs index 83202ab..eb6bdaa 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -226,15 +226,6 @@ async fn test_incr_by_float() { p.cmd("INCRBYFLOAT").arg("incr_by_float_key_3").arg("-1.2"); }) .await; - - test_compare_err(|p| { - // Value is not an integer or out of range error. - p.cmd("SET") - .arg("incr_by_float_key_4") - .arg("234293482390480948029348230948"); - p.cmd("INCRBYFLOAT").arg("incr_by_float_key_4").arg(1); - }) - .await; } #[tokio::test]