Skip to content

Commit e45da60

Browse files
krissternkrisstern
authored
Patch remaining NullPointerException bugs in GitLabCommitStatusStep (jenkinsci#1398)
1 parent d8e6e55 commit e45da60

File tree

5 files changed

+36
-39
lines changed

5 files changed

+36
-39
lines changed

‎src/main/java/com/dabsquared/gitlabjenkins/util/CommitStatusUpdater.java

+22-24
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package com.dabsquared.gitlabjenkins.util;
22

3-
43
import com.dabsquared.gitlabjenkins.cause.CauseData;
54
import com.dabsquared.gitlabjenkins.cause.GitLabWebHookCause;
65
import com.dabsquared.gitlabjenkins.connection.GitLabConnection;
@@ -42,10 +41,9 @@ public class CommitStatusUpdater {
4241

4342
private final static Logger LOGGER = Logger.getLogger(CommitStatusUpdater.class.getName());
4443

45-
4644
public static void updateCommitStatus(Run<?, ?> build, TaskListener listener, BuildState state, String name, List<GitLabBranchBuild> gitLabBranchBuilds, GitLabConnectionProperty connection) {
4745
GitLabClient client;
48-
if(connection != null) {
46+
if (connection != null) {
4947
client = connection.getClient();
5048
} else {
5149
client = getClient(build);
@@ -65,36 +63,38 @@ public static void updateCommitStatus(Run<?, ?> build, TaskListener listener, Bu
6563
}
6664

6765
final String buildUrl = getBuildUrl(build);
68-
for (final GitLabBranchBuild gitLabBranchBuild : gitLabBranchBuilds) {
69-
try {
70-
GitLabClient current_client = client;
71-
if(gitLabBranchBuild.getConnection() != null ) {
72-
GitLabClient build_specific_client = gitLabBranchBuild.getConnection().getClient();
73-
if (build_specific_client != null) {
74-
current_client = build_specific_client;
66+
if (gitLabBranchBuilds != null) {
67+
for (final GitLabBranchBuild gitLabBranchBuild : gitLabBranchBuilds) {
68+
try {
69+
GitLabClient current_client = client;
70+
if (gitLabBranchBuild.getConnection() != null) {
71+
GitLabClient build_specific_client = gitLabBranchBuild.getConnection().getClient();
72+
if (build_specific_client != null) {
73+
current_client = build_specific_client;
74+
}
7575
}
76-
}
7776

78-
String current_build_name = name;
79-
if(gitLabBranchBuild.getName() != null ) {
80-
current_build_name = gitLabBranchBuild.getName();
81-
}
77+
String current_build_name = name;
78+
if (gitLabBranchBuild.getName() != null) {
79+
current_build_name = gitLabBranchBuild.getName();
80+
}
8281

83-
if (existsCommit(current_client, gitLabBranchBuild.getProjectId(), gitLabBranchBuild.getRevisionHash())) {
84-
LOGGER.log(Level.INFO, String.format("Updating build '%s' to '%s'", gitLabBranchBuild.getProjectId(),state));
85-
current_client.changeBuildStatus(gitLabBranchBuild.getProjectId(), gitLabBranchBuild.getRevisionHash(), state, getBuildBranchOrTag(build), current_build_name, buildUrl, state.name());
82+
if (existsCommit(current_client, gitLabBranchBuild.getProjectId(), gitLabBranchBuild.getRevisionHash())) {
83+
LOGGER.log(Level.INFO, String.format("Updating build '%s' to '%s'", gitLabBranchBuild.getProjectId(), state));
84+
current_client.changeBuildStatus(gitLabBranchBuild.getProjectId(), gitLabBranchBuild.getRevisionHash(), state, getBuildBranchOrTag(build), current_build_name, buildUrl, state.name());
85+
}
86+
} catch (WebApplicationException | ProcessingException e) {
87+
printf(listener, "Failed to update Gitlab commit status for project '%s': %s%n", gitLabBranchBuild.getProjectId(), e.getMessage());
88+
LOGGER.log(Level.SEVERE, String.format("Failed to update Gitlab commit status for project '%s'", gitLabBranchBuild.getProjectId()), e);
8689
}
87-
} catch (WebApplicationException | ProcessingException e) {
88-
printf(listener, "Failed to update Gitlab commit status for project '%s': %s%n", gitLabBranchBuild.getProjectId(), e.getMessage());
89-
LOGGER.log(Level.SEVERE, String.format("Failed to update Gitlab commit status for project '%s'", gitLabBranchBuild.getProjectId()), e);
9090
}
9191
}
9292
}
9393

9494

9595
public static void updateCommitStatus(Run<?, ?> build, TaskListener listener, BuildState state, String name) {
9696
try {
97-
updateCommitStatus(build,listener,state,name,null,null);
97+
updateCommitStatus(build, listener, state, name, null, null);
9898
} catch (IllegalStateException e) {
9999
printf(listener, "Failed to update Gitlab commit status: %s%n", e.getMessage());
100100
}
@@ -271,6 +271,4 @@ private static List<GitLabBranchBuild> findBuildsFromUpstreamCauses(List<Cause>
271271
}
272272
return Collections.emptyList();
273273
}
274-
275-
276274
}

‎src/main/java/com/dabsquared/gitlabjenkins/workflow/GitLabBranchBuild.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.dabsquared.gitlabjenkins.workflow;
22

33
import com.dabsquared.gitlabjenkins.connection.GitLabConnectionProperty;
4+
import edu.umd.cs.findbugs.annotations.NonNull;
45
import hudson.Extension;
56
import hudson.model.AbstractDescribableImpl;
67
import hudson.model.Descriptor;
@@ -13,7 +14,6 @@
1314
public class GitLabBranchBuild extends AbstractDescribableImpl<GitLabBranchBuild> {
1415
private static final Logger LOGGER = LoggerFactory.getLogger(GitLabBranchBuild.class);
1516

16-
1717
private String name;
1818
private String projectId;
1919
private String revisionHash;
@@ -70,10 +70,9 @@ public GitLabConnectionProperty getConnection() {
7070
return connection;
7171
}
7272

73-
74-
7573
@Extension
7674
public static class DescriptorImpl extends Descriptor<GitLabBranchBuild> {
75+
@NonNull
7776
@Override
7877
public String getDisplayName() {
7978
return "Gitlab Branch Build";

‎src/main/java/com/dabsquared/gitlabjenkins/workflow/GitLabBuildsStep.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,7 @@ public class GitLabBuildsStep extends Step {
3737
@DataBoundConstructor
3838
public GitLabBuildsStep() {
3939
}
40-
41-
40+
4241
@Override
4342
public StepExecution start(StepContext context) throws Exception {
4443
return new GitLabBuildStepExecution(context, this);

‎src/main/java/com/dabsquared/gitlabjenkins/workflow/GitLabCommitStatusStep.java

+5-4
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ public void stop(@NonNull Throwable cause) throws Exception {
129129
// should be no need to do anything special (but verify in JENKINS-26148)
130130
if (body != null) {
131131
String name = StringUtils.isEmpty(step.name) ? "jenkins" : step.name;
132-
CommitStatusUpdater.updateCommitStatus(run, null, BuildState.canceled, name, step.builds, step.connection);
132+
CommitStatusUpdater.updateCommitStatus(run, null, BuildState.canceled, name, step.builds, step.connection);
133133
body.cancel(cause);
134134
}
135135
}
@@ -149,6 +149,7 @@ private TaskListener getTaskListener(StepContext context) {
149149
@Extension
150150
public static final class DescriptorImpl extends StepDescriptor {
151151

152+
@NonNull
152153
@Override
153154
public String getDisplayName() {
154155
return "Update the commit status in GitLab depending on the build status";
@@ -166,9 +167,9 @@ public boolean takesImplicitBlockArgument() {
166167

167168
@Override
168169
public Set<Class<?>> getRequiredContext() {
169-
Set<Class<?>> context = new HashSet<>();
170-
Collections.addAll(context, TaskListener.class, Run.class);
171-
return Collections.unmodifiableSet(context);
170+
Set<Class<?>> context = new HashSet<>();
171+
Collections.addAll(context, TaskListener.class, Run.class);
172+
return Collections.unmodifiableSet(context);
172173
}
173174
}
174175
}

‎src/test/java/com/dabsquared/gitlabjenkins/workflow/GitLabCommitStatusStepTest.java

+6-6
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ public void bare_gitlabCommitStatus() throws Exception {
1919
String pipelineText = IOUtils.toString(getClass().getResourceAsStream(
2020
"pipeline/bare-gitlabCommitStatus-pipeline.groovy"));
2121
project.setDefinition(new CpsFlowDefinition(pipelineText, false));
22-
Run build = j.buildAndAssertSuccess(project);
22+
Run<?, ?> build = j.buildAndAssertSuccess(project);
2323
j.assertLogContains("this is simple jenkins-build", build);
2424
}
2525

@@ -29,7 +29,7 @@ public void named_simple_pipeline_builds_as_LString() throws Exception {
2929
String pipelineText = IOUtils.toString(getClass().getResourceAsStream(
3030
"pipeline/named-simple-pipeline-builds-as-LString.groovy"));
3131
project.setDefinition(new CpsFlowDefinition(pipelineText, false));
32-
Run build = j.buildAndAssertSuccess(project);
32+
Run<?, ?> build = j.buildAndAssertSuccess(project);
3333
j.assertLogContains("this is pre-build stage", build);
3434
}
3535

@@ -39,7 +39,7 @@ public void named_simple_pipeline_builds_as_String() throws Exception {
3939
String pipelineText = IOUtils.toString(getClass().getResourceAsStream(
4040
"pipeline/named-simple-pipeline-builds-as-String.groovy"));
4141
project.setDefinition(new CpsFlowDefinition(pipelineText, false));
42-
Run build = j.buildAndAssertSuccess(project);
42+
Run<?, ?> build = j.buildAndAssertSuccess(project);
4343
j.assertLogContains("this is pre-build stage", build);
4444
}
4545

@@ -49,7 +49,7 @@ public void multisite() throws Exception {
4949
String pipelineText = IOUtils.toString(getClass().getResourceAsStream(
5050
"pipeline/multisite-pipeline.groovy"));
5151
project.setDefinition(new CpsFlowDefinition(pipelineText, false));
52-
Run build = j.buildAndAssertSuccess(project);
52+
Run<?, ?> build = j.buildAndAssertSuccess(project);
5353
j.assertLogContains("this is stage3", build);
5454
}
5555

@@ -59,7 +59,7 @@ public void multiproject_specific_connection() throws Exception {
5959
String pipelineText = IOUtils.toString(getClass().getResourceAsStream(
6060
"pipeline/multiproject-specific-connection-pipeline.groovy"));
6161
project.setDefinition(new CpsFlowDefinition(pipelineText, false));
62-
Run build = j.buildAndAssertSuccess(project);
62+
Run<?, ?> build = j.buildAndAssertSuccess(project);
6363
j.assertLogContains("this is pre-build stage", build);
6464
}
6565

@@ -69,7 +69,7 @@ public void multiproject() throws Exception {
6969
String pipelineText = IOUtils.toString(getClass().getResourceAsStream(
7070
"pipeline/multiproject-pipeline.groovy"));
7171
project.setDefinition(new CpsFlowDefinition(pipelineText, false));
72-
Run build = j.buildAndAssertSuccess(project);
72+
Run<?, ?> build = j.buildAndAssertSuccess(project);
7373
j.assertLogContains("this is pre-build stage", build);
7474
}
7575
}

0 commit comments

Comments
 (0)