Use direct GerritApi calls in GerritClientDetail
The `GerritClientDetail` used to call REST API in order to retrive the
change details and map them to the internal `GerritPatchSetDetail`
representation. Alter it to use direct `GerritApi` calls with the
following changes:
* obtain user's `Account.Id` from the username and set it into the
`Configuration`
* add `openRequestContext` function to `Configuration` that is
responsible for setting the user's context to the `GerritApi`
* use `GerritApi` in order to retrieve `ChangeInfo`
* map the `ChangeInfo` to `GerritPatchSetDetail`
* load `__files/gerritPatchSetDetail.json` as `ChangeInfo` and mock
`GerritApi` in tests so that they are passing
Bug: Issue 336577745
Change-Id: I5a24c6671529a9cc2c4a06c152fa638c684e99b7
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/config/ConfigCreator.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/config/ConfigCreator.java
index 4a17d31..6e85051 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/config/ConfigCreator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/config/ConfigCreator.java
@@ -1,16 +1,15 @@
package com.googlesource.gerrit.plugins.chatgpt.config;
+import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.server.account.AccountCache;
-import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.AccountState;
-import com.google.gerrit.server.account.Accounts;
-import com.google.gerrit.server.account.externalids.ExternalId;
-import com.google.gerrit.server.account.externalids.ExternalIds;
+import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.server.config.PluginConfig;
import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.gerrit.server.project.NoSuchProjectException;
+import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import lombok.extern.slf4j.Slf4j;
@@ -26,25 +25,43 @@
private final AccountCache accountCache;
private final PluginConfigFactory configFactory;
+ private final OneOffRequestContext context;
+ private final GerritApi gerritApi;
+
@Inject
- ConfigCreator(@PluginName String pluginName, AccountCache accountCache, PluginConfigFactory configFactory) {
+ ConfigCreator(@PluginName String pluginName, AccountCache accountCache, PluginConfigFactory configFactory, OneOffRequestContext context, GerritApi gerritApi) {
this.pluginName = pluginName;
- this.configFactory = configFactory;
this.accountCache = accountCache;
+ this.configFactory = configFactory;
+ this.context = context;
+ this.gerritApi = gerritApi;
}
- public Configuration createConfig(Project.NameKey projectName)
- throws NoSuchProjectException {
+ public Configuration createConfig(Project.NameKey projectName) throws NoSuchProjectException {
PluginConfig globalConfig = configFactory.getFromGerritConfig(pluginName);
- log.debug("These configuration items have been set in the global configuration: {}", globalConfig.getNames());
+ log.debug(
+ "These configuration items have been set in the global configuration: {}",
+ globalConfig.getNames());
PluginConfig projectConfig = configFactory.getFromProjectConfig(projectName, pluginName);
- log.debug("These configuration items have been set in the project configuration: {}", projectConfig.getNames());
- return new Configuration(globalConfig, projectConfig, getGerritUserEmail(globalConfig));
+ log.debug(
+ "These configuration items have been set in the project configuration: {}",
+ projectConfig.getNames());
+ Optional<AccountState> gptAccount = getAccount(globalConfig);
+ String email = gptAccount.map(a -> a.account().preferredEmail()).orElse("");
+ Account.Id accountId =
+ gptAccount
+ .map(a -> a.account().id())
+ .orElseThrow(
+ () ->
+ new RuntimeException(
+ String.format(
+ "Given account %s doesn't exist",
+ globalConfig.getString(Configuration.KEY_GERRIT_USERNAME))));
+ return new Configuration(context, gerritApi, globalConfig, projectConfig, email, accountId);
}
- private String getGerritUserEmail(PluginConfig globalConfig) {
+ private Optional<AccountState> getAccount(PluginConfig globalConfig) {
String gptUser = globalConfig.getString(Configuration.KEY_GERRIT_USERNAME);
- Optional<AccountState> gptAccount = accountCache.getByUsername(gptUser);
- return gptAccount.map(a -> a.account().preferredEmail()).orElse("");
+ return accountCache.getByUsername(gptUser);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/config/Configuration.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/config/Configuration.java
index a9079e1..d5c189c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/config/Configuration.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/config/Configuration.java
@@ -1,6 +1,11 @@
package com.googlesource.gerrit.plugins.chatgpt.config;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.server.config.PluginConfig;
+import com.google.gerrit.server.util.ManualRequestContext;
+import com.google.gerrit.server.util.OneOffRequestContext;
+
import lombok.Getter;
import lombok.extern.slf4j.Slf4j;
@@ -120,21 +125,30 @@
private static final String KEY_IGNORE_OUTDATED_INLINE_COMMENTS = "ignoreOutdatedInlineComments";
private static final String KEY_IGNORE_RESOLVED_CHAT_GPT_COMMENTS = "ignoreResolvedChatGptComments";
+ private final OneOffRequestContext context;
+ @Getter
+ private final Account.Id userId;
@Getter
private final PluginConfig globalConfig;
@Getter
private final PluginConfig projectConfig;
@Getter
private final String gerritUserEmail;
+ @Getter
+ private final GerritApi gerritApi;
- public Configuration(PluginConfig globalConfig, PluginConfig projectConfig) {
- this(globalConfig, projectConfig, "");
- }
- public Configuration(PluginConfig globalConfig, PluginConfig projectConfig, String gerritUserEmail) {
+ public Configuration(OneOffRequestContext context, GerritApi gerritApi, PluginConfig globalConfig, PluginConfig projectConfig, String gerritUserEmail, Account.Id userId) {
+ this.context = context;
+ this.gerritApi = gerritApi;
this.globalConfig = globalConfig;
this.projectConfig = projectConfig;
this.gerritUserEmail = gerritUserEmail;
+ this.userId = userId;
+ }
+
+ public ManualRequestContext openRequestContext() {
+ return context.openAs(userId);
}
public String getGptToken() {
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 f67c82a..4f6350e 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
@@ -1,26 +1,42 @@
package com.googlesource.gerrit.plugins.chatgpt.mode.common.client.api.gerrit;
+import com.google.gerrit.entities.LabelId;
+import com.google.gerrit.extensions.common.AccountInfo;
+import com.google.gerrit.extensions.common.ApprovalInfo;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.ChangeMessageInfo;
+import com.google.gerrit.extensions.common.LabelInfo;
+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.model.api.gerrit.GerritComment;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.model.api.gerrit.GerritPatchSetDetail;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.model.api.gerrit.GerritPermittedVotingRange;
import com.googlesource.gerrit.plugins.chatgpt.mode.common.model.data.ChangeSetData;
import lombok.extern.slf4j.Slf4j;
-import java.net.URI;
-import java.util.List;
+import static java.util.Collections.emptyList;
+import static java.util.stream.Collectors.toList;
-import static com.googlesource.gerrit.plugins.chatgpt.utils.GsonUtils.getGson;
+import java.sql.Timestamp;
+import java.text.SimpleDateFormat;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Optional;
+import java.util.Set;
+import java.util.TimeZone;
@Slf4j
-public class GerritClientDetail extends GerritClientBase {
+public class GerritClientDetail {
+ private static final SimpleDateFormat DATE_FORMAT = newFormat();
+
private GerritPatchSetDetail gerritPatchSetDetail;
private final int gptAccountId;
+ private final Configuration config;
public GerritClientDetail(Configuration config, ChangeSetData changeSetData) {
- super(config);
this.gptAccountId = changeSetData.getGptAccountId();
+ this.config = config;
}
public List<GerritComment> getMessages(GerritChange change) {
@@ -54,18 +70,102 @@
return;
}
try {
- gerritPatchSetDetail = getReviewDetail(change.getFullChangeId());
+ gerritPatchSetDetail = getReviewDetail(change);
}
catch (Exception e) {
log.error("Error retrieving PatchSet details", e);
}
}
- private GerritPatchSetDetail getReviewDetail(String fullChangeId) throws Exception {
- URI uri = URI.create(config.getGerritAuthBaseUrl()
- + UriResourceLocator.gerritGetPatchSetDetailUri(fullChangeId));
- String responseBody = forwardGetRequest(uri);
- return getGson().fromJson(responseBody, GerritPatchSetDetail.class);
+ private GerritPatchSetDetail getReviewDetail(GerritChange change) throws Exception {
+ try (ManualRequestContext requestContext = config.openRequestContext()) {
+ ChangeInfo info =
+ config
+ .getGerritApi()
+ .changes()
+ .id(change.getProjectName(), change.getBranchNameKey().shortName(), change.getChangeKey().get())
+ .get();
+
+ GerritPatchSetDetail detail = new GerritPatchSetDetail();
+ detail.setWorkInProgress(info.workInProgress);
+ Optional.ofNullable(info.labels)
+ .map(Map::entrySet)
+ .map(Set::stream)
+ .flatMap(
+ labels ->
+ labels
+ .filter(label -> LabelId.CODE_REVIEW.equals(label.getKey()))
+ .map(GerritClientDetail::toLabels)
+ .findAny())
+ .ifPresent(detail::setLabels);
+ Optional.ofNullable(info.messages)
+ .map(messages -> messages.stream().map(GerritClientDetail::toComment).collect(toList()))
+ .ifPresent(detail::setMessages);
+
+ return detail;
+ }
}
+ private static GerritPatchSetDetail.Labels toLabels(Entry<String, LabelInfo> label) {
+ List<GerritPatchSetDetail.Permission> permissions =
+ Optional.ofNullable(label.getValue().all)
+ .map(all -> all.stream().map(GerritClientDetail::toPermission).collect(toList()))
+ .orElse(emptyList());
+ GerritPatchSetDetail.CodeReview codeReview = new GerritPatchSetDetail.CodeReview();
+ codeReview.setAll(permissions);
+ GerritPatchSetDetail.Labels labels = new GerritPatchSetDetail.Labels();
+ labels.setCodeReview(codeReview);
+ return labels;
+ }
+
+ private static GerritPatchSetDetail.Permission toPermission(ApprovalInfo value) {
+ GerritPatchSetDetail.Permission permission = new GerritPatchSetDetail.Permission();
+ permission.setValue(value.value);
+ Optional.ofNullable(value.date).ifPresent(date -> permission.setDate(toDateString(date)));
+ Optional.ofNullable(value.permittedVotingRange)
+ .ifPresent(
+ permittedVotingRange -> {
+ GerritPermittedVotingRange range = new GerritPermittedVotingRange();
+ range.setMin(permittedVotingRange.min);
+ range.setMax(permittedVotingRange.max);
+ permission.setPermittedVotingRange(range);
+ });
+ permission.setAccountId(value._accountId);
+ return permission;
+ }
+
+ private static GerritComment toComment(ChangeMessageInfo message) {
+ GerritComment comment = new GerritComment();
+ Optional.ofNullable(message.author).ifPresent(author -> comment.setAuthor(toAuthor(author)));
+ comment.setId(message.id);
+ comment.setTag(message.tag);
+ Optional.ofNullable(message.date).ifPresent(date -> comment.setDate(toDateString(date)));
+ comment.setMessage(message.message);
+ comment.setPatchSet(message._revisionNumber);
+ return comment;
+ }
+
+ private static GerritComment.Author toAuthor(AccountInfo authorInfo ) {
+ GerritComment.Author author = new GerritComment.Author();
+ author.setAccountId(authorInfo._accountId);
+ author.setName(authorInfo.name);
+ author.setDisplayName(author.getDisplayName());
+ author.setEmail(authorInfo.email);
+ author.setUsername(authorInfo.username);
+ return author;
+ }
+
+ /**
+ * Date format copied from <b>com.google.gerrit.json.SqlTimestampDeserializer</b>
+ */
+ private static String toDateString(Timestamp input) {
+ return DATE_FORMAT.format(input) + "000000";
+ }
+
+ private static SimpleDateFormat newFormat() {
+ SimpleDateFormat f = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS");
+ f.setTimeZone(TimeZone.getTimeZone("UTC"));
+ f.setLenient(true);
+ return f;
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/model/api/gerrit/GerritPatchSetDetail.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/model/api/gerrit/GerritPatchSetDetail.java
index 0ada543..425cd86 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/model/api/gerrit/GerritPatchSetDetail.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/mode/common/model/api/gerrit/GerritPatchSetDetail.java
@@ -5,6 +5,7 @@
import java.util.List;
+// TODO remove once migration to GerritApi is finished and com.google.gerrit.extensions.common.ChangeInfo is used
@Data
public class GerritPatchSetDetail {
private Labels labels;
diff --git a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewStatefulTest.java b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewStatefulTest.java
index 852531d..0a0b535 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewStatefulTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewStatefulTest.java
@@ -2,6 +2,7 @@
import com.github.tomakehurst.wiremock.client.WireMock;
import com.google.common.net.HttpHeaders;
+import com.google.gerrit.extensions.restapi.RestApiException;
import com.googlesource.gerrit.plugins.chatgpt.mode.stateful.client.api.UriResourceLocatorStateful;
import com.googlesource.gerrit.plugins.chatgpt.mode.stateful.client.prompt.ChatGptPromptStateful;
import com.googlesource.gerrit.plugins.chatgpt.settings.Settings.MODES;
@@ -41,7 +42,7 @@
chatGptPromptStateful = new ChatGptPromptStateful(config, getGerritChange());
}
- protected void setupMockRequests() {
+ protected void setupMockRequests() throws RestApiException {
super.setupMockRequests();
// Mock the behavior of the ChatGPT create file request
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 07880e8..4a0da3d 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewStatelessTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewStatelessTest.java
@@ -3,6 +3,7 @@
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.restapi.RestApiException;
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;
@@ -47,7 +48,7 @@
chatGptPromptStateless = new ChatGptPromptStateless(config);
}
- protected void setupMockRequests() {
+ protected void setupMockRequests() throws RestApiException {
super.setupMockRequests();
String fullChangeId = getGerritChange().getFullChangeId();
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 a8af5a5..b252280 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewTestBase.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewTestBase.java
@@ -11,6 +11,12 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.extensions.api.GerritApi;
+import com.google.gerrit.extensions.api.changes.ChangeApi;
+import com.google.gerrit.extensions.api.changes.Changes;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.json.OutputFormat;
import com.google.gerrit.server.config.PluginConfig;
import com.google.gerrit.server.data.AccountAttribute;
import com.google.gerrit.server.data.PatchSetAttribute;
@@ -19,6 +25,8 @@
import com.google.gerrit.server.events.PatchSetCreatedEvent;
import com.google.gerrit.server.events.PatchSetEvent;
import com.google.gerrit.server.project.NoSuchProjectException;
+import com.google.gerrit.server.util.OneOffRequestContext;
+import com.google.gson.Gson;
import com.google.gson.JsonArray;
import com.google.gson.JsonObject;
import com.google.inject.AbstractModule;
@@ -96,6 +104,11 @@
@Mock
protected PluginDataHandler pluginDataHandler;
+ @Mock
+ protected OneOffRequestContext context;
+ @Mock
+ protected GerritApi gerritApi;
+
protected PluginConfig globalConfig;
protected PluginConfig projectConfig;
protected Configuration config;
@@ -106,7 +119,7 @@
protected JsonArray prompts;
@Before
- public void before() throws NoSuchProjectException {
+ public void before() throws NoSuchProjectException, RestApiException {
initGlobalAndProjectConfig();
initConfig();
setupMockRequests();
@@ -147,13 +160,13 @@
}
protected void initConfig() {
- config = new Configuration(globalConfig, projectConfig);
+ 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() {
+ protected void setupMockRequests() throws RestApiException {
String fullChangeId = getGerritChange().getFullChangeId();
// Mock the behavior of the gerritAccountIdUri request
@@ -191,6 +204,7 @@
.withStatus(HTTP_OK)
.withHeader(HttpHeaders.CONTENT_TYPE, ContentType.APPLICATION_JSON.toString())
.withBodyFile("gerritPatchSetDetail.json")));
+ mockGerritApi();
// Mock the behavior of the gerritPatchSet comments request
WireMock.stubFor(WireMock.get(gerritGetAllPatchSetCommentsUri(fullChangeId))
@@ -205,6 +219,16 @@
.withStatus(HTTP_OK)));
}
+ private void mockGerritApi() throws RestApiException {
+ Gson gson = OutputFormat.JSON.newGson();
+ ChangeInfo changeInfo = gson.fromJson(readTestFile("__files/gerritPatchSetDetail.json"), ChangeInfo.class);
+ Changes changesMock = mock(Changes.class);
+ when(gerritApi.changes()).thenReturn(changesMock);
+ ChangeApi changeApiMock = mock(ChangeApi.class);
+ when(changesMock.id(PROJECT_NAME.get(), BRANCH_NAME.shortName(), CHANGE_ID.get())).thenReturn(changeApiMock);
+ when(changeApiMock.get()).thenReturn(changeInfo);
+ }
+
protected void initComparisonContent() {}
protected String readTestFile(String filename) {
