Ensure connection open for OE queries and fix connection disposal (#359)

* Ensure connection open for OE queries and fix connection disposal
- Dispose connection in Metadata service, to ensure we cleanly dispose and don't rely on garbage colleciton
- Fixed issue where if the connection was closed, expanding databases in the Server would fail. This is because SMO doesn't always reopen the connection, certainly not for Server level queries. The solution is to always check if open and reopen.
- Added unit tests for this, which required mocking the relevant IsOpen / OpenConnection methods. Refactored SMO wrapper calls into a dedicated class file to handle this

src/Microsoft.SqlTools.ServiceLayer/ObjectExplorer/SmoModel/ServerNode.cs

@@ -30,7 +30,7 @@ namespace Microsoft.SqlTools.ServiceLayer.ObjectExplorer.SmoModel
         private string connectionUri;
         private Lazy<SmoQueryContext> context;
         private ConnectionService connectionService;
-        private SmoServerCreator serverCreator;
+        private SmoWrapper smoWrapper;
         private SqlServerType sqlServerType;
 
         public ServerNode(ConnectionCompleteParams connInfo, IMultiServiceProvider serviceProvider)
@@ -56,19 +56,19 @@ namespace Microsoft.SqlTools.ServiceLayer.ObjectExplorer.SmoModel
             Label = GetConnectionLabel();
         }
 
-        internal SmoServerCreator ServerCreator
+        internal SmoWrapper SmoWrapper
         {
             get
             {
-                if (serverCreator == null)
+                if (smoWrapper == null)
                 {
-                    ServerCreator = new SmoServerCreator();
+                    smoWrapper = new SmoWrapper();
                 }
-                return serverCreator;
+                return smoWrapper;
             }
             set
             {
-                this.serverCreator = value;
+                this.smoWrapper = value;
             }
         }
 
@@ -161,8 +161,8 @@ namespace Microsoft.SqlTools.ServiceLayer.ObjectExplorer.SmoModel
 
             try
             {
-                Server server = ServerCreator.Create(connection);
-                return new SmoQueryContext(server, serviceProvider)
+                Server server = SmoWrapper.CreateServer(connection);
+                return new SmoQueryContext(server, serviceProvider, SmoWrapper)
                 {
                     Parent = server,
                     SqlServerType = this.sqlServerType
@@ -187,16 +187,4 @@ namespace Microsoft.SqlTools.ServiceLayer.ObjectExplorer.SmoModel
             return context.Value;
         }
     }
-
-    /// <summary>
-    /// Internal for testing purposes only
-    /// </summary>
-    internal class SmoServerCreator
-    {
-        public virtual Server Create(SqlConnection connection)
-        {
-            ServerConnection serverConn = new ServerConnection(connection);
-            return new Server(serverConn);
-        }
-    }
 }

src/Microsoft.SqlTools.ServiceLayer/ObjectExplorer/SmoModel/SmoQuerier.cs

@@ -92,7 +92,10 @@ namespace Microsoft.SqlTools.ServiceLayer.ObjectExplorer.SmoModel
                     Enumerator en = new Enumerator();
                     Request request = new Request(new Urn(urn));
                     ServerConnection serverConnection = new ServerConnection(context.Server.ConnectionContext.SqlConnectionObject);
-
+                    if (!serverConnection.IsOpen)
+                    {
+                        serverConnection.Connect();
+                    }
                     EnumResult result = en.Process(serverConnection, request);
 
                     urns = GetUrns(result);

src/Microsoft.SqlTools.ServiceLayer/ObjectExplorer/SmoModel/SmoQueryContext.cs

@@ -7,9 +7,11 @@ using System;
 using System.Collections;
 using System.Collections.Generic;
 using System.Globalization;
+using Microsoft.SqlServer.Management.Common;
 using Microsoft.SqlServer.Management.Smo;
 using Microsoft.SqlTools.Extensibility;
 using Microsoft.SqlTools.ServiceLayer.ObjectExplorer.Nodes;
