From bf0034528874fe0d2cf852801646546f7a16b4b2 Mon Sep 17 00:00:00 2001 From: Ilya Muradyan Date: Tue, 1 Sep 2026 02:05:19 +0200 Subject: [PATCH] WEB-79133 platform: ask a provider for a plugin offer when the advertiser has none The advertiser matches a plugin to an UnknownFeature by string equality on the implementation name. A dependencySupport bean cannot express an offer that depends on which version of a dependency the project declares. A plugin with a version-aware offer therefore raises its own balloon on project open, and that balloon and the advertiser's own can sit on the screen together. PluginSuggestionNotificationProvider is a new extension point, next to pluginSuggestionProvider, which does the same job for the editor banner. A provider returns a ready Notification, or null. The advertiser decides whether to ask. The provider decides what the balloon says, which notification group it belongs to, and which actions it carries. notifyUser now returns whether the advertiser has an offer of its own for the project. It returns true on the suggest-or-enable branch, on the bundled Try Ultimate branch, and on the includeIgnored branch that says no plugin was suggested. It returns false where it built no offer, and run() asks the providers on that false. The return says the advertiser reached notify, and notify raises no balloon while an earlier balloon of the group is visible, and none at all for a group the user set to no popup. The call site sits inside PluginAdvertiserServiceImpl.run, so a path where the advertiser never reaches notifyUser asks nobody: the registry key ide.show.plugin.suggestions.on.open switched off, an untrusted project, and headless. HeadlessPluginAdvertiserServiceImpl.run stays an empty override, so the headless service and the test service ask nobody either. RiderPluginAdvertiserService overrides getAddressedMessagePresentation and inherits the new path. The first answer wins. The advertiser asks to fill the one balloon it did not raise, so a second balloon here recreates the stacking this extension point prevents. A provider is called on a background thread. It may suspend and it may take a read action, because it answers from project content. The advertiser publishes the notification on the EDT, where a provider takes its own notification down, and it publishes nothing the provider has already expired. A provider that fails is logged under its own class and skipped, in the way ExtensionProcessingHelper.computeSafeIfAny handles a provider that does not suspend. That helper takes a non-suspend function, so the handling is written out here. With no provider registered the extension list is empty, the advertiser publishes nothing extra, and the IDE behaves as it did before. Verified: bazel build of intellij.platform.ide.impl; 8 tests in PluginSuggestionNotificationProviderTest and 23 over com.intellij.ide.plugins.advertiser, all passing; lint_files clean over the new files. Six mutation rows, each turning one case red: the call site in run() deleted, notifyUser returning true where it built no offer, the expired-notification guard removed, the first-answer rule replaced by every answer, the EDT hop removed, and a provider exception let out of the extension list. Claude-Session: https://claude.ai/code/session_01XabGLGJyJQCmYSPE6kX9Dd (cherry picked from commit acc9aab3a62f91fc78e81c353838b6a99dcf7ac4) GitOrigin-RevId: b03f28925ec2772b4540ad38af20363d84f2e1e4 --- .../PluginAdvertiserService.kt | 17 +- .../PluginSuggestionNotificationProvider.kt | 89 ++++++++ .../src/META-INF/PlatformExtensionPoints.xml | 2 + ...luginSuggestionNotificationProviderTest.kt | 198 ++++++++++++++++++ 4 files changed, 303 insertions(+), 3 deletions(-) create mode 100644 platform/platform-impl/src/com/intellij/openapi/updateSettings/impl/pluginsAdvertisement/PluginSuggestionNotificationProvider.kt create mode 100644 platform/platform-tests/testSrc/com/intellij/ide/plugins/advertiser/PluginSuggestionNotificationProviderTest.kt diff --git a/platform/platform-impl/src/com/intellij/openapi/updateSettings/impl/pluginsAdvertisement/PluginAdvertiserService.kt b/platform/platform-impl/src/com/intellij/openapi/updateSettings/impl/pluginsAdvertisement/PluginAdvertiserService.kt index ba851265a290..870e7b8b76f8 100644 --- a/platform/platform-impl/src/com/intellij/openapi/updateSettings/impl/pluginsAdvertisement/PluginAdvertiserService.kt +++ b/platform/platform-impl/src/com/intellij/openapi/updateSettings/impl/pluginsAdvertisement/PluginAdvertiserService.kt @@ -202,7 +202,7 @@ open class PluginAdvertiserServiceImpl( launch { val dependencies = serviceAsync().dependencies.get() - withContext(Dispatchers.EDT) { + val hasOwnOffer = withContext(Dispatchers.EDT) { notifyUser( bundledPlugins = getBundledPluginToInstall(plugins, descriptorsById), suggestionPlugins = suggestToInstall, @@ -213,6 +213,9 @@ open class PluginAdvertiserServiceImpl( includeIgnored = includeIgnored, ) } + if (!hasOwnOffer) { + showPluginSuggestionNotification(project) + } } } } @@ -402,6 +405,12 @@ open class PluginAdvertiserServiceImpl( .toList() } + /** + * @return whether the advertiser has an offer of its own for this project. False is the verdict a + * [PluginSuggestionNotificationProvider] is asked on. True says the advertiser reached + * `notify`, which raises no balloon while an earlier balloon of the group is visible and none + * at all for a group the user set to no popup. + */ @RequiresEdt private fun notifyUser( bundledPlugins: List, @@ -411,7 +420,7 @@ open class PluginAdvertiserServiceImpl( allUnknownFeatures: Collection, dependencies: PluginFeatureMap?, includeIgnored: Boolean, - ) { + ): Boolean { for (plugin in suggestionPlugins) { FUSEventSource.NOTIFICATION.logPluginSuggested(project, plugin.id) } @@ -471,13 +480,15 @@ open class PluginAdvertiserServiceImpl( .createNotification(IdeBundle.message("plugins.advertiser.no.suggested.plugins"), NotificationType.INFORMATION) .setDisplayId("advertiser.no.plugins") .notify(project) + return true } - return + return false } notificationManager.notify("", notificationMessage, project) { it.setSuggestionType(true).addActions(notificationActions as Collection) } + return true } private fun createIgnoreUnknownFeaturesAction( diff --git a/platform/platform-impl/src/com/intellij/openapi/updateSettings/impl/pluginsAdvertisement/PluginSuggestionNotificationProvider.kt b/platform/platform-impl/src/com/intellij/openapi/updateSettings/impl/pluginsAdvertisement/PluginSuggestionNotificationProvider.kt new file mode 100644 index 000000000000..a0154a89b651 --- /dev/null +++ b/platform/platform-impl/src/com/intellij/openapi/updateSettings/impl/pluginsAdvertisement/PluginSuggestionNotificationProvider.kt @@ -0,0 +1,89 @@ +// Copyright 2000-2026 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package com.intellij.openapi.updateSettings.impl.pluginsAdvertisement + +import com.intellij.notification.Notification +import com.intellij.openapi.application.EDT +import com.intellij.openapi.diagnostic.logger +import com.intellij.openapi.diagnostic.rethrowControlFlowException +import com.intellij.openapi.extensions.ExtensionPointName +import com.intellij.openapi.project.Project +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import org.jetbrains.annotations.ApiStatus +import org.jetbrains.annotations.VisibleForTesting + +/** + * Provides the balloon the advertiser raises for a project it has no offer of its own for. + * + * The advertiser matches a plugin to an [UnknownFeature] by string equality on the implementation + * name, so a `dependencySupport` bean cannot express an offer that depends on which version of a + * dependency the project declares. A provider decides that question itself and builds the whole + * notification, so it keeps its own notification group, its own actions and its own statistics. + * + * The advertiser asks a provider after it decides that it raises no balloon of its own for this + * project. It asks nobody when `ide.show.plugin.suggestions.on.open` is off, on an untrusted + * project, or in a headless IDE, because the advertiser does not run on those paths. + * + * Implemented by a bundled IDE plugin, in the same way as [PluginSuggestionProvider]. + */ +@ApiStatus.Internal +interface PluginSuggestionNotificationProvider { + /** + * The balloon this provider offers [project], or null when it offers none. + * + * Called on a background thread. It may suspend and it may take a read action. The advertiser + * publishes the notification on the EDT, and publishes nothing when the provider has already + * expired it. + */ + suspend fun createNotification(project: Project): Notification? + + companion object { + @JvmField + val EP_NAME: ExtensionPointName = + ExtensionPointName("com.intellij.pluginSuggestionNotificationProvider") + } +} + +private val LOG = logger() + +/** + * Publishes the first balloon a provider offers [project]. + * + * The first answer wins. The advertiser asks to fill the one balloon it did not raise, so a second + * balloon here recreates the stacking this extension point prevents. + * + * The publish takes the EDT because a provider takes the notification down from there. Off the EDT, + * `Notifications.Bus.notify` queues the publish through `invokeLater`, and a provider can then + * expire a notification the notifications model has not seen. That model adds its tool-window row + * without reading the expiry, so the row outlives the balloon. + */ +@VisibleForTesting +@ApiStatus.Internal +suspend fun showPluginSuggestionNotification(project: Project) { + val notification = createSuggestionNotification(project) ?: return + + withContext(Dispatchers.EDT) { + if (!notification.isExpired) { + notification.notify(project) + } + } +} + +/** + * Asks each provider in turn and answers with the first notification offered. + * + * A provider that fails is logged under its own class and skipped, so a broken plugin costs its own + * offer and leaves the advertiser running. `ExtensionProcessingHelper.computeSafeIfAny` holds the + * same handling for a provider that does not suspend. + */ +private suspend fun createSuggestionNotification(project: Project): Notification? { + for (provider in PluginSuggestionNotificationProvider.EP_NAME.extensionList) { + try { + return provider.createNotification(project) ?: continue + } catch (e: Throwable) { + rethrowControlFlowException(e) + LOG.error("${provider.javaClass.name} failed to create a plugin suggestion", e) + } + } + return null +} diff --git a/platform/platform-resources/src/META-INF/PlatformExtensionPoints.xml b/platform/platform-resources/src/META-INF/PlatformExtensionPoints.xml index 1d5200e98818..0664329be09b 100644 --- a/platform/platform-resources/src/META-INF/PlatformExtensionPoints.xml +++ b/platform/platform-resources/src/META-INF/PlatformExtensionPoints.xml @@ -342,6 +342,8 @@ dynamic="true"/> + diff --git a/platform/platform-tests/testSrc/com/intellij/ide/plugins/advertiser/PluginSuggestionNotificationProviderTest.kt b/platform/platform-tests/testSrc/com/intellij/ide/plugins/advertiser/PluginSuggestionNotificationProviderTest.kt new file mode 100644 index 000000000000..8f16a3377540 --- /dev/null +++ b/platform/platform-tests/testSrc/com/intellij/ide/plugins/advertiser/PluginSuggestionNotificationProviderTest.kt @@ -0,0 +1,198 @@ +// Copyright 2000-2026 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package com.intellij.ide.plugins.advertiser + +import com.intellij.notification.Notification +import com.intellij.notification.NotificationAction +import com.intellij.notification.NotificationType +import com.intellij.notification.Notifications +import com.intellij.openapi.application.ApplicationManager +import com.intellij.openapi.project.Project +import com.intellij.openapi.updateSettings.impl.pluginsAdvertisement.PluginAdvertiserServiceImpl +import com.intellij.openapi.updateSettings.impl.pluginsAdvertisement.PluginSuggestionNotificationProvider +import com.intellij.openapi.updateSettings.impl.pluginsAdvertisement.getPluginSuggestionNotificationGroup +import com.intellij.openapi.updateSettings.impl.pluginsAdvertisement.showPluginSuggestionNotification +import com.intellij.testFramework.DisposableRule +import com.intellij.testFramework.LoggedErrorProcessor +import com.intellij.testFramework.ProjectRule +import com.intellij.platform.util.coroutines.childScope +import kotlinx.coroutines.cancel +import kotlinx.coroutines.job +import kotlinx.coroutines.joinAll +import kotlinx.coroutines.runBlocking +import org.junit.Before +import org.junit.ClassRule +import org.junit.Rule +import org.junit.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class PluginSuggestionNotificationProviderTest { + companion object { + @JvmField + @ClassRule + val projectRule: ProjectRule = ProjectRule() + } + + @JvmField + @Rule + val disposableRule: DisposableRule = DisposableRule() + + private val published = mutableListOf() + + /** The thread each notification was published on. */ + private val publishedOnEdt = mutableListOf() + + @Before + fun subscribe() { + projectRule.project.messageBus.connect(disposableRule.disposable).subscribe(Notifications.TOPIC, object : Notifications { + override fun notify(notification: Notification) { + if (notification.displayId != DISPLAY_ID) return + published.add(notification) + publishedOnEdt.add(ApplicationManager.getApplication().isDispatchThread) + } + }) + } + + /** + * The advertiser reaches the providers from `run`, which is the wiring the rest of the class + * takes for granted. + * + * Empty unknown features leave the advertiser with nothing of its own to say: `fetchFeatures` + * loops zero times, the empty plugin set skips the Marketplace lookup, and `notifyUser` takes the + * branch that raises no balloon. So the case needs no network. + */ + @Test + fun theAdvertiserAsksWhenItHasNoOfferOfItsOwn() = runBlocking { + register("from the advertiser") + val scope = childScope("plugin advertiser service") + try { + PluginAdvertiserServiceImpl(projectRule.project, scope).run( + customPlugins = emptyList(), + unknownFeatures = emptyList(), + includeIgnored = false, + ) + // `run` launches into the service scope and returns, so the case waits for what it started. + scope.coroutineContext.job.children.toList().joinAll() + } + finally { + scope.cancel() + } + + assertEquals("from the advertiser", published.single().content) + } + + /** + * The property an IDE that registers no provider rests on: the advertiser behaves as it did + * before the extension point existed. + */ + @Test + fun nothingIsPublishedWithoutProviders() { + runBlocking { showPluginSuggestionNotification(projectRule.project) } + + assertTrue(published.isEmpty(), "a notification was published with no provider registered") + } + + /** The advertiser asks to fill the one balloon it did not raise, so a second answer is not read. */ + @Test + fun theFirstAnswerWins() { + val asked = mutableListOf() + register("first") { asked.add("first") } + register("second") { asked.add("second") } + + runBlocking { showPluginSuggestionNotification(projectRule.project) } + + assertEquals(listOf("first"), asked) + assertEquals(1, published.size) + assertEquals("first", published.single().content) + } + + /** A provider that offers nothing lets the next one answer. */ + @Test + fun aProviderThatOffersNothingIsSkipped() { + PluginSuggestionNotificationProvider.EP_NAME.point.registerExtension(object : PluginSuggestionNotificationProvider { + override suspend fun createNotification(project: Project): Notification? = null + }, disposableRule.disposable) + register("second") + + runBlocking { showPluginSuggestionNotification(projectRule.project) } + + assertEquals("second", published.single().content) + } + + /** + * A provider that fails costs its own offer. The advertiser runs inside a coroutine of the + * project scope, and an exception let out of the extension list would take that scope down. + */ + @Test + fun aProviderThatFailsIsSkipped() { + PluginSuggestionNotificationProvider.EP_NAME.point.registerExtension(object : PluginSuggestionNotificationProvider { + override suspend fun createNotification(project: Project): Notification = throw UnsupportedOperationException("broken provider") + }, disposableRule.disposable) + register("second") + + LoggedErrorProcessor.executeWith(object : LoggedErrorProcessor() { + override fun processError(category: String, message: String, details: Array, t: Throwable?): Set = setOf(Action.LOG) + }) { + runBlocking { showPluginSuggestionNotification(projectRule.project) } + } + + assertEquals("second", published.single().content) + } + + /** A provider takes its own notification down, and the advertiser publishes no dead balloon. */ + @Test + fun anExpiredNotificationIsNotPublished() { + PluginSuggestionNotificationProvider.EP_NAME.point.registerExtension(object : PluginSuggestionNotificationProvider { + override suspend fun createNotification(project: Project): Notification = notification("expired").also { it.expire() } + }, disposableRule.disposable) + + runBlocking { showPluginSuggestionNotification(projectRule.project) } + + assertTrue(published.isEmpty(), "an expired notification reached the notifications model") + } + + /** + * The publish takes the EDT, which is where a provider takes its notification down. Off the EDT + * the publish is queued through `invokeLater`, and the Notifications tool window then keeps a row + * for a balloon that was expired before it was drawn. + */ + @Test + fun theNotificationIsPublishedOnTheEdt() { + register("on the EDT") + + val done = ApplicationManager.getApplication().executeOnPooledThread { + runBlocking { showPluginSuggestionNotification(projectRule.project) } + } + done.get() + + assertEquals(listOf(true), publishedOnEdt) + } + + /** The provider keeps its own actions, which is what the returned notification carries. */ + @Test + fun theProviderKeepsItsActions() { + register("with an action") + + runBlocking { showPluginSuggestionNotification(projectRule.project) } + + assertFalse(published.single().actions.isEmpty(), "the provider's action was dropped") + } + + private fun register(content: String, onAsked: () -> Unit = {}) { + PluginSuggestionNotificationProvider.EP_NAME.point.registerExtension(object : PluginSuggestionNotificationProvider { + override suspend fun createNotification(project: Project): Notification { + onAsked() + return notification(content) + } + }, disposableRule.disposable) + } + + private fun notification(content: String): Notification = + getPluginSuggestionNotificationGroup() + .createNotification(content, NotificationType.INFORMATION) + .setDisplayId(DISPLAY_ID) + .addAction(NotificationAction.createSimple("action") {}) +} + +private const val DISPLAY_ID: String = "plugin.suggestion.notification.provider.test"