From bad266110d615acd2a47f1a0939be6874f260186 Mon Sep 17 00:00:00 2001 From: Lokesh Dogga Date: Fri, 28 Feb 2025 13:12:20 -0800 Subject: [PATCH 1/5] Added download progress indicator --- .../amazonq/lsp/artifacts/ArtifactHelper.kt | 24 +++++++------- .../amazonq/lsp/artifacts/ArtifactManager.kt | 9 +++-- .../lsp/artifacts/ArtifactHelperTest.kt | 19 ++++++++--- .../lsp/artifacts/ArtifactManagerTest.kt | 33 ++++++++++++------- 4 files changed, 53 insertions(+), 32 deletions(-) diff --git a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt index e22bea572ad..ba4d245ee9f 100644 --- a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt +++ b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt @@ -3,6 +3,8 @@ package software.aws.toolkits.jetbrains.services.amazonq.lsp.artifacts +import com.intellij.openapi.project.Project +import com.intellij.platform.ide.progress.withBackgroundProgress import com.intellij.util.io.createDirectories import com.intellij.util.text.SemVer import org.jetbrains.annotations.VisibleForTesting @@ -99,7 +101,7 @@ class ArtifactHelper(private val lspArtifactsPath: Path = DEFAULT_ARTIFACT_PATH, return !hasInvalidFiles } - fun tryDownloadLspArtifacts(versions: List, target: ManifestManager.VersionTarget?) { + suspend fun tryDownloadLspArtifacts(project: Project, versions: List, target: ManifestManager.VersionTarget?) { val temporaryDownloadPath = lspArtifactsPath.resolve("temp") val downloadPath = lspArtifactsPath.resolve(versions.first().serverVersion.toString()) @@ -108,23 +110,23 @@ class ArtifactHelper(private val lspArtifactsPath: Path = DEFAULT_ARTIFACT_PATH, logger.info { "Attempt ${currentAttempt.get()} of $maxDownloadAttempts to download LSP artifacts" } try { - if (downloadLspArtifacts(temporaryDownloadPath, target) && target != null && !target.contents.isNullOrEmpty()) { - moveFilesFromSourceToDestination(temporaryDownloadPath, downloadPath) - target.contents - .mapNotNull { it.filename } - .forEach { filename -> extractZipFile(downloadPath.resolve(filename), downloadPath) } - logger.info { "Successfully downloaded and moved LSP artifacts to $downloadPath" } - return + withBackgroundProgress(project, "Downloading & Extracting LSP artifacts...", cancellable = true) { + if (downloadLspArtifacts(temporaryDownloadPath, target) && target != null && !target.contents.isNullOrEmpty()) { + moveFilesFromSourceToDestination(temporaryDownloadPath, downloadPath) + target.contents + .mapNotNull { it.filename } + .forEach { filename -> extractZipFile(downloadPath.resolve(filename), downloadPath) } + logger.info { "Successfully downloaded and moved LSP artifacts to $downloadPath" } + } } + return } catch (e: Exception) { logger.error(e) { "Failed to download/move LSP artifacts on attempt ${currentAttempt.get()}" } temporaryDownloadPath.toFile().deleteRecursively() downloadPath.toFile().deleteRecursively() } } - if (currentAttempt.get() >= maxDownloadAttempts) { - throw LspException("Failed to download LSP artifacts after $maxDownloadAttempts attempts", LspException.ErrorCode.DOWNLOAD_FAILED) - } + throw LspException("Failed to download LSP artifacts after $maxDownloadAttempts attempts", LspException.ErrorCode.DOWNLOAD_FAILED) } @VisibleForTesting diff --git a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManager.kt b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManager.kt index 0286bd89939..84531bed13e 100644 --- a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManager.kt +++ b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManager.kt @@ -3,6 +3,7 @@ package software.aws.toolkits.jetbrains.services.amazonq.lsp.artifacts +import com.intellij.openapi.project.Project import com.intellij.util.text.SemVer import org.jetbrains.annotations.VisibleForTesting import software.aws.toolkits.core.utils.error @@ -11,6 +12,7 @@ import software.aws.toolkits.core.utils.info import software.aws.toolkits.jetbrains.services.amazonq.project.manifest.ManifestManager class ArtifactManager( + private val project: Project, private val manifestFetcher: ManifestFetcher = ManifestFetcher(), private val artifactHelper: ArtifactHelper = ArtifactHelper(), manifestRange: SupportedManifestVersionRange?, @@ -27,9 +29,6 @@ class ArtifactManager( private val manifestVersionRanges: SupportedManifestVersionRange = manifestRange ?: DEFAULT_VERSION_RANGE - // Secondary constructor with no parameters - constructor() : this(ManifestFetcher(), ArtifactHelper(), null) - companion object { private val DEFAULT_VERSION_RANGE = SupportedManifestVersionRange( startVersion = SemVer("3.0.0", 3, 0, 0), @@ -38,7 +37,7 @@ class ArtifactManager( private val logger = getLogger() } - fun fetchArtifact() { + suspend fun fetchArtifact() { val manifest = manifestFetcher.fetch() ?: throw LspException( "Language Support is not available, as manifest is missing.", LspException.ErrorCode.MANIFEST_FETCH_FAILED @@ -61,7 +60,7 @@ class ArtifactManager( // Get Local LSP files and check if we can re-use existing LSP Artifacts if (!this.artifactHelper.getExistingLspArtifacts(lspVersions.inRangeVersions, target)) { - this.artifactHelper.tryDownloadLspArtifacts(lspVersions.inRangeVersions, target) + this.artifactHelper.tryDownloadLspArtifacts(project, lspVersions.inRangeVersions, target) } this.artifactHelper.deleteOlderLspArtifacts(manifestVersionRanges) diff --git a/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelperTest.kt b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelperTest.kt index 8357fb78b82..7519b23f1ac 100644 --- a/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelperTest.kt +++ b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelperTest.kt @@ -3,13 +3,16 @@ package software.aws.toolkits.jetbrains.services.amazonq.lsp.artifacts +import com.intellij.openapi.project.Project import com.intellij.util.io.createDirectories import com.intellij.util.text.SemVer import io.mockk.Runs import io.mockk.every import io.mockk.just +import io.mockk.mockk import io.mockk.mockkStatic import io.mockk.spyk +import kotlinx.coroutines.runBlocking import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.Assertions.assertThatThrownBy import org.jetbrains.annotations.TestOnly @@ -30,6 +33,7 @@ class ArtifactHelperTest { private lateinit var manifestVersionRanges: SupportedManifestVersionRange private lateinit var mockManifestManager: ManifestManager private lateinit var contents: List + private lateinit var mockProject: Project @BeforeEach fun setUp() { @@ -41,6 +45,10 @@ class ArtifactHelperTest { hashes = listOf("sha384:1234") ) ) + mockProject = mockk(relaxed = true) { + every { basePath } returns tempDir.toString() + every { name } returns "TestProject" + } } @Test @@ -181,7 +189,7 @@ class ArtifactHelperTest { fun `tryDownloadLspArtifacts should not download artifacts if target does not have contents`() { val versions = listOf(ManifestManager.Version(serverVersion = "2.0.0")) assertThatThrownBy { - artifactHelper.tryDownloadLspArtifacts(versions, null) + runBlocking { artifactHelper.tryDownloadLspArtifacts(mockProject, versions, null) } } .isInstanceOf(LspException::class.java) .hasFieldOrPropertyWithValue("errorCode", LspException.ErrorCode.DOWNLOAD_FAILED) @@ -196,14 +204,14 @@ class ArtifactHelperTest { every { spyArtifactHelper.downloadLspArtifacts(any(), any()) } returns false assertThatThrownBy { - spyArtifactHelper.tryDownloadLspArtifacts(versions, null) + runBlocking { spyArtifactHelper.tryDownloadLspArtifacts(mockProject, versions, null) } } .isInstanceOf(LspException::class.java) .hasFieldOrPropertyWithValue("errorCode", LspException.ErrorCode.DOWNLOAD_FAILED) } @Test - fun `tryDownloadLspArtifacts should not throw error on successful download`() { + fun `tryDownloadLspArtifacts should throw error after attempts are exhausted`() { val versions = listOf(ManifestManager.Version(serverVersion = "1.0.0")) val target = ManifestManager.VersionTarget(contents = contents) val spyArtifactHelper = spyk(artifactHelper) @@ -213,7 +221,10 @@ class ArtifactHelperTest { every { moveFilesFromSourceToDestination(any(), any()) } just Runs every { extractZipFile(any(), any()) } just Runs - spyArtifactHelper.tryDownloadLspArtifacts(versions, target) + assertThatThrownBy { + runBlocking { spyArtifactHelper.tryDownloadLspArtifacts(mockProject, versions, target) } + } + .isInstanceOf(LspException::class.java) } @Test diff --git a/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManagerTest.kt b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManagerTest.kt index 756b8821b37..35103cd5d93 100644 --- a/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManagerTest.kt +++ b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManagerTest.kt @@ -3,13 +3,17 @@ package software.aws.toolkits.jetbrains.services.amazonq.lsp.artifacts +import com.intellij.openapi.project.Project import com.intellij.util.text.SemVer import io.mockk.Runs +import io.mockk.coEvery import io.mockk.every import io.mockk.just +import io.mockk.mockk import io.mockk.mockkStatic import io.mockk.spyk import io.mockk.verify +import kotlinx.coroutines.runBlocking import org.assertj.core.api.Assertions.assertThatThrownBy import org.jetbrains.annotations.TestOnly import org.junit.jupiter.api.BeforeEach @@ -29,6 +33,7 @@ class ArtifactManagerTest { private lateinit var artifactManager: ArtifactManager private lateinit var manifestFetcher: ManifestFetcher private lateinit var manifestVersionRanges: SupportedManifestVersionRange + private lateinit var mockProject: Project @BeforeEach fun setUp() { @@ -38,7 +43,11 @@ class ArtifactManagerTest { startVersion = SemVer("1.0.0", 1, 0, 0), endVersion = SemVer("2.0.0", 2, 0, 0) ) - artifactManager = ArtifactManager(manifestFetcher, artifactHelper, manifestVersionRanges) + mockProject = mockk(relaxed = true) { + every { basePath } returns tempDir.toString() + every { name } returns "TestProject" + } + artifactManager = ArtifactManager(mockProject, manifestFetcher, artifactHelper, manifestVersionRanges) } @Test @@ -46,7 +55,7 @@ class ArtifactManagerTest { every { manifestFetcher.fetch() }.returns(null) assertThatThrownBy { - artifactManager.fetchArtifact() + runBlocking { artifactManager.fetchArtifact() } } .isInstanceOf(LspException::class.java) .hasFieldOrPropertyWithValue("errorCode", LspException.ErrorCode.MANIFEST_FETCH_FAILED) @@ -55,14 +64,14 @@ class ArtifactManagerTest { @Test fun `fetch artifact does not have any valid lsp versions`() { every { manifestFetcher.fetch() }.returns(ManifestManager.Manifest()) - artifactManager = spyk(ArtifactManager(manifestFetcher, artifactHelper, manifestVersionRanges)) + artifactManager = spyk(ArtifactManager(mockProject, manifestFetcher, artifactHelper, manifestVersionRanges)) every { artifactManager.getLSPVersionsFromManifestWithSpecifiedRange(any()) }.returns( ArtifactManager.LSPVersions(deListedVersions = emptyList(), inRangeVersions = emptyList()) ) assertThatThrownBy { - artifactManager.fetchArtifact() + runBlocking { artifactManager.fetchArtifact() } } .isInstanceOf(LspException::class.java) .hasFieldOrPropertyWithValue("errorCode", LspException.ErrorCode.NO_COMPATIBLE_LSP_VERSION) @@ -75,7 +84,7 @@ class ArtifactManagerTest { every { manifestFetcher.fetch() }.returns(ManifestManager.Manifest()) every { artifactHelper.getAllLocalLspArtifactsWithinManifestRange(any()) }.returns(expectedResult) - artifactManager.fetchArtifact() + runBlocking { artifactManager.fetchArtifact() } verify(exactly = 1) { manifestFetcher.fetch() } verify(exactly = 1) { artifactHelper.getAllLocalLspArtifactsWithinManifestRange(any()) } @@ -86,7 +95,7 @@ class ArtifactManagerTest { val target = ManifestManager.VersionTarget(platform = "temp", arch = "temp") val versions = listOf(ManifestManager.Version("1.0.0", targets = listOf(target))) - artifactManager = spyk(ArtifactManager(manifestFetcher, artifactHelper, manifestVersionRanges)) + artifactManager = spyk(ArtifactManager(mockProject, manifestFetcher, artifactHelper, manifestVersionRanges)) every { artifactManager.getLSPVersionsFromManifestWithSpecifiedRange(any()) }.returns( ArtifactManager.LSPVersions(deListedVersions = emptyList(), inRangeVersions = versions) @@ -98,12 +107,12 @@ class ArtifactManagerTest { every { getCurrentArchitecture() }.returns("temp") every { artifactHelper.getExistingLspArtifacts(any(), any()) }.returns(false) - every { artifactHelper.tryDownloadLspArtifacts(any(), any()) } just Runs + coEvery { artifactHelper.tryDownloadLspArtifacts(any(), any(), any()) } just Runs every { artifactHelper.deleteOlderLspArtifacts(any()) } just Runs - artifactManager.fetchArtifact() + runBlocking { artifactManager.fetchArtifact() } - verify(exactly = 1) { artifactHelper.tryDownloadLspArtifacts(any(), any()) } + verify(exactly = 1) { runBlocking { artifactHelper.tryDownloadLspArtifacts(any(), any(), any()) } } verify(exactly = 1) { artifactHelper.deleteOlderLspArtifacts(any()) } } @@ -112,7 +121,7 @@ class ArtifactManagerTest { val target = ManifestManager.VersionTarget(platform = "temp", arch = "temp") val versions = listOf(ManifestManager.Version("1.0.0", targets = listOf(target))) - artifactManager = spyk(ArtifactManager(manifestFetcher, artifactHelper, manifestVersionRanges)) + artifactManager = spyk(ArtifactManager(mockProject, manifestFetcher, artifactHelper, manifestVersionRanges)) every { artifactManager.getLSPVersionsFromManifestWithSpecifiedRange(any()) }.returns( ArtifactManager.LSPVersions(deListedVersions = emptyList(), inRangeVersions = versions) @@ -126,9 +135,9 @@ class ArtifactManagerTest { every { artifactHelper.getExistingLspArtifacts(any(), any()) }.returns(true) every { artifactHelper.deleteOlderLspArtifacts(any()) } just Runs - artifactManager.fetchArtifact() + runBlocking { artifactManager.fetchArtifact() } - verify(exactly = 0) { artifactHelper.tryDownloadLspArtifacts(any(), any()) } + verify(exactly = 0) { runBlocking { artifactHelper.tryDownloadLspArtifacts(any(), any(), any()) } } verify(exactly = 1) { artifactHelper.deleteOlderLspArtifacts(any()) } } } From 9718aadfc0d16bade7ad02580ff530bcbdd0e430 Mon Sep 17 00:00:00 2001 From: Lokesh Dogga Date: Fri, 28 Feb 2025 14:06:22 -0800 Subject: [PATCH 2/5] Addressed code review comments, fetchArtifacts will return path now --- .../amazonq/lsp/artifacts/ArtifactHelper.kt | 18 ++++++++++++++---- .../amazonq/lsp/artifacts/ArtifactManager.kt | 13 ++++++++----- .../lsp/artifacts/ArtifactHelperTest.kt | 18 +++--------------- .../lsp/artifacts/ArtifactManagerTest.kt | 4 +++- 4 files changed, 28 insertions(+), 25 deletions(-) diff --git a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt index ba4d245ee9f..d40c4dc92bf 100644 --- a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt +++ b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt @@ -7,6 +7,7 @@ import com.intellij.openapi.project.Project import com.intellij.platform.ide.progress.withBackgroundProgress import com.intellij.util.io.createDirectories import com.intellij.util.text.SemVer +import kotlinx.coroutines.CancellationException import org.jetbrains.annotations.VisibleForTesting import software.aws.toolkits.core.utils.deleteIfExists import software.aws.toolkits.core.utils.error @@ -16,6 +17,7 @@ import software.aws.toolkits.core.utils.info import software.aws.toolkits.core.utils.warn import software.aws.toolkits.jetbrains.core.saveFileFromUrl import software.aws.toolkits.jetbrains.services.amazonq.project.manifest.ManifestManager +import software.aws.toolkits.resources.AwsCoreBundle import java.nio.file.Path import java.util.concurrent.atomic.AtomicInteger @@ -101,7 +103,7 @@ class ArtifactHelper(private val lspArtifactsPath: Path = DEFAULT_ARTIFACT_PATH, return !hasInvalidFiles } - suspend fun tryDownloadLspArtifacts(project: Project, versions: List, target: ManifestManager.VersionTarget?) { + suspend fun tryDownloadLspArtifacts(project: Project, versions: List, target: ManifestManager.VersionTarget?): Path? { val temporaryDownloadPath = lspArtifactsPath.resolve("temp") val downloadPath = lspArtifactsPath.resolve(versions.first().serverVersion.toString()) @@ -110,7 +112,11 @@ class ArtifactHelper(private val lspArtifactsPath: Path = DEFAULT_ARTIFACT_PATH, logger.info { "Attempt ${currentAttempt.get()} of $maxDownloadAttempts to download LSP artifacts" } try { - withBackgroundProgress(project, "Downloading & Extracting LSP artifacts...", cancellable = true) { + withBackgroundProgress( + project, + AwsCoreBundle.message("amazonqFeatureDev.placeholder.downloading_and_extracting_lsp_artifacts"), + cancellable = true + ) { if (downloadLspArtifacts(temporaryDownloadPath, target) && target != null && !target.contents.isNullOrEmpty()) { moveFilesFromSourceToDestination(temporaryDownloadPath, downloadPath) target.contents @@ -119,14 +125,18 @@ class ArtifactHelper(private val lspArtifactsPath: Path = DEFAULT_ARTIFACT_PATH, logger.info { "Successfully downloaded and moved LSP artifacts to $downloadPath" } } } - return + return downloadPath + } catch (e: CancellationException) { + logger.error(e) { "User cancelled download and extracting of LSP artifacts.." } // fallback to older version of LSP artifacts if available. } catch (e: Exception) { logger.error(e) { "Failed to download/move LSP artifacts on attempt ${currentAttempt.get()}" } + } finally { temporaryDownloadPath.toFile().deleteRecursively() downloadPath.toFile().deleteRecursively() } } - throw LspException("Failed to download LSP artifacts after $maxDownloadAttempts attempts", LspException.ErrorCode.DOWNLOAD_FAILED) + logger.error { "Failed to download LSP artifacts after $maxDownloadAttempts attempts" } + return null } @VisibleForTesting diff --git a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManager.kt b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManager.kt index 84531bed13e..7d41bfaddb5 100644 --- a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManager.kt +++ b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManager.kt @@ -10,6 +10,7 @@ import software.aws.toolkits.core.utils.error import software.aws.toolkits.core.utils.getLogger import software.aws.toolkits.core.utils.info import software.aws.toolkits.jetbrains.services.amazonq.project.manifest.ManifestManager +import java.nio.file.Path class ArtifactManager( private val project: Project, @@ -37,7 +38,7 @@ class ArtifactManager( private val logger = getLogger() } - suspend fun fetchArtifact() { + suspend fun fetchArtifact(): Path { val manifest = manifestFetcher.fetch() ?: throw LspException( "Language Support is not available, as manifest is missing.", LspException.ErrorCode.MANIFEST_FETCH_FAILED @@ -50,20 +51,22 @@ class ArtifactManager( // No versions are found which are in the given range. Fallback to local lsp artifacts. val localLspArtifacts = this.artifactHelper.getAllLocalLspArtifactsWithinManifestRange(manifestVersionRanges) if (localLspArtifacts.isNotEmpty()) { - return + return localLspArtifacts.first().first } throw LspException("Language server versions not found in manifest.", LspException.ErrorCode.NO_COMPATIBLE_LSP_VERSION) } // If there is an LSP Manifest with the same version val target = getTargetFromLspManifest(lspVersions.inRangeVersions) - // Get Local LSP files and check if we can re-use existing LSP Artifacts - if (!this.artifactHelper.getExistingLspArtifacts(lspVersions.inRangeVersions, target)) { + val artifactPath: Path = if (this.artifactHelper.getExistingLspArtifacts(lspVersions.inRangeVersions, target)) { + this.artifactHelper.getAllLocalLspArtifactsWithinManifestRange(manifestVersionRanges).first().first + } else { this.artifactHelper.tryDownloadLspArtifacts(project, lspVersions.inRangeVersions, target) + ?: throw LspException("Failed to download LSP artifacts", LspException.ErrorCode.DOWNLOAD_FAILED) } - this.artifactHelper.deleteOlderLspArtifacts(manifestVersionRanges) + return artifactPath } @VisibleForTesting diff --git a/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelperTest.kt b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelperTest.kt index 7519b23f1ac..cf0dc137901 100644 --- a/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelperTest.kt +++ b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelperTest.kt @@ -14,7 +14,6 @@ import io.mockk.mockkStatic import io.mockk.spyk import kotlinx.coroutines.runBlocking import org.assertj.core.api.Assertions.assertThat -import org.assertj.core.api.Assertions.assertThatThrownBy import org.jetbrains.annotations.TestOnly import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test @@ -188,11 +187,7 @@ class ArtifactHelperTest { @Test fun `tryDownloadLspArtifacts should not download artifacts if target does not have contents`() { val versions = listOf(ManifestManager.Version(serverVersion = "2.0.0")) - assertThatThrownBy { - runBlocking { artifactHelper.tryDownloadLspArtifacts(mockProject, versions, null) } - } - .isInstanceOf(LspException::class.java) - .hasFieldOrPropertyWithValue("errorCode", LspException.ErrorCode.DOWNLOAD_FAILED) + assertThat(runBlocking { artifactHelper.tryDownloadLspArtifacts(mockProject, versions, null) }).isEqualTo(null) assertThat(tempDir.resolve("2.0.0").toFile().exists()).isFalse() } @@ -203,11 +198,7 @@ class ArtifactHelperTest { val spyArtifactHelper = spyk(artifactHelper) every { spyArtifactHelper.downloadLspArtifacts(any(), any()) } returns false - assertThatThrownBy { - runBlocking { spyArtifactHelper.tryDownloadLspArtifacts(mockProject, versions, null) } - } - .isInstanceOf(LspException::class.java) - .hasFieldOrPropertyWithValue("errorCode", LspException.ErrorCode.DOWNLOAD_FAILED) + assertThat(runBlocking { artifactHelper.tryDownloadLspArtifacts(mockProject, versions, null) }).isEqualTo(null) } @Test @@ -221,10 +212,7 @@ class ArtifactHelperTest { every { moveFilesFromSourceToDestination(any(), any()) } just Runs every { extractZipFile(any(), any()) } just Runs - assertThatThrownBy { - runBlocking { spyArtifactHelper.tryDownloadLspArtifacts(mockProject, versions, target) } - } - .isInstanceOf(LspException::class.java) + assertThat(runBlocking { artifactHelper.tryDownloadLspArtifacts(mockProject, versions, target) }).isEqualTo(null) } @Test diff --git a/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManagerTest.kt b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManagerTest.kt index 35103cd5d93..4e163900bff 100644 --- a/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManagerTest.kt +++ b/plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManagerTest.kt @@ -107,7 +107,7 @@ class ArtifactManagerTest { every { getCurrentArchitecture() }.returns("temp") every { artifactHelper.getExistingLspArtifacts(any(), any()) }.returns(false) - coEvery { artifactHelper.tryDownloadLspArtifacts(any(), any(), any()) } just Runs + coEvery { artifactHelper.tryDownloadLspArtifacts(any(), any(), any()) } returns tempDir every { artifactHelper.deleteOlderLspArtifacts(any()) } just Runs runBlocking { artifactManager.fetchArtifact() } @@ -120,6 +120,7 @@ class ArtifactManagerTest { fun `fetch artifact does not have valid version in local system`() { val target = ManifestManager.VersionTarget(platform = "temp", arch = "temp") val versions = listOf(ManifestManager.Version("1.0.0", targets = listOf(target))) + val expectedResult = listOf(Pair(tempDir, SemVer("1.0.0", 1, 0, 0))) artifactManager = spyk(ArtifactManager(mockProject, manifestFetcher, artifactHelper, manifestVersionRanges)) @@ -134,6 +135,7 @@ class ArtifactManagerTest { every { artifactHelper.getExistingLspArtifacts(any(), any()) }.returns(true) every { artifactHelper.deleteOlderLspArtifacts(any()) } just Runs + every { artifactHelper.getAllLocalLspArtifactsWithinManifestRange(any()) }.returns(expectedResult) runBlocking { artifactManager.fetchArtifact() } From 9ce7a2ea4cc7cc1db20e8f276663ded5dc1ce46f Mon Sep 17 00:00:00 2001 From: Lokesh Dogga Date: Fri, 28 Feb 2025 14:17:27 -0800 Subject: [PATCH 3/5] Fixing catch block --- .../services/amazonq/lsp/artifacts/ArtifactHelper.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt index d40c4dc92bf..32a21d4c9fe 100644 --- a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt +++ b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt @@ -126,11 +126,11 @@ class ArtifactHelper(private val lspArtifactsPath: Path = DEFAULT_ARTIFACT_PATH, } } return downloadPath - } catch (e: CancellationException) { - logger.error(e) { "User cancelled download and extracting of LSP artifacts.." } // fallback to older version of LSP artifacts if available. } catch (e: Exception) { - logger.error(e) { "Failed to download/move LSP artifacts on attempt ${currentAttempt.get()}" } - } finally { + when (e) { + is CancellationException -> { logger.error(e) { "User cancelled download and extracting of LSP artifacts.." } } + else -> { logger.error(e) { "Failed to download/move LSP artifacts on attempt ${currentAttempt.get()}" } } + } temporaryDownloadPath.toFile().deleteRecursively() downloadPath.toFile().deleteRecursively() } From 8b8a53a6f30008343d080f479e4278610b13132e Mon Sep 17 00:00:00 2001 From: Lokesh Dogga Date: Fri, 28 Feb 2025 14:29:25 -0800 Subject: [PATCH 4/5] Minor change when user cancels the background task. --- .../services/amazonq/lsp/artifacts/ArtifactHelper.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt index 32a21d4c9fe..e0fe212b9d4 100644 --- a/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt +++ b/plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt @@ -128,7 +128,10 @@ class ArtifactHelper(private val lspArtifactsPath: Path = DEFAULT_ARTIFACT_PATH, return downloadPath } catch (e: Exception) { when (e) { - is CancellationException -> { logger.error(e) { "User cancelled download and extracting of LSP artifacts.." } } + is CancellationException -> { + logger.error(e) { "User cancelled download and extracting of LSP artifacts.." } + currentAttempt.set(maxDownloadAttempts) // To exit the while loop. + } else -> { logger.error(e) { "Failed to download/move LSP artifacts on attempt ${currentAttempt.get()}" } } } temporaryDownloadPath.toFile().deleteRecursively() From 8fbff3e5c3ef849996698c5095bb14d9fd80faf6 Mon Sep 17 00:00:00 2001 From: Lokesh Dogga Date: Fri, 28 Feb 2025 14:58:50 -0800 Subject: [PATCH 5/5] Added localized string --- .../software/aws/toolkits/resources/MessagesBundle.properties | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/core/resources/resources/software/aws/toolkits/resources/MessagesBundle.properties b/plugins/core/resources/resources/software/aws/toolkits/resources/MessagesBundle.properties index 87c11fd21cb..2d756cb51b5 100644 --- a/plugins/core/resources/resources/software/aws/toolkits/resources/MessagesBundle.properties +++ b/plugins/core/resources/resources/software/aws/toolkits/resources/MessagesBundle.properties @@ -141,6 +141,7 @@ amazonqFeatureDev.placeholder.after_code_generation=Choose an option to proceed amazonqFeatureDev.placeholder.after_monthly_limit=Chat input is disabled amazonqFeatureDev.placeholder.closed_session=Open a new chat tab to continue amazonqFeatureDev.placeholder.context_gathering_complete=Gathering context... +amazonqFeatureDev.placeholder.downloading_and_extracting_lsp_artifacts=Downloading and Extracting Lsp Artifacts... amazonqFeatureDev.placeholder.generating_code=Generating code... amazonqFeatureDev.placeholder.new_plan=Describe your task or issue in as much detail as possible amazonqFeatureDev.placeholder.provide_code_feedback=Provide feedback or comments