-
Notifications
You must be signed in to change notification settings - Fork 205
server: Register raw method with connection ID #1297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
26ec635
ef24148
4f8eb03
9eba899
239cafe
0de6d95
4ce51c6
5f2b40a
753224a
d526b70
c8ff969
b4880b9
d2da4b0
0dc48e9
0029418
3a09a7e
1565927
f5202ef
8cf1947
bd571e1
04dfd25
e02ba61
7b72d8a
87585ad
2c50375
66b6a39
cfe1aec
7327d8b
6605409
650e24a
f4d9085
40d2bcf
8185193
11ef18c
4f459ac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
Signed-off-by: Alexandru Vasile <[email protected]>
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -60,6 +60,9 @@ pub type SyncMethod = Arc<dyn Send + Sync + Fn(Id, Params, MaxResponseSize) -> M | |||||||
| /// Similar to [`SyncMethod`], but represents an asynchronous handler. | ||||||||
| pub type AsyncMethod<'a> = | ||||||||
| Arc<dyn Send + Sync + Fn(Id<'a>, Params<'a>, ConnectionId, MaxResponseSize) -> BoxFuture<'a, MethodResponse>>; | ||||||||
| /// Similar to [`SyncMethod`], but represents an asynchronous handler with connection details. | ||||||||
| pub type AsyncMethodWithDetails<'a> = | ||||||||
| Arc<dyn Send + Sync + Fn(Id<'a>, Params<'a>, ConnectionDetails, MaxResponseSize) -> BoxFuture<'a, MethodResponse>>; | ||||||||
| /// Similar to [`SyncMethod`], but represents a raw handler that has access to the connection Id. | ||||||||
niklasad1 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||
| /// Method callback for subscriptions. | ||||||||
| pub type SubscriptionMethod<'a> = | ||||||||
|
|
@@ -80,15 +83,17 @@ pub type MaxResponseSize = usize; | |||||||
| /// - a [`mpsc::UnboundedReceiver<String>`] to receive future subscription results | ||||||||
| pub type RawRpcResponse = (String, mpsc::Receiver<String>); | ||||||||
|
|
||||||||
| #[derive(Debug, Clone)] | ||||||||
| #[allow(missing_copy_implementations)] | ||||||||
| /// The connection details exposed to the server methods. | ||||||||
| pub struct ConnectionDetails { | ||||||||
| id: ConnectionId, | ||||||||
| } | ||||||||
|
|
||||||||
| impl ConnectionDetails { | ||||||||
| /// Construct a new [`ConnectionDetails`] with the given connection ID. | ||||||||
| pub(crate) fn new(id: ConnectionId) -> Self { | ||||||||
| Self { id } | ||||||||
| /// Construct a new [`ConnectionDetailsBuilder`]. | ||||||||
| pub fn builder() -> ConnectionDetailsBuilder { | ||||||||
| ConnectionDetailsBuilder { id: 0 } | ||||||||
| } | ||||||||
|
|
||||||||
| /// Get the connection ID. | ||||||||
|
|
@@ -97,6 +102,26 @@ impl ConnectionDetails { | |||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| #[derive(Debug, Clone)] | ||||||||
| #[allow(missing_copy_implementations)] | ||||||||
| /// The connection details exposed to the server methods. | ||||||||
|
||||||||
| let connection_details = ConnectionDetails::builder().id(conn_id).build(); | |
| let fut = (callback)(id, params, connection_details, max_response_body_size); | |
| ResponseFuture::future(fut) |
I think we could just as easily provide a constructor for this:
impl ConnectionDetails {
pub fn new(id: ConnectionId) ..
}The new method needs to be public, because the ConnectionDetails is declared in the core crate. And the ConnectionDetails needs to be instantiated in the server crate as well.
We'd have to eventually change this method to new(id: ConnectionId, extensions: Extenstions) which will be a breaking change (although we should be the only ones building this object).
TLDR; since we'll do a breaking change either way, I think a new method should be fine for now! 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I see makes sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe have it be #[doc(hidden)] and comment that it's not part of the public APi and can change without notice? I don't think anything outside of jsonrpsee needs to build it, so then we could also just use new() instead of a builder (Or maybe eg _new(connection_id) to really emphasise the hiddenness of it but shrug)
Uh oh!
There was an error while loading. Please reload this page.