From df6086103523838cd3a0b492602780a3f81454a7 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Wed, 17 Nov 2021 19:16:36 +0100 Subject: [PATCH 01/21] feat: override `method` subscription notif --- examples/proc_macro.rs | 2 +- proc-macros/src/render_server.rs | 21 ++++++++++---- proc-macros/src/rpc_macro.rs | 36 +++++++++++++++++++++-- proc-macros/tests/ui/correct/basic.rs | 12 ++++++++ utils/src/server/rpc_module.rs | 42 ++++++++++++++++++++++++++- 5 files changed, 103 insertions(+), 10 deletions(-) diff --git a/examples/proc_macro.rs b/examples/proc_macro.rs index 11825834b3..868d7e0c85 100644 --- a/examples/proc_macro.rs +++ b/examples/proc_macro.rs @@ -45,7 +45,7 @@ where async fn storage_keys(&self, storage_key: StorageKey, hash: Option) -> Result, Error>; /// Subscription that takes a `StorageKey` as input and produces a `Vec`. - #[subscription(name = "subscribeStorage", item = Vec)] + #[subscription(name = "subscribeStorage", override_notif_method = "bar", item = Vec)] fn subscribe_storage(&self, keys: Option>) -> Result<(), Error>; } diff --git a/proc-macros/src/render_server.rs b/proc-macros/src/render_server.rs index 86b96cb757..957a1dd8c5 100644 --- a/proc-macros/src/render_server.rs +++ b/proc-macros/src/render_server.rs @@ -174,6 +174,8 @@ impl RpcDescription { let rust_method_name = &sub.signature.sig.ident; // Name of the RPC method to subscribe to (e.g. `foo_sub`). let rpc_sub_name = self.rpc_identifier(&sub.name); + // + let maybe_custom_notif = self.optional_rpc_identifier(sub.override_notif_method.as_deref()); // Name of the RPC method to unsubscribe (e.g. `foo_sub`). let rpc_unsub_name = self.rpc_identifier(&sub.unsubscribe); // `parsing` is the code associated with parsing structure from the @@ -184,12 +186,21 @@ impl RpcDescription { check_name(&rpc_sub_name, rust_method_name.span()); check_name(&rpc_unsub_name, rust_method_name.span()); - handle_register_result(quote! { - rpc.register_subscription(#rpc_sub_name, #rpc_unsub_name, |params, sink, context| { - #parsing - context.as_ref().#rust_method_name(sink, #params_seq) + if maybe_custom_notif.is_some() { + handle_register_result(quote! { + rpc.register_subscription_with_custom_notif(#rpc_sub_name, #maybe_custom_notif, #rpc_unsub_name, |params, sink, context| { + #parsing + context.as_ref().#rust_method_name(sink, #params_seq) + }) }) - }) + } else { + handle_register_result(quote! { + rpc.register_subscription(#rpc_sub_name, #rpc_unsub_name, |params, sink, context| { + #parsing + context.as_ref().#rust_method_name(sink, #params_seq) + }) + }) + } }) .collect::>(); diff --git a/proc-macros/src/rpc_macro.rs b/proc-macros/src/rpc_macro.rs index be251cae00..9f1dd328da 100644 --- a/proc-macros/src/rpc_macro.rs +++ b/proc-macros/src/rpc_macro.rs @@ -95,6 +95,9 @@ impl RpcMethod { #[derive(Debug, Clone)] pub struct RpcSubscription { pub name: String, + /// By default the server will send out subscriptions `method` in `name` above + /// but it's possible to override it with another name that this field does. + pub override_notif_method: Option, pub docs: TokenStream2, pub unsubscribe: String, pub params: Vec<(syn::PatIdent, syn::Type)>, @@ -107,11 +110,15 @@ pub struct RpcSubscription { impl RpcSubscription { pub fn from_item(attr: syn::Attribute, mut sub: syn::TraitItemMethod) -> syn::Result { - let [aliases, item, name, param_kind, unsubscribe_aliases] = - AttributeMeta::parse(attr)?.retain(["aliases", "item", "name", "param_kind", "unsubscribe_aliases"])?; + let [aliases, item, name, param_kind, unsubscribe_aliases, override_notif_method] = AttributeMeta::parse(attr)? + .retain(["aliases", "item", "name", "param_kind", "unsubscribe_aliases", "override_notif_method"])?; let aliases = parse_aliases(aliases)?; let name = name?.string()?; + let override_notif_method = match override_notif_method { + Ok(arg) => Some(arg.string()?), + Err(_) => None, + }; let item = item?.value()?; let param_kind = parse_param_kind(param_kind)?; let unsubscribe_aliases = parse_aliases(unsubscribe_aliases)?; @@ -135,7 +142,18 @@ impl RpcSubscription { // We've analyzed attributes and don't need them anymore. sub.attrs.clear(); - Ok(Self { name, unsubscribe, unsubscribe_aliases, params, param_kind, item, signature: sub, aliases, docs }) + Ok(Self { + name, + override_notif_method, + unsubscribe, + unsubscribe_aliases, + params, + param_kind, + item, + signature: sub, + aliases, + docs, + }) } } @@ -284,6 +302,18 @@ impl RpcDescription { Cow::Borrowed(method) } } + + /// Based on the namespace, renders the full name of the RPC method/subscription. + /// Examples: + /// For namespace `foo` and method `makeSpam`, result will be `foo_makeSpam`. + /// For no namespace and method `makeSpam` it will be just `makeSpam. + pub(crate) fn optional_rpc_identifier<'a>(&self, method: Option<&'a str>) -> Option> { + if let Some(method) = method { + Some(self.rpc_identifier(method)) + } else { + None + } + } } fn parse_aliases(arg: Result) -> syn::Result> { diff --git a/proc-macros/tests/ui/correct/basic.rs b/proc-macros/tests/ui/correct/basic.rs index c8052ce288..71414b3c4b 100644 --- a/proc-macros/tests/ui/correct/basic.rs +++ b/proc-macros/tests/ui/correct/basic.rs @@ -31,6 +31,10 @@ pub trait Rpc { #[subscription(name = "echo", aliases = ["ECHO"], item = u32, unsubscribe_aliases = ["NotInterested", "listenNoMore"])] fn sub_with_params(&self, val: u32) -> RpcResult<()>; + + // This will send notifications to the client with `method=subscribe_override` + #[subscription(name = "subscribe_method", override_notif_method = "subscribe_override", item = u32)] + fn sub_with_override_notif_method(&self) -> RpcResult<()>; } pub struct RpcServerImpl; @@ -68,6 +72,10 @@ impl RpcServer for RpcServerImpl { sink.send(&val)?; sink.send(&val) } + + fn sub_with_override_notif_method(&self, mut sink: SubscriptionSink) -> RpcResult<()> { + sink.send(&1) + } } pub async fn websocket_server() -> SocketAddr { @@ -102,4 +110,8 @@ async fn main() { assert_eq!(first_recv, Some("Response_A".to_string())); let second_recv = sub.next().await.unwrap(); assert_eq!(second_recv, Some("Response_B".to_string())); + + let mut sub = client.sub_with_override_notif_method().await.unwrap(); + let recv = sub.next().await.unwrap(); + assert_eq!(recv, Some(1)); } diff --git a/utils/src/server/rpc_module.rs b/utils/src/server/rpc_module.rs index fe2aadfb64..c80f74a190 100644 --- a/utils/src/server/rpc_module.rs +++ b/utils/src/server/rpc_module.rs @@ -542,6 +542,41 @@ impl RpcModule { unsubscribe_method_name: &'static str, callback: F, ) -> Result<(), Error> + where + Context: Send + Sync + 'static, + F: Fn(Params, SubscriptionSink, Arc) -> Result<(), Error> + Send + Sync + 'static, + { + self.register_subscription_inner(subscribe_method_name, None, unsubscribe_method_name, callback) + } + + /// Similar to [`RpcModule::register_subscription`] but allows sending notifications to another method + /// than the actual subscription method name. + pub fn register_subscription_with_custom_notif( + &mut self, + subscribe_method_name: &'static str, + custom_notif_method_name: &'static str, + unsubscribe_method_name: &'static str, + callback: F, + ) -> Result<(), Error> + where + Context: Send + Sync + 'static, + F: Fn(Params, SubscriptionSink, Arc) -> Result<(), Error> + Send + Sync + 'static, + { + self.register_subscription_inner( + subscribe_method_name, + Some(custom_notif_method_name), + unsubscribe_method_name, + callback, + ) + } + + fn register_subscription_inner( + &mut self, + subscribe_method_name: &'static str, + custom_notif_method_name: Option<&'static str>, + unsubscribe_method_name: &'static str, + callback: F, + ) -> Result<(), Error> where Context: Send + Sync + 'static, F: Fn(Params, SubscriptionSink, Arc) -> Result<(), Error> + Send + Sync + 'static, @@ -573,9 +608,14 @@ impl RpcModule { send_response(id.clone(), method_sink, sub_id, max_response_size); + let method = match custom_notif_method_name { + Some(m) => m, + None => subscribe_method_name, + }; + let sink = SubscriptionSink { inner: method_sink.clone(), - method: subscribe_method_name, + method, subscribers: subscribers.clone(), uniq_sub: SubscriptionKey { conn_id, sub_id }, is_connected: Some(conn_tx), From d6eb4ea5ea31015d2096062807fb7b9d49a14b5b Mon Sep 17 00:00:00 2001 From: Maciej Hirsz <1096222+maciejhirsz@users.noreply.github.com> Date: Thu, 18 Nov 2021 13:40:49 +0100 Subject: [PATCH 02/21] Arrow syntax for overwrites (#569) --- proc-macros/src/attributes.rs | 39 +++++++++++++++++++++------ proc-macros/src/rpc_macro.rs | 14 +++++----- proc-macros/tests/ui/correct/basic.rs | 2 +- 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/proc-macros/src/attributes.rs b/proc-macros/src/attributes.rs index 02da827efa..7e8ee40c67 100644 --- a/proc-macros/src/attributes.rs +++ b/proc-macros/src/attributes.rs @@ -28,7 +28,7 @@ use proc_macro2::{Span, TokenStream as TokenStream2, TokenTree}; use std::{fmt, iter}; use syn::parse::{Parse, ParseStream, Parser}; use syn::punctuated::Punctuated; -use syn::{spanned::Spanned, Attribute, Error, Token}; +use syn::{spanned::Spanned, Attribute, Error, Token, LitStr, LitInt}; pub(crate) struct AttributeMeta { pub path: syn::Path, @@ -48,15 +48,22 @@ pub enum ParamKind { #[derive(Debug, Clone)] pub struct Resource { - pub name: syn::LitStr, + pub name: LitStr, pub assign: Token![=], - pub value: syn::LitInt, + pub value: LitInt, } -pub struct Aliases { - pub list: Punctuated, +pub struct NameMapping { + pub name: String, + pub mapped: Option, } +pub struct Bracketed { + pub list: Punctuated, +} + +pub type Aliases = Bracketed; + impl Parse for Argument { fn parse(input: ParseStream) -> syn::Result { let label = input.parse()?; @@ -91,7 +98,23 @@ impl Parse for Resource { } } -impl Parse for Aliases { +impl Parse for NameMapping { + fn parse(input: ParseStream) -> syn::Result { + let name = input.parse::()?.value(); + + let mapped = if input.peek(Token![=>]) { + input.parse::]>()?; + + Some(input.parse::()?.value()) + } else { + None + }; + + Ok(NameMapping { name, mapped }) + } +} + +impl Parse for Bracketed { fn parse(input: ParseStream) -> syn::Result { let content; @@ -99,7 +122,7 @@ impl Parse for Aliases { let list = content.parse_terminated(Parse::parse)?; - Ok(Aliases { list }) + Ok(Bracketed { list }) } } @@ -201,7 +224,7 @@ impl Argument { /// Asserts that the argument is `key = "string"` and gets the value of the string pub fn string(self) -> syn::Result { - self.value::().map(|lit| lit.value()) + self.value::().map(|lit| lit.value()) } } diff --git a/proc-macros/src/rpc_macro.rs b/proc-macros/src/rpc_macro.rs index 9f1dd328da..c74fd6e170 100644 --- a/proc-macros/src/rpc_macro.rs +++ b/proc-macros/src/rpc_macro.rs @@ -27,7 +27,7 @@ //! Declaration of the JSON RPC generator procedural macros. use crate::{ - attributes::{optional, parse_param_kind, Aliases, Argument, AttributeMeta, MissingArgument, ParamKind, Resource}, + attributes::{optional, parse_param_kind, Aliases, Argument, AttributeMeta, MissingArgument, NameMapping, ParamKind, Resource}, helpers::extract_doc_comments, }; @@ -110,15 +110,13 @@ pub struct RpcSubscription { impl RpcSubscription { pub fn from_item(attr: syn::Attribute, mut sub: syn::TraitItemMethod) -> syn::Result { - let [aliases, item, name, param_kind, unsubscribe_aliases, override_notif_method] = AttributeMeta::parse(attr)? - .retain(["aliases", "item", "name", "param_kind", "unsubscribe_aliases", "override_notif_method"])?; + let [aliases, item, name, param_kind, unsubscribe_aliases] = AttributeMeta::parse(attr)? + .retain(["aliases", "item", "name", "param_kind", "unsubscribe_aliases"])?; let aliases = parse_aliases(aliases)?; - let name = name?.string()?; - let override_notif_method = match override_notif_method { - Ok(arg) => Some(arg.string()?), - Err(_) => None, - }; + let map = name?.value::()?; + let name = map.name; + let override_notif_method = map.mapped; let item = item?.value()?; let param_kind = parse_param_kind(param_kind)?; let unsubscribe_aliases = parse_aliases(unsubscribe_aliases)?; diff --git a/proc-macros/tests/ui/correct/basic.rs b/proc-macros/tests/ui/correct/basic.rs index 71414b3c4b..3a69fd4103 100644 --- a/proc-macros/tests/ui/correct/basic.rs +++ b/proc-macros/tests/ui/correct/basic.rs @@ -33,7 +33,7 @@ pub trait Rpc { fn sub_with_params(&self, val: u32) -> RpcResult<()>; // This will send notifications to the client with `method=subscribe_override` - #[subscription(name = "subscribe_method", override_notif_method = "subscribe_override", item = u32)] + #[subscription(name = "subscribe_method" => "subscribe_override", item = u32)] fn sub_with_override_notif_method(&self) -> RpcResult<()>; } From 84aad423b20b2a669ea03ccac232d5ed8160b2ef Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Thu, 18 Nov 2021 15:45:25 +0100 Subject: [PATCH 03/21] check that unique notifs are used --- examples/proc_macro.rs | 2 +- proc-macros/src/render_server.rs | 14 ++++++++------ tests/tests/proc_macros.rs | 2 +- utils/src/server/rpc_module.rs | 26 ++++++++++++++++++++++++-- 4 files changed, 34 insertions(+), 10 deletions(-) diff --git a/examples/proc_macro.rs b/examples/proc_macro.rs index 868d7e0c85..f13a8b29b4 100644 --- a/examples/proc_macro.rs +++ b/examples/proc_macro.rs @@ -45,7 +45,7 @@ where async fn storage_keys(&self, storage_key: StorageKey, hash: Option) -> Result, Error>; /// Subscription that takes a `StorageKey` as input and produces a `Vec`. - #[subscription(name = "subscribeStorage", override_notif_method = "bar", item = Vec)] + #[subscription(name = "subscribeStorage" => "override", item = Vec)] fn subscribe_storage(&self, keys: Option>) -> Result<(), Error>; } diff --git a/proc-macros/src/render_server.rs b/proc-macros/src/render_server.rs index 957a1dd8c5..a62f9915be 100644 --- a/proc-macros/src/render_server.rs +++ b/proc-macros/src/render_server.rs @@ -186,21 +186,23 @@ impl RpcDescription { check_name(&rpc_sub_name, rust_method_name.span()); check_name(&rpc_unsub_name, rust_method_name.span()); - if maybe_custom_notif.is_some() { - handle_register_result(quote! { + let register_sub = if let Some(notif) = &maybe_custom_notif { + check_name(notif, rust_method_name.span()); + quote! { rpc.register_subscription_with_custom_notif(#rpc_sub_name, #maybe_custom_notif, #rpc_unsub_name, |params, sink, context| { #parsing context.as_ref().#rust_method_name(sink, #params_seq) }) - }) + } } else { - handle_register_result(quote! { + quote! { rpc.register_subscription(#rpc_sub_name, #rpc_unsub_name, |params, sink, context| { #parsing context.as_ref().#rust_method_name(sink, #params_seq) }) - }) - } + } + }; + handle_register_result(register_sub) }) .collect::>(); diff --git a/tests/tests/proc_macros.rs b/tests/tests/proc_macros.rs index 2597e8caf1..08d2ce9a64 100644 --- a/tests/tests/proc_macros.rs +++ b/tests/tests/proc_macros.rs @@ -312,7 +312,7 @@ async fn multiple_blocking_calls_overlap() { #[tokio::test] async fn subscriptions_do_not_work_for_http_servers() { - let htserver = HttpServerBuilder::default().build("127.0.0.1:0".parse().unwrap()).unwrap(); + let htserver = HttpServerBuilder::default().build("127.0.0.1:0").unwrap(); let addr = htserver.local_addr().unwrap(); let htserver_url = format!("http://{}", addr); let _handle = htserver.start(RpcServerImpl.into_rpc()).unwrap(); diff --git a/utils/src/server/rpc_module.rs b/utils/src/server/rpc_module.rs index 0b91319c75..81bfbeda63 100644 --- a/utils/src/server/rpc_module.rs +++ b/utils/src/server/rpc_module.rs @@ -42,7 +42,7 @@ use jsonrpsee_types::{ }; use parking_lot::Mutex; -use rustc_hash::FxHashMap; +use rustc_hash::{FxHashMap, FxHashSet}; use serde::Serialize; use serde_json::value::RawValue; use std::collections::hash_map::Entry; @@ -406,12 +406,13 @@ impl DerefMut for RpcModule { pub struct RpcModule { ctx: Arc, methods: Methods, + notif_overrides: SubscriptionNotifOverride, } impl RpcModule { /// Create a new module with a given shared `Context`. pub fn new(ctx: Context) -> Self { - Self { ctx: Arc::new(ctx), methods: Default::default() } + Self { ctx: Arc::new(ctx), methods: Default::default(), notif_overrides: SubscriptionNotifOverride::default() } } } @@ -589,6 +590,12 @@ impl RpcModule { self.methods.verify_method_name(subscribe_method_name)?; self.methods.verify_method_name(unsubscribe_method_name)?; + + if let Some(name) = custom_notif_method_name { + self.notif_overrides.verify_and_insert(name)?; + self.methods.verify_method_name(name)?; + } + let ctx = self.ctx.clone(); let subscribers = Subscribers::default(); @@ -681,6 +688,21 @@ impl RpcModule { } } +/// Keeps track of registered overrides for subscription method names +/// Regards duplicates as error. +#[derive(Default, Debug, Clone)] +struct SubscriptionNotifOverride(FxHashSet<&'static str>); + +impl SubscriptionNotifOverride { + fn verify_and_insert(&mut self, notif: &'static str) -> Result<(), Error> { + if self.0.insert(notif) { + Ok(()) + } else { + Err(Error::MethodAlreadyRegistered(notif.into())) + } + } +} + /// Represents a single subscription. #[derive(Debug)] pub struct SubscriptionSink { From eea1b2bbe24fb1f248c32b7368cabf34e6e22a03 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Thu, 18 Nov 2021 16:10:49 +0100 Subject: [PATCH 04/21] check that custom sub name is unique --- proc-macros/src/render_server.rs | 2 +- utils/src/server/rpc_module.rs | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/proc-macros/src/render_server.rs b/proc-macros/src/render_server.rs index a62f9915be..683bf4f443 100644 --- a/proc-macros/src/render_server.rs +++ b/proc-macros/src/render_server.rs @@ -174,7 +174,7 @@ impl RpcDescription { let rust_method_name = &sub.signature.sig.ident; // Name of the RPC method to subscribe to (e.g. `foo_sub`). let rpc_sub_name = self.rpc_identifier(&sub.name); - // + // Custom method name to use when sending notifs on the subscription if configured. let maybe_custom_notif = self.optional_rpc_identifier(sub.override_notif_method.as_deref()); // Name of the RPC method to unsubscribe (e.g. `foo_sub`). let rpc_unsub_name = self.rpc_identifier(&sub.unsubscribe); diff --git a/utils/src/server/rpc_module.rs b/utils/src/server/rpc_module.rs index 81bfbeda63..93fc9f1b31 100644 --- a/utils/src/server/rpc_module.rs +++ b/utils/src/server/rpc_module.rs @@ -592,6 +592,13 @@ impl RpcModule { self.methods.verify_method_name(unsubscribe_method_name)?; if let Some(name) = custom_notif_method_name { + if name == subscribe_method_name || name == unsubscribe_method_name { + return Err(Error::SubscriptionNameConflict(format!( + "Custom subscription notification name: `{}` already used", + name + ))); + } + self.notif_overrides.verify_and_insert(name)?; self.methods.verify_method_name(name)?; } From e72a33aecc593271a6dda535632893db4926f268 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Thu, 18 Nov 2021 16:11:21 +0100 Subject: [PATCH 05/21] cargo fmt --- proc-macros/src/attributes.rs | 2 +- proc-macros/src/rpc_macro.rs | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/proc-macros/src/attributes.rs b/proc-macros/src/attributes.rs index 7e8ee40c67..a143a7a873 100644 --- a/proc-macros/src/attributes.rs +++ b/proc-macros/src/attributes.rs @@ -28,7 +28,7 @@ use proc_macro2::{Span, TokenStream as TokenStream2, TokenTree}; use std::{fmt, iter}; use syn::parse::{Parse, ParseStream, Parser}; use syn::punctuated::Punctuated; -use syn::{spanned::Spanned, Attribute, Error, Token, LitStr, LitInt}; +use syn::{spanned::Spanned, Attribute, Error, LitInt, LitStr, Token}; pub(crate) struct AttributeMeta { pub path: syn::Path, diff --git a/proc-macros/src/rpc_macro.rs b/proc-macros/src/rpc_macro.rs index c74fd6e170..9965507457 100644 --- a/proc-macros/src/rpc_macro.rs +++ b/proc-macros/src/rpc_macro.rs @@ -27,7 +27,9 @@ //! Declaration of the JSON RPC generator procedural macros. use crate::{ - attributes::{optional, parse_param_kind, Aliases, Argument, AttributeMeta, MissingArgument, NameMapping, ParamKind, Resource}, + attributes::{ + optional, parse_param_kind, Aliases, Argument, AttributeMeta, MissingArgument, NameMapping, ParamKind, Resource, + }, helpers::extract_doc_comments, }; @@ -110,8 +112,8 @@ pub struct RpcSubscription { impl RpcSubscription { pub fn from_item(attr: syn::Attribute, mut sub: syn::TraitItemMethod) -> syn::Result { - let [aliases, item, name, param_kind, unsubscribe_aliases] = AttributeMeta::parse(attr)? - .retain(["aliases", "item", "name", "param_kind", "unsubscribe_aliases"])?; + let [aliases, item, name, param_kind, unsubscribe_aliases] = + AttributeMeta::parse(attr)?.retain(["aliases", "item", "name", "param_kind", "unsubscribe_aliases"])?; let aliases = parse_aliases(aliases)?; let map = name?.value::()?; From 673699aeae980b26c71979feecdf8efa41fcd00b Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Thu, 18 Nov 2021 17:02:23 +0100 Subject: [PATCH 06/21] address grumbles --- benches/helpers.rs | 2 +- examples/ws_sub_with_params.rs | 4 +- examples/ws_subscription.rs | 2 +- proc-macros/src/render_server.rs | 29 +++++------ proc-macros/src/rpc_macro.rs | 12 ----- tests/tests/helpers.rs | 33 +++++++------ tests/tests/integration_tests.rs | 2 +- utils/src/server/rpc_module.rs | 83 +++----------------------------- ws-server/src/tests.rs | 12 +++-- 9 files changed, 53 insertions(+), 126 deletions(-) diff --git a/benches/helpers.rs b/benches/helpers.rs index f60fc122b6..dcd3de7394 100644 --- a/benches/helpers.rs +++ b/benches/helpers.rs @@ -101,7 +101,7 @@ pub async fn ws_server(handle: tokio::runtime::Handle) -> (String, jsonrpsee::ws module.register_method(SYNC_METHOD_NAME, |_, _| Ok("lo")).unwrap(); module.register_async_method(ASYNC_METHOD_NAME, |_, _| async { Ok("lo") }).unwrap(); module - .register_subscription(SUB_METHOD_NAME, UNSUB_METHOD_NAME, |_params, mut sink, _ctx| { + .register_subscription(SUB_METHOD_NAME, SUB_METHOD_NAME, UNSUB_METHOD_NAME, |_params, mut sink, _ctx| { let x = "Hello"; tokio::spawn(async move { sink.send(&x) }); Ok(()) diff --git a/examples/ws_sub_with_params.rs b/examples/ws_sub_with_params.rs index 3c3c61c3d1..1275168d64 100644 --- a/examples/ws_sub_with_params.rs +++ b/examples/ws_sub_with_params.rs @@ -62,7 +62,7 @@ async fn run_server() -> anyhow::Result { let server = WsServerBuilder::default().build("127.0.0.1:0").await?; let mut module = RpcModule::new(()); module - .register_subscription("sub_one_param", "unsub_one_param", |params, mut sink, _| { + .register_subscription("sub_one_param", "sub_one_param", "unsub_one_param", |params, mut sink, _| { let idx: usize = params.one()?; std::thread::spawn(move || loop { let _ = sink.send(&LETTERS.chars().nth(idx)); @@ -72,7 +72,7 @@ async fn run_server() -> anyhow::Result { }) .unwrap(); module - .register_subscription("sub_params_two", "unsub_params_two", |params, mut sink, _| { + .register_subscription("sub_params_two", "params_two", "unsub_params_two", |params, mut sink, _| { let (one, two): (usize, usize) = params.parse()?; std::thread::spawn(move || loop { let _ = sink.send(&LETTERS[one..two].to_string()); diff --git a/examples/ws_subscription.rs b/examples/ws_subscription.rs index f9521992dc..4af06c9fa0 100644 --- a/examples/ws_subscription.rs +++ b/examples/ws_subscription.rs @@ -61,7 +61,7 @@ async fn main() -> anyhow::Result<()> { async fn run_server() -> anyhow::Result { let server = WsServerBuilder::default().build("127.0.0.1:0").await?; let mut module = RpcModule::new(()); - module.register_subscription("subscribe_hello", "unsubscribe_hello", |_, mut sink, _| { + module.register_subscription("subscribe_hello", "s_hello", "unsubscribe_hello", |_, mut sink, _| { std::thread::spawn(move || loop { if let Err(Error::SubscriptionClosed(_)) = sink.send(&"hello my friend") { return; diff --git a/proc-macros/src/render_server.rs b/proc-macros/src/render_server.rs index 683bf4f443..5d8de2c8fe 100644 --- a/proc-macros/src/render_server.rs +++ b/proc-macros/src/render_server.rs @@ -175,7 +175,7 @@ impl RpcDescription { // Name of the RPC method to subscribe to (e.g. `foo_sub`). let rpc_sub_name = self.rpc_identifier(&sub.name); // Custom method name to use when sending notifs on the subscription if configured. - let maybe_custom_notif = self.optional_rpc_identifier(sub.override_notif_method.as_deref()); + let maybe_custom_notif = sub.override_notif_method.as_ref().map(|m| self.rpc_identifier(m)); // Name of the RPC method to unsubscribe (e.g. `foo_sub`). let rpc_unsub_name = self.rpc_identifier(&sub.unsubscribe); // `parsing` is the code associated with parsing structure from the @@ -186,23 +186,20 @@ impl RpcDescription { check_name(&rpc_sub_name, rust_method_name.span()); check_name(&rpc_unsub_name, rust_method_name.span()); - let register_sub = if let Some(notif) = &maybe_custom_notif { - check_name(notif, rust_method_name.span()); - quote! { - rpc.register_subscription_with_custom_notif(#rpc_sub_name, #maybe_custom_notif, #rpc_unsub_name, |params, sink, context| { - #parsing - context.as_ref().#rust_method_name(sink, #params_seq) - }) - } - } else { - quote! { - rpc.register_subscription(#rpc_sub_name, #rpc_unsub_name, |params, sink, context| { - #parsing - context.as_ref().#rust_method_name(sink, #params_seq) - }) + let notif_name = match maybe_custom_notif { + Some(notif) => { + check_name(¬if, rust_method_name.span()); + notif } + None => rpc_sub_name.clone(), }; - handle_register_result(register_sub) + + handle_register_result(quote! { + rpc.register_subscription(#rpc_sub_name, #notif_name, #rpc_unsub_name, |params, sink, context| { + #parsing + context.as_ref().#rust_method_name(sink, #params_seq) + }) + }) }) .collect::>(); diff --git a/proc-macros/src/rpc_macro.rs b/proc-macros/src/rpc_macro.rs index 9965507457..e3d10f3c67 100644 --- a/proc-macros/src/rpc_macro.rs +++ b/proc-macros/src/rpc_macro.rs @@ -302,18 +302,6 @@ impl RpcDescription { Cow::Borrowed(method) } } - - /// Based on the namespace, renders the full name of the RPC method/subscription. - /// Examples: - /// For namespace `foo` and method `makeSpam`, result will be `foo_makeSpam`. - /// For no namespace and method `makeSpam` it will be just `makeSpam. - pub(crate) fn optional_rpc_identifier<'a>(&self, method: Option<&'a str>) -> Option> { - if let Some(method) = method { - Some(self.rpc_identifier(method)) - } else { - None - } - } } fn parse_aliases(arg: Result) -> syn::Result> { diff --git a/tests/tests/helpers.rs b/tests/tests/helpers.rs index a9773522f4..7df1d923e6 100644 --- a/tests/tests/helpers.rs +++ b/tests/tests/helpers.rs @@ -40,7 +40,7 @@ pub async fn websocket_server_with_subscription() -> (SocketAddr, WsServerHandle module.register_method("say_hello", |_, _| Ok("hello")).unwrap(); module - .register_subscription("subscribe_hello", "unsubscribe_hello", |_, mut sink, _| { + .register_subscription("subscribe_hello", "subscribe_hello", "unsubscribe_hello", |_, mut sink, _| { std::thread::spawn(move || loop { if let Err(Error::SubscriptionClosed(_)) = sink.send(&"hello from subscription") { break; @@ -52,7 +52,7 @@ pub async fn websocket_server_with_subscription() -> (SocketAddr, WsServerHandle .unwrap(); module - .register_subscription("subscribe_foo", "unsubscribe_foo", |_, mut sink, _| { + .register_subscription("subscribe_foo", "subscribe_foo", "unsubscribe_foo", |_, mut sink, _| { std::thread::spawn(move || loop { if let Err(Error::SubscriptionClosed(_)) = sink.send(&1337) { break; @@ -64,21 +64,26 @@ pub async fn websocket_server_with_subscription() -> (SocketAddr, WsServerHandle .unwrap(); module - .register_subscription("subscribe_add_one", "unsubscribe_add_one", |params, mut sink, _| { - let mut count: usize = params.one()?; - std::thread::spawn(move || loop { - count = count.wrapping_add(1); - if let Err(Error::SubscriptionClosed(_)) = sink.send(&count) { - break; - } - std::thread::sleep(Duration::from_millis(100)); - }); - Ok(()) - }) + .register_subscription( + "subscribe_add_one", + "subscribe_add_one", + "unsubscribe_add_one", + |params, mut sink, _| { + let mut count: usize = params.one()?; + std::thread::spawn(move || loop { + count = count.wrapping_add(1); + if let Err(Error::SubscriptionClosed(_)) = sink.send(&count) { + break; + } + std::thread::sleep(Duration::from_millis(100)); + }); + Ok(()) + }, + ) .unwrap(); module - .register_subscription("subscribe_noop", "unsubscribe_noop", |_, mut sink, _| { + .register_subscription("subscribe_noop", "subscribe_noop", "unsubscribe_noop", |_, mut sink, _| { std::thread::spawn(move || { std::thread::sleep(Duration::from_secs(1)); sink.close("Server closed the stream because it was lazy") diff --git a/tests/tests/integration_tests.rs b/tests/tests/integration_tests.rs index 118c270a81..267a8205ca 100644 --- a/tests/tests/integration_tests.rs +++ b/tests/tests/integration_tests.rs @@ -332,7 +332,7 @@ async fn ws_server_should_stop_subscription_after_client_drop() { let mut module = RpcModule::new(tx); module - .register_subscription("subscribe_hello", "unsubscribe_hello", |_, mut sink, mut tx| { + .register_subscription("subscribe_hello", "subscribe_hello", "unsubscribe_hello", |_, mut sink, mut tx| { tokio::spawn(async move { let close_err = loop { if let Err(Error::SubscriptionClosed(err)) = sink.send(&1) { diff --git a/utils/src/server/rpc_module.rs b/utils/src/server/rpc_module.rs index 93fc9f1b31..81da31b8db 100644 --- a/utils/src/server/rpc_module.rs +++ b/utils/src/server/rpc_module.rs @@ -42,7 +42,7 @@ use jsonrpsee_types::{ }; use parking_lot::Mutex; -use rustc_hash::{FxHashMap, FxHashSet}; +use rustc_hash::FxHashMap; use serde::Serialize; use serde_json::value::RawValue; use std::collections::hash_map::Entry; @@ -406,13 +406,12 @@ impl DerefMut for RpcModule { pub struct RpcModule { ctx: Arc, methods: Methods, - notif_overrides: SubscriptionNotifOverride, } impl RpcModule { /// Create a new module with a given shared `Context`. pub fn new(ctx: Context) -> Self { - Self { ctx: Arc::new(ctx), methods: Default::default(), notif_overrides: SubscriptionNotifOverride::default() } + Self { ctx: Arc::new(ctx), methods: Default::default() } } } @@ -530,7 +529,7 @@ impl RpcModule { /// use jsonrpsee_utils::server::rpc_module::RpcModule; /// /// let mut ctx = RpcModule::new(99_usize); - /// ctx.register_subscription("sub", "unsub", |params, mut sink, ctx| { + /// ctx.register_subscription("sub", "sub_notif", "unsub", |params, mut sink, ctx| { /// let x: usize = params.one()?; /// std::thread::spawn(move || { /// let sum = x + (*ctx); @@ -542,41 +541,7 @@ impl RpcModule { pub fn register_subscription( &mut self, subscribe_method_name: &'static str, - unsubscribe_method_name: &'static str, - callback: F, - ) -> Result<(), Error> - where - Context: Send + Sync + 'static, - F: Fn(Params, SubscriptionSink, Arc) -> Result<(), Error> + Send + Sync + 'static, - { - self.register_subscription_inner(subscribe_method_name, None, unsubscribe_method_name, callback) - } - - /// Similar to [`RpcModule::register_subscription`] but allows sending notifications to another method - /// than the actual subscription method name. - pub fn register_subscription_with_custom_notif( - &mut self, - subscribe_method_name: &'static str, - custom_notif_method_name: &'static str, - unsubscribe_method_name: &'static str, - callback: F, - ) -> Result<(), Error> - where - Context: Send + Sync + 'static, - F: Fn(Params, SubscriptionSink, Arc) -> Result<(), Error> + Send + Sync + 'static, - { - self.register_subscription_inner( - subscribe_method_name, - Some(custom_notif_method_name), - unsubscribe_method_name, - callback, - ) - } - - fn register_subscription_inner( - &mut self, - subscribe_method_name: &'static str, - custom_notif_method_name: Option<&'static str>, + notif_method_name: &'static str, unsubscribe_method_name: &'static str, callback: F, ) -> Result<(), Error> @@ -591,18 +556,6 @@ impl RpcModule { self.methods.verify_method_name(subscribe_method_name)?; self.methods.verify_method_name(unsubscribe_method_name)?; - if let Some(name) = custom_notif_method_name { - if name == subscribe_method_name || name == unsubscribe_method_name { - return Err(Error::SubscriptionNameConflict(format!( - "Custom subscription notification name: `{}` already used", - name - ))); - } - - self.notif_overrides.verify_and_insert(name)?; - self.methods.verify_method_name(name)?; - } - let ctx = self.ctx.clone(); let subscribers = Subscribers::default(); @@ -624,14 +577,9 @@ impl RpcModule { send_response(id.clone(), method_sink, sub_id, max_response_size); - let method = match custom_notif_method_name { - Some(m) => m, - None => subscribe_method_name, - }; - let sink = SubscriptionSink { inner: method_sink.clone(), - method, + method: notif_method_name, subscribers: subscribers.clone(), uniq_sub: SubscriptionKey { conn_id, sub_id }, is_connected: Some(conn_tx), @@ -695,21 +643,6 @@ impl RpcModule { } } -/// Keeps track of registered overrides for subscription method names -/// Regards duplicates as error. -#[derive(Default, Debug, Clone)] -struct SubscriptionNotifOverride(FxHashSet<&'static str>); - -impl SubscriptionNotifOverride { - fn verify_and_insert(&mut self, notif: &'static str) -> Result<(), Error> { - if self.0.insert(notif) { - Ok(()) - } else { - Err(Error::MethodAlreadyRegistered(notif.into())) - } - } -} - /// Represents a single subscription. #[derive(Debug)] pub struct SubscriptionSink { @@ -853,7 +786,7 @@ mod tests { fn rpc_context_modules_can_register_subscriptions() { let cx = (); let mut cxmodule = RpcModule::new(cx); - let _subscription = cxmodule.register_subscription("hi", "goodbye", |_, _, _| Ok(())); + let _subscription = cxmodule.register_subscription("hi", "hi", "goodbye", |_, _, _| Ok(())); assert!(cxmodule.method("hi").is_some()); assert!(cxmodule.method("goodbye").is_some()); @@ -991,7 +924,7 @@ mod tests { async fn subscribing_without_server() { let mut module = RpcModule::new(()); module - .register_subscription("my_sub", "my_unsub", |_, mut sink, _| { + .register_subscription("my_sub", "my_sub", "my_unsub", |_, mut sink, _| { let mut stream_data = vec!['0', '1', '2']; std::thread::spawn(move || loop { tracing::debug!("This is your friendly subscription sending data."); @@ -1025,7 +958,7 @@ mod tests { async fn close_test_subscribing_without_server() { let mut module = RpcModule::new(()); module - .register_subscription("my_sub", "my_unsub", |_, mut sink, _| { + .register_subscription("my_sub", "my_sub", "my_unsub", |_, mut sink, _| { std::thread::spawn(move || loop { if let Err(Error::SubscriptionClosed(_)) = sink.send(&"lo") { return; diff --git a/ws-server/src/tests.rs b/ws-server/src/tests.rs index 121cbdf7fb..67e5f36f66 100644 --- a/ws-server/src/tests.rs +++ b/ws-server/src/tests.rs @@ -117,7 +117,7 @@ async fn server_with_handles() -> (SocketAddr, ServerHandle) { }) .unwrap(); module - .register_subscription("subscribe_hello", "unsubscribe_hello", |_, sink, _| { + .register_subscription("subscribe_hello", "subscribe_hello", "unsubscribe_hello", |_, sink, _| { std::thread::spawn(move || loop { let _ = sink; std::thread::sleep(std::time::Duration::from_secs(30)); @@ -472,8 +472,12 @@ async fn register_methods_works() { let mut module = RpcModule::new(()); assert!(module.register_method("say_hello", |_, _| Ok("lo")).is_ok()); assert!(module.register_method("say_hello", |_, _| Ok("lo")).is_err()); - assert!(module.register_subscription("subscribe_hello", "unsubscribe_hello", |_, _, _| Ok(())).is_ok()); - assert!(module.register_subscription("subscribe_hello_again", "unsubscribe_hello", |_, _, _| Ok(())).is_err()); + assert!(module + .register_subscription("subscribe_hello", "subscribe_hello", "unsubscribe_hello", |_, _, _| Ok(())) + .is_ok()); + assert!(module + .register_subscription("subscribe_hello_again", "subscribe_hello_again", "unsubscribe_hello", |_, _, _| Ok(())) + .is_err()); assert!( module.register_method("subscribe_hello_again", |_, _| Ok("lo")).is_ok(), "Failed register_subscription should not have side-effects" @@ -484,7 +488,7 @@ async fn register_methods_works() { async fn register_same_subscribe_unsubscribe_is_err() { let mut module = RpcModule::new(()); assert!(matches!( - module.register_subscription("subscribe_hello", "subscribe_hello", |_, _, _| Ok(())), + module.register_subscription("subscribe_hello", "subscribe_hello", "subscribe_hello", |_, _, _| Ok(())), Err(Error::SubscriptionNameConflict(_)) )); } From 2913c0b1095bf85bfb6d7d4d8768767258b708c2 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Thu, 18 Nov 2021 17:06:59 +0100 Subject: [PATCH 07/21] Update proc-macros/src/rpc_macro.rs --- proc-macros/src/rpc_macro.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proc-macros/src/rpc_macro.rs b/proc-macros/src/rpc_macro.rs index e3d10f3c67..bbd4d628a5 100644 --- a/proc-macros/src/rpc_macro.rs +++ b/proc-macros/src/rpc_macro.rs @@ -97,7 +97,7 @@ impl RpcMethod { #[derive(Debug, Clone)] pub struct RpcSubscription { pub name: String, - /// By default the server will send out subscriptions `method` in `name` above + /// By default the server will send out subscriptions using `method` in `name` above /// but it's possible to override it with another name that this field does. pub override_notif_method: Option, pub docs: TokenStream2, From 37f77d13c8bab03a6d2787de65b4081f58687aff Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Thu, 18 Nov 2021 17:09:30 +0100 Subject: [PATCH 08/21] commit added tests --- .../tests/ui/incorrect/sub/sub_dup_name_override.rs | 12 ++++++++++++ .../ui/incorrect/sub/sub_dup_name_override.stderr | 5 +++++ 2 files changed, 17 insertions(+) create mode 100644 proc-macros/tests/ui/incorrect/sub/sub_dup_name_override.rs create mode 100644 proc-macros/tests/ui/incorrect/sub/sub_dup_name_override.stderr diff --git a/proc-macros/tests/ui/incorrect/sub/sub_dup_name_override.rs b/proc-macros/tests/ui/incorrect/sub/sub_dup_name_override.rs new file mode 100644 index 0000000000..53adf3fe2e --- /dev/null +++ b/proc-macros/tests/ui/incorrect/sub/sub_dup_name_override.rs @@ -0,0 +1,12 @@ +use jsonrpsee::{proc_macros::rpc, types::RpcResult}; + +// Subscription method must not use the same override name. +#[rpc(client, server)] +pub trait DupOverride { + #[subscription(name = "one" => "override", item = u8)] + fn one(&self) -> RpcResult<()>; + #[subscription(name = "two" => "override", item = u8)] + fn two(&self) -> RpcResult<()>; +} + +fn main() {} diff --git a/proc-macros/tests/ui/incorrect/sub/sub_dup_name_override.stderr b/proc-macros/tests/ui/incorrect/sub/sub_dup_name_override.stderr new file mode 100644 index 0000000000..a34210fe70 --- /dev/null +++ b/proc-macros/tests/ui/incorrect/sub/sub_dup_name_override.stderr @@ -0,0 +1,5 @@ +error: "override" is already defined + --> tests/ui/incorrect/sub/sub_dup_name_override.rs:9:5 + | +9 | fn two(&self) -> RpcResult<()>; + | ^^^ From 66972eff90ead98d5231960d113653d3e0b3ffff Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 19 Nov 2021 14:48:06 +0100 Subject: [PATCH 09/21] Update proc-macros/src/render_server.rs Co-authored-by: David --- proc-macros/src/render_server.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proc-macros/src/render_server.rs b/proc-macros/src/render_server.rs index 5d8de2c8fe..ef93ad36ac 100644 --- a/proc-macros/src/render_server.rs +++ b/proc-macros/src/render_server.rs @@ -174,7 +174,7 @@ impl RpcDescription { let rust_method_name = &sub.signature.sig.ident; // Name of the RPC method to subscribe to (e.g. `foo_sub`). let rpc_sub_name = self.rpc_identifier(&sub.name); - // Custom method name to use when sending notifs on the subscription if configured. + // When subscribing to an RPC, users can override the content of the `method` field in the JSON data sent to subscribers. Each subscription thus has one method name to set up the subscription, one to unsubscribe and, optionally, a third method name used to describe the payload sent back from the server to subscribers. If no override is provided, the subscription method name is used. let maybe_custom_notif = sub.override_notif_method.as_ref().map(|m| self.rpc_identifier(m)); // Name of the RPC method to unsubscribe (e.g. `foo_sub`). let rpc_unsub_name = self.rpc_identifier(&sub.unsubscribe); From 5067f0dbc6770eb84b51cbadbad5191fe967ef49 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 19 Nov 2021 14:53:25 +0100 Subject: [PATCH 10/21] Update proc-macros/src/render_server.rs Co-authored-by: David --- proc-macros/src/render_server.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proc-macros/src/render_server.rs b/proc-macros/src/render_server.rs index ef93ad36ac..9f4bd1a845 100644 --- a/proc-macros/src/render_server.rs +++ b/proc-macros/src/render_server.rs @@ -175,7 +175,7 @@ impl RpcDescription { // Name of the RPC method to subscribe to (e.g. `foo_sub`). let rpc_sub_name = self.rpc_identifier(&sub.name); // When subscribing to an RPC, users can override the content of the `method` field in the JSON data sent to subscribers. Each subscription thus has one method name to set up the subscription, one to unsubscribe and, optionally, a third method name used to describe the payload sent back from the server to subscribers. If no override is provided, the subscription method name is used. - let maybe_custom_notif = sub.override_notif_method.as_ref().map(|m| self.rpc_identifier(m)); + let payload_name = sub.payload_name_override.as_ref().map(|m| self.rpc_identifier(m)); // Name of the RPC method to unsubscribe (e.g. `foo_sub`). let rpc_unsub_name = self.rpc_identifier(&sub.unsubscribe); // `parsing` is the code associated with parsing structure from the From 6886c2b70bd35143a37560b10e57b2fd50221b94 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 19 Nov 2021 14:53:41 +0100 Subject: [PATCH 11/21] Update proc-macros/src/rpc_macro.rs Co-authored-by: David --- proc-macros/src/rpc_macro.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/proc-macros/src/rpc_macro.rs b/proc-macros/src/rpc_macro.rs index bbd4d628a5..aac3def66a 100644 --- a/proc-macros/src/rpc_macro.rs +++ b/proc-macros/src/rpc_macro.rs @@ -97,9 +97,9 @@ impl RpcMethod { #[derive(Debug, Clone)] pub struct RpcSubscription { pub name: String, - /// By default the server will send out subscriptions using `method` in `name` above - /// but it's possible to override it with another name that this field does. - pub override_notif_method: Option, + /// By default the server will send data to subscribers using the `name` field above as the `method` field in the JSON payload + /// but it's possible to override it with another name using this field. + pub payload_name_override: Option, pub docs: TokenStream2, pub unsubscribe: String, pub params: Vec<(syn::PatIdent, syn::Type)>, From 1eb0a8014863984a63ddbe7c8b1438491c4ac535 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 19 Nov 2021 14:53:49 +0100 Subject: [PATCH 12/21] Update proc-macros/src/rpc_macro.rs Co-authored-by: David --- proc-macros/src/rpc_macro.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proc-macros/src/rpc_macro.rs b/proc-macros/src/rpc_macro.rs index aac3def66a..98b479cab7 100644 --- a/proc-macros/src/rpc_macro.rs +++ b/proc-macros/src/rpc_macro.rs @@ -144,7 +144,7 @@ impl RpcSubscription { Ok(Self { name, - override_notif_method, + payload_name_override, unsubscribe, unsubscribe_aliases, params, From 2be32e908388a268e141d018bafb63f97b1c5806 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 19 Nov 2021 14:54:00 +0100 Subject: [PATCH 13/21] Update utils/src/server/rpc_module.rs Co-authored-by: David --- utils/src/server/rpc_module.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/src/server/rpc_module.rs b/utils/src/server/rpc_module.rs index 81da31b8db..9247c3e36c 100644 --- a/utils/src/server/rpc_module.rs +++ b/utils/src/server/rpc_module.rs @@ -529,7 +529,7 @@ impl RpcModule { /// use jsonrpsee_utils::server::rpc_module::RpcModule; /// /// let mut ctx = RpcModule::new(99_usize); - /// ctx.register_subscription("sub", "sub_notif", "unsub", |params, mut sink, ctx| { + /// ctx.register_subscription("sub", "payload_name", "unsub", |params, mut sink, ctx| { /// let x: usize = params.one()?; /// std::thread::spawn(move || { /// let sum = x + (*ctx); From b67be8f1d534d11f36afff4b528fb489ca7c7783 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 19 Nov 2021 15:37:35 +0100 Subject: [PATCH 14/21] grumbles --- proc-macros/src/render_server.rs | 6 +++--- proc-macros/src/rpc_macro.rs | 8 +++++--- proc-macros/tests/ui/correct/basic.rs | 3 ++- utils/src/server/rpc_module.rs | 18 +++++++++++++++--- 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/proc-macros/src/render_server.rs b/proc-macros/src/render_server.rs index 9f4bd1a845..0f7d91d04c 100644 --- a/proc-macros/src/render_server.rs +++ b/proc-macros/src/render_server.rs @@ -175,7 +175,7 @@ impl RpcDescription { // Name of the RPC method to subscribe to (e.g. `foo_sub`). let rpc_sub_name = self.rpc_identifier(&sub.name); // When subscribing to an RPC, users can override the content of the `method` field in the JSON data sent to subscribers. Each subscription thus has one method name to set up the subscription, one to unsubscribe and, optionally, a third method name used to describe the payload sent back from the server to subscribers. If no override is provided, the subscription method name is used. - let payload_name = sub.payload_name_override.as_ref().map(|m| self.rpc_identifier(m)); + let rpc_sub_name_override = sub.name_override.as_ref().map(|m| self.rpc_identifier(m)); // Name of the RPC method to unsubscribe (e.g. `foo_sub`). let rpc_unsub_name = self.rpc_identifier(&sub.unsubscribe); // `parsing` is the code associated with parsing structure from the @@ -186,7 +186,7 @@ impl RpcDescription { check_name(&rpc_sub_name, rust_method_name.span()); check_name(&rpc_unsub_name, rust_method_name.span()); - let notif_name = match maybe_custom_notif { + let rpc_notif_name = match rpc_sub_name_override { Some(notif) => { check_name(¬if, rust_method_name.span()); notif @@ -195,7 +195,7 @@ impl RpcDescription { }; handle_register_result(quote! { - rpc.register_subscription(#rpc_sub_name, #notif_name, #rpc_unsub_name, |params, sink, context| { + rpc.register_subscription(#rpc_sub_name, #rpc_notif_name, #rpc_unsub_name, |params, sink, context| { #parsing context.as_ref().#rust_method_name(sink, #params_seq) }) diff --git a/proc-macros/src/rpc_macro.rs b/proc-macros/src/rpc_macro.rs index 98b479cab7..ebfe1687e9 100644 --- a/proc-macros/src/rpc_macro.rs +++ b/proc-macros/src/rpc_macro.rs @@ -99,7 +99,9 @@ pub struct RpcSubscription { pub name: String, /// By default the server will send data to subscribers using the `name` field above as the `method` field in the JSON payload /// but it's possible to override it with another name using this field. - pub payload_name_override: Option, + /// + /// The `proc macro` itself ensure that each `name override` is unique on each trait/API. + pub name_override: Option, pub docs: TokenStream2, pub unsubscribe: String, pub params: Vec<(syn::PatIdent, syn::Type)>, @@ -118,7 +120,7 @@ impl RpcSubscription { let aliases = parse_aliases(aliases)?; let map = name?.value::()?; let name = map.name; - let override_notif_method = map.mapped; + let name_override = map.mapped; let item = item?.value()?; let param_kind = parse_param_kind(param_kind)?; let unsubscribe_aliases = parse_aliases(unsubscribe_aliases)?; @@ -144,7 +146,7 @@ impl RpcSubscription { Ok(Self { name, - payload_name_override, + name_override, unsubscribe, unsubscribe_aliases, params, diff --git a/proc-macros/tests/ui/correct/basic.rs b/proc-macros/tests/ui/correct/basic.rs index 3a69fd4103..05b062b88a 100644 --- a/proc-macros/tests/ui/correct/basic.rs +++ b/proc-macros/tests/ui/correct/basic.rs @@ -32,7 +32,8 @@ pub trait Rpc { #[subscription(name = "echo", aliases = ["ECHO"], item = u32, unsubscribe_aliases = ["NotInterested", "listenNoMore"])] fn sub_with_params(&self, val: u32) -> RpcResult<()>; - // This will send notifications to the client with `method=subscribe_override` + // This will send data to subscribers with the `method` field in the JSON payload set to `foo_subscribe_override` + // because it's in the `foo` namespace. #[subscription(name = "subscribe_method" => "subscribe_override", item = u32)] fn sub_with_override_notif_method(&self) -> RpcResult<()>; } diff --git a/utils/src/server/rpc_module.rs b/utils/src/server/rpc_module.rs index 9247c3e36c..1ef27b6996 100644 --- a/utils/src/server/rpc_module.rs +++ b/utils/src/server/rpc_module.rs @@ -516,9 +516,21 @@ impl RpcModule { Ok(MethodResourcesBuilder { build: ResourceVec::new(), callback }) } - /// Register a new RPC subscription that invokes callback on every subscription request. - /// The callback itself takes three parameters: - /// - [`Params`]: JSONRPC parameters in the subscription request. + /// Register a new RPC subscription that invokes callback on every subscription call. + /// + /// This method ensures that the `subscription_method_name` and `unsubscription_method_name` are unique + /// but does not check that `notif_method_name` is unique. + /// + /// Thus, if you want to use a different method name in the subscription make sure that + /// names are unique. + /// + /// # Arguments + /// + /// * `subscription_method_name` - name of the method to call to initiate a subscription + /// * `notif_method_name` - name of method to be used in the subscription payload (technically a JSON-RPC notification) + /// * `unsubscription_method` - name of the method to call to terminate a subscription + /// * `callback` - A callback to invoke on each subscrption, it takes three parameters + /// - [`Params`]: JSON-RPC parameters in the subscription call. /// - [`SubscriptionSink`]: A sink to send messages to the subscriber. /// - Context: Any type that can be embedded into the [`RpcModule`]. /// From 3e8a18ee25c47cc57530357d354737c3eaf19edf Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 19 Nov 2021 15:48:20 +0100 Subject: [PATCH 15/21] fix long lines --- proc-macros/src/render_server.rs | 2 +- proc-macros/src/rpc_macro.rs | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/proc-macros/src/render_server.rs b/proc-macros/src/render_server.rs index 0f7d91d04c..c277801921 100644 --- a/proc-macros/src/render_server.rs +++ b/proc-macros/src/render_server.rs @@ -174,7 +174,7 @@ impl RpcDescription { let rust_method_name = &sub.signature.sig.ident; // Name of the RPC method to subscribe to (e.g. `foo_sub`). let rpc_sub_name = self.rpc_identifier(&sub.name); - // When subscribing to an RPC, users can override the content of the `method` field in the JSON data sent to subscribers. Each subscription thus has one method name to set up the subscription, one to unsubscribe and, optionally, a third method name used to describe the payload sent back from the server to subscribers. If no override is provided, the subscription method name is used. + // Name of `method` in the subscription response. let rpc_sub_name_override = sub.name_override.as_ref().map(|m| self.rpc_identifier(m)); // Name of the RPC method to unsubscribe (e.g. `foo_sub`). let rpc_unsub_name = self.rpc_identifier(&sub.unsubscribe); diff --git a/proc-macros/src/rpc_macro.rs b/proc-macros/src/rpc_macro.rs index ebfe1687e9..d880b9a428 100644 --- a/proc-macros/src/rpc_macro.rs +++ b/proc-macros/src/rpc_macro.rs @@ -97,10 +97,12 @@ impl RpcMethod { #[derive(Debug, Clone)] pub struct RpcSubscription { pub name: String, - /// By default the server will send data to subscribers using the `name` field above as the `method` field in the JSON payload - /// but it's possible to override it with another name using this field. - /// - /// The `proc macro` itself ensure that each `name override` is unique on each trait/API. + /// When subscribing to an RPC, users can override the content of the `method` field + /// in the JSON data sent to subscribers. + /// Each subscription thus has one method name to set up the subscription, + /// one to unsubscribe and, optionally, a third method name used to describe the + /// payload sent back from the server to subscribers. + /// If no override is provided, the subscription method name is used. pub name_override: Option, pub docs: TokenStream2, pub unsubscribe: String, From 48ebf925e84ac16df4c23a231633e324628f78dc Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 19 Nov 2021 19:06:43 +0100 Subject: [PATCH 16/21] Update utils/src/server/rpc_module.rs Co-authored-by: David --- utils/src/server/rpc_module.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/src/server/rpc_module.rs b/utils/src/server/rpc_module.rs index 1ef27b6996..061d63afe9 100644 --- a/utils/src/server/rpc_module.rs +++ b/utils/src/server/rpc_module.rs @@ -541,7 +541,7 @@ impl RpcModule { /// use jsonrpsee_utils::server::rpc_module::RpcModule; /// /// let mut ctx = RpcModule::new(99_usize); - /// ctx.register_subscription("sub", "payload_name", "unsub", |params, mut sink, ctx| { + /// ctx.register_subscription("sub", "notif_name", "unsub", |params, mut sink, ctx| { /// let x: usize = params.one()?; /// std::thread::spawn(move || { /// let sum = x + (*ctx); From b01fbe83f0deb0319b20e1c6e646f3f6b67baf6c Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 19 Nov 2021 19:06:52 +0100 Subject: [PATCH 17/21] Update utils/src/server/rpc_module.rs Co-authored-by: David --- utils/src/server/rpc_module.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/utils/src/server/rpc_module.rs b/utils/src/server/rpc_module.rs index 061d63afe9..294b7f9a72 100644 --- a/utils/src/server/rpc_module.rs +++ b/utils/src/server/rpc_module.rs @@ -516,20 +516,19 @@ impl RpcModule { Ok(MethodResourcesBuilder { build: ResourceVec::new(), callback }) } - /// Register a new RPC subscription that invokes callback on every subscription call. + /// Register a new RPC subscription that invokes s callback on every subscription call. /// - /// This method ensures that the `subscription_method_name` and `unsubscription_method_name` are unique - /// but does not check that `notif_method_name` is unique. - /// - /// Thus, if you want to use a different method name in the subscription make sure that - /// names are unique. + /// This method ensures that the `subscription_method_name` and `unsubscription_method_name` are unique. + /// The `notif_method_name` argument sets the content of the `method` field in the JSON document that + /// the server sends back to the client. The uniqueness of this value is not machine checked and it's up to + /// the user to ensure it is not used in any other [`RpcModule`] used in the server. /// /// # Arguments /// /// * `subscription_method_name` - name of the method to call to initiate a subscription /// * `notif_method_name` - name of method to be used in the subscription payload (technically a JSON-RPC notification) /// * `unsubscription_method` - name of the method to call to terminate a subscription - /// * `callback` - A callback to invoke on each subscrption, it takes three parameters + /// * `callback` - A callback to invoke on each subscription; it takes three parameters: /// - [`Params`]: JSON-RPC parameters in the subscription call. /// - [`SubscriptionSink`]: A sink to send messages to the subscriber. /// - Context: Any type that can be embedded into the [`RpcModule`]. From a1a95772eb7a5b7ec77ee552522b5f02ab23b691 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 19 Nov 2021 19:07:38 +0100 Subject: [PATCH 18/21] Update proc-macros/src/rpc_macro.rs Co-authored-by: David --- proc-macros/src/rpc_macro.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proc-macros/src/rpc_macro.rs b/proc-macros/src/rpc_macro.rs index d880b9a428..ca33b51139 100644 --- a/proc-macros/src/rpc_macro.rs +++ b/proc-macros/src/rpc_macro.rs @@ -101,7 +101,7 @@ pub struct RpcSubscription { /// in the JSON data sent to subscribers. /// Each subscription thus has one method name to set up the subscription, /// one to unsubscribe and, optionally, a third method name used to describe the - /// payload sent back from the server to subscribers. + /// payload (aka "notification") sent back from the server to subscribers. /// If no override is provided, the subscription method name is used. pub name_override: Option, pub docs: TokenStream2, From 86b06edf8866493536341b02c8c432b6e9499f50 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 19 Nov 2021 19:07:49 +0100 Subject: [PATCH 19/21] Update proc-macros/src/render_server.rs Co-authored-by: David --- proc-macros/src/render_server.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proc-macros/src/render_server.rs b/proc-macros/src/render_server.rs index c277801921..d6fb594741 100644 --- a/proc-macros/src/render_server.rs +++ b/proc-macros/src/render_server.rs @@ -186,7 +186,7 @@ impl RpcDescription { check_name(&rpc_sub_name, rust_method_name.span()); check_name(&rpc_unsub_name, rust_method_name.span()); - let rpc_notif_name = match rpc_sub_name_override { + let rpc_notif_name = match rpc_notif_name_override { Some(notif) => { check_name(¬if, rust_method_name.span()); notif From b76efe8849f80e68c53d524e2bc9aa3f15c7bc24 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 19 Nov 2021 19:07:59 +0100 Subject: [PATCH 20/21] Update proc-macros/src/render_server.rs Co-authored-by: David --- proc-macros/src/render_server.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proc-macros/src/render_server.rs b/proc-macros/src/render_server.rs index d6fb594741..68d4b7e816 100644 --- a/proc-macros/src/render_server.rs +++ b/proc-macros/src/render_server.rs @@ -175,7 +175,7 @@ impl RpcDescription { // Name of the RPC method to subscribe to (e.g. `foo_sub`). let rpc_sub_name = self.rpc_identifier(&sub.name); // Name of `method` in the subscription response. - let rpc_sub_name_override = sub.name_override.as_ref().map(|m| self.rpc_identifier(m)); + let rpc_notif_name_override = sub.notif_name_override.as_ref().map(|m| self.rpc_identifier(m)); // Name of the RPC method to unsubscribe (e.g. `foo_sub`). let rpc_unsub_name = self.rpc_identifier(&sub.unsubscribe); // `parsing` is the code associated with parsing structure from the From b20a0c3d6b93675bfc71febc5a13f9243e12af20 Mon Sep 17 00:00:00 2001 From: Niklas Adolfsson Date: Fri, 19 Nov 2021 19:23:13 +0100 Subject: [PATCH 21/21] more grumbles --- proc-macros/src/rpc_macro.rs | 6 +++--- .../tests/ui/incorrect/sub/sub_name_override.rs | 10 ++++++++++ .../tests/ui/incorrect/sub/sub_name_override.stderr | 5 +++++ 3 files changed, 18 insertions(+), 3 deletions(-) create mode 100644 proc-macros/tests/ui/incorrect/sub/sub_name_override.rs create mode 100644 proc-macros/tests/ui/incorrect/sub/sub_name_override.stderr diff --git a/proc-macros/src/rpc_macro.rs b/proc-macros/src/rpc_macro.rs index ca33b51139..156f2f051b 100644 --- a/proc-macros/src/rpc_macro.rs +++ b/proc-macros/src/rpc_macro.rs @@ -103,7 +103,7 @@ pub struct RpcSubscription { /// one to unsubscribe and, optionally, a third method name used to describe the /// payload (aka "notification") sent back from the server to subscribers. /// If no override is provided, the subscription method name is used. - pub name_override: Option, + pub notif_name_override: Option, pub docs: TokenStream2, pub unsubscribe: String, pub params: Vec<(syn::PatIdent, syn::Type)>, @@ -122,7 +122,7 @@ impl RpcSubscription { let aliases = parse_aliases(aliases)?; let map = name?.value::()?; let name = map.name; - let name_override = map.mapped; + let notif_name_override = map.mapped; let item = item?.value()?; let param_kind = parse_param_kind(param_kind)?; let unsubscribe_aliases = parse_aliases(unsubscribe_aliases)?; @@ -148,7 +148,7 @@ impl RpcSubscription { Ok(Self { name, - name_override, + notif_name_override, unsubscribe, unsubscribe_aliases, params, diff --git a/proc-macros/tests/ui/incorrect/sub/sub_name_override.rs b/proc-macros/tests/ui/incorrect/sub/sub_name_override.rs new file mode 100644 index 0000000000..740b30699f --- /dev/null +++ b/proc-macros/tests/ui/incorrect/sub/sub_name_override.rs @@ -0,0 +1,10 @@ +use jsonrpsee::{proc_macros::rpc, types::RpcResult}; + +// Subscription method name conflict with notif override. +#[rpc(client, server)] +pub trait DupName { + #[subscription(name = "one" => "one", item = u8)] + fn one(&self) -> RpcResult<()>; +} + +fn main() {} diff --git a/proc-macros/tests/ui/incorrect/sub/sub_name_override.stderr b/proc-macros/tests/ui/incorrect/sub/sub_name_override.stderr new file mode 100644 index 0000000000..719b2e88cf --- /dev/null +++ b/proc-macros/tests/ui/incorrect/sub/sub_name_override.stderr @@ -0,0 +1,5 @@ +error: "one" is already defined + --> tests/ui/incorrect/sub/sub_name_override.rs:7:5 + | +7 | fn one(&self) -> RpcResult<()>; + | ^^^