From 94b74acc9024c6dd77f0d801b566d74e29840936 Mon Sep 17 00:00:00 2001
From: Alexandru Macovei <alexnmaco@gmail.com>
Date: Wed, 16 Dec 2020 18:54:21 +0200
Subject: [PATCH] refactor: a few low-hanging optimizations (#1992)

* refactor: convert some vecs to static arrays and slices

* refactor(openstack): lazy yaml file reading, skip files without expected structure, and avoid panics for some edge cases
---
 src/modules/git_branch.rs |  7 +++--
 src/modules/openstack.rs  | 59 +++++++++++++++++++++++++++------------
 src/modules/username.rs   |  6 ++--
 src/print.rs              |  4 +--
 4 files changed, 50 insertions(+), 26 deletions(-)

diff --git a/src/modules/git_branch.rs b/src/modules/git_branch.rs
index 2456a9d5f..6f006e839 100644
--- a/src/modules/git_branch.rs
+++ b/src/modules/git_branch.rs
@@ -49,11 +49,14 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
     }
 
     // Truncate fields if need be
-    for e in vec![
+    for e in [
         &mut graphemes,
         &mut remote_branch_graphemes,
         &mut remote_name_graphemes,
-    ] {
+    ]
+    .iter_mut()
+    {
+        let e = &mut **e;
         let trunc_len = len.min(e.len());
         if trunc_len < e.len() {
             // The truncation symbol should only be added if we truncate
diff --git a/src/modules/openstack.rs b/src/modules/openstack.rs
index be6c51257..5bb7a8b54 100644
--- a/src/modules/openstack.rs
+++ b/src/modules/openstack.rs
@@ -9,23 +9,29 @@ use crate::utils;
 type Cloud = String;
 type Project = String;
 
-fn get_osp_project_from_config(context: &Context, osp_cloud: Option<&str>) -> Option<Project> {
+fn get_osp_project_from_config(context: &Context, osp_cloud: &str) -> Option<Project> {
     // Attempt to follow OpenStack standards for clouds.yaml location:
     // 1st = $PWD/clouds.yaml, 2nd = $HOME/.config/openstack/clouds.yaml, 3rd = /etc/openstack/clouds.yaml
-    let config = vec![
-        utils::read_file(context.get_env("PWD").unwrap() + "/clouds.yaml"),
-        utils::read_file(dirs_next::home_dir()?.join(".config/openstack/clouds.yaml")),
-        utils::read_file("/etc/openstack/clouds.yaml"),
+    let config = [
+        context.get_env("PWD").map(|pwd| pwd + "/clouds.yaml"),
+        dirs_next::home_dir().map(|home| {
+            home.join(".config/openstack/clouds.yaml")
+                .display()
+                .to_string()
+        }),
+        Some(String::from("/etc/openstack/clouds.yaml")),
     ];
-    let clouds =
-        YamlLoader::load_from_str(&config.into_iter().find_map(Result::ok).unwrap()).ok()?;
-    let project = &clouds[0]["clouds"][osp_cloud.unwrap()]["auth"]["project_name"]
-        .as_str()
-        .unwrap_or("");
-    if project.is_empty() {
-        return None;
-    }
-    Some(project.to_string())
+
+    config
+        .iter()
+        .filter_map(|file| {
+            let config = utils::read_file(file.as_ref()?).ok()?;
+            let clouds = YamlLoader::load_from_str(config.as_str()).ok()?;
+            clouds.get(0)?["clouds"][osp_cloud]["auth"]["project_name"]
+                .as_str()
+                .map(ToOwned::to_owned)
+        })
+        .find(|s| !s.is_empty())
 }
 
 fn get_osp_cloud_and_project(context: &Context) -> (Option<Cloud>, Option<Project>) {
@@ -35,10 +41,7 @@ fn get_osp_cloud_and_project(context: &Context) -> (Option<Cloud>, Option<Projec
     ) {
         (Some(p), Some(r)) => (Some(p), Some(r)),
         (None, Some(r)) => (None, Some(r)),
-        (Some(ref p), None) => (
-            Some(p.to_owned()),
-            get_osp_project_from_config(context, Some(p)),
-        ),
+        (Some(ref p), None) => (Some(p.to_owned()), get_osp_project_from_config(context, p)),
         (None, None) => (None, None),
     }
 }
@@ -141,4 +144,24 @@ dummy_yaml
         assert_eq!(actual, expected);
         dir.close()
     }
+
+    #[test]
+    fn dont_crash_on_empty_config() -> io::Result<()> {
+        let dir = tempfile::tempdir()?;
+        let config_path = dir.path().join("clouds.yaml");
+        let mut file = File::create(&config_path)?;
+        file.write_all(b"")?;
+        drop(file);
+        let actual = ModuleRenderer::new("openstack")
+            .env("PWD", dir.path().to_str().unwrap())
+            .env("OS_CLOUD", "test")
+            .config(toml::toml! {
+                [openstack]
+            })
+            .collect();
+        let expected = Some(format!("on {} ", Color::Yellow.bold().paint("☁️  test")));
+
+        assert_eq!(actual, expected);
+        dir.close()
+    }
 }
diff --git a/src/modules/username.rs b/src/modules/username.rs
index d2732559c..3294cc1b7 100644
--- a/src/modules/username.rs
+++ b/src/modules/username.rs
@@ -56,10 +56,8 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
 }
 
 fn is_ssh_connection(context: &Context) -> bool {
-    let ssh_env: Vec<&str> = vec!["SSH_CONNECTION", "SSH_CLIENT", "SSH_TTY"];
-    ssh_env
-        .into_iter()
-        .any(|env| context.get_env(env).is_some())
+    let ssh_env = ["SSH_CONNECTION", "SSH_CLIENT", "SSH_TTY"];
+    ssh_env.iter().any(|env| context.get_env(env).is_some())
 }
 
 fn get_uid() -> Option<u32> {
diff --git a/src/print.rs b/src/print.rs
index 12b2ea451..41b8b996a 100644
--- a/src/print.rs
+++ b/src/print.rs
@@ -147,11 +147,11 @@ pub fn explain(args: ArgMatches) {
         duration: String,
     }
 
-    let dont_print = vec!["line_break"];
+    static DONT_PRINT: &[&str] = &["line_break"];
 
     let modules = compute_modules(&context)
         .into_iter()
-        .filter(|module| !dont_print.contains(&module.get_name().as_str()))
+        .filter(|module| !DONT_PRINT.contains(&module.get_name().as_str()))
         // this contains empty modules which should not print
         .filter(|module| !module.is_empty())
         .map(|module| {