From b8a1eaab4863c0eccbffb2186404abab83f0a824 Mon Sep 17 00:00:00 2001 From: "j.berlin" Date: Fri, 23 Jun 2023 11:08:03 +0200 Subject: [PATCH 1/5] [JENKINS-71332] Handle https://issues.jenkins.io/browse/JENKINS-71332 by also take care of gitbranch as environment key Do some refactorings to handle SonarLint issues & problems mentioned by IDEA IntelliJ --- .../hudson/plugins/notification/Phase.java | 297 ++++++++++-------- 1 file changed, 162 insertions(+), 135 deletions(-) diff --git a/src/main/java/com/tikal/hudson/plugins/notification/Phase.java b/src/main/java/com/tikal/hudson/plugins/notification/Phase.java index d7411e8..9d82ed2 100755 --- a/src/main/java/com/tikal/hudson/plugins/notification/Phase.java +++ b/src/main/java/com/tikal/hudson/plugins/notification/Phase.java @@ -52,10 +52,10 @@ public enum Phase { QUEUED, STARTED, COMPLETED, FINALIZED, NONE; - private Result findLastBuildThatFinished(Run run){ + private Result findLastBuildThatFinished(final Run run){ Run previousRun = run.getPreviousCompletedBuild(); while(previousRun != null){ - Result previousResults = previousRun.getResult(); + final Result previousResults = previousRun.getResult(); if (previousResults == null) { throw new IllegalStateException("Previous result can't be null here"); } @@ -68,7 +68,7 @@ private Result findLastBuildThatFinished(Run run){ } @SuppressWarnings( "CastToConcreteClass" ) - public void handle(Run run, TaskListener listener, long timestamp) { + public void handle(final Run run, final TaskListener listener, final long timestamp) { handle(run, listener, timestamp, false, null, 0, this); } @@ -80,7 +80,7 @@ public void handle(Run run, TaskListener listener, long timestamp) { * @param logger PrintStream used for logging. * @return True if URL is populated with a non-blank value, or a variable that expands into a URL. */ - private boolean isURLValid(String urlInputValue, String expandedUrl, PrintStream logger){ + private boolean isURLValid(final String urlInputValue, final String expandedUrl, final PrintStream logger){ boolean isValid= false; //If Jenkins variable was used for URL, and it was unresolvable, log warning and return. if (expandedUrl.contains("$")) { @@ -98,44 +98,43 @@ private boolean isURLValid(String urlInputValue, String expandedUrl, PrintStream /** * Determines if the endpoint specified should be notified at the current job phase. */ - private boolean isRun( Endpoint endpoint, Result result, Result previousRunResult ) { - String event = endpoint.getEvent(); + private boolean isRun(final Endpoint endpoint, final Result result, final Result previousRunResult ) { + final String event = endpoint.getEvent(); - if(event == null) + if(event == null || "all".equals(event)) return true; + if(result == null || "manual".equals(event)){ + return false; + } + switch(event){ - case "all": - return true; - case "failed": - if (result == null) {return false;} - return this.equals(FINALIZED) && result.equals(Result.FAILURE); - case "failedAndFirstSuccess": - if (result == null || !this.equals(FINALIZED)) {return false;} - if (result.equals(Result.FAILURE)) {return true;} - return previousRunResult != null && result.equals(Result.SUCCESS) - && previousRunResult.equals(Result.FAILURE); - case "manual": - return false; - default: - return event.equals(this.toString().toLowerCase()); + case "failed": + return this.equals(FINALIZED) && result.equals(Result.FAILURE); + case "failedAndFirstSuccess": + if (!this.equals(FINALIZED)) {return false;} + if (result.equals(Result.FAILURE)) {return true;} + return previousRunResult != null && result.equals(Result.SUCCESS) + && previousRunResult.equals(Result.FAILURE); + default: + return event.equals(this.toString().toLowerCase()); } } - private JobState buildJobState(Job job, Run run, TaskListener listener, long timestamp, Endpoint target, Phase phase) + private JobState buildJobState(final Job job, final Run run, final TaskListener listener, final long timestamp, final Endpoint target, final Phase phase) throws IOException, InterruptedException { - Jenkins jenkins = Jenkins.getInstanceOrNull(); + final Jenkins jenkins = Jenkins.getInstanceOrNull(); assert jenkins != null; - String rootUrl = jenkins.getRootUrl(); - JobState jobState = new JobState(); - BuildState buildState = new BuildState(); - ScmState scmState = new ScmState(); - Result result = run.getResult(); - ParametersAction paramsAction = run.getAction(ParametersAction.class); - EnvVars environment = run.getEnvironment( listener ); - StringBuilder log = this.getLog(run, target); + final String rootUrl = jenkins.getRootUrl(); + final JobState jobState = new JobState(); + final BuildState buildState = new BuildState(); + final ScmState scmState = new ScmState(); + final Result result = run.getResult(); + final ParametersAction paramsAction = run.getAction(ParametersAction.class); + final EnvVars environment = run.getEnvironment( listener ); + final StringBuilder log = this.getLog(run, target); jobState.setName( job.getName()); jobState.setDisplayName(job.getDisplayName()); @@ -165,36 +164,41 @@ private JobState buildJobState(Job job, Run run, TaskListener listener, long tim //TODO: Make this optional to reduce chat overload. if ( paramsAction != null ) { - EnvVars env = new EnvVars(); - for (ParameterValue value : paramsAction.getParameters()){ + final EnvVars env = new EnvVars(); + for (final ParameterValue value : paramsAction.getParameters()){ if ( ! value.isSensitive()) { value.buildEnvironment( run, env ); } } buildState.setParameters(env); } - - BuildData build = job.getAction(BuildData.class); + + setupScmState(job, run, scmState, environment); + + return jobState; + } + + private void setupScmState(final Job job, final Run run, final ScmState scmState, final EnvVars environment) { + final BuildData build = job.getAction(BuildData.class); if ( build != null ) { if ( !build.remoteUrls.isEmpty() ) { - String url = build.remoteUrls.iterator().next(); + final String url = build.remoteUrls.iterator().next(); if ( url != null ) { scmState.setUrl( url ); } } - for (Map.Entry entry : build.buildsByBranchName.entrySet()) { + for (final Map.Entry entry : build.buildsByBranchName.entrySet()) { if ( entry.getValue().hudsonBuildNumber == run.number ) { scmState.setBranch( entry.getKey() ); scmState.setCommit( entry.getValue().revision.getSha1String() ); } } } - + if ( environment.get( "GIT_URL" ) != null ) { scmState.setUrl( environment.get( "GIT_URL" )); } - if ( environment.get( "GIT_BRANCH" ) != null ) { scmState.setBranch( environment.get( "GIT_BRANCH" )); @@ -206,22 +210,20 @@ private JobState buildJobState(Job job, Run run, TaskListener listener, long tim scmState.setChanges(getChangedFiles(run)); scmState.setCulprits(getCulprits(run)); - - return jobState; } - private String resolveMacros(Run build, TaskListener listener, String text) { + private String resolveMacros(final Run build, final TaskListener listener, final String text) { String result = text; try { - Executor executor = build.getExecutor(); + final Executor executor = build.getExecutor(); if(executor != null) { - FilePath workspace = executor.getCurrentWorkspace(); + final FilePath workspace = executor.getCurrentWorkspace(); if(workspace != null) { result = TokenMacro.expandAll(build, workspace, listener, text); } } - } catch (Throwable e) { + } catch (final Throwable e) { // Catching Throwable here because the TokenMacro plugin is optional // so will throw a ClassDefNotFoundError if the plugin is not installed or disabled. e.printStackTrace(listener.error(String.format("Failed to evaluate macro '%s'", text))); @@ -230,14 +232,14 @@ private String resolveMacros(Run build, TaskListener listener, String text) { return result; } - private TestState getTestResults(Run build) { + private TestState getTestResults(final Run build) { TestState resultSummary = null; - AbstractTestResultAction testAction = build.getAction(AbstractTestResultAction.class); + final AbstractTestResultAction testAction = build.getAction(AbstractTestResultAction.class); if(testAction != null) { - int total = testAction.getTotalCount(); - int failCount = testAction.getFailCount(); - int skipCount = testAction.getSkipCount(); + final int total = testAction.getTotalCount(); + final int failCount = testAction.getFailCount(); + final int skipCount = testAction.getSkipCount(); resultSummary = new TestState(); resultSummary.setTotal(total); @@ -251,28 +253,27 @@ private TestState getTestResults(Run build) { return resultSummary; } - private List getFailedTestNames(AbstractTestResultAction testResultAction) { - List failedTests = new ArrayList<>(); - - List results = testResultAction.getFailedTests(); + private List getFailedTestNames(final AbstractTestResultAction testResultAction) { + final List failedTests = new ArrayList<>(); + final List results = testResultAction.getFailedTests(); - for(TestResult t : results) { + for(final TestResult t : results) { failedTests.add(t.getFullName()); } return failedTests; } - private List getChangedFiles(Run run) { - List affectedPaths = new ArrayList<>(); + private List getChangedFiles(final Run run) { + final List affectedPaths = new ArrayList<>(); if(run instanceof AbstractBuild) { - AbstractBuild build = (AbstractBuild) run; + final AbstractBuild build = (AbstractBuild) run; - Object[] items = build.getChangeSet().getItems(); + final Object[] items = build.getChangeSet().getItems(); - if(items != null && items.length > 0) { - for(Object o : items) { + if(items != null) { + for(final Object o : items) { if(o instanceof ChangeLogSet.Entry) { affectedPaths.addAll(((ChangeLogSet.Entry) o).getAffectedPaths()); } @@ -283,13 +284,14 @@ private List getChangedFiles(Run run) { return affectedPaths; } - private List getCulprits(Run run) { - List culprits = new ArrayList<>(); + private List getCulprits(final Run run) { + final List culprits = new ArrayList<>(); if(run instanceof AbstractBuild) { - AbstractBuild build = (AbstractBuild) run; - Set buildCulprits = build.getCulprits(); - for(User user : buildCulprits) { + final AbstractBuild build = (AbstractBuild) run; + final Set buildCulprits = build.getCulprits(); + + for(final User user : buildCulprits) { culprits.add(user.getId()); } } @@ -297,110 +299,135 @@ private List getCulprits(Run run) { return culprits; } - private StringBuilder getLog(Run run, Endpoint target) { - StringBuilder log = new StringBuilder(); - Integer loglines = target.getLoglines(); + private StringBuilder getLog(final Run run, final Endpoint target) { + final StringBuilder log = new StringBuilder(); + final Integer logLines = target.getLoglines(); - if (loglines == null || loglines == 0) { + if (logLines == null || logLines == 0) { return log; } try { // The full log - if (loglines == -1) { + if (logLines == -1) { log.append(run.getLog()); } else { - List logEntries = run.getLog(loglines); - for (String entry : logEntries) { + final List logEntries = run.getLog(logLines); + for (final String entry : logEntries) { log.append(entry); log.append("\n"); } } - } catch (IOException e) { + } catch (final IOException e) { log.append("Unable to retrieve log"); } + return log; } - public void handle(Run run, TaskListener listener, long timestamp, boolean manual, final String buildNotes, final Integer logLines, Phase phase) { + public void handle(final Run run, final TaskListener listener, final long timestamp, final boolean manual, final String buildNotes, final Integer logLines, final Phase phase) { final Job job = run.getParent(); final HudsonNotificationProperty property = (HudsonNotificationProperty) job.getProperty(HudsonNotificationProperty.class); + if ( property == null ) { return; } - Result previousCompletedRunResults = findLastBuildThatFinished(run); + final Result previousCompletedRunResults = findLastBuildThatFinished(run); - for ( Endpoint target : property.getEndpoints()) { + for ( final Endpoint target : property.getEndpoints()) { if ((!manual && !isRun(target, run.getResult(), previousCompletedRunResults)) || Utils.isEmpty(target.getUrlInfo().getUrlOrId())) { continue; } - if(Objects.nonNull(buildNotes)) { - target.setBuildNotes(buildNotes); - } + fixTarget(buildNotes, logLines, target); - if(Objects.nonNull(logLines) && logLines != 0) { - target.setLoglines(logLines); - } + notifyEndpoint(run, listener, timestamp, manual, phase, job, target); + } + } - int triesRemaining = target.getRetries(); - boolean failed = false; - do { - // Represents a string that will be put into the log - // if there is an error contacting the target. - String urlIdString = "url 'unknown'"; - try { - EnvVars environment = run.getEnvironment(listener); - // Expand out the URL from environment + url. - String expandedUrl; - UrlInfo urlInfo = target.getUrlInfo(); - switch (urlInfo.getUrlType()) { - case PUBLIC: - expandedUrl = environment.expand(urlInfo.getUrlOrId()); - urlIdString = String.format("url '%s'", expandedUrl); - break; - case SECRET: - String urlSecretId = urlInfo.getUrlOrId(); - String actualUrl = Utils.getSecretUrl(urlSecretId, job.getParent()); - expandedUrl = environment.expand(actualUrl); - urlIdString = String.format("credentials id '%s'", urlSecretId); - break; - default: - throw new UnsupportedOperationException("Unknown URL type"); - } + private void notifyEndpoint(final Run run, final TaskListener listener, final long timestamp, final boolean manual, final Phase phase, final Job job, final Endpoint target) { + int triesRemaining = target.getRetries(); + boolean failed = false; + final EnvVars environment = run.getEnvironment(listener); + do { + // Represents a string that will be put into the log + // if there is an error contacting the target. + String urlIdString = "url 'unknown'"; + try { + // Expand out the URL from environment + url. + final String expandedUrl; + final UrlInfo urlInfo = target.getUrlInfo(); + switch (urlInfo.getUrlType()) { + case PUBLIC: + expandedUrl = environment.expand(urlInfo.getUrlOrId()); + urlIdString = String.format("url '%s'", expandedUrl); + break; + case SECRET: + final String urlSecretId = urlInfo.getUrlOrId(); + final String actualUrl = Utils.getSecretUrl(urlSecretId, job.getParent()); + expandedUrl = environment.expand(actualUrl); + urlIdString = String.format("credentials id '%s'", urlSecretId); + break; + default: + throw new UnsupportedOperationException("Unknown URL type"); + } - if (!isURLValid(urlIdString, expandedUrl, listener.getLogger())) { - continue; - } + if (!isURLValid(urlIdString, expandedUrl, listener.getLogger())) { + continue; + } - final String branch = target.getBranch(); - if (!manual && environment.containsKey("BRANCH_NAME") && !environment.get("BRANCH_NAME").matches(branch)) { - listener.getLogger().printf("Environment variable %s with value %s does not match configured branch filter %s%n", "BRANCH_NAME", environment.get("BRANCH_NAME"), branch); - continue; - }else if(!manual && !environment.containsKey("BRANCH_NAME") && !".*".equals(branch)){ - listener.getLogger().printf("Environment does not contain %s variable%n", "BRANCH_NAME"); - continue; - } + if (filterByBranch(listener, manual, target, environment)) { + continue; + } - listener.getLogger().printf("Notifying endpoint with %s%n", urlIdString); - JobState jobState = buildJobState(job, run, listener, timestamp, target, phase); - target.getProtocol().send(expandedUrl, - target.getFormat().serialize(jobState), - target.getTimeout(), - target.isJson()); - } catch (Throwable error) { - failed = true; - error.printStackTrace( listener.error( String.format( "Failed to notify endpoint with %s", urlIdString))); - listener.getLogger().printf("Failed to notify endpoint with %s - %s: %s%n", - urlIdString, error.getClass().getName(), error.getMessage()); - if (triesRemaining > 0) { - listener.getLogger().printf( - "Reattempting to notify endpoint with %s (%d tries remaining)%n", urlIdString, triesRemaining); - } + listener.getLogger().printf("Notifying endpoint with %s%n", urlIdString); + final JobState jobState = buildJobState(job, run, listener, timestamp, target, phase); + target.getProtocol().send(expandedUrl, + target.getFormat().serialize(jobState), + target.getTimeout(), + target.isJson()); + } catch (final Throwable error) { + failed = true; + error.printStackTrace( listener.error( String.format( "Failed to notify endpoint with %s", urlIdString))); + listener.getLogger().printf("Failed to notify endpoint with %s - %s: %s%n", + urlIdString, error.getClass().getName(), error.getMessage()); + if (triesRemaining > 0) { + listener.getLogger().printf( + "Reattempting to notify endpoint with %s (%d tries remaining)%n", urlIdString, triesRemaining); } } - while (failed && --triesRemaining >= 0); } + while (failed && --triesRemaining >= 0); + } + + private static void fixTarget(final String buildNotes, final Integer logLines, final Endpoint target) { + if(Objects.nonNull(buildNotes)) { + target.setBuildNotes(buildNotes); + } + + if(Objects.nonNull(logLines) && logLines != 0) { + target.setLoglines(logLines); + } + } + + private static boolean filterByBranch(final TaskListener listener, final boolean manual, final Endpoint target, final EnvVars environment) { + final String branch = target.getBranch(); + + String environmentKey = "BRANCH_NAME"; + if(!environment.containsKey(environmentKey)) + { + environmentKey = "gitbranch"; + } + + if (!manual && environment.containsKey(environmentKey) && !environment.get(environmentKey).matches(branch)) { + listener.getLogger().printf("Environment variable %s with value %s does not match configured branch filter %s%n", environmentKey, environment.get(environmentKey), branch); + return true; + }else if(!manual && !environment.containsKey(environmentKey) && !".*".equals(branch)){ + listener.getLogger().printf("Environment does not contain %s variables%n", "BRANCH_NAME or gitbranch"); + return true; + } + + return false; } } From 0fe4cf1a9ae5cf3dca26d7f38cdb325446f2b336 Mon Sep 17 00:00:00 2001 From: "j.berlin" Date: Fri, 23 Jun 2023 12:10:04 +0200 Subject: [PATCH 2/5] Remove deprecated function call and change related parameter description --- src/main/java/com/tikal/hudson/plugins/notification/Phase.java | 2 +- .../notification/HudsonNotificationProperty/config.jelly | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/tikal/hudson/plugins/notification/Phase.java b/src/main/java/com/tikal/hudson/plugins/notification/Phase.java index 9d82ed2..be6ae47 100755 --- a/src/main/java/com/tikal/hudson/plugins/notification/Phase.java +++ b/src/main/java/com/tikal/hudson/plugins/notification/Phase.java @@ -310,7 +310,7 @@ private StringBuilder getLog(final Run run, final Endpoint target) { try { // The full log if (logLines == -1) { - log.append(run.getLog()); + log.append(run.getLog(128)); } else { final List logEntries = run.getLog(logLines); for (final String entry : logEntries) { diff --git a/src/main/resources/com/tikal/hudson/plugins/notification/HudsonNotificationProperty/config.jelly b/src/main/resources/com/tikal/hudson/plugins/notification/HudsonNotificationProperty/config.jelly index 197750e..ff95b8a 100755 --- a/src/main/resources/com/tikal/hudson/plugins/notification/HudsonNotificationProperty/config.jelly +++ b/src/main/resources/com/tikal/hudson/plugins/notification/HudsonNotificationProperty/config.jelly @@ -78,7 +78,7 @@ - From 75551ed5f98f96e48b06ec034ac65f67c2de566c Mon Sep 17 00:00:00 2001 From: "j.berlin" Date: Fri, 23 Jun 2023 12:32:26 +0200 Subject: [PATCH 3/5] Reduce possible exception throwing method calls & catch them at all --- .../hudson/plugins/notification/Phase.java | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/tikal/hudson/plugins/notification/Phase.java b/src/main/java/com/tikal/hudson/plugins/notification/Phase.java index be6ae47..b815c18 100755 --- a/src/main/java/com/tikal/hudson/plugins/notification/Phase.java +++ b/src/main/java/com/tikal/hudson/plugins/notification/Phase.java @@ -121,9 +121,7 @@ private boolean isRun(final Endpoint endpoint, final Result result, final Result } } - private JobState buildJobState(final Job job, final Run run, final TaskListener listener, final long timestamp, final Endpoint target, final Phase phase) - throws IOException, InterruptedException - { + private JobState buildJobState(final Job job, final Run run, final TaskListener listener, final long timestamp, final Endpoint target, final Phase phase, final EnvVars environment) { final Jenkins jenkins = Jenkins.getInstanceOrNull(); assert jenkins != null; @@ -133,7 +131,6 @@ private JobState buildJobState(final Job job, final Run run, final TaskListener final ScmState scmState = new ScmState(); final Result result = run.getResult(); final ParametersAction paramsAction = run.getAction(ParametersAction.class); - final EnvVars environment = run.getEnvironment( listener ); final StringBuilder log = this.getLog(run, target); jobState.setName( job.getName()); @@ -342,11 +339,18 @@ public void handle(final Run run, final TaskListener listener, final long timest fixTarget(buildNotes, logLines, target); - notifyEndpoint(run, listener, timestamp, manual, phase, job, target); + try { + notifyEndpoint(run, listener, timestamp, manual, phase, job, target); + } catch (final InterruptedException | IOException e) { + final String targetUrl = target.getUrlInfo().getUrlOrId(); + e.printStackTrace( listener.error( String.format( "Failed to notify endpoint with %s", targetUrl))); + listener.getLogger().printf("Failed to notify endpoint with %s - %s: %s%n", + targetUrl, e.getClass().getName(), e.getMessage()); + } } } - private void notifyEndpoint(final Run run, final TaskListener listener, final long timestamp, final boolean manual, final Phase phase, final Job job, final Endpoint target) { + private void notifyEndpoint(final Run run, final TaskListener listener, final long timestamp, final boolean manual, final Phase phase, final Job job, final Endpoint target) throws IOException, InterruptedException { int triesRemaining = target.getRetries(); boolean failed = false; final EnvVars environment = run.getEnvironment(listener); @@ -382,7 +386,7 @@ private void notifyEndpoint(final Run run, final TaskListener listener, final lo } listener.getLogger().printf("Notifying endpoint with %s%n", urlIdString); - final JobState jobState = buildJobState(job, run, listener, timestamp, target, phase); + final JobState jobState = buildJobState(job, run, listener, timestamp, target, phase, environment); target.getProtocol().send(expandedUrl, target.getFormat().serialize(jobState), target.getTimeout(), From 9be1c4dbb0b924f4e606a6159fa49e6fb117d7f7 Mon Sep 17 00:00:00 2001 From: "j.berlin" Date: Fri, 23 Jun 2023 13:01:42 +0200 Subject: [PATCH 4/5] Reduce possible exception throwing method calls & catch them at all Fix refactored isRun method Refactor PhaseTest by IntelliJ input --- .../hudson/plugins/notification/Phase.java | 9 +- .../plugins/notification/PhaseTest.java | 98 ++++++++----------- 2 files changed, 45 insertions(+), 62 deletions(-) diff --git a/src/main/java/com/tikal/hudson/plugins/notification/Phase.java b/src/main/java/com/tikal/hudson/plugins/notification/Phase.java index b815c18..d2f6e41 100755 --- a/src/main/java/com/tikal/hudson/plugins/notification/Phase.java +++ b/src/main/java/com/tikal/hudson/plugins/notification/Phase.java @@ -104,18 +104,17 @@ private boolean isRun(final Endpoint endpoint, final Result result, final Result if(event == null || "all".equals(event)) return true; - if(result == null || "manual".equals(event)){ - return false; - } - switch(event){ case "failed": + if (result == null) {return false;} return this.equals(FINALIZED) && result.equals(Result.FAILURE); case "failedAndFirstSuccess": - if (!this.equals(FINALIZED)) {return false;} + if (result == null || !this.equals(FINALIZED)) {return false;} if (result.equals(Result.FAILURE)) {return true;} return previousRunResult != null && result.equals(Result.SUCCESS) && previousRunResult.equals(Result.FAILURE); + case "manual": + return false; default: return event.equals(this.toString().toLowerCase()); } diff --git a/src/test/java/com/tikal/hudson/plugins/notification/PhaseTest.java b/src/test/java/com/tikal/hudson/plugins/notification/PhaseTest.java index 37412d0..95a5c77 100644 --- a/src/test/java/com/tikal/hudson/plugins/notification/PhaseTest.java +++ b/src/test/java/com/tikal/hudson/plugins/notification/PhaseTest.java @@ -1,26 +1,31 @@ package com.tikal.hudson.plugins.notification; -import static com.tikal.hudson.plugins.notification.UrlType.PUBLIC; -import static com.tikal.hudson.plugins.notification.UrlType.SECRET; -import static java.util.Arrays.asList; -import static org.junit.Assert.assertEquals; -import static org.mockito.Mockito.*; - -import java.io.IOException; -import java.io.PrintStream; -import java.lang.reflect.Method; - import com.tikal.hudson.plugins.notification.model.JobState; import hudson.EnvVars; -import hudson.model.*; +import hudson.model.Job; +import hudson.model.Result; +import hudson.model.Run; +import hudson.model.TaskListener; import jenkins.model.Jenkins; import org.junit.Test; - import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.MockedStatic; import org.mockito.junit.MockitoJUnitRunner; +import java.io.IOException; +import java.io.PrintStream; +import java.lang.reflect.Method; + +import static com.tikal.hudson.plugins.notification.UrlType.PUBLIC; +import static com.tikal.hudson.plugins.notification.UrlType.SECRET; +import static java.lang.Boolean.FALSE; +import static java.lang.Boolean.TRUE; +import static java.util.Arrays.asList; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.mockito.Mockito.*; + @RunWith(MockitoJUnitRunner.class) public class PhaseTest { @Mock @@ -48,71 +53,50 @@ public class PhaseTest { @Test public void testIsRun() throws ReflectiveOperationException { - Endpoint endPoint = new Endpoint(null); - Method isRunMethod = Phase.class.getDeclaredMethod("isRun", Endpoint.class, Result.class, Result.class); + final Endpoint endPoint = new Endpoint(null); + final Method isRunMethod = Phase.class.getDeclaredMethod("isRun", Endpoint.class, Result.class, Result.class); isRunMethod.setAccessible(true); - assertEquals("returns true for null endpoint event", - isRunMethod.invoke(Phase.QUEUED, endPoint, null, null), Boolean.TRUE); + assertThat("returns true for null endpoint event", isRunMethod.invoke(Phase.QUEUED, endPoint, null, null), is(TRUE)); endPoint.setEvent("all"); - for (Phase phaseValue : Phase.values()) { - assertEquals("all Event returns true for Phase " + phaseValue.toString(), - isRunMethod.invoke(phaseValue, endPoint, null, null), Boolean.TRUE); + for (final Phase phaseValue : Phase.values()) { + assertThat("all Event returns true for Phase " + phaseValue.toString(), isRunMethod.invoke(phaseValue, endPoint, null, null), is(TRUE)); } endPoint.setEvent("queued"); - assertEquals("queued Event returns true for Phase Queued", - isRunMethod.invoke(Phase.QUEUED, endPoint, null, null), Boolean.TRUE); - assertEquals("queued Event returns false for Phase Started", - isRunMethod.invoke(Phase.STARTED, endPoint, null, null), Boolean.FALSE); + assertThat("queued Event returns true for Phase Queued", isRunMethod.invoke(Phase.QUEUED, endPoint, null, null), is(TRUE)); + assertThat("queued Event returns false for Phase Started", isRunMethod.invoke(Phase.STARTED, endPoint, null, null), is(FALSE)); endPoint.setEvent("started"); - assertEquals("started Event returns true for Phase Started", - isRunMethod.invoke(Phase.STARTED, endPoint, null, null), Boolean.TRUE); - assertEquals("started Event returns false for Phase Completed", - isRunMethod.invoke(Phase.COMPLETED, endPoint, null, null), Boolean.FALSE); + assertThat("started Event returns true for Phase Started", isRunMethod.invoke(Phase.STARTED, endPoint, null, null), is(TRUE)); + assertThat("started Event returns false for Phase Completed", isRunMethod.invoke(Phase.COMPLETED, endPoint, null, null), is(FALSE)); endPoint.setEvent("completed"); - assertEquals("completed Event returns true for Phase Completed", - isRunMethod.invoke(Phase.COMPLETED, endPoint, null, null), Boolean.TRUE); - assertEquals("completed Event returns false for Phase Finalized", - isRunMethod.invoke(Phase.FINALIZED, endPoint, null, null), Boolean.FALSE); + assertThat("completed Event returns true for Phase Completed", isRunMethod.invoke(Phase.COMPLETED, endPoint, null, null), is(TRUE)); + assertThat("completed Event returns false for Phase Finalized", isRunMethod.invoke(Phase.FINALIZED, endPoint, null, null), is(FALSE)); endPoint.setEvent("finalized"); - assertEquals("finalized Event returns true for Phase Finalized", - isRunMethod.invoke(Phase.FINALIZED, endPoint, null, null), Boolean.TRUE); - assertEquals("finalized Event returns true for Phase Queued", - isRunMethod.invoke(Phase.QUEUED, endPoint, null, null), Boolean.FALSE); + assertThat("finalized Event returns true for Phase Finalized", isRunMethod.invoke(Phase.FINALIZED, endPoint, null, null), is(TRUE)); + assertThat("finalized Event returns true for Phase Queued", isRunMethod.invoke(Phase.QUEUED, endPoint, null, null), is(FALSE)); endPoint.setEvent("failed"); - assertEquals("failed Event returns false for Phase Finalized and no status", - isRunMethod.invoke(Phase.FINALIZED, endPoint, null, null), Boolean.FALSE); - assertEquals("failed Event returns false for Phase Finalized and success status", - isRunMethod.invoke(Phase.FINALIZED, endPoint, Result.SUCCESS, null), Boolean.FALSE); - assertEquals("failed Event returns true for Phase Finalized and success failure", - isRunMethod.invoke(Phase.FINALIZED, endPoint, Result.FAILURE, null), Boolean.TRUE); - assertEquals("failed Event returns false for Phase not Finalized and success failure", - isRunMethod.invoke(Phase.COMPLETED, endPoint, Result.FAILURE, null), Boolean.FALSE); + assertThat("failed Event returns false for Phase Finalized and no status", isRunMethod.invoke(Phase.FINALIZED, endPoint, null, null), is(FALSE)); + assertThat("failed Event returns false for Phase Finalized and success status", isRunMethod.invoke(Phase.FINALIZED, endPoint, Result.SUCCESS, null), is(FALSE)); + assertThat("failed Event returns true for Phase Finalized and success failure", isRunMethod.invoke(Phase.FINALIZED, endPoint, Result.FAILURE, null), is(TRUE)); + assertThat("failed Event returns false for Phase not Finalized and success failure", isRunMethod.invoke(Phase.COMPLETED, endPoint, Result.FAILURE, null), is(FALSE)); endPoint.setEvent("failedAndFirstSuccess"); - assertEquals("failedAndFirstSuccess Event returns false for Phase Finalized and no status", - isRunMethod.invoke(Phase.FINALIZED, endPoint, null, null), Boolean.FALSE); - assertEquals("failedAndFirstSuccess Event returns false for Phase Finalized and no previous status", - isRunMethod.invoke(Phase.FINALIZED, endPoint, Result.SUCCESS, null), Boolean.FALSE); - assertEquals("failedAndFirstSuccess Event returns true for Phase Finalized and no previous status and failed status", - isRunMethod.invoke(Phase.FINALIZED, endPoint, Result.FAILURE, null), Boolean.TRUE); - assertEquals("failedAndFirstSuccess Event returns true for Phase Finalized and failed status", - isRunMethod.invoke(Phase.FINALIZED, endPoint, Result.FAILURE, Result.FAILURE), Boolean.TRUE); - assertEquals("failedAndFirstSuccess Event returns true for Phase Finalized and success status with previous status of failure", - isRunMethod.invoke(Phase.FINALIZED, endPoint, Result.SUCCESS, Result.FAILURE), Boolean.TRUE); - assertEquals("failedAndFirstSuccess Event returns false for Phase Finalized and success status with previous status of success", - isRunMethod.invoke(Phase.FINALIZED, endPoint, Result.SUCCESS, Result.SUCCESS), Boolean.FALSE); - assertEquals("failedAndFirstSuccess Event returns false for Phase not Finalized", - isRunMethod.invoke(Phase.COMPLETED, endPoint, Result.SUCCESS, Result.FAILURE), Boolean.FALSE); + assertThat("failedAndFirstSuccess Event returns false for Phase Finalized and no status", isRunMethod.invoke(Phase.FINALIZED, endPoint, null, null), is(FALSE)); + assertThat("failedAndFirstSuccess Event returns false for Phase Finalized and no previous status", isRunMethod.invoke(Phase.FINALIZED, endPoint, Result.SUCCESS, null), is(FALSE)); + assertThat("failedAndFirstSuccess Event returns true for Phase Finalized and no previous status and failed status", isRunMethod.invoke(Phase.FINALIZED, endPoint, Result.FAILURE, null), is(TRUE)); + assertThat("failedAndFirstSuccess Event returns true for Phase Finalized and failed status", isRunMethod.invoke(Phase.FINALIZED, endPoint, Result.FAILURE, Result.FAILURE), is(TRUE)); + assertThat("failedAndFirstSuccess Event returns true for Phase Finalized and success status with previous status of failure", isRunMethod.invoke(Phase.FINALIZED, endPoint, Result.SUCCESS, Result.FAILURE), is(TRUE)); + assertThat("failedAndFirstSuccess Event returns false for Phase Finalized and success status with previous status of success", isRunMethod.invoke(Phase.FINALIZED, endPoint, Result.SUCCESS, Result.SUCCESS), is(FALSE)); + assertThat("failedAndFirstSuccess Event returns false for Phase not Finalized", isRunMethod.invoke(Phase.COMPLETED, endPoint, Result.SUCCESS, Result.FAILURE), is(FALSE)); } @Test From a1c6227eb22f3299beccaa30f0070d2cfc170a09 Mon Sep 17 00:00:00 2001 From: "j.berlin" Date: Wed, 28 Jun 2023 15:42:39 +0200 Subject: [PATCH 5/5] [JENKINS-71373] Handle https://issues.jenkins.io/projects/JENKINS/issues/JENKINS-71373 by make use of a reference implementation --- .../plugins/notification/NotifyStep.java | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/tikal/hudson/plugins/notification/NotifyStep.java b/src/main/java/com/tikal/hudson/plugins/notification/NotifyStep.java index 0214842..5bccb67 100644 --- a/src/main/java/com/tikal/hudson/plugins/notification/NotifyStep.java +++ b/src/main/java/com/tikal/hudson/plugins/notification/NotifyStep.java @@ -1,11 +1,11 @@ package com.tikal.hudson.plugins.notification; +import com.google.common.collect.ImmutableSet; import hudson.Extension; import hudson.FilePath; import hudson.Util; import hudson.model.Run; import hudson.model.TaskListener; -import org.eclipse.collections.impl.factory.Sets; import org.jenkinsci.plugins.workflow.graph.FlowNode; import org.jenkinsci.plugins.workflow.steps.Step; import org.jenkinsci.plugins.workflow.steps.StepContext; @@ -14,13 +14,12 @@ import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.DataBoundSetter; +import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; import java.io.Serializable; import java.util.Objects; import java.util.Set; -import javax.annotation.CheckForNull; -import javax.annotation.Nonnull; - public class NotifyStep extends Step implements Serializable { private static final long serialVersionUID = -2818860651754465006L; @@ -46,7 +45,7 @@ public String getPhase() { } @DataBoundSetter - public void setPhase(@CheckForNull String phase) { + public void setPhase(@CheckForNull final String phase) { this.phase = Util.fixEmpty(phase); } @@ -59,17 +58,12 @@ public String getLoglines() { } @DataBoundSetter - public void setLoglines(@CheckForNull String loglines) { + public void setLoglines(@CheckForNull final String loglines) { this.loglines = Util.fixEmpty(loglines); } - /** - * Creates a new instance of {@link NotifyStep}. - */ @DataBoundConstructor public NotifyStep() { - super(); - // empty constructor required for Stapler } @@ -93,7 +87,7 @@ static class Execution extends StepExecution { @Override public boolean start() throws Exception { - String logLines = notifyStep.getLoglines(); + final String logLines = notifyStep.getLoglines(); Phase.NONE.handle(Objects.requireNonNull(getContext().get(Run.class)), getContext().get(TaskListener.class), System.currentTimeMillis(), true, notifyStep.getNotes(), Integer.parseInt(logLines != null ? logLines : "0"), Phase.valueOf(notifyStep.getPhase())); @@ -122,9 +116,12 @@ public String getDisplayName() { } @Override + /* + Made use of implementation of + @see org.jenkinsci.plugins.workflow.steps.scm.SCMStep + */ public Set> getRequiredContext() { - return Sets.immutable.of(FilePath.class, FlowNode.class, Run.class, TaskListener.class) - .castToSet(); + return ImmutableSet.of(Run.class, FilePath.class, TaskListener.class, FlowNode.class); } } }