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,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
Expand Down Expand Up @@ -99,7 +101,7 @@
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())

Expand All @@ -108,23 +110,23 @@
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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

localize

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

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 119 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-L119

Added lines #L115 - L119 were not covered by tests
}
}
return

Check warning on line 122 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#L122

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

Check warning on line 129 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#L129

Added line #L129 was not covered by tests
}

@VisibleForTesting
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -11,6 +12,7 @@
import software.aws.toolkits.jetbrains.services.amazonq.project.manifest.ManifestManager

class ArtifactManager(
private val project: Project,

Check warning on line 15 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#L15

Added line #L15 was not covered by tests
private val manifestFetcher: ManifestFetcher = ManifestFetcher(),
private val artifactHelper: ArtifactHelper = ArtifactHelper(),
manifestRange: SupportedManifestVersionRange?,
Expand All @@ -27,9 +29,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 +37,7 @@
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
Expand All @@ -61,7 +60,7 @@

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

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
}

this.artifactHelper.deleteOlderLspArtifacts(manifestVersionRanges)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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() {
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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
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,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()) }
}

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