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

feat(amazonq): Added progress indicator for lsp artifact download #5426

Merged
merged 7 commits into from
Feb 28, 2025
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.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 @@
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 @@
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"),

Check failure on line 117 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Invalid property key

'amazonqFeatureDev.placeholder.downloading_and_extracting_lsp_artifacts' doesn't appear to be a valid property key
cancellable = true

Check warning on line 118 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt

View check run for this annotation

Codecov / codecov/patch

plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt#L115-L118

Added lines #L115 - L118 were not covered by tests
) {
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" }

Check warning on line 125 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt

View check run for this annotation

Codecov / codecov/patch

plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt#L121-L125

Added lines #L121 - L125 were not covered by tests
}
}
return downloadPath

Check warning on line 128 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt

View check run for this annotation

Codecov / codecov/patch

plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt#L128

Added line #L128 was not covered by tests
} catch (e: Exception) {
logger.error(e) { "Failed to download/move LSP artifacts on attempt ${currentAttempt.get()}" }
when (e) {

Check warning on line 130 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt

View check run for this annotation

Codecov / codecov/patch

plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt#L130

Added line #L130 was not covered by tests
is CancellationException -> {
logger.error(e) { "User cancelled download and extracting of LSP artifacts.." }
currentAttempt.set(maxDownloadAttempts) // To exit the while loop.

Check warning on line 133 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt

View check run for this annotation

Codecov / codecov/patch

plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt#L132-L133

Added lines #L132 - L133 were not covered by tests
}
else -> { logger.error(e) { "Failed to download/move LSP artifacts on attempt ${currentAttempt.get()}" } }

Check warning on line 135 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt

View check run for this annotation

Codecov / codecov/patch

plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt#L135

Added line #L135 was not covered by tests
}
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

Check warning on line 142 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt

View check run for this annotation

Codecov / codecov/patch

plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt#L141-L142

Added lines #L141 - L142 were not covered by tests
}

@VisibleForTesting
Expand Down
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,

Check warning on line 16 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManager.kt

View check run for this annotation

Codecov / codecov/patch

plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManager.kt#L16

Added line #L16 was not covered by tests
private val manifestFetcher: ManifestFetcher = ManifestFetcher(),
private val artifactHelper: ArtifactHelper = ArtifactHelper(),
manifestRange: SupportedManifestVersionRange?,
Expand All @@ -27,9 +30,6 @@

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 @@
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 @@
// 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

Check warning on line 54 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManager.kt

View check run for this annotation

Codecov / codecov/patch

plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManager.kt#L54

Added line #L54 was not covered by tests
}
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

Check warning on line 63 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManager.kt

View check run for this annotation

Codecov / codecov/patch

plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManager.kt#L63

Added line #L63 was not covered by tests
} else {
this.artifactHelper.tryDownloadLspArtifacts(project, lspVersions.inRangeVersions, target)
?: throw LspException("Failed to download LSP artifacts", LspException.ErrorCode.DOWNLOAD_FAILED)

Check warning on line 66 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManager.kt

View check run for this annotation

Codecov / codecov/patch

plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManager.kt#L66

Added line #L66 was not covered by tests
}

this.artifactHelper.deleteOlderLspArtifacts(manifestVersionRanges)
return artifactPath

Check warning on line 69 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManager.kt

View check run for this annotation

Codecov / codecov/patch

plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManager.kt#L69

Added line #L69 was not covered by tests
}

@VisibleForTesting
Expand Down
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
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()) }
}
}
Loading