diff --git a/LICENCE b/LICENSE similarity index 100% rename from LICENCE rename to LICENSE diff --git a/README.md b/README.md index 270ebb180..5dd379097 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,7 @@ For more details, see https://wiki.jenkins-ci.org/display/JENKINS/GitHub+pull+re ### Credentials * If you are using Enterprise GitHub set the server api URL in ``GitHub server api URL``. Otherwise leave there ``https://api.github.com``. +* Set the Jenkins URL if you need to override the default (e.g. it's behind a firewall) * A GitHub API token or username password can be used for access to the GitHub API * To setup credentials for a given GitHub Server API URL: * Click Add next to the ``Credentials`` drop down @@ -104,9 +105,152 @@ Make sure you **DON'T** have ``Prune remote branches before build`` advanced opt #### Parameterized Builds If you want to manually build the job, in the job setting check ``This build is parameterized`` and add string parameter named ``sha1`` with a default value of ``master``. When starting build give the ``sha1`` parameter commit id you want to build or refname (eg: ``origin/pr/9/head``). +### Job DSL Support + +Since the plugin contains an extension for the Job DSL plugin to add DSL syntax for configuring the build trigger and +the pull request merger post-build action. + +It is also possible to set Downstream job commit statuses when `displayBuildErrorsOnDownstreamBuilds()` is set in the +upstream job's triggers and downstreamCommitStatus block is included in the downstream job's wrappers. + +Here is an example showing all DSL syntax elements: + +```groovy +job('upstreamJob') { + scm { + git { + remote { + github('test-owner/test-project') + refspec('+refs/pull/*:refs/remotes/origin/pr/*') + } + branch('${sha1}') + } + } + + triggers { + pullRequest { + admin('user_1') + admins(['user_2', 'user_3']) + userWhitelist('you@you.com') + userWhitelist(['me@me.com', 'they@they.com']) + orgWhitelist('my_github_org') + orgWhitelist(['your_github_org', 'another_org']) + cron('H/5 * * * *') + triggerPhrase('OK to test') + onlyTriggerPhrase() + useGitHubHooks() + permitAll() + autoCloseFailedPullRequests() + displayBuildErrorsOnDownstreamBuilds() + whiteListTargetBranches(['master','test', 'test2']) + allowMembersOfWhitelistedOrgsAsAdmin() + extensions { + commitStatus { + context('deploy to staging site') + triggeredStatus('starting deployment to staging site...') + startedStatus('deploying to staging site...') + statusUrl('http://mystatussite.com/prs') + completedStatus('SUCCESS', 'All is well') + completedStatus('FAILURE', 'Something went wrong. Investigate!') + completedStatus('PENDING', 'still in progress...') + completedStatus('ERROR', 'Something went really wrong. Investigate!') + } + } + } + } + publishers { + mergeGithubPullRequest { + mergeComment('merged by Jenkins') + onlyAdminsMerge() + disallowOwnCode() + failOnNonMerge() + deleteOnMerge() + } + } +} + +job('downstreamJob') { + wrappers { + downstreamCommitStatus { + context('CONTEXT NAME') + triggeredStatus("The job has triggered") + startedStatus("The job has started") + statusUrl() + completedStatus('SUCCESS', "The job has passed") + completedStatus('FAILURE', "The job has failed") + completedStatus('ERROR', "The job has resulted in an error") + } + } +} +``` ### Updates +#### -> 1.30.2 +* Don't run through all the builds for changelog, track it in the PR object instead +* Synchronization around the PR object fields + +#### -> 1.30.1 +* Moved pull request state into build/pullrequests directory. +* Dynamic state is no longer kept as part of the trigger +* Merged #258 ignore comments on issues that aren't pull requests + +#### -> 1.30 +* Merged #253, cleaning up code. +* WebHooks are refactored to be closer to the variables it depends on +* PR data is now local per job instead of global +* The config.xml is only saved when there is a change to a PR the job is watching +* GitHub connections are now shared. +* Shouldn't run into rate limits on startup +* Pull Requests are only updated when the trigger runs, instead of on startup. + +#### -> 1.29.8 +* Merged #246 adding job dsl features and updating docs + +#### -> 1.29.7 +* Remove quoting of trigger phrase +* Merged #242 replaces newline and quoted quotes +* Merged #229 Add job dsl +* Merged #238 Update github-api version + +#### -> 1.29.5 +* Merge #232 fixes erroneous no test cases found + +#### -> 1.29.4 +* Add secret when auto creating the web hook +* Accomodate Jenkins being behind a firewall +* Fix for recursive repo initializations. + +#### -> 1.29.1 +* Fix NPE for triggers without a project +* Add back default URL if env variables are missing + +#### -> 1.29 +* Reduced the size of the plugin .xml file drastically. +* Fixed issues with persistance store credentials. +* Reduced the noise caused by webhook logic, and reduced amount of work for webhooks + +#### -> 1.28 +* Fixed [JENKINS-29850] Add author github repo URL +* Fixed NPE and other errors in merge logic +* Removed only trigger phrase, trigger is expected always now +* Fixed [JENKINS-30115] Downstream jobs can now attach GitHub status messages +* Fixed SHA1 for Matrix builds +* Global defaults are now used if the local value isn't set. +* Fixed issues with CSR protection +* Commit updates can be skipped all together or individual steps can be skipped using --none-- + +#### -> 1.27 +* Trigger is now a regex term +* Phrases are no matched agains multiple lines properly +* All regex patterns use ignore case and dotall +* Added variables to the parameters + +#### -> 1.26 +* Extend the checking for own code. +* Fail build only when desired if we can't merge the build. +* Add option to delete branch when merge succeeds. + #### -> 1.25 * Fix condition where admin can't run tests #60 JENKINS-25572, JENKINS-25574, JENKINS-25603 * Check for when webhook encoding is null and default to UTF-8 diff --git a/pom.xml b/pom.xml index 2933ce890..e444d48b2 100644 --- a/pom.xml +++ b/pom.xml @@ -10,7 +10,7 @@ ghprb GitHub Pull Request Builder - 1.26-SNAPSHOT + 1.30.6-SNAPSHOT hpi https://wiki.jenkins-ci.org/display/JENKINS/GitHub+pull+request+builder+plugin @@ -50,10 +50,15 @@ github 1.9.1 + + org.jenkins-ci.plugins + matrix-project + 1.4.1 + org.jenkins-ci.plugins github-api - 1.66 + 1.71 org.jenkins-ci.plugins @@ -65,6 +70,12 @@ ssh-agent 1.3 + + org.jenkins-ci.plugins + job-dsl + 1.39 + true + junit junit diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java b/src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java index d33860e69..0c6ab8589 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java @@ -14,7 +14,6 @@ import com.cloudbees.plugins.credentials.domains.SchemeSpecification; import com.cloudbees.plugins.credentials.domains.URIRequirementBuilder; import com.cloudbees.plugins.credentials.impl.UsernamePasswordCredentialsImpl; -import com.coravy.hudson.plugins.github.GithubProjectProperty; import hudson.Util; import hudson.model.AbstractBuild; @@ -22,6 +21,7 @@ import hudson.model.Cause; import hudson.model.Item; import hudson.model.Result; +import hudson.model.Run; import hudson.model.Saveable; import hudson.model.TaskListener; import hudson.security.ACL; @@ -37,56 +37,26 @@ import org.jenkinsci.plugins.ghprb.extensions.GhprbProjectExtension; import org.jenkinsci.plugins.plaincredentials.impl.StringCredentialsImpl; import org.kohsuke.github.GHCommitState; +import org.kohsuke.github.GHIssue; import org.kohsuke.github.GHUser; import java.net.URI; import java.util.*; -import java.util.concurrent.ConcurrentMap; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.regex.Matcher; import java.util.regex.Pattern; -import jenkins.model.Jenkins; - /** * @author janinko */ public class Ghprb { private static final Logger logger = Logger.getLogger(Ghprb.class.getName()); - private static final Pattern githubUserRepoPattern = Pattern.compile("^(http[s]?://[^/]*)/([^/]*)/([^/]*).*"); + public static final Pattern githubUserRepoPattern = Pattern.compile("^(http[s]?://[^/]*)/([^/]*/[^/]*).*"); private final GhprbTrigger trigger; - private final AbstractProject project; - private GhprbRepository repository; - private GhprbBuilds builds; - - public Ghprb(AbstractProject project, GhprbTrigger trigger, ConcurrentMap pulls) { - this.project = project; - - final GithubProjectProperty ghpp = project.getProperty(GithubProjectProperty.class); - if (ghpp == null || ghpp.getProjectUrl() == null) { - throw new IllegalStateException("A GitHub project url is required."); - } - String baseUrl = ghpp.getProjectUrl().baseUrl(); - Matcher m = githubUserRepoPattern.matcher(baseUrl); - if (!m.matches()) { - throw new IllegalStateException(String.format("Invalid GitHub project url: %s", baseUrl)); - } - final String user = m.group(2); - final String repo = m.group(3); + public Ghprb(GhprbTrigger trigger) { this.trigger = trigger; - - this.repository = new GhprbRepository(user, repo, this, pulls); - this.builds = new GhprbBuilds(trigger, repository); - } - - public void init() { - this.repository.init(); - if (trigger.getUseGitHubHooks()) { - this.repository.createHook(); - } } public void addWhitelist(String author) { @@ -95,51 +65,77 @@ public void addWhitelist(String author) { } public boolean isProjectDisabled() { - return project.isDisabled(); + return !trigger.isActive(); } public GhprbBuilds getBuilds() { - return builds; + return trigger.getBuilds(); } public GhprbTrigger getTrigger() { return trigger; } - public GhprbRepository getRepository() { - return repository; - } public GhprbGitHub getGitHub() { - return new GhprbGitHub(trigger); + return trigger.getGhprbGitHub(); } - void run() { - repository.check(); - } - - void stop() { - repository = null; - builds = null; + public static Pattern compilePattern(String regex) { + return Pattern.compile(regex, Pattern.CASE_INSENSITIVE | Pattern.DOTALL); } // These used to be stored on the object in the constructor. // But because the object is only instantiated once per PR, configuration would go stale. // Some optimization could be done around re-compiling regex/hash sets, but beyond that we still have to re-pull the text. private Pattern retestPhrasePattern() { - return Pattern.compile(trigger.getDescriptor().getRetestPhrase()); + return compilePattern(trigger.getDescriptor().getRetestPhrase()); } - + + /** + * Returns skip build phrases from Jenkins global configuration + * + * @return + */ + public Set getSkipBuildPhrases() { + return new HashSet(Arrays.asList(GhprbTrigger.getDscp().getSkipBuildPhrase().split("[\\r\\n]+"))); + } + + /** + * Checks for skip build phrase in pull request comment. If present it updates shouldRun as false. + * + * @param issue + */ + public String checkSkipBuild(GHIssue issue) { + // check for skip build phrase. + String pullRequestBody = issue.getBody(); + if (StringUtils.isNotBlank(pullRequestBody)) { + pullRequestBody = pullRequestBody.trim(); + Set skipBuildPhrases = getSkipBuildPhrases(); + skipBuildPhrases.remove(""); + + for (String skipBuildPhrase : skipBuildPhrases) { + skipBuildPhrase = skipBuildPhrase.trim(); + Pattern skipBuildPhrasePattern = compilePattern(skipBuildPhrase); + + if (skipBuildPhrasePattern.matcher(pullRequestBody).matches()) { + return skipBuildPhrase; + } + } + } + return null; + } + private Pattern whitelistPhrasePattern() { - return Pattern.compile(trigger.getDescriptor().getWhitelistPhrase()); + return compilePattern(trigger.getDescriptor().getWhitelistPhrase()); } private Pattern oktotestPhrasePattern() { - return Pattern.compile(trigger.getDescriptor().getOkToTestPhrase()); + return compilePattern(trigger.getDescriptor().getOkToTestPhrase()); } - private String triggerPhrase() { - return trigger.getTriggerPhrase(); + private Pattern triggerPhrase() { + return compilePattern(trigger.getTriggerPhrase()); } private HashSet admins() { @@ -176,7 +172,7 @@ public boolean isOktotestPhrase(String comment) { } public boolean isTriggerPhrase(String comment) { - return !triggerPhrase().equals("") && comment != null && comment.contains(triggerPhrase()); + return triggerPhrase().matcher(comment).matches(); } public boolean ifOnlyTriggerPhrase() { @@ -221,11 +217,7 @@ public static String replaceMacros(AbstractBuild build, TaskListener liste String returnString = inputString; if (build != null && inputString != null) { try { - Map messageEnvVars = new HashMap(); - - messageEnvVars.putAll(build.getCharacteristicEnvVars()); - messageEnvVars.putAll(build.getBuildVariables()); - messageEnvVars.putAll(build.getEnvironment(listener)); + Map messageEnvVars = getEnvVars(build, listener); returnString = Util.replaceMacro(inputString, messageEnvVars); @@ -236,6 +228,20 @@ public static String replaceMacros(AbstractBuild build, TaskListener liste return returnString; } + public static Map getEnvVars(AbstractBuild build, TaskListener listener) { + Map messageEnvVars = new HashMap(); + if (build != null) { + messageEnvVars.putAll(build.getCharacteristicEnvVars()); + messageEnvVars.putAll(build.getBuildVariables()); + try { + messageEnvVars.putAll(build.getEnvironment(listener)); + } catch (Exception e) { + logger.log(Level.SEVERE, "Couldn't get Env Variables: ", e); + } + } + return messageEnvVars; + } + public static String replaceMacros(AbstractProject project, String inputString) { String returnString = inputString; @@ -276,7 +282,7 @@ public static Set createSet(String list) { } - public static GhprbCause getCause(AbstractBuild build) { + public static GhprbCause getCause(Run build) { Cause cause = build.getCause(GhprbCause.class); if (cause == null || (!(cause instanceof GhprbCause))) { return null; @@ -284,6 +290,7 @@ public static GhprbCause getCause(AbstractBuild build) { return (GhprbCause) cause; } + public static GhprbTrigger extractTrigger(AbstractBuild build) { return extractTrigger(build.getProject()); } @@ -403,13 +410,10 @@ public static StandardCredentials lookupCredentials(Item context, String credent List credentials; - if (context == null) { - credentials = CredentialsProvider.lookupCredentials(StandardCredentials.class, Jenkins.getInstance(), ACL.SYSTEM, - URIRequirementBuilder.fromUri(uri).build()); - } else { - credentials = CredentialsProvider.lookupCredentials(StandardCredentials.class, context, ACL.SYSTEM, - URIRequirementBuilder.fromUri(uri).build()); - } + logger.log(Level.FINE, "Using null context because of issues not getting all credentias"); + + credentials = CredentialsProvider.lookupCredentials(StandardCredentials.class, (Item) null, ACL.SYSTEM, + URIRequirementBuilder.fromUri(uri).build()); logger.log(Level.FINE, "Found {0} credentials", new Object[]{credentials.size()}); @@ -461,4 +465,53 @@ private static String createCredentials(String serverAPIUrl, StandardCredentials provider.addDomain(domain, credentials); return credentials.getId(); } + + public static T getGlobal(Class clazz) { + DescribableList copyExtensions = new DescribableList(Saveable.NOOP); + + copyExtensions.addAll(GhprbTrigger.DESCRIPTOR.getExtensions()); + + filterList(copyExtensions, InstanceofPredicate.getInstance(clazz)); + + return copyExtensions.get(clazz); + } + + + + @SuppressWarnings({ "unchecked", "rawtypes" }) + public static T getDefaultValue(Object local, Class globalClass, String methodName) { + T toReturn = null; + Object global = getGlobal(globalClass); + if (local == null && global == null) { + return null; + } + try { + + if (local == null) { + return (T) global.getClass().getMethod(methodName).invoke(global); + } else if (global == null) { + return (T) local.getClass().getMethod(methodName).invoke(local); + } + + T localValue = (T) local.getClass().getMethod(methodName).invoke(local); + T globalValue = (T) global.getClass().getMethod(methodName).invoke(global); + + + if (localValue instanceof String) { + if (StringUtils.isEmpty((String) localValue)) { + return globalValue; + } + } else if (localValue instanceof List) { + if (((List) localValue).isEmpty()) { + return globalValue; + } + } + + return localValue; + + } catch (Exception e) { + e.printStackTrace(); + } + return toReturn; + } } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbBuildListener.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbBuildListener.java index 106d2028c..19ef3db68 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbBuildListener.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbBuildListener.java @@ -1,8 +1,13 @@ package org.jenkinsci.plugins.ghprb; -import com.google.common.base.Optional; +import java.io.IOException; + import hudson.Extension; +import hudson.Launcher; import hudson.model.AbstractBuild; +import hudson.model.BuildListener; +import hudson.model.Environment; +import hudson.model.Run; import hudson.model.TaskListener; import hudson.model.listeners.RunListener; @@ -14,21 +19,27 @@ public class GhprbBuildListener extends RunListener> { @Override public void onStarted(AbstractBuild build, TaskListener listener) { - final Optional trigger = findTrigger(build); - if (trigger.isPresent()) { - trigger.get().getBuilds().onStarted(build, listener); + GhprbTrigger trigger = Ghprb.extractTrigger(build); + if (trigger != null && trigger.getBuilds() != null) { + trigger.getBuilds().onStarted(build, listener); } } @Override public void onCompleted(AbstractBuild build, TaskListener listener) { - final Optional trigger = findTrigger(build); - if (trigger.isPresent()) { - trigger.get().getBuilds().onCompleted(build, listener); + GhprbTrigger trigger = Ghprb.extractTrigger(build); + if (trigger != null && trigger.getBuilds() != null) { + trigger.getBuilds().onCompleted(build, listener); } } - private static Optional findTrigger(AbstractBuild build) { - return Optional.fromNullable(Ghprb.extractTrigger(build)); + @Override + public Environment setUpEnvironment(@SuppressWarnings("rawtypes") AbstractBuild build, Launcher launcher, BuildListener listener) throws IOException, InterruptedException, Run.RunnerAbortedException { + GhprbTrigger trigger = Ghprb.extractTrigger(build); + if (trigger != null && trigger.getBuilds() != null) { + trigger.getBuilds().onEnvironmentSetup(build, launcher, listener); + } + + return new hudson.model.Environment(){}; } } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbBuilds.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbBuilds.java index 818553d7c..4b2446af9 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbBuilds.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbBuilds.java @@ -1,12 +1,16 @@ package org.jenkinsci.plugins.ghprb; +import hudson.Launcher; import hudson.Util; import hudson.model.AbstractBuild; +import hudson.model.BuildListener; +import hudson.model.Result; import hudson.model.TaskListener; import hudson.model.queue.QueueTaskFuture; import hudson.plugins.git.util.BuildData; import org.apache.commons.lang.StringUtils; +import org.jenkinsci.plugins.ghprb.extensions.GhprbBuildStep; import org.jenkinsci.plugins.ghprb.extensions.GhprbCommentAppender; import org.jenkinsci.plugins.ghprb.extensions.GhprbCommitStatus; import org.jenkinsci.plugins.ghprb.extensions.GhprbCommitStatusException; @@ -18,9 +22,9 @@ import java.io.IOException; import java.io.PrintStream; +import java.net.URL; import java.util.HashMap; import java.util.Map; -import java.util.concurrent.ConcurrentMap; import java.util.logging.Level; import java.util.logging.Logger; @@ -38,26 +42,47 @@ public GhprbBuilds(GhprbTrigger trigger, GhprbRepository repo) { } public void build(GhprbPullRequest pr, GHUser triggerSender, String commentBody) { + + URL url = null; + GHUser prAuthor = null; + + try { + url = pr.getUrl(); + prAuthor = pr.getPullRequestAuthor(); + } catch (IOException e) { + logger.log(Level.SEVERE, "Unable to get PR author or PR URL", e); + } - GhprbCause cause = new GhprbCause(pr.getHead(), pr.getId(), pr.isMergeable(), pr.getTarget(), pr.getSource(), pr.getAuthorEmail(), pr.getTitle(), pr.getUrl(), triggerSender, commentBody, - pr.getCommitAuthor()); + GhprbCause cause = new GhprbCause(pr.getHead(), + pr.getId(), + pr.isMergeable(), + pr.getTarget(), + pr.getSource(), + pr.getAuthorEmail(), + pr.getTitle(), + url, + triggerSender, + commentBody, + pr.getCommitAuthor(), + prAuthor, + pr.getDescription(), + pr.getAuthorRepoGitUrl()); for (GhprbExtension ext : Ghprb.getJobExtensions(trigger, GhprbCommitStatus.class)) { if (ext instanceof GhprbCommitStatus) { try { - ((GhprbCommitStatus) ext).onBuildTriggered(trigger, pr, repo.getGitHubRepo()); + ((GhprbCommitStatus) ext).onBuildTriggered(trigger.getActualProject(), pr.getHead(), pr.isMergeable(), pr.getId(), repo.getGitHubRepo()); } catch (GhprbCommitStatusException e) { repo.commentOnFailure(null, null, e); } } } - QueueTaskFuture build = trigger.startJob(cause, repo); + QueueTaskFuture build = trigger.scheduleBuild(cause, repo); if (build == null) { logger.log(Level.SEVERE, "Job did not start"); } } - - + public void onStarted(AbstractBuild build, TaskListener listener) { PrintStream logger = listener.getLogger(); GhprbCause c = Ghprb.getCause(build); @@ -66,42 +91,34 @@ public void onStarted(AbstractBuild build, TaskListener listener) { } GhprbTrigger trigger = Ghprb.extractTrigger(build); - - ConcurrentMap pulls = trigger.getDescriptor().getPullRequests(build.getProject().getFullName()); - - GHPullRequest pr = pulls.get(c.getPullID()).getPullRequest(); + GhprbPullRequest pullRequest = trigger.getRepository().getPullRequest(c.getPullID()); + pullRequest.setBuild(build); try { + GHPullRequest pr = pullRequest.getPullRequest(true); int counter = 0; // If the PR is being resolved by GitHub then getMergeable will return null Boolean isMergeable = pr.getMergeable(); - Boolean isMerged = pr.isMerged(); - // Not sure if isMerged can return null, but adding if just in case - if (isMerged == null) { - isMerged = false; - } + boolean isMerged = pr.isMerged(); while (isMergeable == null && !isMerged && counter++ < 60) { Thread.sleep(1000); isMergeable = pr.getMergeable(); isMerged = pr.isMerged(); - if (isMerged == null) { - isMerged = false; - } } - + if (isMerged) { logger.println("PR has already been merged, builds using the merged sha1 will fail!!!"); } else if (isMergeable == null) { logger.println("PR merge status couldn't be retrieved, maybe GitHub hasn't settled yet"); } else if (isMergeable != c.isMerged()) { logger.println("!!! PR mergeability status has changed !!! "); - if (isMergeable) { + if (isMergeable) { logger.println("PR now has NO merge conflicts"); } else if (!isMergeable) { logger.println("PR now has merge conflicts!"); } } - + } catch (Exception e) { logger.print("Unable to query GitHub for status of PullRequest"); e.printStackTrace(logger); @@ -116,7 +133,7 @@ public void onStarted(AbstractBuild build, TaskListener listener) { } } } - + try { String template = trigger.getBuildDescTemplate(); if (StringUtils.isEmpty(template)) { @@ -133,12 +150,16 @@ public void onStarted(AbstractBuild build, TaskListener listener) { } public Map getVariables(GhprbCause c) { - Map vars = new HashMap(); - vars.put("title", c.getTitle()); - vars.put("url", c.getUrl().toString()); - vars.put("pullId", Integer.toString(c.getPullID())); - vars.put("abbrTitle", c.getAbbreviatedTitle()); - return vars; + Map vars = new HashMap(); + vars.put("title", c.getTitle()); + if (c.getUrl() != null) { + vars.put("url", c.getUrl().toString()); + } else { + vars.put("url", ""); + } + vars.put("pullId", Integer.toString(c.getPullID())); + vars.put("abbrTitle", c.getAbbreviatedTitle()); + return vars; } public void onCompleted(AbstractBuild build, TaskListener listener) { @@ -150,15 +171,19 @@ public void onCompleted(AbstractBuild build, TaskListener listener) { // remove the BuildData action that we may have added earlier to avoid // having two of them, and because the one we added isn't correct // @see GhprbTrigger - BuildData fakeOne = null; for (BuildData data : build.getActions(BuildData.class)) { if (data.getLastBuiltRevision() != null && !data.getLastBuiltRevision().getSha1String().equals(c.getCommit())) { - fakeOne = data; + build.getActions().remove(data); break; } } - if (fakeOne != null) { - build.getActions().remove(fakeOne); + + + if (build.getResult() == Result.ABORTED) { + GhprbBuildStep abortAction = build.getAction(GhprbBuildStep.class); + if (abortAction != null) { + return; + } } for (GhprbExtension ext : Ghprb.getJobExtensions(trigger, GhprbCommitStatus.class)) { @@ -176,14 +201,14 @@ public void onCompleted(AbstractBuild build, TaskListener listener) { commentOnBuildResult(build, listener, state, c); // close failed pull request automatically - if (state == GHCommitState.FAILURE && trigger.isAutoCloseFailedPullRequests()) { + if (state == GHCommitState.FAILURE && trigger.getAutoCloseFailedPullRequests()) { closeFailedRequest(listener, c); } } private void closeFailedRequest(TaskListener listener, GhprbCause c) { try { - GHPullRequest pr = repo.getPullRequest(c.getPullID()); + GHPullRequest pr = repo.getActualPullRequest(c.getPullID()); if (pr.getState().equals(GHIssueState.OPEN)) { repo.closePullRequest(c.getPullID()); @@ -209,4 +234,23 @@ private void commentOnBuildResult(AbstractBuild build, TaskListener listen } } + public void onEnvironmentSetup(@SuppressWarnings("rawtypes") AbstractBuild build, Launcher launcher, BuildListener listener) { + GhprbCause c = Ghprb.getCause(build); + if (c == null) { + return; + } + + logger.log(Level.FINE, "Job: " + build.getFullDisplayName() + " Attempting to send GitHub commit status"); + + for (GhprbExtension ext : Ghprb.getJobExtensions(trigger, GhprbCommitStatus.class)) { + if (ext instanceof GhprbCommitStatus) { + try { + ((GhprbCommitStatus) ext).onEnvironmentSetup(build, listener, repo.getGitHubRepo()); + } catch (GhprbCommitStatusException e) { + repo.commentOnFailure(build, listener, e); + } + } + } + } + } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbCause.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbCause.java index ba8cce14c..671d88aa4 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbCause.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbCause.java @@ -23,6 +23,9 @@ public class GhprbCause extends Cause { private final GHUser triggerSender; private final String commentBody; private final GitUser commitAuthor; + private final GHUser pullRequestAuthor; + private final String description; + private final String authorRepoGitUrl; public GhprbCause(String commit, int pullID, @@ -34,7 +37,10 @@ public GhprbCause(String commit, URL url, GHUser triggerSender, String commentBody, - GitUser commitAuthor) { + GitUser commitAuthor, + GHUser pullRequestAuthor, + String description, + String authorRepoGitUrl) { this.commit = commit; this.pullID = pullID; @@ -44,12 +50,15 @@ public GhprbCause(String commit, this.authorEmail = authorEmail; this.title = title; this.url = url; + this.description = description; this.triggerSender = triggerSender; this.commentBody = commentBody; this.commitAuthor = commitAuthor; + this.pullRequestAuthor = pullRequestAuthor; + this.authorRepoGitUrl = authorRepoGitUrl; } - + @Override public String getShortDescription() { return "GitHub pull request #" + pullID + " of commit " + commit + (merged ? ", no merge conflicts." : ", has merge conflicts."); @@ -91,6 +100,10 @@ public String getCommentBody() { return commentBody; } + public GHUser getPullRequestAuthor() { + return pullRequestAuthor; + } + /** * Returns the title of the cause, not null. * @@ -112,5 +125,12 @@ public String getAbbreviatedTitle() { public GitUser getCommitAuthor() { return commitAuthor; } + + public String getDescription() { + return description; + } + public String getAuthorRepoGitUrl() { + return authorRepoGitUrl; + } } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbCrumbExclusion.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbCrumbExclusion.java new file mode 100644 index 000000000..e95e2328b --- /dev/null +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbCrumbExclusion.java @@ -0,0 +1,33 @@ +package org.jenkinsci.plugins.ghprb; + +import hudson.Extension; +import hudson.security.csrf.CrumbExclusion; + +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; + +/** + * Excludes {@link GhprbRootAction} from the CSRF protection. + * @since 1.28 + */ +@Extension +public class GhprbCrumbExclusion extends CrumbExclusion { + + @Override + public boolean process(HttpServletRequest req, HttpServletResponse resp, FilterChain chain) + throws IOException, ServletException { + final String pathInfo = req.getPathInfo(); + if (pathInfo != null && pathInfo.startsWith(getExclusionPath())) { + chain.doFilter(req, resp); + return true; + } + return false; + } + + public String getExclusionPath() { + return "/" + GhprbRootAction.URL + "/"; + } +} \ No newline at end of file diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHub.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHub.java index 625dcf4da..7b22bfd14 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHub.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHub.java @@ -6,7 +6,6 @@ import org.kohsuke.github.GHOrganization; import org.kohsuke.github.GHUser; -import org.kohsuke.github.GitHub; /** * @author janinko @@ -19,14 +18,10 @@ public GhprbGitHub(GhprbTrigger trigger) { this.trigger = trigger; } - public GitHub get() throws IOException { - return trigger.getGitHub(); - } - public boolean isUserMemberOfOrganization(String organisation, GHUser member) { boolean orgHasMember = false; try { - GHOrganization org = get().getOrganization(organisation); + GHOrganization org = trigger.getGitHub().getOrganization(organisation); orgHasMember = org.hasMember(member); logger.log(Level.FINE, "org.hasMember(member)? user:{0} org: {1} == {2}", new Object[] { member.getLogin(), organisation, orgHasMember ? "yes" : "no" }); @@ -40,7 +35,7 @@ public boolean isUserMemberOfOrganization(String organisation, GHUser member) { public String getBotUserLogin() { try { - return get().getMyself().getLogin(); + return trigger.getGitHub().getMyself().getLogin(); } catch (IOException ex) { logger.log(Level.SEVERE, null, ex); return null; diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth.java index 08d0d01e7..b588503a8 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth.java @@ -12,7 +12,12 @@ import java.util.logging.Level; import java.util.logging.Logger; +import javax.crypto.Mac; +import javax.crypto.spec.SecretKeySpec; + import org.kohsuke.github.GHAuthorization; +import org.kohsuke.github.GHCommitState; +import org.kohsuke.github.GHIssue; import org.kohsuke.github.GHMyself; import org.kohsuke.github.GHRepository; import org.kohsuke.github.GitHub; @@ -22,6 +27,7 @@ import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.export.Exported; +import com.cloudbees.plugins.credentials.CredentialsMatcher; import com.cloudbees.plugins.credentials.CredentialsMatchers; import com.cloudbees.plugins.credentials.CredentialsProvider; import com.cloudbees.plugins.credentials.common.IdCredentials; @@ -32,6 +38,7 @@ import com.cloudbees.plugins.credentials.domains.URIRequirementBuilder; import com.google.common.base.Joiner; +import org.apache.commons.codec.binary.Hex; import org.apache.commons.lang.StringUtils; import org.jenkinsci.plugins.plaincredentials.StringCredentials; @@ -50,14 +57,18 @@ public class GhprbGitHubAuth extends AbstractDescribableImpl { public static final DescriptorImpl DESCRIPTOR = new DescriptorImpl(); private final String serverAPIUrl; + private final String jenkinsUrl; private final String credentialsId; private final String id; private final String description; private final String secret; + + private transient GitHub gh; @DataBoundConstructor public GhprbGitHubAuth( String serverAPIUrl, + String jenkinsUrl, String credentialsId, String description, String id, @@ -67,6 +78,7 @@ public GhprbGitHubAuth( serverAPIUrl = "https://api.github.com"; } this.serverAPIUrl = fixEmptyAndTrim(serverAPIUrl); + this.jenkinsUrl = fixEmptyAndTrim(jenkinsUrl); this.credentialsId = fixEmpty(credentialsId); if (StringUtils.isEmpty(id)) { id = UUID.randomUUID().toString(); @@ -82,6 +94,11 @@ public String getServerAPIUrl() { return serverAPIUrl; } + @Exported + public String getJenkinsUrl() { + return jenkinsUrl; + } + @Exported public String getCredentialsId() { return credentialsId; @@ -102,6 +119,39 @@ public String getId() { public String getSecret() { return secret; } + + + public boolean checkSignature(String body, String signature) { + if (StringUtils.isEmpty(secret)) { + return true; + } + + if (signature != null && signature.startsWith("sha1=")) { + String expected = signature.substring(5); + String algorithm = "HmacSHA1"; + try { + SecretKeySpec keySpec = new SecretKeySpec(secret.getBytes(), algorithm); + Mac mac = Mac.getInstance(algorithm); + mac.init(keySpec); + byte[] localSignatureBytes = mac.doFinal(body.getBytes("UTF-8")); + String localSignature = Hex.encodeHexString(localSignatureBytes); + if (! localSignature.equals(expected)) { + logger.log(Level.SEVERE, "Local signature {0} does not match external signature {1}", + new Object[] {localSignature, expected}); + return false; + } + } catch (Exception e) { + logger.log(Level.SEVERE, "Couldn't match both signatures"); + return false; + } + } else { + logger.log(Level.SEVERE, "Request doesn't contain a signature. Check that github has a secret that should be attached to the hook"); + return false; + } + + logger.log(Level.INFO, "Signatures checking OK"); + return true; + } private static GitHubBuilder getBuilder(Item context, String serverAPIUrl, String credentialsId) { GitHubBuilder builder = new GitHubBuilder() @@ -133,21 +183,28 @@ private static GitHubBuilder getBuilder(Item context, String serverAPIUrl, Strin } return builder; } - - public GitHub getConnection(Item context) throws IOException { - GitHub gh = null; + + private void buildConnection(Item context) { GitHubBuilder builder = getBuilder(context, serverAPIUrl, credentialsId); if (builder == null) { logger.log(Level.SEVERE, "Unable to get builder using credentials: {0}", credentialsId); - return null; + return; } try { gh = builder.build(); } catch (IOException e) { logger.log(Level.SEVERE, "Unable to connect using credentials: " + credentialsId, e); } - - return gh; + } + + public GitHub getConnection(Item context) throws IOException { + synchronized (credentialsId) { + if (gh == null) { + buildConnection(context); + } + + return gh; + } } @Override @@ -172,20 +229,30 @@ public String getDisplayName() { * @return list box model. * @throws URISyntaxException */ - public ListBoxModel doFillCredentialsIdItems(@AncestorInPath Item context, @QueryParameter String serverAPIUrl) throws URISyntaxException { + public ListBoxModel doFillCredentialsIdItems(@AncestorInPath Item context, @QueryParameter String serverAPIUrl, @QueryParameter String credentialsId) throws URISyntaxException { List domainRequirements = URIRequirementBuilder.fromUri(serverAPIUrl).build(); + List matchers = new ArrayList(3); + if (!StringUtils.isEmpty(credentialsId)) { + matchers.add(0, CredentialsMatchers.withId(credentialsId)); + } + + matchers.add(CredentialsMatchers.instanceOf(StandardUsernamePasswordCredentials.class)); + matchers.add(CredentialsMatchers.instanceOf(StringCredentials.class)); + + List credentials = CredentialsProvider.lookupCredentials( + StandardCredentials.class, + context, + ACL.SYSTEM, + domainRequirements + ); + return new StandardListBoxModel() - .withEmptySelection() .withMatching( CredentialsMatchers.anyOf( - CredentialsMatchers.instanceOf(StandardUsernamePasswordCredentials.class), - CredentialsMatchers.instanceOf(StringCredentials.class)), - CredentialsProvider.lookupCredentials(StandardCredentials.class, - context, - ACL.SYSTEM, - domainRequirements) - ); + matchers.toArray(new CredentialsMatcher[0])), + credentials + ); } @@ -282,11 +349,77 @@ public FormValidation doTestGithubAccess( } GitHub gh = builder.build(); GHMyself me = gh.getMyself(); - return FormValidation.ok("Connected to " + serverAPIUrl + " as " + me.getName()); + String name = me.getName(); + String email = me.getEmail(); + String login = me.getLogin(); + + String comment = String.format("Connected to %s as %s (%s) login: %s", serverAPIUrl, name, email, login); + return FormValidation.ok(comment); } catch (Exception ex) { return FormValidation.error("Unable to connect to GitHub API: " + ex); } } + + + public FormValidation doTestComment( + @QueryParameter("serverAPIUrl") final String serverAPIUrl, + @QueryParameter("credentialsId") final String credentialsId, + @QueryParameter("repo") final String repoName, + @QueryParameter("issueId") final int issueId, + @QueryParameter("message1") final String comment) { + try { + GitHubBuilder builder = getBuilder(null, serverAPIUrl, credentialsId); + if (builder == null) { + return FormValidation.error("Unable to look up GitHub credentials using ID: " + credentialsId + "!!"); + } + GitHub gh = builder.build(); + GHRepository repo = gh.getRepository(repoName); + GHIssue issue = repo.getIssue(issueId); + issue.comment(comment); + + return FormValidation.ok("Issued comment to issue: " + issue.getHtmlUrl()); + } catch (Exception ex) { + return FormValidation.error("Unable to issue comment: " + ex); + } + } + + public FormValidation doTestUpdateStatus( + @QueryParameter("serverAPIUrl") final String serverAPIUrl, + @QueryParameter("credentialsId") final String credentialsId, + @QueryParameter("repo") final String repoName, + @QueryParameter("sha1") final String sha1, + @QueryParameter("state") final GHCommitState state, + @QueryParameter("url") final String url, + @QueryParameter("message2") final String message, + @QueryParameter("context") final String context) { + try { + GitHubBuilder builder = getBuilder(null, serverAPIUrl, credentialsId); + if (builder == null) { + return FormValidation.error("Unable to look up GitHub credentials using ID: " + credentialsId + "!!"); + } + GitHub gh = builder.build(); + GHRepository repo = gh.getRepository(repoName); + repo.createCommitStatus(sha1, state, url, message, context); + return FormValidation.ok("Updated status of: " + sha1); + } catch (Exception ex) { + return FormValidation.error("Unable to update status: " + ex); + } + } + + public ListBoxModel doFillStateItems(@QueryParameter("state") String state) { + ListBoxModel items = new ListBoxModel(); + for (GHCommitState commitState : GHCommitState.values()) { + + items.add(commitState.toString(), commitState.toString()); + if (state.equals(commitState.toString())) { + items.get(items.size() - 1).selected = true; + } + } + + return items; + } } + + } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java index d5c6cc1d1..c75c8c513 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java @@ -2,7 +2,10 @@ import com.google.common.base.Joiner; +import hudson.model.AbstractBuild; + import org.apache.commons.lang.StringUtils; +import org.kohsuke.github.GHCommitPointer; import org.kohsuke.github.GHIssue; import org.kohsuke.github.GHIssueComment; import org.kohsuke.github.GHPullRequest; @@ -12,17 +15,14 @@ import java.io.IOException; import java.net.URL; -import java.util.Arrays; import java.util.Date; -import java.util.HashSet; import java.util.List; -import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.regex.Pattern; /** - * Maintains state about a Pull Request for a particular Jenkins job. This is what understands the current state of a PR for a particular job. + * Maintains state about a Pull Request for a particular Jenkins job. This is what understands the current state of a PR + * for a particular job. * * @author Honza Brázdil */ @@ -30,128 +30,147 @@ public class GhprbPullRequest { private static final Logger logger = Logger.getLogger(GhprbPullRequest.class.getName()); + @Deprecated + @SuppressWarnings("unused") + private transient GHUser author; + @Deprecated + @SuppressWarnings("unused") + private transient String title; + @Deprecated + @SuppressWarnings("unused") + private transient String reponame; + @Deprecated + @SuppressWarnings("unused") + private transient URL url; + @Deprecated + @SuppressWarnings("unused") + private transient String description; + @Deprecated + @SuppressWarnings("unused") + private transient String target; + @Deprecated + @SuppressWarnings("unused") + private transient String source; + @Deprecated + @SuppressWarnings("unused") + private transient String authorRepoGitUrl; + @Deprecated + @SuppressWarnings("unused") + private transient Boolean changed = true; + + private transient String authorEmail; + private transient Ghprb helper; // will be refreshed each time GhprbRepository.init() is called + private transient GhprbRepository repo; // will be refreshed each time GhprbRepository.init() is called + + private transient GHPullRequest pr; + + private transient GHUser triggerSender; // Only needed for a single build + private transient GitUser commitAuthor; // Only needed for a single build + private transient String commentBody; + + private transient boolean shouldRun = false; // Declares if we should run the build this time. + private transient boolean triggered = false; // Only lets us know if the trigger phrase was used for this run + private transient boolean mergeable = false; // Only works as an easy way to pass the value around for the start of + // this build + private final int id; - private final GHUser author; - private final GHPullRequest pr; - private String title; - private Date updated; + private Date updated; // Needed to track when the PR was updated private String head; - private boolean mergeable; - private String reponame; - private String target; - private String source; - private String authorEmail; - private URL url; + private String base; + private boolean accepted = false; // Needed to see if the PR has been added to the accepted list + private String lastBuildId; - private GHUser triggerSender; - private GitUser commitAuthor; + private void setUpdated(Date lastUpdateTime) { + updated = lastUpdateTime; + } - private boolean shouldRun = false; - private boolean accepted = false; - private boolean triggered = false; + private void setHead(String newHead) { + this.head = StringUtils.isEmpty(newHead) ? head : newHead; + } - private transient Ghprb helper; - private transient GhprbRepository repo; + private void setBase(String newBase) { + this.base = StringUtils.isEmpty(newBase) ? base : newBase; + } - private String commentBody; + private void setAccepted(boolean shouldRun) { + accepted = true; + this.shouldRun = shouldRun; + } + + public GhprbPullRequest(GHPullRequest pr, + Ghprb ghprb, + GhprbRepository repo, + boolean isNew) { - GhprbPullRequest(GHPullRequest pr, Ghprb helper, GhprbRepository repo) { id = pr.getNumber(); - try { - updated = pr.getUpdatedAt(); - } catch (IOException e) { - e.printStackTrace(); - updated = new Date(); - } - head = pr.getHead().getSha(); - title = pr.getTitle(); - author = pr.getUser(); - reponame = repo.getName(); - target = pr.getBase().getRef(); - source = pr.getHead().getRef(); - url = pr.getHtmlUrl(); - this.pr = pr; - obtainAuthorEmail(pr); + setPullRequest(pr); + + this.helper = ghprb; - this.helper = helper; this.repo = repo; - if (helper.isWhitelisted(author)) { - accepted = true; - shouldRun = true; - } else if (!helper.suppressTestingRequest()) { - logger.log(Level.INFO, "Author of #{0} {1} on {2} not in whitelist!", new Object[] { id, author.getLogin(), reponame }); - repo.addComment(id, GhprbTrigger.getDscp().getRequestForTestingPhrase()); + GHUser author = pr.getUser(); + String reponame = repo.getName(); + + try { + if (ghprb.isWhitelisted(getPullRequestAuthor())) { + setAccepted(true); + } else if (!helper.suppressTestingRequest()) { + logger.log(Level.INFO, + "Author of #{0} {1} on {2} not in whitelist!", + new Object[] { id, author.getLogin(), reponame }); + repo.addComment(id, GhprbTrigger.getDscp().getRequestForTestingPhrase()); + } + } catch (IOException e) { + logger.log(Level.SEVERE, "Unable to get pull request author", e); } - logger.log(Level.INFO, "Created Pull Request #{0} on {1} by {2} ({3}) updated at: {4} SHA: {5}", - new Object[] { id, reponame, author.getLogin(), authorEmail, updated, head } - ); + logger.log(Level.INFO, + "Created Pull Request #{0} on {1} by {2} ({3}) updated at: {4} SHA: {5}", + new Object[] { id, reponame, author.getLogin(), getAuthorEmail(), updated, this.head }); } - public void init(Ghprb helper, GhprbRepository repo) { + public void init(Ghprb helper, + GhprbRepository repo) { this.helper = helper; this.repo = repo; - if (reponame == null) { - reponame = repo.getName(); // If this instance was created before v1.8, it can be null. - } } /** - * Returns skip build phrases from Jenkins global configuration - * - * @return - */ - public Set getSkipBuildPhrases() { - return new HashSet(Arrays.asList(GhprbTrigger.getDscp().getSkipBuildPhrase().split("[\\r\\n]+"))); - } - - /** - * Checks for skip build phrase in pull request comment. If present it updates shouldRun as false. - * - * @param issue - */ - private void checkSkipBuild(GHIssue issue) { - // check for skip build phrase. - String pullRequestBody = issue.getBody(); - if (StringUtils.isNotBlank(pullRequestBody)) { - pullRequestBody = pullRequestBody.trim(); - Set skipBuildPhrases = getSkipBuildPhrases(); - skipBuildPhrases.remove(""); - - for (String skipBuildPhrase : skipBuildPhrases) { - skipBuildPhrase = skipBuildPhrase.trim(); - Pattern skipBuildPhrasePattern = Pattern.compile(skipBuildPhrase, Pattern.CASE_INSENSITIVE); - if (skipBuildPhrasePattern.matcher(pullRequestBody).matches()) { - logger.log(Level.INFO, "Pull request commented with {0} skipBuildPhrase. Hence skipping the build.", skipBuildPhrase); - shouldRun = false; - break; - } - } - } - } - - /** - * Checks this Pull Request representation against a GitHub version of the Pull Request, and triggers a build if necessary. + * Checks this Pull Request representation against a GitHub version of the Pull Request, and triggers a build if + * necessary. * - * @param pr + * @param ghpr */ - public void check(GHPullRequest pr) { + public void check(GHPullRequest ghpr) { + setPullRequest(ghpr); if (helper.isProjectDisabled()) { logger.log(Level.FINE, "Project is disabled, ignoring pull request"); return; } - if (target == null) { - target = pr.getBase().getRef(); // If this instance was created before target was introduced (before v1.8), it can be null. - } - if (source == null) { - source = pr.getHead().getRef(); // If this instance was created before target was introduced (before v1.8), it can be null. + + try { + getPullRequest(false); + } catch (IOException e) { + logger.log(Level.SEVERE, "Unable to get the latest copy of the PR from github", e); + return; } - updatePR(pr, author); - - checkSkipBuild(pr); - tryBuild(pr); + updatePR(pr, pr.getUser()); + + tryBuild(); + } + + private void checkSkipBuild() { + synchronized (this) { + String skipBuildPhrase = helper.checkSkipBuild(this.pr); + if (!StringUtils.isEmpty(skipBuildPhrase)) { + logger.log(Level.INFO, + "Pull request commented with {0} skipBuildPhrase. Hence skipping the build.", + skipBuildPhrase); + shouldRun = false; + } + } } public void check(GHIssueComment comment) { @@ -159,46 +178,57 @@ public void check(GHIssueComment comment) { logger.log(Level.FINE, "Project is disabled, ignoring comment"); return; } - try { - checkComment(comment); - } catch (IOException ex) { - logger.log(Level.SEVERE, "Couldn't check comment #" + comment.getId(), ex); - return; - } - - GHPullRequest pr = null; - try { - pr = repo.getPullRequest(id); - updatePR(pr, comment.getUser()); - } catch (IOException e) { - logger.log(Level.SEVERE, "Couldn't get GHPullRequest for checking mergeable state"); - } - checkSkipBuild(comment.getParent()); - tryBuild(pr); - } - - private void updatePR(GHPullRequest pr, GHUser author) { - Date lastUpdateTime = updated; - if (pr != null && isUpdated(pr)) { - logger.log(Level.INFO, "Pull request #{0} was updated on {1} at {2} by {3}", new Object[] { id, reponame, updated, author }); - - // the author of the PR could have been whitelisted since its creation - if (!accepted && helper.isWhitelisted(pr.getUser())) { - logger.log(Level.INFO, "Pull request #{0}'s author has been whitelisted", new Object[]{id}); - accepted = true; + synchronized (this) { + try { + checkComment(comment); + } catch (IOException ex) { + logger.log(Level.SEVERE, "Couldn't check comment #" + comment.getId(), ex); + return; } - - // the title could have been updated since the original PR was opened - title = pr.getTitle(); - int commentsChecked = checkComments(pr, lastUpdateTime); - boolean newCommit = checkCommit(pr.getHead().getSha()); - - if (!newCommit && commentsChecked == 0) { - logger.log(Level.INFO, "Pull request #{0} was updated on repo {1} but there aren''t any new comments nor commits; " - + "that may mean that commit status was updated.", - new Object[] { id, reponame } - ); + + try { + GHUser user = null; + try { + user = comment.getUser(); + } catch (IOException e) { + logger.log(Level.SEVERE, "Couldn't get the user that made the comment", e); + } + updatePR(getPullRequest(true), user); + } catch (IOException ex) { + logger.log(Level.SEVERE, "Unable to get a new copy of the pull request!"); + } + + tryBuild(); + } + } + + private void updatePR(GHPullRequest pr, + GHUser user) { + setPullRequest(pr); + + synchronized (this) { + Date lastUpdateTime = updated; + if (isUpdated(pr)) { + logger.log(Level.INFO, + "Pull request #{0} was updated on {1} at {2} by {3}", + new Object[] { id, repo.getName(), updated, user }); + + // the author of the PR could have been whitelisted since its creation + if (!accepted && helper.isWhitelisted(pr.getUser())) { + logger.log(Level.INFO, "Pull request #{0}'s author has been whitelisted", new Object[] { id }); + setAccepted(false); + } + + int commentsChecked = checkComments(pr, lastUpdateTime); + boolean newCommit = checkCommit(pr); + + if (!newCommit && commentsChecked == 0) { + logger.log(Level.INFO, + "Pull request #{0} was updated on repo {1} but there aren''t any new comments nor commits; " + + "that may mean that commit status was updated.", + new Object[] { id, repo.getName() }); + } } } } @@ -209,92 +239,111 @@ public boolean isWhiteListedTargetBranch() { // no branches in white list means we should test all return true; } + + String target = getTarget(); for (GhprbBranch b : branches) { if (b.matches(target)) { // the target branch is in the whitelist! return true; } } - logger.log(Level.FINEST, "PR #{0} target branch: {1} isn''t in our whitelist of target branches: {2}", - new Object[] { id, target, Joiner.on(',').skipNulls().join(branches) } - ); + logger.log(Level.FINEST, + "PR #{0} target branch: {1} isn''t in our whitelist of target branches: {2}", + new Object[] { id, target, Joiner.on(',').skipNulls().join(branches) }); return false; } private boolean isUpdated(GHPullRequest pr) { - if (pr == null) { - return false; - } - Date lastUpdated = new Date(); - boolean ret = false; - try { - lastUpdated = pr.getUpdatedAt(); - ret = updated.compareTo(lastUpdated) < 0; - updated = lastUpdated; - } catch (Exception e) { - logger.log(Level.WARNING, "Unable to update last updated date", e); + synchronized (this) { + Date lastUpdated = new Date(); + boolean ret = false; + try { + lastUpdated = pr.getUpdatedAt(); + ret = updated.compareTo(lastUpdated) < 0; + setUpdated(lastUpdated); + } catch (Exception e) { + logger.log(Level.WARNING, "Unable to update last updated date", e); + } + GHCommitPointer pointer = pr.getHead(); + String pointerSha = pointer.getSha(); + ret |= !pointerSha.equals(head); + + pointer = pr.getBase(); + pointerSha = pointer.getSha(); + ret |= !pointerSha.equals(base); + + return ret; } - ret = ret || !pr.getHead().getSha().equals(head); - return ret; } - private void tryBuild(GHPullRequest pr) { - if (helper.isProjectDisabled()) { - logger.log(Level.FINEST, "Project is disabled, not trying to build"); - shouldRun = false; - triggered = false; - } - if (helper.ifOnlyTriggerPhrase() && !triggered) { - logger.log(Level.FINEST, "Trigger only phrase but we are not triggered"); - shouldRun = false; - } - if (!isWhiteListedTargetBranch()) { - return; - } - if (shouldRun) { - logger.log(Level.FINEST, "Running the build"); + private void tryBuild() { + synchronized (this) { + checkSkipBuild(); + if (helper.isProjectDisabled()) { + logger.log(Level.FINEST, "Project is disabled, not trying to build"); + shouldRun = false; + triggered = false; + } + if (helper.ifOnlyTriggerPhrase() && !triggered) { + logger.log(Level.FINEST, "Trigger only phrase but we are not triggered"); + shouldRun = false; + } + triggered = false; // Once we have decided that we are triggered then the flag should be set to false. - if (authorEmail == null) { - // If this instance was create before authorEmail was introduced (before v1.10), it can be null. - obtainAuthorEmail(pr); - logger.log(Level.FINEST, "Author email was not set, trying to set it to {0}", authorEmail); + if (!isWhiteListedTargetBranch()) { + logger.log(Level.FINEST, "Branch is not whitelisted, skipping the build"); + return; } + if (shouldRun) { + shouldRun = false; // Change the shouldRun flag as soon as we decide to build. + logger.log(Level.FINEST, "Running the build"); - if (pr != null) { - logger.log(Level.FINEST, "PR is not null, checking if mergable"); - checkMergeable(pr); - try { - for (GHPullRequestCommitDetail commitDetails : pr.listCommits()) { - if (commitDetails.getSha().equals(getHead())) { - commitAuthor = commitDetails.getCommit().getCommitter(); - break; + if (pr != null) { + logger.log(Level.FINEST, "PR is not null, checking if mergable"); + checkMergeable(); + try { + for (GHPullRequestCommitDetail commitDetails : pr.listCommits()) { + if (commitDetails.getSha().equals(getHead())) { + commitAuthor = commitDetails.getCommit().getCommitter(); + break; + } } + } catch (Exception ex) { + logger.log(Level.INFO, "Unable to get PR commits: ", ex); } - } catch (Exception ex) { - logger.log(Level.INFO, "Unable to get PR commits: ", ex); + } + logger.log(Level.FINEST, "Running build..."); + build(); } - - logger.log(Level.FINEST, "Running build..."); - build(); - - shouldRun = false; - triggered = false; } } private void build() { - helper.getBuilds().build(this, triggerSender, commentBody); + GhprbBuilds builder = helper.getBuilds(); + builder.build(this, triggerSender, commentBody); } // returns false if no new commit - private boolean checkCommit(String sha) { - if (head.equals(sha)) { + private boolean checkCommit(GHPullRequest pr) { + GHCommitPointer head = pr.getHead(); + GHCommitPointer base = pr.getBase(); + + String headSha = head.getSha(); + String baseSha = base.getSha(); + + if (StringUtils.equals(headSha, this.head) && StringUtils.equals(baseSha, this.base)) { return false; } - logger.log(Level.FINE, "New commit. Sha: {0} => {1}", new Object[] { head, sha }); - head = sha; + + logger.log(Level.FINE, + "New commit. Sha: Head[{0} => {1}] Base[{2} => {3}]", + new Object[] { this.head, headSha, this.base, baseSha }); + + setHead(headSha); + setBase(baseSha); + if (accepted) { shouldRun = true; } @@ -305,6 +354,8 @@ private void checkComment(GHIssueComment comment) throws IOException { GHUser sender = comment.getUser(); String body = comment.getBody(); + logger.log(Level.FINEST, "[{0}] Added comment: {1}", new Object[] { sender.getName(), body }); + // Disabled until more advanced configs get set up // ignore comments from bot user, this fixes an issue where the bot would auto-whitelist // a user or trigger a build when the 'request for testing' phrase contains the @@ -315,16 +366,16 @@ private void checkComment(GHIssueComment comment) throws IOException { // } if (helper.isWhitelistPhrase(body) && helper.isAdmin(sender)) { // add to whitelist + GHIssue parent = comment.getParent(); + GHUser author = parent.getUser(); if (!helper.isWhitelisted(author)) { logger.log(Level.FINEST, "Author {0} not whitelisted, adding to whitelist.", author); helper.addWhitelist(author.getLogin()); } - accepted = true; - shouldRun = true; + setAccepted(true); } else if (helper.isOktotestPhrase(body) && helper.isAdmin(sender)) { // ok to test logger.log(Level.FINEST, "Admin {0} gave OK to test", sender); - accepted = true; - shouldRun = true; + setAccepted(true); } else if (helper.isRetestPhrase(body)) { // test this please logger.log(Level.FINEST, "Retest phrase"); if (helper.isAdmin(sender)) { @@ -353,11 +404,15 @@ private void checkComment(GHIssueComment comment) throws IOException { } } - private int checkComments(GHPullRequest pr, Date lastUpdatedTime) { + private int checkComments(GHPullRequest ghpr, + Date lastUpdatedTime) { int count = 0; + logger.log(Level.FINEST, "Checking for comments after: {0}", lastUpdatedTime); try { - for (GHIssueComment comment : pr.getComments()) { + for (GHIssueComment comment : ghpr.getComments()) { + logger.log(Level.FINEST, "Comment was made at: {0}", comment.getUpdatedAt()); if (lastUpdatedTime.compareTo(comment.getUpdatedAt()) < 0) { + logger.log(Level.FINEST, "Comment was made after last update time, {0}", comment.getBody()); count++; try { checkComment(comment); @@ -372,7 +427,13 @@ private int checkComments(GHPullRequest pr, Date lastUpdatedTime) { return count; } - private void checkMergeable(GHPullRequest pr) { + public boolean checkMergeable() { + try { + getPullRequest(false); + } catch (IOException e) { + logger.log(Level.SEVERE, "Unable to get a copy of the PR from github", e); + return mergeable; + } try { int r = 5; Boolean isMergeable = pr.getMergeable(); @@ -383,21 +444,12 @@ private void checkMergeable(GHPullRequest pr) { break; } isMergeable = pr.getMergeable(); - pr = repo.getPullRequest(id); } mergeable = isMergeable != null && isMergeable; } catch (IOException e) { - mergeable = false; logger.log(Level.SEVERE, "Couldn't obtain mergeable status.", e); } - } - - private void obtainAuthorEmail(GHPullRequest pr) { - try { - authorEmail = pr.getUser().getEmail(); - } catch (Exception e) { - logger.log(Level.WARNING, "Couldn't obtain author email.", e); - } + return mergeable; } @Override @@ -424,40 +476,161 @@ public String getHead() { return head; } + public String getAuthorRepoGitUrl() { + GHCommitPointer prHead = pr.getHead(); + String authorRepoGitUrl = ""; + + if (prHead != null && prHead.getRepository() != null) { + authorRepoGitUrl = prHead.getRepository().gitHttpTransportUrl(); + } + return authorRepoGitUrl; + } + public boolean isMergeable() { return mergeable; } + /** + * Base and Ref are part of the PullRequest object + * + * @return + */ public String getTarget() { - return target; + try { + return getPullRequest(false).getBase().getRef(); + } catch (IOException e) { + return "UNKNOWN"; + } } + /** + * Head and Ref are part of the PullRequest object + * + * @return + */ public String getSource() { - return source; - } - - public String getAuthorEmail() { - return authorEmail; + try { + return getPullRequest(false).getHead().getRef(); + } catch (IOException e) { + return "UNKNOWN"; + } } + /** + * Title is part of the PullRequest object + * + * @return + */ public String getTitle() { - return title; + try { + return getPullRequest(false).getTitle(); + } catch (IOException e) { + return "UNKNOWN"; + } } /** * Returns the URL to the Github Pull Request. + * This URL is part of the pull request object * * @return the Github Pull Request URL */ - public URL getUrl() { - return url; + public URL getUrl() throws IOException { + return getPullRequest(false).getHtmlUrl(); + } + + /** + * The description body is part of the PullRequest object + * + * @return + */ + public String getDescription() { + try { + return getPullRequest(false).getBody(); + } catch (IOException e) { + return "UNKNOWN"; + } } public GitUser getCommitAuthor() { return commitAuthor; } - public GHPullRequest getPullRequest() { + /** + * Author is part of the PullRequest Object + * + * @return + * @throws IOException + */ + public GHUser getPullRequestAuthor() throws IOException { + return getPullRequest(false).getUser(); + } + + /** + * Get the PullRequest object for this PR + * + * @param force - forces the code to go get the PullRequest from GitHub now + * @return + * @throws IOException + */ + public GHPullRequest getPullRequest(boolean force) throws IOException { + if (this.pr == null || force) { + setPullRequest(repo.getActualPullRequest(this.id)); + } return pr; } + + private void setPullRequest(GHPullRequest pr) { + if (pr == null) { + return; + } + synchronized (this) { + this.pr = pr; + + try { + if (updated == null) { + setUpdated(pr.getCreatedAt()); + } + } catch (IOException e) { + logger.log(Level.WARNING, "Unable to get date for new PR", e); + setUpdated(new Date()); + } + + if (StringUtils.isEmpty(this.head)) { + GHCommitPointer prHead = pr.getHead(); + setHead(prHead.getSha()); + } + + if (StringUtils.isEmpty(this.base)) { + GHCommitPointer prBase = pr.getBase(); + setBase(prBase.getSha()); + } + } + } + + /** + * Email address is collected from GitHub as extra information, so lets cache it. + * + * @return + */ + public String getAuthorEmail() { + if (StringUtils.isEmpty(authorEmail)) { + try { + GHUser user = getPullRequestAuthor(); + authorEmail = user.getEmail(); + } catch (IOException e) { + logger.log(Level.SEVERE, "Unable to fetch author info for " + id); + } + } + authorEmail = StringUtils.isEmpty(authorEmail) ? "" : authorEmail; + return authorEmail; + } + + public void setBuild(AbstractBuild build) { + lastBuildId = build.getId(); + } + + public String getLastBuildId() { + return lastBuildId; + } } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge.java index 6a76b71ed..2f920afc3 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge.java @@ -2,13 +2,18 @@ import java.io.IOException; import java.io.PrintStream; -import java.util.concurrent.ConcurrentMap; +import java.lang.reflect.Field; +import java.lang.reflect.Method; -import org.kohsuke.github.GHBranch; -import org.kohsuke.github.GHPullRequestCommitDetail.Commit; +import org.kohsuke.github.GHIssue; import org.kohsuke.github.GHPullRequest; import org.kohsuke.github.GHPullRequestCommitDetail; +import org.kohsuke.github.GHPullRequestCommitDetail.Commit; +import org.kohsuke.github.GHRef; +import org.kohsuke.github.GHRepository; import org.kohsuke.github.GHUser; +import org.kohsuke.github.GitHub; +import org.kohsuke.github.GitUser; import org.kohsuke.stapler.AncestorInPath; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.QueryParameter; @@ -18,11 +23,10 @@ import hudson.Extension; import hudson.FilePath; import hudson.Launcher; -import hudson.model.BuildListener; -import hudson.model.Cause; -import hudson.model.Result; import hudson.model.AbstractBuild; import hudson.model.AbstractProject; +import hudson.model.BuildListener; +import hudson.model.Result; import hudson.tasks.BuildStepDescriptor; import hudson.tasks.BuildStepMonitor; import hudson.tasks.Publisher; @@ -31,46 +35,52 @@ public class GhprbPullRequestMerge extends Recorder { - private PrintStream logger; + private transient PrintStream logger; - private final boolean onlyAdminsMerge; - private final boolean disallowOwnCode; - private boolean onlyTriggerPhrase; - private String mergeComment; + private final Boolean onlyAdminsMerge; + private final Boolean disallowOwnCode; + private final String mergeComment; + private final Boolean failOnNonMerge; + private final Boolean deleteOnMerge; @DataBoundConstructor - public GhprbPullRequestMerge(String mergeComment, boolean onlyTriggerPhrase, boolean onlyAdminsMerge, boolean disallowOwnCode) { + public GhprbPullRequestMerge(String mergeComment, boolean onlyAdminsMerge, boolean disallowOwnCode, boolean failOnNonMerge, boolean deleteOnMerge) { this.mergeComment = mergeComment; - this.onlyTriggerPhrase = onlyTriggerPhrase; this.onlyAdminsMerge = onlyAdminsMerge; this.disallowOwnCode = disallowOwnCode; + this.failOnNonMerge = failOnNonMerge; + this.deleteOnMerge = deleteOnMerge; } public String getMergeComment() { return mergeComment; } - public boolean isOnlyTriggerPhrase() { - return onlyTriggerPhrase; + public boolean getOnlyAdminsMerge() { + return onlyAdminsMerge == null ? false : onlyAdminsMerge; } - public boolean isOnlyAdminsMerge() { - return onlyAdminsMerge; + public boolean getDisallowOwnCode() { + return disallowOwnCode == null ? false : disallowOwnCode; } - public boolean isDisallowOwnCode() { - return disallowOwnCode; + public boolean getFailOnNonMerge() { + return failOnNonMerge == null ? false : failOnNonMerge; + } + + public boolean getDeleteOnMerge() { + return deleteOnMerge == null ? false : deleteOnMerge; } public BuildStepMonitor getRequiredMonitorService() { return BuildStepMonitor.BUILD; } - private GhprbTrigger trigger; - private Ghprb helper; - private GhprbCause cause; - private GHPullRequest pr; + private transient GhprbTrigger trigger; + private transient Ghprb helper; + private transient GhprbCause cause; + private transient GHPullRequest pr; @VisibleForTesting void setHelper(Ghprb helper) { @@ -90,112 +100,147 @@ public boolean perform(AbstractBuild build, Launcher launcher, final Build if (trigger == null) return false; - cause = getCause(build); + cause = Ghprb.getCause(build); if (cause == null) { return true; } - ConcurrentMap pulls = trigger.getDescriptor().getPullRequests(project.getFullName()); - - pr = pulls.get(cause.getPullID()).getPullRequest(); - - if (pr == null) { - logger.println("Pull request is null for ID: " + cause.getPullID()); - logger.println("" + pulls.toString()); - return false; - } - - Boolean isMergeable = cause.isMerged(); + pr = trigger.getRepository().getActualPullRequest(cause.getPullID()); if (helper == null) { - helper = new Ghprb(project, trigger, pulls); - helper.init(); - } - - if (isMergeable == null || !isMergeable) { - logger.println("Pull request cannot be automerged."); - commentOnRequest("Pull request is not mergeable."); - listener.finished(Result.FAILURE); - return false; + helper = new Ghprb(trigger); } GHUser triggerSender = cause.getTriggerSender(); // ignore comments from bot user, this fixes an issue where the bot would auto-merge // a PR when the 'request for testing' phrase contains the PR merge trigger phrase and - // the bot is a member of a whitelisted organisation + // the bot is a member of a whitelisted organization if (helper.isBotUser(triggerSender)) { logger.println("Comment from bot user " + triggerSender.getLogin() + " ignored."); return false; } - boolean merge = true; + boolean intendToMerge = false; + boolean canMerge = true; String commentBody = cause.getCommentBody(); - if (isOnlyAdminsMerge() && (triggerSender == null || !helper.isAdmin(triggerSender) )) { - merge = false; - logger.println("Only admins can merge this pull request, " + triggerSender.getLogin() + " is not an admin."); - commentOnRequest(String.format("Code not merged because %s is not in the Admin list.", triggerSender.getName())); + // If merge can only be triggered by a comment and there is a + // comment + if (commentBody == null || !helper.isTriggerPhrase(commentBody)) { + logger.println("The comment does not contain the required trigger phrase."); + } else { + intendToMerge = true; } - if (isOnlyTriggerPhrase() && (commentBody == null || !helper.isTriggerPhrase(cause.getCommentBody()) )) { - merge = false; - logger.println("The comment does not contain the required trigger phrase."); + // If there is no intention to merge there is no point checking + if (intendToMerge && getOnlyAdminsMerge() && (triggerSender == null || !helper.isAdmin(triggerSender))) { + canMerge = false; + logger.println("Only admins can merge this pull request, " + (triggerSender != null ? triggerSender.getLogin() + " is not an admin" : " and build was triggered via automation") + "."); + if (triggerSender != null) { + commentOnRequest(String.format("Code not merged because @%s (%s) is not in the Admin list.", triggerSender.getLogin(), triggerSender.getName())); + } + } - commentOnRequest(String.format("Please comment with '%s' to automerge this request", trigger.getTriggerPhrase())); + // If there is no intention to merge there is no point checking + if (intendToMerge && getDisallowOwnCode() && (triggerSender == null || isOwnCode(pr, triggerSender))) { + canMerge = false; + if (triggerSender != null) { + logger.println("The commentor is also one of the contributors."); + commentOnRequest(String.format("Code not merged because @%s (%s) has committed code in the request.", triggerSender.getLogin(), triggerSender.getName())); + } } - if (isDisallowOwnCode() && (triggerSender == null || isOwnCode(pr, triggerSender) )) { - merge = false; - logger.println("The commentor is also one of the contributors."); - commentOnRequest(String.format("Code not merged because %s has committed code in the request.", triggerSender.getName())); + Boolean isMergeable = cause.isMerged(); + + // The build should not fail if no merge is expected + if (intendToMerge && canMerge && (isMergeable == null || !isMergeable)) { + logger.println("Pull request cannot be automerged."); + commentOnRequest("Pull request is not mergeable."); + listener.finished(Result.FAILURE); + return false; } - if (merge) { + if (intendToMerge && canMerge) { logger.println("Merging the pull request"); + try { + Field ghRootField = GHIssue.class.getDeclaredField("root"); + ghRootField.setAccessible(true); + Object ghRoot = ghRootField.get(pr); + Method anonMethod = GitHub.class.getMethod("isAnonymous"); + anonMethod.setAccessible(true); + Boolean isAnonymous = (Boolean) (anonMethod.invoke(ghRoot)); + logger.println("Merging PR[" + pr + "] is anonymous: " + isAnonymous); + } catch (Exception e) { + e.printStackTrace(logger); + } pr.merge(getMergeComment()); logger.println("Pull request successfully merged"); - // deleteBranch(); //TODO: Update so it also deletes the branch being pulled from. probably make it an option. + deleteBranch(build, launcher, listener); } - if (merge) { - listener.finished(Result.SUCCESS); - } else { + // We should only fail the build if there is an intent to merge + if (intendToMerge && !canMerge && getFailOnNonMerge()) { listener.finished(Result.FAILURE); + } else { + listener.finished(Result.SUCCESS); } - return merge; + return canMerge; } - private void deleteBranch() { + private void deleteBranch(AbstractBuild build, Launcher launcher, final BuildListener listener) { + if (!getDeleteOnMerge()) { + return; + } String branchName = pr.getHead().getRef(); try { - GHBranch branch = pr.getRepository().getBranches().get(branchName); + GHRepository repo = pr.getRepository(); + GHRef ref = repo.getRef("heads/" + branchName); + ref.delete(); + listener.getLogger().println("Deleted branch " + branchName); } catch (IOException e) { - // TODO Auto-generated catch block - e.printStackTrace(); + listener.getLogger().println("Unable to delete branch " + branchName); + e.printStackTrace(listener.getLogger()); } } private void commentOnRequest(String comment) { try { - helper.getRepository().addComment(pr.getNumber(), comment); + trigger.getRepository().addComment(pr.getNumber(), comment); } catch (Exception e) { logger.println("Failed to add comment"); e.printStackTrace(logger); } } - private boolean isOwnCode(GHPullRequest pr, GHUser committer) { + private boolean isOwnCode(GHPullRequest pr, GHUser commentor) { try { - String commentorName = committer.getName(); + String commentorName = commentor.getName(); + String commentorEmail = commentor.getEmail(); + String commentorLogin = commentor.getLogin(); + + GHUser prUser = pr.getUser(); + if (prUser.getLogin().equals(commentorLogin)) { + logger.println(commentorName + " (" + commentorLogin + ") has submitted the PR[" + pr.getNumber() + pr.getNumber() + "] that is to be merged"); + return true; + } + for (GHPullRequestCommitDetail detail : pr.listCommits()) { Commit commit = detail.getCommit(); - String committerName = commit.getCommitter().getName(); + GitUser committer = commit.getCommitter(); + String committerName = committer.getName(); + String committerEmail = committer.getEmail(); + + boolean isSame = false; + + isSame |= commentorName != null && commentorName.equals(committerName); + isSame |= commentorEmail != null && commentorEmail.equals(committerEmail); - if (committerName.equalsIgnoreCase(commentorName)) { - return true; + if (isSame) { + logger.println(commentorName + " (" + commentorEmail + ") has commits in PR[" + pr.getNumber() + "] that is to be merged"); + return isSame; } } } catch (IOException e) { @@ -205,13 +250,6 @@ private boolean isOwnCode(GHPullRequest pr, GHUser committer) { return false; } - private GhprbCause getCause(AbstractBuild build) { - Cause cause = build.getCause(GhprbCause.class); - if (cause == null || (!(cause instanceof GhprbCause))) - return null; - return (GhprbCause) cause; - } - @Extension(ordinal = -1) public static class DescriptorImpl extends BuildStepDescriptor { @Override @@ -220,7 +258,7 @@ public String getDisplayName() { } @Override - public boolean isApplicable(Class jobType) { + public boolean isApplicable(@SuppressWarnings("rawtypes") Class jobType) { return true; } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRepository.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRepository.java index 38239fd6b..e83072247 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRepository.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRepository.java @@ -1,137 +1,185 @@ package org.jenkinsci.plugins.ghprb; -import com.google.common.annotations.VisibleForTesting; - +import hudson.BulkChange; +import hudson.XmlFile; import hudson.model.AbstractBuild; +import hudson.model.AbstractProject; +import hudson.model.Items; +import hudson.model.Saveable; import hudson.model.TaskListener; +import hudson.model.listeners.SaveableListener; import jenkins.model.Jenkins; import org.jenkinsci.plugins.ghprb.extensions.GhprbCommentAppender; import org.jenkinsci.plugins.ghprb.extensions.GhprbCommitStatusException; import org.jenkinsci.plugins.ghprb.extensions.GhprbExtension; import org.jenkinsci.plugins.ghprb.extensions.comments.GhprbBuildStatus; -import org.kohsuke.github.*; +import org.kohsuke.github.GHCommitState; +import org.kohsuke.github.GHEvent; import org.kohsuke.github.GHEventPayload.IssueComment; import org.kohsuke.github.GHEventPayload.PullRequest; +import org.kohsuke.github.GHHook; +import org.kohsuke.github.GHIssueState; +import org.kohsuke.github.GHPullRequest; +import org.kohsuke.github.GHRepository; +import org.kohsuke.github.GitHub; +import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; import java.io.PrintStream; +import java.io.UnsupportedEncodingException; import java.net.URL; -import java.util.*; -import java.util.concurrent.ConcurrentMap; +import java.net.URLEncoder; +import java.util.EnumSet; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.logging.Level; import java.util.logging.Logger; /** * @author Honza Brázdil */ -public class GhprbRepository { +public class GhprbRepository implements Saveable{ - private static final Logger logger = Logger.getLogger(GhprbRepository.class.getName()); - private static final EnumSet HOOK_EVENTS = EnumSet.of(GHEvent.ISSUE_COMMENT, GHEvent.PULL_REQUEST); + private static final transient Logger logger = Logger.getLogger(GhprbRepository.class.getName()); + private static final transient EnumSet HOOK_EVENTS = EnumSet.of(GHEvent.ISSUE_COMMENT, GHEvent.PULL_REQUEST); private final String reponame; - private final ConcurrentMap pulls; + private final Map pullRequests; - private GHRepository ghRepository; - private Ghprb helper; + private transient GHRepository ghRepository; + private transient GhprbTrigger trigger; - public GhprbRepository(String user, String repository, Ghprb helper, ConcurrentMap pulls) { - this.reponame = user + "/" + repository; - this.helper = helper; - this.pulls = pulls; + public GhprbRepository(String reponame, GhprbTrigger trigger) { + this.pullRequests = new ConcurrentHashMap(); + this.reponame = reponame; + this.trigger = trigger; } - + + public void addPullRequests(Map prs) { + pullRequests.putAll(prs); + } + public void init() { - for (GhprbPullRequest pull : pulls.values()) { - pull.init(helper, this); - } // make the initial check call to populate our data structures initGhRepository(); + + for (Entry next : pullRequests.entrySet()) { + GhprbPullRequest pull = next.getValue(); + pull.init(trigger.getHelper(), this); + } } private boolean initGhRepository() { + if (ghRepository != null) { + return true; + } + GitHub gitHub = null; + + try { + gitHub = trigger.getGitHub(); + } catch (IOException ex) { + logger.log(Level.SEVERE, "Error while accessing rate limit API", ex); + return false; + } + + if (gitHub == null) { + logger.log(Level.SEVERE, "No connection returned to GitHub server!"); + return false; + } + try { - GhprbGitHub repo = helper.getGitHub(); - if (repo == null) { - return false; - } - gitHub = repo.get(); - if (gitHub == null) { - logger.log(Level.SEVERE, "No connection returned to GitHub server!"); - return false; - } if (gitHub.getRateLimit().remaining == 0) { + logger.log(Level.INFO, "Exceeded rate limit for repository"); return false; } } catch (FileNotFoundException ex) { logger.log(Level.INFO, "Rate limit API not found."); + return false; } catch (IOException ex) { logger.log(Level.SEVERE, "Error while accessing rate limit API", ex); return false; } + - if (ghRepository == null) { - try { - ghRepository = gitHub.getRepository(reponame); - } catch (IOException ex) { - logger.log(Level.SEVERE, "Could not retrieve GitHub repository named " + reponame + " (Do you have properly set 'GitHub project' field in job configuration?)", ex); - return false; - } + try { + ghRepository = gitHub.getRepository(reponame); + } catch (IOException ex) { + logger.log(Level.SEVERE, "Could not retrieve GitHub repository named " + reponame + " (Do you have properly set 'GitHub project' field in job configuration?)", ex); + return false; } return true; } public void check() { - if (!initGhRepository()) { + + if (!trigger.isActive()) { + logger.log(Level.FINE, "Project is not active, not checking github state"); return; } - - if (helper.isProjectDisabled()) { - logger.log(Level.FINE, "Project is disabled, not checking github state"); + + if (!initGhRepository()) { return; } + + GHRepository repo = getGitHubRepo(); List openPulls; try { - openPulls = ghRepository.getPullRequests(GHIssueState.OPEN); + openPulls = repo.getPullRequests(GHIssueState.OPEN); } catch (IOException ex) { logger.log(Level.SEVERE, "Could not retrieve open pull requests.", ex); return; } - Set closedPulls = new HashSet(pulls.keySet()); + + + Set closedPulls = new HashSet(pullRequests.keySet()); for (GHPullRequest pr : openPulls) { - if (pr.getHead() == null) { + if (pr.getHead() == null) { // Not sure if we need this, but leaving it for now. try { - pr = ghRepository.getPullRequest(pr.getNumber()); + pr = getActualPullRequest(pr.getNumber()); } catch (IOException ex) { logger.log(Level.SEVERE, "Could not retrieve pr " + pr.getNumber(), ex); return; } } - check(pr); + check(pr, true); closedPulls.remove(pr.getNumber()); } + // remove closed pulls so we don't check them again for (Integer id : closedPulls) { - pulls.remove(id); + pullRequests.remove(id); + } + try { + this.save(); + } catch (IOException e) { + logger.log(Level.SEVERE, "Unable to save repository!", e); } } - private void check(GHPullRequest pr) { - final Integer id = pr.getNumber(); - GhprbPullRequest pull; - if (pulls.containsKey(id)) { - pull = pulls.get(id); - } else { - pulls.putIfAbsent(id, new GhprbPullRequest(pr, helper, this)); - pull = pulls.get(id); + private void check(GHPullRequest pr, boolean isNew) { + int number = pr.getNumber(); + try { + GhprbPullRequest pull = getPullRequest(null, isNew, number); + pull.check(pr); + } catch (IOException e) { + logger.log(Level.SEVERE, "Unable to check pr: " + number, e); + } + try { + this.save(); + } catch (IOException e) { + logger.log(Level.SEVERE, "Unable to save repository!", e); } - pull.check(pr); } public void commentOnFailure(AbstractBuild build, TaskListener listener, GhprbCommitStatusException ex) { @@ -197,7 +245,9 @@ public void addComment(int id, String comment, AbstractBuild build, TaskLi } try { - getGitHubRepo().getPullRequest(id).comment(comment); + GHRepository repo = getGitHubRepo(); + GHPullRequest pr = repo.getPullRequest(id); + pr.comment(comment); } catch (IOException ex) { logger.log(Level.SEVERE, "Couldn't add comment to pull request #" + id + ": '" + comment + "'", ex); } @@ -205,7 +255,9 @@ public void addComment(int id, String comment, AbstractBuild build, TaskLi public void closePullRequest(int id) { try { - getGitHubRepo().getPullRequest(id).close(); + GHRepository repo = getGitHubRepo(); + GHPullRequest pr = repo.getPullRequest(id); + pr.close(); } catch (IOException ex) { logger.log(Level.SEVERE, "Couldn't close the pull request #" + id + ": '", ex); } @@ -235,8 +287,12 @@ public boolean createHook() { return true; } Map config = new HashMap(); + String secret = getSecret(); config.put("url", new URL(getHookUrl()).toExternalForm()); config.put("insecure_ssl", "1"); + if (secret != "") { + config.put("secret",secret); + } ghRepository.createHook("web", config, HOOK_EVENTS, true); return true; } catch (IOException ex) { @@ -245,72 +301,116 @@ public boolean createHook() { } } - private static String getHookUrl() { - return Jenkins.getInstance().getRootUrl() + GhprbRootAction.URL + "/"; + private String getSecret() { + return trigger.getGitHubApiAuth().getSecret(); } - public GHPullRequest getPullRequest(int id) throws IOException { + private String getHookUrl() { + String baseUrl = trigger.getGitHubApiAuth().getJenkinsUrl(); + if (baseUrl == null) { + baseUrl = Jenkins.getInstance().getRootUrl(); + } + return baseUrl + GhprbRootAction.URL + "/"; + } + + public GhprbPullRequest getPullRequest(int id) { + return pullRequests.get(id); + } + + public GHPullRequest getActualPullRequest(int id) throws IOException { return getGitHubRepo().getPullRequest(id); } void onIssueCommentHook(IssueComment issueComment) throws IOException { - if (helper.isProjectDisabled()) { + if (!trigger.isActive()) { logger.log(Level.FINE, "Not checking comments since build is disabled"); return; } - int id = issueComment.getIssue().getNumber(); - logger.log(Level.FINER, "Comment on issue #{0} from {1}: {2}", new Object[] { id, issueComment.getComment().getUser(), issueComment.getComment().getBody() }); + int number = issueComment.getIssue().getNumber(); + logger.log(Level.FINER, "Comment on issue #{0} from {1}: {2}", + new Object[] { number, issueComment.getComment().getUser(), issueComment.getComment().getBody() }); + if (!"created".equals(issueComment.getAction())) { return; } - GhprbPullRequest pull = pulls.get(id); - if (pull == null) { - pull = new GhprbPullRequest(getGitHubRepo().getPullRequest(id), helper, this); - pulls.put(id, pull); - } + GhprbPullRequest pull = getPullRequest(null, false, number); pull.check(issueComment.getComment()); - GhprbTrigger.getDscp().save(); + try { + this.save(); + } catch (IOException e) { + logger.log(Level.SEVERE, "Unable to save repository!", e); + } } - - void onPullRequestHook(PullRequest pr) { - if ("closed".equals(pr.getAction())) { - pulls.remove(pr.getNumber()); - } else if (helper.isProjectDisabled()) { - logger.log(Level.FINE, "Not processing Pull request since the build is disabled"); - } else if ("opened".equals(pr.getAction()) || "reopened".equals(pr.getAction())) { - GhprbPullRequest pull = pulls.get(pr.getNumber()); - if (pull == null) { - pulls.putIfAbsent(pr.getNumber(), new GhprbPullRequest(pr.getPullRequest(), helper, this)); - pull = pulls.get(pr.getNumber()); - } - pull.check(pr.getPullRequest()); - } else if ("synchronize".equals(pr.getAction())) { - GhprbPullRequest pull = pulls.get(pr.getNumber()); - if (pull == null) { - pulls.putIfAbsent(pr.getNumber(), new GhprbPullRequest(pr.getPullRequest(), helper, this)); - pull = pulls.get(pr.getNumber()); - } - if (pull == null) { - logger.log(Level.SEVERE, "Pull Request #{0} doesn''t exist", pr.getNumber()); - return; + + private GhprbPullRequest getPullRequest(GHPullRequest ghpr, Boolean isNew, Integer number) throws IOException { + if (number == null) { + number = ghpr.getNumber(); + } + synchronized (pullRequests) { + GhprbPullRequest pr = pullRequests.get(number); + if (pr == null) { + if (ghpr == null) { + GHRepository repo = getGitHubRepo(); + ghpr = repo.getPullRequest(number); + } + pr = new GhprbPullRequest(ghpr, trigger.getHelper(), this, isNew); + pullRequests.put(number, pr); } - pull.check(pr.getPullRequest()); - } else { - logger.log(Level.WARNING, "Unknown Pull Request hook action: {0}", pr.getAction()); + + return pr; } - GhprbTrigger.getDscp().save(); } - @VisibleForTesting - void setHelper(Ghprb helper) { - this.helper = helper; - } + void onPullRequestHook(PullRequest pr) throws IOException { + GHPullRequest ghpr = pr.getPullRequest(); + int number = pr.getNumber(); + String action = pr.getAction(); + + if ("closed".equals(action)) { + pullRequests.remove(number); + } else if (!trigger.isActive()) { + logger.log(Level.FINE, "Not processing Pull request since the build is disabled"); + } else if ("opened".equals(action) || "reopened".equals(action) || "synchronize".equals(action)) { + GhprbPullRequest pull = getPullRequest(ghpr, false, number); + pull.check(ghpr); + } else { + logger.log(Level.WARNING, "Unknown Pull Request hook action: {0}", action); + } + } + public GHRepository getGitHubRepo() { if (ghRepository == null) { - init(); + initGhRepository(); } return ghRepository; } + + public void load() throws IOException { + XmlFile xml = getConfigXml(trigger.getActualProject()); + if(xml.exists()){ + xml.unmarshal(this); + } + save(); + } + + public void save() throws IOException { + if(BulkChange.contains(this)) { + return; + } + XmlFile config = getConfigXml(trigger.getActualProject()); + config.write(this); + SaveableListener.fireOnChange(this, config); + } + + protected XmlFile getConfigXml(AbstractProject project) throws IOException { + try { + String escapedRepoName = URLEncoder.encode(reponame, "UTF8"); + File file = new File(project.getBuildDir() + "/pullrequests", escapedRepoName); + return Items.getConfigFile(file); + } catch (UnsupportedEncodingException e) { + throw new IOException(e); + } + } } \ No newline at end of file diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRootAction.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRootAction.java index 217d8d1c4..3cdb73632 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRootAction.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRootAction.java @@ -5,11 +5,14 @@ import hudson.model.UnprotectedRootAction; import hudson.security.ACL; import hudson.security.csrf.CrumbExclusion; -import jenkins.model.Jenkins; import org.acegisecurity.Authentication; import org.acegisecurity.context.SecurityContextHolder; import org.apache.commons.io.IOUtils; +import org.kohsuke.github.GHEventPayload.IssueComment; +import org.kohsuke.github.GHEventPayload.PullRequest; +import org.kohsuke.github.GHIssueState; +import org.kohsuke.github.GitHub; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; @@ -55,18 +58,19 @@ public void doIndex(StaplerRequest req, StaplerResponse resp) { String payload = null; String body = null; - if ("application/json".equals(type)) { + if (type.toLowerCase().startsWith("application/json")) { body = extractRequestBody(req); if (body == null) { logger.log(Level.SEVERE, "Can't get request body for application/json."); + resp.setStatus(StaplerResponse.SC_BAD_REQUEST); return; } payload = body; - } else if ("application/x-www-form-urlencoded".equals(type)) { + } else if (type.toLowerCase().startsWith("application/x-www-form-urlencoded")) { body = extractRequestBody(req); if (body == null || body.length() <= 8) { - logger.log(Level.SEVERE, "Request doesn't contain payload. " - + "You're sending url encoded request, so you should pass github payload through 'payload' request parameter"); + logger.log(Level.SEVERE, "Request doesn't contain payload. " + "You're sending url encoded request, so you should pass github payload through 'payload' request parameter"); + resp.setStatus(StaplerResponse.SC_BAD_REQUEST); return; } try { @@ -74,25 +78,87 @@ public void doIndex(StaplerRequest req, StaplerResponse resp) { payload = URLDecoder.decode(body.substring(8), encoding != null ? encoding : "UTF-8"); } catch (UnsupportedEncodingException e) { logger.log(Level.SEVERE, "Error while trying to decode the payload"); + resp.setStatus(StaplerResponse.SC_BAD_REQUEST); return; } } if (payload == null) { - logger.log(Level.SEVERE, "Payload is null, maybe content type '{0}' is not supported by this plugin. " - + "Please use 'application/json' or 'application/x-www-form-urlencoded'", + logger.log(Level.SEVERE, "Payload is null, maybe content type ''{0}'' is not supported by this plugin. " + "Please use 'application/json' or 'application/x-www-form-urlencoded'", new Object[] { type }); + resp.setStatus(StaplerResponse.SC_UNSUPPORTED_MEDIA_TYPE); return; } - for (GhprbWebHook webHook : getWebHooks()) { - try { - webHook.handleWebHook(event, payload, body, signature); - } catch (Exception e) { - logger.log(Level.SEVERE, "Unable to process web hook for: " + webHook.getProjectName(), e); + logger.log(Level.FINE, "Got payload event: {0}", event); + + // Not sure if this is needed, but it may be to get info about old builds. + Authentication old = SecurityContextHolder.getContext().getAuthentication(); + SecurityContextHolder.getContext().setAuthentication(ACL.SYSTEM); + + try { + GitHub gh = GitHub.connectAnonymously(); + + if ("issue_comment".equals(event)) { + IssueComment issueComment = getIssueComment(payload, gh); + GHIssueState state = issueComment.getIssue().getState(); + if (state == GHIssueState.CLOSED) { + logger.log(Level.INFO, "Skip comment on closed PR"); + return; + } + + if (!issueComment.getIssue().isPullRequest()) { + logger.log(Level.INFO, "Skip comment on Issue"); + return; + } + + String repoName = issueComment.getRepository().getFullName(); + + logger.log(Level.INFO, "Checking issue comment ''{0}'' for repo {1}", new Object[] { issueComment.getComment().getBody(), repoName }); + + for (GhprbTrigger trigger : getTriggers(repoName, body, signature)) { + try { + IssueComment authedComment = getIssueComment(payload, trigger.getGitHub()); + trigger.handleComment(authedComment); + } catch (Exception e) { + logger.log(Level.SEVERE, "Unable to process web hook for: " + trigger.getProjectName(), e); + } + } + + } else if ("pull_request".equals(event)) { + PullRequest pr = getPullRequest(payload, gh); + String repoName = pr.getRepository().getFullName(); + + logger.log(Level.INFO, "Checking PR #{1} for {0}", new Object[] { repoName, pr.getNumber() }); + + for (GhprbTrigger trigger : getTriggers(repoName, body, signature)) { + try { + PullRequest authedPr = getPullRequest(payload, trigger.getGitHub()); + trigger.handlePR(authedPr); + } catch (Exception e) { + logger.log(Level.SEVERE, "Unable to process web hook for: " + trigger.getProjectName(), e); + } + } + } else { + logger.log(Level.WARNING, "Request not known"); } + + } catch (IOException e) { + logger.log(Level.SEVERE, "Unable to connect to GitHub anonymously", e); + } finally { + SecurityContextHolder.getContext().setAuthentication(old); } - + + } + + private PullRequest getPullRequest(String payload, GitHub gh) throws IOException { + PullRequest pr = gh.parseEventPayload(new StringReader(payload), PullRequest.class); + return pr; + } + + private IssueComment getIssueComment(String payload, GitHub gh) throws IOException { + IssueComment issueComment = gh.parseEventPayload(new StringReader(payload), IssueComment.class); + return issueComment; } private String extractRequestBody(StaplerRequest req) { @@ -109,31 +175,24 @@ private String extractRequestBody(StaplerRequest req) { return body; } - - private Set getWebHooks() { - final Set webHooks = new HashSet(); - - // We need this to get access to list of repositories - Authentication old = SecurityContextHolder.getContext().getAuthentication(); - SecurityContextHolder.getContext().setAuthentication(ACL.SYSTEM); + private Set getTriggers(String repoName, String body, String signature) { + Set triggers = new HashSet(); - try { - for (AbstractProject job : Jenkins.getInstance().getAllItems(AbstractProject.class)) { - GhprbTrigger trigger = job.getTrigger(GhprbTrigger.class); - if (trigger == null || trigger.getWebHook() == null) { + Set> projects = GhprbTrigger.getDscp().getRepoTriggers(repoName); + if (projects != null) { + for (AbstractProject project : projects) { + GhprbTrigger trigger = Ghprb.extractTrigger(project); + if (trigger == null) { + logger.log(Level.WARNING, "Warning, trigger unexpectedly null for project " + project.getFullName()); continue; } - webHooks.add(trigger.getWebHook()); + if (trigger.matchSignature(body, signature)) { + triggers.add(trigger); + } } - } finally { - SecurityContextHolder.getContext().setAuthentication(old); } - - if (webHooks.size() == 0) { - logger.log(Level.WARNING, "No projects found using GitHub pull request trigger"); - } - - return webHooks; + return triggers; + } @Extension diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java index 5bccfd9a3..82926a691 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java @@ -7,8 +7,17 @@ import hudson.Extension; import hudson.Util; -import hudson.model.*; +import hudson.matrix.MatrixProject; +import hudson.model.AbstractBuild; import hudson.model.AbstractProject; +import hudson.model.Item; +import hudson.model.ParameterDefinition; +import hudson.model.ParameterValue; +import hudson.model.ParametersAction; +import hudson.model.ParametersDefinitionProperty; +import hudson.model.Run; +import hudson.model.Saveable; +import hudson.model.StringParameterValue; import hudson.model.queue.QueueTaskFuture; import hudson.plugins.git.util.BuildData; import hudson.triggers.TriggerDescriptor; @@ -19,9 +28,11 @@ import net.sf.json.JSONObject; import org.apache.commons.lang.StringUtils; +import org.jenkinsci.plugins.ghprb.extensions.GhprbBuildStep; import org.jenkinsci.plugins.ghprb.extensions.GhprbCommitStatus; import org.jenkinsci.plugins.ghprb.extensions.GhprbExtension; import org.jenkinsci.plugins.ghprb.extensions.GhprbExtensionDescriptor; +import org.jenkinsci.plugins.ghprb.extensions.GhprbGlobalDefault; import org.jenkinsci.plugins.ghprb.extensions.comments.GhprbBuildLog; import org.jenkinsci.plugins.ghprb.extensions.comments.GhprbBuildResultMessage; import org.jenkinsci.plugins.ghprb.extensions.comments.GhprbBuildStatus; @@ -29,6 +40,8 @@ import org.jenkinsci.plugins.ghprb.extensions.status.GhprbSimpleStatus; import org.kohsuke.github.GHCommitState; import org.kohsuke.github.GitHub; +import org.kohsuke.github.GHEventPayload.IssueComment; +import org.kohsuke.github.GHEventPayload.PullRequest; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.StaplerRequest; @@ -36,15 +49,18 @@ import javax.servlet.ServletException; import java.io.IOException; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.WeakHashMap; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.regex.Matcher; import java.util.regex.Pattern; -import jenkins.model.Jenkins; - /** * @author Honza Brázdil */ @@ -57,7 +73,6 @@ public class GhprbTrigger extends GhprbTriggerBackwardsCompatible { private final Boolean allowMembersOfWhitelistedOrgsAsAdmin; private final String orgslist; private final String cron; - private final String triggerPhrase; private final String buildDescTemplate; private final Boolean onlyTriggerPhrase; private final Boolean suppressTestingRequest; @@ -67,11 +82,14 @@ public class GhprbTrigger extends GhprbTriggerBackwardsCompatible { private Boolean autoCloseFailedPullRequests; private Boolean displayBuildErrorsOnDownstreamBuilds; private List whiteListTargetBranches; - private transient Ghprb helper; - private String project; - private AbstractProject _project; private String gitHubAuthId; + private String triggerPhrase; + + private transient Ghprb helper; + private transient GhprbRepository repository; + private transient GhprbBuilds builds; + private transient GhprbGitHub ghprbGitHub; private DescribableList extensions = new DescribableList(Saveable.NOOP); @@ -92,12 +110,15 @@ private void setExtensions(List extensions) { GhprbCommitStatus.class ); - // Now make sure we have at least one of the types we need one of. - Ghprb.addIfMissing(this.extensions, new GhprbSimpleStatus(), GhprbCommitStatus.class); + // Make sure we have at least one of the types we need one of. + for (GhprbExtension ext : getDescriptor().getExtensions()) { + if (ext instanceof GhprbGlobalDefault) { + Ghprb.addIfMissing(this.extensions, Ghprb.getGlobal(ext.getClass()), ext.getClass()); + } + } } @DataBoundConstructor - public GhprbTrigger(String adminlist, String whitelist, String orgslist, @@ -136,7 +157,7 @@ public GhprbTrigger(String adminlist, this.allowMembersOfWhitelistedOrgsAsAdmin = allowMembersOfWhitelistedOrgsAsAdmin; this.buildDescTemplate = buildDescTemplate; setExtensions(extensions); - configVersion = 1; + configVersion = latestVersion; } @Override @@ -145,6 +166,8 @@ public Object readResolve() { checkGitHubApiAuth(); return this; } + + @SuppressWarnings("deprecation") private void checkGitHubApiAuth() { @@ -158,43 +181,94 @@ public static DescriptorImpl getDscp() { return DESCRIPTOR; } + @SuppressWarnings("deprecation") + private void initState() throws IOException { + + final GithubProjectProperty ghpp = super.job.getProperty(GithubProjectProperty.class); + if (ghpp == null || ghpp.getProjectUrl() == null) { + throw new IllegalStateException("A GitHub project url is required."); + } + String baseUrl = ghpp.getProjectUrl().baseUrl(); + Matcher m = Ghprb.githubUserRepoPattern.matcher(baseUrl); + if (!m.matches()) { + throw new IllegalStateException(String.format("Invalid GitHub project url: %s", baseUrl)); + } + final String reponame = m.group(2); + + this.repository = new GhprbRepository(reponame, this); + this.repository.load(); + + Map pulls = this.pullRequests; + this.pullRequests = null; + + try { + Map prs = getDescriptor().getPullRequests(super.job.getFullName()); + if (prs != null) { + prs = new ConcurrentHashMap(prs); + if (pulls == null) { + pulls = prs; + } else { + pulls.putAll(prs); + } + } + } catch (Exception e) { + logger.log(Level.SEVERE, "Unable to transfer map of pull requests", e); + } + + if (pulls != null) { + this.repository.addPullRequests(pulls); + this.repository.save(); + } + this.builds = new GhprbBuilds(this, repository); + + this.repository.init(); + this.ghprbGitHub = new GhprbGitHub(this); + } + + @Override public void start(AbstractProject project, boolean newInstance) { // We should always start the trigger, and handle cases where we don't run in the run function. super.start(project, newInstance); - this._project = project; - this.project = project.getFullName(); + + String name = project.getFullName(); if (project.isDisabled()) { - logger.log(Level.FINE, "Project is disabled, not starting trigger for job " + this.project); + logger.log(Level.FINE, "Project is disabled, not starting trigger for job " + name); return; } if (project.getProperty(GithubProjectProperty.class) == null) { - logger.log(Level.INFO, "GitHub project property is missing the URL, cannot start ghprb trigger for job " + this.project); + logger.log(Level.INFO, "GitHub project property is missing the URL, cannot start ghprb trigger for job " + name); return; } try { - helper = createGhprb(project); - } catch (IllegalStateException ex) { + initState(); + } catch (Exception ex) { logger.log(Level.SEVERE, "Can't start ghprb trigger", ex); return; } logger.log(Level.INFO, "Starting the ghprb trigger for the {0} job; newInstance is {1}", - new String[] { this.project, String.valueOf(newInstance) }); - helper.init(); - } + new String[] { name, String.valueOf(newInstance) }); - Ghprb createGhprb(AbstractProject project) { - return new Ghprb(project, this, getDescriptor().getPullRequests(project.getFullName())); + helper = new Ghprb(this); + + if (getUseGitHubHooks()) { + this.repository.createHook(); + DESCRIPTOR.addRepoTrigger(getRepository().getName(), super.job); + } } + @Override public void stop() { - logger.log(Level.INFO, "Stopping the ghprb trigger for project {0}", this.project); - if (helper != null) { - helper.stop(); - helper = null; + String name = super.job != null ? super.job.getFullName(): "NOT STARTED"; + logger.log(Level.INFO, "Stopping the ghprb trigger for project {0}", name); + if (this.repository != null) { + String repo = this.repository.getName(); + if (!StringUtils.isEmpty(repo)) { + DESCRIPTOR.removeRepoTrigger(repo, super.job); + } } super.stop(); } @@ -207,30 +281,42 @@ public void run() { return; } - if ((helper != null && helper.isProjectDisabled()) || (_project != null && _project.isDisabled())) { - logger.log(Level.FINE, "Project is disabled, ignoring trigger run call for job {0}", this.project); - return; - } - - if (helper == null) { - logger.log(Level.SEVERE, "Helper is null and Project is not disabled, unable to run trigger"); + if (!isActive()) { return; } + logger.log(Level.FINE, "Running trigger for {0}", super.job.getFullName()); - logger.log(Level.FINE, "Running trigger for {0}", project); - - helper.run(); - getDescriptor().save(); + this.repository.check(); } + - public QueueTaskFuture startJob(GhprbCause cause, GhprbRepository repo) { + public QueueTaskFuture scheduleBuild(GhprbCause cause, GhprbRepository repo) { + + + for (GhprbExtension ext : Ghprb.getJobExtensions(this, GhprbBuildStep.class)) { + if (ext instanceof GhprbBuildStep) { + ((GhprbBuildStep)ext).onScheduleBuild(super.job, cause); + } + } + ArrayList values = getDefaultParameters(); final String commitSha = cause.isMerged() ? "origin/pr/" + cause.getPullID() + "/merge" : cause.getCommit(); values.add(new StringParameterValue("sha1", commitSha)); values.add(new StringParameterValue("ghprbActualCommit", cause.getCommit())); String triggerAuthor = ""; String triggerAuthorEmail = ""; + String triggerAuthorLogin = ""; + + GhprbPullRequest pr = getRepository().getPullRequest(cause.getPullID()); + String lastBuildId = pr.getLastBuildId(); + BuildData buildData = null; + if (!(job instanceof MatrixProject) && !StringUtils.isEmpty(lastBuildId)) { + AbstractBuild lastBuild = job.getBuild(lastBuildId); + if (lastBuild != null) { + buildData = lastBuild.getAction(BuildData.class); + } + } try { triggerAuthor = getString(cause.getTriggerSender().getName(), ""); @@ -238,28 +324,54 @@ public QueueTaskFuture startJob(GhprbCause cause, GhprbRepository repo) { try { triggerAuthorEmail = getString(cause.getTriggerSender().getEmail(), ""); } catch (Exception e) {} + try { + triggerAuthorLogin = getString(cause.getTriggerSender().getLogin(), ""); + } catch (Exception e) {} setCommitAuthor(cause, values); + values.add(new StringParameterValue("ghprbAuthorRepoGitUrl", getString(cause.getAuthorRepoGitUrl(), ""))); values.add(new StringParameterValue("ghprbTriggerAuthor", triggerAuthor)); values.add(new StringParameterValue("ghprbTriggerAuthorEmail", triggerAuthorEmail)); + values.add(new StringParameterValue("ghprbTriggerAuthorLogin", triggerAuthorLogin)); + values.add(new StringParameterValue("ghprbTriggerAuthorLoginMention", !triggerAuthorLogin.isEmpty() ? "@" + + triggerAuthorLogin : "")); final StringParameterValue pullIdPv = new StringParameterValue("ghprbPullId", String.valueOf(cause.getPullID())); values.add(pullIdPv); values.add(new StringParameterValue("ghprbTargetBranch", String.valueOf(cause.getTargetBranch()))); values.add(new StringParameterValue("ghprbSourceBranch", String.valueOf(cause.getSourceBranch()))); values.add(new StringParameterValue("GIT_BRANCH", String.valueOf(cause.getSourceBranch()))); + // it's possible the GHUser doesn't have an associated email address values.add(new StringParameterValue("ghprbPullAuthorEmail", getString(cause.getAuthorEmail(), ""))); - values.add(new StringParameterValue("ghprbPullDescription", String.valueOf(cause.getShortDescription()))); + values.add(new StringParameterValue("ghprbPullAuthorLogin", String.valueOf(cause.getPullRequestAuthor().getLogin()))); + values.add(new StringParameterValue("ghprbPullAuthorLoginMention", "@" + cause.getPullRequestAuthor().getLogin())); + + values.add(new StringParameterValue("ghprbPullDescription", escapeText(String.valueOf(cause.getShortDescription())))); values.add(new StringParameterValue("ghprbPullTitle", String.valueOf(cause.getTitle()))); values.add(new StringParameterValue("ghprbPullLink", String.valueOf(cause.getUrl()))); + values.add(new StringParameterValue("ghprbPullLongDescription", escapeText(String.valueOf(cause.getDescription())))); + + values.add(new StringParameterValue("ghprbCommentBody", escapeText(String.valueOf(cause.getCommentBody())))); + + values.add(new StringParameterValue("ghprbGhRepository", repo.getName())); + values.add(new StringParameterValue("ghprbCredentialsId", getString(getGitHubApiAuth().getCredentialsId(), ""))); + + // add the previous pr BuildData as an action so that the correct change log is generated by the GitSCM plugin // note that this will be removed from the Actions list after the job is completed so that the old (and incorrect) // one isn't there - return this.job.scheduleBuild2(job.getQuietPeriod(), cause, new ParametersAction(values), findPreviousBuildForPullId(pullIdPv)); + return this.job.scheduleBuild2(job.getQuietPeriod(), cause, new ParametersAction(values), buildData); + } + + private String escapeText(String text) { + return text.replace("\n", "\\n").replace("\r", "\\r").replace("\"", "\\\""); } + public String getGitHubAuthId() { + return gitHubAuthId == null ? "" : gitHubAuthId; + } public GhprbGitHubAuth getGitHubApiAuth() { if (gitHubAuthId == null) { @@ -275,28 +387,11 @@ public GhprbGitHubAuth getGitHubApiAuth() { public GitHub getGitHub() throws IOException { GhprbGitHubAuth auth = getGitHubApiAuth(); - if (auth == null) { - return null; - } - return auth.getConnection(getActualProject()); } public AbstractProject getActualProject() { - - if (_project != null) { - return _project; - } - - @SuppressWarnings("rawtypes") - List projects = Jenkins.getInstance().getAllItems(AbstractProject.class); - - for (AbstractProject project : projects) { - if (project.getFullName().equals(this.project)) { - return project; - } - } - return null; + return super.job; } private void setCommitAuthor(GhprbCause cause, ArrayList values) { @@ -317,24 +412,26 @@ private String getString(String actual, String d) { /** * Find the previous BuildData for the given pull request number; this may return null + * Currently this doesn't appear to be working, and needs to be revised so it is faster */ private BuildData findPreviousBuildForPullId(StringParameterValue pullIdPv) { + // Don't add the Action if it's a matrix job. + // This is suboptimal, but necessary until we find a way to determine if the build we're about to start is + // the root build or one of the leaves. + if (job instanceof MatrixProject) { + return null; + } + // find the previous build for this particular pull request, it may not be the last build for (Run r : job.getBuilds()) { ParametersAction pa = r.getAction(ParametersAction.class); - if (pa != null) { - for (ParameterValue pv : pa.getParameters()) { - if (pv.equals(pullIdPv)) { - for (BuildData bd : r.getActions(BuildData.class)) { - return bd; - } - } - } + if (pa != null && pa.getParameters().contains(pullIdPv)) { + return r.getAction(BuildData.class); } } return null; } - + private ArrayList getDefaultParameters() { ArrayList values = new ArrayList(); ParametersDefinitionProperty pdp = this.job.getProperty(ParametersDefinitionProperty.class); @@ -390,10 +487,6 @@ public String getCron() { return cron; } - public String getProject() { - return project; - } - public String getTriggerPhrase() { if (triggerPhrase == null) { return ""; @@ -412,12 +505,19 @@ public Boolean getSuppressTestingRequest() { public Boolean getUseGitHubHooks() { return useGitHubHooks != null && useGitHubHooks; } + + public Ghprb getHelper() { + if (helper == null) { + helper = new Ghprb(this); + } + return helper; + } public Boolean getPermitAll() { return permitAll != null && permitAll; } - public Boolean isAutoCloseFailedPullRequests() { + public Boolean getAutoCloseFailedPullRequests() { if (autoCloseFailedPullRequests == null) { Boolean autoClose = getDescriptor().getAutoCloseFailedPullRequests(); return (autoClose != null && autoClose); @@ -425,7 +525,7 @@ public Boolean isAutoCloseFailedPullRequests() { return autoCloseFailedPullRequests; } - public Boolean isDisplayBuildErrorsOnDownstreamBuilds() { + public Boolean getDisplayBuildErrorsOnDownstreamBuilds() { if (displayBuildErrorsOnDownstreamBuilds == null) { Boolean displayErrors = getDescriptor().getDisplayBuildErrorsOnDownstreamBuilds(); return (displayErrors != null && displayErrors); @@ -440,11 +540,6 @@ public List getWhiteListTargetBranches() { return whiteListTargetBranches; } - public GhprbWebHook getWebHook() { - GhprbWebHook webHook = new GhprbWebHook(this); - return webHook; - } - @Override public DescriptorImpl getDescriptor() { return DESCRIPTOR; @@ -456,21 +551,78 @@ void setHelper(Ghprb helper) { } public GhprbBuilds getBuilds() { - if (helper == null) { - logger.log(Level.SEVERE, "The ghprb trigger for {0} wasn''t properly started - helper is null", this.project); - return null; + if (this.builds == null && this.isActive()) { + this.builds = new GhprbBuilds(this, getRepository()); + } + return this.builds; + } + + public GhprbGitHub getGhprbGitHub() { + if (this.ghprbGitHub == null && this.isActive()) { + this.ghprbGitHub = new GhprbGitHub(this); } - return helper.getBuilds(); + return this.ghprbGitHub; } + public boolean isActive() { + String name = super.job != null ? super.job.getFullName() : "NOT STARTED"; + boolean isActive = true; + if (super.job == null) { + logger.log(Level.FINE, "Project was never set, start was never run"); + isActive = false; + } else if (super.job.isDisabled()) { + logger.log(Level.FINE, "Project is disabled, ignoring trigger run call for job {0}", name); + isActive = false; + } else if (getRepository() == null) { + logger.log(Level.SEVERE, "The ghprb trigger for {0} wasn''t properly started - repository is null", name); + isActive = false; + } + + return isActive; + } + public GhprbRepository getRepository() { - if (helper == null) { - logger.log(Level.SEVERE, "The ghprb trigger for {0} wasn''t properly started - helper is null", this.project); - return null; + if (this.repository == null && super.job != null && !super.job.isDisabled()) { + try { + this.initState(); + } catch (IOException e) { + logger.log(Level.SEVERE, "Unable to init trigger state!", e); + } + } + return this.repository; + } + + public String getProjectName() { + String projectName = super.job == null ? "NOT_STARTED" : super.job.getFullName(); + return projectName; + } + + + public boolean matchSignature(String body, String signature) { + if (!isActive()) { + return false; } - return helper.getRepository(); + GhprbGitHubAuth auth = getGitHubApiAuth(); + return auth == null ? false : auth.checkSignature(body, signature); + } + + public void handleComment(IssueComment issueComment) throws IOException { + GhprbRepository repo = getRepository(); + + logger.log(Level.INFO, "Checking comment on PR #{0} for job {1}", new Object[] {issueComment.getIssue().getNumber(), getProjectName()}); + + repo.onIssueCommentHook(issueComment); + } + + public void handlePR(PullRequest pr) throws IOException { + GhprbRepository repo = getRepository(); + + logger.log(Level.INFO, "Checking PR #{0} for job {1}", new Object[] {pr.getNumber(), getProjectName()}); + + repo.onPullRequestHook(pr); } + public static final class DescriptorImpl extends TriggerDescriptor { // GitHub username may only contain alphanumeric characters or dashes and cannot begin with a dash private static final Pattern adminlistPattern = Pattern.compile("((\\p{Alnum}[\\p{Alnum}-]*)|\\s)*"); @@ -519,7 +671,7 @@ public GhprbGitHubAuth getGitHubAuth(String gitHubAuthId) { public List getGithubAuth() { if (githubAuth == null || githubAuth.size() == 0) { githubAuth = new ArrayList(1); - githubAuth.add(new GhprbGitHubAuth(null, null, "Anonymous connection", null, null)); + githubAuth.add(new GhprbGitHubAuth(null, null, null, "Anonymous connection", null, null)); } return githubAuth; } @@ -536,7 +688,13 @@ public List getDefaultAuth(List githubAuth) { private String requestForTestingPhrase; // map of jobs (by their fullName) and their map of pull requests - private Map> jobs; + private transient Map> jobs; + + /** + * map of jobs (by the repo name); No need to keep the projects from shutdown to startup. + * New triggers will register here, and ones that are stopping will remove themselves. + */ + private transient Map>> repoJobs; public List getExtensionDescriptors() { return GhprbExtensionDescriptor.allProject(); @@ -558,15 +716,27 @@ public DescribableList getExtensions() public DescriptorImpl() { load(); readBackFromLegacy(); - if (jobs == null) { - jobs = new HashMap>(); + if (repoJobs == null) { + repoJobs = new ConcurrentHashMap>>(); } -// save(); + saveAfterPause(); + } + + private void saveAfterPause() { + new java.util.Timer().schedule( + new java.util.TimerTask() { + @Override + public void run() { + save(); + } + }, + 5000 + ); } @Override public boolean isApplicable(Item item) { - return true; + return item instanceof AbstractProject; } @Override @@ -595,13 +765,15 @@ public boolean configure(StaplerRequest req, JSONObject formData) throws FormExc try { extensions.rebuildHetero(req, formData, getGlobalExtensionDescriptors(), "extensions"); + // Now make sure we have at least one of the types we need one of. + Ghprb.addIfMissing(this.extensions, new GhprbSimpleStatus(), GhprbCommitStatus.class); } catch (IOException e) { e.printStackTrace(); } readBackFromLegacy(); - save(); + saveAfterPause(); return super.configure(req, formData); } @@ -676,6 +848,10 @@ public Boolean getDisplayBuildErrorsOnDownstreamBuilds() { public GHCommitState getUnstableAs() { return unstableAs; } + + public Integer getConfigVersion() { + return configVersion; + } public boolean isUseComments() { return (useComments != null && useComments); @@ -701,20 +877,59 @@ public ListBoxModel doFillGitHubAuthIdItems(@QueryParameter("gitHubAuthId") Stri } - public ConcurrentMap getPullRequests(String projectName) { - ConcurrentMap ret; - if (jobs.containsKey(projectName)) { - Map map = jobs.get(projectName); - if (!(map instanceof ConcurrentMap)) { - map = new ConcurrentHashMap(map); + public Map getPullRequests(String projectName) { + Map ret = null; + if (jobs != null && jobs.containsKey(projectName)) { + ret = jobs.get(projectName); + jobs.remove(projectName); + } + return ret; + } + + private void addRepoTrigger(String repo, AbstractProject project) { + if (project == null || StringUtils.isEmpty(repo)) { + return; + } + logger.log(Level.FINE, "Adding [{0}] to webhooks repo [{1}]", new Object[]{project.getFullName(), repo}); + + synchronized (repoJobs) { + Set> projects = repoJobs.get(repo); + if (projects == null) { + logger.log(Level.FINE, "No other projects found, creating new repo set"); + projects = Collections.newSetFromMap(new WeakHashMap, Boolean>()); + repoJobs.put(repo, projects); + } else { + logger.log(Level.FINE, "Adding project to current repo set, length: {0}", new Object[]{projects.size()}); + } + + projects.add(project); + } + } + + private void removeRepoTrigger(String repo, AbstractProject project) { + Set> projects = repoJobs.get(repo); + if (project != null && projects != null) { + logger.log(Level.FINE, "Removing [{0}] from webhooks repo [{1}]", new Object[]{repo, project.getFullName()}); + projects.remove(project); + } + } + + public Set> getRepoTriggers(String repo) { + if (repoJobs == null) { + repoJobs = new ConcurrentHashMap>>(5); + } + logger.log(Level.FINE, "Retrieving triggers for repo [{0}]", new Object[]{repo}); + + Set> projects = repoJobs.get(repo); + if (projects != null) { + for (AbstractProject project : projects) { + logger.log(Level.FINE, "Found project [{0}] for webhook repo [{0}]", new Object[]{project.getFullName(), repo}); } - jobs.put(projectName, (ConcurrentMap) map); - ret = (ConcurrentMap) map; } else { - ret = new ConcurrentHashMap(); - jobs.put(projectName, ret); + projects = Collections.newSetFromMap(new WeakHashMap, Boolean>(0)); } - return ret; + + return projects; } public List getWhiteListTargetBranches() { @@ -775,7 +990,7 @@ public void readBackFromLegacy() { if (!StringUtils.isEmpty(accessToken)) { try { - GhprbGitHubAuth auth = new GhprbGitHubAuth(serverAPIUrl, Ghprb.createCredentials(serverAPIUrl, accessToken), "Pre credentials Token", null, null); + GhprbGitHubAuth auth = new GhprbGitHubAuth(serverAPIUrl, null, Ghprb.createCredentials(serverAPIUrl, accessToken), "Pre credentials Token", null, null); if (githubAuth == null) { githubAuth = new ArrayList(1); } @@ -789,7 +1004,7 @@ public void readBackFromLegacy() { if (!StringUtils.isEmpty(username) || !StringUtils.isEmpty(password)) { try { - GhprbGitHubAuth auth = new GhprbGitHubAuth(serverAPIUrl, Ghprb.createCredentials(serverAPIUrl, username, password), "Pre credentials username and password", null, null); + GhprbGitHubAuth auth = new GhprbGitHubAuth(serverAPIUrl, null, Ghprb.createCredentials(serverAPIUrl, username, password), "Pre credentials username and password", null, null); if (githubAuth == null) { githubAuth = new ArrayList(1); } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTriggerBackwardsCompatible.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTriggerBackwardsCompatible.java index cff33d046..db4a51705 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTriggerBackwardsCompatible.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTriggerBackwardsCompatible.java @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.Map; import org.apache.commons.lang.StringUtils; import org.jenkinsci.plugins.ghprb.extensions.GhprbExtension; @@ -22,6 +23,8 @@ public abstract class GhprbTriggerBackwardsCompatible extends Trigger getExtensions(); + protected final int latestVersion = 3; + protected Integer configVersion; @@ -40,6 +43,14 @@ public GhprbTriggerBackwardsCompatible(String cron) throws ANTLRException { protected transient String commitStatusContext; @Deprecated protected transient GhprbGitHubAuth gitHubApiAuth; + @Deprecated + protected transient String project; + @Deprecated + protected transient AbstractProject _project; + @Deprecated + protected transient Map pullRequests; + + protected void convertPropertiesToExtensions() { @@ -51,7 +62,7 @@ protected void convertPropertiesToExtensions() { checkBuildStatusMessages(); checkCommitStatusContext(); - configVersion = 2; + configVersion = latestVersion; } private void checkBuildStatusMessages() { diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbWebHook.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbWebHook.java deleted file mode 100644 index db8b0848d..000000000 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbWebHook.java +++ /dev/null @@ -1,116 +0,0 @@ -package org.jenkinsci.plugins.ghprb; - -import java.io.IOException; -import java.io.StringReader; -import java.util.logging.Level; -import java.util.logging.Logger; - -import javax.crypto.Mac; -import javax.crypto.spec.SecretKeySpec; - -import org.apache.commons.codec.binary.Hex; -import org.apache.commons.lang.StringUtils; -import org.kohsuke.github.GHEventPayload; -import org.kohsuke.github.GHIssueState; -import org.kohsuke.github.GHRepository; -import org.kohsuke.github.GitHub; - -public class GhprbWebHook { - private static final Logger logger = Logger.getLogger(GhprbWebHook.class.getName()); - - private final GhprbTrigger trigger; - - public GhprbWebHook(GhprbTrigger trigger) { - this.trigger = trigger; - } - - public void handleWebHook(String event, String payload, String body, String signature) { - if (!checkSignature(body, signature, trigger.getGitHubApiAuth().getSecret())){ - return; - } - - GhprbRepository repo = trigger.getRepository(); - String repoName = repo.getName(); - - logger.log(Level.INFO, "Got payload event: {0}", event); - try { - GitHub gh = trigger.getGitHub(); - - if ("issue_comment".equals(event)) { - GHEventPayload.IssueComment issueComment = gh.parseEventPayload( - new StringReader(payload), - GHEventPayload.IssueComment.class); - GHIssueState state = issueComment.getIssue().getState(); - if (state == GHIssueState.CLOSED) { - logger.log(Level.INFO, "Skip comment on closed PR"); - return; - } - - if (matchRepo(repo, issueComment.getRepository())) { - logger.log(Level.INFO, "Checking issue comment ''{0}'' for repo {1}", - new Object[] { issueComment.getComment(), repoName } - ); - repo.onIssueCommentHook(issueComment); - } - - } else if ("pull_request".equals(event)) { - GHEventPayload.PullRequest pr = gh.parseEventPayload( - new StringReader(payload), - GHEventPayload.PullRequest.class); - if (matchRepo(repo, pr.getPullRequest().getRepository())) { - logger.log(Level.INFO, "Checking PR #{1} for {0}", new Object[] { repoName, pr.getNumber() }); - repo.onPullRequestHook(pr); - } - - } else { - logger.log(Level.WARNING, "Request not known"); - } - } catch (IOException ex) { - logger.log(Level.SEVERE, "Failed to parse github hook payload for " + trigger.getProject(), ex); - } - } - - public static boolean checkSignature(String body, String signature, String secret) { - if (StringUtils.isEmpty(secret)) { - return true; - } - - if (signature != null && signature.startsWith("sha1=")) { - String expected = signature.substring(5); - String algorithm = "HmacSHA1"; - try { - SecretKeySpec keySpec = new SecretKeySpec(secret.getBytes(), algorithm); - Mac mac = Mac.getInstance(algorithm); - mac.init(keySpec); - byte[] localSignatureBytes = mac.doFinal(body.getBytes("UTF-8")); - String localSignature = Hex.encodeHexString(localSignatureBytes); - if (! localSignature.equals(expected)) { - logger.log(Level.SEVERE, "Local signature {0} does not match external signature {1}", - new Object[] {localSignature, expected}); - return false; - } - } catch (Exception e) { - logger.log(Level.SEVERE, "Couldn't match both signatures"); - return false; - } - } else { - logger.log(Level.SEVERE, "Request doesn't contain a signature. Check that github has a secret that should be attached to the hook"); - return false; - } - - logger.log(Level.INFO, "Signatures checking OK"); - return true; - } - - private boolean matchRepo(GhprbRepository ghprbRepo, GHRepository ghRepo) { - String jobRepoName = ghprbRepo.getName(); - String hookRepoName = ghRepo.getFullName(); - logger.log(Level.FINE, "Comparing repository names: {0} to {1}, case is ignored", new Object[]{jobRepoName, hookRepoName}); - return jobRepoName.equalsIgnoreCase(hookRepoName); - } - - public String getProjectName() { - return trigger.getProject(); - } - -} diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/HttpConnectorWithJenkinsProxy.java b/src/main/java/org/jenkinsci/plugins/ghprb/HttpConnectorWithJenkinsProxy.java index f44dacf67..256db4c78 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/HttpConnectorWithJenkinsProxy.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/HttpConnectorWithJenkinsProxy.java @@ -1,14 +1,24 @@ package org.jenkinsci.plugins.ghprb; -import hudson.ProxyConfiguration; -import org.kohsuke.github.HttpConnector; - import java.io.IOException; import java.net.HttpURLConnection; import java.net.URL; +import org.kohsuke.github.HttpConnector; + +import hudson.ProxyConfiguration; + public class HttpConnectorWithJenkinsProxy implements HttpConnector { public HttpURLConnection connect(URL url) throws IOException { - return (HttpURLConnection) ProxyConfiguration.open(url); + HttpURLConnection con = (HttpURLConnection) ProxyConfiguration.open(url); + + // Set default timeouts in case there are none + if (con.getConnectTimeout() == 0) { + con.setConnectTimeout(10000); + } + if (con.getReadTimeout() == 0) { + con.setReadTimeout(10000); + } + return con; } } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/extensions/GhprbBuildStep.java b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/GhprbBuildStep.java new file mode 100644 index 000000000..6a0ca7292 --- /dev/null +++ b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/GhprbBuildStep.java @@ -0,0 +1,12 @@ +package org.jenkinsci.plugins.ghprb.extensions; + +import org.jenkinsci.plugins.ghprb.GhprbCause; + +import hudson.model.AbstractProject; +import hudson.model.Action; + +public interface GhprbBuildStep extends Action { + public static String buildStep = "GhprbBuildStep"; + + public void onScheduleBuild(AbstractProject project, GhprbCause cause); +} diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/extensions/GhprbCommitStatus.java b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/GhprbCommitStatus.java index f1a20cfef..e13cbca89 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/extensions/GhprbCommitStatus.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/GhprbCommitStatus.java @@ -1,15 +1,15 @@ package org.jenkinsci.plugins.ghprb.extensions; import hudson.model.AbstractBuild; +import hudson.model.AbstractProject; import hudson.model.TaskListener; -import org.jenkinsci.plugins.ghprb.GhprbPullRequest; -import org.jenkinsci.plugins.ghprb.GhprbTrigger; import org.kohsuke.github.GHRepository; public interface GhprbCommitStatus { - public void onBuildTriggered(GhprbTrigger trigger, GhprbPullRequest pr, GHRepository ghRepository) throws GhprbCommitStatusException; + public void onEnvironmentSetup(AbstractBuild build, TaskListener listener, GHRepository repo) throws GhprbCommitStatusException; + public void onBuildTriggered(AbstractProject project, String commitSha, boolean isMergeable, int prId, GHRepository ghRepository) throws GhprbCommitStatusException; public void onBuildStart(AbstractBuild build, TaskListener listener, GHRepository repo) throws GhprbCommitStatusException; public void onBuildComplete(AbstractBuild build, TaskListener listener, GHRepository repo) throws GhprbCommitStatusException; diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/extensions/GhprbGlobalDefault.java b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/GhprbGlobalDefault.java new file mode 100644 index 000000000..dcbd3986c --- /dev/null +++ b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/GhprbGlobalDefault.java @@ -0,0 +1,5 @@ +package org.jenkinsci.plugins.ghprb.extensions; + +public interface GhprbGlobalDefault { + +} diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/extensions/build/GhprbCancelBuildsOnUpdate.java b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/build/GhprbCancelBuildsOnUpdate.java new file mode 100644 index 000000000..14bf1290a --- /dev/null +++ b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/build/GhprbCancelBuildsOnUpdate.java @@ -0,0 +1,128 @@ +package org.jenkinsci.plugins.ghprb.extensions.build; + +import java.util.logging.Level; +import java.util.logging.Logger; + +import org.jenkinsci.plugins.ghprb.Ghprb; +import org.jenkinsci.plugins.ghprb.GhprbCause; +import org.jenkinsci.plugins.ghprb.extensions.GhprbBuildStep; +import org.jenkinsci.plugins.ghprb.extensions.GhprbExtension; +import org.jenkinsci.plugins.ghprb.extensions.GhprbExtensionDescriptor; +import org.jenkinsci.plugins.ghprb.extensions.GhprbGlobalDefault; +import org.jenkinsci.plugins.ghprb.extensions.GhprbGlobalExtension; +import org.jenkinsci.plugins.ghprb.extensions.GhprbProjectExtension; +import org.kohsuke.stapler.DataBoundConstructor; + +import hudson.Extension; +import hudson.model.AbstractProject; +import hudson.model.Cause; +import hudson.model.Queue; +import hudson.model.Result; +import hudson.model.Run; +import hudson.util.RunList; +import jenkins.model.Jenkins; + +public class GhprbCancelBuildsOnUpdate extends GhprbExtension implements GhprbBuildStep, GhprbGlobalExtension, GhprbProjectExtension, GhprbGlobalDefault { + + @Extension + public static final DescriptorImpl DESCRIPTOR = new DescriptorImpl(); + private static final Logger logger = Logger.getLogger(GhprbCancelBuildsOnUpdate.class.getName()); + + private final Boolean overrideGlobal; + + @DataBoundConstructor + public GhprbCancelBuildsOnUpdate(Boolean overrideGlobal) { + this.overrideGlobal = overrideGlobal; + } + + public Boolean getOverrideGlobal() { + return overrideGlobal == null ? false : overrideGlobal; + } + + private void cancelCurrentBuilds(AbstractProject project, + Integer prId) { + if (overrideGlobal) { + return; + } + + Queue queue = Jenkins.getInstance().getQueue(); + for (Queue.Item item : queue.getItems(project)) { + GhprbCause qcause = null; + for (Cause cause : item.getCauses()){ + if (cause instanceof GhprbCause) { + qcause = (GhprbCause) cause; + } + } + if (qcause == null) { + continue; + } + if (qcause.getPullID() == prId) { + try { + queue.cancel(item); + } catch (Exception e) { + logger.log(Level.SEVERE, "Unable to cancel queued build", e); + } + } + } + + RunList runs = project.getBuilds(); + for (Run run : runs) { + if (!run.isBuilding() && !run.hasntStartedYet()) { + break; + } + GhprbCause cause = Ghprb.getCause(run); + if (cause == null) { + continue; + } + if (cause.getPullID() == prId) { + try { + run.addAction(this); + run.getExecutor().interrupt(Result.ABORTED); + } catch (Exception e) { + run.getActions().remove(this); + logger.log(Level.SEVERE, "Unable to interrupt build!", e); + } + } + } + + } + + public void onScheduleBuild(AbstractProject project, + GhprbCause cause) { + + if (project == null || cause == null) { + return; + } + if (project.isBuilding() || project.isInQueue()) { + cancelCurrentBuilds(project, cause.getPullID()); + } + } + + public String getIconFileName() { + return null; + } + + public String getDisplayName() { + return "Cancel Build on Pull Request Update"; + } + + public String getUrlName() { + return null; + } + + @Override + public DescriptorImpl getDescriptor() { + return DESCRIPTOR; + } + + public static final class DescriptorImpl extends GhprbExtensionDescriptor + implements GhprbGlobalExtension, GhprbProjectExtension { + + @Override + public String getDisplayName() { + return "Cancel build on update"; + } + } + + +} diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildLog.java b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildLog.java index 6aa2966bf..8ceeabc9a 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildLog.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildLog.java @@ -12,10 +12,11 @@ import org.jenkinsci.plugins.ghprb.extensions.GhprbExtension; import org.jenkinsci.plugins.ghprb.extensions.GhprbExtensionDescriptor; import org.jenkinsci.plugins.ghprb.extensions.GhprbGlobalExtension; +import org.jenkinsci.plugins.ghprb.extensions.GhprbProjectExtension; import org.kohsuke.github.GHCommitState; import org.kohsuke.stapler.DataBoundConstructor; -public class GhprbBuildLog extends GhprbExtension implements GhprbCommentAppender, GhprbGlobalExtension { +public class GhprbBuildLog extends GhprbExtension implements GhprbCommentAppender, GhprbProjectExtension, GhprbGlobalExtension { @Extension public static final DescriptorImpl DESCRIPTOR = new DescriptorImpl(); @@ -36,7 +37,7 @@ public String postBuildComment(AbstractBuild build, TaskListener listener) StringBuilder msg = new StringBuilder(); GHCommitState state = Ghprb.getState(build); - int numLines = getLogExcerptLines(); + int numLines = getDescriptor().getLogExcerptLinesDefault(this); if (state != GHCommitState.SUCCESS && numLines > 0) { // on failure, append an excerpt of the build log @@ -60,7 +61,6 @@ public String postBuildComment(AbstractBuild build, TaskListener listener) public boolean ignorePublishedUrl() { return false; } - @Override public DescriptorImpl getDescriptor() { @@ -74,6 +74,14 @@ public static final class DescriptorImpl extends GhprbExtensionDescriptor implem public String getDisplayName() { return "Append portion of build log"; } + + public Integer getLogExcerptLinesDefault(GhprbBuildLog local) { + Integer lines = Ghprb.getDefaultValue(local, GhprbBuildLog.class, "getLogExcerptLines"); + if (lines == null) { + lines = 0; + } + return lines; + } } } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildResultMessage.java b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildResultMessage.java index 1726892f5..be52e1817 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildResultMessage.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildResultMessage.java @@ -44,7 +44,9 @@ public String postBuildComment(AbstractBuild build, TaskListener listener) if (state == result) { buildMessage = message; if (StringUtils.isEmpty(buildMessage)) { - return ""; + return ""; // will use default + } else if (buildMessage.equals("--none--")) { + return buildMessage; // will skip update } String message = Ghprb.replaceMacros(build, listener, buildMessage); // Only Append the build's custom message if it has been set. diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildStatus.java b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildStatus.java index 7a7aca3eb..9bd5f6d90 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildStatus.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildStatus.java @@ -7,7 +7,7 @@ import hudson.model.TaskListener; import hudson.model.AbstractBuild; -import org.jenkinsci.plugins.ghprb.GhprbTrigger; +import org.jenkinsci.plugins.ghprb.Ghprb; import org.jenkinsci.plugins.ghprb.extensions.GhprbCommentAppender; import org.jenkinsci.plugins.ghprb.extensions.GhprbExtension; import org.jenkinsci.plugins.ghprb.extensions.GhprbExtensionDescriptor; @@ -30,11 +30,12 @@ public GhprbBuildStatus(List messages) { public List getMessages() { return messages == null ? new ArrayList(0) : messages; } - public String postBuildComment(AbstractBuild build, TaskListener listener) { StringBuilder msg = new StringBuilder(); + List messages = getDescriptor().getMessagesDefault(this); + for (GhprbBuildResultMessage messager: messages) { msg.append(messager.postBuildComment(build, listener)); } @@ -53,21 +54,9 @@ public static class DescriptorImpl extends GhprbExtensionDescriptor implements G public String getDisplayName() { return "Build Status Messages"; } - - public List getMessageList(List messages) { - List newMessages = new ArrayList(10); - if (messages != null){ - newMessages.addAll(messages); - } else { - for(GhprbExtension extension : GhprbTrigger.getDscp().getExtensions()) { - if (extension instanceof GhprbBuildStatus) { - newMessages.addAll(((GhprbBuildStatus)extension).getMessages()); - } - } - } - return newMessages; + public List getMessagesDefault(GhprbBuildStatus local) { + return Ghprb.getDefaultValue(local, GhprbBuildStatus.class, "getMessages"); } - } } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbPublishJenkinsUrl.java b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbPublishJenkinsUrl.java index c7cde037b..efed7e559 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbPublishJenkinsUrl.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbPublishJenkinsUrl.java @@ -41,7 +41,10 @@ public String postBuildComment(AbstractBuild build, TaskListener listener) return msg.toString(); } - + + public boolean addIfMissing() { + return false; + } private String generateCustomizedMessage(AbstractBuild build) { GhprbTrigger trigger = Ghprb.extractTrigger(build); @@ -49,7 +52,7 @@ private String generateCustomizedMessage(AbstractBuild build) { return ""; } JobConfiguration jobConfiguration = JobConfiguration.builder() - .printStackTrace(trigger.isDisplayBuildErrorsOnDownstreamBuilds()).build(); + .printStackTrace(trigger.getDisplayBuildErrorsOnDownstreamBuilds()).build(); GhprbBuildManager buildManager = GhprbBuildManagerFactoryUtil.getBuildManager(build, jobConfiguration); @@ -75,6 +78,10 @@ public static final class DescriptorImpl extends GhprbExtensionDescriptor implem public String getDisplayName() { return "Add link to Jenkins"; } + + public boolean addIfMissing() { + return false; + } } } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/extensions/status/GhprbNoCommitStatus.java b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/status/GhprbNoCommitStatus.java index 94ff88dd5..bbc998fdb 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/extensions/status/GhprbNoCommitStatus.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/status/GhprbNoCommitStatus.java @@ -3,9 +3,8 @@ import hudson.Extension; import hudson.model.TaskListener; import hudson.model.AbstractBuild; +import hudson.model.AbstractProject; -import org.jenkinsci.plugins.ghprb.GhprbPullRequest; -import org.jenkinsci.plugins.ghprb.GhprbTrigger; import org.jenkinsci.plugins.ghprb.extensions.GhprbCommitStatus; import org.jenkinsci.plugins.ghprb.extensions.GhprbCommitStatusException; import org.jenkinsci.plugins.ghprb.extensions.GhprbExtension; @@ -25,15 +24,19 @@ public GhprbNoCommitStatus() { } - public void onBuildTriggered(GhprbTrigger trigger, GhprbPullRequest pr, GHRepository ghRepository) throws GhprbCommitStatusException { + public void onBuildStart(AbstractBuild build, TaskListener listener, GHRepository repo) throws GhprbCommitStatusException { } - public void onBuildStart(AbstractBuild build, TaskListener listener, GHRepository repo) throws GhprbCommitStatusException { + public void onBuildComplete(AbstractBuild build, TaskListener listener, GHRepository repo) throws GhprbCommitStatusException { } - public void onBuildComplete(AbstractBuild build, TaskListener listener, GHRepository repo) throws GhprbCommitStatusException { + public void onEnvironmentSetup(AbstractBuild build, TaskListener listener, GHRepository repo) throws GhprbCommitStatusException { + + } + + public void onBuildTriggered(AbstractProject project, String commitSha, boolean isMergeable, int prId, GHRepository ghRepository) throws GhprbCommitStatusException { } @@ -51,4 +54,5 @@ public String getDisplayName() { } + } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus.java b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus.java index b8d20688b..b6df911db 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus.java @@ -9,19 +9,19 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; +import java.util.Map; import jenkins.model.Jenkins; import org.apache.commons.lang.StringUtils; - import org.jenkinsci.plugins.ghprb.Ghprb; import org.jenkinsci.plugins.ghprb.GhprbCause; -import org.jenkinsci.plugins.ghprb.GhprbPullRequest; import org.jenkinsci.plugins.ghprb.GhprbTrigger; import org.jenkinsci.plugins.ghprb.extensions.GhprbCommitStatus; import org.jenkinsci.plugins.ghprb.extensions.GhprbCommitStatusException; import org.jenkinsci.plugins.ghprb.extensions.GhprbExtension; import org.jenkinsci.plugins.ghprb.extensions.GhprbExtensionDescriptor; +import org.jenkinsci.plugins.ghprb.extensions.GhprbGlobalDefault; import org.jenkinsci.plugins.ghprb.extensions.GhprbGlobalExtension; import org.jenkinsci.plugins.ghprb.extensions.GhprbProjectExtension; import org.jenkinsci.plugins.ghprb.extensions.comments.GhprbBuildResultMessage; @@ -32,44 +32,43 @@ import org.kohsuke.github.GHRepository; import org.kohsuke.stapler.DataBoundConstructor; -public class GhprbSimpleStatus extends GhprbExtension implements GhprbCommitStatus, GhprbGlobalExtension, GhprbProjectExtension { +public class GhprbSimpleStatus extends GhprbExtension + implements GhprbCommitStatus, GhprbGlobalExtension, GhprbProjectExtension, GhprbGlobalDefault { @Extension public static final DescriptorImpl DESCRIPTOR = new DescriptorImpl(); - + private final String commitStatusContext; private final String triggeredStatus; private final String startedStatus; private final String statusUrl; private final List completedStatus; - + public GhprbSimpleStatus() { this(null, null, null, null, new ArrayList(0)); } - + public GhprbSimpleStatus(String commitStatusContext) { this(commitStatusContext, null, null, null, new ArrayList(0)); } - + @DataBoundConstructor - public GhprbSimpleStatus( - String commitStatusContext, - String statusUrl, - String triggeredStatus, - String startedStatus, - List completedStatus - ) { + public GhprbSimpleStatus(String commitStatusContext, + String statusUrl, + String triggeredStatus, + String startedStatus, + List completedStatus) { this.statusUrl = statusUrl; this.commitStatusContext = commitStatusContext == null ? "" : commitStatusContext; this.triggeredStatus = triggeredStatus; this.startedStatus = startedStatus; this.completedStatus = completedStatus; } - + public String getStatusUrl() { return statusUrl == null ? "" : statusUrl; } - + public String getCommitStatusContext() { return commitStatusContext == null ? "" : commitStatusContext; } @@ -77,7 +76,7 @@ public String getCommitStatusContext() { public String getStartedStatus() { return startedStatus == null ? "" : startedStatus; } - + public String getTriggeredStatus() { return triggeredStatus == null ? "" : triggeredStatus; } @@ -85,38 +84,72 @@ public String getTriggeredStatus() { public List getCompletedStatus() { return completedStatus == null ? new ArrayList(0) : completedStatus; } - - public void onBuildTriggered(GhprbTrigger trigger, GhprbPullRequest pr, GHRepository ghRepository) throws GhprbCommitStatusException { + + public boolean addIfMissing() { + return true; + } + + public void onBuildTriggered(AbstractProject project, + String commitSha, + boolean isMergeable, + int prId, + GHRepository ghRepository) throws GhprbCommitStatusException { StringBuilder sb = new StringBuilder(); GHCommitState state = GHCommitState.PENDING; - - AbstractProject project = trigger.getActualProject(); - + String triggeredStatus = getDescriptor().getTriggeredStatusDefault(this); + + // check if we even need to update + if (StringUtils.equals(triggeredStatus, "--none--")) { + return; + } + + String statusUrl = getDescriptor().getStatusUrlDefault(this); + String commitStatusContext = getDescriptor().getCommitStatusContextDefault(this); + String context = Util.fixEmpty(commitStatusContext); context = Ghprb.replaceMacros(project, context); - + if (!StringUtils.isEmpty(triggeredStatus)) { sb.append(Ghprb.replaceMacros(project, triggeredStatus)); } else { sb.append("Build triggered."); - if (pr.isMergeable()) { + if (isMergeable) { sb.append(" sha1 is merged."); } else { sb.append(" sha1 is original commit."); } } - + String url = Ghprb.replaceMacros(project, statusUrl); + if (StringUtils.equals(statusUrl, "--none--")) { + url = ""; + } String message = sb.toString(); try { - ghRepository.createCommitStatus(pr.getHead(), state, url, message, context); + ghRepository.createCommitStatus(commitSha, state, url, message, context); } catch (IOException e) { - throw new GhprbCommitStatusException(e, state, message, pr.getId()); + throw new GhprbCommitStatusException(e, state, message, prId); } } - public void onBuildStart(AbstractBuild build, TaskListener listener, GHRepository repo) throws GhprbCommitStatusException { + public void onEnvironmentSetup(AbstractBuild build, + TaskListener listener, + GHRepository repo) throws GhprbCommitStatusException { + // no need to create a commit here -- the onBuildStart() event will fire + // soon and will respect's the user's settings for startedStatus. + } + + public void onBuildStart(AbstractBuild build, + TaskListener listener, + GHRepository repo) throws GhprbCommitStatusException { + String startedStatus = getDescriptor().getStartedStatusDefault(this); + + // check if we even need to update + if (StringUtils.equals(startedStatus, "--none--")) { + return; + } + GhprbCause c = Ghprb.getCause(build); StringBuilder sb = new StringBuilder(); if (StringUtils.isEmpty(startedStatus)) { @@ -128,8 +161,11 @@ public void onBuildStart(AbstractBuild build, TaskListener listener, GHRep createCommitStatus(build, listener, sb.toString(), repo, GHCommitState.PENDING); } - public void onBuildComplete(AbstractBuild build, TaskListener listener, GHRepository repo) throws GhprbCommitStatusException { - + public void onBuildComplete(AbstractBuild build, + TaskListener listener, + GHRepository repo) throws GhprbCommitStatusException { + List completedStatus = getDescriptor().getCompletedStatusDefault(this); + GHCommitState state = Ghprb.getState(build); StringBuilder sb = new StringBuilder(); @@ -140,45 +176,67 @@ public void onBuildComplete(AbstractBuild build, TaskListener listener, GH for (GhprbBuildResultMessage buildStatus : completedStatus) { sb.append(buildStatus.postBuildComment(build, listener)); } + if (StringUtils.equals(sb.toString(), "--none--")) { + return; + } } - + sb.append(" "); GhprbTrigger trigger = Ghprb.extractTrigger(build); if (trigger == null) { listener.getLogger().println("Unable to get pull request builder trigger!!"); } else { JobConfiguration jobConfiguration = - JobConfiguration.builder() - .printStackTrace(trigger.isDisplayBuildErrorsOnDownstreamBuilds()) - .build(); + JobConfiguration.builder() + .printStackTrace(trigger.getDisplayBuildErrorsOnDownstreamBuilds()).build(); - GhprbBuildManager buildManager = - GhprbBuildManagerFactoryUtil.getBuildManager(build, jobConfiguration); + GhprbBuildManager buildManager = GhprbBuildManagerFactoryUtil.getBuildManager(build, jobConfiguration); sb.append(buildManager.getOneLineTestResults()); } - + createCommitStatus(build, listener, sb.toString(), repo, state); } - private void createCommitStatus(AbstractBuild build, TaskListener listener, String message, GHRepository repo, GHCommitState state) throws GhprbCommitStatusException { - GhprbCause cause = Ghprb.getCause(build); - - String sha1 = cause.getCommit(); - String url = Jenkins.getInstance().getRootUrl() + build.getUrl(); - if (!StringUtils.isEmpty(statusUrl)) { - url = Ghprb.replaceMacros(build, listener, statusUrl); + private void createCommitStatus(AbstractBuild build, + TaskListener listener, + String message, + GHRepository repo, + GHCommitState state) throws GhprbCommitStatusException { + + Map envVars = Ghprb.getEnvVars(build, listener); + + String sha1 = envVars.get("ghprbActualCommit"); + Integer pullId = Integer.parseInt(envVars.get("ghprbPullId")); + + String url = envVars.get("BUILD_URL"); + if (StringUtils.isEmpty(url)) { + url = envVars.get("JOB_URL"); + } + if (StringUtils.isEmpty(url)) { + url = Jenkins.getInstance().getRootUrl() + build.getUrl(); } + + if (StringUtils.equals(statusUrl, "--none--")) { + url = ""; + } else if (!StringUtils.isEmpty(statusUrl)) { + url = Ghprb.replaceMacros(build, listener, statusUrl); + } + String context = Util.fixEmpty(commitStatusContext); context = Ghprb.replaceMacros(build, listener, context); - - listener.getLogger().println(String.format("Setting status of %s to %s with url %s and message: '%s'", sha1, state, url, message)); + + listener.getLogger().println(String.format("Setting status of %s to %s with url %s and message: '%s'", + sha1, + state, + url, + message)); if (context != null) { listener.getLogger().println(String.format("Using context: " + context)); } try { repo.createCommitStatus(sha1, state, url, message, context); } catch (IOException e) { - throw new GhprbCommitStatusException(e, state, message, cause.getPullID()); + throw new GhprbCommitStatusException(e, state, message, pullId); } } @@ -186,66 +244,38 @@ private void createCommitStatus(AbstractBuild build, TaskListener listener public DescriptorImpl getDescriptor() { return DESCRIPTOR; } - - public static final class DescriptorImpl extends GhprbExtensionDescriptor implements GhprbGlobalExtension, GhprbProjectExtension { - + + public static final class DescriptorImpl extends GhprbExtensionDescriptor + implements GhprbGlobalExtension, GhprbProjectExtension { + @Override public String getDisplayName() { return "Update commit status during build"; } - - public String getTriggeredStatusDefault(String triggeredStatusLocal) { - String triggeredStatus = triggeredStatusLocal; - if (triggeredStatus == null) { - for(GhprbExtension extension : GhprbTrigger.getDscp().getExtensions()) { - if (extension instanceof GhprbSimpleStatus) { - triggeredStatus = ((GhprbSimpleStatus) extension).getTriggeredStatus(); - break; - } - } - } - return triggeredStatus; - } - - public String getStartedStatusDefault(String startedStatusLocal) { - String startedStatus = startedStatusLocal; - if (startedStatus == null) { - for(GhprbExtension extension : GhprbTrigger.getDscp().getExtensions()) { - if (extension instanceof GhprbSimpleStatus) { - startedStatus = ((GhprbSimpleStatus) extension).getStartedStatus(); - break; - } - } - } - return startedStatus; - } - - public List getCompletedStatusList(List completedStatusLocal) { - List completedStatus = completedStatusLocal; - if (completedStatus == null) { - for(GhprbExtension extension : GhprbTrigger.getDscp().getExtensions()) { - if (extension instanceof GhprbSimpleStatus) { - completedStatus = ((GhprbSimpleStatus) extension).getCompletedStatus(); - break; - } - } - } - return completedStatus; - } - - public String getCommitContextDefault(String commitStatusContextLocal){ - String commitStatusContext = commitStatusContextLocal; - if (commitStatusContext == null) { - for(GhprbExtension extension : GhprbTrigger.getDscp().getExtensions()) { - if (extension instanceof GhprbSimpleStatus) { - commitStatusContext = ((GhprbSimpleStatus) extension).getCommitStatusContext(); - break; - } - } - } - return commitStatusContext; + + public String getTriggeredStatusDefault(GhprbSimpleStatus local) { + return Ghprb.getDefaultValue(local, GhprbSimpleStatus.class, "getTriggeredStatus"); } - } + public String getStatusUrlDefault(GhprbSimpleStatus local) { + return Ghprb.getDefaultValue(local, GhprbSimpleStatus.class, "getStatusUrl"); + } + public String getStartedStatusDefault(GhprbSimpleStatus local) { + return Ghprb.getDefaultValue(local, GhprbSimpleStatus.class, "getStartedStatus"); + } + + public List getCompletedStatusDefault(GhprbSimpleStatus local) { + return Ghprb.getDefaultValue(local, GhprbSimpleStatus.class, "getCompletedStatus"); + } + + public String getCommitStatusContextDefault(GhprbSimpleStatus local) { + return Ghprb.getDefaultValue(local, GhprbSimpleStatus.class, "getCommitStatusContext"); + } + + public boolean addIfMissing() { + return false; + } + + } } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/jobdsl/GhprbContextExtensionPoint.java b/src/main/java/org/jenkinsci/plugins/ghprb/jobdsl/GhprbContextExtensionPoint.java new file mode 100644 index 000000000..ae4c5593b --- /dev/null +++ b/src/main/java/org/jenkinsci/plugins/ghprb/jobdsl/GhprbContextExtensionPoint.java @@ -0,0 +1,72 @@ +package org.jenkinsci.plugins.ghprb.jobdsl; + +import antlr.ANTLRException; +import com.google.common.base.Joiner; +import hudson.Extension; +import javaposse.jobdsl.dsl.helpers.publisher.PublisherContext; +import javaposse.jobdsl.dsl.helpers.triggers.TriggerContext; +import javaposse.jobdsl.dsl.helpers.wrapper.WrapperContext; +import javaposse.jobdsl.plugin.ContextExtensionPoint; +import javaposse.jobdsl.plugin.DslExtensionMethod; +import org.jenkinsci.plugins.ghprb.GhprbPullRequestMerge; +import org.jenkinsci.plugins.ghprb.GhprbTrigger; +import org.jenkinsci.plugins.ghprb.upstream.GhprbUpstreamStatus; + + +@Extension(optional = true) +public class GhprbContextExtensionPoint extends ContextExtensionPoint { + @DslExtensionMethod(context = TriggerContext.class) + public Object githubPullRequest(Runnable closure) throws ANTLRException { + GhprbTriggerContext context = new GhprbTriggerContext(); + executeInContext(closure, context); + return new GhprbTrigger( + Joiner.on("\n").join(context.admins), + Joiner.on("\n").join(context.userWhitelist), + Joiner.on("\n").join(context.orgWhitelist), + context.cron, + context.triggerPhrase, + context.onlyTriggerPhrase, + context.useGitHubHooks, + context.permitAll, + context.autoCloseFailedPullRequests, + context.displayBuildErrorsOnDownstreamBuilds, + null, + context.whiteListTargetBranches, + context.allowMembersOfWhitelistedOrgsAsAdmin, + null, + null, + null, + null, + null, + context.extensionContext.extensions + ); + } + + @DslExtensionMethod(context = PublisherContext.class) + public Object mergeGithubPullRequest(Runnable closure) { + GhprbPullRequestMergeContext context = new GhprbPullRequestMergeContext(); + executeInContext(closure, context); + + return new GhprbPullRequestMerge( + context.mergeComment, + context.onlyAdminsMerge, + context.disallowOwnCode, + context.failOnNonMerge, + context.deleteOnMerge + ); + } + + @DslExtensionMethod(context = WrapperContext.class) + public Object downstreamCommitStatus(Runnable closure) { + GhprbUpstreamStatusContext context = new GhprbUpstreamStatusContext(); + executeInContext(closure, context); + + return new GhprbUpstreamStatus( + context.context, + context.statusUrl, + context.triggeredStatus, + context.startedStatus, + context.completedStatus + ); + } +} diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/jobdsl/GhprbExtensionContext.java b/src/main/java/org/jenkinsci/plugins/ghprb/jobdsl/GhprbExtensionContext.java new file mode 100644 index 000000000..70e02c7fb --- /dev/null +++ b/src/main/java/org/jenkinsci/plugins/ghprb/jobdsl/GhprbExtensionContext.java @@ -0,0 +1,29 @@ +package org.jenkinsci.plugins.ghprb.jobdsl; + +import javaposse.jobdsl.dsl.Context; +import javaposse.jobdsl.plugin.ContextExtensionPoint; +import org.jenkinsci.plugins.ghprb.extensions.GhprbExtension; +import org.jenkinsci.plugins.ghprb.extensions.status.GhprbSimpleStatus; + +import java.util.ArrayList; +import java.util.List; + +class GhprbExtensionContext implements Context { + List extensions = new ArrayList(); + + /** + * Updates the commit status during the build. + */ + void commitStatus(Runnable closure) { + GhprbSimpleStatusContext context = new GhprbSimpleStatusContext(); + ContextExtensionPoint.executeInContext(closure, context); + + extensions.add(new GhprbSimpleStatus( + context.context, + context.statusUrl, + context.triggeredStatus, + context.startedStatus, + context.completedStatus + )); + } +} diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/jobdsl/GhprbPullRequestMergeContext.java b/src/main/java/org/jenkinsci/plugins/ghprb/jobdsl/GhprbPullRequestMergeContext.java new file mode 100644 index 000000000..eead87e0b --- /dev/null +++ b/src/main/java/org/jenkinsci/plugins/ghprb/jobdsl/GhprbPullRequestMergeContext.java @@ -0,0 +1,74 @@ +package org.jenkinsci.plugins.ghprb.jobdsl; + +import javaposse.jobdsl.dsl.Context; + +public class GhprbPullRequestMergeContext implements Context { + String mergeComment; + boolean onlyAdminsMerge; + boolean disallowOwnCode; + boolean failOnNonMerge; + boolean deleteOnMerge; + + /** + * Sets a comment that should show up when the merge command is sent to GitHub. + */ + public void mergeComment(String mergeComment) { + this.mergeComment = mergeComment; + } + + /** + * Allows only admin users to trigger a pull request merge. Defaults to {@code false}. + */ + public void onlyAdminsMerge(boolean onlyAdminsMerge) { + this.onlyAdminsMerge = onlyAdminsMerge; + } + + /** + * Allows only admin users to trigger a pull request merge. Defaults to {@code false}. + */ + public void onlyAdminsMerge() { + onlyAdminsMerge(true); + } + + /** + * Disallows a user to merge their own code. Defaults to {@code false}. + */ + public void disallowOwnCode(boolean disallowOwnCode) { + this.disallowOwnCode = disallowOwnCode; + } + + /** + * Disallows a user to merge their own code. Defaults to {@code false}. + */ + public void disallowOwnCode() { + disallowOwnCode(true); + } + + /** + * Fails the build if the pull request can't be merged. Defaults to {@code false}. + */ + public void failOnNonMerge(boolean failOnNonMerge) { + this.failOnNonMerge = failOnNonMerge; + } + + /** + * Fails the build if the pull request can't be merged. Defaults to {@code false}. + */ + public void failOnNonMerge() { + failOnNonMerge(true); + } + + /** + * Deletes the branch after a successful merge. Defaults to {@code false}. + */ + public void deleteOnMerge(boolean deleteOnMerge) { + this.deleteOnMerge = deleteOnMerge; + } + + /** + * Deletes the branch after a successful merge. Defaults to {@code false}. + */ + public void deleteOnMerge() { + deleteOnMerge(true); + } +} diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/jobdsl/GhprbSimpleStatusContext.java b/src/main/java/org/jenkinsci/plugins/ghprb/jobdsl/GhprbSimpleStatusContext.java new file mode 100644 index 000000000..0d71cd06f --- /dev/null +++ b/src/main/java/org/jenkinsci/plugins/ghprb/jobdsl/GhprbSimpleStatusContext.java @@ -0,0 +1,55 @@ +package org.jenkinsci.plugins.ghprb.jobdsl; + +import javaposse.jobdsl.dsl.Context; +import org.jenkinsci.plugins.ghprb.extensions.comments.GhprbBuildResultMessage; +import org.kohsuke.github.GHCommitState; + +import java.util.ArrayList; +import java.util.List; + +class GhprbSimpleStatusContext implements Context { + String context; + String triggeredStatus; + String startedStatus; + String statusUrl; + List completedStatus = new ArrayList(); + + /** + * A string label to differentiate this status from the status of other systems. + */ + void context(String context) { + this.context = context; + } + + /** + * Use a custom status for when a build is triggered. + */ + void triggeredStatus(String triggeredStatus) { + this.triggeredStatus = triggeredStatus; + } + + /** + * Use a custom status for when a build is started. + */ + void startedStatus(String startedStatus) { + this.startedStatus = startedStatus; + } + + /** + * Use a custom URL instead of the job default. + */ + void statusUrl(String statusUrl) { + this.statusUrl = statusUrl; + } + + /** + * Use a custom status for when a build is completed. Can be called multiple times to set messages for different + * build results. Valid build results are {@code 'SUCCESS'}, {@code 'FAILURE'}, and {@code 'ERROR'}. + */ + void completedStatus(String buildResult, String message) { + completedStatus.add(new GhprbBuildResultMessage( + GHCommitState.valueOf(buildResult), + message + )); + } +} diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/jobdsl/GhprbTriggerContext.java b/src/main/java/org/jenkinsci/plugins/ghprb/jobdsl/GhprbTriggerContext.java new file mode 100644 index 000000000..0e6638bdf --- /dev/null +++ b/src/main/java/org/jenkinsci/plugins/ghprb/jobdsl/GhprbTriggerContext.java @@ -0,0 +1,194 @@ +package org.jenkinsci.plugins.ghprb.jobdsl; + +import javaposse.jobdsl.dsl.Context; +import javaposse.jobdsl.plugin.ContextExtensionPoint; +import org.jenkinsci.plugins.ghprb.GhprbBranch; + +import java.util.ArrayList; +import java.util.List; + +class GhprbTriggerContext implements Context { + List admins = new ArrayList(); + List userWhitelist = new ArrayList(); + List orgWhitelist = new ArrayList(); + List whiteListTargetBranches = new ArrayList(); + String cron = "H/5 * * * *"; + String triggerPhrase; + boolean onlyTriggerPhrase; + boolean useGitHubHooks; + boolean permitAll; + boolean autoCloseFailedPullRequests; + boolean allowMembersOfWhitelistedOrgsAsAdmin; + boolean displayBuildErrorsOnDownstreamBuilds; + GhprbExtensionContext extensionContext = new GhprbExtensionContext(); + + /** + * Adds admins for this job. + */ + public void admin(String admin) { + admins.add(admin); + } + + /** + * Adds admins for this job. + */ + public void admins(Iterable admins) { + for (String admin : admins) { + admin(admin); + } + } + + /** + * Adds whitelisted users for this job. + */ + public void userWhitelist(String user) { + userWhitelist.add(user); + } + + /** + * Adds whitelisted users for this job. + */ + public void userWhitelist(Iterable users) { + for (String user : users) { + userWhitelist(user); + } + } + + /** + * Adds organisation names whose members are considered whitelisted for this specific job. + */ + public void orgWhitelist(String organization) { + orgWhitelist.add(organization); + } + + /** + * Adds organisation names whose members are considered whitelisted for this specific job. + */ + public void orgWhitelist(Iterable organizations) { + for (String organization : organizations) { + orgWhitelist(organization); + } + } + + + /** + * Add branch names whose they are considered whitelisted for this specific job + */ + public void whiteListTargetBranch(String branch) { + whiteListTargetBranches.add(new GhprbBranch(branch)); + } + + /** + * Add branch names whose they are considered whitelisted for this specific job + */ + public void whiteListTargetBranches(Iterable branches) { + for (String branch : branches) { + whiteListTargetBranches.add(new GhprbBranch(branch)); + } + } + + /** + * This schedules polling to GitHub for new changes in pull requests. + */ + public void cron(String cron) { + this.cron = cron; + } + + /** + * When filled, commenting this phrase in the pull request will trigger a build. + */ + public void triggerPhrase(String triggerPhrase) { + this.triggerPhrase = triggerPhrase; + } + + /** + * When set, only commenting the trigger phrase in the pull request will trigger a build. + */ + public void onlyTriggerPhrase(boolean onlyTriggerPhrase) { + this.onlyTriggerPhrase = onlyTriggerPhrase; + } + + /** + * When set, only commenting the trigger phrase in the pull request will trigger a build. + */ + public void onlyTriggerPhrase() { + onlyTriggerPhrase(true); + } + + /** + * Checking this option will disable regular polling for changes in GitHub and will try to create a GitHub hook. + */ + public void useGitHubHooks(boolean useGitHubHooks) { + this.useGitHubHooks = useGitHubHooks; + } + + /** + * Checking this option will disable regular polling for changes in GitHub and will try to create a GitHub hook. + */ + public void useGitHubHooks() { + useGitHubHooks(true); + } + + /** + * Build every pull request automatically without asking. + */ + public void permitAll(boolean permitAll) { + this.permitAll = permitAll; + } + + /** + * Build every pull request automatically without asking. + */ + public void permitAll() { + permitAll(true); + } + + /** + * Close pull request automatically when the build fails. + */ + public void autoCloseFailedPullRequests(boolean autoCloseFailedPullRequests) { + this.autoCloseFailedPullRequests = autoCloseFailedPullRequests; + } + + /** + * Close pull request automatically when the build fails. + */ + public void autoCloseFailedPullRequests() { + autoCloseFailedPullRequests(true); + } + + /** + * Allows members of whitelisted organisations to behave like admins. + */ + public void allowMembersOfWhitelistedOrgsAsAdmin(boolean allowMembersOfWhitelistedOrgsAsAdmin) { + this.allowMembersOfWhitelistedOrgsAsAdmin = allowMembersOfWhitelistedOrgsAsAdmin; + } + + /** + * Allows members of whitelisted organisations to behave like admins. + */ + public void allowMembersOfWhitelistedOrgsAsAdmin() { + allowMembersOfWhitelistedOrgsAsAdmin(true); + } + + /** + * Allow this upstream job to get commit statuses from downstream builds + */ + public void displayBuildErrorsOnDownstreamBuilds(boolean displayBuildErrorsOnDownstreamBuilds) { + this.displayBuildErrorsOnDownstreamBuilds = displayBuildErrorsOnDownstreamBuilds; + } + + /** + * Allow this upstream job to get commit statuses from downstream builds + */ + public void displayBuildErrorsOnDownstreamBuilds() { + displayBuildErrorsOnDownstreamBuilds(true); + } + + /** + * Adds additional trigger options. + */ + public void extensions(Runnable closure) { + ContextExtensionPoint.executeInContext(closure, extensionContext); + } +} diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/jobdsl/GhprbUpstreamStatusContext.java b/src/main/java/org/jenkinsci/plugins/ghprb/jobdsl/GhprbUpstreamStatusContext.java new file mode 100644 index 000000000..899ae2c85 --- /dev/null +++ b/src/main/java/org/jenkinsci/plugins/ghprb/jobdsl/GhprbUpstreamStatusContext.java @@ -0,0 +1,55 @@ +package org.jenkinsci.plugins.ghprb.jobdsl; + +import javaposse.jobdsl.dsl.Context; +import org.jenkinsci.plugins.ghprb.extensions.comments.GhprbBuildResultMessage; +import org.kohsuke.github.GHCommitState; + +import java.util.ArrayList; +import java.util.List; + +class GhprbUpstreamStatusContext implements Context { + String context; + String triggeredStatus; + String startedStatus; + String statusUrl; + List completedStatus = new ArrayList(); + + /** + * A string label to differentiate this status from the status of other systems. + */ + void context(String context) { + this.context = context; + } + + /** + * Use a custom status for when a build is triggered. + */ + void triggeredStatus(String triggeredStatus) { + this.triggeredStatus = triggeredStatus; + } + + /** + * Use a custom status for when a build is started. + */ + void startedStatus(String startedStatus) { + this.startedStatus = startedStatus; + } + + /** + * Use a custom URL instead of the job default. + */ + void statusUrl(String statusUrl) { + this.statusUrl = statusUrl; + } + + /** + * Use a custom status for when a build is completed. Can be called multiple times to set messages for different + * build results. Valid build results are {@code 'SUCCESS'}, {@code 'FAILURE'}, and {@code 'ERROR'}. + */ + void completedStatus(String buildResult, String message) { + completedStatus.add(new GhprbBuildResultMessage( + GHCommitState.valueOf(buildResult), + message + )); + } +} diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbBaseBuildManager.java b/src/main/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbBaseBuildManager.java index 97ad4acad..1a136d640 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbBaseBuildManager.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbBaseBuildManager.java @@ -12,8 +12,8 @@ import hudson.model.AbstractBuild; import hudson.model.AbstractProject; import hudson.model.Result; +import hudson.tasks.test.AbstractTestResultAction; import hudson.tasks.junit.TestResult; -import hudson.tasks.junit.TestResultAction; import hudson.tasks.junit.CaseResult; import hudson.tasks.test.AggregatedTestResultAction; import hudson.tasks.test.AggregatedTestResultAction.ChildReport; @@ -172,7 +172,7 @@ protected String getAggregatedTestResults(AbstractBuild build) { public String getOneLineTestResults() { - TestResultAction testResultAction = build.getAction(TestResultAction.class); + AbstractTestResultAction testResultAction = build.getAction(AbstractTestResultAction.class); if (testResultAction == null) { return "No test results found."; diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus.java b/src/main/java/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus.java new file mode 100644 index 000000000..d462c6328 --- /dev/null +++ b/src/main/java/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus.java @@ -0,0 +1,120 @@ +package org.jenkinsci.plugins.ghprb.upstream; + +import hudson.Extension; +import hudson.Launcher; +import hudson.model.*; +import hudson.tasks.BuildWrapper; + +import org.jenkinsci.plugins.ghprb.extensions.comments.GhprbBuildResultMessage; +import org.kohsuke.github.GHCommitState; +import org.kohsuke.stapler.DataBoundConstructor; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; + +/** + * @author Kevin Suwala + */ + +public class GhprbUpstreamStatus extends BuildWrapper { + + private final String commitStatusContext; + private final String triggeredStatus; + private final String startedStatus; + private final String statusUrl; + private final List completedStatus; + + // sets the context and message as env vars so that they are available in the Listener class + @Override + public void makeBuildVariables(@SuppressWarnings("rawtypes") AbstractBuild build, Map variables){ + variables.put("ghprbUpstreamStatus", "true"); + variables.put("ghprbCommitStatusContext", getCommitStatusContext()); + variables.put("ghprbTriggeredStatus", getTriggeredStatus()); + variables.put("ghprbStartedStatus", getStartedStatus()); + variables.put("ghprbStatusUrl", getStatusUrl()); + + Map statusMessages = new HashMap(5); + + for (GhprbBuildResultMessage message : getCompletedStatus()) { + GHCommitState state = message.getResult(); + StringBuilder sb; + if (!statusMessages.containsKey(state)) { + sb = new StringBuilder(); + statusMessages.put(state, sb); + } else { + sb = statusMessages.get(state); + sb.append("\n"); + } + sb.append(message.getMessage()); + } + + for (Entry next : statusMessages.entrySet()) { + String key = String.format("ghprb%sMessage", next.getKey().name()); + variables.put(key, next.getValue().toString()); + } + } + + @Override + @SuppressWarnings("unchecked") + public BuildWrapper.Environment setUp(@SuppressWarnings("rawtypes") AbstractBuild build, Launcher launcher, BuildListener listener) throws IOException, InterruptedException { + makeBuildVariables(build, build.getBuildVariables()); + return new Environment(){}; + } + + @Override + @SuppressWarnings("unchecked") + public void preCheckout(@SuppressWarnings("rawtypes") AbstractBuild build, Launcher launcher, BuildListener listener) throws IOException, InterruptedException { + makeBuildVariables(build, build.getBuildVariables()); + } + + @DataBoundConstructor + public GhprbUpstreamStatus( + String commitStatusContext, + String statusUrl, + String triggeredStatus, + String startedStatus, + List completedStatus + ) { + this.statusUrl = statusUrl; + this.commitStatusContext = commitStatusContext == null ? "" : commitStatusContext; + this.triggeredStatus = triggeredStatus; + this.startedStatus = startedStatus; + this.completedStatus = completedStatus; + } + + + public String getStatusUrl() { + return statusUrl == null ? "" : statusUrl; + } + + public String getCommitStatusContext() { + return commitStatusContext == null ? "" : commitStatusContext; + } + + public String getStartedStatus() { + return startedStatus == null ? "" : startedStatus; + } + + public String getTriggeredStatus() { + return triggeredStatus == null ? "" : triggeredStatus; + } + + public List getCompletedStatus() { + return completedStatus == null ? new ArrayList(0) : completedStatus; + } + + @Extension + public static final class DescriptorImpl extends Descriptor { + + @Override + public String getDisplayName() { + return "Set GitHub commit status with custom context and message (Must configure upstream job using GHPRB trigger)"; + } + } + + +} diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatusListener.java b/src/main/java/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatusListener.java new file mode 100644 index 000000000..b9beaa09c --- /dev/null +++ b/src/main/java/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatusListener.java @@ -0,0 +1,127 @@ +package org.jenkinsci.plugins.ghprb.upstream; + +import hudson.Extension; +import hudson.Launcher; +import hudson.model.BuildListener; +import hudson.model.Environment; +import hudson.model.TaskListener; +import hudson.model.AbstractBuild; +import hudson.model.listeners.RunListener; + +import org.apache.commons.lang.StringUtils; +import org.jenkinsci.plugins.ghprb.Ghprb; +import org.jenkinsci.plugins.ghprb.GhprbGitHubAuth; +import org.jenkinsci.plugins.ghprb.GhprbTrigger; +import org.jenkinsci.plugins.ghprb.extensions.GhprbCommitStatusException; +import org.jenkinsci.plugins.ghprb.extensions.comments.GhprbBuildResultMessage; +import org.jenkinsci.plugins.ghprb.extensions.status.GhprbSimpleStatus; +import org.kohsuke.github.GHCommitState; +import org.kohsuke.github.GHRepository; +import org.kohsuke.github.GitHub; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.logging.Level; +import java.util.logging.Logger; + +/** + * @author Kevin Suwala + * This class is responsible for sending the custom status and message on + * downstream jobs that have the option configured. + */ + +@Extension +public class GhprbUpstreamStatusListener extends RunListener> { + private static final Logger logger = Logger.getLogger(GhprbUpstreamStatusListener.class.getName()); + + private GhprbSimpleStatus statusUpdater; + + private GHRepository repo; + + // Gets all the custom env vars needed to send information to GitHub + private boolean updateEnvironmentVars(AbstractBuild build, TaskListener listener){ + + + Map envVars = Ghprb.getEnvVars(build, listener); + + if (!envVars.containsKey("ghprbUpstreamStatus")) { + return false; + } + + String jobName = envVars.get("JOB_NAME"); + + List statusMessages = new ArrayList(5); + + for (GHCommitState state : GHCommitState.values()) { + String envVar = String.format("ghprb%sMessage", state.name()); + String message = envVars.get(envVar); + statusMessages.add(new GhprbBuildResultMessage(state, message)); + } + + String context = envVars.get("commitStatusContext"); + + if (StringUtils.isEmpty(context)) { + context = jobName; + } + + statusUpdater = new GhprbSimpleStatus(envVars.get("ghprbCommitStatusContext"), envVars.get("ghprbStatusUrl"), envVars.get("ghprbTriggeredStatus"), envVars.get("ghprbStartedStatus"), statusMessages); + + String credentialsId = envVars.get("ghprbCredentialsId"); + String repoName = envVars.get("ghprbGhRepository"); + + GhprbGitHubAuth auth = GhprbTrigger.getDscp().getGitHubAuth(credentialsId); + try { + GitHub gh = auth.getConnection(build.getProject()); + repo = gh.getRepository(repoName); + return true; + } catch (Exception e) { + logger.log(Level.SEVERE, "Unable to connect to GitHub repo", e); + return false; + } + + } + + // Sets the status as pending when the job starts and then calls the createCommitStatus method to send it to GitHub + @Override + public Environment setUpEnvironment(@SuppressWarnings("rawtypes") AbstractBuild build, Launcher launcher, BuildListener listener) { + if (updateEnvironmentVars(build, listener)) { + logger.log(Level.FINE, "Job: " + build.getFullDisplayName() + " Attempting to send GitHub commit status"); + + try { + statusUpdater.onEnvironmentSetup(build, listener, repo); + } catch (GhprbCommitStatusException e) { + e.printStackTrace(); + } + } + + return new Environment(){}; + } + + @Override + public void onStarted(AbstractBuild build, TaskListener listener) { + if (!updateEnvironmentVars(build, listener)) { + return; + } + + try { + statusUpdater.onBuildStart(build, listener, repo); + } catch (GhprbCommitStatusException e) { + e.printStackTrace(); + } + } + + // Sets the status to the build result when the job is done, and then calls the createCommitStatus method to send it to GitHub + @Override + public void onCompleted(AbstractBuild build, TaskListener listener) { + if (!updateEnvironmentVars(build, listener)) { + return; + } + + try { + statusUpdater.onBuildComplete(build, listener, repo); + } catch (GhprbCommitStatusException e) { + e.printStackTrace(); + } + } +} diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth/config.groovy b/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth/config.groovy index 6aefeca56..8bd491efb 100644 --- a/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth/config.groovy +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth/config.groovy @@ -5,6 +5,10 @@ f.entry(title:_("GitHub Server API URL"), field:"serverAPIUrl") { f.textbox() } +f.entry(title:_("Jenkins URL override"), field:"jenkinsUrl") { + f.textbox() +} + f.entry(title:_("Shared secret"), field:"secret") { f.password() } @@ -12,7 +16,7 @@ f.entry(title:_("Shared secret"), field:"secret") { f.entry(title:_("Credentials"), field:"credentialsId") { c.select(onchange="""{ var self = this.targetElement ? this.targetElement : this; - var r = findPreviousFormItem(self,'serverAPIUrl'); + var r = findPreviousFormItem(self,'serverAPIUrl','credentialsId'); r.onchange(r); self = null; r = null; @@ -20,12 +24,46 @@ f.entry(title:_("Credentials"), field:"credentialsId") { } f.advanced(title:_("Test Credentials")) { - f.entry(title:_("Test Credentials")) { - f.validateButton(title:_("Connect to API"), progress:_("Connecting..."), with:"serverAPIUrl,credentialsId", method:"testGithubAccess") - f.entry(title:_("Repository owner/name"), field:"repo") { + f.optionalBlock(title:_("Test basic connection to GitHub")) { + f.entry() { + f.validateButton(title:_("Connect to API"), progress:_("Connecting..."), with:"serverAPIUrl,credentialsId", method:"testGithubAccess") + } + } + + f.entry(title:_("Repository owner/name"), field:"repo") { + f.textbox() + } + f.optionalBlock(title:_("Test Permissions to a Repository")) { + f.entry() { + f.validateButton(title:_("Check repo permissions"), progress:_("Checking..."), with:"serverAPIUrl,credentialsId,repo", method:"checkRepoAccess") + } + } + f.optionalBlock(title:_("Test adding comment to Pull Request")) { + f.entry(title:_("Issue ID"), field:"issueId") { + f.number() + } + f.entry(title:_("Comment to post"), field:"message1") { + f.textbox() + } + f.validateButton(title:_("Comment to issue"), progress:_("Commenting..."), with:"serverAPIUrl,credentialsId,repo,issueId,message1", method:"testComment") + } + f.optionalBlock(title:_("Test updating commit status")) { + f.entry(title:_("Commit SHA"), field:"sha1") { + f.textbox() + } + f.entry(title:_("Commit State"), field:"state") { + f.select() + } + f.entry(title:_("Status url"), field:"url") { + f.textbox() + } + f.entry(title:_("Message to post"), field:"message2") { + f.textbox() + } + f.entry(title:_("Context for the status"), field:"context") { f.textbox() } - f.validateButton(title:_("Check repo permissions"), progress:_("Connecting..."), with:"serverAPIUrl,credentialsId,repo", method:"checkRepoAccess") + f.validateButton(title:_("Update status"), progress:_("Updating..."), with:"serverAPIUrl,credentialsId,repo,sha1,state,url,message2,context", method:"testUpdateStatus") } } diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth/help-jenkinsUrl.html b/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth/help-jenkinsUrl.html new file mode 100644 index 000000000..361b94778 --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth/help-jenkinsUrl.html @@ -0,0 +1,3 @@ +
+ Use this to override the Jenkins URL that GitHub should call. +
\ No newline at end of file diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge/config.jelly b/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge/config.jelly index d36347b90..c720ee69c 100644 --- a/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge/config.jelly +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge/config.jelly @@ -2,13 +2,16 @@ - - - + + + + + + diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge/global.jelly b/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge/global.jelly index acb0a3d2e..2f5c4993a 100644 --- a/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge/global.jelly +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge/global.jelly @@ -3,9 +3,6 @@ - - - diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge/help-onlyTriggerPhrase.html b/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge/help-onlyTriggerPhrase.html deleted file mode 100644 index 6a5ba1cec..000000000 --- a/src/main/resources/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge/help-onlyTriggerPhrase.html +++ /dev/null @@ -1,4 +0,0 @@ -
- When checked, only commenting the trigger phrase in the pull request will trigger a build. - All other methods of triggering a pull request build are disabled. -
\ No newline at end of file diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/builds/GhprbCancelBuildsOnUpdate/config.jelly b/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/builds/GhprbCancelBuildsOnUpdate/config.jelly new file mode 100644 index 000000000..eb333a89f --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/builds/GhprbCancelBuildsOnUpdate/config.jelly @@ -0,0 +1,5 @@ + + + + + diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildLog/config.jelly b/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildLog/config.jelly index 3b79b49b7..22345540d 100644 --- a/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildLog/config.jelly +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildLog/config.jelly @@ -1,5 +1,5 @@ - + diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildResultMessage/help-message.html b/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildResultMessage/help-message.html index a86712a55..f06228927 100644 --- a/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildResultMessage/help-message.html +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildResultMessage/help-message.html @@ -1,3 +1,5 @@
The message that is appended to a comment when a build finishes with the desired build status. -
\ No newline at end of file + If no status updates should be made when a build finishes with the indicated + build status, use "--none--" to alert the trigger. + diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildStatus/config.jelly b/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildStatus/config.jelly index dcb6b0fe5..0ab4c2b80 100644 --- a/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildStatus/config.jelly +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildStatus/config.jelly @@ -1,5 +1,5 @@ - + \ No newline at end of file diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildStatus/help-messages.html b/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildStatus/help-messages.html index f99a6a9ce..3c224a0f9 100644 --- a/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildStatus/help-messages.html +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbBuildStatus/help-messages.html @@ -1,5 +1,5 @@
- Global: Sets a default for each job, but won't be used by default.
+ Global: Sets a default for each job, if the job setup has no entries then this is used.

