From 442dd4ecbccdec2a84dd3f5c91dc947d560bcc37 Mon Sep 17 00:00:00 2001
From: suoxiaocong <suoxiaocong@kylinos.cn>
Date: Tue, 10 May 2022 16:21:32 +0800
Subject: [PATCH 1/4] simplify Store api, remove unnecessary validation

move container_id into struct Store, makes api more readable
---
 src/client/client.rs | 62 +++++++++++++++++---------------------------
 src/lib.rs           | 22 +++++++---------
 2 files changed, 34 insertions(+), 50 deletions(-)

diff --git a/src/client/client.rs b/src/client/client.rs
index 3f231f2..aa1c7c1 100644
--- a/src/client/client.rs
+++ b/src/client/client.rs
@@ -25,6 +25,7 @@ use ttrpc::client::Client;
 #[derive(Clone)]
 pub struct Store {
     conn: Client,
+    container_id: String,
     timeout: i64,
 }
 
@@ -101,6 +102,7 @@ pub fn new_conn(container_id: &String, addr: &String) -> Result<()> {
         container_id.clone(),
         Store {
             conn: Client::new(fd),
+            container_id: container_id.clone(),
             timeout: 0,
         },
     );
@@ -140,19 +142,18 @@ impl ValidateTool {
 impl Store {
     pub fn create(
         &self,
-        container_id: &String,
         bundle: &String,
         terminal: bool,
         stdin: &String,
         stdout: &String,
         stderr: &String,
     ) -> Result<i32> {
-        ValidateTool {}.str_empty(container_id)?.str_empty(bundle)?;
+        ValidateTool {}.str_empty(bundle)?;
 
         let client = protocols::shim_ttrpc::TaskClient::new(self.conn.clone());
 
         let mut req = protocols::shim::CreateTaskRequest::new();
-        req.set_id(container_id.clone());
+        req.set_id(self.container_id.clone());
         req.set_bundle(bundle.clone());
         req.set_terminal(terminal);
         req.set_stdin(stdin.clone());
@@ -166,13 +167,12 @@ impl Store {
         Ok(resp.pid as i32)
     }
 
-    pub fn start(&self, container_id: &String, exec_id: &String) -> Result<i32> {
-        ValidateTool {}.str_empty(container_id)?;
+    pub fn start(&self, exec_id: &String) -> Result<i32> {
 
         let client = protocols::shim_ttrpc::TaskClient::new(self.conn.clone());
 
         let mut req = protocols::shim::StartRequest::new();
-        req.set_id(container_id.clone());
+        req.set_id(self.container_id.clone());
         req.set_exec_id(exec_id.clone());
 
         let resp = client
@@ -185,18 +185,14 @@ impl Store {
     #[allow(unused)]
     pub fn kill(
         &self,
-        container_id: &String,
-        exec_id: &String,
         signal: u32,
         all: bool,
     ) -> Result<()> {
-        ValidateTool {}.str_empty(container_id)?;
 
         let client = protocols::shim_ttrpc::TaskClient::new(self.conn.clone());
 
         let mut req = protocols::shim::KillRequest::new();
-        req.set_id(container_id.clone());
-        // unused variable: exec_id
+        req.set_id(self.container_id.clone());
         req.set_signal(signal);
         req.set_all(all);
 
@@ -207,13 +203,12 @@ impl Store {
         Ok(())
     }
 
-    pub fn delete(&self, container_id: &String, exec_id: &String) -> Result<DeleteResponse> {
-        ValidateTool {}.str_empty(container_id)?;
+    pub fn delete(&self, exec_id: &String) -> Result<DeleteResponse> {
 
         let client = protocols::shim_ttrpc::TaskClient::new(self.conn.clone());
 
         let mut req = protocols::shim::DeleteRequest::new();
-        req.set_id(container_id.clone());
+        req.set_id(self.container_id.clone());
         req.set_exec_id(exec_id.clone());
 
         let resp = client
@@ -226,13 +221,12 @@ impl Store {
         })
     }
 
-    pub fn shutdown(&self, container_id: &String) -> Result<()> {
-        ValidateTool {}.str_empty(container_id)?;
+    pub fn shutdown(&self) -> Result<()> {
 
         let client = protocols::shim_ttrpc::TaskClient::new(self.conn.clone());
 
         let mut req = protocols::shim::ShutdownRequest::new();
-        req.set_id(container_id.clone());
+        req.set_id(self.container_id.clone());
 
         client
             .shutdown(&req, self.timeout)
@@ -243,7 +237,6 @@ impl Store {
 
     pub fn exec(
         &self,
-        container_id: &String,
         exec_id: &String,
         terminal: bool,
         stdin: &String,
@@ -252,13 +245,12 @@ impl Store {
         spec: &[u8],
     ) -> Result<()> {
         ValidateTool {}
-            .str_empty(container_id)?
             .str_empty(exec_id)?;
 
         let client = protocols::shim_ttrpc::TaskClient::new(self.conn.clone());
 
         let mut req = protocols::shim::ExecProcessRequest::new();
-        req.set_id(container_id.clone());
+        req.set_id(self.container_id.clone());
         req.set_exec_id(exec_id.clone());
         req.set_terminal(terminal);
         req.set_stdin(stdin.clone());
@@ -281,17 +273,15 @@ impl Store {
 
     pub fn resize_pty(
         &self,
-        container_id: &String,
         exec_id: &String,
         height: u32,
         width: u32,
     ) -> Result<()> {
-        ValidateTool {}.str_empty(container_id)?;
 
         let client = protocols::shim_ttrpc::TaskClient::new(self.conn.clone());
 
         let mut req = protocols::shim::ResizePtyRequest::new();
-        req.set_id(container_id.clone());
+        req.set_id(self.container_id.clone());
         req.set_exec_id(exec_id.clone());
         req.set_height(height);
         req.set_width(width);
@@ -303,13 +293,12 @@ impl Store {
         Ok(())
     }
 
-    pub fn pause(&self, container_id: &String) -> Result<()> {
-        ValidateTool {}.str_empty(container_id)?;
+    pub fn pause(&self) -> Result<()> {
 
         let client = protocols::shim_ttrpc::TaskClient::new(self.conn.clone());
 
         let mut req = protocols::shim::PauseRequest::new();
-        req.set_id(container_id.clone());
+        req.set_id(self.container_id.clone());
 
         client
             .pause(&req, self.timeout)
@@ -318,13 +307,12 @@ impl Store {
         Ok(())
     }
 
-    pub fn resume(&self, container_id: &String) -> Result<()> {
-        ValidateTool {}.str_empty(container_id)?;
+    pub fn resume(&self) -> Result<()> {
 
         let client = protocols::shim_ttrpc::TaskClient::new(self.conn.clone());
 
         let mut req = protocols::shim::ResumeRequest::new();
-        req.set_id(container_id.clone());
+        req.set_id(self.container_id.clone());
 
         client
             .resume(&req, self.timeout)
@@ -333,20 +321,19 @@ impl Store {
         Ok(())
     }
 
-    pub fn state(&self, container_id: &String) -> Result<State> {
-        ValidateTool {}.str_empty(container_id)?;
+    pub fn state(&self) -> Result<State> {
 
         let client = protocols::shim_ttrpc::TaskClient::new(self.conn.clone());
 
         let mut req = protocols::shim::StateRequest::new();
-        req.set_id(container_id.clone());
+        req.set_id(self.container_id.clone());
 
         let resp = client
             .state(&req, self.timeout)
             .map_err(shim_error!(e, "ttrpc call state failed"))?;
 
         Ok(State {
-            id: container_id.clone(),
+            id: self.container_id.clone(),
             pid: resp.pid,
             status: match resp.status {
                 shim_v2_status::CREATED => Status::CreatedStatus,
@@ -364,11 +351,11 @@ impl Store {
         })
     }
 
-    pub fn pids(&self, container_id: &String) -> Result<i32> {
+    pub fn pids(&self) -> Result<i32> {
         let c = protocols::shim_ttrpc::TaskClient::new(self.conn.clone());
 
         let mut req = protocols::shim::PidsRequest::new();
-        req.id = container_id.clone();
+        req.id = self.container_id.clone();
 
         let resp = c
             .pids(&req, self.timeout)
@@ -378,13 +365,12 @@ impl Store {
         Ok(process.pid as i32)
     }
 
-    pub fn wait(&self, container_id: &String, exec_id: &String) -> Result<i32> {
-        ValidateTool {}.str_empty(container_id)?;
+    pub fn wait(&self, exec_id: &String) -> Result<i32> {
 
         let client = protocols::shim_ttrpc::TaskClient::new(self.conn.clone());
 
         let mut req = protocols::shim::WaitRequest::new();
-        req.set_id(container_id.clone());
+        req.set_id(self.container_id.clone());
         req.set_exec_id(exec_id.clone());
 
         let resp = client
diff --git a/src/lib.rs b/src/lib.rs
index 7a7d453..b46d800 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -80,7 +80,6 @@ pub extern "C" fn shim_v2_create(
         .and_then(|client| {
             client
                 .create(
-                    &r_container_id,
                     &r_bundle,
                     terminal,
                     &r_stdin,
@@ -110,7 +109,7 @@ pub extern "C" fn shim_v2_start(
     get_conn(&r_container_id)
         .and_then(|client| {
             client
-                .start(&r_container_id, &r_exec_id)
+                .start(&r_exec_id)
                 .map(|process_pid| {
                     *pid = process_pid;
                     println!("lib-shim-v2::start::{}:: done.", r_container_id);
@@ -135,7 +134,7 @@ pub extern "C" fn shim_v2_kill(
     get_conn(&r_container_id)
         .and_then(|client| {
             client
-                .kill(&r_container_id, &r_exec_id, signal, all)
+                .kill(signal, all)
                 .map(|_| {
                     println!("lib-shim-v2::kill::{}:: done.", r_container_id);
                     0
@@ -163,7 +162,7 @@ pub extern "C" fn shim_v2_delete(
     println!("lib-shim-v2::delete::{}:: [{}]", r_container_id, r_exec_id);
     get_conn(&r_container_id)
         .and_then(|client| {
-            client.delete(&r_container_id, &r_exec_id).map(|response| {
+            client.delete(&r_exec_id).map(|response| {
                 resp.exit_status = response.exit_status;
                 resp.pid = response.pid;
                 println!("lib-shim-v2::delete::{}:: done.", r_container_id);
@@ -182,7 +181,7 @@ pub extern "C" fn shim_v2_shutdown(container_id: *const c_char) -> c_int {
     println!("lib-shim-v2::shutdown::{}::", r_container_id);
     get_conn(&r_container_id)
         .and_then(|client| {
-            client.shutdown(&r_container_id).map(|_| {
+            client.shutdown().map(|_| {
                 println!("lib-shim-v2::shutdown::{}:: done.", r_container_id);
                 0
             })
@@ -222,7 +221,6 @@ pub extern "C" fn shim_v2_exec(
         .and_then(|client| {
             client
                 .exec(
-                    &r_container_id,
                     &r_exec_id,
                     terminal,
                     &r_stdin,
@@ -256,7 +254,7 @@ pub extern "C" fn shim_v2_resize_pty(
     get_conn(&r_container_id)
         .and_then(|client| {
             client
-                .resize_pty(&r_container_id, &r_exec_id, height, width)
+                .resize_pty(&r_exec_id, height, width)
                 .map(|_| {
                     println!("lib-shim-v2::resize_pty::{}:: done.", r_container_id);
                     0
@@ -277,7 +275,7 @@ pub extern "C" fn shim_v2_pause(container_id: *const c_char) -> c_int {
     println!("lib-shim-v2::pause::{}::", r_container_id);
     get_conn(&r_container_id)
         .and_then(|client| {
-            client.pause(&r_container_id).map(|_| {
+            client.pause().map(|_| {
                 println!("lib-shim-v2::pause::{}:: done.", r_container_id);
                 0
             })
@@ -294,7 +292,7 @@ pub extern "C" fn shim_v2_resume(container_id: *const c_char) -> c_int {
     println!("lib-shim-v2::resume::{}::", r_container_id);
     get_conn(&r_container_id)
         .and_then(|client| {
-            client.resume(&r_container_id).map(|_| {
+            client.resume().map(|_| {
                 println!("lib-shim-v2::resume::{}:: done.", r_container_id);
                 0
             })
@@ -361,7 +359,7 @@ pub extern "C" fn shim_v2_state(container_id: *const c_char, state: &mut State)
     println!("lib-shim-v2::state::{}::", r_container_id);
     get_conn(&r_container_id)
         .and_then(|client| {
-            client.state(&r_container_id).map(|container_state| {
+            client.state().map(|container_state| {
                 state.copy(container_state);
                 println!("lib-shim-v2::state::{}:: done.", r_container_id);
                 0
@@ -379,7 +377,7 @@ pub extern "C" fn shim_v2_pids(container_id: *const c_char, pid: &mut c_int) ->
     println!("in rutst::shim_v2_pids::{}:: start.", r_container_id);
     get_conn(&r_container_id)
         .and_then(|client| {
-            client.pids(&r_container_id).map(|process_pid| {
+            client.pids().map(|process_pid| {
                 *pid = process_pid;
                 println!("in rust::shim_v2_pids::{}:: done", r_container_id);
                 0
@@ -401,7 +399,7 @@ pub extern "C" fn shim_v2_wait(
     println!("lib-shim-v2::wait::{}:: [{}]", r_container_id, r_exec_id);
     get_conn(&r_container_id)
         .and_then(|client| {
-            client.wait(&r_container_id, &r_exec_id).map(|exit_code| {
+            client.wait(&r_exec_id).map(|exit_code| {
                 *exit_status = exit_code;
                 println!("lib-shim-v2::wait::{}:: done.", r_container_id);
                 0
-- 
2.40.1