Skip to content

Commit c00dfa0

Browse files
rlirli
authored
Cleanup weird usage of coroutine scopes in ProjectContext / FeatureDevSessionContext (#4861)
A `suspend` function implicitly has a coroutine context
1 parent f009e6f commit c00dfa0

File tree

7 files changed

+39
-32
lines changed

7 files changed

+39
-32
lines changed

‎plugins/amazonq/chat/jetbrains-community/src/software/aws/toolkits/jetbrains/services/cwc/editor/context/project/EncoderServer.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ class EncoderServer(val project: Project) : Disposable {
5151
val port by lazy { NetUtils.findAvailableSocketPort() }
5252
private val nodeRunnableName = if (manifestManager.getOs() == "windows") "node.exe" else "node"
5353
private val maxRetry: Int = 3
54-
val key = generateHmacKey()
54+
private val key = generateHmacKey()
5555
private var processHandler: KillableProcessHandler? = null
5656
private val mapper = jacksonObjectMapper()
5757

‎plugins/amazonq/chat/jetbrains-community/src/software/aws/toolkits/jetbrains/services/cwc/editor/context/project/ProjectContextController.kt‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,18 @@ import com.intellij.openapi.components.Service
88
import com.intellij.openapi.components.service
99
import com.intellij.openapi.project.Project
1010
import com.intellij.openapi.util.Disposer
11+
import kotlinx.coroutines.CoroutineScope
1112
import kotlinx.coroutines.launch
1213
import software.aws.toolkits.core.utils.getLogger
1314
import software.aws.toolkits.core.utils.warn
14-
import software.aws.toolkits.jetbrains.core.coroutines.disposableCoroutineScope
1515
import software.aws.toolkits.jetbrains.services.codewhisperer.settings.CodeWhispererSettings
1616

1717
@Service(Service.Level.PROJECT)
18-
class ProjectContextController(private val project: Project) : Disposable {
18+
class ProjectContextController(private val project: Project, private val cs: CoroutineScope) : Disposable {
1919
private val encoderServer: EncoderServer = EncoderServer(project)
20-
private val projectContextProvider: ProjectContextProvider = ProjectContextProvider(project, encoderServer)
21-
private val scope = disposableCoroutineScope(this)
20+
private val projectContextProvider: ProjectContextProvider = ProjectContextProvider(project, encoderServer, cs)
2221
init {
23-
scope.launch {
22+
cs.launch {
2423
if (CodeWhispererSettings.getInstance().isProjectContextEnabled()) {
2524
encoderServer.downloadArtifactsAndStartServer()
2625
}

‎plugins/amazonq/chat/jetbrains-community/src/software/aws/toolkits/jetbrains/services/cwc/editor/context/project/ProjectContextProvider.kt‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import com.intellij.openapi.vfs.VfsUtilCore
1414
import com.intellij.openapi.vfs.VirtualFile
1515
import com.intellij.openapi.vfs.VirtualFileVisitor
1616
import com.intellij.openapi.vfs.isFile
17+
import kotlinx.coroutines.CoroutineScope
1718
import kotlinx.coroutines.delay
1819
import kotlinx.coroutines.launch
1920
import kotlinx.coroutines.runBlocking
@@ -22,7 +23,6 @@ import software.aws.toolkits.core.utils.debug
2223
import software.aws.toolkits.core.utils.getLogger
2324
import software.aws.toolkits.core.utils.info
2425
import software.aws.toolkits.core.utils.warn
25-
import software.aws.toolkits.jetbrains.core.coroutines.disposableCoroutineScope
2626
import software.aws.toolkits.jetbrains.services.amazonq.FeatureDevSessionContext
2727
import software.aws.toolkits.jetbrains.services.codewhisperer.settings.CodeWhispererSettings
2828
import software.aws.toolkits.jetbrains.services.cwc.controller.chat.telemetry.TelemetryHelper
@@ -33,13 +33,12 @@ import java.net.URL
3333
import java.util.concurrent.atomic.AtomicBoolean
3434
import java.util.concurrent.atomic.AtomicInteger
3535

36-
class ProjectContextProvider(val project: Project, private val encoderServer: EncoderServer) : Disposable {
36+
class ProjectContextProvider(val project: Project, private val encoderServer: EncoderServer, private val cs: CoroutineScope) : Disposable {
3737
private val retryCount = AtomicInteger(0)
3838
val isIndexComplete = AtomicBoolean(false)
3939
private val mapper = jacksonObjectMapper()
40-
private val scope = disposableCoroutineScope(this)
4140
init {
42-
scope.launch {
41+
cs.launch {
4342
if (CodeWhispererSettings.getInstance().isProjectContextEnabled()) {
4443
while (true) {
4544
if (encoderServer.isNodeProcessRunning()) {
@@ -106,7 +105,7 @@ class ProjectContextProvider(val project: Project, private val encoderServer: En
106105
)
107106

108107
private fun initAndIndex() {
109-
scope.launch {
108+
cs.launch {
110109
while (retryCount.get() < 5) {
111110
try {
112111
logger.info { "project context: about to init key" }
@@ -283,7 +282,7 @@ class ProjectContextProvider(val project: Project, private val encoderServer: En
283282
// TODO: refactor this along with /dev & codescan file traversing logic
284283
override fun visitFile(file: VirtualFile): Boolean {
285284
if ((file.isDirectory && isBuildOrBin(file.name)) ||
286-
runBlocking { featureDevSessionContext.ignoreFile(file.name, scope) } ||
285+
runBlocking { featureDevSessionContext.ignoreFile(file.name) } ||
287286
(file.isFile && file.length > 10 * 1024 * 1024)
288287
) {
289288
return false

‎plugins/amazonq/chat/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/workspace/context/EncoderServerTest.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ class EncoderServerTest {
2121
private val inputBytes = BigInteger(32, DigestUtil.random).toByteArray()
2222

2323
@Before
24-
open fun setup() {
24+
fun setup() {
2525
encoderServer = EncoderServer(projectRule.project)
2626
}
2727

‎plugins/amazonq/chat/jetbrains-community/tst/software/aws/toolkits/jetbrains/services/amazonq/workspace/context/ProjectContextProviderTest.kt‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
package software.aws.toolkits.jetbrains.services.amazonq.workspace.context
55

66
import com.intellij.openapi.project.Project
7+
import kotlinx.coroutines.test.TestScope
8+
import kotlinx.coroutines.test.runTest
79
import org.junit.Before
810
import org.junit.Rule
911
import org.mockito.kotlin.any
@@ -22,18 +24,20 @@ class ProjectContextProviderTest {
2224
@Rule
2325
@JvmField
2426
val projectRule: CodeInsightTestFixtureRule = JavaCodeInsightTestFixtureRule()
25-
internal lateinit var project: Project
27+
28+
private val project: Project
29+
get() = projectRule.project
30+
2631
private val encoderServer: EncoderServer = mock()
2732
private lateinit var sut: ProjectContextProvider
2833

2934
@Before
30-
open fun setup() {
31-
project = projectRule.project
32-
sut = ProjectContextProvider(project, encoderServer)
35+
fun setup() {
36+
sut = ProjectContextProvider(project, encoderServer, TestScope())
3337
}
3438

3539
@Test
36-
fun `test index payload is encrypted`() {
40+
fun `test index payload is encrypted`() = runTest {
3741
whenever(encoderServer.port).thenReturn(3000)
3842
try {
3943
sut.index()
@@ -43,7 +47,8 @@ class ProjectContextProviderTest {
4347
verify(encoderServer, times(1)).encrypt(any())
4448
}
4549

46-
fun `test query payload is encrypted`() {
50+
@Test
51+
fun `test query payload is encrypted`() = runTest {
4752
whenever(encoderServer.port).thenReturn(3000)
4853
try {
4954
sut.query("what does this project do")

‎plugins/amazonq/codewhisperer/jetbrains-community/src/software/aws/toolkits/jetbrains/services/codewhisperer/codescan/sessionconfig/CodeScanSessionConfig.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ class CodeScanSessionConfig(
186186

187187
if (!current.isDirectory) {
188188
if (current.isFile && !changeListManager.isIgnoredFile(current) &&
189-
runBlocking { !featureDevSessionContext.ignoreFile(current, this) } &&
189+
runBlocking { !featureDevSessionContext.ignoreFile(current) } &&
190190
runReadAction { !fileIndex.isInLibrarySource(current) }
191191
) {
192192
if (willExceedPayloadLimit(currentTotalFileSize, current.length)) {
@@ -209,7 +209,7 @@ class CodeScanSessionConfig(
209209
} else {
210210
// Directory case: only traverse if not ignored
211211
if (!changeListManager.isIgnoredFile(current) &&
212-
runBlocking { !featureDevSessionContext.ignoreFile(current, this) } &&
212+
runBlocking { !featureDevSessionContext.ignoreFile(current) } &&
213213
!traversedDirectories.contains(current) && current.isValid &&
214214
runReadAction { !fileIndex.isInLibrarySource(current) }
215215
) {

‎plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/FeatureDevSessionContext.kt‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import com.intellij.openapi.vfs.VirtualFile
1010
import com.intellij.openapi.vfs.VirtualFileVisitor
1111
import com.intellij.openapi.vfs.isFile
1212
import com.intellij.platform.ide.progress.withBackgroundProgress
13-
import kotlinx.coroutines.CoroutineScope
1413
import kotlinx.coroutines.async
1514
import kotlinx.coroutines.flow.channelFlow
1615
import kotlinx.coroutines.launch
@@ -29,6 +28,7 @@ import java.nio.file.Files
2928
import java.nio.file.Path
3029
import java.util.Base64
3130
import java.util.zip.ZipOutputStream
31+
import kotlin.coroutines.coroutineContext
3232
import kotlin.io.path.Path
3333
import kotlin.io.path.relativeTo
3434

@@ -97,19 +97,23 @@ class FeatureDevSessionContext(val project: Project, val maxProjectSizeBytes: Lo
9797
return FeatureDevBundleConfig.ALLOWED_CODE_EXTENSIONS.contains(extension)
9898
}
9999

100-
private fun ignoreFileByExtension(file: VirtualFile, scope: CoroutineScope): Boolean = with(scope) {
101-
return !isFileExtensionAllowed(file)
102-
}
100+
private fun ignoreFileByExtension(file: VirtualFile) =
101+
!isFileExtensionAllowed(file)
103102

104-
suspend fun ignoreFile(file: VirtualFile, scope: CoroutineScope): Boolean = ignoreFile(file.path, scope)
103+
suspend fun ignoreFile(file: VirtualFile): Boolean = ignoreFile(file.path)
105104

106-
suspend fun ignoreFile(path: String, scope: CoroutineScope): Boolean = with(scope) {
105+
suspend fun ignoreFile(path: String): Boolean {
106+
// this method reads like something a JS dev would write and doesn't do what the author thinks
107107
val deferredResults = ignorePatternsWithGitIgnore.map { pattern ->
108-
async {
109-
pattern.containsMatchIn(path)
108+
withContext(coroutineContext) {
109+
async { pattern.containsMatchIn(path) }
110110
}
111111
}
112-
deferredResults.any { it.await() }
112+
113+
// this will serially iterate over and block
114+
// ideally we race the results https://github.com/Kotlin/kotlinx.coroutines/issues/2867
115+
// i.e. Promise.any(...)
116+
return deferredResults.any { it.await() }
113117
}
114118

115119
suspend fun zipFiles(projectRoot: VirtualFile): File = withContext(getCoroutineBgContext()) {
@@ -121,13 +125,13 @@ class FeatureDevSessionContext(val project: Project, val maxProjectSizeBytes: Lo
121125
projectRoot,
122126
object : VirtualFileVisitor<Unit>() {
123127
override fun visitFile(file: VirtualFile): Boolean {
124-
val isFileIgnoredByExtension = runBlocking { ignoreFileByExtension(file, this) }
128+
val isFileIgnoredByExtension = runBlocking { ignoreFileByExtension(file) }
125129
if (isFileIgnoredByExtension) {
126130
val extension = file.extension.orEmpty()
127131
ignoredExtensionMap[extension] = (ignoredExtensionMap[extension] ?: 0) + 1
128132
return false
129133
}
130-
val isFileIgnoredByPattern = runBlocking { ignoreFile(file.name, this) }
134+
val isFileIgnoredByPattern = runBlocking { ignoreFile(file.name) }
131135
if (isFileIgnoredByPattern) {
132136
return false
133137
}

0 commit comments

Comments
 (0)