[638] trailing tab in list output (#675)

* tab optional

* update tests

* fix the test

* update document

* remove unused crate

* cargo fmt

* Address review comments

* refactor desrializer

* add a trailing tab test

Co-authored-by: Pradeep Kumar K J <pradeepkumar.kj@sofwareag.com>

3 files changed, 133 insertions, 20 deletions

diff --git a/crates/core/plugin_sm/Cargo.toml b/crates/core/plugin_sm/Cargo.toml
index 90fbf1aa..d314bbc2 100644
--- a/crates/core/plugin_sm/Cargo.toml
+++ b/crates/core/plugin_sm/Cargo.toml
@@ -21,4 +21,7 @@ url = "2.2"
 anyhow = "1.0"
 assert_matches = "1.5"
 structopt = "0.3"
+serial_test = "0.5.1"
 tempfile = "3.2"
+test-case = "1.2.1"
+
diff --git a/crates/core/plugin_sm/src/plugin.rs b/crates/core/plugin_sm/src/plugin.rs
index 977ed88b..2eabad91 100644
--- a/crates/core/plugin_sm/src/plugin.rs
+++ b/crates/core/plugin_sm/src/plugin.rs
@@ -3,6 +3,7 @@ use async_trait::async_trait;
 use csv::ReaderBuilder;
 use download::Downloader;
 use json_sm::*;
+use serde::Deserialize;
 use std::{path::PathBuf, process::Output};
 use tokio::io::BufWriter;
 use tokio::{fs::File, io::AsyncWriteExt};
@@ -195,6 +196,14 @@ pub trait Plugin {
     }
 }
 
