From 7cbb17d6b774f3a911c141e86c18b9bd97ed31dd Mon Sep 17 00:00:00 2001 From: BlueGlassBlock Date: Sat, 13 May 2023 20:40:23 +0800 Subject: [PATCH 1/5] feat: allow notify to use Fn trait --- src/blocking_retry.rs | 8 ++++---- src/retry.rs | 33 +++++++++++++++++++++++++++++---- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/src/blocking_retry.rs b/src/blocking_retry.rs index 9f5038f..24e85b2 100644 --- a/src/blocking_retry.rs +++ b/src/blocking_retry.rs @@ -61,7 +61,7 @@ where pub struct BlockingRetry Result> { backoff: B, retryable: fn(&E) -> bool, - notify: fn(&E, Duration), + notify: Box, f: F, } @@ -75,7 +75,7 @@ where BlockingRetry { backoff, retryable: |_: &E| true, - notify: |_: &E, _: Duration| {}, + notify: Box::new(|_: &E, _: Duration| {}), f, } } @@ -140,8 +140,8 @@ where /// Ok(()) /// } /// ``` - pub fn notify(mut self, notify: fn(&E, Duration)) -> Self { - self.notify = notify; + pub fn notify(mut self, notify: impl Fn(&E, Duration) + 'static) -> Self { + self.notify = Box::new(notify); self } diff --git a/src/retry.rs b/src/retry.rs index c7e8734..72a8369 100644 --- a/src/retry.rs +++ b/src/retry.rs @@ -88,7 +88,7 @@ where pub struct Retry>, FutureFn: FnMut() -> Fut> { backoff: B, retryable: fn(&E) -> bool, - notify: fn(&E, Duration), + notify: Box, future_fn: FutureFn, #[pin] @@ -106,7 +106,7 @@ where Retry { backoff, retryable: |_: &E| true, - notify: |_: &E, _: Duration| {}, + notify: Box::new(|_: &E, _: Duration| {}), future_fn, state: State::Idle, } @@ -179,8 +179,8 @@ where /// Ok(()) /// } /// ``` - pub fn notify(mut self, notify: fn(&E, Duration)) -> Self { - self.notify = notify; + pub fn notify<'s>(mut self, notify: impl Fn(&E, Duration) + 'static) -> Self { + self.notify = Box::new(notify); self } } @@ -320,4 +320,29 @@ mod tests { assert_eq!(*error_times.lock().await, 4); Ok(()) } + + + #[tokio::test] + async fn test_retry_with_notify() -> anyhow::Result<()> { + let v_lock = std::rc::Rc::new(std::sync::Mutex::new(0)); + + let f = || async { + Err::<(), anyhow::Error>(anyhow::anyhow!("not retryable")) + }; + + let backoff = ExponentialBuilder::default().with_min_delay(Duration::from_millis(1)); + let v_lock_copy = v_lock.clone(); + let result = f + .retry(&backoff) + .notify(move |e, dur| { + assert_eq!("not retryable", e.to_string()); + *v_lock_copy.lock().unwrap() += 1; + assert!(dur >= Duration::from_millis(1)); + }) + .await; + + assert!(result.is_err()); + assert_eq!(3, *v_lock.lock().unwrap()); + Ok(()) + } } From ccb7f7512b62a97a98c3ee62d1e510e15388622e Mon Sep 17 00:00:00 2001 From: BlueGlassBlock Date: Sun, 14 May 2023 12:45:37 +0800 Subject: [PATCH 2/5] feat: use FnMut, apply to when --- src/blocking_retry.rs | 12 ++++++------ src/retry.rs | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/blocking_retry.rs b/src/blocking_retry.rs index 24e85b2..0047518 100644 --- a/src/blocking_retry.rs +++ b/src/blocking_retry.rs @@ -60,8 +60,8 @@ where /// Retry struct generated by [`Retryable`]. pub struct BlockingRetry Result> { backoff: B, - retryable: fn(&E) -> bool, - notify: Box, + retryable: Box bool>, + notify: Box, f: F, } @@ -74,7 +74,7 @@ where fn new(f: F, backoff: B) -> Self { BlockingRetry { backoff, - retryable: |_: &E| true, + retryable: Box::new(|_: &E| true), notify: Box::new(|_: &E, _: Duration| {}), f, } @@ -105,8 +105,8 @@ where /// Ok(()) /// } /// ``` - pub fn when(mut self, retryable: fn(&E) -> bool) -> Self { - self.retryable = retryable; + pub fn when(mut self, retryable: impl FnMut(&E) -> bool + 'static) -> Self { + self.retryable = Box::new(retryable); self } @@ -140,7 +140,7 @@ where /// Ok(()) /// } /// ``` - pub fn notify(mut self, notify: impl Fn(&E, Duration) + 'static) -> Self { + pub fn notify(mut self, notify: impl FnMut(&E, Duration) + 'static) -> Self { self.notify = Box::new(notify); self } diff --git a/src/retry.rs b/src/retry.rs index 72a8369..3b01d16 100644 --- a/src/retry.rs +++ b/src/retry.rs @@ -87,8 +87,8 @@ where #[pin_project] pub struct Retry>, FutureFn: FnMut() -> Fut> { backoff: B, - retryable: fn(&E) -> bool, - notify: Box, + retryable: Box bool>, + notify: Box, future_fn: FutureFn, #[pin] @@ -105,7 +105,7 @@ where fn new(future_fn: FutureFn, backoff: B) -> Self { Retry { backoff, - retryable: |_: &E| true, + retryable: Box::new(|_: &E| true), notify: Box::new(|_: &E, _: Duration| {}), future_fn, state: State::Idle, @@ -141,8 +141,8 @@ where /// Ok(()) /// } /// ``` - pub fn when(mut self, retryable: fn(&E) -> bool) -> Self { - self.retryable = retryable; + pub fn when(mut self, retryable: impl FnMut(&E) -> bool + 'static) -> Self { + self.retryable = Box::new(retryable); self } @@ -179,7 +179,7 @@ where /// Ok(()) /// } /// ``` - pub fn notify<'s>(mut self, notify: impl Fn(&E, Duration) + 'static) -> Self { + pub fn notify<'s>(mut self, notify: impl FnMut(&E, Duration) + 'static) -> Self { self.notify = Box::new(notify); self } From ec9c1b27dacbd1ca4aedb218b0bfe00ba7137976 Mon Sep 17 00:00:00 2001 From: BlueGlassBlock Date: Sun, 14 May 2023 12:46:40 +0800 Subject: [PATCH 3/5] chore: remove test for now --- src/retry.rs | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/src/retry.rs b/src/retry.rs index 3b01d16..cc51488 100644 --- a/src/retry.rs +++ b/src/retry.rs @@ -320,29 +320,4 @@ mod tests { assert_eq!(*error_times.lock().await, 4); Ok(()) } - - - #[tokio::test] - async fn test_retry_with_notify() -> anyhow::Result<()> { - let v_lock = std::rc::Rc::new(std::sync::Mutex::new(0)); - - let f = || async { - Err::<(), anyhow::Error>(anyhow::anyhow!("not retryable")) - }; - - let backoff = ExponentialBuilder::default().with_min_delay(Duration::from_millis(1)); - let v_lock_copy = v_lock.clone(); - let result = f - .retry(&backoff) - .notify(move |e, dur| { - assert_eq!("not retryable", e.to_string()); - *v_lock_copy.lock().unwrap() += 1; - assert!(dur >= Duration::from_millis(1)); - }) - .await; - - assert!(result.is_err()); - assert_eq!(3, *v_lock.lock().unwrap()); - Ok(()) - } } From cdcaff3d7434283a7ee2518751844ac5a87ce65a Mon Sep 17 00:00:00 2001 From: BlueGlassBlock Date: Fri, 19 May 2023 12:45:48 +0800 Subject: [PATCH 4/5] feat: remove box --- src/blocking_retry.rs | 46 ++++++++++++++++++++++++--------- src/retry.rs | 59 ++++++++++++++++++++++++++++++++++--------- 2 files changed, 81 insertions(+), 24 deletions(-) diff --git a/src/blocking_retry.rs b/src/blocking_retry.rs index 0047518..89d7e47 100644 --- a/src/blocking_retry.rs +++ b/src/blocking_retry.rs @@ -58,13 +58,19 @@ where } /// Retry struct generated by [`Retryable`]. -pub struct BlockingRetry Result> { +pub struct BlockingRetry< + B: Backoff, + T, + E, + F: FnMut() -> Result, + RF = fn(&E) -> bool, + NF = fn(&E, Duration), +> { backoff: B, - retryable: Box bool>, - notify: Box, + retryable: RF, + notify: NF, f: F, } - impl BlockingRetry where B: Backoff, @@ -74,12 +80,20 @@ where fn new(f: F, backoff: B) -> Self { BlockingRetry { backoff, - retryable: Box::new(|_: &E| true), - notify: Box::new(|_: &E, _: Duration| {}), + retryable: |_: &E| true, + notify: |_: &E, _: Duration| {}, f, } } +} +impl BlockingRetry +where + B: Backoff, + F: FnMut() -> Result, + RF: FnMut(&E) -> bool, + NF: FnMut(&E, Duration), +{ /// Set the conditions for retrying. /// /// If not specified, we treat all errors as retryable. @@ -105,9 +119,13 @@ where /// Ok(()) /// } /// ``` - pub fn when(mut self, retryable: impl FnMut(&E) -> bool + 'static) -> Self { - self.retryable = Box::new(retryable); - self + pub fn when bool>(self, retryable: RN) -> BlockingRetry { + BlockingRetry { + backoff: self.backoff, + retryable, + notify: self.notify, + f: self.f, + } } /// Set to notify for everything retrying. @@ -140,9 +158,13 @@ where /// Ok(()) /// } /// ``` - pub fn notify(mut self, notify: impl FnMut(&E, Duration) + 'static) -> Self { - self.notify = Box::new(notify); - self + pub fn notify(self, notify: NN) -> BlockingRetry { + BlockingRetry { + backoff: self.backoff, + retryable: self.retryable, + notify, + f: self.f, + } } /// Call the retried function. diff --git a/src/retry.rs b/src/retry.rs index cc51488..dc473c9 100644 --- a/src/retry.rs +++ b/src/retry.rs @@ -85,10 +85,18 @@ where /// Retry struct generated by [`Retryable`]. #[pin_project] -pub struct Retry>, FutureFn: FnMut() -> Fut> { +pub struct Retry< + B: Backoff, + T, + E, + Fut: Future>, + FutureFn: FnMut() -> Fut, + RF = fn(&E) -> bool, + NF = fn(&E, Duration), +> { backoff: B, - retryable: Box bool>, - notify: Box, + retryable: RF, + notify: NF, future_fn: FutureFn, #[pin] @@ -105,13 +113,22 @@ where fn new(future_fn: FutureFn, backoff: B) -> Self { Retry { backoff, - retryable: Box::new(|_: &E| true), - notify: Box::new(|_: &E, _: Duration| {}), + retryable: |_: &E| true, + notify: |_: &E, _: Duration| {}, future_fn, state: State::Idle, } } +} +impl Retry +where + B: Backoff, + Fut: Future>, + FutureFn: FnMut() -> Fut, + RF: FnMut(&E) -> bool, + NF: FnMut(&E, Duration), +{ /// Set the conditions for retrying. /// /// If not specified, we treat all errors as retryable. @@ -141,9 +158,17 @@ where /// Ok(()) /// } /// ``` - pub fn when(mut self, retryable: impl FnMut(&E) -> bool + 'static) -> Self { - self.retryable = Box::new(retryable); - self + pub fn when bool>( + self, + retryable: RN, + ) -> Retry { + Retry { + backoff: self.backoff, + retryable, + notify: self.notify, + future_fn: self.future_fn, + state: self.state, + } } /// Set to notify for everything retrying. @@ -179,9 +204,17 @@ where /// Ok(()) /// } /// ``` - pub fn notify<'s>(mut self, notify: impl FnMut(&E, Duration) + 'static) -> Self { - self.notify = Box::new(notify); - self + pub fn notify( + self, + notify: NN, + ) -> Retry { + Retry { + backoff: self.backoff, + retryable: self.retryable, + notify, + future_fn: self.future_fn, + state: self.state, + } } } @@ -201,11 +234,13 @@ enum State>> { Sleeping(#[pin] Pin>), } -impl Future for Retry +impl Future for Retry where B: Backoff, Fut: Future>, FutureFn: FnMut() -> Fut, + RF: FnMut(&E) -> bool, + NF: FnMut(&E, Duration), { type Output = Result; From a29afb8f79c57df0474f78f2b7cbd7226db10cd5 Mon Sep 17 00:00:00 2001 From: BlueGlassBlock Date: Fri, 19 May 2023 13:20:17 +0800 Subject: [PATCH 5/5] chore: add simple test --- src/blocking_retry.rs | 28 ++++++++++++++++++++++++++++ src/retry.rs | 28 ++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/src/blocking_retry.rs b/src/blocking_retry.rs index 89d7e47..6141889 100644 --- a/src/blocking_retry.rs +++ b/src/blocking_retry.rs @@ -267,4 +267,32 @@ mod tests { assert_eq!(*error_times.lock().unwrap(), 4); Ok(()) } + + #[test] + fn test_fn_mut_when_and_notify() -> anyhow::Result<()> { + let mut calls_retryable: Vec<()> = vec![]; + let mut calls_notify: Vec<()> = vec![]; + + let f = || Err::<(), anyhow::Error>(anyhow::anyhow!("retryable")); + + let backoff = ExponentialBuilder::default().with_min_delay(Duration::from_millis(1)); + let result = f + .retry(&backoff) + .when(|_| { + calls_retryable.push(()); + true + }) + .notify(|_, _| { + calls_notify.push(()); + }) + .call(); + + assert!(result.is_err()); + assert_eq!("retryable", result.unwrap_err().to_string()); + // `f` always returns error "retryable", so it should be executed + // 4 times (retry 3 times). + assert_eq!(calls_retryable.len(), 4); + assert_eq!(calls_notify.len(), 3); + Ok(()) + } } diff --git a/src/retry.rs b/src/retry.rs index dc473c9..232c7ba 100644 --- a/src/retry.rs +++ b/src/retry.rs @@ -355,4 +355,32 @@ mod tests { assert_eq!(*error_times.lock().await, 4); Ok(()) } + + #[tokio::test] + async fn test_fn_mut_when_and_notify() -> anyhow::Result<()> { + let mut calls_retryable: Vec<()> = vec![]; + let mut calls_notify: Vec<()> = vec![]; + + let f = || async { Err::<(), anyhow::Error>(anyhow::anyhow!("retryable")) }; + + let backoff = ExponentialBuilder::default().with_min_delay(Duration::from_millis(1)); + let result = f + .retry(&backoff) + .when(|_| { + calls_retryable.push(()); + true + }) + .notify(|_, _| { + calls_notify.push(()); + }) + .await; + + assert!(result.is_err()); + assert_eq!("retryable", result.unwrap_err().to_string()); + // `f` always returns error "retryable", so it should be executed + // 4 times (retry 3 times). + assert_eq!(calls_retryable.len(), 4); + assert_eq!(calls_notify.len(), 3); + Ok(()) + } }