Skip to content

Commit

Permalink
Make HttpNotifierSelector transient
Browse files Browse the repository at this point in the history 
This is resolved at runtime and does not need to be serialized.
Use setter injection and inject HttpNotifierSelector when used to
prevent runtime NPE.
  • Loading branch information
@sghill
sghill committed Oct 25, 2021
1 parent d22851c commit 7ad59f8
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 34 deletions.
32 changes: 25 additions & 7 deletions src/main/java/org/jenkinsci/plugins/stashNotifier/StashNotifier.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.cloudbees.plugins.credentials.common.StandardCredentials;
import com.cloudbees.plugins.credentials.common.StandardListBoxModel;
import com.cloudbees.plugins.credentials.common.UsernamePasswordCredentials;
import com.google.inject.Injector;
import hudson.Extension;
import hudson.FilePath;
import hudson.Launcher;
Expand Down Expand Up @@ -70,6 +71,7 @@
import org.kohsuke.stapler.*;

import javax.annotation.Nonnull;
import javax.inject.Inject;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLPeerUnverifiedException;
import javax.servlet.ServletException;
Expand Down Expand Up @@ -158,7 +160,12 @@ public class StashNotifier extends Notifier implements SimpleBuildStep {
private final boolean considerUnstableAsSuccess;

private final JenkinsLocationConfiguration globalConfig;
private final HttpNotifierSelector httpNotifierSelector;

/**
* gives us the desired {@link HttpNotifier}. Transient because
* we resolve this at runtime rather than serializing.
*/
private transient HttpNotifierSelector httpNotifierSelector;

// public members ----------------------------------------------------------

Expand All @@ -179,8 +186,7 @@ public BuildStepMonitor getRequiredMonitorService() {
boolean prependParentProjectKey,
boolean disableInprogressNotification,
boolean considerUnstableAsSuccess,
JenkinsLocationConfiguration globalConfig,
HttpNotifierSelector httpNotifierSelector
JenkinsLocationConfiguration globalConfig
) {

this.stashServerBaseUrl = stashServerBaseUrl != null && stashServerBaseUrl.endsWith("/")
Expand All @@ -205,7 +211,6 @@ public BuildStepMonitor getRequiredMonitorService() {
this.disableInprogressNotification = disableInprogressNotification;
this.considerUnstableAsSuccess = considerUnstableAsSuccess;
this.globalConfig = globalConfig;
this.httpNotifierSelector = httpNotifierSelector;
}

@DataBoundConstructor
Expand Down Expand Up @@ -234,8 +239,7 @@ public StashNotifier(
prependParentProjectKey,
disableInprogressNotification,
considerUnstableAsSuccess,
JenkinsLocationConfiguration.get(),
Jenkins.getInstance().getInjector().getInstance(HttpNotifierSelector.class)
JenkinsLocationConfiguration.get()
);
}

Expand Down Expand Up @@ -283,6 +287,20 @@ public boolean getPrependParentProjectKey() {
return prependParentProjectKey;
}

@Inject
void setHttpNotifierSelector(HttpNotifierSelector httpNotifierSelector) {
this.httpNotifierSelector = httpNotifierSelector;
}

HttpNotifierSelector getHttpNotifierSelector() {
if (httpNotifierSelector == null) {
Jenkins jenkins = Jenkins.getInstance();
Injector injector = jenkins.getInjector();
injector.injectMembers(this);
}
return httpNotifierSelector;
}

@Override
public boolean prebuild(AbstractBuild<?, ?> build, BuildListener listener) {
return disableInprogressNotification || processJenkinsEvent(build, null, listener, StashBuildState.INPROGRESS);
Expand Down Expand Up @@ -800,7 +818,7 @@ protected NotificationResult notifyStash(
logger,
run.getExternalizableId()
);
HttpNotifier notifier = httpNotifierSelector.select(new SelectionContext(run.getParent().getFullName()));
HttpNotifier notifier = getHttpNotifierSelector().select(new SelectionContext(run.getParent().getFullName()));
return notifier.send(uri, payload, settings, context);
}

Expand Down
55 changes: 28 additions & 27 deletions src/test/java/org/jenkinsci/plugins/stashNotifier/StashNotifierTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ private StashNotifier buildStashNotifier(String stashBaseUrl) {
private StashNotifier buildStashNotifier(String stashBaseUrl,
boolean disableInprogressNotification,
boolean considerUnstableAsSuccess) {
return new StashNotifier(
StashNotifier notifier = new StashNotifier(
stashBaseUrl,
"scot",
true,
Expand All @@ -104,9 +104,10 @@ private StashNotifier buildStashNotifier(String stashBaseUrl,
true,
disableInprogressNotification,
considerUnstableAsSuccess,
mock(JenkinsLocationConfiguration.class),
httpNotifierSelector
mock(JenkinsLocationConfiguration.class)
);
notifier.setHttpNotifierSelector(httpNotifierSelector);
return notifier;
}

StashNotifier sn;
Expand Down Expand Up @@ -248,8 +249,8 @@ public void test_build_http_client_https() throws Exception {
false,
false,
false,
mock(JenkinsLocationConfiguration.class),
httpNotifierSelector);
mock(JenkinsLocationConfiguration.class)
);

PrintStream logger = mock(PrintStream.class);

Expand Down Expand Up @@ -501,8 +502,8 @@ public void lookupCommitSha1s() throws Exception {
false,
false,
false,
mock(JenkinsLocationConfiguration.class),
httpNotifierSelector);
mock(JenkinsLocationConfiguration.class)
);

//when
Collection<String> hashes = sn.lookupCommitSha1s(build, null, buildListener);
Expand Down Expand Up @@ -530,8 +531,8 @@ private void lookupCommitSha1s_Exception(Exception e) throws InterruptedExceptio
false,
false,
false,
mock(JenkinsLocationConfiguration.class),
httpNotifierSelector);
mock(JenkinsLocationConfiguration.class)
);

