Skip to content

Commit 16a6352

Browse files
jgangemiKostyaSha
jgangemi
authored and
KostyaSha
committed
- check the last commit's message if repo isn't known
1 parent 1387c27 commit 16a6352

5 files changed

Lines changed: 190 additions & 70 deletions

File tree

‎github-pullrequest-plugin/src/main/java/com/github/kostyasha/github/integration/branch/events/impl/GitHubBranchCommitEvent.java‎

Lines changed: 45 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,10 @@
44
import com.github.kostyasha.github.integration.branch.GitHubBranchCause;
55
import com.github.kostyasha.github.integration.branch.GitHubBranchRepository;
66
import com.github.kostyasha.github.integration.branch.GitHubBranchTrigger;
7-
import com.github.kostyasha.github.integration.branch.events.impl.commitchecks.GitHubBranchCommitCheck;
8-
import com.github.kostyasha.github.integration.branch.events.impl.commitchecks.GitHubBranchCommitCheckDescriptor;
97
import com.github.kostyasha.github.integration.branch.events.GitHubBranchEvent;
108
import com.github.kostyasha.github.integration.branch.events.GitHubBranchEventDescriptor;
11-
12-
import static org.jenkinsci.plugins.github.pullrequest.utils.ObjectsUtil.isNull;
9+
import com.github.kostyasha.github.integration.branch.events.impl.commitchecks.GitHubBranchCommitCheck;
10+
import com.github.kostyasha.github.integration.branch.events.impl.commitchecks.GitHubBranchCommitCheckDescriptor;
1311

1412
import hudson.Extension;
1513
import hudson.model.TaskListener;
@@ -19,6 +17,7 @@
1917
import org.kohsuke.accmod.Restricted;
2018
import org.kohsuke.accmod.restrictions.NoExternalUse;
2119
import org.kohsuke.github.GHBranch;
20+
import org.kohsuke.github.GHCommit;
2221
import org.kohsuke.github.GHCompare;
2322
import org.kohsuke.stapler.DataBoundConstructor;
2423
import org.kohsuke.stapler.StaplerRequest;
@@ -33,14 +32,17 @@
3332
import java.util.ArrayList;
3433
import java.util.List;
3534
import java.util.Objects;
35+
import java.util.function.Function;
3636
import java.util.stream.Collectors;
3737

