2.3.1

- Improve logic related to registries scanning - Add retries for shell commands - Fix status bar sometimes not refreshing past registries gathering (???) - Fix caching for registries - Use getInstance() in actions - Add some JavaDoc - Update dependencies - Other minor code improvements
WarningImHack3r · May 10, 2024 · 73423f9 · 73423f9
1 parent 803384e
commit 73423f9
Show file tree

Hide file tree

Showing 19 changed files with 131 additions and 95 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,18 @@
 
 ## [Unreleased]
 
+### Changed
+
+- Improve logic related to registries scanning, avoiding rare duplicated checks
+- Retry failed shell commands up to 2 times before giving up, improving success rate
+- Other code improvements and optimizations
+
+### Fixed
+
+- Fix status bar sometimes not being updated correctly (#102)
+- Fix high CPU usage when scanning for registries
+- Fix a crash when scanning for deprecations
+
 ## [2.3.0] - 2024-05-03
 
 ### Added

diff --git a/gradle.properties b/gradle.properties
@@ -4,7 +4,7 @@ pluginGroup = com.github.warningimhack3r.npmupdatedependencies
 pluginName = npm-update-dependencies
 pluginRepositoryUrl = https://github.com/WarningImHack3r/npm-update-dependencies
 # SemVer format -> https://semver.org
-pluginVersion = 2.3.0
+pluginVersion = 2.3.1
 
 # Supported build number ranges and IntelliJ Platform versions -> https://plugins.jetbrains.com/docs/intellij/build-number-ranges.html
 pluginSinceBuild = 221

diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
@@ -4,10 +4,10 @@ annotations = "24.1.0"
 semver4j = "5.3.0"
 
 # plugins
-kotlin = "1.9.23"
+kotlin = "1.9.24"
 changelog = "2.2.0"
 gradleIntelliJPlugin = "1.17.3"
-qodana = "2024.1.3"
+qodana = "2024.1.4"
 kover = "0.7.6"
 
 [libraries]

diff --git a/...ain/kotlin/com/github/warningimhack3r/npmupdatedependencies/backend/engine/NPMJSClient.kt b/...ain/kotlin/com/github/warningimhack3r/npmupdatedependencies/backend/engine/NPMJSClient.kt
@@ -27,9 +27,9 @@ class NPMJSClient(private val project: Project) {
 
     private fun getRegistry(packageName: String): String {
         log.info("Getting registry for package $packageName")
-        val registryForPackage = NUDState.getInstance(project).packageRegistries
+        val state = NUDState.getInstance(project)
         val availableRegistries = RegistriesScanner.getInstance(project).registries
-        return registryForPackage[packageName].also {
+        return state.packageRegistries[packageName].also {
             if (it != null) {
                 log.debug("Registry for package $packageName found in cache: $it")
             }
@@ -54,7 +54,7 @@ class NPMJSClient(private val project: Project) {
             }
             val registry = computedRegistry.substringBefore("/$packageName")
             log.info("Computed registry for package $packageName: $registry")
-            registryForPackage[packageName] = registry
+            state.packageRegistries[packageName] = registry
             registry
         } ?: NPMJS_REGISTRY.also {
             log.info("Using default registry for package $packageName")

diff --git a/src/main/kotlin/com/github/warningimhack3r/npmupdatedependencies/backend/engine/NUDState.kt b/src/main/kotlin/com/github/warningimhack3r/npmupdatedependencies/backend/engine/NUDState.kt
@@ -14,8 +14,24 @@ class NUDState {
         fun getInstance(project: Project): NUDState = project.service()
     }
 
+    /**
+     * A "cache" of available updates for packages.
+     * Managed by [PackageUpdateChecker.areUpdatesAvailable].
+     */
     val availableUpdates = mutableMapOf<String, ScanResult>()
+
+    /**
+     * A "cache" of deprecations for packages.
+     * Managed by [com.github.warningimhack3r.npmupdatedependencies.ui.annotation.DeprecationAnnotator.doAnnotate].
+     */
     val deprecations = mutableMapOf<String, Deprecation>()
+
+    /**
+     * A "cache" of registries for packages, mapping package names to registry URLs.
+     * Managed by [NPMJSClient.getRegistry] and only made to be used by it.
+     *
+     * MUST NOT be accessed from outside the [com.github.warningimhack3r.npmupdatedependencies.backend.engine] package.
+     */
     val packageRegistries = mutableMapOf<String, String>()
 
     var totalPackages = 0

diff --git a/...n/com/github/warningimhack3r/npmupdatedependencies/backend/engine/PackageUpdateChecker.kt b/...n/com/github/warningimhack3r/npmupdatedependencies/backend/engine/PackageUpdateChecker.kt
@@ -94,10 +94,7 @@ class PackageUpdateChecker(private val project: Project) {
         var satisfyingVersion: Semver? = null
         val updateAvailable = isVersionMoreRecentThanComparator(newestVersion, comparator)
         if (!updateAvailable) {
-            availableUpdates[packageName]?.let {
-                availableUpdates.remove(packageName)
-            }
-            log.info("No update available for $packageName, removing cached versions")
+            log.info("No update available for $packageName")
             return null
         }
 

diff --git a/...tlin/com/github/warningimhack3r/npmupdatedependencies/backend/engine/RegistriesScanner.kt b/...tlin/com/github/warningimhack3r/npmupdatedependencies/backend/engine/RegistriesScanner.kt
@@ -6,29 +6,35 @@ import com.intellij.openapi.diagnostic.logger
 import com.intellij.openapi.project.Project
 
 @Service(Service.Level.PROJECT)
-class RegistriesScanner(private val project: Project) {
+class RegistriesScanner {
     companion object {
         private val log = logger<RegistriesScanner>()
 
         @JvmStatic
         fun getInstance(project: Project): RegistriesScanner = project.service()
     }
 
+    /**
+     * Whether the registries have been scanned for this project.
+     */
+    var scanned = false
+
+    /**
+     * The list of registries found parsing the npm configuration.
+     */
     var registries: List<String> = emptyList()
 
     fun scan() {
-        val state = NUDState.getInstance(project)
         log.info("Starting to scan registries")
-        state.isScanningForRegistries = true
         // Run `npm config ls` to get the list of registries
         val config = ShellRunner.execute(arrayOf("npm", "config", "ls")) ?: return
-        registries = config.lines().asSequence().filter { line ->
-            line.isNotEmpty() && line.isNotBlank() && !line.startsWith(";")
-        }.map { it.trim() }.filter { line ->
-            line.contains("registry =") || line.contains("registry=")
-                    || line.startsWith("//")
+        registries = config.lines().asSequence().map { it.trim() }.filter { line ->
+            line.isNotEmpty() && line.isNotBlank() && !line.startsWith(";") &&
+                    (line.contains("registry =") || line.contains("registry=")
+                            || line.startsWith("//"))
         }.map { line ->
             if (line.startsWith("//")) {
+                // We assume that registries use TLS in 2024
                 "https:${line.substringBefore("/:")}"
             } else {
                 line.substringAfter("registry")
@@ -38,6 +44,6 @@ class RegistriesScanner(private val project: Project) {
             }
         }.map { it.removeSuffix("/") }.distinct().toList()
         log.info("Found registries: $registries")
-        state.isScanningForRegistries = false
+        if (!scanned) scanned = true
     }
 }
diff --git a/...ain/kotlin/com/github/warningimhack3r/npmupdatedependencies/backend/engine/ShellRunner.kt b/...ain/kotlin/com/github/warningimhack3r/npmupdatedependencies/backend/engine/ShellRunner.kt
@@ -3,31 +3,47 @@ package com.github.warningimhack3r.npmupdatedependencies.backend.engine
 import com.intellij.openapi.diagnostic.logger
 
 object ShellRunner {
+    private const val MAX_ATTEMPTS = 3
     private val log = logger<ShellRunner>()
-    private val failedCommands = mutableSetOf<String>()
+    private val failedWindowsPrograms = mutableSetOf<String>()
 
     private fun isWindows() = System.getProperty("os.name").lowercase().contains("win")
 
     fun execute(command: Array<String>): String? {
-        val commandName = command.firstOrNull() ?: return null
-        if (isWindows() && failedCommands.contains(commandName)) {
-            command[0] = "$commandName.cmd"
-            log.warn("Retrying command with .cmd extension: \"${command.joinToString(" ")}\"")
-        }
-        return try {
-            log.debug("Executing \"${command.joinToString(" ")}\"")
-            val process = ProcessBuilder(*command)
-                .redirectOutput(ProcessBuilder.Redirect.PIPE)
-                .start()
-            process.waitFor()
-            process.inputStream.bufferedReader().readText()
-        } catch (e: Exception) {
-            if (isWindows() && !commandName.endsWith(".cmd")) {
-                failedCommands.add(commandName)
-                return execute(arrayOf("$commandName.cmd") + command.drop(1).toTypedArray())
+        val program = command.firstOrNull() ?: return null
+        val isWindows = isWindows()
+        var attempts = 0
+
+        fun runCommand(): String? {
+            if (isWindows && failedWindowsPrograms.contains(program)) {
+                command[0] = "$program.cmd"
+                log.warn("(Re)trying command with .cmd extension: \"${command.joinToString(" ")}\"")
+            }
+            return try {
+                log.debug("Executing \"${command.joinToString(" ")}\"")
+                val process = ProcessBuilder(*command)
+                    .redirectOutput(ProcessBuilder.Redirect.PIPE)
+                    .start()
+                process.waitFor()
+                process.inputStream.bufferedReader().readText().also {
+                    log.debug("Executed command \"${command.joinToString(" ")}\" with output:\n$it")
+                }
+            } catch (e: Exception) {
+                if (isWindows && !program.endsWith(".cmd")) {
+                    failedWindowsPrograms.add(program)
+                    return execute(arrayOf("$program.cmd") + command.drop(1).toTypedArray())
+                }
+                log.warn("Error while executing \"${command.joinToString(" ")}\"", e)
+                null
             }
-            log.warn("Error while executing \"${command.joinToString(" ")}\"", e)
-            null
         }
+
+        while (attempts < MAX_ATTEMPTS) {
+            if (attempts > 0) log.warn("Retrying command \"${command.joinToString(" ")}\"")
+            runCommand()?.let { return it }
+            attempts++
+        }
+
+        return null
     }
 }
diff --git a/.../kotlin/com/github/warningimhack3r/npmupdatedependencies/backend/extensions/Extensions.kt b/.../kotlin/com/github/warningimhack3r/npmupdatedependencies/backend/extensions/Extensions.kt
@@ -19,11 +19,8 @@ val JsonElement.asJsonObject
 val JsonElement.asJsonArray
     get() = safeConversion { jsonArray }
 
-var JsonElement.asString
+val JsonElement.asString
     get() = safeConversion { jsonPrimitive.content }
-    set(_) {
-        // Do nothing
-    }
 
 fun JsonValue.stringValue(): String = text.replace("\"", "")
 

diff --git a/...github/warningimhack3r/npmupdatedependencies/errorsubmitter/GitHubErrorReportSubmitter.kt b/...github/warningimhack3r/npmupdatedependencies/errorsubmitter/GitHubErrorReportSubmitter.kt
@@ -22,6 +22,7 @@ import java.nio.charset.StandardCharsets
 // https://github.com/SonarSource/sonarlint-intellij/blob/master/src/main/java/org/sonarlint/intellij/errorsubmitter/BlameSonarSource.java
 class GitHubErrorReportSubmitter : ErrorReportSubmitter() {
     companion object {
+        private const val REPO_URL = "https://github.com/WarningImHack3r/npm-update-dependencies"
         private const val MAX_URL_LENGTH = 2083
         private const val BUG_LOGS_KEY = "bug-logs"
         private const val TRIMMED_STACKTRACE_MARKER = "\n\n<TRIMMED STACKTRACE>"
@@ -136,7 +137,7 @@ class GitHubErrorReportSubmitter : ErrorReportSubmitter() {
      * @return the URL for the GitHub issue.
      */
     private fun buildUrl(fields: Map<String, String>) = buildString {
-        append("https://github.com/WarningImHack3r/npm-update-dependencies/issues/new?labels=bug&template=bug_report.yml")
+        append("$REPO_URL/issues/new?labels=bug&template=bug_report.yml")
         fields.forEach { (key, value) ->
             append("&$key=${URLEncoder.encode(value, StandardCharsets.UTF_8)}")
         }

diff --git a/...rningimhack3r/npmupdatedependencies/ui/actions/deprecation/RemoveAllDeprecationsAction.kt b/...rningimhack3r/npmupdatedependencies/ui/actions/deprecation/RemoveAllDeprecationsAction.kt
@@ -6,11 +6,11 @@ import com.intellij.openapi.actionSystem.AnAction
 import com.intellij.openapi.actionSystem.AnActionEvent
 import com.intellij.openapi.actionSystem.CommonDataKeys
 import com.intellij.openapi.actionSystem.UpdateInBackground
-import com.intellij.openapi.components.service
 
 class RemoveAllDeprecationsAction : AnAction(), UpdateInBackground {
     override fun update(e: AnActionEvent) {
-        e.presentation.isEnabled = e.project?.service<NUDState>()?.deprecations?.isNotEmpty() ?: false
+        e.presentation.isEnabled =
+            e.project?.let { NUDState.getInstance(it) }?.deprecations?.isNotEmpty() ?: false
     }
 
     override fun actionPerformed(e: AnActionEvent) {

diff --git a/...ningimhack3r/npmupdatedependencies/ui/actions/deprecation/ReplaceAllDeprecationsAction.kt b/...ningimhack3r/npmupdatedependencies/ui/actions/deprecation/ReplaceAllDeprecationsAction.kt
@@ -6,11 +6,11 @@ import com.intellij.openapi.actionSystem.AnAction
 import com.intellij.openapi.actionSystem.AnActionEvent
 import com.intellij.openapi.actionSystem.CommonDataKeys
 import com.intellij.openapi.actionSystem.UpdateInBackground
-import com.intellij.openapi.components.service
 
 class ReplaceAllDeprecationsAction : AnAction(), UpdateInBackground {
     override fun update(e: AnActionEvent) {
-        e.presentation.isEnabled = e.project?.service<NUDState>()?.deprecations?.isNotEmpty() ?: false
+        e.presentation.isEnabled =
+            e.project?.let { NUDState.getInstance(it) }?.deprecations?.isNotEmpty() ?: false
     }
 
     override fun actionPerformed(e: AnActionEvent) {

diff --git a/...om/github/warningimhack3r/npmupdatedependencies/ui/actions/scan/InvalidateCachesAction.kt b/...om/github/warningimhack3r/npmupdatedependencies/ui/actions/scan/InvalidateCachesAction.kt
@@ -4,18 +4,17 @@ import com.github.warningimhack3r.npmupdatedependencies.backend.engine.NUDState
 import com.intellij.openapi.actionSystem.AnAction
 import com.intellij.openapi.actionSystem.AnActionEvent
 import com.intellij.openapi.actionSystem.UpdateInBackground
-import com.intellij.openapi.components.service
 
 class InvalidateCachesAction : AnAction(), UpdateInBackground {
     override fun update(e: AnActionEvent) {
-        val state = e.project?.service<NUDState>()
+        val state = e.project?.let { NUDState.getInstance(it) }
         e.presentation.isEnabled = if (state != null) {
             state.availableUpdates.isNotEmpty() || state.deprecations.isNotEmpty()
         } else false
     }
 
     override fun actionPerformed(e: AnActionEvent) {
-        val state = e.project?.service<NUDState>() ?: return
+        val state = e.project?.let { NUDState.getInstance(it) } ?: return
         state.availableUpdates.clear()
         state.deprecations.clear()
     }

diff --git a/...m/github/warningimhack3r/npmupdatedependencies/ui/actions/update/UpdateAllLatestAction.kt b/...m/github/warningimhack3r/npmupdatedependencies/ui/actions/update/UpdateAllLatestAction.kt
@@ -7,11 +7,11 @@ import com.intellij.openapi.actionSystem.AnAction
 import com.intellij.openapi.actionSystem.AnActionEvent
 import com.intellij.openapi.actionSystem.CommonDataKeys
 import com.intellij.openapi.actionSystem.UpdateInBackground
-import com.intellij.openapi.components.service
 
 class UpdateAllLatestAction : AnAction(), UpdateInBackground {
     override fun update(e: AnActionEvent) {
-        e.presentation.isEnabled = e.project?.service<NUDState>()?.availableUpdates?.isNotEmpty() ?: false
+        e.presentation.isEnabled =
+            e.project?.let { NUDState.getInstance(it) }?.availableUpdates?.isNotEmpty() ?: false
     }
 
     override fun actionPerformed(e: AnActionEvent) {

diff --git a/...ithub/warningimhack3r/npmupdatedependencies/ui/actions/update/UpdateAllSatisfiesAction.kt b/...ithub/warningimhack3r/npmupdatedependencies/ui/actions/update/UpdateAllSatisfiesAction.kt
@@ -7,11 +7,10 @@ import com.intellij.openapi.actionSystem.AnAction
 import com.intellij.openapi.actionSystem.AnActionEvent
 import com.intellij.openapi.actionSystem.CommonDataKeys
 import com.intellij.openapi.actionSystem.UpdateInBackground
-import com.intellij.openapi.components.service
 
 class UpdateAllSatisfiesAction : AnAction(), UpdateInBackground {
     override fun update(e: AnActionEvent) {
-        val availableUpdates = e.project?.service<NUDState>()?.availableUpdates
+        val availableUpdates = e.project?.let { NUDState.getInstance(it) }?.availableUpdates
         e.presentation.isEnabled = if (availableUpdates != null) {
             availableUpdates.isNotEmpty()
                     && availableUpdates.values.mapNotNull { it.versions.satisfies }.any()

diff --git a/...in/com/github/warningimhack3r/npmupdatedependencies/ui/annotation/DeprecationAnnotator.kt b/...in/com/github/warningimhack3r/npmupdatedependencies/ui/annotation/DeprecationAnnotator.kt
@@ -37,33 +37,28 @@ class DeprecationAnnotator : DumbAware, ExternalAnnotator<
         val (project, info) = collectedInfo
         if (info.isEmpty()) return emptyMap()
 
-        var state = NUDState.getInstance(project)
-        if (!state.isScanningForRegistries && state.packageRegistries.isEmpty()) {
-            log.debug("No registries found, scanning for registries...")
-            RegistriesScanner.getInstance(project).scan()
+        val state = NUDState.getInstance(project)
+        val registriesScanner = RegistriesScanner.getInstance(project)
+        if (!registriesScanner.scanned && !state.isScanningForRegistries) {
+            log.debug("Registries not scanned yet, scanning now")
+            state.isScanningForRegistries = true
+            registriesScanner.scan()
+            state.isScanningForRegistries = false
             log.debug("Registries scanned")
         }
 
-        if (state.isScanningForRegistries || state.isScanningForUpdates) {
-            log.debug("Waiting for registries and/or updates to be scanned...")
-            while (state.isScanningForRegistries || state.isScanningForUpdates) {
-                // Wait for the registries to be scanned and avoid multiple scans at the same time
-            }
-        }
-
-        log.debug("Scanning for deprecations...")
-        state = NUDState.getInstance(project)
+        val npmjsClient = NPMJSClient.getInstance(project)
         val maxParallelism = NUDSettingsState.instance.maxParallelism
         var activeTasks = 0
-        val npmjsClient = NPMJSClient.getInstance(project)
 
+        log.debug("Scanning for deprecations...")
         return info
-            .also {
+            .also { properties ->
                 // Remove from the cache all deprecations that are no longer in the file
-                val fileDependenciesNames = it.map { property -> property.name }
+                val fileDependenciesNames = properties.map { property -> property.name }
                 state.deprecations.keys.removeAll { key -> !fileDependenciesNames.contains(key) }
                 // Update the status bar widget
-                state.totalPackages = it.size
+                state.totalPackages = properties.size
                 state.scannedDeprecations = 0
                 state.isScanningForDeprecations = true
             }.parallelMap { property ->
@@ -113,7 +108,7 @@ class DeprecationAnnotator : DumbAware, ExternalAnnotator<
                         Deprecation(reason, null)
                     ) // No replacement found in the deprecation reason
                 }.also { result ->
-                    result?.let { (property, deprecation) ->
+                    result?.let { (_, deprecation) ->
                         // Add the deprecation to the cache if any
                         state.deprecations[property.name] = deprecation
                     } ?: state.deprecations[property.name]?.let { _ ->
@@ -127,7 +122,7 @@ class DeprecationAnnotator : DumbAware, ExternalAnnotator<
                     activeTasks--
                 }
             }.filterNotNull().toMap().also {
-                log.debug("Deprecations scanned, ${it.size} found")
+                log.debug("Deprecations scanned, ${it.size} found out of ${info.size}")
                 state.isScanningForDeprecations = false
             }
     }