diff --git a/nifi-extension-bundles/nifi-github-bundle/nifi-github-extensions/src/main/java/org/apache/nifi/github/GitHubRepositoryClient.java b/nifi-extension-bundles/nifi-github-bundle/nifi-github-extensions/src/main/java/org/apache/nifi/github/GitHubRepositoryClient.java index 14c9616c35dc..b6d8093a7ac6 100644 --- a/nifi-extension-bundles/nifi-github-bundle/nifi-github-extensions/src/main/java/org/apache/nifi/github/GitHubRepositoryClient.java +++ b/nifi-extension-bundles/nifi-github-bundle/nifi-github-extensions/src/main/java/org/apache/nifi/github/GitHubRepositoryClient.java @@ -40,6 +40,7 @@ import org.kohsuke.github.GitHub; import org.kohsuke.github.GitHubAbuseLimitHandler; import org.kohsuke.github.GitHubBuilder; +import org.kohsuke.github.GitUser; import org.kohsuke.github.PagedIterable; import org.kohsuke.github.PagedIterator; import org.kohsuke.github.authorization.AppInstallationAuthorizationProvider; @@ -76,6 +77,7 @@ public class GitHubRepositoryClient implements GitRepositoryClient { private static final String BRANCH_REF_PATTERN = "refs/heads/%s"; private static final int COMMIT_PAGE_SIZE = 10; + private static final String UNKNOWN_AUTHOR = "unknown"; private final String repoOwner; private final String repoName; @@ -310,6 +312,7 @@ public List getCommits(final String path, final String branch) throws logger.debug("Getting commits for [{}] from branch [{}] in repo [{}]", resolvedPath, branch, repository.getName()); return execute(() -> { + final List ghCommits; try { final GHRef branchGhRef = repository.getRef(branchRef); final PagedIterable ghCommitsIterable = repository.queryCommits() @@ -320,17 +323,17 @@ public List getCommits(final String path, final String branch) throws // Get the first page of commits final PagedIterator ghCommitsIterator = ghCommitsIterable.iterator(); - final List ghCommits = ghCommitsIterator.nextPage(); - - final List commits = new ArrayList<>(); - for (final GHCommit ghCommit : ghCommits) { - commits.add(toGitCommit(ghCommit)); - } - return commits; + ghCommits = ghCommitsIterator.nextPage(); } catch (final FileNotFoundException fnf) { throwPathOrBranchNotFound(fnf, resolvedPath, branchRef); return null; } + + final List commits = new ArrayList<>(); + for (final GHCommit ghCommit : ghCommits) { + commits.add(toGitCommit(ghCommit)); + } + return commits; }); } @@ -538,8 +541,7 @@ private GitCommit toGitCommit(final GHCommit ghCommit) throws IOException { if (commit == null) { final GHCommit.ShortInfo shortInfo = ghCommit.getCommitShortInfo(); - final GHUser ghUser = ghCommit.getAuthor(); - final String author = ghUser != null ? ghUser.getLogin() : shortInfo.getAuthor().getName(); + final String author = resolveCommitAuthor(ghCommit, shortInfo); commit = new GitCommit( ghCommit.getSHA1(), author, @@ -551,6 +553,46 @@ private GitCommit toGitCommit(final GHCommit ghCommit) throws IOException { return commit; } + /** + * Resolves the author of the given commit. The GitHub account login is preferred, but resolving it + * requires an additional lookup that fails when the account cannot be found, for example bot accounts + * or deleted users whose user lookup returns Not Found. In that case, and when the commit has no + * associated GitHub account, the raw Git author name from the commit is used, falling back to a + * placeholder when the commit does not provide an author name. + * + * @param ghCommit the commit whose author is resolved + * @param shortInfo the commit short info providing the raw Git author + * @return the resolved author, never null + * @throws IOException if a non-recoverable error happens calling GitHub + */ + static String resolveCommitAuthor(final GHCommit ghCommit, final GHCommit.ShortInfo shortInfo) throws IOException { + final GHUser ghUser = getCommitAuthor(ghCommit); + if (ghUser != null) { + return ghUser.getLogin(); + } + + final GitUser gitAuthor = shortInfo.getAuthor(); + final String authorName = gitAuthor == null ? null : gitAuthor.getName(); + return authorName == null ? UNKNOWN_AUTHOR : authorName; + } + + /** + * Returns the GitHub account associated with the commit author, or null when the account cannot be + * resolved. Resolving the account requires an additional lookup that returns Not Found for accounts + * that do not exist, such as bot accounts or deleted users, which are treated as an unresolved author. + * + * @param ghCommit the commit whose author account is resolved + * @return the GitHub account for the commit author, or null when it cannot be resolved + * @throws IOException if a non-recoverable error happens calling GitHub + */ + private static GHUser getCommitAuthor(final GHCommit ghCommit) throws IOException { + try { + return ghCommit.getAuthor(); + } catch (final FileNotFoundException e) { + return null; + } + } + private T execute(final GHRequest action) throws FlowRegistryException, IOException { try { return action.execute(); diff --git a/nifi-extension-bundles/nifi-github-bundle/nifi-github-extensions/src/test/java/org/apache/nifi/github/GitHubRepositoryClientTest.java b/nifi-extension-bundles/nifi-github-bundle/nifi-github-extensions/src/test/java/org/apache/nifi/github/GitHubRepositoryClientTest.java new file mode 100644 index 000000000000..e603df2e063b --- /dev/null +++ b/nifi-extension-bundles/nifi-github-bundle/nifi-github-extensions/src/test/java/org/apache/nifi/github/GitHubRepositoryClientTest.java @@ -0,0 +1,87 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.nifi.github; + +import org.junit.jupiter.api.Test; +import org.kohsuke.github.GHCommit; +import org.kohsuke.github.GHFileNotFoundException; +import org.kohsuke.github.GHUser; +import org.kohsuke.github.GitUser; + +import java.io.IOException; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +class GitHubRepositoryClientTest { + + @Test + void testResolveCommitAuthorReturnsLoginWhenAccountResolvable() throws IOException { + final GHUser ghUser = mock(GHUser.class); + when(ghUser.getLogin()).thenReturn("octocat"); + + final GHCommit ghCommit = mock(GHCommit.class); + when(ghCommit.getAuthor()).thenReturn(ghUser); + + final GHCommit.ShortInfo shortInfo = mock(GHCommit.ShortInfo.class); + + final String author = GitHubRepositoryClient.resolveCommitAuthor(ghCommit, shortInfo); + assertEquals("octocat", author); + } + + @Test + void testResolveCommitAuthorFallsBackToGitAuthorWhenAccountNotFound() throws IOException { + final GHCommit ghCommit = mock(GHCommit.class); + when(ghCommit.getAuthor()).thenThrow(new GHFileNotFoundException("/users/Copilot 404 Not Found")); + + final GitUser gitAuthor = mock(GitUser.class); + when(gitAuthor.getName()).thenReturn("Copilot"); + final GHCommit.ShortInfo shortInfo = mock(GHCommit.ShortInfo.class); + when(shortInfo.getAuthor()).thenReturn(gitAuthor); + + final String author = GitHubRepositoryClient.resolveCommitAuthor(ghCommit, shortInfo); + assertEquals("Copilot", author); + } + + @Test + void testResolveCommitAuthorFallsBackToGitAuthorWhenNoAssociatedAccount() throws IOException { + final GHCommit ghCommit = mock(GHCommit.class); + when(ghCommit.getAuthor()).thenReturn(null); + + final GitUser gitAuthor = mock(GitUser.class); + when(gitAuthor.getName()).thenReturn("Jane Developer"); + final GHCommit.ShortInfo shortInfo = mock(GHCommit.ShortInfo.class); + when(shortInfo.getAuthor()).thenReturn(gitAuthor); + + final String author = GitHubRepositoryClient.resolveCommitAuthor(ghCommit, shortInfo); + assertEquals("Jane Developer", author); + } + + @Test + void testResolveCommitAuthorReturnsPlaceholderWhenNoAuthorAvailable() throws IOException { + final GHCommit ghCommit = mock(GHCommit.class); + when(ghCommit.getAuthor()).thenThrow(new GHFileNotFoundException("/users/Copilot 404 Not Found")); + + final GHCommit.ShortInfo shortInfo = mock(GHCommit.ShortInfo.class); + when(shortInfo.getAuthor()).thenReturn(null); + + final String author = GitHubRepositoryClient.resolveCommitAuthor(ghCommit, shortInfo); + assertEquals("unknown", author); + } +}