Skip to content

Commit ea1752b

Browse files
LokeshDogga13leigaol
LokeshDogga13
authored and
leigaol
committed
feat(amazonq): Added progress indicator for lsp artifact download (aws#5426)
* Added download progress indicator * FetchArtifacts will return path now
1 parent 6cfc158 commit ea1752b

File tree

5 files changed

+74
-46
lines changed

5 files changed

+74
-46
lines changed

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

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@
33

44
package software.aws.toolkits.jetbrains.services.amazonq.lsp.artifacts
55

6+
import com.intellij.openapi.project.Project
7+
import com.intellij.platform.ide.progress.withBackgroundProgress
68
import com.intellij.util.io.createDirectories
79
import com.intellij.util.text.SemVer
10+
import kotlinx.coroutines.CancellationException
811
import org.jetbrains.annotations.VisibleForTesting
912
import software.aws.toolkits.core.utils.deleteIfExists
1013
import software.aws.toolkits.core.utils.error
@@ -14,6 +17,7 @@ import software.aws.toolkits.core.utils.info
1417
import software.aws.toolkits.core.utils.warn
1518
import software.aws.toolkits.jetbrains.core.saveFileFromUrl
1619
import software.aws.toolkits.jetbrains.services.amazonq.project.manifest.ManifestManager
20+
import software.aws.toolkits.resources.AwsCoreBundle
1721
import java.nio.file.Path
1822
import java.util.concurrent.atomic.AtomicInteger
1923

@@ -99,7 +103,7 @@ class ArtifactHelper(private val lspArtifactsPath: Path = DEFAULT_ARTIFACT_PATH,
99103
return !hasInvalidFiles
100104
}
101105

102-
fun tryDownloadLspArtifacts(versions: List<ManifestManager.Version>, target: ManifestManager.VersionTarget?) {
106+
suspend fun tryDownloadLspArtifacts(project: Project, versions: List<ManifestManager.Version>, target: ManifestManager.VersionTarget?): Path? {
103107
val temporaryDownloadPath = lspArtifactsPath.resolve("temp")
104108
val downloadPath = lspArtifactsPath.resolve(versions.first().serverVersion.toString())
105109

@@ -108,23 +112,34 @@ class ArtifactHelper(private val lspArtifactsPath: Path = DEFAULT_ARTIFACT_PATH,
108112
logger.info { "Attempt ${currentAttempt.get()} of $maxDownloadAttempts to download LSP artifacts" }
109113

110114
try {
111-
if (downloadLspArtifacts(temporaryDownloadPath, target) && target != null && !target.contents.isNullOrEmpty()) {
112-
moveFilesFromSourceToDestination(temporaryDownloadPath, downloadPath)
113-
target.contents
114-
.mapNotNull { it.filename }
115-
.forEach { filename -> extractZipFile(downloadPath.resolve(filename), downloadPath) }
116-
logger.info { "Successfully downloaded and moved LSP artifacts to $downloadPath" }
117-
return
115+
withBackgroundProgress(
116+
project,
117+
AwsCoreBundle.message("amazonqFeatureDev.placeholder.downloading_and_extracting_lsp_artifacts"),
118+
cancellable = true
119+
) {
120+
if (downloadLspArtifacts(temporaryDownloadPath, target) && target != null && !target.contents.isNullOrEmpty()) {
121+
moveFilesFromSourceToDestination(temporaryDownloadPath, downloadPath)
122+
target.contents
123+
.mapNotNull { it.filename }
124+
.forEach { filename -> extractZipFile(downloadPath.resolve(filename), downloadPath) }
125+
logger.info { "Successfully downloaded and moved LSP artifacts to $downloadPath" }
126+
}
118127
}
128+
return downloadPath
119129
} catch (e: Exception) {
120-
logger.error(e) { "Failed to download/move LSP artifacts on attempt ${currentAttempt.get()}" }
130+
when (e) {
131+
is CancellationException -> {
132+
logger.error(e) { "User cancelled download and extracting of LSP artifacts.." }
133+
currentAttempt.set(maxDownloadAttempts) // To exit the while loop.
134+
}
135+
else -> { logger.error(e) { "Failed to download/move LSP artifacts on attempt ${currentAttempt.get()}" } }
136+
}
121137
temporaryDownloadPath.toFile().deleteRecursively()
122138
downloadPath.toFile().deleteRecursively()
123139
}
124140
}
125-
if (currentAttempt.get() >= maxDownloadAttempts) {
126-
throw LspException("Failed to download LSP artifacts after $maxDownloadAttempts attempts", LspException.ErrorCode.DOWNLOAD_FAILED)
127-
}
141+
logger.error { "Failed to download LSP artifacts after $maxDownloadAttempts attempts" }
142+
return null
128143
}
129144

130145
@VisibleForTesting

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

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,17 @@
33

44
package software.aws.toolkits.jetbrains.services.amazonq.lsp.artifacts
55

6+
import com.intellij.openapi.project.Project
67
import com.intellij.util.text.SemVer
78
import org.jetbrains.annotations.VisibleForTesting
89
import software.aws.toolkits.core.utils.error
910
import software.aws.toolkits.core.utils.getLogger
1011
import software.aws.toolkits.core.utils.info
1112
import software.aws.toolkits.jetbrains.services.amazonq.project.manifest.ManifestManager
13+
import java.nio.file.Path
1214

1315
class ArtifactManager(
16+
private val project: Project,
1417
private val manifestFetcher: ManifestFetcher = ManifestFetcher(),
1518
private val artifactHelper: ArtifactHelper = ArtifactHelper(),
1619
manifestRange: SupportedManifestVersionRange?,
@@ -27,9 +30,6 @@ class ArtifactManager(
2730

2831
private val manifestVersionRanges: SupportedManifestVersionRange = manifestRange ?: DEFAULT_VERSION_RANGE
2932

30-
// Secondary constructor with no parameters
31-
constructor() : this(ManifestFetcher(), ArtifactHelper(), null)
32-
3333
companion object {
3434
private val DEFAULT_VERSION_RANGE = SupportedManifestVersionRange(
3535
startVersion = SemVer("3.0.0", 3, 0, 0),
@@ -38,7 +38,7 @@ class ArtifactManager(
3838
private val logger = getLogger<ArtifactManager>()
3939
}
4040

41-
fun fetchArtifact() {
41+
suspend fun fetchArtifact(): Path {
4242
val manifest = manifestFetcher.fetch() ?: throw LspException(
4343
"Language Support is not available, as manifest is missing.",
4444
LspException.ErrorCode.MANIFEST_FETCH_FAILED
@@ -51,20 +51,22 @@ class ArtifactManager(
5151
// No versions are found which are in the given range. Fallback to local lsp artifacts.
5252
val localLspArtifacts = this.artifactHelper.getAllLocalLspArtifactsWithinManifestRange(manifestVersionRanges)
5353
if (localLspArtifacts.isNotEmpty()) {
54-
return
54+
return localLspArtifacts.first().first
5555
}
5656
throw LspException("Language server versions not found in manifest.", LspException.ErrorCode.NO_COMPATIBLE_LSP_VERSION)
5757
}
5858

5959
// If there is an LSP Manifest with the same version
6060
val target = getTargetFromLspManifest(lspVersions.inRangeVersions)
61-
6261
// Get Local LSP files and check if we can re-use existing LSP Artifacts
63-
if (!this.artifactHelper.getExistingLspArtifacts(lspVersions.inRangeVersions, target)) {
64-
this.artifactHelper.tryDownloadLspArtifacts(lspVersions.inRangeVersions, target)
62+
val artifactPath: Path = if (this.artifactHelper.getExistingLspArtifacts(lspVersions.inRangeVersions, target)) {
63+
this.artifactHelper.getAllLocalLspArtifactsWithinManifestRange(manifestVersionRanges).first().first
64+
} else {
65+
this.artifactHelper.tryDownloadLspArtifacts(project, lspVersions.inRangeVersions, target)
66+
?: throw LspException("Failed to download LSP artifacts", LspException.ErrorCode.DOWNLOAD_FAILED)
6567
}
66-
6768
this.artifactHelper.deleteOlderLspArtifacts(manifestVersionRanges)
69+
return artifactPath
6870
}
6971

7072
@VisibleForTesting

‎plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelperTest.kt‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,17 @@
33

44
package software.aws.toolkits.jetbrains.services.amazonq.lsp.artifacts
55

6+
import com.intellij.openapi.project.Project
67
import com.intellij.util.io.createDirectories
78
import com.intellij.util.text.SemVer
89
import io.mockk.Runs
910
import io.mockk.every
1011
import io.mockk.just
12+
import io.mockk.mockk
1113
import io.mockk.mockkStatic
1214
import io.mockk.spyk
15+
import kotlinx.coroutines.runBlocking
1316
import org.assertj.core.api.Assertions.assertThat
14-
import org.assertj.core.api.Assertions.assertThatThrownBy
1517
import org.jetbrains.annotations.TestOnly
1618
import org.junit.jupiter.api.BeforeEach
1719
import org.junit.jupiter.api.Test
@@ -30,6 +32,7 @@ class ArtifactHelperTest {
3032
private lateinit var manifestVersionRanges: SupportedManifestVersionRange
3133
private lateinit var mockManifestManager: ManifestManager
3234
private lateinit var contents: List<ManifestManager.TargetContent>
35+
private lateinit var mockProject: Project
3336

3437
@BeforeEach
3538
fun setUp() {
@@ -41,6 +44,10 @@ class ArtifactHelperTest {
4144
hashes = listOf("sha384:1234")
4245
)
4346
)
47+
mockProject = mockk<Project>(relaxed = true) {
48+
every { basePath } returns tempDir.toString()
49+
every { name } returns "TestProject"
50+
}
4451
}
4552

4653
@Test
@@ -180,11 +187,7 @@ class ArtifactHelperTest {
180187
@Test
181188
fun `tryDownloadLspArtifacts should not download artifacts if target does not have contents`() {
182189
val versions = listOf(ManifestManager.Version(serverVersion = "2.0.0"))
183-
assertThatThrownBy {
184-
artifactHelper.tryDownloadLspArtifacts(versions, null)
185-
}
186-
.isInstanceOf(LspException::class.java)
187-
.hasFieldOrPropertyWithValue("errorCode", LspException.ErrorCode.DOWNLOAD_FAILED)
190+
assertThat(runBlocking { artifactHelper.tryDownloadLspArtifacts(mockProject, versions, null) }).isEqualTo(null)
188191
assertThat(tempDir.resolve("2.0.0").toFile().exists()).isFalse()
189192
}
190193

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

198-
assertThatThrownBy {
199-
spyArtifactHelper.tryDownloadLspArtifacts(versions, null)
200-
}
201-
.isInstanceOf(LspException::class.java)
202-
.hasFieldOrPropertyWithValue("errorCode", LspException.ErrorCode.DOWNLOAD_FAILED)
201+
assertThat(runBlocking { artifactHelper.tryDownloadLspArtifacts(mockProject, versions, null) }).isEqualTo(null)
203202
}
204203

205204
@Test
206-
fun `tryDownloadLspArtifacts should not throw error on successful download`() {
205+
fun `tryDownloadLspArtifacts should throw error after attempts are exhausted`() {
207206
val versions = listOf(ManifestManager.Version(serverVersion = "1.0.0"))
208207
val target = ManifestManager.VersionTarget(contents = contents)
209208
val spyArtifactHelper = spyk(artifactHelper)
@@ -213,7 +212,7 @@ class ArtifactHelperTest {
213212
every { moveFilesFromSourceToDestination(any(), any()) } just Runs
214213
every { extractZipFile(any(), any()) } just Runs
215214

216-
spyArtifactHelper.tryDownloadLspArtifacts(versions, target)
215+
assertThat(runBlocking { artifactHelper.tryDownloadLspArtifacts(mockProject, versions, target) }).isEqualTo(null)
217216
}
218217

219218
@Test

‎plugins/amazonq/shared/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactManagerTest.kt‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,17 @@
33

44
package software.aws.toolkits.jetbrains.services.amazonq.lsp.artifacts
55

6+
import com.intellij.openapi.project.Project
67
import com.intellij.util.text.SemVer
78
import io.mockk.Runs
9+
import io.mockk.coEvery
810
import io.mockk.every
911
import io.mockk.just
12+
import io.mockk.mockk
1013
import io.mockk.mockkStatic
1114
import io.mockk.spyk
1215
import io.mockk.verify
16+
import kotlinx.coroutines.runBlocking
1317
import org.assertj.core.api.Assertions.assertThatThrownBy
1418
import org.jetbrains.annotations.TestOnly
1519
import org.junit.jupiter.api.BeforeEach
@@ -29,6 +33,7 @@ class ArtifactManagerTest {
2933
private lateinit var artifactManager: ArtifactManager
3034
private lateinit var manifestFetcher: ManifestFetcher
3135
private lateinit var manifestVersionRanges: SupportedManifestVersionRange
36+
private lateinit var mockProject: Project
3237

3338
@BeforeEach
3439
fun setUp() {
@@ -38,15 +43,19 @@ class ArtifactManagerTest {
3843
startVersion = SemVer("1.0.0", 1, 0, 0),
3944
endVersion = SemVer("2.0.0", 2, 0, 0)
4045
)
41-
artifactManager = ArtifactManager(manifestFetcher, artifactHelper, manifestVersionRanges)
46+
mockProject = mockk<Project>(relaxed = true) {
47+
every { basePath } returns tempDir.toString()
48+
every { name } returns "TestProject"
49+
}
50+
artifactManager = ArtifactManager(mockProject, manifestFetcher, artifactHelper, manifestVersionRanges)
4251
}
4352

4453
@Test
4554
fun `fetch artifact fetcher throws exception if manifest is null`() {
4655
every { manifestFetcher.fetch() }.returns(null)
4756

4857
assertThatThrownBy {
49-
artifactManager.fetchArtifact()
58+
runBlocking { artifactManager.fetchArtifact() }
5059
}
5160
.isInstanceOf(LspException::class.java)
5261
.hasFieldOrPropertyWithValue("errorCode", LspException.ErrorCode.MANIFEST_FETCH_FAILED)
@@ -55,14 +64,14 @@ class ArtifactManagerTest {
5564
@Test
5665
fun `fetch artifact does not have any valid lsp versions`() {
5766
every { manifestFetcher.fetch() }.returns(ManifestManager.Manifest())
58-
artifactManager = spyk(ArtifactManager(manifestFetcher, artifactHelper, manifestVersionRanges))
67+
artifactManager = spyk(ArtifactManager(mockProject, manifestFetcher, artifactHelper, manifestVersionRanges))
5968

6069
every { artifactManager.getLSPVersionsFromManifestWithSpecifiedRange(any()) }.returns(
6170
ArtifactManager.LSPVersions(deListedVersions = emptyList(), inRangeVersions = emptyList())
6271
)
6372

6473
assertThatThrownBy {
65-
artifactManager.fetchArtifact()
74+
runBlocking { artifactManager.fetchArtifact() }
6675
}
6776
.isInstanceOf(LspException::class.java)
6877
.hasFieldOrPropertyWithValue("errorCode", LspException.ErrorCode.NO_COMPATIBLE_LSP_VERSION)
@@ -75,7 +84,7 @@ class ArtifactManagerTest {
7584
every { manifestFetcher.fetch() }.returns(ManifestManager.Manifest())
7685
every { artifactHelper.getAllLocalLspArtifactsWithinManifestRange(any()) }.returns(expectedResult)
7786

78-
artifactManager.fetchArtifact()
87+
runBlocking { artifactManager.fetchArtifact() }
7988

8089
verify(exactly = 1) { manifestFetcher.fetch() }
8190
verify(exactly = 1) { artifactHelper.getAllLocalLspArtifactsWithinManifestRange(any()) }
@@ -86,7 +95,7 @@ class ArtifactManagerTest {
8695
val target = ManifestManager.VersionTarget(platform = "temp", arch = "temp")
8796
val versions = listOf(ManifestManager.Version("1.0.0", targets = listOf(target)))
8897

89-
artifactManager = spyk(ArtifactManager(manifestFetcher, artifactHelper, manifestVersionRanges))
98+
artifactManager = spyk(ArtifactManager(mockProject, manifestFetcher, artifactHelper, manifestVersionRanges))
9099

91100
every { artifactManager.getLSPVersionsFromManifestWithSpecifiedRange(any()) }.returns(
92101
ArtifactManager.LSPVersions(deListedVersions = emptyList(), inRangeVersions = versions)
@@ -98,21 +107,22 @@ class ArtifactManagerTest {
98107
every { getCurrentArchitecture() }.returns("temp")
99108

100109
every { artifactHelper.getExistingLspArtifacts(any(), any()) }.returns(false)
101-
every { artifactHelper.tryDownloadLspArtifacts(any(), any()) } just Runs
110+
coEvery { artifactHelper.tryDownloadLspArtifacts(any(), any(), any()) } returns tempDir
102111
every { artifactHelper.deleteOlderLspArtifacts(any()) } just Runs
103112

104-
artifactManager.fetchArtifact()
113+
runBlocking { artifactManager.fetchArtifact() }
105114

106-
verify(exactly = 1) { artifactHelper.tryDownloadLspArtifacts(any(), any()) }
115+
verify(exactly = 1) { runBlocking { artifactHelper.tryDownloadLspArtifacts(any(), any(), any()) } }
107116
verify(exactly = 1) { artifactHelper.deleteOlderLspArtifacts(any()) }
108117
}
109118

110119
@Test
111120
fun `fetch artifact does not have valid version in local system`() {
112121
val target = ManifestManager.VersionTarget(platform = "temp", arch = "temp")
113122
val versions = listOf(ManifestManager.Version("1.0.0", targets = listOf(target)))
123+
val expectedResult = listOf(Pair(tempDir, SemVer("1.0.0", 1, 0, 0)))
114124

115-
artifactManager = spyk(ArtifactManager(manifestFetcher, artifactHelper, manifestVersionRanges))
125+
artifactManager = spyk(ArtifactManager(mockProject, manifestFetcher, artifactHelper, manifestVersionRanges))
116126

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

126136
every { artifactHelper.getExistingLspArtifacts(any(), any()) }.returns(true)
127137
every { artifactHelper.deleteOlderLspArtifacts(any()) } just Runs
138+
every { artifactHelper.getAllLocalLspArtifactsWithinManifestRange(any()) }.returns(expectedResult)
128139

129-
artifactManager.fetchArtifact()
140+
runBlocking { artifactManager.fetchArtifact() }
130141

131-
verify(exactly = 0) { artifactHelper.tryDownloadLspArtifacts(any(), any()) }
142+
verify(exactly = 0) { runBlocking { artifactHelper.tryDownloadLspArtifacts(any(), any(), any()) } }
132143
verify(exactly = 1) { artifactHelper.deleteOlderLspArtifacts(any()) }
133144
}
134145
}

‎plugins/core/resources/resources/software/aws/toolkits/resources/MessagesBundle.properties‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ amazonqFeatureDev.placeholder.after_code_generation=Choose an option to proceed
141141
amazonqFeatureDev.placeholder.after_monthly_limit=Chat input is disabled
142142
amazonqFeatureDev.placeholder.closed_session=Open a new chat tab to continue
143143
amazonqFeatureDev.placeholder.context_gathering_complete=Gathering context...
144+
amazonqFeatureDev.placeholder.downloading_and_extracting_lsp_artifacts=Downloading and Extracting Lsp Artifacts...
144145
amazonqFeatureDev.placeholder.generating_code=Generating code...
145146
amazonqFeatureDev.placeholder.new_plan=Describe your task or issue in as much detail as possible
146147
amazonqFeatureDev.placeholder.provide_code_feedback=Provide feedback or comments

0 commit comments

Comments
 (0)