From 8ccfdf13ef2b0be6e0920fc92f985f3e75ac85d7 Mon Sep 17 00:00:00 2001 From: Kezhi Xiong Date: Sun, 16 Aug 2020 23:20:33 +0800 Subject: [PATCH] Try resolve commit from run first instead of always fetching head commit from remote --- .../checks/github/GitHubChecksContext.java | 37 ++++++++++++++++--- .../checks/github/GitHubSCMFacade.java | 18 ++++++++- .../github/GitHubChecksContextTest.java | 35 ++++++++++++++++++ 3 files changed, 83 insertions(+), 7 deletions(-) diff --git a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksContext.java b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksContext.java index 96361b4f..fcb9c06e 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksContext.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksContext.java @@ -7,6 +7,7 @@ import jenkins.plugins.git.AbstractGitSCMSource; import jenkins.scm.api.SCMHead; import jenkins.scm.api.SCMRevision; +import org.apache.commons.lang3.StringUtils; import org.jenkinsci.plugins.displayurlapi.DisplayURLProvider; import org.jenkinsci.plugins.github_branch_source.GitHubAppCredentials; import org.jenkinsci.plugins.github_branch_source.GitHubSCMSource; @@ -75,7 +76,13 @@ class GitHubChecksContext { * @return the commit sha of the run or null */ public String getHeadSha() { - return resolveHeadSha(); + String commit = resolveCommit(); + if (StringUtils.isEmpty(commit)) { + return resolveHeadSha(); + } + else { + return commit; + } } /** @@ -150,14 +157,32 @@ private String resolveHeadSha() { getRepository(), head.get().getName())); } - if (revision.get() instanceof AbstractGitSCMSource.SCMRevisionImpl) { - return ((AbstractGitSCMSource.SCMRevisionImpl) revision.get()).getHash(); + return resolveCommit(revision.get()); + } + + private String resolveCommit() { + if (run == null) { + return StringUtils.EMPTY; + } + + Optional revision = scmFacade.findRevision(resolveSource(), run); + if (revision.isPresent()) { + return resolveCommit(revision.get()); + } + else { + return StringUtils.EMPTY; + } + } + + private String resolveCommit(final SCMRevision revision) { + if (revision instanceof AbstractGitSCMSource.SCMRevisionImpl) { + return ((AbstractGitSCMSource.SCMRevisionImpl) revision).getHash(); } - else if (revision.get() instanceof PullRequestSCMRevision) { - return ((PullRequestSCMRevision) revision.get()).getPullHash(); + else if (revision instanceof PullRequestSCMRevision) { + return ((PullRequestSCMRevision) revision).getPullHash(); } else { - throw new IllegalStateException("Unsupported revision type: " + revision.get().getClass().getName()); + throw new IllegalStateException("Unsupported revision type: " + revision.getClass().getName()); } } } diff --git a/src/main/java/io/jenkins/plugins/checks/github/GitHubSCMFacade.java b/src/main/java/io/jenkins/plugins/checks/github/GitHubSCMFacade.java index 39b0f2bc..74a70911 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubSCMFacade.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubSCMFacade.java @@ -3,9 +3,11 @@ import com.cloudbees.plugins.credentials.CredentialsMatchers; import com.cloudbees.plugins.credentials.CredentialsProvider; import hudson.model.Job; +import hudson.model.Run; import hudson.security.ACL; import jenkins.scm.api.SCMHead; import jenkins.scm.api.SCMRevision; +import jenkins.scm.api.SCMRevisionAction; import jenkins.scm.api.SCMSource; import org.jenkinsci.plugins.github_branch_source.GitHubAppCredentials; import org.jenkinsci.plugins.github_branch_source.GitHubSCMSource; @@ -61,7 +63,7 @@ public Optional findHead(final Job job) { } /** - * Fetch the current {@link SCMRevision} used by the {@code head} of the {@code source}. + * Fetch the current {@link SCMRevision} of the {@code source} and {@code head} remotely from GitHub. * * @param source * the GitHub repository @@ -78,4 +80,18 @@ public Optional findRevision(final GitHubSCMSource source, final SC source.getRepoOwner() + "/" + source.getRepository(), head.getName()), e); } } + + /** + * Find the current {@link SCMRevision} of the {@code source} and {@code run} locally through + * {@link jenkins.scm.api.SCMRevisionAction}. + * + * @param source + * the GitHub repository + * @param run + * the Jenkins run + * @return the found revision or empty + */ + public Optional findRevision(final GitHubSCMSource source, final Run run) { + return Optional.ofNullable(SCMRevisionAction.getRevision(source, run)); + } } diff --git a/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksContextTest.java b/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksContextTest.java index 20c91ae1..144675ce 100644 --- a/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksContextTest.java +++ b/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksContextTest.java @@ -19,6 +19,41 @@ import static org.mockito.Mockito.when; class GitHubChecksContextTest { + @Test + void shouldGetHeadShaWhenResolveRevisionFromRun() { + Job job = mock(Job.class); + Run run = mock(Run.class); + PullRequestSCMRevision revision = mock(PullRequestSCMRevision.class); + GitHubSCMSource source = mock(GitHubSCMSource.class); + GitHubSCMFacade scmFacade = mock(GitHubSCMFacade.class); + + when(run.getParent()).thenReturn(job); + when(scmFacade.findGitHubSCMSource(job)).thenReturn(Optional.of(source)); + when(scmFacade.findRevision(source, run)).thenReturn(Optional.of(revision)); + when(revision.getPullHash()).thenReturn("a1b2c3"); + + assertThat(new GitHubChecksContext(run, scmFacade, null) + .getHeadSha()) + .isEqualTo("a1b2c3"); + } + + @Test + void shouldGetHeadShaWhenResolveRevisionFromHead() { + Job job = mock(Job.class); + Run run = mock(Run.class); + SCMHead head = mock(SCMHead.class); + PullRequestSCMRevision revision = mock(PullRequestSCMRevision.class); + GitHubSCMSource source = mock(GitHubSCMSource.class); + GitHubSCMFacade scmFacade = createGitHubSCMFacadeWithRevision(job, source, head, revision); + + when(run.getParent()).thenReturn(job); + when(revision.getPullHash()).thenReturn("a1b2c3"); + + assertThat(new GitHubChecksContext(run, scmFacade, null) + .getHeadSha()) + .isEqualTo("a1b2c3"); + } + @Test void shouldGetHeadShaFromMasterBranch() { Job job = mock(Job.class);