From 4c4f82eeed921377bd0355fb95314d27d31f6592 Mon Sep 17 00:00:00 2001 From: Ilya Bizyaev Date: Fri, 28 Feb 2025 21:22:26 +0100 Subject: [PATCH] Make edit_message_text return ApiError::MessageNotModified Editing message text to the same value is an easy mistake to make, so I would like to test my code against it. I would also like to receive the error properly as an ApiError enum variant, so this commit introduces a way for teloxide_tests actions to return standard error values of Telegram Bot API that teloxide can actually parse. In the future, other usages of check_if_message_exists and ErrorBadRequest should also be replaced with ApiErrors where possible. --- .../src/server/routes/edit_message_text.rs | 20 +++++-- teloxide_tests/src/server/routes/mod.rs | 47 ++++++++++++++++- teloxide_tests/src/tests.rs | 52 +++++++++++++++---- 3 files changed, 103 insertions(+), 16 deletions(-) diff --git a/teloxide_tests/src/server/routes/edit_message_text.rs b/teloxide_tests/src/server/routes/edit_message_text.rs index 959722b..bfeeccf 100644 --- a/teloxide_tests/src/server/routes/edit_message_text.rs +++ b/teloxide_tests/src/server/routes/edit_message_text.rs @@ -1,10 +1,13 @@ use std::sync::Mutex; -use actix_web::{error::ErrorBadRequest, web, Responder}; +use actix_web::{error::ErrorBadRequest, web, Responder, ResponseError}; use serde::Deserialize; -use teloxide::types::{LinkPreviewOptions, MessageEntity, ParseMode, ReplyMarkup}; +use teloxide::{ + types::{LinkPreviewOptions, MessageEntity, ParseMode, ReplyMarkup}, + ApiError, +}; -use super::{check_if_message_exists, BodyChatId}; +use super::{BodyChatId, BotApiError}; use crate::{ server::{routes::make_telegram_result, EditedMessageText}, state::State, @@ -33,7 +36,16 @@ pub async fn edit_message_text( ) { (Some(_), Some(message_id), None) => { let mut lock = state.lock().unwrap(); - check_if_message_exists!(lock, message_id); + let Some(old_message) = lock.messages.get_message(message_id) else { + return BotApiError::new(ApiError::MessageToEditNotFound).error_response(); + }; + + let old_reply_markup = old_message + .reply_markup() + .map(|kb| ReplyMarkup::InlineKeyboard(kb.clone())); + if old_message.text() == Some(&body.text) && old_reply_markup == body.reply_markup { + return BotApiError::new(ApiError::MessageNotModified).error_response(); + } lock.messages .edit_message_field(message_id, "text", body.text.clone()); diff --git a/teloxide_tests/src/server/routes/mod.rs b/teloxide_tests/src/server/routes/mod.rs index 574aa41..51188bf 100644 --- a/teloxide_tests/src/server/routes/mod.rs +++ b/teloxide_tests/src/server/routes/mod.rs @@ -1,11 +1,14 @@ use std::{collections::HashMap, str::from_utf8}; -use actix_web::HttpResponse; +use actix_web::{error::ResponseError, http::header::ContentType, HttpResponse}; use futures_util::{stream::StreamExt as _, TryStreamExt}; use rand::distributions::{Alphanumeric, DistString}; use serde::{Deserialize, Serialize}; use serde_json::json; -use teloxide::types::{Chat, MessageEntity, ParseMode, Seconds}; +use teloxide::{ + types::{Chat, MessageEntity, ParseMode, Seconds}, + ApiError, +}; use crate::dataset::{MockPrivateChat, MockSupergroupChat}; @@ -167,6 +170,46 @@ pub trait SerializeRawFields { Self: Sized; } +#[derive(Debug, Serialize)] +struct TelegramResponse { + ok: bool, + description: String, +} + +#[derive(Debug, PartialEq, Hash, Eq, Clone)] +struct BotApiError { + error: ApiError, +} + +impl BotApiError { + pub fn new(error: ApiError) -> Self { + Self { error } + } +} + +impl std::fmt::Display for BotApiError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.error.fmt(f) + } +} + +impl ResponseError for BotApiError { + fn status_code(&self) -> actix_web::http::StatusCode { + actix_web::http::StatusCode::BAD_REQUEST + } + + fn error_response(&self) -> HttpResponse { + let response = TelegramResponse { + ok: false, + description: self.error.to_string(), + }; + HttpResponse::build(self.status_code()) + .insert_header(ContentType::json()) + .body(serde_json::to_string(&response).unwrap()) + } +} + +// TODO: replace usages with appropriate error values from teloxide::ApiError. macro_rules! check_if_message_exists { ($lock:expr, $msg_id:expr) => { if $lock.messages.get_message($msg_id).is_none() { diff --git a/teloxide_tests/src/tests.rs b/teloxide_tests/src/tests.rs index 46f3d22..cdd15d6 100644 --- a/teloxide_tests/src/tests.rs +++ b/teloxide_tests/src/tests.rs @@ -1,6 +1,6 @@ use std::{ fmt::Display, - sync::{atomic::AtomicBool, Arc}, + sync::{Arc, RwLock}, thread, }; @@ -185,6 +185,8 @@ pub enum AllCommands { #[command()] Edit, #[command()] + EditUnchanged, + #[command()] Delete, #[command()] DeleteBatch, @@ -264,6 +266,11 @@ async fn handler( .link_preview_options(link_preview_options) .await?; } + AllCommands::EditUnchanged => { + bot.edit_message_text(msg.chat.id, sent_message.id, msg.text().unwrap()) + .link_preview_options(link_preview_options) + .await?; + } AllCommands::Delete => { bot.delete_message(msg.chat.id, sent_message.id).await?; } @@ -554,22 +561,33 @@ async fn test_panic() { } pub struct MyErrorHandler { - some_bool: Arc, + errors: Arc>>, +} + +impl MyErrorHandler { + pub fn new() -> Self { + Self { + errors: Arc::new(RwLock::new(vec![])), + } + } + + pub fn errors(&self) -> Vec { + self.errors.read().unwrap().clone() + } } impl ErrorHandler for MyErrorHandler where E: std::fmt::Debug + Display + 'static + Sync + Send, { - fn handle_error(self: Arc, _error: E) -> BoxFuture<'static, ()> { + fn handle_error(self: Arc, error: E) -> BoxFuture<'static, ()> { thread::spawn(|| { respond_to_error(); }) .join() .unwrap(); - self.some_bool - .swap(true, std::sync::atomic::Ordering::SeqCst); + self.errors.write().unwrap().push(format!("{error:?}")); Box::pin(async {}) } } @@ -585,14 +603,14 @@ async fn respond_to_error() { #[tokio::test] async fn test_error_handler() { let mut bot = MockBot::new(MockMessageText::new().text("/panic"), get_schema()); - let some_bool = Arc::new(AtomicBool::new(false)); + let error_handler = Arc::new(MyErrorHandler::new()); + bot.error_handler(error_handler.clone()); - bot.error_handler(Arc::new(MyErrorHandler { - some_bool: some_bool.clone(), - })); bot.dispatch_and_check_last_text("Error detected!").await; - assert!(some_bool.load(std::sync::atomic::Ordering::SeqCst)); + let errors = error_handler.errors(); + assert_eq!(errors.len(), 1); + assert!(errors[0].contains("Message not found")); } #[tokio::test] @@ -981,6 +999,20 @@ async fn test_edit_message() { ); } +#[tokio::test] +async fn test_edit_message_unchanged() { + let mut bot = MockBot::new(MockMessageText::new().text("/editunchanged"), get_schema()); + let error_handler = Arc::new(MyErrorHandler::new()); + bot.error_handler(error_handler.clone()); + + bot.dispatch().await; + + assert!(bot.get_responses().edited_messages_text.is_empty()); + let errors = error_handler.errors(); + assert_eq!(errors.len(), 1); + assert_eq!(errors[0], "Api(MessageNotModified)"); +} + #[tokio::test] async fn test_edit_caption() { let mut bot = MockBot::new(MockMessageText::new().text("/editcaption"), get_schema());