Set expiry for dismissed notifications (#5186)

* dismiss notification expiry

* test

* fix tests

* redundant cleanExpired function calls

* converter

* detekt

* remove whitespace from conflict

* detekt

* remove attribute tags, unnecessary clear

* replace attribute tags

* change properties to var for serialization

* simplify getState

* detekt

* reduntant test

Author: samgst-amazon

plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationStateUtils.kt

@@ -9,36 +9,54 @@ import com.intellij.openapi.components.State
 import com.intellij.openapi.components.Storage
 import com.intellij.openapi.components.service
 import software.aws.toolkits.core.utils.ETagProvider
+import java.time.Duration
+import java.time.Instant
+
+data class DismissedNotification(
+    var id: String = "",
+    var dismissedAt: String = Instant.now().toEpochMilli().toString(),
+)
+
+data class NotificationDismissalConfiguration(
+    var dismissedNotifications: MutableSet<DismissedNotification> = mutableSetOf(),
+)
 
 @Service
 @State(name = "notificationDismissals", storages = [Storage("aws.xml")])
 class NotificationDismissalState : PersistentStateComponent<NotificationDismissalConfiguration> {
-    private val state = NotificationDismissalConfiguration()
+    private var state = NotificationDismissalConfiguration()
+    private val retentionPeriod = Duration.ofDays(60) // 2 months
 
     override fun getState(): NotificationDismissalConfiguration = state
 
     override fun loadState(state: NotificationDismissalConfiguration) {
-        this.state.dismissedNotificationIds.clear()
-        this.state.dismissedNotificationIds.addAll(state.dismissedNotificationIds)
+        this.state = state
+        cleanExpiredNotifications()
     }
 
     fun isDismissed(notificationId: String): Boolean =
-        state.dismissedNotificationIds.contains(notificationId)
+        state.dismissedNotifications.any { it.id == notificationId }
 
     fun dismissNotification(notificationId: String) {
-        state.dismissedNotificationIds.add(notificationId)
+        state.dismissedNotifications.add(
+            DismissedNotification(
+                id = notificationId
+            )
+        )
+    }
+
+    private fun cleanExpiredNotifications() {
+        val now = Instant.now()
+        state.dismissedNotifications.removeAll { notification ->
+            Duration.between(Instant.ofEpochMilli(notification.dismissedAt.toLong()), now) > retentionPeriod
+        }
     }
 
     companion object {
-        fun getInstance(): NotificationDismissalState =
-            service()
+        fun getInstance(): NotificationDismissalState = service()
     }
 }
 
-data class NotificationDismissalConfiguration(
-    var dismissedNotificationIds: MutableSet<String> = mutableSetOf(),
-)
-
 @Service
 @State(name = "notificationEtag", storages = [Storage("aws.xml")])
 class NotificationEtagState : PersistentStateComponent<NotificationEtagConfiguration>, ETagProvider {

plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationDismissalStateTest.kt

@@ -0,0 +1,83 @@
+// 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 org.junit.jupiter.api.Assertions.assertEquals
+import org.junit.jupiter.api.Assertions.assertFalse
+import org.junit.jupiter.api.Assertions.assertTrue
+import org.junit.jupiter.api.BeforeEach
+import org.junit.jupiter.api.Test
+import java.time.Instant
+import java.time.temporal.ChronoUnit
+
+class NotificationDismissalStateTest {
+    private lateinit var state: NotificationDismissalState
+
+    @BeforeEach
+    fun setUp() {
+        state = NotificationDismissalState()
+    }
+
+    @Test
+    fun `notifications less than 2 months old are not removed`() {
+        val recentNotification = DismissedNotification(
+            id = "recent-notification",
+            dismissedAt = Instant.now().minus(30, ChronoUnit.DAYS).toEpochMilli().toString()
+        )
+
+        state.loadState(NotificationDismissalConfiguration(mutableSetOf(recentNotification)))
+
+        val persistedState = state.getState()
+
+        assertEquals(1, persistedState.dismissedNotifications.size)
+        assertTrue(persistedState.dismissedNotifications.any { it.id == "recent-notification" })
+        assertTrue(state.isDismissed("recent-notification"))
+    }
+
+    @Test
+    fun `notifications older than 2 months are removed`() {
+        val oldNotification = DismissedNotification(
+            id = "old-notification",
+            dismissedAt = Instant.now().minus(61, ChronoUnit.DAYS).toEpochMilli().toString()
+        )
+
+        state.loadState(NotificationDismissalConfiguration(mutableSetOf(oldNotification)))
+
+        val persistedState = state.getState()
+
+        assertEquals(0, persistedState.dismissedNotifications.size)
+        assertFalse(state.isDismissed("old-notification"))
+    }
+
+    @Test
+    fun `mixed age notifications are handled correctly`() {
+        val recentNotification = DismissedNotification(
+            id = "recent-notification",
+            dismissedAt = Instant.now().minus(30, ChronoUnit.DAYS).toEpochMilli().toString()
+        )
+        val oldNotification = DismissedNotification(
+            id = "old-notification",
+            dismissedAt = Instant.now().minus(61, ChronoUnit.DAYS).toEpochMilli().toString()
+        )
+
+        state.loadState(
+            NotificationDismissalConfiguration(
+                mutableSetOf(recentNotification, oldNotification)
+            )
+        )
+
+        val persistedState = state.getState()
+
+        assertEquals(1, persistedState.dismissedNotifications.size)
+        assertTrue(state.isDismissed("recent-notification"))
+        assertFalse(state.isDismissed("old-notification"))
+    }
+
+    @Test
+    fun `dismissing new notification retains it`() {
+        state.dismissNotification("new-notification")
+
+        assertTrue(state.isDismissed("new-notification"))
+    }
+}