Enhance code readability and log message clarity
- Made code simpler and aligned it with standards;
- Refined logging;
- Boosted readability with entity renaming.
Jira-Id: IT-103
Change-Id: Id31197b2b4c3ec948c53f8aa273e244760a90dcf
Signed-off-by: Patrizio <patrizio.gelosi@amarulasolutions.com>
diff --git a/README.md b/README.md
index 6440b78..af79cf4 100644
--- a/README.md
+++ b/README.md
@@ -202,7 +202,7 @@
- `gptSystemPrompt`: You can modify the default system prompt to your preferred prompt.
- `gptTemperature`: The default value is 1. What sampling temperature to use, between 0 and 2. Higher values like 0.8
will make the output more random, while lower values like 0.2 will make it more focused and deterministic.
-- `gptReviewPatchset`: Set to true by default. When switched to false, it disables the automatic review of Patchsets as
+- `gptReviewPatchSet`: Set to true by default. When switched to false, it disables the automatic review of PatchSets as
they are created or updated.
- `gptReviewCommitMessages`: The default value is false. When enabled by setting to true, this option also verifies if
the commit message matches with the content of the review.
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/PatchSetReviewer.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/PatchSetReviewer.java
index aade668..fb8f5db 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/PatchSetReviewer.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/PatchSetReviewer.java
@@ -36,10 +36,10 @@
commentProperties = gerritClient.getCommentProperties();
String patchSet = gerritClient.getPatchSet(fullChangeId);
if (patchSet.isEmpty()) {
- log.info("No file to review has been found in the Patchset");
+ log.info("No file to review has been found in the PatchSet");
return;
}
- config.configureDynamically(Configuration.KEY_GPT_USER_PROMPT, gerritClient.getTaggedPrompt());
+ config.configureDynamically(Configuration.KEY_GPT_USER_PROMPT, gerritClient.getUserPrompt());
String reviewSuggestion = getReviewSuggestion(config, fullChangeId, patchSet);
splitReviewIntoBatches(reviewSuggestion);
@@ -47,20 +47,24 @@
gerritClient.postComments(fullChangeId, reviewBatches);
}
- private Integer getBatchID() {
+ private Integer getBatchId() {
try {
return Integer.parseInt(currentTag);
}
catch (NumberFormatException ex){
- return -1;
+ return null;
}
}
private void addReviewBatch(StringBuilder batch) {
HashMap<String, Object> batchMap = new HashMap<>();
batchMap.put("content", batch.toString());
- Integer batchID = getBatchID();
- if (batchID >= 0 && commentProperties != null && batchID < commentProperties.size()) {
+ Integer batchID = getBatchId();
+ if (batchID == null) {
+ log.warn("Error retrieving batch ID from currentTag {}", currentTag);
+ return;
+ }
+ if (commentProperties != null && batchID < commentProperties.size()) {
JsonObject commentProperty = commentProperties.get(batchID);
if (commentProperty != null &&
(commentProperty.has("line") || commentProperty.has("range"))) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClient.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClient.java
index bfc2e98..854b4f0 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClient.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClient.java
@@ -45,8 +45,8 @@
return gerritClientComments.retrieveLastComments(event, fullChangeId);
}
- public String getTaggedPrompt() {
- return gerritClientComments.getTaggedPrompt(gerritClientPatchSet.getFilesNewContent());
+ public String getUserPrompt() {
+ return gerritClientComments.getUserPrompt(gerritClientPatchSet.getFilesNewContent());
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClientAccount.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClientAccount.java
index 18191e7..f8f3de3 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClientAccount.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClientAccount.java
@@ -25,7 +25,7 @@
return Optional.of(responseArray.get(0).getAsJsonObject().get("_account_id").getAsInt());
}
catch (Exception e) {
- log.info("Could not find account ID for username '{}'", authorUsername);
+ log.error("Could not find account ID for username '{}'", authorUsername);
return Optional.empty();
}
}
@@ -41,7 +41,7 @@
.collect(Collectors.toList());
}
catch (Exception e) {
- log.info("Could not find groups for account ID {}", accountId);
+ log.error("Could not find groups for account ID {}", accountId);
return null;
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClientBase.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClientBase.java
index 4e61211..8225747 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClientBase.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClientBase.java
@@ -46,10 +46,10 @@
HttpResponse<String> response = httpClientWithRetry.execute(request);
if (response.statusCode() != HTTP_OK) {
- log.error("Failed to get response. Response: {}", response);
+ log.error("Failed to get response from Gerrit. Response: {}", response);
throw new IOException("Failed to get response from Gerrit");
}
- log.debug("Successfully obtained response.");
+ log.debug("Successfully obtained response from Gerrit.");
return response.body();
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClientComments.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClientComments.java
index 470fbab..3b6835c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClientComments.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClientComments.java
@@ -107,7 +107,7 @@
}
}
} catch (Exception e) {
- log.error("Error while processing change: {}", fullChangeId, e);
+ log.error("Error while retrieving comments for change: {}", fullChangeId, e);
}
}
@@ -167,10 +167,9 @@
commentsStartTimestamp = event.eventCreatedOn;
CommentAddedEvent commentAddedEvent = (CommentAddedEvent) event;
authorUsername = commentAddedEvent.author.get().username;
- log.debug("Comments start datetime: {}", commentsStartTimestamp);
- log.debug("Author username: {} - ChatGPT username: {}", authorUsername, config.getGerritUserName());
+ log.debug("Found comments by '{}' on {}", authorUsername, commentsStartTimestamp);
if (authorUsername.equals(config.getGerritUserName())) {
- log.debug("These are the Chatbot's own comments, do not process them.");
+ log.debug("These are the Bot's own comments, do not process them.");
return false;
}
if (isDisabledUser(authorUsername)) {
@@ -187,14 +186,13 @@
if (map.isEmpty()) {
return;
}
- String json = gson.toJson(map);
-
URI uri = URI.create(config.getGerritAuthBaseUrl()
+ UriResourceLocator.gerritCommentUri(fullChangeId));
+ log.debug("Post-Comment uri: {}", uri);
String auth = generateBasicAuth(config.getGerritUserName(),
config.getGerritPassword());
- log.debug("postComment uri: {}", uri);
- log.debug("postComment json: {}", json);
+ String json = gson.toJson(map);
+ log.debug("Post-Comment JSON: {}", json);
HttpRequest request = HttpRequest.newBuilder()
.header(HttpHeaders.AUTHORIZATION, auth)
.header(HttpHeaders.CONTENT_TYPE, ContentType.APPLICATION_JSON.toString())
@@ -205,11 +203,11 @@
HttpResponse<String> response = httpClientWithRetry.execute(request);
if (response.statusCode() != HTTP_OK) {
- log.error("Review post failed with status code: {}", response.statusCode());
+ log.error("Comment posting failed with status code: {}", response.statusCode());
}
}
- public String getTaggedPrompt(HashMap<String, List<String>> filesNewContent) {
+ public String getUserPrompt(HashMap<String, List<String>> filesNewContent) {
this.filesNewContent = filesNewContent;
StringBuilder taggedPrompt = new StringBuilder();
for (int i = 0; i < commentProperties.size(); i++) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClientPatchSet.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClientPatchSet.java
index f3426ec..31c8a2e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClientPatchSet.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClientPatchSet.java
@@ -15,7 +15,7 @@
@Slf4j
public class GerritClientPatchSet extends GerritClientAccount {
- private static final String[] COMMIT_MESSAGE_FILTER_PREFIXES = {
+ private static final String[] COMMIT_MESSAGE_FILTER_OUT_PREFIXES = {
"Parent:",
"Author:",
"AuthorDate:",
@@ -61,7 +61,7 @@
private List<String> filterCommitMessageContent(List<String> fieldValue) {
fieldValue.removeIf(s ->
- s.isEmpty() || Arrays.stream(COMMIT_MESSAGE_FILTER_PREFIXES).anyMatch(s::startsWith));
+ s.isEmpty() || Arrays.stream(COMMIT_MESSAGE_FILTER_OUT_PREFIXES).anyMatch(s::startsWith));
return fieldValue;
}
@@ -98,7 +98,7 @@
add("DUMMY LINE #0");
}};
InputFileDiff inputFileDiff = gson.fromJson(fileDiffJson, InputFileDiff.class);
- // Initialize the reduced file diff with fields `meta_a` and `meta_b`
+ // Initialize the reduced output file diff with fields `meta_a` and `meta_b`
OutputFileDiff outputFileDiff = new OutputFileDiff(inputFileDiff.getMeta_a(), inputFileDiff.getMeta_b());
List<OutputFileDiff.Content> outputDiffContent = new ArrayList<>();
List<InputFileDiff.Content> inputDiffContent = inputFileDiff.getContent();
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/OpenAiClient.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/OpenAiClient.java
index ac3357a..7909d3e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/OpenAiClient.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/OpenAiClient.java
@@ -35,17 +35,17 @@
public String ask(Configuration config, String patchSet) throws Exception {
HttpRequest request = createRequest(config, patchSet);
- log.debug("request: {}", request.toString());
+ log.debug("ChatGPT request: {}", request.toString());
HttpResponse<String> response = httpClientWithRetry.execute(request);
String body = response.body();
log.debug("body: {}", body);
if (body == null) {
- throw new IOException("responseBody is null");
+ throw new IOException("ChatGPT response body is null");
}
String content = extractContent(config, body);
- log.debug("content: {}", content);
+ log.debug("ChatGPT response content: {}", content);
return content;
}
@@ -69,10 +69,10 @@
}
private HttpRequest createRequest(Configuration config, String patchSet) {
- requestBody = createRequestBody(config, patchSet);
URI uri = URI.create(URI.create(config.getGptDomain()) + UriResourceLocator.chatCompletionsUri());
- log.info("GPT request URI: {}", uri);
- log.info("GPT requestBody: {}", requestBody);
+ log.info("ChatGPT request URI: {}", uri);
+ requestBody = createRequestBody(config, patchSet);
+ log.info("ChatGPT request body: {}", requestBody);
return HttpRequest.newBuilder()
.header(HttpHeaders.AUTHORIZATION, "Bearer " + config.getGptToken())
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 231ed9a..9997b9b 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
@@ -14,8 +14,8 @@
public static final String OPENAI_DOMAIN = "https://api.openai.com";
public static final String DEFAULT_GPT_MODEL = "gpt-3.5-turbo";
- public static final String DEFAULT_GPT_SYSTEM_PROMPT = "Act as a Patchset Reviewer. I will provide you with " +
- "Patchset Diffs for various files in a JSON format. Each changed file's content will be detailed in the " +
+ public static final String DEFAULT_GPT_SYSTEM_PROMPT = "Act as a PatchSet Reviewer. I will provide you with " +
+ "PatchSet Diffs for various files in a JSON format. Each changed file's content will be detailed in the " +
"\"content\" field of the JSON object. In this \"content\", the \"a\" items are the lines removed, the " +
"\"b\" items are the lines added, and the \"ab\" items are the unchanged lines. In your response, avoid " +
"explicitly referring to the \"a\", \"b\", and other fields from the JSON object. Instead, use more " +
@@ -24,15 +24,15 @@
public static final String DEFAULT_GPT_USER_PROMPT = "Focus your review on the \"a\" and \"b\" items, but use " +
"the \"ab\" items as context to understand the changes better. Provide insights on whether the changes " +
"make sense, any potential issues you foresee, and suggestions for improvements if necessary.\n";
- public static final String DEFAULT_GPT_CUSTOM_USER_PROMPT_1 = "I have some requests about the following Patchset " +
+ public static final String DEFAULT_GPT_CUSTOM_USER_PROMPT_1 = "I have some requests about the following PatchSet " +
"Diff:\n";
public static final String DEFAULT_GPT_CUSTOM_USER_PROMPT_2 = "Here are my requests:\n";
public static final String DEFAULT_GPT_CUSTOM_USER_CONTEXT_PROMPT = "In reference to the code `%s` (from line %d " +
"of file \"%s\"), ";
public static final String DEFAULT_GPT_COMMIT_MESSAGES_REVIEW_USER_PROMPT = "Also, perform a check on the commit " +
- "message of the Patchset. The commit message is provided in the \"content\" field of \"/COMMIT_MSG\" in " +
+ "message of the PatchSet. The commit message is provided in the \"content\" field of \"/COMMIT_MSG\" in " +
"the same way as the file changes. Ensure that the commit message accurately and succinctly describes the " +
- "changes made, and verify if it matches the nature and scope of the changes in the Patchset.";
+ "changes made, and verify if it matches the nature and scope of the changes in the PatchSet.";
public static final String NOT_CONFIGURED_ERROR_MSG = "%s is not configured";
public static final String KEY_GPT_SYSTEM_PROMPT = "gptSystemPrompt";
public static final String KEY_GPT_USER_PROMPT = "gptUserPrompt";
@@ -92,7 +92,7 @@
private static final String KEY_GPT_TEMPERATURE = "gptTemperature";
private static final String KEY_STREAM_OUTPUT = "gptStreamOutput";
private static final String KEY_REVIEW_COMMIT_MESSAGES = "gptReviewCommitMessages";
- private static final String KEY_REVIEW_PATCHSET = "gptReviewPatchset";
+ private static final String KEY_REVIEW_PATCHSET = "gptReviewPatchSet";
private static final String KEY_FULL_FILE_REVIEW = "gptFullFileReview";
private static final String KEY_PROJECT_ENABLE = "isEnabled";
private static final String KEY_GLOBAL_ENABLE = "globalEnable";
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/listener/EventListenerHandler.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/listener/EventListenerHandler.java
index 8cea753..60ac1c8 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/listener/EventListenerHandler.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/listener/EventListenerHandler.java
@@ -95,7 +95,7 @@
String topic = getTopic(patchSetEvent).orElse("");
log.debug("PatchSet Topic retrieved: '{}'", topic);
if (gerritClient.isDisabledTopic(topic)) {
- log.info("Disabled review for Patchsets with Topic '{}'", topic);
+ log.info("Disabled review for PatchSets with Topic '{}'", topic);
return false;
}
@@ -104,7 +104,7 @@
private boolean isPatchSetReviewEnabled(PatchSetEvent patchSetEvent) {
if (!config.getGptReviewPatchSet()) {
- log.debug("Disabled review function for created or updated Patchsets.");
+ log.info("Disabled review function for created or updated PatchSets.");
return false;
}
Optional<PatchSetAttribute> patchSetAttributeOptional = getPatchSetAttribute(patchSetEvent);
@@ -120,7 +120,7 @@
}
String authorUsername = patchSetAttribute.author.username;
if (gerritClient.isDisabledUser(authorUsername)) {
- log.info("Review of Patchsets from user '{}' is disabled.", authorUsername);
+ log.info("Review of PatchSets from user '{}' is disabled.", authorUsername);
return false;
}
return true;
@@ -150,7 +150,7 @@
break;
case "comment-added":
if (!gerritClient.retrieveLastComments(event, fullChangeId)) {
- log.info("Found no comment to review");
+ log.info("No comments found for review");
return;
}
break;
diff --git a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewTest.java b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewTest.java
index 9c37ce5..9acde44 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewTest.java
@@ -280,9 +280,7 @@
GerritListener gerritListener = new GerritListener(mockConfigCreator, eventListenerHandler);
gerritListener.onEvent(event);
CompletableFuture<Void> future = eventListenerHandler.getLatestFuture();
- Assert.assertThrows(NullPointerException.class, () -> {
- future.get();
- });
+ Assert.assertThrows(NullPointerException.class, () -> future.get());
}
