diff --git a/packages/core/src/awsService/cloudformation/auth/credentials.ts b/packages/core/src/awsService/cloudformation/auth/credentials.ts index 58a732807d6..95da665c47b 100644 --- a/packages/core/src/awsService/cloudformation/auth/credentials.ts +++ b/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() } diff --git a/packages/core/src/awsService/cloudformation/explorer/nodes/stacksNode.ts b/packages/core/src/awsService/cloudformation/explorer/nodes/stacksNode.ts index 1b01e5456b1..b601efcc122 100644 --- a/packages/core/src/awsService/cloudformation/explorer/nodes/stacksNode.ts +++ b/packages/core/src/awsService/cloudformation/explorer/nodes/stacksNode.ts @@ -33,6 +33,7 @@ export class StacksNode extends AWSTreeNodeBase { } public override async getChildren(): Promise { + await this.stacksManager.ensureLoaded() this.updateNode() const stacks = this.stacksManager.get() const nodes = stacks.map((stack: StackSummary) => new StackNode(stack, this.changeSetsManager)) @@ -40,10 +41,15 @@ export class StacksNode extends AWSTreeNodeBase { } 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 { diff --git a/packages/core/src/awsService/cloudformation/stacks/stacksManager.ts b/packages/core/src/awsService/cloudformation/stacks/stacksManager.ts index 658bb2eaacd..360e799f801 100644 --- a/packages/core/src/awsService/cloudformation/stacks/stacksManager.ts +++ b/packages/core/src/awsService/cloudformation/stacks/stacksManager.ts @@ -23,7 +23,6 @@ type ListStacksResult = { } const ListStacksRequest = new RequestType('aws/cfn/stacks') -const PollIntervalMs = 1000 type StacksChangeListener = (stacks: StackSummary[]) => void @@ -31,7 +30,7 @@ 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,6 +109,7 @@ 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 = [] @@ -113,9 +117,6 @@ export class StacksManager implements Disposable { } finally { await setContext('aws.cloudformation.refreshingStacks', false) this.notifyListeners() - if (this.stacks.length === 0) { - this.stopPolling() - } } } diff --git a/packages/core/src/test/awsService/cloudformation/auth/credentials.test.ts b/packages/core/src/test/awsService/cloudformation/auth/credentials.test.ts index b9a2bf6997d..138c5812288 100644 --- a/packages/core/src/test/awsService/cloudformation/auth/credentials.test.ts +++ b/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) + }) }) }) diff --git a/packages/core/src/test/awsService/cloudformation/explorer/nodes/stacksNode.test.ts b/packages/core/src/test/awsService/cloudformation/explorer/nodes/stacksNode.test.ts new file mode 100644 index 00000000000..d37d847432d --- /dev/null +++ b/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 + 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') + }) + }) +}) diff --git a/packages/core/src/test/awsService/cloudformation/stacks/stacksManager.test.ts b/packages/core/src/test/awsService/cloudformation/stacks/stacksManager.test.ts index 50fea4913b8..6140cdc4539 100644 --- a/packages/core/src/test/awsService/cloudformation/stacks/stacksManager.test.ts +++ b/packages/core/src/test/awsService/cloudformation/stacks/stacksManager.test.ts @@ -74,4 +74,80 @@ describe('StacksManager', () => { assert.deepStrictEqual(stacksBefore, stacksAfter) }) }) + + describe('lazy loading', () => { + it('should not load stacks on construction', () => { + assert.strictEqual(mockClient.sendRequest.called, false) + }) + + it('should return empty array when not loaded', () => { + const stacks = manager.get() + assert.deepStrictEqual(stacks, []) + }) + + it('should report not loaded initially', () => { + assert.strictEqual(manager.isLoaded(), false) + }) + + it('should load stacks on ensureLoaded', async () => { + await manager.ensureLoaded() + assert.strictEqual(mockClient.sendRequest.calledOnce, true) + }) + + it('should not reload on subsequent ensureLoaded calls', async () => { + await manager.ensureLoaded() + await manager.ensureLoaded() + assert.strictEqual(mockClient.sendRequest.calledOnce, true) + }) + + it('should report loaded after ensureLoaded', async () => { + await manager.ensureLoaded() + assert.strictEqual(manager.isLoaded(), true) + }) + + it('should return stacks after ensureLoaded', async () => { + await manager.ensureLoaded() + const stacks = manager.get() + assert.strictEqual(stacks.length, 2) + assert.strictEqual(stacks[0].StackName, 'stack-1') + }) + }) + + describe('clear', () => { + beforeEach(async () => { + await manager.ensureLoaded() + }) + + it('should clear stacks', () => { + manager.clear() + const stacks = manager.get() + assert.deepStrictEqual(stacks, []) + }) + + it('should reset loaded state', () => { + manager.clear() + assert.strictEqual(manager.isLoaded(), false) + }) + + it('should clear nextToken', () => { + manager.clear() + assert.strictEqual(manager.hasMore(), false) + }) + + it('should notify listeners', () => { + let listenerCalled = false + manager.addListener(() => { + listenerCalled = true + }) + manager.clear() + assert.strictEqual(listenerCalled, true) + }) + + it('should allow reload after clear', async () => { + manager.clear() + mockClient.sendRequest.resetHistory() + await manager.ensureLoaded() + assert.strictEqual(mockClient.sendRequest.calledOnce, true) + }) + }) }) diff --git a/packages/toolkit/.changes/next-release/Bug Fix-d34fd326-d62d-495c-8f94-d3aa3c773412.json b/packages/toolkit/.changes/next-release/Bug Fix-d34fd326-d62d-495c-8f94-d3aa3c773412.json new file mode 100644 index 00000000000..c684a3d6526 --- /dev/null +++ b/packages/toolkit/.changes/next-release/Bug Fix-d34fd326-d62d-495c-8f94-d3aa3c773412.json @@ -0,0 +1,4 @@ +{ + "type": "Bug Fix", + "description": "CloudFormation: prevent eager loading of CloudFormation stacks" +}