Skip to content

Commit cdc4564

Browse files
rlirli
committed
chore: cleanup/reduce de/ser cycles introduced in #5970 
1 parent 2d5ca0b commit cdc4564

File tree

2 files changed

+128
-87
lines changed

2 files changed

+128
-87
lines changed

‎plugins/amazonq/chat/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/webview/BrowserConnector.kt‎

Lines changed: 40 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package software.aws.toolkits.jetbrains.services.amazonq.webview
55

66
import com.fasterxml.jackson.databind.JsonNode
77
import com.fasterxml.jackson.databind.node.ObjectNode
8+
import com.fasterxml.jackson.module.kotlin.readValue
89
import com.fasterxml.jackson.module.kotlin.treeToValue
910
import com.google.gson.Gson
1011
import com.intellij.ide.BrowserUtil
@@ -34,6 +35,7 @@ import org.cef.browser.CefBrowser
3435
import org.eclipse.lsp4j.TextDocumentIdentifier
3536
import org.eclipse.lsp4j.jsonrpc.ResponseErrorException
3637
import org.eclipse.lsp4j.jsonrpc.messages.ResponseErrorCode
38+
import org.intellij.lang.annotations.Language
3739
import software.aws.toolkits.core.utils.error
3840
import software.aws.toolkits.core.utils.getLogger
3941
import software.aws.toolkits.core.utils.info
@@ -581,6 +583,21 @@ class BrowserConnector(
581583
}
582584
}
583585

586+
/**
587+
* Solely used to extract the aggregated findings from the response
588+
*/
589+
// TODO: move to software.aws.toolkits.jetbrains.services.amazonq.lsp.model
590+
data class FlareAdditionalMessages(
591+
val additionalMessages: List<FlareAggregatedFindings>?,
592+
)
593+
594+
// TODO: move to software.aws.toolkits.jetbrains.services.amazonq.lsp.model
595+
data class FlareAggregatedFindings(
596+
val messageId: String,
597+
val body: List<AggregatedCodeScanIssue>,
598+
)
599+
600+
// TODO: move to software.aws.toolkits.jetbrains.services.amazonq.lsp.model
584601
data class FlareCodeScanIssue(
585602
val startLine: Int,
586603
val endLine: Int,
@@ -602,6 +619,7 @@ class BrowserConnector(
602619
val findingContext: String,
603620
)
604621

622+
// TODO: move to software.aws.toolkits.jetbrains.services.amazonq.lsp.model
605623
data class AggregatedCodeScanIssue(
606624
val filePath: String,
607625
val issues: List<FlareCodeScanIssue>,
@@ -620,14 +638,13 @@ class BrowserConnector(
620638
throw error
621639
}
622640
chatCommunicationManager.removePartialChatMessage(partialResultToken)
623-
val decryptedMessage = Gson().fromJson(value?.let { encryptionManager?.decrypt(it) }.orEmpty(), Map::class.java)
624-
as Map<String, *>
641+
val decryptedMessage = value?.let { encryptionManager?.decrypt(it) }.orEmpty()
625642
parseFindingsMessages(decryptedMessage)
626643

627644
val messageToChat = ChatCommunicationManager.convertToJsonToSendToChat(
628645
SEND_CHAT_COMMAND_PROMPT,
629646
tabId,
630-
Gson().toJson(decryptedMessage),
647+
decryptedMessage,
631648
isPartialResult = false
632649
)
633650
browser.postChat(messageToChat)
@@ -641,26 +658,18 @@ class BrowserConnector(
641658
}
642659
}
643660

644-
fun parseFindingsMessages(messagesMap: Map<String, *>) {
661+
fun parseFindingsMessages(@Language("JSON") responsePayload: String) {
645662
try {
646-
val additionalMessages = messagesMap["additionalMessages"] as? MutableList<Map<String, Any>>
663+
val additionalMessages = serializer.objectMapper.readValue<FlareAdditionalMessages>(responsePayload).additionalMessages
647664
val findingsMessages = additionalMessages?.filter { message ->
648-
if (message.contains("messageId")) {
649-
(message["messageId"] as String).endsWith(CODE_REVIEW_FINDINGS_SUFFIX) ||
650-
(message["messageId"] as String).endsWith(DISPLAY_FINDINGS_SUFFIX)
651-
} else {
652-
false
653-
}
654-
}
665+
message.messageId.endsWith(CODE_REVIEW_FINDINGS_SUFFIX) ||
666+
message.messageId.endsWith(DISPLAY_FINDINGS_SUFFIX)
667+
} ?: return
668+
655669
val scannedFiles = mutableListOf<VirtualFile>()
656-
if (findingsMessages != null) {
670+
val mappedFindings = buildList {
657671
for (findingsMessage in findingsMessages) {
658-
additionalMessages.remove(findingsMessage)
659-
val gson = Gson()
660-
val jsonFindings = gson.fromJson(findingsMessage["body"] as String, List::class.java)
661-
val mappedFindings = mutableListOf<CodeWhispererCodeScanIssue>()
662-
for (aggregatedIssueUnformatted in jsonFindings) {
663-
val aggregatedIssue = gson.fromJson(gson.toJson(aggregatedIssueUnformatted), AggregatedCodeScanIssue::class.java)
672+
for (aggregatedIssue in findingsMessage.body) {
664673
val file = LocalFileSystem.getInstance().findFileByIoFile(
665674
Path.of(aggregatedIssue.filePath).toFile()
666675
)
@@ -677,7 +686,8 @@ class BrowserConnector(
677686
if (isIssueIgnored) {
678687
continue
679688
}
680-
mappedFindings.add(
689+
690+
add(
681691
CodeWhispererCodeScanIssue(
682692
startLine = issue.startLine,
683693
startCol = 1,
@@ -705,16 +715,18 @@ class BrowserConnector(
705715
}
706716
}
707717
}
708-
709-
CodeWhispererCodeScanManager.getInstance(project)
710-
.addOnDemandIssues(
711-
mappedFindings,
712-
scannedFiles,
713-
CodeWhispererConstants.CodeAnalysisScope.AGENTIC
714-
)
715-
CodeWhispererCodeScanManager.getInstance(project).showCodeScanUI()
716718
}
717719
}
720+
721+
if (mappedFindings.isNotEmpty()) {
722+
CodeWhispererCodeScanManager.getInstance(project)
723+
.addOnDemandIssues(
724+
mappedFindings,
725+
scannedFiles,
726+
CodeWhispererConstants.CodeAnalysisScope.AGENTIC
727+
)
728+
CodeWhispererCodeScanManager.getInstance(project).showCodeScanUI()
729+
}
718730
} catch (e: Exception) {
719731
LOG.error { "Failed to parse findings message $e" }
720732
}

‎plugins/amazonq/chat/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/webview/BrowserConnectorTest.kt‎

Lines changed: 88 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -53,54 +53,76 @@ class BrowserConnectorTest : AmazonQTestBase() {
5353
}
5454

5555
@Test
56-
fun `parseFindingsMessages should handle empty additionalMessages`() {
57-
val messagesMap = mapOf<String, Any>()
58-
59-
browserConnector.parseFindingsMessages(messagesMap)
56+
fun `parseFindingsMessages should handle no additionalMessages`() {
57+
browserConnector.parseFindingsMessages("""""")
6058

6159
verify(mockCodeScanManager, never()).addOnDemandIssues(any(), any(), any())
6260
}
6361

6462
@Test
6563
fun `parseFindingsMessages should handle null additionalMessages`() {
66-
val messagesMap = mapOf("additionalMessages" to null)
67-
68-
browserConnector.parseFindingsMessages(messagesMap)
64+
browserConnector.parseFindingsMessages(
65+
"""
66+
{
67+
"additionalMessages": null
68+
}
69+
""".trimIndent()
70+
)
6971

7072
verify(mockCodeScanManager, never()).addOnDemandIssues(any(), any(), any())
7173
}
7274

7375
@Test
74-
fun `parseFindingsMessages should filter messages with CODE_REVIEW_FINDINGS_SUFFIX`() {
75-
val findingsMessage = mapOf(
76-
"messageId" to "test_codeReviewFindings",
77-
"body" to """[{"filePath": "/test/file.kt", "issues": []}]"""
78-
)
79-
val otherMessage = mapOf(
80-
"messageId" to "other_message",
81-
"body" to "other content"
76+
fun `parseFindingsMessages should handle empty additionalMessages`() {
77+
browserConnector.parseFindingsMessages(
78+
"""
79+
{
80+
"additionalMessages": []
81+
}
82+
""".trimIndent()
8283
)
83-
val additionalMessages = mutableListOf<Map<String, Any>>(findingsMessage, otherMessage)
84-
val messagesMap = mapOf("additionalMessages" to additionalMessages)
8584

86-
browserConnector.parseFindingsMessages(messagesMap)
85+
verify(mockCodeScanManager, never()).addOnDemandIssues(any(), any(), any())
86+
}
8787

88-
assertThat(additionalMessages.size == 1)
89-
assertThat(additionalMessages[0] == otherMessage)
88+
@Test
89+
fun `parseFindingsMessages should filter messages with CODE_REVIEW_FINDINGS_SUFFIX`() {
90+
val findingsMessage = """
91+
{
92+
"additionalMessages": [
93+
{
94+
"messageId": "test_codeReviewFindings",
95+
"body": [{"filePath": "/test/file.kt", "issues": []}]
96+
},
97+
{
98+
"messageId": "other_message",
99+
"body": "other content"
100+
}
101+
]
102+
}
103+
""".trimIndent()
104+
105+
browserConnector.parseFindingsMessages(findingsMessage)
90106
}
91107

92108
@Test
93109
fun `parseFindingsMessages should filter messages with DISPLAY_FINDINGS_SUFFIX`() {
94-
val findingsMessage = mapOf(
95-
"messageId" to "test_displayFindings",
96-
"body" to """[{"filePath": "/test/file.kt", "issues": []}]"""
97-
)
98-
val additionalMessages = mutableListOf<Map<String, Any>>(findingsMessage)
99-
val messagesMap = mapOf("additionalMessages" to additionalMessages)
100-
101-
browserConnector.parseFindingsMessages(messagesMap)
110+
val findingsMessage = """
111+
{
112+
"additionalMessages": [
113+
{
114+
"messageId": "test_displayFindings",
115+
"body": [{"filePath": "/test/file.kt", "issues": []}]
116+
},
117+
{
118+
"messageId": "other_message",
119+
"body": "other content"
120+
}
121+
]
122+
}
123+
""".trimIndent()
102124

103-
assertThat(additionalMessages).isEmpty()
125+
browserConnector.parseFindingsMessages(findingsMessage)
104126
}
105127

106128
@Test
@@ -135,14 +157,18 @@ class BrowserConnectorTest : AmazonQTestBase() {
135157
)
136158

137159
val aggregatedIssue = BrowserConnector.AggregatedCodeScanIssue("/test/file.kt", listOf(issue))
138-
val findingsMessage = mapOf(
139-
"messageId" to "test_codeReviewFindings",
140-
"body" to jacksonObjectMapper().writeValueAsString(listOf(aggregatedIssue))
141-
)
142-
val additionalMessages = mutableListOf<Map<String, Any>>(findingsMessage)
143-
val messagesMap = mapOf("additionalMessages" to additionalMessages)
144-
145-
browserConnector.parseFindingsMessages(messagesMap)
160+
val findingsMessage = """
161+
{
162+
"additionalMessages": [
163+
{
164+
"messageId": "test_codeReviewFindings",
165+
"body": ${jacksonObjectMapper().writeValueAsString(listOf(aggregatedIssue))}
166+
}
167+
]
168+
}
169+
""".trimIndent()
170+
171+
browserConnector.parseFindingsMessages(findingsMessage)
146172

147173
val issuesCaptor = argumentCaptor<List<CodeWhispererCodeScanIssue>>()
148174
verify(mockCodeScanManager).addOnDemandIssues(
@@ -151,7 +177,6 @@ class BrowserConnectorTest : AmazonQTestBase() {
151177
eq(CodeWhispererConstants.CodeAnalysisScope.AGENTIC)
152178
)
153179

154-
assertThat(additionalMessages).isEmpty()
155180
assertThat(issuesCaptor.firstValue.isNotEmpty())
156181
assertThat(issuesCaptor.firstValue[0].title == "Test Issue")
157182
}
@@ -177,38 +202,42 @@ class BrowserConnectorTest : AmazonQTestBase() {
177202
)
178203

179204
val aggregatedIssue = BrowserConnector.AggregatedCodeScanIssue("/test/directory", listOf(issue))
180-
val findingsMessage = mapOf(
181-
"messageId" to "test_displayFindings",
182-
"body" to jacksonObjectMapper().writeValueAsString(listOf(aggregatedIssue))
183-
)
184-
val additionalMessages = mutableListOf<Map<String, Any>>(findingsMessage)
185-
val messagesMap = mapOf("additionalMessages" to additionalMessages)
205+
val findingsMessage = """
206+
{
207+
"additionalMessages": [
208+
{
209+
"messageId": "test_displayFindings",
210+
"body": ${jacksonObjectMapper().writeValueAsString(listOf(aggregatedIssue))}
211+
}
212+
]
213+
}
214+
""".trimIndent()
186215

187-
browserConnector.parseFindingsMessages(messagesMap)
216+
browserConnector.parseFindingsMessages(findingsMessage)
188217

189-
val issuesCaptor = argumentCaptor<List<CodeWhispererCodeScanIssue>>()
190-
verify(mockCodeScanManager).addOnDemandIssues(
191-
issuesCaptor.capture(),
218+
verify(mockCodeScanManager, never()).addOnDemandIssues(
219+
any(),
192220
any(),
193221
eq(CodeWhispererConstants.CodeAnalysisScope.AGENTIC)
194222
)
195-
196-
assertThat(issuesCaptor.firstValue.isEmpty())
197-
assertThat(additionalMessages).isEmpty()
198223
}
199224
}
200225

201226
@Test
202227
fun `parseFindingsMessages should handle invalid JSON gracefully`() {
203-
val findingsMessage = mapOf(
204-
"messageId" to "test_codeReviewFindings",
205-
"body" to "invalid json"
206-
)
207-
val additionalMessages = mutableListOf<Map<String, Any>>(findingsMessage)
208-
val messagesMap = mapOf("additionalMessages" to additionalMessages)
228+
val findingsMessage = """
229+
{
230+
"additionalMessages": [
231+
{
232+
"messageId": "test_codeReviewFindings",
233+
"body": "invalid json"
234+
}
235+
]
236+
}
237+
""".trimIndent()
209238

210-
browserConnector.parseFindingsMessages(messagesMap)
239+
browserConnector.parseFindingsMessages(findingsMessage)
211240

212-
assertThat(additionalMessages).isEmpty()
241+
verify(mockCodeScanManager, never()).addOnDemandIssues(any(), any(), any())
213242
}
214243
}

0 commit comments

Comments
 (0)