From 09b483c94e80690d327eba859dce6f772e7a5080 Mon Sep 17 00:00:00 2001 From: Kezhi Xiong Date: Mon, 24 Aug 2020 00:03:23 +0800 Subject: [PATCH 01/11] Use isValid method for GitHubSCMSourceChecksContext --- .../checks/github/GitHubChecksContext.java | 44 +++--- .../github/GitHubChecksPublisherFactory.java | 29 ++-- .../github/GitHubSCMSourceChecksContext.java | 129 ++++++++++-------- .../checks/github/GitSCMChecksContext.java | 10 +- .../plugins/checks/github/SCMFacade.java | 36 +++++ .../GitHubChecksPublisherFactoryTest.java | 82 ++++++----- .../GitHubSCMSourceChecksContextTest.java | 83 ++++++----- 7 files changed, 252 insertions(+), 161 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 c1100295..48acd91d 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksContext.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksContext.java @@ -24,15 +24,6 @@ abstract class GitHubChecksContext { this.scmFacade = scmFacade; } - /** - * Returns the Jenkins job. - * - * @return job for which the checks will be based on - */ - public Job getJob() { - return job; - } - /** * Returns the commit sha of the run. * @@ -48,6 +39,18 @@ abstract class GitHubChecksContext { */ public abstract String getRepository(); + /** + * Returns whether the context is valid (with all properties functional) to use. + * + * @param logger + * the plugin logger + * @return whether the context is valid to use + */ + public abstract boolean isValid(PluginLogger logger); + + @Nullable + protected abstract String getCredentialsId(); + /** * Returns the credentials to access the remote GitHub repository. * @@ -55,16 +58,9 @@ abstract class GitHubChecksContext { */ public GitHubAppCredentials getCredentials() { String credentialsId = getCredentialsId(); - if (credentialsId == null) { - throw new IllegalStateException("No credentials available for job: " + getJob().getName()); - } - return getGitHubAppCredentials(credentialsId); } - @Nullable - protected abstract String getCredentialsId(); - /** * Returns the URL of the run's summary page, e.g. https://ci.jenkins.io/job/Core/job/jenkins/job/master/2000/. * @@ -74,7 +70,11 @@ public String getURL() { return url; } - SCMFacade getScmFacade() { + protected Job getJob() { + return job; + } + + protected SCMFacade getScmFacade() { return scmFacade; } @@ -87,12 +87,6 @@ protected GitHubAppCredentials getGitHubAppCredentials(final String credentialsI return foundCredentials.get(); } - Optional findGitHubAppCredentials(final String credentialsId) { - return getScmFacade().findGitHubAppCredentials(getJob(), credentialsId); - } - - abstract boolean isValid(PluginLogger listener); - protected boolean hasGitHubAppCredentials() { return findGitHubAppCredentials(getCredentialsId()).isPresent(); } @@ -117,4 +111,8 @@ protected boolean hasValidCredentials(final PluginLogger logger) { return true; } + + private Optional findGitHubAppCredentials(final String credentialsId) { + return getScmFacade().findGitHubAppCredentials(getJob(), credentialsId); + } } diff --git a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java index 0d1c88fb..ff0432b6 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java @@ -19,7 +19,7 @@ */ @Extension public class GitHubChecksPublisherFactory extends ChecksPublisherFactory { - private final SCMFacade scmFacade; + private SCMFacade scmFacade; /** * Creates a new instance of {@link GitHubChecksPublisherFactory}. @@ -40,39 +40,40 @@ protected Optional createPublisher(final Run run, final T return createPublisher(run, DisplayURLProvider.get().getRunURL(run), listener); } + @Override + protected Optional createPublisher(final Job job, final TaskListener listener) { + return createPublisher(job, DisplayURLProvider.get().getJobURL(job), listener); + } + @VisibleForTesting Optional createPublisher(final Run run, final String runURL, final TaskListener listener) { PluginLogger logger = createLogger(getListener(listener)); - - GitSCMChecksContext gitSCMContext = new GitSCMChecksContext(run, runURL); + + GitSCMChecksContext gitSCMContext = new GitSCMChecksContext(run, runURL, scmFacade); if (gitSCMContext.isValid(logger)) { return Optional.of(new GitHubChecksPublisher(gitSCMContext, getListener(listener))); } - return createPublisher(listener, logger, new GitHubSCMSourceChecksContext(run, runURL, scmFacade)); - } + GitHubSCMSourceChecksContext gitHubSCMSourceContext = new GitHubSCMSourceChecksContext(run, runURL, scmFacade); + if (gitHubSCMSourceContext.isValid(logger)) { + return Optional.of(new GitHubChecksPublisher(gitHubSCMSourceContext, getListener(listener))); + } - @Override - protected Optional createPublisher(final Job job, final TaskListener listener) { - return createPublisher(job, DisplayURLProvider.get().getJobURL(job), listener); + return Optional.empty(); } @VisibleForTesting Optional createPublisher(final Job job, final String jobURL, final TaskListener listener) { PluginLogger logger = createLogger(getListener(listener)); - return createPublisher(listener, logger, new GitHubSCMSourceChecksContext(job, jobURL, scmFacade)); - } - - private Optional createPublisher(final TaskListener listener, final PluginLogger logger, - final GitHubChecksContext gitHubSCMSourceContext) { + GitHubSCMSourceChecksContext gitHubSCMSourceContext = new GitHubSCMSourceChecksContext(job, jobURL, scmFacade); if (gitHubSCMSourceContext.isValid(logger)) { return Optional.of(new GitHubChecksPublisher(gitHubSCMSourceContext, getListener(listener))); } + return Optional.empty(); } - private TaskListener getListener(final TaskListener taskListener) { // FIXME: checks-API should use a Null listener if (taskListener == null) { diff --git a/src/main/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContext.java b/src/main/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContext.java index 74e836e3..a5c99a8e 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContext.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContext.java @@ -2,14 +2,11 @@ import java.util.Optional; -import org.jenkinsci.plugins.displayurlapi.DisplayURLProvider; +import org.apache.commons.lang3.StringUtils; import org.jenkinsci.plugins.github_branch_source.GitHubSCMSource; -import org.jenkinsci.plugins.github_branch_source.PullRequestSCMRevision; -import edu.hm.hafner.util.VisibleForTesting; import edu.umd.cs.findbugs.annotations.Nullable; import io.jenkins.plugins.util.PluginLogger; -import jenkins.plugins.git.AbstractGitSCMSource; import jenkins.scm.api.SCMHead; import jenkins.scm.api.SCMRevision; @@ -20,100 +17,118 @@ * Provides a {@link GitHubChecksContext} for a Jenkins job that uses a supported {@link GitHubSCMSource}. */ class GitHubSCMSourceChecksContext extends GitHubChecksContext { + private final boolean runAvailable; + /** * Creates a {@link GitHubSCMSourceChecksContext} according to the run. All attributes are computed during this period. * * @param run a run of a GitHub Branch Source project + * @param runURL the URL to the Jenkins run + * @param scmFacade a facade for Jenkins SCM */ - GitHubSCMSourceChecksContext(final Run run) { - this(run, DisplayURLProvider.get().getRunURL(run), new SCMFacade()); + GitHubSCMSourceChecksContext(final Run run, final String runURL, final SCMFacade scmFacade) { + super(run.getParent(), runURL, scmFacade); + runAvailable = true; } /** * Creates a {@link GitHubSCMSourceChecksContext} according to the job. All attributes are computed during this period. * * @param job a GitHub Branch Source project + * @param jobURL the URL to the Jenkins job + * @param scmFacade a facade for Jenkins SCM */ - GitHubSCMSourceChecksContext(final Job job) { - this(job, DisplayURLProvider.get().getJobURL(job), new SCMFacade()); - } - - @VisibleForTesting - GitHubSCMSourceChecksContext(final Run run, final String runURL, final SCMFacade scmFacade) { - super(run.getParent(), runURL, scmFacade); - } - - @VisibleForTesting GitHubSCMSourceChecksContext(final Job job, final String jobURL, final SCMFacade scmFacade) { super(job, jobURL, scmFacade); + runAvailable = false; } @Override public String getHeadSha() { - return resolveHeadSha(); + String headSha = resolveHeadSha(); + if (StringUtils.isBlank(headSha)) { + throw new IllegalStateException("No SHA found for job: " + getJob().getName()); + } + + return headSha; } @Override public String getRepository() { GitHubSCMSource source = resolveSource(); - return source.getRepoOwner() + "/" + source.getRepository(); + if (source == null) { + throw new IllegalStateException("No GitHub SCM source found for job: " + getJob().getName()); + } + else { + return source.getRepoOwner() + "/" + source.getRepository(); + } } - @Override @Nullable - protected String getCredentialsId() { - return resolveSource().getCredentialsId(); - } + @Override + public boolean isValid(final PluginLogger logger) { + if (resolveSource() == null) { + logger.log("No GitHub SCM source found"); - private GitHubSCMSource resolveSource() { - Optional source = getScmFacade().findGitHubSCMSource(getJob()); - if (!source.isPresent()) { - throw new IllegalStateException("No GitHub SCM source available for job: " + getJob().getName()); + return false; + } + + if (!hasValidCredentials(logger)) { + return false; } - return source.get(); + return StringUtils.isNotBlank(resolveHeadSha()); } - private String resolveHeadSha() { - Optional head = getScmFacade().findHead(getJob()); - if (!head.isPresent()) { - throw new IllegalStateException("No SCM head available for job: " + getJob().getName()); + @Override + @Nullable + protected String getCredentialsId() { + GitHubSCMSource source = resolveSource(); + if (source == null) { + return null; } - Optional revision = getScmFacade().findRevision(resolveSource(), head.get()); - if (!revision.isPresent()) { - throw new IllegalStateException( - String.format("No SCM revision available for repository: %s and head: %s", - getRepository(), head.get().getName())); - } + return source.getCredentialsId(); + } - if (revision.get() instanceof AbstractGitSCMSource.SCMRevisionImpl) { - return ((AbstractGitSCMSource.SCMRevisionImpl) revision.get()).getHash(); - } - else if (revision.get() instanceof PullRequestSCMRevision) { - return ((PullRequestSCMRevision) revision.get()).getPullHash(); + @Nullable + private GitHubSCMSource resolveSource() { + return getScmFacade().findGitHubSCMSource(getJob()).orElse(null); + } + + @Nullable + private String resolveHeadSha() { + if (runAvailable) { + return resolveHeadSha(getJob().getLastBuild()); } else { - throw new IllegalStateException("Unsupported revision type: " + revision.get().getClass().getName()); + return resolveHeadSha(getJob()); } } - @Override - boolean isValid(final PluginLogger logger) { - Job job = getJob(); - - Optional source = getScmFacade().findGitHubSCMSource(job); - if (!source.isPresent()) { - logger.log("No GitHub SCM source found"); - - return false; + @Nullable + private String resolveHeadSha(final Run run) { + GitHubSCMSource source = resolveSource(); + if (source != null) { + Optional revision = getScmFacade().findRevision(source, run); + if (revision.isPresent()) { + return getScmFacade().findHash(revision.get()).orElse(null); + } } - if (!hasValidCredentials(logger)) { - return false; + return null; + } + + @Nullable + private String resolveHeadSha(final Job job) { + GitHubSCMSource source = resolveSource(); + Optional head = getScmFacade().findHead(job); + if (source != null && head.isPresent()) { + Optional revision = getScmFacade().findRevision(source, head.get()); + if (revision.isPresent()) { + return getScmFacade().findHash(revision.get()).orElse(null); + } } - logger.log("Using GitHub SCM source '%s' for GitHub checks", getRepository()); - - return true; + return null; } } diff --git a/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java b/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java index ad22109d..42bd3731 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java @@ -3,6 +3,7 @@ import java.io.IOException; import java.util.Optional; +import edu.hm.hafner.util.VisibleForTesting; import org.apache.commons.lang3.StringUtils; import edu.umd.cs.findbugs.annotations.Nullable; @@ -31,7 +32,12 @@ class GitSCMChecksContext extends GitHubChecksContext { * @param runURL the URL to the Jenkins run */ GitSCMChecksContext(final Run run, final String runURL) { - super(run.getParent(), runURL, new SCMFacade()); + this(run, runURL, new SCMFacade()); + } + + @VisibleForTesting + GitSCMChecksContext(final Run run, final String runURL, final SCMFacade scmFacade) { + super(run.getParent(), runURL, scmFacade); this.run = run; } @@ -100,7 +106,7 @@ private GitSCM resolveGitSCM() { } @Override - boolean isValid(final PluginLogger logger) { + public boolean isValid(final PluginLogger logger) { if (!getScmFacade().findGitSCM(run).isPresent()) { logger.log("Job does not use GitSCM"); diff --git a/src/main/java/io/jenkins/plugins/checks/github/SCMFacade.java b/src/main/java/io/jenkins/plugins/checks/github/SCMFacade.java index 9bc09d32..05d3b905 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/SCMFacade.java +++ b/src/main/java/io/jenkins/plugins/checks/github/SCMFacade.java @@ -6,8 +6,11 @@ import java.util.List; import java.util.Optional; +import jenkins.plugins.git.AbstractGitSCMSource; +import jenkins.scm.api.SCMRevisionAction; import org.jenkinsci.plugins.github_branch_source.GitHubAppCredentials; import org.jenkinsci.plugins.github_branch_source.GitHubSCMSource; +import org.jenkinsci.plugins.github_branch_source.PullRequestSCMRevision; import org.jenkinsci.plugins.workflow.cps.CpsScmFlowDefinition; import org.jenkinsci.plugins.workflow.flow.FlowDefinition; import org.jenkinsci.plugins.workflow.job.WorkflowJob; @@ -136,6 +139,39 @@ public Optional findRevision(final SCMSource source, final SCMHead } } + /** + * 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)); + } + + /** + * Find the hash value in {@code revision}. + * + * @param revision + * the revision for a build + * @return the found hash or empty + */ + public Optional findHash(final SCMRevision revision) { + if (revision instanceof AbstractGitSCMSource.SCMRevisionImpl) { + return Optional.of(((AbstractGitSCMSource.SCMRevisionImpl) revision).getHash()); + } + else if (revision instanceof PullRequestSCMRevision) { + return Optional.of(((PullRequestSCMRevision) revision).getPullHash()); + } + else { + return Optional.empty(); + } + } + /** * Returns the SCM in a given build. If no SCM can be determined, then a {@link NullSCM} instance will be returned. * diff --git a/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactoryTest.java b/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactoryTest.java index 055234e2..7950d357 100644 --- a/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactoryTest.java +++ b/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactoryTest.java @@ -1,9 +1,18 @@ package io.jenkins.plugins.checks.github; +import java.io.IOException; import java.util.Optional; +import hudson.EnvVars; +import hudson.plugins.git.GitSCM; +import hudson.plugins.git.UserRemoteConfig; +import io.jenkins.plugins.util.PluginLogger; +import jenkins.scm.api.SCMHead; +import jenkins.scm.api.SCMRevision; import org.jenkinsci.plugins.github_branch_source.GitHubAppCredentials; import org.jenkinsci.plugins.github_branch_source.GitHubSCMSource; +import org.jenkinsci.plugins.github_branch_source.PullRequestSCMRevision; +import org.junit.Ignore; import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.*; @@ -18,79 +27,90 @@ class GitHubChecksPublisherFactoryTest { private static final String URL = "URL"; @Test - void shouldCreateGitHubChecksPublisherFromRun() { + void shouldCreateGitHubChecksPublisherFromRunForProjectWithValidGitHubSCMSource() { Run run = mock(Run.class); Job job = mock(Job.class); GitHubSCMSource source = mock(GitHubSCMSource.class); GitHubAppCredentials credentials = mock(GitHubAppCredentials.class); + PullRequestSCMRevision revision = mock(PullRequestSCMRevision.class); SCMFacade scmFacade = mock(SCMFacade.class); when(run.getParent()).thenReturn(job); + when(job.getLastBuild()).thenReturn(run); when(scmFacade.findGitHubSCMSource(job)).thenReturn(Optional.of(source)); when(source.getCredentialsId()).thenReturn("credentials id"); when(scmFacade.findGitHubAppCredentials(job, "credentials id")).thenReturn(Optional.of(credentials)); + when(scmFacade.findRevision(source, run)).thenReturn(Optional.of(revision)); + when(scmFacade.findHash(revision)).thenReturn(Optional.of("a1b2c3")); GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(scmFacade); assertThat(factory.createPublisher(run, URL, TaskListener.NULL)) - .isPresent() .containsInstanceOf(GitHubChecksPublisher.class); } @Test - void shouldReturnGitHubChecksPublisherFromJob() { - Job job = mock(Job.class); + void shouldReturnGitHubChecksPublisherFromJobProjectWithValidGitHubSCMSource() { + Run run = mock(Run.class); + Job job = mock(Job.class); GitHubSCMSource source = mock(GitHubSCMSource.class); + GitHubAppCredentials credentials = mock(GitHubAppCredentials.class); + PullRequestSCMRevision revision = mock(PullRequestSCMRevision.class); + SCMHead head = mock(SCMHead.class); SCMFacade scmFacade = mock(SCMFacade.class); + when(run.getParent()).thenReturn(job); + when(job.getLastBuild()).thenReturn(run); when(scmFacade.findGitHubSCMSource(job)).thenReturn(Optional.of(source)); when(source.getCredentialsId()).thenReturn("credentials id"); - when(scmFacade.findGitHubAppCredentials(job, "credentials id")) - .thenReturn(Optional.empty()); + when(scmFacade.findGitHubAppCredentials(job, "credentials id")).thenReturn(Optional.of(credentials)); + when(scmFacade.findHead(job)).thenReturn(Optional.of(head)); + when(scmFacade.findRevision(source, head)).thenReturn(Optional.of(revision)); + when(scmFacade.findHash(revision)).thenReturn(Optional.of("a1b2c3")); GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(scmFacade); assertThat(factory.createPublisher(job, URL, TaskListener.NULL)) - .isNotPresent(); + .containsInstanceOf(GitHubChecksPublisher.class); } @Test - void shouldReturnEmptyWhenNoGitHubSCMSourceIsConfigured() { + void shouldCreateGitHubChecksPublisherFromRunForProjectWithValidGitSCM() throws IOException, InterruptedException { + Job job = mock(Job.class); Run run = mock(Run.class); + GitSCM gitSCM = mock(GitSCM.class); + UserRemoteConfig config = mock(UserRemoteConfig.class); + GitHubAppCredentials credentials = mock(GitHubAppCredentials.class); + SCMFacade scmFacade = mock(SCMFacade.class); + EnvVars envVars = mock(EnvVars.class); - GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(); + when(run.getParent()).thenReturn(job); + when(run.getEnvironment(TaskListener.NULL)).thenReturn(envVars); + when(envVars.get("GIT_COMMIT")).thenReturn("a1b2c3"); + when(scmFacade.getScm(job)).thenReturn(gitSCM); + when(scmFacade.findGitSCM(run)).thenReturn(Optional.of(gitSCM)); + when(scmFacade.getUserRemoteConfig(gitSCM)).thenReturn(config); + when(config.getCredentialsId()).thenReturn("1"); + when(scmFacade.findGitHubAppCredentials(job, "1")).thenReturn(Optional.of(credentials)); + when(config.getUrl()).thenReturn("https://github.com/jenkinsci/github-checks-plugin"); + + GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(scmFacade); assertThat(factory.createPublisher(run, URL, TaskListener.NULL)) - .isNotPresent(); + .containsInstanceOf(GitHubChecksPublisher.class); } @Test - void shouldReturnEmptyWhenNoCredentialsIsConfigured() { + void shouldReturnEmptyFromRunForInvalidProject() { Run run = mock(Run.class); - Job job = mock(Job.class); - GitHubSCMSource source = mock(GitHubSCMSource.class); - SCMFacade scmFacade = mock(SCMFacade.class); - - when(run.getParent()).thenReturn(job); - when(scmFacade.findGitHubSCMSource(run.getParent())).thenReturn(Optional.of(source)); - when(source.getCredentialsId()).thenReturn(null); - GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(scmFacade); - assertThat(factory.createPublisher(job, URL, TaskListener.NULL)) + GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(); + assertThat(factory.createPublisher(run, URL, TaskListener.NULL)) .isNotPresent(); } @Test - void shouldReturnEmptyWhenNoGitHubAppCredentialsIsConfigured() { - Run run = mock(Run.class); + void shouldCreateNullPublisherFromJobForInvalidProject() { Job job = mock(Job.class); - GitHubSCMSource source = mock(GitHubSCMSource.class); - SCMFacade scmFacade = mock(SCMFacade.class); - - when(run.getParent()).thenReturn(job); - when(scmFacade.findGitHubSCMSource(run.getParent())).thenReturn(Optional.of(source)); - when(source.getCredentialsId()).thenReturn("credentials id"); - when(scmFacade.findGitHubAppCredentials(run.getParent(), "credentials id")) - .thenReturn(Optional.empty()); - GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(scmFacade); + GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(); assertThat(factory.createPublisher(job, URL, TaskListener.NULL)) .isNotPresent(); } diff --git a/src/test/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContextTest.java b/src/test/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContextTest.java index 7bc7f232..8a91d0ae 100644 --- a/src/test/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContextTest.java +++ b/src/test/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContextTest.java @@ -28,9 +28,8 @@ void shouldGetHeadShaFromMasterBranch() { AbstractGitSCMSource.SCMRevisionImpl revision = mock(AbstractGitSCMSource.SCMRevisionImpl.class); GitHubSCMSource source = mock(GitHubSCMSource.class); - when(revision.getHash()).thenReturn("a1b2c3"); - - assertThat(new GitHubSCMSourceChecksContext(job, URL, createGitHubSCMFacadeWithRevision(job, source, head, revision)) + assertThat(new GitHubSCMSourceChecksContext(job, URL, + createGitHubSCMFacadeWithRevision(job, source, head, revision, "a1b2c3")) .getHeadSha()) .isEqualTo("a1b2c3"); } @@ -42,9 +41,24 @@ void shouldGetHeadShaFromPullRequest() { PullRequestSCMRevision revision = mock(PullRequestSCMRevision.class); GitHubSCMSource source = mock(GitHubSCMSource.class); - when(revision.getPullHash()).thenReturn("a1b2c3"); + assertThat(new GitHubSCMSourceChecksContext(job, URL, + createGitHubSCMFacadeWithRevision(job, source, head, revision, "a1b2c3")) + .getHeadSha()) + .isEqualTo("a1b2c3"); + } + + @Test + void shouldGetHeadShaFromRun() { + Job job = mock(Job.class); + Run run = mock(Run.class); + PullRequestSCMRevision revision = mock(PullRequestSCMRevision.class); + GitHubSCMSource source = mock(GitHubSCMSource.class); - assertThat(new GitHubSCMSourceChecksContext(job, URL, createGitHubSCMFacadeWithRevision(job, source, head, revision)) + when(run.getParent()).thenReturn(job); + when(job.getLastBuild()).thenReturn(run); + + assertThat(new GitHubSCMSourceChecksContext(run, URL, + createGitHubSCMFacadeWithRevision(run, source, revision, "a1b2c3")) .getHeadSha()) .isEqualTo("a1b2c3"); } @@ -52,14 +66,14 @@ void shouldGetHeadShaFromPullRequest() { @Test void shouldThrowIllegalStateExceptionWhenGetHeadShaButNoSCMHeadAvailable() { Job job = mock(Job.class); + GitHubSCMSource source = mock(GitHubSCMSource.class); + when(job.getName()).thenReturn("github-checks-plugin"); - assertThatThrownBy(() -> { - SCMFacade scmFacade = mock(SCMFacade.class); - new GitHubSCMSourceChecksContext(job, URL, scmFacade).getHeadSha(); - }) + assertThatThrownBy(new GitHubSCMSourceChecksContext(job, URL, createGitHubSCMFacadeWithSource(job, source)) + ::getHeadSha) .isInstanceOf(IllegalStateException.class) - .hasMessage("No SCM head available for job: github-checks-plugin"); + .hasMessage("No SHA found for job: github-checks-plugin"); } @Test @@ -68,15 +82,15 @@ void shouldThrowIllegalStateExceptionWhenGetHeadShaButNoSCMRevisionAvailable() { SCMHead head = mock(SCMHead.class); GitHubSCMSource source = mock(GitHubSCMSource.class); + when(job.getName()).thenReturn("github-checks-plugin"); when(source.getRepoOwner()).thenReturn("jenkinsci"); when(source.getRepository()).thenReturn("github-checks-plugin"); when(head.getName()).thenReturn("master"); - assertThatThrownBy(() -> new GitHubSCMSourceChecksContext(job, URL, createGitHubSCMFacadeWithRevision(job, source, head, null)) - .getHeadSha()) + assertThatThrownBy(new GitHubSCMSourceChecksContext(job, URL, createGitHubSCMFacadeWithRevision(job, source, + head, null, null))::getHeadSha) .isInstanceOf(IllegalStateException.class) - .hasMessage("No SCM revision available for repository: jenkinsci/github-checks-plugin and " - + "head: master"); + .hasMessage("No SHA found for job: github-checks-plugin"); } @Test @@ -86,10 +100,12 @@ void shouldThrowIllegalStateExceptionWhenGetHeadShaButNoSuitableSCMRevisionAvail SCMRevision revision = mock(SCMRevision.class); GitHubSCMSource source = mock(GitHubSCMSource.class); - assertThatThrownBy(() -> new GitHubSCMSourceChecksContext(job, URL, createGitHubSCMFacadeWithRevision(job, source, head, revision)) - .getHeadSha()) + when(job.getName()).thenReturn("github-checks-plugin"); + + assertThatThrownBy(new GitHubSCMSourceChecksContext(job, URL, createGitHubSCMFacadeWithRevision(job, source, + head, revision, null))::getHeadSha) .isInstanceOf(IllegalStateException.class) - .hasMessage("Unsupported revision type: " + revision.getClass().getName()); + .hasMessage("No SHA found for job: github-checks-plugin"); } @Test @@ -112,7 +128,7 @@ void shouldThrowIllegalStateExceptionWhenGetRepositoryButNoGitHubSCMSourceAvaila assertThatThrownBy(() -> new GitHubSCMSourceChecksContext(job, URL, createGitHubSCMFacadeWithSource(job, null)) .getRepository()) .isInstanceOf(IllegalStateException.class) - .hasMessage("No GitHub SCM source available for job: github-checks-plugin"); + .hasMessage("No GitHub SCM source found for job: github-checks-plugin"); } @Test @@ -133,21 +149,8 @@ void shouldThrowIllegalStateExceptionWhenGetCredentialsButNoCredentialsAvailable when(job.getName()).thenReturn("github-checks-plugin"); - assertThatThrownBy(() -> new GitHubSCMSourceChecksContext(job, URL, createGitHubSCMFacadeWithCredentials(job, source, null, null)) - .getCredentials()) - .isInstanceOf(IllegalStateException.class) - .hasMessage("No credentials available for job: github-checks-plugin"); - } - - @Test - void shouldThrowIllegalStateExceptionWhenGetCredentialsButNoGitHubAPPCredentialsAvailable() { - Job job = mock(Job.class); - GitHubSCMSource source = mock(GitHubSCMSource.class); - - when(job.getName()).thenReturn("github-checks-plugin"); - - assertThatThrownBy(() -> new GitHubSCMSourceChecksContext(job, URL, createGitHubSCMFacadeWithCredentials(job, source, null, "1")) - .getCredentials()) + assertThatThrownBy(new GitHubSCMSourceChecksContext(job, URL, createGitHubSCMFacadeWithCredentials(job, source, + null, null))::getCredentials) .isInstanceOf(IllegalStateException.class) .hasMessage("No GitHub APP credentials available for job: github-checks-plugin"); } @@ -173,11 +176,23 @@ void shouldGetURLForRun() { } private SCMFacade createGitHubSCMFacadeWithRevision(final Job job, final GitHubSCMSource source, - final SCMHead head, final SCMRevision revision) { + final SCMHead head, final SCMRevision revision, + final String hash) { SCMFacade facade = createGitHubSCMFacadeWithSource(job, source); when(facade.findHead(job)).thenReturn(Optional.ofNullable(head)); when(facade.findRevision(source, head)).thenReturn(Optional.ofNullable(revision)); + when(facade.findHash(revision)).thenReturn(Optional.ofNullable(hash)); + + return facade; + } + + private SCMFacade createGitHubSCMFacadeWithRevision(final Run run, final GitHubSCMSource source, + final SCMRevision revision, final String hash) { + SCMFacade facade = createGitHubSCMFacadeWithSource(run.getParent(), source); + + when(facade.findRevision(source, run)).thenReturn(Optional.of(revision)); + when(facade.findHash(revision)).thenReturn(Optional.of(hash)); return facade; } From 68457d4e488ccc346e98da803031ee7e6a8b1422 Mon Sep 17 00:00:00 2001 From: Kezhi Xiong Date: Mon, 24 Aug 2020 02:16:37 +0800 Subject: [PATCH 02/11] Avoid log unnecessary information to console when successfully published checks. --- .../checks/github/GitHubChecksPublisher.java | 28 +++----- .../github/GitHubChecksPublisherFactory.java | 72 +++++++++++++------ .../GitHubChecksPublisherFactoryTest.java | 12 ++-- .../github/GitHubChecksPublisherITest.java | 13 ++-- 4 files changed, 76 insertions(+), 49 deletions(-) diff --git a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisher.java b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisher.java index b345dd27..7b052332 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisher.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisher.java @@ -6,6 +6,7 @@ import java.util.logging.Level; import java.util.logging.Logger; +import io.jenkins.plugins.util.PluginLogger; import org.apache.commons.lang3.StringUtils; import org.jenkinsci.plugins.github_branch_source.Connector; import org.jenkinsci.plugins.github_branch_source.GitHubAppCredentials; @@ -13,12 +14,9 @@ import org.kohsuke.github.GitHub; import edu.hm.hafner.util.VisibleForTesting; -import edu.umd.cs.findbugs.annotations.Nullable; import io.jenkins.plugins.checks.api.ChecksDetails; import io.jenkins.plugins.checks.api.ChecksPublisher; -import hudson.model.TaskListener; - /** * A publisher which publishes GitHub check runs. */ @@ -27,8 +25,7 @@ public class GitHubChecksPublisher extends ChecksPublisher { private static final Logger LOGGER = Logger.getLogger(GitHubChecksPublisher.class.getName()); private final GitHubChecksContext context; - @Nullable - private final TaskListener listener; + private final PluginLogger logger; private final String gitHubUrl; /** @@ -37,16 +34,15 @@ public class GitHubChecksPublisher extends ChecksPublisher { * @param context * a context which contains SCM properties */ - public GitHubChecksPublisher(final GitHubChecksContext context, @Nullable final TaskListener listener) { - this(context, listener, GITHUB_URL); + public GitHubChecksPublisher(final GitHubChecksContext context, final PluginLogger logger) { + this(context, logger, GITHUB_URL); } - GitHubChecksPublisher(final GitHubChecksContext context, @Nullable final TaskListener listener, - final String gitHubUrl) { + GitHubChecksPublisher(final GitHubChecksContext context, final PluginLogger logger, final String gitHubUrl) { super(); this.context = context; - this.listener = listener; + this.logger = logger; this.gitHubUrl = gitHubUrl; } @@ -65,17 +61,13 @@ public void publish(final ChecksDetails details) { GitHubChecksDetails gitHubDetails = new GitHubChecksDetails(details); createBuilder(gitHub, gitHubDetails).create(); - if (listener != null) { - listener.getLogger().printf("GitHub check (name: %s, status: %s) has been published.%n", - gitHubDetails.getName(), gitHubDetails.getStatus()); - } + logger.log("GitHub check (name: %s, status: %s) has been published.", gitHubDetails.getName(), + gitHubDetails.getStatus()); } - catch (IllegalStateException | IOException e) { + catch (IOException e) { String message = "Failed Publishing GitHub checks: "; LOGGER.log(Level.WARNING, (message + details).replaceAll("[\r\n]", ""), e); - if (listener != null) { - listener.getLogger().println(message + e); - } + logger.log(message + e); } } diff --git a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java index ff0432b6..68712016 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java @@ -1,6 +1,12 @@ package io.jenkins.plugins.checks.github; +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; +import java.io.UnsupportedEncodingException; +import java.nio.charset.StandardCharsets; import java.util.Optional; +import java.util.logging.Level; +import java.util.logging.Logger; import org.jenkinsci.plugins.displayurlapi.DisplayURLProvider; @@ -19,6 +25,8 @@ */ @Extension public class GitHubChecksPublisherFactory extends ChecksPublisherFactory { + private static final Logger LOGGER = Logger.getLogger(ChecksPublisherFactory.class.getName()); + private SCMFacade scmFacade; /** @@ -37,40 +45,64 @@ public GitHubChecksPublisherFactory() { @Override protected Optional createPublisher(final Run run, final TaskListener listener) { - return createPublisher(run, DisplayURLProvider.get().getRunURL(run), listener); + try { + return createPublisher(run, DisplayURLProvider.get().getRunURL(run), listener); + } + catch (UnsupportedEncodingException e) { + LOGGER.log(Level.WARNING, "Could not create logger.", e); + } + + return Optional.empty(); } @Override protected Optional createPublisher(final Job job, final TaskListener listener) { - return createPublisher(job, DisplayURLProvider.get().getJobURL(job), listener); + try { + return createPublisher(job, DisplayURLProvider.get().getJobURL(job), listener); + } + catch (UnsupportedEncodingException e) { + LOGGER.log(Level.WARNING, "Could not create logger.", e); + } + + return Optional.empty(); } @VisibleForTesting - Optional createPublisher(final Run run, final String runURL, final TaskListener listener) { - PluginLogger logger = createLogger(getListener(listener)); - - GitSCMChecksContext gitSCMContext = new GitSCMChecksContext(run, runURL, scmFacade); - if (gitSCMContext.isValid(logger)) { - return Optional.of(new GitHubChecksPublisher(gitSCMContext, getListener(listener))); - } - - GitHubSCMSourceChecksContext gitHubSCMSourceContext = new GitHubSCMSourceChecksContext(run, runURL, scmFacade); - if (gitHubSCMSourceContext.isValid(logger)) { - return Optional.of(new GitHubChecksPublisher(gitHubSCMSourceContext, getListener(listener))); + Optional createPublisher(final Run run, final String runURL, final TaskListener listener) + throws UnsupportedEncodingException { + ByteArrayOutputStream cause = new ByteArrayOutputStream(); + try (PrintStream ps = new PrintStream(cause, true, StandardCharsets.UTF_8.name())) { + PluginLogger causeLogger = new PluginLogger(ps, "GitHub Checks"); + + GitSCMChecksContext gitSCMContext = new GitSCMChecksContext(run, runURL, scmFacade); + if (gitSCMContext.isValid(causeLogger)) { + return Optional.of(new GitHubChecksPublisher(gitSCMContext, createConsoleLogger(getListener(listener)))); + } + + GitHubSCMSourceChecksContext gitHubSCMSourceContext = new GitHubSCMSourceChecksContext(run, runURL, scmFacade); + if (gitHubSCMSourceContext.isValid(causeLogger)) { + return Optional.of(new GitHubChecksPublisher(gitHubSCMSourceContext, createConsoleLogger(getListener(listener)))); + } } + listener.getLogger().print(cause.toString(StandardCharsets.UTF_8.name())); return Optional.empty(); } @VisibleForTesting - Optional createPublisher(final Job job, final String jobURL, final TaskListener listener) { - PluginLogger logger = createLogger(getListener(listener)); - - GitHubSCMSourceChecksContext gitHubSCMSourceContext = new GitHubSCMSourceChecksContext(job, jobURL, scmFacade); - if (gitHubSCMSourceContext.isValid(logger)) { - return Optional.of(new GitHubChecksPublisher(gitHubSCMSourceContext, getListener(listener))); + Optional createPublisher(final Job job, final String jobURL, final TaskListener listener) + throws UnsupportedEncodingException { + ByteArrayOutputStream cause = new ByteArrayOutputStream(); + try (PrintStream ps = new PrintStream(cause, true, StandardCharsets.UTF_8.name())) { + PluginLogger causeLogger = new PluginLogger(ps, "GitHub Checks"); + + GitHubSCMSourceChecksContext gitHubSCMSourceContext = new GitHubSCMSourceChecksContext(job, jobURL, scmFacade); + if (gitHubSCMSourceContext.isValid(causeLogger)) { + return Optional.of(new GitHubChecksPublisher(gitHubSCMSourceContext, createConsoleLogger(getListener(listener)))); + } } + listener.getLogger().print(cause.toString(StandardCharsets.UTF_8.name())); return Optional.empty(); } @@ -82,7 +114,7 @@ private TaskListener getListener(final TaskListener taskListener) { return taskListener; } - private PluginLogger createLogger(final TaskListener listener) { + private PluginLogger createConsoleLogger(final TaskListener listener) { return new PluginLogger(listener.getLogger(), "GitHub Checks"); } } diff --git a/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactoryTest.java b/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactoryTest.java index 7950d357..d52073ea 100644 --- a/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactoryTest.java +++ b/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactoryTest.java @@ -1,18 +1,16 @@ package io.jenkins.plugins.checks.github; import java.io.IOException; +import java.io.UnsupportedEncodingException; import java.util.Optional; import hudson.EnvVars; import hudson.plugins.git.GitSCM; import hudson.plugins.git.UserRemoteConfig; -import io.jenkins.plugins.util.PluginLogger; import jenkins.scm.api.SCMHead; -import jenkins.scm.api.SCMRevision; import org.jenkinsci.plugins.github_branch_source.GitHubAppCredentials; import org.jenkinsci.plugins.github_branch_source.GitHubSCMSource; import org.jenkinsci.plugins.github_branch_source.PullRequestSCMRevision; -import org.junit.Ignore; import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.*; @@ -27,7 +25,7 @@ class GitHubChecksPublisherFactoryTest { private static final String URL = "URL"; @Test - void shouldCreateGitHubChecksPublisherFromRunForProjectWithValidGitHubSCMSource() { + void shouldCreateGitHubChecksPublisherFromRunForProjectWithValidGitHubSCMSource() throws UnsupportedEncodingException { Run run = mock(Run.class); Job job = mock(Job.class); GitHubSCMSource source = mock(GitHubSCMSource.class); @@ -49,7 +47,7 @@ void shouldCreateGitHubChecksPublisherFromRunForProjectWithValidGitHubSCMSource( } @Test - void shouldReturnGitHubChecksPublisherFromJobProjectWithValidGitHubSCMSource() { + void shouldReturnGitHubChecksPublisherFromJobProjectWithValidGitHubSCMSource() throws UnsupportedEncodingException { Run run = mock(Run.class); Job job = mock(Job.class); GitHubSCMSource source = mock(GitHubSCMSource.class); @@ -98,7 +96,7 @@ void shouldCreateGitHubChecksPublisherFromRunForProjectWithValidGitSCM() throws } @Test - void shouldReturnEmptyFromRunForInvalidProject() { + void shouldReturnEmptyFromRunForInvalidProject() throws UnsupportedEncodingException { Run run = mock(Run.class); GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(); @@ -107,7 +105,7 @@ void shouldReturnEmptyFromRunForInvalidProject() { } @Test - void shouldCreateNullPublisherFromJobForInvalidProject() { + void shouldCreateNullPublisherFromJobForInvalidProject() throws UnsupportedEncodingException { Job job = mock(Job.class); GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(); diff --git a/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherITest.java b/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherITest.java index cc30248c..bf203dc9 100644 --- a/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherITest.java +++ b/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherITest.java @@ -7,6 +7,7 @@ import java.util.Optional; import java.util.logging.Level; +import io.jenkins.plugins.util.PluginLogger; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.Issue; @@ -109,7 +110,8 @@ public void shouldPublishGitHubCheckRunCorrectly() { new ChecksAction("re-run", "re-run Jenkins build", "#0"))) .build(); - new GitHubChecksPublisher(createGitHubChecksContextWithGitHubSCM(), jenkinsRule.createTaskListener(), + new GitHubChecksPublisher(createGitHubChecksContextWithGitHubSCM(), + new PluginLogger(jenkinsRule.createTaskListener().getLogger(), "GitHub Checks"), wireMockRule.baseUrl()) .publish(details); } @@ -142,7 +144,8 @@ public void shouldLogChecksParametersIfExceptionHappensWhenPublishChecks() { .build()) .build(); - new GitHubChecksPublisher(createGitHubChecksContextWithGitHubSCM(), jenkinsRule.createTaskListener(), + new GitHubChecksPublisher(createGitHubChecksContextWithGitHubSCM(), + new PluginLogger(jenkinsRule.createTaskListener().getLogger(), "GitHub Checks"), wireMockRule.baseUrl()) .publish(details); @@ -174,16 +177,18 @@ private GitHubChecksContext createGitHubChecksContextWithGitHubSCM() { ClassicDisplayURLProvider urlProvider = mock(ClassicDisplayURLProvider.class); when(run.getParent()).thenReturn(job); + when(job.getLastBuild()).thenReturn(run); + when(source.getCredentialsId()).thenReturn("1"); when(source.getRepoOwner()).thenReturn("XiongKezhi"); when(source.getRepository()).thenReturn("Sandbox"); - when(revision.getPullHash()).thenReturn("18c8e2fd86e7aa3748e279c14a00dc3f0b963e7f"); when(credentials.getPassword()).thenReturn(Secret.fromString("password")); when(scmFacade.findGitHubSCMSource(job)).thenReturn(Optional.of(source)); when(scmFacade.findGitHubAppCredentials(job, "1")).thenReturn(Optional.of(credentials)); when(scmFacade.findHead(job)).thenReturn(Optional.of(head)); - when(scmFacade.findRevision(source, head)).thenReturn(Optional.of(revision)); + when(scmFacade.findRevision(source, run)).thenReturn(Optional.of(revision)); + when(scmFacade.findHash(revision)).thenReturn(Optional.of("18c8e2fd86e7aa3748e279c14a00dc3f0b963e7f")); when(urlProvider.getRunURL(run)).thenReturn("https://ci.jenkins.io"); From fedd2ba45c7a5eefa434734537d7d469566f3696 Mon Sep 17 00:00:00 2001 From: Kezhi Xiong Date: Mon, 24 Aug 2020 16:07:49 +0800 Subject: [PATCH 03/11] Close stream resources --- .../github/GitHubChecksPublisherFactory.java | 26 ++++++++++--------- .../GitHubChecksPublisherFactoryTest.java | 9 +++---- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java index 68712016..fc898c90 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java @@ -1,8 +1,8 @@ package io.jenkins.plugins.checks.github; import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.io.PrintStream; -import java.io.UnsupportedEncodingException; import java.nio.charset.StandardCharsets; import java.util.Optional; import java.util.logging.Level; @@ -27,7 +27,7 @@ public class GitHubChecksPublisherFactory extends ChecksPublisherFactory { private static final Logger LOGGER = Logger.getLogger(ChecksPublisherFactory.class.getName()); - private SCMFacade scmFacade; + private final SCMFacade scmFacade; /** * Creates a new instance of {@link GitHubChecksPublisherFactory}. @@ -48,7 +48,7 @@ protected Optional createPublisher(final Run run, final T try { return createPublisher(run, DisplayURLProvider.get().getRunURL(run), listener); } - catch (UnsupportedEncodingException e) { + catch (IOException e) { LOGGER.log(Level.WARNING, "Could not create logger.", e); } @@ -60,7 +60,7 @@ protected Optional createPublisher(final Job job, final T try { return createPublisher(job, DisplayURLProvider.get().getJobURL(job), listener); } - catch (UnsupportedEncodingException e) { + catch (IOException e) { LOGGER.log(Level.WARNING, "Could not create logger.", e); } @@ -69,9 +69,9 @@ protected Optional createPublisher(final Job job, final T @VisibleForTesting Optional createPublisher(final Run run, final String runURL, final TaskListener listener) - throws UnsupportedEncodingException { - ByteArrayOutputStream cause = new ByteArrayOutputStream(); - try (PrintStream ps = new PrintStream(cause, true, StandardCharsets.UTF_8.name())) { + throws IOException { + try (ByteArrayOutputStream cause = new ByteArrayOutputStream(); + PrintStream ps = new PrintStream(cause, true, StandardCharsets.UTF_8.name())) { PluginLogger causeLogger = new PluginLogger(ps, "GitHub Checks"); GitSCMChecksContext gitSCMContext = new GitSCMChecksContext(run, runURL, scmFacade); @@ -83,26 +83,28 @@ Optional createPublisher(final Run run, final String runU if (gitHubSCMSourceContext.isValid(causeLogger)) { return Optional.of(new GitHubChecksPublisher(gitHubSCMSourceContext, createConsoleLogger(getListener(listener)))); } + + listener.getLogger().print(cause.toString(StandardCharsets.UTF_8.name())); } - listener.getLogger().print(cause.toString(StandardCharsets.UTF_8.name())); return Optional.empty(); } @VisibleForTesting Optional createPublisher(final Job job, final String jobURL, final TaskListener listener) - throws UnsupportedEncodingException { - ByteArrayOutputStream cause = new ByteArrayOutputStream(); - try (PrintStream ps = new PrintStream(cause, true, StandardCharsets.UTF_8.name())) { + throws IOException { + try (ByteArrayOutputStream cause = new ByteArrayOutputStream(); + PrintStream ps = new PrintStream(cause, true, StandardCharsets.UTF_8.name())) { PluginLogger causeLogger = new PluginLogger(ps, "GitHub Checks"); GitHubSCMSourceChecksContext gitHubSCMSourceContext = new GitHubSCMSourceChecksContext(job, jobURL, scmFacade); if (gitHubSCMSourceContext.isValid(causeLogger)) { return Optional.of(new GitHubChecksPublisher(gitHubSCMSourceContext, createConsoleLogger(getListener(listener)))); } + + listener.getLogger().print(cause.toString(StandardCharsets.UTF_8.name())); } - listener.getLogger().print(cause.toString(StandardCharsets.UTF_8.name())); return Optional.empty(); } diff --git a/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactoryTest.java b/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactoryTest.java index d52073ea..fbd2cc3a 100644 --- a/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactoryTest.java +++ b/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactoryTest.java @@ -1,7 +1,6 @@ package io.jenkins.plugins.checks.github; import java.io.IOException; -import java.io.UnsupportedEncodingException; import java.util.Optional; import hudson.EnvVars; @@ -25,7 +24,7 @@ class GitHubChecksPublisherFactoryTest { private static final String URL = "URL"; @Test - void shouldCreateGitHubChecksPublisherFromRunForProjectWithValidGitHubSCMSource() throws UnsupportedEncodingException { + void shouldCreateGitHubChecksPublisherFromRunForProjectWithValidGitHubSCMSource() throws IOException { Run run = mock(Run.class); Job job = mock(Job.class); GitHubSCMSource source = mock(GitHubSCMSource.class); @@ -47,7 +46,7 @@ void shouldCreateGitHubChecksPublisherFromRunForProjectWithValidGitHubSCMSource( } @Test - void shouldReturnGitHubChecksPublisherFromJobProjectWithValidGitHubSCMSource() throws UnsupportedEncodingException { + void shouldReturnGitHubChecksPublisherFromJobProjectWithValidGitHubSCMSource() throws IOException { Run run = mock(Run.class); Job job = mock(Job.class); GitHubSCMSource source = mock(GitHubSCMSource.class); @@ -96,7 +95,7 @@ void shouldCreateGitHubChecksPublisherFromRunForProjectWithValidGitSCM() throws } @Test - void shouldReturnEmptyFromRunForInvalidProject() throws UnsupportedEncodingException { + void shouldReturnEmptyFromRunForInvalidProject() throws IOException { Run run = mock(Run.class); GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(); @@ -105,7 +104,7 @@ void shouldReturnEmptyFromRunForInvalidProject() throws UnsupportedEncodingExcep } @Test - void shouldCreateNullPublisherFromJobForInvalidProject() throws UnsupportedEncodingException { + void shouldCreateNullPublisherFromJobForInvalidProject() throws IOException { Job job = mock(Job.class); GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(); From 35b6ab8bc50d7f7fe3d68dac0d0d1814aa67361b Mon Sep 17 00:00:00 2001 From: Kezhi Xiong Date: Mon, 24 Aug 2020 21:22:15 +0800 Subject: [PATCH 04/11] Assign sha in context's constructor --- .../github/GitHubSCMSourceChecksContext.java | 24 ++++++------------- .../GitHubSCMSourceChecksContextTest.java | 6 +++-- 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/src/main/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContext.java b/src/main/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContext.java index a5c99a8e..5eeb0600 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContext.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContext.java @@ -17,7 +17,8 @@ * Provides a {@link GitHubChecksContext} for a Jenkins job that uses a supported {@link GitHubSCMSource}. */ class GitHubSCMSourceChecksContext extends GitHubChecksContext { - private final boolean runAvailable; + @Nullable + private final String sha; /** * Creates a {@link GitHubSCMSourceChecksContext} according to the run. All attributes are computed during this period. @@ -28,7 +29,7 @@ class GitHubSCMSourceChecksContext extends GitHubChecksContext { */ GitHubSCMSourceChecksContext(final Run run, final String runURL, final SCMFacade scmFacade) { super(run.getParent(), runURL, scmFacade); - runAvailable = true; + sha = resolveHeadSha(run); } /** @@ -40,17 +41,16 @@ class GitHubSCMSourceChecksContext extends GitHubChecksContext { */ GitHubSCMSourceChecksContext(final Job job, final String jobURL, final SCMFacade scmFacade) { super(job, jobURL, scmFacade); - runAvailable = false; + sha = resolveHeadSha(job); } @Override public String getHeadSha() { - String headSha = resolveHeadSha(); - if (StringUtils.isBlank(headSha)) { + if (StringUtils.isBlank(sha)) { throw new IllegalStateException("No SHA found for job: " + getJob().getName()); } - return headSha; + return sha; } @Override @@ -76,7 +76,7 @@ public boolean isValid(final PluginLogger logger) { return false; } - return StringUtils.isNotBlank(resolveHeadSha()); + return StringUtils.isNotBlank(sha); } @Override @@ -95,16 +95,6 @@ private GitHubSCMSource resolveSource() { return getScmFacade().findGitHubSCMSource(getJob()).orElse(null); } - @Nullable - private String resolveHeadSha() { - if (runAvailable) { - return resolveHeadSha(getJob().getLastBuild()); - } - else { - return resolveHeadSha(getJob()); - } - } - @Nullable private String resolveHeadSha(final Run run) { GitHubSCMSource source = resolveSource(); diff --git a/src/test/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContextTest.java b/src/test/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContextTest.java index 8a91d0ae..db603b73 100644 --- a/src/test/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContextTest.java +++ b/src/test/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContextTest.java @@ -159,19 +159,21 @@ void shouldThrowIllegalStateExceptionWhenGetCredentialsButNoCredentialsAvailable void shouldGetURLForJob() { Job job = mock(Job.class); - assertThat(new GitHubSCMSourceChecksContext(job, URL, null).getURL()) + assertThat(new GitHubSCMSourceChecksContext(job, URL, createGitHubSCMFacadeWithSource(job, null)).getURL()) .isEqualTo(URL); } @Test void shouldGetURLForRun() { Run run = mock(Run.class); + Job job = mock(Job.class); ClassicDisplayURLProvider urlProvider = mock(ClassicDisplayURLProvider.class); when(urlProvider.getRunURL(run)) .thenReturn("http://127.0.0.1:8080/job/github-checks-plugin/job/master/200"); - assertThat(new GitHubSCMSourceChecksContext(run, urlProvider.getRunURL(run), null).getURL()) + assertThat(new GitHubSCMSourceChecksContext(run, urlProvider.getRunURL(run), + createGitHubSCMFacadeWithSource(job, null)).getURL()) .isEqualTo("http://127.0.0.1:8080/job/github-checks-plugin/job/master/200"); } From 874bcbdc9b882e65681504e5402a1927c02a6762 Mon Sep 17 00:00:00 2001 From: Kezhi Xiong Date: Mon, 24 Aug 2020 23:35:49 +0800 Subject: [PATCH 05/11] Refactor to avoid duplicate code --- .../github/GitHubChecksPublisherFactory.java | 56 ++++++------------- .../checks/github/GitSCMChecksContext.java | 1 - .../GitHubChecksPublisherFactoryTest.java | 52 ++++++++++------- 3 files changed, 48 insertions(+), 61 deletions(-) diff --git a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java index fc898c90..c05eb3c9 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java @@ -5,8 +5,6 @@ import java.io.PrintStream; import java.nio.charset.StandardCharsets; import java.util.Optional; -import java.util.logging.Level; -import java.util.logging.Logger; import org.jenkinsci.plugins.displayurlapi.DisplayURLProvider; @@ -25,31 +23,33 @@ */ @Extension public class GitHubChecksPublisherFactory extends ChecksPublisherFactory { - private static final Logger LOGGER = Logger.getLogger(ChecksPublisherFactory.class.getName()); - private final SCMFacade scmFacade; + private final DisplayURLProvider urlProvider; /** * Creates a new instance of {@link GitHubChecksPublisherFactory}. */ public GitHubChecksPublisherFactory() { - this(new SCMFacade()); + this(new SCMFacade(), DisplayURLProvider.get()); } @VisibleForTesting - GitHubChecksPublisherFactory(final SCMFacade scmFacade) { + GitHubChecksPublisherFactory(final SCMFacade scmFacade, final DisplayURLProvider urlProvider) { super(); this.scmFacade = scmFacade; + this.urlProvider = urlProvider; } @Override protected Optional createPublisher(final Run run, final TaskListener listener) { try { - return createPublisher(run, DisplayURLProvider.get().getRunURL(run), listener); + final String runURL = urlProvider.getRunURL(run); + return createPublisher(listener, new GitSCMChecksContext(run, runURL, scmFacade), + new GitHubSCMSourceChecksContext(run, runURL, scmFacade)); } catch (IOException e) { - LOGGER.log(Level.WARNING, "Could not create logger.", e); + createConsoleLogger(listener).log("Could not create publisher.", e); } return Optional.empty(); @@ -58,48 +58,26 @@ protected Optional createPublisher(final Run run, final T @Override protected Optional createPublisher(final Job job, final TaskListener listener) { try { - return createPublisher(job, DisplayURLProvider.get().getJobURL(job), listener); + return createPublisher(listener, new GitHubSCMSourceChecksContext(job, urlProvider.getJobURL(job), + scmFacade)); } catch (IOException e) { - LOGGER.log(Level.WARNING, "Could not create logger.", e); + createConsoleLogger(listener).log("Could not create publisher.", e); } return Optional.empty(); } - @VisibleForTesting - Optional createPublisher(final Run run, final String runURL, final TaskListener listener) - throws IOException { - try (ByteArrayOutputStream cause = new ByteArrayOutputStream(); - PrintStream ps = new PrintStream(cause, true, StandardCharsets.UTF_8.name())) { - PluginLogger causeLogger = new PluginLogger(ps, "GitHub Checks"); - - GitSCMChecksContext gitSCMContext = new GitSCMChecksContext(run, runURL, scmFacade); - if (gitSCMContext.isValid(causeLogger)) { - return Optional.of(new GitHubChecksPublisher(gitSCMContext, createConsoleLogger(getListener(listener)))); - } - - GitHubSCMSourceChecksContext gitHubSCMSourceContext = new GitHubSCMSourceChecksContext(run, runURL, scmFacade); - if (gitHubSCMSourceContext.isValid(causeLogger)) { - return Optional.of(new GitHubChecksPublisher(gitHubSCMSourceContext, createConsoleLogger(getListener(listener)))); - } - - listener.getLogger().print(cause.toString(StandardCharsets.UTF_8.name())); - } - - return Optional.empty(); - } - - @VisibleForTesting - Optional createPublisher(final Job job, final String jobURL, final TaskListener listener) + private Optional createPublisher(final TaskListener listener, final GitHubChecksContext... contexts) throws IOException { try (ByteArrayOutputStream cause = new ByteArrayOutputStream(); - PrintStream ps = new PrintStream(cause, true, StandardCharsets.UTF_8.name())) { + PrintStream ps = new PrintStream(cause, true, StandardCharsets.UTF_8.name())) { PluginLogger causeLogger = new PluginLogger(ps, "GitHub Checks"); - GitHubSCMSourceChecksContext gitHubSCMSourceContext = new GitHubSCMSourceChecksContext(job, jobURL, scmFacade); - if (gitHubSCMSourceContext.isValid(causeLogger)) { - return Optional.of(new GitHubChecksPublisher(gitHubSCMSourceContext, createConsoleLogger(getListener(listener)))); + for (GitHubChecksContext ctx : contexts) { + if (ctx.isValid(causeLogger)) { + return Optional.of(new GitHubChecksPublisher(ctx, createConsoleLogger(getListener(listener)))); + } } listener.getLogger().print(cause.toString(StandardCharsets.UTF_8.name())); diff --git a/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java b/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java index 42bd3731..95edff08 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java @@ -35,7 +35,6 @@ class GitSCMChecksContext extends GitHubChecksContext { this(run, runURL, new SCMFacade()); } - @VisibleForTesting GitSCMChecksContext(final Run run, final String runURL, final SCMFacade scmFacade) { super(run.getParent(), runURL, scmFacade); diff --git a/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactoryTest.java b/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactoryTest.java index fbd2cc3a..2363eb14 100644 --- a/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactoryTest.java +++ b/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactoryTest.java @@ -7,6 +7,7 @@ import hudson.plugins.git.GitSCM; import hudson.plugins.git.UserRemoteConfig; import jenkins.scm.api.SCMHead; +import org.jenkinsci.plugins.displayurlapi.DisplayURLProvider; import org.jenkinsci.plugins.github_branch_source.GitHubAppCredentials; import org.jenkinsci.plugins.github_branch_source.GitHubSCMSource; import org.jenkinsci.plugins.github_branch_source.PullRequestSCMRevision; @@ -19,12 +20,9 @@ import hudson.model.Run; import hudson.model.TaskListener; -@SuppressWarnings({"PMD.CloseResource", "rawtypes"})// no need to close mocked PrintStream class GitHubChecksPublisherFactoryTest { - private static final String URL = "URL"; - @Test - void shouldCreateGitHubChecksPublisherFromRunForProjectWithValidGitHubSCMSource() throws IOException { + void shouldCreateGitHubChecksPublisherFromRunForProjectWithValidGitHubSCMSource() { Run run = mock(Run.class); Job job = mock(Job.class); GitHubSCMSource source = mock(GitHubSCMSource.class); @@ -40,13 +38,13 @@ void shouldCreateGitHubChecksPublisherFromRunForProjectWithValidGitHubSCMSource( when(scmFacade.findRevision(source, run)).thenReturn(Optional.of(revision)); when(scmFacade.findHash(revision)).thenReturn(Optional.of("a1b2c3")); - GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(scmFacade); - assertThat(factory.createPublisher(run, URL, TaskListener.NULL)) - .containsInstanceOf(GitHubChecksPublisher.class); + GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(scmFacade, createDisplayURLProvider(run, + job)); + assertThat(factory.createPublisher(run, TaskListener.NULL)).containsInstanceOf(GitHubChecksPublisher.class); } @Test - void shouldReturnGitHubChecksPublisherFromJobProjectWithValidGitHubSCMSource() throws IOException { + void shouldReturnGitHubChecksPublisherFromJobProjectWithValidGitHubSCMSource() { Run run = mock(Run.class); Job job = mock(Job.class); GitHubSCMSource source = mock(GitHubSCMSource.class); @@ -64,9 +62,9 @@ void shouldReturnGitHubChecksPublisherFromJobProjectWithValidGitHubSCMSource() t when(scmFacade.findRevision(source, head)).thenReturn(Optional.of(revision)); when(scmFacade.findHash(revision)).thenReturn(Optional.of("a1b2c3")); - GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(scmFacade); - assertThat(factory.createPublisher(job, URL, TaskListener.NULL)) - .containsInstanceOf(GitHubChecksPublisher.class); + GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(scmFacade, createDisplayURLProvider(run, + job)); + assertThat(factory.createPublisher(job, TaskListener.NULL)).containsInstanceOf(GitHubChecksPublisher.class); } @Test @@ -89,26 +87,38 @@ void shouldCreateGitHubChecksPublisherFromRunForProjectWithValidGitSCM() throws when(scmFacade.findGitHubAppCredentials(job, "1")).thenReturn(Optional.of(credentials)); when(config.getUrl()).thenReturn("https://github.com/jenkinsci/github-checks-plugin"); - GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(scmFacade); - assertThat(factory.createPublisher(run, URL, TaskListener.NULL)) - .containsInstanceOf(GitHubChecksPublisher.class); + GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(scmFacade, createDisplayURLProvider(run, + job)); + assertThat(factory.createPublisher(run, TaskListener.NULL)).containsInstanceOf(GitHubChecksPublisher.class); } @Test - void shouldReturnEmptyFromRunForInvalidProject() throws IOException { + void shouldReturnEmptyFromRunForInvalidProject() { Run run = mock(Run.class); + SCMFacade facade = mock(SCMFacade.class); + DisplayURLProvider urlProvider = mock(DisplayURLProvider.class); - GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(); - assertThat(factory.createPublisher(run, URL, TaskListener.NULL)) - .isNotPresent(); + GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(facade, urlProvider); + assertThat(factory.createPublisher(run, TaskListener.NULL)).isNotPresent(); } @Test - void shouldCreateNullPublisherFromJobForInvalidProject() throws IOException { + void shouldCreateNullPublisherFromJobForInvalidProject() { Job job = mock(Job.class); + SCMFacade facade = mock(SCMFacade.class); + DisplayURLProvider urlProvider = mock(DisplayURLProvider.class); - GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(); - assertThat(factory.createPublisher(job, URL, TaskListener.NULL)) + GitHubChecksPublisherFactory factory = new GitHubChecksPublisherFactory(facade, urlProvider); + assertThat(factory.createPublisher(job, TaskListener.NULL)) .isNotPresent(); } + + private DisplayURLProvider createDisplayURLProvider(final Run run, final Job job) { + DisplayURLProvider urlProvider = mock(DisplayURLProvider.class); + + when(urlProvider.getRunURL(run)).thenReturn(null); + when(urlProvider.getJobURL(job)).thenReturn(null); + + return urlProvider; + } } From 8b440f0c6f3a155e406b2e6df5543c68e72668b9 Mon Sep 17 00:00:00 2001 From: Kezhi Xiong Date: Tue, 25 Aug 2020 01:01:31 +0800 Subject: [PATCH 06/11] Use filtered log --- .../checks/github/GitHubChecksContext.java | 14 ++--- .../github/GitHubChecksPublisherFactory.java | 60 ++++++------------- .../github/GitHubSCMSourceChecksContext.java | 16 +++-- .../checks/github/GitSCMChecksContext.java | 15 ++--- 4 files changed, 45 insertions(+), 60 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 48acd91d..47646d6a 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksContext.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksContext.java @@ -2,11 +2,11 @@ import java.util.Optional; +import edu.hm.hafner.util.FilteredLog; import org.apache.commons.lang3.StringUtils; import org.jenkinsci.plugins.github_branch_source.GitHubAppCredentials; import edu.umd.cs.findbugs.annotations.Nullable; -import io.jenkins.plugins.util.PluginLogger; import hudson.model.Job; @@ -43,10 +43,10 @@ abstract class GitHubChecksContext { * Returns whether the context is valid (with all properties functional) to use. * * @param logger - * the plugin logger + * the filtered logger * @return whether the context is valid to use */ - public abstract boolean isValid(PluginLogger logger); + public abstract boolean isValid(FilteredLog logger); @Nullable protected abstract String getCredentialsId(); @@ -95,16 +95,16 @@ protected boolean hasCredentialsId() { return StringUtils.isNoneBlank(getCredentialsId()); } - protected boolean hasValidCredentials(final PluginLogger logger) { + protected boolean hasValidCredentials(final FilteredLog logger) { if (!hasCredentialsId()) { - logger.log("No credentials found"); + logger.logError("No credentials found"); return false; } if (!hasGitHubAppCredentials()) { - logger.log("No GitHub app credentials found: '%s'", getCredentialsId()); - logger.log("See: https://github.com/jenkinsci/github-branch-source-plugin/blob/master/docs/github-app.adoc"); + logger.logError("No GitHub app credentials found: '%s'", getCredentialsId()); + logger.logError("See: https://github.com/jenkinsci/github-branch-source-plugin/blob/master/docs/github-app.adoc"); return false; } diff --git a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java index c05eb3c9..9a9353d3 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java @@ -1,22 +1,17 @@ package io.jenkins.plugins.checks.github; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.PrintStream; -import java.nio.charset.StandardCharsets; -import java.util.Optional; - -import org.jenkinsci.plugins.displayurlapi.DisplayURLProvider; - +import edu.hm.hafner.util.FilteredLog; import edu.hm.hafner.util.VisibleForTesting; -import io.jenkins.plugins.checks.api.ChecksPublisher; -import io.jenkins.plugins.checks.api.ChecksPublisherFactory; -import io.jenkins.plugins.util.PluginLogger; - import hudson.Extension; import hudson.model.Job; import hudson.model.Run; import hudson.model.TaskListener; +import io.jenkins.plugins.checks.api.ChecksPublisher; +import io.jenkins.plugins.checks.api.ChecksPublisherFactory; +import io.jenkins.plugins.util.PluginLogger; +import org.jenkinsci.plugins.displayurlapi.DisplayURLProvider; + +import java.util.Optional; /** * An factory which produces {@link GitHubChecksPublisher}. @@ -43,46 +38,27 @@ public GitHubChecksPublisherFactory() { @Override protected Optional createPublisher(final Run run, final TaskListener listener) { - try { - final String runURL = urlProvider.getRunURL(run); - return createPublisher(listener, new GitSCMChecksContext(run, runURL, scmFacade), - new GitHubSCMSourceChecksContext(run, runURL, scmFacade)); - } - catch (IOException e) { - createConsoleLogger(listener).log("Could not create publisher.", e); - } - - return Optional.empty(); + final String runURL = urlProvider.getRunURL(run); + return createPublisher(listener, new GitSCMChecksContext(run, runURL, scmFacade), + new GitHubSCMSourceChecksContext(run, runURL, scmFacade)); } @Override protected Optional createPublisher(final Job job, final TaskListener listener) { - try { - return createPublisher(listener, new GitHubSCMSourceChecksContext(job, urlProvider.getJobURL(job), - scmFacade)); - } - catch (IOException e) { - createConsoleLogger(listener).log("Could not create publisher.", e); - } - - return Optional.empty(); + return createPublisher(listener, new GitHubSCMSourceChecksContext(job, urlProvider.getJobURL(job), scmFacade)); } - private Optional createPublisher(final TaskListener listener, final GitHubChecksContext... contexts) - throws IOException { - try (ByteArrayOutputStream cause = new ByteArrayOutputStream(); - PrintStream ps = new PrintStream(cause, true, StandardCharsets.UTF_8.name())) { - PluginLogger causeLogger = new PluginLogger(ps, "GitHub Checks"); + private Optional createPublisher(final TaskListener listener, final GitHubChecksContext... contexts) { + FilteredLog causeLogger = new FilteredLog("Causes for no suitable publisher found: "); + PluginLogger consoleLogger = createConsoleLogger(getListener(listener)); - for (GitHubChecksContext ctx : contexts) { - if (ctx.isValid(causeLogger)) { - return Optional.of(new GitHubChecksPublisher(ctx, createConsoleLogger(getListener(listener)))); - } + for (GitHubChecksContext ctx : contexts) { + if (ctx.isValid(causeLogger)) { + return Optional.of(new GitHubChecksPublisher(ctx, createConsoleLogger(getListener(listener)))); } - - listener.getLogger().print(cause.toString(StandardCharsets.UTF_8.name())); } + consoleLogger.logEachLine(causeLogger.getErrorMessages()); return Optional.empty(); } diff --git a/src/main/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContext.java b/src/main/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContext.java index 5eeb0600..ecea3144 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContext.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContext.java @@ -2,11 +2,11 @@ import java.util.Optional; +import edu.hm.hafner.util.FilteredLog; import org.apache.commons.lang3.StringUtils; import org.jenkinsci.plugins.github_branch_source.GitHubSCMSource; import edu.umd.cs.findbugs.annotations.Nullable; -import io.jenkins.plugins.util.PluginLogger; import jenkins.scm.api.SCMHead; import jenkins.scm.api.SCMRevision; @@ -65,9 +65,11 @@ public String getRepository() { } @Override - public boolean isValid(final PluginLogger logger) { + public boolean isValid(final FilteredLog logger) { + logger.logError("Trying to resolve checks parameters from GitHub SCM..."); + if (resolveSource() == null) { - logger.log("No GitHub SCM source found"); + logger.logError("Job does not use GitHub SCM"); return false; } @@ -76,7 +78,13 @@ public boolean isValid(final PluginLogger logger) { return false; } - return StringUtils.isNotBlank(sha); + if (StringUtils.isBlank(sha)) { + logger.logError("No HEAD SHA found for %s", getRepository()); + + return false; + } + + return true; } @Override diff --git a/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java b/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java index 95edff08..9a147901 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java @@ -3,11 +3,10 @@ import java.io.IOException; import java.util.Optional; -import edu.hm.hafner.util.VisibleForTesting; +import edu.hm.hafner.util.FilteredLog; import org.apache.commons.lang3.StringUtils; import edu.umd.cs.findbugs.annotations.Nullable; -import io.jenkins.plugins.util.PluginLogger; import hudson.model.Run; import hudson.model.TaskListener; @@ -105,16 +104,18 @@ private GitSCM resolveGitSCM() { } @Override - public boolean isValid(final PluginLogger logger) { + public boolean isValid(final FilteredLog logger) { + logger.logError("Trying to resolve checks parameters from Git SCM..."); + if (!getScmFacade().findGitSCM(run).isPresent()) { - logger.log("Job does not use GitSCM"); + logger.logError("Job does not use Git SCM"); return false; } String remoteUrl = getRemoteUrl(); if (!isValidUrl(remoteUrl)) { - logger.log("No supported GitSCM repository URL: " + remoteUrl); + logger.logError("No supported GitSCM repository URL: " + remoteUrl); return false; } @@ -125,12 +126,12 @@ public boolean isValid(final PluginLogger logger) { String repository = getRepository(); if (getHeadSha().isEmpty()) { - logger.log("No HEAD SHA found for '%s'", repository); + logger.logError("No HEAD SHA found for '%s'", repository); return false; } - logger.log("Using GitSCM repository '%s' for GitHub checks", repository); + logger.logInfo("Using GitSCM repository '%s' for GitHub checks", repository); return true; } From 3ed27c30a2f141ab0e648b81d5e01983a452cbcd Mon Sep 17 00:00:00 2001 From: Kezhi Xiong Date: Tue, 25 Aug 2020 01:49:19 +0800 Subject: [PATCH 07/11] Add more test for GitHubSCMSourceChecksContextTest --- .../GitHubSCMSourceChecksContextTest.java | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/src/test/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContextTest.java b/src/test/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContextTest.java index db603b73..5bd6e4e7 100644 --- a/src/test/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContextTest.java +++ b/src/test/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContextTest.java @@ -2,6 +2,7 @@ import java.util.Optional; +import edu.hm.hafner.util.FilteredLog; import org.jenkinsci.plugins.displayurlapi.ClassicDisplayURLProvider; import org.jenkinsci.plugins.github_branch_source.GitHubAppCredentials; import org.jenkinsci.plugins.github_branch_source.GitHubSCMSource; @@ -17,7 +18,6 @@ import hudson.model.Job; import hudson.model.Run; -@SuppressWarnings("rawtypes") class GitHubSCMSourceChecksContextTest { private static final String URL = "URL"; @@ -155,6 +155,18 @@ void shouldThrowIllegalStateExceptionWhenGetCredentialsButNoCredentialsAvailable .hasMessage("No GitHub APP credentials available for job: github-checks-plugin"); } + @Test + void shouldThrowIllegalStateExceptionWhenGetCredentialsButNoSourceAvailable() { + Job job = mock(Job.class); + SCMFacade scmFacade = mock(SCMFacade.class); + + when(job.getName()).thenReturn("github-checks-plugin"); + + assertThatThrownBy(new GitHubSCMSourceChecksContext(job, URL, scmFacade)::getCredentials) + .isInstanceOf(IllegalStateException.class) + .hasMessage("No GitHub APP credentials available for job: github-checks-plugin"); + } + @Test void shouldGetURLForJob() { Job job = mock(Job.class); @@ -177,6 +189,37 @@ void shouldGetURLForRun() { .isEqualTo("http://127.0.0.1:8080/job/github-checks-plugin/job/master/200"); } + @Test + void shouldReturnFalseWhenValidateContextWhenCredentialsIsNotValid() { + Job job = mock(Job.class); + GitHubSCMSource source = mock(GitHubSCMSource.class); + FilteredLog logger = new FilteredLog(""); + + assertThat(new GitHubSCMSourceChecksContext(job, URL, createGitHubSCMFacadeWithSource(job, source)) + .isValid(logger)) + .isFalse(); + assertThat(logger.getErrorMessages()).contains("No credentials found"); + } + + @Test + void shouldReturnFalseWhenValidateContextWhenNoSHA() { + Run run = mock(Run.class); + Job job = mock(Job.class); + GitHubSCMSource source = mock(GitHubSCMSource.class); + GitHubAppCredentials credentials = mock(GitHubAppCredentials.class); + FilteredLog logger = new FilteredLog(""); + + when(run.getParent()).thenReturn(job); + + when(source.getRepoOwner()).thenReturn("jenkinsci"); + when(source.getRepository()).thenReturn("github-checks"); + + assertThat(new GitHubSCMSourceChecksContext(run, URL, createGitHubSCMFacadeWithCredentials(job, source, + credentials, "1")).isValid(logger)) + .isFalse(); + assertThat(logger.getErrorMessages()).contains("No HEAD SHA found for jenkinsci/github-checks"); + } + private SCMFacade createGitHubSCMFacadeWithRevision(final Job job, final GitHubSCMSource source, final SCMHead head, final SCMRevision revision, final String hash) { From 67636fdd45dd2ae2e3e45c94aa5e4fa6ac5c2e4c Mon Sep 17 00:00:00 2001 From: Kezhi Xiong Date: Tue, 25 Aug 2020 01:51:45 +0800 Subject: [PATCH 08/11] Fix PMD waring on creating console logger --- .../plugins/checks/github/GitHubChecksPublisherFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java index 9a9353d3..696cac7a 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java @@ -54,7 +54,7 @@ private Optional createPublisher(final TaskListener listener, f for (GitHubChecksContext ctx : contexts) { if (ctx.isValid(causeLogger)) { - return Optional.of(new GitHubChecksPublisher(ctx, createConsoleLogger(getListener(listener)))); + return Optional.of(new GitHubChecksPublisher(ctx, consoleLogger)); } } From 492974e1d696f296e3cb7783073def03825a2f6e Mon Sep 17 00:00:00 2001 From: Kezhi Xiong Date: Tue, 25 Aug 2020 11:57:02 +0800 Subject: [PATCH 09/11] More tests for GitHubChecksContext --- .../GitHubSCMSourceChecksContextTest.java | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/test/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContextTest.java b/src/test/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContextTest.java index 5bd6e4e7..27bba632 100644 --- a/src/test/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContextTest.java +++ b/src/test/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContextTest.java @@ -190,7 +190,7 @@ void shouldGetURLForRun() { } @Test - void shouldReturnFalseWhenValidateContextWhenCredentialsIsNotValid() { + void shouldReturnFalseWhenValidateContextButHasNoValidCredentials() { Job job = mock(Job.class); GitHubSCMSource source = mock(GitHubSCMSource.class); FilteredLog logger = new FilteredLog(""); @@ -202,7 +202,23 @@ void shouldReturnFalseWhenValidateContextWhenCredentialsIsNotValid() { } @Test - void shouldReturnFalseWhenValidateContextWhenNoSHA() { + void shouldReturnFalseWhenValidateContextButHasNoValidGitHubAppCredentials() { + Job job = mock(Job.class); + GitHubSCMSource source = mock(GitHubSCMSource.class); + FilteredLog logger = new FilteredLog(""); + + when(source.getCredentialsId()).thenReturn("oauth-credentials"); + + assertThat(new GitHubSCMSourceChecksContext(job, URL, createGitHubSCMFacadeWithSource(job, source)) + .isValid(logger)) + .isFalse(); + assertThat(logger.getErrorMessages()) + .contains("No GitHub app credentials found: 'oauth-credentials'") + .contains("See: https://github.com/jenkinsci/github-branch-source-plugin/blob/master/docs/github-app.adoc"); + } + + @Test + void shouldReturnFalseWhenValidateContextButHasNoValidSHA() { Run run = mock(Run.class); Job job = mock(Job.class); GitHubSCMSource source = mock(GitHubSCMSource.class); From b545853ebe102b24240c20f32273fa9735876a45 Mon Sep 17 00:00:00 2001 From: Kezhi Xiong Date: Tue, 25 Aug 2020 12:01:31 +0800 Subject: [PATCH 10/11] Remove check for null task listener since checks-api has used TaskListener.NULL. --- .../github/GitHubChecksPublisherFactory.java | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java index 696cac7a..dee48f02 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherFactory.java @@ -50,7 +50,7 @@ protected Optional createPublisher(final Job job, final T private Optional createPublisher(final TaskListener listener, final GitHubChecksContext... contexts) { FilteredLog causeLogger = new FilteredLog("Causes for no suitable publisher found: "); - PluginLogger consoleLogger = createConsoleLogger(getListener(listener)); + PluginLogger consoleLogger = new PluginLogger(listener.getLogger(), "GitHub Checks"); for (GitHubChecksContext ctx : contexts) { if (ctx.isValid(causeLogger)) { @@ -61,16 +61,4 @@ private Optional createPublisher(final TaskListener listener, f consoleLogger.logEachLine(causeLogger.getErrorMessages()); return Optional.empty(); } - - private TaskListener getListener(final TaskListener taskListener) { - // FIXME: checks-API should use a Null listener - if (taskListener == null) { - return TaskListener.NULL; - } - return taskListener; - } - - private PluginLogger createConsoleLogger(final TaskListener listener) { - return new PluginLogger(listener.getLogger(), "GitHub Checks"); - } } From 92ce6e0a4c2e3586f031b5a6bcf8b2b5f54584c4 Mon Sep 17 00:00:00 2001 From: Kezhi Xiong Date: Tue, 25 Aug 2020 13:07:49 +0800 Subject: [PATCH 11/11] Clean up works after merge master. --- .../checks/github/GitHubChecksContext.java | 32 ++++--------- .../checks/github/GitHubChecksPublisher.java | 25 ++++------ .../github/GitHubSCMSourceChecksContext.java | 48 ++++++++----------- .../checks/github/GitSCMChecksContext.java | 15 +++--- 4 files changed, 46 insertions(+), 74 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 ed2c0b72..06215753 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksContext.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksContext.java @@ -1,16 +1,12 @@ package io.jenkins.plugins.checks.github; -import java.util.Optional; - import edu.hm.hafner.util.FilteredLog; -import org.apache.commons.lang3.StringUtils; - import edu.umd.cs.findbugs.annotations.CheckForNull; - -import org.jenkinsci.plugins.github_branch_source.GitHubAppCredentials; import hudson.model.Job; +import org.apache.commons.lang3.StringUtils; +import org.jenkinsci.plugins.github_branch_source.GitHubAppCredentials; -import io.jenkins.plugins.util.PluginLogger; +import java.util.Optional; /** * Base class for a context that publishes GitHub checks. @@ -29,7 +25,7 @@ abstract class GitHubChecksContext { /** * Returns the commit sha of the run. * - * @return the commit sha of the run or null + * @return the commit sha of the run */ public abstract String getHeadSha(); @@ -50,22 +46,18 @@ abstract class GitHubChecksContext { */ public abstract boolean isValid(FilteredLog logger); - @Nullable + @CheckForNull protected abstract String getCredentialsId(); /** * Returns the credentials to access the remote GitHub repository. * - * @return the credentials or null + * @return the credentials */ public GitHubAppCredentials getCredentials() { - String credentialsId = getCredentialsId(); - return getGitHubAppCredentials(credentialsId); + return getGitHubAppCredentials(StringUtils.defaultIfEmpty(getCredentialsId(), "")); } - @CheckForNull - protected abstract String getCredentialsId(); - /** * Returns the URL of the run's summary page, e.g. https://ci.jenkins.io/job/Core/job/jenkins/job/master/2000/. * @@ -84,12 +76,8 @@ protected SCMFacade getScmFacade() { } protected GitHubAppCredentials getGitHubAppCredentials(final String credentialsId) { - Optional foundCredentials = findGitHubAppCredentials(credentialsId); - if (!foundCredentials.isPresent()) { - throw new IllegalStateException("No GitHub APP credentials available for job: " + getJob().getName()); - } - - return foundCredentials.get(); + return findGitHubAppCredentials(credentialsId).orElseThrow(() -> + new IllegalStateException("No GitHub APP credentials available for job: " + getJob().getName())); } protected boolean hasGitHubAppCredentials() { @@ -113,7 +101,7 @@ protected boolean hasValidCredentials(final FilteredLog logger) { return false; } - + return true; } diff --git a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisher.java b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisher.java index 188b1870..af951dc3 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisher.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisher.java @@ -1,25 +1,20 @@ package io.jenkins.plugins.checks.github; -import java.io.IOException; -import java.time.Instant; -import java.util.Date; -import java.util.logging.Level; -import java.util.logging.Logger; - +import edu.hm.hafner.util.VisibleForTesting; +import io.jenkins.plugins.checks.api.ChecksDetails; +import io.jenkins.plugins.checks.api.ChecksPublisher; import io.jenkins.plugins.util.PluginLogger; import org.apache.commons.lang3.StringUtils; - -import edu.hm.hafner.util.VisibleForTesting; -import edu.umd.cs.findbugs.annotations.CheckForNull; - -import org.kohsuke.github.GHCheckRunBuilder; -import org.kohsuke.github.GitHub; import org.jenkinsci.plugins.github_branch_source.Connector; import org.jenkinsci.plugins.github_branch_source.GitHubAppCredentials; -import hudson.model.TaskListener; +import org.kohsuke.github.GHCheckRunBuilder; +import org.kohsuke.github.GitHub; -import io.jenkins.plugins.checks.api.ChecksDetails; -import io.jenkins.plugins.checks.api.ChecksPublisher; +import java.io.IOException; +import java.time.Instant; +import java.util.Date; +import java.util.logging.Level; +import java.util.logging.Logger; /** * A publisher which publishes GitHub check runs. diff --git a/src/main/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContext.java b/src/main/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContext.java index fa73e29a..2259c236 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContext.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContext.java @@ -1,38 +1,32 @@ package io.jenkins.plugins.checks.github; -import java.util.Optional; - import edu.hm.hafner.util.FilteredLog; -import org.apache.commons.lang3.StringUtils; - -import edu.umd.cs.findbugs.annotations.Nullable; -import edu.hm.hafner.util.VisibleForTesting; import edu.umd.cs.findbugs.annotations.CheckForNull; - -import org.jenkinsci.plugins.displayurlapi.DisplayURLProvider; -import org.jenkinsci.plugins.github_branch_source.GitHubSCMSource; -import org.jenkinsci.plugins.github_branch_source.PullRequestSCMRevision; import hudson.model.Job; import hudson.model.Run; -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.github_branch_source.GitHubSCMSource; -import io.jenkins.plugins.util.PluginLogger; +import java.util.Optional; /** * Provides a {@link GitHubChecksContext} for a Jenkins job that uses a supported {@link GitHubSCMSource}. */ class GitHubSCMSourceChecksContext extends GitHubChecksContext { - @Nullable + @CheckForNull private final String sha; /** * Creates a {@link GitHubSCMSourceChecksContext} according to the run. All attributes are computed during this period. * - * @param run a run of a GitHub Branch Source project - * @param runURL the URL to the Jenkins run - * @param scmFacade a facade for Jenkins SCM + * @param run + * a run of a GitHub Branch Source project + * @param runURL + * the URL to the Jenkins run + * @param scmFacade + * a facade for Jenkins SCM */ GitHubSCMSourceChecksContext(final Run run, final String runURL, final SCMFacade scmFacade) { super(run.getParent(), runURL, scmFacade); @@ -42,9 +36,12 @@ class GitHubSCMSourceChecksContext extends GitHubChecksContext { /** * Creates a {@link GitHubSCMSourceChecksContext} according to the job. All attributes are computed during this period. * - * @param job a GitHub Branch Source project - * @param jobURL the URL to the Jenkins job - * @param scmFacade a facade for Jenkins SCM + * @param job + * a GitHub Branch Source project + * @param jobURL + * the URL to the Jenkins job + * @param scmFacade + * a facade for Jenkins SCM */ GitHubSCMSourceChecksContext(final Job job, final String jobURL, final SCMFacade scmFacade) { super(job, jobURL, scmFacade); @@ -71,11 +68,6 @@ public String getRepository() { } } - @Override @CheckForNull - protected String getCredentialsId() { - return resolveSource().getCredentialsId(); - } - @Override public boolean isValid(final FilteredLog logger) { logger.logError("Trying to resolve checks parameters from GitHub SCM..."); @@ -100,7 +92,7 @@ public boolean isValid(final FilteredLog logger) { } @Override - @Nullable + @CheckForNull protected String getCredentialsId() { GitHubSCMSource source = resolveSource(); if (source == null) { @@ -110,12 +102,12 @@ protected String getCredentialsId() { return source.getCredentialsId(); } - @Nullable + @CheckForNull private GitHubSCMSource resolveSource() { return getScmFacade().findGitHubSCMSource(getJob()).orElse(null); } - @Nullable + @CheckForNull private String resolveHeadSha(final Run run) { GitHubSCMSource source = resolveSource(); if (source != null) { @@ -128,7 +120,7 @@ private String resolveHeadSha(final Run run) { return null; } - @Nullable + @CheckForNull private String resolveHeadSha(final Job job) { GitHubSCMSource source = resolveSource(); Optional head = getScmFacade().findHead(job); diff --git a/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java b/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java index c5d4ddf1..056a01ae 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitSCMChecksContext.java @@ -1,21 +1,17 @@ package io.jenkins.plugins.checks.github; -import java.io.IOException; -import java.util.Optional; - import edu.hm.hafner.util.FilteredLog; -import org.apache.commons.lang3.StringUtils; - import edu.umd.cs.findbugs.annotations.CheckForNull; - import hudson.model.Run; import hudson.model.TaskListener; import hudson.plugins.git.GitSCM; import hudson.plugins.git.Revision; import hudson.plugins.git.UserRemoteConfig; import hudson.plugins.git.util.BuildData; +import org.apache.commons.lang3.StringUtils; -import io.jenkins.plugins.util.PluginLogger; +import java.io.IOException; +import java.util.Optional; /** * Provides a {@link GitHubChecksContext} for a Jenkins job that uses a supported {@link GitSCM}. @@ -87,7 +83,8 @@ private String removeProtocol(final String url) { return StringUtils.removeStart(StringUtils.removeStart(url, GIT_PROTOCOL), HTTPS_PROTOCOL); } - @Override @CheckForNull + @Override + @CheckForNull protected String getCredentialsId() { return getUserRemoteConfig().getCredentialsId(); } @@ -139,7 +136,7 @@ public boolean isValid(final FilteredLog logger) { } private boolean isValidUrl(@CheckForNull final String remoteUrl) { - return StringUtils.startsWith(remoteUrl, GIT_PROTOCOL) + return StringUtils.startsWith(remoteUrl, GIT_PROTOCOL) || StringUtils.startsWith(remoteUrl, HTTPS_PROTOCOL); } }