38+
import static org.jenkinsci.plugins.github.pullrequest.utils.ObjectsUtil.isNull;
39+
3840
/**
39-
* This branch event acts as a wrapper around checks that can be performed against commit data that requires
40-
* an additional round trip to GitHub to retrieve.
41+
* This branch event acts as a wrapper around checks that can be performed against commit data that requires an additional round trip to
42+
* GitHub to retrieve.
4143
* <p>
42-
* Commit data is retrieved and then passed to each implementing instance of <code>GitHubBranchCommitCheck</code>
43-
* to determine information about the commit should trigger a build.
44+
* Commit data is retrieved and then passed to each implementing instance of <code>GitHubBranchCommitCheck</code> to determine information
45+
* about the commit should trigger a build.
4446
* </p>
4547
*
4648
* @author Kanstantsin Shautsou
@@ -67,32 +69,20 @@ public GitHubBranchCommitEvent(List<GitHubBranchCommitCheck> checks) {
6769

6870
@Override
6971
public GitHubBranchCause check(GitHubBranchTrigger trigger, GHBranch remoteBranch, @CheckForNull GitHubBranch localBranch,
70-
GitHubBranchRepository localRepo, TaskListener listener)
71-
throws IOException {
72-
final PrintStream lloger = listener.getLogger();
72+
GitHubBranchRepository localRepo, TaskListener listener)
73+
throws IOException {
74+
final PrintStream logger = listener.getLogger();
75+
Function<GitHubBranchCommitCheck, GitHubBranchCause> function;
7376

7477
if (localBranch == null) {
75-
LOG.debug("First build of branch '%s' detected, skipping check.", remoteBranch.getName());
76-
lloger.println("First build of branch '" + remoteBranch.getName() + "' detected, skipping check.");
77-
return null;
78+
GHCommit commit = getLastCommit(remoteBranch);
79+
function = event -> event.check(remoteBranch, localRepo, commit);
80+
} else {
81+
GHCompare.Commit[] commits = getComparedCommits(localBranch, remoteBranch);
82+
function = event -> event.check(remoteBranch, localRepo, commits);
7883
}
7984

80-
GHCompare.Commit[] commits = getComparedCommits(localBranch, remoteBranch);
81-
List<GitHubBranchCause> causes = checks.stream()
82-
.map(event -> event.check(remoteBranch, localRepo, commits))
83-
.filter(Objects::nonNull)
84-
.collect(Collectors.toList());
85-
86-
String name = remoteBranch.getName();
87-
if (causes.isEmpty()) {
88-
LOG.debug("Commits for branch [{}] had no effect, not triggering run.", name);
89-
return null;
90-
}
91-
92-
GitHubBranchCause cause = causes.get(0);
93-
LOG.info("Building branch [{}] skipped due to commit check: {}", name, cause.getReason());
94-
95-
return cause;
85+
return check(remoteBranch, function, logger);
9686
}
9787

9888
// visible for testing to avoid complex mocking
@@ -106,6 +96,11 @@ GHCompare.Commit[] getComparedCommits(GitHubBranch localBranch, GHBranch remoteB
10696
.getCommits();
10797
}
10898

99+
// visible for testing to avoid complex mocking
100+
GHCommit getLastCommit(GHBranch remoteBranch) throws IOException {
101+
return remoteBranch.getOwner().getCommit(remoteBranch.getSHA1());
102+
}
103+
109104
@Nonnull
110105
public List<GitHubBranchCommitCheck> getChecks() {
111106
if (isNull(checks)) {
@@ -118,6 +113,26 @@ public void setChecks(List<GitHubBranchCommitCheck> checks) {
118113
this.checks = checks;
119114
}
120115

116+
private GitHubBranchCause check(GHBranch remoteBranch, Function<GitHubBranchCommitCheck, GitHubBranchCause> function,
117+
PrintStream logger) {
118+
List<GitHubBranchCause> causes = checks.stream()
119+
.map(function::apply)
120+
.filter(Objects::nonNull)
121+
.collect(Collectors.toList());
122+
123+
String name = remoteBranch.getName();
124+
if (causes.isEmpty()) {
125+
LOG.debug("Commits for branch [{}] had no effect, not triggering run.", name);
126+
return null;
127+
}
128+
129+
GitHubBranchCause cause = causes.get(0);
130+
LOG.info("Building branch [{}] skipped due to commit check: {}", name, cause.getReason());
131+
logger.printf("Building branch [%s] skipped due to commit check: %s", cause.getReason());
132+
133+
return cause;
134+
}
135+
121136
@Extension
122137
public static class DescriptorImpl extends GitHubBranchEventDescriptor {
123138
@Override

‎github-pullrequest-plugin/src/main/java/com/github/kostyasha/github/integration/branch/events/impl/commitchecks/GitHubBranchCommitCheck.java‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,27 @@
22

33
import com.github.kostyasha.github.integration.branch.GitHubBranchCause;
44
import com.github.kostyasha.github.integration.branch.GitHubBranchRepository;
5-
65
import com.github.kostyasha.github.integration.branch.events.impl.GitHubBranchCommitEvent;
6+
77
import hudson.ExtensionPoint;
88
import hudson.model.AbstractDescribableImpl;
99

1010
import org.kohsuke.github.GHBranch;
11+
import org.kohsuke.github.GHCommit;
1112
import org.kohsuke.github.GHCompare.Commit;
13+
import org.slf4j.Logger;
14+
import org.slf4j.LoggerFactory;
15+
16+
import java.io.IOException;
1217

1318
/**
1419
* @see GitHubBranchCommitEvent
1520
*/
1621
public abstract class GitHubBranchCommitCheck extends AbstractDescribableImpl<GitHubBranchCommitCheck>
1722
implements ExtensionPoint {
1823

24+
private static final Logger LOG = LoggerFactory.getLogger(GitHubBranchCommitCheck.class);
25+
1926
@Override
2027
public GitHubBranchCommitCheckDescriptor getDescriptor() {
2128
return (GitHubBranchCommitCheckDescriptor) super.getDescriptor();
@@ -30,4 +37,27 @@ public GitHubBranchCommitCheckDescriptor getDescriptor() {
3037
* @return <code>GitHubBranchCause</code> instance indicating if the build should be skipped, <code>null</code> otherwise.
3138
*/
3239
public abstract GitHubBranchCause check(GHBranch remoteBranch, GitHubBranchRepository localRepo, Commit[] commits);
40+
41+
/**
42+
* Check used to determine if some associated commit property, such as the commit message, should prevent a build from being triggered.
43+
* <p>
44+
* This method is called when the repository is not yet known to the plugin.
45+
* </p>
46+
*
47+
* @param remoteBranch current branch state from GH.
48+
* @param localRepo local repository state.
49+
* @param commit last commit in the remote repository.
50+
* @return <code>GitHubBranchCause</code> instance indicating if the build should be skipped, <code>null</code> otherwise.
51+
*/
52+
public GitHubBranchCause check(GHBranch remoteBranch, GitHubBranchRepository localRepo, GHCommit commit) {
53+
try {
54+
return doCheck(remoteBranch, localRepo, commit);
55+
} catch (IOException e) {
56+
LOG.error("Failed to check commit for hash [{}]", commit.getSHA1(), e);
57+
return new GitHubBranchCause(remoteBranch, localRepo, e.getMessage(), true);
58+
}
59+
}
60+
61+
protected abstract GitHubBranchCause doCheck(GHBranch remoteBranch, GitHubBranchRepository localRepo, GHCommit commit)
62+
throws IOException;
3363
}

‎github-pullrequest-plugin/src/main/java/com/github/kostyasha/github/integration/branch/events/impl/commitchecks/impl/GitHubBranchCommitMessageCheck.java‎

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@
44
import com.github.kostyasha.github.integration.branch.GitHubBranchRepository;
55
import com.github.kostyasha.github.integration.branch.events.impl.commitchecks.GitHubBranchCommitCheck;
66
import com.github.kostyasha.github.integration.branch.events.impl.commitchecks.GitHubBranchCommitCheckDescriptor;
7+
78
import hudson.Extension;
89
import hudson.ExtensionPoint;
10+
911
import org.kohsuke.github.GHBranch;
12+
import org.kohsuke.github.GHCommit;
1013
import org.kohsuke.github.GHCompare.Commit;
1114
import org.kohsuke.stapler.DataBoundConstructor;
1215
import org.kohsuke.stapler.DataBoundSetter;
@@ -15,9 +18,12 @@
1518

1619
import javax.annotation.CheckForNull;
1720

21+
import java.io.IOException;
22+
import java.util.Arrays;
1823
import java.util.Collections;
1924
import java.util.List;
2025
import java.util.Set;
26+
import java.util.function.Supplier;
2127
import java.util.regex.Pattern;
2228
import java.util.regex.PatternSyntaxException;
2329
import java.util.stream.Collectors;
@@ -39,23 +45,18 @@ public GitHubBranchCommitMessageCheck() {
3945
}
4046

4147
@Override
42-
public GitHubBranchCause check(GHBranch remoteBranch, GitHubBranchRepository localRepo, Commit[] commits) {
43-
String name = remoteBranch.getName();
44-
if (matchCriteria.isEmpty()) {
45-
LOG.warn("Commit message event added but no match criteria set, all commits are allowed.");
46-
return null;
47-
}
48-
49-
List<String> messages = Stream.of(commits)
50-
.map(commit -> commit.getCommit().getMessage())
51-
.collect(Collectors.toList());
52-
53-
if (commitsAreAllowed(messages)) {
54-
LOG.debug("Commit messages {} for branch [{}] allowed, commit ignored.", messages, name);
55-
return null;
56-
}
48+
public GitHubBranchCause doCheck(GHBranch remoteBranch, GitHubBranchRepository localRepo, GHCommit commit) throws IOException {
49+
List<String> messages = Arrays.asList(commit.getCommitShortInfo().getMessage());
50+
return check(remoteBranch, localRepo, () -> messages);
51+
}
5752

58-
return toCause(remoteBranch, localRepo, true, "Commit messages %s for branch [%s] not allowed by check.", messages, name);
53+
@Override
54+
public GitHubBranchCause check(GHBranch remoteBranch, GitHubBranchRepository localRepo, Commit[] commits) {
55+
return check(remoteBranch, localRepo, () -> {
56+
return Stream.of(commits)
57+
.map(commit -> commit.getCommit().getMessage())
58+
.collect(Collectors.toList());
59+
});
5960
}
6061

6162
public String getMatchCriteria() {
@@ -79,6 +80,22 @@ public void setMatchCriteria(String matchCriteria) {
7980
.collect(Collectors.toSet());
8081
}
8182

83+
private <T> GitHubBranchCause check(GHBranch remoteBranch, GitHubBranchRepository localRepo, Supplier<List<String>> supplier) {
84+
if (matchCriteria.isEmpty()) {
85+
LOG.warn("Commit message event added but no match criteria set, all commits are allowed.");
86+
return null;
87+
}
88+
89+
String name = remoteBranch.getName();
90+
List<String> messages = supplier.get();
91+
if (commitsAreAllowed(messages)) {
92+
LOG.debug("Commit messages {} for branch [{}] allowed, commit ignored.", messages, name);
93+
return null;
94+
}
95+
96+
return toCause(remoteBranch, localRepo, true, "Commit messages %s for branch [%s] not allowed by check.", messages, name);
97+
}
98+
8299
private boolean commitsAreAllowed(List<String> messages) {
83100
boolean allowed = false;
84101
for (String message : messages) {
@@ -117,7 +134,7 @@ private boolean matches(String message, String pattern) {
117134
}
118135

119136
private GitHubBranchCause toCause(GHBranch remoteBranch, GitHubBranchRepository localRepo, boolean skip, String message,
120-
Object... args) {
137+
Object... args) {
121138
return new GitHubBranchCause(remoteBranch, localRepo, String.format(message, args), skip);
122139
}
123140

‎github-pullrequest-plugin/src/test/java/com/github/kostyasha/github/integration/branch/events/impl/GitHubBranchCommitEventTest.java‎

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import org.junit.Before;
1010
import org.junit.Test;
1111
import org.kohsuke.github.GHBranch;
12+
import org.kohsuke.github.GHCommit;
1213
import org.kohsuke.github.GHCompare;
1314
import org.mockito.Mock;
1415
import org.mockito.MockitoAnnotations;
@@ -25,21 +26,24 @@
2526

2627
public class GitHubBranchCommitEventTest {
2728

29+
private GitHubBranchCommitEvent check;
30+
2831
private GHCompare.Commit[] commits;
2932

30-
private GitHubBranchCommitEvent check;
33+
@Mock
34+
private PrintStream logger;
3135

3236
@Mock
3337
private GitHubBranchCause mockCause;
3438

3539
@Mock
36-
private GitHubBranchCommitCheck mockCommitCheck;
40+
private GHCommit mockCommit;
3741

3842
@Mock
39-
private LoggingTaskListenerWrapper mockListener;
43+
private GitHubBranchCommitCheck mockCommitCheck;
4044

4145
@Mock
42-
private PrintStream lloger;
46+
private LoggingTaskListenerWrapper mockListener;
4347

4448
@Mock
4549
private GitHubBranch mockLocalBranch;
@@ -58,13 +62,19 @@ public class GitHubBranchCommitEventTest {
5862
@Before
5963
public void before() {
6064
MockitoAnnotations.initMocks(this);
65+
when(mockListener.getLogger()).thenReturn(logger);
6166

6267
commits = new GHCompare.Commit[0];
6368
check = new GitHubBranchCommitEvent(Arrays.asList(mockCommitCheck)) {
6469
@Override
6570
GHCompare.Commit[] getComparedCommits(GitHubBranch localBranch, GHBranch remoteBranch) throws IOException {
6671
return commits;
6772
}
73+
74+
@Override
75+
GHCommit getLastCommit(GHBranch remoteBranch) throws IOException {
76+
return mockCommit;
77+
}
6878
};
6979
}
7080

@@ -90,17 +100,38 @@ public void testCommitsNotConfigured() throws Exception {
90100
}
91101

92102
@Test
93-
public void testFirstCommit() throws Exception {
94-
when(mockListener.getLogger()).thenReturn(lloger);
95-
givenTheFirstCommit();
103+
public void testFirstSeenCommitDoesNotTriggerBuild() throws Exception {
104+
givenLocalRepositoryIsNull();
105+
givenCheckLastCommitReturnsCause();
106+
givenSkippableBranchCause();
107+
whenCheckCommits();
108+
thenCheckIsSkipped();
109+
}
110+
111+
@Test
112+
public void testFirstSeenCommitTriggersBuild() throws Exception {
113+
givenLocalRepositoryIsNull();
114+
givenCheckLastCommitReturnsNull();
96115
whenCheckCommits();
97116
thenNoCauseReturned();
98117
}
99118

119+
private void givenCheckLastCommitReturnsCause() {
120+
when(mockCommitCheck.check(mockRemoteBranch, mockRepo, mockCommit)).thenReturn(mockCause);
121+
}
122+
123+
private void givenCheckLastCommitReturnsNull() {
124+
when(mockCommitCheck.check(mockRemoteBranch, mockRepo, mockCommit)).thenReturn(null);
125+
}
126+
100127
private void givenChecksReturnNull() {
101128
when(mockCommitCheck.check(mockRemoteBranch, mockRepo, commits)).thenReturn(null);
102129
}
103130

131+
private void givenLocalRepositoryIsNull() throws Exception {
132+
mockLocalBranch = null;
133+
}
134+
104135
private void givenNoChecksAreConfigured() {
105136
check.setChecks(Collections.emptyList());
106137
}
@@ -110,10 +141,6 @@ private void givenSkippableBranchCause() {
110141
when(mockCommitCheck.check(mockRemoteBranch, mockRepo, commits)).thenReturn(mockCause);
111142
}
112143

113-
private void givenTheFirstCommit() throws Exception {
114-
mockLocalBranch = null;
115-
}
116-
117144
private void thenAdditionalTriggersWillBeChecked() {
118145
assertNull(result);
119146
}

0 commit comments

Comments
 (0)