Added download progress indicator

aws · Feb 28, 2025 · bad2661 · bad2661
1 parent bd4dd63
commit bad2661
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 32 deletions.
diff --git a/...nity/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt b/...nity/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<ManifestManager.Version>, target: ManifestManager.VersionTarget?) {
+    suspend fun tryDownloadLspArtifacts(project: Project, versions: List<ManifestManager.Version>, 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/...ity/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManager.kt b/...ity/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<ArtifactManager>()
     }
 
-    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/.../tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelperTest.kt b/.../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<ManifestManager.TargetContent>
+    private lateinit var mockProject: Project
 
     @BeforeEach
     fun setUp() {
@@ -41,6 +45,10 @@ class ArtifactHelperTest {
                 hashes = listOf("sha384:1234")
             )
         )
+        mockProject = mockk<Project>(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/...tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManagerTest.kt b/...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,15 +43,19 @@ 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<Project>(relaxed = true) {
+            every { basePath } returns tempDir.toString()
+            every { name } returns "TestProject"
+        }
+        artifactManager = ArtifactManager(mockProject, manifestFetcher, artifactHelper, manifestVersionRanges)
     }
 
     @Test
     fun `fetch artifact fetcher throws exception if manifest is null`() {
         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()) }
     }
 }