feat: remove NullBulkString / NullArray

kindywu · May 6, 2024 · 8a36102 · 8a36102
1 parent 3cb4ed3
commit 8a36102
Show file tree

Hide file tree

Showing 7 changed files with 135 additions and 106 deletions.
diff --git a/README.md b/README.md
@@ -18,8 +18,8 @@
 - git commit -a --amend
 - git push
 
-- git tag -a v2-thread
-- git push origin v2-thread
+- git tag -a v3-1-simple-redis
+- git push origin v3-1-simple-redis
 
 - git tag
 - git checkout v2-3-metrics-1
diff --git a/examples/demo.rs b/examples/demo.rs
@@ -0,0 +1,3 @@
+fn main() {
+    println!("{:?}", b"$-1\r\n".to_vec())
+}
diff --git a/src/main.rs b/src/main.rs
@@ -4,7 +4,7 @@ use anyhow::Result;
 use futures::SinkExt;
 use tokio_stream::StreamExt;
 
-use simple_redis::{Command, CommandExecutor, RespFrameCodec};
+use simple_redis::{Command, CommandExecutor, RespEncode, RespFrameCodec};
 use tokio::net::{TcpListener, TcpStream};
 use tokio_util::codec::Framed;
 use tracing::{info, warn};
@@ -40,6 +40,10 @@ async fn process_conn(stream: TcpStream, _: SocketAddr) -> Result<()> {
         match framed.next().await {
             Some(Ok(frame)) => {
                 info!("Received frame: {:?}", frame);
+                info!(
+                    "Received frame: {:?}",
+                    String::from_utf8(frame.clone().encode())
+                );
 
                 let cmd = Command::try_from(frame)?;
                 info!("Executing command: {:?}", cmd);

diff --git a/src/resp/array.rs b/src/resp/array.rs
@@ -4,23 +4,26 @@ use bytes::{Buf, BytesMut};
 
 use crate::{RespDecode, RespEncode, RespError, RespFrame};
 
-use super::{calc_total_length, extract_fixed_data, parse_length, BUF_CAP, CRLF_LEN};
+use super::{calc_total_length, parse_length, BUF_CAP, CRLF_LEN};
 
 #[derive(Debug, Clone, PartialEq, PartialOrd)]
 pub struct RespArray(pub(crate) Vec<RespFrame>);
 
-#[derive(Debug, Clone, PartialEq, Eq, PartialOrd)]
-pub struct RespNullArray;
+const NULL_ARRAY_STRING: &[u8; 5] = b"*-1\r\n";
 
 // - array: "*<number-of-elements>\r\n<element-1>...<element-n>"
 impl RespEncode for RespArray {
     fn encode(self) -> Vec<u8> {
-        let mut buf = Vec::with_capacity(BUF_CAP);
-        buf.extend_from_slice(&format!("*{}\r\n", self.0.len()).into_bytes());
-        for frame in self.0 {
-            buf.extend_from_slice(&frame.encode());
+        if self.0.is_empty() {
+            NULL_ARRAY_STRING.to_vec()
+        } else {
+            let mut buf = Vec::with_capacity(BUF_CAP);
+            buf.extend_from_slice(&format!("*{}\r\n", self.0.len()).into_bytes());
+            for frame in self.0 {
+                buf.extend_from_slice(&frame.encode());
+            }
+            buf
         }
-        buf
     }
 }
 
@@ -33,6 +36,8 @@ impl RespDecode for RespArray {
         let (end, len) = parse_length(buf, Self::PREFIX)?;
         let total_len = calc_total_length(buf, end, len, Self::PREFIX)?;
 
+        // println!("len={},total_len={}", len, total_len);
+
         if buf.len() < total_len {
             return Err(RespError::NotComplete);
         }
@@ -53,24 +58,27 @@ impl RespDecode for RespArray {
     }
 }
 
-// - null array: "*-1\r\n"
-impl RespEncode for RespNullArray {
-    fn encode(self) -> Vec<u8> {
-        b"*-1\r\n".to_vec()
-    }
-}
+// #[derive(Debug, Clone, PartialEq, Eq, PartialOrd)]
+// pub struct RespNullArray;
 
-impl RespDecode for RespNullArray {
-    const PREFIX: &'static str = "*";
-    fn decode(buf: &mut BytesMut) -> Result<Self, RespError> {
-        extract_fixed_data(buf, "*-1\r\n", "NullArray")?;
-        Ok(RespNullArray)
-    }
+// // - null array: "*-1\r\n"
+// impl RespEncode for RespNullArray {
+//     fn encode(self) -> Vec<u8> {
+//         b"*-1\r\n".to_vec()
+//     }
+// }
 
-    fn expect_length(_buf: &[u8]) -> Result<usize, RespError> {
-        Ok(4)
-    }
-}
+// impl RespDecode for RespNullArray {
+//     const PREFIX: &'static str = "*";
+//     fn decode(buf: &mut BytesMut) -> Result<Self, RespError> {
+//         extract_fixed_data(buf, "*-1\r\n", "NullArray")?;
+//         Ok(RespNullArray)
+//     }
+
+//     fn expect_length(_buf: &[u8]) -> Result<usize, RespError> {
+//         Ok(4)
+//     }
+// }
 
 impl RespArray {
     pub fn new(s: impl Into<Vec<RespFrame>>) -> Self {
@@ -106,23 +114,6 @@ mod tests {
         );
     }
 
-    #[test]
-    fn test_null_array_encode() {
-        let frame: RespFrame = RespNullArray.into();
-        assert_eq!(frame.encode(), b"*-1\r\n");
-    }
-
-    #[test]
-    fn test_null_array_decode() -> Result<()> {
-        let mut buf = BytesMut::new();
-        buf.extend_from_slice(b"*-1\r\n");
-
-        let frame = RespNullArray::decode(&mut buf)?;
-        assert_eq!(frame, RespNullArray);
-
-        Ok(())
-    }
-
     #[test]
     fn test_array_decode() -> Result<()> {
         let mut buf = BytesMut::new();
@@ -145,4 +136,23 @@ mod tests {
 
         Ok(())
     }
+
+    #[test]
+    fn test_null_array_encode() {
+        let frame: RespFrame = RespArray::new(Vec::new()).into();
+        assert_eq!(frame.encode(), b"*-1\r\n");
+    }
+
+    #[test]
+    fn test_null_array_decode() -> Result<()> {
+        let mut buf = BytesMut::new();
+        buf.extend_from_slice(b"*-1\r\n");
+
+        assert_eq!(RespArray::expect_length(&buf), Ok(5));
+
+        let frame = RespArray::decode(&mut buf)?;
+        assert_eq!(frame, RespArray::new(Vec::new()));
+
+        Ok(())
+    }
 }
diff --git a/src/resp/bulk_string.rs b/src/resp/bulk_string.rs
@@ -4,62 +4,77 @@ use bytes::{Buf, BytesMut};
 
 use crate::{RespDecode, RespEncode, RespError};
 
-use super::{extract_fixed_data, parse_length, CRLF_LEN};
+use super::{parse_length, CRLF_LEN};
+
+// #[derive(Debug, Clone, PartialEq, Eq, PartialOrd)]
+// pub struct RespNullBulkString;
+
+// // - null bulk string: "$-1\r\n"
+// impl RespEncode for RespNullBulkString {
+//     fn encode(self) -> Vec<u8> {
+//         b"$-1\r\n".to_vec()
+//     }
+// }
+
+// impl RespDecode for RespNullBulkString {
+//     const PREFIX: &'static str = "$";
+//     fn decode(buf: &mut BytesMut) -> Result<Self, RespError> {
+//         extract_fixed_data(buf, "$-1\r\n", "NullBulkString")?;
+//         Ok(RespNullBulkString)
+//     }
+
+//     fn expect_length(_buf: &[u8]) -> Result<usize, RespError> {
+//         Ok(5)
+//     }
+// }
 
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd)]
 pub struct BulkString(pub(crate) Vec<u8>);
 
-#[derive(Debug, Clone, PartialEq, Eq, PartialOrd)]
-pub struct RespNullBulkString;
+//
+const NULL_BULK_STRING: &[u8; 5] = b"$-1\r\n";
 
 // - bulk string: "$<length>\r\n<data>\r\n"
 impl RespEncode for BulkString {
     fn encode(self) -> Vec<u8> {
-        let mut buf = Vec::with_capacity(self.len() + 16);
-        buf.extend_from_slice(&format!("${}\r\n", self.len()).into_bytes());
-        buf.extend_from_slice(&self);
-        buf.extend_from_slice(b"\r\n");
-        buf
+        if self.len() == 0 {
+            NULL_BULK_STRING.to_vec()
+        } else {
+            let mut buf = Vec::with_capacity(self.len() + 16);
+            buf.extend_from_slice(&format!("${}\r\n", self.len()).into_bytes());
+            buf.extend_from_slice(&self);
+            buf.extend_from_slice(b"\r\n");
+            buf
+        }
     }
 }
 
 impl RespDecode for BulkString {
     const PREFIX: &'static str = "$";
     fn decode(buf: &mut BytesMut) -> Result<Self, RespError> {
         let (end, len) = parse_length(buf, Self::PREFIX)?;
-        let remained = &buf[end + CRLF_LEN..];
-        if remained.len() < len + CRLF_LEN {
-            return Err(RespError::NotComplete);
+        if len == 0 {
+            Ok(BulkString::new(Vec::new()))
+        } else {
+            let remained = &buf[end + CRLF_LEN..];
+            if remained.len() < len + CRLF_LEN {
+                return Err(RespError::NotComplete);
+            }
+
+            buf.advance(end + CRLF_LEN);
+
+            let data = buf.split_to(len + CRLF_LEN);
+            Ok(BulkString::new(data[..len].to_vec()))
         }
-
-        buf.advance(end + CRLF_LEN);
-
-        let data = buf.split_to(len + CRLF_LEN);
-        Ok(BulkString::new(data[..len].to_vec()))
     }
 
     fn expect_length(buf: &[u8]) -> Result<usize, RespError> {
         let (end, len) = parse_length(buf, Self::PREFIX)?;
-        Ok(end + CRLF_LEN + len + CRLF_LEN)
-    }
-}
-
-// - null bulk string: "$-1\r\n"
-impl RespEncode for RespNullBulkString {
-    fn encode(self) -> Vec<u8> {
-        b"$-1\r\n".to_vec()
-    }
-}
-
-impl RespDecode for RespNullBulkString {
-    const PREFIX: &'static str = "$";
-    fn decode(buf: &mut BytesMut) -> Result<Self, RespError> {
-        extract_fixed_data(buf, "$-1\r\n", "NullBulkString")?;
-        Ok(RespNullBulkString)
-    }
-
-    fn expect_length(_buf: &[u8]) -> Result<usize, RespError> {
-        Ok(5)
+        if len == 0 {
+            Ok(5)
+        } else {
+            Ok(end + CRLF_LEN + len + CRLF_LEN)
+        }
     }
 }
 
@@ -120,12 +135,6 @@ mod tests {
         assert_eq!(frame.encode(), b"$5\r\nhello\r\n");
     }
 
-    #[test]
-    fn test_null_bulk_string_encode() {
-        let frame: RespFrame = RespNullBulkString.into();
-        assert_eq!(frame.encode(), b"$-1\r\n");
-    }
-
     #[test]
     fn test_bulk_string_decode() -> Result<()> {
         let mut buf = BytesMut::new();
@@ -147,13 +156,21 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn test_null_bulk_string_encode() {
+        let frame: RespFrame = BulkString::new(Vec::new()).into();
+        assert_eq!(frame.encode(), b"$-1\r\n");
+    }
+
     #[test]
     fn test_null_bulk_string_decode() -> Result<()> {
         let mut buf = BytesMut::new();
         buf.extend_from_slice(b"$-1\r\n");
 
-        let frame = RespNullBulkString::decode(&mut buf)?;
-        assert_eq!(frame, RespNullBulkString);
+        assert_eq!(BulkString::expect_length(&buf), Ok(5));
+
+        let frame = BulkString::decode(&mut buf)?;
+        assert_eq!(frame, BulkString::new(Vec::new()));
 
         Ok(())
     }

diff --git a/src/resp/mod.rs b/src/resp/mod.rs
@@ -6,8 +6,8 @@ mod simple_string;
 
 use bytes::{Buf, BytesMut};
 
-pub use array::{RespArray, RespNullArray};
-pub use bulk_string::{BulkString, RespNullBulkString};
+pub use array::RespArray;
+pub use bulk_string::BulkString;
 pub use resp_frame::*;
 pub use simple_error::*;
 pub use simple_string::*;
@@ -17,6 +17,7 @@ const CRLF: &[u8] = b"\r\n";
 const CRLF_LEN: usize = CRLF.len();
 
 // utility functions
+#[allow(dead_code)]
 fn extract_fixed_data(
     buf: &mut BytesMut,
     expect: &str,
@@ -72,7 +73,11 @@ fn find_crlf(buf: &[u8], nth: usize) -> Option<usize> {
 fn parse_length(buf: &[u8], prefix: &str) -> Result<(usize, usize), RespError> {
     let end = extract_simple_frame_data(buf, prefix)?;
     let s = String::from_utf8_lossy(&buf[prefix.len()..end]);
-    Ok((end, s.parse()?))
+    if s == "-1" {
+        Ok((end, 0usize))
+    } else {
+        Ok((end, s.parse()?))
+    }
 }
 
 fn calc_total_length(buf: &[u8], end: usize, len: usize, prefix: &str) -> Result<usize, RespError> {