From cef74cbc8ccfac2b371db2cc9e99dda3c1fce166 Mon Sep 17 00:00:00 2001
From: Teddy Wing
Date: Fri, 17 Nov 2017 00:47:26 +0100
Subject: pull_request.rs: Add `pull_request_opened_or_synchronized()`
A new function that, given the parsed JSON from a pull request webhook,
tells us whether the pull request was "opened" or "synchronize"d with a
boolean.
This will enable us to only update commit statuses when actions on the
pull request result in new commits being available. Otherwise, we don't
want to set a commit status that we've already set before.
---
src/pull_request.rs | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
(limited to 'src')
diff --git a/src/pull_request.rs b/src/pull_request.rs
index fb06544..c4081f4 100644
--- a/src/pull_request.rs
+++ b/src/pull_request.rs
@@ -42,6 +42,18 @@ impl CommitRef {
}
}
+pub fn pull_request_opened_or_synchronized(
+ mut github_push_event: json::JsonValue
+) -> bool {
+ let action = github_push_event["action"].take_string().unwrap_or_default();
+
+ if action == "opened" || action == "synchronize" {
+ return true
+ }
+
+ false
+}
+
#[cfg(test)]
mod tests {
@@ -473,4 +485,52 @@ mod tests {
assert_eq!(commit_ref.sha, "0d1a26e67d8f5eaf1f6ba5c57fc3c7d91ac0fd1c");
assert_eq!(commit_ref.branch, "changes");
}
+
+ #[test]
+ fn pull_request_opened_or_synchronized_returns_true_when_opened() {
+ let payload = r#"{
+ "action": "opened"
+ }"#;
+
+ let json = json::parse(payload)
+ .expect("Failed to parse payload.");
+
+ assert_eq!(
+ pull_request_opened_or_synchronized(json),
+ true
+ );
+ }
+
+ #[test]
+ fn pull_request_opened_or_synchronized_returns_true_when_synchronized() {
+ let payload = r#"{
+ "action": "synchronize"
+ }"#;
+
+ let json = json::parse(payload)
+ .expect("Failed to parse payload.");
+
+ assert_eq!(
+ pull_request_opened_or_synchronized(json),
+ true
+ );
+ }
+
+ #[test]
+ fn pull_request_opened_or_synchronized_returns_false_when_not_opened_or_synchronized() {
+ // "assigned", "unassigned", "review_requested",
+ // "review_request_removed", "labeled", "unlabeled", "opened",
+ // "edited", "closed", or "reopened"
+ let payload = r#"{
+ "action": "review_requested"
+ }"#;
+
+ let json = json::parse(payload)
+ .expect("Failed to parse payload.");
+
+ assert_eq!(
+ pull_request_opened_or_synchronized(json),
+ false
+ );
+ }
}
--
cgit v1.2.3
From 205fc3b71bd44067685c8517e7d126e3574827e1 Mon Sep 17 00:00:00 2001
From: Teddy Wing
Date: Fri, 17 Nov 2017 00:54:27 +0100
Subject: CommitRef::new(): Take a `JsonValue` instead of an `&str`
We expect to have pre-parsed the JSON as this function will get called
after `pull_request_opened_or_synchronized()`. To avoid doing the same
JSON parsing work twice, take a `JsonValue` argument in both functions.
---
src/main.rs | 12 +++++++++++-
src/pull_request.rs | 11 +++++++----
2 files changed, 18 insertions(+), 5 deletions(-)
(limited to 'src')
diff --git a/src/main.rs b/src/main.rs
index 9ac9570..3bb2dbb 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -16,6 +16,7 @@
// along with Kipper. If not, see .
extern crate getopts;
+extern crate json;
#[macro_use]
extern crate log;
extern crate stderrlog;
@@ -129,7 +130,16 @@ fn main() {
let mut body = String::new();
try_or_400!(data.read_to_string(&mut body));
- let commit_ref = match CommitRef::new(body.as_ref()) {
+ let json = match json::parse(body.as_ref()) {
+ Ok(j) => j,
+ Err(e) => {
+ error!("{}", e.to_string());
+
+ return internal_server_error()
+ },
+ };
+
+ let commit_ref = match CommitRef::new(json) {
Ok(cr) => cr,
Err(e) => {
error!("{}", e.to_string());
diff --git a/src/pull_request.rs b/src/pull_request.rs
index c4081f4..b110ea4 100644
--- a/src/pull_request.rs
+++ b/src/pull_request.rs
@@ -28,9 +28,9 @@ pub struct CommitRef {
}
impl CommitRef {
- pub fn new(json_str: &str) -> Result> {
- let mut github_push_event = json::parse(json_str)?;
-
+ pub fn new(
+ mut github_push_event: json::JsonValue
+ ) -> Result> {
Ok(
CommitRef {
owner: github_push_event["pull_request"]["head"]["repo"]["owner"]["login"].take_string().unwrap_or_default(),
@@ -477,7 +477,10 @@ mod tests {
}
}"#;
- let commit_ref = CommitRef::new(payload)
+ let json = json::parse(payload)
+ .expect("Failed to parse payload.");
+
+ let commit_ref = CommitRef::new(json)
.expect("Failed to create CommitRef from payload");
assert_eq!(commit_ref.owner, "baxterthehacker");
--
cgit v1.2.3
From 399e04a697e42c9a684f14349f6d94208e719832 Mon Sep 17 00:00:00 2001
From: Teddy Wing
Date: Fri, 17 Nov 2017 01:08:55 +0100
Subject: main(): Don't update commit status unless commits were pushed
There are a bunch of events that can trigger the `pull_request` webhook.
These are:
"assigned", "unassigned", "review_requested",
"review_request_removed", "labeled", "unlabeled", "opened",
"edited", "closed", or "reopened". And "synchronize".
We only care about "opened" and "synchronize" because those are the only
two where new commits can come through. Since we don't set commit
statuses unless that commit is part of a pull request, we need a way to
update the status the first time a pull request is opened, thus the
"opened" action. When new commits are added we also want to update
statuses, so "synchronize". But for the others, we don't want to be
duplicating work and adding duplicate unnecessary statuses to PR
commits.
---
src/main.rs | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
(limited to 'src')
diff --git a/src/main.rs b/src/main.rs
index 3bb2dbb..12d71eb 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -33,7 +33,7 @@ use std::time::Duration;
use getopts::Options;
use kipper::jenkins;
-use kipper::pull_request::CommitRef;
+use kipper::pull_request::{CommitRef, pull_request_opened_or_synchronized};
const DEFAULT_PORT: u16 = 8000;
@@ -139,6 +139,11 @@ fn main() {
},
};
+ if !pull_request_opened_or_synchronized(json.clone()) {
+ return rouille::Response::text("No status update needed.")
+ .with_status_code(200)
+ }
+
let commit_ref = match CommitRef::new(json) {
Ok(cr) => cr,
Err(e) => {
--
cgit v1.2.3