diff --git a/phabricator-plugin.iml b/phabricator-plugin.iml index cc0f613e..373c7a10 100644 --- a/phabricator-plugin.iml +++ b/phabricator-plugin.iml @@ -130,6 +130,7 @@ + diff --git a/pom.xml b/pom.xml index c20691ad..71ac2c0d 100644 --- a/pom.xml +++ b/pom.xml @@ -96,6 +96,14 @@ test 4.12 + + + org.apache.httpcomponents + httpclient + 4.3 + tests + test + diff --git a/src/main/java/com/uber/jenkins/phabricator/PhabricatorBuildWrapper.java b/src/main/java/com/uber/jenkins/phabricator/PhabricatorBuildWrapper.java index 82348613..6911336d 100644 --- a/src/main/java/com/uber/jenkins/phabricator/PhabricatorBuildWrapper.java +++ b/src/main/java/com/uber/jenkins/phabricator/PhabricatorBuildWrapper.java @@ -20,11 +20,13 @@ package com.uber.jenkins.phabricator; -import com.uber.jenkins.phabricator.conduit.ArcanistClient; -import com.uber.jenkins.phabricator.conduit.ArcanistUsageException; +import com.uber.jenkins.phabricator.conduit.ConduitAPIClient; +import com.uber.jenkins.phabricator.conduit.ConduitAPIException; import com.uber.jenkins.phabricator.conduit.Differential; import com.uber.jenkins.phabricator.conduit.DifferentialClient; import com.uber.jenkins.phabricator.credentials.ConduitCredentials; +import com.uber.jenkins.phabricator.tasks.ApplyPatchTask; +import com.uber.jenkins.phabricator.tasks.Task; import com.uber.jenkins.phabricator.utils.CommonUtils; import com.uber.jenkins.phabricator.utils.Logger; import hudson.EnvVars; @@ -36,9 +38,14 @@ import org.kohsuke.stapler.DataBoundConstructor; import java.io.IOException; -import java.util.*; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; public class PhabricatorBuildWrapper extends BuildWrapper { + private static final String CONDUIT_TAG = "conduit"; + private static final String DEFAULT_GIT_PATH = "git"; + private final boolean createCommit; private final boolean applyToMaster; private final boolean uberDotArcanist; @@ -52,6 +59,7 @@ public PhabricatorBuildWrapper(boolean createCommit, boolean applyToMaster, bool this.showBuildStartedMessage = showBuildStartedMessage; } + /** {@inheritDoc} */ @Override public Environment setUp(AbstractBuild build, Launcher launcher, @@ -62,94 +70,70 @@ public Environment setUp(AbstractBuild build, return this.ignoreBuild(logger, "No environment variables found?!"); } - final String arcPath = this.getArcPath(); - final Map envAdditions = new HashMap(); envAdditions.put(PhabricatorPlugin.WRAP_KEY, "true"); - envAdditions.put(PhabricatorPlugin.ARCANIST_PATH, arcPath); - - final String conduitToken = this.getConduitToken(build.getParent(), logger); String diffID = environment.get(PhabricatorPlugin.DIFFERENTIAL_ID_FIELD); if (CommonUtils.isBlank(diffID)) { this.addShortText(build); this.ignoreBuild(logger, "No differential ID found."); - } else { - LauncherFactory starter = new LauncherFactory(launcher, environment, listener.getLogger(), build.getWorkspace()); - - if (uberDotArcanist) { - int npmCode = starter.launch() - .cmds(Arrays.asList("npm", "install", "uber-dot-arcanist")) - .stdout(logger.getStream()) - .join(); - - if (npmCode != 0) { - logger.warn("uber-dot-arcanist", "Got non-zero exit code installing uber-dot-arcanist from npm: " + npmCode); - } - } - - DifferentialClient diffClient = new DifferentialClient(diffID, starter, conduitToken, arcPath); - Differential diff; - try { - diff = new Differential(diffClient.fetchDiff()); - diff.decorate(build, this.getPhabricatorURL(build.getParent())); - - logger.info("arcanist", "Applying patch for differential"); - - // Post a silent notification if option is enabled - if (showBuildStartedMessage) { - diffClient.postComment(diff.getRevisionID(false), diff.getBuildStartedMessage(environment)); - } - } catch (ArcanistUsageException e) { - logger.warn("arcanist", "Unable to apply patch"); - logger.warn("arcanist", e.getMessage()); - return null; - } + return new Environment(){}; + } - String baseCommit = "origin/master"; - if (!applyToMaster) { - baseCommit = diff.getBaseCommit(); - } + LauncherFactory starter = new LauncherFactory(launcher, environment, listener.getLogger(), build.getWorkspace()); - int resetCode = starter.launch() - .cmds(Arrays.asList("git", "reset", "--hard", baseCommit)) + if (uberDotArcanist) { + int npmCode = starter.launch() + .cmds(Arrays.asList("npm", "install", "uber-dot-arcanist")) .stdout(logger.getStream()) .join(); - if (resetCode != 0) { - logger.warn("arcanist", "Got non-zero exit code resetting to base commit " + baseCommit + ": " + resetCode); + if (npmCode != 0) { + logger.warn("uber-dot-arcanist", "Got non-zero exit code installing uber-dot-arcanist from npm: " + npmCode); } + } - // Clean workspace, otherwise `arc patch` may fail - starter.launch() - .stdout(logger.getStream()) - .cmds(Arrays.asList("git", "clean", "-fd", "-f")) - .join(); + ConduitAPIClient conduitClient; + try { + conduitClient = getConduitClient(build.getParent(), logger); + } catch (ConduitAPIException e) { + e.printStackTrace(logger.getStream()); + logger.warn(CONDUIT_TAG, e.getMessage()); + return null; + } - // Update submodules recursively. - starter.launch() - .stdout(logger.getStream()) - .cmds(Arrays.asList("git", "submodule", "update", "--init", "--recursive")) - .join(); + DifferentialClient diffClient = new DifferentialClient(diffID, conduitClient); + Differential diff; + try { + diff = new Differential(diffClient.fetchDiff()); + diff.decorate(build, this.getPhabricatorURL(build.getParent())); - List params = new ArrayList(Arrays.asList("--nobranch", "--diff", diffID)); - if (!createCommit) { - params.add("--nocommit"); - } + logger.info(CONDUIT_TAG, "Fetching differential from Conduit API"); - ArcanistClient arc = new ArcanistClient( - arcPath, - "patch", - null, - conduitToken, - params.toArray(new String[params.size()])); + // Post a silent notification if option is enabled + if (showBuildStartedMessage) { + diffClient.postComment(diff.getRevisionID(false), diff.getBuildStartedMessage(environment)); + } + } catch (ConduitAPIException e) { + logger.warn(CONDUIT_TAG, "Unable to apply patch"); + logger.warn(CONDUIT_TAG, e.getMessage()); + return null; + } - int result = arc.callConduit(starter.launch(), logger.getStream()); + String baseCommit = "origin/master"; + if (!applyToMaster) { + baseCommit = diff.getBaseCommit(); + } - if (result != 0) { - logger.warn("arcanist", "Error applying arc patch; got non-zero exit code " + result); - return null; - } + final String conduitToken = this.getConduitToken(build.getParent(), logger); + Task.Result result = new ApplyPatchTask( + logger, starter, baseCommit, diffID, conduitToken, getArcPath(), + DEFAULT_GIT_PATH, createCommit + ).run(); + + if (result != Task.Result.SUCCESS) { + logger.warn("arcanist", "Error applying arc patch; got non-zero exit code " + result); + return null; } return new Environment() { @@ -173,6 +157,18 @@ private Environment ignoreBuild(Logger logger, String message) { return new Environment(){}; } + private ConduitAPIClient getConduitClient(Job owner, Logger logger) throws ConduitAPIException { + ConduitCredentials credentials = getConduitCredentials(owner); + if (credentials == null) { + throw new ConduitAPIException("No credentials configured for conduit"); + } + return new ConduitAPIClient(credentials.getUrl(), getConduitToken(owner, logger)); + } + + private ConduitCredentials getConduitCredentials(Job owner) { + return getDescriptor().getCredentials(owner); + } + /** * This is used in config.jelly to populate the state of the checkbox */ @@ -197,7 +193,7 @@ public boolean isShowBuildStartedMessage() { } public String getPhabricatorURL(Job owner) { - ConduitCredentials credentials = this.getDescriptor().getCredentials(owner); + ConduitCredentials credentials = getConduitCredentials(owner); if (credentials != null) { return credentials.getUrl(); } @@ -205,7 +201,7 @@ public String getPhabricatorURL(Job owner) { } public String getConduitToken(Job owner, Logger logger) { - ConduitCredentials credentials = this.getDescriptor().getCredentials(owner); + ConduitCredentials credentials = getConduitCredentials(owner); if (credentials != null) { return credentials.getToken().getPlainText(); } diff --git a/src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java b/src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java index bd809fa5..1dc396d7 100644 --- a/src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java +++ b/src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java @@ -20,9 +20,7 @@ package com.uber.jenkins.phabricator; -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.conduit.*; import com.uber.jenkins.phabricator.credentials.ConduitCredentials; import com.uber.jenkins.phabricator.tasks.NonDifferentialBuildTask; import com.uber.jenkins.phabricator.tasks.PostCommentTask; @@ -48,6 +46,8 @@ import java.io.PrintStream; public class PhabricatorNotifier extends Notifier { + private static final String UBERALLS_TAG = "uberalls"; + private static final String CONDUIT_TAG = "conduit"; // Post a comment on success. Useful for lengthy builds. private final boolean commentOnSuccess; private final boolean uberallsEnabled; @@ -86,9 +86,6 @@ public final boolean perform(final AbstractBuild build, final Launcher lau environment.get("GIT_URL"), branch); final boolean needsDecoration = environment.get(PhabricatorPlugin.WRAP_KEY, null) == null; - final String conduitToken = getConduitToken(build.getParent(), logger); - - final String arcPath = environment.get(PhabricatorPlugin.ARCANIST_PATH, "arc"); final boolean uberallsConfigured = !CommonUtils.isBlank(uberalls.getBaseURL()); final String diffID = environment.get(PhabricatorPlugin.DIFFERENTIAL_ID_FIELD); @@ -107,14 +104,21 @@ public final boolean perform(final AbstractBuild build, final Launcher lau return true; } - LauncherFactory starter = new LauncherFactory(launcher, environment, listener.getLogger(), build.getWorkspace()); + ConduitAPIClient conduitClient; + try { + conduitClient = getConduitClient(build.getParent(), logger); + } catch (ConduitAPIException e) { + e.printStackTrace(logger.getStream()); + logger.warn(CONDUIT_TAG, e.getMessage()); + return false; + } - DifferentialClient diffClient = new DifferentialClient(diffID, starter, conduitToken, arcPath); + DifferentialClient diffClient = new DifferentialClient(diffID, conduitClient); Differential diff; try { diff = new Differential(diffClient.fetchDiff()); - } catch (ArcanistUsageException e) { - logger.info("arcanist", "unable to fetch differential"); + } catch (ConduitAPIException e) { + logger.info(CONDUIT_TAG, "unable to fetch differential"); return true; } @@ -136,10 +140,10 @@ public final boolean perform(final AbstractBuild build, final Launcher lau if (uberallsConfigured) { commenter.processParentCoverage(uberalls.getParentCoverage(diff), diff.getBranch()); } else { - logger.info("uberalls", "no backend configured, skipping..."); + logger.info(UBERALLS_TAG, "no backend configured, skipping..."); } } else { - logger.info("uberalls", "no line coverage found, skipping..."); + logger.info(UBERALLS_TAG, "no line coverage found, skipping..."); } // Add in comments about the build result @@ -155,8 +159,8 @@ public final boolean perform(final AbstractBuild build, final Launcher lau String.format("Error from Harbormaster: %s", result.getString("errorMessage"))); return false; } - } catch (ArcanistUsageException e) { - logger.info("arcanist", "unable to post to harbormaster"); + } catch (ConduitAPIException e) { + logger.info(CONDUIT_TAG, "unable to post to harbormaster"); return true; } } else { @@ -192,6 +196,14 @@ public final boolean perform(final AbstractBuild build, final Launcher lau return true; } + private ConduitAPIClient getConduitClient(Job owner, Logger logger) throws ConduitAPIException { + ConduitCredentials credentials = getConduitCredentials(owner); + if (credentials == null) { + throw new ConduitAPIException("No credentials configured for conduit"); + } + return new ConduitAPIClient(credentials.getUrl(), credentials.getToken().getPlainText()); + } + private CoverageResult getUberallsCoverage(AbstractBuild build, BuildListener listener) { if (!build.getResult().isBetterOrEqualTo(Result.UNSTABLE) || !uberallsEnabled) { return null; @@ -235,8 +247,12 @@ public String getCommentFile() { return commentFile; } + private ConduitCredentials getConduitCredentials(Job owner) { + return getDescriptor().getCredentials(owner); + } + public String getConduitToken(Job owner, Logger logger) { - ConduitCredentials credentials = this.getDescriptor().getCredentials(owner); + ConduitCredentials credentials = getConduitCredentials(owner); if (credentials != null) { return credentials.getToken().getPlainText(); } diff --git a/src/main/java/com/uber/jenkins/phabricator/PhabricatorPlugin.java b/src/main/java/com/uber/jenkins/phabricator/PhabricatorPlugin.java index 72bc4eac..9a044a27 100644 --- a/src/main/java/com/uber/jenkins/phabricator/PhabricatorPlugin.java +++ b/src/main/java/com/uber/jenkins/phabricator/PhabricatorPlugin.java @@ -27,8 +27,6 @@ import java.io.File; public class PhabricatorPlugin extends Plugin { - public static final String ARCANIST_PATH = "ARCANIST_PATH"; - // Diff ID (not differential ID) static final String DIFFERENTIAL_ID_FIELD = "DIFF_ID"; // Phabricator object ID (for Harbormaster) diff --git a/src/main/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClient.java b/src/main/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClient.java new file mode 100644 index 00000000..733dcb97 --- /dev/null +++ b/src/main/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClient.java @@ -0,0 +1,96 @@ +// 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 net.sf.json.JSONObject; +import net.sf.json.groovy.JsonSlurper; +import org.apache.commons.httpclient.HttpStatus; +import org.apache.http.HttpResponse; +import org.apache.http.NameValuePair; +import org.apache.http.client.ClientProtocolException; +import org.apache.http.client.entity.UrlEncodedFormEntity; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.client.methods.HttpUriRequest; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.http.message.BasicNameValuePair; + +import java.io.IOException; +import java.io.InputStream; +import java.io.UnsupportedEncodingException; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +public class ConduitAPIClient { + private static final String API_TOKEN_KEY = "api.token"; + + private final String conduitURL; + private final String conduitToken; + + public ConduitAPIClient(String conduitURL, String conduitToken) { + this.conduitURL = conduitURL; + 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 { + CloseableHttpClient client = HttpClientBuilder.create().build(); + HttpUriRequest request = createRequest(action, data); + + HttpResponse response; + try { + response = client.execute(request); + } catch (ClientProtocolException e) { + throw new ConduitAPIException(e.getMessage()); + } + + InputStream responseBody = response.getEntity().getContent(); + + if (response.getStatusLine().getStatusCode() != HttpStatus.SC_OK) { + throw new ConduitAPIException(responseBody.toString(), response.getStatusLine().getStatusCode()); + } + + 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)); + + 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/ConduitAPIException.java b/src/main/java/com/uber/jenkins/phabricator/conduit/ConduitAPIException.java new file mode 100644 index 00000000..2d20a5c5 --- /dev/null +++ b/src/main/java/com/uber/jenkins/phabricator/conduit/ConduitAPIException.java @@ -0,0 +1,35 @@ +// 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; + +public class ConduitAPIException extends Exception { + public final int code; + + public ConduitAPIException(String message) { + super(message); + this.code = 0; + } + + public ConduitAPIException(String message, int code) { + super(message); + this.code = code; + } +} 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 f6510757..3932702c 100644 --- a/src/main/java/com/uber/jenkins/phabricator/conduit/Differential.java +++ b/src/main/java/com/uber/jenkins/phabricator/conduit/Differential.java @@ -27,14 +27,13 @@ import net.sf.json.JSONNull; import net.sf.json.JSONObject; -import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; public class Differential { private JSONObject rawJSON; - public Differential(JSONObject rawJSON) throws ArcanistUsageException, IOException, InterruptedException { + 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 bef369e4..95082c11 100644 --- a/src/main/java/com/uber/jenkins/phabricator/conduit/DifferentialClient.java +++ b/src/main/java/com/uber/jenkins/phabricator/conduit/DifferentialClient.java @@ -20,7 +20,6 @@ package com.uber.jenkins.phabricator.conduit; -import com.uber.jenkins.phabricator.LauncherFactory; import net.sf.json.JSONException; import net.sf.json.JSONObject; @@ -33,15 +32,11 @@ */ public class DifferentialClient { private final String diffID; - private final LauncherFactory launcher; - private final String arcPath; - private final String conduitToken; + private final ConduitAPIClient conduit; - public DifferentialClient(String diffID, LauncherFactory launcher, String conduitToken, String arcPath) { + public DifferentialClient(String diffID, ConduitAPIClient conduit) { this.diffID = diffID; - this.launcher = launcher; - this.conduitToken = conduitToken; - this.arcPath = arcPath; + this.conduit = conduit; } /** @@ -51,7 +46,7 @@ public DifferentialClient(String diffID, LauncherFactory launcher, String condui * @param silent whether or not to trigger an email * @param action phabricator comment action, e.g. 'resign', 'reject', 'none' */ - public JSONObject postComment(String revisionID, String message, boolean silent, String action) throws IOException, InterruptedException, ArcanistUsageException { + public JSONObject postComment(String revisionID, String message, boolean silent, String action) throws ConduitAPIException, IOException { Map params = new HashMap(); params.put("revision_id", revisionID); params.put("action", action); @@ -61,7 +56,7 @@ public JSONObject postComment(String revisionID, String message, boolean silent, return this.callConduit("differential.createcomment", params); } - public JSONObject fetchDiff() throws ArcanistUsageException, IOException, InterruptedException { + public JSONObject fetchDiff() throws IOException, ConduitAPIException { Map params = new HashMap(); params.put("ids", new String[]{this.diffID}); JSONObject query = this.callConduit("differential.querydiffs", params); @@ -69,7 +64,7 @@ public JSONObject fetchDiff() throws ArcanistUsageException, IOException, Interr try { response = query.getJSONObject("response"); } catch (JSONException e) { - throw new ArcanistUsageException( + throw new ConduitAPIException( String.format("No 'response' object found in conduit call: (%s) %s", e.getMessage(), query.toString(2))); @@ -77,7 +72,7 @@ public JSONObject fetchDiff() throws ArcanistUsageException, IOException, Interr try { return response.getJSONObject(diffID); } catch (JSONException e) { - throw new ArcanistUsageException( + throw new ConduitAPIException( String.format("Unable to find '%s' key in response: (%s) %s", diffID, e.getMessage(), @@ -93,7 +88,7 @@ public JSONObject fetchDiff() throws ArcanistUsageException, IOException, Interr * @throws IOException * @throws InterruptedException */ - public JSONObject sendHarbormasterMessage(String phid, boolean pass) throws IOException, InterruptedException, ArcanistUsageException { + public JSONObject sendHarbormasterMessage(String phid, boolean pass) throws ConduitAPIException, IOException { Map params = new HashMap(); params.put("type", pass ? "pass" : "fail"); params.put("buildTargetPHID", phid); @@ -110,27 +105,11 @@ public JSONObject sendHarbormasterMessage(String phid, boolean pass) throws IOEx * @throws InterruptedException * @throws ArcanistUsageException */ - public JSONObject postComment(String revisionID, String message) throws IOException, InterruptedException, ArcanistUsageException { + public JSONObject postComment(String revisionID, String message) throws ConduitAPIException, IOException { return postComment(revisionID, message, true, "none"); } - protected JSONObject callConduit(String methodName, Map params) throws IOException, InterruptedException, ArcanistUsageException { - ArcanistClient arc = getArcanistClient(methodName, params); - return arc.parseConduit(this.launcher.launch(), this.launcher.getStderr()); - } - - /** - * Get a new arcanist client. - * @param params parameters to pass to arcanist - * @return a new ArcanistClient - */ - protected ArcanistClient getArcanistClient(String methodName, Map params) { - return new ArcanistClient( - this.arcPath, - "call-conduit", - params, - conduitToken, - methodName - ); + protected JSONObject callConduit(String methodName, Map params) throws ConduitAPIException, IOException { + return conduit.perform(methodName, params); } } diff --git a/src/main/java/com/uber/jenkins/phabricator/tasks/ApplyPatchTask.java b/src/main/java/com/uber/jenkins/phabricator/tasks/ApplyPatchTask.java new file mode 100644 index 00000000..24a7abc6 --- /dev/null +++ b/src/main/java/com/uber/jenkins/phabricator/tasks/ApplyPatchTask.java @@ -0,0 +1,135 @@ +// 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.tasks; + +import com.uber.jenkins.phabricator.LauncherFactory; +import com.uber.jenkins.phabricator.conduit.ArcanistClient; +import com.uber.jenkins.phabricator.utils.Logger; + +import java.io.IOException; +import java.io.PrintStream; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +public class ApplyPatchTask extends Task { + private final LauncherFactory starter; + private final String baseCommit; + private final String diffID; + private final PrintStream logStream; + private final String conduitToken; + private final String arcPath; + private final boolean createCommit; + private final String gitPath; + + public ApplyPatchTask(Logger logger, LauncherFactory starter, String baseCommit, + String diffID, String conduitToken, String arcPath, + String gitPath, boolean createCommit) { + super(logger); + this.starter = starter; + this.baseCommit = baseCommit; + this.diffID = diffID; + this.conduitToken = conduitToken; + this.arcPath = arcPath; + this.gitPath = gitPath; + this.createCommit = createCommit; + + this.logStream = logger.getStream(); + } + + /** + * {@inheritDoc} + */ + @Override + protected String getTag() { + return "arc-patch"; + } + + /** + * {@inheritDoc} + */ + @Override + protected void setup() { + // Do nothing + } + + /** + * {@inheritDoc} + */ + @Override + protected void execute() { + try { + int resetCode = starter.launch() + .cmds(Arrays.asList(gitPath, "reset", "--hard", baseCommit)) + .stdout(logStream) + .join(); + + if (resetCode != 0) { + info("Got non-zero exit code resetting to base commit " + baseCommit + ": " + resetCode); + } + + // Clean workspace, otherwise `arc patch` may fail + starter.launch() + .stdout(logStream) + .cmds(Arrays.asList(gitPath, "clean", "-fd", "-f")) + .join(); + + // Update submodules recursively. + starter.launch() + .stdout(logStream) + .cmds(Arrays.asList(gitPath, "submodule", "update", "--init", "--recursive")) + .join(); + + List params = new ArrayList(Arrays.asList("--nobranch", "--diff", diffID)); + if (!createCommit) { + params.add("--nocommit"); + } + + ArcanistClient arc = new ArcanistClient( + arcPath, + "patch", + null, + conduitToken, + params.toArray(new String[params.size()])); + + int result = arc.callConduit(starter.launch(), logStream); + if (result == 0) { + this.result = Result.SUCCESS; + } else { + this.result = Result.FAILURE; + } + } catch (IOException e) { + e.printStackTrace(logStream); + this.result = Result.FAILURE; + } catch (InterruptedException e) { + e.printStackTrace(logStream); + this.result = Result.FAILURE; + } + } + + /** + * {@inheritDoc} + */ + @Override + protected void tearDown() { + // Do nothing + } +} diff --git a/src/main/java/com/uber/jenkins/phabricator/tasks/PostCommentTask.java b/src/main/java/com/uber/jenkins/phabricator/tasks/PostCommentTask.java index 6c5a28dc..19ec1ac6 100644 --- a/src/main/java/com/uber/jenkins/phabricator/tasks/PostCommentTask.java +++ b/src/main/java/com/uber/jenkins/phabricator/tasks/PostCommentTask.java @@ -20,7 +20,7 @@ package com.uber.jenkins.phabricator.tasks; -import com.uber.jenkins.phabricator.conduit.ArcanistUsageException; +import com.uber.jenkins.phabricator.conduit.ConduitAPIException; import com.uber.jenkins.phabricator.conduit.DifferentialClient; import com.uber.jenkins.phabricator.utils.Logger; import net.sf.json.JSONNull; @@ -104,12 +104,10 @@ private JSONObject postDifferentialComment(String message, boolean silent, Strin message, silent, action); result = Result.SUCCESS; return postDifferentialCommentResult; - } catch (ArcanistUsageException e) { - info("unable to post comment"); } catch (IOException e) { throw new RuntimeException(e); - } catch (InterruptedException e) { - throw new RuntimeException(e); + } catch (ConduitAPIException e) { + info("unable to post comment"); } result = Result.FAILURE; diff --git a/src/test/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClientTest.java b/src/test/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClientTest.java new file mode 100644 index 00000000..5f404990 --- /dev/null +++ b/src/test/java/com/uber/jenkins/phabricator/conduit/ConduitAPIClientTest.java @@ -0,0 +1,92 @@ +// 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.utils.TestUtils; +import net.sf.json.JSONObject; +import org.apache.commons.httpclient.HttpStatus; +import org.apache.http.localserver.LocalTestServer; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.UnsupportedEncodingException; +import java.util.HashMap; +import java.util.Map; + +import static org.junit.Assert.assertEquals; + +public class ConduitAPIClientTest { + private LocalTestServer server; + private ConduitAPIClient client; + private final Map emptyParams = new HashMap(); + + @Before + public void setUp() throws Exception { + server = new LocalTestServer(null, null); + server.start(); + } + + @After + public void tearDown() throws Exception { + server.stop(); + } + + @Test(expected = ConduitAPIException.class) + public void testInvalidURI() throws Exception { + client = new ConduitAPIClient("gopher://foo", TestUtils.TEST_CONDUIT_TOKEN); + client.perform("anything", emptyParams); + } + + @Test + public void testSuccessfullFetch() throws Exception { + server.register("/api/valid", TestUtils.makeHttpHandler(HttpStatus.SC_OK, "{\"hello\": \"world\"}")); + + client = new ConduitAPIClient(getTestServerAddress(), TestUtils.TEST_CONDUIT_TOKEN); + JSONObject response = client.perform("valid", emptyParams); + assertEquals("world", response.getString("hello")); + } + + @Test(expected = ConduitAPIException.class) + public void testBadRequestErrorCode() throws Exception { + server.register("/api/foo", TestUtils.makeHttpHandler(HttpStatus.SC_BAD_REQUEST, "nothing")); + + client = new ConduitAPIClient(getTestServerAddress(), TestUtils.TEST_CONDUIT_TOKEN); + client.perform("foo", emptyParams); + } + + @Test + public void testWithParams() throws UnsupportedEncodingException { + client = new ConduitAPIClient("http://foo.bar", TestUtils.TEST_CONDUIT_TOKEN); + Map params = new HashMap(); + params.put("hello", "world"); + client.createRequest("action", params); + } + + private String getTestServerAddress() { + return String.format( + "http://%s:%s", + server.getServiceAddress().getHostName(), + server.getServiceAddress().getPort() + ); + } + +} 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 c34da4bc..4dd91de8 100644 --- a/src/test/java/com/uber/jenkins/phabricator/conduit/DifferentialClientTest.java +++ b/src/test/java/com/uber/jenkins/phabricator/conduit/DifferentialClientTest.java @@ -64,7 +64,7 @@ public void testPostCommentSingleArgument() throws Exception { assertEquals(response, sentinel); } - @Test(expected = ArcanistUsageException.class) + @Test(expected = ConduitAPIException.class) public void testFetchDiffWithEmptyResponse() throws Exception { JSONObject empty = new JSONObject(); mockConduitResponse(differentialClient, empty); @@ -72,7 +72,7 @@ public void testFetchDiffWithEmptyResponse() throws Exception { differentialClient.fetchDiff(); } - @Test(expected = ArcanistUsageException.class) + @Test(expected = ConduitAPIException.class) public void testFetchDiffWithNoDiff() throws Exception { JSONObject noDiff = new JSONObject(); noDiff.put("response", null); @@ -99,7 +99,22 @@ public void testFetchDiffWithValidResponse() throws Exception { assertEquals("world", response.get("hello")); } - private void mockConduitResponse(DifferentialClient client, JSONObject response) throws InterruptedException, ArcanistUsageException, IOException { + @Test(expected = ConduitAPIException.class) + public void testFetchDiffWithJSONException() throws Exception { + JSONObject badResponse = TestUtils.getJSONFromFile(getClass(), "fetchDiffWithResponseArray"); + mockConduitResponse(differentialClient, badResponse); + + differentialClient.fetchDiff(); + } + + @Test + public void testSendHarbormasterSuccess() throws IOException, ConduitAPIException { + JSONObject empty = new JSONObject(); + mockConduitResponse(differentialClient, empty); + differentialClient.sendHarbormasterMessage(TestUtils.TEST_PHID, true); + } + + private void mockConduitResponse(DifferentialClient client, JSONObject response) throws IOException, ConduitAPIException { doReturn(response).when(client).callConduit( anyString(), anyMap() diff --git a/src/test/java/com/uber/jenkins/phabricator/tasks/ApplyPatchTaskTest.java b/src/test/java/com/uber/jenkins/phabricator/tasks/ApplyPatchTaskTest.java new file mode 100644 index 00000000..35865b89 --- /dev/null +++ b/src/test/java/com/uber/jenkins/phabricator/tasks/ApplyPatchTaskTest.java @@ -0,0 +1,60 @@ +// 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.tasks; + +import com.uber.jenkins.phabricator.utils.TestUtils; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; + +import static org.junit.Assert.assertEquals; + +public class ApplyPatchTaskTest { + @Rule + public JenkinsRule j = new JenkinsRule(); + + @Test + public void testApplyPatchWithValidArc() throws Exception { + ApplyPatchTask task = getTask("echo", "true"); + Task.Result result = task.run(); + assertEquals(result.SUCCESS, result); + } + + @Test + public void testApplyPatchWithInvalidArc() throws Exception { + ApplyPatchTask task = getTask("false", "echo"); + Task.Result result = task.run(); + assertEquals(result.FAILURE, result); + } + + private ApplyPatchTask getTask(String arcPath, String gitPath) throws Exception { + return new ApplyPatchTask( + TestUtils.getDefaultLogger(), + TestUtils.createLauncherFactory(j), + TestUtils.TEST_SHA, + TestUtils.TEST_DIFFERENTIAL_ID, + TestUtils.TEST_CONDUIT_TOKEN, + arcPath, + gitPath, // git path + false // createCommit + ); + } +} 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 1eaf5c08..0fdc4eeb 100644 --- a/src/test/java/com/uber/jenkins/phabricator/tasks/PostCommentTaskTest.java +++ b/src/test/java/com/uber/jenkins/phabricator/tasks/PostCommentTaskTest.java @@ -20,7 +20,7 @@ package com.uber.jenkins.phabricator.tasks; -import com.uber.jenkins.phabricator.conduit.ArcanistUsageException; +import com.uber.jenkins.phabricator.conduit.ConduitAPIException; import com.uber.jenkins.phabricator.conduit.DifferentialClient; import com.uber.jenkins.phabricator.utils.Logger; import com.uber.jenkins.phabricator.utils.TestUtils; @@ -50,7 +50,7 @@ public void setup() { @Test public void testPostDifferentialFailed() throws Exception { - doThrow(new ArcanistUsageException("")).when(differentialClient).postComment( + doThrow(new ConduitAPIException("")).when(differentialClient).postComment( anyString(), anyString(), anyBoolean(), diff --git a/src/test/java/com/uber/jenkins/phabricator/utils/TestUtils.java b/src/test/java/com/uber/jenkins/phabricator/utils/TestUtils.java index 175c0db6..c6bb789f 100644 --- a/src/test/java/com/uber/jenkins/phabricator/utils/TestUtils.java +++ b/src/test/java/com/uber/jenkins/phabricator/utils/TestUtils.java @@ -22,6 +22,7 @@ import com.uber.jenkins.phabricator.CodeCoverageMetrics; import com.uber.jenkins.phabricator.LauncherFactory; +import com.uber.jenkins.phabricator.conduit.ConduitAPIClient; import com.uber.jenkins.phabricator.conduit.DifferentialClient; import com.uber.jenkins.phabricator.uberalls.UberallsClient; import hudson.EnvVars; @@ -31,6 +32,12 @@ import hudson.plugins.cobertura.targets.CoverageResult; import net.sf.json.JSONObject; import net.sf.json.groovy.JsonSlurper; +import org.apache.http.HttpException; +import org.apache.http.HttpRequest; +import org.apache.http.HttpResponse; +import org.apache.http.entity.StringEntity; +import org.apache.http.protocol.HttpContext; +import org.apache.http.protocol.HttpRequestHandler; import org.jvnet.hudson.test.JenkinsRule; import java.io.ByteArrayOutputStream; @@ -48,7 +55,8 @@ public class TestUtils { public static final String TEST_SHA = "test-sha"; public static final String TEST_DIFFERENTIAL_ID = "123"; - private static final String TEST_CONDUIT_TOKEN = "notarealtoken"; + public static final String TEST_CONDUIT_TOKEN = "notarealtoken"; + public static final String TEST_PHID = "PHID-not-real"; private static final String TEST_ARC_PATH = "echo"; public static Logger getDefaultLogger() { @@ -65,9 +73,8 @@ public static UberallsClient getDefaultUberallsClient() { } public static DifferentialClient getDefaultDifferentialClient() { - LauncherFactory factory = mock(LauncherFactory.class); - return spy(new DifferentialClient(TEST_DIFFERENTIAL_ID, factory, TEST_CONDUIT_TOKEN, - TEST_ARC_PATH)); + ConduitAPIClient client = mock(ConduitAPIClient.class); + return spy(new DifferentialClient(TEST_DIFFERENTIAL_ID, client)); } public static EnvVars getDefaultEnvVars() { @@ -121,7 +128,7 @@ public static CoverageResult getEmptyCoverageResult() { return coverageResult; } - public static JSONObject getJSONFromFile(Class klass, String filename) throws IOException { + public static JSONObject getJSONFromFile(Class klass, String filename) throws IOException { InputStream in = klass.getResourceAsStream(String.format("%s.json", filename)); return slurpFromInputStream(in); } @@ -137,4 +144,15 @@ private static void setCoverage(CoverageResult coverageResult, CoverageMetric co when(coverageResult.getCoverage(coverageMetric)).thenReturn(ratio); } } + + public static HttpRequestHandler makeHttpHandler(final int statusCode, final String body) { + return new HttpRequestHandler() { + @Override + public void handle(HttpRequest request, HttpResponse response, HttpContext context) throws HttpException, IOException { + response.setStatusCode(statusCode); + response.setEntity(new StringEntity(body)); + + } + }; + } } diff --git a/src/test/resources/com/uber/jenkins/phabricator/conduit/fetchDiffWithResponseArray.json b/src/test/resources/com/uber/jenkins/phabricator/conduit/fetchDiffWithResponseArray.json new file mode 100644 index 00000000..b0959685 --- /dev/null +++ b/src/test/resources/com/uber/jenkins/phabricator/conduit/fetchDiffWithResponseArray.json @@ -0,0 +1,3 @@ +{ + "response": [] +}