+using Microsoft.SqlTools.Utility;
 
 namespace Microsoft.SqlTools.ServiceLayer.ObjectExplorer.SmoModel
 {
@@ -18,14 +20,25 @@ namespace Microsoft.SqlTools.ServiceLayer.ObjectExplorer.SmoModel
     /// </summary>
     public class SmoQueryContext
     {
+        private Server server;
+        private Database database;
+        private SmoObjectBase parent;
+        private SmoWrapper smoWrapper;
+
         /// <summary>
         /// Creates a context object with a server to use as the basis for any queries
         /// </summary>
         /// <param name="server"></param>
         public SmoQueryContext(Server server, IMultiServiceProvider serviceProvider)
+            : this(server, serviceProvider, null)
         {
-            Server = server;
+        }
+
+        internal SmoQueryContext(Server server, IMultiServiceProvider serviceProvider, SmoWrapper serverManager)
+        {
+            this.server = server;
             ServiceProvider = serviceProvider;
+            this.smoWrapper = serverManager ?? new SmoWrapper();
         }
 
         /// <summary>
@@ -36,17 +49,40 @@ namespace Microsoft.SqlTools.ServiceLayer.ObjectExplorer.SmoModel
         /// <summary>
         /// The server SMO will query against
         /// </summary>
-        public Server Server { get; private set; }
+        public Server Server { 
+            get
+            {
+                return GetObjectWithOpenedConnection(server);
+            } 
+        }
 
         /// <summary>
         /// Optional Database context object to query against
         /// </summary>
-        public Database Database { get; set; }
+        public Database Database { 
+            get
+            {
+                return GetObjectWithOpenedConnection(database);
+            }
+            set
+            {
+                database = value;
+            }
+        }
 
         /// <summary>
         /// Parent of a give node to use for queries
         /// </summary>
-        public SmoObjectBase Parent { get; set; }
+        public SmoObjectBase Parent { 
+            get
+            {
+                return GetObjectWithOpenedConnection(parent);
+            }
+            set
+            {
+                parent = value;
+            }
+        }
 
         /// <summary>
         /// A query loader that can be used to find <see cref="SmoQuerier"/> objects
@@ -89,6 +125,7 @@ namespace Microsoft.SqlTools.ServiceLayer.ObjectExplorer.SmoModel
 
             return service;
         }
+
         /// <summary>
         /// Copies the context for use by another node
         /// </summary>
@@ -96,13 +133,39 @@ namespace Microsoft.SqlTools.ServiceLayer.ObjectExplorer.SmoModel
         /// <returns>new <see cref="SmoQueryContext"/> with all fields except <see cref="Parent"/> the same</returns>
         public SmoQueryContext CopyWithParent(SmoObjectBase parent)
         {
-            SmoQueryContext context = new SmoQueryContext(this.Server, this.ServiceProvider)
+            SmoQueryContext context = new SmoQueryContext(this.Server, this.ServiceProvider, this.smoWrapper)
             {
-                Database = this.Database,
+                database = this.Database,
                 Parent = parent,
                 SqlServerType = this.SqlServerType
             };
             return context;
         }
+
+        private T GetObjectWithOpenedConnection<T>(T smoObj)
+            where T : SmoObjectBase
+        {
+            if (smoObj != null)
+            {
+                EnsureConnectionOpen(smoObj);
+            }
+            return smoObj;
+        }
+
+        /// <summary>
+        /// Ensures the server objects connection context is open. This is used by all child objects, and 
+        /// the only way to easily access is via the server object. This should be called during access of
+        /// any of the object properties
+        /// </summary>
+        private void EnsureConnectionOpen(SmoObjectBase smoObj)
+        {
+            if (!smoWrapper.IsConnectionOpen(smoObj))
+            {
+                // We have a closed server connection. Reopen this
+                // Note: not currently catching connection exceptions. Expect this to bubble
+                // up to calling methods and be logged there as this would be happening there in any case
+                smoWrapper.OpenConnection(smoObj);
+            }
+        }
     }
 }

src/Microsoft.SqlTools.ServiceLayer/ObjectExplorer/SmoModel/SmoWrapper.cs

@@ -0,0 +1,55 @@
+//
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+//
+
+using System;
+using System.Data.Common;
+using System.Data.SqlClient;
+using System.Globalization;
+using System.Linq;
+using Microsoft.SqlServer.Management.Common;
+using Microsoft.SqlServer.Management.Smo;
+using Microsoft.SqlTools.Extensibility;
+using Microsoft.SqlTools.ServiceLayer.Connection;
+using Microsoft.SqlTools.ServiceLayer.Connection.Contracts;
+using Microsoft.SqlTools.ServiceLayer.Connection.ReliableConnection;
+using Microsoft.SqlTools.ServiceLayer.ObjectExplorer.Nodes;
+using Microsoft.SqlTools.ServiceLayer.Utility;
+using Microsoft.SqlTools.Utility;
+
+namespace Microsoft.SqlTools.ServiceLayer.ObjectExplorer.SmoModel
+{
+    /// <summary>
+    /// Internal for testing purposes only. This class provides wrapper functionality
+    /// over SMO objects in order to facilitate unit testing
+    /// </summary>
+    internal class SmoWrapper
+    {
+        public virtual Server CreateServer(SqlConnection connection)
+        {
+            ServerConnection serverConn = new ServerConnection(connection);
+            return new Server(serverConn);
+        }
+
+        public virtual bool IsConnectionOpen(SmoObjectBase smoObj)
+        {
+            SqlSmoObject sqlObj = smoObj as SqlSmoObject;
+            return sqlObj != null
+                && sqlObj.ExecutionManager != null
+                && sqlObj.ExecutionManager.ConnectionContext != null
+                && sqlObj.ExecutionManager.ConnectionContext.IsOpen;
+        }
+
+        public virtual void OpenConnection(SmoObjectBase smoObj)
+        {
+            SqlSmoObject sqlObj = smoObj as SqlSmoObject;
+            if (sqlObj != null
+                && sqlObj.ExecutionManager != null
+                && sqlObj.ExecutionManager.ConnectionContext != null)
+            {
+                sqlObj.ExecutionManager.ConnectionContext.Connect();
+            }
+        }
+    }
+}