From 035dac632057068d2ed8ed27acc5e5bf9d93d038 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Sat, 13 Apr 2019 11:43:18 +0200 Subject: [PATCH 1/6] add a per-repository configuration file --- Cargo.lock | 17 ++++++++++++ Cargo.toml | 2 ++ src/config.rs | 61 +++++++++++++++++++++++++++++++++++++++++++ src/github.rs | 20 +++++++++++++- src/handlers/label.rs | 40 ++++++++++++---------------- src/main.rs | 7 +++++ 6 files changed, 122 insertions(+), 25 deletions(-) create mode 100644 src/config.rs diff --git a/Cargo.lock b/Cargo.lock index 0914b9c37..ec3f64acb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -387,6 +387,11 @@ dependencies = [ "unicode-width 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "glob" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "h2" version = "0.1.16" @@ -1504,6 +1509,14 @@ dependencies = [ "serde 1.0.87 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "toml" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "serde 1.0.87 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "traitobject" version = "0.1.0" @@ -1516,6 +1529,7 @@ dependencies = [ "dotenv 0.13.0 (registry+https://github.com/rust-lang/crates.io-index)", "env_logger 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", "failure 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)", + "glob 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "hex 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1528,6 +1542,7 @@ dependencies = [ "rust_team_data 1.0.0 (git+https://github.com/rust-lang/team)", "serde 1.0.87 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 1.0.38 (registry+https://github.com/rust-lang/crates.io-index)", + "toml 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -1771,6 +1786,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum futures 0.1.25 (registry+https://github.com/rust-lang/crates.io-index)" = "49e7653e374fe0d0c12de4250f0bdb60680b8c80eed558c5c7538eec9c89e21b" "checksum futures-cpupool 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)" = "ab90cde24b3319636588d0c35fe03b1333857621051837ed769faefb4c2162e4" "checksum getopts 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)" = "0a7292d30132fb5424b354f5dc02512a86e4c516fe544bb7a25e7f266951b797" +"checksum glob 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "9b919933a397b79c37e33b77bb2aa3dc8eb6e165ad809e58ff75bc7db2e34574" "checksum h2 0.1.16 (registry+https://github.com/rust-lang/crates.io-index)" = "ddb2b25a33e231484694267af28fec74ac63b5ccf51ee2065a5e313b834d836e" "checksum hex 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)" = "805026a5d0141ffc30abb3be3173848ad46a1b1664fe632428479619a3644d77" "checksum http 0.1.15 (registry+https://github.com/rust-lang/crates.io-index)" = "1a10e5b573b9a0146545010f50772b9e8b1dd0a256564cc4307694c68832a2f5" @@ -1887,6 +1903,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum tokio-threadpool 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)" = "c3fd86cb15547d02daa2b21aadaf4e37dee3368df38a526178a5afa3c034d2fb" "checksum tokio-timer 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)" = "2910970404ba6fa78c5539126a9ae2045d62e3713041e447f695f41405a120c6" "checksum toml 0.4.10 (registry+https://github.com/rust-lang/crates.io-index)" = "758664fc71a3a69038656bee8b6be6477d2a6c315a6b81f7081f591bffa4111f" +"checksum toml 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)" = "87c5890a989fa47ecdc7bcb4c63a77a82c18f306714104b1decfd722db17b39e" "checksum traitobject 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "efd1f82c56340fdf16f2a953d7bda4f8fdffba13d93b00844c25572110b26079" "checksum try-lock 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "e604eb7b43c06650e854be16a2a03155743d3752dd1c943f6829e26b7a36e382" "checksum typeable 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "1410f6f91f21d1612654e7cc69193b0334f909dcf2c790c4826254fbb86f8887" diff --git a/Cargo.toml b/Cargo.toml index 167bcff05..8d2dbcfad 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,6 +21,8 @@ hex = "0.3.2" env_logger = "0.6" parser = { path = "parser" } rust_team_data = { git = "https://github.com/rust-lang/team" } +glob = "0.3.0" +toml = "0.5.0" [dependencies.serde] version = "1" diff --git a/src/config.rs b/src/config.rs new file mode 100644 index 000000000..f69fc12f8 --- /dev/null +++ b/src/config.rs @@ -0,0 +1,61 @@ +use crate::github::GithubClient; +use failure::Error; +use std::collections::HashMap; +use std::sync::{Arc, RwLock}; +use std::time::{Duration, Instant}; + +static CONFIG_FILE_NAME: &str = "triagebot.toml"; +const REFRESH_EVERY: Duration = Duration::from_secs(2 * 60); // Every two minutes + +lazy_static::lazy_static! { + static ref CONFIG_CACHE: RwLock, Instant)>> = + RwLock::new(HashMap::new()); +} + +#[derive(serde::Deserialize)] +pub(crate) struct Config { + pub(crate) label: Option, +} + +#[derive(serde::Deserialize)] +#[serde(rename_all = "kebab-case")] +pub(crate) struct LabelConfig { + #[serde(default)] + pub(crate) allow_unauthenticated: Vec, +} + +pub(crate) fn get(gh: &GithubClient, repo: &str) -> Result, Error> { + if let Some(config) = get_cached_config(repo) { + Ok(config) + } else { + get_fresh_config(gh, repo) + } +} + +fn get_cached_config(repo: &str) -> Option> { + let cache = CONFIG_CACHE.read().unwrap(); + cache.get(repo).and_then(|(config, fetch_time)| { + if fetch_time.elapsed() < REFRESH_EVERY { + Some(config.clone()) + } else { + None + } + }) +} + +fn get_fresh_config(gh: &GithubClient, repo: &str) -> Result, Error> { + let contents = gh + .raw_file(repo, "master", CONFIG_FILE_NAME)? + .ok_or_else(|| { + failure::err_msg( + "This repository is not enabled to use triagebot.\n\ + Add a `triagebot.toml` in the root of the master branch to enable it.", + ) + })?; + let config = Arc::new(toml::from_slice::(&contents)?); + CONFIG_CACHE + .write() + .unwrap() + .insert(repo.to_string(), (config.clone(), Instant::now())); + Ok(config) +} diff --git a/src/github.rs b/src/github.rs index bcb1419d5..715166230 100644 --- a/src/github.rs +++ b/src/github.rs @@ -1,6 +1,7 @@ use failure::{Error, ResultExt}; use reqwest::header::{AUTHORIZATION, USER_AGENT}; -use reqwest::{Client, Error as HttpError, RequestBuilder, Response}; +use reqwest::{Client, Error as HttpError, RequestBuilder, Response, StatusCode}; +use std::io::Read; #[derive(Debug, serde::Deserialize)] pub struct User { @@ -183,6 +184,23 @@ impl GithubClient { &self.client } + pub fn raw_file(&self, repo: &str, branch: &str, path: &str) -> Result>, Error> { + let url = format!( + "https://raw.githubusercontent.com/{}/{}/{}", + repo, branch, path + ); + let mut resp = self.get(&url).send()?; + match resp.status() { + StatusCode::OK => { + let mut buf = Vec::with_capacity(resp.content_length().unwrap_or(4) as usize); + resp.read_to_end(&mut buf)?; + Ok(Some(buf)) + } + StatusCode::NOT_FOUND => Ok(None), + status => failure::bail!("failed to GET {}: {}", url, status), + } + } + fn get(&self, url: &str) -> RequestBuilder { log::trace!("get {:?}", url); self.client.get(url).configure(self) diff --git a/src/handlers/label.rs b/src/handlers/label.rs index 82c5717f1..a21fa0bc2 100644 --- a/src/handlers/label.rs +++ b/src/handlers/label.rs @@ -33,6 +33,7 @@ impl Handler for LabelHandler { return Ok(()); }; + let repo = &event.repository.full_name; let mut issue_labels = event.issue.labels().to_owned(); let mut input = Input::new(&event.comment.body, &self.username); @@ -59,8 +60,8 @@ impl Handler for LabelHandler { let mut changed = false; for delta in &deltas { let name = delta.label().as_str(); - if let Err(msg) = check_filter(name, &event.comment.user, &self.client) { - ErrorComment::new(&event.issue, msg).post(&self.client)?; + if let Err(msg) = check_filter(name, repo, &event.comment.user, &self.client) { + ErrorComment::new(&event.issue, msg.to_string()).post(&self.client)?; return Ok(()); } match delta { @@ -89,7 +90,12 @@ impl Handler for LabelHandler { } } -fn check_filter(label: &str, user: &github::User, client: &GithubClient) -> Result<(), String> { +fn check_filter( + label: &str, + repo: &str, + user: &github::User, + client: &GithubClient, +) -> Result<(), Error> { let is_team_member; match user.is_team_member(client) { Ok(true) => return Ok(()), @@ -102,34 +108,20 @@ fn check_filter(label: &str, user: &github::User, client: &GithubClient) -> Resu // continue on; if we failed to check their membership assume that they are not members. } } - if label.starts_with("C-") // categories - || label.starts_with("A-") // areas - || label.starts_with("E-") // easy, mentor, etc. - || label.starts_with("NLL-") - || label.starts_with("O-") // operating systems - || label.starts_with("S-") // status labels - || label.starts_with("T-") - || label.starts_with("WG-") - { - return Ok(()); - } - match label { - "I-compilemem" | "I-compiletime" | "I-crash" | "I-hang" | "I-ICE" | "I-slow" => { + let config = crate::config::get(client, repo)?; + for pattern in &config.label.as_ref().unwrap().allow_unauthenticated { + let pattern = glob::Pattern::new(pattern)?; + if pattern.matches(label) { return Ok(()); } - _ => {} } - if is_team_member.is_ok() { - Err(format!( - "Label {} can only be set by Rust team members", - label - )) + failure::bail!("Label {} can only be set by Rust team members", label); } else { - Err(format!( + failure::bail!( "Label {} can only be set by Rust team members;\ we were unable to check if you are a team member.", label - )) + ); } } diff --git a/src/main.rs b/src/main.rs index d69dcfd0c..39444c311 100644 --- a/src/main.rs +++ b/src/main.rs @@ -15,6 +15,7 @@ use std::sync::Arc; mod handlers; mod registry; +mod config; mod github; mod interactions; mod payload; @@ -37,6 +38,12 @@ pub struct IssueCommentEvent { action: IssueCommentAction, issue: Issue, comment: Comment, + repository: Repository, +} + +#[derive(Debug, serde::Deserialize)] +pub struct Repository { + full_name: String, } enum Event { From 7b9ca8a59e04b8324a58412acba1971978bb0111 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Sat, 13 Apr 2019 12:24:33 +0200 Subject: [PATCH 2/6] move events definition to github.rs --- src/github.rs | 26 +++++++++++++++++++++++++ src/handlers/label.rs | 4 ++-- src/main.rs | 44 +++++++++++-------------------------------- src/registry.rs | 7 +------ 4 files changed, 40 insertions(+), 41 deletions(-) diff --git a/src/github.rs b/src/github.rs index 715166230..d99065b62 100644 --- a/src/github.rs +++ b/src/github.rs @@ -150,6 +150,32 @@ impl Issue { } } +#[derive(PartialEq, Eq, Debug, serde::Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum IssueCommentAction { + Created, + Edited, + Deleted, +} + +#[derive(Debug, serde::Deserialize)] +pub struct IssueCommentEvent { + pub action: IssueCommentAction, + pub issue: Issue, + pub comment: Comment, + pub repository: Repository, +} + +#[derive(Debug, serde::Deserialize)] +pub struct Repository { + pub full_name: String, +} + +#[derive(Debug)] +pub enum Event { + IssueComment(IssueCommentEvent), +} + trait RequestSend: Sized { fn configure(self, g: &GithubClient) -> Self; fn send_req(self) -> Result; diff --git a/src/handlers/label.rs b/src/handlers/label.rs index a21fa0bc2..2b43d1d73 100644 --- a/src/handlers/label.rs +++ b/src/handlers/label.rs @@ -9,9 +9,9 @@ //! notification noise. use crate::{ - github::{self, GithubClient}, + github::{self, Event, GithubClient}, interactions::ErrorComment, - registry::{Event, Handler}, + registry::Handler, }; use failure::Error; use parser::command::label::{LabelCommand, LabelDelta}; diff --git a/src/main.rs b/src/main.rs index 39444c311..51f39dc01 100644 --- a/src/main.rs +++ b/src/main.rs @@ -21,37 +21,15 @@ mod interactions; mod payload; mod team; -use github::{Comment, GithubClient, Issue, User}; use payload::SignedPayload; use registry::HandleRegistry; -#[derive(PartialEq, Eq, Debug, serde::Deserialize)] -#[serde(rename_all = "lowercase")] -pub enum IssueCommentAction { - Created, - Edited, - Deleted, -} - -#[derive(Debug, serde::Deserialize)] -pub struct IssueCommentEvent { - action: IssueCommentAction, - issue: Issue, - comment: Comment, - repository: Repository, -} - -#[derive(Debug, serde::Deserialize)] -pub struct Repository { - full_name: String, -} - -enum Event { +enum EventName { IssueComment, Other, } -impl<'a, 'r> request::FromRequest<'a, 'r> for Event { +impl<'a, 'r> request::FromRequest<'a, 'r> for EventName { type Error = String; fn from_request(req: &'a Request<'r>) -> request::Outcome { let ev = if let Some(ev) = req.headers().get_one("X-GitHub-Event") { @@ -60,8 +38,8 @@ impl<'a, 'r> request::FromRequest<'a, 'r> for Event { return Outcome::Failure((Status::BadRequest, "Needs a X-GitHub-Event".into())); }; let ev = match ev { - "issue_comment" => Event::IssueComment, - _ => Event::Other, + "issue_comment" => EventName::IssueComment, + _ => EventName::Other, }; Outcome::Success(ev) } @@ -89,22 +67,22 @@ impl From for WebhookError { #[post("/github-hook", data = "")] fn webhook( - event: Event, + event: EventName, payload: SignedPayload, reg: State, ) -> Result<(), WebhookError> { match event { - Event::IssueComment => { + EventName::IssueComment => { let payload = payload - .deserialize::() + .deserialize::() .context("IssueCommentEvent failed to deserialize") .map_err(Error::from)?; - let event = registry::Event::IssueComment(payload); + let event = github::Event::IssueComment(payload); reg.handle(&event).map_err(Error::from)?; } // Other events need not be handled - Event::Other => {} + EventName::Other => {} } Ok(()) } @@ -117,11 +95,11 @@ fn not_found(_: &Request) -> &'static str { fn main() { dotenv::dotenv().ok(); let client = Client::new(); - let gh = GithubClient::new( + let gh = github::GithubClient::new( client.clone(), env::var("GITHUB_API_TOKEN").expect("Missing GITHUB_API_TOKEN"), ); - let username = Arc::new(User::current(&gh).unwrap().login); + let username = Arc::new(github::User::current(&gh).unwrap().login); let mut registry = HandleRegistry::new(); handlers::register_all(&mut registry, gh.clone(), username); diff --git a/src/registry.rs b/src/registry.rs index 5785ef0e9..696eb1faf 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -1,4 +1,4 @@ -use crate::IssueCommentEvent; +use crate::github::Event; use failure::Error; pub struct HandleRegistry { @@ -35,11 +35,6 @@ impl HandleRegistry { } } -#[derive(Debug)] -pub enum Event { - IssueComment(IssueCommentEvent), -} - pub trait Handler: Sync + Send { fn handle_event(&self, event: &Event) -> Result<(), Error>; } From be2ac6e4ded5b454eda392264b93ccb63a1df7ea Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Sat, 13 Apr 2019 12:33:04 +0200 Subject: [PATCH 3/6] store shared data needed by handlers in a Context struct --- src/handlers.rs | 13 +++++++------ src/handlers/label.rs | 19 ++++++++----------- src/main.rs | 10 ++++++---- src/registry.rs | 9 ++++++--- 4 files changed, 27 insertions(+), 24 deletions(-) diff --git a/src/handlers.rs b/src/handlers.rs index b76a82013..ca8da7d1c 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -1,16 +1,17 @@ use crate::github::GithubClient; use crate::registry::HandleRegistry; -use std::sync::Arc; //mod assign; mod label; //mod tracking_issue; -pub fn register_all(registry: &mut HandleRegistry, client: GithubClient, username: Arc) { - registry.register(label::LabelHandler { - client: client.clone(), - username: username.clone(), - }); +pub struct Context { + pub github: GithubClient, + pub username: String, +} + +pub fn register_all(registry: &mut HandleRegistry) { + registry.register(label::LabelHandler); //registry.register(assign::AssignmentHandler { // client: client.clone(), //}); diff --git a/src/handlers/label.rs b/src/handlers/label.rs index 2b43d1d73..92abe88bd 100644 --- a/src/handlers/label.rs +++ b/src/handlers/label.rs @@ -10,21 +10,18 @@ use crate::{ github::{self, Event, GithubClient}, + handlers::Context, interactions::ErrorComment, registry::Handler, }; use failure::Error; use parser::command::label::{LabelCommand, LabelDelta}; use parser::command::{Command, Input}; -use std::sync::Arc; -pub struct LabelHandler { - pub client: GithubClient, - pub username: Arc, -} +pub struct LabelHandler; impl Handler for LabelHandler { - fn handle_event(&self, event: &Event) -> Result<(), Error> { + fn handle_event(&self, ctx: &Context, event: &Event) -> Result<(), Error> { #[allow(irrefutable_let_patterns)] let event = if let Event::IssueComment(e) = event { e @@ -36,7 +33,7 @@ impl Handler for LabelHandler { let repo = &event.repository.full_name; let mut issue_labels = event.issue.labels().to_owned(); - let mut input = Input::new(&event.comment.body, &self.username); + let mut input = Input::new(&event.comment.body, &ctx.username); let deltas = match input.parse_command() { Command::Label(Ok(LabelCommand(deltas))) => deltas, Command::Label(Err(err)) => { @@ -47,7 +44,7 @@ impl Handler for LabelHandler { event.comment.html_url, err ), ) - .post(&self.client)?; + .post(&ctx.github)?; failure::bail!( "label parsing failed for issue #{}, error: {:?}", event.issue.number, @@ -60,8 +57,8 @@ impl Handler for LabelHandler { let mut changed = false; for delta in &deltas { let name = delta.label().as_str(); - if let Err(msg) = check_filter(name, repo, &event.comment.user, &self.client) { - ErrorComment::new(&event.issue, msg.to_string()).post(&self.client)?; + if let Err(msg) = check_filter(name, repo, &event.comment.user, &ctx.github) { + ErrorComment::new(&event.issue, msg.to_string()).post(&ctx.github)?; return Ok(()); } match delta { @@ -83,7 +80,7 @@ impl Handler for LabelHandler { } if changed { - event.issue.set_labels(&self.client, issue_labels)?; + event.issue.set_labels(&ctx.github, issue_labels)?; } Ok(()) diff --git a/src/main.rs b/src/main.rs index 51f39dc01..915303d11 100644 --- a/src/main.rs +++ b/src/main.rs @@ -10,7 +10,6 @@ use rocket::request; use rocket::State; use rocket::{http::Status, Outcome, Request}; use std::env; -use std::sync::Arc; mod handlers; mod registry; @@ -99,9 +98,12 @@ fn main() { client.clone(), env::var("GITHUB_API_TOKEN").expect("Missing GITHUB_API_TOKEN"), ); - let username = Arc::new(github::User::current(&gh).unwrap().login); - let mut registry = HandleRegistry::new(); - handlers::register_all(&mut registry, gh.clone(), username); + let ctx = handlers::Context { + github: gh.clone(), + username: github::User::current(&gh).unwrap().login, + }; + let mut registry = HandleRegistry::new(ctx); + handlers::register_all(&mut registry); let mut config = rocket::Config::active().unwrap(); config.set_port( diff --git a/src/registry.rs b/src/registry.rs index 696eb1faf..b443b468b 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -1,14 +1,17 @@ use crate::github::Event; +use crate::handlers::Context; use failure::Error; pub struct HandleRegistry { handlers: Vec>, + ctx: Context, } impl HandleRegistry { - pub fn new() -> HandleRegistry { + pub fn new(ctx: Context) -> HandleRegistry { HandleRegistry { handlers: Vec::new(), + ctx, } } @@ -19,7 +22,7 @@ impl HandleRegistry { pub fn handle(&self, event: &Event) -> Result<(), Error> { let mut last_error = None; for h in &self.handlers { - match h.handle_event(event) { + match h.handle_event(&self.ctx, event) { Ok(()) => {} Err(e) => { eprintln!("event handling failed: {:?}", e); @@ -36,5 +39,5 @@ impl HandleRegistry { } pub trait Handler: Sync + Send { - fn handle_event(&self, event: &Event) -> Result<(), Error>; + fn handle_event(&self, ctx: &Context, event: &Event) -> Result<(), Error>; } From 568737977d6585fb4b0a3b1a4ace4f0d12687d2e Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Sat, 13 Apr 2019 13:13:19 +0200 Subject: [PATCH 4/6] refactor handlers to register them using a macro --- src/github.rs | 8 +++++++ src/handlers.rs | 56 +++++++++++++++++++++++++++++++++---------- src/handlers/label.rs | 48 ++++++++++++++++++++++++------------- src/main.rs | 11 +++------ src/registry.rs | 43 --------------------------------- 5 files changed, 86 insertions(+), 80 deletions(-) delete mode 100644 src/registry.rs diff --git a/src/github.rs b/src/github.rs index d99065b62..3a9aa50b5 100644 --- a/src/github.rs +++ b/src/github.rs @@ -176,6 +176,14 @@ pub enum Event { IssueComment(IssueCommentEvent), } +impl Event { + pub fn repo_name(&self) -> &str { + match self { + Event::IssueComment(event) => &event.repository.full_name, + } + } +} + trait RequestSend: Sized { fn configure(self, g: &GithubClient) -> Self; fn send_req(self) -> Result; diff --git a/src/handlers.rs b/src/handlers.rs index ca8da7d1c..7b2972994 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -1,21 +1,51 @@ -use crate::github::GithubClient; -use crate::registry::HandleRegistry; +use crate::github::{Event, GithubClient}; +use failure::Error; -//mod assign; -mod label; -//mod tracking_issue; +macro_rules! handlers { + ($($name:ident = $handler:expr,)*) => { + $(mod $name;)* + + pub fn handle(ctx: &Context, event: &Event) -> Result<(), Error> { + $(if let Some(input) = Handler::parse_input(&$handler, ctx, event)? { + let config = crate::config::get(&ctx.github, event.repo_name())?; + if let Some(config) = &config.$name { + Handler::handle_input(&$handler, ctx, config, event, input)?; + } else { + failure::bail!( + "The feature `{}` is not enabled in this repository.\n\ + To enable it add its section in the `triagebot.toml` \ + in the root of the repository.", + stringify!($name) + ); + } + })* + Ok(()) + } + } +} + +handlers! { + //assign = assign::AssignmentHandler, + label = label::LabelHandler, + //tracking_issue = tracking_issue::TrackingIssueHandler, +} pub struct Context { pub github: GithubClient, pub username: String, } -pub fn register_all(registry: &mut HandleRegistry) { - registry.register(label::LabelHandler); - //registry.register(assign::AssignmentHandler { - // client: client.clone(), - //}); - //registry.register(tracking_issue::TrackingIssueHandler { - // client: client.clone(), - //}); +pub trait Handler: Sync + Send { + type Input; + type Config; + + fn parse_input(&self, ctx: &Context, event: &Event) -> Result, Error>; + + fn handle_input( + &self, + ctx: &Context, + config: &Self::Config, + event: &Event, + input: Self::Input, + ) -> Result<(), Error>; } diff --git a/src/handlers/label.rs b/src/handlers/label.rs index 92abe88bd..eb590be90 100644 --- a/src/handlers/label.rs +++ b/src/handlers/label.rs @@ -9,33 +9,33 @@ //! notification noise. use crate::{ + config::LabelConfig, github::{self, Event, GithubClient}, - handlers::Context, + handlers::{Context, Handler}, interactions::ErrorComment, - registry::Handler, }; use failure::Error; use parser::command::label::{LabelCommand, LabelDelta}; use parser::command::{Command, Input}; -pub struct LabelHandler; +pub(super) struct LabelHandler; impl Handler for LabelHandler { - fn handle_event(&self, ctx: &Context, event: &Event) -> Result<(), Error> { + type Input = LabelCommand; + type Config = LabelConfig; + + fn parse_input(&self, ctx: &Context, 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(()); + return Ok(None); }; - let repo = &event.repository.full_name; - let mut issue_labels = event.issue.labels().to_owned(); - let mut input = Input::new(&event.comment.body, &ctx.username); - let deltas = match input.parse_command() { - Command::Label(Ok(LabelCommand(deltas))) => deltas, + match input.parse_command() { + Command::Label(Ok(command)) => Ok(Some(command)), Command::Label(Err(err)) => { ErrorComment::new( &event.issue, @@ -51,13 +51,30 @@ impl Handler for LabelHandler { err ); } - _ => return Ok(()), + _ => Ok(None), + } + } + + fn handle_input( + &self, + ctx: &Context, + config: &LabelConfig, + event: &Event, + input: LabelCommand, + ) -> Result<(), Error> { + #[allow(irrefutable_let_patterns)] + let event = if let Event::IssueComment(e) = event { + e + } else { + // not interested in other events + return Ok(()); }; + let mut issue_labels = event.issue.labels().to_owned(); let mut changed = false; - for delta in &deltas { + for delta in &input.0 { let name = delta.label().as_str(); - if let Err(msg) = check_filter(name, repo, &event.comment.user, &ctx.github) { + if let Err(msg) = check_filter(name, config, &event.comment.user, &ctx.github) { ErrorComment::new(&event.issue, msg.to_string()).post(&ctx.github)?; return Ok(()); } @@ -89,7 +106,7 @@ impl Handler for LabelHandler { fn check_filter( label: &str, - repo: &str, + config: &LabelConfig, user: &github::User, client: &GithubClient, ) -> Result<(), Error> { @@ -105,8 +122,7 @@ fn check_filter( // continue on; if we failed to check their membership assume that they are not members. } } - let config = crate::config::get(client, repo)?; - for pattern in &config.label.as_ref().unwrap().allow_unauthenticated { + for pattern in &config.allow_unauthenticated { let pattern = glob::Pattern::new(pattern)?; if pattern.matches(label) { return Ok(()); diff --git a/src/main.rs b/src/main.rs index 915303d11..ee11c6404 100644 --- a/src/main.rs +++ b/src/main.rs @@ -12,8 +12,6 @@ use rocket::{http::Status, Outcome, Request}; use std::env; mod handlers; -mod registry; - mod config; mod github; mod interactions; @@ -21,7 +19,6 @@ mod payload; mod team; use payload::SignedPayload; -use registry::HandleRegistry; enum EventName { IssueComment, @@ -68,7 +65,7 @@ impl From for WebhookError { fn webhook( event: EventName, payload: SignedPayload, - reg: State, + ctx: State, ) -> Result<(), WebhookError> { match event { EventName::IssueComment => { @@ -78,7 +75,7 @@ fn webhook( .map_err(Error::from)?; let event = github::Event::IssueComment(payload); - reg.handle(&event).map_err(Error::from)?; + handlers::handle(&ctx, &event)?; } // Other events need not be handled EventName::Other => {} @@ -102,8 +99,6 @@ fn main() { github: gh.clone(), username: github::User::current(&gh).unwrap().login, }; - let mut registry = HandleRegistry::new(ctx); - handlers::register_all(&mut registry); let mut config = rocket::Config::active().unwrap(); config.set_port( @@ -113,7 +108,7 @@ fn main() { ); rocket::custom(config) .manage(gh) - .manage(registry) + .manage(ctx) .mount("/", routes![webhook]) .register(catchers![not_found]) .launch(); diff --git a/src/registry.rs b/src/registry.rs deleted file mode 100644 index b443b468b..000000000 --- a/src/registry.rs +++ /dev/null @@ -1,43 +0,0 @@ -use crate::github::Event; -use crate::handlers::Context; -use failure::Error; - -pub struct HandleRegistry { - handlers: Vec>, - ctx: Context, -} - -impl HandleRegistry { - pub fn new(ctx: Context) -> HandleRegistry { - HandleRegistry { - handlers: Vec::new(), - ctx, - } - } - - pub fn register(&mut self, h: H) { - self.handlers.push(Box::new(h)); - } - - pub fn handle(&self, event: &Event) -> Result<(), Error> { - let mut last_error = None; - for h in &self.handlers { - match h.handle_event(&self.ctx, event) { - Ok(()) => {} - Err(e) => { - eprintln!("event handling failed: {:?}", e); - last_error = Some(e); - } - } - } - if let Some(err) = last_error { - Err(err) - } else { - Ok(()) - } - } -} - -pub trait Handler: Sync + Send { - fn handle_event(&self, ctx: &Context, event: &Event) -> Result<(), Error>; -} From 60dcfba99db72eb7b8aa74238982806e9bbabc9d Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Sat, 13 Apr 2019 13:28:48 +0200 Subject: [PATCH 5/6] post errors in the issues whenever possible --- src/github.rs | 6 ++++++ src/handlers/label.rs | 13 ++----------- src/main.rs | 10 ++++++++-- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/github.rs b/src/github.rs index 3a9aa50b5..9b8dc0930 100644 --- a/src/github.rs +++ b/src/github.rs @@ -182,6 +182,12 @@ impl Event { Event::IssueComment(event) => &event.repository.full_name, } } + + pub fn issue(&self) -> Option<&Issue> { + match self { + Event::IssueComment(event) => Some(&event.issue), + } + } } trait RequestSend: Sized { diff --git a/src/handlers/label.rs b/src/handlers/label.rs index eb590be90..a20fb4d7d 100644 --- a/src/handlers/label.rs +++ b/src/handlers/label.rs @@ -37,18 +37,9 @@ impl Handler for LabelHandler { match input.parse_command() { Command::Label(Ok(command)) => Ok(Some(command)), Command::Label(Err(err)) => { - ErrorComment::new( - &event.issue, - format!( - "Parsing label command in [comment]({}) failed: {}", - event.comment.html_url, err - ), - ) - .post(&ctx.github)?; failure::bail!( - "label parsing failed for issue #{}, error: {:?}", - event.issue.number, - err + "Parsing label command in [comment]({}) failed: {}", + event.comment.html_url, err ); } _ => Ok(None), diff --git a/src/main.rs b/src/main.rs index ee11c6404..a7015cf67 100644 --- a/src/main.rs +++ b/src/main.rs @@ -11,13 +11,14 @@ use rocket::State; use rocket::{http::Status, Outcome, Request}; use std::env; -mod handlers; mod config; mod github; +mod handlers; mod interactions; mod payload; mod team; +use interactions::ErrorComment; use payload::SignedPayload; enum EventName { @@ -75,7 +76,12 @@ fn webhook( .map_err(Error::from)?; let event = github::Event::IssueComment(payload); - handlers::handle(&ctx, &event)?; + if let Err(err) = handlers::handle(&ctx, &event) { + if let Some(issue) = event.issue() { + ErrorComment::new(issue, err.to_string()).post(&ctx.github)?; + } + return Err(err.into()); + } } // Other events need not be handled EventName::Other => {} From 5496e8f46241f5a40991727043316bbed62c03d4 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Sat, 13 Apr 2019 13:37:10 +0200 Subject: [PATCH 6/6] rename label to relabel --- parser/src/command.rs | 12 +++++------ parser/src/command/{label.rs => relabel.rs} | 8 +++---- src/config.rs | 4 ++-- src/handlers.rs | 2 +- src/handlers/{label.rs => relabel.rs} | 24 ++++++++++----------- 5 files changed, 25 insertions(+), 25 deletions(-) rename parser/src/command/{label.rs => relabel.rs} (97%) rename src/handlers/{label.rs => relabel.rs} (88%) diff --git a/parser/src/command.rs b/parser/src/command.rs index 2f02777c8..06378de1c 100644 --- a/parser/src/command.rs +++ b/parser/src/command.rs @@ -2,7 +2,7 @@ use crate::code_block::ColorCodeBlocks; use crate::error::Error; use crate::token::{Token, Tokenizer}; -pub mod label; +pub mod relabel; pub fn find_commmand_start(input: &str, bot: &str) -> Option { input.find(&format!("@{}", bot)) @@ -10,7 +10,7 @@ pub fn find_commmand_start(input: &str, bot: &str) -> Option { #[derive(Debug)] pub enum Command<'a> { - Label(Result>), + Relabel(Result>), None, } @@ -50,14 +50,14 @@ impl<'a> Input<'a> { { let mut tok = original_tokenizer.clone(); - let res = label::LabelCommand::parse(&mut tok); + let res = relabel::RelabelCommand::parse(&mut tok); match res { Ok(None) => {} Ok(Some(cmd)) => { - success.push((tok, Command::Label(Ok(cmd)))); + success.push((tok, Command::Relabel(Ok(cmd)))); } Err(err) => { - success.push((tok, Command::Label(Err(err)))); + success.push((tok, Command::Relabel(Err(err)))); } } } @@ -94,7 +94,7 @@ impl<'a> Input<'a> { impl<'a> Command<'a> { pub fn is_ok(&self) -> bool { match self { - Command::Label(r) => r.is_ok(), + Command::Relabel(r) => r.is_ok(), Command::None => true, } } diff --git a/parser/src/command/label.rs b/parser/src/command/relabel.rs similarity index 97% rename from parser/src/command/label.rs rename to parser/src/command/relabel.rs index 2a15f0311..a73ede6dd 100644 --- a/parser/src/command/label.rs +++ b/parser/src/command/relabel.rs @@ -30,7 +30,7 @@ use std::error::Error as _; use std::fmt; #[derive(Debug)] -pub struct LabelCommand(pub Vec); +pub struct RelabelCommand(pub Vec); #[derive(Debug, PartialEq, Eq)] pub enum LabelDelta { @@ -124,7 +124,7 @@ fn delta_empty() { assert_eq!(err.position(), 1); } -impl LabelCommand { +impl RelabelCommand { pub fn parse<'a>(input: &mut Tokenizer<'a>) -> Result, Error<'a>> { let mut toks = input.clone(); if let Some(Token::Word("modify")) = toks.next_token()? { @@ -163,7 +163,7 @@ impl LabelCommand { if let Some(Token::Dot) | Some(Token::EndOfLine) = toks.peek_token()? { toks.next_token()?; *input = toks; - return Ok(Some(LabelCommand(deltas))); + return Ok(Some(RelabelCommand(deltas))); } } } @@ -172,7 +172,7 @@ impl LabelCommand { #[cfg(test)] fn parse<'a>(input: &'a str) -> Result>, Error<'a>> { let mut toks = Tokenizer::new(input); - Ok(LabelCommand::parse(&mut toks)?.map(|c| c.0)) + Ok(RelabelCommand::parse(&mut toks)?.map(|c| c.0)) } #[test] diff --git a/src/config.rs b/src/config.rs index f69fc12f8..d932556bf 100644 --- a/src/config.rs +++ b/src/config.rs @@ -14,12 +14,12 @@ lazy_static::lazy_static! { #[derive(serde::Deserialize)] pub(crate) struct Config { - pub(crate) label: Option, + pub(crate) relabel: Option, } #[derive(serde::Deserialize)] #[serde(rename_all = "kebab-case")] -pub(crate) struct LabelConfig { +pub(crate) struct RelabelConfig { #[serde(default)] pub(crate) allow_unauthenticated: Vec, } diff --git a/src/handlers.rs b/src/handlers.rs index 7b2972994..75ebf7a26 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -26,7 +26,7 @@ macro_rules! handlers { handlers! { //assign = assign::AssignmentHandler, - label = label::LabelHandler, + relabel = relabel::RelabelHandler, //tracking_issue = tracking_issue::TrackingIssueHandler, } diff --git a/src/handlers/label.rs b/src/handlers/relabel.rs similarity index 88% rename from src/handlers/label.rs rename to src/handlers/relabel.rs index a20fb4d7d..e060ce1ac 100644 --- a/src/handlers/label.rs +++ b/src/handlers/relabel.rs @@ -3,26 +3,26 @@ //! Labels are checked against the labels in the project; the bot does not support creating new //! labels. //! -//! Parsing is done in the `parser::command::label` module. +//! Parsing is done in the `parser::command::relabel` module. //! //! If the command was successful, there will be no feedback beyond the label change to reduce //! notification noise. use crate::{ - config::LabelConfig, + config::RelabelConfig, github::{self, Event, GithubClient}, handlers::{Context, Handler}, interactions::ErrorComment, }; use failure::Error; -use parser::command::label::{LabelCommand, LabelDelta}; +use parser::command::relabel::{RelabelCommand, LabelDelta}; use parser::command::{Command, Input}; -pub(super) struct LabelHandler; +pub(super) struct RelabelHandler; -impl Handler for LabelHandler { - type Input = LabelCommand; - type Config = LabelConfig; +impl Handler for RelabelHandler { + type Input = RelabelCommand; + type Config = RelabelConfig; fn parse_input(&self, ctx: &Context, event: &Event) -> Result, Error> { #[allow(irrefutable_let_patterns)] @@ -35,8 +35,8 @@ impl Handler for LabelHandler { let mut input = Input::new(&event.comment.body, &ctx.username); match input.parse_command() { - Command::Label(Ok(command)) => Ok(Some(command)), - Command::Label(Err(err)) => { + Command::Relabel(Ok(command)) => Ok(Some(command)), + Command::Relabel(Err(err)) => { failure::bail!( "Parsing label command in [comment]({}) failed: {}", event.comment.html_url, err @@ -49,9 +49,9 @@ impl Handler for LabelHandler { fn handle_input( &self, ctx: &Context, - config: &LabelConfig, + config: &RelabelConfig, event: &Event, - input: LabelCommand, + input: RelabelCommand, ) -> Result<(), Error> { #[allow(irrefutable_let_patterns)] let event = if let Event::IssueComment(e) = event { @@ -97,7 +97,7 @@ impl Handler for LabelHandler { fn check_filter( label: &str, - config: &LabelConfig, + config: &RelabelConfig, user: &github::User, client: &GithubClient, ) -> Result<(), Error> {