From 4529dea88badca801fea64b3f578b2ea98f842c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Pochat?= Date: Wed, 14 Aug 2024 10:21:29 +0200 Subject: [PATCH 1/3] Infer owner from source context --- .../plugins/checks/github/GitHubChecksPublisher.java | 7 ++++++- .../checks/github/GitHubSCMSourceChecksContext.java | 4 ++++ 2 files changed, 10 insertions(+), 1 deletion(-) 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 72665956..c534328a 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisher.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisher.java @@ -75,7 +75,12 @@ public void publish(final ChecksDetails details) { String apiUri = null; if (credentials instanceof GitHubAppCredentials) { - apiUri = ((GitHubAppCredentials) credentials).getApiUri(); + final var gitHubAppCredentials = (GitHubAppCredentials) credentials; + apiUri = gitHubAppCredentials.getApiUri(); + if (context instanceof GitHubSCMSourceChecksContext) { + final var gitHubSCMSourceChecksContext = (GitHubSCMSourceChecksContext) context; + credentials = gitHubAppCredentials.withOwner(gitHubSCMSourceChecksContext.getOwner()); + } } GitHub gitHub = Connector.connect(StringUtils.defaultIfBlank(apiUri, gitHubUrl), 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 afc31c72..f6cedef0 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContext.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubSCMSourceChecksContext.java @@ -66,6 +66,10 @@ public String getRepository() { } } + String getOwner() { + return Optional.ofNullable(resolveSource()).map(GitHubSCMSource::getRepoOwner).orElse(null); + } + @Override public boolean isValid(final FilteredLog logger) { logger.logError("Trying to resolve checks parameters from GitHub SCM..."); From 89d09520f04c8f99c1ecd19e88a145f27cdcba28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Pochat?= Date: Wed, 21 Aug 2024 11:00:04 +0200 Subject: [PATCH 2/3] Add automated test --- pom.xml | 12 ++++-- .../checks/github/GitHubChecksContext.java | 1 - .../checks/github/GitHubChecksPublisher.java | 39 ++++++++++++------- .../github/GitHubChecksPublisherITest.java | 12 ++++++ 4 files changed, 47 insertions(+), 17 deletions(-) diff --git a/pom.xml b/pom.xml index 01922606..483b633c 100644 --- a/pom.xml +++ b/pom.xml @@ -18,7 +18,7 @@ 999999-SNAPSHOT - 2.361.4 + 2.440.3 true @@ -41,11 +41,17 @@ io.jenkins.tools.bom - bom-2.361.x - 2102.v854b_fec19c92 + bom-2.440.x + 3276.vcd71db_867fb_2 import pom + + + org.jenkins-ci.plugins + github-branch-source + 1797.v86fdb_4d57d43 + 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 ff6ac580..2844ef01 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksContext.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksContext.java @@ -8,7 +8,6 @@ import edu.hm.hafner.util.FilteredLog; import edu.umd.cs.findbugs.annotations.CheckForNull; -import org.jenkinsci.plugins.github_branch_source.GitHubAppCredentials; import hudson.model.Job; import hudson.model.Run; 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 c534328a..647572e6 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisher.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisher.java @@ -11,6 +11,7 @@ import org.apache.commons.lang3.StringUtils; import edu.hm.hafner.util.VisibleForTesting; +import edu.umd.cs.findbugs.annotations.Nullable; import org.kohsuke.github.GHCheckRun; import org.kohsuke.github.GHCheckRunBuilder; @@ -35,6 +36,9 @@ public class GitHubChecksPublisher extends ChecksPublisher { private final PluginLogger buildLogger; private final String gitHubUrl; + @Nullable + private StandardUsernameCredentials credentials; + /** * Creates a new instance of GitHubChecksPublisher. * @@ -63,9 +67,8 @@ public GitHubChecksPublisher(final GitHubChecksContext context, final PluginLogg @Override public void publish(final ChecksDetails details) { try { - StandardUsernameCredentials credentials = context.getCredentials(); // Prevent publication with unsupported credential types - switch (credentials.getClass().getSimpleName()) { + switch (getCredentials().getClass().getSimpleName()) { case "GitHubAppCredentials": case "VaultUsernamePasswordCredentialImpl": break; @@ -74,17 +77,12 @@ public void publish(final ChecksDetails details) { } String apiUri = null; - if (credentials instanceof GitHubAppCredentials) { - final var gitHubAppCredentials = (GitHubAppCredentials) credentials; - apiUri = gitHubAppCredentials.getApiUri(); - if (context instanceof GitHubSCMSourceChecksContext) { - final var gitHubSCMSourceChecksContext = (GitHubSCMSourceChecksContext) context; - credentials = gitHubAppCredentials.withOwner(gitHubSCMSourceChecksContext.getOwner()); - } + if (getCredentials() instanceof GitHubAppCredentials) { + apiUri = ((GitHubAppCredentials) getCredentials()).getApiUri(); } GitHub gitHub = Connector.connect(StringUtils.defaultIfBlank(apiUri, gitHubUrl), - credentials); + getCredentials()); GitHubChecksDetails gitHubDetails = new GitHubChecksDetails(details); @@ -129,12 +127,27 @@ GHCheckRunBuilder getUpdater(final GitHub github, final GitHubChecksDetails deta @VisibleForTesting GHCheckRunBuilder getCreator(final GitHub gitHub, final GitHubChecksDetails details) throws IOException { GHCheckRunBuilder builder = gitHub.getRepository(context.getRepository()) - .createCheckRun(details.getName(), context.getHeadSha()) - .withStartedAt(details.getStartedAt().orElse(Date.from(Instant.now()))); - + .createCheckRun(details.getName(), context.getHeadSha()) + .withStartedAt(details.getStartedAt().orElse(Date.from(Instant.now()))); + return applyDetails(builder, details); } + @VisibleForTesting + StandardUsernameCredentials getCredentials() { + if (credentials == null) { + credentials = context.getCredentials(); + if (credentials instanceof GitHubAppCredentials) { + final var gitHubAppCredentials = (GitHubAppCredentials) credentials; + if (context instanceof GitHubSCMSourceChecksContext) { + final var gitHubSCMSourceChecksContext = (GitHubSCMSourceChecksContext) context; + credentials = gitHubAppCredentials.withOwner(gitHubSCMSourceChecksContext.getOwner()); + } + } + } + return credentials; + } + private GHCheckRunBuilder applyDetails(final GHCheckRunBuilder builder, final GitHubChecksDetails details) { builder .withStatus(details.getStatus()) 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 b09a00c8..344a758b 100644 --- a/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherITest.java +++ b/src/test/java/io/jenkins/plugins/checks/github/GitHubChecksPublisherITest.java @@ -66,6 +66,9 @@ import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.*; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.*; /** @@ -325,6 +328,15 @@ public void testChecksPublisherUpdatesCorrectly() throws Exception { "https://github.example.com/" ); + // Check that the owner is passed from context to credentials + if (context instanceof GitHubSCMSourceChecksContext) { + var credentials = publisher.getCredentials(); + if (credentials instanceof GitHubAppCredentials) { + var gitHubAppCredentials = (GitHubAppCredentials) credentials; + assertThat(gitHubAppCredentials.getOwner()).isEqualTo("XiongKezhi"); + } + } + assertThat(context.getId(checksName1)).isNotPresent(); assertThat(context.getId(checksName2)).isNotPresent(); From 136885169e20a56c593a9a2cd93a47fce4a52b0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Pochat?= Date: Wed, 21 Aug 2024 12:12:45 +0200 Subject: [PATCH 3/3] Cleanup indent --- .../plugins/checks/github/GitHubChecksPublisher.java | 6 +++--- 1 file changed, 3 insertions(+), 3 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 647572e6..64f3c411 100644 --- a/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisher.java +++ b/src/main/java/io/jenkins/plugins/checks/github/GitHubChecksPublisher.java @@ -127,9 +127,9 @@ GHCheckRunBuilder getUpdater(final GitHub github, final GitHubChecksDetails deta @VisibleForTesting GHCheckRunBuilder getCreator(final GitHub gitHub, final GitHubChecksDetails details) throws IOException { GHCheckRunBuilder builder = gitHub.getRepository(context.getRepository()) - .createCheckRun(details.getName(), context.getHeadSha()) - .withStartedAt(details.getStartedAt().orElse(Date.from(Instant.now()))); - + .createCheckRun(details.getName(), context.getHeadSha()) + .withStartedAt(details.getStartedAt().orElse(Date.from(Instant.now()))); + return applyDetails(builder, details); }