Skip to content

Commit

Permalink
feat(amazonq): Added progress indicator for lsp artifact download (#5426
Browse files Browse the repository at this point in the history 
)

* Added download progress indicator

* FetchArtifacts will return path now
  • Loading branch information
@LokeshDogga13
LokeshDogga13 authored Feb 28, 2025
1 parent 6bfc0e5 commit 0af95e3
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 46 deletions.
39 changes: 27 additions & 12 deletions ...nity/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@

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 kotlinx.coroutines.CancellationException
import org.jetbrains.annotations.VisibleForTesting
import software.aws.toolkits.core.utils.deleteIfExists
import software.aws.toolkits.core.utils.error
Expand All @@ -14,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

Expand Down Expand Up @@ -99,7 +103,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?): Path? {
val temporaryDownloadPath = lspArtifactsPath.resolve("temp")
val downloadPath = lspArtifactsPath.resolve(versions.first().serverVersion.toString())

Expand All @@ -108,23 +112,34 @@ 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,
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
.mapNotNull { it.filename }
.forEach { filename -> extractZipFile(downloadPath.resolve(filename), downloadPath) }
logger.info { "Successfully downloaded and moved LSP artifacts to $downloadPath" }
}
}
return downloadPath
} catch (e: Exception) {
logger.error(e) { "Failed to download/move LSP artifacts on attempt ${currentAttempt.get()}" }
when (e) {
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()
downloadPath.toFile().deleteRecursively()
}
}
if (currentAttempt.get() >= maxDownloadAttempts) {
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
Expand Down
20 changes: 11 additions & 9 deletions ...ity/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManager.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,17 @@

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
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,
private val manifestFetcher: ManifestFetcher = ManifestFetcher(),
private val artifactHelper: ArtifactHelper = ArtifactHelper(),
manifestRange: SupportedManifestVersionRange?,
Expand All @@ -27,9 +30,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),
Expand All @@ -38,7 +38,7 @@ class ArtifactManager(
private val logger = getLogger<ArtifactManager>()
}

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
Expand All @@ -51,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)) {
this.artifactHelper.tryDownloadLspArtifacts(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
Expand Down
25 changes: 12 additions & 13 deletions .../tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelperTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,17 @@

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
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
Expand All @@ -30,6 +32,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() {
Expand All @@ -41,6 +44,10 @@ class ArtifactHelperTest {
hashes = listOf("sha384:1234")
)
)
mockProject = mockk<Project>(relaxed = true) {
every { basePath } returns tempDir.toString()
every { name } returns "TestProject"
}
}

@Test
Expand Down Expand Up @@ -180,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 {
artifactHelper.tryDownloadLspArtifacts(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()
}

Expand All @@ -195,15 +198,11 @@ class ArtifactHelperTest {
val spyArtifactHelper = spyk(artifactHelper)
every { spyArtifactHelper.downloadLspArtifacts(any(), any()) } returns false

assertThatThrownBy {
spyArtifactHelper.tryDownloadLspArtifacts(versions, null)
}
.isInstanceOf(LspException::class.java)
.hasFieldOrPropertyWithValue("errorCode", LspException.ErrorCode.DOWNLOAD_FAILED)
assertThat(runBlocking { artifactHelper.tryDownloadLspArtifacts(mockProject, versions, null) }).isEqualTo(null)
}

@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)
Expand All @@ -213,7 +212,7 @@ class ArtifactHelperTest {
every { moveFilesFromSourceToDestination(any(), any()) } just Runs
every { extractZipFile(any(), any()) } just Runs

spyArtifactHelper.tryDownloadLspArtifacts(versions, target)
assertThat(runBlocking { artifactHelper.tryDownloadLspArtifacts(mockProject, versions, target) }).isEqualTo(null)
}

@Test
Expand Down
35 changes: 23 additions & 12 deletions ...tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManagerTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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() {
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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()) }
Expand All @@ -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)
Expand All @@ -98,21 +107,22 @@ 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()) } returns tempDir
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()) }
}

@Test
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(manifestFetcher, artifactHelper, manifestVersionRanges))
artifactManager = spyk(ArtifactManager(mockProject, manifestFetcher, artifactHelper, manifestVersionRanges))

every { artifactManager.getLSPVersionsFromManifestWithSpecifiedRange(any()) }.returns(
ArtifactManager.LSPVersions(deListedVersions = emptyList(), inRangeVersions = versions)
Expand All @@ -125,10 +135,11 @@ class ArtifactManagerTest {

every { artifactHelper.getExistingLspArtifacts(any(), any()) }.returns(true)
every { artifactHelper.deleteOlderLspArtifacts(any()) } just Runs
every { artifactHelper.getAllLocalLspArtifactsWithinManifestRange(any()) }.returns(expectedResult)

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()) }
}
}
1 change: 1 addition & 0 deletions plugins/core/resources/resources/software/aws/toolkits/resources/MessagesBundle.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 0af95e3

Please sign in to comment.