From 4b79ecc3d93655afe37a3f2d03f0c1d60c513b34 Mon Sep 17 00:00:00 2001 From: Matt Irvine Date: Mon, 8 Oct 2018 15:42:13 -0700 Subject: [PATCH] Fix bug disconnecting during stuck OE operation (#2773) --- .../common/objectExplorerService.ts | 22 +++++++++++-- .../objectExplorer/viewlet/serverTreeView.ts | 7 +++-- .../connection/objectExplorerService.test.ts | 31 ++++++++++++++++--- 3 files changed, 50 insertions(+), 10 deletions(-) diff --git a/src/sql/parts/objectExplorer/common/objectExplorerService.ts b/src/sql/parts/objectExplorer/common/objectExplorerService.ts index 46ab83a7f1..8680e0b864 100644 --- a/src/sql/parts/objectExplorer/common/objectExplorerService.ts +++ b/src/sql/parts/objectExplorer/common/objectExplorerService.ts @@ -53,7 +53,7 @@ export interface IObjectExplorerService { updateObjectExplorerNodes(connectionProfile: IConnectionProfile): Promise; - deleteObjectExplorerNode(connection: IConnectionProfile): void; + deleteObjectExplorerNode(connection: IConnectionProfile): Thenable; onUpdateObjectExplorerNodes: Event; @@ -142,16 +142,17 @@ export class ObjectExplorerService implements IObjectExplorerService { }); } - public deleteObjectExplorerNode(connection: IConnectionProfile): void { + public deleteObjectExplorerNode(connection: IConnectionProfile): Thenable { let self = this; var connectionUri = connection.id; var nodeTree = this._activeObjectExplorerNodes[connectionUri]; if (nodeTree) { - self.closeSession(connection.providerName, nodeTree.getSession()).then(() => { + return self.closeSession(connection.providerName, nodeTree.getSession()).then(() => { delete self._activeObjectExplorerNodes[connectionUri]; delete self._sessions[nodeTree.getSession().sessionId]; }); } + return Promise.resolve(); } /** @@ -328,6 +329,21 @@ export class ObjectExplorerService implements IObjectExplorerService { } public closeSession(providerId: string, session: sqlops.ObjectExplorerSession): Thenable { + // Complete any requests that are still open for the session + let sessionStatus = this._sessions[session.sessionId]; + if (sessionStatus && sessionStatus.nodes) { + Object.entries(sessionStatus.nodes).forEach(([nodePath, nodeStatus]: [string, NodeStatus]) => { + if (nodeStatus.expandEmitter) { + nodeStatus.expandEmitter.fire({ + sessionId: session.sessionId, + nodes: [], + nodePath: nodePath, + errorMessage: undefined + }); + } + }); + } + let provider = this._providers[providerId]; if (provider) { return provider.closeSession({ diff --git a/src/sql/parts/objectExplorer/viewlet/serverTreeView.ts b/src/sql/parts/objectExplorer/viewlet/serverTreeView.ts index cbc77e8ce4..38c9dc49b5 100644 --- a/src/sql/parts/objectExplorer/viewlet/serverTreeView.ts +++ b/src/sql/parts/objectExplorer/viewlet/serverTreeView.ts @@ -231,9 +231,10 @@ export class ServerTreeView { if (connection) { var conn = this.getConnectionInTreeInput(connection.id); if (conn) { - this._objectExplorerService.deleteObjectExplorerNode(conn); - this._tree.collapse(conn); - this._tree.refresh(conn); + this._objectExplorerService.deleteObjectExplorerNode(conn).then(() => { + this._tree.collapse(conn); + this._tree.refresh(conn); + }); } } } diff --git a/src/sqltest/parts/connection/objectExplorerService.test.ts b/src/sqltest/parts/connection/objectExplorerService.test.ts index 6641083cea..67cc030fb5 100644 --- a/src/sqltest/parts/connection/objectExplorerService.test.ts +++ b/src/sqltest/parts/connection/objectExplorerService.test.ts @@ -447,10 +447,11 @@ suite('SQL Object Explorer Service tests', () => { objectExplorerService.updateObjectExplorerNodes(connection).then(() => { var treeNode = objectExplorerService.getObjectExplorerNode(connection); assert.equal(treeNode !== null && treeNode !== undefined, true); - objectExplorerService.deleteObjectExplorerNode(connection); - treeNode = objectExplorerService.getObjectExplorerNode(connection); - assert.equal(treeNode === null || treeNode === undefined, true); - done(); + objectExplorerService.deleteObjectExplorerNode(connection).then(() => { + treeNode = objectExplorerService.getObjectExplorerNode(connection); + assert.equal(treeNode === null || treeNode === undefined, true); + done(); + }); }, err => { // Must call done here so test indicates it's finished if errors occur done(err); @@ -754,6 +755,28 @@ suite('SQL Object Explorer Service tests', () => { }); }); + test('Session can be closed even if expand requests are pending', async () => { + const providerId = 'MSSQL'; + + // Set up the session + await objectExplorerService.createNewSession(providerId, connection); + objectExplorerService.onSessionCreated(1, objectExplorerSession); + + // Set up the provider to not respond to the second expand request, simulating a request that takes a long time to complete + const nodePath = objectExplorerSession.rootNode.nodePath; + sqlOEProvider.setup(x => x.expandNode(TypeMoq.It.is(x => x.nodePath === nodePath))).callback(() => { }).returns(() => TPromise.as(true)); + + // If I queue a second expand request (the first completes normally because of the original mock) and then close the session + await objectExplorerService.expandNode(providerId, objectExplorerSession, objectExplorerSession.rootNode.nodePath); + let expandPromise = objectExplorerService.expandNode(providerId, objectExplorerSession, objectExplorerSession.rootNode.nodePath); + let closeSessionResult = await objectExplorerService.closeSession(providerId, objectExplorerSession); + + // Then the expand request has completed and the session is closed + let expandResult = await expandPromise; + assert.equal(expandResult.nodes.length, 0); + assert.equal(closeSessionResult.success, true); + }); + test('resolveTreeNodeChildren refreshes a node if it currently has an error', async () => { await objectExplorerService.createNewSession('MSSQL', connection); objectExplorerService.onSessionCreated(1, objectExplorerSession);