Skip to content
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
d8b6caf
always fetch from endpoint during poll
samgst-amazon Dec 3, 2024
02f335b
notification resource resolver
samgst-amazon Dec 3, 2024
54c7d02
ResourceResolver etag handling
samgst-amazon Dec 5, 2024
d96aa00
Merge branch 'main' into samgst/NotificationResourceResolver
samgst-amazon Dec 5, 2024
4231ba5
codescan
samgst-amazon Dec 5, 2024
ba0c57b
Merge branch 'main' into samgst/NotificationResourceResolver
samgst-amazon Dec 5, 2024
08c6001
remove redundant modifier
samgst-amazon Dec 5, 2024
de5c8be
Merge branch 'main' into samgst/NotificationResourceResolver
samgst-amazon Dec 5, 2024
3bcc9ea
Merge branch 'main' into samgst/NotificationResourceResolver
samgst-amazon Dec 6, 2024
4b80f54
move functionality to DefaultRemoteResourceResolver
samgst-amazon Dec 6, 2024
0aa2074
implement defaultRemoteResourceResolverProvider instead
samgst-amazon Dec 6, 2024
d18b689
url deprecated
samgst-amazon Dec 6, 2024
bc9396d
detekt
samgst-amazon Dec 9, 2024
ca8e967
Merge branch 'main' into samgst/NotificationResourceResolver
samgst-amazon Dec 9, 2024
5dcd476
detektTest fix mock functions
samgst-amazon Dec 9, 2024
c78a536
detekt
samgst-amazon Dec 9, 2024
12ffed4
Merge branch 'main' into samgst/NotificationResourceResolver
samgst-amazon Dec 9, 2024
0ffa31e
Merge branch 'main' into samgst/NotificationResourceResolver
samgst-amazon Dec 9, 2024
0d992c7
default function in interface
samgst-amazon Dec 9, 2024
9d4a1bb
re-implement HTTPRequest
samgst-amazon Dec 9, 2024
b2b9849
re-implement HTTPRequest
samgst-amazon Dec 9, 2024
af54b0f
ETag fix
samgst-amazon Dec 10, 2024
b289ce1
LazyLogRule
samgst-amazon Dec 10, 2024
794364d
Merge branch 'main' into samgst/NotificationResourceResolver
samgst-amazon Dec 10, 2024
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 @@ -5,6 +5,8 @@

import java.io.FileInputStream
import java.io.InputStream
import java.net.HttpURLConnection
import java.net.URI
import java.nio.file.Files
import java.nio.file.Path
import java.nio.file.StandardCopyOption
Expand All @@ -13,30 +15,62 @@
import java.util.UUID
import java.util.concurrent.Callable
import java.util.concurrent.CompletionStage
import java.util.concurrent.atomic.AtomicBoolean

interface RemoteResourceResolver {
fun resolve(resource: RemoteResource): CompletionStage<Path>
fun checkForUpdates(endpoint: String, eTagProvider: ETagProvider): UpdateCheckResult
fun getLocalResourcePath(filename: String): Path?
}
interface RemoteResolveParser {
fun canBeParsed(data: InputStream): Boolean
}

interface ETagProvider {
var etag: String?
fun updateETag(newETag: String?)
}

sealed class UpdateCheckResult {
data object HasUpdates : UpdateCheckResult()
data object NoUpdates : UpdateCheckResult()
data object FirstPollCheck : UpdateCheckResult()
}

