From d55548d063f73bdb706c99499cba3f9a34852a1e Mon Sep 17 00:00:00 2001 From: Jaroslav Beran Date: Wed, 13 Mar 2024 18:28:52 +0100 Subject: [PATCH 1/2] Rpc{Message,Value}: Simplify methods implementation Replace manual implementation of common constructs by provided functions. --- src/rpc.rs | 2 +- src/rpcmessage.rs | 31 ++++++------------------------- src/rpcvalue.rs | 27 ++++++--------------------- 3 files changed, 13 insertions(+), 47 deletions(-) diff --git a/src/rpc.rs b/src/rpc.rs index 347e961..1fcd21f 100644 --- a/src/rpc.rs +++ b/src/rpc.rs @@ -85,7 +85,7 @@ impl SubscriptionPattern { Self::new(paths, methods) } pub fn from_subscription(subscription: &Subscription) -> crate::Result { - Ok(Self::new(&subscription.paths, &subscription.methods)?) + Self::new(&subscription.paths, &subscription.methods) } pub fn to_rpcvalue(&self) -> RpcValue { self.as_subscription().to_rpcvalue() diff --git a/src/rpcmessage.rs b/src/rpcmessage.rs index bfabdd6..4e76c10 100644 --- a/src/rpcmessage.rs +++ b/src/rpcmessage.rs @@ -83,10 +83,7 @@ impl RpcMessage { self } pub fn error(&self) -> Option { - if let Some(rv) = self.key(Key::Error as i32) { - return RpcError::from_rpcvalue(rv) - } - None + self.key(Key::Error as i32).and_then(RpcError::from_rpcvalue) } pub fn set_error(&mut self, err: RpcError) -> &mut Self { self.set_key(Key::Error, Some(err.to_rpcvalue())); @@ -119,8 +116,7 @@ impl RpcMessage { } fn key(&self, key: i32) -> Option<&RpcValue> { if let Value::IMap(m) = self.0.value() { - let v = m.get(&key); - return v; + return m.get(&key); } None } @@ -201,20 +197,13 @@ pub trait RpcMessageMetaTags { self.tag(Tag::RequestId as i32).map(|rv| rv.as_i64()) } fn try_request_id(&self) -> crate::Result { - match self.request_id() { - None => Err("Request id not exists.".into()), - Some(id) => Ok(id), - } + self.request_id().ok_or_else(|| "Request id not exists.".into()) } fn set_request_id(&mut self, id: RqId) -> &mut Self::Target { self.set_tag(Tag::RequestId as i32, Some(RpcValue::from(id))) } fn shv_path(&self) -> Option<&str> { - let t = self.tag(Tag::ShvPath as i32); - match t { - None => None, - Some(rv) => Some(rv.as_str()), - } + self.tag(Tag::ShvPath as i32).map(RpcValue::as_str) } //fn shv_path_or_empty(&self) -> &str { // self.shv_path().unwrap_or("") @@ -223,21 +212,13 @@ pub trait RpcMessageMetaTags { self.set_tag(Tag::ShvPath as i32, Some(RpcValue::from(shv_path))) } fn method(&self) -> Option<&str> { - let t = self.tag(Tag::Method as i32); - match t { - None => None, - Some(rv) => Some(rv.as_str()), - } + self.tag(Tag::Method as i32).map(RpcValue::as_str) } fn set_method(&mut self, method: &str) -> &mut Self::Target { self.set_tag(Tag::Method as i32, Some(RpcValue::from(method))) } fn access(&self) -> Option<&str> { - let t = self.tag(Tag::Access as i32); - match t { - None => None, - Some(rv) => Some(rv.as_str()), - } + self.tag(Tag::Access as i32).map(RpcValue::as_str) } fn set_access(&mut self, grant: &str) -> &mut Self::Target { self.set_tag(Tag::Access as i32, Some(RpcValue::from(grant))) diff --git a/src/rpcvalue.rs b/src/rpcvalue.rs index ba22389..39c1df4 100644 --- a/src/rpcvalue.rs +++ b/src/rpcvalue.rs @@ -214,21 +214,12 @@ impl RpcValue { self.meta.is_some() } pub fn meta(&self) -> &MetaMap { - match &self.meta { - Some(mm) => { - mm - } - None => { - let mm = EMPTY_METAMAP.get_or_init(MetaMap::new); - mm - } - } + self.meta.as_ref().map_or_else( + || EMPTY_METAMAP.get_or_init(MetaMap::new), + >::as_ref) } pub fn meta_mut(&mut self) -> Option<&mut MetaMap> { - match &mut self.meta { - Some(mm) => Some(mm.as_mut()), - _ => None, - } + self.meta.as_mut().map(>::as_mut) } pub fn clear_meta(&mut self) { self.meta = None; @@ -371,10 +362,7 @@ impl RpcValue { pub fn to_cpon(&self) -> String { self.to_cpon_indented("").unwrap_or("".to_string()) } pub fn to_cpon_indented(&self, indent: &str) -> crate::Result { let buff = self.to_cpon_bytes_indented(indent.as_bytes())?; - match String::from_utf8(buff) { - Ok(s) => Ok(s), - Err(e) => Err(e.into()), - } + String::from_utf8(buff).map_err(|e| e.into()) } pub fn to_cpon_bytes_indented(&self, indent: &[u8]) -> crate::Result> { let mut buff: Vec = Vec::new(); @@ -389,10 +377,7 @@ impl RpcValue { let mut buff: Vec = Vec::new(); let mut wr = ChainPackWriter::new(&mut buff); let r = wr.write(self); - match r { - Ok(_) => buff, - Err(_) => Vec::new(), - } + r.map_or_else(|_| Vec::new(), |_| buff) } pub fn from_cpon(s: &str) -> ReadResult { From 4d77ef75d645d681b7aed0e87cce109f55c32508 Mon Sep 17 00:00:00 2001 From: Jaroslav Beran Date: Wed, 13 Mar 2024 18:32:43 +0100 Subject: [PATCH 2/2] Fix warnings generated by clippy shvcall: Avoid redundant references in `send_request` arguments Bypass clippy checks on uppercase constants in the chainpack module and allow Access::from_str to return Option instead of Result as in the definition of from_str in FromStr trait. --- src/bin/shvcall.rs | 2 +- src/chainpack.rs | 1 + src/metamethod.rs | 2 ++ 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/bin/shvcall.rs b/src/bin/shvcall.rs index 1d5ff8b..ad61f1d 100644 --- a/src/bin/shvcall.rs +++ b/src/bin/shvcall.rs @@ -272,7 +272,7 @@ async fn make_call(url: &Url, opts: &Opts) -> Result { match parse_line(line) { Ok((path, method, param)) => { let rqid = - send_request(&mut *frame_writer, &path, &method, ¶m) + send_request(&mut *frame_writer, path, method, param) .await?; loop { let resp = frame_reader.receive_message().await?; diff --git a/src/chainpack.rs b/src/chainpack.rs index 20417d8..ff83ae2 100644 --- a/src/chainpack.rs +++ b/src/chainpack.rs @@ -6,6 +6,7 @@ use std::collections::BTreeMap; use crate::reader::{Reader, ByteReader, ReadError, ReadErrorReason}; use crate::rpcvalue::{Map, IMap}; +#[allow(clippy::upper_case_acronyms)] #[warn(non_camel_case_types)] #[allow(dead_code)] pub(crate) enum PackingSchema { diff --git a/src/metamethod.rs b/src/metamethod.rs index a528a4c..2d34d21 100644 --- a/src/metamethod.rs +++ b/src/metamethod.rs @@ -28,6 +28,8 @@ impl From for Flag { #[derive(Debug, Copy, Clone, PartialEq, PartialOrd)] pub enum Access { Browse = 0, Read, Write, Command, Config, Service, SuperService, Developer, Superuser } impl Access { + // It makes sense to return Option rather than Result as the `FromStr` trait does. + #[allow(clippy::should_implement_trait)] pub fn from_str(value: &str) -> Option { match value { "bws" => Some(Access::Browse),