Skip to content

Commit

Permalink
chore: freeze & shutdown improvements (#575)
Browse files Browse the repository at this point in the history
* fix: progress scanning

* fix: ui freezes & display

* fix: wait for ls initialization to retrieve active user

add ff convenience method to keep things dry

* fix: update feature flags when retrieving new token

* fix: update jackson because of vulnerability

* fix: better disposing/shutdown handling

* fix: better shutdown

* fix: better error logging (ignore stream closed exception)

* docs: update CHANGELOG.md

* chore: make annotators disposable

* chore: remove login method (for now)

* fix: init message

* fix: ignore exceptions on shutdown

* fix: move navigation to background thread

* fix: use readactions and async where necessary to de-freeze UI

* fix: warnings
  • Loading branch information
bastiandoetsch authored Aug 5, 2024
1 parent 576936c commit cf827a6
Show file tree
Hide file tree
Showing 24 changed files with 300 additions and 148 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Snyk Security Changelog

## [2.8.11]
### Added
- Improved UI thread usage and app shutdown handling

## [2.8.9]
### Added
- Updated Open Source, Containers and IaC products to include `.snyk` in the list of supported build files.
Expand Down
2 changes: 1 addition & 1 deletion build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ dependencies {
implementation("com.squareup.retrofit2:retrofit")
implementation("com.squareup.okhttp3:okhttp")
implementation("com.squareup.okhttp3:logging-interceptor")
implementation("com.fasterxml.jackson.core:jackson-databind:2.12.7.1")
implementation("com.fasterxml.jackson.core:jackson-databind:2.15.0")
implementation("org.json:json:20231013")
implementation("org.slf4j:slf4j-api:2.0.5")
implementation("ly.iterative.itly:plugin-iteratively:1.2.11") {
Expand Down
59 changes: 36 additions & 23 deletions src/main/kotlin/io/snyk/plugin/Utils.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import com.intellij.openapi.Disposable
import com.intellij.openapi.application.ApplicationManager
import com.intellij.openapi.application.PathManager
import com.intellij.openapi.application.ReadAction
import com.intellij.openapi.application.invokeLater
import com.intellij.openapi.application.runReadAction
import com.intellij.openapi.components.service
import com.intellij.openapi.diagnostic.Logger
Expand Down Expand Up @@ -44,6 +45,7 @@ import io.snyk.plugin.ui.SnykBalloonNotificationHelper
import io.snyk.plugin.ui.toolwindow.SnykToolWindowFactory
import io.snyk.plugin.ui.toolwindow.SnykToolWindowPanel
import org.apache.commons.lang3.SystemUtils
import org.jetbrains.concurrency.runAsync
import snyk.advisor.AdvisorService
import snyk.advisor.AdvisorServiceImpl
import snyk.advisor.SnykAdvisorModel
Expand Down Expand Up @@ -197,7 +199,7 @@ fun isOssRunning(project: Project): Boolean {
(indicator != null && indicator.isRunning && !indicator.isCanceled)
}

fun cancelOss(project: Project) {
fun cancelOssIndicator(project: Project) {
val indicator = getSnykTaskQueueService(project)?.ossScanProgressIndicator
indicator?.cancel()
}
Expand Down Expand Up @@ -234,6 +236,7 @@ fun startSastEnablementCheckLoop(parentDisposable: Disposable, onSuccess: () ->
var currentAttempt = 1
val maxAttempts = 20
lateinit var checkIfSastEnabled: () -> Unit
// TODO use ls
checkIfSastEnabled = {
if (settings.sastOnServerEnabled != true) {
settings.sastOnServerEnabled = try {
Expand Down Expand Up @@ -344,30 +347,40 @@ fun navigateToSource(
project: Project,
virtualFile: VirtualFile,
selectionStartOffset: Int,
selectionEndOffset: Int? = null
selectionEndOffset: Int? = null,
) {
if (!virtualFile.isValid) return
val textLength = virtualFile.contentsToByteArray().size
if (selectionStartOffset in (0 until textLength)) {
// jump to Source
PsiNavigationSupport.getInstance().createNavigatable(
project,
virtualFile,
selectionStartOffset
).navigate(false)
} else {
logger.warn("Navigation to wrong offset: $selectionStartOffset with file length=$textLength")
}

if (selectionEndOffset != null) {
// highlight(by selection) suggestion range in source file
if (selectionEndOffset in (0 until textLength) &&
selectionStartOffset < selectionEndOffset
) {
val editor = FileEditorManager.getInstance(project).selectedTextEditor
editor?.selectionModel?.setSelection(selectionStartOffset, selectionEndOffset)
runAsync {
if (!virtualFile.isValid) return@runAsync
val textLength = virtualFile.contentsToByteArray().size
if (selectionStartOffset in (0 until textLength)) {
// jump to Source
val navigatable =
PsiNavigationSupport.getInstance().createNavigatable(
project,
virtualFile,
selectionStartOffset,
)
invokeLater {
if (navigatable.canNavigateToSource()) {
navigatable.navigate(false)
}
}
} else {
logger.warn("Selection of wrong range: [$selectionStartOffset:$selectionEndOffset]")
logger.warn("Navigation to wrong offset: $selectionStartOffset with file length=$textLength")
}

if (selectionEndOffset != null) {
// highlight(by selection) suggestion range in source file
if (selectionEndOffset in (0 until textLength) &&
selectionStartOffset < selectionEndOffset
) {
invokeLater {
val editor = FileEditorManager.getInstance(project).selectedTextEditor
editor?.selectionModel?.setSelection(selectionStartOffset, selectionEndOffset)
}
} else {
logger.warn("Selection of wrong range: [$selectionStartOffset:$selectionEndOffset]")
}
}
}
}
Expand Down
12 changes: 5 additions & 7 deletions src/main/kotlin/io/snyk/plugin/net/TokenInterceptor.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@ package io.snyk.plugin.net

import com.google.gson.Gson
import com.intellij.openapi.project.ProjectManager
import io.snyk.plugin.getSnykCliAuthenticationService
import io.snyk.plugin.getUserAgentString
import io.snyk.plugin.getWhoamiService
import io.snyk.plugin.pluginSettings
import okhttp3.Interceptor
import okhttp3.Response
import snyk.common.lsp.LanguageServerWrapper
import snyk.common.needsSnykToken
import snyk.pluginInfo
import java.time.OffsetDateTime
Expand All @@ -31,12 +30,11 @@ class TokenInterceptor(private var projectManager: ProjectManager? = null) : Int
if (projectManager == null) {
projectManager = ProjectManager.getInstance()
}
val project = projectManager?.openProjects!!.firstOrNull()
val oAuthToken = Gson().fromJson(token, OAuthToken::class.java)
val expiry = OffsetDateTime.parse(oAuthToken.expiry)
// when the token is about to expire, call the whoami workflow to refresh it
val oAuthToken = Gson().fromJson(token, OAuthToken::class.java) ?: return chain.proceed(request.build())
val expiry = OffsetDateTime.parse(oAuthToken.expiry!!)
if (expiry.isBefore(OffsetDateTime.now().plusMinutes(2))) {
getWhoamiService(project)?.execute()
getSnykCliAuthenticationService(project)?.executeGetConfigApiCommand()
LanguageServerWrapper.getInstance().getAuthenticatedUser()
}
request.addHeader(authorizationHeaderName, "Bearer ${oAuthToken.access_token}")
request.addHeader(oldSnykCodeHeaderName, "Bearer ${oAuthToken.access_token}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import com.intellij.openapi.progress.BackgroundTaskQueue
import com.intellij.openapi.progress.ProgressIndicator
import com.intellij.openapi.progress.Task
import com.intellij.openapi.project.Project
import io.snyk.plugin.cancelOss
import io.snyk.plugin.cancelOssIndicator
import io.snyk.plugin.events.SnykCliDownloadListener
import io.snyk.plugin.events.SnykScanListener
import io.snyk.plugin.events.SnykSettingsListener
Expand Down Expand Up @@ -295,7 +295,7 @@ class SnykTaskQueueService(val project: Project) {

fun stopScan() {
val wasOssRunning = isOssRunning(project)
cancelOss(project)
cancelOssIndicator(project)

val wasSnykCodeRunning = isSnykCodeRunning(project)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,11 @@ class SnykProjectSettingsConfigurable(val project: Project) : SearchableConfigur

runBackgroundableTask("Updating Snyk Code settings", project, true) {
settingsStateService.isGlobalIgnoresFeatureEnabled =
LanguageServerWrapper.getInstance().getFeatureFlagStatus("snykCodeConsistentIgnores")
}
LanguageServerWrapper.getInstance().isGlobalIgnoresFeatureEnabled()

if (snykSettingsDialog.getCliReleaseChannel().trim() != pluginSettings().cliReleaseChannel) {
handleReleaseChannelChanged()
if (snykSettingsDialog.getCliReleaseChannel().trim() != pluginSettings().cliReleaseChannel) {
handleReleaseChannelChanged()
}
}

if (rescanNeeded) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package io.snyk.plugin.ui.jcef

import com.intellij.openapi.application.ApplicationManager
import com.intellij.openapi.editor.colors.EditorColorsManager
import com.intellij.openapi.project.Project
import com.intellij.openapi.vfs.VirtualFile
Expand All @@ -26,25 +25,18 @@ class OpenFileLoadHandlerGenerator(

val virtualFile = virtualFiles[filePath] ?: return JBCefJSQuery.Response("success")

ApplicationManager.getApplication().invokeLater {
val document = virtualFile.getDocument()
val startLineStartOffset = document?.getLineStartOffset(startLine) ?: 0
val startOffset = startLineStartOffset + (startCharacter)
val endLineStartOffset = document?.getLineStartOffset(endLine) ?: 0
val endOffset = endLineStartOffset + endCharacter - 1

navigateToSource(project, virtualFile, startOffset, endOffset)
}
val document = virtualFile.getDocument()
val startLineStartOffset = document?.getLineStartOffset(startLine) ?: 0
val startOffset = startLineStartOffset + (startCharacter)
val endLineStartOffset = document?.getLineStartOffset(endLine) ?: 0
val endOffset = endLineStartOffset + endCharacter - 1

navigateToSource(project, virtualFile, startOffset, endOffset)
return JBCefJSQuery.Response("success")
}

fun generate(jbCefBrowser: JBCefBrowserBase): CefLoadHandlerAdapter {
val openFileQuery = JBCefJSQuery.create(jbCefBrowser)
val isDarkTheme = EditorColorsManager.getInstance().isDarkEditor
val isHighContrast =
EditorColorsManager.getInstance().globalScheme.name.contains("High contrast", ignoreCase = true)

openFileQuery.addHandler { openFile(it) }

return object : CefLoadHandlerAdapter() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
package io.snyk.plugin.ui.toolwindow

import com.intellij.ide.AppLifecycleListener
import com.intellij.openapi.Disposable
import com.intellij.openapi.application.ApplicationManager
import com.intellij.openapi.components.Service
import com.intellij.openapi.project.Project
import org.jetbrains.annotations.NotNull
import snyk.common.lsp.LanguageServerWrapper
import java.util.concurrent.TimeUnit


/**
* Top-Level disposable for the Snyk plugin.
*/
@Service(Service.Level.APP, Service.Level.PROJECT)
class SnykPluginDisposable : Disposable {
class SnykPluginDisposable : Disposable, AppLifecycleListener {
companion object {
@NotNull
fun getInstance(): Disposable {
Expand All @@ -24,5 +27,26 @@ class SnykPluginDisposable : Disposable {
}
}

init {
ApplicationManager.getApplication().messageBus.connect().subscribe(AppLifecycleListener.TOPIC, this)
}

override fun appClosing() {
try {
LanguageServerWrapper.getInstance().shutdown().get(2, TimeUnit.SECONDS)
} catch (ignored: Exception) {
// do nothing
}
}

override fun appWillBeClosed(isRestart: Boolean) {
try {
LanguageServerWrapper.getInstance().shutdown().get(2, TimeUnit.SECONDS)
} catch (ignored: Exception) {
// do nothing
}
}

override fun dispose() = Unit

}
46 changes: 29 additions & 17 deletions src/main/kotlin/io/snyk/plugin/ui/toolwindow/SnykToolWindowPanel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package io.snyk.plugin.ui.toolwindow
import com.intellij.notification.NotificationAction
import com.intellij.openapi.Disposable
import com.intellij.openapi.application.ApplicationManager
import com.intellij.openapi.application.ReadAction
import com.intellij.openapi.components.Service
import com.intellij.openapi.diagnostic.Logger
import com.intellij.openapi.project.Project
Expand Down Expand Up @@ -71,6 +72,7 @@ import io.snyk.plugin.ui.toolwindow.panels.StatePanel
import io.snyk.plugin.ui.toolwindow.panels.TreePanel
import io.snyk.plugin.ui.wrapWithScrollPane
import org.jetbrains.annotations.TestOnly
import org.jetbrains.concurrency.runAsync
import snyk.analytics.AnalysisIsTriggered
import snyk.analytics.WelcomeIsViewed
import snyk.analytics.WelcomeIsViewed.Ide.JETBRAINS
Expand Down Expand Up @@ -578,8 +580,9 @@ class SnykToolWindowPanel(
) {
val settings = pluginSettings()

val realError = getSnykCachedResults(project)?.currentOssError != null
&& ossResultsCount != NODE_NOT_SUPPORTED_STATE
val realError =
getSnykCachedResults(project)?.currentOssError != null &&
ossResultsCount != NODE_NOT_SUPPORTED_STATE

val newOssTreeNodeText =
when {
Expand All @@ -594,7 +597,7 @@ class SnykToolWindowPanel(
count == 0 -> NO_ISSUES_FOUND_TEXT
count > 0 -> ProductType.OSS.getCountText(count, isUniqueCount = true) + addHMLPostfix
count == NODE_NOT_SUPPORTED_STATE -> NO_SUPPORTED_PACKAGE_MANAGER_FOUND
else -> throw IllegalStateException("ResultsCount is meaningful")
else -> throw IllegalStateException("ResultsCount is not meaningful")
}
}
}
Expand All @@ -603,9 +606,7 @@ class SnykToolWindowPanel(
val newSecurityIssuesNodeText =
when {
getSnykCachedResults(project)?.currentSnykCodeError != null -> "$CODE_SECURITY_ROOT_TEXT (error)"
isSnykCodeRunning(
project,
) &&
isSnykCodeRunning(project) &&
settings.snykCodeSecurityIssuesScanEnable -> "$CODE_SECURITY_ROOT_TEXT (scanning...)"

else ->
Expand Down Expand Up @@ -747,19 +748,30 @@ class SnykToolWindowPanel(
vulnerability: Vulnerability?,
): () -> Unit =
{
val virtualFile = VirtualFileManager.getInstance().findFileByNioPath(Paths.get(filePath))
if (virtualFile != null && virtualFile.isValid) {
runAsync {
var virtualFile: VirtualFile? = null
ReadAction.run<RuntimeException> {
virtualFile = VirtualFileManager.getInstance().findFileByNioPath(Paths.get(filePath))
}
val vf = virtualFile
if (vf == null || !vf.isValid) {
return@runAsync
}

if (vulnerability == null) {
navigateToSource(project, virtualFile, 0)
navigateToSource(project, vf, 0)
} else {
val psiFile = PsiManager.getInstance(project).findFile(virtualFile)
val textRange = psiFile?.let { getOssTextRangeFinderService().findTextRange(it, vulnerability) }
navigateToSource(
project = project,
virtualFile = virtualFile,
selectionStartOffset = textRange?.startOffset ?: 0,
selectionEndOffset = textRange?.endOffset,
)
ReadAction.run<RuntimeException> {
val psiFile = PsiManager.getInstance(project).findFile(vf)
val textRange =
psiFile?.let { getOssTextRangeFinderService().findTextRange(it, vulnerability) }
navigateToSource(
project = project,
virtualFile = vf,
selectionStartOffset = textRange?.startOffset ?: 0,
selectionEndOffset = textRange?.endOffset,
)
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import com.intellij.openapi.vfs.VirtualFile
import com.intellij.util.ui.tree.TreeUtil
import io.snyk.plugin.Severity
import io.snyk.plugin.SnykFile
import io.snyk.plugin.cancelOssIndicator
import io.snyk.plugin.events.SnykScanListenerLS
import io.snyk.plugin.getSnykCachedResults
import io.snyk.plugin.pluginSettings
Expand All @@ -25,9 +26,8 @@ import io.snyk.plugin.ui.toolwindow.nodes.root.RootOssTreeNode
import io.snyk.plugin.ui.toolwindow.nodes.root.RootQualityIssuesTreeNode
import io.snyk.plugin.ui.toolwindow.nodes.root.RootSecurityIssuesTreeNode
import io.snyk.plugin.ui.toolwindow.nodes.secondlevel.InfoTreeNode
import io.snyk.plugin.ui.toolwindow.nodes.secondlevel.SnykCodeFileTreeNode
import io.snyk.plugin.ui.toolwindow.nodes.secondlevel.SnykFileTreeNode
import snyk.common.ProductType
import snyk.common.lsp.CliError
import snyk.common.lsp.ScanIssue
import snyk.common.lsp.SnykScanParams
import javax.swing.JTree
Expand Down Expand Up @@ -78,6 +78,7 @@ class SnykToolWindowSnykScanListenerLS(
override fun scanningOssFinished(snykResults: Map<SnykFile, List<ScanIssue>>) {
if (disposed) return
ApplicationManager.getApplication().invokeLater {
cancelOssIndicator(project)
this.snykToolWindowPanel.navigateToSourceEnabled = false
displayOssResults(snykResults)
refreshAnnotationsForOpenFiles(project)
Expand Down Expand Up @@ -341,7 +342,7 @@ class SnykToolWindowSnykScanListenerLS(
}

val fileTreeNode =
SnykCodeFileTreeNode(entry, productType)
SnykFileTreeNode(entry, productType)
rootNode.add(fileTreeNode)
entry.value.sortedByDescending { it.priority() }
.forEach { issue ->
Expand Down
Loading

0 comments on commit cf827a6

Please sign in to comment.