make the ffcp-auto-close/postpone behavior configurable. fixes #201

Author: Centril

src/github/nag.rs

@@ -14,7 +14,7 @@ use domain::rfcbot::{FcpConcern, FcpProposal, FcpReviewRequest, FeedbackRequest,
 use domain::schema::*;
 use error::*;
 use github::models::CommentFromJson;
-use teams::TEAMS;
+use teams::SETUP;
 use super::GH;
 
 #[derive(Copy, Clone, PartialEq, Eq, Hash)]
@@ -464,24 +464,31 @@ fn evaluate_nags() -> DashResult<()> {
     Ok(())
 }
 
-fn execute_ffcp_actions(issue: &Issue, disposition: FcpDisposition) {
-    if !is_rfc_repo(&issue) { return; }
+fn can_ffcp_close(issue: &Issue) -> bool {
+    SETUP.ffcp_auto_close(&issue.repository)
+}
+
+fn can_ffcp_postpone(issue: &Issue) -> bool {
+    SETUP.ffcp_auto_postpone(&issue.repository)
+}
 
+fn execute_ffcp_actions(issue: &Issue, disposition: FcpDisposition) {
     match disposition {
         FcpDisposition::Merge => {
             // TODO: This one will require a lot of work to
             // auto-merge RFCs and create the tracking issue.
         },
-        FcpDisposition::Close => {
+        FcpDisposition::Close if can_ffcp_close(issue) => {
             let _ = issue.add_label(Label::Closed);
             issue.remove_label(Label::DispositionClose);
             issue.close();
         },
-        FcpDisposition::Postpone => {
+        FcpDisposition::Postpone if can_ffcp_postpone(issue) => {
             let _ = issue.add_label(Label::Postponed);
             issue.remove_label(Label::DispositionPostpone);
             issue.close();
         },
+        _ => {},
     }
 }
 
@@ -566,9 +573,9 @@ fn subteam_members(issue: &Issue) -> DashResult<Vec<GitHubUser>> {
 
     // retrieve all of the teams tagged on this issue
     // cannot WAIT for by-ref/by-val inference
-    let member_logins = TEAMS.iter()
+    let member_logins = SETUP.teams()
         .filter(|&(ref label, _)| issue.labels.contains(&label.0))
-        .flat_map(|(_, team)| &team.member_logins)
+        .flat_map(|(_, team)| team.member_logins())
         .collect::<BTreeSet<_>>()
         .into_iter() // diesel won't work with btreeset, and dedup has weird lifetime errors
         .collect::<Vec<_>>();
@@ -1028,10 +1035,6 @@ enum CommentType<'a> {
     },
 }
 
-fn is_rfc_repo(issue: &Issue) -> bool {
-    issue.repository == "rust-lang/rfcs"
-}
-
 impl<'a> RfcBotComment<'a> {
     fn new(issue: &'a Issue, comment_type: CommentType<'a>) -> RfcBotComment<'a> {
 
@@ -1146,16 +1149,15 @@ impl<'a> RfcBotComment<'a> {
                 Self::add_comment_url(issue, &mut msg, status_comment_id);
                 msg.push_str("), is now **complete**.");
 
-                if is_rfc_repo(issue) {
-                    match disposition {
-                        FcpDisposition::Merge => {}
-                        FcpDisposition::Close => {
-                            msg.push_str("\n\nBy the power vested in me by Rust, I hereby close this RFC.");
-                        },
-                        FcpDisposition::Postpone => {
-                            msg.push_str("\n\nBy the power vested in me by Rust, I hereby postpone this RFC.");
-                        },
-                    }
+                match disposition {
+                    FcpDisposition::Merge => {}
+                    FcpDisposition::Close if can_ffcp_close(issue) => {
+                        msg.push_str("\n\nBy the power vested in me by Rust, I hereby close this RFC.");
+                    },
+                    FcpDisposition::Postpone if can_ffcp_postpone(issue) => {
+                        msg.push_str("\n\nBy the power vested in me by Rust, I hereby postpone this RFC.");
+                    },
+                    _ => {},
                 }
 
                 if !added_label {

src/main.rs

@@ -72,7 +72,7 @@ fn main() {
     let _ = DB_POOL.get().expect("Unable to test connection pool.");
 
     // we want to panic if we're unable to find any of the usernames
-    let parsed_teams = teams::TEAMS.keys().collect::<Vec<_>>();
+    let parsed_teams = teams::SETUP.team_labels().collect::<Vec<_>>();
     info!("parsed teams: {:?}", parsed_teams);
 
     // FIXME(anp) need to handle panics in both the listeners and crash the server

src/teams.rs

@@ -4,84 +4,232 @@ use std::collections::BTreeMap;
 
 use diesel::prelude::*;
 use toml;
+use serde::de;
 
 use super::DB_POOL;
 use domain::github::GitHubUser;
 use error::*;
 
+//==============================================================================
+// Public API
+//==============================================================================
+
 lazy_static! {
-    pub static ref TEAMS: Teams = {
-        let teams_file =
-            include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/teams.toml"));
-        let teams_from_file: TeamsFromFile =
-            toml::from_str(teams_file).expect("couldn't parse teams");
-
-        let mut teams = BTreeMap::new();
-
-        for (label, team_from_file) in teams_from_file {
-            let label = TeamLabel(label);
-            let team = team_from_file.validate()
-                .expect("unable to verify team member from database.
-if you're running this for tests, make sure you've pulled github users from prod");
-            teams.insert(label, team);
-        }
+    pub static ref SETUP: RfcbotConfig = read_rfcbot_cfg_validated();
+}
+
+#[derive(Debug, Deserialize)]
+pub struct RfcbotConfig {
+    fcp_behaviors: BTreeMap<String, FcpBehavior>,
+    teams: BTreeMap<TeamLabel, Team>,
+}
+
+impl RfcbotConfig {
+    /// Retrive an iterator over all the team labels.
+    pub fn team_labels(&self) -> impl Iterator<Item = &TeamLabel> {
+        self.teams.keys()
+    }
+
+    /// Retrive an iterator over all the (team label, team) pairs.
+    pub fn teams(&self) -> impl Iterator<Item = (&TeamLabel, &Team)> {
+        self.teams.iter()
+    }
+
+    /// Are we allowed to auto-close issues after F-FCP in this repo?
+    pub fn ffcp_auto_close(&self, repo: &str) -> bool {
+        self.fcp_behaviors.get(repo).map(|fcp| fcp.close).unwrap_or_default()
+    }
 
-        teams
-    };
+    /// Are we allowed to auto-postpone issues after F-FCP in this repo?
+    pub fn ffcp_auto_postpone(&self, repo: &str) -> bool {
+        self.fcp_behaviors.get(repo).map(|fcp| fcp.postpone).unwrap_or_default()
+    }
+}
+
+#[derive(Debug, Deserialize)]
+pub struct FcpBehavior {
+    #[serde(default)]
+    pub close: bool,
+    #[serde(default)]
+    pub postpone: bool,
+}
+
+#[derive(Debug, Deserialize)]
+pub struct Team {
+    // FIXME: The two following first fields don't seem to be used anywhere...
+    // If this is intended, document why.
+    name: String,
+    ping: String,
+    members: Vec<String>,
+}
+
+impl Team {
+    pub fn member_logins(&self) -> impl Iterator<Item = &str> {
+        self.members.iter().map(|s| s.as_str())
+    }
 }
 
 #[derive(Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
 pub struct TeamLabel(pub String);
 
-type TeamsFromFile = BTreeMap<String, TeamFromFile>;
-pub type Teams = BTreeMap<TeamLabel, Team>;
+impl<'de> de::Deserialize<'de> for TeamLabel {
+    fn deserialize<D: de::Deserializer<'de>>(de: D) -> Result<Self, D::Error> {
+        String::deserialize(de).map(TeamLabel)
+    }
 
-#[derive(Debug, Deserialize)]
-struct TeamFromFile {
-  name: String,
-  ping: String,
-  members: Vec<String>,
+    fn deserialize_in_place<D: de::Deserializer<'de>>(de: D, place: &mut Self)
+        -> Result<(), D::Error>
+    {
+        String::deserialize_in_place(de, &mut place.0)
+    }
+}
+
+//==============================================================================
+// Implementation details
+//==============================================================================
+
+/// Read the validated `rfcbot.toml` configuration file.
+fn read_rfcbot_cfg_validated() -> RfcbotConfig {
+    let cfg = read_rfcbot_cfg();
+
+    cfg.teams.values().for_each(|team|
+        team.validate()
+            .expect("unable to verify team member from database.
+if you're running this for tests, make sure you've pulled github users from prod")
+    );
+
+    cfg
+}
+
+/// Read the unprocessed `rfcbot.toml` configuration file.
+fn read_rfcbot_cfg() -> RfcbotConfig {
+    read_rfcbot_cfg_from(
+        include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/rfcbot.toml")))
+}
+
+fn read_rfcbot_cfg_from(input: &str) -> RfcbotConfig {
+    toml::from_str(input).expect("couldn't parse rfcbot.toml!")
 }
 
-impl TeamFromFile {
-  pub fn validate(self) -> DashResult<Team> {
-    use domain::schema::githubuser::dsl::*;
-    let conn = &*(DB_POOL.get()?);
-
-    // bail if they don't exist, but we don't want to actually keep the id in ram
-    for member_login in &self.members {
-      match githubuser
-        .filter(login.eq(member_login))
-        .first::<GitHubUser>(conn) {
-            Ok(_) => (),
-            Err(why) => {
-                error!("unable to find {} in database: {:?}", member_login, why);
-                return Err(why.into());
+impl Team {
+    fn validate(&self) -> DashResult<()> {
+        use domain::schema::githubuser::dsl::*;
+        let conn = &*(DB_POOL.get()?);
+
+        // bail if they don't exist, but we don't want to actually keep the id in ram
+        for member_login in self.member_logins() {
+            match githubuser
+                    .filter(login.eq(member_login))
+                    .first::<GitHubUser>(conn)
+            {
+                Ok(_) => (),
+                Err(why) => {
+                    error!("unable to find {} in database: {:?}", member_login, why);
+                    return Err(why.into());
+                }
             }
         }
-    }
 
-    Ok(Team {
-      name: self.name,
-      ping: self.ping,
-      member_logins: self.members,
-    })
-  }
+        Ok(())
+    }
 }
 
-pub struct Team {
-  pub name: String,
-  pub ping: String,
-  pub member_logins: Vec<String>,
-}
+//==============================================================================
+// Tests
+//==============================================================================
 
 #[cfg(test)]
 mod test {
     use super::*;
 
+    #[test]
+    fn setup_parser_correct() {
+let test = r#"
+[fcp_behaviors]
+
+[fcp_behaviors."rust-lang/alpha"]
+close = true
+postpone = true
+
+[fcp_behaviors."foobar/beta"]
+close = false
+
+[fcp_behaviors."bazquux/gamma"]
+postpone = false
+
+[fcp_behaviors."wibble/epsilon"]
+
+[teams]
+
+[teams.avengers]
+name = "The Avengers"
+ping = "marvel/avengers"
+members = [
+  "hulk",
+  "thor",
+  "thevision",
+  "blackwidow",
+  "spiderman",
+  "captainamerica",
+]
+
+[teams.justice-league]
+name = "Justice League of America"
+ping = "dc-comics/justice-league"
+members = [
+  "superman",
+  "wonderwoman",
+  "aquaman",
+  "batman",
+  "theflash"
+]
+"#;
+        let cfg = read_rfcbot_cfg_from(test);
+
+        // Labels are correct:
+        assert_eq!(cfg.team_labels().map(|tl| tl.0.clone()).collect::<Vec<_>>(),
+                   vec!["avengers", "justice-league"]);
+
+        // Teams are correct:
+        let map: BTreeMap<_, _> =
+            cfg.teams().map(|(k, v)| (k.0.clone(), v.clone())).collect();
+
+        let avengers = map.get("avengers").unwrap();
+        assert_eq!(avengers.name, "The Avengers");
+        assert_eq!(avengers.ping, "marvel/avengers");
+        assert_eq!(avengers.member_logins().collect::<Vec<_>>(),
+            vec!["hulk", "thor", "thevision", "blackwidow",
+                 "spiderman", "captainamerica"]);
+
+        let jsa = map.get("justice-league").unwrap();
+        assert_eq!(jsa.name, "Justice League of America");
+        assert_eq!(jsa.ping, "dc-comics/justice-league");
+        assert_eq!(jsa.member_logins().collect::<Vec<_>>(),
+            vec!["superman", "wonderwoman", "aquaman", "batman", "theflash"]);
+
+        // FFCP behavior correct:
+        assert!(cfg.ffcp_auto_close("rust-lang/alpha"));
+        assert!(cfg.ffcp_auto_postpone("rust-lang/alpha"));
+        assert!(!cfg.ffcp_auto_close("foobar/beta"));
+        assert!(!cfg.ffcp_auto_postpone("foobar/beta"));
+        assert!(!cfg.ffcp_auto_close("bazquux/gamma"));
+        assert!(!cfg.ffcp_auto_postpone("bazquux/gamma"));
+        assert!(!cfg.ffcp_auto_close("wibble/epsilon"));
+        assert!(!cfg.ffcp_auto_postpone("wibble/epsilon"));
+        assert!(!cfg.ffcp_auto_close("random"));
+        assert!(!cfg.ffcp_auto_postpone("random"));
+    }
+
+    #[test]
+    fn cfg_file_wellformed() {
+        // Just parse it and ensure that we get no panics for now!
+        // This is a crap test; but, better than nothing.
+        let _ = read_rfcbot_cfg();
+    }
+
     #[test]
     fn team_members_exist() {
-        for (label, team) in TEAMS.iter() {
+        for (label, _) in SETUP.teams.iter() {
             println!("found team {:?}", label);
         }
     }