From 410a35948af5559b96756a2d65255e4a91dfd0b6 Mon Sep 17 00:00:00 2001 From: SkandaBhat Date: Sat, 24 Aug 2024 13:15:05 +0530 Subject: [PATCH 1/5] feat(rpc): Add method removal functionality for RPC transports --- crates/rpc/rpc-builder/src/lib.rs | 122 ++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) diff --git a/crates/rpc/rpc-builder/src/lib.rs b/crates/rpc/rpc-builder/src/lib.rs index 3deaba61864b..7d723d33dac9 100644 --- a/crates/rpc/rpc-builder/src/lib.rs +++ b/crates/rpc/rpc-builder/src/lib.rs @@ -1705,6 +1705,50 @@ impl TransportRpcModules { self.merge_ipc(other)?; Ok(()) } + + /// Removes the method with the given name from the configured http methods. + /// + /// Returns `true` if the method was found and removed, `false` otherwise. + pub fn remove_http_method(&mut self, method_name: &'static str) -> bool { + if let Some(http_module) = &mut self.http { + http_module.remove_method(method_name).is_some() + } else { + false + } + } + + /// Removes the method with the given name from the configured ws methods. + /// + /// Returns `true` if the method was found and removed, `false` otherwise. + pub fn remove_ws_method(&mut self, method_name: &'static str) -> bool { + if let Some(ws_module) = &mut self.ws { + ws_module.remove_method(method_name).is_some() + } else { + false + } + } + + /// Removes the method with the given name from the configured ipc methods. + /// + /// Returns `true` if the method was found and removed, `false` otherwise. + pub fn remove_ipc_method(&mut self, method_name: &'static str) -> bool { + if let Some(ipc_module) = &mut self.ipc { + ipc_module.remove_method(method_name).is_some() + } else { + false + } + } + + /// Removes the method with the given name from all configured transports. + /// + /// Returns `true` if the method was found and removed, `false` otherwise. + pub fn remove_method_from_all_transports(&mut self, method_name: &'static str) -> bool { + let http_removed = self.remove_http_method(method_name); + let ws_removed = self.remove_ws_method(method_name); + let ipc_removed = self.remove_ipc_method(method_name); + + http_removed || ws_removed || ipc_removed + } } /// A handle to the spawned servers. @@ -1967,4 +2011,82 @@ mod tests { } ) } + + mod remove_methods { + use super::*; + + fn create_test_module() -> RpcModule<()> { + let mut module = RpcModule::new(()); + module.register_method("anything", |_, _, _| "succeed").unwrap(); + module + } + + #[test] + fn test_remove_http_method() { + let mut modules = + TransportRpcModules { http: Some(create_test_module()), ..Default::default() }; + // Remove a method that exists + assert!(modules.remove_http_method("anything")); + + // Remove a method that does not exist + assert!(!modules.remove_http_method("non_existent_method")); + + // Verify that the method was removed + assert!(modules.http.as_ref().unwrap().method("anything").is_none()); + } + + #[test] + fn test_remove_ws_method() { + let mut modules = + TransportRpcModules { ws: Some(create_test_module()), ..Default::default() }; + + // Remove a method that exists + assert!(modules.remove_ws_method("anything")); + + // Remove a method that does not exist + assert!(!modules.remove_ws_method("non_existent_method")); + + // Verify that the method was removed + assert!(modules.ws.as_ref().unwrap().method("anything").is_none()); + } + + #[test] + fn test_remove_ipc_method() { + let mut modules = + TransportRpcModules { ipc: Some(create_test_module()), ..Default::default() }; + + // Remove a method that exists + assert!(modules.remove_ipc_method("anything")); + + // Remove a method that does not exist + assert!(!modules.remove_ipc_method("non_existent_method")); + + // Verify that the method was removed + assert!(modules.ipc.as_ref().unwrap().method("anything").is_none()); + } + + #[test] + fn test_remove_method_from_all_transports() { + let mut modules = TransportRpcModules { + http: Some(create_test_module()), + ws: Some(create_test_module()), + ipc: Some(create_test_module()), + ..Default::default() + }; + + // Remove a method that exists + assert!(modules.remove_method_from_all_transports("anything")); + + // Remove a method that was just removed (it does not exist anymore) + assert!(!modules.remove_method_from_all_transports("anything")); + + // Remove a method that does not exist + assert!(!modules.remove_method_from_all_transports("non_existent_method")); + + // Verify that the method was removed from all transports + assert!(modules.http.as_ref().unwrap().method("anything").is_none()); + assert!(modules.ws.as_ref().unwrap().method("anything").is_none()); + assert!(modules.ipc.as_ref().unwrap().method("anything").is_none()); + } + } } From a7d28ce08b6b1bf62285bf5e4919686cf11f72f6 Mon Sep 17 00:00:00 2001 From: Skanda Bhat Date: Sat, 24 Aug 2024 13:29:45 +0530 Subject: [PATCH 2/5] Apply suggestions from code review Co-authored-by: Matthias Seitz --- crates/rpc/rpc-builder/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/rpc/rpc-builder/src/lib.rs b/crates/rpc/rpc-builder/src/lib.rs index 7d723d33dac9..30129a0b7866 100644 --- a/crates/rpc/rpc-builder/src/lib.rs +++ b/crates/rpc/rpc-builder/src/lib.rs @@ -1742,7 +1742,7 @@ impl TransportRpcModules { /// Removes the method with the given name from all configured transports. /// /// Returns `true` if the method was found and removed, `false` otherwise. - pub fn remove_method_from_all_transports(&mut self, method_name: &'static str) -> bool { + pub fn remove_method_from_configured(&mut self, method_name: &'static str) -> bool { let http_removed = self.remove_http_method(method_name); let ws_removed = self.remove_ws_method(method_name); let ipc_removed = self.remove_ipc_method(method_name); From 93a510aa620cb50134a9275842b95a3ffbe6aa9f Mon Sep 17 00:00:00 2001 From: SkandaBhat Date: Sat, 24 Aug 2024 13:43:24 +0530 Subject: [PATCH 3/5] docs(rpc): Add disclaimers for remove_method calls --- crates/rpc/rpc-builder/src/lib.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/rpc/rpc-builder/src/lib.rs b/crates/rpc/rpc-builder/src/lib.rs index 7d723d33dac9..968362f9d6a3 100644 --- a/crates/rpc/rpc-builder/src/lib.rs +++ b/crates/rpc/rpc-builder/src/lib.rs @@ -1709,6 +1709,10 @@ impl TransportRpcModules { /// Removes the method with the given name from the configured http methods. /// /// Returns `true` if the method was found and removed, `false` otherwise. + /// + /// Be aware that a subscription consist of two methods, `subscribe` and `unsubscribe` and + /// it's the caller responsibility to remove both `subscribe` and `unsubscribe` methods for + /// subscriptions. pub fn remove_http_method(&mut self, method_name: &'static str) -> bool { if let Some(http_module) = &mut self.http { http_module.remove_method(method_name).is_some() @@ -1720,6 +1724,10 @@ impl TransportRpcModules { /// Removes the method with the given name from the configured ws methods. /// /// Returns `true` if the method was found and removed, `false` otherwise. + /// + /// Be aware that a subscription consist of two methods, `subscribe` and `unsubscribe` and + /// it's the caller responsibility to remove both `subscribe` and `unsubscribe` methods for + /// subscriptions. pub fn remove_ws_method(&mut self, method_name: &'static str) -> bool { if let Some(ws_module) = &mut self.ws { ws_module.remove_method(method_name).is_some() @@ -1731,6 +1739,10 @@ impl TransportRpcModules { /// Removes the method with the given name from the configured ipc methods. /// /// Returns `true` if the method was found and removed, `false` otherwise. + /// + /// Be aware that a subscription consist of two methods, `subscribe` and `unsubscribe` and + /// it's the caller responsibility to remove both `subscribe` and `unsubscribe` methods for + /// subscriptions. pub fn remove_ipc_method(&mut self, method_name: &'static str) -> bool { if let Some(ipc_module) = &mut self.ipc { ipc_module.remove_method(method_name).is_some() From dd0a71de60404716357c9d9e224f90b9c56c1135 Mon Sep 17 00:00:00 2001 From: SkandaBhat Date: Sat, 24 Aug 2024 13:44:48 +0530 Subject: [PATCH 4/5] chore(rpc): Rename tests --- crates/rpc/rpc-builder/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/rpc/rpc-builder/src/lib.rs b/crates/rpc/rpc-builder/src/lib.rs index 2b77ea471bb7..e85c3026b317 100644 --- a/crates/rpc/rpc-builder/src/lib.rs +++ b/crates/rpc/rpc-builder/src/lib.rs @@ -2078,7 +2078,7 @@ mod tests { } #[test] - fn test_remove_method_from_all_transports() { + fn test_remove_method_from_configured() { let mut modules = TransportRpcModules { http: Some(create_test_module()), ws: Some(create_test_module()), @@ -2087,13 +2087,13 @@ mod tests { }; // Remove a method that exists - assert!(modules.remove_method_from_all_transports("anything")); + assert!(modules.remove_method_from_configured("anything")); // Remove a method that was just removed (it does not exist anymore) - assert!(!modules.remove_method_from_all_transports("anything")); + assert!(!modules.remove_method_from_configured("anything")); // Remove a method that does not exist - assert!(!modules.remove_method_from_all_transports("non_existent_method")); + assert!(!modules.remove_method_from_configured("non_existent_method")); // Verify that the method was removed from all transports assert!(modules.http.as_ref().unwrap().method("anything").is_none()); From cebeefff1a9961bcc53b1ebab4813d91b1f901a7 Mon Sep 17 00:00:00 2001 From: SkandaBhat Date: Sat, 24 Aug 2024 13:51:24 +0530 Subject: [PATCH 5/5] chore(rpc): run fmt --- crates/rpc/rpc-builder/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/rpc/rpc-builder/src/lib.rs b/crates/rpc/rpc-builder/src/lib.rs index e85c3026b317..94a290bd668f 100644 --- a/crates/rpc/rpc-builder/src/lib.rs +++ b/crates/rpc/rpc-builder/src/lib.rs @@ -1739,7 +1739,7 @@ impl TransportRpcModules { /// Removes the method with the given name from the configured ipc methods. /// /// Returns `true` if the method was found and removed, `false` otherwise. - /// + /// /// Be aware that a subscription consist of two methods, `subscribe` and `unsubscribe` and /// it's the caller responsibility to remove both `subscribe` and `unsubscribe` methods for /// subscriptions.