From 2bcbd9dbc99143cc0dec03dfeb66ba3deaaba12c Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Mon, 17 Aug 2015 19:11:57 -0700 Subject: [PATCH 1/3] Move all conduit invocations to JSONObject Instead of having two APIs, where we were confusingly using a Map even when it contained complex objects, use a real JSONObject everywhere. --- .../phabricator/conduit/ConduitAPIClient.java | 14 ------- .../conduit/DifferentialClient.java | 42 ++++++++----------- .../conduit/ConduitAPIClientTest.java | 2 +- .../conduit/DifferentialClientTest.java | 4 +- 4 files changed, 21 insertions(+), 41 deletions(-) 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 8b7c07db..e50c2c19 100644 --- a/src/main/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClient.java +++ b/src/main/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClient.java @@ -55,20 +55,6 @@ public ConduitAPIClient(String conduitURL, String conduitToken) { this.conduitToken = conduitToken; } - /** - * Call the conduit API of Phabricator - * @param action Name of the API call - * @param data The data to send to Harbormaster - * @return The result as a JSONObject - * @throws IOException If there was a problem reading the response - * @throws ConduitAPIException If there was an error calling conduit - */ - public JSONObject perform(String action, Map data) throws IOException, ConduitAPIException { - JSONObject params = new JSONObject(); - params.putAll(data); - return perform(action, params); - } - /** * Call the conduit API of Phabricator * @param action Name of the API call 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 406e2a4a..15a21009 100644 --- a/src/main/java/com/uber/jenkins/phabricator/conduit/DifferentialClient.java +++ b/src/main/java/com/uber/jenkins/phabricator/conduit/DifferentialClient.java @@ -24,8 +24,6 @@ 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 @@ -50,11 +48,11 @@ public DifferentialClient(String diffID, ConduitAPIClient conduit) { * @throws ConduitAPIException if any error is experienced talking to Conduit */ public JSONObject postComment(String revisionID, String message, boolean silent, String action) throws IOException, ConduitAPIException { - Map params = new HashMap(); - params.put("revision_id", revisionID); - params.put("action", action); - params.put("message", message); - params.put("silent", silent); + JSONObject params = new JSONObject(); + params.element("revision_id", revisionID) + .element("action", action) + .element("message", message) + .element("silent", silent); return this.callConduit("differential.createcomment", params); } @@ -66,8 +64,7 @@ public JSONObject postComment(String revisionID, String message, boolean silent, * @throws ConduitAPIException if any error is experienced talking to Conduit */ public JSONObject fetchDiff() throws IOException, ConduitAPIException { - Map params = new HashMap(); - params.put("ids", new String[]{diffID}); + JSONObject params = new JSONObject().element("ids", new String[]{diffID}); JSONObject query = this.callConduit("differential.querydiffs", params); JSONObject response; try { @@ -99,10 +96,9 @@ public JSONObject fetchDiff() throws IOException, ConduitAPIException { * @throws ConduitAPIException if any error is experienced talking to Conduit */ public JSONObject sendHarbormasterMessage(String phid, boolean pass) throws ConduitAPIException, IOException { - Map params = new HashMap(); - params.put("type", pass ? "pass" : "fail"); - params.put("buildTargetPHID", phid); - + JSONObject params = new JSONObject(); + params.element("type", pass ? "pass" : "fail") + .element("buildTargetPHID", phid); return this.callConduit("harbormaster.sendmessage", params); } @@ -115,15 +111,17 @@ public JSONObject sendHarbormasterMessage(String phid, boolean pass) throws Cond * @throws ConduitAPIException if any error is experienced talking to Conduit */ public JSONObject sendHarbormasterUri(String phid, String buildUri) throws ConduitAPIException, IOException { - Map params = new HashMap(); - params.put("buildTargetPHID", phid); - params.put("artifactKey", "jenkins.uri"); - params.put("artifactType", "uri"); JSONObject artifactData = new JSONObject(); artifactData = artifactData.element("uri", buildUri) - .element("name", "Jenkins") - .element("ui.external", true); - params.put("artifactData", artifactData); + .element("name", "Jenkins") + .element("ui.external", true); + + JSONObject params = new JSONObject(); + params.element("buildTargetPHID", phid) + .element("artifactKey", "jenkins.uri") + .element("artifactType", "uri") + .element("artifactData", artifactData); + return this.callConduit("harbormaster.createartifact", params); } @@ -139,10 +137,6 @@ public JSONObject postComment(String revisionID, String message) throws ConduitA return postComment(revisionID, message, true, "none"); } - protected JSONObject callConduit(String methodName, Map params) throws ConduitAPIException, IOException { - return conduit.perform(methodName, params); - } - protected JSONObject callConduit(String methodName, JSONObject params) throws ConduitAPIException, IOException { return conduit.perform(methodName, params); } 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 50f1200d..a9565f9e 100644 --- a/src/test/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClientTest.java +++ b/src/test/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClientTest.java @@ -37,7 +37,7 @@ public class ConduitAPIClientTest { private LocalTestServer server; private ConduitAPIClient client; - private final Map emptyParams = new HashMap(); + private final JSONObject emptyParams = new JSONObject(); @Before public void setUp() throws Exception { 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 319dc3c2..c4e04c06 100644 --- a/src/test/java/com/uber/jenkins/phabricator/conduit/DifferentialClientTest.java +++ b/src/test/java/com/uber/jenkins/phabricator/conduit/DifferentialClientTest.java @@ -29,7 +29,7 @@ import static junit.framework.TestCase.assertEquals; import static org.junit.Assert.assertTrue; -import static org.mockito.Matchers.anyMap; +import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.doReturn; @@ -117,7 +117,7 @@ public void testSendHarbormasterSuccess() throws IOException, ConduitAPIExceptio private void mockConduitResponse(DifferentialClient client, JSONObject response) throws IOException, ConduitAPIException { doReturn(response).when(client).callConduit( anyString(), - anyMap() + any(JSONObject.class) ); } } From cf53a72bb0585de1725d6e8cecabd025629f0089 Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Tue, 18 Aug 2015 09:48:22 -0700 Subject: [PATCH 2/3] Remove unused map method for createRequest --- .../phabricator/conduit/ConduitAPIClient.java | 14 -------------- .../phabricator/conduit/ConduitAPIClientTest.java | 4 +--- 2 files changed, 1 insertion(+), 17 deletions(-) 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 e50c2c19..0c034bc3 100644 --- a/src/main/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClient.java +++ b/src/main/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClient.java @@ -84,20 +84,6 @@ public JSONObject perform(String action, JSONObject params) throws IOException, return (JSONObject)jsonParser.parse(responseBody); } - /** - * Post a URL-encoded "params" key with a JSON-encoded body as per the Conduit API - * @param action The name of the Conduit method - * @param data The data to be sent to the Conduit method - * @return The request to perform - * @throws UnsupportedEncodingException when the POST data can't be encoded - * @throws ConduitAPIException when the conduit URL is misconfigured - */ - public HttpUriRequest createRequest(String action, Map data) throws UnsupportedEncodingException, ConduitAPIException { - JSONObject params = new JSONObject(); - params.putAll(data); - return createRequest(action, params); - } - /** * Post a URL-encoded "params" key with a JSON-encoded body as per the Conduit API * @param action The name of the Conduit method 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 a9565f9e..b3cd35ed 100644 --- a/src/test/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClientTest.java +++ b/src/test/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClientTest.java @@ -29,8 +29,6 @@ import org.junit.Test; import java.io.UnsupportedEncodingException; -import java.util.HashMap; -import java.util.Map; import static org.junit.Assert.assertEquals; @@ -76,7 +74,7 @@ public void testBadRequestErrorCode() throws Exception { @Test public void testWithParams() throws UnsupportedEncodingException, ConduitAPIException { client = new ConduitAPIClient("http://foo.bar", TestUtils.TEST_CONDUIT_TOKEN); - Map params = new HashMap(); + JSONObject params = new JSONObject().element("hello", "world"); params.put("hello", "world"); client.createRequest("action", params); } From 6c9ea63f69510e4c02c694ce7af6ba12fe0cdf0c Mon Sep 17 00:00:00 2001 From: Aiden Scandella Date: Tue, 18 Aug 2015 09:49:02 -0700 Subject: [PATCH 3/3] Fix whitespace --- .../uber/jenkins/phabricator/conduit/ConduitAPIClient.java | 1 - .../jenkins/phabricator/conduit/DifferentialClient.java | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) 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 0c034bc3..1b690a08 100644 --- a/src/main/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClient.java +++ b/src/main/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClient.java @@ -41,7 +41,6 @@ import java.net.URL; import java.util.ArrayList; import java.util.List; -import java.util.Map; public class ConduitAPIClient { private static final String API_TOKEN_KEY = "token"; 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 15a21009..2328f240 100644 --- a/src/main/java/com/uber/jenkins/phabricator/conduit/DifferentialClient.java +++ b/src/main/java/com/uber/jenkins/phabricator/conduit/DifferentialClient.java @@ -50,9 +50,9 @@ public DifferentialClient(String diffID, ConduitAPIClient conduit) { public JSONObject postComment(String revisionID, String message, boolean silent, String action) throws IOException, ConduitAPIException { JSONObject params = new JSONObject(); params.element("revision_id", revisionID) - .element("action", action) - .element("message", message) - .element("silent", silent); + .element("action", action) + .element("message", message) + .element("silent", silent); return this.callConduit("differential.createcomment", params); }