fix(cloudformation): load stacks lazily to avoid error notification o… (#8349)

…n startup

## Problem
New CloudFormation feature eagerly loaded stacks resulting in error
notification if credentials were not set properly. Eager loading is not
desirable even if credentials would work because user may not open the
panel or intend to use the data.

## Solution
Lazy load stacks.
Tested:
- stacks do not load on startup
- switching regions does not eagerly load stacks
- pagination with load more works as before
- deploying template if stacks were never loaded triggers loading as
stacks are refreshed (desired behavior)
- if StacksNode is expanded and region is switched then stacks are
loaded (desired behavior)

Dead code removal: polling was disabled earlier and unused.

---

- Treat all work as PUBLIC. Private `feature/x` branches will not be
squash-merged at release time.
- Your code changes must meet the guidelines in
[CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines).
- License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.

Author: Zee2413

packages/core/src/awsService/cloudformation/auth/credentials.ts

@@ -19,9 +19,9 @@ export class AwsCredentialsService implements Disposable {
     private client: LanguageClient | undefined
 
     constructor(
-        private stacksManager: StacksManager,
-        private resourcesManager: ResourcesManager,
-        private regionManager: CloudFormationRegionManager
+        private readonly stacksManager: StacksManager,
+        private readonly resourcesManager: ResourcesManager,
+        private readonly regionManager: CloudFormationRegionManager
     ) {
         this.authChangeListener = globals.awsContext.onDidChangeContext(() => {
             void this.updateCredentialsFromActiveConnection()
@@ -53,7 +53,7 @@ export class AwsCredentialsService implements Disposable {
             await this.client.sendRequest('aws/credentials/iam/update', encryptedRequest)
         }
 
-        void this.stacksManager.reload()
+        this.stacksManager.clear()
         void this.resourcesManager.reload()
     }
 

packages/core/src/awsService/cloudformation/explorer/nodes/stacksNode.ts

@@ -33,17 +33,23 @@ export class StacksNode extends AWSTreeNodeBase {
     }
 
     public override async getChildren(): Promise<AWSTreeNodeBase[]> {
+        await this.stacksManager.ensureLoaded()
         this.updateNode()
         const stacks = this.stacksManager.get()
         const nodes = stacks.map((stack: StackSummary) => new StackNode(stack, this.changeSetsManager))
         return this.stacksManager.hasMore() ? [...nodes, new LoadMoreStacksNode(this)] : nodes
     }
 
     private updateNode(): void {
-        const count = this.stacksManager.get().length
-        const hasMore = this.stacksManager.hasMore()
-        this.description = hasMore ? `(${count}+)` : `(${count})`
-        this.contextValue = hasMore ? 'stackSectionWithMore' : 'stackSection'
+        if (this.stacksManager.isLoaded()) {
+            const count = this.stacksManager.get().length
+            const hasMore = this.stacksManager.hasMore()
+            this.description = hasMore ? `(${count}+)` : `(${count})`
+            this.contextValue = hasMore ? 'stackSectionWithMore' : 'stackSection'
+        } else {
+            this.description = undefined
+            this.contextValue = 'stackSection'
+        }
     }
 
     public async loadMoreStacks(): Promise<void> {

packages/core/src/awsService/cloudformation/stacks/stacksManager.ts

@@ -23,15 +23,14 @@ type ListStacksResult = {
 }
 
 const ListStacksRequest = new RequestType<ListStacksParams, ListStacksResult, void>('aws/cfn/stacks')
-const PollIntervalMs = 1000
 
 type StacksChangeListener = (stacks: StackSummary[]) => void
 
 export class StacksManager implements Disposable {
     private stacks: StackSummary[] = []
     private nextToken?: string
     private readonly listeners: StacksChangeListener[] = []
-    private poller?: NodeJS.Timeout
+    private loaded = false
 
     constructor(private readonly client: LanguageClient) {}
 
@@ -51,6 +50,23 @@ export class StacksManager implements Disposable {
         void this.loadStacks()
     }
 
+    isLoaded() {
+        return this.loaded
+    }
+
+    async ensureLoaded() {
+        if (!this.loaded) {
+            await this.loadStacks()
+        }
+    }
+
+    clear() {
+        this.stacks = []
+        this.nextToken = undefined
+        this.loaded = false
+        this.notifyListeners()
+    }
+
     updateStackStatus(stackName: string, stackStatus: string) {
         const stack = this.stacks.find((s) => s.StackName === stackName)
         if (stack) {
@@ -80,21 +96,8 @@ export class StacksManager implements Disposable {
         }
     }
 
-    startPolling() {
-        this.poller ??= setInterval(() => {
-            this.reload()
-        }, PollIntervalMs)
-    }
-
-    stopPolling() {
-        if (this.poller) {
-            clearInterval(this.poller)
-            this.poller = undefined
-        }
-    }
-
     dispose() {
-        this.stopPolling()
+        // do nothing
     }
 
     private async loadStacks() {
@@ -106,16 +109,14 @@ export class StacksManager implements Disposable {
             })
             this.stacks = response.stacks
             this.nextToken = response.nextToken
+            this.loaded = true
         } catch (error) {
             await handleLspError(error, 'Error loading stacks')
             this.stacks = []
             this.nextToken = undefined
         } finally {
             await setContext('aws.cloudformation.refreshingStacks', false)
             this.notifyListeners()
-            if (this.stacks.length === 0) {
-                this.stopPolling()
-            }
         }
     }
 

packages/core/src/test/awsService/cloudformation/auth/credentials.test.ts

@@ -18,7 +18,7 @@ describe('AwsCredentialsService', function () {
 
     beforeEach(function () {
         sandbox = sinon.createSandbox()
-        mockStacksManager = { reload: sandbox.stub(), hasMore: sandbox.stub().returns(false) }
+        mockStacksManager = { reload: sandbox.stub(), hasMore: sandbox.stub().returns(false), clear: sandbox.stub() }
         mockResourcesManager = { reload: sandbox.stub() }
         mockClient = { sendRequest: sandbox.stub() }
 
@@ -31,13 +31,6 @@ describe('AwsCredentialsService', function () {
         sandbox.restore()
     })
 
-    describe('constructor', function () {
-        it('should initialize credentials service', function () {
-            credentialsService = new AwsCredentialsService(mockStacksManager, mockResourcesManager, mockRegionManager)
-            assert(credentialsService !== undefined)
-        })
-    })
-
     describe('createEncryptedCredentialsRequest', function () {
         beforeEach(function () {
             credentialsService = new AwsCredentialsService(mockStacksManager, mockResourcesManager, mockRegionManager)
@@ -89,5 +82,11 @@ describe('AwsCredentialsService', function () {
             // Test passes if no error thrown
             assert(true)
         })
+
+        it('should clear stacks', async function () {
+            credentialsService = new AwsCredentialsService(mockStacksManager, mockResourcesManager, mockRegionManager)
+            await credentialsService.initialize(mockClient)
+            assert(mockStacksManager.clear.calledOnce)
+        })
     })
 })

packages/core/src/test/awsService/cloudformation/explorer/nodes/stacksNode.test.ts

@@ -0,0 +1,204 @@
+/*!
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import assert from 'assert'
+import * as sinon from 'sinon'
+import { TreeItemCollapsibleState } from 'vscode'
+import { StacksNode } from '../../../../../awsService/cloudformation/explorer/nodes/stacksNode'
+import { StacksManager } from '../../../../../awsService/cloudformation/stacks/stacksManager'
+import { ChangeSetsManager } from '../../../../../awsService/cloudformation/stacks/changeSetsManager'
+import { StackSummary } from '@aws-sdk/client-cloudformation'
+
+describe('StacksNode', function () {
+    let stacksNode: StacksNode
+    let mockStacksManager: sinon.SinonStubbedInstance<StacksManager>
+    let mockChangeSetsManager: ChangeSetsManager
+    let sandbox: sinon.SinonSandbox
+
+    beforeEach(function () {
+        sandbox = sinon.createSandbox()
+        mockStacksManager = {
+            get: sandbox.stub(),
+            hasMore: sandbox.stub(),
+            isLoaded: sandbox.stub(),
+            ensureLoaded: sandbox.stub(),
+            loadMoreStacks: sandbox.stub(),
+        } as any
+        mockChangeSetsManager = {} as ChangeSetsManager
+    })
+
+    afterEach(function () {
+        sandbox.restore()
+    })
+
+    describe('constructor', function () {
+        it('should set correct properties when not loaded', function () {
+            mockStacksManager.get.returns([])
+            mockStacksManager.hasMore.returns(false)
+            mockStacksManager.isLoaded.returns(false)
+
+            stacksNode = new StacksNode(mockStacksManager as any, mockChangeSetsManager)
+
+            assert.strictEqual(stacksNode.label, 'Stacks')
+            assert.strictEqual(stacksNode.collapsibleState, TreeItemCollapsibleState.Collapsed)
+            assert.strictEqual(stacksNode.description, undefined)
+            assert.strictEqual(stacksNode.contextValue, 'stackSection')
+        })
+
+        it('should set description when loaded', function () {
+            const mockStacks: StackSummary[] = [
+                { StackName: 'stack-1', StackStatus: 'CREATE_COMPLETE' } as StackSummary,
+                { StackName: 'stack-2', StackStatus: 'UPDATE_COMPLETE' } as StackSummary,
+            ]
+            mockStacksManager.get.returns(mockStacks)
+            mockStacksManager.hasMore.returns(false)
+            mockStacksManager.isLoaded.returns(true)
+
+            stacksNode = new StacksNode(mockStacksManager as any, mockChangeSetsManager)
+
+            assert.strictEqual(stacksNode.description, '(2)')
+            assert.strictEqual(stacksNode.contextValue, 'stackSection')
+        })
+
+        it('should set contextValue to stackSectionWithMore when hasMore', function () {
+            const mockStacks: StackSummary[] = [
+                { StackName: 'stack-1', StackStatus: 'CREATE_COMPLETE' } as StackSummary,
+            ]
+            mockStacksManager.get.returns(mockStacks)
+            mockStacksManager.hasMore.returns(true)
+            mockStacksManager.isLoaded.returns(true)
+
+            stacksNode = new StacksNode(mockStacksManager as any, mockChangeSetsManager)
+
+            assert.strictEqual(stacksNode.description, '(1+)')
+            assert.strictEqual(stacksNode.contextValue, 'stackSectionWithMore')
+        })
+    })
+
+    describe('getChildren', function () {
+        beforeEach(function () {
+            mockStacksManager.get.returns([])
+            mockStacksManager.hasMore.returns(false)
+            mockStacksManager.isLoaded.returns(false)
+            stacksNode = new StacksNode(mockStacksManager as any, mockChangeSetsManager)
+        })
+
+        it('should call ensureLoaded', async function () {
+            mockStacksManager.ensureLoaded.resolves()
+
+            await stacksNode.getChildren()
+
+            assert.strictEqual(mockStacksManager.ensureLoaded.calledOnce, true)
+        })
+
+        it('should return StackNode for each stack', async function () {
+            const mockStacks: StackSummary[] = [
+                { StackName: 'stack-1', StackStatus: 'CREATE_COMPLETE' } as StackSummary,
+                { StackName: 'stack-2', StackStatus: 'UPDATE_COMPLETE' } as StackSummary,
+            ]
+            mockStacksManager.ensureLoaded.resolves()
+            mockStacksManager.get.returns(mockStacks)
+            mockStacksManager.hasMore.returns(false)
+            mockStacksManager.isLoaded.returns(true)
+
+            const children = await stacksNode.getChildren()
+
+            assert.strictEqual(children.length, 2)
+            assert.strictEqual(children[0].label, 'stack-1')
+            assert.strictEqual(children[1].label, 'stack-2')
+        })
+
+        it('should include LoadMoreStacksNode when hasMore', async function () {
+            const mockStacks: StackSummary[] = [
+                { StackName: 'stack-1', StackStatus: 'CREATE_COMPLETE' } as StackSummary,
+            ]
+            mockStacksManager.ensureLoaded.resolves()
+            mockStacksManager.get.returns(mockStacks)
+            mockStacksManager.hasMore.returns(true)
+            mockStacksManager.isLoaded.returns(true)
+
+            const children = await stacksNode.getChildren()
+
+            assert.strictEqual(children.length, 2)
+            assert.strictEqual(children[0].label, 'stack-1')
+            assert.strictEqual(children[1].label, '[Load More...]')
+            assert.strictEqual(children[1].contextValue, 'loadMoreStacks')
+        })
+
+        it('should update node description after load', async function () {
+            const mockStacks: StackSummary[] = [
+                { StackName: 'stack-1', StackStatus: 'CREATE_COMPLETE' } as StackSummary,
+            ]
+            mockStacksManager.ensureLoaded.resolves()
+            mockStacksManager.get.returns(mockStacks)
+            mockStacksManager.hasMore.returns(false)
+            mockStacksManager.isLoaded.returns(true)
+
+            await stacksNode.getChildren()
+
+            assert.strictEqual(stacksNode.description, '(1)')
+            assert.strictEqual(stacksNode.contextValue, 'stackSection')
+        })
+
+        it('should return empty array when no stacks', async function () {
+            mockStacksManager.ensureLoaded.resolves()
+            mockStacksManager.get.returns([])
+            mockStacksManager.hasMore.returns(false)
+            mockStacksManager.isLoaded.returns(true)
+
+            const children = await stacksNode.getChildren()
+
+            assert.strictEqual(children.length, 0)
+        })
+    })
+
+    describe('loadMoreStacks', function () {
+        beforeEach(function () {
+            mockStacksManager.get.returns([])
+            mockStacksManager.hasMore.returns(false)
+            mockStacksManager.isLoaded.returns(false)
+            stacksNode = new StacksNode(mockStacksManager as any, mockChangeSetsManager)
+        })
+
+        it('should call stacksManager.loadMoreStacks', async function () {
+            mockStacksManager.loadMoreStacks.resolves()
+
+            await stacksNode.loadMoreStacks()
+
+            assert.strictEqual(mockStacksManager.loadMoreStacks.calledOnce, true)
+        })
+
+        it('should update node description after loading more', async function () {
+            const mockStacks: StackSummary[] = [
+                { StackName: 'stack-1', StackStatus: 'CREATE_COMPLETE' } as StackSummary,
+                { StackName: 'stack-2', StackStatus: 'UPDATE_COMPLETE' } as StackSummary,
+            ]
+            mockStacksManager.loadMoreStacks.resolves()
+            mockStacksManager.get.returns(mockStacks)
+            mockStacksManager.hasMore.returns(true)
+            mockStacksManager.isLoaded.returns(true)
+
+            await stacksNode.loadMoreStacks()
+
+            assert.strictEqual(stacksNode.description, '(2+)')
+            assert.strictEqual(stacksNode.contextValue, 'stackSectionWithMore')
+        })
+
+        it('should update contextValue when no more stacks', async function () {
+            const mockStacks: StackSummary[] = [
+                { StackName: 'stack-1', StackStatus: 'CREATE_COMPLETE' } as StackSummary,
+            ]
+            mockStacksManager.loadMoreStacks.resolves()
+            mockStacksManager.get.returns(mockStacks)
+            mockStacksManager.hasMore.returns(false)
+            mockStacksManager.isLoaded.returns(true)
+
+            await stacksNode.loadMoreStacks()
+
+            assert.strictEqual(stacksNode.description, '(1)')
+            assert.strictEqual(stacksNode.contextValue, 'stackSection')
+        })
+    })
+})