[v3,3/3] settings: improve parsing of jobs

Job entries in the config file are a mixture of parameters which should be
passed straight through to Jenkins, and flags that have meaning to
snowpatch.

I don't really feel like splitting these two things apart in the config, as
we already have enough levels of nesting. Instead, add our own deserialize
method to do this more intelligently. This also serves as an example of how
to do more advanced parsing such that we can reject invalid values at parse
time.

Thanks-to: David Tolnay <dtolnay@gmail.com> # help with boilerplate code
Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
---
v2->v3:
* I figure I should just throw this patch in now...

---
 src/main.rs     | 28 ++++++++---------
 src/settings.rs | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 98 insertions(+), 25 deletions(-)

On Fri, 2017-03-24 at 14:28 +1100, Andrew Donnellan wrote:
> Job entries in the config file are a mixture of parameters which should be
> passed straight through to Jenkins, and flags that have meaning to
> snowpatch.
> 
> I don't really feel like splitting these two things apart in the config, as
> we already have enough levels of nesting. Instead, add our own deserialize
> method to do this more intelligently. This also serves as an example of how
> to do more advanced parsing such that we can reject invalid values at parse
> time.
> 
> Thanks-to: David Tolnay <dtolnay@gmail.com> # help with boilerplate code
> Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> ---
> v2->v3:
> * I figure I should just throw this patch in now...

Kinda sketchy just throwing this in but whatever

Acked-by: Russell Currey <ruscur@russell.cc>

