From 5f3ab8ef1da1193968d6ccda7265f2d27fbf50a9 Mon Sep 17 00:00:00 2001 From: kindywu Date: Mon, 6 May 2024 10:39:06 +0800 Subject: [PATCH] feat: remove NullBulkString / NullArray --- examples/demo.rs | 3 ++ src/main.rs | 6 ++- src/resp/array.rs | 92 +++++++++++++++++++---------------- src/resp/bulk_string.rs | 105 +++++++++++++++++++++++----------------- src/resp/mod.rs | 11 +++-- src/resp/resp_frame.rs | 20 ++------ 6 files changed, 133 insertions(+), 104 deletions(-) create mode 100644 examples/demo.rs diff --git a/examples/demo.rs b/examples/demo.rs new file mode 100644 index 0000000..a6233b5 --- /dev/null +++ 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 index 8d51160..9faf2f0 100644 --- 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 index 0e103ea..0e4be98 100644 --- 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); -#[derive(Debug, Clone, PartialEq, Eq, PartialOrd)] -pub struct RespNullArray; +const NULL_ARRAY_STRING: &[u8; 5] = b"*-1\r\n"; // - array: "*\r\n..." impl RespEncode for RespArray { fn encode(self) -> Vec { - 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 { - 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 { - extract_fixed_data(buf, "*-1\r\n", "NullArray")?; - Ok(RespNullArray) - } +// // - null array: "*-1\r\n" +// impl RespEncode for RespNullArray { +// fn encode(self) -> Vec { +// b"*-1\r\n".to_vec() +// } +// } - fn expect_length(_buf: &[u8]) -> Result { - Ok(4) - } -} +// impl RespDecode for RespNullArray { +// const PREFIX: &'static str = "*"; +// fn decode(buf: &mut BytesMut) -> Result { +// extract_fixed_data(buf, "*-1\r\n", "NullArray")?; +// Ok(RespNullArray) +// } + +// fn expect_length(_buf: &[u8]) -> Result { +// Ok(4) +// } +// } impl RespArray { pub fn new(s: impl Into>) -> 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 index 135a7db..1d43f32 100644 --- a/src/resp/bulk_string.rs +++ b/src/resp/bulk_string.rs @@ -4,22 +4,48 @@ 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 { +// b"$-1\r\n".to_vec() +// } +// } + +// impl RespDecode for RespNullBulkString { +// const PREFIX: &'static str = "$"; +// fn decode(buf: &mut BytesMut) -> Result { +// extract_fixed_data(buf, "$-1\r\n", "NullBulkString")?; +// Ok(RespNullBulkString) +// } + +// fn expect_length(_buf: &[u8]) -> Result { +// Ok(5) +// } +// } #[derive(Debug, Clone, PartialEq, Eq, PartialOrd)] pub struct BulkString(pub(crate) Vec); -#[derive(Debug, Clone, PartialEq, Eq, PartialOrd)] -pub struct RespNullBulkString; +// +const NULL_BULK_STRING: &[u8; 5] = b"$-1\r\n"; // - bulk string: "$\r\n\r\n" impl RespEncode for BulkString { fn encode(self) -> Vec { - 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 + } } } @@ -27,39 +53,28 @@ impl RespDecode for BulkString { const PREFIX: &'static str = "$"; fn decode(buf: &mut BytesMut) -> Result { 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 { 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 { - b"$-1\r\n".to_vec() - } -} - -impl RespDecode for RespNullBulkString { - const PREFIX: &'static str = "$"; - fn decode(buf: &mut BytesMut) -> Result { - extract_fixed_data(buf, "$-1\r\n", "NullBulkString")?; - Ok(RespNullBulkString) - } - - fn expect_length(_buf: &[u8]) -> Result { - 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 index 37d29cf..eabd15d 100644 --- 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 { 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 { diff --git a/src/resp/resp_frame.rs b/src/resp/resp_frame.rs index 5ffb662..cf26c04 100644 --- a/src/resp/resp_frame.rs +++ b/src/resp/resp_frame.rs @@ -2,7 +2,7 @@ use bytes::BytesMut; use enum_dispatch::enum_dispatch; use thiserror::Error; -use crate::{BulkString, RespArray, RespNullArray, RespNullBulkString, SimpleError, SimpleString}; +use crate::{BulkString, RespArray, SimpleError, SimpleString}; #[enum_dispatch(RespEncode)] #[derive(Debug, Clone, PartialEq, PartialOrd)] @@ -10,9 +10,7 @@ pub enum RespFrame { SimpleString(SimpleString), SimpleError(SimpleError), BulkString(BulkString), - NullBulkString(RespNullBulkString), Array(RespArray), - NullArray(RespNullArray), } // impl RespEncode for RespFrame { @@ -63,24 +61,16 @@ impl RespDecode for RespFrame { } Some(b'*') => { // try null array first - match RespNullArray::decode(buf) { + match RespArray::decode(buf) { Ok(frame) => Ok(frame.into()), - Err(RespError::NotComplete) => Err(RespError::NotComplete), - Err(_) => { - let frame = RespArray::decode(buf)?; - Ok(frame.into()) - } + Err(e) => Err(e), } } Some(b'$') => { // try null bulk string first - match RespNullBulkString::decode(buf) { + match BulkString::decode(buf) { Ok(frame) => Ok(frame.into()), - Err(RespError::NotComplete) => Err(RespError::NotComplete), - Err(_) => { - let frame = BulkString::decode(buf)?; - Ok(frame.into()) - } + Err(e) => Err(e), } } None => Err(RespError::NotComplete),