From bf1ed601ac805326fc3035e1bc14303da2641205 Mon Sep 17 00:00:00 2001 From: Temirkhan Myrzamadi Date: Sun, 28 Mar 2021 05:34:25 +0600 Subject: [PATCH 1/9] Fix the storage persistency bug --- CHANGELOG.md | 12 ++++++ .../dialogue/dialogue_dispatcher.rs | 24 +++++------ .../dialogue/storage/in_mem_storage.rs | 30 +++++++++---- src/dispatching/dialogue/storage/mod.rs | 20 ++++----- .../dialogue/storage/trace_storage.rs | 42 +++++++++---------- src/dispatching/repls/dialogues_repl.rs | 4 +- 6 files changed, 76 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4761c3d0..64fa0e81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [unreleased] +### Added + + - `Storage::get_dialogue` + +### Changed + + - Do not return a dialogue from `Storage::{remove_dialogue, update_dialogue}`. + +### Fixed + + - A storage persistency bug ([issue 304](https://github.com/teloxide/teloxide/issues/304)). + ### Fixed - Remove `reqwest` dependency. It's not needed after the [teloxide-core] integration. diff --git a/src/dispatching/dialogue/dialogue_dispatcher.rs b/src/dispatching/dialogue/dialogue_dispatcher.rs index 706cf952..f2418596 100644 --- a/src/dispatching/dialogue/dialogue_dispatcher.rs +++ b/src/dispatching/dialogue/dialogue_dispatcher.rs @@ -4,7 +4,7 @@ use crate::dispatching::{ }, DispatcherHandler, UpdateWithCx, }; -use std::{convert::Infallible, marker::PhantomData}; +use std::{convert::Infallible, fmt::Debug, marker::PhantomData}; use futures::{future::BoxFuture, FutureExt, StreamExt}; use tokio::sync::mpsc; @@ -65,7 +65,7 @@ where Upd: GetChatId + Send + 'static, D: Default + Send + 'static, S: Storage + Send + Sync + 'static, - S::Error: Send + 'static, + S::Error: Debug + Send + 'static, { /// Creates a dispatcher with the specified `handler` and `storage`. #[must_use] @@ -97,18 +97,13 @@ where async move { let chat_id = cx.update.chat_id(); - let dialogue = Arc::clone(&storage) - .remove_dialogue(chat_id) - .await - .map(Option::unwrap_or_default); + let dialogue = + Arc::clone(&storage).get_dialogue(chat_id).await.map(Option::unwrap_or_default); match handler.handle(DialogueWithCx { cx, dialogue }).await { DialogueStage::Next(new_dialogue) => { - if let Ok(Some(_)) = storage.update_dialogue(chat_id, new_dialogue).await { - panic!( - "Oops, you have an bug in your Storage: update_dialogue returns \ - Some after remove_dialogue" - ); + if let Err(e) = storage.update_dialogue(chat_id, new_dialogue).await { + log::error!("Storage::update_dialogue failed: {:?}", e); } } DialogueStage::Exit => { @@ -117,8 +112,9 @@ where // sender right here: senders.remove(&chat_id); - // We already removed a dialogue from `storage` (see - // the beginning of this async block). + if let Err(e) = storage.remove_dialogue(chat_id).await { + log::error!("Storage::remove_dialogue failed: {:?}", e); + } } } } @@ -134,7 +130,7 @@ where Upd: GetChatId + Send + 'static, D: Default + Send + 'static, S: Storage + Send + Sync + 'static, - S::Error: Send + 'static, + S::Error: Debug + Send + 'static, R: Requester + Send, { fn handle( diff --git a/src/dispatching/dialogue/storage/in_mem_storage.rs b/src/dispatching/dialogue/storage/in_mem_storage.rs index 468a305e..476010d3 100644 --- a/src/dispatching/dialogue/storage/in_mem_storage.rs +++ b/src/dispatching/dialogue/storage/in_mem_storage.rs @@ -25,27 +25,41 @@ impl InMemStorage { } } -impl Storage for InMemStorage { +impl Storage for InMemStorage +where + D: ToOwned, + D: Send + 'static, +{ type Error = std::convert::Infallible; - fn remove_dialogue( - self: Arc, - chat_id: i64, - ) -> BoxFuture<'static, Result, Self::Error>> + fn remove_dialogue(self: Arc, chat_id: i64) -> BoxFuture<'static, Result<(), Self::Error>> where D: Send + 'static, { - Box::pin(async move { Ok(self.map.lock().await.remove(&chat_id)) }) + Box::pin(async move { + self.map.lock().await.remove(&chat_id); + Ok(()) + }) } fn update_dialogue( self: Arc, chat_id: i64, dialogue: D, - ) -> BoxFuture<'static, Result, Self::Error>> + ) -> BoxFuture<'static, Result<(), Self::Error>> where D: Send + 'static, { - Box::pin(async move { Ok(self.map.lock().await.insert(chat_id, dialogue)) }) + Box::pin(async move { + self.map.lock().await.insert(chat_id, dialogue); + Ok(()) + }) + } + + fn get_dialogue( + self: Arc, + chat_id: i64, + ) -> BoxFuture<'static, Result, Self::Error>> { + Box::pin(async move { Ok(self.map.lock().await.get(&chat_id).map(ToOwned::to_owned)) }) } } diff --git a/src/dispatching/dialogue/storage/mod.rs b/src/dispatching/dialogue/storage/mod.rs index 65cedcea..a7cd5a32 100644 --- a/src/dispatching/dialogue/storage/mod.rs +++ b/src/dispatching/dialogue/storage/mod.rs @@ -42,26 +42,26 @@ pub use sqlite_storage::{SqliteStorage, SqliteStorageError}; pub trait Storage { type Error; - /// Removes a dialogue with the specified `chat_id`. - /// - /// Returns `None` if there wasn't such a dialogue, `Some(dialogue)` if a - /// `dialogue` was deleted. + /// Removes a dialogue indexed by `chat_id`. fn remove_dialogue( self: Arc, chat_id: i64, - ) -> BoxFuture<'static, Result, Self::Error>> + ) -> BoxFuture<'static, Result<(), Self::Error>> where D: Send + 'static; - /// Updates a dialogue with the specified `chat_id`. - /// - /// Returns `None` if there wasn't such a dialogue, `Some(dialogue)` if a - /// `dialogue` was updated. + /// Updates a dialogue indexed by `chat_id` with `dialogue`. fn update_dialogue( self: Arc, chat_id: i64, dialogue: D, - ) -> BoxFuture<'static, Result, Self::Error>> + ) -> BoxFuture<'static, Result<(), Self::Error>> where D: Send + 'static; + + /// Provides a dialogue indexed by `chat_id`. + fn get_dialogue( + self: Arc, + chat_id: i64, + ) -> BoxFuture<'static, Result, Self::Error>>; } diff --git a/src/dispatching/dialogue/storage/trace_storage.rs b/src/dispatching/dialogue/storage/trace_storage.rs index 2e28263f..80b37887 100644 --- a/src/dispatching/dialogue/storage/trace_storage.rs +++ b/src/dispatching/dialogue/storage/trace_storage.rs @@ -5,14 +5,13 @@ use std::{ }; use futures::future::BoxFuture; -use log::{log_enabled, trace, Level::Trace}; use crate::dispatching::dialogue::Storage; -/// Storage wrapper for logging purposes +/// Storage wrapper for logging purposes. /// -/// Reports about any dialogue update or removal action on `trace` level -/// using `log` crate. +/// Reports about any dialogue action using the `trace` level in the `log` +/// crate. pub struct TraceStorage { inner: Arc, } @@ -35,14 +34,11 @@ where { type Error = >::Error; - fn remove_dialogue( - self: Arc, - chat_id: i64, - ) -> BoxFuture<'static, Result, Self::Error>> + fn remove_dialogue(self: Arc, chat_id: i64) -> BoxFuture<'static, Result<(), Self::Error>> where D: Send + 'static, { - trace!("Removing dialogue with {}", chat_id); + log::trace!("Removing dialogue #{}", chat_id); >::remove_dialogue(self.inner.clone(), chat_id) } @@ -50,21 +46,23 @@ where self: Arc, chat_id: i64, dialogue: D, - ) -> BoxFuture<'static, Result, Self::Error>> + ) -> BoxFuture<'static, Result<(), Self::Error>> where D: Send + 'static, { - if log_enabled!(Trace) { - Box::pin(async move { - let to = format!("{:#?}", dialogue); - let from = - >::update_dialogue(self.inner.clone(), chat_id, dialogue) - .await?; - trace!("Updated dialogue with {}, {:#?} -> {}", chat_id, from, to); - Ok(from) - }) - } else { - >::update_dialogue(self.inner.clone(), chat_id, dialogue) - } + Box::pin(async move { + let to = format!("{:#?}", dialogue); + >::update_dialogue(self.inner.clone(), chat_id, dialogue).await?; + log::trace!("Updated a dialogue #{}: {:#?}", chat_id, to); + Ok(()) + }) + } + + fn get_dialogue( + self: Arc, + chat_id: i64, + ) -> BoxFuture<'static, Result, Self::Error>> { + log::trace!("Requested a dialogue #{}", chat_id); + >::get_dialogue(self.inner.clone(), chat_id) } } diff --git a/src/dispatching/repls/dialogues_repl.rs b/src/dispatching/repls/dialogues_repl.rs index 706d26a1..c46a2dc4 100644 --- a/src/dispatching/repls/dialogues_repl.rs +++ b/src/dispatching/repls/dialogues_repl.rs @@ -26,7 +26,7 @@ use teloxide_core::{requests::Requester, types::Message}; pub async fn dialogues_repl<'a, R, H, D, Fut>(requester: R, handler: H) where H: Fn(UpdateWithCx, D) -> Fut + Send + Sync + 'static, - D: Default + Send + 'static, + D: ToOwned + Default + Send + 'static, Fut: Future> + Send + 'static, R: Requester + Send + Clone + 'static, ::GetUpdatesFaultTolerant: Send, @@ -61,7 +61,7 @@ pub async fn dialogues_repl_with_listener<'a, R, H, D, Fut, L, ListenerE>( listener: L, ) where H: Fn(UpdateWithCx, D) -> Fut + Send + Sync + 'static, - D: Default + Send + 'static, + D: ToOwned + Default + Send + 'static, Fut: Future> + Send + 'static, L: UpdateListener + Send + 'a, ListenerE: Debug + Send + 'a, From abbbc41892c314b2562cbff51c728b02e19709eb Mon Sep 17 00:00:00 2001 From: Temirkhan Myrzamadi Date: Sun, 28 Mar 2021 06:20:13 +0600 Subject: [PATCH 2/9] Fix RedisStorage, SqliteStorage --- .../dialogue/storage/redis_storage.rs | 21 ++++++-- .../dialogue/storage/sqlite_storage.rs | 48 ++++++++++--------- 2 files changed, 42 insertions(+), 27 deletions(-) diff --git a/src/dispatching/dialogue/storage/redis_storage.rs b/src/dispatching/dialogue/storage/redis_storage.rs index 31a358e8..a89576cf 100644 --- a/src/dispatching/dialogue/storage/redis_storage.rs +++ b/src/dispatching/dialogue/storage/redis_storage.rs @@ -56,7 +56,7 @@ where fn remove_dialogue( self: Arc, chat_id: i64, - ) -> BoxFuture<'static, Result, Self::Error>> { + ) -> BoxFuture<'static, Result<(), Self::Error>> { Box::pin(async move { let res = redis::pipe() .atomic() @@ -70,13 +70,14 @@ where // bulk, so all other branches should be unreachable match res { redis::Value::Bulk(bulk) if bulk.len() == 1 => { - Ok(Option::>::from_redis_value(&bulk[0])? + Option::>::from_redis_value(&bulk[0])? .map(|v| { self.serializer .deserialize(&v) .map_err(RedisStorageError::SerdeError) }) - .transpose()?) + .transpose()?; + Ok(()) } _ => unreachable!(), } @@ -87,14 +88,24 @@ where self: Arc, chat_id: i64, dialogue: D, - ) -> BoxFuture<'static, Result, Self::Error>> { + ) -> BoxFuture<'static, Result<(), Self::Error>> { Box::pin(async move { let dialogue = self.serializer.serialize(&dialogue).map_err(RedisStorageError::SerdeError)?; + self.conn.lock().await.getset::<_, Vec, Option>>(chat_id, dialogue).await?; + Ok(()) + }) + } + + fn get_dialogue( + self: Arc, + chat_id: i64, + ) -> BoxFuture<'static, Result, Self::Error>> { + Box::pin(async move { self.conn .lock() .await - .getset::<_, Vec, Option>>(chat_id, dialogue) + .get::<_, Option>>(chat_id) .await? .map(|d| self.serializer.deserialize(&d).map_err(RedisStorageError::SerdeError)) .transpose() diff --git a/src/dispatching/dialogue/storage/sqlite_storage.rs b/src/dispatching/dialogue/storage/sqlite_storage.rs index f4e4d98c..ceaeacf2 100644 --- a/src/dispatching/dialogue/storage/sqlite_storage.rs +++ b/src/dispatching/dialogue/storage/sqlite_storage.rs @@ -63,20 +63,16 @@ where fn remove_dialogue( self: Arc, chat_id: i64, - ) -> BoxFuture<'static, Result, Self::Error>> { + ) -> BoxFuture<'static, Result<(), Self::Error>> { Box::pin(async move { - Ok(match get_dialogue(&self.pool, chat_id).await? { - Some(d) => { - let prev_dialogue = - self.serializer.deserialize(&d).map_err(SqliteStorageError::SerdeError)?; - sqlx::query("DELETE FROM teloxide_dialogues WHERE chat_id = ?") - .bind(chat_id) - .execute(&self.pool) - .await?; - Some(prev_dialogue) - } - _ => None, - }) + if get_dialogue(&self.pool, chat_id).await?.is_some() { + sqlx::query("DELETE FROM teloxide_dialogues WHERE chat_id = ?") + .bind(chat_id) + .execute(&self.pool) + .await?; + } + + Ok(()) }) } @@ -84,14 +80,10 @@ where self: Arc, chat_id: i64, dialogue: D, - ) -> BoxFuture<'static, Result, Self::Error>> { + ) -> BoxFuture<'static, Result<(), Self::Error>> { Box::pin(async move { - let prev_dialogue = get_dialogue(&self.pool, chat_id) - .await? - .map(|d| self.serializer.deserialize(&d).map_err(SqliteStorageError::SerdeError)) - .transpose()?; - let upd_dialogue = - self.serializer.serialize(&dialogue).map_err(SqliteStorageError::SerdeError)?; + let d = self.serializer.serialize(&dialogue).map_err(SqliteStorageError::SerdeError)?; + self.pool .acquire() .await? @@ -103,10 +95,22 @@ where "#, ) .bind(chat_id) - .bind(upd_dialogue), + .bind(d), ) .await?; - Ok(prev_dialogue) + Ok(()) + }) + } + + fn get_dialogue( + self: Arc, + chat_id: i64, + ) -> BoxFuture<'static, Result, Self::Error>> { + Box::pin(async move { + get_dialogue(&self.pool, chat_id) + .await? + .map(|d| self.serializer.deserialize(&d).map_err(SqliteStorageError::SerdeError)) + .transpose() }) } } From 01b7b91bda595daff7159b9cca4e662d9135027f Mon Sep 17 00:00:00 2001 From: Temirkhan Myrzamadi Date: Sun, 28 Mar 2021 06:30:35 +0600 Subject: [PATCH 3/9] Document how Storage failures are handled in DialogueDispatcher --- src/dispatching/dialogue/dialogue_dispatcher.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/dispatching/dialogue/dialogue_dispatcher.rs b/src/dispatching/dialogue/dialogue_dispatcher.rs index f2418596..b98c029c 100644 --- a/src/dispatching/dialogue/dialogue_dispatcher.rs +++ b/src/dispatching/dialogue/dialogue_dispatcher.rs @@ -19,6 +19,11 @@ use tokio_stream::wrappers::UnboundedReceiverStream; /// Note that it implements [`DispatcherHandler`], so you can just put an /// instance of this dispatcher into the [`Dispatcher`]'s methods. /// +/// Note that when the storage methods [`Storage::remove_dialogue`] and +/// [`Storage::update_dialogue`] are failed, the errors are logged, but a result +/// from [`Storage::get_dialogue`] is provided to a user handler as-is so you +/// can respond to a concrete user with an error description. +/// /// See the [module-level documentation](crate::dispatching::dialogue) for the /// design overview. /// From 9b75378572dec42a5d9d4891bf55cf8c17eea447 Mon Sep 17 00:00:00 2001 From: Temirkhan Myrzamadi Date: Sun, 28 Mar 2021 08:20:35 +0600 Subject: [PATCH 4/9] Fix the tests and examples --- CHANGELOG.md | 1 + examples/dialogue_bot/src/dialogue/mod.rs | 2 +- .../src/dialogue/states/receive_age.rs | 2 +- .../src/dialogue/states/receive_full_name.rs | 2 +- .../src/dialogue/states/receive_location.rs | 2 +- .../dialogue_bot/src/dialogue/states/start.rs | 1 + src/dispatching/dialogue/mod.rs | 5 ++- tests/redis.rs | 42 ++++++++++--------- tests/sqlite.rs | 42 ++++++++++--------- 9 files changed, 54 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 64fa0e81..b42a9c83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Do not return a dialogue from `Storage::{remove_dialogue, update_dialogue}`. + - Require `D: ToOwned` in `dialogues_repl` and `InMemStorage`. ### Fixed diff --git a/examples/dialogue_bot/src/dialogue/mod.rs b/examples/dialogue_bot/src/dialogue/mod.rs index 4bae759a..43ad8db3 100644 --- a/examples/dialogue_bot/src/dialogue/mod.rs +++ b/examples/dialogue_bot/src/dialogue/mod.rs @@ -6,7 +6,7 @@ use crate::dialogue::states::{ use derive_more::From; use teloxide::macros::Transition; -#[derive(Transition, From)] +#[derive(Transition, Clone, From)] pub enum Dialogue { Start(StartState), ReceiveFullName(ReceiveFullNameState), diff --git a/examples/dialogue_bot/src/dialogue/states/receive_age.rs b/examples/dialogue_bot/src/dialogue/states/receive_age.rs index 36c72a23..099b3407 100644 --- a/examples/dialogue_bot/src/dialogue/states/receive_age.rs +++ b/examples/dialogue_bot/src/dialogue/states/receive_age.rs @@ -1,7 +1,7 @@ use crate::dialogue::{states::receive_location::ReceiveLocationState, Dialogue}; use teloxide::prelude::*; -#[derive(Generic)] +#[derive(Clone, Generic)] pub struct ReceiveAgeState { pub full_name: String, } diff --git a/examples/dialogue_bot/src/dialogue/states/receive_full_name.rs b/examples/dialogue_bot/src/dialogue/states/receive_full_name.rs index 21d3fef2..2ea60a1c 100644 --- a/examples/dialogue_bot/src/dialogue/states/receive_full_name.rs +++ b/examples/dialogue_bot/src/dialogue/states/receive_full_name.rs @@ -1,7 +1,7 @@ use crate::dialogue::{states::receive_age::ReceiveAgeState, Dialogue}; use teloxide::prelude::*; -#[derive(Generic)] +#[derive(Clone, Generic)] pub struct ReceiveFullNameState; #[teloxide(subtransition)] diff --git a/examples/dialogue_bot/src/dialogue/states/receive_location.rs b/examples/dialogue_bot/src/dialogue/states/receive_location.rs index aaa1af2d..3c1f6407 100644 --- a/examples/dialogue_bot/src/dialogue/states/receive_location.rs +++ b/examples/dialogue_bot/src/dialogue/states/receive_location.rs @@ -1,7 +1,7 @@ use crate::dialogue::Dialogue; use teloxide::prelude::*; -#[derive(Generic)] +#[derive(Clone, Generic)] pub struct ReceiveLocationState { pub full_name: String, pub age: u8, diff --git a/examples/dialogue_bot/src/dialogue/states/start.rs b/examples/dialogue_bot/src/dialogue/states/start.rs index a4f3c192..f3f12e0c 100644 --- a/examples/dialogue_bot/src/dialogue/states/start.rs +++ b/examples/dialogue_bot/src/dialogue/states/start.rs @@ -1,6 +1,7 @@ use crate::dialogue::{states::ReceiveFullNameState, Dialogue}; use teloxide::prelude::*; +#[derive(Clone)] pub struct StartState; #[teloxide(subtransition)] diff --git a/src/dispatching/dialogue/mod.rs b/src/dispatching/dialogue/mod.rs index 303d161c..ee182dce 100644 --- a/src/dispatching/dialogue/mod.rs +++ b/src/dispatching/dialogue/mod.rs @@ -35,8 +35,11 @@ //! //! use teloxide::{dispatching::dialogue::Transition, prelude::*, teloxide, RequestError}; //! +//! #[derive(Clone)] //! struct _1State; +//! #[derive(Clone)] //! struct _2State; +//! #[derive(Clone)] //! struct _3State; //! //! type Out = TransitionOut; @@ -56,7 +59,7 @@ //! todo!() //! } //! -//! #[derive(Transition)] +//! #[derive(Clone, Transition)] //! enum D { //! _1(_1State), //! _2(_2State), diff --git a/tests/redis.rs b/tests/redis.rs index 2b88ad8e..9777597e 100644 --- a/tests/redis.rs +++ b/tests/redis.rs @@ -1,6 +1,5 @@ use std::{ fmt::{Debug, Display}, - future::Future, sync::Arc, }; use teloxide::dispatching::dialogue::{RedisStorage, Serializer, Storage}; @@ -40,32 +39,35 @@ async fn test_redis_cbor() { type Dialogue = String; +macro_rules! test_dialogues { + ($storage:expr, $_0:expr, $_1:expr, $_2:expr) => { + assert_eq!(Arc::clone(&$storage).get_dialogue(1).await.unwrap(), $_0); + assert_eq!(Arc::clone(&$storage).get_dialogue(11).await.unwrap(), $_1); + assert_eq!(Arc::clone(&$storage).get_dialogue(256).await.unwrap(), $_2); + }; +} + async fn test_redis(storage: Arc>) where S: Send + Sync + Serializer + 'static, >::Error: Debug + Display, { - check_dialogue(None, Arc::clone(&storage).update_dialogue(1, "ABC".to_owned())).await; - check_dialogue(None, Arc::clone(&storage).update_dialogue(11, "DEF".to_owned())).await; - check_dialogue(None, Arc::clone(&storage).update_dialogue(256, "GHI".to_owned())).await; + test_dialogues!(storage, None, None, None); - // 1 - ABC, 11 - DEF, 256 - GHI + Arc::clone(&storage).update_dialogue(1, "ABC".to_owned()).await.unwrap(); + Arc::clone(&storage).update_dialogue(11, "DEF".to_owned()).await.unwrap(); + Arc::clone(&storage).update_dialogue(256, "GHI".to_owned()).await.unwrap(); - check_dialogue("ABC", Arc::clone(&storage).update_dialogue(1, "JKL".to_owned())).await; - check_dialogue("GHI", Arc::clone(&storage).update_dialogue(256, "MNO".to_owned())).await; + test_dialogues!( + storage, + Some("ABC".to_owned()), + Some("DEF".to_owned()), + Some("GHI".to_owned()) + ); - // 1 - GKL, 11 - DEF, 256 - MNO + Arc::clone(&storage).remove_dialogue(1).await.unwrap(); + Arc::clone(&storage).remove_dialogue(11).await.unwrap(); + Arc::clone(&storage).remove_dialogue(256).await.unwrap(); - check_dialogue("JKL", Arc::clone(&storage).remove_dialogue(1)).await; - check_dialogue("DEF", Arc::clone(&storage).remove_dialogue(11)).await; - check_dialogue("MNO", Arc::clone(&storage).remove_dialogue(256)).await; -} - -async fn check_dialogue( - expected: impl Into>, - actual: impl Future, E>>, -) where - E: Debug, -{ - assert_eq!(expected.into().map(ToOwned::to_owned), actual.await.unwrap()) + test_dialogues!(storage, None, None, None); } diff --git a/tests/sqlite.rs b/tests/sqlite.rs index bf08142d..de37de6c 100644 --- a/tests/sqlite.rs +++ b/tests/sqlite.rs @@ -1,6 +1,5 @@ use std::{ fmt::{Debug, Display}, - future::Future, sync::Arc, }; use teloxide::dispatching::dialogue::{Serializer, SqliteStorage, Storage}; @@ -36,32 +35,35 @@ async fn test_sqlite_cbor() { type Dialogue = String; +macro_rules! test_dialogues { + ($storage:expr, $_0:expr, $_1:expr, $_2:expr) => { + assert_eq!(Arc::clone(&$storage).get_dialogue(1).await.unwrap(), $_0); + assert_eq!(Arc::clone(&$storage).get_dialogue(11).await.unwrap(), $_1); + assert_eq!(Arc::clone(&$storage).get_dialogue(256).await.unwrap(), $_2); + }; +} + async fn test_sqlite(storage: Arc>) where S: Send + Sync + Serializer + 'static, >::Error: Debug + Display, { - check_dialogue(None, Arc::clone(&storage).update_dialogue(1, "ABC".to_owned())).await; - check_dialogue(None, Arc::clone(&storage).update_dialogue(11, "DEF".to_owned())).await; - check_dialogue(None, Arc::clone(&storage).update_dialogue(256, "GHI".to_owned())).await; + test_dialogues!(storage, None, None, None); - // 1 - ABC, 11 - DEF, 256 - GHI + Arc::clone(&storage).update_dialogue(1, "ABC".to_owned()).await.unwrap(); + Arc::clone(&storage).update_dialogue(11, "DEF".to_owned()).await.unwrap(); + Arc::clone(&storage).update_dialogue(256, "GHI".to_owned()).await.unwrap(); - check_dialogue("ABC", Arc::clone(&storage).update_dialogue(1, "JKL".to_owned())).await; - check_dialogue("GHI", Arc::clone(&storage).update_dialogue(256, "MNO".to_owned())).await; + test_dialogues!( + storage, + Some("ABC".to_owned()), + Some("DEF".to_owned()), + Some("GHI".to_owned()) + ); - // 1 - GKL, 11 - DEF, 256 - MNO + Arc::clone(&storage).remove_dialogue(1).await.unwrap(); + Arc::clone(&storage).remove_dialogue(11).await.unwrap(); + Arc::clone(&storage).remove_dialogue(256).await.unwrap(); - check_dialogue("JKL", Arc::clone(&storage).remove_dialogue(1)).await; - check_dialogue("DEF", Arc::clone(&storage).remove_dialogue(11)).await; - check_dialogue("MNO", Arc::clone(&storage).remove_dialogue(256)).await; -} - -async fn check_dialogue( - expected: impl Into>, - actual: impl Future, E>>, -) where - E: Debug, -{ - assert_eq!(expected.into().map(ToOwned::to_owned), actual.await.unwrap()) + test_dialogues!(storage, None, None, None); } From eac67af27a06699102c092223d6c8ff94d34ddc5 Mon Sep 17 00:00:00 2001 From: Temirkhan Myrzamadi Date: Sun, 28 Mar 2021 08:30:05 +0600 Subject: [PATCH 5/9] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b42a9c83..d1e267b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - A storage persistency bug ([issue 304](https://github.com/teloxide/teloxide/issues/304)). + - Log errors from `Storage::{remove_dialogue, update_dialogue}` in `DialogueDispatcher` ([issue 302](https://github.com/teloxide/teloxide/issues/302)). ### Fixed From 68135d004f377ce800a817e482982d6aabdd65ea Mon Sep 17 00:00:00 2001 From: Temirkhan Myrzamadi Date: Sun, 28 Mar 2021 16:30:12 +0600 Subject: [PATCH 6/9] ToOwned -> D: Clone --- CHANGELOG.md | 2 +- src/dispatching/dialogue/storage/in_mem_storage.rs | 2 +- src/dispatching/repls/dialogues_repl.rs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d1e267b8..e0a688d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Do not return a dialogue from `Storage::{remove_dialogue, update_dialogue}`. - - Require `D: ToOwned` in `dialogues_repl` and `InMemStorage`. + - Require `D: Clone` in `dialogues_repl(_with_listener)` and `InMemStorage`. ### Fixed diff --git a/src/dispatching/dialogue/storage/in_mem_storage.rs b/src/dispatching/dialogue/storage/in_mem_storage.rs index 476010d3..e5ca38b9 100644 --- a/src/dispatching/dialogue/storage/in_mem_storage.rs +++ b/src/dispatching/dialogue/storage/in_mem_storage.rs @@ -27,7 +27,7 @@ impl InMemStorage { impl Storage for InMemStorage where - D: ToOwned, + D: Clone, D: Send + 'static, { type Error = std::convert::Infallible; diff --git a/src/dispatching/repls/dialogues_repl.rs b/src/dispatching/repls/dialogues_repl.rs index c46a2dc4..3d2c0f66 100644 --- a/src/dispatching/repls/dialogues_repl.rs +++ b/src/dispatching/repls/dialogues_repl.rs @@ -26,7 +26,7 @@ use teloxide_core::{requests::Requester, types::Message}; pub async fn dialogues_repl<'a, R, H, D, Fut>(requester: R, handler: H) where H: Fn(UpdateWithCx, D) -> Fut + Send + Sync + 'static, - D: ToOwned + Default + Send + 'static, + D: Clone + Default + Send + 'static, Fut: Future> + Send + 'static, R: Requester + Send + Clone + 'static, ::GetUpdatesFaultTolerant: Send, @@ -61,7 +61,7 @@ pub async fn dialogues_repl_with_listener<'a, R, H, D, Fut, L, ListenerE>( listener: L, ) where H: Fn(UpdateWithCx, D) -> Fut + Send + Sync + 'static, - D: ToOwned + Default + Send + 'static, + D: Clone + Default + Send + 'static, Fut: Future> + Send + 'static, L: UpdateListener + Send + 'a, ListenerE: Debug + Send + 'a, From 701dcbcb6864801bbafb35bd01bc88af20584fba Mon Sep 17 00:00:00 2001 From: Hirrolot Date: Sun, 28 Mar 2021 16:37:51 +0600 Subject: [PATCH 7/9] Amalgamate 'Fixed' sections in CHANGELOG.md --- CHANGELOG.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 391b6db5..d34b2fb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,13 +18,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed + - Remove the `reqwest` dependency. It's not needed after the [teloxide-core] integration. - A storage persistency bug ([issue 304](https://github.com/teloxide/teloxide/issues/304)). - Log errors from `Storage::{remove_dialogue, update_dialogue}` in `DialogueDispatcher` ([issue 302](https://github.com/teloxide/teloxide/issues/302)). -### Fixed - -- Remove `reqwest` dependency. It's not needed after the [teloxide-core] integration. - ## [0.4.0] - 2021-03-22 ### Added From ca60e52f4397f9874f4fd8a3a472d5fc2f90a5f2 Mon Sep 17 00:00:00 2001 From: Hirrolot Date: Sun, 28 Mar 2021 22:18:47 -0700 Subject: [PATCH 8/9] Update src/dispatching/dialogue/storage/redis_storage.rs Co-authored-by: Waffle Lapkin --- src/dispatching/dialogue/storage/redis_storage.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dispatching/dialogue/storage/redis_storage.rs b/src/dispatching/dialogue/storage/redis_storage.rs index a89576cf..540fad0e 100644 --- a/src/dispatching/dialogue/storage/redis_storage.rs +++ b/src/dispatching/dialogue/storage/redis_storage.rs @@ -92,7 +92,7 @@ where Box::pin(async move { let dialogue = self.serializer.serialize(&dialogue).map_err(RedisStorageError::SerdeError)?; - self.conn.lock().await.getset::<_, Vec, Option>>(chat_id, dialogue).await?; + self.conn.lock().await.set::<_, Vec, Option>>(chat_id, dialogue).await?; Ok(()) }) } From fff0b670fb0c55b0a3f10eea42986e2bed07a7a9 Mon Sep 17 00:00:00 2001 From: Temirkhan Myrzamadi Date: Mon, 29 Mar 2021 12:20:17 +0600 Subject: [PATCH 9/9] Fix RedisStorage::update_dialogue --- src/dispatching/dialogue/storage/redis_storage.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dispatching/dialogue/storage/redis_storage.rs b/src/dispatching/dialogue/storage/redis_storage.rs index 540fad0e..03313624 100644 --- a/src/dispatching/dialogue/storage/redis_storage.rs +++ b/src/dispatching/dialogue/storage/redis_storage.rs @@ -92,7 +92,7 @@ where Box::pin(async move { let dialogue = self.serializer.serialize(&dialogue).map_err(RedisStorageError::SerdeError)?; - self.conn.lock().await.set::<_, Vec, Option>>(chat_id, dialogue).await?; + self.conn.lock().await.set::<_, Vec, _>(chat_id, dialogue).await?; Ok(()) }) }