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

Support overriding existing Methods Callbacks #1053

Open
mattsse opened this issue Mar 22, 2023 · 3 comments · May be fixed by #1387
Open

Support overriding existing Methods Callbacks #1053

mattsse opened this issue Mar 22, 2023 · 3 comments · May be fixed by #1387

Comments

@mattsse
Copy link
Contributor

mattsse commented Mar 22, 2023

Currently, it's not possible to override MethodCallback in Methods because Methods::merge fails early if a callback with the name already exists

for name in other.callbacks.keys() {
self.verify_method_name(name)?;
}

This makes it impossible to disable various callbacks dynamically when RpcModules (via server proc macro's into_rpc) are merged.

now that verify_method_name is public this could be solved by:

  • support Methods::iter and Methods::into_iter so callbacks can be inserted individually.
  • support Methods::merge_replace which replaces existing handlers
@niklasad1
Copy link
Member

Hey @mattsse

The methods are really "static" i.e, after the server has been started then it's not possible change or override the methods.
Can you explain with an example what you want to do?

I think Methods::iter and Method::merge_replace make sense.

@mattsse
Copy link
Contributor Author

mattsse commented Mar 28, 2023

The methods are really "static" i.e, after the server has been started then it's not possible change or override the methods.

right, I'm talking about the setup phase.

for example, you have a bunch of server traits and default implementations that are then merged via:

#[rpc(server))]
pub trait MyApi {
  async fn do_stuff() {}
}

#[rpc(server))]
pub trait MyApi2 {
  async fn do_other_stuff() {}
  
}

fn default_setup () -> RpcModule {
   let mut module = RpcModule::new(());
   module.merge(MyApi::new().into_rpc()).unwrap();
   module.merge(MyApi2::new().into_rpc()).unwrap();
   module
}

now RpcModule is a list of methods.

it would be useful to now replace certain methods with something like

#[rpc(server))]
pub trait Unsupported {
  async fn do_stuff() {
    return Err("Unsupported method: do_stuff")
  }
}

fn customize(m: Unsupported) -> RpcModule {
  default_setup().merge_replace(m.into_rpc())

}

So the motivation behind this is to make it easier to selectively replace certain methods in the default config.

I think Methods::iter and Method::merge_replace make sense.

cool, I can try adding these

@niklasad1
Copy link
Member

sounds good 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants