From 0a30fd02952e92c6b915e460c9fc5e385267eebb Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Mon, 13 Jul 2020 11:46:23 -0700 Subject: [PATCH] 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 187d7189a..c1b9a8071 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 {