Incremental checkin of connection work

src/Microsoft.SqlTools.ServiceLayer/Connection/ConnectionService.cs

@@ -34,7 +34,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection
 
         public string OwnerUri { get; private set; }
 
-        private ISqlConnectionFactory Factory {get; set;}
+        public ISqlConnectionFactory Factory {get; private set;}
 
         public ConnectionDetails ConnectionDetails { get; private set; }
         
@@ -123,16 +123,9 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection
         }
 
         // Attempts to link a URI to an actively used connection for this URI
-        public bool TryFindConnection(string ownerUri, out ConnectionSummary connectionSummary)
+        public bool TryFindConnection(string ownerUri, out ConnectionInfo connectionInfo)
         {
-            connectionSummary = null;
+            return this.ownerToConnectionMap.TryGetValue(ownerUri, out connectionInfo);
-            ConnectionInfo connectionInfo;
-            if (this.ownerToConnectionMap.TryGetValue(ownerUri, out connectionInfo))
-            {
-                connectionSummary = CopySummary(connectionInfo.ConnectionDetails);
-                return true;
-            }
-            return false;
         }
         
         private static ConnectionSummary CopySummary(ConnectionSummary summary)
@@ -151,16 +144,33 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection
         /// <param name="connectionParams"></param>
         public ConnectResponse Connect(ConnectParams connectionParams)
         {
+            // Validate parameters
+            if(connectionParams == null || !connectionParams.IsValid())
+            {
+                return new ConnectResponse()
+                {
+                    Messages = "Error: Invalid connection parameters provided."
+                };
+            }
+
             ConnectionInfo connectionInfo;
             if (ownerToConnectionMap.TryGetValue(connectionParams.OwnerUri, out connectionInfo) )
             {
                 // TODO disconnect
             }
-            connectionInfo = new ConnectionInfo(this.connectionFactory, connectionParams.OwnerUri, connectionParams.Connection);
+            connectionInfo = new ConnectionInfo(ConnectionFactory, connectionParams.OwnerUri, connectionParams.Connection);
 
             // try to connect
+            var response = new ConnectResponse();
+            try
+            {
                 connectionInfo.OpenConnection();
-            // TODO: check that connection worked
+            }
+            catch(Exception ex)
+            {
+                response.Messages = ex.Message;
+                return response;
+            }
 
             ownerToConnectionMap[connectionParams.OwnerUri] = connectionInfo;
 
@@ -171,10 +181,8 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection
             }
 
             // return the connection result
-            return new ConnectResponse()
+            response.ConnectionId = connectionInfo.ConnectionId.ToString();
-            {
+            return response;
-                ConnectionId = connectionInfo.ConnectionId.ToString()
-            };
         }
 
         public void InitializeService(IProtocolEndpoint serviceHost)

src/Microsoft.SqlTools.ServiceLayer/Connection/Contracts/ConnectionMessagesExtensions.cs

@@ -0,0 +1,30 @@
+//
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+//
+
+using System;
+
+namespace Microsoft.SqlTools.ServiceLayer.Connection.Contracts
+{
+    /// <summary>
+    /// Extension methods to ConnectParams
+    /// </summary>
+    public static class ConnectParamsExtensions
+    {
+        /// <summary>
+        /// Check that the fields in ConnectParams are all valid
+        /// </summary>
+        public static bool IsValid(this ConnectParams parameters)
+        {
+            return !(
+                String.IsNullOrEmpty(parameters.OwnerUri) ||
+                parameters.Connection == null ||
+                String.IsNullOrEmpty(parameters.Connection.DatabaseName) ||
+                String.IsNullOrEmpty(parameters.Connection.Password) ||
+                String.IsNullOrEmpty(parameters.Connection.ServerName) ||
+                String.IsNullOrEmpty(parameters.Connection.UserName)
+            );
+        }
+    }
+}

src/Microsoft.SqlTools.ServiceLayer/LanguageServices/AutoCompleteService.cs

@@ -203,6 +203,7 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
             new Dictionary<ConnectionSummary, IntellisenseCache>(new ConnectionSummaryComparer());
 
         private ISqlConnectionFactory factory;
+        private Object factoryLock = new Object();
 
         /// <summary>
         /// Internal for testing purposes only
