Refactor command to make is shorter

Description

I noticed that most functions of commands had a similar shape, so I made it more compact by making calls to the same function for all the commands, with different parameters

List of changes

• Factorize the match.receiver by writing a new async function
• Write code to convert to serialize (which was the main thing that didn't allow the previous point)
• Write a macro to further factorize the code that was left in functions

Additional notes

I think that the first two changes should be kept as it's pretty clear was it is done and it organizes the code better. For the macro part, while it indeed shortens the code, macros are generally a bit harder to read and maintain, so we may want to keep code very similar between functions.

The reason for using a macro is that the number of arguments for each variant of DragoonCommand is not always the same, so factorizing means writing a function with a variable number of parameters (thus the macro).

I think it would be interesting to run this first after building it, to check that everything works as intended (since we don't have tests yet).

src/commands.rs

@@ -44,103 +44,103 @@ use crate::to_serialize::{ConvertSer, JsonWrapper};
 type Sender<T> = oneshot::Sender<Result<T, Box<dyn Error + Send>>>;
 #[derive(Debug)]
 pub(crate) enum DragoonCommand {
-    Listen {
-        multiaddr: String,
-        sender: Sender<u64>,
-    },
-    GetListeners {
-        sender: Sender<Vec<Multiaddr>>,
-    },
-    GetPeerId {
-        sender: Sender<PeerId>,
-    },
-    GetNetworkInfo {
-        sender: Sender<NetworkInfo>,
-    },
-    RemoveListener {
-        listener_id: u64,
-        sender: Sender<bool>,
-    },
-    GetConnectedPeers {
-        sender: Sender<Vec<PeerId>>,
+    #[cfg(feature = "file-sharing")]
+    AddFile {
+        file: Vec<u8>,
+        channel: ResponseChannel<FileResponse>,
     },
-    Dial {
+    AddPeer {
         multiaddr: String,
         sender: Sender<()>,
     },
-    AddPeer {
-        multiaddr: String,
+    Bootstrap {
         sender: Sender<()>,
     },
-    StartProvide {
-        key: String,
+    Dial {
+        multiaddr: String,
         sender: Sender<()>,
     },
-    GetProviders {
+    DragoonGet {
+        peerid: String,
         key: String,
+        sender: Sender<Vec<u8>>,
+    },
+    DragoonPeers {
         sender: Sender<HashSet<PeerId>>,
     },
-    Bootstrap {
+    DragoonSend {
+        data: String,
+        peerid: String,
         sender: Sender<()>,
     },
+    GetConnectedPeers {
+        sender: Sender<Vec<PeerId>>,
+    },
     #[cfg(feature = "file-sharing")]
     GetFile {
         key: String,
         peer: PeerId,
         sender: Sender<Vec<u8>>,
     },
-    #[cfg(feature = "file-sharing")]
-    AddFile {
-        file: Vec<u8>,
-        channel: ResponseChannel<FileResponse>,
+    GetListeners {
+        sender: Sender<Vec<Multiaddr>>,
     },
-    PutRecord {
+    GetNetworkInfo {
+        sender: Sender<NetworkInfo>,
+    },
+    GetPeerId {
+        sender: Sender<PeerId>,
+    },
+    GetProviders {
         key: String,
-        value: Vec<u8>,
-        sender: Sender<()>,
+        sender: Sender<HashSet<PeerId>>,
     },
     GetRecord {
         key: String,
         sender: Sender<Vec<u8>>,
     },
-    DragoonPeers {
-        sender: Sender<HashSet<PeerId>>,
+    Listen {
+        multiaddr: String,
+        sender: Sender<u64>,
     },
-    DragoonSend {
-        data: String,
-        peerid: String,
+    PutRecord {
+        key: String,
+        value: Vec<u8>,
         sender: Sender<()>,
     },
-    DragoonGet {
-        peerid: String,
+    RemoveListener {
+        listener_id: u64,
+        sender: Sender<bool>,
+    },
+    StartProvide {
         key: String,
-        sender: Sender<Vec<u8>>,
+        sender: Sender<()>,
     },
 }
 
 impl std::fmt::Display for DragoonCommand {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
         match self {
-            DragoonCommand::Listen { .. } => write!(f, "listen"),
-            DragoonCommand::GetListeners { .. } => write!(f, "get-listener"),
-            DragoonCommand::GetPeerId { .. } => write!(f, "get-peer-id"),
-            DragoonCommand::GetNetworkInfo { .. } => write!(f, "get-network-info"),
-            DragoonCommand::RemoveListener { .. } => write!(f, "remove-listener"),
-            DragoonCommand::GetConnectedPeers { .. } => write!(f, "get-connected-peers"),
-            DragoonCommand::Dial { .. } => write!(f, "dial"),
+            #[cfg(feature = "file-sharing")]
+            DragoonCommand::AddFile { .. } => write!(f, "add-file"),
             DragoonCommand::AddPeer { .. } => write!(f, "add-peer"),
-            DragoonCommand::StartProvide { .. } => write!(f, "start-provide"),
-            DragoonCommand::GetProviders { .. } => write!(f, "get-providers"),
             DragoonCommand::Bootstrap { .. } => write!(f, "bootstrap"),
+            DragoonCommand::Dial { .. } => write!(f, "dial"),
+            DragoonCommand::DragoonGet { .. } => write!(f, "dragoon-get"),
+            DragoonCommand::DragoonPeers { .. } => write!(f, "dragoon-peers"),
+            DragoonCommand::DragoonSend { .. } => write!(f, "dragoon-send"),
+            DragoonCommand::GetConnectedPeers { .. } => write!(f, "get-connected-peers"),
             #[cfg(feature = "file-sharing")]
             DragoonCommand::GetFile { .. } => write!(f, "get-file"),
-            #[cfg(feature = "file-sharing")]
-            DragoonCommand::AddFile { .. } => write!(f, "add-file"),
-            DragoonCommand::PutRecord { .. } => write!(f, "put-record"),
+            DragoonCommand::GetListeners { .. } => write!(f, "get-listener"),
+            DragoonCommand::GetPeerId { .. } => write!(f, "get-peer-id"),
+            DragoonCommand::GetNetworkInfo { .. } => write!(f, "get-network-info"),
+            DragoonCommand::GetProviders { .. } => write!(f, "get-providers"),
             DragoonCommand::GetRecord { .. } => write!(f, "get-record"),
-            DragoonCommand::DragoonPeers { .. } => write!(f, "dragoon-peers"),
-            DragoonCommand::DragoonSend { .. } => write!(f, "dragoon-send"),
-            DragoonCommand::DragoonGet { .. } => write!(f, "dragoon-get"),
+            DragoonCommand::Listen { .. } => write!(f, "listen"),
+            DragoonCommand::PutRecord { .. } => write!(f, "put-record"),
+            DragoonCommand::RemoveListener { .. } => write!(f, "remove-listener"),
+            DragoonCommand::StartProvide { .. } => write!(f, "start-provide"),
         }
     }
 }
@@ -192,70 +192,48 @@ macro_rules! dragoon_command {
 }
 
 // dragoon_command(state, DragoonCommand::Something, peerid, data)
+// Implementation of dragoon commands
 
-pub(crate) async fn listen(
-    Path(multiaddr): Path<String>,
+#[cfg(feature = "file-sharing")]
+pub(crate) async fn add_file(
+    Path((key, content)): Path<(String, String)>,
     State(state): State<Arc<AppState>>,
 ) -> Response {
-    info!("running command `listen`");
-    dragoon_command!(state, Listen, multiaddr)
-}
-
-pub(crate) async fn get_listeners(State(state): State<Arc<AppState>>) -> Response {
-    info!("running command `get_listeners`");
-    dragoon_command!(state, GetListeners)
-}
-
-pub(crate) async fn get_peer_id(State(state): State<Arc<AppState>>) -> Response {
-    info!("running command `get_peer_id`");
-    dragoon_command!(state, GetPeerId)
-}
-
-#[derive(Serialize, Deserialize)]
-pub(crate) struct SerNetworkInfo {
-    peers: usize,
-    pending: u32,
-    connections: u32,
-    established: u32,
-    pending_incoming: u32,
-    pending_outgoing: u32,
-    established_incoming: u32,
-    established_outgoing: u32,
-}
-// as each field of SerNetworkInfo is Serialize, SerNetworkInfo becomes Serialize by extension
+    info!(
+        "running command `add_file`: key = {}, content = {}",
+        key, content
+    );
+    let mut event_receiver = state.event_receiver.lock().await;
 
-impl SerNetworkInfo {
-    pub(crate) fn new(network_info: &NetworkInfo) -> Self {
-        let connections = network_info.connection_counters();
-        SerNetworkInfo {
-            peers: network_info.num_peers(),
-            pending: connections.num_pending(),
-            connections: connections.num_connections(),
-            established: connections.num_established(),
-            pending_incoming: connections.num_pending_incoming(),
-            pending_outgoing: connections.num_pending_outgoing(),
-            established_incoming: connections.num_established_incoming(),
-            established_outgoing: connections.num_established_outgoing(),
+    loop {
+        match event_receiver.next().await {
+            Some(DragoonEvent::InboundRequest { channel, request }) => {
+                debug!("add_file: request '{}'", request);
+                if request == key {
+                    debug!("add_file: request accepted");
+                    let cmd = DragoonCommand::AddFile {
+                        file: content.as_bytes().to_vec(),
+                        channel,
+                    };
+                    send_command(cmd, state.clone()).await;
+                }
+            }
+            e => todo!("{:?}", e),
         }
     }
 }
 
-pub(crate) async fn get_network_info(State(state): State<Arc<AppState>>) -> Response {
-    info!("running command `get_network_info`");
-    dragoon_command!(state, GetNetworkInfo)
-}
-
-pub(crate) async fn remove_listener(
-    Path(listener_id): Path<u64>,
+pub(crate) async fn add_peer(
+    Path(multiaddr): Path<String>,
     State(state): State<Arc<AppState>>,
 ) -> Response {
-    info!("running command `remove_listener`");
-    dragoon_command!(state, RemoveListener, listener_id)
+    info!("running command `add_peer`");
+    dragoon_command!(state, AddPeer, multiaddr)
 }
 
-pub(crate) async fn get_connected_peers(State(state): State<Arc<AppState>>) -> Response {
-    info!("running command `get_connected_peers`");
-    dragoon_command!(state, GetConnectedPeers)
+pub(crate) async fn bootstrap(State(state): State<Arc<AppState>>) -> Response {
+    info!("running command `bootstrap`");
+    dragoon_command!(state, Bootstrap)
 }
 
 pub(crate) async fn dial(
@@ -266,20 +244,12 @@ pub(crate) async fn dial(
     dragoon_command!(state, Dial, multiaddr)
 }
 
-pub(crate) async fn add_peer(
-    Path(multiaddr): Path<String>,
-    State(state): State<Arc<AppState>>,
-) -> Response {
-    info!("running command `add_peer`");
-    dragoon_command!(state, AddPeer, multiaddr)
-}
-
-pub(crate) async fn start_provide(
-    Path(key): Path<String>,
+pub(crate) async fn dragoon_get(
+    Path((peerid, key)): Path<(String, String)>,
     State(state): State<Arc<AppState>>,
 ) -> Response {
-    info!("running command `start_provide`");
-    dragoon_command!(state, StartProvide, key)
+    info!("running command `dragoon_get`");
+    dragoon_command!(state, DragoonGet, peerid, key)
 }
 
 pub(crate) async fn dragoon_peers(State(state): State<Arc<AppState>>) -> Response {
@@ -295,25 +265,9 @@ pub(crate) async fn dragoon_send(
     dragoon_command!(state, DragoonSend, data, peerid)
 }
 
-pub(crate) async fn dragoon_get(
-    Path((peerid, key)): Path<(String, String)>,
-    State(state): State<Arc<AppState>>,
-) -> Response {
-    info!("running command `dragoon_get`");
-    dragoon_command!(state, DragoonGet, peerid, key)
-}
-
-pub(crate) async fn get_providers(
-    Path(key): Path<String>,
-    State(state): State<Arc<AppState>>,
-) -> Response {
-    info!("running command `get_providers`");
-    dragoon_command!(state, GetProviders, key)
-}
-
-pub(crate) async fn bootstrap(State(state): State<Arc<AppState>>) -> Response {
-    info!("running command `bootstrap`");
-    dragoon_command!(state, Bootstrap)
+pub(crate) async fn get_connected_peers(State(state): State<Arc<AppState>>) -> Response {
+    info!("running command `get_connected_peers`");
+    dragoon_command!(state, GetConnectedPeers)
 }
 
 #[cfg(feature = "file-sharing")]
@@ -357,35 +311,74 @@ pub(crate) async fn get_file(
     command_res_match(receiver, cmd_name).await
 }
 
-#[cfg(feature = "file-sharing")]
-pub(crate) async fn add_file(
-    Path((key, content)): Path<(String, String)>,
+pub(crate) async fn get_listeners(State(state): State<Arc<AppState>>) -> Response {
+    info!("running command `get_listeners`");
+    dragoon_command!(state, GetListeners)
+}
+
+pub(crate) async fn get_peer_id(State(state): State<Arc<AppState>>) -> Response {
+    info!("running command `get_peer_id`");
+    dragoon_command!(state, GetPeerId)
+}
+
+pub(crate) async fn get_providers(
+    Path(key): Path<String>,
     State(state): State<Arc<AppState>>,
 ) -> Response {
-    info!(
-        "running command `add_file`: key = {}, content = {}",
-        key, content
-    );
-    let mut event_receiver = state.event_receiver.lock().await;
+    info!("running command `get_providers`");
+    dragoon_command!(state, GetProviders, key)
+}
 
-    loop {
-        match event_receiver.next().await {
-            Some(DragoonEvent::InboundRequest { channel, request }) => {
-                debug!("add_file: request '{}'", request);
-                if request == key {
-                    debug!("add_file: request accepted");
-                    let cmd = DragoonCommand::AddFile {
-                        file: content.as_bytes().to_vec(),
-                        channel,
-                    };
-                    send_command(cmd, state.clone()).await;
-                }
-            }
-            e => todo!("{:?}", e),
+#[derive(Serialize, Deserialize)]
+pub(crate) struct SerNetworkInfo {
+    peers: usize,
+    pending: u32,
+    connections: u32,
+    established: u32,
+    pending_incoming: u32,
+    pending_outgoing: u32,
+    established_incoming: u32,
+    established_outgoing: u32,
+}
+// as each field of SerNetworkInfo is Serialize, SerNetworkInfo becomes Serialize by extension
+
+impl SerNetworkInfo {
+    pub(crate) fn new(network_info: &NetworkInfo) -> Self {
+        let connections = network_info.connection_counters();
+        SerNetworkInfo {
+            peers: network_info.num_peers(),
+            pending: connections.num_pending(),
+            connections: connections.num_connections(),
+            established: connections.num_established(),
+            pending_incoming: connections.num_pending_incoming(),
+            pending_outgoing: connections.num_pending_outgoing(),
+            established_incoming: connections.num_established_incoming(),
+            established_outgoing: connections.num_established_outgoing(),
         }
     }
 }
 
+pub(crate) async fn get_network_info(State(state): State<Arc<AppState>>) -> Response {
+    info!("running command `get_network_info`");
+    dragoon_command!(state, GetNetworkInfo)
+}
+
+pub(crate) async fn get_record(
+    Path(key): Path<String>,
+    State(state): State<Arc<AppState>>,
+) -> Response {
+    info!("running command `get_record`");
+    dragoon_command!(state, GetRecord, key)
+}
+
+pub(crate) async fn listen(
+    Path(multiaddr): Path<String>,
+    State(state): State<Arc<AppState>>,
+) -> Response {
+    info!("running command `listen`");
+    dragoon_command!(state, Listen, multiaddr)
+}
+
 pub(crate) async fn put_record(
     Path((key, value)): Path<(String, String)>,
     State(state): State<Arc<AppState>>,
@@ -395,14 +388,24 @@ pub(crate) async fn put_record(
     dragoon_command!(state, PutRecord, key, value)
 }
 
-pub(crate) async fn get_record(
+pub(crate) async fn remove_listener(
+    Path(listener_id): Path<u64>,
+    State(state): State<Arc<AppState>>,
+) -> Response {
+    info!("running command `remove_listener`");
+    dragoon_command!(state, RemoveListener, listener_id)
+}
+
+pub(crate) async fn start_provide(
     Path(key): Path<String>,
     State(state): State<Arc<AppState>>,
 ) -> Response {
-    info!("running command `get_record`");
-    dragoon_command!(state, GetRecord, key)
+    info!("running command `start_provide`");
+    dragoon_command!(state, StartProvide, key)
 }
 
+// End of dragoon command implementation
+
 fn handle_dragoon_error(err: Box<dyn Error + Send>, command: &str) -> Response {
     if let Ok(dragoon_error) = err.downcast::<DragoonError>() {
         dragoon_error.into_response()