From 7425d64abad1929e252e7b69a976794d48259110 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Wed, 11 Dec 2024 14:43:39 +0100 Subject: [PATCH] Cache retrieval of user permissions Because if we're going to inspect comments, it's going to be a lot of inspected users. --- .../lottery/github/GitHubRepository.java | 23 +++++++++++++++---- .../github/lottery/util/MockHelper.java | 2 +- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java b/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java index ab0c45a..a0811d0 100644 --- a/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java +++ b/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java @@ -14,6 +14,7 @@ import java.io.IOException; import java.sql.Date; import java.time.Clock; +import java.time.Duration; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.HashMap; @@ -31,9 +32,11 @@ import org.kohsuke.github.GHIssueSearchBuilder; import org.kohsuke.github.GHIssueState; import org.kohsuke.github.GHRepository; -import org.kohsuke.github.GHUser; import org.kohsuke.github.GitHub; +import com.github.benmanes.caffeine.cache.Caffeine; +import com.github.benmanes.caffeine.cache.LoadingCache; + import io.quarkiverse.githubapp.ConfigFile; import io.quarkiverse.githubapp.GitHubClientProvider; import io.quarkiverse.githubapp.GitHubConfigFileProvider; @@ -54,6 +57,7 @@ public class GitHubRepository implements AutoCloseable { private final GitHubConfigFileProvider configFileProvider; private final MessageFormatter messageFormatter; private final GitHubRepositoryRef ref; + private final LoadingCache noContextIssueActionSideCache; private GitHub client; private GHRepository repository; @@ -66,6 +70,10 @@ public GitHubRepository(Clock clock, GitHubClientProvider clientProvider, GitHub this.configFileProvider = configFileProvider; this.messageFormatter = messageFormatter; this.ref = ref; + this.noContextIssueActionSideCache = Caffeine.newBuilder() + .maximumSize(100) + .expireAfterWrite(Duration.ofMinutes(1)) + .build(this::computeNoContextIssueActionSide); } @Override @@ -205,18 +213,23 @@ private IssueActionSide lastActionSide(GHIssue ghIssue, Set initialActio // No action since the label was assigned. return IssueActionSide.TEAM; } - return getIssueActionSide(ghIssue, lastComment.get().getUser()); + return getIssueActionSide(ghIssue, lastComment.get().getUser().getLogin()); } - private IssueActionSide getIssueActionSide(GHIssue issue, GHUser user) throws IOException { - if (issue.getUser().getLogin().equals(user.getLogin())) { + private IssueActionSide getIssueActionSide(GHIssue issue, String login) throws IOException { + if (issue.getUser().getLogin().equals(login)) { // This is the reporter; even if part of the team, // we'll consider he's acting as an outsider here, // because he's unlikely to ask for feedback from himself. return IssueActionSide.OUTSIDER; } - return switch (repository().getPermission(user)) { + // Caching in case a same user is encountered multiple times in the same run. + return noContextIssueActionSideCache.get(login); + } + + private IssueActionSide computeNoContextIssueActionSide(String login) throws IOException { + return switch (repository().getPermission(login)) { case ADMIN, WRITE, UNKNOWN -> IssueActionSide.TEAM; // "Unknown" includes "triage" case READ, NONE -> IssueActionSide.OUTSIDER; }; diff --git a/src/test/java/io/quarkus/github/lottery/util/MockHelper.java b/src/test/java/io/quarkus/github/lottery/util/MockHelper.java index c7ea255..d2568d5 100644 --- a/src/test/java/io/quarkus/github/lottery/util/MockHelper.java +++ b/src/test/java/io/quarkus/github/lottery/util/MockHelper.java @@ -225,7 +225,7 @@ public static GHUser mockUserForInspectedComments(GitHubMockContext context, GHR GHUser mock = context.ghObject(GHUser.class, id); when(mock.getLogin()).thenReturn(login); if (permissionType != null) { - when(repositoryMock.getPermission(mock)).thenReturn(permissionType); + when(repositoryMock.getPermission(login)).thenReturn(permissionType); } return mock; }