Skip to content

Commit 8bc071d

Browse files
rlirli
committed
fix(amazonq): only download single copy of LSP at once
This addresses multiple problems with the Flare dependency resolution: * Multiple projects starting will attempt to download the same artifact concurrently to the same location * If the user has never downloaded the LSP, they always get an error because we incorrectly assume that a file exists * Multiple projects can resolve different versions of the server/client
1 parent 2fafb49 commit 8bc071d

File tree

8 files changed

+91
-76
lines changed

8 files changed

+91
-76
lines changed

‎plugins/amazonq/chat/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/toolwindow/AmazonQPanel.kt‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import com.intellij.idea.AppMode
77
import com.intellij.openapi.Disposable
88
import com.intellij.openapi.application.ApplicationManager
99
import com.intellij.openapi.application.runInEdt
10+
import com.intellij.openapi.components.service
1011
import com.intellij.openapi.project.Project
1112
import com.intellij.openapi.util.Disposer
1213
import com.intellij.ui.components.JBLoadingPanel
@@ -18,8 +19,10 @@ import com.intellij.ui.dsl.builder.AlignX
1819
import com.intellij.ui.dsl.builder.AlignY
1920
import com.intellij.ui.dsl.builder.panel
2021
import com.intellij.ui.jcef.JBCefApp
22+
import kotlinx.coroutines.runBlocking
2123
import software.aws.toolkits.jetbrains.isDeveloperMode
2224
import software.aws.toolkits.jetbrains.services.amazonq.lsp.artifacts.ArtifactHelper
25+
import software.aws.toolkits.jetbrains.services.amazonq.lsp.artifacts.ArtifactManager
2326
import software.aws.toolkits.jetbrains.services.amazonq.webview.Browser
2427
import java.awt.event.ActionListener
2528
import java.util.concurrent.CompletableFuture
@@ -88,7 +91,7 @@ class AmazonQPanel(private val parent: Disposable, val project: Project) {
8891
wrapper.setContent(loadingPanel)
8992

9093
ApplicationManager.getApplication().executeOnPooledThread {
91-
val webUri = ArtifactHelper().getLatestLocalLspArtifact().resolve("amazonq-ui.js").toUri()
94+
val webUri = runBlocking { service<ArtifactManager>().fetchArtifact(project).resolve("amazonq-ui.js").toUri() }
9295
loadingPanel.stopLoading()
9396
runInEdt {
9497
browser.complete(

‎plugins/amazonq/chat/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/webview/Browser.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ class Browser(parent: Disposable, private val webUri: URI, val project: Project)
116116
): String {
117117
val quickActionConfig = generateQuickActionConfig()
118118
val postMessageToJavaJsCode = receiveMessageQuery.inject("JSON.stringify(message)")
119+
// language=HTML
119120
val jsScripts = """
120121
<script type="text/javascript" src="$webUri" defer onload="init()"></script>
121122
<script type="text/javascript">

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ private class AmazonQServerInstance(private val project: Project, private val cs
266266

267267
init {
268268
// will cause slow service init, but maybe fine for now. will not block UI since fetch/extract will be under background progress
269-
val artifact = runBlocking { ArtifactManager(project, manifestRange = null).fetchArtifact() }.toAbsolutePath()
269+
val artifact = runBlocking { service<ArtifactManager>().fetchArtifact(project) }.toAbsolutePath()
270270
val node = if (SystemInfo.isWindows) "node.exe" else "node"
271271
val cmd = GeneralCommandLine(
272272
artifact.resolve(node).toString(),

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

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

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

6+
import com.intellij.openapi.progress.ProgressManager
67
import com.intellij.openapi.project.Project
78
import com.intellij.platform.ide.progress.withBackgroundProgress
89
import com.intellij.util.io.createDirectories
@@ -18,16 +19,14 @@ import software.aws.toolkits.core.utils.warn
1819
import software.aws.toolkits.jetbrains.core.saveFileFromUrl
1920
import software.aws.toolkits.jetbrains.services.amazonq.project.manifest.ManifestManager
2021
import software.aws.toolkits.resources.AwsCoreBundle
22+
import java.nio.file.Files
2123
import java.nio.file.Path
2224
import java.util.concurrent.atomic.AtomicInteger
2325

24-
class ArtifactHelper(private val lspArtifactsPath: Path = DEFAULT_ARTIFACT_PATH, private val maxDownloadAttempts: Int = MAX_DOWNLOAD_ATTEMPTS) {
25-
26-
companion object {
27-
private val DEFAULT_ARTIFACT_PATH = getToolkitsCommonCacheRoot().resolve("aws").resolve("toolkits").resolve("language-servers")
28-
private val logger = getLogger<ArtifactHelper>()
29-
private const val MAX_DOWNLOAD_ATTEMPTS = 3
30-
}
26+
class ArtifactHelper internal constructor(
27+
private val lspArtifactsPath: Path = DEFAULT_ARTIFACT_PATH,
28+
private val maxDownloadAttempts: Int = MAX_DOWNLOAD_ATTEMPTS,
29+
) {
3130
private val currentAttempt = AtomicInteger(0)
3231

3332
fun removeDelistedVersions(delistedVersions: List<ManifestManager.Version>) {
@@ -79,16 +78,6 @@ class ArtifactHelper(private val lspArtifactsPath: Path = DEFAULT_ARTIFACT_PATH,
7978
.sortedByDescending { (_, semVer) -> semVer }
8079
}
8180

82-
fun getLatestLocalLspArtifact(): Path {
83-
val localFolders = getSubFolders(lspArtifactsPath)
84-
return localFolders.map { localFolder ->
85-
localFolder to SemVer.parseFromText(localFolder.fileName.toString())
86-
}
87-
.sortedByDescending { (_, semVer) -> semVer }
88-
.first()
89-
.first
90-
}
91-
9281
fun getExistingLspArtifacts(versions: List<ManifestManager.Version>, target: ManifestManager.VersionTarget?): Boolean {
9382
if (versions.isEmpty() || target?.contents == null) return false
9483

@@ -114,7 +103,7 @@ class ArtifactHelper(private val lspArtifactsPath: Path = DEFAULT_ARTIFACT_PATH,
114103
}
115104

116105
suspend fun tryDownloadLspArtifacts(project: Project, versions: List<ManifestManager.Version>, target: ManifestManager.VersionTarget?): Path? {
117-
val temporaryDownloadPath = lspArtifactsPath.resolve("temp")
106+
val temporaryDownloadPath = Files.createTempDirectory("lsp-dl")
118107
val downloadPath = lspArtifactsPath.resolve(versions.first().serverVersion.toString())
119108

120109
while (currentAttempt.get() < maxDownloadAttempts) {
@@ -188,7 +177,7 @@ class ArtifactHelper(private val lspArtifactsPath: Path = DEFAULT_ARTIFACT_PATH,
188177
try {
189178
if (!filePath.exists()) {
190179
logger.info { "Downloading file: ${filePath.fileName}" }
191-
saveFileFromUrl(url, filePath)
180+
saveFileFromUrl(url, filePath, ProgressManager.getInstance().progressIndicator)
192181
}
193182
if (!validateFileHash(filePath, expectedHash)) {
194183
logger.warn { "Hash mismatch for ${filePath.fileName}, re-downloading" }
@@ -222,4 +211,10 @@ class ArtifactHelper(private val lspArtifactsPath: Path = DEFAULT_ARTIFACT_PATH,
222211
throw LspException(errorMessage, LspException.ErrorCode.DOWNLOAD_FAILED)
223212
}
224213
}
214+
215+
companion object {
216+
private val DEFAULT_ARTIFACT_PATH = getToolkitsCommonCacheRoot().resolve("aws").resolve("toolkits").resolve("language-servers")
217+
private val logger = getLogger<ArtifactHelper>()
218+
private const val MAX_DOWNLOAD_ATTEMPTS = 3
219+
}
225220
}

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

Lines changed: 53 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,29 @@
33

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

6+
import com.intellij.openapi.components.Service
67
import com.intellij.openapi.project.Project
78
import com.intellij.util.text.SemVer
9+
import kotlinx.coroutines.Deferred
10+
import kotlinx.coroutines.async
11+
import kotlinx.coroutines.coroutineScope
12+
import kotlinx.coroutines.sync.Mutex
13+
import kotlinx.coroutines.sync.withLock
814
import org.jetbrains.annotations.VisibleForTesting
915
import software.aws.toolkits.core.utils.error
1016
import software.aws.toolkits.core.utils.getLogger
1117
import software.aws.toolkits.core.utils.info
1218
import software.aws.toolkits.jetbrains.services.amazonq.project.manifest.ManifestManager
1319
import java.nio.file.Path
1420

15-
class ArtifactManager(
16-
private val project: Project,
17-
private val manifestFetcher: ManifestFetcher = ManifestFetcher(),
18-
private val artifactHelper: ArtifactHelper = ArtifactHelper(),
19-
manifestRange: SupportedManifestVersionRange?,
20-
) {
21+
@Service
22+
class ArtifactManager {
23+
private val manifestFetcher: ManifestFetcher = ManifestFetcher()
24+
private val artifactHelper: ArtifactHelper = ArtifactHelper()
25+
26+
// we currently cannot handle the versions swithing in the middle of a user's session
27+
private val mutex = Mutex()
28+
private var artifactDeferred: Deferred<Path>? = null
2129

2230
data class SupportedManifestVersionRange(
2331
val startVersion: SemVer,
@@ -28,8 +36,6 @@ class ArtifactManager(
2836
val inRangeVersions: List<ManifestManager.Version>,
2937
)
3038

31-
private val manifestVersionRanges: SupportedManifestVersionRange = manifestRange ?: DEFAULT_VERSION_RANGE
32-
3339
companion object {
3440
private val DEFAULT_VERSION_RANGE = SupportedManifestVersionRange(
3541
startVersion = SemVer("0.0.0", 0, 0, 0),
@@ -38,35 +44,47 @@ class ArtifactManager(
3844
private val logger = getLogger<ArtifactManager>()
3945
}
4046

41-
suspend fun fetchArtifact(): Path {
42-
val manifest = manifestFetcher.fetch() ?: throw LspException(
43-
"Language Support is not available, as manifest is missing.",
44-
LspException.ErrorCode.MANIFEST_FETCH_FAILED
45-
)
46-
val lspVersions = getLSPVersionsFromManifestWithSpecifiedRange(manifest)
47+
suspend fun fetchArtifact(project: Project): Path {
48+
mutex.withLock { artifactDeferred }?.let {
49+
return it.await()
50+
}
4751

48-
this.artifactHelper.removeDelistedVersions(lspVersions.deListedVersions)
52+
return mutex.withLock {
53+
coroutineScope {
54+
async {
55+
val manifest = manifestFetcher.fetch() ?: throw LspException(
56+
"Language Support is not available, as manifest is missing.",
57+
LspException.ErrorCode.MANIFEST_FETCH_FAILED
58+
)
59+
val lspVersions = getLSPVersionsFromManifestWithSpecifiedRange(manifest)
4960

50-
if (lspVersions.inRangeVersions.isEmpty()) {
51-
// No versions are found which are in the given range. Fallback to local lsp artifacts.
52-
val localLspArtifacts = this.artifactHelper.getAllLocalLspArtifactsWithinManifestRange(manifestVersionRanges)
53-
if (localLspArtifacts.isNotEmpty()) {
54-
return localLspArtifacts.first().first
55-
}
56-
throw LspException("Language server versions not found in manifest.", LspException.ErrorCode.NO_COMPATIBLE_LSP_VERSION)
57-
}
61+
artifactHelper.removeDelistedVersions(lspVersions.deListedVersions)
5862

59-
// If there is an LSP Manifest with the same version
60-
val target = getTargetFromLspManifest(lspVersions.inRangeVersions)
61-
// Get Local LSP files and check if we can re-use existing LSP Artifacts
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)
67-
}
68-
this.artifactHelper.deleteOlderLspArtifacts(manifestVersionRanges)
69-
return artifactPath
63+
if (lspVersions.inRangeVersions.isEmpty()) {
64+
// No versions are found which are in the given range. Fallback to local lsp artifacts.
65+
val localLspArtifacts = artifactHelper.getAllLocalLspArtifactsWithinManifestRange(DEFAULT_VERSION_RANGE)
66+
if (localLspArtifacts.isNotEmpty()) {
67+
return@async localLspArtifacts.first().first
68+
}
69+
throw LspException("Language server versions not found in manifest.", LspException.ErrorCode.NO_COMPATIBLE_LSP_VERSION)
70+
}
71+
72+
// If there is an LSP Manifest with the same version
73+
val target = getTargetFromLspManifest(lspVersions.inRangeVersions)
74+
// Get Local LSP files and check if we can re-use existing LSP Artifacts
75+
val artifactPath: Path = if (artifactHelper.getExistingLspArtifacts(lspVersions.inRangeVersions, target)) {
76+
artifactHelper.getAllLocalLspArtifactsWithinManifestRange(DEFAULT_VERSION_RANGE).first().first
77+
} else {
78+
artifactHelper.tryDownloadLspArtifacts(project, lspVersions.inRangeVersions, target)
79+
?: throw LspException("Failed to download LSP artifacts", LspException.ErrorCode.DOWNLOAD_FAILED)
80+
}
81+
artifactHelper.deleteOlderLspArtifacts(DEFAULT_VERSION_RANGE)
82+
return@async artifactPath
83+
}
84+
}.also {
85+
artifactDeferred = it
86+
}
87+
}.await()
7088
}
7189

7290
@VisibleForTesting
@@ -78,7 +96,7 @@ class ArtifactManager(
7896
SemVer.parseFromText(serverVersion)?.let { semVer ->
7997
when {
8098
version.isDelisted != false -> Pair(version, true) // Is deListed
81-
semVer in manifestVersionRanges.startVersion..manifestVersionRanges.endVersion -> Pair(version, false) // Is in range
99+
semVer in DEFAULT_VERSION_RANGE.let { it.startVersion..it.endVersion } -> Pair(version, false) // Is in range
82100
else -> null
83101
}
84102
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ class ManifestFetcher(
2525
private val logger = getLogger<ManifestFetcher>()
2626

2727
private const val DEFAULT_MANIFEST_URL =
28-
"https://aws-toolkit-language-servers.amazonaws.com/remoteWorkspaceContext/0/manifest.json"
28+
"https://d3akiidp1wvqyg.cloudfront.net/qAgenticChatServer/0/manifest.json"
2929

3030
private val DEFAULT_MANIFEST_PATH: Path = getToolkitsCommonCacheRoot()
3131
.resolve("aws")

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

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,28 +3,31 @@
33

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

6-
import com.intellij.openapi.project.Project
6+
import com.intellij.testFramework.ProjectExtension
77
import com.intellij.util.text.SemVer
88
import io.mockk.Runs
99
import io.mockk.coEvery
1010
import io.mockk.every
1111
import io.mockk.just
12-
import io.mockk.mockk
1312
import io.mockk.mockkStatic
1413
import io.mockk.spyk
1514
import io.mockk.verify
1615
import kotlinx.coroutines.runBlocking
1716
import org.assertj.core.api.Assertions.assertThatThrownBy
18-
import org.jetbrains.annotations.TestOnly
1917
import org.junit.jupiter.api.BeforeEach
2018
import org.junit.jupiter.api.Test
19+
import org.junit.jupiter.api.extension.RegisterExtension
2120
import org.junit.jupiter.api.io.TempDir
2221
import software.aws.toolkits.jetbrains.services.amazonq.lsp.artifacts.ArtifactManager.SupportedManifestVersionRange
2322
import software.aws.toolkits.jetbrains.services.amazonq.project.manifest.ManifestManager
2423
import java.nio.file.Path
2524

26-
@TestOnly
2725
class ArtifactManagerTest {
26+
companion object {
27+
@JvmField
28+
@RegisterExtension
29+
val projectExtension = ProjectExtension()
30+
}
2831

2932
@TempDir
3033
lateinit var tempDir: Path
@@ -33,7 +36,6 @@ class ArtifactManagerTest {
3336
private lateinit var artifactManager: ArtifactManager
3437
private lateinit var manifestFetcher: ManifestFetcher
3538
private lateinit var manifestVersionRanges: SupportedManifestVersionRange
36-
private lateinit var mockProject: Project
3739

3840
@BeforeEach
3941
fun setUp() {
@@ -43,19 +45,15 @@ class ArtifactManagerTest {
4345
startVersion = SemVer("1.0.0", 1, 0, 0),
4446
endVersion = SemVer("2.0.0", 2, 0, 0)
4547
)
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)
48+
artifactManager = ArtifactManager()
5149
}
5250

5351
@Test
5452
fun `fetch artifact fetcher throws exception if manifest is null`() {
5553
every { manifestFetcher.fetch() }.returns(null)
5654

5755
assertThatThrownBy {
58-
runBlocking { artifactManager.fetchArtifact() }
56+
runBlocking { artifactManager.fetchArtifact(projectExtension.project) }
5957
}
6058
.isInstanceOf(LspException::class.java)
6159
.hasFieldOrPropertyWithValue("errorCode", LspException.ErrorCode.MANIFEST_FETCH_FAILED)
@@ -64,14 +62,14 @@ class ArtifactManagerTest {
6462
@Test
6563
fun `fetch artifact does not have any valid lsp versions`() {
6664
every { manifestFetcher.fetch() }.returns(ManifestManager.Manifest())
67-
artifactManager = spyk(ArtifactManager(mockProject, manifestFetcher, artifactHelper, manifestVersionRanges))
65+
artifactManager = spyk(ArtifactManager())
6866

6967
every { artifactManager.getLSPVersionsFromManifestWithSpecifiedRange(any()) }.returns(
7068
ArtifactManager.LSPVersions(deListedVersions = emptyList(), inRangeVersions = emptyList())
7169
)
7270

7371
assertThatThrownBy {
74-
runBlocking { artifactManager.fetchArtifact() }
72+
runBlocking { artifactManager.fetchArtifact(projectExtension.project) }
7573
}
7674
.isInstanceOf(LspException::class.java)
7775
.hasFieldOrPropertyWithValue("errorCode", LspException.ErrorCode.NO_COMPATIBLE_LSP_VERSION)
@@ -84,7 +82,7 @@ class ArtifactManagerTest {
8482
every { manifestFetcher.fetch() }.returns(ManifestManager.Manifest())
8583
every { artifactHelper.getAllLocalLspArtifactsWithinManifestRange(any()) }.returns(expectedResult)
8684

87-
runBlocking { artifactManager.fetchArtifact() }
85+
runBlocking { artifactManager.fetchArtifact(projectExtension.project) }
8886

8987
verify(exactly = 1) { manifestFetcher.fetch() }
9088
verify(exactly = 1) { artifactHelper.getAllLocalLspArtifactsWithinManifestRange(any()) }
@@ -95,7 +93,7 @@ class ArtifactManagerTest {
9593
val target = ManifestManager.VersionTarget(platform = "temp", arch = "temp")
9694
val versions = listOf(ManifestManager.Version("1.0.0", targets = listOf(target)))
9795

98-
artifactManager = spyk(ArtifactManager(mockProject, manifestFetcher, artifactHelper, manifestVersionRanges))
96+
artifactManager = spyk(ArtifactManager())
9997

10098
every { artifactManager.getLSPVersionsFromManifestWithSpecifiedRange(any()) }.returns(
10199
ArtifactManager.LSPVersions(deListedVersions = emptyList(), inRangeVersions = versions)
@@ -110,7 +108,7 @@ class ArtifactManagerTest {
110108
coEvery { artifactHelper.tryDownloadLspArtifacts(any(), any(), any()) } returns tempDir
111109
every { artifactHelper.deleteOlderLspArtifacts(any()) } just Runs
112110

113-
runBlocking { artifactManager.fetchArtifact() }
111+
runBlocking { artifactManager.fetchArtifact(projectExtension.project) }
114112

115113
verify(exactly = 1) { runBlocking { artifactHelper.tryDownloadLspArtifacts(any(), any(), any()) } }
116114
verify(exactly = 1) { artifactHelper.deleteOlderLspArtifacts(any()) }
@@ -122,7 +120,7 @@ class ArtifactManagerTest {
122120
val versions = listOf(ManifestManager.Version("1.0.0", targets = listOf(target)))
123121
val expectedResult = listOf(Pair(tempDir, SemVer("1.0.0", 1, 0, 0)))
124122

125-
artifactManager = spyk(ArtifactManager(mockProject, manifestFetcher, artifactHelper, manifestVersionRanges))
123+
artifactManager = spyk(ArtifactManager())
126124

127125
every { artifactManager.getLSPVersionsFromManifestWithSpecifiedRange(any()) }.returns(
128126
ArtifactManager.LSPVersions(deListedVersions = emptyList(), inRangeVersions = versions)
@@ -137,7 +135,7 @@ class ArtifactManagerTest {
137135
every { artifactHelper.deleteOlderLspArtifacts(any()) } just Runs
138136
every { artifactHelper.getAllLocalLspArtifactsWithinManifestRange(any()) }.returns(expectedResult)
139137

140-
runBlocking { artifactManager.fetchArtifact() }
138+
runBlocking { artifactManager.fetchArtifact(projectExtension.project) }
141139

142140
verify(exactly = 0) { runBlocking { artifactHelper.tryDownloadLspArtifacts(any(), any(), any()) } }
143141
verify(exactly = 1) { artifactHelper.deleteOlderLspArtifacts(any()) }

0 commit comments

Comments
 (0)