Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(amazonq): implement VirtualFile -> URI util for messaging params #5462

Open
wants to merge 11 commits into
base: feature/q-lsp
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import org.eclipse.lsp4j.TextDocumentIdentifier
import org.eclipse.lsp4j.TextDocumentItem
import org.eclipse.lsp4j.VersionedTextDocumentIdentifier
import software.aws.toolkits.jetbrains.services.amazonq.lsp.AmazonQLspService
import software.aws.toolkits.jetbrains.services.amazonq.lsp.util.FileUriUtil.toUriString
import software.aws.toolkits.jetbrains.utils.pluginAwareExecuteOnPooledThread

class TextDocumentServiceHandler(
Expand Down Expand Up @@ -56,7 +57,7 @@ class TextDocumentServiceHandler(
override fun beforeDocumentSaving(document: Document) {
AmazonQLspService.executeIfRunning(project) { languageServer ->
val file = FileDocumentManager.getInstance().getFile(document) ?: return@executeIfRunning
file.toNioPath().toUri().toString().takeIf { it.isNotEmpty() }?.let { uri ->
toUriString(file)?.let { uri ->
languageServer.textDocumentService.didSave(
DidSaveTextDocumentParams().apply {
textDocument = TextDocumentIdentifier().apply {
Expand All @@ -74,7 +75,7 @@ class TextDocumentServiceHandler(
pluginAwareExecuteOnPooledThread {
events.filterIsInstance<VFileContentChangeEvent>().forEach { event ->
val document = FileDocumentManager.getInstance().getCachedDocument(event.file) ?: return@forEach
event.file.toNioPath().toUri().toString().takeIf { it.isNotEmpty() }?.let { uri ->
toUriString(event.file)?.let { uri ->
languageServer.textDocumentService.didChange(
DidChangeTextDocumentParams().apply {
textDocument = VersionedTextDocumentIdentifier().apply {
Expand All @@ -99,7 +100,7 @@ class TextDocumentServiceHandler(
file: VirtualFile,
) {
AmazonQLspService.executeIfRunning(project) { languageServer ->
file.toNioPath().toUri().toString().takeIf { it.isNotEmpty() }?.let { uri ->
toUriString(file)?.let { uri ->
languageServer.textDocumentService.didOpen(
DidOpenTextDocumentParams().apply {
textDocument = TextDocumentItem().apply {
Expand All @@ -117,7 +118,7 @@ class TextDocumentServiceHandler(
file: VirtualFile,
) {
AmazonQLspService.executeIfRunning(project) { languageServer ->
file.toNioPath().toUri().toString().takeIf { it.isNotEmpty() }?.let { uri ->
toUriString(file)?.let { uri ->
languageServer.textDocumentService.didClose(
DidCloseTextDocumentParams().apply {
textDocument = TextDocumentIdentifier().apply {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.intellij.openapi.project.Project
import com.intellij.openapi.roots.ProjectRootManager
import org.eclipse.lsp4j.WorkspaceFolder
import software.aws.toolkits.jetbrains.services.amazonq.lsp.util.FileUriUtil.toUriString

object WorkspaceFolderUtil {
fun createWorkspaceFolders(project: Project): List<WorkspaceFolder> =
Expand All @@ -15,7 +16,7 @@
ProjectRootManager.getInstance(project).contentRoots.map { contentRoot ->
WorkspaceFolder().apply {
name = contentRoot.name
this.uri = contentRoot.url
this.uri = toUriString(contentRoot)

Check warning on line 19 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/util/WorkspaceFolderUtil.kt

View check run for this annotation

Codecov / codecov/patch

plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/util/WorkspaceFolderUtil.kt#L19

Added line #L19 was not covered by tests
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.eclipse.lsp4j.WorkspaceFolder
import org.eclipse.lsp4j.WorkspaceFoldersChangeEvent
import software.aws.toolkits.jetbrains.services.amazonq.lsp.AmazonQLspService
import software.aws.toolkits.jetbrains.services.amazonq.lsp.util.FileUriUtil.toUriString
import software.aws.toolkits.jetbrains.services.amazonq.lsp.util.WorkspaceFolderUtil.createWorkspaceFolders
import software.aws.toolkits.jetbrains.utils.pluginAwareExecuteOnPooledThread
import java.nio.file.FileSystems
Expand Down Expand Up @@ -59,7 +60,7 @@
AmazonQLspService.executeIfRunning(project) { languageServer ->
val validFiles = events.mapNotNull { event ->
val file = event.file?.takeIf { shouldHandleFile(it) } ?: return@mapNotNull null
file.toNioPath().toUri().toString().takeIf { it.isNotEmpty() }?.let { uri ->
toUriString(file)?.let { uri ->
FileCreate().apply {
this.uri = uri
}
Expand All @@ -80,7 +81,7 @@
AmazonQLspService.executeIfRunning(project) { languageServer ->
val validFiles = events.mapNotNull { event ->
val file = event.file?.takeIf { shouldHandleFile(it) } ?: return@mapNotNull null
file.toNioPath().toUri().toString().takeIf { it.isNotEmpty() }?.let { uri ->
toUriString(file)?.let { uri ->
FileDelete().apply {
this.uri = uri
}
Expand All @@ -103,13 +104,12 @@
.filter { it.propertyName == VirtualFile.PROP_NAME }
.mapNotNull { event ->
val file = event.file.takeIf { shouldHandleFile(it) } ?: return@mapNotNull null
val oldName = event.oldValue as? String ?: return@mapNotNull null
if (event.newValue !is String) return@mapNotNull null

// Construct old and new URIs
val parentPath = file.parent?.toNioPath() ?: return@mapNotNull null
val oldUri = parentPath.resolve(oldName).toUri().toString()
val newUri = file.toNioPath().toUri().toString()
val parentFile = file.parent ?: return@mapNotNull null
val oldUri = toUriString(parentFile)
val newUri = toUriString(file)

Check warning on line 112 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/workspace/WorkspaceServiceHandler.kt

View check run for this annotation

Codecov / codecov/patch

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

Added lines #L111 - L112 were not covered by tests

FileRename().apply {
this.oldUri = oldUri
Expand All @@ -130,7 +130,7 @@
private fun didChangeWatchedFiles(events: List<VFileEvent>) {
AmazonQLspService.executeIfRunning(project) { languageServer ->
val validChanges = events.mapNotNull { event ->
event.file?.toNioPath()?.toUri()?.toString()?.takeIf { it.isNotEmpty() }?.let { uri ->
event.file?.let { toUriString(it) }?.let { uri ->
FileEvent().apply {
this.uri = uri
type = when (event) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import com.intellij.util.messages.MessageBusConnection
import io.mockk.every
import io.mockk.just
import io.mockk.mockk
import io.mockk.mockkObject
import io.mockk.mockkStatic
import io.mockk.runs
import io.mockk.slot
Expand All @@ -34,6 +35,7 @@ import org.junit.Before
import org.junit.Test
import software.aws.toolkits.jetbrains.services.amazonq.lsp.AmazonQLanguageServer
import software.aws.toolkits.jetbrains.services.amazonq.lsp.AmazonQLspService
import software.aws.toolkits.jetbrains.services.amazonq.lsp.util.FileUriUtil
import java.net.URI
import java.nio.file.Path
import java.util.concurrent.Callable
Expand Down Expand Up @@ -99,14 +101,7 @@ class TextDocumentServiceHandlerTest {
every { text } returns "test content"
}

val path = mockk<Path> {
every { toUri() } returns uri
}

val file = mockk<VirtualFile> {
every { this@mockk.path } returns uri.path
every { toNioPath() } returns path
}
val file = createMockVirtualFile(uri)

// Mock FileDocumentManager
val fileDocumentManager = mockk<FileDocumentManager> {
Expand All @@ -125,7 +120,7 @@ class TextDocumentServiceHandlerTest {
verify { mockTextDocumentService.didSave(capture(paramsSlot)) }

with(paramsSlot.captured) {
assertEquals(uri.toString(), textDocument.uri)
assertEquals(normalizeFileUri(uri.toString()), textDocument.uri)
assertEquals("test content", text)
}
}
Expand All @@ -136,17 +131,8 @@ class TextDocumentServiceHandlerTest {
// Create test file
val uri = URI.create("file:///test/path/file.txt")
val content = "test content"
val inputStream = content.byteInputStream()

val path = mockk<Path> {
every { toUri() } returns uri
}

val file = mockk<VirtualFile> {
every { this@mockk.path } returns uri.path
every { toNioPath() } returns path
every { this@mockk.inputStream } returns inputStream
}
val file = createMockVirtualFile(uri, content)

// Call the handler method
sut.fileOpened(mockk(), file)
Expand All @@ -156,28 +142,22 @@ class TextDocumentServiceHandlerTest {
verify { mockTextDocumentService.didOpen(capture(paramsSlot)) }

with(paramsSlot.captured.textDocument) {
assertEquals(uri.toString(), this.uri)
assertEquals(normalizeFileUri(uri.toString()), this.uri)
assertEquals(content, text)
}
}

@Test
fun `didClose runs on fileClosed`() = runTest {
val uri = URI.create("file:///test/path/file.txt")
val path = mockk<Path> {
every { toUri() } returns uri
}
val file = mockk<VirtualFile> {
every { this@mockk.path } returns uri.path
every { toNioPath() } returns path
}
val file = createMockVirtualFile(uri)

sut.fileClosed(mockk(), file)

val paramsSlot = slot<DidCloseTextDocumentParams>()
verify { mockTextDocumentService.didClose(capture(paramsSlot)) }

assertEquals(uri.toString(), paramsSlot.captured.textDocument.uri)
assertEquals(normalizeFileUri(uri.toString()), paramsSlot.captured.textDocument.uri)
}

@Test
Expand All @@ -188,14 +168,7 @@ class TextDocumentServiceHandlerTest {
every { modificationStamp } returns 123L
}

val path = mockk<Path> {
every { toUri() } returns uri
}

val file = mockk<VirtualFile> {
every { this@mockk.path } returns uri.path
every { toNioPath() } returns path
}
val file = createMockVirtualFile(uri)

val changeEvent = mockk<VFileContentChangeEvent> {
every { this@mockk.file } returns file
Expand All @@ -218,7 +191,7 @@ class TextDocumentServiceHandlerTest {
verify { mockTextDocumentService.didChange(capture(paramsSlot)) }

with(paramsSlot.captured) {
assertEquals(uri.toString(), textDocument.uri)
assertEquals(normalizeFileUri(uri.toString()), textDocument.uri)
assertEquals(123, textDocument.version)
assertEquals("changed content", contentChanges[0].text)
}
Expand All @@ -227,23 +200,22 @@ class TextDocumentServiceHandlerTest {
@Test
fun `didSave does not run when URI is empty`() = runTest {
val document = mockk<Document>()
val path = mockk<Path> {
every { toUri() } returns URI.create("")
}
val file = mockk<VirtualFile> {
every { toNioPath() } returns path
}
val file = createMockVirtualFile(URI.create(""))

val fileDocumentManager = mockk<FileDocumentManager> {
every { getFile(document) } returns file
}
mockkObject(FileUriUtil) {
every { FileUriUtil.toUriString(file) } returns null

mockkStatic(FileDocumentManager::class) {
every { FileDocumentManager.getInstance() } returns fileDocumentManager
val fileDocumentManager = mockk<FileDocumentManager> {
every { getFile(document) } returns file
}

sut.beforeDocumentSaving(document)
mockkStatic(FileDocumentManager::class) {
every { FileDocumentManager.getInstance() } returns fileDocumentManager

verify(exactly = 0) { mockTextDocumentService.didSave(any()) }
sut.beforeDocumentSaving(document)

verify(exactly = 0) { mockTextDocumentService.didSave(any()) }
}
}
}

Expand Down Expand Up @@ -298,4 +270,33 @@ class TextDocumentServiceHandlerTest {
verify(exactly = 0) { mockTextDocumentService.didChange(any()) }
}
}

private fun createMockVirtualFile(uri: URI, content: String = ""): VirtualFile {
val path = mockk<Path> {
every { toUri() } returns uri
}
val inputStream = content.byteInputStream()
return mockk<VirtualFile> {
every { url } returns uri.path
every { toNioPath() } returns path
every { isDirectory } returns false
every { fileSystem } returns mockk {
every { protocol } returns "file"
}
every { this@mockk.inputStream } returns inputStream
}
}

private fun normalizeFileUri(uri: String): String {
if (!System.getProperty("os.name").lowercase().contains("windows")) {
return uri
}

if (!uri.startsWith("file:///")) {
return uri
}

val path = uri.substringAfter("file:///")
return "file:///C:/$path"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import io.mockk.every
import io.mockk.mockk
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Test
import java.net.URI
import java.nio.file.Path

class WorkspaceFolderUtilTest {

Expand All @@ -27,29 +29,30 @@ class WorkspaceFolderUtilTest {
fun `createWorkspaceFolders returns workspace folders for non-default project`() {
val mockProject = mockk<Project>()
val mockProjectRootManager = mockk<ProjectRootManager>()
val mockContentRoot1 = mockk<VirtualFile>()
val mockContentRoot2 = mockk<VirtualFile>()
val mockContentRoot1 = createMockVirtualFile(
URI("file:///path/to/root1"),
name = "root1"
)
val mockContentRoot2 = createMockVirtualFile(
URI("file:///path/to/root2"),
name = "root2"
)

every { mockProject.isDefault } returns false
every { ProjectRootManager.getInstance(mockProject) } returns mockProjectRootManager
every { mockProjectRootManager.contentRoots } returns arrayOf(mockContentRoot1, mockContentRoot2)

every { mockContentRoot1.name } returns "root1"
every { mockContentRoot1.url } returns "file:///path/to/root1"
every { mockContentRoot2.name } returns "root2"
every { mockContentRoot2.url } returns "file:///path/to/root2"

val result = WorkspaceFolderUtil.createWorkspaceFolders(mockProject)

assertEquals(2, result.size)
assertEquals("file:///path/to/root1", result[0].uri)
assertEquals("file:///path/to/root2", result[1].uri)
assertEquals(normalizeFileUri("file:///path/to/root1"), result[0].uri)
assertEquals(normalizeFileUri("file:///path/to/root2"), result[1].uri)
assertEquals("root1", result[0].name)
assertEquals("root2", result[1].name)
}

@Test
fun `reateWorkspaceFolders returns empty list when project has no content roots`() {
fun `createWorkspaceFolders returns empty list when project has no content roots`() {
val mockProject = mockk<Project>()
val mockProjectRootManager = mockk<ProjectRootManager>()

Expand All @@ -61,4 +64,33 @@ class WorkspaceFolderUtilTest {

assertEquals(emptyList<VirtualFile>(), result)
}

private fun createMockVirtualFile(uri: URI, name: String): VirtualFile {
val path = mockk<Path> {
every { toUri() } returns uri
}
return mockk<VirtualFile> {
every { url } returns uri.toString()
every { getName() } returns name
every { toNioPath() } returns path
every { isDirectory } returns false
every { fileSystem } returns mockk {
every { protocol } returns "file"
}
}
}

// for windows unit tests
private fun normalizeFileUri(uri: String): String {
if (!System.getProperty("os.name").lowercase().contains("windows")) {
return uri
}

if (!uri.startsWith("file:///")) {
return uri
}

val path = uri.substringAfter("file:///")
return "file:///C:/$path"
}
}
Loading
Loading