diff --git a/phabricator-plugin.iml b/phabricator-plugin.iml
index ba37b2f1..6f7ce38a 100644
--- a/phabricator-plugin.iml
+++ b/phabricator-plugin.iml
@@ -7,6 +7,7 @@
+
@@ -121,6 +122,10 @@
+
+
+
+
@@ -259,7 +264,6 @@
-
diff --git a/pom.xml b/pom.xml
index 455dc026..0cef59f3 100644
--- a/pom.xml
+++ b/pom.xml
@@ -44,6 +44,7 @@
+
org.apache.httpcomponents
httpclient
@@ -68,6 +69,14 @@
jna
3.2.2
+
+
+
+ test
+ org.mockito
+ mockito-core
+ 2.0.23-beta
+
@@ -83,7 +92,10 @@
cobertura-maven-plugin
2.6
- xml
+
+ xml
+ html
+
256m
true
diff --git a/src/main/java/com/uber/jenkins/phabricator/PhabricatorBuildWrapper.java b/src/main/java/com/uber/jenkins/phabricator/PhabricatorBuildWrapper.java
index 73c6c984..ebc6d6a9 100644
--- a/src/main/java/com/uber/jenkins/phabricator/PhabricatorBuildWrapper.java
+++ b/src/main/java/com/uber/jenkins/phabricator/PhabricatorBuildWrapper.java
@@ -24,6 +24,7 @@
import com.uber.jenkins.phabricator.conduit.ArcanistUsageException;
import com.uber.jenkins.phabricator.conduit.Differential;
import com.uber.jenkins.phabricator.utils.CommonUtils;
+import com.uber.jenkins.phabricator.conduit.DifferentialClient;
import hudson.EnvVars;
import hudson.Launcher;
import hudson.model.AbstractBuild;
@@ -89,16 +90,17 @@ public Environment setUp(AbstractBuild build,
}
}
+ DifferentialClient diffClient = new DifferentialClient(diffID, starter, conduitToken, arcPath);
Differential diff;
try {
- diff = new Differential(diffID, starter, conduitToken, arcPath);
+ diff = new Differential(diffClient.fetchDiff());
diff.decorate(build, this.getPhabricatorURL());
logger.println("Applying patch for differential");
// Post a silent notification if option is enabled
if (showBuildStartedMessage) {
- diff.postComment(diff.getBuildStartedMessage(environment));
+ diffClient.postComment(diff.getBuildStartedMessage(environment));
}
} catch (ArcanistUsageException e) {
logger.println("[arcanist] unable to apply patch");
diff --git a/src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java b/src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java
index dd81577f..ca3c7d93 100644
--- a/src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java
+++ b/src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java
@@ -23,6 +23,7 @@
import com.uber.jenkins.phabricator.conduit.ArcanistUsageException;
import com.uber.jenkins.phabricator.conduit.Differential;
+import com.uber.jenkins.phabricator.conduit.DifferentialClient;
import com.uber.jenkins.phabricator.uberalls.UberallsClient;
import com.uber.jenkins.phabricator.utils.CommonUtils;
import com.uber.jenkins.phabricator.utils.Logger;
@@ -118,15 +119,16 @@ public final boolean perform(final AbstractBuild, ?> build, final Launcher lau
LauncherFactory starter = new LauncherFactory(launcher, environment, listener.getLogger(), build.getWorkspace());
+ DifferentialClient diffClient = new DifferentialClient(diffID, starter, conduitToken, arcPath);
Differential diff;
try {
- diff = new Differential(diffID, starter, conduitToken, arcPath);
+ diff = new Differential(diffClient.fetchDiff());
} catch (ArcanistUsageException e) {
logger.info("arcanist", "unable to fetch differential");
return true;
}
- String revisionID = diff.getRevisionID();
+ String revisionID = diff.getRevisionID(true);
if (CommonUtils.isBlank(revisionID)) {
return this.ignoreBuild(logger.getStream(), "Unable to load revisionID from conduit for diff ID " + diffID);
}
@@ -173,9 +175,9 @@ public final boolean perform(final AbstractBuild, ?> build, final Launcher lau
if (runHarbormaster) {
logger.info("uberalls", "Sending build result to Harbormaster with PHID '" + phid + "', success: " + harbormasterSuccess);
try {
- diff.harbormaster(phid, harbormasterSuccess);
+ diffClient.sendHarbormasterMessage(phid, harbormasterSuccess);
} catch (ArcanistUsageException e) {
- logger.info("arcanist", "unable to post to harbormaster");
+ logger.info("arcanist", "unable to post to sendHarbormasterMessage");
return true;
}
} else {
@@ -215,7 +217,7 @@ public final boolean perform(final AbstractBuild, ?> build, final Launcher lau
JSONObject result = null;
try {
- result = diff.postComment(comment, silent, commentAction);
+ result = diffClient.postComment(comment, silent, commentAction);
} catch (ArcanistUsageException e) {
logger.info("arcanist", "unable to post comment");
}
@@ -223,7 +225,7 @@ public final boolean perform(final AbstractBuild, ?> build, final Launcher lau
logger.info("arcanist", "Get error " + result.get("errorMessage") + " with action " +
commentAction + "; trying again with action 'none'");
try {
- diff.postComment(comment, silent, "none");
+ diffClient.postComment(comment, silent, "none");
} catch (ArcanistUsageException e) {
logger.info("arcanist", "unable to post comment");
}
diff --git a/src/main/java/com/uber/jenkins/phabricator/conduit/Differential.java b/src/main/java/com/uber/jenkins/phabricator/conduit/Differential.java
index 00873923..021dd0b5 100644
--- a/src/main/java/com/uber/jenkins/phabricator/conduit/Differential.java
+++ b/src/main/java/com/uber/jenkins/phabricator/conduit/Differential.java
@@ -20,57 +20,23 @@
package com.uber.jenkins.phabricator.conduit;
-import com.uber.jenkins.phabricator.LauncherFactory;
import com.uber.jenkins.phabricator.PhabricatorPostbuildAction;
import com.uber.jenkins.phabricator.PhabricatorPostbuildSummaryAction;
import hudson.EnvVars;
import hudson.model.AbstractBuild;
-import hudson.model.Result;
-import net.sf.json.JSONException;
import net.sf.json.JSONNull;
import net.sf.json.JSONObject;
import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
-import java.util.HashMap;
-import java.util.Map;
public class Differential {
- private final JSONObject rawJSON;
- private final LauncherFactory launcher;
- private final String conduitToken;
- private final String arcPath;
+ private JSONObject rawJSON;
- public Differential(String diffID, LauncherFactory launcher, String conduitToken, String arcPath) throws IOException, InterruptedException, ArcanistUsageException {
- this.conduitToken = conduitToken;
- this.arcPath = arcPath;
- this.launcher = launcher;
- Map params = new HashMap();
- params.put("ids", new String[]{diffID});
- JSONObject query = this.callConduit("differential.querydiffs", params);
- JSONObject response;
- try {
- response = query.getJSONObject("response");
- } catch (JSONException e) {
- e.printStackTrace();
- throw new ArcanistUsageException(
- String.format("No 'response' object found in conduit call: (%s) %s",
- e.getMessage(),
- query.toString(2)));
- }
- try {
- this.rawJSON = response.getJSONObject(diffID);
- } catch (JSONException e) {
- e.printStackTrace();
- throw new ArcanistUsageException(
- String.format("Unable to find '%s' key in response: (%s) %s",
- diffID,
- e.getMessage(),
- response.toString(2)));
-
- }
+ public Differential(JSONObject rawJSON) throws ArcanistUsageException, IOException, InterruptedException {
+ this.rawJSON = rawJSON;
}
public String getRevisionID(boolean formatted) {
@@ -84,50 +50,6 @@ public String getRevisionID(boolean formatted) {
return rawRevisionId;
}
- public String getRevisionID() {
- return this.getRevisionID(true);
- }
-
- /**
- * Sets a harbormaster build status
- * @param phid Phabricator object ID
- * @param pass whether or not the build passed
- * @throws IOException
- * @throws InterruptedException
- */
- public void harbormaster(String phid, boolean pass) throws IOException, InterruptedException, ArcanistUsageException {
- Map params = new HashMap();
- params.put("type", pass ? "pass" : "fail");
- params.put("buildTargetPHID", phid);
-
- this.callConduit("harbormaster.sendmessage", params);
- }
-
- private JSONObject callConduit(String methodName, Map params) throws IOException, InterruptedException, ArcanistUsageException {
- ArcanistClient arc = new ArcanistClient(this.arcPath, "call-conduit", params, this.conduitToken, methodName);
- return arc.parseConduit(this.launcher.launch(), this.launcher.getStderr());
- }
-
- /**
- * Posts a comment to a differential
- * @param message
- * @param silent whether or not to trigger an email
- * @param action phabricator comment action, e.g. 'resign', 'reject', 'none'
- */
- public JSONObject postComment(String message, boolean silent, String action) throws IOException, InterruptedException, ArcanistUsageException {
- Map params = new HashMap();
- params.put("revision_id", this.getRevisionID(false));
- params.put("action", action);
- params.put("message", message);
- params.put("silent", silent);
-
- return this.callConduit("differential.createcomment", params);
- }
-
- public JSONObject postComment(String message) throws IOException, InterruptedException, ArcanistUsageException {
- return postComment(message, true, "none");
- }
-
/**
* Get the summary message
* @param phabricatorURL
@@ -146,7 +68,6 @@ public String getPhabricatorLink(String phabricatorURL) {
URL base = new URL(phabricatorURL);
return new URL(base, revisionID).toString();
} catch (MalformedURLException e) {
- e.printStackTrace();
return String.format("%s%s", phabricatorURL, revisionID);
}
}
diff --git a/src/main/java/com/uber/jenkins/phabricator/conduit/DifferentialClient.java b/src/main/java/com/uber/jenkins/phabricator/conduit/DifferentialClient.java
new file mode 100644
index 00000000..270cfaee
--- /dev/null
+++ b/src/main/java/com/uber/jenkins/phabricator/conduit/DifferentialClient.java
@@ -0,0 +1,121 @@
+// Copyright (c) 2015 Uber Technologies, Inc.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+package com.uber.jenkins.phabricator.conduit;
+
+import com.uber.jenkins.phabricator.LauncherFactory;
+import net.sf.json.JSONException;
+import net.sf.json.JSONObject;
+
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * DifferentialClient handles all interaction with conduit/arc for differentials
+ */
+public class DifferentialClient {
+ private final String arcPath;
+ private final String conduitToken;
+ private final LauncherFactory launcher;
+ private final String diffID;
+
+ public DifferentialClient(String diffID, LauncherFactory launcher, String conduitToken, String arcPath) {
+ this.diffID = diffID;
+ this.launcher = launcher;
+ this.conduitToken = conduitToken;
+ this.arcPath = arcPath;
+ }
+
+ /**
+ * Posts a comment to a differential
+ * @param message
+ * @param silent whether or not to trigger an email
+ * @param action phabricator comment action, e.g. 'resign', 'reject', 'none'
+ */
+ public JSONObject postComment(String message, boolean silent, String action) throws IOException, InterruptedException, ArcanistUsageException {
+ Map params = new HashMap();
+ params.put("revision_id", this.diffID);
+ params.put("action", action);
+ params.put("message", message);
+ params.put("silent", silent);
+
+ return this.callConduit("differential.createcomment", params);
+ }
+
+ public JSONObject fetchDiff() throws ArcanistUsageException, IOException, InterruptedException {
+ Map params = new HashMap();
+ params.put("ids", new String[]{this.diffID});
+ JSONObject query = this.callConduit("differential.querydiffs", params);
+ JSONObject response;
+ try {
+ response = query.getJSONObject("response");
+ } catch (JSONException e) {
+ throw new ArcanistUsageException(
+ String.format("No 'response' object found in conduit call: (%s) %s",
+ e.getMessage(),
+ query.toString(2)));
+ }
+ try {
+ return response.getJSONObject(diffID);
+ } catch (JSONException e) {
+ throw new ArcanistUsageException(
+ String.format("Unable to find '%s' key in response: (%s) %s",
+ diffID,
+ e.getMessage(),
+ response.toString(2)));
+
+ }
+ }
+
+ /**
+ * Sets a sendHarbormasterMessage build status
+ * @param phid Phabricator object ID
+ * @param pass whether or not the build passed
+ * @throws IOException
+ * @throws InterruptedException
+ */
+ public JSONObject sendHarbormasterMessage(String phid, boolean pass) throws IOException, InterruptedException, ArcanistUsageException {
+ Map params = new HashMap();
+ params.put("type", pass ? "pass" : "fail");
+ params.put("buildTargetPHID", phid);
+
+ return this.callConduit("sendHarbormasterMessage.sendmessage", params);
+ }
+
+
+ /**
+ * Post a comment on the differential
+ * @param message the string message to post
+ * @return
+ * @throws IOException
+ * @throws InterruptedException
+ * @throws ArcanistUsageException
+ */
+ public JSONObject postComment(String message) throws IOException, InterruptedException, ArcanistUsageException {
+ return postComment(message, true, "none");
+ }
+
+ protected JSONObject callConduit(String methodName, Map params) throws IOException, InterruptedException, ArcanistUsageException {
+ ArcanistClient arc = new ArcanistClient(this.arcPath, "call-conduit", params, this.conduitToken, methodName);
+ return arc.parseConduit(this.launcher.launch(), this.launcher.getStderr());
+ }
+
+}
diff --git a/src/test/java/com/uber/jenkins/phabricator/conduit/DifferentialTest.java b/src/test/java/com/uber/jenkins/phabricator/conduit/DifferentialTest.java
new file mode 100644
index 00000000..612e0e18
--- /dev/null
+++ b/src/test/java/com/uber/jenkins/phabricator/conduit/DifferentialTest.java
@@ -0,0 +1,68 @@
+// Copyright (c) 2015 Uber Technologies, Inc.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+package com.uber.jenkins.phabricator.conduit;
+
+import hudson.EnvVars;
+import junit.framework.TestCase;
+import net.sf.json.JSONObject;
+import net.sf.json.groovy.JsonSlurper;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.io.InputStream;
+
+public class DifferentialTest extends TestCase {
+ private static final String FAKE_DIFF_ID = "not-a-real-id";
+
+ Differential differential;
+
+ protected void setUp() throws IOException, ArcanistUsageException, InterruptedException {
+ differential = new Differential(getValidQueryResponse());
+ }
+
+ @Test
+ public void testFetchRevisionID() throws Exception {
+ assertEquals(FAKE_DIFF_ID, differential.getRevisionID(false));
+ }
+
+ @Test
+ public void testGetPhabricatorLink() throws Exception {
+ assertTrue(differential.getPhabricatorLink("http://example.com").contains(FAKE_DIFF_ID));
+ }
+
+ @Test
+ public void testGetPhabricatorLinkInvalidURL() throws Exception {
+ // Try our best to join URLs, even when they are wrong
+ assertTrue(differential.getPhabricatorLink("aoeu").contains("aoeu"));
+ }
+
+ @Test
+ public void testGetBuildStartedMessage() throws Exception {
+ assertTrue(differential.getBuildStartedMessage(new EnvVars()).contains("Build started"));
+ }
+
+ private JSONObject getValidQueryResponse() throws IOException {
+ InputStream input = getClass().getResourceAsStream(
+ "validDifferentialQueryResponse.json"
+ );
+ return (JSONObject) new JsonSlurper().parse(input);
+ }
+}
diff --git a/src/test/resources/com/uber/jenkins/phabricator/conduit/validDifferentialQueryResponse.json b/src/test/resources/com/uber/jenkins/phabricator/conduit/validDifferentialQueryResponse.json
new file mode 100644
index 00000000..55cb8da0
--- /dev/null
+++ b/src/test/resources/com/uber/jenkins/phabricator/conduit/validDifferentialQueryResponse.json
@@ -0,0 +1,3 @@
+{
+ "revisionID": "not-a-real-id"
+}
\ No newline at end of file