Use direct `accountCache` access instead if calling REST API
In `GerritClientAccount` and all derived classes get `Account.Id` from
cache instead of calling GerritApi.
Note that when classes are injected their constructor doesn't have to be
public unless they are created manually in tests. Such cases were marked
with `@VisibleForTesting` annotation.
Bug: Issue 336577745
Change-Id: I6c72043960de0b45c6d265f93a20fc9614142507
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/PatchSetReviewer.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/PatchSetReviewer.java
index 3a77620..fcbafd9 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/PatchSetReviewer.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/PatchSetReviewer.java
@@ -1,6 +1,7 @@
package com.googlesource.gerrit.plugins.chatgpt;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.googlesource.gerrit.plugins.chatgpt.config.Configuration;
import com.googlesource.gerrit.plugins.chatgpt.data.ChangeSetDataHandler;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit.GerritChange;
@@ -30,6 +31,7 @@
private final Configuration config;
private final GerritClient gerritClient;
private final ChangeSetData changeSetData;
+ private final Provider<GerritClientReview> clientReviewProvider;
@Getter
private final IChatGptClient chatGptClient;
@@ -39,10 +41,16 @@
private List<Integer> reviewScores;
@Inject
- PatchSetReviewer(GerritClient gerritClient, Configuration config, ChangeSetData changeSetData, IChatGptClient chatGptClient) {
+ PatchSetReviewer(
+ GerritClient gerritClient,
+ Configuration config,
+ ChangeSetData changeSetData,
+ Provider<GerritClientReview> clientReviewProvider,
+ IChatGptClient chatGptClient) {
this.config = config;
this.gerritClient = gerritClient;
this.changeSetData = changeSetData;
+ this.clientReviewProvider = clientReviewProvider;
this.chatGptClient = chatGptClient;
}
@@ -51,7 +59,6 @@
reviewScores = new ArrayList<>();
commentProperties = gerritClient.getClientData(change).getCommentProperties();
gerritCommentRange = new GerritCommentRange(gerritClient, change);
- GerritClientReview gerritClientReview = new GerritClientReview(config);
String patchSet = gerritClient.getPatchSet(change);
if (patchSet.isEmpty()) {
log.info("No file to review has been found in the PatchSet");
@@ -63,7 +70,7 @@
log.debug("ChatGPT response: {}", reviewReply);
retrieveReviewBatches(reviewReply, change);
- gerritClientReview.setReview(change, reviewBatches, getReviewScore());
+ clientReviewProvider.get().setReview(change, reviewBatches, getReviewScore());
}
private void setCommentBatchMap(ReviewBatch batchMap, Integer batchID) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientAccount.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientAccount.java
index 8004845..563ca6d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientAccount.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientAccount.java
@@ -1,7 +1,7 @@
package com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit;
-import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.GroupInfo;
+import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.util.ManualRequestContext;
import com.googlesource.gerrit.plugins.chatgpt.config.Configuration;
import lombok.extern.slf4j.Slf4j;
@@ -14,9 +14,11 @@
@Slf4j
public class GerritClientAccount extends GerritClientBase {
+ private final AccountCache accountCache;
- public GerritClientAccount(Configuration config) {
+ public GerritClientAccount(Configuration config, AccountCache accountCache) {
super(config);
+ this.accountCache = accountCache;
}
public boolean isDisabledUser(String authorUsername) {
@@ -37,12 +39,10 @@
}
protected Optional<Integer> getAccountId(String authorUsername) {
- try (ManualRequestContext requestContext = config.openRequestContext()) {
- List<AccountInfo> accounts = config.getGerritApi().accounts().query(authorUsername).get();
- if (accounts.isEmpty()) {
- return Optional.empty();
- }
- return Optional.of(accounts.get(0)).map(a -> a._accountId);
+ try {
+ return accountCache
+ .getByUsername(authorUsername)
+ .map(accountState -> accountState.account().id().get());
}
catch (Exception e) {
log.error("Could not find account ID for username '{}'", authorUsername);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientComments.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientComments.java
index 1a3d968..26df1f2 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientComments.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientComments.java
@@ -1,8 +1,11 @@
package com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit;
+import com.google.common.annotations.VisibleForTesting;
import com.google.gerrit.extensions.common.CommentInfo;
+import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.events.CommentAddedEvent;
import com.google.gerrit.server.util.ManualRequestContext;
+import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.chatgpt.config.Configuration;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.client.messages.ClientMessage;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.model.api.gerrit.GerritCodeRange;
@@ -33,8 +36,10 @@
@Getter
private List<GerritComment> commentProperties;
- public GerritClientComments(Configuration config, ChangeSetData changeSetData) {
- super(config);
+ @VisibleForTesting
+ @Inject
+ public GerritClientComments(Configuration config, AccountCache accountCache, ChangeSetData changeSetData) {
+ super(config, accountCache);
this.changeSetData = changeSetData;
commentProperties = new ArrayList<>();
commentMap = new HashMap<>();
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientFacade.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientFacade.java
index ff1a404..c1b4dac 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientFacade.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientFacade.java
@@ -1,5 +1,6 @@
package com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit;
+import com.google.common.annotations.VisibleForTesting;
import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.chatgpt.config.Configuration;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.client.patch.diff.FileDiffProcessed;
@@ -15,15 +16,20 @@
public class GerritClientFacade {
private final ChangeSetData changeSetData;
private final GerritClientDetail gerritClientDetail;
- private final IGerritClientPatchSet gerritClientPatchSet;
private final GerritClientComments gerritClientComments;
+ private final IGerritClientPatchSet gerritClientPatchSet;
+ @VisibleForTesting
@Inject
- public GerritClientFacade(Configuration config, ChangeSetData changeSetData, IGerritClientPatchSet gerritClientPatchSet) {
+ public GerritClientFacade(
+ Configuration config,
+ ChangeSetData changeSetData,
+ GerritClientComments gerritClientComments,
+ IGerritClientPatchSet gerritClientPatchSet) {
gerritClientDetail = new GerritClientDetail(config, changeSetData);
this.gerritClientPatchSet = gerritClientPatchSet;
this.changeSetData = changeSetData;
- gerritClientComments = new GerritClientComments(config, changeSetData);
+ this.gerritClientComments = gerritClientComments;
}
public GerritPermittedVotingRange getPermittedVotingRange(GerritChange change) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientPatchSet.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientPatchSet.java
index 6caa8b7..c409cc5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientPatchSet.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientPatchSet.java
@@ -4,6 +4,7 @@
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.util.ManualRequestContext;
import com.googlesource.gerrit.plugins.chatgpt.config.Configuration;
import com.googlesource.gerrit.plugins.chatgpt.data.ChangeSetDataHandler;
@@ -16,8 +17,8 @@
@Getter
protected Integer revisionBase = 0;
- public GerritClientPatchSet(Configuration config) {
- super(config);
+ public GerritClientPatchSet(Configuration config, AccountCache accountCache) {
+ super(config, accountCache);
}
public void retrieveRevisionBase(GerritChange change) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientReview.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientReview.java
index 2234dd1..556789d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientReview.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientReview.java
@@ -1,12 +1,15 @@
package com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
import com.google.gerrit.extensions.client.Comment;
import com.google.gerrit.entities.LabelId;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput;
import com.google.gerrit.extensions.api.changes.ReviewResult;
+import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.util.ManualRequestContext;
+import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.chatgpt.config.Configuration;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.model.review.ReviewBatch;
import lombok.extern.slf4j.Slf4j;
@@ -22,8 +25,10 @@
@Slf4j
public class GerritClientReview extends GerritClientAccount {
- public GerritClientReview(Configuration config) {
- super(config);
+ @VisibleForTesting
+ @Inject
+ public GerritClientReview(Configuration config, AccountCache accountCache) {
+ super(config, accountCache);
}
public void setReview(GerritChange change, List<ReviewBatch> reviewBatches, Integer reviewScore) throws Exception {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/stateful/client/api/gerrit/GerritClientPatchSetStateful.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/stateful/client/api/gerrit/GerritClientPatchSetStateful.java
index 257cddf..a987954 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/stateful/client/api/gerrit/GerritClientPatchSetStateful.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/stateful/client/api/gerrit/GerritClientPatchSetStateful.java
@@ -1,5 +1,7 @@
package com.googlesource.gerrit.plugins.chatgpt.mode.stateful.client.api.gerrit;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.gerrit.server.account.AccountCache;
import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.chatgpt.config.Configuration;
import com.googlesource.gerrit.plugins.chatgpt.data.PluginDataHandler;
@@ -16,9 +18,14 @@
private final GitRepoFiles gitRepoFiles;
private final PluginDataHandler pluginDataHandler;
+ @VisibleForTesting
@Inject
- public GerritClientPatchSetStateful(Configuration config, GitRepoFiles gitRepoFiles, PluginDataHandler pluginDataHandler) {
- super(config);
+ public GerritClientPatchSetStateful(
+ Configuration config,
+ AccountCache accountCache,
+ GitRepoFiles gitRepoFiles,
+ PluginDataHandler pluginDataHandler) {
+ super(config, accountCache);
this.gitRepoFiles = gitRepoFiles;
this.pluginDataHandler = pluginDataHandler;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/stateless/client/api/gerrit/GerritClientPatchSetStateless.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/stateless/client/api/gerrit/GerritClientPatchSetStateless.java
index de313da..bbba27f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/stateless/client/api/gerrit/GerritClientPatchSetStateless.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/stateless/client/api/gerrit/GerritClientPatchSetStateless.java
@@ -1,8 +1,10 @@
package com.googlesource.gerrit.plugins.chatgpt.mode.stateless.client.api.gerrit;
import com.google.inject.Inject;
+import com.google.common.annotations.VisibleForTesting;
import com.google.gerrit.extensions.common.DiffInfo;
import com.google.gerrit.extensions.common.FileInfo;
+import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.util.ManualRequestContext;
import com.googlesource.gerrit.plugins.chatgpt.config.Configuration;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit.GerritChange;
@@ -29,9 +31,10 @@
private final List<String> diffs;
private boolean isCommitMessage;
+ @VisibleForTesting
@Inject
- public GerritClientPatchSetStateless(Configuration config) {
- super(config);
+ public GerritClientPatchSetStateless(Configuration config, AccountCache accountCache) {
+ super(config, accountCache);
diffs = new ArrayList<>();
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewTestBase.java b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewTestBase.java
index 0c024f8..9362896 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewTestBase.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewTestBase.java
@@ -11,13 +11,11 @@
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.accounts.AccountApi;
import com.google.gerrit.extensions.api.accounts.Accounts;
-import com.google.gerrit.extensions.api.accounts.Accounts.QueryRequest;
import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.ChangeApi.CommentsRequest;
import com.google.gerrit.extensions.api.changes.Changes;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.RevisionApi;
-import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.common.GroupInfo;
@@ -43,9 +41,12 @@
import com.googlesource.gerrit.plugins.chatgpt.listener.EventHandlerTask;
import com.googlesource.gerrit.plugins.chatgpt.listener.GerritEventContextModule;
import com.google.inject.TypeLiteral;
+import com.google.inject.util.Providers;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit.GerritChange;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit.GerritClient;
+import com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit.GerritClientComments;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit.GerritClientFacade;
+import com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit.GerritClientReview;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.model.data.ChangeSetData;
import com.googlesource.gerrit.plugins.chatgpt.mode.interfaces.client.api.chatgpt.IChatGptClient;
import com.googlesource.gerrit.plugins.chatgpt.mode.interfaces.client.api.gerrit.IGerritClientPatchSet;
@@ -122,6 +123,8 @@
protected RevisionApi revisionApiMock;
@Mock
protected CommentsRequest commentsRequestMock;
+ @Mock
+ protected AccountCache accountCacheMock;
protected PluginConfig globalConfig;
protected PluginConfig projectConfig;
@@ -177,10 +180,10 @@
protected void setupMockRequests() throws RestApiException {
Accounts accountsMock = mockGerritAccountsRestEndpoint();
// Mock the behavior of the gerritAccountIdUri request
- mockGerritAccountsQueryApiCall(accountsMock, GERRIT_GPT_USERNAME, GERRIT_GPT_ACCOUNT_ID);
+ mockGerritAccountsQueryApiCall(GERRIT_GPT_USERNAME, GERRIT_GPT_ACCOUNT_ID);
// Mock the behavior of the gerritAccountIdUri request
- mockGerritAccountsQueryApiCall(accountsMock, GERRIT_USER_USERNAME, GERRIT_USER_ACCOUNT_ID);
+ mockGerritAccountsQueryApiCall(GERRIT_USER_USERNAME, GERRIT_USER_ACCOUNT_ID);
// Mock the behavior of the gerritAccountGroups request
mockGerritAccountGroupsApiCall(accountsMock, GERRIT_USER_ACCOUNT_ID);
@@ -201,10 +204,12 @@
}
private void mockGerritAccountsQueryApiCall(
- Accounts accountsMock, String username, int expectedAccountId) throws RestApiException {
- QueryRequest queryRequestMock = mock(QueryRequest.class);
- when(accountsMock.query(username)).thenReturn(queryRequestMock);
- when(queryRequestMock.get()).thenReturn(List.of(new AccountInfo(expectedAccountId)));
+ String username, int expectedAccountId) throws RestApiException {
+ AccountState accountStateMock = mock(AccountState.class);
+ Account accountMock = mock(Account.class);
+ when(accountStateMock.account()).thenReturn(accountMock);
+ when(accountMock.id()).thenReturn(Account.id(expectedAccountId));
+ when(accountCacheMock.getByUsername(username)).thenReturn(Optional.of(accountStateMock));
}
private void mockGerritAccountGroupsApiCall(Accounts accountsMock, int accountId)
@@ -291,8 +296,20 @@
private void initTest() {
ChangeSetData changeSetData = new ChangeSetData(GPT_USER_ACCOUNT_ID, config.getVotingMinScore(), config.getMaxReviewFileSize());
- gerritClient = new GerritClient(new GerritClientFacade(config, changeSetData, getGerritClientPatchSet()));
- patchSetReviewer = new PatchSetReviewer(gerritClient, config, changeSetData, getChatGptClient());
+ gerritClient =
+ new GerritClient(
+ new GerritClientFacade(
+ config,
+ changeSetData,
+ new GerritClientComments(config, accountCacheMock, changeSetData),
+ getGerritClientPatchSet()));
+ patchSetReviewer =
+ new PatchSetReviewer(
+ gerritClient,
+ config,
+ changeSetData,
+ Providers.of(new GerritClientReview(config, accountCacheMock)),
+ getChatGptClient());
mockConfigCreator = mock(ConfigCreator.class);
}
@@ -357,8 +374,8 @@
private IGerritClientPatchSet getGerritClientPatchSet() {
return switch (config.getGptMode()) {
- case stateful -> new GerritClientPatchSetStateful(config, gitRepoFiles, pluginDataHandler);
- case stateless -> new GerritClientPatchSetStateless(config);
+ case stateful -> new GerritClientPatchSetStateful(config, accountCacheMock, gitRepoFiles, pluginDataHandler);
+ case stateless -> new GerritClientPatchSetStateless(config, accountCacheMock);
};
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/integration/CodeReviewPluginIT.java b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/integration/CodeReviewPluginIT.java
index fa33a2d..4cf4436 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/integration/CodeReviewPluginIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/integration/CodeReviewPluginIT.java
@@ -1,5 +1,6 @@
package com.googlesource.gerrit.plugins.chatgpt.integration;
+import com.google.gerrit.server.account.AccountCache;
import com.googlesource.gerrit.plugins.chatgpt.config.Configuration;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit.GerritChange;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit.GerritClient;
@@ -36,6 +37,9 @@
@InjectMocks
private IChatGptClient chatGptClient;
+ @InjectMocks
+ private AccountCache accountCache;
+
@Test
public void sayHelloToGPT() throws Exception {
ChangeSetData changeSetData = new ChangeSetData(1, config.getVotingMinScore(), config.getMaxReviewFileSize());
@@ -67,7 +71,7 @@
reviewBatches.add(new ReviewBatch());
reviewBatches.get(0).setContent("message");
- GerritClientReview gerritClientReview = new GerritClientReview(config);
+ GerritClientReview gerritClientReview = new GerritClientReview(config, accountCache);
gerritClientReview.setReview(new GerritChange("Your changeId"), reviewBatches);
}
}
