Fix persisting connection even when file is renamed (#540)

* added new onDidRename function for connection reuse

* detection for renamed file logic started

* simplifying logic for passing connection between a renamed file

* adding a timer check for verifying a file was actually renamed

* adding a constant for the threshold of time for a rename to occur

* reworked previous logic to fit all cases

* updating comments to match function

* adding comments to untitled and rename logic

* test framework setup

* setting up functions to be accessible for testing

* tests to cover onDidCloseTestDoc

* merged into one function called transferFileConnection

src/controllers/connectionManager.ts

@@ -597,17 +597,17 @@ export default class ConnectionManager {
         }
     }
 
-    public onUntitledFileSaved(untitledUri: string, savedUri: string): void {
+    public transferFileConnection(oldFileUri: string, newFileUri: string): void {
         // Is the new file connected or the old file not connected?
-        if (!this.isConnected(untitledUri) || this.isConnected(savedUri)) {
+        if (!this.isConnected(oldFileUri) || this.isConnected(newFileUri)) {
             return;
         }
 
         // Connect the saved uri and disconnect the untitled uri on successful connection
-        let creds: Interfaces.IConnectionCredentials = this._connections[untitledUri].credentials;
-        this.connect(savedUri, creds).then(result => {
+        let creds: Interfaces.IConnectionCredentials = this._connections[oldFileUri].credentials;
+        this.connect(newFileUri, creds).then(result => {
             if (result) {
-                this.disconnect(untitledUri);
+                this.disconnect(oldFileUri);
             }
         });
     }

src/controllers/mainController.ts

@@ -36,6 +36,8 @@ export default class MainController implements vscode.Disposable {
     private _initialized: boolean = false;
     private _lastSavedUri: string;
     private _lastSavedTimer: Utils.Timer;
+    private _lastOpenedUri: string;
+    private _lastOpenedTimer: Utils.Timer;
 
     /**
      * The main controller constructor
@@ -300,6 +302,11 @@ export default class MainController implements vscode.Disposable {
         return this._connectionMgr;
     }
 
+    public set connectionManager(connectionManager: ConnectionManager) {
+        this._connectionMgr = connectionManager;
+    }
+
+
     /**
      * Verifies the extension is initilized and if not shows an error message
      */
@@ -378,49 +385,72 @@ export default class MainController implements vscode.Disposable {
 
     /**
      * Called by VS Code when a text document closes. This will dispatch calls to other
-     * controllers as needed. Determines if this was a closed file or if it was an instance
-     * where a file was saved to disk after being an untitled file.
+     * controllers as needed. Determines if this was a normal closed file, a untitled closed file,
+     * or a renamed file
      * @param doc The document that was closed
      */
-    private onDidCloseTextDocument(doc: vscode.TextDocument): void {
+    public onDidCloseTextDocument(doc: vscode.TextDocument): void {
         let closedDocumentUri: string = doc.uri.toString();
         let closedDocumentUriScheme: string = doc.uri.scheme;
 
-        // Did we save a document before this close event? Was it an untitled document?
-        if (this._lastSavedUri && this._lastSavedTimer && closedDocumentUriScheme === Constants.untitledScheme) {
-            // Stop the save timer
+        // Stop timers if they have been started
+        if (this._lastSavedTimer) {
             this._lastSavedTimer.end();
+        }
 
-            // Check that we saved a document *just* before this close event
-            // If so, then we saved an untitled document and need to update where necessary
-            if (this._lastSavedTimer.getDuration() < Constants.untitledSaveTimeThreshold) {
-                this._connectionMgr.onUntitledFileSaved(closedDocumentUri, this._lastSavedUri);
-            }
+        if (this._lastOpenedTimer) {
+            this._lastOpenedTimer.end();
+        }
+
+        // Determine which event caused this close event
+
+        // If there was a saveTextDoc event just before this closeTextDoc event and it
+        // was untitled then we know it was an untitled save
+        if (this._lastSavedUri &&
+                closedDocumentUriScheme === Constants.untitledScheme &&
+                this._lastSavedTimer.getDuration() < Constants.untitledSaveTimeThreshold) {
+            // Untitled file was saved and connection will be transfered
+            this._connectionMgr.transferFileConnection(closedDocumentUri, this._lastSavedUri);
+
+        // If there was an openTextDoc event just before this closeTextDoc event then we know it was a rename
+        } else if (this._lastOpenedUri &&
+                this._lastOpenedTimer.getDuration() < Constants.renamedOpenTimeThreshold) {
+            // File was renamed and connection will be transfered
+            this._connectionMgr.transferFileConnection(closedDocumentUri, this._lastOpenedUri);
 
-            // Reset the save timer
-            this._lastSavedTimer = undefined;
-            this._lastSavedUri = undefined;
         } else {
-            // Pass along the close event to the other handlers
+            // Pass along the close event to the other handlers for a normal closed file
             this._connectionMgr.onDidCloseTextDocument(doc);
             this._outputContentProvider.onDidCloseTextDocument(doc);
         }
+
+
+        // Reset special case timers and events
+        this._lastSavedUri = undefined;
+        this._lastSavedTimer = undefined;
+        this._lastOpenedTimer = undefined;
+        this._lastOpenedUri = undefined;
     }
 
     /**
      * Called by VS Code when a text document is opened. Checks if a SQL file was opened
      * to enable features of our extension for the document.
      */
-    private onDidOpenTextDocument(doc: vscode.TextDocument): void {
+    public onDidOpenTextDocument(doc: vscode.TextDocument): void {
         this._connectionMgr.onDidOpenTextDocument(doc);
+
+        // Setup properties incase of rename
+        this._lastOpenedTimer = new Utils.Timer();
+        this._lastOpenedTimer.start();
+        this._lastOpenedUri = doc.uri.toString();
     }
 
     /**
      * Called by VS Code when a text document is saved. Will trigger a timer to
      * help determine if the file was a file saved from an untitled file.
      * @param doc The document that was saved
      */
-    private onDidSaveTextDocument(doc: vscode.TextDocument): void {
+    public onDidSaveTextDocument(doc: vscode.TextDocument): void {
         let savedDocumentUri: string = doc.uri.toString();
 
         // Keep track of which file was last saved and when for detecting the case when we save an untitled document to disk

src/models/constants.ts

@@ -195,6 +195,7 @@ export const extensionNotInitializedError = 'Unable to execute the command while
 
 export const untitledScheme = 'untitled';
 export const untitledSaveTimeThreshold = 10.0;
+export const renamedOpenTimeThreshold = 10.0;
 
 export const msgChangeLanguageMode = 'To use this command, you must set the language to \"SQL\". Confirm to change language mode.';
 export const timeToWaitForLanguageModeChange = 10000.0;

test/mainController.test.ts

@@ -0,0 +1,74 @@
+'use strict';
+import * as TypeMoq from 'typemoq';
+
+import vscode = require('vscode');
+import MainController from '../src/controllers/mainController';
+import ConnectionManager from '../src/controllers/connectionManager';
+import * as Extension from '../src/extension';
+import Constants = require('../src/models/constants');
+
+// import assert = require('assert');
+
+suite('MainController Tests', () => {
+    let document: vscode.TextDocument;
+    let mainController: MainController;
+    let connectionManager: TypeMoq.Mock<ConnectionManager>;
+
+    setup(() => {
+        // Setup a standard document
+        document = <vscode.TextDocument> {
+            uri : {
+                toString(skipEncoding?: boolean): string {
+                    return 'testingURI.sql';
+                }
+            }
+        };
+
+        connectionManager = TypeMoq.Mock.ofType(ConnectionManager);
+
+        mainController = Extension.getController();
+        mainController.connectionManager = connectionManager.object;
+        connectionManager.setup(x => x.onDidCloseTextDocument(TypeMoq.It.isAny()));
+        connectionManager.setup(x => x.transferFileConnection(TypeMoq.It.isAny(), TypeMoq.It.isAny()));
+    });
+
+    test('onDidCloseTextDocument should propogate onDidCloseTextDocument to connectionManager' , done => {
+        mainController.onDidCloseTextDocument(document);
+        try {
+            connectionManager.verify(x => x.onDidCloseTextDocument(TypeMoq.It.isAny()), TypeMoq.Times.once());
+            done();
+        } catch (err) {
+            done(new Error(err));
+        }
+    });
+
+    test('onDidCloseTextDocument should call renamedDoc function when rename occurs' , done => {
+        // A renamed doc constitutes an openDoc event directly followed by a closeDoc event
+        mainController.onDidOpenTextDocument(document);
+        mainController.onDidCloseTextDocument(document);
+
+        // Verify renameDoc function was called
+        try {
+            connectionManager.verify(x => x.transferFileConnection(TypeMoq.It.isAny(), TypeMoq.It.isAny()), TypeMoq.Times.once());
+            done();
+        } catch (err) {
+            done(new Error(err));
+        }
+    });
+
+    test('onDidCloseTextDocument should untitledDoc function when an untitled file is saved' , done => {
+        // Scheme of older doc must be untitled
+        document.uri.scheme = Constants.untitledScheme;
+
+        // A save untitled doc constitutes an saveDoc event directly followed by a closeDoc event
+        mainController.onDidSaveTextDocument(document);
+        mainController.onDidCloseTextDocument(document);
+        try {
+            connectionManager.verify(x => x.transferFileConnection(TypeMoq.It.isAny(), TypeMoq.It.isAny()), TypeMoq.Times.once());
+            done();
+        } catch (err) {
+            done(new Error(err));
+        }
+    });
+
+});