NIFI-16087 - GitHub Flow Registry fails to retrieve commits when a commit author's GitHub account cannot be resolved

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<GitCommit> 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<GHCommit> ghCommits;
             try {
                 final GHRef branchGhRef = repository.getRef(branchRef);
                 final PagedIterable<GHCommit> ghCommitsIterable = repository.queryCommits()
@@ -320,17 +323,17 @@ public List<GitCommit> getCommits(final String path, final String branch) throws
 
                 // Get the first page of commits
                 final PagedIterator<GHCommit> ghCommitsIterator = ghCommitsIterable.iterator();
-                final List<GHCommit> ghCommits = ghCommitsIterator.nextPage();
-
-                final List<GitCommit> 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<GitCommit> 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> T execute(final GHRequest<T> action) throws FlowRegistryException, IOException {
         try {
             return action.execute();

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);
+    }
+}