Use EnvironmentVariableName type for names of ENV variables

This makes the typing a bit more helpful by using a type for the name of
environment variables.

Signed-off-by: Matthias Beyer <mail@beyermatthias.de>

7 files changed, 44 insertions, 15 deletions

diff --git a/src/commands/build.rs b/src/commands/build.rs
index 60c61e7..d122609 100644
--- a/src/commands/build.rs
+++ b/src/commands/build.rs
@@ -26,6 +26,7 @@ use crate::package::Shebang;
 use crate::package::Tree;
 use crate::repository::Repository;
 use crate::source::SourceCache;
+use crate::util::EnvironmentVariableName;
 use crate::util::docker::ImageName;
 use crate::util::progress::ProgressBars;
 
@@ -100,11 +101,11 @@ pub async fn build(matches: &ArgMatches,
         .map(|s| {
             let v = s.split("=").collect::<Vec<_>>();
             Ok((
-                 String::from(*v.get(0).ok_or_else(|| anyhow!("Environment variable has no key: {}", s))?),
+                 EnvironmentVariableName::from(*v.get(0).ok_or_else(|| anyhow!("Environment variable has no key: {}", s))?),
                  String::from(*v.get(1).ok_or_else(|| anyhow!("Environment variable has no key: {}", s))?)
             ))
         })
-        .collect::<Result<Vec<(String, String)>>>()?;
+        .collect::<Result<Vec<(EnvironmentVariableName, String)>>>()?;
 
     let packages = if let Some(pvers) = pvers {
         repo.find(&pname, &pvers)
@@ -191,7 +192,7 @@ pub async fn build(matches: &ArgMatches,
         additional_env.clone()
             .into_iter()
             .map(|(k, v)| async {
-                let k: String = k; // hack to work around move semantics
+                let k: EnvironmentVariableName = k; // hack to work around move semantics
                 let v: String = v; // hack to work around move semantics
                 EnvVar::create_or_fetch(&database_connection, &k, &v)
             })
diff --git a/src/db/models/envvar.rs b/src/db/models/envvar.rs
index 51ad32a..7e5ac7d 100644
--- a/src/db/models/envvar.rs
+++ b/src/db/models/envvar.rs
@@ -5,6 +5,7 @@ use diesel::prelude::*;
 
 use crate::schema::envvars::*;
 use crate::schema::envvars;
+use crate::util::EnvironmentVariableName;
 
 #[derive(Identifiable, Queryable)]
 #[table_name="envvars"]
@@ -22,8 +23,8 @@ struct NewEnvVar<'a> {
 }
 
 impl EnvVar {
-    pub fn create_or_fetch(database_connection: &PgConnection, k: &str, v: &str) -> Result<EnvVar> {
-        let new_envvar = NewEnvVar { name: k, value: v };
+    pub fn create_or_fetch(database_connection: &PgConnection, k: &EnvironmentVariableName, v: &str) -> Result<EnvVar> {
+        let new_envvar = NewEnvVar { name: k.as_ref(), value: v };
 
         diesel::insert_into(envvars::table)
             .values(&new_envvar)
@@ -31,7 +32,7 @@ impl EnvVar {
             .execute(database_connection)?;
 
         dsl::envvars
-            .filter(name.eq(k).and(value.eq(v)))
+            .filter(name.eq(k.as_ref()).and(value.eq(v)))
             .first::<EnvVar>(database_connection)
             .map_err(Error::from)
     }
diff --git a/src/endpoint/configured.rs b/src/endpoint/configured.rs
index 46515dc..191e565 100644
--- a/src/endpoint/configured.rs
+++ b/src/endpoint/configured.rs
@@ -180,8 +180,12 @@ impl Endpoint {
         let (container_id, _warnings) = {
             let envs = job.environment()
                 .into_iter()
-                .chain(job.package_environment().into_iter())
-                .map(|(k, v)| format!("{}={}", k, v))
+                .map(|(k, v)| format!("{}={}", k.as_ref(), v))
+                .chain({
+                    job.package_environment()
+                        .into_iter()
+                        .map(|(k, v)| format!("{}={}", k, v))
+                })
                 .collect::<Vec<_>>();
             trace!("Job resources: Environment variables = {:?}", envs);
 
diff --git a/src/job/resource.rs b/src/job/resource.rs
index 96ee250..145a825 100644
--- a/src/job/resource.rs
+++ b/src/job/resource.rs
@@ -1,19 +1,20 @@
 use crate::filestore::Artifact;
+use crate::util::EnvironmentVariableName;
 
 #[derive(Clone, Debug)]
 pub enum JobResource {
-    Environment(String, String),
+    Environment(EnvironmentVariableName, String),
     Artifact(Artifact)
 }
 
-impl From<(String, String)> for JobResource {
-    fn from(tpl: (String, String)) -> Self {
+impl From<(EnvironmentVariableName, String)> for JobResource {
+    fn from(tpl: (EnvironmentVariableName, String)) -> Self {
         JobResource::Environment(tpl.0, tpl.1)
     }
 }
 
 impl JobResource {
-    pub fn env(&self) -> Option<(&String, &String)> {
+    pub fn env(&self) -> Option<(&EnvironmentVariableName, &String)> {
         match self {
             JobResource::Environment(k, v) => Some((k, v)),
             _ => None
diff --git a/src/job/runnable.rs b/src/job/runnable.rs
index de5b32d..a0acaff 100644
--- a/src/job/runnable.rs
+++ b/src/job/runnable.rs
@@ -17,6 +17,7 @@ use crate::package::Script;
 use crate::package::ScriptBuilder;
 use crate::source::SourceCache;
 use crate::source::SourceEntry;
+use crate::util::EnvironmentVariableName;
 use crate::util::docker::ImageName;
 
 /// A job configuration that can be run. All inputs are clear here.
@@ -86,7 +87,7 @@ impl RunnableJob {
         self.source_cache.sources_for(&self.package())
     }
 
-    pub fn environment(&self) -> Vec<(String, String)> {
+    pub fn environment(&self) -> Vec<(EnvironmentVariableName, String)> {
         let iter = self.resources
             .iter()
             .filter_map(JobResource::env)
diff --git a/src/package/package.rs b/src/package/package.rs
index 4be822f..2dc7f1d 100644
--- a/src/package/package.rs
+++ b/src/package/package.rs
@@ -12,6 +12,7 @@ use crate::package::name::*;
 use crate::package::source::*;
 use crate::package::version::*;
 use crate::phase::{PhaseName, Phase};
+use crate::util::EnvironmentVariableName;
 use crate::util::docker::ImageName;
 
 #[derive(Clone, Serialize, Deserialize, Getters)]
@@ -36,7 +37,7 @@ pub struct Package {
 
     #[getset(get = "pub")]
     #[serde(skip_serializing_if = "Option::is_none")]
-    environment: Option<HashMap<String, String>>,
+    environment: Option<HashMap<EnvironmentVariableName, String>>,
 
     #[getset(get = "pub")]
     #[serde(skip_serializing_if = "Option::is_none")]
diff --git a/src/util/mod.rs b/src/util/mod.rs
index 5f6a35f..478962a 100644
--- a/src/util/mod.rs
+++ b/src/util/mod.rs
@@ -1,8 +1,28 @@
+use serde::Serialize;
 use serde::Deserialize;
 
-#[derive(Deserialize, Debug, Hash, Eq, PartialEq, Ord, PartialOrd)]
+#[derive(Serialize, Deserialize, Clone, Debug, Hash, Eq, PartialEq, Ord, PartialOrd)]
+#[serde(transparent)]
 pub struct EnvironmentVariableName(String);
 
+impl From<&str> for EnvironmentVariableName {
+    fn from(s: &str) -> EnvironmentVariableName {
+        EnvironmentVariableName(s.to_string())
+    }
+}
+
+impl AsRef<str> for EnvironmentVariableName {
+    fn as_ref(&self) -> &str {
+        self.0.as_ref()
+    }
+}
+
+impl std::fmt::Display for EnvironmentVariableName {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error> {
+        self.0.fmt(f)
+    }
+}
+
 pub mod docker;
 pub mod filters;
 pub mod git;