Special case S3 Bucket in Resources node (#2896)

kiiadi · web-flow · commit 8de5eb873701 · 2021-10-20T18:16:52.000-07:00
* Adding ability to create a view which is region aware - using it for S3 buckets.

* Special case S3 for Cloud API

* Adding test and moving string into constant

* Adding changelog
diff --git a/.changes/next-release/bugfix-66e9d82e-d495-4128-8547-215cedba1f32.json b/.changes/next-release/bugfix-66e9d82e-d495-4128-8547-215cedba1f32.json
@@ -0,0 +1,4 @@
+{
+  "type" : "bugfix",
+  "description" : "Fix issue in Resources where some S3 Buckets fail to open"
+}
diff --git a/jetbrains-core/it/software/aws/toolkits/jetbrains/services/lambda/upload/CreateFunctionIntegrationTest.kt b/jetbrains-core/it/software/aws/toolkits/jetbrains/services/lambda/upload/CreateFunctionIntegrationTest.kt
@@ -129,7 +129,7 @@ class CreateFunctionIntegrationTest {
         val s3Bucket = temporaryBucket.createBucket()
         resourceCache.addEntry(
             projectRule.project,
-            S3Resources.LIST_REGIONALIZED_BUCKETS,
+            "s3.list_buckets",
             listOf(S3Resources.RegionalizedBucket(Bucket.builder().name(s3Bucket).build(), projectRule.project.activeRegion()))
         )
 
diff --git a/jetbrains-core/src/software/aws/toolkits/jetbrains/core/AwsResourceCache.kt b/jetbrains-core/src/software/aws/toolkits/jetbrains/core/AwsResourceCache.kt
@@ -210,17 +210,22 @@ sealed class Resource<T> {
      * in order to return the desired type [Output]. The [transform] result is not cached, [transform]s are re-applied on each fetch - thus should
      * should be relatively cheap.
      */
-    class View<Input, Output>(val underlying: Resource<Input>, private val transform: Input.() -> Output) : Resource<Output>() {
+    class View<Input, Output>(val underlying: Resource<Input>, private val transform: (Input, AwsRegion) -> Output) : Resource<Output>() {
         @Suppress("UNCHECKED_CAST")
-        fun doMap(input: Any) = transform(input as Input)
+        fun doMap(input: Any, region: AwsRegion) = transform(input as Input, region)
+    }
+
+    companion object {
+        fun <Input, Output> view(underlying: Resource<Input>, transform: Input.() -> Output): Resource<Output> =
+            View(underlying) { input, _ -> transform(input) }
     }
 }
 
-fun <Input, Output> Resource<out Iterable<Input>>.map(transform: (Input) -> Output): Resource<List<Output>> = Resource.View(this) { map(transform) }
+fun <Input, Output> Resource<out Iterable<Input>>.map(transform: (Input) -> Output): Resource<List<Output>> = Resource.view(this) { map(transform) }
 
-fun <T> Resource<out Iterable<T>>.filter(predicate: (T) -> Boolean): Resource<List<T>> = Resource.View(this) { filter(predicate) }
+fun <T> Resource<out Iterable<T>>.filter(predicate: (T) -> Boolean): Resource<List<T>> = Resource.view(this) { filter(predicate) }
 
-fun <T> Resource<out Iterable<T>>.find(predicate: (T) -> Boolean): Resource<T?> = Resource.View(this) { find(predicate) }
+fun <T> Resource<out Iterable<T>>.find(predicate: (T) -> Boolean): Resource<T?> = Resource.view(this) { find(predicate) }
 
 class ClientBackedCachedResource<ReturnType, ClientType : SdkClient>(
     private val sdkClientClass: KClass<ClientType>,
@@ -294,7 +299,13 @@ class DefaultAwsResourceCache(
         useStale: Boolean,
         forceFetch: Boolean
     ): CompletionStage<T> = when (resource) {
-        is Resource.View<*, T> -> getResource(resource.underlying, region, credentialProvider, useStale, forceFetch).thenApply { resource.doMap(it as Any) }
+        is Resource.View<*, T> -> getResource(
+            resource.underlying,
+            region,
+            credentialProvider,
+            useStale,
+            forceFetch
+        ).thenApply { resource.doMap(it as Any, region) }
         is Resource.Cached<T> -> Context(resource, region, credentialProvider, useStale, forceFetch).also { getCachedResource(it) }.future
     }
 
@@ -360,7 +371,12 @@ class DefaultAwsResourceCache(
                     else -> null
                 }
             }
-            is Resource.View<*, T> -> getResourceIfPresent(resource.underlying, region, credentialProvider, useStale)?.let { resource.doMap(it) }
+            is Resource.View<*, T> -> getResourceIfPresent(resource.underlying, region, credentialProvider, useStale)?.let {
+                resource.doMap(
+                    it,
+                    region
+                )
+            }
         }
 
     override fun clear(resource: Resource<*>, connectionSettings: ConnectionSettings) {
diff --git a/jetbrains-core/src/software/aws/toolkits/jetbrains/services/dynamic/CloudControlApiResources.kt b/jetbrains-core/src/software/aws/toolkits/jetbrains/services/dynamic/CloudControlApiResources.kt
@@ -12,26 +12,26 @@ import software.amazon.awssdk.services.cloudformation.model.RegistryType
 import software.amazon.awssdk.services.cloudformation.model.Visibility
 import software.aws.toolkits.jetbrains.core.ClientBackedCachedResource
 import software.aws.toolkits.jetbrains.core.Resource
+import software.aws.toolkits.jetbrains.core.map
+import software.aws.toolkits.jetbrains.services.s3.resources.S3Resources
 
 object CloudControlApiResources {
-
-    fun listResources(typeName: String): Resource.Cached<List<DynamicResource>> =
-        ClientBackedCachedResource(CloudControlClient::class, "cloudcontrolapi.dynamic.resources.$typeName") {
-            this.listResourcesPaginator {
-                it.typeName(typeName)
-            }.flatMap {
-                it.resourceDescriptions().map { resource ->
-                    DynamicResource(resourceTypeFromResourceTypeName(it.typeName()), resource.identifier())
-                }
+    fun listResources(typeName: String): Resource<List<DynamicResource>> {
+        return when (typeName) {
+            S3_BUCKET -> S3Resources.LIST_BUCKETS.map { it.name() }
+            else -> ClientBackedCachedResource(CloudControlClient::class, "cloudcontrolapi.dynamic.resources.$typeName") {
+                this.listResourcesPaginator { req -> req.typeName(typeName) }
+                    .flatMap { page -> page.resourceDescriptions().map { it.identifier() } }
             }
-        }
+        }.map { DynamicResource(resourceTypeFromResourceTypeName(typeName), it) }
+    }
 
     fun resourceTypeFromResourceTypeName(typeName: String): ResourceType {
         val (_, svc, type) = typeName.split("::")
         return ResourceType(typeName, svc, type)
     }
 
-    fun listResources(resourceType: ResourceType): Resource.Cached<List<DynamicResource>> = listResources(resourceType.fullName)
+    fun listResources(resourceType: ResourceType): Resource<List<DynamicResource>> = listResources(resourceType.fullName)
 
     fun getResourceDisplayName(identifier: String): String =
         if (identifier.startsWith("arn:")) {
@@ -49,14 +49,13 @@ object CloudControlApiResources {
             LightVirtualFile("${resourceType}Schema.json", schema)
         }
 
-    fun listTypes(): Resource.Cached<List<String>> = ClientBackedCachedResource(
-        CloudFormationClient::class, "cloudformation.listTypes"
-    ) {
+    fun listTypes(): Resource.Cached<List<String>> = ClientBackedCachedResource(CloudFormationClient::class, "cloudformation.listTypes") {
         this.listTypesPaginator {
             it.visibility(Visibility.PUBLIC)
             it.type(RegistryType.RESOURCE)
         }.flatMap { it.typeSummaries().map { it.typeName() } }
     }
+    private const val S3_BUCKET = "AWS::S3::Bucket"
 }
 
 data class ResourceDetails(val operations: List<PermittedOperation>, val arnRegex: String?, val documentation: String?)
diff --git a/jetbrains-core/src/software/aws/toolkits/jetbrains/services/ecs/resources/EcsResources.kt b/jetbrains-core/src/software/aws/toolkits/jetbrains/services/ecs/resources/EcsResources.kt
@@ -43,5 +43,5 @@ object EcsResources {
     fun listTaskIds(clusterArn: String, serviceArn: String) = listTasks(clusterArn, serviceArn).map { it.substringAfterLast("/") }
 
     fun listContainers(taskDefinitionArn: String): Resource<List<ContainerDefinition>> =
-        Resource.View(describeTaskDefinition(taskDefinitionArn)) { containerDefinitions() }
+        Resource.view(describeTaskDefinition(taskDefinitionArn)) { containerDefinitions() }
 }
diff --git a/jetbrains-core/src/software/aws/toolkits/jetbrains/services/iam/Iam.kt b/jetbrains-core/src/software/aws/toolkits/jetbrains/services/iam/Iam.kt
@@ -31,12 +31,12 @@ object IamResources {
     }
 
     @JvmField
-    val LIST_ALL: Resource<List<IamRole>> = Resource.View(LIST_RAW_ROLES) {
+    val LIST_ALL: Resource<List<IamRole>> = Resource.view(LIST_RAW_ROLES) {
         map { IamRole(it.arn()) }.toList()
     }
 
     @JvmField
-    val LIST_LAMBDA_ROLES: Resource<List<IamRole>> = Resource.View(LIST_RAW_ROLES) {
+    val LIST_LAMBDA_ROLES: Resource<List<IamRole>> = Resource.view(LIST_RAW_ROLES) {
         filter { it.assumeRolePolicyDocument().contains(LAMBDA_PRINCIPAL) }
             .map { IamRole(it.arn()) }
             .toList()
diff --git a/jetbrains-core/src/software/aws/toolkits/jetbrains/services/lambda/deploy/DeployServerlessApplicationDialog.kt b/jetbrains-core/src/software/aws/toolkits/jetbrains/services/lambda/deploy/DeployServerlessApplicationDialog.kt
@@ -28,6 +28,7 @@ import software.amazon.awssdk.services.lambda.model.PackageType
 import software.amazon.awssdk.services.s3.S3Client
 import software.aws.toolkits.jetbrains.core.awsClient
 import software.aws.toolkits.jetbrains.core.help.HelpIds
+import software.aws.toolkits.jetbrains.core.map
 import software.aws.toolkits.jetbrains.services.cloudformation.CloudFormationTemplate
 import software.aws.toolkits.jetbrains.services.cloudformation.Parameter
 import software.aws.toolkits.jetbrains.services.cloudformation.SamFunction
@@ -90,7 +91,7 @@ class DeployServerlessApplicationDialog(
         .build()
 
     private val s3BucketSelector = ResourceSelector.builder()
-        .resource(S3Resources.listBucketNamesByActiveRegion(project))
+        .resource(S3Resources.LIST_BUCKETS.map { it.name() })
         .awsConnection(project)
         .apply {
             if (!loadResourcesOnCreate) {
diff --git a/jetbrains-core/src/software/aws/toolkits/jetbrains/services/lambda/upload/CodeStoragePanel.kt b/jetbrains-core/src/software/aws/toolkits/jetbrains/services/lambda/upload/CodeStoragePanel.kt
@@ -8,11 +8,12 @@ import com.intellij.ui.IdeBorderFactory
 import com.intellij.ui.SimpleListCellRenderer
 import software.amazon.awssdk.services.lambda.model.PackageType
 import software.aws.toolkits.jetbrains.core.awsClient
+import software.aws.toolkits.jetbrains.core.map
 import software.aws.toolkits.jetbrains.services.ecr.CreateEcrRepoDialog
 import software.aws.toolkits.jetbrains.services.ecr.resources.EcrResources
 import software.aws.toolkits.jetbrains.services.ecr.resources.Repository
 import software.aws.toolkits.jetbrains.services.s3.CreateS3BucketDialog
-import software.aws.toolkits.jetbrains.services.s3.resources.S3Resources.listBucketNamesByActiveRegion
+import software.aws.toolkits.jetbrains.services.s3.resources.S3Resources.LIST_BUCKETS
 import software.aws.toolkits.jetbrains.ui.ResourceSelector
 import software.aws.toolkits.jetbrains.utils.ui.validationInfo
 import software.aws.toolkits.resources.message
@@ -74,7 +75,7 @@ class CodeStoragePanel(private val project: Project) : JPanel(BorderLayout()) {
     }
 
     private fun createUIComponents() {
-        sourceBucket = ResourceSelector.builder().resource(listBucketNamesByActiveRegion(project)).awsConnection(project).build()
+        sourceBucket = ResourceSelector.builder().resource(LIST_BUCKETS.map { it.name() }).awsConnection(project).build()
         ecrRepo = ResourceSelector.builder()
             .resource(EcrResources.LIST_REPOS)
             .customRenderer(SimpleListCellRenderer.create("") { it.repositoryName })
diff --git a/jetbrains-core/src/software/aws/toolkits/jetbrains/services/s3/S3ExplorerNode.kt b/jetbrains-core/src/software/aws/toolkits/jetbrains/services/s3/S3ExplorerNode.kt
@@ -11,15 +11,14 @@ import software.aws.toolkits.jetbrains.core.credentials.activeRegion
 import software.aws.toolkits.jetbrains.core.explorer.nodes.AwsExplorerNode
 import software.aws.toolkits.jetbrains.core.explorer.nodes.AwsExplorerResourceNode
 import software.aws.toolkits.jetbrains.core.explorer.nodes.AwsExplorerServiceNode
-import software.aws.toolkits.jetbrains.core.explorer.nodes.AwsExplorerServiceRootNode
-import software.aws.toolkits.jetbrains.core.getResourceNow
+import software.aws.toolkits.jetbrains.core.explorer.nodes.CacheBackedAwsExplorerServiceRootNode
 import software.aws.toolkits.jetbrains.services.s3.resources.S3Resources
 import software.aws.toolkits.resources.message
 
-class S3ServiceNode(project: Project, service: AwsExplorerServiceNode) : AwsExplorerServiceRootNode(project, service) {
+class S3ServiceNode(project: Project, service: AwsExplorerServiceNode) :
+    CacheBackedAwsExplorerServiceRootNode<Bucket>(project, service, S3Resources.LIST_BUCKETS) {
     override fun displayName(): String = message("explorer.node.s3")
-    override fun getChildrenInternal(): List<AwsExplorerNode<*>> =
-        nodeProject.getResourceNow(S3Resources.listBucketsByActiveRegion(nodeProject)).map { S3BucketNode(nodeProject, it) }
+    override fun toNode(child: Bucket): AwsExplorerNode<*> = S3BucketNode(nodeProject, child)
 }
 
 class S3BucketNode(project: Project, val bucket: Bucket) :
diff --git a/jetbrains-core/src/software/aws/toolkits/jetbrains/services/s3/resources/S3Resources.kt b/jetbrains-core/src/software/aws/toolkits/jetbrains/services/s3/resources/S3Resources.kt
@@ -3,11 +3,11 @@
 
 package software.aws.toolkits.jetbrains.services.s3.resources
 
-import com.intellij.openapi.project.Project
 import kotlinx.coroutines.async
 import kotlinx.coroutines.awaitAll
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.withContext
+import org.jetbrains.annotations.TestOnly
 import org.slf4j.event.Level
 import software.amazon.awssdk.services.s3.S3Client
 import software.amazon.awssdk.services.s3.model.Bucket
@@ -18,9 +18,6 @@ import software.aws.toolkits.core.utils.tryOrNull
 import software.aws.toolkits.jetbrains.core.ClientBackedCachedResource
 import software.aws.toolkits.jetbrains.core.Resource
 import software.aws.toolkits.jetbrains.core.coroutines.getCoroutineBgContext
-import software.aws.toolkits.jetbrains.core.credentials.AwsConnectionManager
-import software.aws.toolkits.jetbrains.core.filter
-import software.aws.toolkits.jetbrains.core.map
 import software.aws.toolkits.jetbrains.core.region.AwsRegionProvider
 import java.time.Instant
 import java.time.LocalDateTime
@@ -31,7 +28,8 @@ object S3Resources {
     private val LOG = getLogger<S3Resources>()
     private val regions by lazy { AwsRegionProvider.getInstance().allRegions() }
 
-    val LIST_REGIONALIZED_BUCKETS = ClientBackedCachedResource(S3Client::class, "s3.list_buckets") {
+    @TestOnly
+    internal val LIST_REGIONALIZED_BUCKETS = ClientBackedCachedResource(S3Client::class, "s3.list_buckets") {
         val buckets = listBuckets().buckets()
         // TODO when the resource cache is coroutine based, remove the runBlocking and withContext
         runBlocking {
@@ -50,16 +48,10 @@ object S3Resources {
         }
     }
 
-    val LIST_BUCKETS: Resource<List<Bucket>> = LIST_REGIONALIZED_BUCKETS.map { it.bucket }
-
-    fun listBucketsByActiveRegion(project: Project): Resource<List<Bucket>> {
-        val activeRegion = AwsConnectionManager.getInstance(project).activeRegion
-        return LIST_REGIONALIZED_BUCKETS.filter { it.region == activeRegion }.map { it.bucket }
+    val LIST_BUCKETS: Resource<List<Bucket>> = Resource.View(LIST_REGIONALIZED_BUCKETS) { bucketList, region ->
+        bucketList.filter { it.region == region }.map { it.bucket }
     }
 
-    @JvmStatic
-    fun listBucketNamesByActiveRegion(project: Project): Resource<List<String>> = listBucketsByActiveRegion(project).map { it.name() }
-
     @JvmStatic
     fun formatDate(date: Instant): String {
         val datetime = LocalDateTime.ofInstant(date, ZoneId.systemDefault())
diff --git a/jetbrains-core/tst/software/aws/toolkits/jetbrains/core/AwsResourceCacheTest.kt b/jetbrains-core/tst/software/aws/toolkits/jetbrains/core/AwsResourceCacheTest.kt
@@ -226,7 +226,7 @@ class AwsResourceCacheTest {
     @Test
     fun viewsCanBeCreatedOnTopOfOtherCachedItems() {
         whenever(mockResource.fetch(any(), any())).thenReturn("hello")
-        val viewResource = Resource.View(mockResource) { toList() }
+        val viewResource = Resource.view(mockResource) { toList() }
 
         assertThat(sut.getResource(mockResource, connectionSettings)).hasValue("hello")
         assertThat(sut.getResource(viewResource, connectionSettings)).hasValue(listOf('h', 'e', 'l', 'l', 'o'))
@@ -236,7 +236,7 @@ class AwsResourceCacheTest {
     @Test
     fun mapFilterAndFindExtensionsToEasilyCreateViews() {
         whenever(mockResource.fetch(any(), any())).thenReturn("hello")
-        val viewResource = Resource.View(mockResource) { toList() }
+        val viewResource = Resource.view(mockResource) { toList() }
 
         val filteredAndMapped = viewResource.filter { it != 'l' }.map { it.toUpperCase() }
         assertThat(sut.getResource(filteredAndMapped, connectionSettings)).hasValue(listOf('H', 'E', 'O'))
@@ -248,14 +248,22 @@ class AwsResourceCacheTest {
     @Test
     fun clearingViewsClearTheUnderlyingCachedResource() {
         whenever(mockResource.fetch(any(), any())).thenReturn("hello")
-        val viewResource = Resource.View(mockResource) { toList() }
+        val viewResource = Resource.view(mockResource) { toList() }
         sut.getResource(viewResource, connectionSettings).value
         sut.clear(viewResource, connectionSettings)
         sut.getResource(viewResource, connectionSettings).value
 
         verifyResourceCalled(times = 2)
     }
 
+    @Test
+    fun viewsCanBeRegionAware() {
+        whenever(mockResource.fetch(any(), any())).thenReturn("hello")
+        val viewResource = Resource.View(mockResource) { _, region -> region }
+
+        assertThat(sut.getResource(viewResource, connectionSettings)).hasValue(connectionSettings.region)
+    }
+
     @Test
     fun cacheIsRegularlyPrunedToEnsureItDoesntGrowTooLarge() {
         val localSut = DefaultAwsResourceCache(mockClock, 5, Duration.ofMillis(50))
@@ -450,7 +458,7 @@ class AwsResourceCacheTest {
     @Test
     fun canConditionallyFetchViewOnlyIfAvailableInCache() {
         whenever(mockResource.fetch(any(), any())).thenReturn("hello")
-        val viewResource = Resource.View(mockResource) { reversed() }
+        val viewResource = Resource.view(mockResource) { reversed() }
 
         assertThat(sut.getResourceIfPresent(viewResource, US_WEST_1, cred1Provider)).isNull()
         sut.getResource(viewResource, US_WEST_1, cred1Provider).value
@@ -516,3 +524,9 @@ class AwsResourceCacheTest {
         fun dummyResource(value: String = aString()): Resource.Cached<String> = StringResource(value)
     }
 }
+
+val Resource<*>.id: String
+    get() = when (this) {
+        is Resource.Cached -> this.id
+        is Resource.View<*, *> -> this.underlying.id
+    }
diff --git a/jetbrains-core/tst/software/aws/toolkits/jetbrains/core/MockResourceCache.kt b/jetbrains-core/tst/software/aws/toolkits/jetbrains/core/MockResourceCache.kt
diff --git a/jetbrains-core/tst/software/aws/toolkits/jetbrains/services/dynamic/explorer/CloudControlApiResourcesTest.kt b/jetbrains-core/tst/software/aws/toolkits/jetbrains/services/dynamic/explorer/CloudControlApiResourcesTest.kt
diff --git a/jetbrains-core/tst/software/aws/toolkits/jetbrains/services/dynamic/explorer/DynamicResourceResourceTypeNodeTest.kt b/jetbrains-core/tst/software/aws/toolkits/jetbrains/services/dynamic/explorer/DynamicResourceResourceTypeNodeTest.kt
diff --git a/jetbrains-rider/tst/software/aws/toolkits/jetbrains/services/ecs/execution/DotNetEcsCloudDebugRunConfigurationTest.kt b/jetbrains-rider/tst/software/aws/toolkits/jetbrains/services/ecs/execution/DotNetEcsCloudDebugRunConfigurationTest.kt

-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +{
 +  "type" : "bugfix",
 +  "description" : "Fix issue in Resources where some S3 Buckets fail to open"
 +}
Original file line number	Diff line number	Diff line change
`@@ -129,7 +129,7 @@ class CreateFunctionIntegrationTest {`
`129`	`129`	`val s3Bucket = temporaryBucket.createBucket()`
`130`	`130`	`resourceCache.addEntry(`
`131`	`131`	`projectRule.project,`
`132`		`- S3Resources.LIST_REGIONALIZED_BUCKETS,`
	`132`	`+ "s3.list_buckets",`
`133`	`133`	`listOf(S3Resources.RegionalizedBucket(Bucket.builder().name(s3Bucket).build(), projectRule.project.activeRegion()))`
`134`	`134`	`)`
`135`	`135`
Original file line number	Diff line number	Diff line change
`@@ -43,5 +43,5 @@ object EcsResources {`
`43`	`43`	`fun listTaskIds(clusterArn: String, serviceArn: String) = listTasks(clusterArn, serviceArn).map { it.substringAfterLast("/") }`
`44`	`44`
`45`	`45`	`fun listContainers(taskDefinitionArn: String): Resource<List<ContainerDefinition>> =`
`46`		`- Resource.View(describeTaskDefinition(taskDefinitionArn)) { containerDefinitions() }`
	`46`	`+ Resource.view(describeTaskDefinition(taskDefinitionArn)) { containerDefinitions() }`
`47`	`47`	`}`
Original file line number	Diff line number	Diff line change
`@@ -31,12 +31,12 @@ object IamResources {`
`31`	`31`	`}`
`32`	`32`
`33`	`33`	`@JvmField`
`34`		`- val LIST_ALL: Resource<List<IamRole>> = Resource.View(LIST_RAW_ROLES) {`
	`34`	`+ val LIST_ALL: Resource<List<IamRole>> = Resource.view(LIST_RAW_ROLES) {`
`35`	`35`	`map { IamRole(it.arn()) }.toList()`
`36`	`36`	`}`
`37`	`37`
`38`	`38`	`@JvmField`
`39`		`- val LIST_LAMBDA_ROLES: Resource<List<IamRole>> = Resource.View(LIST_RAW_ROLES) {`
	`39`	`+ val LIST_LAMBDA_ROLES: Resource<List<IamRole>> = Resource.view(LIST_RAW_ROLES) {`
`40`	`40`	`filter { it.assumeRolePolicyDocument().contains(LAMBDA_PRINCIPAL) }`
`41`	`41`	`.map { IamRole(it.arn()) }`
`42`	`42`	`.toList()`