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