Skip to content
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

Method aliases + RpcModule: Clone #383

Merged
merged 8 commits into from
Jun 18, 2021
Merged

Conversation

maciejhirsz
Copy link
Contributor

@maciejhirsz
Copy link
Contributor Author

maciejhirsz commented Jun 15, 2021

If we change the design a bit so that the server uses Methods instead of RpcModule, we can remove Clone from RpcModule and get rid of the whole Arc::make_mut I reckon. What do you think @niklasad1?

Copy link
Collaborator

@jsdw jsdw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

I hadn't used Arc::make_mut before; that's a handy function to know about!

Co-authored-by: James Wilson <james@jsdw.me>
@@ -101,15 +102,22 @@ impl Methods {
Ok(())
}

/// Helper for obtaining a mut ref to the callbacks HashMap.
fn mut_callbacks(&mut self) -> &mut FxHashMap<&'static str, MethodCallback> {
Arc::make_mut(&mut self.callbacks)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

neat TIL.

pub fn into_methods(self) -> Methods {
self.methods
}
/// Register an `alias` name for an `existing_method`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

ws-server/src/server.rs Outdated Show resolved Hide resolved
@niklasad1
Copy link
Member

If we change the design a bit so that the server uses Methods instead of RpcModule, we can remove Clone from RpcModule and get rid of the whole Arc::make_mut I reckon. What do you think @niklasad1?

I like it as long as we don't have to provide duplicated methods on it as before i.e, register_method, register_subscriptions and so on

Copy link
Member

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo the Arc::ref_count comment

@niklasad1 niklasad1 merged commit 6c69a8c into master Jun 18, 2021
@niklasad1 niklasad1 deleted the mh-make-rpc-module-clone branch June 18, 2021 11:31
dvdplm added a commit that referenced this pull request Jul 6, 2021
* master: (21 commits)
  New proc macro (#387)
  Streaming RpcParams parsing (#401)
  Set allowed Host header values (#399)
  Synchronization-less async connections in ws-server (#388)
  [ws server]: terminate already established connection(s) when the server is stopped (#396)
  feat: customizable JSON-RPC error codes via new enum variant on `CallErrror` (#394)
  [ci]: test each individual crate's manifest (#392)
  Add a way to stop servers (#386)
  [jsonrpsee types]: unify a couple of types + more tests (#389)
  Update roadmap link in readme (#390)
  Cross-origin protection (#375)
  Method aliases + RpcModule: Clone (#383)
  Use criterion's async bencher (#385)
  Async/subscription benches (#372)
  send text (#374)
  Fix link to ws server in README.md (#373)
  Concat -> simple push (#370)
  Add missing `rt` feature (#369)
  Release prep for v0.2 (#368)
  chore(scripts): publish script (#354)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants