Skip to content

Commit 30f3cca

Browse files
authored
fix(amazonq): fix UI freeze when editing large files (#5685)
1 parent 24e7749 commit 30f3cca

File tree

3 files changed

+129
-101
lines changed

3 files changed

+129
-101
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type" : "bugfix",
3+
"description" : "Fix UI freezes that may occur when interacting with large files in the editor"
4+
}

plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/textdocument/TextDocumentServiceHandler.kt

Lines changed: 60 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@
44
package software.aws.toolkits.jetbrains.services.amazonq.lsp.textdocument
55

66
import com.intellij.openapi.Disposable
7+
import com.intellij.openapi.application.ApplicationManager
78
import com.intellij.openapi.editor.Document
9+
import com.intellij.openapi.editor.event.DocumentEvent
10+
import com.intellij.openapi.editor.event.DocumentListener
811
import com.intellij.openapi.fileEditor.FileDocumentManager
912
import com.intellij.openapi.fileEditor.FileDocumentManagerListener
1013
import com.intellij.openapi.fileEditor.FileEditorManager
@@ -29,10 +32,11 @@ import software.aws.toolkits.jetbrains.utils.pluginAwareExecuteOnPooledThread
2932

3033
class TextDocumentServiceHandler(
3134
private val project: Project,
32-
serverInstance: Disposable,
35+
private val serverInstance: Disposable,
3336
) : FileDocumentManagerListener,
3437
FileEditorManagerListener,
35-
BulkFileListener {
38+
BulkFileListener,
39+
DocumentListener {
3640

3741
init {
3842
// didOpen & didClose events
@@ -61,18 +65,30 @@ class TextDocumentServiceHandler(
6165
}
6266

6367
private fun handleFileOpened(file: VirtualFile) {
68+
ApplicationManager.getApplication().runReadAction {
69+
FileDocumentManager.getInstance().getDocument(file)?.addDocumentListener(
70+
object : DocumentListener {
71+
override fun documentChanged(event: DocumentEvent) {
72+
realTimeEdit(event)
73+
}
74+
},
75+
serverInstance
76+
)
77+
}
6478
AmazonQLspService.executeIfRunning(project) { languageServer ->
6579
toUriString(file)?.let { uri ->
66-
languageServer.textDocumentService.didOpen(
67-
DidOpenTextDocumentParams().apply {
68-
textDocument = TextDocumentItem().apply {
69-
this.uri = uri
70-
text = file.inputStream.readAllBytes().decodeToString()
71-
languageId = file.fileType.name.lowercase()
72-
version = file.modificationStamp.toInt()
80+
pluginAwareExecuteOnPooledThread {
81+
languageServer.textDocumentService.didOpen(
82+
DidOpenTextDocumentParams().apply {
83+
textDocument = TextDocumentItem().apply {
84+
this.uri = uri
85+
text = file.inputStream.readAllBytes().decodeToString()
86+
languageId = file.fileType.name.lowercase()
87+
version = file.modificationStamp.toInt()
88+
}
7389
}
74-
}
75-
)
90+
)
91+
}
7692
}
7793
}
7894
}
@@ -81,14 +97,16 @@ class TextDocumentServiceHandler(
8197
AmazonQLspService.executeIfRunning(project) { languageServer ->
8298
val file = FileDocumentManager.getInstance().getFile(document) ?: return@executeIfRunning
8399
toUriString(file)?.let { uri ->
84-
languageServer.textDocumentService.didSave(
85-
DidSaveTextDocumentParams().apply {
86-
textDocument = TextDocumentIdentifier().apply {
87-
this.uri = uri
100+
pluginAwareExecuteOnPooledThread {
101+
languageServer.textDocumentService.didSave(
102+
DidSaveTextDocumentParams().apply {
103+
textDocument = TextDocumentIdentifier().apply {
104+
this.uri = uri
105+
}
106+
text = document.text
88107
}
89-
text = document.text
90-
}
91-
)
108+
)
109+
}
92110
}
93111
}
94112
}
@@ -141,4 +159,28 @@ class TextDocumentServiceHandler(
141159
}
142160
}
143161
}
162+
163+
private fun realTimeEdit(event: DocumentEvent) {
164+
AmazonQLspService.executeIfRunning(project) { languageServer ->
165+
pluginAwareExecuteOnPooledThread {
166+
val vFile = FileDocumentManager.getInstance().getFile(event.document) ?: return@pluginAwareExecuteOnPooledThread
167+
toUriString(vFile)?.let { uri ->
168+
languageServer.textDocumentService.didChange(
169+
DidChangeTextDocumentParams().apply {
170+
textDocument = VersionedTextDocumentIdentifier().apply {
171+
this.uri = uri
172+
version = event.document.modificationStamp.toInt()
173+
}
174+
contentChanges = listOf(
175+
TextDocumentContentChangeEvent().apply {
176+
text = event.document.text
177+
}
178+
)
179+
}
180+
)
181+
}
182+
}
183+
}
184+
// Process document changes here
185+
}
144186
}

plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/textdocument/TextDocumentServiceHandlerTest.kt

Lines changed: 65 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -3,29 +3,26 @@
33

44
package software.aws.toolkits.jetbrains.services.amazonq.lsp.textdocument
55

6-
import com.intellij.openapi.Disposable
7-
import com.intellij.openapi.application.Application
8-
import com.intellij.openapi.application.ApplicationManager
9-
import com.intellij.openapi.components.serviceIfCreated
6+
import com.intellij.openapi.application.writeAction
107
import com.intellij.openapi.editor.Document
118
import com.intellij.openapi.fileEditor.FileDocumentManager
12-
import com.intellij.openapi.fileEditor.FileEditorManager
139
import com.intellij.openapi.fileTypes.FileType
14-
import com.intellij.openapi.project.Project
1510
import com.intellij.openapi.vfs.VirtualFile
1611
import com.intellij.openapi.vfs.newvfs.events.VFileContentChangeEvent
1712
import com.intellij.openapi.vfs.newvfs.events.VFileEvent
18-
import com.intellij.util.messages.MessageBus
19-
import com.intellij.util.messages.MessageBusConnection
13+
import com.intellij.openapi.vfs.writeText
14+
import com.intellij.testFramework.DisposableRule
15+
import com.intellij.testFramework.fixtures.CodeInsightTestFixture
16+
import com.intellij.testFramework.fixtures.IdeaTestFixtureFactory
17+
import com.intellij.testFramework.replaceService
2018
import io.mockk.every
21-
import io.mockk.just
2219
import io.mockk.mockk
2320
import io.mockk.mockkObject
2421
import io.mockk.mockkStatic
25-
import io.mockk.runs
2622
import io.mockk.slot
2723
import io.mockk.verify
2824
import kotlinx.coroutines.test.runTest
25+
import kotlinx.coroutines.withContext
2926
import org.assertj.core.api.Assertions.assertThat
3027
import org.eclipse.lsp4j.DidChangeTextDocumentParams
3128
import org.eclipse.lsp4j.DidCloseTextDocumentParams
@@ -34,42 +31,51 @@ import org.eclipse.lsp4j.DidSaveTextDocumentParams
3431
import org.eclipse.lsp4j.jsonrpc.messages.ResponseMessage
3532
import org.eclipse.lsp4j.services.TextDocumentService
3633
import org.junit.Before
34+
import org.junit.Rule
3735
import org.junit.Test
36+
import software.aws.toolkits.jetbrains.core.coroutines.EDT
3837
import software.aws.toolkits.jetbrains.services.amazonq.lsp.AmazonQLanguageServer
3938
import software.aws.toolkits.jetbrains.services.amazonq.lsp.AmazonQLspService
4039
import software.aws.toolkits.jetbrains.services.amazonq.lsp.util.FileUriUtil
40+
import software.aws.toolkits.jetbrains.utils.rules.CodeInsightTestFixtureRule
41+
import software.aws.toolkits.jetbrains.utils.satisfiesKt
4142
import java.net.URI
4243
import java.nio.file.Path
43-
import java.util.concurrent.Callable
4444
import java.util.concurrent.CompletableFuture
45+
import kotlin.collections.first
4546

4647
class TextDocumentServiceHandlerTest {
47-
private lateinit var project: Project
48-
private lateinit var mockFileEditorManager: FileEditorManager
4948
private lateinit var mockLanguageServer: AmazonQLanguageServer
5049
private lateinit var mockTextDocumentService: TextDocumentService
5150
private lateinit var sut: TextDocumentServiceHandler
52-
private lateinit var mockApplication: Application
51+
52+
@get:Rule
53+
val projectRule = object : CodeInsightTestFixtureRule() {
54+
override fun createTestFixture(): CodeInsightTestFixture {
55+
val fixtureFactory = IdeaTestFixtureFactory.getFixtureFactory()
56+
val fixtureBuilder = fixtureFactory.createLightFixtureBuilder(testDescription, testName)
57+
val newFixture = fixtureFactory
58+
.createCodeInsightFixture(fixtureBuilder.fixture, fixtureFactory.createTempDirTestFixture())
59+
newFixture.setUp()
60+
newFixture.testDataPath = testDataPath
61+
62+
return newFixture
63+
}
64+
}
65+
66+
@get:Rule
67+
val disposableRule = DisposableRule()
5368

5469
@Before
5570
fun setup() {
56-
project = mockk<Project>()
5771
mockTextDocumentService = mockk<TextDocumentService>()
5872
mockLanguageServer = mockk<AmazonQLanguageServer>()
5973

60-
mockApplication = mockk<Application>()
61-
mockkStatic(ApplicationManager::class)
62-
every { ApplicationManager.getApplication() } returns mockApplication
63-
every { mockApplication.executeOnPooledThread(any<Callable<*>>()) } answers {
64-
CompletableFuture.completedFuture(firstArg<Callable<*>>().call())
65-
}
66-
6774
// Mock the LSP service
68-
val mockLspService = mockk<AmazonQLspService>()
75+
val mockLspService = mockk<AmazonQLspService>(relaxed = true)
6976

7077
// Mock the service methods on Project
71-
every { project.getService(AmazonQLspService::class.java) } returns mockLspService
72-
every { project.serviceIfCreated<AmazonQLspService>() } returns mockLspService
78+
projectRule.project.replaceService(AmazonQLspService::class.java, mockLspService, disposableRule.disposable)
7379

7480
// Mock the LSP service's executeSync method as a suspend function
7581
every {
@@ -86,19 +92,7 @@ class TextDocumentServiceHandlerTest {
8692
every { mockTextDocumentService.didOpen(any()) } returns Unit
8793
every { mockTextDocumentService.didClose(any()) } returns Unit
8894

89-
// Mock message bus
90-
val messageBus = mockk<MessageBus>()
91-
every { project.messageBus } returns messageBus
92-
val mockConnection = mockk<MessageBusConnection>()
93-
every { messageBus.connect(any<Disposable>()) } returns mockConnection
94-
every { mockConnection.subscribe(any(), any()) } just runs
95-
96-
// Mock FileEditorManager
97-
mockFileEditorManager = mockk<FileEditorManager>()
98-
every { mockFileEditorManager.openFiles } returns emptyArray()
99-
every { project.getService(FileEditorManager::class.java) } returns mockFileEditorManager
100-
101-
sut = TextDocumentServiceHandler(project, mockk())
95+
sut = TextDocumentServiceHandler(projectRule.project, mockk())
10296
}
10397

10498
@Test
@@ -136,41 +130,39 @@ class TextDocumentServiceHandlerTest {
136130

137131
@Test
138132
fun `didOpen runs on service init`() = runTest {
139-
val uri = URI.create("file:///test/path/file.txt")
140133
val content = "test content"
141-
val file = createMockVirtualFile(uri, content)
142-
143-
every { mockFileEditorManager.openFiles } returns arrayOf(file)
134+
val file = withContext(EDT) {
135+
projectRule.fixture.createFile("name", content).also { projectRule.fixture.openFileInEditor(it) }
136+
}
144137

145-
sut = TextDocumentServiceHandler(project, mockk())
138+
sut = TextDocumentServiceHandler(projectRule.project, mockk())
146139

147-
val paramsSlot = slot<DidOpenTextDocumentParams>()
140+
val paramsSlot = mutableListOf<DidOpenTextDocumentParams>()
148141
verify { mockTextDocumentService.didOpen(capture(paramsSlot)) }
149142

150-
with(paramsSlot.captured.textDocument) {
151-
assertThat(this.uri).isEqualTo(normalizeFileUri(uri.toString()))
152-
assertThat(text).isEqualTo(content)
153-
assertThat(languageId).isEqualTo("java")
154-
assertThat(version).isEqualTo(1)
143+
assertThat(paramsSlot.first().textDocument).satisfiesKt {
144+
assertThat(it.uri).isEqualTo(file.toNioPath().toUri().toString())
145+
assertThat(it.text).isEqualTo(content)
146+
assertThat(it.languageId).isEqualTo("plain_text")
155147
}
156148
}
157149

158150
@Test
159151
fun `didOpen runs on fileOpened`() = runTest {
160-
val uri = URI.create("file:///test/path/file.txt")
161152
val content = "test content"
162-
val file = createMockVirtualFile(uri, content)
153+
val file = withContext(EDT) {
154+
projectRule.fixture.createFile("name", content).also { projectRule.fixture.openFileInEditor(it) }
155+
}
163156

164157
sut.fileOpened(mockk(), file)
165158

166-
val paramsSlot = slot<DidOpenTextDocumentParams>()
159+
val paramsSlot = mutableListOf<DidOpenTextDocumentParams>()
167160
verify { mockTextDocumentService.didOpen(capture(paramsSlot)) }
168161

169-
with(paramsSlot.captured.textDocument) {
170-
assertThat(this.uri).isEqualTo(normalizeFileUri(uri.toString()))
171-
assertThat(text).isEqualTo(content)
172-
assertThat(languageId).isEqualTo("java")
173-
assertThat(version).isEqualTo(1)
162+
assertThat(paramsSlot.first().textDocument).satisfiesKt {
163+
assertThat(it.uri).isEqualTo(file.toNioPath().toUri().toString())
164+
assertThat(it.text).isEqualTo(content)
165+
assertThat(it.languageId).isEqualTo("plain_text")
174166
}
175167
}
176168

@@ -189,38 +181,23 @@ class TextDocumentServiceHandlerTest {
189181

190182
@Test
191183
fun `didChange runs on content change events`() = runTest {
192-
val uri = URI.create("file:///test/path/file.txt")
193-
val document = mockk<Document> {
194-
every { text } returns "changed content"
195-
every { modificationStamp } returns 123L
196-
}
197-
198-
val file = createMockVirtualFile(uri)
199-
200-
val changeEvent = mockk<VFileContentChangeEvent> {
201-
every { this@mockk.file } returns file
202-
}
203-
204-
// Mock FileDocumentManager
205-
val fileDocumentManager = mockk<FileDocumentManager> {
206-
every { getCachedDocument(file) } returns document
207-
}
184+
val file = withContext(EDT) {
185+
projectRule.fixture.createFile("name", "").also {
186+
projectRule.fixture.openFileInEditor(it)
208187

209-
mockkStatic(FileDocumentManager::class) {
210-
every { FileDocumentManager.getInstance() } returns fileDocumentManager
211-
212-
// Call the handler method
213-
sut.after(mutableListOf(changeEvent))
188+
writeAction {
189+
it.writeText("changed content")
190+
}
191+
}
214192
}
215193

216194
// Verify the correct LSP method was called with matching parameters
217-
val paramsSlot = slot<DidChangeTextDocumentParams>()
195+
val paramsSlot = mutableListOf<DidChangeTextDocumentParams>()
218196
verify { mockTextDocumentService.didChange(capture(paramsSlot)) }
219197

220-
with(paramsSlot.captured) {
221-
assertThat(textDocument.uri).isEqualTo(normalizeFileUri(uri.toString()))
222-
assertThat(textDocument.version).isEqualTo(123)
223-
assertThat(contentChanges[0].text).isEqualTo("changed content")
198+
assertThat(paramsSlot.first()).satisfiesKt {
199+
assertThat(it.textDocument.uri).isEqualTo(file.toNioPath().toUri().toString())
200+
assertThat(it.contentChanges[0].text).isEqualTo("changed content")
224201
}
225202
}
226203

@@ -335,6 +312,11 @@ class TextDocumentServiceHandlerTest {
335312
return uri
336313
}
337314

315+
if (uri.startsWith("file://C:/")) {
316+
val path = uri.substringAfter("file://C:/")
317+
return "file:///C:/$path"
318+
}
319+
338320
val path = uri.substringAfter("file:///")
339321
return "file:///C:/$path"
340322
}

0 commit comments

Comments
 (0)