From d8b6caf8414a414c74c5c983ba242cc541b909f2 Mon Sep 17 00:00:00 2001 From: samgst-amazon Date: Tue, 3 Dec 2024 12:22:28 -0800 Subject: [PATCH 01/16] always fetch from endpoint during poll --- .../aws/toolkits/core/utils/RemoteResourceResolver.kt | 8 +++----- .../core/notifications/NotificationPollingService.kt | 2 ++ 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt b/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt index c4efe685884..ce0603bf0fe 100644 --- a/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt +++ b/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt @@ -32,11 +32,9 @@ class DefaultRemoteResourceResolver( 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}" } diff --git a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingService.kt b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingService.kt index a52a9eb5c1c..5d8a5e70048 100644 --- a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingService.kt +++ b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingService.kt @@ -56,6 +56,8 @@ internal final class NotificationPollingService : Disposable { override val name: String = "notifications.json" override val urls: List = 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() { From 02f335bb39c6e26abef83a9b5683fc932076138e Mon Sep 17 00:00:00 2001 From: samgst-amazon Date: Tue, 3 Dec 2024 14:20:38 -0800 Subject: [PATCH 02/16] notification resource resolver --- .../NotificationResourceResolver.kt | 127 ++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolver.kt diff --git a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolver.kt b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolver.kt new file mode 100644 index 00000000000..2bcf0c340de --- /dev/null +++ b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolver.kt @@ -0,0 +1,127 @@ +// 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 software.aws.toolkits.core.utils.DefaultRemoteResourceResolver +import software.aws.toolkits.core.utils.RemoteResource +import software.aws.toolkits.core.utils.RemoteResourceResolver +import software.aws.toolkits.jetbrains.core.saveFileFromUrl +import software.aws.toolkits.core.utils.exists +import java.nio.file.Path +import java.util.concurrent.Callable +import java.util.concurrent.CompletionStage +import software.aws.toolkits.core.utils.getLogger +import software.aws.toolkits.core.utils.info +import software.aws.toolkits.core.utils.warn +import com.intellij.util.io.HttpRequests +import com.intellij.openapi.application.PathManager +import com.intellij.util.io.createDirectories +import software.aws.toolkits.core.utils.UrlFetcher +import software.aws.toolkits.jetbrains.core.RemoteResourceResolverProvider +import software.aws.toolkits.jetbrains.utils.pluginAwareExecuteOnPooledThread +import java.nio.file.Paths +import java.util.concurrent.CompletableFuture + +interface NotificationRemoteResourceResolverProvider { + fun get(): NotificationResourceResolver + + companion object { + fun getInstance(): NotificationRemoteResourceResolverProvider = service() + } +} + +class DefaultNotificationRemoteResourceResolverProvider { + override fun get() = RESOLVER_INSTANCE + + companion object { + private val RESOLVER_INSTANCE by lazy { + val cachePath = Paths.get(PathManager.getSystemPath(), "aws-notifications").createDirectories() + + NotificationResourceResolver( + urlFetcher = HttpRequestUrlFetcher, + cacheBasePath = cachePath, + executor = { callable -> + val future = CompletableFuture() + pluginAwareExecuteOnPooledThread { + try { + future.complete(callable.call()) + } catch (e: Exception) { + future.completeExceptionally(e) + } + } + future + } + ) + } + + object HttpRequestUrlFetcher : UrlFetcher { + override fun fetch(url: String, file: Path) { + saveFileFromUrl(url, file) + } + } + } +} + + +class NotificationResourceResolver( + private val urlFetcher: UrlFetcher, + private val cacheBasePath: Path, + private val executor: (Callable) -> CompletionStage, + private val etagState: NotificationEtagState = NotificationEtagState.getInstance() +) : RemoteResourceResolver { + private val delegate = DefaultRemoteResourceResolver(urlFetcher, cacheBasePath, executor) + + fun getLocalResourcePath(resourceName: String): Path? { + val expectedLocation = cacheBasePath.resolve(resourceName) + return expectedLocation.existsOrNull() + } + + override fun resolve(resource: RemoteResource): CompletionStage { + return executor(Callable { internalResolve(resource) }) + } + + private fun internalResolve(resource: RemoteResource): Path { + val expectedLocation = cacheBasePath.resolve(resource.name) + val current = expectedLocation.existsOrNull() + + if (current != null) { + val currentEtag = etagState.etag + try { + val remoteEtag = getEndpointETag() + if (currentEtag == remoteEtag) { + LOG.info { "Existing file ($current) matches remote etag - using cached version" } + return current + } + } catch (e: Exception) { + LOG.warn(e) { "Failed to check remote etag, using cached version if available" } + return current + } + } + + // Use delegate for download logic + return delegate.resolve(resource).toCompletableFuture().get() + } + + private fun getEndpointETag(): 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 + } + + companion object { + private val LOG = getLogger() + fun Path.existsOrNull() = if (this.exists()) { + this + } else { + null + } + + } +} From 54c7d02c04999d48425e95e414af7be41da66504 Mon Sep 17 00:00:00 2001 From: samgst-amazon Date: Wed, 4 Dec 2024 17:16:38 -0800 Subject: [PATCH 03/16] ResourceResolver etag handling --- .../resources/META-INF/aws.toolkit.core.xml | 3 + .../NotificationPollingService.kt | 55 +++-------- .../NotificationResourceResolver.kt | 99 ++++++++++--------- .../notifications/ProcessNotificationsBase.kt | 25 +++-- .../NotificationPollingServiceTest.kt | 51 ++-------- .../NotificationResourceResolverTest.kt | 97 ++++++++++++++++++ 6 files changed, 193 insertions(+), 137 deletions(-) create mode 100644 plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolverTest.kt diff --git a/plugins/core/jetbrains-community/resources/META-INF/aws.toolkit.core.xml b/plugins/core/jetbrains-community/resources/META-INF/aws.toolkit.core.xml index ec52967c5b7..1701ee90493 100644 --- a/plugins/core/jetbrains-community/resources/META-INF/aws.toolkit.core.xml +++ b/plugins/core/jetbrains-community/resources/META-INF/aws.toolkit.core.xml @@ -36,6 +36,9 @@ testServiceImplementation="software.aws.toolkits.jetbrains.core.region.MockRegionProvider"/> + + 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 resourceResolver: NotificationResourceResolverProvider = DefaultNotificationResourceResolverProvider() private val notificationsResource = object : RemoteResource { override val name: String = "notifications.json" override val urls: List = listOf(NotificationEndpoint.getEndpoint()) @@ -62,7 +56,6 @@ internal final class NotificationPollingService : Disposable { fun startPolling() { val newNotifications = runBlocking { pollForNotifications() } - isFirstPoll.set(false) if (newNotifications) { notifyObservers() } @@ -72,37 +65,33 @@ internal final class NotificationPollingService : Disposable { ) } - /** - * 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()) { + 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)" } retryCount++ if (retryCount < MAX_RETRIES) { val backoffDelay = RETRY_DELAY_MS * (1L shl (retryCount - 1)) @@ -114,18 +103,6 @@ internal final class NotificationPollingService : Disposable { 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, diff --git a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolver.kt b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolver.kt index 2bcf0c340de..cc9d8a48dd8 100644 --- a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolver.kt +++ b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolver.kt @@ -3,56 +3,52 @@ package software.aws.toolkits.jetbrains.core.notifications +import com.intellij.openapi.application.PathManager +import com.intellij.openapi.components.service +import com.intellij.util.io.HttpRequests +import com.intellij.util.io.createDirectories import software.aws.toolkits.core.utils.DefaultRemoteResourceResolver import software.aws.toolkits.core.utils.RemoteResource import software.aws.toolkits.core.utils.RemoteResourceResolver -import software.aws.toolkits.jetbrains.core.saveFileFromUrl +import software.aws.toolkits.core.utils.UrlFetcher import software.aws.toolkits.core.utils.exists -import java.nio.file.Path -import java.util.concurrent.Callable -import java.util.concurrent.CompletionStage import software.aws.toolkits.core.utils.getLogger -import software.aws.toolkits.core.utils.info import software.aws.toolkits.core.utils.warn -import com.intellij.util.io.HttpRequests -import com.intellij.openapi.application.PathManager -import com.intellij.util.io.createDirectories -import software.aws.toolkits.core.utils.UrlFetcher -import software.aws.toolkits.jetbrains.core.RemoteResourceResolverProvider +import software.aws.toolkits.jetbrains.core.saveFileFromUrl import software.aws.toolkits.jetbrains.utils.pluginAwareExecuteOnPooledThread +import java.nio.file.Path import java.nio.file.Paths +import java.util.concurrent.Callable import java.util.concurrent.CompletableFuture +import java.util.concurrent.CompletionStage +import java.util.concurrent.atomic.AtomicBoolean -interface NotificationRemoteResourceResolverProvider { +interface NotificationResourceResolverProvider { fun get(): NotificationResourceResolver companion object { - fun getInstance(): NotificationRemoteResourceResolverProvider = service() + fun getInstance(): NotificationResourceResolverProvider = service() } } -class DefaultNotificationRemoteResourceResolverProvider { +class DefaultNotificationResourceResolverProvider : NotificationResourceResolverProvider { override fun get() = RESOLVER_INSTANCE companion object { private val RESOLVER_INSTANCE by lazy { val cachePath = Paths.get(PathManager.getSystemPath(), "aws-notifications").createDirectories() - NotificationResourceResolver( - urlFetcher = HttpRequestUrlFetcher, - cacheBasePath = cachePath, - executor = { callable -> - val future = CompletableFuture() - pluginAwareExecuteOnPooledThread { - try { - future.complete(callable.call()) - } catch (e: Exception) { - future.completeExceptionally(e) - } + NotificationResourceResolver(HttpRequestUrlFetcher, cachePath) { + val future = CompletableFuture() + pluginAwareExecuteOnPooledThread { + try { + future.complete(it.call()) + } catch (e: Exception) { + future.completeExceptionally(e) } - future } - ) + future + } } object HttpRequestUrlFetcher : UrlFetcher { @@ -63,44 +59,50 @@ class DefaultNotificationRemoteResourceResolverProvider { } } +sealed class UpdateCheckResult { + object HasUpdates : UpdateCheckResult() + object NoUpdates : UpdateCheckResult() + object FirstPollCheck : UpdateCheckResult() +} class NotificationResourceResolver( private val urlFetcher: UrlFetcher, private val cacheBasePath: Path, private val executor: (Callable) -> CompletionStage, - private val etagState: NotificationEtagState = NotificationEtagState.getInstance() ) : RemoteResourceResolver { private val delegate = DefaultRemoteResourceResolver(urlFetcher, cacheBasePath, executor) + private val etagState: NotificationEtagState = NotificationEtagState.getInstance() + private val isFirstPoll = AtomicBoolean(true) fun getLocalResourcePath(resourceName: String): Path? { val expectedLocation = cacheBasePath.resolve(resourceName) return expectedLocation.existsOrNull() } - override fun resolve(resource: RemoteResource): CompletionStage { - return executor(Callable { internalResolve(resource) }) - } + fun checkForUpdates(): UpdateCheckResult { + val hasETagUpdate = updateETags() - private fun internalResolve(resource: RemoteResource): Path { - val expectedLocation = cacheBasePath.resolve(resource.name) - val current = expectedLocation.existsOrNull() - - if (current != null) { - val currentEtag = etagState.etag - try { - val remoteEtag = getEndpointETag() - if (currentEtag == remoteEtag) { - LOG.info { "Existing file ($current) matches remote etag - using cached version" } - return current - } - } catch (e: Exception) { - LOG.warn(e) { "Failed to check remote etag, using cached version if available" } - return current - } + // for when we need to notify on first poll even when there's no new ETag + if (isFirstPoll.compareAndSet(true, false) && !hasETagUpdate) { + return UpdateCheckResult.FirstPollCheck } - // Use delegate for download logic - return delegate.resolve(resource).toCompletableFuture().get() + return if (hasETagUpdate) { + UpdateCheckResult.HasUpdates + } else { + UpdateCheckResult.NoUpdates + } + } + + fun updateETags(): Boolean { + val currentEtag = etagState.etag + val remoteEtag = getEndpointETag() + etagState.etag = remoteEtag + return currentEtag != remoteEtag + } + + override fun resolve(resource: RemoteResource): CompletionStage { + return delegate.resolve(resource) } private fun getEndpointETag(): String = @@ -122,6 +124,5 @@ class NotificationResourceResolver( } else { null } - } } diff --git a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/ProcessNotificationsBase.kt b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/ProcessNotificationsBase.kt index 324a70e3a0f..a012591bb9b 100644 --- a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/ProcessNotificationsBase.kt +++ b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/ProcessNotificationsBase.kt @@ -9,15 +9,14 @@ import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper 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.utils.notifyStickyWithData -import java.nio.file.Paths import java.util.concurrent.atomic.AtomicBoolean object NotificationMapperUtil { @@ -38,12 +37,24 @@ class ProcessNotificationsBase( } private fun getNotificationsFromFile(): NotificationsList? { - val path = Paths.get(PathManager.getSystemPath(), NOTIFICATIONS_PATH) - val content = path.inputStream().bufferedReader().use { it.readText() } - if (content.isEmpty()) { + try { + val path = NotificationResourceResolverProvider + .getInstance() + .get() + .getLocalResourcePath("notifications.json") + if (path == null) { + LOG.warn { "Notifications file not found" } + return null + } + val content = path.inputStream().bufferedReader().use { it.readText() } + if (content.isEmpty()) { + return null + } + return NotificationMapperUtil.mapper.readValue(content) + } catch (e: Exception) { + LOG.warn { "Error reading notifications file: $e" } return null } - return NotificationMapperUtil.mapper.readValue(content) } fun retrieveStartupAndEmergencyNotifications() { @@ -113,8 +124,6 @@ class ProcessNotificationsBase( companion object { private val LOG = getLogger() fun getInstance(project: Project): ProcessNotificationsBase = project.service() - - private const val NOTIFICATIONS_PATH = "aws-static-resources/notifications.json" } } diff --git a/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingServiceTest.kt b/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingServiceTest.kt index 3e16c4dd28e..0f171fdf396 100644 --- a/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingServiceTest.kt +++ b/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingServiceTest.kt @@ -4,28 +4,23 @@ 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.jetbrains.core.RemoteResourceResolverProvider import java.nio.file.Path import java.util.concurrent.CompletableFuture -import java.util.concurrent.atomic.AtomicBoolean @ExtendWith(ApplicationExtension::class) class NotificationPollingServiceTest { private lateinit var sut: NotificationPollingService - private lateinit var mockResolver: RemoteResourceResolver - private lateinit var mockProvider: RemoteResourceResolverProvider + private lateinit var mockResolver: NotificationResourceResolver + private lateinit var mockProvider: NotificationResourceResolverProvider private lateinit var observer: () -> Unit private val testPath = Path.of("/test/path") @@ -33,11 +28,11 @@ class NotificationPollingServiceTest { fun setUp() { sut = NotificationPollingService() - mockResolver = mockk { + mockResolver = mockk { every { resolve(any()) } returns CompletableFuture.completedFuture(testPath) } - mockProvider = mockk { + mockProvider = mockk { every { get() } returns mockResolver } @@ -64,48 +59,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()) - .userAgent(any()) - .connect(any()) - } returns "same" - sut.startPolling() - } + every { mockResolver.checkForUpdates() } 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()) - .userAgent(any()) - .connect(any()) - } returns "same" - sut.startPolling() - } + every { mockResolver.checkForUpdates() } 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()) - .userAgent(any()) - .connect(any()) - } returns "newEtag" - sut.startPolling() - } + every { mockResolver.checkForUpdates() } returns UpdateCheckResult.HasUpdates + sut.startPolling() verify(exactly = 1) { observer.invoke() } } } diff --git a/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolverTest.kt b/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolverTest.kt new file mode 100644 index 00000000000..fb92adb692f --- /dev/null +++ b/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolverTest.kt @@ -0,0 +1,97 @@ +// 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.testFramework.ApplicationExtension +import com.intellij.util.io.HttpRequests +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkStatic +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.ExtendWith +import org.junit.jupiter.api.io.TempDir +import software.aws.toolkits.core.utils.UrlFetcher +import java.nio.file.Path +import java.util.concurrent.Callable +import java.util.concurrent.CompletableFuture + +@ExtendWith(ApplicationExtension::class) +class NotificationResourceResolverTest { + private lateinit var urlFetcher: UrlFetcher + private lateinit var sut: NotificationResourceResolver + + @TempDir + lateinit var tempDir: Path + + @BeforeEach + fun setUp() { + urlFetcher = mockk() + sut = NotificationResourceResolver( + urlFetcher = urlFetcher, + cacheBasePath = tempDir, + executor = { callable: Callable -> CompletableFuture.completedFuture(callable.call()) } + ) + } + + @Test + fun `first poll with no ETag changes returns FirstPollCheck`() { + NotificationEtagState.getInstance().etag = "same-etag" + mockkStatic(HttpRequests::class) { + every { + HttpRequests.request(any()) + .userAgent(any()) + .connect(any()) + } returns "same-etag" + + val result = sut.checkForUpdates() + assertThat(result).isEqualTo(UpdateCheckResult.FirstPollCheck) + + // Second call should not return FirstPollCheck + val secondResult = sut.checkForUpdates() + assertThat(secondResult).isEqualTo(UpdateCheckResult.NoUpdates) + } + } + + @Test + fun `ETag changes returns HasUpdates`() { + NotificationEtagState.getInstance().etag = "old-etag" + mockkStatic(HttpRequests::class) { + every { + HttpRequests.request(any()) + .userAgent(any()) + .connect(any()) + } returns "new-etag" + + val result = sut.checkForUpdates() + assertThat(result).isEqualTo(UpdateCheckResult.HasUpdates) + } + } + + @Test + fun `no ETag changes returns NoUpdates after first poll`() { + NotificationEtagState.getInstance().etag = "same-etag" + mockkStatic(HttpRequests::class) { + every { + HttpRequests.request(any()) + .userAgent(any()) + .connect(any()) + } returns "same-etag" + + // sets isFirstPoll to false + val firstResult = sut.checkForUpdates() + assertThat(firstResult).isEqualTo(UpdateCheckResult.FirstPollCheck) + + val secondResult = sut.checkForUpdates() + assertThat(secondResult).isEqualTo(UpdateCheckResult.NoUpdates) + } + } + + @Test + fun `getLocalResourcePath returns null for non-existent file`() { + val result = sut.getLocalResourcePath("non-existent-file") + assertThat(result).isNull() + } +} From 4231ba5204b0571bfb2b1efa71290a1bc7f0598a Mon Sep 17 00:00:00 2001 From: samgst-amazon Date: Thu, 5 Dec 2024 09:57:04 -0800 Subject: [PATCH 04/16] codescan --- .../NotificationResourceResolver.kt | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolver.kt b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolver.kt index cc9d8a48dd8..2a15693c28f 100644 --- a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolver.kt +++ b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolver.kt @@ -31,7 +31,7 @@ interface NotificationResourceResolverProvider { } } -class DefaultNotificationResourceResolverProvider : NotificationResourceResolverProvider { +internal final class DefaultNotificationResourceResolverProvider : NotificationResourceResolverProvider { override fun get() = RESOLVER_INSTANCE companion object { @@ -60,9 +60,9 @@ class DefaultNotificationResourceResolverProvider : NotificationResourceResolver } sealed class UpdateCheckResult { - object HasUpdates : UpdateCheckResult() - object NoUpdates : UpdateCheckResult() - object FirstPollCheck : UpdateCheckResult() + data object HasUpdates : UpdateCheckResult() + data object NoUpdates : UpdateCheckResult() + data object FirstPollCheck : UpdateCheckResult() } class NotificationResourceResolver( @@ -74,6 +74,9 @@ class NotificationResourceResolver( private val etagState: NotificationEtagState = NotificationEtagState.getInstance() private val isFirstPoll = AtomicBoolean(true) + override fun resolve(resource: RemoteResource): CompletionStage = + delegate.resolve(resource) + fun getLocalResourcePath(resourceName: String): Path? { val expectedLocation = cacheBasePath.resolve(resourceName) return expectedLocation.existsOrNull() @@ -94,17 +97,13 @@ class NotificationResourceResolver( } } - fun updateETags(): Boolean { + private fun updateETags(): Boolean { val currentEtag = etagState.etag val remoteEtag = getEndpointETag() etagState.etag = remoteEtag return currentEtag != remoteEtag } - override fun resolve(resource: RemoteResource): CompletionStage { - return delegate.resolve(resource) - } - private fun getEndpointETag(): String = try { HttpRequests.request(NotificationEndpoint.getEndpoint()) From 08c6001dff7370bb43cf7f691b3ef1093a7431d3 Mon Sep 17 00:00:00 2001 From: samgst-amazon Date: Thu, 5 Dec 2024 11:00:37 -0800 Subject: [PATCH 05/16] remove redundant modifier --- .../core/notifications/NotificationResourceResolver.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolver.kt b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolver.kt index 2a15693c28f..3a8f937bed9 100644 --- a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolver.kt +++ b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolver.kt @@ -31,7 +31,7 @@ interface NotificationResourceResolverProvider { } } -internal final class DefaultNotificationResourceResolverProvider : NotificationResourceResolverProvider { +internal class DefaultNotificationResourceResolverProvider : NotificationResourceResolverProvider { override fun get() = RESOLVER_INSTANCE companion object { From 4b80f54320a3f1ebb567550db640340e3d73c607 Mon Sep 17 00:00:00 2001 From: samgst-amazon Date: Fri, 6 Dec 2024 10:13:21 -0800 Subject: [PATCH 06/16] move functionality to DefaultRemoteResourceResolver --- .../core/utils/RemoteResourceResolver.kt | 52 +++++++++++++++++++ .../resources/META-INF/aws.toolkit.core.xml | 3 -- .../notifications/NotificationStateUtils.kt | 9 +++- 3 files changed, 59 insertions(+), 5 deletions(-) diff --git a/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt b/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt index ce0603bf0fe..d01cc081dd0 100644 --- a/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt +++ b/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt @@ -3,6 +3,7 @@ package software.aws.toolkits.core.utils +import com.intellij.util.io.HttpRequests import java.io.FileInputStream import java.io.InputStream import java.nio.file.Files @@ -13,6 +14,7 @@ import java.time.Instant 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 @@ -21,14 +23,45 @@ 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) -> CompletionStage, ) : RemoteResourceResolver { + private val isFirstPoll = AtomicBoolean(true) override fun resolve(resource: RemoteResource): CompletionStage = executor(Callable { internalResolve(resource) }) + fun getLocalResourcePath(resourceName: String): Path? { + val expectedLocation = cacheBasePath.resolve(resourceName) + return expectedLocation.existsOrNull() + } + + 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 + 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() @@ -82,6 +115,25 @@ class DefaultRemoteResourceResolver( 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 { + HttpRequests.request(endpoint) + .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 companion object { val LOG = getLogger() fun Path.existsOrNull() = if (this.exists()) { diff --git a/plugins/core/jetbrains-community/resources/META-INF/aws.toolkit.core.xml b/plugins/core/jetbrains-community/resources/META-INF/aws.toolkit.core.xml index 1701ee90493..ec52967c5b7 100644 --- a/plugins/core/jetbrains-community/resources/META-INF/aws.toolkit.core.xml +++ b/plugins/core/jetbrains-community/resources/META-INF/aws.toolkit.core.xml @@ -36,9 +36,6 @@ testServiceImplementation="software.aws.toolkits.jetbrains.core.region.MockRegionProvider"/> - - { +class NotificationEtagState : PersistentStateComponent, ETagProvider { private val state = NotificationEtagConfiguration() + override fun updateETag(newETag: String?) { + etag = newETag + } + 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 From 0aa207474d30d8ec10032a95ad7e30b95cb94739 Mon Sep 17 00:00:00 2001 From: samgst-amazon Date: Fri, 6 Dec 2024 15:05:05 -0800 Subject: [PATCH 07/16] implement defaultRemoteResourceResolverProvider instead --- .../core/utils/RemoteResourceResolver.kt | 28 ++-- .../NotificationPollingService.kt | 15 ++- .../NotificationResourceResolver.kt | 127 ------------------ .../notifications/ProcessNotificationsBase.kt | 5 +- .../NotificationPollingServiceTest.kt | 17 ++- .../NotificationResourceResolverTest.kt | 96 +++++++------ 6 files changed, 99 insertions(+), 189 deletions(-) delete mode 100644 plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolver.kt diff --git a/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt b/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt index d01cc081dd0..5d0a15ad26d 100644 --- a/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt +++ b/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt @@ -3,9 +3,10 @@ package software.aws.toolkits.core.utils -import com.intellij.util.io.HttpRequests import java.io.FileInputStream import java.io.InputStream +import java.net.HttpURLConnection +import java.net.URL import java.nio.file.Files import java.nio.file.Path import java.nio.file.StandardCopyOption @@ -18,6 +19,8 @@ import java.util.concurrent.atomic.AtomicBoolean interface RemoteResourceResolver { fun resolve(resource: RemoteResource): CompletionStage + fun checkForUpdates(endpoint: String, eTagProvider: ETagProvider): UpdateCheckResult + fun getLocalResourcePath(filename: String): Path? } interface RemoteResolveParser { fun canBeParsed(data: InputStream): Boolean @@ -43,12 +46,12 @@ class DefaultRemoteResourceResolver( override fun resolve(resource: RemoteResource): CompletionStage = executor(Callable { internalResolve(resource) }) - fun getLocalResourcePath(resourceName: String): Path? { - val expectedLocation = cacheBasePath.resolve(resourceName) + override fun getLocalResourcePath(filename: String): Path? { + val expectedLocation = cacheBasePath.resolve(filename) return expectedLocation.existsOrNull() } - fun checkForUpdates(endpoint: String, eTagProvider: ETagProvider): UpdateCheckResult { + 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 if (isFirstPoll.compareAndSet(true, false) && !hasETagUpdate) { @@ -124,13 +127,18 @@ class DefaultRemoteResourceResolver( private fun getEndpointETag(endpoint: String): String = try { - HttpRequests.request(endpoint) - .userAgent("AWS Toolkit for JetBrains") - .connect { request -> - request.connection.headerFields["ETag"]?.firstOrNull().orEmpty() - } + val url = URL(endpoint) + (url.openConnection() as HttpURLConnection).let { connection -> + connection.requestMethod = "HEAD" + connection.setRequestProperty("User-Agent", "AWS Toolkit for JetBrains") + connection.connect() + + val eTag = connection.getHeaderField("ETag") ?: "" + connection.disconnect() + eTag + } } catch (e: Exception) { - LOG.warn { "Failed to fetch notification ETag: $e.message" } + LOG.warn { "Failed to fetch notification ETag: ${e.message}" } throw e } diff --git a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingService.kt b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingService.kt index bb56d8fcd3d..2709aa180ab 100644 --- a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingService.kt +++ b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingService.kt @@ -14,9 +14,12 @@ 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.UpdateCheckResult import software.aws.toolkits.core.utils.getLogger import software.aws.toolkits.core.utils.info import software.aws.toolkits.core.utils.warn +import software.aws.toolkits.jetbrains.core.DefaultRemoteResourceResolverProvider +import software.aws.toolkits.jetbrains.core.RemoteResourceResolverProvider import software.aws.toolkits.telemetry.Component import software.aws.toolkits.telemetry.ToolkitTelemetry import java.io.InputStream @@ -24,6 +27,7 @@ import java.time.Duration 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 = @@ -45,9 +49,9 @@ internal final class NotificationPollingService : Disposable { 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: NotificationResourceResolverProvider = DefaultNotificationResourceResolverProvider() + 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 = listOf(NotificationEndpoint.getEndpoint()) override val remoteResolveParser: RemoteResolveParser = NotificationFileValidator override val ttl: Duration = Duration.ofMillis(1) @@ -71,7 +75,12 @@ internal final class NotificationPollingService : Disposable { while (retryCount < MAX_RETRIES) { LOG.info { "Polling for notifications" } try { - when (resourceResolver.get().checkForUpdates()) { + when ( + resourceResolver.get().checkForUpdates( + NotificationEndpoint.getEndpoint(), + NotificationEtagState.getInstance() + ) + ) { is UpdateCheckResult.HasUpdates -> { resourceResolver.get() .resolve(notificationsResource) diff --git a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolver.kt b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolver.kt deleted file mode 100644 index 3a8f937bed9..00000000000 --- a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolver.kt +++ /dev/null @@ -1,127 +0,0 @@ -// 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.PathManager -import com.intellij.openapi.components.service -import com.intellij.util.io.HttpRequests -import com.intellij.util.io.createDirectories -import software.aws.toolkits.core.utils.DefaultRemoteResourceResolver -import software.aws.toolkits.core.utils.RemoteResource -import software.aws.toolkits.core.utils.RemoteResourceResolver -import software.aws.toolkits.core.utils.UrlFetcher -import software.aws.toolkits.core.utils.exists -import software.aws.toolkits.core.utils.getLogger -import software.aws.toolkits.core.utils.warn -import software.aws.toolkits.jetbrains.core.saveFileFromUrl -import software.aws.toolkits.jetbrains.utils.pluginAwareExecuteOnPooledThread -import java.nio.file.Path -import java.nio.file.Paths -import java.util.concurrent.Callable -import java.util.concurrent.CompletableFuture -import java.util.concurrent.CompletionStage -import java.util.concurrent.atomic.AtomicBoolean - -interface NotificationResourceResolverProvider { - fun get(): NotificationResourceResolver - - companion object { - fun getInstance(): NotificationResourceResolverProvider = service() - } -} - -internal class DefaultNotificationResourceResolverProvider : NotificationResourceResolverProvider { - override fun get() = RESOLVER_INSTANCE - - companion object { - private val RESOLVER_INSTANCE by lazy { - val cachePath = Paths.get(PathManager.getSystemPath(), "aws-notifications").createDirectories() - - NotificationResourceResolver(HttpRequestUrlFetcher, cachePath) { - val future = CompletableFuture() - pluginAwareExecuteOnPooledThread { - try { - future.complete(it.call()) - } catch (e: Exception) { - future.completeExceptionally(e) - } - } - future - } - } - - object HttpRequestUrlFetcher : UrlFetcher { - override fun fetch(url: String, file: Path) { - saveFileFromUrl(url, file) - } - } - } -} - -sealed class UpdateCheckResult { - data object HasUpdates : UpdateCheckResult() - data object NoUpdates : UpdateCheckResult() - data object FirstPollCheck : UpdateCheckResult() -} - -class NotificationResourceResolver( - private val urlFetcher: UrlFetcher, - private val cacheBasePath: Path, - private val executor: (Callable) -> CompletionStage, -) : RemoteResourceResolver { - private val delegate = DefaultRemoteResourceResolver(urlFetcher, cacheBasePath, executor) - private val etagState: NotificationEtagState = NotificationEtagState.getInstance() - private val isFirstPoll = AtomicBoolean(true) - - override fun resolve(resource: RemoteResource): CompletionStage = - delegate.resolve(resource) - - fun getLocalResourcePath(resourceName: String): Path? { - val expectedLocation = cacheBasePath.resolve(resourceName) - return expectedLocation.existsOrNull() - } - - fun checkForUpdates(): UpdateCheckResult { - val hasETagUpdate = updateETags() - - // for when we need to notify on first poll even when there's no new ETag - if (isFirstPoll.compareAndSet(true, false) && !hasETagUpdate) { - return UpdateCheckResult.FirstPollCheck - } - - return if (hasETagUpdate) { - UpdateCheckResult.HasUpdates - } else { - UpdateCheckResult.NoUpdates - } - } - - private fun updateETags(): Boolean { - val currentEtag = etagState.etag - val remoteEtag = getEndpointETag() - etagState.etag = remoteEtag - return currentEtag != remoteEtag - } - - private fun getEndpointETag(): 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 - } - - companion object { - private val LOG = getLogger() - fun Path.existsOrNull() = if (this.exists()) { - this - } else { - null - } - } -} diff --git a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/ProcessNotificationsBase.kt b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/ProcessNotificationsBase.kt index a012591bb9b..aa0bb5de142 100644 --- a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/ProcessNotificationsBase.kt +++ b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/ProcessNotificationsBase.kt @@ -16,6 +16,7 @@ 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.util.concurrent.atomic.AtomicBoolean @@ -38,10 +39,10 @@ class ProcessNotificationsBase( private fun getNotificationsFromFile(): NotificationsList? { try { - val path = NotificationResourceResolverProvider + val path = RemoteResourceResolverProvider .getInstance() .get() - .getLocalResourcePath("notifications.json") + .getLocalResourcePath(FILENAME) if (path == null) { LOG.warn { "Notifications file not found" } return null diff --git a/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingServiceTest.kt b/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingServiceTest.kt index 0f171fdf396..e4922519c86 100644 --- a/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingServiceTest.kt +++ b/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationPollingServiceTest.kt @@ -13,14 +13,17 @@ 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 @ExtendWith(ApplicationExtension::class) class NotificationPollingServiceTest { private lateinit var sut: NotificationPollingService - private lateinit var mockResolver: NotificationResourceResolver - private lateinit var mockProvider: NotificationResourceResolverProvider + private lateinit var mockResolver: RemoteResourceResolver + private lateinit var mockProvider: RemoteResourceResolverProvider private lateinit var observer: () -> Unit private val testPath = Path.of("/test/path") @@ -28,11 +31,11 @@ class NotificationPollingServiceTest { fun setUp() { sut = NotificationPollingService() - mockResolver = mockk { + mockResolver = mockk { every { resolve(any()) } returns CompletableFuture.completedFuture(testPath) } - mockProvider = mockk { + mockProvider = mockk { every { get() } returns mockResolver } @@ -59,21 +62,21 @@ class NotificationPollingServiceTest { @Test fun `test pollForNotifications when ETag matches - no new notifications`() { - every { mockResolver.checkForUpdates() } returns UpdateCheckResult.NoUpdates + 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`() { - every { mockResolver.checkForUpdates() } returns UpdateCheckResult.FirstPollCheck + every { mockResolver.checkForUpdates(any(), any()) } returns UpdateCheckResult.FirstPollCheck sut.startPolling() verify(exactly = 1) { observer.invoke() } } @Test fun `test pollForNotifications when ETag different - notify observers`() { - every { mockResolver.checkForUpdates() } returns UpdateCheckResult.HasUpdates + every { mockResolver.checkForUpdates(any(), any()) } returns UpdateCheckResult.HasUpdates sut.startPolling() verify(exactly = 1) { observer.invoke() } } diff --git a/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolverTest.kt b/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolverTest.kt index fb92adb692f..abc50dcc222 100644 --- a/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolverTest.kt +++ b/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolverTest.kt @@ -4,24 +4,29 @@ package software.aws.toolkits.jetbrains.core.notifications import com.intellij.testFramework.ApplicationExtension -import com.intellij.util.io.HttpRequests import io.mockk.every +import io.mockk.just import io.mockk.mockk -import io.mockk.mockkStatic +import io.mockk.mockkConstructor +import io.mockk.runs import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.ExtendWith import org.junit.jupiter.api.io.TempDir +import software.aws.toolkits.core.utils.DefaultRemoteResourceResolver +import software.aws.toolkits.core.utils.UpdateCheckResult import software.aws.toolkits.core.utils.UrlFetcher +import java.net.HttpURLConnection +import java.net.URL import java.nio.file.Path import java.util.concurrent.Callable import java.util.concurrent.CompletableFuture @ExtendWith(ApplicationExtension::class) -class NotificationResourceResolverTest { +class RemoteResourceResolverTest { private lateinit var urlFetcher: UrlFetcher - private lateinit var sut: NotificationResourceResolver + private lateinit var sut: DefaultRemoteResourceResolver @TempDir lateinit var tempDir: Path @@ -29,7 +34,7 @@ class NotificationResourceResolverTest { @BeforeEach fun setUp() { urlFetcher = mockk() - sut = NotificationResourceResolver( + sut = DefaultRemoteResourceResolver( urlFetcher = urlFetcher, cacheBasePath = tempDir, executor = { callable: Callable -> CompletableFuture.completedFuture(callable.call()) } @@ -39,54 +44,65 @@ class NotificationResourceResolverTest { @Test fun `first poll with no ETag changes returns FirstPollCheck`() { NotificationEtagState.getInstance().etag = "same-etag" - mockkStatic(HttpRequests::class) { - every { - HttpRequests.request(any()) - .userAgent(any()) - .connect(any()) - } returns "same-etag" - - val result = sut.checkForUpdates() - assertThat(result).isEqualTo(UpdateCheckResult.FirstPollCheck) - - // Second call should not return FirstPollCheck - val secondResult = sut.checkForUpdates() - assertThat(secondResult).isEqualTo(UpdateCheckResult.NoUpdates) + val expectedETag = "same-etag" + val mockConnection = mockk { + every { requestMethod = any() } just runs + every { setRequestProperty(any(), any()) } just runs + every { connect() } just runs + every { getHeaderField("ETag") } returns expectedETag + every { disconnect() } just runs } + + mockkConstructor(URL::class) + every { anyConstructed().openConnection() } returns mockConnection + + val result = sut.checkForUpdates("http://notification.test", NotificationEtagState.getInstance()) + assertThat(result).isEqualTo(UpdateCheckResult.FirstPollCheck) + + // Second call should not return FirstPollCheck + val secondResult = sut.checkForUpdates("http://notification.test", NotificationEtagState.getInstance()) + assertThat(secondResult).isEqualTo(UpdateCheckResult.NoUpdates) } @Test fun `ETag changes returns HasUpdates`() { NotificationEtagState.getInstance().etag = "old-etag" - mockkStatic(HttpRequests::class) { - every { - HttpRequests.request(any()) - .userAgent(any()) - .connect(any()) - } returns "new-etag" - - val result = sut.checkForUpdates() - assertThat(result).isEqualTo(UpdateCheckResult.HasUpdates) + val expectedETag = "new-etag" + val mockConnection = mockk { + every { requestMethod = any() } just runs + every { setRequestProperty(any(), any()) } just runs + every { connect() } just runs + every { getHeaderField("ETag") } returns expectedETag + every { disconnect() } just runs } + + mockkConstructor(URL::class) + every { anyConstructed().openConnection() } returns mockConnection + + val result = sut.checkForUpdates("http://notification.test", NotificationEtagState.getInstance()) + assertThat(result).isEqualTo(UpdateCheckResult.HasUpdates) } @Test fun `no ETag changes returns NoUpdates after first poll`() { NotificationEtagState.getInstance().etag = "same-etag" - mockkStatic(HttpRequests::class) { - every { - HttpRequests.request(any()) - .userAgent(any()) - .connect(any()) - } returns "same-etag" - - // sets isFirstPoll to false - val firstResult = sut.checkForUpdates() - assertThat(firstResult).isEqualTo(UpdateCheckResult.FirstPollCheck) - - val secondResult = sut.checkForUpdates() - assertThat(secondResult).isEqualTo(UpdateCheckResult.NoUpdates) + val expectedETag = "same-etag" + val mockConnection = mockk { + every { requestMethod = any() } just runs + every { setRequestProperty(any(), any()) } just runs + every { connect() } just runs + every { getHeaderField("ETag") } returns expectedETag + every { disconnect() } just runs } + + mockkConstructor(URL::class) + every { anyConstructed().openConnection() } returns mockConnection + // sets isFirstPoll to false + val firstResult = sut.checkForUpdates("http://notification.test", NotificationEtagState.getInstance()) + assertThat(firstResult).isEqualTo(UpdateCheckResult.FirstPollCheck) + + val secondResult = sut.checkForUpdates("http://notification.test", NotificationEtagState.getInstance()) + assertThat(secondResult).isEqualTo(UpdateCheckResult.NoUpdates) } @Test From d18b689bcc846eefa65d502835d39dc8fd0e33af Mon Sep 17 00:00:00 2001 From: samgst-amazon Date: Fri, 6 Dec 2024 15:36:28 -0800 Subject: [PATCH 08/16] url deprecated --- .../aws/toolkits/core/utils/RemoteResourceResolver.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt b/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt index 5d0a15ad26d..0872bcb8af2 100644 --- a/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt +++ b/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt @@ -6,7 +6,7 @@ package software.aws.toolkits.core.utils import java.io.FileInputStream import java.io.InputStream import java.net.HttpURLConnection -import java.net.URL +import java.net.URI import java.nio.file.Files import java.nio.file.Path import java.nio.file.StandardCopyOption @@ -127,13 +127,13 @@ class DefaultRemoteResourceResolver( private fun getEndpointETag(endpoint: String): String = try { - val url = URL(endpoint) + val url = URI(endpoint).toURL() (url.openConnection() as HttpURLConnection).let { connection -> connection.requestMethod = "HEAD" connection.setRequestProperty("User-Agent", "AWS Toolkit for JetBrains") connection.connect() - val eTag = connection.getHeaderField("ETag") ?: "" + val eTag = connection.getHeaderField("ETag").orEmpty() connection.disconnect() eTag } From bc9396d610de8f1ecba77819f651e13b509bc1b0 Mon Sep 17 00:00:00 2001 From: samgst-amazon Date: Mon, 9 Dec 2024 09:32:21 -0800 Subject: [PATCH 09/16] detekt --- .../core/notifications/NotificationResourceResolverTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolverTest.kt b/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolverTest.kt index abc50dcc222..dda8e50c6a4 100644 --- a/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolverTest.kt +++ b/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolverTest.kt @@ -24,7 +24,7 @@ import java.util.concurrent.Callable import java.util.concurrent.CompletableFuture @ExtendWith(ApplicationExtension::class) -class RemoteResourceResolverTest { +class NotificationResourceResolverTest { private lateinit var urlFetcher: UrlFetcher private lateinit var sut: DefaultRemoteResourceResolver From 5dcd476affcef10ec5a6df807eccf8e6d5aeb01e Mon Sep 17 00:00:00 2001 From: samgst-amazon Date: Mon, 9 Dec 2024 10:30:14 -0800 Subject: [PATCH 10/16] detektTest fix mock functions --- .../jetbrains/core/region/AwsRegionProviderTest.kt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/plugins/toolkit/jetbrains-core/tst/software/aws/toolkits/jetbrains/core/region/AwsRegionProviderTest.kt b/plugins/toolkit/jetbrains-core/tst/software/aws/toolkits/jetbrains/core/region/AwsRegionProviderTest.kt index c39d23ad90c..0bbf685d75e 100644 --- a/plugins/toolkit/jetbrains-core/tst/software/aws/toolkits/jetbrains/core/region/AwsRegionProviderTest.kt +++ b/plugins/toolkit/jetbrains-core/tst/software/aws/toolkits/jetbrains/core/region/AwsRegionProviderTest.kt @@ -14,8 +14,10 @@ import org.junit.Rule import org.junit.Test import software.aws.toolkits.core.rules.EnvironmentVariableHelper import software.aws.toolkits.core.rules.SystemPropertyHelper +import software.aws.toolkits.core.utils.ETagProvider import software.aws.toolkits.core.utils.RemoteResource import software.aws.toolkits.core.utils.RemoteResourceResolver +import software.aws.toolkits.core.utils.UpdateCheckResult import software.aws.toolkits.core.utils.exists import software.aws.toolkits.core.utils.writeText import software.aws.toolkits.jetbrains.core.RemoteResourceResolverProvider @@ -165,6 +167,14 @@ class AwsRegionProviderTest { override fun resolve(resource: RemoteResource): CompletionStage = CompletableFuture().apply { complete(file) } + // following two methods are not used in this test, make compiler happy + override fun checkForUpdates(endpoint: String, eTagProvider: ETagProvider): UpdateCheckResult { + return UpdateCheckResult.NoUpdates + } + + override fun getLocalResourcePath(filename: String): Path? { + return null + } } } ApplicationManager.getApplication().replaceService(RemoteResourceResolverProvider::class.java, mockRemoteResource, disposableRule.disposable) From c78a536f30c884b2e72ba6f31c2981b2bf64c488 Mon Sep 17 00:00:00 2001 From: samgst-amazon Date: Mon, 9 Dec 2024 11:30:40 -0800 Subject: [PATCH 11/16] detekt --- .../jetbrains/core/region/AwsRegionProviderTest.kt | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/plugins/toolkit/jetbrains-core/tst/software/aws/toolkits/jetbrains/core/region/AwsRegionProviderTest.kt b/plugins/toolkit/jetbrains-core/tst/software/aws/toolkits/jetbrains/core/region/AwsRegionProviderTest.kt index 0bbf685d75e..a0dc1c3ba6d 100644 --- a/plugins/toolkit/jetbrains-core/tst/software/aws/toolkits/jetbrains/core/region/AwsRegionProviderTest.kt +++ b/plugins/toolkit/jetbrains-core/tst/software/aws/toolkits/jetbrains/core/region/AwsRegionProviderTest.kt @@ -167,14 +167,13 @@ class AwsRegionProviderTest { override fun resolve(resource: RemoteResource): CompletionStage = CompletableFuture().apply { complete(file) } + // following two methods are not used in this test, make compiler happy - override fun checkForUpdates(endpoint: String, eTagProvider: ETagProvider): UpdateCheckResult { - return UpdateCheckResult.NoUpdates - } + override fun checkForUpdates(endpoint: String, eTagProvider: ETagProvider): UpdateCheckResult = + UpdateCheckResult.NoUpdates - override fun getLocalResourcePath(filename: String): Path? { - return null - } + override fun getLocalResourcePath(filename: String): Path? = + null } } ApplicationManager.getApplication().replaceService(RemoteResourceResolverProvider::class.java, mockRemoteResource, disposableRule.disposable) From 0d992c71f1bc90f5b5256ade992fd625cf823c47 Mon Sep 17 00:00:00 2001 From: samgst-amazon Date: Mon, 9 Dec 2024 15:15:50 -0800 Subject: [PATCH 12/16] default function in interface --- .../aws/toolkits/core/utils/RemoteResourceResolver.kt | 4 ++-- .../jetbrains/core/region/AwsRegionProviderTest.kt | 7 ------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt b/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt index 0872bcb8af2..64e507590f2 100644 --- a/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt +++ b/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt @@ -19,8 +19,8 @@ import java.util.concurrent.atomic.AtomicBoolean interface RemoteResourceResolver { fun resolve(resource: RemoteResource): CompletionStage - fun checkForUpdates(endpoint: String, eTagProvider: ETagProvider): UpdateCheckResult - fun getLocalResourcePath(filename: String): Path? + fun checkForUpdates(endpoint: String, eTagProvider: ETagProvider): UpdateCheckResult = UpdateCheckResult.NoUpdates + fun getLocalResourcePath(filename: String): Path? = null } interface RemoteResolveParser { fun canBeParsed(data: InputStream): Boolean diff --git a/plugins/toolkit/jetbrains-core/tst/software/aws/toolkits/jetbrains/core/region/AwsRegionProviderTest.kt b/plugins/toolkit/jetbrains-core/tst/software/aws/toolkits/jetbrains/core/region/AwsRegionProviderTest.kt index a0dc1c3ba6d..32304155a24 100644 --- a/plugins/toolkit/jetbrains-core/tst/software/aws/toolkits/jetbrains/core/region/AwsRegionProviderTest.kt +++ b/plugins/toolkit/jetbrains-core/tst/software/aws/toolkits/jetbrains/core/region/AwsRegionProviderTest.kt @@ -167,13 +167,6 @@ class AwsRegionProviderTest { override fun resolve(resource: RemoteResource): CompletionStage = CompletableFuture().apply { complete(file) } - - // following two methods are not used in this test, make compiler happy - override fun checkForUpdates(endpoint: String, eTagProvider: ETagProvider): UpdateCheckResult = - UpdateCheckResult.NoUpdates - - override fun getLocalResourcePath(filename: String): Path? = - null } } ApplicationManager.getApplication().replaceService(RemoteResourceResolverProvider::class.java, mockRemoteResource, disposableRule.disposable) From 9d4a1bb0db776e5d2f72f7511a27a515bcabbb36 Mon Sep 17 00:00:00 2001 From: samgst-amazon Date: Mon, 9 Dec 2024 15:34:23 -0800 Subject: [PATCH 13/16] re-implement HTTPRequest --- .../core/utils/RemoteResourceResolver.kt | 25 +++++++++++-------- .../core/region/AwsRegionProviderTest.kt | 2 -- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt b/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt index 64e507590f2..b413703e697 100644 --- a/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt +++ b/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt @@ -5,8 +5,10 @@ package software.aws.toolkits.core.utils import java.io.FileInputStream import java.io.InputStream -import java.net.HttpURLConnection import java.net.URI +import java.net.http.HttpClient +import java.net.http.HttpRequest +import java.net.http.HttpResponse import java.nio.file.Files import java.nio.file.Path import java.nio.file.StandardCopyOption @@ -127,16 +129,17 @@ class DefaultRemoteResourceResolver( private fun getEndpointETag(endpoint: String): String = try { - val url = URI(endpoint).toURL() - (url.openConnection() as HttpURLConnection).let { connection -> - connection.requestMethod = "HEAD" - connection.setRequestProperty("User-Agent", "AWS Toolkit for JetBrains") - connection.connect() - - val eTag = connection.getHeaderField("ETag").orEmpty() - connection.disconnect() - eTag - } + HttpRequest.newBuilder(URI(endpoint)) + .method("HEAD", HttpRequest.BodyPublishers.noBody()) + .header("User-Agent", "AWS Toolkit for JetBrains") + .build() + .let { request -> + HttpClient.newHttpClient() + .send(request, HttpResponse.BodyHandlers.discarding()) + .headers() + .firstValue("ETag") + .orElse("") + } } catch (e: Exception) { LOG.warn { "Failed to fetch notification ETag: ${e.message}" } throw e diff --git a/plugins/toolkit/jetbrains-core/tst/software/aws/toolkits/jetbrains/core/region/AwsRegionProviderTest.kt b/plugins/toolkit/jetbrains-core/tst/software/aws/toolkits/jetbrains/core/region/AwsRegionProviderTest.kt index 32304155a24..c39d23ad90c 100644 --- a/plugins/toolkit/jetbrains-core/tst/software/aws/toolkits/jetbrains/core/region/AwsRegionProviderTest.kt +++ b/plugins/toolkit/jetbrains-core/tst/software/aws/toolkits/jetbrains/core/region/AwsRegionProviderTest.kt @@ -14,10 +14,8 @@ import org.junit.Rule import org.junit.Test import software.aws.toolkits.core.rules.EnvironmentVariableHelper import software.aws.toolkits.core.rules.SystemPropertyHelper -import software.aws.toolkits.core.utils.ETagProvider import software.aws.toolkits.core.utils.RemoteResource import software.aws.toolkits.core.utils.RemoteResourceResolver -import software.aws.toolkits.core.utils.UpdateCheckResult import software.aws.toolkits.core.utils.exists import software.aws.toolkits.core.utils.writeText import software.aws.toolkits.jetbrains.core.RemoteResourceResolverProvider From b2b9849784f037f82ab041149d22cd3c1bbcc490 Mon Sep 17 00:00:00 2001 From: samgst-amazon Date: Mon, 9 Dec 2024 15:59:31 -0800 Subject: [PATCH 14/16] re-implement HTTPRequest --- .../core/utils/RemoteResourceResolver.kt | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt b/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt index b413703e697..dcbfdb12bdd 100644 --- a/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt +++ b/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt @@ -3,12 +3,9 @@ package software.aws.toolkits.core.utils +import com.intellij.util.io.HttpRequests import java.io.FileInputStream import java.io.InputStream -import java.net.URI -import java.net.http.HttpClient -import java.net.http.HttpRequest -import java.net.http.HttpResponse import java.nio.file.Files import java.nio.file.Path import java.nio.file.StandardCopyOption @@ -129,16 +126,10 @@ class DefaultRemoteResourceResolver( private fun getEndpointETag(endpoint: String): String = try { - HttpRequest.newBuilder(URI(endpoint)) - .method("HEAD", HttpRequest.BodyPublishers.noBody()) - .header("User-Agent", "AWS Toolkit for JetBrains") - .build() - .let { request -> - HttpClient.newHttpClient() - .send(request, HttpResponse.BodyHandlers.discarding()) - .headers() - .firstValue("ETag") - .orElse("") + HttpRequests.head(endpoint) + .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}" } From af54b0f521f2234b298c7f6e98be12982815ef56 Mon Sep 17 00:00:00 2001 From: samgst-amazon Date: Tue, 10 Dec 2024 09:27:01 -0800 Subject: [PATCH 15/16] ETag fix --- .../core/utils/RemoteResourceResolver.kt | 10 ++--- .../DefaultRemoteResourceResolverProvider.kt | 8 ++++ .../notifications/ProcessNotificationsBase.kt | 1 - .../NotificationResourceResolverTest.kt | 41 ++----------------- 4 files changed, 14 insertions(+), 46 deletions(-) diff --git a/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt b/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt index dcbfdb12bdd..02f6b711e2e 100644 --- a/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt +++ b/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt @@ -3,7 +3,6 @@ package software.aws.toolkits.core.utils -import com.intellij.util.io.HttpRequests import java.io.FileInputStream import java.io.InputStream import java.nio.file.Files @@ -126,13 +125,9 @@ class DefaultRemoteResourceResolver( private fun getEndpointETag(endpoint: String): String = try { - HttpRequests.head(endpoint) - .userAgent("AWS Toolkit for JetBrains") - .connect { request -> - request.connection.headerFields["ETag"]?.firstOrNull().orEmpty() - } + urlFetcher.getETag(endpoint) } catch (e: Exception) { - LOG.warn { "Failed to fetch notification ETag: ${e.message}" } + LOG.warn(e) { "Failed to fetch ETag: $e.message" } throw e } @@ -157,6 +152,7 @@ class DefaultRemoteResourceResolver( interface UrlFetcher { fun fetch(url: String, file: Path) + fun getETag(url: String): String } /** diff --git a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/DefaultRemoteResourceResolverProvider.kt b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/DefaultRemoteResourceResolverProvider.kt index 3e570126b66..af45361ac3d 100644 --- a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/DefaultRemoteResourceResolverProvider.kt +++ b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/DefaultRemoteResourceResolverProvider.kt @@ -4,6 +4,7 @@ package software.aws.toolkits.jetbrains.core import com.intellij.openapi.application.PathManager +import com.intellij.util.io.HttpRequests import com.intellij.util.io.createDirectories import software.aws.toolkits.core.utils.DefaultRemoteResourceResolver import software.aws.toolkits.core.utils.UrlFetcher @@ -38,6 +39,13 @@ class DefaultRemoteResourceResolverProvider : RemoteResourceResolverProvider { override fun fetch(url: String, file: Path) { saveFileFromUrl(url, file) } + + override fun getETag(url: String): String = + HttpRequests.head(url) + .userAgent("AWS Toolkit for JetBrains") + .connect { request -> + request.connection.headerFields["ETag"]?.firstOrNull().orEmpty() + } } } } diff --git a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/ProcessNotificationsBase.kt b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/ProcessNotificationsBase.kt index f189ffb0632..1313f2b6871 100644 --- a/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/ProcessNotificationsBase.kt +++ b/plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/ProcessNotificationsBase.kt @@ -21,7 +21,6 @@ import software.aws.toolkits.jetbrains.utils.notifyStickyWithData import software.aws.toolkits.telemetry.Component import software.aws.toolkits.telemetry.Result import software.aws.toolkits.telemetry.ToolkitTelemetry -import java.nio.file.Paths import java.util.concurrent.atomic.AtomicBoolean object NotificationMapperUtil { diff --git a/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolverTest.kt b/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolverTest.kt index dda8e50c6a4..a9c0ed8496f 100644 --- a/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolverTest.kt +++ b/plugins/core/jetbrains-community/tst/software/aws/toolkits/jetbrains/core/notifications/NotificationResourceResolverTest.kt @@ -5,10 +5,7 @@ package software.aws.toolkits.jetbrains.core.notifications import com.intellij.testFramework.ApplicationExtension import io.mockk.every -import io.mockk.just import io.mockk.mockk -import io.mockk.mockkConstructor -import io.mockk.runs import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test @@ -17,8 +14,6 @@ import org.junit.jupiter.api.io.TempDir import software.aws.toolkits.core.utils.DefaultRemoteResourceResolver import software.aws.toolkits.core.utils.UpdateCheckResult import software.aws.toolkits.core.utils.UrlFetcher -import java.net.HttpURLConnection -import java.net.URL import java.nio.file.Path import java.util.concurrent.Callable import java.util.concurrent.CompletableFuture @@ -45,39 +40,17 @@ class NotificationResourceResolverTest { fun `first poll with no ETag changes returns FirstPollCheck`() { NotificationEtagState.getInstance().etag = "same-etag" val expectedETag = "same-etag" - val mockConnection = mockk { - every { requestMethod = any() } just runs - every { setRequestProperty(any(), any()) } just runs - every { connect() } just runs - every { getHeaderField("ETag") } returns expectedETag - every { disconnect() } just runs - } - - mockkConstructor(URL::class) - every { anyConstructed().openConnection() } returns mockConnection + every { urlFetcher.getETag(any()) } returns expectedETag val result = sut.checkForUpdates("http://notification.test", NotificationEtagState.getInstance()) assertThat(result).isEqualTo(UpdateCheckResult.FirstPollCheck) - - // Second call should not return FirstPollCheck - val secondResult = sut.checkForUpdates("http://notification.test", NotificationEtagState.getInstance()) - assertThat(secondResult).isEqualTo(UpdateCheckResult.NoUpdates) } @Test fun `ETag changes returns HasUpdates`() { NotificationEtagState.getInstance().etag = "old-etag" val expectedETag = "new-etag" - val mockConnection = mockk { - every { requestMethod = any() } just runs - every { setRequestProperty(any(), any()) } just runs - every { connect() } just runs - every { getHeaderField("ETag") } returns expectedETag - every { disconnect() } just runs - } - - mockkConstructor(URL::class) - every { anyConstructed().openConnection() } returns mockConnection + every { urlFetcher.getETag(any()) } returns expectedETag val result = sut.checkForUpdates("http://notification.test", NotificationEtagState.getInstance()) assertThat(result).isEqualTo(UpdateCheckResult.HasUpdates) @@ -87,16 +60,8 @@ class NotificationResourceResolverTest { fun `no ETag changes returns NoUpdates after first poll`() { NotificationEtagState.getInstance().etag = "same-etag" val expectedETag = "same-etag" - val mockConnection = mockk { - every { requestMethod = any() } just runs - every { setRequestProperty(any(), any()) } just runs - every { connect() } just runs - every { getHeaderField("ETag") } returns expectedETag - every { disconnect() } just runs - } + every { urlFetcher.getETag(any()) } returns expectedETag - mockkConstructor(URL::class) - every { anyConstructed().openConnection() } returns mockConnection // sets isFirstPoll to false val firstResult = sut.checkForUpdates("http://notification.test", NotificationEtagState.getInstance()) assertThat(firstResult).isEqualTo(UpdateCheckResult.FirstPollCheck) From b289ce10cb3ff8da69c87930ecfa2707c36cfdec Mon Sep 17 00:00:00 2001 From: samgst-amazon Date: Tue, 10 Dec 2024 11:38:39 -0800 Subject: [PATCH 16/16] LazyLogRule --- .../software/aws/toolkits/core/utils/RemoteResourceResolver.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt b/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt index 02f6b711e2e..a8107c5b3e8 100644 --- a/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt +++ b/plugins/core/core/src/software/aws/toolkits/core/utils/RemoteResourceResolver.kt @@ -127,7 +127,7 @@ class DefaultRemoteResourceResolver( try { urlFetcher.getETag(endpoint) } catch (e: Exception) { - LOG.warn(e) { "Failed to fetch ETag: $e.message" } + LOG.warn { "Failed to fetch ETag: $e.message" } throw e }