Migrate GerritClientPatchSetStateless to direct GerritApi
The corresponing tests were updated to mock required GerritApi
endpoints.
Bug: Issue 336577745
Change-Id: I13cbfc773ae9e44ab74928144092f49fd26845b5
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 531d872..de313da 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,25 +1,27 @@
package com.googlesource.gerrit.plugins.chatgpt.mode.stateless.client.api.gerrit;
-import com.google.gson.JsonElement;
-import com.google.gson.JsonObject;
import com.google.inject.Inject;
+import com.google.gerrit.extensions.common.DiffInfo;
+import com.google.gerrit.extensions.common.FileInfo;
+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;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit.GerritClientPatchSet;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.client.patch.diff.FileDiffProcessed;
+import com.googlesource.gerrit.plugins.chatgpt.mode.common.model.api.gerrit.GerritFileDiff;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.model.api.gerrit.GerritPatchSetFileDiff;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.model.api.gerrit.GerritReviewFileDiff;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.model.data.ChangeSetData;
import com.googlesource.gerrit.plugins.chatgpt.mode.interfaces.client.api.gerrit.IGerritClientPatchSet;
-import com.googlesource.gerrit.plugins.chatgpt.mode.stateless.client.api.UriResourceLocatorStateless;
import lombok.extern.slf4j.Slf4j;
-import java.net.URI;
+import static java.util.stream.Collectors.toList;
+
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
-import static com.googlesource.gerrit.plugins.chatgpt.utils.GsonUtils.getGson;
import static com.googlesource.gerrit.plugins.chatgpt.utils.GsonUtils.getNoEscapedGson;
@Slf4j
@@ -37,41 +39,65 @@
int revisionBase = getChangeSetRevisionBase(changeSetData);
log.debug("Revision base: {}", revisionBase);
- List<String> files = getAffectedFiles(change.getFullChangeId(), revisionBase);
+ List<String> files = getAffectedFiles(change, revisionBase);
log.debug("Patch files: {}", files);
- String fileDiffsJson = getFileDiffsJson(change.getFullChangeId(), files, revisionBase);
+ String fileDiffsJson = getFileDiffsJson(change, files, revisionBase);
log.debug("File diffs: {}", fileDiffsJson);
return fileDiffsJson;
}
- private List<String> getAffectedFiles(String fullChangeId, int revisionBase) throws Exception {
- URI uri = URI.create(config.getGerritAuthBaseUrl()
- + UriResourceLocatorStateless.gerritPatchSetFilesUri(fullChangeId)
- + UriResourceLocatorStateless.gerritRevisionBasePostfixUri(revisionBase));
- log.debug("Affected Files URI: '{}'", uri);
- JsonObject affectedFileMap = forwardGetRequestReturnJsonObject(uri);
- List<String> files = new ArrayList<>();
- for (Map.Entry<String, JsonElement> file : affectedFileMap.entrySet()) {
- String filename = file.getKey();
- if (!filename.equals("/COMMIT_MSG") || config.getGptReviewCommitMessages()) {
- int size = Integer.parseInt(file.getValue().getAsJsonObject().get("size").getAsString());
- if (size > config.getMaxReviewFileSize()) {
- log.info("File '{}' not reviewed because its size exceeds the fixed maximum allowable size.",
- filename);
- }
- else {
- files.add(filename);
- }
- }
+ private List<String> getAffectedFiles(GerritChange change, int revisionBase) throws Exception {
+ try (ManualRequestContext requestContext = config.openRequestContext()) {
+ Map<String, FileInfo> files =
+ config
+ .getGerritApi()
+ .changes()
+ .id(
+ change.getProjectName(),
+ change.getBranchNameKey().shortName(),
+ change.getChangeKey().get())
+ .current()
+ .files(revisionBase);
+ return files.entrySet().stream()
+ .filter(
+ fileEntry -> {
+ String filename = fileEntry.getKey();
+ if (!filename.equals("/COMMIT_MSG") || config.getGptReviewCommitMessages()) {
+ if (fileEntry.getValue().size > config.getMaxReviewFileSize()) {
+ log.info(
+ "File '{}' not reviewed because its size exceeds the fixed maximum allowable size.",
+ filename);
+ } else {
+ return true;
+ }
+ }
+ return false;
+ })
+ .map(Map.Entry::getKey)
+ .collect(toList());
}
- return files;
}
- private void processFileDiff(String filename, String fileDiffJson) {
- log.debug("FileDiff Json processed: {}", fileDiffJson);
- GerritPatchSetFileDiff gerritPatchSetFileDiff = getGson().fromJson(fileDiffJson, GerritPatchSetFileDiff.class);
+ private void processFileDiff(String filename, DiffInfo diff) {
+ log.debug("FileDiff content processed: {}", filename);
+
+ GerritPatchSetFileDiff gerritPatchSetFileDiff = new GerritPatchSetFileDiff();
+ Optional.ofNullable(diff.metaA)
+ .ifPresent(
+ meta -> gerritPatchSetFileDiff.setMetaA(GerritClientPatchSetStateless.toMeta(meta)));
+ Optional.ofNullable(diff.metaB)
+ .ifPresent(
+ meta -> gerritPatchSetFileDiff.setMetaB(GerritClientPatchSetStateless.toMeta(meta)));
+ Optional.ofNullable(diff.content)
+ .ifPresent(
+ content ->
+ gerritPatchSetFileDiff.setContent(
+ content.stream()
+ .map(GerritClientPatchSetStateless::toContent)
+ .collect(toList())));
+
// Initialize the reduced file diff for the Gerrit review with fields `meta_a` and `meta_b`
GerritReviewFileDiff gerritReviewFileDiff = new GerritReviewFileDiff(gerritPatchSetFileDiff.getMetaA(),
gerritPatchSetFileDiff.getMetaB());
@@ -81,24 +107,45 @@
diffs.add(getNoEscapedGson().toJson(gerritReviewFileDiff));
}
- private String getFileDiffsJson(String fullChangeId, List<String> files, int revisionBase) throws Exception {
+ private String getFileDiffsJson(GerritChange change, List<String> files, int revisionBase) throws Exception {
List<String> enabledFileExtensions = config.getEnabledFileExtensions();
- for (String filename : files) {
- isCommitMessage = filename.equals("/COMMIT_MSG");
- if (!isCommitMessage && (filename.lastIndexOf(".") < 1 ||
- !enabledFileExtensions.contains(filename.substring(filename.lastIndexOf("."))))) {
- continue;
+ try (ManualRequestContext requestContext = config.openRequestContext()) {
+ for (String filename : files) {
+ isCommitMessage = filename.equals("/COMMIT_MSG");
+ if (!isCommitMessage && (filename.lastIndexOf(".") < 1 ||
+ !enabledFileExtensions.contains(filename.substring(filename.lastIndexOf("."))))) {
+ continue;
+ }
+ DiffInfo diff =
+ config
+ .getGerritApi()
+ .changes()
+ .id(
+ change.getProjectName(),
+ change.getBranchNameKey().shortName(),
+ change.getChangeKey().get())
+ .current()
+ .file(filename)
+ .diff(revisionBase);
+ processFileDiff(filename, diff);
}
- URI uri = URI.create(config.getGerritAuthBaseUrl()
- + UriResourceLocatorStateless.gerritPatchSetFilesUri(fullChangeId)
- + UriResourceLocatorStateless.gerritDiffPostfixUri(filename)
- + UriResourceLocatorStateless.gerritRevisionBasePostfixUri(revisionBase));
- log.debug("getFileDiffsJson URI: '{}'", uri);
- String fileDiffJson = forwardGetRequest(uri).replaceAll("^[')\\]}]+", "");
- processFileDiff(filename, fileDiffJson);
}
- diffs.add(String.format("{\"changeId\": \"%s\"}", fullChangeId));
+ diffs.add(String.format("{\"changeId\": \"%s\"}", change.getFullChangeId()));
return "[" + String.join(",", diffs) + "]\n";
}
+ private static GerritFileDiff.Meta toMeta(DiffInfo.FileMeta input) {
+ GerritFileDiff.Meta meta = new GerritFileDiff.Meta();
+ meta.setContentType(input.contentType);
+ meta.setName(input.name);
+ return meta;
+ }
+
+ private static GerritPatchSetFileDiff.Content toContent(DiffInfo.ContentEntry input) {
+ GerritPatchSetFileDiff.Content content = new GerritPatchSetFileDiff.Content();
+ content.a = input.a;
+ content.b = input.b;
+ content.ab = input.ab;
+ return content;
+ }
}
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 de04876..c4fb24f 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewStatelessTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewStatelessTest.java
@@ -3,13 +3,18 @@
import com.github.tomakehurst.wiremock.client.WireMock;
import com.google.common.net.HttpHeaders;
import com.googlesource.gerrit.plugins.chatgpt.listener.EventHandlerTask;
+import com.google.gerrit.extensions.api.changes.FileApi;
import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.common.DiffInfo;
+import com.google.gerrit.extensions.common.FileInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.json.OutputFormat;
import com.google.gson.Gson;
import com.googlesource.gerrit.plugins.chatgpt.mode.stateless.client.api.UriResourceLocatorStateless;
import com.googlesource.gerrit.plugins.chatgpt.mode.stateless.client.prompt.ChatGptPromptStateless;
import lombok.extern.slf4j.Slf4j;
+
+import org.apache.commons.lang3.reflect.TypeLiteral;
import org.apache.http.entity.ContentType;
import org.junit.Assert;
import org.junit.Ignore;
@@ -21,9 +26,11 @@
import java.net.URI;
import java.util.Arrays;
+import java.util.Map;
import static com.googlesource.gerrit.plugins.chatgpt.utils.TextUtils.joinWithNewLine;
import static java.net.HttpURLConnection.HTTP_OK;
+import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@Slf4j
@@ -55,28 +62,25 @@
protected void setupMockRequests() throws RestApiException {
super.setupMockRequests();
- String fullChangeId = getGerritChange().getFullChangeId();
+ // Mock the GerritApi's revision API
+ when(changeApiMock.current()).thenReturn(revisionApiMock);
// Mock the behavior of the gerritPatchSetFiles request
- WireMock.stubFor(WireMock.get(UriResourceLocatorStateless.gerritPatchSetFilesUri(fullChangeId))
- .willReturn(WireMock.aResponse()
- .withStatus(HTTP_OK)
- .withHeader(HttpHeaders.CONTENT_TYPE, ContentType.APPLICATION_JSON.toString())
- .withBodyFile("gerritPatchSetFiles.json")));
+ Map<String, FileInfo> files =
+ readTestFileToType(
+ "__files/gerritPatchSetFiles.json",
+ new TypeLiteral<Map<String, FileInfo>>() {}.getType());
+ when(revisionApiMock.files(0)).thenReturn(files);
// Mock the behavior of the gerritPatchSet diff requests
- WireMock.stubFor(WireMock.get(UriResourceLocatorStateless.gerritPatchSetFilesUri(fullChangeId) +
- UriResourceLocatorStateless.gerritDiffPostfixUri("/COMMIT_MSG"))
- .willReturn(WireMock.aResponse()
- .withStatus(HTTP_OK)
- .withHeader(HttpHeaders.CONTENT_TYPE, ContentType.APPLICATION_JSON.toString())
- .withBodyFile("gerritPatchSetDiffCommitMsg.json")));
- WireMock.stubFor(WireMock.get(UriResourceLocatorStateless.gerritPatchSetFilesUri(fullChangeId) +
- UriResourceLocatorStateless.gerritDiffPostfixUri("test_file.py"))
- .willReturn(WireMock.aResponse()
- .withStatus(HTTP_OK)
- .withHeader(HttpHeaders.CONTENT_TYPE, ContentType.APPLICATION_JSON.toString())
- .withBodyFile("gerritPatchSetDiffTestFile.json")));
+ FileApi commitMsgFileMock = mock(FileApi.class);
+ when(revisionApiMock.file("/COMMIT_MSG")).thenReturn(commitMsgFileMock);
+ DiffInfo commitMsgFileDiff = readTestFileToClass("__files/gerritPatchSetDiffCommitMsg.json", DiffInfo.class);
+ when(commitMsgFileMock.diff(0)).thenReturn(commitMsgFileDiff);
+ FileApi testFileMock = mock(FileApi.class);
+ when(revisionApiMock.file("test_file.py")).thenReturn(testFileMock);
+ DiffInfo testFileDiff = readTestFileToClass("__files/gerritPatchSetDiffTestFile.json", DiffInfo.class);
+ when(testFileMock.diff(0)).thenReturn(testFileDiff);
// Mock the behavior of the askGpt request
WireMock.stubFor(WireMock.post(WireMock.urlEqualTo(URI.create(config.getGptDomain()
@@ -85,9 +89,6 @@
.withStatus(HTTP_OK)
.withHeader(HttpHeaders.CONTENT_TYPE, ContentType.APPLICATION_JSON.toString())
.withBodyFile("chatGptResponseStreamed.txt")));
-
- // Mock the GerritApi's revision API
- when(changeApiMock.current()).thenReturn(revisionApiMock);
}
protected void initComparisonContent() {
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 a8ee958..c969231 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewTestBase.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewTestBase.java
@@ -64,6 +64,7 @@
import org.mockito.stubbing.Answer;
import java.io.IOException;
+import java.lang.reflect.Type;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
@@ -263,6 +264,11 @@
return gson.fromJson(readTestFile(filename), clazz);
}
+ protected <T> T readTestFileToType(String filename, Type type) {
+ Gson gson = OutputFormat.JSON.newGson();
+ return gson.fromJson(readTestFile(filename), type);
+ }
+
protected String readTestFile(String filename) {
try {
return new String(Files.readAllBytes(basePath.resolve(filename)));
