diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 00000000..7222abe8 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,14 @@ +# CHANGELOG + +`@rfcbot` is Rust's automated process manager for RFCs, tracking issues, etc. +As such, this application does not follow semver of any form. + +However, we do maintain this changelog explaining high level changes to the way +rfcbot behaves so that the people who develop rfcbot or interact with it can +understand the bot better. + +## Changes + ++ The bot will now optionally (per configuration in `rfcbot.toml`) remove + unwanted reactions from RFC (and PR..) readers. + These include 👎, 😕 on the `rust-lang/rfcs` and `rust-lang/rust` repositories. diff --git a/Cargo.lock b/Cargo.lock index bbc73804..a6797a44 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -72,7 +72,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "num-integer 0.1.36 (registry+https://github.com/rust-lang/crates.io-index)", "num-traits 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", - "serde 1.0.41 (registry+https://github.com/rust-lang/crates.io-index)", + "serde 1.0.59 (registry+https://github.com/rust-lang/crates.io-index)", "time 0.1.39 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -276,7 +276,7 @@ dependencies = [ "pest 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", "quick-error 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "regex 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)", - "serde 1.0.41 (registry+https://github.com/rust-lang/crates.io-index)", + "serde 1.0.59 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 1.0.15 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -582,7 +582,7 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "0.3.6" +version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "unicode-xid 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -600,10 +600,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "quote" -version = "0.5.1" +version = "0.6.2" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ - "proc-macro2 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", + "proc-macro2 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -702,6 +702,7 @@ dependencies = [ name = "rfcbot-rs" version = "0.1.0" dependencies = [ + "arrayvec 0.4.7 (registry+https://github.com/rust-lang/crates.io-index)", "chrono 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", "diesel 0.16.0 (registry+https://github.com/rust-lang/crates.io-index)", "diesel_codegen 0.16.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -719,8 +720,8 @@ dependencies = [ "rocket_codegen 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", "rocket_contrib 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", "rust-crypto 0.2.36 (registry+https://github.com/rust-lang/crates.io-index)", - "serde 1.0.41 (registry+https://github.com/rust-lang/crates.io-index)", - "serde_derive 1.0.41 (registry+https://github.com/rust-lang/crates.io-index)", + "serde 1.0.59 (registry+https://github.com/rust-lang/crates.io-index)", + "serde_derive 1.0.59 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 1.0.15 (registry+https://github.com/rust-lang/crates.io-index)", "toml 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "url 1.7.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -781,7 +782,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "log 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)", "rocket 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)", - "serde 1.0.41 (registry+https://github.com/rust-lang/crates.io-index)", + "serde 1.0.59 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 1.0.15 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -856,27 +857,17 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "serde" -version = "1.0.41" +version = "1.0.59" source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "serde_derive" -version = "1.0.41" -source = "registry+https://github.com/rust-lang/crates.io-index" -dependencies = [ - "proc-macro2 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", - "quote 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", - "serde_derive_internals 0.23.1 (registry+https://github.com/rust-lang/crates.io-index)", - "syn 0.13.1 (registry+https://github.com/rust-lang/crates.io-index)", -] - -[[package]] -name = "serde_derive_internals" -version = "0.23.1" +version = "1.0.59" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ - "proc-macro2 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", - "syn 0.13.1 (registry+https://github.com/rust-lang/crates.io-index)", + "proc-macro2 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", + "quote 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", + "syn 0.14.0 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -898,7 +889,7 @@ dependencies = [ "dtoa 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", "itoa 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)", "num-traits 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", - "serde 1.0.41 (registry+https://github.com/rust-lang/crates.io-index)", + "serde 1.0.59 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -923,11 +914,11 @@ dependencies = [ [[package]] name = "syn" -version = "0.13.1" +version = "0.14.0" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ - "proc-macro2 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", - "quote 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", + "proc-macro2 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)", + "quote 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", "unicode-xid 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -972,7 +963,7 @@ name = "toml" version = "0.4.6" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ - "serde 1.0.41 (registry+https://github.com/rust-lang/crates.io-index)", + "serde 1.0.59 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -1200,10 +1191,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum pkg-config 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)" = "3a8b4c6b8165cd1a1cd4b9b120978131389f64bdaf456435caa41e630edba903" "checksum plugin 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)" = "1a6a0dc3910bc8db877ffed8e457763b317cf880df4ae19109b9f77d277cf6e0" "checksum pq-sys 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)" = "4dfb5e575ef93a1b7b2a381d47ba7c5d4e4f73bff37cee932195de769aad9a54" -"checksum proc-macro2 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)" = "49b6a521dc81b643e9a51e0d1cf05df46d5a2f3c0280ea72bcb68276ba64a118" +"checksum proc-macro2 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)" = "a45f2f0ae0b5757f6fe9e68745ba25f5246aea3598984ed81d013865873c1f84" "checksum quick-error 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "eda5fe9b71976e62bc81b781206aaa076401769b2143379d3eb2118388babac4" "checksum quote 0.3.15 (registry+https://github.com/rust-lang/crates.io-index)" = "7a6e920b65c65f10b2ae65c831a81a073a89edd28c7cce89475bff467ab4167a" -"checksum quote 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "7b0ff51282f28dc1b53fd154298feaa2e77c5ea0dba68e1fd8b03b72fbe13d2a" +"checksum quote 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)" = "9e53eeda07ddbd8b057dde66d9beded11d0dfda13f0db0769e6b71d6bcf2074e" "checksum r2d2 0.7.4 (registry+https://github.com/rust-lang/crates.io-index)" = "2c8284508b38df440f8f3527395e23c4780b22f74226b270daf58fee38e4bcce" "checksum r2d2-diesel 0.16.0 (registry+https://github.com/rust-lang/crates.io-index)" = "f6b921696a6c45991296d21b52ed973b9fb56f6c47524fda1f99458c2d6c0478" "checksum rand 0.3.22 (registry+https://github.com/rust-lang/crates.io-index)" = "15a732abf9d20f0ad8eeb6f909bf6868722d9a06e1e50802b6a70351f40b4eb1" @@ -1227,15 +1218,14 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum security-framework 0.1.16 (registry+https://github.com/rust-lang/crates.io-index)" = "dfa44ee9c54ce5eecc9de7d5acbad112ee58755239381f687e564004ba4a2332" "checksum security-framework-sys 0.1.16 (registry+https://github.com/rust-lang/crates.io-index)" = "5421621e836278a0b139268f36eee0dc7e389b784dc3f79d8f11aabadf41bead" "checksum serde 0.8.23 (registry+https://github.com/rust-lang/crates.io-index)" = "9dad3f759919b92c3068c696c15c3d17238234498bbdcc80f2c469606f948ac8" -"checksum serde 1.0.41 (registry+https://github.com/rust-lang/crates.io-index)" = "5f751e9d57bd42502e4362b2d84f916ed9578e9a1a46852dcdeb6f91f6de7c14" -"checksum serde_derive 1.0.41 (registry+https://github.com/rust-lang/crates.io-index)" = "81ec46cb12594da6750ad5a913f8175798c371d6c5ffd07f082ac4fea32789c3" -"checksum serde_derive_internals 0.23.1 (registry+https://github.com/rust-lang/crates.io-index)" = "9d30c4596450fd7bbda79ef15559683f9a79ac0193ea819db90000d7e1cae794" +"checksum serde 1.0.59 (registry+https://github.com/rust-lang/crates.io-index)" = "2a4d976362a13caad61c38cf841401d2d4d480496a9391c3842c288b01f9de95" +"checksum serde_derive 1.0.59 (registry+https://github.com/rust-lang/crates.io-index)" = "94bb618afe46430c6b089e9b111dc5b2fcd3e26a268da0993f6d16bea51c6021" "checksum serde_json 0.8.6 (registry+https://github.com/rust-lang/crates.io-index)" = "67f7d2e9edc3523a9c8ec8cd6ec481b3a27810aafee3e625d311febd3e656b4c" "checksum serde_json 1.0.15 (registry+https://github.com/rust-lang/crates.io-index)" = "7bf1cbb1387028a13739cb018ee0d9b3db534f22ca3c84a5904f7eadfde14e75" "checksum smallvec 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)" = "ee4f357e8cd37bf8822e1b964e96fd39e2cb5a0424f8aaa284ccaccc2162411c" "checksum state 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "d5562ac59585fe3d9a1ccf6b4e298ce773f5063db80d59f783776b410c1714c2" "checksum syn 0.11.11 (registry+https://github.com/rust-lang/crates.io-index)" = "d3b891b9015c88c576343b9b3e41c2c11a51c219ef067b264bd9c8aa9b441dad" -"checksum syn 0.13.1 (registry+https://github.com/rust-lang/crates.io-index)" = "91b52877572087400e83d24b9178488541e3d535259e04ff17a63df1e5ceff59" +"checksum syn 0.14.0 (registry+https://github.com/rust-lang/crates.io-index)" = "99d991a9e7c33123925e511baab68f7ec25c3795962fe326a2395e5a42a614f0" "checksum synom 0.11.3 (registry+https://github.com/rust-lang/crates.io-index)" = "a393066ed9010ebaed60b9eafa373d4b1baac186dd7e008555b0f702b51945b6" "checksum tempdir 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)" = "15f2b5fb00ccdf689e0149d1b1b3c03fead81c2b37735d812fa8bddbbf41b6d8" "checksum thread_local 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)" = "279ef31c19ededf577bfd12dfae728040a21f635b06a24cd670ff510edd38963" diff --git a/Cargo.toml b/Cargo.toml index 73560eaf..8f0adaed 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,12 +18,13 @@ rocket = "0.3.3" rocket_codegen = "0.3.3" rocket_contrib = "0.3.3" rust-crypto = "0.2.36" -serde = "1.0" -serde_derive = "1.0" +serde = "1.0.59" +serde_derive = "1.0.59" serde_json = "1.0" toml = "0.4" url = "1.4" urlencoded = "0.5" +arrayvec = "0.4.7" [dependencies.chrono] features = ["serde"] diff --git a/rfcbot.toml b/rfcbot.toml index def9665c..8e92efbc 100644 --- a/rfcbot.toml +++ b/rfcbot.toml @@ -1,3 +1,21 @@ +[prohibited_reactions] + +[prohibited_reactions."rust-lang/rfcs".issue] +down_vote = true +confused = true + +[prohibited_reactions."rust-lang/rfcs".comment] +down_vote = true +confused = true + +[prohibited_reactions."rust-lang/rust".issue] +down_vote = true +confused = true + +[prohibited_reactions."rust-lang/rust".comment] +down_vote = true +confused = true + [fcp_behaviors] [fcp_behaviors."rust-lang/rfcs"] diff --git a/src/config.rs b/src/config.rs index 3f3df535..03100b5d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -71,26 +71,17 @@ pub fn init() -> Result> { .collect::>(); let db_url = vars.remove(DB_URL).unwrap(); - let db_pool_size = vars.remove(DB_POOL_SIZE).unwrap(); - let db_pool_size = match db_pool_size.parse::() { - Ok(size) => size, - Err(_) => return Err(vec![DB_POOL_SIZE]), - }; + let db_pool_size = vars.remove(DB_POOL_SIZE).unwrap().parse::(); + let db_pool_size = ok_or!(db_pool_size, throw!(vec![DB_POOL_SIZE])); let gh_token = vars.remove(GITHUB_TOKEN).unwrap(); let gh_ua = vars.remove(GITHUB_UA).unwrap(); - let gh_interval = vars.remove(GITHUB_INTERVAL).unwrap(); - let gh_interval = match gh_interval.parse::() { - Ok(interval) => interval, - Err(_) => return Err(vec![GITHUB_INTERVAL]), - }; + let gh_interval = vars.remove(GITHUB_INTERVAL).unwrap().parse::(); + let gh_interval = ok_or!(gh_interval, throw!(vec![GITHUB_INTERVAL])); - let post_comments = vars.remove(POST_COMMENTS).unwrap(); - let post_comments = match post_comments.parse::() { - Ok(pc) => pc, - Err(_) => return Err(vec![POST_COMMENTS]), - }; + let post_comments = vars.remove(POST_COMMENTS).unwrap().parse::(); + let post_comments = ok_or!(post_comments, throw!(vec![POST_COMMENTS])); let webhook_secrets = vars.remove(GITHUB_WEBHOOK_SECRETS).unwrap(); let webhook_secrets = webhook_secrets.split(',').map(String::from).collect(); @@ -106,11 +97,9 @@ pub fn init() -> Result> { }) } else { - Err(vars.iter() .filter(|&(_, v)| v.is_err()) .map(|(&k, _)| k) .collect()) - } } diff --git a/src/github/client.rs b/src/github/client.rs index fd2e0357..e57eecff 100644 --- a/src/github/client.rs +++ b/src/github/client.rs @@ -18,7 +18,11 @@ use serde_json; use config::CONFIG; use error::{DashError, DashResult}; -use github::models::{CommentFromJson, IssueFromJson, PullRequestFromJson, PullRequestUrls}; +use github::models::{ + CommentFromJson, IssueFromJson, + PullRequestFromJson, PullRequestUrls, + ReactionsIssueFromJson, ReactionsCommentFromJson, ReactionFromJson, Reaction +}; pub const BASE_URL: &'static str = "https://api.github.com"; @@ -26,6 +30,16 @@ pub const DELAY: u64 = 300; type ParameterMap = BTreeMap<&'static str, String>; +macro_rules! params { + ($($key: expr => $val: expr),*) => {{ + let mut map = BTreeMap::<_, _>::new(); + $( + map.insert($key, $val); + )* + map + }}; +} + header! { (TZ, "Time-Zone") => [String] } header! { (Accept, "Accept") => [String] } header! { (RateLimitRemaining, "X-RateLimit-Remaining") => [u32] } @@ -43,6 +57,12 @@ pub struct Client { rate_limit_timeout: DateTime, } +fn read_to_string(reader: &mut R) -> DashResult { + let mut string = String::new(); + reader.read_to_string(&mut string)?; + Ok(string) +} + impl Client { pub fn new() -> Self { let tls_connector = HttpsConnector::new(NativeTlsClient::new().unwrap()); @@ -60,7 +80,7 @@ impl Client { pub fn org_repos(&self, org: &str) -> DashResult> { let url = format!("{}/orgs/{}/repos", BASE_URL, org); - let vals: Vec = try!(self.get_models(&url, None)); + let vals: Vec = self.get_models(&url, None)?; let mut repos = Vec::new(); for v in vals { @@ -72,39 +92,36 @@ impl Client { } } } - return Err(DashError::Misc(None)); + throw!(DashError::Misc(None)) } Ok(repos) } - pub fn issues_since(&self, repo: &str, start: DateTime) -> DashResult> { - - let url = format!("{}/repos/{}/issues", BASE_URL, repo); - let mut params = ParameterMap::new(); - - params.insert("state", "all".to_string()); - params.insert("since", format!("{:?}", start)); - params.insert("state", "all".to_string()); - params.insert("per_page", format!("{}", PER_PAGE)); - params.insert("direction", "asc".to_string()); - - self.get_models(&url, Some(¶ms)) + pub fn issues_since(&self, repo: &str, start: DateTime) + -> DashResult> + { + self.get_models(&format!("{}/repos/{}/issues", BASE_URL, repo), + Some(¶ms! { + "state" => "all".to_string(), + "since" => format!("{:?}", start), + "state" => "all".to_string(), + "per_page" => format!("{}", PER_PAGE), + "direction" => "asc".to_string() + })) } pub fn comments_since(&self, repo: &str, start: DateTime) -> DashResult> { - let url = format!("{}/repos/{}/issues/comments", BASE_URL, repo); - let mut params = ParameterMap::new(); - - params.insert("sort", "created".to_string()); - params.insert("direction", "asc".to_string()); - params.insert("since", format!("{:?}", start)); - params.insert("per_page", format!("{}", PER_PAGE)); - - self.get_models(&url, Some(¶ms)) + self.get_models(&format!("{}/repos/{}/issues/comments", BASE_URL, repo), + Some(¶ms! { + "sort" => "created".to_string(), + "direction" => "asc".to_string(), + "since" => format!("{:?}", start), + "per_page" => format!("{}", PER_PAGE) + })) } fn get_models(&self, @@ -112,7 +129,7 @@ impl Client { params: Option<&ParameterMap>) -> DashResult> { - let mut res = try!(self.get(start_url, params)); + let mut res = self.get(start_url, params)?; let mut models = self.deserialize::>(&mut res)?; while let Some(url) = Self::next_page(&res.headers) { sleep(Duration::from_millis(DELAY)); @@ -122,14 +139,26 @@ impl Client { Ok(models) } - pub fn fetch_pull_request(&self, pr_info: &PullRequestUrls) -> DashResult { - let url = pr_info.get("url"); + fn get_models_preview + (&self, start_url: &str, params: Option<&ParameterMap>) + -> DashResult> { + + let mut res = self.get_preview(start_url, params)?; + let mut models = self.deserialize::>(&mut res)?; + while let Some(url) = Self::next_page(&res.headers) { + sleep(Duration::from_millis(DELAY)); + res = self.get(&url, None)?; + models.extend(self.deserialize::>(&mut res)?); + } + Ok(models) + } - if let Some(url) = url { - let mut res = try!(self.get(url, None)); + pub fn fetch_pull_request(&self, pr_info: &PullRequestUrls) -> DashResult { + if let Some(url) = pr_info.get("url") { + let mut res = self.get(url, None)?; self.deserialize(&mut res) } else { - Err(DashError::Misc(None)) + throw!(DashError::Misc(None)) } } @@ -158,21 +187,14 @@ impl Client { pub fn close_issue(&self, repo: &str, issue_num: i32) -> DashResult<()> { let url = format!("{}/repos/{}/issues/{}", BASE_URL, repo, issue_num); - - let mut obj = BTreeMap::new(); - obj.insert("state", "closed"); - let payload = serde_json::to_string(&obj)?; - + let payload = serde_json::to_string(¶ms!("state" => "closed"))?; let mut res = self.patch(&url, &payload)?; - match res.status { - StatusCode::Ok => Ok(()), - _ => { - let mut body = String::new(); - res.read_to_string(&mut body)?; - Err(DashError::Misc(Some(body))) - } + if StatusCode::Ok != res.status { + throw!(DashError::Misc(Some(read_to_string(&mut res)?))) } + + Ok(()) } pub fn add_label(&self, repo: &str, issue_num: i32, label: &str) -> DashResult<()> { @@ -181,14 +203,11 @@ impl Client { let mut res = self.post(&url, &payload)?; - match res.status { - StatusCode::Ok => Ok(()), - _ => { - let mut body = String::new(); - res.read_to_string(&mut body)?; - Err(DashError::Misc(Some(body))) - } + if StatusCode::Ok != res.status { + throw!(DashError::Misc(Some(read_to_string(&mut res)?))) } + + Ok(()) } pub fn remove_label(&self, repo: &str, issue_num: i32, label: &str) -> DashResult<()> { @@ -199,14 +218,11 @@ impl Client { label); let mut res = self.delete(&url)?; - match res.status { - StatusCode::NoContent => Ok(()), - _ => { - let mut body = String::new(); - res.read_to_string(&mut body)?; - Err(DashError::Misc(Some(body))) - } + if StatusCode::NoContent != res.status { + throw!(DashError::Misc(Some(read_to_string(&mut res)?))) } + + Ok(()) } pub fn new_comment(&self, @@ -215,11 +231,7 @@ impl Client { text: &str) -> DashResult { let url = format!("{}/repos/{}/issues/{}/comments", BASE_URL, repo, issue_num); - - let mut obj = BTreeMap::new(); - obj.insert("body", text); - - let payload = serde_json::to_string(&obj)?; + let payload = serde_json::to_string(¶ms!("body" => text))?; // FIXME propagate an error if it's a 404 or other error self.deserialize(&mut self.post(&url, &payload)?) @@ -234,16 +246,72 @@ impl Client { BASE_URL, repo, comment_num); - - let mut obj = BTreeMap::new(); - obj.insert("body", text); - - let payload = serde_json::to_string(&obj)?; + let payload = serde_json::to_string(¶ms!("body" => text))?; // FIXME propagate an error if it's a 404 or other error self.deserialize(&mut self.patch(&url, &payload)?) } + pub fn comments_of_issue(&self, repo: &str, issue_num: i32) + -> DashResult> + { + let url = format!("{}/repos/{}/issues/{}/comments", BASE_URL, repo, issue_num); + let params = params! { + "per_page" => format!("{}", PER_PAGE), + "direction" => "asc".to_string() + }; + Ok(self.get_models_preview(&url, Some(¶ms))?.into_iter()) + } + + pub fn open_issues_with_reactions(&self, repo: &str) + -> DashResult> + { + let url = format!("{}/repos/{}/issues", BASE_URL, repo); + let params = params! { + "state" => "open".to_string(), + "per_page" => format!("{}", PER_PAGE), + "direction" => "asc".to_string() + }; + Ok(self.get_models_preview(&url, Some(¶ms))?.into_iter()) + } + + pub fn issue_reactions(&self, repo: &str, issue_num: i32, reaction: Reaction) + -> DashResult> + { + self.reactions(reaction, format!( + "{}/repos/{}/issues/{}/reactions", + BASE_URL, repo, issue_num)) + } + + pub fn comment_reactions(&self, repo: &str, comment_id: i32, reaction: Reaction) + -> DashResult> + { + self.reactions(reaction, format!( + "{}/repos/{}/issues/comments/{}/reactions", + BASE_URL, repo, comment_id)) + } + + fn reactions(&self, reaction: Reaction, url: String) + -> DashResult> + { + let params = params! { + "content" => reaction.to_param().into(), + "per_page" => format!("{}", PER_PAGE) + }; + Ok(self.get_models_preview(&url, Some(¶ms))? + .into_iter() + .map(|rfj: ReactionFromJson| rfj.id)) + } + + pub fn delete_reaction(&self, reaction_id: usize) -> DashResult<()> { + let url = format!("{}/reactions/{}", BASE_URL, reaction_id); + let mut res = self.delete_preview(&url)?; + if StatusCode::NoContent != res.status { + throw!(DashError::Misc(Some(read_to_string(&mut res)?))) + } + Ok(()) + } + fn patch(&self, url: &str, payload: &str) -> Result { self.set_headers(self.client.patch(url).body(payload)) .send() @@ -257,11 +325,32 @@ impl Client { self.set_headers(self.client.delete(url)).send() } + fn delete_preview(&self, url: &str) -> Result { + self.set_headers_preview(self.client.delete(url)).send() + } + fn get(&self, url: &str, params: Option<&ParameterMap>) -> Result { - let qp_string = match params { + let qp_string = Self::serialize_qp(params); + let url = format!("{}{}", url, qp_string); + debug!("GETing: {}", &url); + self.set_headers(self.client.get(&url)).send() + } + + fn get_preview(&self, + url: &str, + params: Option<&ParameterMap>) + -> Result { + let qp_string = Self::serialize_qp(params); + let url = format!("{}{}", url, qp_string); + debug!("GETing: {}", &url); + self.set_headers_preview(self.client.get(&url)).send() + } + + fn serialize_qp(params: Option<&ParameterMap>) -> String { + match params { Some(p) => { let mut qp = String::from("?"); for (k, v) in p { @@ -273,28 +362,24 @@ impl Client { qp } None => "".to_string(), - }; - - let url = format!("{}{}", url, qp_string); - - debug!("GETing: {}", &url); - - self.set_headers(self.client.get(&url)).send() + } } fn deserialize(&self, res: &mut Response) -> DashResult { - let mut buf = String::new(); - res.read_to_string(&mut buf)?; - + let buf = read_to_string(res)?; match serde_json::from_str(&buf) { Ok(m) => Ok(m), Err(why) => { error!("Unable to parse from JSON ({:?}): {}", why, buf); - Err(why.into()) + throw!(why) } } } + fn set_headers_preview<'a>(&self, req: RequestBuilder<'a>) -> RequestBuilder<'a> { + req.header(Accept("application/vnd.github.squirrel-girl-preview+json".to_string())) + } + fn set_headers<'a>(&self, req: RequestBuilder<'a>) -> RequestBuilder<'a> { req.header(Authorization(format!("token {}", &self.token))) .header(UserAgent(self.ua.clone())) diff --git a/src/github/mod.rs b/src/github/mod.rs index 999ba072..ad64732c 100644 --- a/src/github/mod.rs +++ b/src/github/mod.rs @@ -16,6 +16,7 @@ use DB_POOL; use domain::github::*; use domain::schema::*; use error::DashResult; +use teams::SETUP; use self::client::Client; use self::models::{CommentFromJson, IssueFromJson, PullRequestFromJson}; @@ -61,8 +62,8 @@ pub fn record_successful_update(ingest_start: NaiveDateTime) -> DashResult<()> { pub fn ingest_since(repo: &str, start: DateTime) -> DashResult<()> { info!("fetching all {} issues and comments since {}", repo, start); - let issues = try!(GH.issues_since(repo, start)); - let mut comments = try!(GH.comments_since(repo, start)); + let issues = GH.issues_since(repo, start)?; + let mut comments = GH.comments_since(repo, start)?; // make sure we process the new comments in creation order comments.sort_by_key(|c| c.created_at); @@ -70,13 +71,10 @@ pub fn ingest_since(repo: &str, start: DateTime) -> DashResult<()> { for issue in &issues { // sleep(Duration::from_millis(github::client::DELAY)); if let Some(ref pr_info) = issue.pull_request { - match GH.fetch_pull_request(pr_info) { - Ok(pr) => prs.push(pr), - Err(why) => { - error!("ERROR fetching PR info: {:?}", why); - break; - } - } + prs.push(ok_or!(GH.fetch_pull_request(pr_info), why => { + error!("ERROR fetching PR info: {:?}", why); + break; + })); } } @@ -96,37 +94,23 @@ pub fn ingest_since(repo: &str, start: DateTime) -> DashResult<()> { // make sure we have all of the users to ensure referential integrity for issue in issues { let issue_number = issue.number; - match handle_issue(conn, issue, repo) { - Ok(()) => (), - Err(why) => { - error!("Error processing issue {}#{}: {:?}", - repo, - issue_number, - why) - } - } + ok_or!(handle_issue(conn, issue, repo), why => + error!("Error processing issue {}#{}: {:?}", + repo, issue_number, why)); } // insert the comments for comment in comments { let comment_id = comment.id; - match handle_comment(conn, comment, repo) { - Ok(()) => (), - Err(why) => { - error!("Error processing comment {}#{}: {:?}", - repo, - comment_id, - why) - } - } + ok_or!(handle_comment(conn, comment, repo), why => + error!("Error processing comment {}#{}: {:?}", + repo, comment_id, why)); } for pr in prs { let pr_number = pr.number; - match handle_pr(conn, pr, repo) { - Ok(()) => (), - Err(why) => error!("Error processing PR {}#{}: {:?}", repo, pr_number, why), - } + ok_or!(handle_pr(conn, pr, repo), why => + error!("Error processing PR {}#{}: {:?}", repo, pr_number, why)); } Ok(()) @@ -159,19 +143,18 @@ pub fn handle_comment(conn: &PgConnection, comment: CommentFromJson, repo: &str) diesel::update(issuecomment::table.find(comment.id)) .set(&comment) .execute(conn)?; - Ok(()) } else { diesel::insert(&comment) .into(issuecomment::table) .execute(conn)?; - match nag::update_nags(&comment) { - Ok(()) => Ok(()), - Err(why) => { - error!("Problem updating FCPs: {:?}", &why); - Err(why) - } - } + + ok_or!(nag::update_nags(&comment), why => { + error!("Problem updating FCPs: {:?}", &why); + throw!(why); + }); } + + Ok(()) } pub fn handle_issue(conn: &PgConnection, issue: IssueFromJson, repo: &str) -> DashResult<()> { @@ -210,6 +193,52 @@ pub fn handle_user(conn: &PgConnection, user: &GitHubUser) -> DashResult<()> { Ok(()) } +pub fn nuke_reactions(repo: &str) -> DashResult<()> { + let issue_reactions = SETUP.prohibited_issue_reactions(repo); + let comment_reactions = SETUP.prohibited_comment_reactions(repo); + + if issue_reactions.is_empty() && comment_reactions.is_empty() { + // All reactions are permitted, bail early to avoid work. + return Ok(()); + } + + // Fetch all issues with some forbidden reaction: + let issues = GH.open_issues_with_reactions(repo)?.filter(|rifj| + issue_reactions.iter().any(|&react| rifj.reactions.has_reaction(react)) + ); + + for issue in issues { + // Remove all forbidden reactions for this issue: + for &reaction in issue_reactions.iter() { + for to_delete in GH.issue_reactions(repo, issue.number, reaction)? { + GH.delete_reaction(to_delete)?; + } + } + + // Ensure we forbid some reactions and that there are comments: + if comment_reactions.is_empty() || issue.comments == 0 { + continue; + } + + // Fetch all comments with some forbidden reaction: + let comments = GH.comments_of_issue(repo, issue.number)?.filter(|rifj| + comment_reactions.iter() + .any(|&react| rifj.reactions.has_reaction(react)) + ); + + for comment in comments { + // Remove all forbidden reactions for this comment: + for &reaction in comment_reactions.iter() { + for to_delete in GH.comment_reactions(repo, comment.id, reaction)? { + GH.delete_reaction(to_delete)?; + } + } + } + } + + Ok(()) +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/github/models.rs b/src/github/models.rs index 9e310f2c..74b212c7 100644 --- a/src/github/models.rs +++ b/src/github/models.rs @@ -129,13 +129,12 @@ impl CommentFromJson { } }; - let conn = try!(DB_POOL.get()); + let conn = DB_POOL.get()?; - let issue_id = try!(issue - .select(id) - .filter(number.eq(issue_number)) - .filter(repository.eq(repo)) - .first::(&*conn)); + let issue_id = issue.select(id) + .filter(number.eq(issue_number)) + .filter(repository.eq(repo)) + .first::(&*conn)?; Ok(IssueComment { id: self.id, @@ -191,3 +190,75 @@ impl PullRequestFromJson { } } } + +#[derive(Debug, Deserialize)] +pub struct ReactionsIssueFromJson { + pub number: i32, + pub comments: i32, + pub reactions: ReactionSummary, +} + +#[derive(Debug, Deserialize)] +pub struct ReactionsCommentFromJson { + pub id: i32, + pub reactions: ReactionSummary, +} + +#[derive(Debug, Deserialize)] +pub struct ReactionSummary { + #[serde(rename = "+1")] + pub up_vote: u32, + #[serde(rename = "-1")] + pub down_vote: u32, + pub laugh: u32, + pub hooray: u32, + pub confused: u32, + pub heart: u32, +} + +impl ReactionSummary { + pub fn has_reaction(&self, reaction: Reaction) -> bool { + self.reaction_count(reaction) > 0 + } + + pub fn reaction_count(&self, reaction: Reaction) -> u32 { + use self::Reaction::*; + match reaction { + Upvote => self.up_vote, + Downvote => self.down_vote, + Laugh => self.laugh, + Hooray => self.hooray, + Confused => self.confused, + Heart => self.heart, + } + } +} + +#[derive(Debug, Deserialize)] +pub struct ReactionFromJson { + pub id: usize, +} + +#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] +pub enum Reaction { + Upvote, + Downvote, + Laugh, + Hooray, + Confused, + Heart, +} + +impl Reaction { + pub fn to_param(&self) -> &str { + use self::Reaction::*; + match *self { + Upvote => "+1", + Downvote => "-1", + Laugh => "laugh", + Hooray => "hooray", + Confused => "confused", + Heart => "heart", + } + } +} diff --git a/src/github/nag.rs b/src/github/nag.rs index 23b2e92f..72247e20 100644 --- a/src/github/nag.rs +++ b/src/github/nag.rs @@ -61,9 +61,8 @@ impl Issue { } fn close(&self) { - if let Err(why) = GH.close_issue(&self.repository, self.number) { - error!("Unable to close issue {:?}: {:?}", self, why); - } + ok_or!(GH.close_issue(&self.repository, self.number), why => + error!("Unable to close issue {:?}: {:?}", self, why)); } } @@ -97,35 +96,23 @@ pub fn update_nags(comment: &IssueComment) -> DashResult<()> { } debug!("processing rfcbot command: {:?}", &command); - match command.process(&author, &issue, comment, &subteam_members) { - Ok(_) => (), - Err(why) => { - error!("Unable to process command for comment id {}: {:?}", - comment.id, - why); - return Ok(()); - } - }; + let process = command.process(&author, &issue, comment, &subteam_members); + ok_or!(process, why => { + error!("Unable to process command for comment id {}: {:?}", + comment.id, why); + return Ok(()); + }); debug!("rfcbot command is processed"); } else { - match resolve_applicable_feedback_requests(&author, &issue, comment) { - Ok(_) => (), - Err(why) => { - error!("Unable to resolve feedback requests for comment id {}: {:?}", - comment.id, - why); - } - }; + ok_or!(resolve_applicable_feedback_requests(&author, &issue, comment), + why => error!("Unable to resolve feedback requests for comment id {}: {:?}", + comment.id, why)); } - match evaluate_nags() { - Ok(_) => (), - Err(why) => { - error!("Unable to evaluate outstanding proposals: {:?}", why); - } - }; + ok_or!(evaluate_nags(), why => + error!("Unable to evaluate outstanding proposals: {:?}", why)); Ok(()) } @@ -202,84 +189,45 @@ fn evaluate_nags() -> DashResult<()> { let conn = &*DB_POOL.get()?; // first process all "pending" proposals (unreviewed or remaining concerns) - let pending_proposals = match fcp_proposal - .filter(fcp_start.is_null()) - .load::(conn) { - Ok(p) => p, - Err(why) => { - error!("Unable to retrieve list of pending proposals: {:?}", why); - return Err(why.into()); - } - }; + let pending = fcp_proposal.filter(fcp_start.is_null()).load::(conn); + let pending_proposals = ok_or!(pending, why => { + error!("Unable to retrieve list of pending proposals: {:?}", why); + throw!(why) + }); for mut proposal in pending_proposals { - let initiator = match githubuser::table - .find(proposal.fk_initiator) - .first::(conn) { - Ok(i) => i, - Err(why) => { - error!("Unable to retrieve proposal initiator for proposal id {}: {:?}", - proposal.id, - why); - continue; - } - }; + let initiator = githubuser::table.find(proposal.fk_initiator) + .first::(conn); + let initiator = ok_or_continue!(initiator, why => + error!("Unable to retrieve proposal initiator for proposal id {}: {:?}", + proposal.id, why)); - let issue = match issue::table.find(proposal.fk_issue).first::(conn) { - Ok(i) => i, - Err(why) => { - error!("Unable to retrieve issue for proposal {}: {:?}", - proposal.id, - why); - continue; - } - }; + let issue = issue::table.find(proposal.fk_issue).first::(conn); + let issue = ok_or_continue!(issue, why => + error!("Unable to retrieve issue for proposal {}: {:?}", + proposal.id, why)); // if the issue has been closed before an FCP starts, // then we just need to cancel the FCP entirely if !issue.open { - match cancel_fcp(&initiator, &issue, &proposal) { - Ok(_) => (), - Err(why) => { - error!("Unable to cancel FCP for proposal {}: {:?}", - proposal.id, - why); - continue; - } - }; + ok_or_continue!(cancel_fcp(&initiator, &issue, &proposal), why => + error!("Unable to cancel FCP for proposal {}: {:?}", + proposal.id, why)); } // check to see if any checkboxes were modified before we end up replacing the comment - match update_proposal_review_status(proposal.id) { - Ok(_) => (), - Err(why) => { - error!("Unable to update review status for proposal {}: {:?}", - proposal.id, - why); - continue; - } - } + ok_or_continue!(update_proposal_review_status(proposal.id), why => + error!("Unable to update review status for proposal {}: {:?}", + proposal.id, why)); // get associated concerns and reviews - let reviews = match list_review_requests(proposal.id) { - Ok(r) => r, - Err(why) => { - error!("Unable to retrieve review requests for proposal {}: {:?}", - proposal.id, - why); - continue; - } - }; + let reviews = ok_or_continue!(list_review_requests(proposal.id), why => + error!("Unable to retrieve review requests for proposal {}: {:?}", + proposal.id, why)); - let concerns = match list_concerns_with_authors(proposal.id) { - Ok(c) => c, - Err(why) => { - error!("Unable to retrieve concerns for proposal {}: {:?}", - proposal.id, - why); - continue; - } - }; + let concerns = ok_or_continue!(list_concerns_with_authors(proposal.id), + why => error!("Unable to retrieve concerns for proposal {}: {:?}", + proposal.id, why)); let num_outstanding_reviews = reviews.iter().filter(|&&(_, ref r)| !r.reviewed).count(); let num_complete_reviews = reviews.len() - num_outstanding_reviews; @@ -303,15 +251,10 @@ fn evaluate_nags() -> DashResult<()> { // if the comment body in the database equals the new one we generated, then no change // is needed from github (this assumes our DB accurately reflects GH's, which should // be true in most cases by the time this is called) - match status_comment.post(Some(proposal.fk_bot_tracking_comment)) { - Ok(_) => (), - Err(why) => { - error!("Unable to update status comment for proposal {}: {:?}", - proposal.id, - why); - continue; - } - }; + let post = status_comment.post(Some(proposal.fk_bot_tracking_comment)); + ok_or_continue!(post, why => + error!("Unable to update status comment for proposal {}: {:?}", + proposal.id, why)); } let majority_complete = num_outstanding_reviews < num_complete_reviews; @@ -322,15 +265,11 @@ fn evaluate_nags() -> DashResult<()> { // FCP can start now -- update the database proposal.fcp_start = Some(Utc::now().naive_utc()); - match diesel::update(fcp_proposal.find(proposal.id)) - .set(&proposal) - .execute(conn) { - Ok(_) => (), - Err(why) => { - error!("Unable to mark FCP {} as started: {:?}", proposal.id, why); - continue; - } - } + let update = diesel::update(fcp_proposal.find(proposal.id)) + .set(&proposal).execute(conn); + ok_or_continue!(update, why => + error!("Unable to mark FCP {} as started: {:?}", + proposal.id, why)); // attempt to add the final-comment-period label // TODO only add label if FCP > 1 day @@ -357,69 +296,45 @@ fn evaluate_nags() -> DashResult<()> { // leave a comment for FCP start let fcp_start_comment = RfcBotComment::new(&issue, comment_type); - match fcp_start_comment.post(None) { - Ok(_) => (), - Err(why) => { - error!("Unable to post comment for FCP {}'s start: {:?}", - proposal.id, - why); - continue; - } - }; + ok_or_continue!(fcp_start_comment.post(None), why => + error!("Unable to post comment for FCP {}'s start: {:?}", + proposal.id, why)); } } } // look for any FCP proposals that entered FCP a week or more ago but aren't marked as closed let one_business_week_ago = Utc::now().naive_utc() - Duration::days(10); - let finished_fcps = match fcp_proposal - .filter(fcp_start.le(one_business_week_ago)) - .filter(fcp_closed.eq(false)) - .load::(conn) { - Ok(f) => f, - Err(why) => { - error!("Unable to retrieve FCPs that need to be marked as finished: {:?}", - why); - return Err(why.into()); - } - }; + let ffcps = fcp_proposal.filter(fcp_start.le(one_business_week_ago)) + .filter(fcp_closed.eq(false)) + .load::(conn); + let finished_fcps = ok_or!(ffcps, why => { + error!("Unable to retrieve FCPs that need to be marked as finished: {:?}", + why); + throw!(why); + }); for mut proposal in finished_fcps { - let initiator = match githubuser::table - .find(proposal.fk_initiator) - .first::(conn) { - Ok(i) => i, - Err(why) => { - error!("Unable to retrieve proposal initiator for proposal id {}: {:?}", - proposal.id, - why); - continue; - } - }; - - let issue = match issue::table.find(proposal.fk_issue).first::(conn) { - Ok(i) => i, - Err(why) => { - error!("Unable to find issue to match proposal {}: {:?}", - proposal.id, - why); - continue; - } - }; + let initiator = githubuser::table.find(proposal.fk_initiator) + .first::(conn); + let initiator = ok_or_continue!(initiator, why => + error!("Unable to retrieve proposal initiator for proposal id {}: {:?}", + proposal.id, + why)); + + let issue = issue::table.find(proposal.fk_issue).first::(conn); + let issue = ok_or_continue!(issue, why => + error!("Unable to find issue to match proposal {}: {:?}", + proposal.id, why)); // TODO only update the db if the comment posts, but reconcile if we find out it worked // update the fcp proposal.fcp_closed = true; - match diesel::update(fcp_proposal.find(proposal.id)) - .set(&proposal) - .execute(conn) { - Ok(_) => (), - Err(why) => { - error!("Unable to update FCP {}: {:?}", proposal.id, why); - continue; - } - } + let update_fcp = diesel::update(fcp_proposal.find(proposal.id)) + .set(&proposal).execute(conn); + ok_or_continue!(update_fcp, why => + error!("Unable to update FCP {}: {:?}", proposal.id, why)); // parse the disposition: let disp = FcpDisposition::from_str(&proposal.disposition)?; @@ -428,7 +343,7 @@ fn evaluate_nags() -> DashResult<()> { let label_res = issue.add_label(Label::FFCP); issue.remove_label(Label::FCP); let added_label = match label_res { - Ok(()) => true, + Ok(_) => true, Err(why) => { warn!("Unable to add Finished-FCP label to {}#{}: {:?}", &issue.repository, @@ -448,15 +363,9 @@ fn evaluate_nags() -> DashResult<()> { let fcp_close_comment = RfcBotComment::new(&issue, comment_type); // Post it! - match fcp_close_comment.post(None) { - Ok(_) => (), - Err(why) => { - error!("Unable to post FCP-ending comment for proposal {}: {:?}", - proposal.id, - why); - continue; - } - }; + ok_or_continue!(fcp_close_comment.post(None), why => + error!("Unable to post FCP-ending comment for proposal {}: {:?}", + proposal.id, why)); execute_ffcp_actions(&issue, disp); } @@ -643,12 +552,12 @@ impl FcpDisposition { } pub fn from_str(string: &str) -> DashResult { - match string { - FCP_REPR_MERGE => Ok(FcpDisposition::Merge), - FCP_REPR_CLOSE => Ok(FcpDisposition::Close), - FCP_REPR_POSTPONE => Ok(FcpDisposition::Postpone), - _ => Err(DashError::Misc(None)), - } + Ok(match string { + FCP_REPR_MERGE => FcpDisposition::Merge, + FCP_REPR_CLOSE => FcpDisposition::Close, + FCP_REPR_POSTPONE => FcpDisposition::Postpone, + _ => throw!(DashError::Misc(None)), + }) } pub fn label(self) -> Label { @@ -674,50 +583,50 @@ fn parse_fcp_subcommand<'a>( subcommand: &'a str, fcp_context: bool ) -> DashResult> { - match subcommand { + Ok(match subcommand { // Parse a FCP merge command: "merge" | "merged" | "merging" | "merges" => - Ok(RfcBotCommand::FcpPropose(FcpDisposition::Merge)), + RfcBotCommand::FcpPropose(FcpDisposition::Merge), // Parse a FCP close command: "close" | "closed" | "closing" | "closes" => - Ok(RfcBotCommand::FcpPropose(FcpDisposition::Close)), + RfcBotCommand::FcpPropose(FcpDisposition::Close), // Parse a FCP postpone command: "postpone" | "postponed" | "postponing" | "postpones" => - Ok(RfcBotCommand::FcpPropose(FcpDisposition::Postpone)), + RfcBotCommand::FcpPropose(FcpDisposition::Postpone), // Parse a FCP cancel command: "cancel" | "canceled" | "canceling" | "cancels" => - Ok(RfcBotCommand::FcpCancel), + RfcBotCommand::FcpCancel, // Parse a FCP reviewed command: "reviewed" | "review" | "reviewing" | "reviews" => - Ok(RfcBotCommand::Reviewed), + RfcBotCommand::Reviewed, // Parse a FCP concern command: "concern" | "concerned" | "concerning" | "concerns" => { debug!("Parsed command as NewConcern"); let what = parse_command_text(command, subcommand); - Ok(RfcBotCommand::NewConcern(what)) + RfcBotCommand::NewConcern(what) }, // Parse a FCP resolve command: "resolve" | "resolved" | "resolving" | "resolves" => { debug!("Parsed command as ResolveConcern"); let what = parse_command_text(command, subcommand); - Ok(RfcBotCommand::ResolveConcern(what)) + RfcBotCommand::ResolveConcern(what) }, _ => { - Err(if fcp_context { + throw!(DashError::Misc(if fcp_context { error!("unrecognized subcommand for fcp: {}", subcommand); - DashError::Misc(Some(format!("found bad subcommand: {}", subcommand))) + Some(format!("found bad subcommand: {}", subcommand)) } else { - DashError::Misc(None) - }) + None + })) } - } + }) } impl<'a> RfcBotCommand<'a> { @@ -760,14 +669,13 @@ impl<'a> RfcBotCommand<'a> { // at this point our new comment doesn't yet exist in the database, so // we need to insert it let gh_comment = gh_comment.with_repo(&issue.repository)?; - match diesel::insert(&gh_comment) + if let Err(why) = + diesel::insert(&gh_comment) .into(issuecomment::table) - .execute(conn) { - Ok(_) => {} - Err(why) => { - warn!("issue inserting new record, maybe received webhook for it: {:?}", - why); - } + .execute(conn) + { + warn!("issue inserting new record, maybe received webhook for it: {:?}", + why); } let proposal = NewFcpProposal { @@ -814,10 +722,8 @@ impl<'a> RfcBotCommand<'a> { let new_gh_comment = RfcBotComment::new(issue, - CommentType::FcpProposed(author, - disp, - &review_requests, - &[])); + CommentType::FcpProposed( + author, disp, &review_requests, &[])); new_gh_comment.post(Some(gh_comment.id))?; @@ -886,15 +792,14 @@ impl<'a> RfcBotCommand<'a> { if proposal.fcp_start.is_some() { // Update DB: FCP is not started anymore. proposal.fcp_start = None; - match diesel::update(fcp_proposal.find(proposal.id)) - .set(&proposal) - .execute(conn) { - Ok(_) => (), - Err(why) => { - error!("Unable to mark FCP {} as unstarted: {:?}", proposal.id, why); - return Ok(()); - } - } + let update = + diesel::update(fcp_proposal.find(proposal.id)) + .set(&proposal) + .execute(conn); + ok_or!(update, why => { + error!("Unable to mark FCP {} as unstarted: {:?}", proposal.id, why); + return Ok(()); + }); // Update labels: let _ = issue.add_label(Label::PFCP); @@ -1000,7 +905,7 @@ impl<'a> RfcBotCommand<'a> { .ok_or_else(|| DashError::Misc(Some("no user specified".to_string())))?; if user.is_empty() { - return Err(DashError::Misc(Some("no user specified".to_string()))); + throw!(DashError::Misc(Some("no user specified".to_string()))); } Ok(RfcBotCommand::FeedbackRequest(&user[1..])) @@ -1188,30 +1093,25 @@ impl<'a> RfcBotComment<'a> { use config::CONFIG; if CONFIG.post_comments { - if self.issue.open { - match existing_comment { - Some(comment_id) => { - self.maybe_add_pfcp_label(); - GH.edit_comment(&self.issue.repository, comment_id, &self.body) - } - None => { - GH.new_comment(&self.issue.repository, self.issue.number, &self.body) - } + if let Some(comment_id) = existing_comment { + self.maybe_add_pfcp_label(); + GH.edit_comment(&self.issue.repository, comment_id, &self.body) + } else { + GH.new_comment(&self.issue.repository, self.issue.number, &self.body) } } else { info!("Skipping comment to {}#{}, the issue is no longer open", self.issue.repository, self.issue.number); - Err(DashError::Misc(None)) + throw!(DashError::Misc(None)) } - } else { info!("Skipping comment to {}#{}, comment posts are disabled.", self.issue.repository, self.issue.number); - Err(DashError::Misc(None)) + throw!(DashError::Misc(None)) } } } diff --git a/src/github/webhooks.rs b/src/github/webhooks.rs index a7120e8e..13b1cf6d 100644 --- a/src/github/webhooks.rs +++ b/src/github/webhooks.rs @@ -46,13 +46,10 @@ impl FromData for Event { }; let mut body = String::new(); - match data.open().read_to_string(&mut body) { - Ok(_) => (), - Err(why) => { - error!("unable to read request body: {:?}", why); - return Failure((Status::InternalServerError, "unable to read request body")); - } - }; + if let Err(why) = data.open().read_to_string(&mut body) { + error!("unable to read request body: {:?}", why); + return Failure((Status::InternalServerError, "unable to read request body")); + } for secret in &CONFIG.github_webhook_secrets { if authenticate(secret, &body, signature) { @@ -96,14 +93,13 @@ impl FromData for Event { fn authenticate(secret: &str, payload: &str, signature: &str) -> bool { // https://developer.github.com/webhooks/securing/#validating-payloads-from-github let sans_prefix = signature[5..].as_bytes(); - match Vec::from_hex(sans_prefix) { - Ok(sigbytes) => { - let mut mac = Hmac::new(Sha1::new(), secret.as_bytes()); - mac.input(payload.as_bytes()); - // constant time comparison - mac.result() == MacResult::new(&sigbytes) - } - Err(_) => false, + if let Ok(sigbytes) = Vec::from_hex(sans_prefix) { + let mut mac = Hmac::new(Sha1::new(), secret.as_bytes()); + mac.input(payload.as_bytes()); + // constant time comparison + mac.result() == MacResult::new(&sigbytes) + } else { + false } } diff --git a/src/macros.rs b/src/macros.rs new file mode 100644 index 00000000..45b6c25b --- /dev/null +++ b/src/macros.rs @@ -0,0 +1,23 @@ +macro_rules! ok_or { + ($test: expr, $on_err: expr) => { + ok_or!($test, _e => $on_err); + }; + ($test: expr, $why: ident => $on_err: expr) => { + match $test { + Ok(ok) => ok, + Err($why) => $on_err, + }; + }; +} + +macro_rules! ok_or_continue { + ($test: expr, $why: ident => $on_err: expr) => { + ok_or!($test, $why => { $on_err; continue; }) + }; +} + +macro_rules! throw { + ($err: expr) => { + Err::($err)? + }; +} diff --git a/src/main.rs b/src/main.rs index 095b59a1..5bd8e55d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -30,6 +30,10 @@ extern crate serde_json; extern crate toml; extern crate url; extern crate urlencoded; +extern crate arrayvec; + +#[macro_use] +mod macros; mod config; mod domain; diff --git a/src/scraper.rs b/src/scraper.rs index 01be4b71..2bd6da6a 100644 --- a/src/scraper.rs +++ b/src/scraper.rs @@ -27,26 +27,29 @@ pub fn start_scraping() -> JoinHandle<()> { pub fn scrape_github(since: DateTime) { let mut repos = Vec::new(); for org in &GH_ORGS { - match github::GH.org_repos(org) { - Ok(r) => repos.extend(r), - Err(why) => { - error!("Unable to retrieve repos for {}: {:?}", org, why); - return; - } - } + repos.extend(ok_or!(github::GH.org_repos(org), why => { + error!("Unable to retrieve repos for {}: {:?}", org, why); + return; + })); } info!("Scraping github activity since {:?}", since); let start_time = Utc::now().naive_utc(); - for repo in repos { + for repo in repos.iter() { match github::ingest_since(&repo, since) { - Ok(()) => info!("Scraped {} github successfully", repo), + Ok(_) => info!("Scraped {} github successfully", repo), Err(why) => error!("Unable to scrape github {}: {:?}", repo, why), } } - match github::record_successful_update(start_time) { - Ok(_) => {} - Err(why) => error!("Problem recording successful update: {:?}", why), + info!("Nuking reactions at github since {:?}", since); + for repo in repos.iter() { + match github::nuke_reactions(&repo) { + Ok(_) => info!("Nuked reactions at {} successfully", repo), + Err(why) => error!("Unable to nuke reactions {}: {:?}", repo, why), + } } + + ok_or!(github::record_successful_update(start_time), why => + error!("Problem recording successful update: {:?}", why)); } diff --git a/src/server.rs b/src/server.rs index a55cb449..d4e01f83 100644 --- a/src/server.rs +++ b/src/server.rs @@ -19,9 +19,7 @@ pub fn serve() { .launch(); }); - if let Err(why) = result { - error!("Rocket failed to ignite: {:?}", why); - } + ok_or!(result, why => error!("Rocket failed to ignite: {:?}", why)); } } diff --git a/src/teams.rs b/src/teams.rs index 756795ab..13bfdcc7 100644 --- a/src/teams.rs +++ b/src/teams.rs @@ -4,10 +4,11 @@ use std::collections::BTreeMap; use diesel::prelude::*; use toml; -use serde::de; +use arrayvec::ArrayVec; use super::DB_POOL; -use domain::github::GitHubUser; +use domain::github::{GitHubUser}; +use github::models::Reaction; use error::*; //============================================================================== @@ -20,6 +21,7 @@ lazy_static! { #[derive(Debug, Deserialize)] pub struct RfcbotConfig { + prohibited_reactions: BTreeMap, fcp_behaviors: BTreeMap, teams: BTreeMap, } @@ -44,10 +46,66 @@ impl RfcbotConfig { pub fn should_ffcp_auto_postpone(&self, repo: &str) -> bool { self.fcp_behaviors.get(repo).map(|fcp| fcp.postpone).unwrap_or_default() } + + /// Returns all the prohibited reactions on issues for this repo. + pub fn prohibited_issue_reactions(&self, repo: &str) -> ReactionVec { + self.prohibited_reactions.get(repo) + .map(|rb| rb.issue.prohibited_reactions()) + .unwrap_or_default() + } + + /// Returns all the prohibited reactions on comments for this repo. + pub fn prohibited_comment_reactions(&self, repo: &str) -> ReactionVec { + self.prohibited_reactions.get(repo) + .map(|rb| rb.comment.prohibited_reactions()) + .unwrap_or_default() + } +} + +#[derive(Copy, Clone, Debug, Default, Deserialize)] +#[serde(default)] +struct ReactionBehaviorConfig { + issue: ProhibitedReactions, + comment: ProhibitedReactions, +} + +#[derive(Copy, Clone, Debug, Default, Deserialize)] +#[serde(default)] +struct ProhibitedReactions { + up_vote: bool, + down_vote: bool, + laugh: bool, + hooray: bool, + confused: bool, + heart: bool, +} + +type ReactionVec = ArrayVec<[Reaction; 6]>; + +impl ProhibitedReactions { + fn prohibited_reactions(&self) -> ReactionVec { + use self::Reaction::*; + [Upvote, Downvote, Laugh, Hooray, Confused, Heart].iter() + .filter(move |&&reaction| self.is_reaction_prohibited(reaction)) + .cloned() + .collect() + } + + fn is_reaction_prohibited(&self, reaction: Reaction) -> bool { + use self::Reaction::*; + match reaction { + Upvote => self.up_vote, + Downvote => self.down_vote, + Laugh => self.laugh, + Hooray => self.hooray, + Confused => self.confused, + Heart => self.heart, + } + } } #[derive(Debug, Deserialize)] -pub struct FcpBehavior { +struct FcpBehavior { #[serde(default)] close: bool, #[serde(default)] @@ -72,21 +130,10 @@ impl Team { } } -#[derive(Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] +#[derive(Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Deserialize)] +#[serde(transparent)] pub struct TeamLabel(pub String); -impl<'de> de::Deserialize<'de> for TeamLabel { - fn deserialize>(de: D) -> Result { - String::deserialize(de).map(TeamLabel) - } - - fn deserialize_in_place>(de: D, place: &mut Self) - -> Result<(), D::Error> - { - String::deserialize_in_place(de, &mut place.0) - } -} - //============================================================================== // Implementation details //============================================================================== @@ -121,16 +168,12 @@ impl Team { // 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::(conn) - { - Ok(_) => (), - Err(why) => { - error!("unable to find {} in database: {:?}", member_login, why); - return Err(why.into()); - } - } + let check_login = githubuser.filter(login.eq(member_login)) + .first::(conn); + ok_or!(check_login, why => { + error!("unable to find {} in database: {:?}", member_login, why); + throw!(why); + }); } Ok(()) @@ -148,6 +191,16 @@ mod test { #[test] fn setup_parser_correct() { let test = r#" +[prohibited_reactions] + +[prohibited_reactions."foo-org/bar".issue] +down_vote = true +confused = true + +[prohibited_reactions."foo-org/bar".comment] +down_vote = true +confused = false + [fcp_behaviors] [fcp_behaviors."rust-lang/alpha"] @@ -224,6 +277,16 @@ members = [ assert!(!cfg.should_ffcp_auto_postpone("wibble/epsilon")); assert!(!cfg.should_ffcp_auto_close("random")); assert!(!cfg.should_ffcp_auto_postpone("random")); + + // Reaction behavior correct: + let foo_issue_nono = cfg.prohibited_issue_reactions("foo-org/bar") + .into_iter().collect::>(); + assert_eq!(foo_issue_nono, &[Reaction::Downvote, Reaction::Confused]); + let foo_comment_nono = cfg.prohibited_comment_reactions("foo-org/bar") + .into_iter().collect::>(); + assert_eq!(foo_comment_nono, &[Reaction::Downvote]); + assert!(cfg.prohibited_issue_reactions("random").is_empty()); + assert!(cfg.prohibited_comment_reactions("random").is_empty()); } #[test]