From a458c6a76e7d3ac03c585479dff245eabaf8117c Mon Sep 17 00:00:00 2001 From: Kubat <mael.martin31@gmail.com> Date: Fri, 9 Sep 2022 15:51:06 +0200 Subject: [PATCH] AMADEUS: The decode of the lektord response won't panic anymore on invalid input + less spagetti code Signed-off-by: Kubat <mael.martin31@gmail.com> --- .../amadeus-rs/amadeus/src/utils/deamon.rs | 36 ++++----- src/rust/amadeus-rs/lkt-lib/src/connexion.rs | 29 ++++++- src/rust/amadeus-rs/lkt-lib/src/lib.rs | 1 + src/rust/amadeus-rs/lkt-lib/src/macros.rs | 10 +++ src/rust/amadeus-rs/lkt-lib/src/response.rs | 77 +++++++++++-------- src/rust/amadeus-rs/lkt-rs/src/main.rs | 12 ++- 6 files changed, 103 insertions(+), 62 deletions(-) create mode 100644 src/rust/amadeus-rs/lkt-lib/src/macros.rs diff --git a/src/rust/amadeus-rs/amadeus/src/utils/deamon.rs b/src/rust/amadeus-rs/amadeus/src/utils/deamon.rs index fa774c68..481f06da 100644 --- a/src/rust/amadeus-rs/amadeus/src/utils/deamon.rs +++ b/src/rust/amadeus-rs/amadeus/src/utils/deamon.rs @@ -1,6 +1,6 @@ #![allow(dead_code)] -use lkt_lib::{query::LektorQuery, response::LektorFormatedResponse}; +use lkt_lib::{query::*, response::LektorFormatedResponse}; use log::error; use std::{ cell::Cell, @@ -114,13 +114,8 @@ impl Deamon for CommandDeamon { continue; } }; - match lkt_lib::response::LektorFormatedResponse::try_from(res) { - Ok(response) => { - if let Err(e) = responses_send.send(response) { - error!("failed to send response to amadeus: {e}") - } - } - Err(e) => error!("failed to format the response: {e}"), + if let Err(e) = responses_send.send(res) { + error!("failed to send response to amadeus: {e}") } } Err(e) => error!("failed to get command from amadeus: {e}"), @@ -158,33 +153,34 @@ impl Deamon for StatusDeamon { return_when_flagged!(quit_deamon, joined_deamon); let status = { - let res = match connexion.send_query(lkt_lib::query::LektorQuery::PlaybackStatus) { + let mut res = match connexion.send_query(LektorQuery::PlaybackStatus) { Ok(res) => res, Err(e) => { error!("failed to send the playback status command to lektor: {e}"); continue; } }; - match lkt_lib::response::LektorFormatedResponse::try_from(res) { - Err(e) => panic!("{e}"), - Ok(mut response) => lkt_lib::response::PlaybackStatus::consume(&mut response), + match lkt_lib::response::PlaybackStatus::consume(&mut res) { + Err(e) => { + error!("failed to build response from formated response: {e}"); + continue; + } + Ok(res) => res, } }; let current = if status.state != lkt_lib::types::LektorState::Stopped { - let res = match connexion.send_query(lkt_lib::query::LektorQuery::CurrentKara) { + let mut res = match connexion.send_query(lkt_lib::query::LektorQuery::CurrentKara) { Ok(res) => res, Err(e) => { error!("failed to send the current kara command to lektor: {e}",); continue; } }; - match lkt_lib::response::LektorFormatedResponse::try_from(res) { - Ok(mut response) => { - Some(lkt_lib::response::CurrentKara::consume(&mut response)) - } - Err(e) => { - error!("failed to format response from lektor: {e}"); + match lkt_lib::response::CurrentKara::consume(&mut res) { + Ok(res) => Some(res), + Err(err) => { + error!("failed to build response from formated response: {err}"); None } } @@ -193,7 +189,7 @@ impl Deamon for StatusDeamon { }; if let Err(e) = responses_send.send((status, current)) { - error!("Failed to send a status response to amadeus: {}", e); + error!("Failed to send a status response to amadeus: {e}"); } }); diff --git a/src/rust/amadeus-rs/lkt-lib/src/connexion.rs b/src/rust/amadeus-rs/lkt-lib/src/connexion.rs index 552a992a..8d88132e 100644 --- a/src/rust/amadeus-rs/lkt-lib/src/connexion.rs +++ b/src/rust/amadeus-rs/lkt-lib/src/connexion.rs @@ -1,7 +1,9 @@ //! Create a connexion to a lektord instande then send commands and recieve data //! from the server. -use crate::{constants, query::LektorQuery, query::LektorQueryLineType}; +use crate::{ + constants, query::LektorQuery, query::LektorQueryLineType, response::LektorFormatedResponse, +}; use log::error; use std::{ io::{self, BufRead, BufReader, Write}, @@ -9,6 +11,21 @@ use std::{ str::FromStr, }; +pub enum LektorQueryError { + IO(io::Error), + Other(String), +} + +impl std::fmt::Display for LektorQueryError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + use LektorQueryError::*; + match self { + IO(io) => write!(f, "lektor query error io: {io}"), + Other(other) => write!(f, "lektor query error: {other}"), + } + } +} + pub struct LektorConnexion { stream: TcpStream, pub version: String, @@ -54,10 +71,14 @@ impl LektorConnexion { } } - pub fn send_query(&mut self, query: LektorQuery) -> io::Result<Vec<String>> { + pub fn send_query( + &mut self, + query: LektorQuery, + ) -> Result<LektorFormatedResponse, LektorQueryError> { let mut res: Vec<String> = Vec::new(); - self.send_query_inner(query, &mut res)?; - Ok(res) + self.send_query_inner(query, &mut res) + .map_err(LektorQueryError::IO)?; + LektorFormatedResponse::try_from(res).map_err(LektorQueryError::Other) } fn send_query_inner( diff --git a/src/rust/amadeus-rs/lkt-lib/src/lib.rs b/src/rust/amadeus-rs/lkt-lib/src/lib.rs index 100741fd..7eeb178c 100644 --- a/src/rust/amadeus-rs/lkt-lib/src/lib.rs +++ b/src/rust/amadeus-rs/lkt-lib/src/lib.rs @@ -3,6 +3,7 @@ pub mod connexion; mod constants; +mod macros; pub mod query; pub mod response; pub mod types; diff --git a/src/rust/amadeus-rs/lkt-lib/src/macros.rs b/src/rust/amadeus-rs/lkt-lib/src/macros.rs new file mode 100644 index 00000000..88ba1211 --- /dev/null +++ b/src/rust/amadeus-rs/lkt-lib/src/macros.rs @@ -0,0 +1,10 @@ +#[macro_export] +macro_rules! then_some { + ($cond: expr => $some: expr) => { + if $cond { + Some($some) + } else { + None + } + }; +} diff --git a/src/rust/amadeus-rs/lkt-lib/src/response.rs b/src/rust/amadeus-rs/lkt-lib/src/response.rs index bbedf6d4..ccd2b826 100644 --- a/src/rust/amadeus-rs/lkt-lib/src/response.rs +++ b/src/rust/amadeus-rs/lkt-lib/src/response.rs @@ -1,32 +1,47 @@ //! Contains types for typed response. +use crate::then_some; use crate::types::LektorState; -use std::collections::HashMap; #[derive(Debug)] pub struct LektorFormatedResponse { - content: HashMap<String, String>, + content: Vec<(String, String)>, } impl LektorFormatedResponse { - pub fn pop(&mut self, key: &str) -> String { - match self.content.remove(key) { - Some(x) => x, - None => panic!("Failed to find a value with the key {key} in the formated response"), + pub fn pop(&mut self, key: &str) -> Result<String, String> { + match self + .content + .iter() + .enumerate() + .find_map(|(index, (what, _))| then_some!(what == key => index)) + { + Some(index) => Ok(self.content.remove(index).1), + None => Err(format!("no key {key} was found in formated response")), } } } +impl IntoIterator for LektorFormatedResponse { + type Item = (String, String); + type IntoIter = + <std::vec::Vec<(std::string::String, std::string::String)> as IntoIterator>::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.content.into_iter() + } +} + impl TryFrom<Vec<String>> for LektorFormatedResponse { type Error = String; fn try_from(vec: Vec<String>) -> Result<Self, Self::Error> { - let mut content = HashMap::with_capacity(vec.len()); + let mut content = Vec::with_capacity(vec.len()); for line in vec { let key_pair: Vec<&str> = line.splitn(2, ':').collect(); match key_pair[..] { [key, value] => { - content.insert(key.trim().to_lowercase(), value.trim().to_string()); + content.push((key.trim().to_lowercase(), value.trim().to_string())); } _ => return Err(format!("Invalid line for formated response: {}", line)), } @@ -58,38 +73,38 @@ pub struct PlaybackStatus { } impl PlaybackStatus { - pub fn consume(response: &mut LektorFormatedResponse) -> Self { + pub fn consume(response: &mut LektorFormatedResponse) -> Result<Self, String> { let mut ret = Self { - elapsed: response.pop("elapsed").parse::<usize>().unwrap_or(0), - songid: match response.pop("songid").parse::<isize>() { + elapsed: response.pop("elapsed")?.parse::<usize>().unwrap_or(0), + songid: match response.pop("songid")?.parse::<isize>() { Ok(x) if x <= 0 => None, Ok(x) => Some(x as usize), Err(_) => None, }, - song: match response.pop("song").parse::<usize>() { + song: match response.pop("song")?.parse::<usize>() { Ok(x) => Some(x), Err(_) => None, }, volume: response - .pop("volume") + .pop("volume")? .parse::<usize>() .unwrap() .clamp(0, 100), - duration: response.pop("duration").parse::<usize>().unwrap(), - updating_db: response.pop("updating_db").parse::<usize>().unwrap(), - playlistlength: response.pop("playlistlength").parse::<usize>().unwrap(), - random: response.pop("random").parse::<usize>().unwrap() != 0, - consume: response.pop("consume").parse::<usize>().unwrap() != 0, - single: response.pop("single").parse::<usize>().unwrap() != 0, - repeat: response.pop("repeat").parse::<usize>().unwrap() != 0, + duration: response.pop("duration")?.parse::<usize>().unwrap(), + updating_db: response.pop("updating_db")?.parse::<usize>().unwrap(), + playlistlength: response.pop("playlistlength")?.parse::<usize>().unwrap(), + random: response.pop("random")?.parse::<usize>().unwrap() != 0, + consume: response.pop("consume")?.parse::<usize>().unwrap() != 0, + single: response.pop("single")?.parse::<usize>().unwrap() != 0, + repeat: response.pop("repeat")?.parse::<usize>().unwrap() != 0, state: LektorState::Stopped, }; - ret.state = match &response.pop("state")[..] { + ret.state = match &response.pop("state")?[..] { "play" => LektorState::Play(ret.songid.unwrap()), "pause" => LektorState::Pause(ret.songid.unwrap()), _ => LektorState::Stopped, }; - ret + Ok(ret) } } @@ -105,8 +120,8 @@ pub struct CurrentKara { } impl CurrentKara { - pub fn consume(response: &mut LektorFormatedResponse) -> Self { - let song_type_number = response.pop("type"); + pub fn consume(response: &mut LektorFormatedResponse) -> Result<Self, String> { + let song_type_number = response.pop("type")?; let (song_type, song_number) = match song_type_number.find(char::is_numeric) { Some(index) => ( (&song_type_number[..index]).to_owned(), @@ -118,14 +133,14 @@ impl CurrentKara { None => panic!("Oupsy"), }; - Self { - title: response.pop("title"), - author: response.pop("author"), - source: response.pop("source"), - category: response.pop("category"), - language: response.pop("language"), + Ok(Self { + title: response.pop("title")?, + author: response.pop("author")?, + source: response.pop("source")?, + category: response.pop("category")?, + language: response.pop("language")?, song_type, song_number, - } + }) } } diff --git a/src/rust/amadeus-rs/lkt-rs/src/main.rs b/src/rust/amadeus-rs/lkt-rs/src/main.rs index 3e939515..26faabce 100644 --- a/src/rust/amadeus-rs/lkt-rs/src/main.rs +++ b/src/rust/amadeus-rs/lkt-rs/src/main.rs @@ -4,21 +4,19 @@ fn main() { let mut lektor = LektorConnexion::new("localhost".to_owned(), 6600).unwrap(); if lektor.send_query(LektorQuery::Ping).is_ok() {} - if let Ok(res) = lektor.send_query(LektorQuery::CurrentKara) { - let mut response = response::LektorFormatedResponse::try_from(res).unwrap(); + if let Ok(mut response) = lektor.send_query(LektorQuery::CurrentKara) { let current_kara = response::CurrentKara::consume(&mut response); println!("CURRENT {:?}", current_kara); } - if let Ok(res) = lektor.send_query(LektorQuery::PlaybackStatus) { - let mut response = response::LektorFormatedResponse::try_from(res).unwrap(); + if let Ok(mut response) = lektor.send_query(LektorQuery::PlaybackStatus) { let playback_status = response::PlaybackStatus::consume(&mut response); println!("PLAYBACK-STATUS {:?}", playback_status); } - if let Ok(res) = lektor.send_query(LektorQuery::ListAllPlaylists) { - for item in res { - println!("ALL PLAYLISTS {}", item); + if let Ok(response) = lektor.send_query(LektorQuery::ListAllPlaylists) { + for (what, item) in response.into_iter() { + println!("ALL PLAYLISTS {what}:{item}"); } } } -- GitLab