+// This struct is used for deserializing the list of modules that are returned by a plugin.
+#[derive(Debug, Deserialize)]
+struct ModuleInfo {
+    name: String,
+    #[serde(default)]
+    version: Option<String>,
+}
+
 #[derive(Debug)]
 pub struct ExternalPluginCommand {
     pub name: SoftwareType,
@@ -417,24 +426,10 @@ impl Plugin for ExternalPluginCommand {
         let command = self.command(LIST, None)?;
         let output = self.execute(command, logger).await?;
         if output.status.success() {
-            let mut software_list = Vec::new();
-            let mut rdr = ReaderBuilder::new()
-                .has_headers(false)
-                .delimiter(b'\t')
-                .from_reader(output.stdout.as_slice());
-
-            for module in rdr.deserialize() {
-                let (name, version): (String, Option<String>) = module?;
-                software_list.push(SoftwareModule {
-                    name,
-                    version,
-                    module_type: Some(self.name.clone()),
-                    file_path: None,
-                    url: None,
-                });
-            }
-
-            Ok(software_list)
+            Ok(deserialize_module_info(
+                self.name.clone(),
+                output.stdout.as_slice(),
+            )?)
         } else {
             Err(SoftwareError::Plugin {
                 software_type: self.name.clone(),
@@ -466,3 +461,26 @@ impl Plugin for ExternalPluginCommand {
         }
     }
 }
+
+pub fn deserialize_module_info(
+    module_type: String,
+    input: impl std::io::Read,
+) -> Result<Vec<SoftwareModule>, SoftwareError> {
+    let mut records = ReaderBuilder::new()
+        .has_headers(false)
+        .delimiter(b'\t')
+        .flexible(true)
+        .from_reader(input);
+    let mut software_list = Vec::new();
+    for module in records.deserialize() {
+        let minfo: ModuleInfo = module?;
+        software_list.push(SoftwareModule {
+            name: minfo.name,
+            version: minfo.version,
+            module_type: Some(module_type.clone()),
+            file_path: None,
+            url: None,
+        });
+    }
+    Ok(software_list)
+}
diff --git a/crates/core/plugin_sm/tests/plugin.rs b/crates/core/plugin_sm/tests/plugin.rs
index 26738d4f..11b31f3f 100644
--- a/crates/core/plugin_sm/tests/plugin.rs
+++ b/crates/core/plugin_sm/tests/plugin.rs
@@ -3,12 +3,15 @@ mod tests {
 
     use assert_matches::assert_matches;
     use json_sm::{SoftwareError, SoftwareModule, SoftwareModuleUpdate};
-    use plugin_sm::plugin::{ExternalPluginCommand, Plugin};
+    use plugin_sm::plugin::{deserialize_module_info, ExternalPluginCommand, Plugin};
+    use serial_test::serial;
     use std::{fs, io::Write, path::PathBuf, str::FromStr};
+    use test_case::test_case;
     use tokio::fs::File;
     use tokio::io::BufWriter;
 
     #[tokio::test]
+    #[serial]
     async fn plugin_get_command_prepare() {
         // Prepare dummy plugin.
         let (plugin, _plugin_path) = get_dummy_plugin("test");
@@ -22,6 +25,7 @@ mod tests {
     }
 
     #[tokio::test]
+    #[serial]
     async fn plugin_get_command_finalize() {
         // Prepare dummy plugin.
         let (plugin, _plugin_path) = get_dummy_plugin("test");
@@ -34,8 +38,53 @@ mod tests {
         assert_eq!(res, Ok(()));
     }
 
+    #[test_case("abc", Some("1.0")  ; "with version")]
+    #[test_case("abc",None  ; "without version")]
+    fn desrialize_plugin_result(module_name: &str, version: Option<&str>) {
+        let mut data = String::from(module_name);
+        match version {
+            Some(v) => {
+                data.push_str("\t");
+                data.push_str(v)
+            }
+            None => {}
+        }
+
+        let mut expected_software_list = Vec::new();
+
+        expected_software_list.push(SoftwareModule {
+            name: module_name.into(),
+            version: version.map(|s| s.to_string()),
+            module_type: Some("test".into()),
+            file_path: None,
+            url: None,
+        });
+
+        let software_list = deserialize_module_info("test".into(), data.as_bytes()).unwrap();
+        assert_eq!(expected_software_list, software_list);
+    }
+
+    #[test]
+    fn desrialize_plugin_result_with_trailing_tab() {
+        let data = "abc\t";
+
+        let mut expected_software_list = Vec::new();
+
+        expected_software_list.push(SoftwareModule {
+            name: "abc".into(),
+            version: None,
+            module_type: Some("test".into()),
+            file_path: None,
+            url: None,
+        });
+
+        let software_list = deserialize_module_info("test".into(), data.as_bytes()).unwrap();
+        assert_eq!(expected_software_list, software_list);
+    }
+
     #[tokio::test]
-    async fn plugin_get_command_list() {
+    #[serial]
+    async fn plugin_get_command_list_with_version() {
         // Prepare dummy plugin with .0 which will give specific exit code ==0.
         let (plugin, _plugin_path) = get_dummy_plugin("test");
         let path = get_dummy_plugin_tmp_path();
@@ -69,6 +118,42 @@ mod tests {
     }
 
     #[tokio::test]
+    #[serial]
+    async fn plugin_get_command_list_without_version() {
+        // Prepare dummy plugin with .0 which will give specific exit code ==0.
+        let (plugin, _plugin_path) = get_dummy_plugin("test");
+        let path = get_dummy_plugin_tmp_path();
+
+        let mut file = tempfile::Builder::new()
+            .suffix(".0")
+            .tempfile_in(path)
+            .unwrap();
+
+        // Add content of the expected stdout to the dummy plugin.
+        let content = "abc";
+        let _a = file.write_all(content.as_bytes()).unwrap();
+
+        // Create expected response.
+        let module = SoftwareModule {
+            module_type: Some("test".into()),
+            name: "abc".into(),
+            version: None,
+            url: None,
+            file_path: None,
+        };
+        let expected_response = vec![module];
+
+        // Call plugin via API.
+        let mut logger = dev_null().await;
+        let res = plugin.list(&mut logger).await;
+
+        // Expect Ok as plugin should exit with code 0.
+        assert!(res.is_ok());
+        assert_eq!(res.unwrap(), expected_response);
+    }
+
+    #[tokio::test]
+    #[serial]
     async fn plugin_get_command_install() {
         // Prepare dummy plugin with .0 which will give specific exit code ==0.
         let (plugin, _plugin_path) = get_dummy_plugin("test");
@@ -101,6 +186,7 @@ mod tests {
     }
 
     #[tokio::test]
+    #[serial]
     async fn plugin_get_command_remove() {
         // Prepare dummy plugin with .0 which will give specific exit code ==0.
         let (plugin, _plugin_path) = get_dummy_plugin("test");
@@ -133,6 +219,7 @@ mod tests {
     }
 
     #[test]
+    #[serial]
     fn plugin_call_name_and_path() {
         let dummy_plugin_path = get_dummy_plugin_path();
         let plugin = ExternalPluginCommand::new("test", &dummy_plugin_path);
@@ -141,6 +228,7 @@ mod tests {
     }
 
     #[test]
+    #[serial]
     fn plugin_check_module_type_both_same() {
         let dummy_plugin_path = get_dummy_plugin_path();
         let plugin = ExternalPluginCommand::new("test", &dummy_plugin_path);
@@ -160,6 +248,7 @@ mod tests {
     }
 
     #[test]
+    #[serial]
     fn plugin_check_module_type_both_different() {
         // Create dummy plugin.
         let dummy_plugin_path = get_dummy_plugin_path();
@@ -190,6 +279,7 @@ mod tests {
     }
 
     #[test]
+    #[serial]
     fn plugin_check_module_type_default() {
         // Create dummy plugin.
         let dummy_plugin_path = get_dummy_plugin_path();
@@ -211,6 +301,7 @@ mod tests {
     }
 
     #[tokio::test]
+    #[serial]
     async fn plugin_get_command_update_list() {
         // Prepare dummy plugin with .0 which will give specific exit code ==0.
         let (plugin, _plugin_path) = get_dummy_plugin("test");
@@ -249,6 +340,7 @@ mod tests {
 
     // Test validating if the plugin will fall back to `install` and `remove` options if the `update-list` option is not supported
     #[tokio::test]
+    #[serial]
     async fn plugin_command_update_list_fallback() {
         // Prepare dummy plugin with .0 which will give specific exit code ==0.
         let (plugin, _plugin_path) = get_dummy_plugin("test");