class DefaultRemoteResourceResolver(
private val urlFetcher: UrlFetcher,
private val cacheBasePath: Path,
private val executor: (Callable<Path>) -> CompletionStage<Path>,
) : RemoteResourceResolver {
private val isFirstPoll = AtomicBoolean(true)

override fun resolve(resource: RemoteResource): CompletionStage<Path> = executor(Callable { internalResolve(resource) })

override fun getLocalResourcePath(filename: String): Path? {
val expectedLocation = cacheBasePath.resolve(filename)
return expectedLocation.existsOrNull()
}

override fun checkForUpdates(endpoint: String, eTagProvider: ETagProvider): UpdateCheckResult {
val hasETagUpdate = updateETags(eTagProvider, endpoint)
// for when we need to notify on first poll even when there's no new ETag
Copy link
Contributor

Choose a reason for hiding this comment

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

it would make more sense to see if ETagProvider has a value set to determine if this were the first request

if (isFirstPoll.compareAndSet(true, false) && !hasETagUpdate) {
return UpdateCheckResult.FirstPollCheck
}

return if (hasETagUpdate) {
UpdateCheckResult.HasUpdates
} else {
UpdateCheckResult.NoUpdates
}
}

private fun internalResolve(resource: RemoteResource): Path {
val expectedLocation = cacheBasePath.resolve(resource.name)
val current = expectedLocation.existsOrNull()
if (resource.name != "notifications.json") {
if ((current != null && !isExpired(current, resource))) {
LOG.debug { "Existing file ($current) for ${resource.name} is present and not expired - using it." }
return current
}
if (current != null && !isExpired(current, resource)) {
LOG.debug { "Existing file ($current) for ${resource.name} is present and not expired - using it." }
return current
}

LOG.debug { "Current file for ${resource.name} does not exist or is expired. Attempting to fetch from ${resource.urls}" }
Expand Down Expand Up @@ -84,6 +118,30 @@
return expectedLocation
}

private fun updateETags(eTagProvider: ETagProvider, endpoint: String): Boolean {
val currentEtag = eTagProvider.etag
val remoteEtag = getEndpointETag(endpoint)
eTagProvider.etag = remoteEtag
return currentEtag != remoteEtag
}

private fun getEndpointETag(endpoint: String): String =
try {
val url = URI(endpoint).toURL()
(url.openConnection() as HttpURLConnection).let { connection ->
Copy link
Contributor

Choose a reason for hiding this comment

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

java 11 has a newer way of doing this, but let's actually use the HttpRequests util you were using in a previous PR

connection.requestMethod = "HEAD"
connection.setRequestProperty("User-Agent", "AWS Toolkit for JetBrains")
connection.connect()

val eTag = connection.getHeaderField("ETag").orEmpty()
connection.disconnect()
eTag
}
} catch (e: Exception) {
LOG.warn { "Failed to fetch notification ETag: ${e.message}" }

Check warning on line 141 in plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt

View check run for this annotation

Codecov / codecov/patch

plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt#L140-L141

Added lines #L140 - L141 were not covered by tests
throw e
}

private companion object {
val LOG = getLogger<RemoteResourceResolver>()
fun Path.existsOrNull() = if (this.exists()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,11 @@
import com.intellij.openapi.util.registry.Registry
import com.intellij.util.Alarm
import com.intellij.util.AlarmFactory
import com.intellij.util.io.HttpRequests
import kotlinx.coroutines.delay
import kotlinx.coroutines.runBlocking
import software.aws.toolkits.core.utils.RemoteResolveParser
import software.aws.toolkits.core.utils.RemoteResource
import software.aws.toolkits.core.utils.error
import software.aws.toolkits.core.utils.UpdateCheckResult
import software.aws.toolkits.core.utils.getLogger
import software.aws.toolkits.core.utils.info
import software.aws.toolkits.core.utils.warn
Expand All @@ -25,10 +24,10 @@
import software.aws.toolkits.telemetry.ToolkitTelemetry
import java.io.InputStream
import java.time.Duration
import java.util.concurrent.atomic.AtomicBoolean

private const val MAX_RETRIES = 3
private const val RETRY_DELAY_MS = 1000L
internal const val FILENAME = "notifications.json"

object NotificationFileValidator : RemoteResolveParser {
override fun canBeParsed(data: InputStream): Boolean =
Expand All @@ -47,20 +46,20 @@

@Service(Service.Level.APP)
internal final class NotificationPollingService : Disposable {
private val isFirstPoll = AtomicBoolean(true)
private val observers = mutableListOf<() -> Unit>()
private val alarm = AlarmFactory.getInstance().create(Alarm.ThreadToUse.POOLED_THREAD, this)
private val pollingIntervalMs = Duration.ofMinutes(10).toMillis()
private val resourceResolver: RemoteResourceResolverProvider = DefaultRemoteResourceResolverProvider()
private val notificationsResource = object : RemoteResource {
override val name: String = "notifications.json"
override val name: String = FILENAME
override val urls: List<String> = listOf(NotificationEndpoint.getEndpoint())
override val remoteResolveParser: RemoteResolveParser = NotificationFileValidator
override val ttl: Duration = Duration.ofMillis(1)
// ttl forces resolver to fetch from endpoint every time
}

fun startPolling() {
val newNotifications = runBlocking { pollForNotifications() }
isFirstPoll.set(false)
if (newNotifications) {
notifyObservers()
}
Expand All @@ -70,37 +69,38 @@
)
}

/**
* Main polling function that checks for updates and downloads if necessary
* Returns the parsed notifications if successful, null otherwise
*/
private suspend fun pollForNotifications(): Boolean {
var retryCount = 0
var lastException: Exception? = null

while (retryCount < MAX_RETRIES) {
LOG.info { "Polling for notifications" }
try {
val newETag = getNotificationETag()
if (newETag == NotificationEtagState.getInstance().etag) {
// for when we need to notify on first poll even when there's no new ETag
if (isFirstPoll.compareAndSet(true, false)) {
when (
resourceResolver.get().checkForUpdates(
NotificationEndpoint.getEndpoint(),
NotificationEtagState.getInstance()
)
) {
is UpdateCheckResult.HasUpdates -> {
resourceResolver.get()
.resolve(notificationsResource)
.toCompletableFuture()
.get()
LOG.info { "New notifications fetched" }
return true
}
is UpdateCheckResult.FirstPollCheck -> {
LOG.info { "No new notifications, checking cached notifications on first poll" }
return true
}
LOG.info { "No new notifications to fetch" }
return false
is UpdateCheckResult.NoUpdates -> {
LOG.info { "No new notifications to fetch" }
return false
}
}
resourceResolver.get()
.resolve(notificationsResource)
.toCompletableFuture()
.get()
NotificationEtagState.getInstance().etag = newETag
LOG.info { "New notifications fetched" }
return true
} catch (e: Exception) {
lastException = e
LOG.error(e) { "Failed to poll for notifications (attempt ${retryCount + 1}/$MAX_RETRIES)" }
LOG.warn { "Failed to poll for notifications (attempt ${retryCount + 1}/$MAX_RETRIES)" }

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

View check run for this annotation

Codecov / codecov/patch

plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingService.kt#L103

Added line #L103 was not covered by tests
retryCount++
if (retryCount < MAX_RETRIES) {
val backoffDelay = RETRY_DELAY_MS * (1L shl (retryCount - 1))
Expand All @@ -112,18 +112,6 @@
return false
}

private fun getNotificationETag(): String =
try {
HttpRequests.request(NotificationEndpoint.getEndpoint())
.userAgent("AWS Toolkit for JetBrains")
.connect { request ->
request.connection.headerFields["ETag"]?.firstOrNull().orEmpty()
}
} catch (e: Exception) {
LOG.warn { "Failed to fetch notification ETag: $e.message" }
throw e
}

private fun emitFailureMetric(e: Exception?) {
ToolkitTelemetry.showNotification(
project = null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
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

@Service
@State(name = "notificationDismissals", storages = [Storage("aws.xml", roamingType = RoamingType.DISABLED)])
Expand Down Expand Up @@ -41,16 +42,20 @@

@Service
@State(name = "notificationEtag", storages = [Storage("aws.xml", roamingType = RoamingType.DISABLED)])
class NotificationEtagState : PersistentStateComponent<NotificationEtagConfiguration> {
class NotificationEtagState : PersistentStateComponent<NotificationEtagConfiguration>, ETagProvider {
private val state = NotificationEtagConfiguration()

override fun updateETag(newETag: String?) {
etag = newETag
}

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

View check run for this annotation

Codecov / codecov/patch

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

Added lines #L49 - L50 were not covered by tests

override fun getState(): NotificationEtagConfiguration = state

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

var etag: String?
override var etag: String?
get() = state.etag
set(value) {
state.etag = value
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,15 @@
import com.fasterxml.jackson.module.kotlin.readValue
import com.intellij.notification.NotificationType
import com.intellij.openapi.actionSystem.AnAction
import com.intellij.openapi.application.PathManager
import com.intellij.openapi.components.Service
import com.intellij.openapi.components.service
import com.intellij.openapi.project.Project
import software.aws.toolkits.core.utils.getLogger
import software.aws.toolkits.core.utils.info
import software.aws.toolkits.core.utils.inputStream
import software.aws.toolkits.core.utils.warn
import software.aws.toolkits.jetbrains.core.RemoteResourceResolverProvider
import software.aws.toolkits.jetbrains.utils.notifyStickyWithData
import java.nio.file.Paths
import java.util.concurrent.atomic.AtomicBoolean

object NotificationMapperUtil {
Expand All @@ -40,7 +39,14 @@

private fun getNotificationsFromFile(): NotificationsList? {
try {
val path = Paths.get(PathManager.getSystemPath(), NOTIFICATIONS_PATH)
val path = RemoteResourceResolverProvider
.getInstance()
.get()
.getLocalResourcePath(FILENAME)

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

View check run for this annotation

Codecov / codecov/patch

plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/ProcessNotificationsBase.kt#L42-L45

Added lines #L42 - L45 were not covered by tests
if (path == null) {
LOG.warn { "Notifications file not found" }
return null

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

View check run for this annotation

Codecov / codecov/patch

plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/ProcessNotificationsBase.kt#L47-L48

Added lines #L47 - L48 were not covered by tests
}
val content = path.inputStream().bufferedReader().use { it.readText() }
if (content.isEmpty()) {
return null
Expand Down Expand Up @@ -119,8 +125,6 @@
companion object {
private val LOG = getLogger<ProcessNotificationsBase>()
fun getInstance(project: Project): ProcessNotificationsBase = project.service()

private const val NOTIFICATIONS_PATH = "aws-static-resources/notifications.json"
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,20 @@
package software.aws.toolkits.jetbrains.core.notifications

import com.intellij.testFramework.ApplicationExtension
import com.intellij.util.io.HttpRequests
import io.mockk.Runs
import io.mockk.every
import io.mockk.just
import io.mockk.mockk
import io.mockk.mockkStatic
import io.mockk.verify
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.ExtendWith
import software.aws.toolkits.core.utils.RemoteResourceResolver
import software.aws.toolkits.core.utils.UpdateCheckResult
import software.aws.toolkits.jetbrains.core.RemoteResourceResolverProvider
import java.nio.file.Path
import java.util.concurrent.CompletableFuture
import java.util.concurrent.atomic.AtomicBoolean

@ExtendWith(ApplicationExtension::class)
class NotificationPollingServiceTest {
Expand Down Expand Up @@ -64,48 +62,22 @@ class NotificationPollingServiceTest {

@Test
fun `test pollForNotifications when ETag matches - no new notifications`() {
NotificationEtagState.getInstance().etag = "same"
val firstPollField = NotificationPollingService::class.java
.getDeclaredField("isFirstPoll")
.apply { isAccessible = true }
firstPollField.set(sut, AtomicBoolean(false))

mockkStatic(HttpRequests::class) {
every {
HttpRequests.request(any<String>())
.userAgent(any())
.connect<String>(any())
} returns "same"
sut.startPolling()
}
every { mockResolver.checkForUpdates(any(), any()) } returns UpdateCheckResult.NoUpdates
sut.startPolling()
verify(exactly = 0) { observer.invoke() }
}

@Test
fun `test pollForNotifications when ETag matches on startup - notify observers`() {
NotificationEtagState.getInstance().etag = "same"
mockkStatic(HttpRequests::class) {
every {
HttpRequests.request(any<String>())
.userAgent(any())
.connect<String>(any())
} returns "same"
sut.startPolling()
}
every { mockResolver.checkForUpdates(any(), any()) } returns UpdateCheckResult.FirstPollCheck
sut.startPolling()
verify(exactly = 1) { observer.invoke() }
}

@Test
fun `test pollForNotifications when ETag different - notify observers`() {
NotificationEtagState.getInstance().etag = "oldETag"
mockkStatic(HttpRequests::class) {
every {
HttpRequests.request(any<String>())
.userAgent(any())
.connect<String>(any())
} returns "newEtag"
sut.startPolling()
}
every { mockResolver.checkForUpdates(any(), any()) } returns UpdateCheckResult.HasUpdates
sut.startPolling()
verify(exactly = 1) { observer.invoke() }
}
}
Loading
Loading