Skip to content

Commit

Permalink
Refactor PackageSearchApiPackageCache ins separate module
Browse files Browse the repository at this point in the history
PackageSearchApiPackageCache tests classpath was poised by the IJ Gradle plugin. Moving it in a simple Kotlin/JVM module solved the issue.
  • Loading branch information
lamba92 committed May 13, 2024
1 parent f1252e6 commit 4ee453e
Show file tree
Hide file tree
Showing 28 changed files with 21,092 additions and 214 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -150,4 +150,4 @@ jobs:
- name: Run tests
env:
GRADLE_ENTERPRISE_KEY: ${{ secrets.GRADLE_ENTERPRISE_KEY }}
run: ./gradlew :plugin:test --continue --tests "com.jetbrains.packagesearch.plugin.tests.unit.*"
run: ./gradlew :plugin:utils:test
2 changes: 2 additions & 0 deletions plugin/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,14 @@ dependencies {
implementation(projects.plugin.gradle.base)
implementation(projects.plugin.gradle.kmp)
implementation(projects.plugin.maven)
implementation(projects.plugin.utils)

sourceElements(projects.plugin.core)
sourceElements(projects.plugin.gradle)
sourceElements(projects.plugin.gradle.base)
sourceElements(projects.plugin.gradle.kmp)
sourceElements(projects.plugin.maven)
sourceElements(projects.plugin.utils)

tooling(projects.plugin.gradle.tooling)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -366,8 +366,6 @@ class ProjectDataImportListenerAdapter(private val project: Project) : ProjectDa
val Module.isSourceSet
get() = ExternalSystemApiUtil.getExternalModuleType(this) == "sourceSet"

fun <T> Result<T>.suspendSafe() = onFailure { if (it is CancellationException) throw it }

fun Path.isSameFileAsSafe(other: Path): Boolean = kotlin.runCatching { Files.isSameFile(this, other) }
.getOrDefault(false)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import com.intellij.internal.statistic.service.fus.collectors.CounterUsagesColle
import com.intellij.openapi.diagnostic.RuntimeExceptionWithAttachments
import com.jetbrains.packagesearch.plugin.PackageSearchBundle
import com.jetbrains.packagesearch.plugin.core.data.PackageSearchModule
import com.jetbrains.packagesearch.plugin.utils.logError
import com.jetbrains.packagesearch.plugin.utils.PackageSearchLogger
import org.jetbrains.idea.reposearch.statistics.TopPackageIdValidationRule
import org.jetbrains.packagesearch.api.v3.ApiRepository

Expand Down Expand Up @@ -319,12 +319,14 @@ private fun <T : BaseEventId> runSafelyIfEnabled(event: T, action: T.() -> Unit)
try {
event.action()
} catch (e: RuntimeException) {
logError(
val throwable = RuntimeExceptionWithAttachments(
/* message = */ "Non-critical error while logging analytics event. " +
"This doesn't impact plugin functionality.",
/* cause = */ e
)
PackageSearchLogger.logError(
message = PackageSearchBundle.message("packagesearch.logging.error", event.eventId),
throwable = RuntimeExceptionWithAttachments(
"Non-critical error while logging analytics event. This doesn't impact plugin functionality.",
e
)
throwable = throwable
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import com.jetbrains.packagesearch.plugin.utils.ApiRepositoryCacheEntry
import com.jetbrains.packagesearch.plugin.utils.ApiSearchCacheEntry
import com.jetbrains.packagesearch.plugin.utils.KtorDebugLogger
import com.jetbrains.packagesearch.plugin.utils.PackageSearchApiPackageCache
import com.jetbrains.packagesearch.plugin.utils.PackageSearchLogger
import com.jetbrains.packagesearch.plugin.utils.PackageSearchProjectService
import io.ktor.client.engine.java.Java
import io.ktor.client.plugins.DefaultRequest
Expand Down Expand Up @@ -98,6 +99,7 @@ class PackageSearchApplicationCachesService(private val coroutineScope: Coroutin
repositoryCache = repositoryCache,
searchCache = searchesRepository,
apiClient = apiClient,
logger = PackageSearchLogger,
isOnline = { isOnlineFlow.value }
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import com.intellij.openapi.components.Service
import com.intellij.openapi.components.Service.Level
import com.jetbrains.packagesearch.plugin.fus.PackageSearchFUSEvent
import com.jetbrains.packagesearch.plugin.fus.log
import com.jetbrains.packagesearch.plugin.utils.logWarn
import com.jetbrains.packagesearch.plugin.utils.PackageSearchLogger
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.flow.SharingStarted
Expand All @@ -24,7 +24,10 @@ class PackageSearchFUSService(coroutineScope: CoroutineScope) {
fusEventsFlow
.onEach { it.log() }
.retry(5) {
logWarn("${this::class.qualifiedName}#eventReportingJob", it) { "Failed to log FUS" }
PackageSearchLogger.logWarn(
contextName = "${this::class.qualifiedName}#eventReportingJob",
throwable = it
) { "Failed to log FUS" }
true
}
.launchIn(coroutineScope)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,11 @@ import com.jetbrains.packagesearch.plugin.core.utils.fileOpenedFlow
import com.jetbrains.packagesearch.plugin.core.utils.replayOn
import com.jetbrains.packagesearch.plugin.core.utils.toolWindowOpenedFlow
import com.jetbrains.packagesearch.plugin.utils.PackageSearchApplicationCachesService
import com.jetbrains.packagesearch.plugin.utils.PackageSearchLogger
import com.jetbrains.packagesearch.plugin.utils.PackageSearchSettingsService
import com.jetbrains.packagesearch.plugin.utils.WindowedModuleBuilderContext
import com.jetbrains.packagesearch.plugin.utils.drop
import com.jetbrains.packagesearch.plugin.utils.filterNotNullKeys
import com.jetbrains.packagesearch.plugin.utils.logDebug
import com.jetbrains.packagesearch.plugin.utils.logWarn
import com.jetbrains.packagesearch.plugin.utils.nativeModulesFlow
import com.jetbrains.packagesearch.plugin.utils.startWithNull
import com.jetbrains.packagesearch.plugin.utils.throttle
Expand Down Expand Up @@ -74,7 +73,10 @@ class PackageSearchProjectService(
.associateBy { it.id }
}
.retry(5) {
logWarn("${this::class.simpleName}#knownRepositoriesStateFlow", throwable = it)
PackageSearchLogger.logDebug(
contextName = "${this::class.simpleName}#knownRepositoriesStateFlow",
throwable = it
)
true
}
.stateIn(coroutineScope, SharingStarted.Eagerly, emptyMap())
Expand Down Expand Up @@ -114,10 +116,10 @@ class PackageSearchProjectService(
.onStart { emit(Unit) }
.flatMapLatest { moduleProvidersList }
.retry(5) {
logWarn("${this::class.simpleName}#modulesStateFlow", throwable = it)
PackageSearchLogger.logWarn("${this::class.simpleName}#modulesStateFlow", throwable = it)
true
}
.onEach { logDebug("${this::class.qualifiedName}#modulesStateFlow") { "modules.size = ${it.size}" } }
.onEach { PackageSearchLogger.logDebug("${this::class.qualifiedName}#modulesStateFlow") { "modules.size = ${it.size}" } }
.stateIn(coroutineScope, SharingStarted.Lazily, emptyList())

val modulesByBuildFile = modulesStateFlow
Expand Down Expand Up @@ -155,7 +157,7 @@ class PackageSearchProjectService(
.throttle(30.minutes)
.onEach { restart() }
.retry(5) {
logWarn("${this::class.simpleName}#isOnlineFlow", throwable = it)
PackageSearchLogger.logWarn("${this::class.simpleName}#isOnlineFlow", throwable = it)
true
}
.launchIn(coroutineScope)
Expand All @@ -174,7 +176,7 @@ class PackageSearchProjectService(
}
}
.retry(5) {
logWarn("${this::class.simpleName}#fileOpenedFlow", throwable = it)
PackageSearchLogger.logWarn("${this::class.simpleName}#fileOpenedFlow", throwable = it)
true
}
.launchIn(coroutineScope)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import com.intellij.openapi.components.Service.Level
import com.intellij.openapi.project.Project
import com.jetbrains.packagesearch.plugin.core.utils.packageSearchProjectDataPath
import com.jetbrains.packagesearch.plugin.ui.PackageSearchMetrics
import com.jetbrains.packagesearch.plugin.utils.logDebug
import com.jetbrains.packagesearch.plugin.utils.PackageSearchLogger
import kotlin.io.path.div
import kotlin.io.path.inputStream
import kotlin.io.path.readText
Expand Down Expand Up @@ -38,9 +38,9 @@ class PackageSearchSettingsService(private val project: Project, private val cor

private fun loadSettings() =
kotlin.runCatching { json.decodeFromStream<Settings>(settingsFile.inputStream()) }
.onSuccess { logDebug { "Settings loaded: \n${settingsFile.readText()}" } }
.onSuccess { PackageSearchLogger.logDebug { "Settings loaded: \n${settingsFile.readText()}" } }
.map { it.asSafe() }
.onFailure { logDebug("Failed to load settings", it) }
.onFailure { PackageSearchLogger.logDebug(throwable = it, message = "Failed to load settings") }
.getOrElse { Settings() }

private suspend fun Settings.save() =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,10 @@ import com.jetbrains.packagesearch.plugin.ui.model.infopanel.InfoPanelViewModel
import com.jetbrains.packagesearch.plugin.ui.model.packageslist.PackageListItemEvent.SetHeaderState.TargetState
import com.jetbrains.packagesearch.plugin.ui.model.packageslist.PackageListItemEvent.SetHeaderState.TargetState.OPEN
import com.jetbrains.packagesearch.plugin.utils.PackageSearchApplicationCachesService
import com.jetbrains.packagesearch.plugin.utils.PackageSearchLogger
import com.jetbrains.packagesearch.plugin.utils.PackageSearchProjectService
import com.jetbrains.packagesearch.plugin.utils.PackageSearchSettingsService
import com.jetbrains.packagesearch.plugin.utils.logFUSEvent
import com.jetbrains.packagesearch.plugin.utils.logTODO
import com.jetbrains.packagesearch.plugin.utils.logWarn
import kotlin.time.Duration.Companion.milliseconds
import kotlin.time.Duration.Companion.seconds
import kotlinx.coroutines.CoroutineScope
Expand Down Expand Up @@ -330,7 +329,7 @@ class PackageListViewModel(
}

private fun handle(event: PackageListItemEvent.InfoPanelEvent.OnHeaderVariantsClick) {
logTODO()
PackageSearchLogger.logTODO()
logFUSEvent(PackageSearchFUSEvent.InfoPanelOpened)
}

Expand Down Expand Up @@ -650,13 +649,13 @@ class PackageListViewModel(
targetModule = module
)
)
logTODO()
PackageSearchLogger.logTODO()
}
}
}
}
.onFailure {
logWarn("Failed to set scope for package:\n${json.encodeToString(event)}", it)
PackageSearchLogger.logWarn("Failed to set scope for package:\n${json.encodeToString(event)}", it)
}
}

Expand Down Expand Up @@ -760,7 +759,7 @@ class PackageListViewModel(
}
}
.onFailure {
logWarn("Failed to update packages:\n${json.encodeToString(event)}", it)
PackageSearchLogger.logWarn("Failed to update packages:\n${json.encodeToString(event)}", it)
}
}

