Update incr by precision

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"

Makefile

@@ -3,3 +3,8 @@ test:
 	cargo test -- \
   		--nocapture \
   		--color=always
+
+
+.PHONY: run 
+run:
+	cargo run

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
 

src/commands/incrbyfloat.rs

@@ -27,9 +27,9 @@ pub struct IncrByFloat {
 impl Executable for IncrByFloat {
     fn exec(self, store: Store) -> Result<Frame, Error> {
         let mut store = store.lock();
-        let res = store.incr_by(&self.key, self.increment);
+        let res = store.incr_by::<f64, String>(&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")));
     }
 

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<T>(&mut self, key: &str, increment: T) -> Result<T, String>
+    pub fn incr_by<T, R>(&mut self, key: &str, increment: T) -> Result<R, String>
     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,25 @@ impl<'a> InnerStoreLocked<'a> {
                 .map_err(|_| err.to_string())
                 .and_then(|s| s.parse::<T>().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::<R>().map_err(|_| err.to_string())
     }
 
     fn remove_expired_keys(&mut self) -> Option<Instant> {

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]