Skip to content

Commit aa3b21b

Browse files
samgst-amazonleigaol
authored andcommitted
fix(amazonq): capture more file events for workspace/ messages (aws#5474)
* capture more events for didCreate * Emit correct data for move and copy events * detekt
1 parent f9c0360 commit aa3b21b

File tree

2 files changed

+218
-48
lines changed

2 files changed

+218
-48
lines changed

plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/workspace/WorkspaceServiceHandler.kt

Lines changed: 78 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,11 @@ import com.intellij.openapi.roots.ModuleRootListener
1010
import com.intellij.openapi.vfs.VirtualFile
1111
import com.intellij.openapi.vfs.VirtualFileManager
1212
import com.intellij.openapi.vfs.newvfs.BulkFileListener
13+
import com.intellij.openapi.vfs.newvfs.events.VFileCopyEvent
1314
import com.intellij.openapi.vfs.newvfs.events.VFileCreateEvent
1415
import com.intellij.openapi.vfs.newvfs.events.VFileDeleteEvent
1516
import com.intellij.openapi.vfs.newvfs.events.VFileEvent
17+
import com.intellij.openapi.vfs.newvfs.events.VFileMoveEvent
1618
import com.intellij.openapi.vfs.newvfs.events.VFilePropertyChangeEvent
1719
import org.eclipse.lsp4j.CreateFilesParams
1820
import org.eclipse.lsp4j.DeleteFilesParams
@@ -59,10 +61,24 @@ class WorkspaceServiceHandler(
5961
private fun didCreateFiles(events: List<VFileEvent>) {
6062
AmazonQLspService.executeIfRunning(project) { languageServer ->
6163
val validFiles = events.mapNotNull { event ->
62-
val file = event.file?.takeIf { shouldHandleFile(it) } ?: return@mapNotNull null
63-
toUriString(file)?.let { uri ->
64-
FileCreate().apply {
65-
this.uri = uri
64+
when (event) {
65+
is VFileCopyEvent -> {
66+
val newFile = event.newParent.findChild(event.newChildName)?.takeIf { shouldHandleFile(it) }
67+
?: return@mapNotNull null
68+
toUriString(newFile)?.let { uri ->
69+
FileCreate().apply {
70+
this.uri = uri
71+
}
72+
}
73+
}
74+
else -> {
75+
val file = event.file?.takeIf { shouldHandleFile(it) }
76+
?: return@mapNotNull null
77+
toUriString(file)?.let { uri ->
78+
FileCreate().apply {
79+
this.uri = uri
80+
}
81+
}
6682
}
6783
}
6884
}
@@ -80,8 +96,17 @@ class WorkspaceServiceHandler(
8096
private fun didDeleteFiles(events: List<VFileEvent>) {
8197
AmazonQLspService.executeIfRunning(project) { languageServer ->
8298
val validFiles = events.mapNotNull { event ->
83-
val file = event.file?.takeIf { shouldHandleFile(it) } ?: return@mapNotNull null
84-
toUriString(file)?.let { uri ->
99+
when (event) {
100+
is VFileDeleteEvent -> {
101+
val file = event.file.takeIf { shouldHandleFile(it) } ?: return@mapNotNull null
102+
toUriString(file)
103+
}
104+
is VFileMoveEvent -> {
105+
val oldFile = event.oldParent?.takeIf { shouldHandleFile(it) } ?: return@mapNotNull null
106+
toUriString(oldFile)
107+
}
108+
else -> null
109+
}?.let { uri ->
85110
FileDelete().apply {
86111
this.uri = uri
87112
}
@@ -129,15 +154,51 @@ class WorkspaceServiceHandler(
129154

130155
private fun didChangeWatchedFiles(events: List<VFileEvent>) {
131156
AmazonQLspService.executeIfRunning(project) { languageServer ->
132-
val validChanges = events.mapNotNull { event ->
133-
event.file?.let { toUriString(it) }?.let { uri ->
134-
FileEvent().apply {
135-
this.uri = uri
136-
type = when (event) {
137-
is VFileCreateEvent -> FileChangeType.Created
138-
is VFileDeleteEvent -> FileChangeType.Deleted
139-
else -> FileChangeType.Changed
140-
}
157+
val validChanges = events.flatMap { event ->
158+
when (event) {
159+
is VFileCopyEvent -> {
160+
event.newParent.findChild(event.newChildName)?.let { newFile ->
161+
toUriString(newFile)?.let { uri ->
162+
listOf(
163+
FileEvent().apply {
164+
this.uri = uri
165+
type = FileChangeType.Created
166+
}
167+
)
168+
}
169+
}.orEmpty()
170+
}
171+
is VFileMoveEvent -> {
172+
listOfNotNull(
173+
toUriString(event.oldParent)?.let { oldUri ->
174+
FileEvent().apply {
175+
uri = oldUri
176+
type = FileChangeType.Deleted
177+
}
178+
},
179+
toUriString(event.file)?.let { newUri ->
180+
FileEvent().apply {
181+
uri = newUri
182+
type = FileChangeType.Created
183+
}
184+
}
185+
)
186+
}
187+
else -> {
188+
event.file?.let { file ->
189+
toUriString(file)?.let { uri ->
190+
listOf(
191+
FileEvent().apply {
192+
this.uri = uri
193+
type = when (event) {
194+
is VFileCreateEvent -> FileChangeType.Created
195+
is VFileDeleteEvent -> FileChangeType.Deleted
196+
else -> FileChangeType.Changed
197+
}
198+
}
199+
)
200+
}
201+
}.orEmpty()
141202
}
142203
}
143204
}
@@ -155,8 +216,8 @@ class WorkspaceServiceHandler(
155216
override fun after(events: List<VFileEvent>) {
156217
// since we are using synchronous FileListener
157218
pluginAwareExecuteOnPooledThread {
158-
didCreateFiles(events.filterIsInstance<VFileCreateEvent>())
159-
didDeleteFiles(events.filterIsInstance<VFileDeleteEvent>())
219+
didCreateFiles(events.filter { it is VFileCreateEvent || it is VFileMoveEvent || it is VFileCopyEvent })
220+
didDeleteFiles(events.filter { it is VFileMoveEvent || it is VFileDeleteEvent })
160221
didRenameFiles(events.filterIsInstance<VFilePropertyChangeEvent>())
161222
didChangeWatchedFiles(events)
162223
}

plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/workspace/WorkspaceServiceHandlerTest.kt

Lines changed: 140 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@ import com.intellij.openapi.application.ApplicationManager
99
import com.intellij.openapi.components.serviceIfCreated
1010
import com.intellij.openapi.project.Project
1111
import com.intellij.openapi.vfs.VirtualFile
12+
import com.intellij.openapi.vfs.newvfs.events.VFileCopyEvent
1213
import com.intellij.openapi.vfs.newvfs.events.VFileCreateEvent
1314
import com.intellij.openapi.vfs.newvfs.events.VFileDeleteEvent
1415
import com.intellij.openapi.vfs.newvfs.events.VFileEvent
16+
import com.intellij.openapi.vfs.newvfs.events.VFileMoveEvent
1517
import com.intellij.openapi.vfs.newvfs.events.VFilePropertyChangeEvent
1618
import com.intellij.util.messages.MessageBus
1719
import com.intellij.util.messages.MessageBusConnection
@@ -167,6 +169,32 @@ class WorkspaceServiceHandlerTest {
167169
verify(exactly = 0) { mockWorkspaceService.didCreateFiles(any()) }
168170
}
169171

172+
@Test
173+
fun `test didCreateFiles with move event`() = runTest {
174+
val oldUri = URI("file:///test/oldPath")
175+
val newUri = URI("file:///test/newPath")
176+
val moveEvent = createMockVFileMoveEvent(oldUri, newUri, "test.py")
177+
178+
sut.after(listOf(moveEvent))
179+
180+
val paramsSlot = slot<CreateFilesParams>()
181+
verify { mockWorkspaceService.didCreateFiles(capture(paramsSlot)) }
182+
assertEquals(normalizeFileUri(newUri.toString()), paramsSlot.captured.files[0].uri)
183+
}
184+
185+
@Test
186+
fun `test didCreateFiles with copy event`() = runTest {
187+
val originalUri = URI("file:///test/original")
188+
val newUri = URI("file:///test/new")
189+
val copyEvent = createMockVFileCopyEvent(originalUri, newUri, "test.py")
190+
191+
sut.after(listOf(copyEvent))
192+
193+
val paramsSlot = slot<CreateFilesParams>()
194+
verify { mockWorkspaceService.didCreateFiles(capture(paramsSlot)) }
195+
assertEquals(normalizeFileUri(newUri.toString()), paramsSlot.captured.files[0].uri)
196+
}
197+
170198
@Test
171199
fun `test didDeleteFiles with Python file`() = runTest {
172200
val pyUri = URI("file:///test/path")
@@ -237,6 +265,48 @@ class WorkspaceServiceHandlerTest {
237265
assertEquals(normalizeFileUri(dirUri.toString()), paramsSlot.captured.files[0].uri)
238266
}
239267

268+
@Test
269+
fun `test didDeleteFiles handles both delete and move events in same batch`() = runTest {
270+
val deleteUri = URI("file:///test/deleteFile")
271+
val oldMoveUri = URI("file:///test/oldMoveFile")
272+
val newMoveUri = URI("file:///test/newMoveFile")
273+
274+
val deleteEvent = createMockVFileEvent(deleteUri, FileChangeType.Deleted, false, "py")
275+
val moveEvent = createMockVFileMoveEvent(oldMoveUri, newMoveUri, "test.py")
276+
277+
sut.after(listOf(deleteEvent, moveEvent))
278+
279+
val deleteParamsSlot = slot<DeleteFilesParams>()
280+
verify { mockWorkspaceService.didDeleteFiles(capture(deleteParamsSlot)) }
281+
assertEquals(2, deleteParamsSlot.captured.files.size)
282+
assertEquals(normalizeFileUri(deleteUri.toString()), deleteParamsSlot.captured.files[0].uri)
283+
assertEquals(normalizeFileUri(oldMoveUri.toString()), deleteParamsSlot.captured.files[1].uri)
284+
}
285+
286+
@Test
287+
fun `test didDeleteFiles with move event of unsupported file type`() = runTest {
288+
val oldUri = URI("file:///test/oldPath")
289+
val newUri = URI("file:///test/newPath")
290+
val moveEvent = createMockVFileMoveEvent(oldUri, newUri, "test.txt")
291+
292+
sut.after(listOf(moveEvent))
293+
294+
verify(exactly = 0) { mockWorkspaceService.didDeleteFiles(any()) }
295+
}
296+
297+
@Test
298+
fun `test didDeleteFiles with move event of directory`() = runTest {
299+
val oldUri = URI("file:///test/oldDir")
300+
val newUri = URI("file:///test/newDir")
301+
val moveEvent = createMockVFileMoveEvent(oldUri, newUri, "", true)
302+
303+
sut.after(listOf(moveEvent))
304+
305+
val deleteParamsSlot = slot<DeleteFilesParams>()
306+
verify { mockWorkspaceService.didDeleteFiles(capture(deleteParamsSlot)) }
307+
assertEquals(normalizeFileUri(oldUri.toString()), deleteParamsSlot.captured.files[0].uri)
308+
}
309+
240310
@Test
241311
fun `test didChangeWatchedFiles with valid events`() = runTest {
242312
// Arrange
@@ -262,6 +332,38 @@ class WorkspaceServiceHandlerTest {
262332
assertEquals(FileChangeType.Changed, paramsSlot.captured.changes[2].type)
263333
}
264334

335+
@Test
336+
fun `test didChangeWatchedFiles with move event reports both delete and create`() = runTest {
337+
val oldUri = URI("file:///test/oldPath")
338+
val newUri = URI("file:///test/newPath")
339+
val moveEvent = createMockVFileMoveEvent(oldUri, newUri, "test.py")
340+
341+
sut.after(listOf(moveEvent))
342+
343+
val paramsSlot = slot<DidChangeWatchedFilesParams>()
344+
verify { mockWorkspaceService.didChangeWatchedFiles(capture(paramsSlot)) }
345+
346+
assertEquals(2, paramsSlot.captured.changes.size)
347+
assertEquals(normalizeFileUri(oldUri.toString()), paramsSlot.captured.changes[0].uri)
348+
assertEquals(FileChangeType.Deleted, paramsSlot.captured.changes[0].type)
349+
assertEquals(normalizeFileUri(newUri.toString()), paramsSlot.captured.changes[1].uri)
350+
assertEquals(FileChangeType.Created, paramsSlot.captured.changes[1].type)
351+
}
352+
353+
@Test
354+
fun `test didChangeWatchedFiles with copy event`() = runTest {
355+
val originalUri = URI("file:///test/original")
356+
val newUri = URI("file:///test/new")
357+
val copyEvent = createMockVFileCopyEvent(originalUri, newUri, "test.py")
358+
359+
sut.after(listOf(copyEvent))
360+
361+
val paramsSlot = slot<DidChangeWatchedFilesParams>()
362+
verify { mockWorkspaceService.didChangeWatchedFiles(capture(paramsSlot)) }
363+
assertEquals(normalizeFileUri(newUri.toString()), paramsSlot.captured.changes[0].uri)
364+
assertEquals(FileChangeType.Created, paramsSlot.captured.changes[0].type)
365+
}
366+
265367
@Test
266368
fun `test no invoked messages when events are empty`() = runTest {
267369
// Act
@@ -510,20 +612,28 @@ class WorkspaceServiceHandlerTest {
510612
assertEquals("folder2", paramsSlot.captured.event.removed[0].name)
511613
}
512614

513-
private fun createMockVFileEvent(uri: URI, type: FileChangeType = FileChangeType.Changed, isDirectory: Boolean, extension: String = "py"): VFileEvent {
615+
private fun createMockVirtualFile(uri: URI, fileName: String, isDirectory: Boolean = false): VirtualFile {
514616
val nioPath = mockk<Path> {
515617
every { toUri() } returns uri
516618
}
517-
val virtualFile = mockk<VirtualFile> {
619+
return mockk<VirtualFile> {
518620
every { this@mockk.isDirectory } returns isDirectory
519621
every { toNioPath() } returns nioPath
520622
every { url } returns uri.path
521-
every { path } returns "${uri.path}.$extension"
623+
every { path } returns "${uri.path}/$fileName"
522624
every { fileSystem } returns mockk {
523625
every { protocol } returns "file"
524626
}
525627
}
628+
}
526629

630+
private fun createMockVFileEvent(
631+
uri: URI,
632+
type: FileChangeType = FileChangeType.Changed,
633+
isDirectory: Boolean = false,
634+
extension: String = "py",
635+
): VFileEvent {
636+
val virtualFile = createMockVirtualFile(uri, "test.$extension", isDirectory)
527637
return when (type) {
528638
FileChangeType.Deleted -> mockk<VFileDeleteEvent>()
529639
FileChangeType.Created -> mockk<VFileCreateEvent>()
@@ -533,46 +643,45 @@ class WorkspaceServiceHandlerTest {
533643
}
534644
}
535645

536-
// for didRename events
537646
private fun createMockPropertyChangeEvent(
538647
oldName: String,
539648
newName: String,
540649
isDirectory: Boolean = false,
541650
): VFilePropertyChangeEvent {
542-
val parentPath = mockk<Path>()
543-
val filePath = mockk<Path>()
651+
val oldUri = URI("file:///test/$oldName")
652+
val newUri = URI("file:///test/$newName")
653+
val file = createMockVirtualFile(newUri, newName, isDirectory)
654+
every { file.parent } returns createMockVirtualFile(oldUri, oldName, isDirectory)
544655

545-
val parent = mockk<VirtualFile> {
546-
every { toNioPath() } returns parentPath
547-
every { this@mockk.isDirectory } returns isDirectory
548-
every { path } returns "/test/$oldName"
549-
every { url } returns "file:///test/$oldName"
550-
every { fileSystem } returns mockk {
551-
every { protocol } returns "file"
552-
}
656+
return mockk<VFilePropertyChangeEvent>().apply {
657+
every { propertyName } returns VirtualFile.PROP_NAME
658+
every { this@apply.file } returns file
659+
every { oldValue } returns oldName
660+
every { newValue } returns newName
553661
}
662+
}
554663

555-
val file = mockk<VirtualFile> {
556-
every { toNioPath() } returns filePath
557-
every { this@mockk.parent } returns parent
558-
every { this@mockk.isDirectory } returns isDirectory
559-
every { path } returns "/test/$newName"
560-
every { url } returns "file:///test/$newName"
664+
private fun createMockVFileMoveEvent(oldUri: URI, newUri: URI, fileName: String, isDirectory: Boolean = false): VFileMoveEvent {
665+
val oldFile = createMockVirtualFile(oldUri, fileName, isDirectory)
666+
val newFile = createMockVirtualFile(newUri, fileName, isDirectory)
667+
return mockk<VFileMoveEvent>().apply {
668+
every { file } returns newFile
669+
every { oldPath } returns oldUri.path
670+
every { oldParent } returns oldFile
671+
}
672+
}
673+
674+
private fun createMockVFileCopyEvent(originalUri: URI, newUri: URI, fileName: String): VFileCopyEvent {
675+
val newParent = mockk<VirtualFile> {
676+
every { findChild(any()) } returns createMockVirtualFile(newUri, fileName)
561677
every { fileSystem } returns mockk {
562678
every { protocol } returns "file"
563679
}
564680
}
565-
566-
every { parentPath.resolve(oldName) } returns mockk {
567-
every { toUri() } returns URI("file:///test/$oldName")
568-
}
569-
every { filePath.toUri() } returns URI("file:///test/$newName")
570-
571-
return mockk<VFilePropertyChangeEvent>().apply {
572-
every { propertyName } returns VirtualFile.PROP_NAME
573-
every { this@apply.file } returns file
574-
every { oldValue } returns oldName
575-
every { newValue } returns newName
681+
return mockk<VFileCopyEvent>().apply {
682+
every { file } returns createMockVirtualFile(originalUri, fileName)
683+
every { this@apply.newParent } returns newParent
684+
every { newChildName } returns fileName
576685
}
577686
}
578687

0 commit comments

Comments
 (0)