From faf24c6ad8eee1c3d5ccc9a4d4835717b8e2903f Mon Sep 17 00:00:00 2001 From: Basti Ortiz <39114273+Some-Dood@users.noreply.github.com> Date: Wed, 27 Apr 2022 02:29:49 +0800 Subject: [PATCH] refactor(http1): assorted code readability improvements in `h1/conn.rs` (#2817) * refactor: use `matches` macro to flatten code * refactor: prefer `matches` over explicit matching * refactor: reuse `can_write_body` method * refactor: use `matches` over `if`-`let` * refactor: nested `if`-`else` as early return * refactor: move inner `match` logic outside * refactor: unneeded `return` in `match` * refactor: remove unneeded reference matching * refactor: use early returns for idle check * refactor: use `matches` macro for Boolean `match` --- src/proto/h1/conn.rs | 141 +++++++++++++++++++------------------------ 1 file changed, 62 insertions(+), 79 deletions(-) diff --git a/src/proto/h1/conn.rs b/src/proto/h1/conn.rs index 74571baa..7d7c3fd2 100644 --- a/src/proto/h1/conn.rs +++ b/src/proto/h1/conn.rs @@ -155,19 +155,15 @@ where } pub(crate) fn can_read_head(&self) -> bool { - match self.state.reading { - Reading::Init => { - if T::should_read_first() { - true - } else { - match self.state.writing { - Writing::Init => false, - _ => true, - } - } - } - _ => false, + if !matches!(self.state.reading, Reading::Init) { + return false; } + + if T::should_read_first() { + return true; + } + + !matches!(self.state.writing, Writing::Init) } pub(crate) fn can_read_body(&self) -> bool { @@ -367,10 +363,10 @@ where } fn is_mid_message(&self) -> bool { - match (&self.state.reading, &self.state.writing) { - (&Reading::Init, &Writing::Init) => false, - _ => true, - } + !matches!( + (&self.state.reading, &self.state.writing), + (&Reading::Init, &Writing::Init) + ) } // This will check to make sure the io object read is empty. @@ -493,11 +489,10 @@ where } pub(crate) fn can_write_head(&self) -> bool { - if !T::should_read_first() { - if let Reading::Closed = self.state.reading { - return false; - } + if !T::should_read_first() && matches!(self.state.reading, Reading::Closed) { + return false; } + match self.state.writing { Writing::Init => self.io.can_headers_buf(), _ => false, @@ -641,15 +636,15 @@ where Writing::Body(ref mut encoder) => { self.io.buffer(encoder.encode(chunk)); - if encoder.is_eof() { - if encoder.is_last() { - Writing::Closed - } else { - Writing::KeepAlive - } - } else { + if !encoder.is_eof() { return; } + + if encoder.is_last() { + Writing::Closed + } else { + Writing::KeepAlive + } } _ => unreachable!("write_body invalid state: {:?}", self.state.writing), }; @@ -680,32 +675,31 @@ where pub(crate) fn end_body(&mut self) -> crate::Result<()> { debug_assert!(self.can_write_body()); - let mut res = Ok(()); - let state = match self.state.writing { - Writing::Body(ref mut encoder) => { - // end of stream, that means we should try to eof - match encoder.end() { - Ok(end) => { - if let Some(end) = end { - self.io.buffer(end); - } - if encoder.is_last() || encoder.is_close_delimited() { - Writing::Closed - } else { - Writing::KeepAlive - } - } - Err(not_eof) => { - res = Err(crate::Error::new_body_write_aborted().with(not_eof)); - Writing::Closed - } - } - } + let encoder = match self.state.writing { + Writing::Body(ref mut enc) => enc, _ => return Ok(()), }; - self.state.writing = state; - res + // end of stream, that means we should try to eof + match encoder.end() { + Ok(end) => { + if let Some(end) = end { + self.io.buffer(end); + } + + self.state.writing = if encoder.is_last() || encoder.is_close_delimited() { + Writing::Closed + } else { + Writing::KeepAlive + }; + + Ok(()) + } + Err(not_eof) => { + self.state.writing = Writing::Closed; + Err(crate::Error::new_body_write_aborted().with(not_eof)) + } + } } // When we get a parse error, depending on what side we are, we might be able @@ -758,10 +752,7 @@ where // If still in Reading::Body, just give up match self.state.reading { - Reading::Init | Reading::KeepAlive => { - trace!("body drained"); - return; - } + Reading::Init | Reading::KeepAlive => trace!("body drained"), _ => self.close_read(), } } @@ -1012,43 +1003,35 @@ impl State { self.method = None; self.keep_alive.idle(); - if self.is_idle() { - self.reading = Reading::Init; - self.writing = Writing::Init; - // !T::should_read_first() means Client. - // - // If Client connection has just gone idle, the Dispatcher - // should try the poll loop one more time, so as to poll the - // pending requests stream. - if !T::should_read_first() { - self.notify_read = true; - } - } else { + if !self.is_idle() { self.close(); + return; + } + + self.reading = Reading::Init; + self.writing = Writing::Init; + + // !T::should_read_first() means Client. + // + // If Client connection has just gone idle, the Dispatcher + // should try the poll loop one more time, so as to poll the + // pending requests stream. + if !T::should_read_first() { + self.notify_read = true; } } fn is_idle(&self) -> bool { - if let KA::Idle = self.keep_alive.status() { - true - } else { - false - } + matches!(self.keep_alive.status(), KA::Idle) } fn is_read_closed(&self) -> bool { - match self.reading { - Reading::Closed => true, - _ => false, - } + matches!(self.reading, Reading::Closed) } fn is_write_closed(&self) -> bool { - match self.writing { - Writing::Closed => true, - _ => false, - } + matches!(self.writing, Writing::Closed) } fn prepare_upgrade(&mut self) -> crate::upgrade::OnUpgrade {