From 1ee7f6b9226e26f7f582fa410affd9e3439fdc0c Mon Sep 17 00:00:00 2001 From: Bastian Doetsch Date: Tue, 11 Jun 2024 17:51:04 +0200 Subject: [PATCH] fix: make plugin compatible to 2024.2, reduce time in UI Thread [IDE-395] (#542) * fix: enable 2024.2, suppress some incorrect/unneeded warnings * fix: improve UI thread handling, update CHANGELOG.md * fix: improve shutdown handling and debouncing of file changes * fix: improve startup & progress handling --- CHANGELOG.md | 2 + gradle.properties | 4 +- .../snyk/plugin/SnykProjectManagerListener.kt | 6 +- .../snykcode/SnykCodeBulkFileListener.kt | 77 +++++++----- .../snyk/common/lsp/LanguageServerWrapper.kt | 19 +-- .../snyk/common/lsp/SnykLanguageClient.kt | 111 +++++++++--------- src/main/kotlin/snyk/common/lsp/Types.kt | 2 +- 7 files changed, 119 insertions(+), 102 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2018cc714..fa75a0253 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ ### Added - Add folder path to report analytics request. +- Support for 2024.4 +- Spend less time in the UI thread ## [2.8.2] diff --git a/gradle.properties b/gradle.properties index 8901ab96c..32f3113db 100644 --- a/gradle.properties +++ b/gradle.properties @@ -4,7 +4,7 @@ pluginName=Snyk Security # for insight into build numbers and IntelliJ Platform versions # see https://plugins.jetbrains.com/docs/intellij/build-number-ranges.html pluginSinceBuild=233 -pluginUntilBuild=241.* +pluginUntilBuild=242.* platformVersion=2023.3 platformDownloadSources=true @@ -13,7 +13,7 @@ platformDownloadSources=true # see https://plugins.jetbrains.com/docs/intellij/plugin-dependencies.html platformPlugins=org.intellij.plugins.hcl:233.11799.172,org.jetbrains.plugins.yaml,org.jetbrains.kotlin,com.intellij.java,org.intellij.groovy # list of versions for which to check the plugin for api compatibility -pluginVerifierIdeVersions=2023.3,2024.1 +pluginVerifierIdeVersions=2023.3,2024.1,2024.2 localIdeDirectory= # opt-out flag for bundling Kotlin standard library # see https://plugins.jetbrains.com/docs/intellij/kotlin.html#kotlin-standard-library diff --git a/src/main/kotlin/io/snyk/plugin/SnykProjectManagerListener.kt b/src/main/kotlin/io/snyk/plugin/SnykProjectManagerListener.kt index 676bce0dc..cc80eed4a 100644 --- a/src/main/kotlin/io/snyk/plugin/SnykProjectManagerListener.kt +++ b/src/main/kotlin/io/snyk/plugin/SnykProjectManagerListener.kt @@ -10,7 +10,7 @@ import snyk.common.lsp.LanguageServerWrapper import java.util.concurrent.Executors import java.util.concurrent.TimeUnit -private const val TIMEOUT = 5L +private const val TIMEOUT = 1L class SnykProjectManagerListener : ProjectManagerListener { override fun projectClosing(project: Project) { @@ -18,10 +18,10 @@ class SnykProjectManagerListener : ProjectManagerListener { override fun run(indicator: ProgressIndicator) { // limit clean up to 5s try { - Executors.newSingleThreadExecutor().submit { + Executors.newCachedThreadPool().submit { // lets all running ProgressIndicators release MUTEX first val ls = LanguageServerWrapper.getInstance() - if (ls.ensureLanguageServerInitialized()) { + if (ls.isInitialized) { ls.updateWorkspaceFolders(emptySet(), ls.getWorkspaceFolders(project)) } }.get(TIMEOUT, TimeUnit.SECONDS) diff --git a/src/main/kotlin/io/snyk/plugin/snykcode/SnykCodeBulkFileListener.kt b/src/main/kotlin/io/snyk/plugin/snykcode/SnykCodeBulkFileListener.kt index 522af42bb..e8965902b 100644 --- a/src/main/kotlin/io/snyk/plugin/snykcode/SnykCodeBulkFileListener.kt +++ b/src/main/kotlin/io/snyk/plugin/snykcode/SnykCodeBulkFileListener.kt @@ -1,7 +1,9 @@ package io.snyk.plugin.snykcode +import com.google.common.cache.CacheBuilder import com.intellij.codeInsight.daemon.DaemonCodeAnalyzer -import com.intellij.ide.impl.ProjectUtil +import com.intellij.openapi.application.runInEdt +import com.intellij.openapi.diagnostic.logger import com.intellij.openapi.progress.ProgressIndicator import com.intellij.openapi.progress.ProgressManager import com.intellij.openapi.progress.Task.Backgroundable @@ -11,51 +13,66 @@ import com.intellij.openapi.vfs.VirtualFileManager import com.intellij.openapi.vfs.newvfs.events.VFileEvent import com.intellij.openapi.vfs.readText import io.snyk.plugin.SnykBulkFileListener -import io.snyk.plugin.SnykFile import io.snyk.plugin.getSnykCachedResults import io.snyk.plugin.toLanguageServerURL import io.snyk.plugin.toSnykFileSet import org.eclipse.lsp4j.DidSaveTextDocumentParams import org.eclipse.lsp4j.TextDocumentIdentifier import snyk.common.lsp.LanguageServerWrapper +import java.time.Duration class SnykCodeBulkFileListener : SnykBulkFileListener() { + // Cache for debouncing file updates that come in within one second of the last + // Key = path, Value is irrelevant + private val debounceFileCache = + CacheBuilder.newBuilder() + .expireAfterWrite(Duration.ofMillis(1000)).build() + override fun before(project: Project, virtualFilesAffected: Set) = Unit override fun after(project: Project, virtualFilesAffected: Set) { - val filesAffected = toSnykFileSet(project, virtualFilesAffected) - updateCacheAndUI(filesAffected, project) - } - - override fun forwardEvents(events: MutableList) { - val languageServerWrapper = LanguageServerWrapper.getInstance() - - if (!languageServerWrapper.isInitialized) return - - val languageServer = languageServerWrapper.languageServer - for (event in events) { - if (event.file == null || !event.isFromSave) continue - val file = event.file!! - val activeProject = ProjectUtil.getActiveProject() - ProgressManager.getInstance().run(object : Backgroundable( - activeProject, - "Snyk: forwarding save event to Language Server" - ) { - override fun run(indicator: ProgressIndicator) { + if (virtualFilesAffected.isEmpty()) return + ProgressManager.getInstance().run(object : Backgroundable( + project, + "Snyk: forwarding save event to Language Server" + ) { + override fun run(indicator: ProgressIndicator) { + val languageServerWrapper = LanguageServerWrapper.getInstance() + if (!languageServerWrapper.isInitialized) return + val languageServer = languageServerWrapper.languageServer + val cache = getSnykCachedResults(project)?.currentSnykCodeResultsLS ?: return + val filesAffected = toSnykFileSet(project, virtualFilesAffected) + for (file in filesAffected) { + val virtualFile = file.virtualFile + if (!shouldProcess(virtualFile)) continue + cache.remove(file) val param = - DidSaveTextDocumentParams(TextDocumentIdentifier(file.toLanguageServerURL()), file.readText()) + DidSaveTextDocumentParams( + TextDocumentIdentifier(virtualFile.toLanguageServerURL()), + virtualFile.readText() + ) languageServer.textDocumentService.didSave(param) } - }) - } + + VirtualFileManager.getInstance().asyncRefresh() + runInEdt { DaemonCodeAnalyzer.getInstance(project).restart() } + } + }) + } - private fun updateCacheAndUI(filesAffected: Set, project: Project) { - val cache = getSnykCachedResults(project)?.currentSnykCodeResultsLS ?: return - filesAffected.forEach { - cache.remove(it) + private fun shouldProcess(file: VirtualFile): Boolean { + val inCache = debounceFileCache.getIfPresent(file.path) + + return if (inCache != null) { + logger().info("not forwarding file event to ls, debouncing") + false + } else { + debounceFileCache.put(file.path, true) + logger().info("forwarding file event to ls, not debouncing") + true } - VirtualFileManager.getInstance().asyncRefresh() - DaemonCodeAnalyzer.getInstance(project).restart() } + + override fun forwardEvents(events: MutableList) = Unit } diff --git a/src/main/kotlin/snyk/common/lsp/LanguageServerWrapper.kt b/src/main/kotlin/snyk/common/lsp/LanguageServerWrapper.kt index fe4e5186a..3bea3927f 100644 --- a/src/main/kotlin/snyk/common/lsp/LanguageServerWrapper.kt +++ b/src/main/kotlin/snyk/common/lsp/LanguageServerWrapper.kt @@ -3,6 +3,7 @@ package snyk.common.lsp import com.google.common.util.concurrent.CycleDetectingLockFactory import com.intellij.openapi.components.service import com.intellij.openapi.diagnostic.Logger +import com.intellij.openapi.project.DumbService import com.intellij.openapi.project.Project import com.intellij.openapi.project.ProjectManager import com.intellij.openapi.util.io.toNioPathOrNull @@ -263,8 +264,10 @@ class LanguageServerWrapper( fun sendScanCommand(project: Project) { if (!ensureLanguageServerInitialized()) return - getTrustedContentRoots(project).forEach { - sendFolderScanCommand(it.path) + DumbService.getInstance(project).runWhenSmart { + getTrustedContentRoots(project).forEach { + sendFolderScanCommand(it.path) + } } } @@ -325,12 +328,12 @@ class LanguageServerWrapper( cliPath = getCliFile().absolutePath, token = ps.token, filterSeverity = - SeverityFilter( - critical = ps.criticalSeverityEnabled, - high = ps.highSeverityEnabled, - medium = ps.mediumSeverityEnabled, - low = ps.lowSeverityEnabled, - ), + SeverityFilter( + critical = ps.criticalSeverityEnabled, + high = ps.highSeverityEnabled, + medium = ps.mediumSeverityEnabled, + low = ps.lowSeverityEnabled, + ), enableTrustedFoldersFeature = "false", scanningMode = if (!ps.scanOnSave) "manual" else "auto", integrationName = pluginInfo.integrationName, diff --git a/src/main/kotlin/snyk/common/lsp/SnykLanguageClient.kt b/src/main/kotlin/snyk/common/lsp/SnykLanguageClient.kt index 2a9cf2335..5d0947aea 100644 --- a/src/main/kotlin/snyk/common/lsp/SnykLanguageClient.kt +++ b/src/main/kotlin/snyk/common/lsp/SnykLanguageClient.kt @@ -9,7 +9,6 @@ import com.intellij.openapi.actionSystem.AnActionEvent import com.intellij.openapi.application.ReadAction import com.intellij.openapi.command.WriteCommandAction import com.intellij.openapi.components.service -import com.intellij.openapi.diagnostic.LogLevel import com.intellij.openapi.diagnostic.Logger import com.intellij.openapi.progress.ProgressIndicator import com.intellij.openapi.progress.ProgressManager @@ -47,6 +46,7 @@ import org.eclipse.lsp4j.WorkDoneProgressNotification import org.eclipse.lsp4j.WorkDoneProgressReport import org.eclipse.lsp4j.jsonrpc.services.JsonNotification import org.eclipse.lsp4j.services.LanguageClient +import org.jetbrains.concurrency.runAsync import org.jetbrains.kotlin.idea.util.application.executeOnPooledThread import snyk.common.ProductType import snyk.common.SnykFileIssueComparator @@ -54,11 +54,8 @@ import snyk.trust.WorkspaceTrustService import java.util.concurrent.ArrayBlockingQueue import java.util.concurrent.CompletableFuture import java.util.concurrent.TimeUnit -import java.util.concurrent.locks.ReentrantLock class SnykLanguageClient() : LanguageClient { - private val progressLock: ReentrantLock = ReentrantLock() - // TODO FIX Log Level val logger = Logger.getInstance("Snyk Language Server") @@ -199,7 +196,7 @@ class SnykLanguageClient() : LanguageClient { @Suppress("UselessCallOnNotNull") // because lsp4j doesn't care about Kotlin non-null safety private fun getSnykResult(project: Project, snykScan: SnykScanParams): Map> { - check(snykScan.product == "code" || snykScan.product == "oss" ) { "Expected Snyk Code or Snyk OSS scan result" } + check(snykScan.product == "code" || snykScan.product == "oss") { "Expected Snyk Code or Snyk OSS scan result" } if (snykScan.issues.isNullOrEmpty()) return emptyMap() val pluginSettings = pluginSettings() @@ -251,77 +248,75 @@ class SnykLanguageClient() : LanguageClient { } private fun createProgressInternal(token: String, begin: WorkDoneProgressBegin) { - ProgressManager.getInstance().run(object : Task.Backgroundable(ProjectUtil.getActiveProject(), "Snyk: ${begin.title}", true) { - override fun run(indicator: ProgressIndicator) { - logger.debug("###### Creating progress indicator for: $token, title: ${begin.title}, message: ${begin.message}") - indicator.isIndeterminate = false - indicator.text = begin.title - indicator.text2 = begin.message - indicator.fraction = 0.1 - progresses.put(token, indicator) - while (!indicator.isCanceled) { - Thread.sleep(1000) + ProgressManager.getInstance() + .run(object : Task.Backgroundable(ProjectUtil.getActiveProject(), "Snyk: ${begin.title}", true) { + override fun run(indicator: ProgressIndicator) { + logger.debug("Creating progress indicator for: $token, title: ${begin.title}, message: ${begin.message}") + indicator.isIndeterminate = false + indicator.text = begin.title + indicator.text2 = begin.message + indicator.fraction = 0.1 + progresses.put(token, indicator) + while (!indicator.isCanceled) { + Thread.sleep(1000) + } + logger.debug("Progress indicator canceled for token: $token") } - logger.debug("###### Progress indicator canceled for token: $token") - } - }) + }) } override fun notifyProgress(params: ProgressParams) { // first: check if progress has begun - val token = params.token?.left ?: return - if (progresses.getIfPresent(token) != null) { - processProgress(params) - } else { - when (val progressNotification = params.value.left) { - is WorkDoneProgressEnd -> { - progressEndMsgCache.put(token, progressNotification) - } + runAsync { + val token = params.token?.left ?: return@runAsync + if (progresses.getIfPresent(token) != null) { + processProgress(params) + } else { + when (val progressNotification = params.value.left) { + is WorkDoneProgressEnd -> { + progressEndMsgCache.put(token, progressNotification) + } - is WorkDoneProgressReport -> { - val list = progressReportMsgCache.get(token) { mutableListOf() } - list.add(progressNotification) - } + is WorkDoneProgressReport -> { + val list = progressReportMsgCache.get(token) { mutableListOf() } + list.add(progressNotification) + } - else -> { - processProgress(params) + else -> { + processProgress(params) + } } + return@runAsync } - return } } private fun processProgress(params: ProgressParams?) { - progressLock.lock() - try { - val token = params?.token?.left ?: return - val workDoneProgressNotification = params.value.left ?: return - when (workDoneProgressNotification.kind) { - begin -> { - val begin: WorkDoneProgressBegin = workDoneProgressNotification as WorkDoneProgressBegin - createProgressInternal(token, begin) - // wait until the progress indicator is created in the background thread - while (progresses.getIfPresent(token) == null) { - Thread.sleep(100) - } - - // process previously reported progress and end messages for token - processCachedProgressReports(token) - processCachedEndReport(token) + val token = params?.token?.left ?: return + val workDoneProgressNotification = params.value.left ?: return + when (workDoneProgressNotification.kind) { + begin -> { + val begin: WorkDoneProgressBegin = workDoneProgressNotification as WorkDoneProgressBegin + createProgressInternal(token, begin) + // wait until the progress indicator is created in the background thread + while (progresses.getIfPresent(token) == null) { + Thread.sleep(100) } - report -> { - progressReport(token, workDoneProgressNotification) - } + // process previously reported progress and end messages for token + processCachedProgressReports(token) + processCachedEndReport(token) + } - end -> { - progressEnd(token, workDoneProgressNotification) - } + report -> { + progressReport(token, workDoneProgressNotification) + } - null -> {} + end -> { + progressEnd(token, workDoneProgressNotification) } - } finally { - progressLock.unlock() + + null -> {} } } diff --git a/src/main/kotlin/snyk/common/lsp/Types.kt b/src/main/kotlin/snyk/common/lsp/Types.kt index 03278293c..84284c9d9 100644 --- a/src/main/kotlin/snyk/common/lsp/Types.kt +++ b/src/main/kotlin/snyk/common/lsp/Types.kt @@ -1,4 +1,4 @@ -@file:Suppress("unused", "SENSELESS_COMPARISON") +@file:Suppress("unused", "SENSELESS_COMPARISON", "UNNECESSARY_SAFE_CALL", "USELESS_ELVIS", "DuplicatedCode") package snyk.common.lsp