Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ class NotificationPanel : BorderLayoutPanel() {
}

private fun removeNotificationPanel(notificationId: String) = runInEdt {
ProcessNotificationsBase.showBannerNotification.remove(notificationId) // TODO: add id to dismissed notification list
ProcessNotificationsBase.showBannerNotification.remove(notificationId)
NotificationDismissalState.getInstance().dismissNotification(notificationId)
wrapper.removeAll()
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

package software.aws.toolkits.jetbrains.core.notifications

import com.intellij.openapi.application.ApplicationManager
import com.intellij.openapi.components.PersistentStateComponent
import com.intellij.openapi.components.State
import com.intellij.openapi.components.Storage

@State(name = "notificationDismissals", storages = [Storage("aws.xml")])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider using BaseState

class NotificationDismissalState : PersistentStateComponent<NotificationDismissalConfiguration> {

Check warning on line 12 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationPersistantState.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Extension class should be final and non-public

Service implementation should not be public. If a service is supposed to be used outside its module, extract an interface from it and specify it as serviceInterface in plugin.xml.
private var state = NotificationDismissalConfiguration()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

val


override fun getState(): NotificationDismissalConfiguration = state

override fun loadState(state: NotificationDismissalConfiguration) {
this.state = state
}

fun isDismissed(notificationId: String): Boolean =
state.dismissedNotificationIds.contains(notificationId)

fun dismissNotification(notificationId: String) {
state.dismissedNotificationIds.add(notificationId)
}

companion object {
fun getInstance(): NotificationDismissalState =
ApplicationManager.getApplication().getService(NotificationDismissalState::class.java)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fun getInstance(): NotificationDismissalState =
ApplicationManager.getApplication().getService(NotificationDismissalState::class.java)
fun getInstance(): NotificationDismissalState =
ApplicationManager.getApplication().getService<NotificationDismissalState>()

}
}

data class NotificationDismissalConfiguration(
var dismissedNotificationIds: MutableSet<String> = mutableSetOf(),
)


@State(name = "notificationEtag", storages = [Storage("aws.xml")])
class NotificationEtagState : PersistentStateComponent<NotificationEtagConfiguration> {

Check warning on line 40 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationPersistantState.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Extension class should be final and non-public

Service implementation should not be public. If a service is supposed to be used outside its module, extract an interface from it and specify it as serviceInterface in plugin.xml.
private var state = NotificationEtagConfiguration()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

val


override fun getState(): NotificationEtagConfiguration = state

override fun loadState(state: NotificationEtagConfiguration) {
this.state = state
}

var etag: String?
get() = state.etag
set(value) {
state.etag = value
}

companion object {
fun getInstance(): NotificationEtagState =
ApplicationManager.getApplication().getService(NotificationEtagState::class.java)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fun getInstance(): NotificationEtagState =
ApplicationManager.getApplication().getService(NotificationEtagState::class.java)
fun getInstance(): NotificationEtagState =
ApplicationManager.getApplication().getService<NotificationEtagState>()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm getting a syntax error when I do this. I think this is for newer versions of Intellij SDK. I don't see any other usages of the bottom code in our repository, but I do see the ::class.java version.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you try using getInstance(): NotificationETagState = service()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that works!

}
}

data class NotificationEtagConfiguration(
var etag: String? = null,
)
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
import com.fasterxml.jackson.module.kotlin.readValue
import com.intellij.openapi.Disposable
import com.intellij.openapi.application.ApplicationManager
import com.intellij.openapi.components.PersistentStateComponent

Check warning on line 9 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingService.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Unused import directive

Unused import directive
import com.intellij.openapi.components.Service
import com.intellij.openapi.components.State

Check warning on line 11 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingService.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Unused import directive

Unused import directive
import com.intellij.openapi.components.Storage

Check warning on line 12 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingService.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Unused import directive

Unused import directive
import com.intellij.util.Alarm
import com.intellij.util.AlarmFactory
import com.intellij.util.io.HttpRequests
Expand All @@ -27,7 +27,7 @@
import software.aws.toolkits.jetbrains.core.RemoteResourceResolverProvider
import software.aws.toolkits.jetbrains.core.coroutines.getCoroutineBgContext
import software.aws.toolkits.telemetry.Component
import software.aws.toolkits.telemetry.ToolkitTelemetry

Check warning on line 30 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingService.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Usage of redundant or deprecated syntax or deprecated symbols

'ToolkitTelemetry' is deprecated. Use type-safe metric builders

Check warning on line 30 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingService.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Usage of redundant or deprecated syntax or deprecated symbols

Remove deprecated symbol import
import java.io.InputStream
import java.time.Duration
import java.util.concurrent.atomic.AtomicBoolean
Expand All @@ -51,46 +51,20 @@
fun getEndpoint(): String = overriddenEndpoint ?: DEFAULT_ENDPOINT

@VisibleForTesting
fun setTestEndpoint(endpoint: String) {

Check warning on line 54 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingService.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Unused symbol

Function "setTestEndpoint" is never used
overriddenEndpoint = endpoint
}

@VisibleForTesting
fun resetEndpoint() {

Check warning on line 59 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingService.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Unused symbol

Function "resetEndpoint" is never used
overriddenEndpoint = null
}

private const val DEFAULT_ENDPOINT = "" // TODO: Replace with actual endpoint
}

