From 056294550b643e2fd94ff785ff8c440fb92d224e Mon Sep 17 00:00:00 2001 From: Filipp Vakhitov Date: Sun, 16 Jul 2023 22:16:07 +0300 Subject: [PATCH] IDEA-291623 Show a warning if the settings pack exceeds some reasonably big limit Post code review improvements GitOrigin-RevId: c3875ea81f130b69e8435c89a8cc906648340638 --- .../messages/SettingsSyncBundle.properties | 22 ++++++-- .../CloudConfigServerCommunicator.kt | 5 -- .../SettingsSnapshotZipSerializer.kt | 14 ++--- .../settingsSync/SettingsSyncEvents.kt | 54 +++++++++++-------- .../SettingsSyncIdeMediatorImpl.kt | 17 +++++- .../notification/NotificationService.kt | 3 -- .../notification/NotificationServiceImpl.kt | 25 +++++---- 7 files changed, 85 insertions(+), 55 deletions(-) diff --git a/plugins/settings-sync/resources/messages/SettingsSyncBundle.properties b/plugins/settings-sync/resources/messages/SettingsSyncBundle.properties index e78322d1ceb5..68ad1039c87d 100644 --- a/plugins/settings-sync/resources/messages/SettingsSyncBundle.properties +++ b/plugins/settings-sync/resources/messages/SettingsSyncBundle.properties @@ -65,9 +65,20 @@ subcategory.config.link=Configure settings.category.ui.editor.font=Editor font settings.sync.info.message=Sync UI, Code and System settings, Keymaps, Plugins, and Tools. sync.restart.notification.title=Restart required after syncing settings -sync.restart.notification.message=Restart the IDE to {0} -sync.restart.notification.message.subtitle=Restart the IDE to complete the following actions: -sync.restart.notification.submessage.registry=apply the new UI + +# {0} - count of plugins, {1} - coma separated list of plugins (max two plugins) +sync.notification.restart.message.plugin.install=Restart the IDE to install {0,choice,1#plugin|2#plugins}: {1}{0,choice,3#...} +sync.notification.restart.message.plugin.enable=Restart the IDE to enable {0,choice,1#plugin|2#plugins}: {1}{0,choice,3#...} +sync.notification.restart.message.plugin.disable=Restart the IDE to disable {0,choice,1#plugin|2#plugins}: {1}{0,choice,3#...} +sync.notification.restart.message.registry=Restart the IDE to enable the new UI + +sync.notification.restart.message.list.title=Restart the IDE to complete the following actions: +# {0} - count of plugins, {1} - coma separated list of plugins (max two plugins) +sync.notification.restart.message.list.entry.plugin.install=Install {0,choice,1#plugin|2#plugins}: {1}{0,choice,3#...} +sync.notification.restart.message.list.entry.plugin.enable=Enable {0,choice,1#plugin|2#plugins}: {1}{0,choice,3#...} +sync.notification.restart.message.list.entry.plugin.disable=Disable {0,choice,1#plugin|2#plugins}: {1}{0,choice,3#...} +sync.notification.restart.message.list.entry.registry=Enable the new UI + # {0} - action (install, enable, disable), {1} - comma separated list of plugins sync.restart.notification.submessage.plugins={0} plugin(s): {1}... # {0} - IDE name, i.e. Android Studio, MPS, etc. @@ -85,8 +96,9 @@ sync.status.login.not.available=To enable Setting Sync, install sync.status.restart.required=To enable Setting Sync, restart {0} sync.status.restart.ide.button=Restart -sync.notification.size.exceed.title=Settings pack exceeds maximum allowed limit -sync.notification.size.exceed.text=Please reach out to us at https://youtrack.jetbrains.com/ and attach your logs and settings to address the issue +sync.notification.size.exceed.title=Settings archive exceeds size limit +sync.notification.size.exceed.text=Please reach out to us at YouTrack and attach your logs and settings to address the issue +sync.notification.do.not.ask.again=Don't ask again settings.cross.product.sync=Sync settings across: # {0} is the current product name, e.g. 'IntelliJ IDEA Community Edition' or 'WebStorm' diff --git a/plugins/settings-sync/src/com/intellij/settingsSync/CloudConfigServerCommunicator.kt b/plugins/settings-sync/src/com/intellij/settingsSync/CloudConfigServerCommunicator.kt index 563d69f60194..a9eafeb99abc 100644 --- a/plugins/settings-sync/src/com/intellij/settingsSync/CloudConfigServerCommunicator.kt +++ b/plugins/settings-sync/src/com/intellij/settingsSync/CloudConfigServerCommunicator.kt @@ -189,11 +189,6 @@ internal open class CloudConfigServerCommunicator(serverUrl: String? = null) : S val zip = try { SettingsSnapshotZipSerializer.serializeToZip(snapshot) } - catch (e: SettingsSnapshotZipSerializer.ZipSizeExceedException) { - LOG.warn(e) - NotificationService.getInstance().notifyZipSizeExceed() - return SettingsSyncPushResult.Error(e.message ?: "Couldn't prepare zip file") - } catch (e: Throwable) { LOG.warn(e) return SettingsSyncPushResult.Error(e.message ?: "Couldn't prepare zip file") diff --git a/plugins/settings-sync/src/com/intellij/settingsSync/SettingsSnapshotZipSerializer.kt b/plugins/settings-sync/src/com/intellij/settingsSync/SettingsSnapshotZipSerializer.kt index 8f66ee13e89a..844c8161d2e0 100644 --- a/plugins/settings-sync/src/com/intellij/settingsSync/SettingsSnapshotZipSerializer.kt +++ b/plugins/settings-sync/src/com/intellij/settingsSync/SettingsSnapshotZipSerializer.kt @@ -5,10 +5,12 @@ import com.fasterxml.jackson.databind.ObjectMapper import com.intellij.openapi.diagnostic.logger import com.intellij.openapi.util.BuildNumber import com.intellij.openapi.util.io.FileUtil +import com.intellij.settingsSync.notification.NotificationService import com.intellij.settingsSync.plugins.SettingsSyncPluginsState import com.intellij.util.io.* import kotlinx.serialization.encodeToString import kotlinx.serialization.json.Json +import java.io.File import java.io.OutputStream import java.lang.RuntimeException import java.nio.file.Files @@ -28,7 +30,7 @@ internal object SettingsSnapshotZipSerializer { private const val INFO = "info.json" private const val PLUGINS = "plugins.json" - private const val MAX_ZIP_SIZE = 524288 // bytes + private const val ZIP_SIZE_SOFT_LIMIT = 524288 // bytes private val LOG = logger() @@ -37,8 +39,8 @@ internal object SettingsSnapshotZipSerializer { fun serializeToZip(snapshot: SettingsSnapshot): Path { val file = FileUtil.createTempFile(SETTINGS_SYNC_SNAPSHOT_ZIP, null) serialize(snapshot, Compressor.Zip(file)) - if (file.length() > MAX_ZIP_SIZE) { - throw ZipSizeExceedException(file.length()) + if (file.length() > ZIP_SIZE_SOFT_LIMIT) { + NotificationService.getInstance().notifyZipSizeExceed() } return file.toPath() } @@ -189,10 +191,4 @@ internal object SettingsSnapshotZipSerializer { var configFolder: String = "" var isDeleted: Boolean = false } - - class ZipSizeExceedException(private val size: Long): RuntimeException() { - override fun toString(): String { - return "Zip size $size excesses maximum allowed zip size" - } - } } \ No newline at end of file diff --git a/plugins/settings-sync/src/com/intellij/settingsSync/SettingsSyncEvents.kt b/plugins/settings-sync/src/com/intellij/settingsSync/SettingsSyncEvents.kt index 8765e90aa54f..e56561ff603a 100644 --- a/plugins/settings-sync/src/com/intellij/settingsSync/SettingsSyncEvents.kt +++ b/plugins/settings-sync/src/com/intellij/settingsSync/SettingsSyncEvents.kt @@ -60,51 +60,61 @@ interface SettingsSyncEventListener : EventListener { sealed class RestartReason: Comparable { abstract val sortingPriority: Int - abstract fun getNotificationSubMessage(): String @NlsSafe - fun getSingleReasonNotificationMessage(): String { - return SettingsSyncBundle.message("sync.restart.notification.message", getNotificationSubMessage()) - } + abstract fun getSingleReasonNotificationMessage(): String @NlsSafe - fun getMultiReasonNotificationListEntry(number: Int): String { - return "$number. ${getNotificationSubMessage().capitalize()}\n" - } - - private fun String.capitalize() : String { - return this.replaceFirstChar { if (it.isLowerCase()) it.titlecase(Locale.getDefault()) else it.toString() } - } + abstract fun getMultiReasonNotificationListEntry(number: Int): String override fun compareTo(other: RestartReason): Int { return this.sortingPriority.compareTo(other.sortingPriority) } } -internal class RestartForPluginInstall(private val plugins: Collection) : RestartReason() { +internal class RestartForPluginInstall(val plugins: Collection) : RestartReason() { override val sortingPriority = 0 - override fun getNotificationSubMessage(): String { - return SettingsSyncBundle.message("sync.restart.notification.submessage.plugins", "install", plugins.joinToString(", ")) + + override fun getSingleReasonNotificationMessage(): String { + return SettingsSyncBundle.message("sync.notification.restart.message.plugin.install", plugins.size, plugins.take(2).joinToString(", ")) + } + + override fun getMultiReasonNotificationListEntry(number: Int): String { + return "$number. " + SettingsSyncBundle.message("sync.notification.restart.message.list.entry.plugin.install", plugins.size, plugins.take(2).joinToString(", ")) } } -internal class RestartForPluginEnable(private val plugins: Collection) : RestartReason() { +internal class RestartForPluginEnable(val plugins: Collection) : RestartReason() { override val sortingPriority = 1 - override fun getNotificationSubMessage(): String { - return SettingsSyncBundle.message("sync.restart.notification.submessage.plugins", "enable", plugins.joinToString(", ")) + + override fun getSingleReasonNotificationMessage(): String { + return SettingsSyncBundle.message("sync.notification.restart.message.plugin.enable", plugins.size, plugins.take(2).joinToString(", ")) + } + + override fun getMultiReasonNotificationListEntry(number: Int): String { + return "$number. " + SettingsSyncBundle.message("sync.notification.restart.message.list.entry.plugin.enable", plugins.size, plugins.take(2).joinToString(", ")) } } -internal class RestartForPluginDisable(private val plugins: Collection) : RestartReason() { +internal class RestartForPluginDisable(val plugins: Collection) : RestartReason() { override val sortingPriority = 2 - override fun getNotificationSubMessage(): String { - return SettingsSyncBundle.message("sync.restart.notification.submessage.plugins", "disable", plugins.joinToString(", ")) + + override fun getSingleReasonNotificationMessage(): String { + return SettingsSyncBundle.message("sync.notification.restart.message.plugin.disable", plugins.size, plugins.take(2).joinToString(", ")) + } + + override fun getMultiReasonNotificationListEntry(number: Int): String { + return "$number. " + SettingsSyncBundle.message("sync.notification.restart.message.list.entry.plugin.disable", plugins.size, plugins.take(2).joinToString(", ")) } } internal object RestartForNewUI : RestartReason() { override val sortingPriority = 3 - override fun getNotificationSubMessage(): String { - return SettingsSyncBundle.message("sync.restart.notification.submessage.registry") + override fun getSingleReasonNotificationMessage(): String { + return SettingsSyncBundle.message("sync.notification.restart.message.registry") + } + + override fun getMultiReasonNotificationListEntry(number: Int): String { + return "$number. " + SettingsSyncBundle.message("sync.notification.restart.message.list.entry.registry") } } diff --git a/plugins/settings-sync/src/com/intellij/settingsSync/SettingsSyncIdeMediatorImpl.kt b/plugins/settings-sync/src/com/intellij/settingsSync/SettingsSyncIdeMediatorImpl.kt index 3a57df112bca..eed472d37799 100644 --- a/plugins/settings-sync/src/com/intellij/settingsSync/SettingsSyncIdeMediatorImpl.kt +++ b/plugins/settings-sync/src/com/intellij/settingsSync/SettingsSyncIdeMediatorImpl.kt @@ -96,8 +96,21 @@ internal class SettingsSyncIdeMediatorImpl(private val componentStore: Component } private fun notifyRestartNeeded() { - if (restartRequiredReasons.isEmpty()) return - NotificationService.getInstance().notifyRestartNeeded(restartRequiredReasons) + val mergedReasons = mergeRestartReasons() + if (mergedReasons.isEmpty()) return + NotificationService.getInstance().notifyRestartNeeded(mergedReasons) + } + + private fun mergeRestartReasons(): List { + return restartRequiredReasons.groupBy { it::class.java }.mapNotNull { (clazz, reasons) -> + when (clazz) { + RestartForPluginInstall::class.java -> RestartForPluginInstall(reasons.flatMap { (it as RestartForPluginInstall).plugins }) + RestartForPluginEnable::class.java -> RestartForPluginEnable(reasons.flatMap { (it as RestartForPluginEnable).plugins }) + RestartForPluginDisable::class.java -> RestartForPluginDisable(reasons.flatMap { (it as RestartForPluginDisable).plugins }) + RestartForNewUI::class.java -> RestartForNewUI + else -> null + } + } } override fun activateStreamProvider() { diff --git a/plugins/settings-sync/src/com/intellij/settingsSync/notification/NotificationService.kt b/plugins/settings-sync/src/com/intellij/settingsSync/notification/NotificationService.kt index 38774fb54f47..5a7121524572 100644 --- a/plugins/settings-sync/src/com/intellij/settingsSync/notification/NotificationService.kt +++ b/plugins/settings-sync/src/com/intellij/settingsSync/notification/NotificationService.kt @@ -1,6 +1,5 @@ package com.intellij.settingsSync.notification -import com.intellij.notification.Notification import com.intellij.openapi.components.service import com.intellij.settingsSync.RestartReason @@ -10,7 +9,5 @@ internal interface NotificationService { } fun notifyZipSizeExceed() - fun buildZipSizeExceedNotification(): Notification fun notifyRestartNeeded(reasons: Collection) - fun buildRestartNeededNotification(reasons: Collection): Notification } \ No newline at end of file diff --git a/plugins/settings-sync/src/com/intellij/settingsSync/notification/NotificationServiceImpl.kt b/plugins/settings-sync/src/com/intellij/settingsSync/notification/NotificationServiceImpl.kt index 747fef4c67d0..433d67a6310a 100644 --- a/plugins/settings-sync/src/com/intellij/settingsSync/notification/NotificationServiceImpl.kt +++ b/plugins/settings-sync/src/com/intellij/settingsSync/notification/NotificationServiceImpl.kt @@ -1,5 +1,6 @@ package com.intellij.settingsSync.notification +import com.intellij.ide.util.propComponentProperty import com.intellij.notification.Notification import com.intellij.notification.NotificationAction import com.intellij.notification.NotificationGroupManager @@ -10,19 +11,26 @@ import com.intellij.openapi.application.ex.ApplicationEx import com.intellij.settingsSync.NOTIFICATION_GROUP import com.intellij.settingsSync.RestartReason import com.intellij.settingsSync.SettingsSyncBundle -import java.lang.RuntimeException internal class NotificationServiceImpl: NotificationService { override fun notifyZipSizeExceed() { - val notification = buildZipSizeExceedNotification() + val notification = buildZipSizeExceedNotification() ?: return notification.notify(null) } - override fun buildZipSizeExceedNotification(): Notification { - return NotificationGroupManager.getInstance().getNotificationGroup(NOTIFICATION_GROUP) + private fun buildZipSizeExceedNotification(): Notification? { + var showNotification: Boolean by propComponentProperty(null, "sync.notification.zip.size.exceed.show", defaultValue = true) + if (!showNotification) return null + val notification = NotificationGroupManager.getInstance().getNotificationGroup(NOTIFICATION_GROUP) .createNotification(SettingsSyncBundle.message("sync.notification.size.exceed.title"), SettingsSyncBundle.message("sync.notification.size.exceed.text"), NotificationType.ERROR) + notification.addAction( + NotificationAction.createSimpleExpiring(SettingsSyncBundle.message("sync.notification.do.not.ask.again")) { + showNotification = false + } + ) + return notification } override fun notifyRestartNeeded(reasons: Collection) { @@ -36,22 +44,21 @@ internal class NotificationServiceImpl: NotificationService { notification.notify(null) } - override fun buildRestartNeededNotification(reasons: Collection): Notification { + private fun buildRestartNeededNotification(reasons: Collection): Notification { fun getMultiReasonRestartMessage(): String { - assert(reasons.size > 1) - val message = StringBuilder(SettingsSyncBundle.message("sync.restart.notification.message.subtitle")).append('\n') + val message = StringBuilder(SettingsSyncBundle.message("sync.notification.restart.message.list.title")).append("
") val sortedRestartReasons = reasons.sorted() for ((counter, reason) in sortedRestartReasons.withIndex()) { message.append(reason.getMultiReasonNotificationListEntry(counter + 1)) + if (counter < sortedRestartReasons.lastIndex) message.append("
") } - message.dropLast(0) // we do not need the new line in the end return message.toString() } val message = when { - reasons.isEmpty() -> throw RuntimeException("No restart reasons provided") + reasons.isEmpty() -> "" reasons.size == 1 -> reasons.first().getSingleReasonNotificationMessage() else -> getMultiReasonRestartMessage() }