diff --git a/phabricator-plugin.iml b/phabricator-plugin.iml
index 373c7a10..1bc46cdc 100644
--- a/phabricator-plugin.iml
+++ b/phabricator-plugin.iml
@@ -63,10 +63,10 @@
-
-
-
-
+
+
+
+
diff --git a/pom.xml b/pom.xml
index 71ac2c0d..698cbffd 100644
--- a/pom.xml
+++ b/pom.xml
@@ -48,7 +48,7 @@
org.apache.httpcomponents
httpclient
- 4.3
+ 4.4.1
diff --git a/src/main/java/com/uber/jenkins/phabricator/CommentBuilder.java b/src/main/java/com/uber/jenkins/phabricator/CommentBuilder.java
index ddccfb74..f26fe6e0 100644
--- a/src/main/java/com/uber/jenkins/phabricator/CommentBuilder.java
+++ b/src/main/java/com/uber/jenkins/phabricator/CommentBuilder.java
@@ -27,7 +27,7 @@
import hudson.plugins.cobertura.targets.CoverageMetric;
import hudson.plugins.cobertura.targets.CoverageResult;
-public class CommentBuilder {
+class CommentBuilder {
private static final String UBERALLS_TAG = "uberalls";
private final Logger logger;
private final CoverageResult currentCoverage;
diff --git a/src/main/java/com/uber/jenkins/phabricator/ConduitCredentialsDescriptor.java b/src/main/java/com/uber/jenkins/phabricator/ConduitCredentialsDescriptor.java
index ec1ca05d..5468df68 100644
--- a/src/main/java/com/uber/jenkins/phabricator/ConduitCredentialsDescriptor.java
+++ b/src/main/java/com/uber/jenkins/phabricator/ConduitCredentialsDescriptor.java
@@ -37,7 +37,7 @@
import java.util.List;
public class ConduitCredentialsDescriptor {
- public static List availableCredentials(Job owner) {
+ private static List availableCredentials(Job owner) {
return CredentialsProvider.lookupCredentials(
ConduitCredentials.class,
owner,
diff --git a/src/main/java/com/uber/jenkins/phabricator/PhabricatorBuildWrapper.java b/src/main/java/com/uber/jenkins/phabricator/PhabricatorBuildWrapper.java
index 8091db70..9cb7683d 100644
--- a/src/main/java/com/uber/jenkins/phabricator/PhabricatorBuildWrapper.java
+++ b/src/main/java/com/uber/jenkins/phabricator/PhabricatorBuildWrapper.java
@@ -39,8 +39,6 @@
import java.io.IOException;
import java.util.Arrays;
-import java.util.HashMap;
-import java.util.Map;
public class PhabricatorBuildWrapper extends BuildWrapper {
private static final String CONDUIT_TAG = "conduit";
@@ -70,8 +68,6 @@ public Environment setUp(AbstractBuild build,
return this.ignoreBuild(logger, "No environment variables found?!");
}
- final Map envAdditions = new HashMap();
-
String diffID = environment.get(PhabricatorPlugin.DIFFERENTIAL_ID_FIELD);
if (CommonUtils.isBlank(diffID)) {
this.addShortText(build);
@@ -114,6 +110,7 @@ public Environment setUp(AbstractBuild build,
diffClient.postComment(diff.getRevisionID(false), diff.getBuildStartedMessage(environment));
}
} catch (ConduitAPIException e) {
+ e.printStackTrace(logger.getStream());
logger.warn(CONDUIT_TAG, "Unable to apply patch");
logger.warn(CONDUIT_TAG, e.getMessage());
return null;
@@ -135,16 +132,7 @@ logger, starter, baseCommit, diffID, conduitToken, getArcPath(),
return null;
}
- return new Environment() {
- @Override
- public void buildEnvVars(Map env) {
- // A little roundabout, but allows us to do overrides per
- // how EnvVars#override works (PATH+unique=/foo/bar)
- EnvVars envVars = new EnvVars(env);
- envVars.putAll(envAdditions);
- env.putAll(envVars);
- }
- };
+ return new Environment(){};
}
private void addShortText(final AbstractBuild build) {
@@ -191,7 +179,7 @@ public boolean isShowBuildStartedMessage() {
return showBuildStartedMessage;
}
- public String getPhabricatorURL(Job owner) {
+ private String getPhabricatorURL(Job owner) {
ConduitCredentials credentials = getConduitCredentials(owner);
if (credentials != null) {
return credentials.getUrl();
@@ -199,7 +187,7 @@ public String getPhabricatorURL(Job owner) {
return this.getDescriptor().getConduitURL();
}
- public String getConduitToken(Job owner, Logger logger) {
+ private String getConduitToken(Job owner, Logger logger) {
ConduitCredentials credentials = getConduitCredentials(owner);
if (credentials != null) {
return credentials.getToken().getPlainText();
@@ -212,7 +200,7 @@ public String getConduitToken(Job owner, Logger logger) {
* Return the path to the arcanist executable
* @return a string, fully-qualified or not, could just be "arc"
*/
- public String getArcPath() {
+ private String getArcPath() {
final String providedPath = this.getDescriptor().getArcPath();
if (CommonUtils.isBlank(providedPath)) {
return "arc";
diff --git a/src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java b/src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java
index 96c7a5c8..81d1ca29 100644
--- a/src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java
+++ b/src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java
@@ -106,7 +106,7 @@ public final boolean perform(final AbstractBuild, ?> build, final Launcher lau
ConduitAPIClient conduitClient;
try {
- conduitClient = getConduitClient(build.getParent(), logger);
+ conduitClient = getConduitClient(build.getParent());
} catch (ConduitAPIException e) {
e.printStackTrace(logger.getStream());
logger.warn(CONDUIT_TAG, e.getMessage());
@@ -118,7 +118,7 @@ public final boolean perform(final AbstractBuild, ?> build, final Launcher lau
try {
diff = new Differential(diffClient.fetchDiff());
} catch (ConduitAPIException e) {
- logger.info(CONDUIT_TAG, "unable to fetch differential");
+ logger.info(CONDUIT_TAG, "Unable to fetch differential");
return true;
}
@@ -200,7 +200,7 @@ public final boolean perform(final AbstractBuild, ?> build, final Launcher lau
return true;
}
- private ConduitAPIClient getConduitClient(Job owner, Logger logger) throws ConduitAPIException {
+ private ConduitAPIClient getConduitClient(Job owner) throws ConduitAPIException {
ConduitCredentials credentials = getConduitCredentials(owner);
if (credentials == null) {
throw new ConduitAPIException("No credentials configured for conduit");
@@ -255,16 +255,7 @@ private ConduitCredentials getConduitCredentials(Job owner) {
return getDescriptor().getCredentials(owner);
}
- public String getConduitToken(Job owner, Logger logger) {
- ConduitCredentials credentials = getConduitCredentials(owner);
- if (credentials != null) {
- return credentials.getToken().getPlainText();
- }
- logger.warn("credentials", "No credentials configured.");
- return null;
- }
-
- public String getPhabricatorURL(Job owner) {
+ private String getPhabricatorURL(Job owner) {
ConduitCredentials credentials = getDescriptor().getCredentials(owner);
if (credentials != null) {
return credentials.getUrl();
diff --git a/src/main/java/com/uber/jenkins/phabricator/conduit/ArcanistClient.java b/src/main/java/com/uber/jenkins/phabricator/conduit/ArcanistClient.java
index 2988f6d4..e9282ce7 100644
--- a/src/main/java/com/uber/jenkins/phabricator/conduit/ArcanistClient.java
+++ b/src/main/java/com/uber/jenkins/phabricator/conduit/ArcanistClient.java
@@ -25,25 +25,20 @@
import hudson.util.ArgumentListBuilder;
import net.sf.json.JSONObject;
import net.sf.json.groovy.JsonSlurper;
-import org.apache.commons.io.IOUtils;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
-import java.io.InputStream;
import java.io.PrintStream;
-import java.util.Map;
public class ArcanistClient {
private final String arcPath;
private final String methodName;
- private final Map params;
private final String conduitToken;
private final String[] arguments;
- public ArcanistClient(String arcPath, String methodName, Map params, String conduitToken, String... arguments) {
+ public ArcanistClient(String arcPath, String methodName, String conduitToken, String... arguments) {
this.arcPath = arcPath;
this.methodName = methodName;
- this.params = params;
this.conduitToken = conduitToken;
this.arguments = arguments;
}
@@ -59,16 +54,7 @@ private ArgumentListBuilder getConduitCommand() {
}
private Launcher.ProcStarter getCommand(Launcher.ProcStarter starter) throws IOException {
- Launcher.ProcStarter command = starter.cmds(this.getConduitCommand());
-
- if (this.params != null) {
- JSONObject obj = new JSONObject();
- obj.putAll(this.params);
-
- InputStream jsonStream = IOUtils.toInputStream(obj.toString(), "UTF-8");
- command = command.stdin(jsonStream);
- }
- return command;
+ return starter.cmds(this.getConduitCommand());
}
public int callConduit(Launcher.ProcStarter starter, PrintStream stderr) throws IOException, InterruptedException {
diff --git a/src/main/java/com/uber/jenkins/phabricator/conduit/ArcanistUsageException.java b/src/main/java/com/uber/jenkins/phabricator/conduit/ArcanistUsageException.java
index b2366fac..163e5fc2 100644
--- a/src/main/java/com/uber/jenkins/phabricator/conduit/ArcanistUsageException.java
+++ b/src/main/java/com/uber/jenkins/phabricator/conduit/ArcanistUsageException.java
@@ -20,7 +20,7 @@
package com.uber.jenkins.phabricator.conduit;
-public class ArcanistUsageException extends Exception {
+class ArcanistUsageException extends Exception {
public ArcanistUsageException(String message) {
super(message);
}
diff --git a/src/main/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClient.java b/src/main/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClient.java
index 733dcb97..120e802b 100644
--- a/src/main/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClient.java
+++ b/src/main/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClient.java
@@ -36,6 +36,9 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.UnsupportedEncodingException;
+import java.net.MalformedURLException;
+import java.net.URISyntaxException;
+import java.net.URL;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
@@ -77,20 +80,30 @@ public JSONObject perform(String action, Map data) throws IOExce
}
JsonSlurper jsonParser = new JsonSlurper();
-
return (JSONObject)jsonParser.parse(responseBody);
}
- public HttpUriRequest createRequest(String action, Map data) throws UnsupportedEncodingException {
- HttpPost post = new HttpPost(String.format("%s/api/%s", conduitURL, action));
+ public HttpUriRequest createRequest(String action, Map data) throws UnsupportedEncodingException, ConduitAPIException {
+ HttpPost post;
+ try {
+ post = new HttpPost(
+ new URL(new URL(new URL(conduitURL), "/api/"), action).toURI()
+ );
+ } catch (MalformedURLException e) {
+ throw new ConduitAPIException(e.getMessage());
+ } catch (URISyntaxException e) {
+ throw new ConduitAPIException(e.getMessage());
+ }
List params = new ArrayList();
params.add(new BasicNameValuePair(API_TOKEN_KEY, conduitToken));
for (Map.Entry datum : data.entrySet()) {
params.add(new BasicNameValuePair(datum.getKey(), datum.getValue()));
}
+
UrlEncodedFormEntity entity = new UrlEncodedFormEntity(params);
post.setEntity(entity);
+
return post;
}
}
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 3932702c..f65982c5 100644
--- a/src/main/java/com/uber/jenkins/phabricator/conduit/Differential.java
+++ b/src/main/java/com/uber/jenkins/phabricator/conduit/Differential.java
@@ -31,7 +31,7 @@
import java.net.URL;
public class Differential {
- private JSONObject rawJSON;
+ private final JSONObject rawJSON;
public Differential(JSONObject rawJSON) {
this.rawJSON = rawJSON;
diff --git a/src/main/java/com/uber/jenkins/phabricator/conduit/DifferentialClient.java b/src/main/java/com/uber/jenkins/phabricator/conduit/DifferentialClient.java
index 95082c11..9f506a22 100644
--- a/src/main/java/com/uber/jenkins/phabricator/conduit/DifferentialClient.java
+++ b/src/main/java/com/uber/jenkins/phabricator/conduit/DifferentialClient.java
@@ -51,21 +51,21 @@ public JSONObject postComment(String revisionID, String message, boolean silent,
params.put("revision_id", revisionID);
params.put("action", action);
params.put("message", message);
- params.put("silent", silent);
+ params.put("silent", Boolean.toString(silent));
return this.callConduit("differential.createcomment", params);
}
public JSONObject fetchDiff() throws IOException, ConduitAPIException {
Map params = new HashMap();
- params.put("ids", new String[]{this.diffID});
+ params.put("ids[]", diffID);
JSONObject query = this.callConduit("differential.querydiffs", params);
JSONObject response;
try {
- response = query.getJSONObject("response");
+ response = query.getJSONObject("result");
} catch (JSONException e) {
throw new ConduitAPIException(
- String.format("No 'response' object found in conduit call: (%s) %s",
+ String.format("No 'result' object found in conduit call: (%s) %s",
e.getMessage(),
query.toString(2)));
}
diff --git a/src/main/java/com/uber/jenkins/phabricator/tasks/ApplyPatchTask.java b/src/main/java/com/uber/jenkins/phabricator/tasks/ApplyPatchTask.java
index 24a7abc6..fb3d6152 100644
--- a/src/main/java/com/uber/jenkins/phabricator/tasks/ApplyPatchTask.java
+++ b/src/main/java/com/uber/jenkins/phabricator/tasks/ApplyPatchTask.java
@@ -106,7 +106,6 @@ protected void execute() {
ArcanistClient arc = new ArcanistClient(
arcPath,
"patch",
- null,
conduitToken,
params.toArray(new String[params.size()]));
diff --git a/src/main/java/com/uber/jenkins/phabricator/tasks/NonDifferentialBuildTask.java b/src/main/java/com/uber/jenkins/phabricator/tasks/NonDifferentialBuildTask.java
index 4821b761..14848d28 100644
--- a/src/main/java/com/uber/jenkins/phabricator/tasks/NonDifferentialBuildTask.java
+++ b/src/main/java/com/uber/jenkins/phabricator/tasks/NonDifferentialBuildTask.java
@@ -30,10 +30,10 @@
*/
public class NonDifferentialBuildTask extends Task {
- protected UberallsClient uberallsClient;
- protected CodeCoverageMetrics codeCoverageMetrics;
- protected boolean uberallsEnabled;
- protected String commitSha;
+ protected final UberallsClient uberallsClient;
+ protected final CodeCoverageMetrics codeCoverageMetrics;
+ protected final boolean uberallsEnabled;
+ protected final String commitSha;
/**
* GenericBuildTask constructor.
diff --git a/src/main/java/com/uber/jenkins/phabricator/tasks/Task.java b/src/main/java/com/uber/jenkins/phabricator/tasks/Task.java
index 4b602450..9a12fd80 100644
--- a/src/main/java/com/uber/jenkins/phabricator/tasks/Task.java
+++ b/src/main/java/com/uber/jenkins/phabricator/tasks/Task.java
@@ -39,7 +39,7 @@ public enum Result {
}
protected Result result = Result.UNKNOWN;
- private Logger logger;
+ private final Logger logger;
/**
* Task constructor.
diff --git a/src/main/java/com/uber/jenkins/phabricator/utils/Logger.java b/src/main/java/com/uber/jenkins/phabricator/utils/Logger.java
index c8703d25..e6e23a63 100644
--- a/src/main/java/com/uber/jenkins/phabricator/utils/Logger.java
+++ b/src/main/java/com/uber/jenkins/phabricator/utils/Logger.java
@@ -28,7 +28,7 @@
public class Logger {
private static final String LOG_FORMAT = "[%s] %s";
- private PrintStream stream;
+ private final PrintStream stream;
/**
* Logger constructor.
diff --git a/src/test/java/com/uber/jenkins/phabricator/BuildIntegrationTest.java b/src/test/java/com/uber/jenkins/phabricator/BuildIntegrationTest.java
index ca7b6d57..599ca042 100644
--- a/src/test/java/com/uber/jenkins/phabricator/BuildIntegrationTest.java
+++ b/src/test/java/com/uber/jenkins/phabricator/BuildIntegrationTest.java
@@ -33,14 +33,14 @@ public class BuildIntegrationTest {
@Rule
public JenkinsRule j = new JenkinsRule();
- protected FreeStyleProject p;
+ FreeStyleProject p;
- protected void assertSuccessfulBuild(Result result) {
+ void assertSuccessfulBuild(Result result) {
assertTrue(result.isCompleteBuild());
assertTrue(result.isBetterOrEqualTo(Result.SUCCESS));
}
- protected FreeStyleProject createProject() throws IOException {
+ FreeStyleProject createProject() throws IOException {
return j.createFreeStyleProject();
}
}
diff --git a/src/test/java/com/uber/jenkins/phabricator/conduit/ArcanistClientTest.java b/src/test/java/com/uber/jenkins/phabricator/conduit/ArcanistClientTest.java
index 52c7676a..049f746f 100644
--- a/src/test/java/com/uber/jenkins/phabricator/conduit/ArcanistClientTest.java
+++ b/src/test/java/com/uber/jenkins/phabricator/conduit/ArcanistClientTest.java
@@ -27,9 +27,6 @@
import org.junit.Test;
import org.jvnet.hudson.test.JenkinsRule;
-import java.util.HashMap;
-import java.util.Map;
-
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
@@ -37,11 +34,9 @@ public class ArcanistClientTest {
@Rule
public JenkinsRule j = new JenkinsRule();
- private final Map emptyParams = new HashMap();
-
@Test
public void testEcho() throws Exception {
- ArcanistClient client = new ArcanistClient("echo", "hello", emptyParams, null);
+ ArcanistClient client = new ArcanistClient("echo", "hello", null);
int result = client.callConduit(getLauncher().launch(), System.err);
assertEquals(result, 0);
@@ -49,7 +44,7 @@ public void testEcho() throws Exception {
@Test
public void testEchoWithToken() throws Exception {
- ArcanistClient client = new ArcanistClient("echo", "tokentest", emptyParams, "notarealtoken");
+ ArcanistClient client = new ArcanistClient("echo", "tokentest", "notarealtoken");
int result = client.callConduit(getLauncher().launch(), System.err);
assertEquals(result, 0);
@@ -58,7 +53,7 @@ public void testEchoWithToken() throws Exception {
@Test
public void testParseConduit() throws Exception {
String jsonString = "{\"hello\": \"world\"}";
- ArcanistClient client = new ArcanistClient("echo", jsonString, emptyParams, null);
+ ArcanistClient client = new ArcanistClient("echo", jsonString, null);
JSONObject result = client.parseConduit(getLauncher().launch(), System.err);
assertTrue(result.has("hello"));
@@ -67,14 +62,14 @@ public void testParseConduit() throws Exception {
@Test(expected = ArcanistUsageException.class)
public void testNonZeroExitCode() throws Exception {
- ArcanistClient client = new ArcanistClient("false", "", emptyParams, null);
+ ArcanistClient client = new ArcanistClient("false", "", null);
client.parseConduit(getLauncher().launch(), System.err);
}
@Test(expected = JSONException.class)
public void testNonJsonOutput() throws Exception {
- ArcanistClient client = new ArcanistClient("echo", "not-json", emptyParams, null);
+ ArcanistClient client = new ArcanistClient("echo", "not-json", null);
client.parseConduit(getLauncher().launch(), System.err);
}
diff --git a/src/test/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClientTest.java b/src/test/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClientTest.java
index 5f404990..89e48577 100644
--- a/src/test/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClientTest.java
+++ b/src/test/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClientTest.java
@@ -74,7 +74,7 @@ public void testBadRequestErrorCode() throws Exception {
}
@Test
- public void testWithParams() throws UnsupportedEncodingException {
+ public void testWithParams() throws UnsupportedEncodingException, ConduitAPIException {
client = new ConduitAPIClient("http://foo.bar", TestUtils.TEST_CONDUIT_TOKEN);
Map params = new HashMap();
params.put("hello", "world");
diff --git a/src/test/java/com/uber/jenkins/phabricator/conduit/DifferentialClientTest.java b/src/test/java/com/uber/jenkins/phabricator/conduit/DifferentialClientTest.java
index 4dd91de8..319dc3c2 100644
--- a/src/test/java/com/uber/jenkins/phabricator/conduit/DifferentialClientTest.java
+++ b/src/test/java/com/uber/jenkins/phabricator/conduit/DifferentialClientTest.java
@@ -75,7 +75,7 @@ public void testFetchDiffWithEmptyResponse() throws Exception {
@Test(expected = ConduitAPIException.class)
public void testFetchDiffWithNoDiff() throws Exception {
JSONObject noDiff = new JSONObject();
- noDiff.put("response", null);
+ noDiff.put("result", null);
mockConduitResponse(differentialClient, noDiff);
differentialClient.fetchDiff();
diff --git a/src/test/java/com/uber/jenkins/phabricator/tasks/PostCommentTaskTest.java b/src/test/java/com/uber/jenkins/phabricator/tasks/PostCommentTaskTest.java
index 0fdc4eeb..cfaffa8f 100644
--- a/src/test/java/com/uber/jenkins/phabricator/tasks/PostCommentTaskTest.java
+++ b/src/test/java/com/uber/jenkins/phabricator/tasks/PostCommentTaskTest.java
@@ -34,13 +34,12 @@
import static org.mockito.Mockito.doThrow;
public class PostCommentTaskTest {
-
private Logger logger;
private DifferentialClient differentialClient;
- private String TEST_COMMENT = "They are selling like hotcakes!";
- private String TEST_COMMENT_ACTION = "none";
- private String TEST_REVISION_ID = "something";
+ private final String TEST_COMMENT = "They are selling like hotcakes!";
+ private final String TEST_COMMENT_ACTION = "none";
+ private final String TEST_REVISION_ID = "something";
@Before
public void setup() {
diff --git a/src/test/resources/com/uber/jenkins/phabricator/conduit/fetchDiffResponseMissingDiff.json b/src/test/resources/com/uber/jenkins/phabricator/conduit/fetchDiffResponseMissingDiff.json
index 1bcefa84..7a43ca58 100644
--- a/src/test/resources/com/uber/jenkins/phabricator/conduit/fetchDiffResponseMissingDiff.json
+++ b/src/test/resources/com/uber/jenkins/phabricator/conduit/fetchDiffResponseMissingDiff.json
@@ -1,5 +1,5 @@
{
- "response": {
+ "result": {
"not-my-diff": {
"garbaj": true
}
diff --git a/src/test/resources/com/uber/jenkins/phabricator/conduit/fetchDiffWithResponseArray.json b/src/test/resources/com/uber/jenkins/phabricator/conduit/fetchDiffWithResponseArray.json
index b0959685..4677642a 100644
--- a/src/test/resources/com/uber/jenkins/phabricator/conduit/fetchDiffWithResponseArray.json
+++ b/src/test/resources/com/uber/jenkins/phabricator/conduit/fetchDiffWithResponseArray.json
@@ -1,3 +1,3 @@
{
- "response": []
+ "result": []
}
diff --git a/src/test/resources/com/uber/jenkins/phabricator/conduit/validFetchDiffResponse.json b/src/test/resources/com/uber/jenkins/phabricator/conduit/validFetchDiffResponse.json
index 3208af59..1d4742db 100644
--- a/src/test/resources/com/uber/jenkins/phabricator/conduit/validFetchDiffResponse.json
+++ b/src/test/resources/com/uber/jenkins/phabricator/conduit/validFetchDiffResponse.json
@@ -1,5 +1,5 @@
{
- "response": {
+ "result": {
"123": {
"hello": "world"
}