Skip to content

Commit 3ed48fa

Browse files
lqxhublqxhub
authored
feat(parse): refactor inline parsing (#258)
* feat(parse): refactor inline parsing to handle whitespace and empty lines * feat(parse): remove inline command handling and update related tests
1 parent 95a57c6 commit 3ed48fa

5 files changed

Lines changed: 143 additions & 75 deletions

File tree

‎src/resp/src/command.rs‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -552,18 +552,6 @@ impl Command for RespData {
552552

553553
Ok(RespCommand::new(command_type, args, false))
554554
}
555-
RespData::Inline(parts) if !parts.is_empty() => {
556-
let command_name = std::str::from_utf8(&parts[0]).map_err(|_| {
557-
RespError::InvalidData("Command name must be a valid UTF-8 string".to_string())
558-
})?;
559-
560-
let command_type =
561-
CommandType::from_str(command_name).unwrap_or(CommandType::Unknown);
562-
563-
let args = parts.iter().skip(1).cloned().collect();
564-
565-
Ok(RespCommand::new(command_type, args, false))
566-
}
567555
_ => Err(RespError::InvalidData("Invalid command format".to_string())),
568556
}
569557
}

‎src/resp/src/encode.rs‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -384,15 +384,6 @@ impl RespEncode for RespEncoder {
384384
self
385385
}
386386
RespData::Array(None) => self.set_array_len(-1),
387-
RespData::Inline(parts) => {
388-
for (i, part) in parts.iter().enumerate() {
389-
if i > 0 {
390-
self.buffer.extend_from_slice(b" ");
391-
}
392-
self.buffer.extend_from_slice(part);
393-
}
394-
self.append_crlf()
395-
}
396387
// RESP3 types
397388
RespData::Null => {
398389
self.buffer.extend_from_slice(b"_");

‎src/resp/src/negotiation.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -248,8 +248,7 @@ impl ProtocolNegotiator {
248248
RespData::SimpleString(_)
249249
| RespData::Error(_)
250250
| RespData::Integer(_)
251-
| RespData::BulkString(_)
252-
| RespData::Inline(_) => data.clone(),
251+
| RespData::BulkString(_) => data.clone(),
253252
RespData::Array(Some(items)) => {
254253
// Recursively convert array items
255254
let converted_items: Vec<_> = items.iter().map(Self::convert_to_resp2).collect();

‎src/resp/src/parse.rs‎

Lines changed: 142 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,9 @@ use bytes::{Buf, Bytes, BytesMut};
2222
use nom::Parser;
2323
use nom::{
2424
IResult,
25-
bytes::streaming::{take, take_while1},
26-
character::streaming::{char, digit1, line_ending, not_line_ending, space1},
27-
combinator::{map, map_res, opt, recognize},
28-
multi::separated_list0,
25+
bytes::streaming::take,
26+
character::streaming::{char, digit1, line_ending, not_line_ending},
27+
combinator::{map_res, opt, recognize},
2928
sequence::terminated,
3029
};
3130

@@ -106,17 +105,13 @@ impl RespParse {
106105
}
107106

108107
fn parse_inline(input: &[u8]) -> IResult<&[u8], RespData> {
109-
let mut parse_parts = separated_list0(
110-
space1,
111-
map(
112-
take_while1(|c| c != b' ' && c != b'\r' && c != b'\n'),
113-
|s: &[u8]| Bytes::copy_from_slice(s),
114-
),
115-
);
116-
117-
let (input, parts) = parse_parts.parse(input)?;
108+
let (input, line) = terminated(not_line_ending, line_ending).parse(input)?;
118109

119-
let (input, _) = line_ending(input)?;
110+
let parts = line
111+
.split(|byte| byte.is_ascii_whitespace())
112+
.filter(|part| !part.is_empty())
113+
.map(Bytes::copy_from_slice)
114+
.collect::<Vec<_>>();
120115

121116
if parts.is_empty() {
122117
return Err(nom::Err::Error(nom::error::Error::new(
@@ -125,7 +120,27 @@ impl RespParse {
125120
)));
126121
}
127122

128-
Ok((input, RespData::Inline(parts)))
123+
Ok((
124+
input,
125+
RespData::Array(Some(
126+
parts
127+
.into_iter()
128+
.map(|part| RespData::BulkString(Some(part)))
129+
.collect(),
130+
)),
131+
))
132+
}
133+
134+
fn skip_empty_lines(&mut self) {
135+
loop {
136+
if self.buffer.starts_with(b"\r\n") {
137+
self.buffer.advance(2);
138+
} else if self.buffer.starts_with(b"\n") {
139+
self.buffer.advance(1);
140+
} else {
141+
break;
142+
}
143+
}
129144
}
130145

131146
fn parse_simple_string(input: &[u8]) -> IResult<&[u8], RespData> {
@@ -432,6 +447,8 @@ impl RespParse {
432447
}
433448

434449
fn process_buffer(&mut self) -> RespParseResult {
450+
self.skip_empty_lines();
451+
435452
if self.buffer.is_empty() {
436453
return RespParseResult::Incomplete;
437454
}
@@ -497,6 +514,8 @@ impl Drop for RespParse {
497514
#[allow(clippy::unwrap_used)]
498515
#[cfg(test)]
499516
mod tests {
517+
use crate::command::CommandType;
518+
500519
use super::Bytes;
501520
use super::{Parse, RespData, RespParse, RespParseResult, RespVersion};
502521

@@ -533,52 +552,111 @@ mod tests {
533552
let res = parser.parse(Bytes::from("ping\r\n"));
534553
assert_eq!(
535554
res,
536-
RespParseResult::Complete(RespData::Inline(vec![Bytes::from("ping")]))
555+
RespParseResult::Complete(RespData::Array(Some(vec![RespData::BulkString(Some(
556+
Bytes::from("ping"),
557+
))])))
537558
);
538559
parser.reset();
539560

540561
let res = parser.parse(Bytes::from("PING\r\n\r\n"));
541562
assert_eq!(
542563
res,
543-
RespParseResult::Complete(RespData::Inline(vec![Bytes::from("PING")]))
564+
RespParseResult::Complete(RespData::Array(Some(vec![RespData::BulkString(Some(
565+
Bytes::from("PING"),
566+
))])))
544567
);
545568
parser.reset();
546569

547570
let res = parser.parse(Bytes::from("PING\n"));
548571
assert_eq!(
549572
res,
550-
RespParseResult::Complete(RespData::Inline(vec![Bytes::from("PING")]))
573+
RespParseResult::Complete(RespData::Array(Some(vec![RespData::BulkString(Some(
574+
Bytes::from("PING"),
575+
))])))
551576
);
552577
parser.reset();
553578

554579
let res = parser.parse(Bytes::from("PING\n\n"));
555580
assert_eq!(
556581
res,
557-
RespParseResult::Complete(RespData::Inline(vec![Bytes::from("PING")]))
582+
RespParseResult::Complete(RespData::Array(Some(vec![RespData::BulkString(Some(
583+
Bytes::from("PING"),
584+
))])))
558585
);
559586
parser.reset();
560587

561588
let res = parser.parse(Bytes::from("PING\r\n\n"));
562589
assert_eq!(
563590
res,
564-
RespParseResult::Complete(RespData::Inline(vec![Bytes::from("PING")]))
591+
RespParseResult::Complete(RespData::Array(Some(vec![RespData::BulkString(Some(
592+
Bytes::from("PING"),
593+
))])))
565594
);
566595
parser.reset();
567596
}
568597

598+
#[test]
599+
fn test_parse_inline_info_command() {
600+
let mut parser = RespParse::new(RespVersion::RESP2);
601+
let res = parser.parse(Bytes::from("info\r\n"));
602+
assert_eq!(
603+
res,
604+
RespParseResult::Complete(RespData::Array(Some(vec![RespData::BulkString(Some(
605+
Bytes::from("info"),
606+
))])))
607+
);
608+
609+
let command = parser.next_command().unwrap().unwrap();
610+
assert_eq!(command.command_type, CommandType::Info);
611+
assert!(command.args.is_empty());
612+
}
613+
614+
#[test]
615+
fn test_parse_inline_command_with_args() {
616+
let mut parser = RespParse::new(RespVersion::RESP2);
617+
let res = parser.parse(Bytes::from("ping hello\r\n"));
618+
assert_eq!(
619+
res,
620+
RespParseResult::Complete(RespData::Array(Some(vec![
621+
RespData::BulkString(Some(Bytes::from("ping"))),
622+
RespData::BulkString(Some(Bytes::from("hello"))),
623+
])))
624+
);
625+
626+
let command = parser.next_command().unwrap().unwrap();
627+
assert_eq!(command.command_type, CommandType::Ping);
628+
assert_eq!(command.arg_count(), 1);
629+
assert_eq!(command.arg(0), Some(&Bytes::from("hello")));
630+
}
631+
632+
#[test]
633+
fn test_parse_inline_with_surrounding_whitespace() {
634+
let mut parser = RespParse::new(RespVersion::RESP2);
635+
let res = parser.parse(Bytes::from(" \tinfo \t\r\n"));
636+
assert_eq!(
637+
res,
638+
RespParseResult::Complete(RespData::Array(Some(vec![RespData::BulkString(Some(
639+
Bytes::from("info"),
640+
))])))
641+
);
642+
643+
let command = parser.next_command().unwrap().unwrap();
644+
assert_eq!(command.command_type, CommandType::Info);
645+
}
646+
569647
#[test]
570648
fn test_parse_inline_params() {
571649
let mut parser = RespParse::new(RespVersion::RESP2);
572650
let res = parser.parse(Bytes::from("hmget fruit apple banana watermelon\r\n"));
573651
assert_eq!(
574652
res,
575-
RespParseResult::Complete(RespData::Inline(vec![
576-
Bytes::from("hmget"),
577-
Bytes::from("fruit"),
578-
Bytes::from("apple"),
579-
Bytes::from("banana"),
580-
Bytes::from("watermelon")
581-
]))
653+
RespParseResult::Complete(RespData::Array(Some(vec![
654+
RespData::BulkString(Some(Bytes::from("hmget"))),
655+
RespData::BulkString(Some(Bytes::from("fruit"))),
656+
RespData::BulkString(Some(Bytes::from("apple"))),
657+
RespData::BulkString(Some(Bytes::from("banana"))),
658+
RespData::BulkString(Some(Bytes::from("watermelon"))),
659+
])))
582660
);
583661
}
584662

@@ -591,20 +669,49 @@ mod tests {
591669
));
592670
assert_eq!(
593671
res,
594-
RespParseResult::Complete(RespData::Inline(vec![Bytes::from("ping")]))
672+
RespParseResult::Complete(RespData::Array(Some(vec![RespData::BulkString(Some(
673+
Bytes::from("ping"),
674+
))])))
595675
);
596676

597677
let res = parser.parse(Bytes::new());
598678
assert_eq!(
599679
res,
600-
RespParseResult::Complete(RespData::Inline(vec![
601-
Bytes::from("hmget"),
602-
Bytes::from("fruit"),
603-
Bytes::from("apple"),
604-
Bytes::from("banana"),
605-
Bytes::from("watermelon")
606-
]))
680+
RespParseResult::Complete(RespData::Array(Some(vec![
681+
RespData::BulkString(Some(Bytes::from("hmget"))),
682+
RespData::BulkString(Some(Bytes::from("fruit"))),
683+
RespData::BulkString(Some(Bytes::from("apple"))),
684+
RespData::BulkString(Some(Bytes::from("banana"))),
685+
RespData::BulkString(Some(Bytes::from("watermelon"))),
686+
])))
687+
);
688+
}
689+
690+
#[test]
691+
fn test_parse_multiple_inline_with_blank_lines() {
692+
let mut parser = RespParse::new(RespVersion::RESP2);
693+
694+
let res = parser.parse(Bytes::from("ping\r\n\r\ninfo\r\n"));
695+
assert_eq!(
696+
res,
697+
RespParseResult::Complete(RespData::Array(Some(vec![RespData::BulkString(Some(
698+
Bytes::from("ping"),
699+
))])))
607700
);
701+
702+
let res = parser.parse(Bytes::new());
703+
assert_eq!(
704+
res,
705+
RespParseResult::Complete(RespData::Array(Some(vec![RespData::BulkString(Some(
706+
Bytes::from("info"),
707+
))])))
708+
);
709+
710+
let command = parser.next_command().unwrap().unwrap();
711+
assert_eq!(command.command_type, CommandType::Ping);
712+
713+
let command = parser.next_command().unwrap().unwrap();
714+
assert_eq!(command.command_type, CommandType::Info);
608715
}
609716

610717
#[test]

‎src/resp/src/types.rs‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ pub enum RespType {
3434
Integer,
3535
BulkString,
3636
Array,
37-
Inline,
3837
// RESP3 types
3938
Null,
4039
Boolean,
@@ -76,7 +75,6 @@ impl RespType {
7675
RespType::Integer => Some(b':'),
7776
RespType::BulkString => Some(b'$'),
7877
RespType::Array => Some(b'*'),
79-
RespType::Inline => None,
8078
// RESP3 types
8179
RespType::Null => Some(b'_'),
8280
RespType::Boolean => Some(b'#'),
@@ -98,7 +96,6 @@ pub enum RespData {
9896
Integer(i64),
9997
BulkString(Option<Bytes>),
10098
Array(Option<Vec<RespData>>),
101-
Inline(Vec<Bytes>),
10299
// RESP3 types
103100
Null,
104101
Boolean(bool),
@@ -125,7 +122,6 @@ impl RespData {
125122
RespData::Integer(_) => RespType::Integer,
126123
RespData::BulkString(_) => RespType::BulkString,
127124
RespData::Array(_) => RespType::Array,
128-
RespData::Inline(_) => RespType::Inline,
129125
// RESP3 types
130126
RespData::Null => RespType::Null,
131127
RespData::Boolean(_) => RespType::Boolean,
@@ -146,9 +142,6 @@ impl RespData {
146142
RespData::Integer(num) => Some(num.to_string()),
147143
RespData::BulkString(Some(bytes)) => String::from_utf8(bytes.to_vec()).ok(),
148144
RespData::BulkString(None) => None,
149-
RespData::Inline(parts) if !parts.is_empty() => {
150-
String::from_utf8(parts[0].to_vec()).ok()
151-
}
152145
// RESP3 types
153146
RespData::Null => None,
154147
RespData::Boolean(b) => Some(b.to_string()),
@@ -167,7 +160,6 @@ impl RespData {
167160
RespData::Integer(num) => Some(Bytes::from(num.to_string())),
168161
RespData::BulkString(Some(bytes)) => Some(bytes.clone()),
169162
RespData::BulkString(None) => None,
170-
RespData::Inline(parts) if !parts.is_empty() => Some(parts[0].clone()),
171163
// RESP3 types
172164
RespData::Null => None,
173165
RespData::Boolean(b) => Some(Bytes::from(b.to_string())),
@@ -306,15 +298,6 @@ impl fmt::Debug for RespData {
306298
RespData::BulkString(None) => write!(f, "BulkString(nil)"),
307299
RespData::Array(Some(array)) => write!(f, "Array({array:?})"),
308300
RespData::Array(None) => write!(f, "Array(nil)"),
309-
RespData::Inline(parts) => {
310-
write!(f, "Inline(")?;
311-
let parts_str: Vec<_> = parts
312-
.iter()
313-
.filter_map(|b| std::str::from_utf8(b).ok())
314-
.collect();
315-
write!(f, "{parts_str:?}")?;
316-
write!(f, ")")
317-
}
318301
// RESP3 types
319302
RespData::Null => write!(f, "Null"),
320303
RespData::Boolean(b) => write!(f, "Boolean({b})"),

0 commit comments

Comments
 (0)