Restructure into individual handlers

This allows easier splitting (and testing) of functionality.

src/github.rs

@@ -44,11 +44,7 @@ pub struct Comment {
 }
 
 impl Issue {
-    pub fn set_labels(
-        &mut self,
-        client: &GithubClient,
-        mut labels: Vec<Label>,
-    ) -> Result<(), Error> {
+    pub fn set_labels(&self, client: &GithubClient, mut labels: Vec<Label>) -> Result<(), Error> {
         // PUT /repos/:owner/:repo/issues/:number/labels
         // repo_url = https://api.github.com/repos/Codertocat/Hello-World
         // Might need `Accept: application/vnd.github.symmetra-preview+json` for emoji and descriptions
@@ -72,7 +68,6 @@ impl Issue {
             .send_req()
             .context("failed to set labels")?;
 
-        self.labels = labels;
         Ok(())
     }
 
@@ -81,7 +76,7 @@ impl Issue {
     }
 
     #[allow(unused)]
-    pub fn add_assignee(&mut self, client: &GithubClient, user: &str) -> Result<(), Error> {
+    pub fn add_assignee(&self, client: &GithubClient, user: &str) -> Result<(), Error> {
         unimplemented!()
     }
 }
@@ -106,6 +101,7 @@ impl RequestSend for RequestBuilder {
     }
 }
 
+#[derive(Clone)]
 pub struct GithubClient {
     token: String,
     client: Client,

src/handlers.rs

@@ -0,0 +1,18 @@
+use crate::github::GithubClient;
+use crate::registry::HandleRegistry;
+
+mod assign;
+mod label;
+mod tracking_issue;
+
+pub fn register_all(registry: &mut HandleRegistry, client: GithubClient) {
+    registry.register(label::LabelHandler {
+        client: client.clone(),
+    });
+    registry.register(assign::AssignmentHandler {
+        client: client.clone(),
+    });
+    registry.register(tracking_issue::TrackingIssueHandler {
+        client: client.clone(),
+    });
+}

src/handlers/assign.rs

@@ -0,0 +1,66 @@
+//! Permit assignment of any user to issues, without requiring "write" access to the repository.
+//!
+//! It is unknown which approach is needed here: we may need to fake-assign ourselves and add a
+//! 'claimed by' section to the top-level comment. That would be very unideal.
+//!
+//! The ideal workflow here is that the user is added to a read-only team with no access to the
+//! repository and immediately thereafter assigned to the issue.
+//!
+//! Such assigned issues should also be placed in a queue to ensure that the user remains
+//! active; the assigned user will be asked for a status report every 2 weeks (XXX: timing).
+//!
+//! If we're intending to ask for a status report but no comments from the assigned user have
+//! been given for the past 2 weeks, the bot will de-assign the user. They can once more claim
+//! the issue if necessary.
+//!
+//! Assign users with `assign: @gh-user` or `@bot claim` (self-claim).
+
+use crate::{
+    github::GithubClient,
+    registry::{Event, Handler},
+};
+use failure::Error;
+use lazy_static::lazy_static;
+use regex::Regex;
+
+pub struct AssignmentHandler {
+    pub client: GithubClient,
+}
+
+impl Handler for AssignmentHandler {
+    fn handle_event(&self, event: &Event) -> Result<(), Error> {
+        #[allow(irrefutable_let_patterns)]
+        let event = if let Event::IssueComment(e) = event {
+            e
+        } else {
+            // not interested in other events
+            return Ok(());
+        };
+
+        lazy_static! {
+            static ref RE_ASSIGN: Regex = Regex::new(r"\bassign: @(\S+)").unwrap();
+            static ref RE_CLAIM: Regex =
+                Regex::new(&format!(r"\b@{} claim\b", crate::BOT_USER_NAME)).unwrap();
+        }
+
+        // XXX: Handle updates to the comment specially to avoid double-queueing or double-assigning
+        // and other edge cases.
+
+        if RE_CLAIM.is_match(&event.comment.body) {
+            event
+                .issue
+                .add_assignee(&self.client, &event.comment.user.login)?;
+        } else {
+            if let Some(capture) = RE_ASSIGN.captures(&event.comment.body) {
+                event.issue.add_assignee(&self.client, &capture[1])?;
+            }
+        }
+
+        // TODO: Enqueue a check-in in two weeks.
+        // TODO: Post a comment documenting the biweekly check-in? Maybe just give them two weeks
+        //       without any commentary from us.
+        // TODO: How do we handle `claim`/`assign:` if someone's already assigned? Error out?
+
+        Ok(())
+    }
+}

src/handlers/label.rs

@@ -0,0 +1,67 @@
+//! Purpose: Allow any user to modify issue labels on GitHub via comments.
+//!
+//! The current syntax allows adding labels (+labelname or just labelname) following the
+//! `label:` prefix. Users can also remove labels with -labelname.
+//!
+//! No verification is currently attempted of the added labels (only currently present labels
+//! can be removed). XXX: How does this affect users workflow?
+//!
+//! There will be no feedback beyond the label change to reduce notification noise.
+
+use crate::{
+    github::{GithubClient, Label},
+    registry::{Event, Handler},
+};
+use failure::Error;
+use lazy_static::lazy_static;
+use regex::Regex;
+
+pub struct LabelHandler {
+    pub client: GithubClient,
+}
+
+impl Handler for LabelHandler {
+    fn handle_event(&self, event: &Event) -> Result<(), Error> {
+        #[allow(irrefutable_let_patterns)]
+        let event = if let Event::IssueComment(e) = event {
+            e
+        } else {
+            // not interested in other events
+            return Ok(());
+        };
+
+        lazy_static! {
+            static ref LABEL_RE: Regex = Regex::new(r#"\blabel: (\S+\s*)+"#).unwrap();
+        }
+
+        let mut issue_labels = event.issue.labels().to_owned();
+
+        for label_block in LABEL_RE.find_iter(&event.comment.body) {
+            let label_block = &label_block.as_str()["label: ".len()..]; // guaranteed to start with this
+            for label in label_block.split_whitespace() {
+                if label.starts_with('-') {
+                    if let Some(label) = issue_labels.iter().position(|el| el.name == &label[1..]) {
+                        issue_labels.remove(label);
+                    } else {
+                        // do nothing, if the user attempts to remove a label that's not currently
+                        // set simply skip it
+                    }
+                } else if label.starts_with('+') {
+                    // add this label, but without the +
+                    issue_labels.push(Label {
+                        name: label[1..].to_string(),
+                    });
+                } else {
+                    // add this label (literally)
+                    issue_labels.push(Label {
+                        name: label.to_string(),
+                    });
+                }
+            }
+        }
+
+        event.issue.set_labels(&self.client, issue_labels)?;
+
+        Ok(())
+    }
+}

src/handlers/tracking_issue.rs

@@ -0,0 +1,99 @@
+use crate::{
+    github::GithubClient,
+    registry::{Event, Handler},
+    team::Team,
+    IssueCommentAction, IssueCommentEvent,
+};
+use failure::Error;
+use lazy_static::lazy_static;
+use regex::Regex;
+
+pub struct TrackingIssueHandler {
+    pub client: GithubClient,
+}
+
+impl TrackingIssueHandler {
+    /// Automates creating tracking issues.
+    ///
+    /// This command is initially restricted to members of Rust teams.
+    ///
+    /// This command is rare, and somewhat high-impact, so it requires the `@bot` prefix.
+    /// The syntax for creating a tracking issue follows. Note that only the libs and lang teams are
+    /// currently supported; it's presumed that the other teams may want significantly different
+    /// issue formats, so only these two are supported for the time being.
+    ///
+    /// `@bot tracking-issue create feature="<short feature description>" team=[libs|lang]`
+    ///
+    /// This creates the tracking issue, though it's likely that the invokee will want to edit its
+    /// body/title.
+    ///
+    /// Long-term, this will also create a thread on internals and lock the tracking issue,
+    /// directing commentary to the thread, but for the time being we limit the scope of work as
+    /// well as project impact.
+    fn handle_create(&self, event: &IssueCommentEvent) -> Result<(), Error> {
+        lazy_static! {
+            static ref RE_TRACKING: Regex = Regex::new(&format!(
+                r#"\b@{} tracking-issue create feature=("[^"]+|\S+) team=(libs|lang)"#,
+                crate::BOT_USER_NAME,
+            ))
+            .unwrap();
+        }
+
+        // Skip this event if the comment is edited or deleted.
+        if event.action != IssueCommentAction::Created {
+            return Ok(());
+        }
+
+        #[allow(unused)]
+        let feature;
+        #[allow(unused)]
+        let team;
+
+        if let Some(captures) = RE_TRACKING.captures(&event.comment.body) {
+            #[allow(unused)]
+            {
+                feature = captures.get(1).unwrap();
+                team = captures.get(2).unwrap().as_str().parse::<Team>()?;
+            }
+        } else {
+            // no tracking issue creation comment
+            return Ok(());
+        }
+
+        // * Create tracking issue (C-tracking-issue, T-{team})
+        // * Post comment with link to issue and suggestion on what to do next
+
+        Ok(())
+    }
+
+    /// Links issues to tracking issues.
+    ///
+    /// We verify that the tracking issue listed is in fact a tracking issue (i.e., has the
+    /// C-tracking-issue label). Next, the tracking issue's top comment is updated with a link and
+    /// title of the issue linked as a checkbox in the bugs list.
+    ///
+    /// We also label the issue with `tracked-bug`.
+    ///
+    /// TODO: Check the checkbox in the tracking issue when `tracked-bug` is closed.
+    ///
+    /// Syntax: `link: #xxx`
+    fn handle_link(&self, _event: &IssueCommentEvent) -> Result<(), Error> {
+        Ok(())
+    }
+}
+
+impl Handler for TrackingIssueHandler {
+    fn handle_event(&self, event: &Event) -> Result<(), Error> {
+        #[allow(irrefutable_let_patterns)]
+        let event = if let Event::IssueComment(e) = event {
+            e
+        } else {
+            // not interested in other events
+            return Ok(());
+        };
+
+        self.handle_create(&event)?;
+        self.handle_link(&event)?;
+        Ok(())
+    }
+}

src/main.rs

@@ -3,26 +3,24 @@
 #[macro_use]
 extern crate rocket;
 
-use failure::Error;
-use failure::ResultExt;
-use lazy_static::lazy_static;
-use regex::Regex;
 use reqwest::Client;
 use rocket::request;
 use rocket::State;
 use rocket::{http::Status, Outcome, Request};
 use std::env;
 
+mod handlers;
+mod registry;
+
 mod github;
 mod payload;
 mod team;
 
-use payload::SignedPayload;
-
 static BOT_USER_NAME: &str = "rust-highfive";
 
-use github::{Comment, GithubClient, Issue, Label};
-use team::Team;
+use github::{Comment, GithubClient, Issue};
+use payload::SignedPayload;
+use registry::HandleRegistry;
 
 #[derive(PartialEq, Eq, Debug, serde::Deserialize)]
 #[serde(rename_all = "lowercase")]
@@ -33,176 +31,12 @@ pub enum IssueCommentAction {
 }
 
 #[derive(Debug, serde::Deserialize)]
-struct IssueCommentEvent {
+pub struct IssueCommentEvent {
     action: IssueCommentAction,
     issue: Issue,
     comment: Comment,
 }
 
-impl IssueCommentEvent {
-    /// Purpose: Allow any user to modify issue labels on GitHub via comments.
-    ///
-    /// The current syntax allows adding labels (+labelname or just labelname) following the
-    /// `label:` prefix. Users can also remove labels with -labelname.
-    ///
-    /// No verification is currently attempted of the added labels (only currently present labels
-    /// can be removed). XXX: How does this affect users workflow?
-    ///
-    /// There will be no feedback beyond the label change to reduce notification noise.
-    fn handle_labels(&mut self, g: &GithubClient) -> Result<(), Error> {
-        lazy_static! {
-            static ref LABEL_RE: Regex = Regex::new(r#"\blabel: (\S+\s*)+"#).unwrap();
-        }
-
-        let mut issue_labels = self.issue.labels().to_owned();
-
-        for label_block in LABEL_RE.find_iter(&self.comment.body) {
-            let label_block = &label_block.as_str()["label: ".len()..]; // guaranteed to start with this
-            for label in label_block.split_whitespace() {
-                if label.starts_with('-') {
-                    if let Some(label) = issue_labels.iter().position(|el| el.name == &label[1..]) {
-                        issue_labels.remove(label);
-                    } else {
-                        // do nothing, if the user attempts to remove a label that's not currently
-                        // set simply skip it
-                    }
-                } else if label.starts_with('+') {
-                    // add this label, but without the +
-                    issue_labels.push(Label {
-                        name: label[1..].to_string(),
-                    });
-                } else {
-                    // add this label (literally)
-                    issue_labels.push(Label {
-                        name: label.to_string(),
-                    });
-                }
-            }
-        }
-
-        self.issue.set_labels(&g, issue_labels)
-    }
-
-    /// Permit assignment of any user to issues, without requiring "write" access to the repository.
-    ///
-    /// It is unknown which approach is needed here: we may need to fake-assign ourselves and add a
-    /// 'claimed by' section to the top-level comment. That would be very unideal.
-    ///
-    /// The ideal workflow here is that the user is added to a read-only team with no access to the
-    /// repository and immediately thereafter assigned to the issue.
-    ///
-    /// Such assigned issues should also be placed in a queue to ensure that the user remains
-    /// active; the assigned user will be asked for a status report every 2 weeks (XXX: timing).
-    ///
-    /// If we're intending to ask for a status report but no comments from the assigned user have
-    /// been given for the past 2 weeks, the bot will de-assign the user. They can once more claim
-    /// the issue if necessary.
-    ///
-    /// Assign users with `assign: @gh-user` or `@bot claim` (self-claim).
-    #[allow(unused)]
-    fn handle_assign(&mut self, g: &GithubClient) -> Result<(), Error> {
-        lazy_static! {
-            static ref RE_ASSIGN: Regex = Regex::new(r"\bassign: @(\S+)").unwrap();
-            static ref RE_CLAIM: Regex =
-                Regex::new(&format!(r"\b@{} claim\b", BOT_USER_NAME)).unwrap();
-        }
-
-        // XXX: Handle updates to the comment specially to avoid double-queueing or double-assigning
-        // and other edge cases.
-
-        if RE_CLAIM.is_match(&self.comment.body) {
-            self.issue.add_assignee(g, &self.comment.user.login)?;
-        } else {
-            if let Some(capture) = RE_ASSIGN.captures(&self.comment.body) {
-                self.issue.add_assignee(g, &capture[1])?;
-            }
-        }
-
-        // TODO: Enqueue a check-in in two weeks.
-        // TODO: Post a comment documenting the biweekly check-in? Maybe just give them two weeks
-        //       without any commentary from us.
-        // TODO: How do we handle `claim`/`assign:` if someone's already assigned? Error out?
-
-        Ok(())
-    }
-
-    /// Automates creating tracking issues.
-    ///
-    /// This command is initially restricted to members of Rust teams.
-    ///
-    /// This command is rare, and somewhat high-impact, so it requires the `@bot` prefix.
-    /// The syntax for creating a tracking issue follows. Note that only the libs and lang teams are
-    /// currently supported; it's presumed that the other teams may want significantly different
-    /// issue formats, so only these two are supported for the time being.
-    ///
-    /// `@bot tracking-issue create feature="<short feature description>" team=[libs|lang]`
-    ///
-    /// This creates the tracking issue, though it's likely that the invokee will want to edit its
-    /// body/title.
-    ///
-    /// Long-term, this will also create a thread on internals and lock the tracking issue,
-    /// directing commentary to the thread, but for the time being we limit the scope of work as
-    /// well as project impact.
-    #[allow(unused)]
-    fn handle_create_tracking_issue(&mut self, g: &GithubClient) -> Result<(), Error> {
-        lazy_static! {
-            static ref RE_TRACKING: Regex = Regex::new(&format!(
-                r#"\b@{} tracking-issue create feature=("[^"]+|\S+) team=(libs|lang)"#,
-                BOT_USER_NAME,
-            ))
-            .unwrap();
-        }
-
-        // Skip this event if the comment is edited or deleted.
-        if self.action != IssueCommentAction::Created {
-            return Ok(());
-        }
-
-        let feature;
-        let team;
-
-        if let Some(captures) = RE_TRACKING.captures(&self.comment.body) {
-            feature = captures.get(1).unwrap();
-            team = captures.get(2).unwrap().as_str().parse::<Team>()?;
-        } else {
-            // no tracking issue creation comment
-            return Ok(());
-        }
-
-        // * Create tracking issue (C-tracking-issue, T-{team})
-        // * Post comment with link to issue and suggestion on what to do next
-
-        unimplemented!()
-    }
-
-    /// Links issues to tracking issues.
-    ///
-    /// We verify that the tracking issue listed is in fact a tracking issue (i.e., has the
-    /// C-tracking-issue label). Next, the tracking issue's top comment is updated with a link and
-    /// title of the issue linked as a checkbox in the bugs list.
-    ///
-    /// We also label the issue with `tracked-bug`.
-    ///
-    /// TODO: Check the checkbox in the tracking issue when `tracked-bug` is closed.
-    ///
-    /// Syntax: `link: #xxx`
-    #[allow(unused)]
-    fn handle_link_tracking_issue(&mut self, g: &GithubClient) -> Result<(), Error> {
-        unimplemented!()
-    }
-
-    fn run(mut self, g: &GithubClient) -> Result<(), Error> {
-        // Don't do anything on deleted comments.
-        //
-        // XXX: Should we attempt to roll back the action instead?
-        if self.action == IssueCommentAction::Deleted {
-            return Ok(());
-        }
-        self.handle_labels(g).context("handle_labels")?;
-        Ok(())
-    }
-}
-
 enum Event {
     IssueComment,
     Other,
@@ -225,17 +59,16 @@ impl<'a, 'r> request::FromRequest<'a, 'r> for Event {
 }
 
 #[post("/github-hook", data = "<payload>")]
-fn webhook(
-    event: Event,
-    payload: SignedPayload,
-    client: State<GithubClient>,
-) -> Result<(), String> {
+fn webhook(event: Event, payload: SignedPayload, reg: State<HandleRegistry>) -> Result<(), String> {
     match event {
-        Event::IssueComment => payload
-            .deserialize::<IssueCommentEvent>()
-            .map_err(|e| format!("IssueCommentEvent failed to deserialize: {:?}", e))?
-            .run(&client)
-            .map_err(|e| format!("{:?}", e))?,
+        Event::IssueComment => {
+            let payload = payload
+                .deserialize::<IssueCommentEvent>()
+                .map_err(|e| format!("IssueCommentEvent failed to deserialize: {:?}", e))?;
+
+            let event = registry::Event::IssueComment(payload);
+            reg.handle(&event).map_err(|e| format!("{:?}", e))?;
+        }
         // Other events need not be handled
         Event::Other => {}
     }
@@ -250,11 +83,12 @@ fn not_found(_: &Request) -> &'static str {
 fn main() {
     dotenv::dotenv().ok();
     let client = Client::new();
+    let gh = GithubClient::new(client.clone(), env::var("GITHUB_API_TOKEN").unwrap());
+    let mut registry = HandleRegistry::new();
+    handlers::register_all(&mut registry, gh.clone());
     rocket::ignite()
-        .manage(GithubClient::new(
-            client.clone(),
-            env::var("GITHUB_API_TOKEN").unwrap(),
-        ))
+        .manage(gh)
+        .manage(registry)
         .mount("/", routes![webhook])
         .register(catchers![not_found])
         .launch();

src/registry.rs

@@ -0,0 +1,34 @@
+use crate::IssueCommentEvent;
+use failure::Error;
+
+pub struct HandleRegistry {
+    handlers: Vec<Box<dyn Handler>>,
+}
+
+impl HandleRegistry {
+    pub fn new() -> HandleRegistry {
+        HandleRegistry {
+            handlers: Vec::new(),
+        }
+    }
+
+    pub fn register<H: Handler + 'static>(&mut self, h: H) {
+        self.handlers.push(Box::new(h));
+    }
+
+    pub fn handle(&self, event: &Event) -> Result<(), Error> {
+        for h in &self.handlers {
+            h.handle_event(event)?;
+        }
+        Ok(())
+    }
+}
+
+#[derive(Debug)]
+pub enum Event {
+    IssueComment(IssueCommentEvent),
+}
+
+pub trait Handler: Sync + Send {
+    fn handle_event(&self, event: &Event) -> Result<(), Error>;
+}