- Add a text message to the comment appended to the pull request on build completion. + Add a text message to the comment posted to the pull request on build completion.
\ No newline at end of file diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus/config.jelly b/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus/config.jelly index 93b5fe8b8..d883f91dd 100644 --- a/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus/config.jelly +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus/config.jelly @@ -1,17 +1,17 @@ - + - + - + - + - + \ No newline at end of file diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus/help-completedStatus.html b/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus/help-completedStatus.html index 8095eedc4..9d4442d6a 100644 --- a/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus/help-completedStatus.html +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus/help-completedStatus.html @@ -1,4 +1,4 @@
- Use a custom status for when a build is completed. + Use a custom status on the commit for when a build is completed. If the field is left blank then the default value is used instead. -
\ No newline at end of file + diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus/help-startedStatus.html b/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus/help-startedStatus.html index 4b6235478..b4e1c9fbb 100644 --- a/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus/help-startedStatus.html +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus/help-startedStatus.html @@ -1,4 +1,5 @@
Use a custom status for when a build is started. If the field is left blank then the default value is used instead. -
\ No newline at end of file + If no status updates should be made when a build is started, use "--none--" to alert the trigger. + diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus/help-statusUrl.html b/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus/help-statusUrl.html index af04b3035..82c957c7c 100644 --- a/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus/help-statusUrl.html +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus/help-statusUrl.html @@ -1,3 +1,4 @@
Use a custom url instead of the Jenkins job url. + If the desired url should be blank, use "--none--" to alert the trigger to use a blank url.
\ No newline at end of file diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus/help-triggeredStatus.html b/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus/help-triggeredStatus.html index f942bd38c..4c32d7185 100644 --- a/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus/help-triggeredStatus.html +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatus/help-triggeredStatus.html @@ -1,4 +1,5 @@
Use a custom status for when a build is triggered. If the field is left blank then the default value is used instead. -
\ No newline at end of file + If no status updates should be made when a build is triggered, use "--none--" to alert the trigger. + diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus/config.jelly b/src/main/resources/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus/config.jelly new file mode 100644 index 000000000..d883f91dd --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus/config.jelly @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus/help-commitStatusContext.html b/src/main/resources/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus/help-commitStatusContext.html new file mode 100644 index 000000000..f9f488e71 --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus/help-commitStatusContext.html @@ -0,0 +1,4 @@ +
+ Context: + A string label to differentiate this status from the status of other systems. Default: "default" +
\ No newline at end of file diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus/help-completedStatus.html b/src/main/resources/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus/help-completedStatus.html new file mode 100644 index 000000000..0301c9968 --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus/help-completedStatus.html @@ -0,0 +1,4 @@ +
+ Use a custom status on the commit for when a build is completed. + If the field is left blank then the default value is used instead. +
\ No newline at end of file diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus/help-startedStatus.html b/src/main/resources/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus/help-startedStatus.html new file mode 100644 index 000000000..4b6235478 --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus/help-startedStatus.html @@ -0,0 +1,4 @@ +
+ Use a custom status for when a build is started. + If the field is left blank then the default value is used instead. +
\ No newline at end of file diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus/help-statusUrl.html b/src/main/resources/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus/help-statusUrl.html new file mode 100644 index 000000000..82c957c7c --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus/help-statusUrl.html @@ -0,0 +1,4 @@ +
+ Use a custom url instead of the Jenkins job url. + If the desired url should be blank, use "--none--" to alert the trigger to use a blank url. +
\ No newline at end of file diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus/help-triggeredStatus.html b/src/main/resources/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus/help-triggeredStatus.html new file mode 100644 index 000000000..f942bd38c --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus/help-triggeredStatus.html @@ -0,0 +1,4 @@ +
+ Use a custom status for when a build is triggered. + If the field is left blank then the default value is used instead. +
\ No newline at end of file diff --git a/src/main/resources/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus/help.html b/src/main/resources/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus/help.html new file mode 100644 index 000000000..664d45aec --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/ghprb/upstream/GhprbUpstreamStatus/help.html @@ -0,0 +1,8 @@ +
+ Allows you to set a custom context and message on a pull request pulled using GHPRB. +
This will add the context and message to the pull request found in the upstream job NOT on any pull requests taken on this job +
To add a custom context and message on any pull requests on THIS job see the GHPRB section in the Build Triggers section +
+
IMPORTANT: +
This will only work if you configure a upstream job that uses the Github Pull Request Builder Plugin +
\ No newline at end of file diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GeneralTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/GeneralTest.java new file mode 100644 index 000000000..7a62ce65e --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GeneralTest.java @@ -0,0 +1,72 @@ +package org.jenkinsci.plugins.ghprb; + +import static org.fest.assertions.Assertions.assertThat; + +import java.util.List; + +import org.jenkinsci.plugins.ghprb.extensions.build.GhprbCancelBuildsOnUpdate; +import org.jenkinsci.plugins.ghprb.extensions.comments.GhprbBuildLog; +import org.jenkinsci.plugins.ghprb.extensions.comments.GhprbBuildResultMessage; +import org.jenkinsci.plugins.ghprb.extensions.comments.GhprbBuildStatus; +import org.jenkinsci.plugins.ghprb.extensions.comments.GhprbCommentFile; +import org.jenkinsci.plugins.ghprb.extensions.status.GhprbSimpleStatus; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.runners.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public class GeneralTest { + + private void checkClassForGetters(Class clazz) { + List errors = GhprbTestUtil.checkClassForGetters(clazz); + assertThat(errors).isEmpty(); + } + + @Test + public void checkTriggerForGetters() { + checkClassForGetters(GhprbTrigger.class); + } + + @Test + public void checkTriggerDescriptorForGetters() { + checkClassForGetters(GhprbTrigger.DescriptorImpl.class); + } + + @Test + public void checkPullRequestMergeForGetters() { + checkClassForGetters(GhprbPullRequestMerge.class); + } + + @Test + public void checkBuildLogForGetters() { + checkClassForGetters(GhprbBuildLog.class); + } + + + @Test + public void checkBuildResultMessageForGetters() { + checkClassForGetters(GhprbBuildResultMessage.class); + } + + @Test + public void checkBuildStatusForGetters() { + checkClassForGetters(GhprbBuildStatus.class); + } + + @Test + public void checkCommentFileForGetters() { + checkClassForGetters(GhprbCommentFile.class); + } + + + @Test + public void checkSimpleStatusForGetters() { + checkClassForGetters(GhprbSimpleStatus.class); + } + + @Test + public void checkCancelBuildsOnUpdateForGetters() { + checkClassForGetters(GhprbCancelBuildsOnUpdate.class); + } + +} diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbIT.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbIT.java index e79527e8d..155437197 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbIT.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbIT.java @@ -1,10 +1,8 @@ package org.jenkinsci.plugins.ghprb; -import com.coravy.hudson.plugins.github.GithubProjectProperty; import com.google.common.collect.Lists; import hudson.model.FreeStyleProject; -import hudson.plugins.git.GitSCM; import org.joda.time.DateTime; import org.junit.Before; @@ -16,17 +14,13 @@ import org.kohsuke.github.GHIssueComment; import org.kohsuke.stapler.RequestImpl; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.runners.MockitoJUnitRunner; import static com.google.common.collect.Lists.newArrayList; import static org.fest.assertions.Assertions.assertThat; import static org.mockito.BDDMockito.given; -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Matchers.any; +import static org.mockito.Mockito.any; @RunWith(MockitoJUnitRunner.class) public class GhprbIT extends GhprbITBaseTestCase { @@ -36,38 +30,23 @@ public class GhprbIT extends GhprbITBaseTestCase { @Mock private RequestImpl req; + @Mock + private GHIssueComment comment; + + + private FreeStyleProject project; + @Before - public void setUp() throws Exception { - // GhprbTestUtil.mockGithubUserPage(); - super.beforeTest(); + public void setUp() throws Exception {// GIVEN + project = jenkinsRule.createFreeStyleProject("PRJ"); + super.beforeTest(null, null, project); } @Test public void shouldBuildTriggersOnNewPR() throws Exception { - // GIVEN - FreeStyleProject project = jenkinsRule.createFreeStyleProject("PRJ"); - GhprbTrigger trigger = GhprbTestUtil.getTrigger(null); - given(commitPointer.getSha()).willReturn("sha"); - GhprbTestUtil.setupGhprbTriggerDescriptor(null); - project.addProperty(new GithubProjectProperty("https://github.com/user/dropwizard")); given(ghPullRequest.getNumber()).willReturn(1); - - // Creating spy on ghprb, configuring repo - Ghprb ghprb = spy(trigger.createGhprb(project)); - doReturn(ghprbGitHub).when(ghprb).getGitHub(); - ghprb.getRepository().setHelper(ghprb); - - // Configuring and adding Ghprb trigger - project.addTrigger(trigger); - - // Configuring Git SCM - GitSCM scm = GhprbTestUtil.provideGitSCM(); - project.setScm(scm); - - trigger.start(project, true); - trigger.setHelper(ghprb); - + GhprbTestUtil.triggerRunAndWait(10, trigger, project); assertThat(project.getBuilds().toArray().length).isEqualTo(1); @@ -76,22 +55,11 @@ public void shouldBuildTriggersOnNewPR() throws Exception { @Test public void shouldBuildTriggersOnUpdatingNewCommitsPR() throws Exception { // GIVEN - FreeStyleProject project = jenkinsRule.createFreeStyleProject("PRJ"); - GhprbTrigger trigger = GhprbTestUtil.getTrigger(null); - given(commitPointer.getSha()).willReturn("sha").willReturn("sha").willReturn("newOne").willReturn("newOne"); + given(commitPointer.getSha()).willReturn("sha").willReturn("newOne").willReturn("newOne"); given(ghPullRequest.getComments()).willReturn(Lists. newArrayList()); - GhprbTestUtil.setupGhprbTriggerDescriptor(null); - project.addProperty(new GithubProjectProperty("https://github.com/user/dropwizard")); + given(ghPullRequest.getNumber()).willReturn(2).willReturn(2).willReturn(3).willReturn(3); - Ghprb ghprb = spy(trigger.createGhprb(project)); - doReturn(ghprbGitHub).when(ghprb).getGitHub(); - trigger.start(project, true); - trigger.setHelper(ghprb); - ghprb.getRepository().setHelper(ghprb); - project.addTrigger(trigger); - GitSCM scm = GhprbTestUtil.provideGitSCM(); - project.setScm(scm); - + GhprbTestUtil.triggerRunAndWait(10, trigger, project); assertThat(project.getBuilds().toArray().length).isEqualTo(2); @@ -100,29 +68,17 @@ public void shouldBuildTriggersOnUpdatingNewCommitsPR() throws Exception { @Test public void shouldBuildTriggersOnUpdatingRetestMessagePR() throws Exception { // GIVEN - FreeStyleProject project = jenkinsRule.createFreeStyleProject("PRJ"); - GhprbTrigger trigger = GhprbTestUtil.getTrigger(null); - - given(commitPointer.getSha()).willReturn("sha"); - - GHIssueComment comment = mock(GHIssueComment.class); + GhprbTestUtil.triggerRunAndWait(10, trigger, project); + assertThat(project.getBuilds().toArray().length).isEqualTo(1); + given(comment.getBody()).willReturn("retest this please"); given(comment.getUpdatedAt()).willReturn(new DateTime().plusDays(1).toDate()); given(comment.getUser()).willReturn(ghUser); + given(ghPullRequest.getComments()).willReturn(newArrayList(comment)); given(ghPullRequest.getNumber()).willReturn(5).willReturn(5); - GhprbTestUtil.setupGhprbTriggerDescriptor(null); - project.addProperty(new GithubProjectProperty("https://github.com/user/dropwizard")); - - Ghprb ghprb = spy(trigger.createGhprb(project)); - doReturn(ghprbGitHub).when(ghprb).getGitHub(); - trigger.start(project, true); - trigger.setHelper(ghprb); - ghprb.getRepository().setHelper(ghprb); - project.addTrigger(trigger); - GitSCM scm = GhprbTestUtil.provideGitSCM(); - project.setScm(scm); + GhprbTestUtil.triggerRunAndWait(10, trigger, project); assertThat(project.getBuilds().toArray().length).isEqualTo(2); } @@ -131,35 +87,26 @@ public void shouldBuildTriggersOnUpdatingRetestMessagePR() throws Exception { @Test public void shouldNotBuildDisabledBuild() throws Exception { // GIVEN - FreeStyleProject project = jenkinsRule.createFreeStyleProject("PRJ"); - GhprbTrigger trigger = GhprbTestUtil.getTrigger(null); - + given(commitPointer.getSha()).willReturn("sha"); - GHIssueComment comment = mock(GHIssueComment.class); given(comment.getBody()).willReturn("retest this please"); given(comment.getUpdatedAt()).willReturn(new DateTime().plusDays(1).toDate()); given(comment.getUser()).willReturn(ghUser); given(ghPullRequest.getComments()).willReturn(newArrayList(comment)); given(ghPullRequest.getNumber()).willReturn(5); - GhprbTestUtil.setupGhprbTriggerDescriptor(null); - project.addProperty(new GithubProjectProperty("https://github.com/user/dropwizard")); - - Ghprb ghprb = spy(trigger.createGhprb(project)); - doReturn(ghprbGitHub).when(ghprb).getGitHub(); - trigger.start(project, true); - trigger.setHelper(ghprb); - ghprb.getRepository().setHelper(ghprb); - project.addTrigger(trigger); - GitSCM scm = GhprbTestUtil.provideGitSCM(); - project.setScm(scm); project.disable(); GhprbTestUtil.triggerRunAndWait(10, trigger, project); assertThat(project.getBuilds().toArray().length).isEqualTo(0); - verify(ghRepository, times(0)).createCommitStatus(any(String.class), any(GHCommitState.class), any(String.class), any(String.class)); + Mockito.verify(ghRepository, Mockito.times(0)).createCommitStatus(any(String.class), any(GHCommitState.class), any(String.class), any(String.class)); + } + + @Test + public void triggerIsRemovedFromListWhenProjectChanges() { + } } diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbITBaseTestCase.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbITBaseTestCase.java index c0bb1a97b..30c85304a 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbITBaseTestCase.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbITBaseTestCase.java @@ -1,11 +1,17 @@ package org.jenkinsci.plugins.ghprb; import static com.google.common.collect.Lists.newArrayList; -import static org.kohsuke.github.GHIssueState.OPEN; import static org.mockito.BDDMockito.given; +import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; + +import java.util.Map; + +import hudson.model.AbstractBuild; import hudson.model.AbstractProject; +import hudson.model.TaskListener; +import hudson.plugins.git.GitSCM; import org.joda.time.DateTime; import org.kohsuke.github.GHCommitPointer; @@ -16,6 +22,9 @@ import org.kohsuke.github.GitHub; import org.mockito.Mock; import org.mockito.Mockito; +import org.kohsuke.github.GHIssueState; + +import com.coravy.hudson.plugins.github.GithubProjectProperty; /** * @author mdelapenya (Manuel de la Peña) @@ -33,43 +42,77 @@ public abstract class GhprbITBaseTestCase { @Mock protected GHUser ghUser; @Mock - protected GitHub gitHub; + protected Ghprb helper; + @Mock + protected GhprbPullRequest ghprbPullRequest; + + protected GhprbBuilds builds; + + protected GhprbTrigger trigger; // Stubs protected GHRateLimit ghRateLimit = new GHRateLimit(); - protected void beforeTest() throws Exception { - given(ghprbGitHub.get()).willReturn(gitHub); - given(gitHub.getRateLimit()).willReturn(ghRateLimit); + protected void beforeTest(Map globalConfig, Map triggerConfig, AbstractProject project) throws Exception { + project.addProperty(new GithubProjectProperty("https://github.com/user/dropwizard")); + GhprbTestUtil.setupGhprbTriggerDescriptor(globalConfig); + + trigger = GhprbTestUtil.getTrigger(triggerConfig); + + GitHub gitHub = trigger.getGitHub(); + given(gitHub.getRepository(anyString())).willReturn(ghRepository); + + given(ghPullRequest.getHead()).willReturn(commitPointer); + given(ghPullRequest.getUser()).willReturn(ghUser); + given(commitPointer.getRef()).willReturn("ref"); + given(commitPointer.getSha()).willReturn("sha"); + given(ghRepository.getName()).willReturn("dropwizard"); GhprbTestUtil.mockPR(ghPullRequest, commitPointer, new DateTime(), new DateTime().plusDays(1)); + - given(ghRepository.getPullRequests(eq(OPEN))).willReturn(newArrayList(ghPullRequest)).willReturn(newArrayList(ghPullRequest)); + given(ghRepository.getPullRequests(eq(GHIssueState.OPEN))).willReturn(newArrayList(ghPullRequest)).willReturn(newArrayList(ghPullRequest)); + given(ghRepository.getPullRequest(Mockito.anyInt())).willReturn(ghPullRequest); - given(ghPullRequest.getUser()).willReturn(ghUser); given(ghUser.getEmail()).willReturn("email@email.com"); given(ghUser.getLogin()).willReturn("user"); ghRateLimit.remaining = GhprbTestUtil.INITIAL_RATE_LIMIT; GhprbTestUtil.mockCommitList(ghPullRequest); - } + - protected void setRepositoryHelper(Ghprb ghprb) { - ghprb.getRepository().setHelper(ghprb); - } + GhprbRepository repo = Mockito.spy(new GhprbRepository("user/dropwizard", trigger)); + Mockito.doReturn(ghRepository).when(repo).getGitHubRepo(); + Mockito.doNothing().when(repo).addComment(Mockito.anyInt(), Mockito.anyString(), any(AbstractBuild.class), any(TaskListener.class)); + Mockito.doReturn(ghprbPullRequest).when(repo).getPullRequest(Mockito.anyInt()); + + Mockito.doReturn(repo).when(trigger).getRepository(); - protected void setTriggerHelper(GhprbTrigger trigger, Ghprb ghprb) { - trigger.setHelper(ghprb); - } + builds = new GhprbBuilds(trigger, repo); - protected Ghprb spyCreatingGhprb(GhprbTrigger trigger, AbstractProject project) { + // Creating spy on ghprb, configuring repo + given(helper.getGitHub()).willReturn(ghprbGitHub); + given(helper.getTrigger()).willReturn(trigger); + given(helper.isWhitelisted(ghUser)).willReturn(true); + given(helper.getBuilds()).willReturn(builds); + + + Mockito.doCallRealMethod().when(trigger).run(); - return Mockito.spy(trigger.createGhprb(project)); - } + // Configuring and adding Ghprb trigger + project.addTrigger(trigger); + + // Configuring Git SCM + GitSCM scm = GhprbTestUtil.provideGitSCM(); + project.setScm(scm); + + trigger.start(project, true); + trigger.setHelper(helper); + } } \ No newline at end of file diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMergeTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMergeTest.java index ff56c0b08..c9d17ac3f 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMergeTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMergeTest.java @@ -26,6 +26,7 @@ import org.kohsuke.github.GHPullRequest; import org.kohsuke.github.GHPullRequestCommitDetail; import org.kohsuke.github.GHPullRequestCommitDetail.Commit; +import org.kohsuke.github.GHRepository; import org.kohsuke.github.GHUser; import org.kohsuke.github.GitUser; import org.kohsuke.github.PagedIterable; @@ -44,6 +45,8 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; @RunWith(MockitoJUnitRunner.class) public class GhprbPullRequestMergeTest { @@ -64,11 +67,15 @@ public class GhprbPullRequestMergeTest { @Mock private GHUser triggerSender; @Mock + private GHUser prCreator; + @Mock private GhprbCause cause; @Mock private Ghprb helper; @Mock private GhprbRepository repo; + @Mock + private GHRepository ghRepo; @Mock private StreamBuildListener listener; @@ -86,6 +93,9 @@ public class GhprbPullRequestMergeTest { private final String committerName = "committer"; private final String nonCommitterName = "noncommitter"; + + private final String committerEmail = "committer@mail.com"; + private final String nonCommitterEmail = "noncommitter@mail.com"; private final String mergeComment = "merge"; @@ -99,13 +109,22 @@ public void beforeTest() throws Exception { triggerValues.put("adminlist", adminList); triggerValues.put("triggerPhrase", triggerPhrase); - GhprbTrigger trigger = spy(GhprbTestUtil.getTrigger(triggerValues)); + GhprbTrigger trigger = GhprbTestUtil.getTrigger(triggerValues); + Mockito.doReturn(repo).when(trigger).getRepository(); ConcurrentMap pulls = new ConcurrentHashMap(1); + pulls.put(pullId, pullRequest); Map> jobs = new HashMap>(1); jobs.put("project", pulls); + Mockito.doReturn(project).when(trigger).getActualProject(); + Mockito.doReturn(repo).when(trigger).getRepository(); + repo.addPullRequests(pulls); + Mockito.doReturn(pullRequest).when(repo).getPullRequest(pullId); + Mockito.doReturn(pr).when(repo).getActualPullRequest(pullId); + + GithubProjectProperty projectProperty = new GithubProjectProperty("https://github.com/jenkinsci/ghprb-plugin"); DescriptorImpl descriptor = trigger.getDescriptor(); @@ -123,7 +142,7 @@ public void beforeTest() throws Exception { given(build.getResult()).willReturn(Result.SUCCESS); given(build.getParent()).willCallRealMethod(); - given(pullRequest.getPullRequest()).willReturn(pr); + given(pullRequest.getPullRequest(Mockito.anyBoolean())).willReturn(pr); given(cause.getPullID()).willReturn(pullId); given(cause.isMerged()).willReturn(true); @@ -143,9 +162,8 @@ public void beforeTest() throws Exception { jobsField.setAccessible(true); jobsField.set(descriptor, jobs); - helper = spy(new Ghprb(project, trigger, pulls)); + helper = spy(new Ghprb(trigger)); trigger.setHelper(helper); - given(helper.getRepository()).willReturn(repo); given(helper.isBotUser(any(GHUser.class))).willReturn(false); } @@ -155,10 +173,14 @@ public void afterClass() { } @SuppressWarnings("unchecked") - private void setupConditions(String triggerLogin, String committerName, String comment) throws IOException { + private void setupConditions(String prUserLogin, String triggerLogin, String committerName, String committerEmail, String comment) throws IOException { given(triggerSender.getLogin()).willReturn(triggerLogin); given(triggerSender.getName()).willReturn(committerName); + given(triggerSender.getEmail()).willReturn(committerEmail); given(committer.getName()).willReturn(this.committerName); + + given(prCreator.getLogin()).willReturn(prUserLogin); + given(pr.getUser()).willReturn(prCreator); PagedIterator itr = Mockito.mock(PagedIterator.class); PagedIterable pagedItr = Mockito.mock(PagedIterable.class); @@ -178,129 +200,173 @@ private void setupConditions(String triggerLogin, String committerName, String c given(cause.getCommentBody()).willReturn(comment); } - - private GhprbPullRequestMerge setupMerger(boolean onlyTriggerPhrase, boolean onlyAdminsMerge, boolean disallowOwnCode) { - GhprbPullRequestMerge merger = spy(new GhprbPullRequestMerge(mergeComment, onlyTriggerPhrase, onlyAdminsMerge, disallowOwnCode)); + + private void setupConditions(String triggerLogin, String committerName, String committerEmail, String comment) throws IOException { + setupConditions(nonCommitterName, triggerLogin, committerName, committerEmail, comment); + } + + private GhprbPullRequestMerge setupMerger( + boolean onlyAdminsMerge, + boolean disallowOwnCode, + boolean failOnNonMerge, + boolean deleteOnMerge + ) { + + GhprbPullRequestMerge merger = spy(new GhprbPullRequestMerge( + mergeComment, + onlyAdminsMerge, + disallowOwnCode, + failOnNonMerge, + deleteOnMerge)); merger.setHelper(helper); + Mockito.reset(pr); return merger; } + private GhprbPullRequestMerge setupMerger( + boolean onlyAdminsMerge, + boolean disallowOwnCode) { + return setupMerger(onlyAdminsMerge, disallowOwnCode, false, false); + } + @Test public void testApproveMerge() throws Exception { - boolean onlyTriggerPhrase = false; boolean onlyAdminsMerge = false; boolean disallowOwnCode = false; - GhprbPullRequestMerge merger = setupMerger(onlyTriggerPhrase, onlyAdminsMerge, disallowOwnCode); + GhprbPullRequestMerge merger = setupMerger(onlyAdminsMerge, disallowOwnCode); - setupConditions(nonAdminLogin, committerName, triggerPhrase); + setupConditions(nonAdminLogin, committerName, committerEmail, triggerPhrase); assertThat(merger.perform(build, null, listener)).isEqualTo(true); + verify(pr, times(1)).merge(mergeComment); - setupConditions(adminLogin, nonCommitterName, triggerPhrase); + setupConditions(adminLogin, nonCommitterName, nonCommitterEmail, triggerPhrase); assertThat(merger.perform(build, null, listener)).isEqualTo(true); + verify(pr, times(2)).merge(mergeComment); - setupConditions(adminLogin, committerName, nonTriggerPhrase); + setupConditions(adminLogin, committerName, committerEmail, nonTriggerPhrase); assertThat(merger.perform(build, null, listener)).isEqualTo(true); + verify(pr, times(2)).merge(mergeComment); - setupConditions(nonAdminLogin, nonCommitterName, triggerPhrase); + setupConditions(nonAdminLogin, nonCommitterName, nonCommitterEmail, triggerPhrase); assertThat(merger.perform(build, null, listener)).isEqualTo(true); + verify(pr, times(3)).merge(mergeComment); - setupConditions(nonAdminLogin, nonCommitterName, nonTriggerPhrase); + setupConditions(nonAdminLogin, nonCommitterName, nonCommitterEmail, nonTriggerPhrase); assertThat(merger.perform(build, null, listener)).isEqualTo(true); + verify(pr, times(3)).merge(mergeComment); - setupConditions(adminLogin, nonCommitterName, nonTriggerPhrase); + setupConditions(adminLogin, nonCommitterName, nonCommitterEmail, nonTriggerPhrase); assertThat(merger.perform(build, null, listener)).isEqualTo(true); + verify(pr, times(3)).merge(mergeComment); - setupConditions(nonAdminLogin, nonCommitterName, nonTriggerPhrase); + setupConditions(nonAdminLogin, nonCommitterName, nonCommitterEmail, nonTriggerPhrase); assertThat(merger.perform(build, null, listener)).isEqualTo(true); + verify(pr, times(3)).merge(mergeComment); - setupConditions(adminLogin, committerName, triggerPhrase); + setupConditions(adminLogin, committerName, committerEmail, triggerPhrase); assertThat(merger.perform(build, null, listener)).isEqualTo(true); + verify(pr, times(4)).merge(mergeComment); } @Test public void testAdminMerge() throws Exception { - boolean onlyTriggerPhrase = false; boolean onlyAdminsMerge = true; boolean disallowOwnCode = false; - GhprbPullRequestMerge merger = setupMerger(onlyTriggerPhrase, onlyAdminsMerge, disallowOwnCode); + GhprbPullRequestMerge merger = setupMerger(onlyAdminsMerge, disallowOwnCode); - setupConditions(adminLogin, committerName, triggerPhrase); + setupConditions(adminLogin, committerName, committerEmail, triggerPhrase); assertThat(merger.perform(build, null, listener)).isEqualTo(true); + verify(pr, times(1)).merge(mergeComment); - setupConditions(nonAdminLogin, committerName, triggerPhrase); + setupConditions(nonAdminLogin, committerName, committerEmail, triggerPhrase); assertThat(merger.perform(build, null, listener)).isEqualTo(false); + verify(pr, times(1)).merge(mergeComment); } @Test public void testTriggerMerge() throws Exception { - boolean onlyTriggerPhrase = true; boolean onlyAdminsMerge = false; boolean disallowOwnCode = false; - GhprbPullRequestMerge merger = setupMerger(onlyTriggerPhrase, onlyAdminsMerge, disallowOwnCode); + GhprbPullRequestMerge merger = setupMerger(onlyAdminsMerge, disallowOwnCode); - setupConditions(adminLogin, committerName, triggerPhrase); + setupConditions(adminLogin, committerName, committerEmail, triggerPhrase); assertThat(merger.perform(build, null, listener)).isEqualTo(true); + verify(pr, times(1)).merge(mergeComment); - setupConditions(adminLogin, committerName, nonTriggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(false); + setupConditions(adminLogin, committerName, committerEmail, nonTriggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(true); + verify(pr, times(1)).merge(mergeComment); } @Test public void testOwnCodeMerge() throws Exception { - boolean onlyTriggerPhrase = false; boolean onlyAdminsMerge = false; boolean disallowOwnCode = true; - GhprbPullRequestMerge merger = setupMerger(onlyTriggerPhrase, onlyAdminsMerge, disallowOwnCode); + GhprbPullRequestMerge merger = setupMerger(onlyAdminsMerge, disallowOwnCode); - setupConditions(adminLogin, nonCommitterName, triggerPhrase); + setupConditions(adminLogin, nonCommitterName, nonCommitterEmail, triggerPhrase); assertThat(merger.perform(build, null, listener)).isEqualTo(true); + verify(pr, times(1)).merge(mergeComment); - setupConditions(adminLogin, committerName, triggerPhrase); + setupConditions(adminLogin, committerName, committerEmail, triggerPhrase); assertThat(merger.perform(build, null, listener)).isEqualTo(false); + verify(pr, times(1)).merge(mergeComment); } @Test public void testDenyMerge() throws Exception { - boolean onlyTriggerPhrase = true; boolean onlyAdminsMerge = true; boolean disallowOwnCode = true; - GhprbPullRequestMerge merger = setupMerger(onlyTriggerPhrase, onlyAdminsMerge, disallowOwnCode); + GhprbPullRequestMerge merger = setupMerger(onlyAdminsMerge, disallowOwnCode); - setupConditions(nonAdminLogin, nonCommitterName, triggerPhrase); + setupConditions(nonAdminLogin, nonAdminLogin, nonCommitterName, nonCommitterEmail, triggerPhrase); assertThat(merger.perform(build, null, listener)).isEqualTo(false); + verify(pr, times(0)).merge(mergeComment); - setupConditions(adminLogin, committerName, triggerPhrase); + setupConditions(nonAdminLogin, adminLogin, committerName, committerEmail, triggerPhrase); assertThat(merger.perform(build, null, listener)).isEqualTo(false); + verify(pr, times(0)).merge(mergeComment); - setupConditions(adminLogin, nonCommitterName, nonTriggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(false); + setupConditions(nonAdminLogin, adminLogin, nonCommitterName, nonCommitterEmail, nonTriggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(true); + verify(pr, times(0)).merge(mergeComment); - setupConditions(nonAdminLogin, nonCommitterName, triggerPhrase); + setupConditions(nonAdminLogin, nonAdminLogin, nonCommitterName, nonCommitterEmail, triggerPhrase); assertThat(merger.perform(build, null, listener)).isEqualTo(false); + verify(pr, times(0)).merge(mergeComment); - setupConditions(nonAdminLogin, nonCommitterName, nonTriggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(false); + setupConditions(nonAdminLogin, nonAdminLogin, nonCommitterName, nonCommitterEmail, nonTriggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(true); + verify(pr, times(0)).merge(mergeComment); - setupConditions(adminLogin, committerName, nonTriggerPhrase); - assertThat(merger.perform(build, null, listener)).isEqualTo(false); + setupConditions(nonAdminLogin, adminLogin, committerName, committerEmail, nonTriggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(true); + verify(pr, times(0)).merge(mergeComment); - setupConditions(nonAdminLogin, committerName, nonTriggerPhrase); + setupConditions(nonAdminLogin, nonAdminLogin, committerName, committerEmail, nonTriggerPhrase); + assertThat(merger.perform(build, null, listener)).isEqualTo(true); + verify(pr, times(0)).merge(mergeComment); + + setupConditions(adminLogin, adminLogin, nonCommitterName, nonCommitterEmail, triggerPhrase); assertThat(merger.perform(build, null, listener)).isEqualTo(false); + verify(pr, times(0)).merge(mergeComment); - setupConditions(adminLogin, nonCommitterName, triggerPhrase); + setupConditions(nonAdminLogin, adminLogin, nonCommitterName, nonCommitterEmail, triggerPhrase); assertThat(merger.perform(build, null, listener)).isEqualTo(true); + verify(pr, times(1)).merge(mergeComment); + } } diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestTest.java index fccf277d3..e34a4c18d 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestTest.java @@ -1,11 +1,14 @@ package org.jenkinsci.plugins.ghprb; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.kohsuke.github.GHCommitPointer; import org.kohsuke.github.GHPullRequest; +import org.kohsuke.github.GHRepository; import org.kohsuke.github.GHUser; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.runners.MockitoJUnitRunner; import java.io.IOException; @@ -14,6 +17,7 @@ import static org.fest.assertions.Assertions.assertThat; import static org.mockito.BDDMockito.given; import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.mock; @@ -33,33 +37,54 @@ public class GhprbPullRequestTest { private Ghprb helper; @Mock private GhprbRepository repo; - - @Test - public void testConstructorWhenAuthorIsWhitelisted() throws IOException { - // GIVEN - GHUser ghUser = mock(GHUser.class); - GHCommitPointer head = mock(GHCommitPointer.class); - GHCommitPointer base = mock(GHCommitPointer.class); + @Mock + private GHCommitPointer head, base; + @Mock + private GhprbRepository ghprbRepository; + @Mock + private GHUser ghUser; + @Mock + private GhprbBuilds builds; + + @Before + public void setup() throws IOException { given(head.getSha()).willReturn("some sha"); given(base.getRef()).willReturn("some ref"); // Mocks for GHPullRequest given(pr.getNumber()).willReturn(10); + given(pr.getCreatedAt()).willReturn(new Date()); given(pr.getUpdatedAt()).willReturn(new Date()); given(pr.getTitle()).willReturn("title"); given(pr.getHead()).willReturn(head); given(pr.getBase()).willReturn(base); - given(pr.getUser()).willReturn(ghUser); + given(ghUser.getEmail()).willReturn("email"); - + + given(ghprbRepository.getActualPullRequest(10)).willReturn(pr); + given(ghprbRepository.getName()).willReturn("name"); + + given(pr.getHead()).willReturn(head); + given(pr.getUser()).willReturn(ghUser); + + // Mocks for Ghprb + given(helper.isWhitelisted(ghUser)).willReturn(true); + given(helper.getBuilds()).willReturn(builds); + + doNothing().when(builds).build(any(GhprbPullRequest.class), any(GHUser.class), anyString()); + // Mocks for GhprbRepository given(repo.getName()).willReturn("repoName"); - // Mocks for Ghprb - given(helper.isWhitelisted(ghUser)).willReturn(true); + // Mocks for GhprbRepository + doNothing().when(repo).addComment(Mockito.anyInt(), anyString()); + } + + @Test + public void testConstructorWhenAuthorIsWhitelisted() throws IOException { // WHEN - GhprbPullRequest ghprbPullRequest = new GhprbPullRequest(pr, helper, repo); + GhprbPullRequest ghprbPullRequest = new GhprbPullRequest(pr, helper, repo, false); // THEN assertThat(ghprbPullRequest.getId()).isEqualTo(10); @@ -72,75 +97,66 @@ public void testConstructorWhenAuthorIsWhitelisted() throws IOException { @Test public void testInitRepoNameNull() throws IOException { - // GIVEN - GHUser ghUser = mock(GHUser.class); - GHCommitPointer head = mock(GHCommitPointer.class); - GHCommitPointer base = mock(GHCommitPointer.class); - - // Mocks for GHPullRequest - given(pr.getNumber()).willReturn(10); - given(pr.getUpdatedAt()).willReturn(new Date()); - given(pr.getTitle()).willReturn("title"); - given(pr.getHead()).willReturn(head); - given(pr.getBase()).willReturn(base); - given(head.getSha()).willReturn("some sha"); - given(base.getRef()).willReturn("some ref"); - given(pr.getUser()).willReturn(ghUser); - given(ghUser.getEmail()).willReturn("email"); - // Mocks for GhprbRepository given(repo.getName()).willReturn(null); - doNothing().when(repo).addComment(eq(10), anyString()); - // Mocks for Ghprb - given(helper.isWhitelisted(ghUser)).willReturn(true); - - GhprbPullRequest ghprbPullRequest = new GhprbPullRequest(pr, helper, repo); - GhprbRepository ghprbRepository = mock(GhprbRepository.class); - given(ghprbRepository.getName()).willReturn("name"); + GhprbPullRequest ghprbPullRequest = new GhprbPullRequest(pr, helper, repo, false); // WHEN ghprbPullRequest.init(helper, ghprbRepository); // THEN - verify(ghprbRepository, times(1)).getName(); + verify(pr, times(1)).getHead(); + verify(pr, times(1)).getBase(); + verify(pr, times(1)).getNumber(); + verify(pr, times(1)).getCreatedAt(); + verify(pr, times(3)).getUser(); + Mockito.verifyNoMoreInteractions(pr); } @Test public void testInitRepoNameNotNull() throws IOException { - // GIVEN - GHUser ghUser = mock(GHUser.class); - GHCommitPointer head = mock(GHCommitPointer.class); - GHCommitPointer base = mock(GHCommitPointer.class); - - // Mocks for GHPullRequest - given(pr.getNumber()).willReturn(10); - given(pr.getUpdatedAt()).willReturn(new Date()); - given(pr.getTitle()).willReturn("title"); - given(pr.getHead()).willReturn(head); - given(pr.getBase()).willReturn(base); - given(head.getSha()).willReturn("some sha"); - given(base.getRef()).willReturn("some ref"); - given(pr.getUser()).willReturn(ghUser); - given(ghUser.getEmail()).willReturn("email"); // Mocks for GhprbRepository given(repo.getName()).willReturn("name"); doNothing().when(repo).addComment(eq(10), anyString()); - // Mocks for Ghprb - given(helper.isWhitelisted(ghUser)).willReturn(true); - - GhprbPullRequest ghprbPullRequest = new GhprbPullRequest(pr, helper, repo); - GhprbRepository ghprbRepository = mock(GhprbRepository.class); - given(ghprbRepository.getName()).willReturn("name"); + GhprbPullRequest ghprbPullRequest = new GhprbPullRequest(pr, helper, repo, false); // WHEN ghprbPullRequest.init(helper, ghprbRepository); // THEN verify(ghprbRepository, never()).getName(); + Mockito.verifyNoMoreInteractions(ghprbRepository); + } + + @Test + public void authorRepoGitUrlShouldBeNullWhenNoRepository() throws Exception { + // GIVEN + + GhprbPullRequest ghprbPullRequest = new GhprbPullRequest(pr, helper, repo, false); + + // WHEN + ghprbPullRequest.init(helper, ghprbRepository); + + // THEN + assertThat(ghprbPullRequest.getAuthorRepoGitUrl()).isEqualTo(""); } + @Test + public void authorRepoGitUrlShouldBeSetWhenRepository() throws Exception { + // GIVEN + String expectedAuthorRepoGitUrl = "https://github.com/jenkinsci/ghprb-plugin"; + GHRepository repository = mock(GHRepository.class); + given(repository.gitHttpTransportUrl()).willReturn(expectedAuthorRepoGitUrl); + + given(head.getRepository()).willReturn(repository); + + GhprbPullRequest ghprbPullRequest = new GhprbPullRequest(pr, helper, repo, false); + + // THEN + assertThat(ghprbPullRequest.getAuthorRepoGitUrl()).isEqualTo(expectedAuthorRepoGitUrl); + } } diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java index 8011fdae5..3a519f8ae 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java @@ -24,9 +24,14 @@ import org.mockito.Mockito; import org.mockito.runners.MockitoJUnitRunner; +import com.coravy.hudson.plugins.github.GithubProjectProperty; + import javax.crypto.Mac; import javax.crypto.spec.SecretKeySpec; + +import hudson.model.AbstractBuild; import hudson.model.AbstractProject; +import hudson.model.TaskListener; import hudson.model.queue.QueueTaskFuture; import java.io.FileNotFoundException; @@ -48,7 +53,6 @@ import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; -import static org.mockito.Matchers.isNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.only; import static org.mockito.Mockito.reset; @@ -65,8 +69,7 @@ @RunWith(MockitoJUnitRunner.class) public class GhprbRepositoryTest { - private static final String TEST_USER_NAME = "test-user"; - private static final String TEST_REPO_NAME = "test-repo"; + private static final String TEST_REPO_NAME = "test-user/test-repo"; private static final Date UPDATE_DATE = new Date(); private static final String msg = "Build triggered. sha1 is merged."; @@ -99,16 +102,29 @@ public class GhprbRepositoryTest { public JenkinsRule jenkinsRule = new JenkinsRule(); @Before - @SuppressWarnings("unused") public void setUp() throws Exception { AbstractProject project = jenkinsRule.createFreeStyleProject("GhprbRepoTest"); - trigger = spy(GhprbTestUtil.getTrigger(null)); - doReturn(mock(QueueTaskFuture.class)).when(trigger).startJob(any(GhprbCause.class), any(GhprbRepository.class)); + project.addProperty(new GithubProjectProperty("https://github.com/" + TEST_REPO_NAME)); + trigger = GhprbTestUtil.getTrigger(null); + doReturn(gt).when(trigger).getGitHub(); + + given(gt.getRepository(anyString())).willReturn(ghRepository); + + trigger.start(project, true); + trigger.setHelper(helper); + + + pulls = new ConcurrentHashMap(); + + + doReturn(mock(QueueTaskFuture.class)).when(trigger).scheduleBuild(any(GhprbCause.class), any(GhprbRepository.class)); initGHPRWithTestData(); + + given(ghPullRequest.getUser()).willReturn(ghUser); // Mock github API given(helper.getGitHub()).willReturn(gitHub); - given(gitHub.get()).willReturn(gt); + given(helper.getTrigger()).willReturn(trigger); given(gt.getRepository(anyString())).willReturn(ghRepository); // Mock rate limit @@ -118,9 +134,13 @@ public void setUp() throws Exception { } - private void addSimpleStatus() throws IOException { + private void addSimpleStatus() { GhprbSimpleStatus status = new GhprbSimpleStatus("default"); - trigger.getExtensions().remove(GhprbSimpleStatus.class); + try { + trigger.getExtensions().remove(GhprbSimpleStatus.class); + } catch (Exception e) { + e.printStackTrace(); + } trigger.getExtensions().add(status); } @@ -130,6 +150,7 @@ public void testCheckMethodWhenUsingGitHubEnterprise() throws IOException { given(gt.getRateLimit()).willThrow(new FileNotFoundException()); List ghPullRequests = createListWithMockPR(); given(ghRepository.getPullRequests(eq(GHIssueState.OPEN))).willReturn(ghPullRequests); + given(ghRepository.getPullRequest(ghPullRequest.getId())).willReturn(ghPullRequest); mockHeadAndBase(); mockCommitList(); @@ -145,7 +166,7 @@ public void testCheckMethodWhenUsingGitHubEnterprise() throws IOException { ghprbRepository.check(); // THEN - verifyGetGithub(1); + verifyGetGithub(2, 0); } @Test @@ -153,13 +174,16 @@ public void testCheckMethodWithOnlyExistingPRs() throws IOException { // GIVEN List ghPullRequests = createListWithMockPR(); given(ghRepository.getPullRequests(eq(GHIssueState.OPEN))).willReturn(ghPullRequests); + given(ghRepository.getPullRequest(Mockito.anyInt())).willReturn(ghPullRequest); + doReturn(ghRepository).when(ghprbRepository).getGitHubRepo(); mockHeadAndBase(); mockCommitList(); given(helper.ifOnlyTriggerPhrase()).willReturn(true); pulls.put(1, ghprbPullRequest); + ghprbRepository.addPullRequests(pulls); given(ghPullRequest.getUpdatedAt()).willReturn(UPDATE_DATE); given(ghPullRequest.getNumber()).willReturn(1); @@ -168,23 +192,25 @@ public void testCheckMethodWithOnlyExistingPRs() throws IOException { ghprbRepository.check(); // THEN - verifyGetGithub(1); + verifyGetGithub(2, 1); /** GH Repo verifications */ verify(ghRepository, only()).getPullRequests(OPEN); // Call to Github API verifyNoMoreInteractions(ghRepository); /** GH PR verifications */ - verify(ghPullRequest, times(3)).getHead(); - verify(ghPullRequest, times(1)).getBase(); + verify(ghPullRequest, times(2)).getHead(); verify(ghPullRequest, times(2)).getNumber(); verify(ghPullRequest, times(1)).getUpdatedAt(); - verify(ghPullRequest, times(1)).getBody(); + verify(ghPullRequest, times(1)).getCreatedAt(); + verify(ghPullRequest, times(1)).getUser(); + verify(ghPullRequest, times(1)).getBase(); verifyNoMoreInteractions(ghPullRequest); verify(helper).ifOnlyTriggerPhrase(); verify(helper).getWhiteListTargetBranches(); - verify(helper, times(3)).isProjectDisabled(); + verify(helper, times(2)).isProjectDisabled(); + verify(helper).checkSkipBuild(eq(ghPullRequest)); verifyNoMoreInteractions(helper); verifyNoMoreInteractions(gt); @@ -211,7 +237,9 @@ public void testCheckMethodWithNewPR() throws IOException { given(ghPullRequest.getUser()).willReturn(ghUser); given(ghPullRequest.getHtmlUrl()).willReturn(new URL("https://github.com/org/repo/pull/100")); given(ghPullRequest.getApiURL()).willReturn(new URL("https://github.com/org/repo/pull/100")); - + given(ghPullRequest.getId()).willReturn(100); + given(ghRepository.getPullRequest(ghPullRequest.getId())).willReturn(ghPullRequest); + given(ghUser.getEmail()).willReturn("email"); given(helper.ifOnlyTriggerPhrase()).willReturn(false); @@ -222,33 +250,36 @@ public void testCheckMethodWithNewPR() throws IOException { ghprbRepository.check(); // THEN - - verifyGetGithub(1); + verifyGetGithub(2, 1); verifyNoMoreInteractions(gt); /** GH PR verifications */ verify(builds, times(1)).build(any(GhprbPullRequest.class), any(GHUser.class), any(String.class)); verify(ghRepository, times(1)).getPullRequests(OPEN); // Call to Github API - verify(ghRepository, times(1)).createCommitStatus(eq("head sha"), eq(PENDING), isNull(String.class), eq(msg), eq("default")); // Call to Github API + verify(ghRepository, times(1)).createCommitStatus(eq("head sha"), eq(PENDING), eq(""), eq(msg), eq("default")); // Call to Github API + verify(ghRepository, times(1)).getPullRequest(Mockito.anyInt()); verifyNoMoreInteractions(ghRepository); verify(ghPullRequest, times(1)).getTitle(); - verify(ghPullRequest, times(2)).getUser(); + verify(ghPullRequest, times(6)).getUser(); verify(ghPullRequest, times(1)).getMergeable(); // Call to Github API - verify(ghPullRequest, times(8)).getHead(); - verify(ghPullRequest, times(3)).getBase(); + verify(ghPullRequest, times(7)).getHead(); + verify(ghPullRequest, times(4)).getBase(); verify(ghPullRequest, times(5)).getNumber(); - verify(ghPullRequest, times(3)).getUpdatedAt(); + verify(ghPullRequest, times(2)).getUpdatedAt(); + verify(ghPullRequest, times(1)).getCreatedAt(); verify(ghPullRequest, times(1)).getHtmlUrl(); verify(ghPullRequest, times(1)).listCommits(); - verify(ghPullRequest, times(2)).getBody(); + verify(ghPullRequest, times(1)).getBody(); + verify(ghPullRequest, times(1)).getId(); verifyNoMoreInteractions(ghPullRequest); verify(helper, times(1)).isWhitelisted(eq(ghUser)); // Call to Github API verify(helper, times(2)).ifOnlyTriggerPhrase(); verify(helper, times(1)).getBuilds(); verify(helper, times(2)).getWhiteListTargetBranches(); - verify(helper, times(5)).isProjectDisabled(); + verify(helper, times(4)).isProjectDisabled(); + verify(helper, times(2)).checkSkipBuild(eq(ghPullRequest)); verifyNoMoreInteractions(helper); verify(ghUser, times(1)).getEmail(); // Call to Github API @@ -272,19 +303,22 @@ public void testCheckMethodWhenPrWasUpdatedWithNonKeyPhrase() throws IOException mockCommitList(); GhprbBuilds builds = mockBuilds(); - Date now = new Date(); + Date later = new DateTime().plusHours(3).toDate(); Date tomorrow = new DateTime().plusDays(1).toDate(); given(ghRepository.getPullRequests(eq(GHIssueState.OPEN))).willReturn(ghPullRequests); - given(ghPullRequest.getUpdatedAt()).willReturn(now).willReturn(now).willReturn(tomorrow); + + given(ghPullRequest.getUpdatedAt()).willReturn(later).willReturn(tomorrow); given(ghPullRequest.getNumber()).willReturn(100); given(ghPullRequest.getMergeable()).willReturn(true); given(ghPullRequest.getTitle()).willReturn("title"); given(ghPullRequest.getUser()).willReturn(ghUser); given(ghPullRequest.getHtmlUrl()).willReturn(new URL("https://github.com/org/repo/pull/100")); given(ghPullRequest.getApiURL()).willReturn(new URL("https://github.com/org/repo/pull/100")); - + given(ghPullRequest.getId()).willReturn(100); + given(ghRepository.getPullRequest(ghPullRequest.getId())).willReturn(ghPullRequest); + given(ghUser.getEmail()).willReturn("email"); given(ghUser.getLogin()).willReturn("login"); @@ -299,27 +333,30 @@ public void testCheckMethodWhenPrWasUpdatedWithNonKeyPhrase() throws IOException ghprbRepository.check(); // PR was updated // THEN - verifyGetGithub(2); + verifyGetGithub(2, 1); verifyNoMoreInteractions(gt); /** GH PR verifications */ verify(builds, times(1)).build(any(GhprbPullRequest.class), any(GHUser.class), any(String.class)); verify(ghRepository, times(2)).getPullRequests(eq(OPEN)); // Call to Github API - verify(ghRepository, times(1)).createCommitStatus(eq("head sha"), eq(PENDING), isNull(String.class), eq(msg), eq("default")); // Call to Github API + verify(ghRepository, times(1)).createCommitStatus(eq("head sha"), eq(PENDING), eq(""), eq(msg), eq("default")); // Call to Github API + verify(ghRepository, times(1)).getPullRequest(Mockito.anyInt()); verifyNoMoreInteractions(ghRepository); - verify(ghPullRequest, times(2)).getTitle(); - verify(ghPullRequest, times(2)).getUser(); + verify(ghPullRequest, times(1)).getTitle(); + verify(ghPullRequest, times(6)).getUser(); verify(ghPullRequest, times(1)).getMergeable(); // Call to Github API - verify(ghPullRequest, times(8)).getHead(); - verify(ghPullRequest, times(3)).getBase(); + verify(ghPullRequest, times(9)).getHead(); + verify(ghPullRequest, times(6)).getBase(); verify(ghPullRequest, times(5)).getNumber(); verify(ghPullRequest, times(1)).getHtmlUrl(); - verify(ghPullRequest, times(3)).getUpdatedAt(); + verify(ghPullRequest, times(2)).getUpdatedAt(); + verify(ghPullRequest, times(1)).getCreatedAt(); - verify(ghPullRequest, times(1)).getComments(); + verify(ghPullRequest, times(2)).getComments(); verify(ghPullRequest, times(1)).listCommits(); - verify(ghPullRequest, times(2)).getBody(); + verify(ghPullRequest, times(1)).getBody(); + verify(ghPullRequest, times(1)).getId(); verifyNoMoreInteractions(ghPullRequest); verify(helper, times(1)).isWhitelisted(eq(ghUser)); // Call to Github API @@ -332,15 +369,19 @@ public void testCheckMethodWhenPrWasUpdatedWithNonKeyPhrase() throws IOException verify(helper).isOktotestPhrase(eq("comment body")); verify(helper).isRetestPhrase(eq("comment body")); verify(helper).isTriggerPhrase(eq("comment body")); - verify(helper, times(6)).isProjectDisabled(); + verify(helper, times(4)).isProjectDisabled(); + verify(helper, times(2)).checkSkipBuild(eq(ghPullRequest)); verifyNoMoreInteractions(helper); verify(ghUser, times(1)).getEmail(); // Call to Github API verify(ghUser, times(1)).getLogin(); + verify(ghUser, times(1)).getName(); verifyNoMoreInteractions(ghUser); } - private List createListWithMockPR() { + private List createListWithMockPR() throws IOException { + + given(ghPullRequest.getCreatedAt()).willReturn(new Date()); List ghPullRequests = new ArrayList(); ghPullRequests.add(ghPullRequest); return ghPullRequests; @@ -357,15 +398,17 @@ public void testCheckMethodWhenPrWasUpdatedWithRetestPhrase() throws IOException mockCommitList(); GhprbBuilds builds = mockBuilds(); - given(ghRepository.getPullRequests(eq(GHIssueState.OPEN))).willReturn(ghPullRequests); - given(ghPullRequest.getUpdatedAt()).willReturn(now).willReturn(now).willReturn(tomorrow); + given(ghPullRequest.getUpdatedAt()).willReturn(now).willReturn(tomorrow); given(ghPullRequest.getNumber()).willReturn(100); given(ghPullRequest.getMergeable()).willReturn(true); given(ghPullRequest.getTitle()).willReturn("title"); given(ghPullRequest.getUser()).willReturn(ghUser); given(ghPullRequest.getHtmlUrl()).willReturn(new URL("https://github.com/org/repo/pull/100")); given(ghPullRequest.getApiURL()).willReturn(new URL("https://github.com/org/repo/pull/100")); - + given(ghPullRequest.getId()).willReturn(100); + given(ghRepository.getPullRequest(ghPullRequest.getId())).willReturn(ghPullRequest); + given(ghRepository.getPullRequests(eq(GHIssueState.OPEN))).willReturn(ghPullRequests); + given(ghUser.getEmail()).willReturn("email"); given(ghUser.getLogin()).willReturn("login"); @@ -381,7 +424,7 @@ public void testCheckMethodWhenPrWasUpdatedWithRetestPhrase() throws IOException ghprbRepository.check(); // PR was updated // THEN - verifyGetGithub(2); + verifyGetGithub(2, 1); verifyNoMoreInteractions(gt); /** GH PR verifications */ @@ -389,18 +432,21 @@ public void testCheckMethodWhenPrWasUpdatedWithRetestPhrase() throws IOException verifyNoMoreInteractions(builds); verify(ghRepository, times(2)).getPullRequests(eq(OPEN)); // Call to Github API - verify(ghRepository, times(2)).createCommitStatus(eq("head sha"), eq(PENDING), isNull(String.class), eq(msg), eq("default")); // Call to Github API + verify(ghRepository, times(2)).createCommitStatus(eq("head sha"), eq(PENDING), eq(""), eq(msg), eq("default")); // Call to Github API + verify(ghRepository, times(1)).getPullRequest(Mockito.anyInt()); verifyNoMoreInteractions(ghRepository); verify(ghPullRequest, times(2)).getTitle(); - verify(ghPullRequest, times(2)).getUser(); + verify(ghPullRequest, times(7)).getUser(); verify(ghPullRequest, times(2)).getMergeable(); // Call to Github API - verify(ghPullRequest, times(8)).getHead(); - verify(ghPullRequest, times(3)).getBase(); + verify(ghPullRequest, times(10)).getHead(); + verify(ghPullRequest, times(6)).getBase(); verify(ghPullRequest, times(5)).getNumber(); - verify(ghPullRequest, times(3)).getUpdatedAt(); - verify(ghPullRequest, times(1)).getHtmlUrl(); + verify(ghPullRequest, times(2)).getUpdatedAt(); + verify(ghPullRequest, times(1)).getCreatedAt(); + verify(ghPullRequest, times(2)).getHtmlUrl(); + verify(ghPullRequest, times(1)).getId(); verify(ghPullRequest, times(1)).getComments(); verify(ghPullRequest, times(2)).listCommits(); @@ -416,11 +462,13 @@ public void testCheckMethodWhenPrWasUpdatedWithRetestPhrase() throws IOException verify(helper).isOktotestPhrase(eq("test this please")); verify(helper).isRetestPhrase(eq("test this please")); verify(helper).isAdmin(eq(ghUser)); - verify(helper, times(6)).isProjectDisabled(); + verify(helper, times(4)).isProjectDisabled(); + verify(helper, times(2)).checkSkipBuild(eq(ghPullRequest)); verifyNoMoreInteractions(helper); verify(ghUser, times(1)).getEmail(); // Call to Github API verify(ghUser, times(1)).getLogin(); + verify(ghUser, times(1)).getName(); verifyNoMoreInteractions(ghUser); verify(builds, times(2)).build(any(GhprbPullRequest.class), any(GHUser.class), any(String.class)); @@ -459,18 +507,18 @@ public void testCheckMethodWithNoPR() throws IOException { // GIVEN List ghPullRequests = new ArrayList(); given(ghRepository.getPullRequests(eq(GHIssueState.OPEN))).willReturn(ghPullRequests); + given(ghRepository.getPullRequest(ghPullRequest.getId())).willReturn(ghPullRequest); // WHEN ghprbRepository.check(); - verify(helper).isProjectDisabled(); + verify(trigger).isActive(); // THEN - verifyGetGithub(1); + verifyGetGithub(2, 1); verifyNoMoreInteractions(gt); verify(ghRepository, times(1)).getPullRequests(OPEN); // Call to Github API - verifyNoMoreInteractions(ghRepository); - verifyZeroInteractions(helper); + verifyNoMoreInteractions(helper, ghRepository); } @Test @@ -482,9 +530,8 @@ public void testExceedRateLimit() throws IOException { ghprbRepository.check(); // THEN - verify(helper, only()).getGitHub(); - verify(gitHub, only()).get(); - verify(gt, only()).getRateLimit(); + verify(trigger, times(2)).getGitHub(); + verifyGetGithub(2, 0); verifyZeroInteractions(ghRepository); verifyZeroInteractions(gitHub); verifyZeroInteractions(gt); @@ -497,9 +544,16 @@ public void testSignature() throws IOException, InvalidKeyException, NoSuchAlgor String actualSignature = createSHA1Signature(actualSecret, body); String fakeSignature = createSHA1Signature("abc", body); + GhprbGitHubAuth ghAuth = Mockito.spy(new GhprbGitHubAuth("", "", "", "", "", actualSecret)); + doReturn(true).when(trigger).isActive(); + + doReturn(ghAuth).when(trigger).getGitHubApiAuth(); + Assert.assertFalse(actualSignature.equals(fakeSignature)); - Assert.assertTrue(GhprbWebHook.checkSignature(body, actualSignature, actualSecret)); - Assert.assertFalse(GhprbWebHook.checkSignature(body, fakeSignature, actualSecret)); + Assert.assertTrue(actualSecret.equals(ghAuth.getSecret())); + + Assert.assertTrue(trigger.matchSignature(body, actualSignature)); + Assert.assertFalse(trigger.matchSignature(body, fakeSignature)); } private String createSHA1Signature(String secret, String body) throws UnsupportedEncodingException, NoSuchAlgorithmException, InvalidKeyException { @@ -527,10 +581,15 @@ private void initGHPRWithTestData() throws IOException { given(ghPullRequest.getHead()).willReturn(head); given(head.getSha()).willReturn("head sha"); - pulls = new ConcurrentHashMap(); - ghprbRepository = new GhprbRepository(TEST_USER_NAME, TEST_REPO_NAME, helper, pulls); - ghprbPullRequest = new GhprbPullRequest(ghPullRequest, helper, ghprbRepository); + ghprbRepository = spy(new GhprbRepository(TEST_REPO_NAME, trigger)); + Mockito.doNothing().when(ghprbRepository).addComment(Mockito.anyInt(), anyString()); + Mockito.doNothing().when(ghprbRepository).addComment(Mockito.anyInt(), anyString(), any(AbstractBuild.class), any(TaskListener.class)); + + doReturn(ghprbRepository).when(trigger).getRepository(); + + ghprbPullRequest = new GhprbPullRequest(ghPullRequest, helper, ghprbRepository, false); + // Reset mocks not to mix init data invocations with tests reset(ghPullRequest, ghUser, helper, head, base); } @@ -540,10 +599,9 @@ private void increaseRateLimitToDefaults() { } // Verifications - private void verifyGetGithub(int callsCount) throws IOException { - verify(helper, times(callsCount)).getGitHub(); - verify(gitHub, times(callsCount)).get(); // Call to Github API (once, than cached) - verify(gt, times(1)).getRepository(anyString()); // Call to Github API + private void verifyGetGithub(int callsCount, int repoTimes) throws IOException { + verify(trigger, times(callsCount)).getGitHub(); verify(gt, times(callsCount)).getRateLimit(); + verify(gt, times(repoTimes)).getRepository(anyString()); } } diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRootActionTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRootActionTest.java index 033fc2fc3..afcb41968 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRootActionTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRootActionTest.java @@ -7,6 +7,7 @@ import java.io.ByteArrayInputStream; import java.io.InputStream; import java.io.InputStreamReader; +import java.io.Reader; import java.io.StringReader; import java.net.URLEncoder; @@ -17,13 +18,15 @@ import org.junit.runner.RunWith; import org.jvnet.hudson.test.JenkinsRule; import org.kohsuke.github.GHCommitPointer; +import org.kohsuke.github.GHEventPayload.IssueComment; +import org.kohsuke.github.GHIssueComment; import org.kohsuke.github.GHPullRequest; -import org.kohsuke.github.GHRateLimit; import org.kohsuke.github.GHRepository; import org.kohsuke.github.GHUser; import org.kohsuke.github.GitHub; import org.kohsuke.stapler.StaplerRequest; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.runners.MockitoJUnitRunner; import com.coravy.hudson.plugins.github.GithubProjectProperty; @@ -54,10 +57,6 @@ public class GhprbRootActionTest { @Mock protected GHUser ghUser; - protected GitHub gitHub; - // Stubs - protected GHRateLimit ghRateLimit = new GHRateLimit(); - @Rule public JenkinsRule jenkinsRule = new JenkinsRule(); @@ -66,13 +65,18 @@ public class GhprbRootActionTest { private StaplerRequest req; private BufferedReader br; + + private GhprbTrigger trigger; + + + private final int prId = 1; @Before public void setup() throws Exception { - gitHub = spy(GitHub.connectAnonymously()); - given(ghprbGitHub.get()).willReturn(gitHub); - given(gitHub.getRateLimit()).willReturn(ghRateLimit); - doReturn(ghRepository).when(gitHub).getRepository(anyString()); + trigger = GhprbTestUtil.getTrigger(); + GitHub gitHub = trigger.getGitHub(); + + given(gitHub.getRepository(anyString())).willReturn(ghRepository); given(commitPointer.getRef()).willReturn("ref"); given(ghRepository.getName()).willReturn("dropwizard"); @@ -83,8 +87,8 @@ public void setup() throws Exception { given(ghPullRequest.getUser()).willReturn(ghUser); given(ghUser.getEmail()).willReturn("email@email.com"); given(ghUser.getLogin()).willReturn("user"); + given(ghUser.getName()).willReturn("User"); - ghRateLimit.remaining = GhprbTestUtil.INITIAL_RATE_LIMIT; GhprbTestUtil.mockCommitList(ghPullRequest); } @@ -93,28 +97,35 @@ public void setup() throws Exception { public void testUrlEncoded() throws Exception { // GIVEN FreeStyleProject project = jenkinsRule.createFreeStyleProject("testUrlEncoded"); - GhprbTrigger trigger = spy(GhprbTestUtil.getTrigger(null)); + + doReturn(project).when(trigger).getActualProject(); + doReturn(true).when(trigger).getUseGitHubHooks(); + given(commitPointer.getSha()).willReturn("sha1"); GhprbTestUtil.setupGhprbTriggerDescriptor(null); project.addProperty(new GithubProjectProperty("https://github.com/user/dropwizard")); - given(ghPullRequest.getNumber()).willReturn(1); - Ghprb ghprb = spy(trigger.createGhprb(project)); + given(ghPullRequest.getId()).willReturn(prId); + given(ghPullRequest.getNumber()).willReturn(prId); + given(ghRepository.getPullRequest(prId)).willReturn(ghPullRequest); + Ghprb ghprb = spy(new Ghprb(trigger)); doReturn(ghprbGitHub).when(ghprb).getGitHub(); + doReturn(true).when(ghprb).isAdmin(Mockito.any(GHUser.class)); + trigger.start(project, true); trigger.setHelper(ghprb); - ghprb.getRepository().setHelper(ghprb); + project.addTrigger(trigger); GitSCM scm = GhprbTestUtil.provideGitSCM(); project.setScm(scm); GhprbTestUtil.triggerRunAndWait(10, trigger, project); - assertThat(project.getBuilds().toArray().length).isEqualTo(1); - - doReturn(gitHub).when(trigger).getGitHub(); + assertThat(project.getBuilds().toArray().length).isEqualTo(0); BufferedReader br = new BufferedReader(new StringReader( "payload=" + URLEncoder.encode(GhprbTestUtil.PAYLOAD, "UTF-8"))); + + given(req.getContentType()).willReturn("application/x-www-form-urlencoded"); given(req.getParameter("payload")).willReturn(GhprbTestUtil.PAYLOAD); @@ -122,27 +133,44 @@ public void testUrlEncoded() throws Exception { given(req.getReader()).willReturn(br); given(req.getCharacterEncoding()).willReturn("UTF-8"); + + StringReader brTest = new StringReader(GhprbTestUtil.PAYLOAD); + + IssueComment issueComment = spy(GitHub.connectAnonymously().parseEventPayload(brTest, IssueComment.class)); + brTest.close(); + + GHIssueComment ghIssueComment = spy(issueComment.getComment()); + + Mockito.when(issueComment.getComment()).thenReturn(ghIssueComment); + Mockito.when(ghIssueComment.getUser()).thenReturn(ghUser); + + + given(trigger.getGitHub().parseEventPayload(Mockito.any(Reader.class), Mockito.eq(IssueComment.class))).willReturn(issueComment); + GhprbRootAction ra = new GhprbRootAction(); ra.doIndex(req, null); GhprbTestUtil.waitForBuildsToFinish(project); - assertThat(project.getBuilds().toArray().length).isEqualTo(2); + assertThat(project.getBuilds().toArray().length).isEqualTo(1); } @Test public void disabledJobsDontBuild() throws Exception { // GIVEN FreeStyleProject project = jenkinsRule.createFreeStyleProject("disabledJobsDontBuild"); - GhprbTrigger trigger = spy(GhprbTestUtil.getTrigger(null)); + doReturn(project).when(trigger).getActualProject(); + given(commitPointer.getSha()).willReturn("sha1"); GhprbTestUtil.setupGhprbTriggerDescriptor(null); project.addProperty(new GithubProjectProperty("https://github.com/user/dropwizard")); - given(ghPullRequest.getNumber()).willReturn(1); - Ghprb ghprb = spy(trigger.createGhprb(project)); + given(ghPullRequest.getId()).willReturn(prId); + given(ghPullRequest.getNumber()).willReturn(prId); + given(ghRepository.getPullRequest(prId)).willReturn(ghPullRequest); + Ghprb ghprb = spy(new Ghprb(trigger)); doReturn(ghprbGitHub).when(ghprb).getGitHub(); trigger.start(project, true); trigger.setHelper(ghprb); - ghprb.getRepository().setHelper(ghprb); + project.addTrigger(trigger); GitSCM scm = GhprbTestUtil.provideGitSCM(); project.setScm(scm); @@ -153,8 +181,6 @@ public void disabledJobsDontBuild() throws Exception { project.disable(); - doReturn(gitHub).when(trigger).getGitHub(); - BufferedReader br = new BufferedReader(new StringReader( "payload=" + URLEncoder.encode(GhprbTestUtil.PAYLOAD, "UTF-8"))); diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java index 381ea1b02..e7410d653 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java @@ -15,16 +15,28 @@ package org.jenkinsci.plugins.ghprb; import static com.google.common.collect.Lists.newArrayList; +import static org.fest.assertions.Assertions.assertThat; import static org.mockito.BDDMockito.given; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import java.net.URL; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import org.codehaus.plexus.util.StringUtils; import org.joda.time.DateTime; +import org.junit.Test; import org.kohsuke.github.GHCommitPointer; import org.kohsuke.github.GHPullRequest; +import org.kohsuke.github.GHRateLimit; +import org.kohsuke.github.GitHub; import org.kohsuke.github.PagedIterable; import org.kohsuke.github.PagedIterator; import org.kohsuke.stapler.BindInterceptor; @@ -37,6 +49,7 @@ import hudson.plugins.git.UserRemoteConfig; import net.sf.json.JSONObject; import static org.mockito.Matchers.any; +import static org.mockito.Mockito.spy; public class GhprbTestUtil { @@ -345,6 +358,11 @@ public static GitSCM provideGitSCM() { null); } + + public static GhprbTrigger getTrigger() throws Exception { + return getTrigger(null); + } + public static GhprbTrigger getTrigger(Map values) throws Exception { setupReq(); if (values == null) { @@ -369,7 +387,15 @@ public static GhprbTrigger getTrigger(Map values) throws Excepti defaults.put(next.getKey(), next.getValue()); } - GhprbTrigger trigger = req.bindJSON(GhprbTrigger.class, defaults); + GhprbTrigger trigger = spy(req.bindJSON(GhprbTrigger.class, defaults)); + + GHRateLimit limit = new GHRateLimit(); + limit.remaining = INITIAL_RATE_LIMIT; + + GitHub github = Mockito.mock(GitHub.class); + given(github.getRateLimit()).willReturn(limit); + + Mockito.doReturn(github).when(trigger).getGitHub(); return trigger; } @@ -388,6 +414,40 @@ public static void triggerRunAndWait(int numOfTriggers, GhprbTrigger trigger, Ab } } + + + public static List checkClassForGetters(Class clazz) { + Field[] fields = clazz.getDeclaredFields(); + List xmlFields = new ArrayList(); + List errors = new ArrayList(); + + for (Field field : fields) { + int modifiers = field.getModifiers(); + if (modifiers == (Modifier.PRIVATE) || modifiers == (Modifier.PRIVATE | Modifier.FINAL)) { + xmlFields.add(field); + } + } + + for (Field field : xmlFields) { + String getter = "get" + StringUtils.capitalise(field.getName()); + try { + Method method = clazz.getDeclaredMethod(getter); + int modifier = method.getModifiers(); + if (!Modifier.isPublic(modifier)) { + errors.add(getter + " is not a public method"); + } + } catch (Exception e) { + String wrongGetter = "is" + StringUtils.capitalise(field.getName()); + try { + clazz.getDeclaredMethod(wrongGetter); + errors.add("Setter is using the wrong name, is " + wrongGetter + " and should be " + getter); + } catch(Exception err) { + errors.add("Missing " + getter); + } + } + } + return errors; + } private GhprbTestUtil() {} diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTriggerTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTriggerTest.java index 0022e0412..b7853f827 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTriggerTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTriggerTest.java @@ -5,16 +5,23 @@ import static org.mockito.Mockito.*; import java.lang.reflect.Field; -import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.Arrays; +import java.util.HashMap; import java.util.HashSet; +import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import java.util.regex.Pattern; +import org.jenkinsci.plugins.ghprb.extensions.GhprbExtension; +import org.jenkinsci.plugins.ghprb.extensions.GhprbGlobalDefault; +import org.jenkinsci.plugins.ghprb.extensions.build.GhprbCancelBuildsOnUpdate; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; -import org.kohsuke.github.GHIssue; +import org.jvnet.hudson.test.JenkinsRule; +import org.kohsuke.github.GHPullRequest; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; @@ -24,42 +31,119 @@ @RunWith(MockitoJUnitRunner.class) public class GhprbTriggerTest { + @Rule + public JenkinsRule jenkinsRule = new JenkinsRule(); + @Mock private GhprbPullRequest pr; + + @Mock + private Ghprb helper; + + @Test + public void testGlobalExtensions() throws Exception { + GhprbTrigger.getDscp().getExtensions().add(new GhprbCancelBuildsOnUpdate(false)); + + GhprbTrigger trigger = GhprbTestUtil.getTrigger(); + + for (GhprbExtension ext : trigger.getDescriptor().getExtensions()) { + if (ext instanceof GhprbGlobalDefault) { + assertThat(trigger.getExtensions().contains(ext)); + } + } + } @Test public void testCheckSkipBuild() throws Exception { - GHIssue issue = mock(GHIssue.class); - - String[] comments = { "Some dumb comment\r\nThat shouldn't match", "[skip ci]" }; - String[] phraseArray = { "\\[skip\\W+ci\\]", "skip ci" }; - - Method checkSkip = GhprbPullRequest.class.getDeclaredMethod("checkSkipBuild", GHIssue.class); + GHPullRequest issue = mock(GHPullRequest.class); + + boolean skipBuild = false; + boolean build = true; + + String nonMatch = "Some dumb comment\r\nThat shouldn't match"; + String multiLine = "This is a multiline skip\r\n[skip ci]"; + String justSkipCi = "skip ci"; + String fullSkipCi = "[skip ci]"; + + Map> stringsToTest = new HashMap>(10); + + Map comment = new HashMap(5); + comment.put(nonMatch, build); + comment.put(multiLine, skipBuild); + comment.put(justSkipCi, build); + comment.put(fullSkipCi, skipBuild); + stringsToTest.put(".*\\[skip\\W+ci\\].*", comment); + + comment = new HashMap(5); + comment.put(nonMatch, build); + comment.put(multiLine, build); + comment.put(justSkipCi, build); + comment.put(fullSkipCi, skipBuild); + stringsToTest.put("\\[skip ci\\]", comment); + + comment = new HashMap(5); + comment.put(nonMatch, build); + comment.put(multiLine, skipBuild); + comment.put(justSkipCi, skipBuild); + comment.put(fullSkipCi, skipBuild); + stringsToTest.put("\\[skip ci\\]\n.*\\[skip\\W+ci\\].*\nskip ci", comment); + + Method checkSkip = GhprbPullRequest.class.getDeclaredMethod("checkSkipBuild"); checkSkip.setAccessible(true); + + + Field prField = GhprbPullRequest.class.getDeclaredField("pr"); + prField.setAccessible(true); + prField.set(pr, issue); Field shouldRun = GhprbPullRequest.class.getDeclaredField("shouldRun"); shouldRun.setAccessible(true); + + Field prHelper = GhprbPullRequest.class.getDeclaredField("helper"); + prHelper.setAccessible(true); + prHelper.set(pr, helper); + + for (Entry> skipMap : stringsToTest.entrySet()) { + String skipPhrases = skipMap.getKey(); - for (String phraseString : phraseArray) { - for (String comment : comments) { + Set phrases = new HashSet(Arrays.asList(skipPhrases.split("[\\r\\n]+"))); - Set phrases = new HashSet(Arrays.asList(phraseString.split("[\\r\\n]+"))); - given(issue.getBody()).willReturn(comment); - given(pr.getSkipBuildPhrases()).willReturn(phrases); - boolean isMatch = false; - for (String phrase : phrases) { - isMatch = Pattern.matches(phrase, comment); - if (isMatch) { + given(helper.getSkipBuildPhrases()).willReturn(phrases); + + for (Entry skipResults : skipMap.getValue().entrySet()) { + String nextComment = skipResults.getKey(); + Boolean shouldBuild = skipResults.getValue(); + + given(issue.getBody()).willReturn(nextComment); + String skipPhrase = ""; + + for (String skipBuildPhrase : phrases) { + skipBuildPhrase = skipBuildPhrase.trim(); + Pattern skipBuildPhrasePattern = Ghprb.compilePattern(skipBuildPhrase); + + if (skipBuildPhrasePattern.matcher(nextComment).matches()) { + skipPhrase = skipBuildPhrase; break; } } + + given(helper.checkSkipBuild(issue)).willReturn(skipPhrase); + shouldRun.set(pr, true); - checkSkip.invoke(pr, issue); - assertThat(shouldRun.get(pr)).isEqualTo(!isMatch); + checkSkip.invoke(pr); + String errorMessage = String.format("Comment does %scontain skip phrase \n(\n%s\n)\n[\n%s\n]", shouldBuild ? "not ": "", nextComment, skipPhrases); + + if (shouldBuild) { + assertThat(skipPhrase).overridingErrorMessage(errorMessage).isEmpty(); + } else { + assertThat(skipPhrase).overridingErrorMessage(errorMessage).isNotEmpty(); + } + + assertThat(shouldRun.get(pr)).overridingErrorMessage(errorMessage).isEqualTo(shouldBuild); } } - } + } diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatusTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatusTest.java index 9396c4d2b..212546b46 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatusTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/extensions/status/GhprbSimpleStatusTest.java @@ -1,9 +1,13 @@ package org.jenkinsci.plugins.ghprb.extensions.status; import org.jenkinsci.plugins.ghprb.GhprbPullRequest; +import org.jenkinsci.plugins.ghprb.GhprbTestUtil; import org.jenkinsci.plugins.ghprb.GhprbTrigger; +import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; +import org.jvnet.hudson.test.JenkinsRule; import org.kohsuke.github.GHCommitState; import org.kohsuke.github.GHRepository; import org.mockito.Mock; @@ -19,13 +23,22 @@ @RunWith(MockitoJUnitRunner.class) public class GhprbSimpleStatusTest { + @Rule + public JenkinsRule jenkinsRule = new JenkinsRule(); + @Mock private GHRepository ghRepository; @Mock private GhprbPullRequest ghprbPullRequest; - @Mock + private GhprbTrigger trigger; + + @Before + public void setUp() throws Exception { + trigger = GhprbTestUtil.getTrigger(null); + } + @Test public void testMergedMessage() throws Exception { String mergedMessage = "Build triggered. sha1 is merged."; @@ -33,13 +46,11 @@ public void testMergedMessage() throws Exception { given(ghprbPullRequest.isMergeable()).willReturn(true); GhprbSimpleStatus status = spy(new GhprbSimpleStatus("default")); - status.onBuildTriggered(trigger, ghprbPullRequest, ghRepository); + status.onBuildTriggered(trigger.getActualProject(), "sha", true, 1, ghRepository); - verify(ghRepository).createCommitStatus(eq("sha"), eq(GHCommitState.PENDING), isNull(String.class), eq(mergedMessage), eq("default")); + verify(ghRepository).createCommitStatus(eq("sha"), eq(GHCommitState.PENDING), eq(""), eq(mergedMessage), eq("default")); verifyNoMoreInteractions(ghRepository); - verify(ghprbPullRequest).getHead(); - verify(ghprbPullRequest).isMergeable(); verifyNoMoreInteractions(ghprbPullRequest); } @@ -50,13 +61,11 @@ public void testMergeConflictMessage() throws Exception { given(ghprbPullRequest.isMergeable()).willReturn(false); GhprbSimpleStatus status = spy(new GhprbSimpleStatus("default")); - status.onBuildTriggered(trigger, ghprbPullRequest, ghRepository); + status.onBuildTriggered(trigger.getActualProject(), "sha", false, 1, ghRepository); - verify(ghRepository).createCommitStatus(eq("sha"), eq(GHCommitState.PENDING), isNull(String.class), eq(mergedMessage), eq("default")); + verify(ghRepository).createCommitStatus(eq("sha"), eq(GHCommitState.PENDING), eq(""), eq(mergedMessage), eq("default")); verifyNoMoreInteractions(ghRepository); - verify(ghprbPullRequest).getHead(); - verify(ghprbPullRequest).isMergeable(); verifyNoMoreInteractions(ghprbPullRequest); } @@ -67,13 +76,11 @@ public void testDoesNotSendEmptyContext() throws Exception { given(ghprbPullRequest.isMergeable()).willReturn(false); GhprbSimpleStatus status = spy(new GhprbSimpleStatus("")); - status.onBuildTriggered(trigger, ghprbPullRequest, ghRepository); + status.onBuildTriggered(trigger.getActualProject(), "sha", false, 1, ghRepository); - verify(ghRepository).createCommitStatus(eq("sha"), eq(GHCommitState.PENDING), isNull(String.class), eq(mergedMessage), isNull(String.class)); + verify(ghRepository).createCommitStatus(eq("sha"), eq(GHCommitState.PENDING), eq(""), eq(mergedMessage), isNull(String.class)); verifyNoMoreInteractions(ghRepository); - verify(ghprbPullRequest).getHead(); - verify(ghprbPullRequest).isMergeable(); verifyNoMoreInteractions(ghprbPullRequest); } } diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbDefaultBuildManagerTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbDefaultBuildManagerTest.java index 3045eb833..ed12d6ffc 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbDefaultBuildManagerTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/GhprbDefaultBuildManagerTest.java @@ -2,20 +2,16 @@ import static org.fest.assertions.Assertions.assertThat; import static org.mockito.BDDMockito.given; -import static org.mockito.Mockito.doReturn; import java.util.HashMap; import java.util.Map; -import com.coravy.hudson.plugins.github.GithubProjectProperty; import hudson.matrix.MatrixBuild; import hudson.matrix.MatrixProject; -import org.jenkinsci.plugins.ghprb.Ghprb; import org.jenkinsci.plugins.ghprb.GhprbITBaseTestCase; import org.jenkinsci.plugins.ghprb.GhprbTestUtil; -import org.jenkinsci.plugins.ghprb.GhprbTrigger; import org.jenkinsci.plugins.ghprb.manager.GhprbBuildManager; import org.jenkinsci.plugins.ghprb.manager.factory.GhprbBuildManagerFactoryUtil; import org.junit.Before; @@ -33,17 +29,25 @@ public class GhprbDefaultBuildManagerTest extends GhprbITBaseTestCase { @Rule public JenkinsRule jenkinsRule = new JenkinsRule(); + + private MatrixProject project; @Before public void setUp() throws Exception { // GhprbTestUtil.mockGithubUserPage(); - super.beforeTest(); + project = jenkinsRule.createMatrixProject("MTXPRJ"); + + Map config = new HashMap(1); + + config.put("publishedURL", "defaultPublishedURL"); + super.beforeTest(config, null, project); } @Test public void shouldCalculateUrlFromDefault() throws Exception { + // GIVEN - MatrixProject project = givenThatGhprbHasBeenTriggeredForAMatrixProject(); + givenThatGhprbHasBeenTriggeredForAMatrixProject(); // THEN assertThat(project.getBuilds().toArray().length).isEqualTo(1); @@ -57,44 +61,14 @@ public void shouldCalculateUrlFromDefault() throws Exception { assertThat(buildManager.calculateBuildUrl("defaultPublishedURL")).isEqualTo("defaultPublishedURL/" + matrixBuild.getUrl()); } - private MatrixProject givenThatGhprbHasBeenTriggeredForAMatrixProject() throws Exception { - MatrixProject project = jenkinsRule.createMatrixProject("MTXPRJ"); - - GhprbTrigger trigger = GhprbTestUtil.getTrigger(null); - + private void givenThatGhprbHasBeenTriggeredForAMatrixProject() throws Exception { given(commitPointer.getSha()).willReturn("sha"); - Map config = new HashMap(1); - config.put("publishedURL", "defaultPublishedURL"); - - GhprbTestUtil.setupGhprbTriggerDescriptor(config); - - - project.addProperty(new GithubProjectProperty("https://github.com/user/dropwizard")); - given(ghPullRequest.getNumber()).willReturn(1); - // Creating spy on ghprb, configuring repo - Ghprb ghprb = spyCreatingGhprb(trigger, project); - - doReturn(ghprbGitHub).when(ghprb).getGitHub(); - - setRepositoryHelper(ghprb); - given(ghRepository.getPullRequest(1)).willReturn(ghPullRequest); - // Configuring and adding Ghprb trigger - project.addTrigger(trigger); - - // Configuring Git SCM - project.setScm(GhprbTestUtil.provideGitSCM()); - - trigger.start(project, true); - - setTriggerHelper(trigger, ghprb); - GhprbTestUtil.triggerRunAndWait(10, trigger, project); - return project; } } \ No newline at end of file diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/downstreambuilds/BuildFlowBuildManagerTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/downstreambuilds/BuildFlowBuildManagerTest.java index 99810ca93..603e3e650 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/downstreambuilds/BuildFlowBuildManagerTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/manager/impl/downstreambuilds/BuildFlowBuildManagerTest.java @@ -3,19 +3,15 @@ import static org.fest.assertions.Assertions.assertThat; import static org.mockito.BDDMockito.given; -import static org.mockito.Mockito.doReturn; import com.cloudbees.plugins.flow.BuildFlow; import com.cloudbees.plugins.flow.FlowRun; import com.cloudbees.plugins.flow.JobInvocation; -import com.coravy.hudson.plugins.github.GithubProjectProperty; import java.util.Iterator; -import org.jenkinsci.plugins.ghprb.Ghprb; import org.jenkinsci.plugins.ghprb.GhprbITBaseTestCase; import org.jenkinsci.plugins.ghprb.GhprbTestUtil; -import org.jenkinsci.plugins.ghprb.GhprbTrigger; import org.jenkinsci.plugins.ghprb.manager.GhprbBuildManager; import org.jenkinsci.plugins.ghprb.manager.factory.GhprbBuildManagerFactoryUtil; import org.jenkinsci.plugins.ghprb.rules.JenkinsRuleWithBuildFlow; @@ -35,16 +31,39 @@ public class BuildFlowBuildManagerTest extends GhprbITBaseTestCase { @Rule public JenkinsRuleWithBuildFlow jenkinsRule = new JenkinsRuleWithBuildFlow(); + + private BuildFlow buildFlowProject; @Before public void setUp() throws Exception { - super.beforeTest(); + + buildFlowProject = jenkinsRule.createBuildFlowProject(); + + jenkinsRule.createFreeStyleProject("downstreamProject1"); + jenkinsRule.createFreeStyleProject("downstreamProject2"); + jenkinsRule.createFreeStyleProject("downstreamProject3"); + + StringBuilder dsl = new StringBuilder(); + + dsl.append("parallel ("); + dsl.append(" { build(\"downstreamProject1\") },"); + dsl.append(" { build(\"downstreamProject2\") }"); + dsl.append(")"); + dsl.append("{ build(\"downstreamProject3\") }"); + + buildFlowProject.setDsl(dsl.toString()); + + given(ghPullRequest.getNumber()).willReturn(1); + given(ghRepository.getPullRequest(1)).willReturn(ghPullRequest); + + super.beforeTest(null, null, buildFlowProject); } @Test public void shouldCalculateUrlWithDownstreamBuilds() throws Exception { // GIVEN - BuildFlow buildFlowProject = givenThatGhprbHasBeenTriggeredForABuildFlowProject(); + + GhprbTestUtil.triggerRunAndWait(10, trigger, buildFlowProject); // THEN assertThat(buildFlowProject.getBuilds().toArray().length).isEqualTo(1); @@ -84,55 +103,4 @@ public void shouldCalculateUrlWithDownstreamBuilds() throws Exception { assertThat(buildManager.calculateBuildUrl(null)).isEqualTo(expectedUrl.toString()); } - private BuildFlow givenThatGhprbHasBeenTriggeredForABuildFlowProject() throws Exception { - - BuildFlow buildFlowProject = jenkinsRule.createBuildFlowProject(); - - jenkinsRule.createFreeStyleProject("downstreamProject1"); - jenkinsRule.createFreeStyleProject("downstreamProject2"); - jenkinsRule.createFreeStyleProject("downstreamProject3"); - - StringBuilder dsl = new StringBuilder(); - - dsl.append("parallel ("); - dsl.append(" { build(\"downstreamProject1\") },"); - dsl.append(" { build(\"downstreamProject2\") }"); - dsl.append(")"); - dsl.append("{ build(\"downstreamProject3\") }"); - - buildFlowProject.setDsl(dsl.toString()); - - GhprbTrigger trigger = GhprbTestUtil.getTrigger(null); - - given(commitPointer.getSha()).willReturn("sha"); - GhprbTestUtil.setupGhprbTriggerDescriptor(null); - - buildFlowProject.addProperty(new GithubProjectProperty("https://github.com/user/dropwizard")); - - given(ghPullRequest.getNumber()).willReturn(1); - - // Creating spy on ghprb, configuring repo - Ghprb ghprb = spyCreatingGhprb(trigger, buildFlowProject); - - doReturn(ghprbGitHub).when(ghprb).getGitHub(); - - setRepositoryHelper(ghprb); - - given(ghRepository.getPullRequest(1)).willReturn(ghPullRequest); - - // Configuring and adding Ghprb trigger - buildFlowProject.addTrigger(trigger); - - // Configuring Git SCM - buildFlowProject.setScm(GhprbTestUtil.provideGitSCM()); - - trigger.start(buildFlowProject, true); - - setTriggerHelper(trigger, ghprb); - - GhprbTestUtil.triggerRunAndWait(10, trigger, buildFlowProject); - - return buildFlowProject; - } - }