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