@State(name = "notificationEtag", storages = [Storage("aws.xml")])
class NotificationEtagState : PersistentStateComponent<NotificationEtagConfiguration> {
private var state = NotificationEtagConfiguration()

override fun getState(): NotificationEtagConfiguration = state

override fun loadState(state: NotificationEtagConfiguration) {
this.state = state
}

var etag: String?
get() = state.etag
set(value) {
state.etag = value
}

companion object {
fun getInstance(): NotificationEtagState =
ApplicationManager.getApplication().getService(NotificationEtagState::class.java)
}
}

data class NotificationEtagConfiguration(
var etag: String? = null,
)

@Service(Service.Level.APP)
internal final class NotificationPollingService : Disposable {

Check warning on line 67 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingService.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Redundant modality modifier

Redundant modality modifier
private val isFirstPoll = AtomicBoolean(true)
private val isStartup = AtomicBoolean(true)
private val observers = mutableListOf<(Boolean) -> Unit>()
Expand Down Expand Up @@ -168,7 +142,7 @@
}

private fun emitFailureMetric(e: Exception?) {
ToolkitTelemetry.showNotification(

Check warning on line 145 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingService.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Usage of redundant or deprecated syntax or deprecated symbols

'ToolkitTelemetry' is deprecated. Use type-safe metric builders
project = null,
component = Component.Filesystem,
id = "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ internal class NotificationServiceInitializer : ProjectActivity {
override suspend fun execute(project: Project) {
if (initialized.compareAndSet(false, true)) {
val service = NotificationPollingService.getInstance()
ProcessNotificationsBase()
ProcessNotificationsBase(project)
service.startPolling()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@
import java.nio.file.Paths

object NotificationMapperUtil {
val mapper = jacksonObjectMapper().configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)

Check notice on line 20 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/ProcessNotificationsBase.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Function or property has platform type

Declaration has type inferred from a platform call, which can lead to unchecked nullability issues. Specify type explicitly as nullable or non-nullable.
}

@Service(Service.Level.PROJECT)
class ProcessNotificationsBase {
class ProcessNotificationsBase(
private val project: Project,
) {
private val notifListener = mutableListOf<NotifListener>()
init {
NotificationPollingService.getInstance().addObserver { isStartup ->
Expand All @@ -38,12 +40,25 @@
return NotificationMapperUtil.mapper.readValue(content)
}

fun retrieveStartupAndEmergencyNotifications(isStartup: Boolean) {

Check notice on line 43 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/ProcessNotificationsBase.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Class member can have 'private' visibility

Function 'retrieveStartupAndEmergencyNotifications' could be private
// TODO: separates notifications into startup and emergency
// iterates through the 2 lists and processes each notification(if it isn't dismissed)
val notifications = getNotificationsFromFile()

notifications?.let { notificationsList ->
val (startupNotifications, emergencyNotifications) = notificationsList.notifications
?.partition { notification ->
notification.schedule.type.equals("StartUp", ignoreCase = true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be modeled in the deserializer

}
?: Pair(emptyList(), emptyList())

val dismissalState = NotificationDismissalState.getInstance()
(if (isStartup) startupNotifications else emptyList())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this part handle the fact that startup notifications should be shown only once per session?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we move the handling into separate list handling for readability

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes and yes, I was passing the boolean in from the polling service but I think the flag can live here instead.

.plus(emergencyNotifications)
.filter { !dismissalState.isDismissed(it.id) }
.forEach { processNotification(project, it) }
}
}

fun processNotification(project: Project, notificationData: NotificationData) {

Check notice on line 61 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/ProcessNotificationsBase.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Class member can have 'private' visibility

Function 'processNotification' could be private
val shouldShow = RulesEngine.displayNotification(project, notificationData)
if (shouldShow) {
val notificationContent = notificationData.content.locale
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import com.intellij.ui.ScrollPaneFactory
import org.slf4j.LoggerFactory
import software.aws.toolkits.core.utils.warn
import software.aws.toolkits.jetbrains.core.help.HelpIds
import software.aws.toolkits.jetbrains.core.notifications.NotificationDismissalState
import software.aws.toolkits.jetbrains.core.notifications.ProcessNotificationsBase
import software.aws.toolkits.resources.AwsCoreBundle
import javax.swing.JLabel
Expand Down Expand Up @@ -67,7 +68,7 @@ fun notifyStickyWithData(
object : AnAction("Dismiss") {
override fun actionPerformed(e: AnActionEvent) {
ProcessNotificationsBase.showBannerNotification.remove(id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

state should not be stored in companion objects

// TODO: add id to dismissed notification list
NotificationDismissalState.getInstance().dismissNotification(id)
}
}
)
Expand Down
1 change: 1 addition & 0 deletions plugins/core/src/main/resources/META-INF/plugin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
<extensions defaultExtensionNs="com.intellij">
<!-- each plugin needs its own instance of these -->
<applicationService serviceImplementation="software.aws.toolkits.jetbrains.core.notifications.NotificationEtagState"/>
<applicationService serviceImplementation="software.aws.toolkits.jetbrains.core.notifications.NotificationDismissalState"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these can be light services

<applicationService serviceImplementation="migration.software.aws.toolkits.jetbrains.core.coroutines.PluginCoroutineScopeTracker"/>
<projectService serviceImplementation="migration.software.aws.toolkits.jetbrains.core.coroutines.PluginCoroutineScopeTracker"/>
<postStartupActivity implementation = "software.aws.toolkits.jetbrains.core.notifications.NotificationServiceInitializer"/>
Expand Down
Loading