mirror of
https://github.com/ckaczor/sqltoolsservice.git
synced 2026-01-13 17:23:02 -05:00
Addressing code review feedback
This commit is contained in:
@@ -111,11 +111,19 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection
|
||||
public ConnectResponse Connect(ConnectParams connectionParams)
|
||||
{
|
||||
// Validate parameters
|
||||
if(connectionParams == null || !connectionParams.IsValid())
|
||||
string paramValidationErrorMessage;
|
||||
if (connectionParams == null)
|
||||
{
|
||||
return new ConnectResponse()
|
||||
{
|
||||
Messages = "Error: Invalid connection parameters provided."
|
||||
Messages = "Error: Connection parameters cannot be null."
|
||||
};
|
||||
}
|
||||
else if (!connectionParams.IsValid(out paramValidationErrorMessage))
|
||||
{
|
||||
return new ConnectResponse()
|
||||
{
|
||||
Messages = paramValidationErrorMessage
|
||||
};
|
||||
}
|
||||
|
||||
@@ -168,7 +176,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection
|
||||
public bool Disconnect(DisconnectParams disconnectParams)
|
||||
{
|
||||
// Validate parameters
|
||||
if (disconnectParams == null || String.IsNullOrEmpty(disconnectParams.OwnerUri))
|
||||
if (disconnectParams == null || string.IsNullOrEmpty(disconnectParams.OwnerUri))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
@@ -203,7 +211,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection
|
||||
{
|
||||
// Verify parameters
|
||||
var owner = listDatabasesParams.OwnerUri;
|
||||
if (String.IsNullOrEmpty(owner))
|
||||
if (string.IsNullOrEmpty(owner))
|
||||
{
|
||||
throw new ArgumentException("OwnerUri cannot be null or empty");
|
||||
}
|
||||
@@ -355,11 +363,11 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection
|
||||
connectionBuilder["Password"] = connectionDetails.Password;
|
||||
|
||||
// Check for any optional parameters
|
||||
if (!String.IsNullOrEmpty(connectionDetails.DatabaseName))
|
||||
if (!string.IsNullOrEmpty(connectionDetails.DatabaseName))
|
||||
{
|
||||
connectionBuilder["Initial Catalog"] = connectionDetails.DatabaseName;
|
||||
}
|
||||
if (!String.IsNullOrEmpty(connectionDetails.AuthenticationType))
|
||||
if (!string.IsNullOrEmpty(connectionDetails.AuthenticationType))
|
||||
{
|
||||
switch(connectionDetails.AuthenticationType)
|
||||
{
|
||||
@@ -370,7 +378,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection
|
||||
connectionBuilder.IntegratedSecurity = false;
|
||||
break;
|
||||
default:
|
||||
throw new ArgumentException("Invalid value \"" + connectionDetails.AuthenticationType + "\" for AuthenticationType. Valid values are \"Integrated\" and \"SqlLogin\".");
|
||||
throw new ArgumentException(string.Format("Invalid value \"{0}\" for AuthenticationType. Valid values are \"Integrated\" and \"SqlLogin\".", connectionDetails.AuthenticationType));
|
||||
}
|
||||
}
|
||||
if (connectionDetails.Encrypt.HasValue)
|
||||
@@ -397,15 +405,15 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection
|
||||
{
|
||||
connectionBuilder.ConnectRetryInterval = connectionDetails.ConnectRetryInterval.Value;
|
||||
}
|
||||
if (!String.IsNullOrEmpty(connectionDetails.ApplicationName))
|
||||
if (!string.IsNullOrEmpty(connectionDetails.ApplicationName))
|
||||
{
|
||||
connectionBuilder.ApplicationName = connectionDetails.ApplicationName;
|
||||
}
|
||||
if (!String.IsNullOrEmpty(connectionDetails.WorkstationId))
|
||||
if (!string.IsNullOrEmpty(connectionDetails.WorkstationId))
|
||||
{
|
||||
connectionBuilder.WorkstationID = connectionDetails.WorkstationId;
|
||||
}
|
||||
if (!String.IsNullOrEmpty(connectionDetails.ApplicationIntent))
|
||||
if (!string.IsNullOrEmpty(connectionDetails.ApplicationIntent))
|
||||
{
|
||||
ApplicationIntent intent;
|
||||
switch (connectionDetails.ApplicationIntent)
|
||||
@@ -417,11 +425,11 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection
|
||||
intent = ApplicationIntent.ReadWrite;
|
||||
break;
|
||||
default:
|
||||
throw new ArgumentException("Invalid value \"" + connectionDetails.ApplicationIntent + "\" for ApplicationIntent. Valid values are \"ReadWrite\" and \"ReadOnly\".");
|
||||
throw new ArgumentException(string.Format("Invalid value \"{0}\" for ApplicationIntent. Valid values are \"ReadWrite\" and \"ReadOnly\".", connectionDetails.ApplicationIntent));
|
||||
}
|
||||
connectionBuilder.ApplicationIntent = intent;
|
||||
}
|
||||
if (!String.IsNullOrEmpty(connectionDetails.CurrentLanguage))
|
||||
if (!string.IsNullOrEmpty(connectionDetails.CurrentLanguage))
|
||||
{
|
||||
connectionBuilder.CurrentLanguage = connectionDetails.CurrentLanguage;
|
||||
}
|
||||
@@ -445,11 +453,11 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection
|
||||
{
|
||||
connectionBuilder.Replication = connectionDetails.Replication.Value;
|
||||
}
|
||||
if (!String.IsNullOrEmpty(connectionDetails.AttachDbFilename))
|
||||
if (!string.IsNullOrEmpty(connectionDetails.AttachDbFilename))
|
||||
{
|
||||
connectionBuilder.AttachDBFilename = connectionDetails.AttachDbFilename;
|
||||
}
|
||||
if (!String.IsNullOrEmpty(connectionDetails.FailoverPartner))
|
||||
if (!string.IsNullOrEmpty(connectionDetails.FailoverPartner))
|
||||
{
|
||||
connectionBuilder.FailoverPartner = connectionDetails.FailoverPartner;
|
||||
}
|
||||
@@ -465,7 +473,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection
|
||||
{
|
||||
connectionBuilder.PacketSize = connectionDetails.PacketSize.Value;
|
||||
}
|
||||
if (!String.IsNullOrEmpty(connectionDetails.TypeSystemVersion))
|
||||
if (!string.IsNullOrEmpty(connectionDetails.TypeSystemVersion))
|
||||
{
|
||||
connectionBuilder.TypeSystemVersion = connectionDetails.TypeSystemVersion;
|
||||
}
|
||||
|
||||
@@ -15,15 +15,42 @@ namespace Microsoft.SqlTools.ServiceLayer.Connection.Contracts
|
||||
/// <summary>
|
||||
/// Check that the fields in ConnectParams are all valid
|
||||
/// </summary>
|
||||
public static bool IsValid(this ConnectParams parameters)
|
||||
public static bool IsValid(this ConnectParams parameters, out string errorMessage)
|
||||
{
|
||||
return !(
|
||||
String.IsNullOrEmpty(parameters.OwnerUri) ||
|
||||
parameters.Connection == null ||
|
||||
String.IsNullOrEmpty(parameters.Connection.Password) ||
|
||||
String.IsNullOrEmpty(parameters.Connection.ServerName) ||
|
||||
String.IsNullOrEmpty(parameters.Connection.UserName)
|
||||
);
|
||||
errorMessage = string.Empty;
|
||||
if (string.IsNullOrEmpty(parameters.OwnerUri))
|
||||
{
|
||||
errorMessage = "Error: OwnerUri cannot be null or empty.";
|
||||
}
|
||||
else if (parameters.Connection == null)
|
||||
{
|
||||
errorMessage = "Error: Connection details object cannot be null.";
|
||||
}
|
||||
else if (string.IsNullOrEmpty(parameters.Connection.ServerName))
|
||||
{
|
||||
errorMessage = "Error: ServerName cannot be null or empty.";
|
||||
}
|
||||
else if (string.IsNullOrEmpty(parameters.Connection.AuthenticationType) || parameters.Connection.AuthenticationType == "SqlLogin")
|
||||
{
|
||||
// For SqlLogin, username/password cannot be empty
|
||||
if (string.IsNullOrEmpty(parameters.Connection.UserName))
|
||||
{
|
||||
errorMessage = "Error: UserName cannot be null or empty when using SqlLogin authentication.";
|
||||
}
|
||||
else if( string.IsNullOrEmpty(parameters.Connection.Password))
|
||||
{
|
||||
errorMessage = "Error: Password cannot be null or empty when using SqlLogin authentication.";
|
||||
}
|
||||
}
|
||||
|
||||
if (string.IsNullOrEmpty(errorMessage))
|
||||
{
|
||||
return true;
|
||||
}
|
||||
else
|
||||
{
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -152,15 +152,19 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.Connection
|
||||
/// 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", "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", "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)
|
||||
[InlineData("SqlLogin", null, "my-server", "test", "sa", "123456")]
|
||||
[InlineData("SqlLogin", "file://my/sample/file.sql", null, "test", "sa", "123456")]
|
||||
[InlineData("SqlLogin", "file://my/sample/file.sql", "my-server", "test", null, "123456")]
|
||||
[InlineData("SqlLogin", "file://my/sample/file.sql", "my-server", "test", "sa", null)]
|
||||
[InlineData("SqlLogin", "", "my-server", "test", "sa", "123456")]
|
||||
[InlineData("SqlLogin", "file://my/sample/file.sql", "", "test", "sa", "123456")]
|
||||
[InlineData("SqlLogin", "file://my/sample/file.sql", "my-server", "test", "", "123456")]
|
||||
[InlineData("SqlLogin", "file://my/sample/file.sql", "my-server", "test", "sa", "")]
|
||||
[InlineData("Integrated", null, "my-server", "test", "sa", "123456")]
|
||||
[InlineData("Integrated", "file://my/sample/file.sql", null, "test", "sa", "123456")]
|
||||
[InlineData("Integrated", "", "my-server", "test", "sa", "123456")]
|
||||
[InlineData("Integrated", "file://my/sample/file.sql", "", "test", "sa", "123456")]
|
||||
public void ConnectingWithInvalidParametersYieldsErrorMessage(string authType, string ownerUri, string server, string database, string userName, string password)
|
||||
{
|
||||
// Connect with invalid parameters
|
||||
var connectionResult =
|
||||
@@ -172,7 +176,8 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.Connection
|
||||
ServerName = server,
|
||||
DatabaseName = database,
|
||||
UserName = userName,
|
||||
Password = password
|
||||
Password = password,
|
||||
AuthenticationType = authType
|
||||
}
|
||||
});
|
||||
|
||||
@@ -181,6 +186,40 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.Connection
|
||||
Assert.NotEqual(String.Empty, connectionResult.Messages);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Verify that when using integrated authentication, the username and/or password can be empty.
|
||||
/// </summary>
|
||||
[Theory]
|
||||
[InlineData(null, null)]
|
||||
[InlineData(null, "")]
|
||||
[InlineData("", null)]
|
||||
[InlineData("", "")]
|
||||
[InlineData("sa", null)]
|
||||
[InlineData("sa", "")]
|
||||
[InlineData(null, "12345678")]
|
||||
[InlineData("", "12345678")]
|
||||
public void ConnectingWithNoUsernameOrPasswordWorksForIntegratedAuth(string userName, string password)
|
||||
{
|
||||
// Connect
|
||||
var connectionResult =
|
||||
TestObjects.GetTestConnectionService()
|
||||
.Connect(new ConnectParams()
|
||||
{
|
||||
OwnerUri = "file:///my/test/file.sql",
|
||||
Connection = new ConnectionDetails() {
|
||||
ServerName = "my-server",
|
||||
DatabaseName = "test",
|
||||
UserName = userName,
|
||||
Password = password,
|
||||
AuthenticationType = "Integrated"
|
||||
}
|
||||
});
|
||||
|
||||
// check that the connection was successful
|
||||
Assert.NotEmpty(connectionResult.ConnectionId);
|
||||
Assert.Null(connectionResult.Messages);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Verify that when connecting with a null parameters object, an error is thrown.
|
||||
/// </summary>
|
||||
@@ -201,38 +240,38 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.Connection
|
||||
/// Verify that optional parameters can be built into a connection string for connecting.
|
||||
/// </summary>
|
||||
[Theory]
|
||||
[InlineDataAttribute("AuthenticationType", "Integrated")]
|
||||
[InlineDataAttribute("AuthenticationType", "SqlLogin")]
|
||||
[InlineDataAttribute("Encrypt", true)]
|
||||
[InlineDataAttribute("Encrypt", false)]
|
||||
[InlineDataAttribute("TrustServerCertificate", true)]
|
||||
[InlineDataAttribute("TrustServerCertificate", false)]
|
||||
[InlineDataAttribute("PersistSecurityInfo", true)]
|
||||
[InlineDataAttribute("PersistSecurityInfo", false)]
|
||||
[InlineDataAttribute("ConnectTimeout", 15)]
|
||||
[InlineDataAttribute("ConnectRetryCount", 1)]
|
||||
[InlineDataAttribute("ConnectRetryInterval", 10)]
|
||||
[InlineDataAttribute("ApplicationName", "vscode-mssql")]
|
||||
[InlineDataAttribute("WorkstationId", "mycomputer")]
|
||||
[InlineDataAttribute("ApplicationIntent", "ReadWrite")]
|
||||
[InlineDataAttribute("ApplicationIntent", "ReadOnly")]
|
||||
[InlineDataAttribute("CurrentLanguage", "test")]
|
||||
[InlineDataAttribute("Pooling", false)]
|
||||
[InlineDataAttribute("Pooling", true)]
|
||||
[InlineDataAttribute("MaxPoolSize", 100)]
|
||||
[InlineDataAttribute("MinPoolSize", 0)]
|
||||
[InlineDataAttribute("LoadBalanceTimeout", 0)]
|
||||
[InlineDataAttribute("Replication", true)]
|
||||
[InlineDataAttribute("Replication", false)]
|
||||
[InlineDataAttribute("AttachDbFilename", "myfile")]
|
||||
[InlineDataAttribute("FailoverPartner", "partner")]
|
||||
[InlineDataAttribute("MultiSubnetFailover", true)]
|
||||
[InlineDataAttribute("MultiSubnetFailover", false)]
|
||||
[InlineDataAttribute("MultipleActiveResultSets", false)]
|
||||
[InlineDataAttribute("MultipleActiveResultSets", true)]
|
||||
[InlineDataAttribute("PacketSize", 8192)]
|
||||
[InlineDataAttribute("TypeSystemVersion", "Latest")]
|
||||
public void ConnectingWithOptionalParametersBuildsConnectionString(string propertyName, object propertyValue)
|
||||
[InlineData("AuthenticationType", "Integrated", "Integrated Security")]
|
||||
[InlineData("AuthenticationType", "SqlLogin", "Integrated Security")]
|
||||
[InlineData("Encrypt", true, "Encrypt")]
|
||||
[InlineData("Encrypt", false, "Encrypt")]
|
||||
[InlineData("TrustServerCertificate", true, "TrustServerCertificate")]
|
||||
[InlineData("TrustServerCertificate", false, "TrustServerCertificate")]
|
||||
[InlineData("PersistSecurityInfo", true, "Persist Security Info")]
|
||||
[InlineData("PersistSecurityInfo", false, "Persist Security Info")]
|
||||
[InlineData("ConnectTimeout", 15, "Connect Timeout")]
|
||||
[InlineData("ConnectRetryCount", 1, "ConnectRetryCount")]
|
||||
[InlineData("ConnectRetryInterval", 10, "ConnectRetryInterval")]
|
||||
[InlineData("ApplicationName", "vscode-mssql", "Application Name")]
|
||||
[InlineData("WorkstationId", "mycomputer", "Workstation ID")]
|
||||
[InlineData("ApplicationIntent", "ReadWrite", "ApplicationIntent")]
|
||||
[InlineData("ApplicationIntent", "ReadOnly", "ApplicationIntent")]
|
||||
[InlineData("CurrentLanguage", "test", "Current Language")]
|
||||
[InlineData("Pooling", false, "Pooling")]
|
||||
[InlineData("Pooling", true, "Pooling")]
|
||||
[InlineData("MaxPoolSize", 100, "Max Pool Size")]
|
||||
[InlineData("MinPoolSize", 0, "Min Pool Size")]
|
||||
[InlineData("LoadBalanceTimeout", 0, "Load Balance Timeout")]
|
||||
[InlineData("Replication", true, "Replication")]
|
||||
[InlineData("Replication", false, "Replication")]
|
||||
[InlineData("AttachDbFilename", "myfile", "AttachDbFilename")]
|
||||
[InlineData("FailoverPartner", "partner", "Failover Partner")]
|
||||
[InlineData("MultiSubnetFailover", true, "MultiSubnetFailover")]
|
||||
[InlineData("MultiSubnetFailover", false, "MultiSubnetFailover")]
|
||||
[InlineData("MultipleActiveResultSets", false, "MultipleActiveResultSets")]
|
||||
[InlineData("MultipleActiveResultSets", true, "MultipleActiveResultSets")]
|
||||
[InlineData("PacketSize", 8192, "Packet Size")]
|
||||
[InlineData("TypeSystemVersion", "Latest", "Type System Version")]
|
||||
public void ConnectingWithOptionalParametersBuildsConnectionString(string propertyName, object propertyValue, string connectionStringMarker)
|
||||
{
|
||||
// Create a test connection details object and set the property to a specific value
|
||||
ConnectionDetails details = TestObjects.GetTestConnectionDetails();
|
||||
@@ -243,6 +282,9 @@ namespace Microsoft.SqlTools.ServiceLayer.Test.Connection
|
||||
string connectionString = ConnectionService.BuildConnectionString(details);
|
||||
Assert.NotNull(connectionString);
|
||||
Assert.NotEmpty(connectionString);
|
||||
|
||||
// Verify that the parameter is in the connection string
|
||||
Assert.True(connectionString.Contains(connectionStringMarker));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
Reference in New Issue
Block a user