From 0c61b38d2fdfc5675f5f623d9937d7e00bf479a3 Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Thu, 18 Jun 2020 19:14:38 -0700 Subject: [PATCH 1/4] Move to github-api 1.114.2 --- checkstyle.xml | 11 +- pom.xml | 121 ++++++++---------- .../plugins/ghprb/GhprbGitHubAuth.java | 4 +- .../plugins/ghprb/GhprbRepository.java | 2 +- .../ghprb/HttpConnectorWithJenkinsProxy.java | 55 ++++++-- .../plugins/ghprb/GhprbITBaseTestCase.java | 6 +- .../plugins/ghprb/GhprbRepositoryTest.java | 3 +- .../plugins/ghprb/GhprbTestUtil.java | 5 +- 8 files changed, 112 insertions(+), 95 deletions(-) diff --git a/checkstyle.xml b/checkstyle.xml index b4fde8d45..584eb04c8 100644 --- a/checkstyle.xml +++ b/checkstyle.xml @@ -75,11 +75,15 @@ - + + + - + + + @@ -116,9 +120,6 @@ - - - diff --git a/pom.xml b/pom.xml index d4de30e93..7aa8f5454 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.jenkins-ci.plugins plugin - 3.2 + 4.2 @@ -51,12 +51,15 @@ UTF-8 UTF-8 1.4 - 2.7 + 2.164.3 1.6.2 - 7 + 8 false - 6.7 - 2.17 + 8.33 + 3.1.1 + + + true - - - org.jenkins-ci.plugins - scm-api - 2.1.0 - - - org.jenkins-ci - symbol-annotation - 1.9 - - - org.jenkins-ci - annotation-indexer - 1.12 - - - org.jenkins-ci.plugins - script-security - 1.25 - - - org.codehaus.groovy - groovy-all - 2.4.11 - - - org.apache.ant - ant - 1.9.2 - - - org.codehaus.plexus - plexus-utils - 3.0.10 - - - org.codehaus.plexus - plexus-classworlds - 2.4.2 - - - org.jenkins-ci.plugins - bouncycastle-api - 2.16.1 - - - - com.coravy.hudson.plugins.github github @@ -136,7 +89,7 @@ org.jenkins-ci.plugins github-api - 1.92 + 1.114.2 org.jenkins-ci.plugins @@ -152,7 +105,7 @@ junit junit - 4.12 + 4.13 test @@ -208,17 +161,58 @@ + + + + + org.jenkins-ci.plugins + scm-api + 2.1.0 + + + org.jenkins-ci + symbol-annotation + 1.9 + + + org.jenkins-ci + annotation-indexer + 1.12 + + + org.jenkins-ci.plugins + script-security + 1.25 + + + org.apache.commons + commons-lang3 + 3.9 + + + org.codehaus.groovy + groovy-all + 2.4.12 + + + org.hamcrest + hamcrest-core + 2.2 + + + + org.apache.maven.plugins maven-surefire-plugin - 2.16 + 3.0.0-M4 org.apache.maven.surefire surefire-junit47 - 2.16 + 3.0.0-M4 @@ -256,15 +250,6 @@ - - org.apache.maven.plugins - maven-compiler-plugin - 3.5.1 - - 1.7 - 1.7 - - org.apache.maven.plugins maven-checkstyle-plugin diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth.java index e116ebe6e..8c073f9bb 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth.java @@ -172,7 +172,7 @@ public boolean checkSignature(String body, String signature) { private static GitHubBuilder getBuilder(Item context, String serverAPIUrl, String credentialsId) { GitHubBuilder builder = new GitHubBuilder() .withEndpoint(serverAPIUrl) - .withConnector(new HttpConnectorWithJenkinsProxy()); + .withConnector(new HttpConnectorWithJenkinsProxy(serverAPIUrl)); String contextName = context == null ? "(Jenkins.instance)" : context.getFullDisplayName(); if (StringUtils.isEmpty(credentialsId)) { @@ -285,7 +285,7 @@ public FormValidation doCreateApiToken( GitHubBuilder builder = new GitHubBuilder() .withEndpoint(serverAPIUrl) - .withConnector(new HttpConnectorWithJenkinsProxy()); + .withConnector(new HttpConnectorWithJenkinsProxy(serverAPIUrl)); if (StringUtils.isEmpty(credentialsId)) { if (StringUtils.isEmpty(username) || StringUtils.isEmpty(password)) { diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRepository.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRepository.java index 1e1dba5dd..c04f1e1fc 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRepository.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbRepository.java @@ -101,7 +101,7 @@ private boolean initGhRepository() { } try { - if (gitHub.getRateLimit().remaining == 0) { + if (gitHub.getRateLimit().getRemaining() == 0) { LOGGER.log(Level.INFO, "Exceeded rate limit for repository"); return false; } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/HttpConnectorWithJenkinsProxy.java b/src/main/java/org/jenkinsci/plugins/ghprb/HttpConnectorWithJenkinsProxy.java index baf1cdf55..8881391a8 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/HttpConnectorWithJenkinsProxy.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/HttpConnectorWithJenkinsProxy.java @@ -1,28 +1,55 @@ package org.jenkinsci.plugins.ghprb; -import hudson.ProxyConfiguration; -import org.kohsuke.github.HttpConnector; +import jenkins.model.Jenkins; +import jenkins.util.JenkinsJVM; +import okhttp3.OkHttpClient; +import org.kohsuke.github.extras.okhttp3.OkHttpConnector; -import java.io.IOException; -import java.net.HttpURLConnection; -import java.net.URL; +import javax.annotation.Nonnull; +import java.net.Proxy; +import java.util.concurrent.TimeUnit; + +public class HttpConnectorWithJenkinsProxy extends OkHttpConnector { -public class HttpConnectorWithJenkinsProxy implements HttpConnector { private static final int DEFAULT_CONNECT_TIMEOUT = 10000; private static final int DEFAULT_READ_TIMEOUT = 10000; - public HttpURLConnection connect(URL url) throws IOException { - HttpURLConnection con = (HttpURLConnection) ProxyConfiguration.open(url); + // Create a default base client with our default connect timeouts + private static final OkHttpClient BASECLIENT = new OkHttpClient().newBuilder() + .connectTimeout(DEFAULT_CONNECT_TIMEOUT, TimeUnit.MILLISECONDS) + .readTimeout(DEFAULT_READ_TIMEOUT, TimeUnit.MILLISECONDS) + .build(); + + public HttpConnectorWithJenkinsProxy(String host) { + super(getClient(host)); + } + + private static OkHttpClient getClient(String host) { + OkHttpClient.Builder builder = BASECLIENT.newBuilder(); - // Set default timeouts in case there are none - if (con.getConnectTimeout() == 0) { - con.setConnectTimeout(DEFAULT_CONNECT_TIMEOUT); + if (JenkinsJVM.isJenkinsJVM()) { + builder.proxy(getProxy(host)); } - if (con.getReadTimeout() == 0) { - con.setReadTimeout(DEFAULT_READ_TIMEOUT); + + return builder.build(); + } + + /** + * Uses proxy if configured on pluginManager/advanced page + * + * @param host GitHub's hostname to build proxy to + * + * @return proxy to use it in connector. Should not be null as it can lead to unexpected behaviour + */ + @Nonnull + private static Proxy getProxy(@Nonnull String host) { + Jenkins jenkins = Jenkins.getInstanceOrNull(); + if (jenkins == null || jenkins.proxy == null) { + return Proxy.NO_PROXY; + } else { + return jenkins.proxy.createProxy(host); } - return con; } } diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbITBaseTestCase.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbITBaseTestCase.java index 3e7ba8321..d4ecacd18 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbITBaseTestCase.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbITBaseTestCase.java @@ -54,7 +54,8 @@ public abstract class GhprbITBaseTestCase { protected GhprbTrigger trigger; - protected GHRateLimit ghRateLimit = new GHRateLimit(); + @Mock + protected GHRateLimit ghRateLimit; protected void beforeTest( Map globalConfig, @@ -88,7 +89,8 @@ protected void beforeTest( given(ghUser.getEmail()).willReturn("email@email.com"); given(ghUser.getLogin()).willReturn("user"); - ghRateLimit.remaining = GhprbTestUtil.INITIAL_RATE_LIMIT; + + given(ghRateLimit.getRemaining()).willReturn(GhprbTestUtil.INITIAL_RATE_LIMIT); GhprbTestUtil.mockCommitList(ghPullRequest); diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java index 32653c7c3..25cab45dd 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java @@ -759,7 +759,8 @@ public void testCheckMethodWithNoPR() throws Exception { public void testExceedRateLimit() throws Exception { // GIVEN getNewTrigger(); - rateLimit.remaining = 0; + + given(rateLimit.getRemaining()).willReturn(0); verify(gt, times(1)).getRateLimit(); // WHEN diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java index 8f9960b4e..337653be6 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java @@ -422,8 +422,9 @@ public static GhprbTrigger getTrigger(Map values) throws Excepti GhprbTrigger trigger = spy(req.bindJSON(GhprbTrigger.class, defaults)); - GHRateLimit limit = new GHRateLimit(); - limit.remaining = INITIAL_RATE_LIMIT; + + GHRateLimit limit = Mockito.mock(GHRateLimit.class); + given(limit.getRemaining()).willReturn(INITIAL_RATE_LIMIT); GitHub github = Mockito.mock(GitHub.class); given(github.getRateLimit()).willReturn(limit); From 9f41dfc7fd0cfdf812aecdaa7ddf56c17507674b Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Mon, 13 Jul 2020 11:46:23 -0700 Subject: [PATCH 2/4] Clean up --- pom.xml | 2 +- .../org/jenkinsci/plugins/ghprb/Ghprb.java | 1 - .../jenkinsci/plugins/ghprb/GhprbBuilds.java | 4 ++- .../plugins/ghprb/GhprbGitHubAuth.java | 2 +- .../plugins/ghprb/GhprbPullRequest.java | 34 +++++++++++-------- .../plugins/ghprb/GhprbPullRequestMerge.java | 3 +- .../jenkinsci/plugins/ghprb/GhprbTrigger.java | 28 +++++++++------ .../build/GhprbCancelBuildsOnUpdate.java | 6 +++- .../extensions/comments/GhprbCommentFile.java | 7 ++-- .../extensions/status/GhprbSimpleStatus.java | 2 +- .../ghprb/jobdsl/GhprbTriggerContext.java | 12 +++---- .../manager/impl/GhprbBaseBuildManager.java | 3 +- 12 files changed, 62 insertions(+), 42 deletions(-) diff --git a/pom.xml b/pom.xml index 7aa8f5454..9ca3fdcfd 100644 --- a/pom.xml +++ b/pom.xml @@ -52,7 +52,7 @@ UTF-8 1.4 2.164.3 - 1.6.2 + 1.7.4 8 false 8.33 diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java b/src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java index def9cfecd..0f5295885 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java @@ -132,7 +132,6 @@ public String checkBlackListCommitAuthor(String author) { Set authors = getBlacklistedCommitAuthors(); authors.remove(""); - Map skipPatterns = new HashMap(); for (String s : authors) { s = s.trim(); if (compilePattern(s).matcher(author).matches()) { diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbBuilds.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbBuilds.java index ef12d3519..f5e6c903c 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbBuilds.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbBuilds.java @@ -7,6 +7,7 @@ import hudson.model.Run; import hudson.model.TaskListener; import hudson.model.queue.QueueTaskFuture; +import hudson.plugins.git.Revision; import hudson.plugins.git.util.BuildData; import org.apache.commons.lang.StringUtils; import org.jenkinsci.plugins.ghprb.extensions.GhprbBuildStep; @@ -186,7 +187,8 @@ public void onCompleted(Run build, TaskListener listener) { // having two of them, and because the one we added isn't correct // @see GhprbTrigger for (BuildData data : build.getActions(BuildData.class)) { - if (data.getLastBuiltRevision() != null && !data.getLastBuiltRevision().getSha1String().equals(c.getCommit())) { + final Revision revision = data.getLastBuiltRevision(); + if (revision != null && !revision.getSha1String().equals(c.getCommit())) { build.getActions().remove(data); break; } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth.java index 8c073f9bb..7212d3862 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth.java @@ -152,7 +152,7 @@ public boolean checkSignature(String body, String signature) { new Object[] {localSignature, expected}); return false; } - } catch (Exception e) { + } catch (Throwable e) { LOGGER.log(Level.SEVERE, "Couldn't match both signatures"); return false; } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java index f682cebc5..510b87310 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequest.java @@ -235,7 +235,9 @@ private void checkBlackListLabels() { LOGGER.log(Level.INFO, "Found label {0} in ignore list, pull request will be ignored.", label.getName()); - shouldRun = false; + synchronized (this) { + shouldRun = false; + } } } } catch (Error e) { @@ -262,7 +264,9 @@ private void checkWhiteListLabels() { if (!containsWhiteListLabel) { LOGGER.log(Level.INFO, "Can't find any of whitelist label."); - shouldRun = false; + synchronized (this) { + shouldRun = false; + } } } catch (Error e) { LOGGER.log(Level.SEVERE, "Failed to read whitelist labels", e); @@ -330,20 +334,20 @@ public void check(GHIssueComment comment) { private void updatePR(GHPullRequest ghpr, GHIssueComment comment, boolean isWebhook) { // Get the updated time try { - Date lastUpdateTime = updated; - Date updatedDate = comment != null ? comment.getUpdatedAt() : ghpr.getUpdatedAt(); - // Don't log unless it was actually updated - if (updated == null || updated.compareTo(updatedDate) < 0) { - String user = comment != null ? comment.getUser().getName() : ghpr.getUser().getName(); - LOGGER.log( - Level.INFO, - "Pull request #{0} was updated/initialized on {1} at {2} by {3} ({4})", - new Object[] {this.id, this.repo.getName(), updatedDate, user, - comment != null ? "comment" : "PR update"} - ); - } - synchronized (this) { + Date lastUpdateTime = updated; + Date updatedDate = comment != null ? comment.getUpdatedAt() : ghpr.getUpdatedAt(); + // Don't log unless it was actually updated + if (updated == null || updated.compareTo(updatedDate) < 0) { + String user = comment != null ? comment.getUser().getName() : ghpr.getUser().getName(); + LOGGER.log( + Level.INFO, + "Pull request #{0} was updated/initialized on {1} at {2} by {3} ({4})", + new Object[] {this.id, this.repo.getName(), updatedDate, user, + comment != null ? "comment" : "PR update"} + ); + } + boolean wasUpdated = setUpdated(updatedDate); // Update the PR object with the new pull request object if diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge.java index 89d9320d7..fd415cf94 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbPullRequestMerge.java @@ -110,7 +110,8 @@ public void perform( ) throws InterruptedException, IOException { listener = taskListener; Job project = run.getParent(); - if (run.getResult().isWorseThan(Result.SUCCESS)) { + Result result = run.getResult(); + if (result != null && result.isWorseThan(Result.SUCCESS)) { listener.getLogger().println("Build did not succeed, merge will not be run"); return; } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java index 5ba3e756f..fa289782f 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java @@ -51,6 +51,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.WeakHashMap; import java.util.concurrent.ConcurrentHashMap; @@ -248,7 +249,9 @@ public static DescriptorImpl getDscp() { @SuppressWarnings("deprecation") private void initState() throws IOException { - final GithubProjectProperty ghpp = super.job.getProperty(GithubProjectProperty.class); + final Job job = super.job; + Objects.requireNonNull(job); + final GithubProjectProperty ghpp = job.getProperty(GithubProjectProperty.class); if (ghpp == null || ghpp.getProjectUrl() == null) { throw new IllegalStateException("A GitHub project url is required."); } @@ -266,7 +269,7 @@ private void initState() throws IOException { this.pullRequests = null; try { - Map prs = getDescriptor().getPullRequests(super.job.getFullName()); + Map prs = getDescriptor().getPullRequests(job.getFullName()); if (prs != null) { prs = new ConcurrentHashMap(prs); if (pulls == null) { @@ -366,7 +369,8 @@ public void run() { return; } - LOGGER.log(Level.FINE, "Running trigger for {0}", super.job.getFullName()); + Job job = this.job; + LOGGER.log(Level.FINE, "Running trigger for {0}", job == null ? "unknown Job" : job.getFullName()); this.repository.check(); } @@ -480,10 +484,13 @@ private String escapeText(String text) { private ArrayList getDefaultParameters() { ArrayList values = new ArrayList(); - ParametersDefinitionProperty pdp = this.job.getProperty(ParametersDefinitionProperty.class); - if (pdp != null) { - for (ParameterDefinition pd : pdp.getParameterDefinitions()) { - values.add(pd.getDefaultParameterValue()); + Job job = this.job; + if (job != null) { + ParametersDefinitionProperty pdp = job.getProperty(ParametersDefinitionProperty.class); + if (pdp != null) { + for (ParameterDefinition pd : pdp.getParameterDefinitions()) { + values.add(pd.getDefaultParameterValue()); + } } } return values; @@ -518,10 +525,11 @@ public GitHub getGitHub() throws IOException { } public void addWhitelist(String author) { - whitelist = whitelist + " " + author; try { - this.job.save(); - } catch (IOException ex) { + Job job = Objects.requireNonNull(this.job); + whitelist = whitelist + " " + author; + job.save(); + } catch (Exception ex) { LOGGER.log(Level.SEVERE, "Failed to save new whitelist", ex); } } 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 index 228181bcc..1707747ff 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/extensions/build/GhprbCancelBuildsOnUpdate.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/build/GhprbCancelBuildsOnUpdate.java @@ -2,6 +2,7 @@ import hudson.Extension; import hudson.model.Cause; +import hudson.model.Executor; import hudson.model.Job; import hudson.model.Queue; import hudson.model.Result; @@ -98,7 +99,10 @@ protected void cancelCurrentBuilds(Job project, + project.getName() + " for PR # " + cause.getPullID() ); run.addAction(this); - run.getExecutor().interrupt(Result.ABORTED); + Executor executor = run.getExecutor(); + if (executor != null) { + executor.interrupt(Result.ABORTED); + } } catch (Exception e) { LOGGER.log(Level.SEVERE, "Error while trying to interrupt build!", e); } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbCommentFile.java b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbCommentFile.java index f167a709f..1368b377b 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbCommentFile.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/extensions/comments/GhprbCommentFile.java @@ -15,6 +15,7 @@ import java.io.File; import java.io.IOException; +import java.util.Objects; public class GhprbCommentFile extends GhprbExtension implements GhprbCommentAppender, GhprbProjectExtension { @Extension @@ -35,15 +36,15 @@ public boolean ignorePublishedUrl() { return false; } - public String postBuildComment(Run build, TaskListener listener) { + public String postBuildComment(final Run build, TaskListener listener) { StringBuilder msg = new StringBuilder(); if (commentFilePath != null && !commentFilePath.isEmpty()) { - String scriptFilePathResolved = Ghprb.replaceMacros(build, listener, commentFilePath); - + final String scriptFilePathResolved = Objects.requireNonNull(Ghprb.replaceMacros(build, listener, commentFilePath)); try { String content = null; if (build instanceof Build) { final FilePath workspace = ((Build) build).getWorkspace(); + assert workspace != null; final FilePath path = workspace.child(scriptFilePathResolved); if (path.exists()) { 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 7d86d2075..36cd1b630 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 @@ -124,7 +124,7 @@ public void onBuildTriggered(Job project, } String statusUrl = getDescriptor().getStatusUrlDefault(this); - if (commitStatusContext == "") { + if ("".equals(commitStatusContext)) { commitStatusContext = getDescriptor().getCommitStatusContextDefault(this); } diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/jobdsl/GhprbTriggerContext.java b/src/main/java/org/jenkinsci/plugins/ghprb/jobdsl/GhprbTriggerContext.java index 795a08253..cd713acb6 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/jobdsl/GhprbTriggerContext.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/jobdsl/GhprbTriggerContext.java @@ -300,19 +300,19 @@ public void excludedRegions(String regions) { } public void includedRegions(Iterable regions) { - String includedRegionsStr = ""; + StringBuilder builder = new StringBuilder(); for (String region : regions) { - includedRegionsStr += (region + "\n"); + builder.append(region).append("\n"); } - includedRegions(includedRegionsStr); + includedRegions(builder.toString()); } public void excludedRegions(Iterable regions) { - String excludedRegionsStr = ""; + StringBuilder builder = new StringBuilder(); for (String region : regions) { - excludedRegionsStr += (region + "\n"); + builder.append(region).append("\n"); } - excludedRegions(excludedRegionsStr); + excludedRegions(builder.toString()); } } 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 de5dd15cd..19d9e2ee6 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 @@ -88,7 +88,8 @@ protected String getAggregatedTestResults(Run build) { if (build.getResult() != Result.UNSTABLE) { sb.append("

Build result: "); - sb.append(build.getResult().toString()); + Result result = build.getResult(); + sb.append(result == null ? "None" : result.toString()); sb.append("

"); try { From e452a90b639c56c2d80b27c3a81c6ad155ead0c2 Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Mon, 13 Jul 2020 16:16:11 -0700 Subject: [PATCH 3/4] Use BOM --- pom.xml | 47 ++++++++--------------------------------------- 1 file changed, 8 insertions(+), 39 deletions(-) diff --git a/pom.xml b/pom.xml index 9ca3fdcfd..2da3c477d 100644 --- a/pom.xml +++ b/pom.xml @@ -50,7 +50,6 @@ UTF-8 UTF-8 - 1.4 2.164.3 1.7.4 8 @@ -79,12 +78,11 @@ com.coravy.hudson.plugins.github github - 1.27.0 + 1.31.0 org.jenkins-ci.plugins matrix-project - 1.11 org.jenkins-ci.plugins @@ -94,12 +92,11 @@ org.jenkins-ci.plugins git - 3.3.1 org.jenkins-ci.plugins job-dsl - 1.63 + 1.77 true @@ -135,7 +132,6 @@ org.jenkins-ci.plugins token-macro - 2.1 true @@ -147,53 +143,32 @@ org.jenkins-ci.plugins credentials - 2.1.14 org.jenkins-ci.plugins plain-credentials - 1.4 org.jenkins-ci.plugins structs - 1.9 - - - org.jenkins-ci.plugins - scm-api - 2.1.0 - - - org.jenkins-ci - symbol-annotation - 1.9 - - - org.jenkins-ci - annotation-indexer - 1.12 - - org.jenkins-ci.plugins - script-security - 1.25 + io.jenkins.tools.bom + bom-2.164.x + 9 + import + pom + org.apache.commons commons-lang3 3.9 - - org.codehaus.groovy - groovy-all - 2.4.12 - org.hamcrest hamcrest-core @@ -287,13 +262,7 @@ repo.jenkins-ci.org https://repo.jenkins-ci.org/public/ - - jgit-repository - Eclipse JGit Repository - http://download.eclipse.org/jgit/maven - - repo.jenkins-ci.org From 36967eb08e483868cb1a484338520e2eec5d9041 Mon Sep 17 00:00:00 2001 From: Antoine DUPRAT Date: Tue, 14 Apr 2020 10:03:15 +0200 Subject: [PATCH 4/4] Switch to Java 8 --- .../org/jenkinsci/plugins/ghprb/GhprbIT.java | 19 +++++++++++---- .../plugins/ghprb/GhprbITBaseTestCase.java | 10 ++++++-- .../plugins/ghprb/GhprbRepositoryTest.java | 23 +++++++++++++++---- .../plugins/ghprb/GhprbRootActionTest.java | 10 ++++++-- .../plugins/ghprb/GhprbTestUtil.java | 17 +++++++------- 5 files changed, 58 insertions(+), 21 deletions(-) diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbIT.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbIT.java index 61a29846e..0201c1424 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbIT.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbIT.java @@ -6,7 +6,6 @@ import hudson.model.ParameterValue; import hudson.model.Run; import net.sf.json.JSONObject; -import org.joda.time.DateTime; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -20,7 +19,11 @@ import org.mockito.Mockito; import org.mockito.runners.MockitoJUnitRunner; +import java.time.ZoneOffset; +import java.time.ZonedDateTime; +import java.time.temporal.ChronoUnit; import java.util.ArrayList; +import java.util.Date; import java.util.List; import java.util.Map; @@ -89,12 +92,17 @@ public void shouldBuildTriggersOnUpdatingNewCommitsPR() throws Exception { @Test public void shouldBuildTriggersOnUpdatingRetestMessagePR() throws Exception { // GIVEN - given(ghPullRequest.getCreatedAt()).willReturn(new DateTime().toDate()); + given(ghPullRequest.getCreatedAt()).willReturn(Date.from( + ZonedDateTime.now(ZoneOffset.UTC) + .toInstant())); 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.getUpdatedAt()).willReturn(Date.from( + ZonedDateTime.now(ZoneOffset.UTC) + .plus(1, ChronoUnit.DAYS) + .toInstant())); given(comment.getUser()).willReturn(ghUser); given(ghPullRequest.getComments()).willReturn(newArrayList(comment)); @@ -111,7 +119,10 @@ public void shouldNotBuildDisabledBuild() throws Exception { given(commitPointer.getSha()).willReturn("sha"); given(comment.getBody()).willReturn("retest this please"); - given(comment.getUpdatedAt()).willReturn(new DateTime().plusDays(1).toDate()); + given(comment.getUpdatedAt()).willReturn(Date.from( + ZonedDateTime.now(ZoneOffset.UTC) + .plus(1, ChronoUnit.DAYS) + .toInstant())); given(comment.getUser()).willReturn(ghUser); given(ghPullRequest.getComments()).willReturn(newArrayList(comment)); given(ghPullRequest.getNumber()).willReturn(5); diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbITBaseTestCase.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbITBaseTestCase.java index d4ecacd18..4bdcc1264 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbITBaseTestCase.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbITBaseTestCase.java @@ -5,7 +5,6 @@ import hudson.model.Run; import hudson.model.TaskListener; import hudson.plugins.git.GitSCM; -import org.joda.time.DateTime; import org.kohsuke.github.GHCommitPointer; import org.kohsuke.github.GHIssueState; import org.kohsuke.github.GHPullRequest; @@ -16,6 +15,9 @@ import org.mockito.Mock; import org.mockito.Mockito; +import java.time.ZoneOffset; +import java.time.ZonedDateTime; +import java.time.temporal.ChronoUnit; import java.util.Map; import static com.google.common.collect.Lists.newArrayList; @@ -79,7 +81,11 @@ protected void beforeTest( given(ghRepository.getName()).willReturn("dropwizard"); - GhprbTestUtil.mockPR(ghPullRequest, commitPointer, new DateTime(), new DateTime().plusDays(1)); + GhprbTestUtil.mockPR(ghPullRequest, + commitPointer, + ZonedDateTime.now(ZoneOffset.UTC), + ZonedDateTime.now(ZoneOffset.UTC) + .plus(1, ChronoUnit.DAYS)); given(ghRepository.getPullRequests(eq(GHIssueState.OPEN))) .willReturn(newArrayList(ghPullRequest)) diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java index 25cab45dd..09904d214 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRepositoryTest.java @@ -9,7 +9,6 @@ import org.apache.commons.codec.binary.Hex; import org.fest.util.Collections; import org.jenkinsci.plugins.ghprb.extensions.status.GhprbSimpleStatus; -import org.joda.time.DateTime; import org.junit.Assert; import org.junit.Before; import org.junit.Rule; @@ -38,6 +37,9 @@ import java.io.IOException; import java.net.URL; import java.net.URLEncoder; +import java.time.ZoneOffset; +import java.time.ZonedDateTime; +import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.Arrays; import java.util.Date; @@ -519,8 +521,14 @@ public void testCheckMethodWhenPrWasUpdatedWithNonKeyPhrase() throws Exception { mockCommitList(); GhprbBuilds builds = mockBuilds(); - Date later = new DateTime().plusHours(3).toDate(); - Date tomorrow = new DateTime().plusDays(1).toDate(); + Date later = Date.from( + ZonedDateTime.now(ZoneOffset.UTC) + .plus(3, ChronoUnit.HOURS) + .toInstant()); + Date tomorrow = Date.from( + ZonedDateTime.now(ZoneOffset.UTC) + .plus(1, ChronoUnit.DAYS) + .toInstant()); given(ghRepository.getPullRequests(eq(GHIssueState.OPEN))).willReturn(ghPullRequests); @@ -616,8 +624,13 @@ private List createListWithMockPR() throws IOException { public void testCheckMethodWhenPrWasUpdatedWithRetestPhrase() throws Exception { // GIVEN List ghPullRequests = createListWithMockPR(); - Date now = new Date(); - Date tomorrow = new DateTime().plusDays(1).toDate(); + Date now = Date.from( + ZonedDateTime.now(ZoneOffset.UTC) + .toInstant()); + Date tomorrow = Date.from( + ZonedDateTime.now(ZoneOffset.UTC) + .plus(1, ChronoUnit.DAYS) + .toInstant()); mockHeadAndBase(); mockCommitList(); diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRootActionTest.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRootActionTest.java index d83c8a20b..c0d2a2a23 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRootActionTest.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbRootActionTest.java @@ -3,7 +3,6 @@ import com.coravy.hudson.plugins.github.GithubProjectProperty; import hudson.model.FreeStyleProject; import hudson.plugins.git.GitSCM; -import org.joda.time.DateTime; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -28,6 +27,9 @@ import java.io.Reader; import java.io.StringReader; import java.net.URLEncoder; +import java.time.ZoneOffset; +import java.time.ZonedDateTime; +import java.time.temporal.ChronoUnit; import static com.google.common.collect.Lists.newArrayList; import static org.fest.assertions.Assertions.assertThat; @@ -79,7 +81,11 @@ public void setup() throws Exception { given(commitPointer.getRef()).willReturn("ref"); given(ghRepository.getName()).willReturn("dropwizard"); - GhprbTestUtil.mockPR(ghPullRequest, commitPointer, new DateTime(), new DateTime().plusDays(1)); + GhprbTestUtil.mockPR(ghPullRequest, + commitPointer, + ZonedDateTime.now(ZoneOffset.UTC), + ZonedDateTime.now(ZoneOffset.UTC) + .plus(1, ChronoUnit.DAYS)); given(ghRepository.getPullRequests(eq(OPEN))).willReturn(newArrayList(ghPullRequest)).willReturn(newArrayList(ghPullRequest)); diff --git a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java index 337653be6..1cba79ec3 100644 --- a/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java +++ b/src/test/java/org/jenkinsci/plugins/ghprb/GhprbTestUtil.java @@ -20,7 +20,6 @@ import hudson.plugins.git.UserRemoteConfig; import net.sf.json.JSONObject; import org.apache.commons.lang.StringUtils; -import org.joda.time.DateTime; import org.kohsuke.github.GHCommitPointer; import org.kohsuke.github.GHPullRequest; import org.kohsuke.github.GHRateLimit; @@ -39,7 +38,9 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.net.URL; +import java.time.ZonedDateTime; import java.util.ArrayList; +import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -253,7 +254,7 @@ public static void mockCommitList(GHPullRequest ghPullRequest) { Mockito.when(itr.hasNext()).thenReturn(false); } - public static void mockPR(GHPullRequest prToMock, GHCommitPointer commitPointer, DateTime... updatedDate) throws Exception { + public static void mockPR(GHPullRequest prToMock, GHCommitPointer commitPointer, ZonedDateTime... updatedDate) throws Exception { given(prToMock.getHead()).willReturn(commitPointer); given(prToMock.getBase()).willReturn(commitPointer); @@ -262,13 +263,13 @@ public static void mockPR(GHPullRequest prToMock, GHCommitPointer commitPointer, if (updatedDate.length > 1) { given(prToMock.getUpdatedAt()) - .willReturn(updatedDate[0].toDate()) - .willReturn(updatedDate[0].toDate()) - .willReturn(updatedDate[1].toDate()) - .willReturn(updatedDate[1].toDate()) - .willReturn(updatedDate[1].toDate()); + .willReturn(Date.from(updatedDate[0].toInstant())) + .willReturn(Date.from(updatedDate[0].toInstant())) + .willReturn(Date.from(updatedDate[1].toInstant())) + .willReturn(Date.from(updatedDate[1].toInstant())) + .willReturn(Date.from(updatedDate[1].toInstant())); } else { - given(prToMock.getUpdatedAt()).willReturn(updatedDate[0].toDate()); + given(prToMock.getUpdatedAt()).willReturn(Date.from(updatedDate[0].toInstant())); } }