//when
Collection<String> hashes = sn.lookupCommitSha1s(build, null, buildListener);
Expand Down Expand Up @@ -612,8 +613,8 @@ public void test_getPushedBuildState_overwritten() {
true,
false,
false,
mock(JenkinsLocationConfiguration.class),
httpNotifierSelector);
mock(JenkinsLocationConfiguration.class)
);

//when
StashBuildState pushedBuildStatus = sn.getPushedBuildStatus(StashBuildState.FAILED);
Expand All @@ -637,8 +638,8 @@ public void test_getPushedBuildState_not_overwritten() {
true,
false,
false,
mock(JenkinsLocationConfiguration.class),
httpNotifierSelector);
mock(JenkinsLocationConfiguration.class)
);

//when
StashBuildState pushedBuildStatus = sn.getPushedBuildStatus(StashBuildState.FAILED);
Expand All @@ -665,8 +666,8 @@ public void test_getBuildName_overwritten() {
true,
false,
false,
mock(JenkinsLocationConfiguration.class),
httpNotifierSelector);
mock(JenkinsLocationConfiguration.class)
);

//when
String buildName = sn.getBuildName(run);
Expand All @@ -692,8 +693,8 @@ public void test_getBuildName_not_overwritten() {
true,
false,
false,
mock(JenkinsLocationConfiguration.class),
httpNotifierSelector);
mock(JenkinsLocationConfiguration.class)
);

//when
String buildName = sn.getBuildName(run);
Expand Down Expand Up @@ -723,8 +724,8 @@ public void test_getBuildKey() throws Exception {
true,
false,
false,
mock(JenkinsLocationConfiguration.class),
httpNotifierSelector);
mock(JenkinsLocationConfiguration.class)
);

//when
String buildKey = sn.getBuildKey(build, buildListener);
Expand Down Expand Up @@ -755,8 +756,8 @@ public void test_getBuildKey_withBuildName() {
true,
false,
false,
mock(JenkinsLocationConfiguration.class),
httpNotifierSelector);
mock(JenkinsLocationConfiguration.class)
);

//when
String buildKey = sn.getBuildKey(build, buildListener);
Expand Down Expand Up @@ -788,8 +789,8 @@ public void test_getRunKey() throws Exception {
true,
false,
false,
mock(JenkinsLocationConfiguration.class),
httpNotifierSelector);
mock(JenkinsLocationConfiguration.class)
);

//when
String buildKey = sn.getBuildKey(run, buildListener);
Expand Down Expand Up @@ -818,8 +819,8 @@ private void getBuildKey_Exception(Exception e) throws InterruptedException, Mac
true,
false,
false,
mock(JenkinsLocationConfiguration.class),
httpNotifierSelector);
mock(JenkinsLocationConfiguration.class)
);

//when
String buildKey = sn.getBuildKey(build, buildListener);
Expand Down Expand Up @@ -851,8 +852,8 @@ private void getRunKey_Exception(Exception e) throws InterruptedException, Macro
true,
false,
false,
mock(JenkinsLocationConfiguration.class),
httpNotifierSelector);
mock(JenkinsLocationConfiguration.class)
);

//when
String buildKey = sn.getBuildKey(run, buildListener);
Expand Down

0 comments on commit 7ad59f8

Please sign in to comment.