@@ -211,18 +212,23 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
         {
             get
             {
-                // TODO consider protecting against multi-threaded access
+                lock(factoryLock)
+                {
                     if(factory == null)
                     {
                         factory = new SqlConnectionFactory();
                     }
+                }
                 return factory;
             }
             set
+            {
+                lock(factoryLock)
                 {
                     factory = value;
                 }
             }
+        }
         public void InitializeService(ServiceHost serviceHost)
         {
             // Register a callback for when a connection is created
@@ -265,10 +271,10 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices
             // If we have a connection but no cache, we don't care - assuming the OnConnect and OnDisconnect listeners
             // behave well, there should be a cache for any actively connected document. This also helps skip documents 
             // that are not backed by a SQL connection
-            ConnectionSummary connectionSummary;
+            ConnectionInfo info;
             IntellisenseCache cache;
-            if (ConnectionService.Instance.TryFindConnection(textDocumentPosition.Uri, out connectionSummary)
+            if (ConnectionService.Instance.TryFindConnection(textDocumentPosition.Uri, out info)
-                && caches.TryGetValue(connectionSummary, out cache))
+                && caches.TryGetValue((ConnectionSummary)info.ConnectionDetails, out cache))
             {
                 return cache.GetAutoCompleteItems(textDocumentPosition).ToArray();
             }

test/Microsoft.SqlTools.ServiceLayer.Test/Connection/ConnectionServiceTests.cs

@@ -19,6 +19,83 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.Connection
     /// </summary>
     public class ConnectionServiceTests
     {
+        /// <summary>
+        /// Verify that when connecting with invalid credentials, an error is thrown.
+        /// </summary>
+        [Fact]
+        public void ConnectingWithInvalidCredentialsYieldsErrorMessage()
+        {
+            var testConnectionDetails = TestObjects.GetTestConnectionDetails();
+            var invalidConnectionDetails = new ConnectionDetails();
+            invalidConnectionDetails.ServerName = testConnectionDetails.ServerName;
+            invalidConnectionDetails.DatabaseName = testConnectionDetails.DatabaseName;
+            invalidConnectionDetails.UserName = "invalidUsername"; // triggers exception when opening mock connection
+            invalidConnectionDetails.Password = "invalidPassword";
+
+            // Connect to test db with invalid credentials
+            var connectionResult =
+                TestObjects.GetTestConnectionService()
+                .Connect(new ConnectParams()
+                {
+                    OwnerUri = "file://my/sample/file.sql",
+                    Connection = invalidConnectionDetails
+                });
+
+            // check that an error was caught
+            Assert.NotNull(connectionResult.Messages);
+            Assert.NotEqual(String.Empty, connectionResult.Messages);
+        }
+
+        /// <summary>
+        /// Verify that when connecting with invalid parameters, an error is thrown.
+        /// </summary>
+        [Theory]
+        [InlineDataAttribute(null, "my-server", "test", "sa", "123456")]
+        [InlineDataAttribute("file://my/sample/file.sql", null, "test", "sa", "123456")]
+        [InlineDataAttribute("file://my/sample/file.sql", "my-server", null, "sa", "123456")]
+        [InlineDataAttribute("file://my/sample/file.sql", "my-server", "test", null, "123456")]
+        [InlineDataAttribute("file://my/sample/file.sql", "my-server", "test", "sa", null)]
+        [InlineDataAttribute("", "my-server", "test", "sa", "123456")]
+        [InlineDataAttribute("file://my/sample/file.sql", "", "test", "sa", "123456")]
+        [InlineDataAttribute("file://my/sample/file.sql", "my-server", "", "sa", "123456")]
+        [InlineDataAttribute("file://my/sample/file.sql", "my-server", "test", "", "123456")]
+        [InlineDataAttribute("file://my/sample/file.sql", "my-server", "test", "sa", "")]
+        public void ConnectingWithInvalidParametersYieldsErrorMessage(string ownerUri, string server, string database, string userName, string password)
+        {
+            // Connect with invalid parameters
+            var connectionResult =
+                TestObjects.GetTestConnectionService()
+                .Connect(new ConnectParams()
+                {
+                    OwnerUri = ownerUri,
+                    Connection = new ConnectionDetails() {
+                        ServerName = server,
+                        DatabaseName = database,
+                        UserName = userName,
+                        Password = password
+                    }
+                });
+            
+            // check that an error was caught
+            Assert.NotNull(connectionResult.Messages);
+            Assert.NotEqual(String.Empty, connectionResult.Messages);
+        }
+
+        /// <summary>
+        /// Verify that when connecting with a null parameters object, an error is thrown.
+        /// </summary>
+        [Fact]
+        public void ConnectingWithNullParametersObjectYieldsErrorMessage()
+        {
+            // Connect with null parameters
+            var connectionResult =
+                TestObjects.GetTestConnectionService()
+                .Connect(null);
+            
+            // check that an error was caught
+            Assert.NotNull(connectionResult.Messages);
+            Assert.NotEqual(String.Empty, connectionResult.Messages);
+        }
 
         /// <summary>
         /// Verify that the SQL parser correctly detects errors in text
@@ -64,43 +141,45 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.Connection
             Assert.True(callbackInvoked);
         }
 
-        //[Fact]
+        /// <summary>
-        //public void TestConnectRequestRegistersOwner()
+        /// Verify when a connection is created that the URI -> Connection mapping is created in the connection service.
-        //{
+        /// </summary>
-        //    // Given a request to connect to a database
+        [Fact]
-        //    var service = new ConnectionService(new TestSqlConnectionFactory());
+        public void TestConnectRequestRegistersOwner()
-        //    ConnectionDetails connectionDetails = TestObjects.GetTestConnectionDetails();
+        {
-        //    var connectParams = new ConnectParams()
+            // Given a request to connect to a database
-        //    {
+            var service = TestObjects.GetTestConnectionService();
-        //        OwnerUri = "file://path/to/my.sql",
+            var connectParams = TestObjects.GetTestConnectionParams();
-        //        Connection = connectionDetails
-        //    };
 
-        //    var endpoint = new Mock<IProtocolEndpoint>();
+            //var endpoint = new Mock<IProtocolEndpoint>();
-        //    Func<ConnectParams, RequestContext<ConnectResponse>, Task> connectRequestHandler = null;
+            //Func<ConnectParams, RequestContext<ConnectResponse>, Task> connectRequestHandler = null;
-        //    endpoint.Setup(e => e.SetRequestHandler(ConnectionRequest.Type, It.IsAny<Func<ConnectParams, RequestContext<ConnectResponse>, Task>>()))
+            //endpoint.Setup(e => e.SetRequestHandler(ConnectionRequest.Type, It.IsAny<Func<ConnectParams, RequestContext<ConnectResponse>, Task>>()))
             //    .Callback<Func<ConnectParams, RequestContext<ConnectResponse>, Task>>(handler => connectRequestHandler = handler);
 
-        //    // when I initialize the service
+            // when I initialize the service
-        //    service.InitializeService(endpoint.Object);
+            //service.InitializeService(endpoint.Object);
 
-        //    // then I expect the handler to be captured
+            // then I expect the handler to be captured
-        //    Assert.NotNull(connectRequestHandler);
+            //Assert.NotNull(connectRequestHandler);
 
-        //    // when I call the service
+            // when I call the service
-        //    var requestContext = new Mock<RequestContext<ConnectResponse>>();
+            //var requestContext = new Mock<RequestContext<ConnectResponse>>();
 
-        //    connectRequestHandler(connectParams, requestContext);
+            //connectRequestHandler(connectParams, requestContext.Object);
-        //    // then I should get a live connection
+            // then I should get a live connection
 
-        //    // and then I should have 
+            // and then I should have 
-        //    // connect to a database instance 
+            // connect to a database instance 
-        //    var connectionResult =
+            var connectionResult = service.Connect(connectParams);
-        //        TestObjects.GetTestConnectionService()
-        //        .Connect(TestObjects.GetTestConnectionDetails());
 
-        //    // verify that a valid connection id was returned
+            // verify that a valid connection id was returned
-        //    Assert.True(connectionResult.ConnectionId > 0);
+            Assert.NotNull(connectionResult.ConnectionId);
-        //}
+            Assert.NotEqual(String.Empty, connectionResult.ConnectionId);
+            Assert.NotNull(new Guid(connectionResult.ConnectionId));
+            
+            // verify that the (URI -> connection) mapping was created
+            ConnectionInfo info;
+            Assert.True(service.TryFindConnection(connectParams.OwnerUri, out info));
+        }
     }
 }

test/Microsoft.SqlTools.ServiceLayer.Test/Utility/TestObjects.cs

@@ -327,7 +327,11 @@ namespace Microsoft.SqlTools.Test.Utility
 
         public override void Open()
         {
-            // No Op
+            // No Op, unless credentials are bad
+            if(ConnectionString.Contains("invalidUsername"))
+            {
+                throw new Exception("Invalid credentials provided");
+            }
         }
 
         public override string ConnectionString { get; set; }