From 616a79c83dd8ca1023d5e87328e11b5706136caa Mon Sep 17 00:00:00 2001 From: Udeesha Gautam <46980425+udeeshagautam@users.noreply.github.com> Date: Sun, 9 Aug 2020 19:17:16 -0700 Subject: [PATCH] GetConnectionString API fix to not change the cache only return the value (#1040) * Get connection string call was changing the connection info (ref object) received from cache. Changing it to just get and make changes to only the returned string.. * Change to ensure PR validation works --- .../Connection/ConnectionService.cs | 8 +++++--- .../Connection/ConnectionServiceTests.cs | 12 +++++++++++- .../EditData/RowDeleteTests.cs | 10 +++++----- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.SqlTools.ServiceLayer/Connection/ConnectionService.cs b/src/Microsoft.SqlTools.ServiceLayer/Connection/ConnectionService.cs index 099b68cb..75bb4fa7 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Connection/ConnectionService.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/Connection/ConnectionService.cs @@ -1292,14 +1292,16 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection { try { + SqlConnectionStringBuilder connStringBuilder = CreateConnectionStringBuilder(info.ConnectionDetails); + if (!connStringParams.IncludePassword) { - info.ConnectionDetails.Password = ConnectionService.PasswordPlaceholder; + connStringBuilder.Password = ConnectionService.PasswordPlaceholder; } - info.ConnectionDetails.ApplicationName = "sqlops-connection-string"; + connStringBuilder.ApplicationName = "sqlops-connection-string"; - connectionString = BuildConnectionString(info.ConnectionDetails); + connectionString = connStringBuilder.ConnectionString; } catch (Exception e) { diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/Connection/ConnectionServiceTests.cs b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/Connection/ConnectionServiceTests.cs index c9b3e3b4..82677d09 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/Connection/ConnectionServiceTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/Connection/ConnectionServiceTests.cs @@ -99,7 +99,7 @@ namespace Microsoft.SqlTools.ServiceLayer.IntegrationTests.Connection // All open DbConnections (Query and Default) should have newDatabaseName as their database foreach (DbConnection connection in connectionInfo.AllConnections) { - if (connection != null && connection.State == ConnectionState.Open) + if (connection != null && connection.State == ConnectionState.Open) { Assert.AreEqual(connection.Database, newDatabaseName); } @@ -115,6 +115,7 @@ namespace Microsoft.SqlTools.ServiceLayer.IntegrationTests.Connection // If we make a connection to a live database ConnectionService service = ConnectionService.Instance; var result = LiveConnectionHelper.InitLiveConnectionInfo(); + var resultPassword = result.ConnectionInfo.ConnectionDetails.Password; var requestContext = new Mock>(); requestContext.Setup(x => x.SendResult(It.Is((connectionString) => connectionString.Contains("Password=" + ConnectionService.PasswordPlaceholder)))) @@ -128,6 +129,15 @@ namespace Microsoft.SqlTools.ServiceLayer.IntegrationTests.Connection await service.HandleGetConnectionStringRequest(requestParams, requestContext.Object); requestContext.VerifyAll(); + + // validate that the get command doesn't change any connection property and the following get commands work as expected + requestParams.IncludePassword = true; + + requestContext.Setup(x => x.SendResult(It.Is((connectionString) => connectionString.Contains("Password=" + resultPassword)))) + .Returns(Task.FromResult(new object())); + + await service.HandleGetConnectionStringRequest(requestParams, requestContext.Object); + requestContext.VerifyAll(); } } } diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/EditData/RowDeleteTests.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/EditData/RowDeleteTests.cs index 5da6dcbd..75f53673 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/EditData/RowDeleteTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/EditData/RowDeleteTests.cs @@ -74,7 +74,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.EditData } [Test] - public async Task GetCommand([Values]bool includeIdentity, [Values]bool isMemoryOptimized) + public async Task GetCommand([Values]bool includeIdentity, [Values] bool isMemoryOptimized) { // Setup: // ... Create a row delete @@ -109,7 +109,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.EditData Assert.AreEqual(data.TableMetadata.EscapedMultipartName, tbl); // ... There should be as many where components as there are keys - string[] whereComponents = m.Groups[2].Value.Split(new[] {"AND"}, StringSplitOptions.None); + string[] whereComponents = m.Groups[2].Value.Split(new[] {"AND" }, StringSplitOptions.None); Assert.AreEqual(expectedKeys, whereComponents.Length); Assert.That(whereComponents.Select(c => c.Trim()), Has.All.Match(@"\(.+ = @.+\)"), "Each component should be equal to a parameter"); @@ -193,7 +193,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.EditData Assert.Throws(() => rd.RevertCell(0)); } - [Fact] + [Test] public async Task GetVerifyQuery() { // Setup: Create a row update and set the first row cell to have values @@ -228,11 +228,11 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.EditData // ... There should be a table string tbl = m.Groups[1].Value; - Assert.Equal(data.TableMetadata.EscapedMultipartName, tbl); + Assert.AreEqual(data.TableMetadata.EscapedMultipartName, tbl); // ... There should be as many where components as there are keys string[] whereComponents = m.Groups[2].Value.Split(new[] { "AND" }, StringSplitOptions.None); - Assert.Equal(expectedKeys, whereComponents.Length); + Assert.AreEqual(expectedKeys, whereComponents.Length); // ... Mock db connection for building the command var mockConn = new TestSqlConnection(new[] { testResultSet });