Expand Down Expand Up @@ -794,7 +793,7 @@ class PackageListViewModel(
}
}
.onFailure {
logWarn("Failed to update packages:\n${json.encodeToString(event)}", it)
PackageSearchLogger.logWarn("Failed to update packages:\n${json.encodeToString(event)}", it)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import com.intellij.openapi.project.Project
import com.jetbrains.packagesearch.plugin.core.data.PackageSearchModule
import com.jetbrains.packagesearch.plugin.core.utils.IntelliJApplication
import com.jetbrains.packagesearch.plugin.utils.PackageSearchApplicationCachesService
import com.jetbrains.packagesearch.plugin.utils.PackageSearchLogger
import com.jetbrains.packagesearch.plugin.utils.PackageSearchProjectService
import com.jetbrains.packagesearch.plugin.utils.PackageSearchSettingsService
import com.jetbrains.packagesearch.plugin.utils.logDebug
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
Expand All @@ -35,7 +35,7 @@ internal class TreeViewModel(
modules.asTree(stableOnly)
}
.retry(5)
.onEach { logDebug("${this::class.qualifiedName}#treeStateFlow") { it.print() } }
.onEach { PackageSearchLogger.logDebug("${this::class.qualifiedName}#treeStateFlow") { it.print() } }
.stateIn(viewModelScope, SharingStarted.Lazily, emptyTree())

private fun Tree<TreeItemModel>.print(): String {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
package com.jetbrains.packagesearch.plugin.utils

import com.intellij.openapi.components.Service
import com.intellij.openapi.components.Service.Level
import com.intellij.openapi.diagnostic.Logger
import com.jetbrains.packagesearch.plugin.FeatureFlags
import com.jetbrains.packagesearch.plugin.core.PackageSearch
import java.util.concurrent.atomic.AtomicBoolean

@Service(Level.APP)
class IntelliJLogger : PluginLogger {

private val logger: Logger = Logger.getInstance("#${PackageSearch.pluginId}")
private val warned = AtomicBoolean(false)

override fun logError(message: String, throwable: Throwable?) {
logger.error(message, throwable)
}

override fun logError(contextName: String?, throwable: Throwable?, messageProvider: () -> String) {
logError(buildMessageFrom(contextName, messageProvider), throwable)
}

override fun logWarn(message: String, throwable: Throwable?) {
logger.warn(message, throwable)
}

override fun logWarn(contextName: String?, throwable: Throwable?, messageProvider: () -> String) {
logWarn(buildMessageFrom(contextName, messageProvider), throwable)
}

override fun logInfo(message: String, throwable: Throwable?) {
logger.info(message, throwable)
}

override fun logInfo(contextName: String?, throwable: Throwable?, messageProvider: () -> String) {
logInfo(buildMessageFrom(contextName, messageProvider), throwable)
}

override fun logDebug(message: String, throwable: Throwable?) {
if (!logger.isDebugEnabled) warnNotLoggable()
logger.debug(message, throwable)
}

override fun logDebug(contextName: String?, throwable: Throwable?, message: String?) {
logDebug(buildMessageFrom(contextName, message = message), throwable)
}

override fun logDebug(contextName: String?, throwable: Throwable?, messageProvider: () -> String) {
logDebug(buildMessageFrom(contextName, messageProvider), throwable)
}

override fun logTrace(message: String) {
if (!FeatureFlags.useDebugLogging || !logger.isTraceEnabled) warnNotLoggable()
logger.trace(message)
}

override fun logTrace(throwable: Throwable) {
if (!FeatureFlags.useDebugLogging || !logger.isTraceEnabled) warnNotLoggable()
logger.trace(throwable)
}

override fun logTODO() {
logWarn("This feature is not implemented yet") {
"Stacktrace:\n" + Thread.currentThread().stackTrace.joinToString("\n") {
"${it.className}.${it.methodName}(${it.fileName}:${it.lineNumber})"
}
}
}

private fun warnNotLoggable() {
if (warned.getAndSet(true)) return
logger.warn(
"""
|!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
|Debug logging not enabled. Make sure you have a line like this:
| #${PackageSearch.pluginId}:trace
|in your debug log settings (Help | Diagnostic Tools | Debug Log Settings)
|then restart the IDE.
|!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
|""".trimMargin()
)
}
}

fun buildMessageFrom(
contextName: String?,
messageProvider: (() -> String)? = null,
message: String? = null,
): String = buildString {
if (!contextName.isNullOrBlank()) {
append(contextName)
append(' ')
}
if (isNotEmpty()) append("- ")
messageProvider?.let { append(it()) }
message?.let { append(it) }
}
Loading

0 comments on commit 4ee453e

Please sign in to comment.