From bb735c73daf3fd37b6d18fe6ce80a56222ac391a Mon Sep 17 00:00:00 2001 From: Lamberto Basti Date: Mon, 19 Feb 2024 18:01:20 +0100 Subject: [PATCH] 233 - Improved services lazyness and reduced HTTP client retry attempts. (#74) * Improved services lazyness and reduced HTTP client retry attempts. * Remove unused gradleIdentityPathOrNull utility This commit removes the unused 'gradleIdentityPathOrNull' utility from the Utils.kt file. This method was previously used to retrieve Gradle module identity path, but is no longer needed. The appearance and readability of the code have significantly improved as a result. * Refactor PackageSearchApiPackageCache handling The code for handling the network results in the PackageSearchApiPackageCache class has been refactored. Instead of performing a removal of old entries followed by an insertion of new entries, the code now performs an update operation for each new entry. This makes the handling of network results more efficient and concise. --- package-search-api-models | 2 +- .../packagesearch/plugin/core/utils/Utils.kt | 108 ++++++++++++------ .../PackageSearchProjectResolverExtension.kt | 3 +- .../plugin/gradle/utils/Utils.kt | 6 - .../services/PackageSearchProjectService.kt | 49 ++++---- .../packageslist/PackageListViewModel.kt | 4 +- .../utils/PackageSearchApiPackageCache.kt | 56 +++++---- 7 files changed, 141 insertions(+), 87 deletions(-) diff --git a/package-search-api-models b/package-search-api-models index aa00d675..8f144648 160000 --- a/package-search-api-models +++ b/package-search-api-models @@ -1 +1 @@ -Subproject commit aa00d67522436bf4ae38707b0d3fdcc04698ab7e +Subproject commit 8f14464807da90ce08980e589d9d805e6b9d8e76 diff --git a/plugin/core/src/main/kotlin/com/jetbrains/packagesearch/plugin/core/utils/Utils.kt b/plugin/core/src/main/kotlin/com/jetbrains/packagesearch/plugin/core/utils/Utils.kt index c5a6ae92..bb13e54f 100644 --- a/plugin/core/src/main/kotlin/com/jetbrains/packagesearch/plugin/core/utils/Utils.kt +++ b/plugin/core/src/main/kotlin/com/jetbrains/packagesearch/plugin/core/utils/Utils.kt @@ -27,6 +27,8 @@ import com.intellij.openapi.vfs.VirtualFileEvent import com.intellij.openapi.vfs.VirtualFileListener import com.intellij.openapi.vfs.VirtualFileManager import com.intellij.openapi.vfs.newvfs.events.VFileEvent +import com.intellij.openapi.wm.ToolWindowManager +import com.intellij.openapi.wm.ex.ToolWindowManagerListener import com.intellij.util.application import com.intellij.util.messages.MessageBus import com.intellij.util.messages.Topic @@ -44,7 +46,6 @@ import kotlinx.coroutines.flow.FlowCollector import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.callbackFlow import kotlinx.coroutines.flow.channelFlow -import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.merge @@ -68,6 +69,40 @@ fun MessageBus.flow( awaitClose { connection.disconnect() } } +sealed interface FlowEvent { + + @JvmInline + value class Added(val item: T) : FlowEvent + + @JvmInline + value class Removed(val item: T) : FlowEvent + + @JvmInline + value class Initial(val items: List) : FlowEvent +} + +fun MessageBus.bufferFlow( + topic: Topic, + initialValue: (() -> List)? = null, + listener: ProducerScope>.() -> T, +) = channelFlow { + val buffer = mutableSetOf() + flow(topic, listener).onEach { event -> + when (event) { + is FlowEvent.Added -> buffer.add(event.item) + is FlowEvent.Removed -> buffer.remove(event.item) + is FlowEvent.Initial -> { + buffer.clear() + buffer.addAll(event.items) + } + } + send(buffer.toList()) + } + .launchIn(this) + initialValue?.invoke()?.let { send(it) } + awaitClose() +} + val filesChangedEventFlow: Flow> get() = callbackFlow { val disposable = Disposer.newDisposable() @@ -173,43 +208,52 @@ fun Flow.replayOn(vararg replayFlows: Flow<*>) = channelFlow { merge(*replayFlows).collect { mutex.withLock { last?.let { send(it) } } } } -val Project.fileOpenedFlow: Flow> - get() { - val flow = flow { - val buffer: MutableList = FileEditorManager.getInstance(this@fileOpenedFlow).openFiles - .toMutableList() - emit(buffer.toList()) - messageBus.flow(FileEditorManagerListener.FILE_EDITOR_MANAGER) { - object : FileEditorManagerListener { - override fun fileOpened(source: FileEditorManager, file: VirtualFile) { - trySend(FileEditorEvent.FileOpened(file)) - } - - override fun fileClosed(source: FileEditorManager, file: VirtualFile) { - trySend(FileEditorEvent.FileClosed(file)) - } - } - }.collect { - when (it) { - is FileEditorEvent.FileClosed -> buffer.remove(it.file) - is FileEditorEvent.FileOpened -> buffer.add(it.file) - } - emit(buffer.toList()) - } +fun Project.toolWindowOpenedFlow(toolWindowId: String): Flow = callbackFlow { + val manager = ToolWindowManager.getInstance(this@toolWindowOpenedFlow) + val toolWindow = manager.getToolWindow(toolWindowId) + + // Initial state + trySend(toolWindow?.isVisible ?: false) + + val listener = object : ToolWindowManagerListener { + override fun stateChanged(toolWindowManager: ToolWindowManager) { + trySend(manager.getToolWindow(toolWindowId)?.isVisible ?: false) } - return flow.withInitialValue(FileEditorManager.getInstance(this@fileOpenedFlow).openFiles.toList()) } -internal sealed interface FileEditorEvent { + // Register the listener + val connection = messageBus.connect() + connection.subscribe(ToolWindowManagerListener.TOPIC, listener) - val file: VirtualFile + // Cleanup on close + awaitClose { connection.disconnect() } +} - @JvmInline - value class FileOpened(override val file: VirtualFile) : FileEditorEvent +// Usage: +// val toolWindowFlow = project.toolWindowOpenedFlow("YourToolWindowId") +// toolWindowFlow.collect { isOpen -> +// println("Tool window is open: $isOpen") +// } + + +val Project.fileOpenedFlow + get() = messageBus.bufferFlow( + topic = FileEditorManagerListener.FILE_EDITOR_MANAGER, + initialValue = { FileEditorManager.getInstance(this).openFiles.toList() } + ) { + object : FileEditorManagerListener { + override fun fileOpened(source: FileEditorManager, file: VirtualFile) { + trySend(FlowEvent.Added(file)) + } - @JvmInline - value class FileClosed(override val file: VirtualFile) : FileEditorEvent -} + override fun fileClosed(source: FileEditorManager, file: VirtualFile) { + trySend(FlowEvent.Removed(file)) + } + } + } + +val Project.project + get() = this val ExtensionPointName.availableExtensionsFlow: FlowWithInitialValue> get() { diff --git a/plugin/gradle/src/main/kotlin/com/jetbrains/packagesearch/plugin/gradle/PackageSearchProjectResolverExtension.kt b/plugin/gradle/src/main/kotlin/com/jetbrains/packagesearch/plugin/gradle/PackageSearchProjectResolverExtension.kt index 2135e48a..d620403d 100644 --- a/plugin/gradle/src/main/kotlin/com/jetbrains/packagesearch/plugin/gradle/PackageSearchProjectResolverExtension.kt +++ b/plugin/gradle/src/main/kotlin/com/jetbrains/packagesearch/plugin/gradle/PackageSearchProjectResolverExtension.kt @@ -5,6 +5,7 @@ import com.intellij.openapi.externalSystem.model.project.ModuleData import com.jetbrains.packagesearch.plugin.gradle.PackageSearchGradleModel.Configuration import com.jetbrains.packagesearch.plugin.gradle.PackageSearchGradleModel.Dependency import com.jetbrains.packagesearch.plugin.gradle.tooling.PackageSearchGradleJavaModel +import com.jetbrains.packagesearch.plugin.gradle.tooling.PackageSearchGradleModelBuilder import java.nio.file.Paths import org.gradle.tooling.model.idea.IdeaModule import org.jetbrains.plugins.gradle.service.project.AbstractProjectResolverExtension @@ -15,7 +16,7 @@ class PackageSearchProjectResolverExtension : AbstractProjectResolverExtension() setOf(PackageSearchGradleJavaModel::class.java) override fun getToolingExtensionsClasses() = - setOf(com.jetbrains.packagesearch.plugin.gradle.tooling.PackageSearchGradleModelBuilder::class.java) + setOf(PackageSearchGradleModelBuilder::class.java) private inline fun IdeaModule.getExtraProject(): T? = resolverCtx.getExtraProject(this@getExtraProject, T::class.java) diff --git a/plugin/gradle/src/main/kotlin/com/jetbrains/packagesearch/plugin/gradle/utils/Utils.kt b/plugin/gradle/src/main/kotlin/com/jetbrains/packagesearch/plugin/gradle/utils/Utils.kt index 759cc5f5..167bf5f2 100644 --- a/plugin/gradle/src/main/kotlin/com/jetbrains/packagesearch/plugin/gradle/utils/Utils.kt +++ b/plugin/gradle/src/main/kotlin/com/jetbrains/packagesearch/plugin/gradle/utils/Utils.kt @@ -20,12 +20,6 @@ val Module.isGradleSourceSet: Boolean return ExternalSystemApiUtil.getExternalModuleType(this) == GradleConstants.GRADLE_SOURCE_SET_MODULE_TYPE_KEY } -val Module.gradleIdentityPathOrNull: String? - get() = CachedModuleDataFinder.getInstance(project) - .findMainModuleData(this) - ?.data - ?.gradleIdentityPathOrNull - suspend fun Project.awaitExternalSystemInitialization() = suspendCoroutine { ExternalProjectsManager.getInstance(this@awaitExternalSystemInitialization) .runWhenInitialized { it.resume(Unit) } diff --git a/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/services/PackageSearchProjectService.kt b/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/services/PackageSearchProjectService.kt index 2797e46e..f9fab075 100644 --- a/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/services/PackageSearchProjectService.kt +++ b/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/services/PackageSearchProjectService.kt @@ -15,9 +15,8 @@ import com.jetbrains.packagesearch.plugin.core.utils.IntelliJApplication import com.jetbrains.packagesearch.plugin.core.utils.PackageSearchProjectCachesService import com.jetbrains.packagesearch.plugin.core.utils.fileOpenedFlow import com.jetbrains.packagesearch.plugin.core.utils.replayOn -import com.jetbrains.packagesearch.plugin.core.utils.withInitialValue +import com.jetbrains.packagesearch.plugin.core.utils.toolWindowOpenedFlow import com.jetbrains.packagesearch.plugin.fus.logOnlyStableToggle -import com.jetbrains.packagesearch.plugin.ui.model.packageslist.modifiedBy import com.jetbrains.packagesearch.plugin.utils.PackageSearchApplicationCachesService import com.jetbrains.packagesearch.plugin.utils.WindowedModuleBuilderContext import com.jetbrains.packagesearch.plugin.utils.filterNotNullKeys @@ -38,7 +37,7 @@ import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.consumeAsFlow import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.drop +import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flatMapMerge @@ -48,12 +47,8 @@ import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.retry -import kotlinx.coroutines.flow.retryWhen import kotlinx.coroutines.flow.shareIn import kotlinx.coroutines.flow.stateIn -import kotlinx.coroutines.flow.transform -import kotlinx.coroutines.sync.Mutex -import kotlinx.coroutines.sync.withLock import org.jetbrains.packagesearch.api.v3.ApiRepository @Service(Level.PROJECT) @@ -74,7 +69,7 @@ class PackageSearchProjectService( val isProjectExecutingSyncStateFlow = PackageSearchModuleBaseTransformerUtils.extensionsFlow .map { it.map { it.getSyncStateFlow(project) } } .flatMapLatest { combine(it) { it.all { it } } } - .stateIn(coroutineScope, SharingStarted.Eagerly, false) + .stateIn(coroutineScope, SharingStarted.Lazily, false) private val knownRepositoriesStateFlow = timer(12.hours) { IntelliJApplication.PackageSearchApplicationCachesService @@ -103,7 +98,7 @@ class PackageSearchProjectService( val packagesBeingDownloadedFlow = context.getLoadingFLow() .distinctUntilChanged() .onEach { logDebug("${this::class.qualifiedName}#packagesBeingDownloadedFlow") { "$it" } } - .stateIn(coroutineScope, SharingStarted.Eagerly, false) + .stateIn(coroutineScope, SharingStarted.Lazily, false) private val moduleProvidersList get() = combine( @@ -131,15 +126,23 @@ class PackageSearchProjectService( .flatMapLatest { moduleProvidersList } .retry(5) .onEach { logDebug("${this::class.qualifiedName}#modulesStateFlow") { "modules.size = ${it.size}" } } - .stateIn(coroutineScope, SharingStarted.Eagerly, emptyList()) + .stateIn(coroutineScope, SharingStarted.Lazily, emptyList()) val modulesByBuildFile = modulesStateFlow .map { it.associateBy { it.buildFilePath }.filterNotNullKeys() } - .stateIn(coroutineScope, SharingStarted.Eagerly, emptyMap()) + .stateIn(coroutineScope, SharingStarted.Lazily, emptyMap()) val modulesByIdentity = modulesStateFlow .map { it.associateBy { it.identity } } - .stateIn(coroutineScope, SharingStarted.Eagerly, emptyMap()) + .stateIn(coroutineScope, SharingStarted.Lazily, emptyMap()) + + private val openedBuildFiles = combine( + project.fileOpenedFlow, + modulesByBuildFile.map { it.keys } + ) { openedFiles, buildFiles -> + openedFiles.filter { it.toNioPathOrNull()?.let { it in buildFiles } ?: false } + } + .shareIn(coroutineScope, SharingStarted.Lazily, 0) init { @@ -151,8 +154,18 @@ class PackageSearchProjectService( } .launchIn(coroutineScope) - IntelliJApplication.PackageSearchApplicationCachesService - .isOnlineFlow + combine( + openedBuildFiles.map { it.isEmpty() }, + project.toolWindowOpenedFlow("Package Search") + ) { noOpenedFiles, toolWindowOpened -> noOpenedFiles || !toolWindowOpened } + .flatMapLatest { + // if the tool window is not opened and there are no opened build files, + // we don't need to do anything, and we turn off the isOnlineFlow + when { + it -> IntelliJApplication.PackageSearchApplicationCachesService.isOnlineFlow + else -> emptyFlow() + } + } .filter { it } .onEach { restart() } .retry { @@ -161,12 +174,8 @@ class PackageSearchProjectService( } .launchIn(coroutineScope) - combine( - project.fileOpenedFlow, - modulesByBuildFile.map { it.keys } - ) { openedFiles, buildFiles -> - openedFiles.filter { it.toNioPathOrNull()?.let { it in buildFiles } ?: false } - } + + openedBuildFiles .filter { it.isNotEmpty() } .replayOn(stableOnlyStateFlow) .flatMapMerge { it.asFlow() } diff --git a/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/PackageListViewModel.kt b/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/PackageListViewModel.kt index ebb991b1..6591e3ab 100644 --- a/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/PackageListViewModel.kt +++ b/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/ui/model/packageslist/PackageListViewModel.kt @@ -90,7 +90,7 @@ class PackageListViewModel( combine(listOf(selectedModuleIdsSharedFlow.map { it.size == 1 }, isOnline)) { it.all { it } } - .stateIn(viewModelScope, SharingStarted.Eagerly, true) + .stateIn(viewModelScope, SharingStarted.WhileSubscribed(), true) private val selectedModulesFlow = combine( selectedModuleIdsSharedFlow, @@ -213,7 +213,7 @@ class PackageListViewModel( } } .retry() - .stateIn(viewModelScope, SharingStarted.Eagerly, emptyList()) + .stateIn(viewModelScope, SharingStarted.WhileSubscribed(), emptyList()) private suspend fun PackageSearchModule.Base.getSearchQuery( searchQuery: String, diff --git a/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/utils/PackageSearchApiPackageCache.kt b/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/utils/PackageSearchApiPackageCache.kt index 6ded0313..011ba69d 100644 --- a/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/utils/PackageSearchApiPackageCache.kt +++ b/plugin/src/main/kotlin/com/jetbrains/packagesearch/plugin/utils/PackageSearchApiPackageCache.kt @@ -68,15 +68,21 @@ class PackageSearchApiPackageCache( override suspend fun getKnownRepositories(): List { val cached = repositoryCache.find().singleOrNull() - if (cached != null && (Clock.System.now() < cached.lastUpdate + maxAge || !isOnline())) { + val isOnlineStatus = isOnline() + if (cached != null && (Clock.System.now() < cached.lastUpdate + maxAge || !isOnlineStatus)) { return cached.data } - return if (isOnline()) apiClient.getKnownRepositories() - .also { - repositoryCache.removeAll() - repositoryCache.insert(ApiRepositoryCacheEntry(it)) - } - else emptyList() + return when { + isOnlineStatus -> runCatching { apiClient.getKnownRepositories() } + .suspendSafe() + .onSuccess { + repositoryCache.removeAll() + repositoryCache.insert(ApiRepositoryCacheEntry(it)) + } + .getOrDefault(cached?.data ?: emptyList()) + + else -> emptyList() + } } private suspend fun getPackages( @@ -127,26 +133,12 @@ class PackageSearchApiPackageCache( .suspendSafe() .onFailure { logDebug("${this::class.qualifiedName}#getPackages", it) } if (networkResults.isSuccess) { - val packageEntries = networkResults.getOrThrow() + val cacheEntriesFromNetwork = networkResults.getOrThrow() .values .map { it.asCacheEntry() } - if (packageEntries.isNotEmpty()) { - logDebug(contextName) { "No packages found | missingIds.size = ${missingIds.size}" } - - // remove the old entries - apiPackageCache.remove( - filter = NitriteFilters.Object.`in`( - path = packageIdSelector, - value = packageEntries.mapNotNull { it.packageId } - ) - ) - logDebug(contextName) { - "Removing old entries | packageEntries.size = ${packageEntries.size}" - } - } // evaluate packages that are missing from our backend val retrievedPackageIds = - packageEntries.mapNotNull { if (useHashes) it.packageIdHash else it.packageId } + cacheEntriesFromNetwork.mapNotNull { if (useHashes) it.packageIdHash else it.packageId } .toSet() val unknownPackages = missingIds.minus(retrievedPackageIds) .map { id -> @@ -159,8 +151,22 @@ class PackageSearchApiPackageCache( "New unknown packages | unknownPackages.size = ${unknownPackages.size}" } // insert the new entries - val toInsert = packageEntries + unknownPackages - if (toInsert.isNotEmpty()) apiPackageCache.insert(toInsert) + val toInsert = cacheEntriesFromNetwork + unknownPackages + if (toInsert.isNotEmpty()) { + toInsert.forEach { insert -> + apiPackageCache.update( + filter = NitriteFilters.Object.eq( + path = packageIdSelector, + value = when { + useHashes -> insert.packageIdHash + else -> insert.packageId + } + ), + update = insert, + upsert = true + ) + } + } } val networkResultsData = networkResults.getOrDefault(emptyMap()) logDebug(contextName) {