VSCODE-155: Allow connecting to a new connection when already connecting (#156)

Author: Anemy

src/connectionController.ts

@@ -20,7 +20,6 @@ const MAX_CONNECTION_NAME_LENGTH = 512;
 export enum DataServiceEventTypes {
   CONNECTIONS_DID_CHANGE = 'CONNECTIONS_DID_CHANGE',
   ACTIVE_CONNECTION_CHANGED = 'ACTIVE_CONNECTION_CHANGED',
-  ACTIVE_CONNECTION_CHANGING = 'ACTIVE_CONNECTION_CHANGING'
 }
 
 export enum ConnectionTypes {
@@ -55,6 +54,12 @@ export default class ConnectionController {
   _activeConnectionModel: null | ConnectionModelType = null;
   private _currentConnectionId: null | string = null;
 
+  // When we are connecting to a server we save a connection version to
+  // the request. That way if a new connection attempt is made while
+  // the connection is being established, we know we can ignore the
+  // request when it is completed so we don't have two live connections at once.
+  private _connectingVersion = 0;
+
   private _connecting = false;
   private _connectingConnectionId: null | string = null;
   private _disconnecting = false;
@@ -231,32 +236,24 @@ export default class ConnectionController {
   public sendTelemetry(
     newDataService: DataServiceType,
     connectionType: ConnectionTypes
-  ) {
+  ): void {
     // Send metrics to Segment
     this._telemetryController.trackNewConnection(
       newDataService,
       connectionType
     );
   }
 
-  public parseNewConnectionAndConnect = (
+  public parseNewConnection = (
     newConnectionModel: ConnectionModelType
-  ): Promise<boolean> => {
+  ): ConnectionModelType => {
     // Here we re-parse the connection, as it can be loaded from storage or
     // passed by the connection model without the class methods.
-    let connectionModel: ConnectionModelType;
-
-    try {
-      connectionModel = new Connection(newConnectionModel);
-    } catch (error) {
-      vscode.window.showErrorMessage(`Unable to load connection: ${error}`);
-      return Promise.reject(new Error(`Unable to load connection: ${error}`));
-    }
-
-    return this.saveNewConnectionAndConnect(
-      connectionModel,
-      ConnectionTypes.CONNECTION_FORM
+    const connectionModel: ConnectionModelType = new Connection(
+      newConnectionModel
     );
+
+    return connectionModel;
   };
 
   public saveNewConnectionAndConnect = async (
@@ -330,27 +327,19 @@ export default class ConnectionController {
       }).instanceId
     );
 
-    if (this._connecting) {
-      return Promise.reject(
-        new Error('Unable to connect: already connecting.')
-      );
-    }
-
-    if (this._disconnecting) {
-      return Promise.reject(
-        new Error('Unable to connect: currently disconnecting.')
-      );
-    }
-
-    if (this._activeDataService) {
-      await this.disconnect();
-    }
+    // Store a version of this connection, so we can see when the conection
+    // is successful if it is still the most recent connection attempt.
+    this._connectingVersion++;
+    const connectingAttemptVersion = this._connectingVersion;
 
     this._connecting = true;
     this._connectingConnectionId = connectionId;
 
     this.eventEmitter.emit(DataServiceEventTypes.CONNECTIONS_DID_CHANGE);
-    this.eventEmitter.emit(DataServiceEventTypes.ACTIVE_CONNECTION_CHANGING);
+
+    if (this._activeDataService) {
+      await this.disconnect();
+    }
 
     this._statusView.showMessage('Connecting to MongoDB...');
 
@@ -361,6 +350,22 @@ export default class ConnectionController {
       const newDataService: DataServiceType = new DataService(connectionModel);
 
       newDataService.connect((err: Error | undefined) => {
+        if (
+          connectingAttemptVersion !== this._connectingVersion ||
+          !this._connections[connectionId]
+        ) {
+          // If the current attempt is no longer the most recent attempt
+          // or the connection no longer exists we silently end the connection
+          // and return.
+          try {
+            newDataService.disconnect(() => {});
+          } catch (e) {
+            /* */
+          }
+
+          return resolve(false);
+        }
+
         this._statusView.hideMessage();
 
         if (err) {
@@ -392,7 +397,7 @@ export default class ConnectionController {
 
   public connectWithConnectionId = (connectionId: string): Promise<boolean> => {
     if (this._connections[connectionId]) {
-      let connectionModel: any;
+      let connectionModel: ConnectionModelType;
 
       try {
         const savedConnectionModel = this._connections[connectionId]
@@ -433,36 +438,28 @@ export default class ConnectionController {
       this._currentConnectionId
     );
 
-    if (this._disconnecting) {
+    if (!this._activeDataService) {
       vscode.window.showErrorMessage(
-        'Unable to disconnect: already disconnecting from an instance.'
+        'Unable to disconnect: no active connection.'
       );
 
       return Promise.resolve(false);
     }
 
-    if (this._connecting) {
-      // TODO: The desired UX here may be for the connection to be interrupted.
-      vscode.window.showErrorMessage(
-        'Unable to disconnect: currently connecting to an instance.'
-      );
+    const dataServiceToDisconnectFrom = this._activeDataService;
 
-      return Promise.resolve(false);
-    }
+    this._activeDataService = null;
+    this._currentConnectionId = null;
+    this._activeConnectionModel = null;
+    this._disconnecting = true;
+
+    this.eventEmitter.emit(DataServiceEventTypes.CONNECTIONS_DID_CHANGE);
+    this.eventEmitter.emit(DataServiceEventTypes.ACTIVE_CONNECTION_CHANGED);
 
     // Disconnect from the active connection.
     return new Promise<boolean>((resolve) => {
-      if (!this._activeDataService) {
-        vscode.window.showErrorMessage(
-          'Unable to disconnect: no active connection.'
-        );
-
-        return resolve(false);
-      }
-
-      this._disconnecting = true;
       this._statusView.showMessage('Disconnecting from current connection...');
-      this._activeDataService.disconnect((err: Error | undefined): void => {
+      dataServiceToDisconnectFrom.disconnect((err: Error | undefined): void => {
         if (err) {
           // Show an error, however we still reset the active connection to free up the extension.
           vscode.window.showErrorMessage(
@@ -471,17 +468,9 @@ export default class ConnectionController {
         } else {
           vscode.window.showInformationMessage('MongoDB disconnected.');
         }
-
-        this._activeDataService = null;
-        this._currentConnectionId = null;
-        this._activeConnectionModel = null;
-
         this._disconnecting = false;
         this._statusView.hideMessage();
 
-        this.eventEmitter.emit(DataServiceEventTypes.CONNECTIONS_DID_CHANGE);
-        this.eventEmitter.emit(DataServiceEventTypes.ACTIVE_CONNECTION_CHANGED);
-
         return resolve(true);
       });
     });
@@ -506,24 +495,6 @@ export default class ConnectionController {
 
   // Prompts the user to remove the connection then removes it on affirmation.
   public async removeMongoDBConnection(connectionId: string): Promise<boolean> {
-    // Ensure we aren't currently connecting.
-    if (this._connecting) {
-      vscode.window.showErrorMessage(
-        'Unable to remove connection: currently connecting.'
-      );
-
-      return Promise.resolve(false);
-    }
-
-    // Ensure we aren't currently disconnecting.
-    if (this._disconnecting) {
-      vscode.window.showErrorMessage(
-        'Unable to remove connection: currently disconnecting.'
-      );
-
-      return Promise.resolve(false);
-    }
-
     if (!this._connections[connectionId]) {
       // No active connection(s) to remove.
       vscode.window.showErrorMessage('Connection does not exist.');
@@ -545,6 +516,11 @@ export default class ConnectionController {
       await this.disconnect();
     }
 
+    if (!this._connections[connectionId]) {
+      // If the connection was removed while we were disconnecting we resolve.
+      return Promise.resolve(false);
+    }
+
     await this.removeSavedConnection(connectionId);
 
     vscode.window.showInformationMessage('MongoDB connection removed.');
@@ -555,24 +531,6 @@ export default class ConnectionController {
   public async onRemoveMongoDBConnection(): Promise<boolean> {
     log.info('mdb.removeConnection command called');
 
-    // Ensure we aren't currently connecting.
-    if (this._connecting) {
-      vscode.window.showErrorMessage(
-        'Unable to remove connection: currently connecting.'
-      );
-
-      return Promise.resolve(false);
-    }
-
-    // Ensure we aren't currently disconnecting.
-    if (this._disconnecting) {
-      vscode.window.showErrorMessage(
-        'Unable to remove connection: currently disconnecting.'
-      );
-
-      return Promise.resolve(false);
-    }
-
     const connectionIds = Object.keys(this._connections);
 
     if (connectionIds.length === 0) {
@@ -591,13 +549,13 @@ export default class ConnectionController {
     const connectionNameToRemove:
       | string
       | undefined = await vscode.window.showQuickPick(
-      connectionIds.map(
-        (id, index) => `${index + 1}: ${this._connections[id].name}`
-      ),
-      {
-        placeHolder: 'Choose a connection to remove...'
-      }
-    );
+        connectionIds.map(
+          (id, index) => `${index + 1}: ${this._connections[id].name}`
+        ),
+        {
+          placeHolder: 'Choose a connection to remove...'
+        }
+      );
 
     if (!connectionNameToRemove) {
       return Promise.resolve(false);
@@ -729,13 +687,6 @@ export default class ConnectionController {
 
     return !!this._connections[connectionId];
   }
-  public getConnectingConnectionName(): string | null {
-    if (this._connectingConnectionId === null) {
-      return null;
-    }
-
-    return this._connections[this._connectingConnectionId].name;
-  }
   public getConnectingConnectionId(): string | null {
     return this._connectingConnectionId;
   }
@@ -765,6 +716,11 @@ export default class ConnectionController {
     this._connecting = false;
     this._disconnecting = false;
     this._connectingConnectionId = '';
+    this._connectingVersion = 0;
+  }
+
+  public getConnectingVersion(): number {
+    return this._connectingVersion;
   }
 
   public setActiveConnection(newActiveConnection: any): void {

src/explorer/mdbConnectionsTreeItem.ts

@@ -88,28 +88,6 @@ export default class MDBConnectionsTreeItem extends vscode.TreeItem
       );
     });
 
-    if (
-      this._connectionController.isConnecting() &&
-      !this._connectionController.isConnectionWithIdSaved(
-        this._connectionController.getConnectingConnectionId()
-      )
-    ) {
-      const notYetEstablishConnectionTreeItem = new vscode.TreeItem(
-        this._connectionController.getConnectingConnectionName() ||
-          'New Connection'
-      );
-
-      notYetEstablishConnectionTreeItem.description = 'connecting...';
-
-      // When we're connecting to a new connection we add simple node showing the connecting status.
-      return Promise.resolve(
-        sortTreeItemsByLabel([
-          ...Object.values(this._connectionTreeItems),
-          notYetEstablishConnectionTreeItem
-        ])
-      );
-    }
-
     return Promise.resolve(
       sortTreeItemsByLabel(Object.values(this._connectionTreeItems))
     );