Migrate GerritClientComments to direct GerritApi
Note that test configuration has to be cleaned a bit so that Mockito was
not complayning about not used mocks...
Bug: Issue 336577745
Change-Id: I06aeadfcd265c2794fda80d5bbbd115305266c00
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 9729c4a..1a3d968 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,22 +1,24 @@
package com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit;
+import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.server.events.CommentAddedEvent;
-import com.google.gson.reflect.TypeToken;
+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.UriResourceLocator;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.client.messages.ClientMessage;
+import com.googlesource.gerrit.plugins.chatgpt.mode.common.model.api.gerrit.GerritCodeRange;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.model.api.gerrit.GerritComment;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.model.data.ChangeSetData;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.model.data.CommentData;
import lombok.Getter;
import lombok.extern.slf4j.Slf4j;
-import java.lang.reflect.Type;
-import java.net.URI;
+import static java.util.stream.Collectors.toList;
+
import java.util.*;
-import static com.googlesource.gerrit.plugins.chatgpt.utils.GsonUtils.getGson;
import static com.googlesource.gerrit.plugins.chatgpt.utils.TimeUtils.getTimeStamp;
+import static com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit.GerritClientDetail.toAuthor;
+import static com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit.GerritClientDetail.toDateString;
import static com.googlesource.gerrit.plugins.chatgpt.settings.Settings.GERRIT_PATCH_SET_FILENAME;
@Slf4j
@@ -69,41 +71,64 @@
}
private List<GerritComment> retrieveComments(GerritChange change) throws Exception {
- URI uri = URI.create(config.getGerritAuthBaseUrl()
- + UriResourceLocator.gerritGetAllPatchSetCommentsUri(change.getFullChangeId()));
- String responseBody = forwardGetRequest(uri);
- Type mapEntryType = new TypeToken<Map<String, List<GerritComment>>>(){}.getType();
- Map<String, List<GerritComment>> lastCommentMap = getGson().fromJson(responseBody, mapEntryType);
+ try (ManualRequestContext requestContext = config.openRequestContext()) {
+ Map<String, List<CommentInfo>> comments =
+ config
+ .getGerritApi()
+ .changes()
+ .id(
+ change.getProjectName(),
+ change.getBranchNameKey().shortName(),
+ change.getChangeKey().get())
+ .commentsRequest()
+ .get();
- String latestChangeMessageId = null;
- HashMap<String, List<GerritComment>> latestComments = new HashMap<>();
- for (Map.Entry<String, List<GerritComment>> entry : lastCommentMap.entrySet()) {
- String filename = entry.getKey();
- log.info("Commented filename: {}", filename);
+ // note that list of Map.Entry was used in order to keep the original response order
+ List<Map.Entry<String, List<GerritComment>>> lastCommentEntries =
+ comments.entrySet().stream()
+ .map(
+ entry ->
+ Map.entry(
+ entry.getKey(),
+ entry.getValue().stream()
+ .map(GerritClientComments::toComment)
+ .collect(toList())))
+ .collect(toList());
- List<GerritComment> commentsArray = entry.getValue();
+ String latestChangeMessageId = null;
+ HashMap<String, List<GerritComment>> latestComments = new HashMap<>();
+ for (Map.Entry<String, List<GerritComment>> entry : lastCommentEntries) {
+ String filename = entry.getKey();
+ log.info("Commented filename: {}", filename);
- for (GerritComment commentObject : commentsArray) {
- commentObject.setFilename(filename);
- String commentId = commentObject.getId();
- String changeMessageId = commentObject.getChangeMessageId();
- String commentAuthorUsername = commentObject.getAuthor().getUsername();
- log.debug("Change Message Id: {} - Author: {}", latestChangeMessageId, commentAuthorUsername);
- long updatedTimeStamp = getTimeStamp(commentObject.getUpdated());
- if (commentAuthorUsername.equals(authorUsername) &&
- updatedTimeStamp >= change.getEventTimeStamp() - MAX_SECS_GAP_BETWEEN_EVENT_AND_COMMENT) {
- log.debug("Found comment with updatedTimeStamp : {}", updatedTimeStamp);
- latestChangeMessageId = changeMessageId;
- }
- latestComments.computeIfAbsent(changeMessageId, k -> new ArrayList<>()).add(commentObject);
- commentMap.put(commentId, commentObject);
- if (filename.equals(GERRIT_PATCH_SET_FILENAME)) {
- patchSetCommentMap.put(changeMessageId, commentObject);
+ List<GerritComment> commentsArray = entry.getValue();
+
+ for (GerritComment commentObject : commentsArray) {
+ commentObject.setFilename(filename);
+ String commentId = commentObject.getId();
+ String changeMessageId = commentObject.getChangeMessageId();
+ String commentAuthorUsername = commentObject.getAuthor().getUsername();
+ log.debug(
+ "Change Message Id: {} - Author: {}", latestChangeMessageId, commentAuthorUsername);
+ long updatedTimeStamp = getTimeStamp(commentObject.getUpdated());
+ if (commentAuthorUsername.equals(authorUsername)
+ && updatedTimeStamp
+ >= change.getEventTimeStamp() - MAX_SECS_GAP_BETWEEN_EVENT_AND_COMMENT) {
+ log.debug("Found comment with updatedTimeStamp : {}", updatedTimeStamp);
+ latestChangeMessageId = changeMessageId;
+ }
+ latestComments
+ .computeIfAbsent(changeMessageId, k -> new ArrayList<>())
+ .add(commentObject);
+ commentMap.put(commentId, commentObject);
+ if (filename.equals(GERRIT_PATCH_SET_FILENAME)) {
+ patchSetCommentMap.put(changeMessageId, commentObject);
+ }
}
}
- }
- return latestComments.getOrDefault(latestChangeMessageId, null);
+ return latestComments.getOrDefault(latestChangeMessageId, null);
+ }
}
private void addLastComments(GerritChange change) {
@@ -131,4 +156,30 @@
}
}
+ private static GerritComment toComment(CommentInfo comment) {
+ GerritComment gerritComment = new GerritComment();
+ gerritComment.setAuthor(toAuthor(comment.author));
+ gerritComment.setChangeMessageId(comment.changeMessageId);
+ gerritComment.setUnresolved(comment.unresolved);
+ gerritComment.setPatchSet(comment.patchSet);
+ gerritComment.setId(comment.id);
+ gerritComment.setTag(comment.tag);
+ gerritComment.setLine(comment.line);
+ Optional.ofNullable(comment.range)
+ .ifPresent(
+ range ->
+ gerritComment.setRange(
+ GerritCodeRange.builder()
+ .startLine(range.startLine)
+ .endLine(range.endLine)
+ .startCharacter(range.startCharacter)
+ .endCharacter(range.endCharacter)
+ .build()));
+ gerritComment.setInReplyTo(comment.inReplyTo);
+ Optional.ofNullable(comment.updated)
+ .ifPresent(updated -> gerritComment.setUpdated(toDateString(updated)));
+ gerritComment.setMessage(comment.message);
+ gerritComment.setCommitId(comment.commitId);
+ return gerritComment;
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientDetail.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientDetail.java
index 4f6350e..7bda0e3 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientDetail.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/client/api/gerrit/GerritClientDetail.java
@@ -145,7 +145,7 @@
return comment;
}
- private static GerritComment.Author toAuthor(AccountInfo authorInfo ) {
+ static GerritComment.Author toAuthor(AccountInfo authorInfo ) {
GerritComment.Author author = new GerritComment.Author();
author.setAccountId(authorInfo._accountId);
author.setName(authorInfo.name);
@@ -158,7 +158,7 @@
/**
* Date format copied from <b>com.google.gerrit.json.SqlTimestampDeserializer</b>
*/
- private static String toDateString(Timestamp input) {
+ static String toDateString(Timestamp input) {
return DATE_FORMAT.format(input) + "000000";
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewStatelessTest.java b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewStatelessTest.java
index c4fb24f..a7797ca 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewStatelessTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewStatelessTest.java
@@ -52,6 +52,7 @@
.thenReturn(GPT_STREAM_OUTPUT);
when(globalConfig.getBoolean(Mockito.eq("gptReviewCommitMessages"), Mockito.anyBoolean()))
.thenReturn(true);
+ when(globalConfig.getString("gerritUserName")).thenReturn(GERRIT_GPT_USERNAME);
super.initConfig();
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 c969231..5c557ef 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewTestBase.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewTestBase.java
@@ -15,11 +15,13 @@
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;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.json.OutputFormat;
@@ -72,6 +74,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Optional;
+import java.util.Map;
import java.util.function.Consumer;
import static com.google.gerrit.extensions.client.ChangeKind.REWORK;
@@ -122,6 +125,8 @@
protected ChangeApi changeApiMock;
@Mock
protected RevisionApi revisionApiMock;
+ @Mock
+ protected CommentsRequest commentsRequestMock;
protected PluginConfig globalConfig;
protected PluginConfig projectConfig;
@@ -153,9 +158,6 @@
};
// Mock the Global Config values not provided by Default
- when(globalConfig.getString("gerritAuthBaseUrl")).thenReturn(GERRIT_AUTH_BASE_URL);
- when(globalConfig.getString("gerritUserName")).thenReturn(GERRIT_USER_USERNAME);
- when(globalConfig.getString("gerritPassword")).thenReturn(GERRIT_USER_PASSWORD);
when(globalConfig.getString("gptToken")).thenReturn(GPT_TOKEN);
// Mock the Global Config values to the Defaults passed as second arguments of the `get*` methods.
@@ -175,9 +177,6 @@
protected void initConfig() {
config = new Configuration(context, gerritApi, globalConfig, projectConfig, "gpt@email.com", Account.id(1000000));
-
- // Mock the config instance values
- when(config.getGerritUserName()).thenReturn(GERRIT_GPT_USERNAME);
}
protected void setupMockRequests() throws RestApiException {
@@ -215,6 +214,7 @@
.withStatus(HTTP_OK)
.withHeader(HttpHeaders.CONTENT_TYPE, ContentType.APPLICATION_JSON.toString())
.withBodyFile("gerritPatchSetComments.json")));
+ mockGerritChangeCommentsApiCall();
// Mock the behavior of the postReview request
WireMock.stubFor(WireMock.post(gerritSetReviewUri(fullChangeId))
@@ -252,6 +252,15 @@
when(changeApiMock.get()).thenReturn(changeInfo);
}
+ private void mockGerritChangeCommentsApiCall() throws RestApiException {
+ Map<String, List<CommentInfo>> comments =
+ readTestFileToType(
+ "__files/gerritPatchSetComments.json",
+ new TypeLiteral<Map<String, List<CommentInfo>>>() {}.getType());
+ when(changeApiMock.commentsRequest()).thenReturn(commentsRequestMock);
+ when(commentsRequestMock.get()).thenReturn(comments);
+ }
+
private void mockGerritChangeApiRestEndpoint() throws RestApiException {
when(gerritApi.changes()).thenReturn(changesMock);
when(changesMock.id(PROJECT_NAME.get(), BRANCH_NAME.shortName(), CHANGE_ID.get())).thenReturn(changeApiMock);