> 
> ---
>  src/main.rs     | 28 ++++++++---------
>  src/settings.rs | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
> -
>  2 files changed, 98 insertions(+), 25 deletions(-)
> 
> diff --git a/src/main.rs b/src/main.rs
> index 6ba550e..a0e6576 100644
> --- a/src/main.rs
> +++ b/src/main.rs
> @@ -108,22 +108,18 @@ fn run_tests(settings: &Config, client: Arc<Client>,
> project: &Project, tag: &st
>          token: settings.jenkins.token.clone(),
>      };
>      let project = project.clone();
> -    for job_params in &project.jobs {
> -        let job_name = job_params.get("job").unwrap();
> -        let job_title = settings::get_job_title(job_params);
> +    for job in &project.jobs {
>          let mut jenkins_params = Vec::<(&str, &str)>::new();
> -        for (param_name, param_value) in job_params.iter() {
> +        for (param_name, param_value) in job.parameters.iter() {
> +            // TODO(ajd): do this more neatly
>              debug!("Param name {}, value {}", &param_name, &param_value);
> -            match param_name.as_ref() {
> -                // TODO: Validate special parameter names in config at start
> of program
> -                "job" | "title" => { },
> -                "remote" => jenkins_params.push((param_value,
> &project.remote_uri)),
> -                "branch" => jenkins_params.push((param_value, tag)),
> -                _ => jenkins_params.push((param_name, param_value)),
> -            }
> +            jenkins_params.push((param_name, param_value));
>          }
> -        info!("Starting job: {}", &job_title);
> -        let res = jenkins.start_test(job_name, jenkins_params)
> +        jenkins_params.push((&job.remote, &project.remote_uri));
> +        jenkins_params.push((&job.branch, tag));
> +
> +        info!("Starting job: {}", &job.title);
> +        let res = jenkins.start_test(&job.job, jenkins_params)
>              .unwrap_or_else(|err| panic!("Starting Jenkins test failed: {}",
> err));
>          debug!("{:?}", &res);
>          let build_url_real;
> @@ -137,12 +133,12 @@ fn run_tests(settings: &Config, client: Arc<Client>,
> project: &Project, tag: &st
>          debug!("Build URL: {}", build_url_real);
>          jenkins.wait_build(&build_url_real);
>          let test_result = jenkins.get_build_result(&build_url_real).unwrap();
> -        info!("Jenkins job for {}/{} complete.", branch_name, job_title);
> +        info!("Jenkins job for {}/{} complete.", branch_name, job.title);
>          results.push(TestResult {
> -            test_name: format!("Test {} on branch {}", job_title.to_string(),
> +            test_name: format!("Test {} on branch {}", job.title,
>                                 branch_name.to_string()).to_string(),
>              state: test_result,
> -            url: Some(jenkins.get_results_url(&build_url_real, job_params)),
> +            url: Some(jenkins.get_results_url(&build_url_real,
> &job.parameters)),
>              summary: Some("TODO: get this summary from Jenkins".to_string()),
>          });
>      }
> diff --git a/src/settings.rs b/src/settings.rs
> index 826f792..44c321c 100644
> --- a/src/settings.rs
> +++ b/src/settings.rs
> @@ -16,8 +16,11 @@
>  
>  use toml;
>  
> +use serde::de::{self, MapVisitor, Visitor, Deserializer, Deserialize};
> +
>  use git2::{Repository, Error};
>  
> +use std::fmt;
>  use std::fs::File;
>  use std::io::Read;
>  use std::collections::BTreeMap;
> @@ -58,8 +61,8 @@ pub struct Project {
>      pub test_all_branches: Option<bool>,
>      pub remote_name: String,
>      pub remote_uri: String,
> -    pub jobs: Vec<BTreeMap<String, String>>,
> -    pub push_results: bool
> +    pub jobs: Vec<Job>,
> +    pub push_results: bool,
>  }
>  
>  impl Project {
> @@ -68,6 +71,87 @@ impl Project {
>      }
>  }
>  
> +#[derive(Clone)]
> +pub struct Job {
> +    pub job: String,
> +    pub title: String,
> +    pub remote: String,
> +    pub branch: String,
> +    pub parameters: BTreeMap<String, String>,
> +}
> +
> +impl Deserialize for Job {
> +    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
> +        where D: Deserializer
> +    {
> +        struct JobVisitor;
> +
> +        impl Visitor for JobVisitor {
> +            type Value = Job;
> +
> +            fn expecting(&self, formatter: &mut fmt::Formatter) ->
> fmt::Result {
> +                formatter.write_str("struct Job with arbitrary fields")
> +            }
> +
> +            fn visit_map<V>(self, mut visitor: V) -> Result<Job, V::Error>
> +                where V: MapVisitor
> +            {
> +                let mut job = None;
> +                let mut title = None;
> +                let mut remote = None;
> +                let mut branch = None;
> +                let mut parameters = BTreeMap::new();
> +                while let Some(key) = visitor.visit_key::<String>()? {
> +                    match key.as_str() {
> +                        "job" => {
> +                            if job.is_some() {
> +                                return
> Err(de::Error::duplicate_field("job"));
> +                            }
> +                            job = Some(visitor.visit_value()?);
> +                        }
> +                        "title" => {
> +                            if title.is_some() {
> +                                return
> Err(de::Error::duplicate_field("title"));
> +                            }
> +                            title = Some(visitor.visit_value()?);
> +                        }
> +                        "remote" => {
> +                            if remote.is_some() {
> +                                return
> Err(de::Error::duplicate_field("remote"));
> +                            }
> +                            remote = Some(visitor.visit_value()?);
> +                        }
> +                        "branch" => {
> +                            if branch.is_some() {
> +                                return
> Err(de::Error::duplicate_field("branch"));
> +                            }
> +                            branch = Some(visitor.visit_value()?);
> +                        }
> +                        _ => {
> +                            parameters.insert(key, visitor.visit_value()?);
> +                        }
> +                    }
> +                }
> +
> +                let job: String = job.ok_or_else(||
> de::Error::missing_field("job"))?;
> +                let remote = remote.ok_or_else(||
> de::Error::missing_field("remote"))?;
> +                let branch = branch.ok_or_else(||
> de::Error::missing_field("branch"))?;
> +                let title = title.unwrap_or(job.clone());
> +
> +                Ok(Job {
> +                    job: job,
> +                    title: title,
> +                    remote: remote,
> +                    branch: branch,
> +                    parameters: parameters,
> +                })
> +            }
> +        }
> +
> +        deserializer.deserialize_map(JobVisitor)
> +    }
> +}
> +
>  #[derive(Deserialize, Clone)]
>  pub struct Config {
>      pub git: Git,
> @@ -76,13 +160,6 @@ pub struct Config {
>      pub projects: BTreeMap<String, Project>
>  }
>  
> -pub fn get_job_title(job: &BTreeMap<String, String>) -> String {
> -    match job.get("title") {
> -        Some(title) => title.to_string(),
> -        None => job.get("job").unwrap().to_string()
> -    }
> -}
> -
>  pub fn parse(path: &str) -> Config {
>      let mut toml_config = String::new();
>