Fixing bad data issue w/closing/opening untitled doc (#142)

* Adding useful unit tests for this functionality
* Adding callback functionality for when a file is closed

* Fixing bad data issue w/closing/opening untitled doc

* Adding useful unit tests for this functionality
* Adding callback functionality for when a file is closed

* Moving from public to internal

src/Microsoft.SqlTools.ServiceLayer/Hosting/Protocol/EventContext.cs

@@ -14,7 +14,12 @@ namespace Microsoft.SqlTools.ServiceLayer.Hosting.Protocol
     /// </summary>
     public class EventContext
     {
-        private MessageWriter messageWriter;
+        private readonly MessageWriter messageWriter;
+
+        /// <summary>
+        /// Parameterless constructor required for mocking
+        /// </summary>
+        public EventContext() { }
 
         public EventContext(MessageWriter messageWriter)
         {

src/Microsoft.SqlTools.ServiceLayer/Workspace/WorkspaceService.cs

@@ -44,6 +44,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace
             ConfigChangeCallbacks = new List<ConfigChangeCallback>();
             TextDocChangeCallbacks = new List<TextDocChangeCallback>();
             TextDocOpenCallbacks = new List<TextDocOpenCallback>();
+            TextDocCloseCallbacks = new List<TextDocCloseCallback>();
 
             CurrentSettings = new TConfig();
         }
@@ -55,7 +56,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace
         /// <summary>
         /// Workspace object for the service. Virtual to allow for mocking
         /// </summary>
-        public virtual Workspace Workspace { get; private set; }
+        public virtual Workspace Workspace { get; internal set; }
 
         /// <summary>
         /// Current settings for the workspace
@@ -84,7 +85,14 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace
         /// <param name="openFile">File that was opened</param>
         /// <param name="eventContext">Context of the event raised for the changed files</param>
         public delegate Task TextDocOpenCallback(ScriptFile openFile, EventContext eventContext);
-        
+
+        /// <summary>
+        /// Delegate for callbacks that occur when a text document is closed
+        /// </summary>
+        /// <param name="closedFile">File that was closed</param>
+        /// <param name="eventContext">Context of the event raised for changed files</param>
+        public delegate Task TextDocCloseCallback(ScriptFile closedFile, EventContext eventContext);
+
         /// <summary>
         /// List of callbacks to call when the configuration of the workspace changes
         /// </summary>
@@ -100,6 +108,10 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace
         /// </summary>
         private List<TextDocOpenCallback> TextDocOpenCallbacks { get; set; }
 
+        /// <summary>
+        /// List of callbacks to call when a text document is closed
+        /// </summary>
+        private List<TextDocCloseCallback> TextDocCloseCallbacks { get; set; }
 
         #endregion
 
@@ -161,6 +173,15 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace
             TextDocChangeCallbacks.Add(task);
         }
 
+        /// <summary>
+        /// Adds a new task to be called when a text document closes.
+        /// </summary>
+        /// <param name="task">Delegate to call when the document closes</param>
+        public void RegisterTextDocCloseCallback(TextDocCloseCallback task)
+        {
+            TextDocCloseCallbacks.Add(task);
+        }
+
         /// <summary>
         /// Adds a new task to be called when a file is opened
         /// </summary>
@@ -177,7 +198,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace
         /// <summary>
         /// Handles text document change events
         /// </summary>
-        protected Task HandleDidChangeTextDocumentNotification(
+        internal Task HandleDidChangeTextDocumentNotification(
             DidChangeTextDocumentParams textChangeParams,
             EventContext eventContext)
         {
@@ -216,7 +237,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace
             }
         }
 
-        protected async Task HandleDidOpenTextDocumentNotification(
+        internal async Task HandleDidOpenTextDocumentNotification(
             DidOpenTextDocumentNotification openParams,
             EventContext eventContext)
         {
@@ -232,18 +253,31 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace
             await Task.WhenAll(textDocOpenTasks);
         }
 
-        protected Task HandleDidCloseTextDocumentNotification(
+        internal async Task HandleDidCloseTextDocumentNotification(
            DidCloseTextDocumentParams closeParams,
            EventContext eventContext)
         {
             Logger.Write(LogLevel.Verbose, "HandleDidCloseTextDocumentNotification");
-            return Task.FromResult(true);
+
+            // Skip closing this file if the file doesn't exist
+            var closedFile = Workspace.GetFile(closeParams.TextDocument.Uri);
+            if (closedFile == null)
+            {
+                return;
+            }
+
+            // Trash the existing document from our mapping
+            Workspace.CloseFile(closedFile);
+
+            // Send out a notification to other services that have subscribed to this event
+            var textDocClosedTasks = TextDocCloseCallbacks.Select(t => t(closedFile, eventContext));
+            await Task.WhenAll(textDocClosedTasks);
         }
 
         /// <summary>
         /// Handles the configuration change event
         /// </summary>
-        protected async Task HandleDidChangeConfigurationNotification(
+        internal async Task HandleDidChangeConfigurationNotification(
             DidChangeConfigurationParams<TConfig> configChangeParams,
             EventContext eventContext)
         {

test/Microsoft.SqlTools.ServiceLayer.Test/Workspace/WorkspaceTests.cs

@@ -0,0 +1,94 @@
+//
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+//
+
+using System.IO;
+using System.Threading.Tasks;
+using Microsoft.SqlTools.ServiceLayer.Hosting.Protocol;
+using Microsoft.SqlTools.ServiceLayer.SqlContext;
+using Microsoft.SqlTools.ServiceLayer.Workspace;
+using Microsoft.SqlTools.ServiceLayer.Workspace.Contracts;
+using Microsoft.SqlTools.Test.Utility;
+using Moq;
+using Xunit;
+
+namespace Microsoft.SqlTools.ServiceLayer.Test.Workspace
+{
+    public class WorkspaceTests
+    {
+        [Fact]
+        public async Task FileClosedSuccessfully()
+        {
+            // Given:
+            // ... A workspace that has a single file open
+            var workspace = new ServiceLayer.Workspace.Workspace();
+            var workspaceService = new WorkspaceService<SqlToolsSettings> {Workspace = workspace};
+            var openedFile = workspace.GetFileBuffer(TestObjects.ScriptUri, string.Empty);
+            Assert.NotNull(openedFile);
+            Assert.NotEmpty(workspace.GetOpenedFiles());
+
+            // ... And there is a callback registered for the file closed event
+            ScriptFile closedFile = null;
+            workspaceService.RegisterTextDocCloseCallback((f, c) =>
+            {
+                closedFile = f;
+                return Task.FromResult(true);
+            });
+
+            // If:
+            // ... An event to close the open file occurs
+            var eventContext = new Mock<EventContext>().Object;
+            var requestParams = new DidCloseTextDocumentParams
+            {
+                TextDocument = new TextDocumentItem {Uri = TestObjects.ScriptUri}
+            };
+            await workspaceService.HandleDidCloseTextDocumentNotification(requestParams, eventContext);
+
+            // Then:
+            // ... The file should no longer be in the open files
+            Assert.Empty(workspace.GetOpenedFiles());
+
+            // ... The callback should have been called
+            // ... The provided script file should be the one we created
+            Assert.NotNull(closedFile);
+            Assert.Equal(openedFile, closedFile);
+        }
+
+        [Fact]
+        public async Task FileClosedNotOpen()
+        {
+            // Given:
+            // ... A workspace that has no files open
+            var workspace = new ServiceLayer.Workspace.Workspace();
+            var workspaceService = new WorkspaceService<SqlToolsSettings> {Workspace = workspace};
+            Assert.Empty(workspace.GetOpenedFiles());
+
+            // ... And there is a callback registered for the file closed event
+            bool callbackCalled = false;
+            workspaceService.RegisterTextDocCloseCallback((f, c) =>
+            {
+                callbackCalled = true;
+                return Task.FromResult(true);
+            });
+
+            // If:
+            // ... An event to close the a file occurs
+            var eventContext = new Mock<EventContext>().Object;
+            var requestParams = new DidCloseTextDocumentParams
+            {
+                TextDocument = new TextDocumentItem {Uri = TestObjects.ScriptUri}
+            };
+            // Then:
+            // ... There should be a file not found exception thrown
+            // TODO: This logic should be changed to not create the ScriptFile
+            await Assert.ThrowsAsync<IOException>(
+                () => workspaceService.HandleDidCloseTextDocumentNotification(requestParams, eventContext));
+
+            // ... There should still be no open files
+            // ... The callback should not have been called
+            Assert.Empty(workspace.GetOpenedFiles());
+            Assert.False(callbackCalled);
+        }
+    }
+}