diff --git a/src/Microsoft.SqlTools.ServiceLayer/Metadata/MetadataService.cs b/src/Microsoft.SqlTools.ServiceLayer/Metadata/MetadataService.cs index 99e52cfb..e5ec100f 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Metadata/MetadataService.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/Metadata/MetadataService.cs @@ -74,8 +74,10 @@ namespace Microsoft.SqlTools.ServiceLayer.Metadata var metadata = new List(); if (connInfo != null) { - SqlConnection sqlConn = OpenMetadataConnection(connInfo); - ReadMetadata(sqlConn, metadata); + using (SqlConnection sqlConn = OpenMetadataConnection(connInfo)) + { + ReadMetadata(sqlConn, metadata); + } } await requestContext.SendResult(new MetadataQueryResult diff --git a/src/Microsoft.SqlTools.ServiceLayer/ObjectExplorer/SmoModel/ServerNode.cs b/src/Microsoft.SqlTools.ServiceLayer/ObjectExplorer/SmoModel/ServerNode.cs index 208930ea..eb06453d 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/ObjectExplorer/SmoModel/ServerNode.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/ObjectExplorer/SmoModel/ServerNode.cs @@ -30,7 +30,7 @@ namespace Microsoft.SqlTools.ServiceLayer.ObjectExplorer.SmoModel private string connectionUri; private Lazy 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; } } - - /// - /// Internal for testing purposes only - /// - internal class SmoServerCreator - { - public virtual Server Create(SqlConnection connection) - { - ServerConnection serverConn = new ServerConnection(connection); - return new Server(serverConn); - } - } } diff --git a/src/Microsoft.SqlTools.ServiceLayer/ObjectExplorer/SmoModel/SmoQuerier.cs b/src/Microsoft.SqlTools.ServiceLayer/ObjectExplorer/SmoModel/SmoQuerier.cs index d4da5699..85c619e5 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/ObjectExplorer/SmoModel/SmoQuerier.cs +++ b/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); diff --git a/src/Microsoft.SqlTools.ServiceLayer/ObjectExplorer/SmoModel/SmoQueryContext.cs b/src/Microsoft.SqlTools.ServiceLayer/ObjectExplorer/SmoModel/SmoQueryContext.cs index d6ebdfec..3da36c6c 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/ObjectExplorer/SmoModel/SmoQueryContext.cs +++ b/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 /// public class SmoQueryContext { + private Server server; + private Database database; + private SmoObjectBase parent; + private SmoWrapper smoWrapper; + /// /// Creates a context object with a server to use as the basis for any queries /// /// 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(); } /// @@ -36,17 +49,40 @@ namespace Microsoft.SqlTools.ServiceLayer.ObjectExplorer.SmoModel /// /// The server SMO will query against /// - public Server Server { get; private set; } + public Server Server { + get + { + return GetObjectWithOpenedConnection(server); + } + } /// /// Optional Database context object to query against /// - public Database Database { get; set; } + public Database Database { + get + { + return GetObjectWithOpenedConnection(database); + } + set + { + database = value; + } + } /// /// Parent of a give node to use for queries /// - public SmoObjectBase Parent { get; set; } + public SmoObjectBase Parent { + get + { + return GetObjectWithOpenedConnection(parent); + } + set + { + parent = value; + } + } /// /// A query loader that can be used to find objects @@ -89,6 +125,7 @@ namespace Microsoft.SqlTools.ServiceLayer.ObjectExplorer.SmoModel return service; } + /// /// Copies the context for use by another node /// @@ -96,13 +133,39 @@ namespace Microsoft.SqlTools.ServiceLayer.ObjectExplorer.SmoModel /// new with all fields except the same 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 smoObj) + where T : SmoObjectBase + { + if (smoObj != null) + { + EnsureConnectionOpen(smoObj); + } + return smoObj; + } + + /// + /// 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 + /// + 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); + } + } } } diff --git a/src/Microsoft.SqlTools.ServiceLayer/ObjectExplorer/SmoModel/SmoWrapper.cs b/src/Microsoft.SqlTools.ServiceLayer/ObjectExplorer/SmoModel/SmoWrapper.cs new file mode 100644 index 00000000..33e8ebb6 --- /dev/null +++ b/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 +{ + /// + /// Internal for testing purposes only. This class provides wrapper functionality + /// over SMO objects in order to facilitate unit testing + /// + 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(); + } + } + } +} \ No newline at end of file diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/ObjectExplorer/NodeTests.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/ObjectExplorer/NodeTests.cs index d971cd01..5f384a3b 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/ObjectExplorer/NodeTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/ObjectExplorer/NodeTests.cs @@ -276,6 +276,74 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.ObjectExplorer string.Format(CultureInfo.CurrentCulture, SR.TreeNodeError, expectedMsg), node.ErrorStateMessage); } + + [Fact] + public void QueryContextShouldNotCallOpenOnAlreadyOpenConnection() + { + // given a server connection that will state its connection is open + SetupAndRegisterTestConnectionService(); + Server smoServer = new Server(new ServerConnection(new SqlConnection(fakeConnectionString))); + Mock wrapper = SetupSmoWrapperForIsOpenTest(smoServer, isOpen: true); + + SmoQueryContext context = new SmoQueryContext(smoServer, ServiceProvider, wrapper.Object); + + // when I access the Server property + Server actualServer = context.Server; + + // Then I do not expect to have open called + Assert.NotNull(actualServer); + wrapper.Verify(c => c.OpenConnection(It.IsAny()), Times.Never); + } + + private Mock SetupSmoWrapperForIsOpenTest(Server smoServer, bool isOpen) + { + Mock wrapper = new Mock(); + int count = 0; + wrapper.Setup(c => c.CreateServer(It.IsAny())) + .Returns(() => smoServer); + wrapper.Setup(c => c.IsConnectionOpen(It.IsAny())) + .Returns(() => isOpen); + wrapper.Setup(c => c.OpenConnection(It.IsAny())) + .Callback(() => count++) + .Verifiable(); + return wrapper; + } + + [Fact] + public void QueryContextShouldReopenClosedConnectionWhenGettingServer() + { + // given a server connection that will state its connection is closed + SetupAndRegisterTestConnectionService(); + Server smoServer = new Server(new ServerConnection(new SqlConnection(fakeConnectionString))); + Mock wrapper = SetupSmoWrapperForIsOpenTest(smoServer, isOpen: false); + + SmoQueryContext context = new SmoQueryContext(smoServer, ServiceProvider, wrapper.Object); + + // when I access the Server property + Server actualServer = context.Server; + + // Then I expect to have open called + Assert.NotNull(actualServer); + wrapper.Verify(c => c.OpenConnection(It.IsAny()), Times.Once); + } + + [Fact] + public void QueryContextShouldReopenClosedConnectionWhenGettingParent() + { + // given a server connection that will state its connection is closed + SetupAndRegisterTestConnectionService(); + Server smoServer = new Server(new ServerConnection(new SqlConnection(fakeConnectionString))); + Mock wrapper = SetupSmoWrapperForIsOpenTest(smoServer, isOpen: false); + + SmoQueryContext context = new SmoQueryContext(smoServer, ServiceProvider, wrapper.Object); + context.Parent = smoServer; + // when I access the Parent property + SmoObjectBase actualParent = context.Parent; + + // Then I expect to have open called + Assert.NotNull(actualParent); + wrapper.Verify(c => c.OpenConnection(It.IsAny()), Times.Once); + } private ConnectionService SetupAndRegisterTestConnectionService() { @@ -291,27 +359,31 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.ObjectExplorer private ServerNode SetupServerNodeWithServer(Server smoServer) { - Mock creator = new Mock(); - creator.Setup(c => c.Create(It.IsAny())) + Mock creator = new Mock(); + creator.Setup(c => c.CreateServer(It.IsAny())) .Returns(() => smoServer); + creator.Setup(c => c.IsConnectionOpen(It.IsAny())) + .Returns(() => true); ServerNode node = SetupServerNodeWithCreator(creator.Object); return node; } private ServerNode SetupServerNodeWithExceptionCreator(Exception ex) { - Mock creator = new Mock(); - creator.Setup(c => c.Create(It.IsAny())) + Mock creator = new Mock(); + creator.Setup(c => c.CreateServer(It.IsAny())) .Throws(ex); + creator.Setup(c => c.IsConnectionOpen(It.IsAny())) + .Returns(() => false); ServerNode node = SetupServerNodeWithCreator(creator.Object); return node; } - private ServerNode SetupServerNodeWithCreator(SmoServerCreator creator) + private ServerNode SetupServerNodeWithCreator(SmoWrapper creator) { ServerNode node = new ServerNode(defaultConnParams, ServiceProvider); - node.ServerCreator = creator; + node.SmoWrapper = creator; return node; }