From f7036f3f735cb7eafa1d4ede0558e6c19c831050 Mon Sep 17 00:00:00 2001 From: Benjamin Russell Date: Mon, 27 Mar 2017 17:14:21 -0700 Subject: [PATCH] Adding decoding of multipart identifiers, default schema workaround (#295) This change adds a couple things _Multipart Identifier Decoding_ The ability to decode a multipart identifier (with or without escaping) has been added to the SqlScriptFormatter utility class. This code is utilized to split a table name provided to the edit/initialize request into schema and table name. _Default Schema Workaround_ The code that retrieves the SMO metadata objects originally used the `[]` operator to access the objects. Due to a bug(?) in SMO, this results in problems when loading tables without a default schema (in our case if you're logged in as SA). Using the metadata object constructors gets around this issue, we are explicitly using them. * Adding decoding of multipart identifiers Adding code fix for default schema issue * Adding some more localizable strings for errors when loading metadata * Adding localization files... again? * Changes as per pull request comments --- .../EditData/EditSession.cs | 5 +- .../EditData/IEditMetadataFactory.cs | 8 +- .../EditData/SmoEditMetadataFactory.cs | 28 +++++-- .../Localization/sr.cs | 33 ++++++++ .../Localization/sr.resx | 12 +++ .../Localization/sr.strings | 6 ++ .../Localization/sr.xlf | 15 ++++ .../Utility/SqlScriptFormatter.cs | 83 +++++++++++++++++++ .../EditData/Common.cs | 2 +- .../EditData/SessionTests.cs | 8 +- .../Utility/SqlScriptFormatterTests.cs | 52 ++++++++++++ 11 files changed, 235 insertions(+), 17 deletions(-) diff --git a/src/Microsoft.SqlTools.ServiceLayer/EditData/EditSession.cs b/src/Microsoft.SqlTools.ServiceLayer/EditData/EditSession.cs index c4b2407b..81b72067 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/EditData/EditSession.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/EditData/EditSession.cs @@ -15,6 +15,7 @@ using Microsoft.SqlTools.ServiceLayer.EditData.Contracts; using Microsoft.SqlTools.ServiceLayer.EditData.UpdateManagement; using Microsoft.SqlTools.ServiceLayer.QueryExecution; using Microsoft.SqlTools.ServiceLayer.QueryExecution.Contracts; +using Microsoft.SqlTools.ServiceLayer.Utility; using Microsoft.SqlTools.Utility; namespace Microsoft.SqlTools.ServiceLayer.EditData @@ -423,14 +424,14 @@ namespace Microsoft.SqlTools.ServiceLayer.EditData try { // Step 1) Look up the SMO metadata - objectMetadata = metadataFactory.GetObjectMetadata(await connector(), initParams.ObjectName, + string[] namedParts = SqlScriptFormatter.DecodeMultipartIdenfitier(initParams.ObjectName); + objectMetadata = metadataFactory.GetObjectMetadata(await connector(), namedParts, initParams.ObjectType); // Step 2) Get and execute a query for the rows in the object we're looking up EditSessionQueryExecutionState state = await queryRunner(ConstructInitializeQuery(objectMetadata, initParams.Filters)); if (state.Query == null) { - // TODO: Move to SR file string message = state.Message ?? SR.EditDataQueryFailed; throw new Exception(message); } diff --git a/src/Microsoft.SqlTools.ServiceLayer/EditData/IEditMetadataFactory.cs b/src/Microsoft.SqlTools.ServiceLayer/EditData/IEditMetadataFactory.cs index c145d24e..654ad3f5 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/EditData/IEditMetadataFactory.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/EditData/IEditMetadataFactory.cs @@ -16,9 +16,13 @@ namespace Microsoft.SqlTools.ServiceLayer.EditData /// Generates a edit-ready metadata object /// /// Connection to use for getting metadata - /// Name of the object to return metadata for + /// + /// The multipart name for the object split and unwrapped. At most two components can be + /// provided (schema, table/view name). At minimum table/view name can be provided, and + /// default schema will be used for schema name. + /// /// Type of the object to return metadata for /// Metadata about the object requested - EditTableMetadata GetObjectMetadata(DbConnection connection, string objectName, string objectType); + EditTableMetadata GetObjectMetadata(DbConnection connection, string[] objectNamedParts, string objectType); } } \ No newline at end of file diff --git a/src/Microsoft.SqlTools.ServiceLayer/EditData/SmoEditMetadataFactory.cs b/src/Microsoft.SqlTools.ServiceLayer/EditData/SmoEditMetadataFactory.cs index c70fa51a..1e738fff 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/EditData/SmoEditMetadataFactory.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/EditData/SmoEditMetadataFactory.cs @@ -11,6 +11,7 @@ using Microsoft.SqlServer.Management.Common; using Microsoft.SqlServer.Management.Smo; using Microsoft.SqlTools.ServiceLayer.Connection.ReliableConnection; using Microsoft.SqlTools.ServiceLayer.Utility; +using Microsoft.SqlTools.Utility; namespace Microsoft.SqlTools.ServiceLayer.EditData { @@ -23,11 +24,21 @@ namespace Microsoft.SqlTools.ServiceLayer.EditData /// Generates a edit-ready metadata object using SMO /// /// Connection to use for getting metadata - /// Name of the object to return metadata for + /// Split and unwrapped name parts /// Type of the object to return metadata for /// Metadata about the object requested - public EditTableMetadata GetObjectMetadata(DbConnection connection, string objectName, string objectType) + public EditTableMetadata GetObjectMetadata(DbConnection connection, string[] objectNamedParts, string objectType) { + Validate.IsNotNull(nameof(objectNamedParts), objectNamedParts); + if (objectNamedParts.Length <= 0) + { + throw new ArgumentNullException(nameof(objectNamedParts), SR.EditDataMetadataObjectNameRequired); + } + if (objectNamedParts.Length > 2) + { + throw new InvalidOperationException(SR.EditDataMetadataTooManyIdentifiers); + } + // Get a connection to the database for SMO purposes SqlConnection sqlConn = connection as SqlConnection; if (sqlConn == null) @@ -46,22 +57,23 @@ namespace Microsoft.SqlTools.ServiceLayer.EditData // Connect with SMO and get the metadata for the table Server server = new Server(new ServerConnection(sqlConn)); + Database db = new Database(server, sqlConn.Database); TableViewTableTypeBase smoResult; switch (objectType.ToLowerInvariant()) { case "table": - smoResult = server.Databases[sqlConn.Database].Tables[objectName]; + smoResult = objectNamedParts.Length == 1 + ? new Table(db, objectNamedParts[0]) // No schema provided + : new Table(db, objectNamedParts[1], objectNamedParts[0]); // Schema provided break; case "view": - smoResult = server.Databases[sqlConn.Database].Views[objectName]; + smoResult = objectNamedParts.Length == 1 + ? new View(db, objectNamedParts[0]) // No schema provided + : new View(db, objectNamedParts[1], objectNamedParts[0]); // Schema provided break; default: throw new ArgumentOutOfRangeException(nameof(objectType), SR.EditDataUnsupportedObjectType(objectType)); } - if (smoResult == null) - { - throw new ArgumentOutOfRangeException(nameof(objectName), SR.EditDataObjectMetadataNotFound); - } // Generate the edit column metadata List editColumns = new List(); diff --git a/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.cs b/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.cs index 7cec4391..575baee4 100755 --- a/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.cs @@ -429,6 +429,22 @@ namespace Microsoft.SqlTools.ServiceLayer } } + public static string EditDataMetadataObjectNameRequired + { + get + { + return Keys.GetString(Keys.EditDataMetadataObjectNameRequired); + } + } + + public static string EditDataMetadataTooManyIdentifiers + { + get + { + return Keys.GetString(Keys.EditDataMetadataTooManyIdentifiers); + } + } + public static string EditDataFilteringNegativeLimit { get @@ -861,6 +877,14 @@ namespace Microsoft.SqlTools.ServiceLayer } } + public static string SqlScriptFormatterMultipartDecodeFail + { + get + { + return Keys.GetString(Keys.SqlScriptFormatterMultipartDecodeFail); + } + } + public static string ConnectionServiceListDbErrorNotConnected(string uri) { return Keys.GetString(Keys.ConnectionServiceListDbErrorNotConnected, uri); @@ -1128,6 +1152,12 @@ namespace Microsoft.SqlTools.ServiceLayer public const string EditDataMetadataNotExtended = "EditDataMetadataNotExtended"; + public const string EditDataMetadataObjectNameRequired = "EditDataMetadataObjectNameRequired"; + + + public const string EditDataMetadataTooManyIdentifiers = "EditDataMetadataTooManyIdentifiers"; + + public const string EditDataFilteringNegativeLimit = "EditDataFilteringNegativeLimit"; @@ -1293,6 +1323,9 @@ namespace Microsoft.SqlTools.ServiceLayer public const string SqlScriptFormatterDecimalMissingPrecision = "SqlScriptFormatterDecimalMissingPrecision"; + public const string SqlScriptFormatterMultipartDecodeFail = "SqlScriptFormatterMultipartDecodeFail"; + + private Keys() { } diff --git a/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.resx b/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.resx index 7c957864..6ffd0b0c 100755 --- a/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.resx +++ b/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.resx @@ -382,6 +382,14 @@ Table metadata does not have extended properties + + A object name must be provided + + + + Explicitly specifying server or database is not supported + + Result limit cannot be negative @@ -603,4 +611,8 @@ Decimal column is missing numeric precision or numeric scale + + Multipart identifier is incorrectly formatted + + diff --git a/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.strings b/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.strings index d36df7a9..9d7b1d92 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.strings +++ b/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.strings @@ -180,6 +180,10 @@ EditDataSessionAlreadyInitializing = Edit session has already been initialized o EditDataMetadataNotExtended = Table metadata does not have extended properties +EditDataMetadataObjectNameRequired = A object name must be provided + +EditDataMetadataTooManyIdentifiers = Explicitly specifying server or database is not supported + EditDataFilteringNegativeLimit = Result limit cannot be negative EditDataUnsupportedObjectType(string typeName) = Database object {0} cannot be used for editing. @@ -298,3 +302,5 @@ TestLocalizationConstant = EN_LOCALIZATION # Utilities SqlScriptFormatterDecimalMissingPrecision = Decimal column is missing numeric precision or numeric scale + +SqlScriptFormatterMultipartDecodeFail = Multipart identifier is incorrectly formatted diff --git a/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.xlf b/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.xlf index aa6ee853..4e0d9930 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.xlf +++ b/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.xlf @@ -601,6 +601,21 @@ NULL + + Multipart identifier is incorrectly formatted + Multipart identifier is incorrectly formatted + + + + A object name must be provided + A object name must be provided + + + + Explicitly specifying server or database is not supported + Explicitly specifying server or database is not supported + + Table metadata does not have extended properties Table metadata does not have extended properties diff --git a/src/Microsoft.SqlTools.ServiceLayer/Utility/SqlScriptFormatter.cs b/src/Microsoft.SqlTools.ServiceLayer/Utility/SqlScriptFormatter.cs index 5d9e6e6e..47559de9 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Utility/SqlScriptFormatter.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/Utility/SqlScriptFormatter.cs @@ -176,6 +176,83 @@ namespace Microsoft.SqlTools.ServiceLayer.Utility return literal; } + public static string[] DecodeMultipartIdenfitier(string multipartIdentifier) + { + StringBuilder sb = new StringBuilder(); + List namedParts = new List(); + bool insideBrackets = false; + bool bracketsClosed = false; + for (int i = 0; i < multipartIdentifier.Length; i++) + { + char iChar = multipartIdentifier[i]; + if (insideBrackets) + { + if (iChar == ']') + { + if (HasNextCharacter(multipartIdentifier, ']', i)) + { + // This is an escaped ] + sb.Append(iChar); + i++; + } + else + { + // This bracket closes the bracket we were in + insideBrackets = false; + bracketsClosed = true; + } + } + else + { + // This is a standard character + sb.Append(iChar); + } + } + else + { + switch (iChar) + { + case '[': + if (bracketsClosed) + { + throw new FormatException(); + } + + // We're opening a set of brackets + insideBrackets = true; + bracketsClosed = false; + break; + case '.': + if (sb.Length == 0) + { + throw new FormatException(); + } + + // We're splitting the identifier into a new part + namedParts.Add(sb.ToString()); + sb = new StringBuilder(); + bracketsClosed = false; + break; + default: + if (bracketsClosed) + { + throw new FormatException(); + } + + // This is a standard character + sb.Append(iChar); + break; + } + } + } + if (sb.Length == 0) + { + throw new FormatException(); + } + namedParts.Add(sb.ToString()); + return namedParts.ToArray(); + } + #region Private Helpers private static string SimpleFormatter(object value) @@ -260,6 +337,12 @@ namespace Microsoft.SqlTools.ServiceLayer.Utility return "0x" + BitConverter.ToString(bytes).Replace("-", string.Empty); } + private static bool HasNextCharacter(string haystack, char needle, int position) + { + return position + 1 < haystack.Length + && haystack[position + 1] == needle; + } + /// /// Returns a valid SQL string packaged in single quotes with single quotes inside escaped /// diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/EditData/Common.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/EditData/Common.cs index 1a0c2f2f..e5801d05 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/EditData/Common.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/EditData/Common.cs @@ -42,7 +42,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.EditData // Mock metadata factory Mock metaFactory = new Mock(); metaFactory - .Setup(f => f.GetObjectMetadata(It.IsAny(), It.IsAny(), It.IsAny())) + .Setup(f => f.GetObjectMetadata(It.IsAny(), It.IsAny(), It.IsAny())) .Returns(etm); EditSession session = new EditSession(metaFactory.Object); diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/EditData/SessionTests.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/EditData/SessionTests.cs index 180c6c87..3602bd0a 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/EditData/SessionTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/EditData/SessionTests.cs @@ -379,7 +379,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.EditData // Setup: // ... Create a metadata factory that throws Mock emf = new Mock(); - emf.Setup(f => f.GetObjectMetadata(It.IsAny(), It.IsAny(), It.IsAny())) + emf.Setup(f => f.GetObjectMetadata(It.IsAny(), It.IsAny(), It.IsAny())) .Throws(); // ... Create a session that hasn't been initialized @@ -412,7 +412,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.EditData var b = QueryExecution.Common.GetBasicExecutedBatch(); var etm = Common.GetStandardMetadata(b.ResultSets[0].Columns); Mock emf = new Mock(); - emf.Setup(f => f.GetObjectMetadata(It.IsAny(), It.IsAny(), It.IsAny())) + emf.Setup(f => f.GetObjectMetadata(It.IsAny(), It.IsAny(), It.IsAny())) .Returns(etm); // ... Create a session that hasn't been initialized @@ -451,7 +451,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.EditData var b = QueryExecution.Common.GetBasicExecutedBatch(); var etm = Common.GetStandardMetadata(b.ResultSets[0].Columns); Mock emf = new Mock(); - emf.Setup(f => f.GetObjectMetadata(It.IsAny(), It.IsAny(), It.IsAny())) + emf.Setup(f => f.GetObjectMetadata(It.IsAny(), It.IsAny(), It.IsAny())) .Returns(etm); // ... Create a session that hasn't been initialized @@ -490,7 +490,7 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.EditData var rs = q.Batches[0].ResultSets[0]; var etm = Common.GetStandardMetadata(rs.Columns); Mock emf = new Mock(); - emf.Setup(f => f.GetObjectMetadata(It.IsAny(), It.IsAny(), It.IsAny())) + emf.Setup(f => f.GetObjectMetadata(It.IsAny(), It.IsAny(), It.IsAny())) .Returns(etm); // ... Create a session that hasn't been initialized diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Utility/SqlScriptFormatterTests.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Utility/SqlScriptFormatterTests.cs index b34ea0a5..f077d9bb 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Utility/SqlScriptFormatterTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Utility/SqlScriptFormatterTests.cs @@ -7,6 +7,8 @@ using System; using System.Collections.Generic; using System.Data.Common; using System.Text.RegularExpressions; +using Microsoft.SqlServer.Management.Smo; +using Microsoft.SqlServer.Management.SqlParser.SqlCodeDom; using Microsoft.SqlTools.ServiceLayer.QueryExecution.Contracts; using Microsoft.SqlTools.ServiceLayer.Utility; using Xunit; @@ -308,6 +310,56 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Utility #endregion + #region DecodeMultipartIdentifier Tests + + [Theory] + [MemberData(nameof(DecodeMultipartIdentifierTestData))] + public void DecodeMultipartIdentifierTest(string input, string[] output) + { + // If: I decode the input + string[] decoded = SqlScriptFormatter.DecodeMultipartIdenfitier(input); + + // Then: The output should match what was expected + Assert.Equal(output, decoded); + } + + public static IEnumerable DecodeMultipartIdentifierTestData + { + get + { + yield return new object[] {"identifier", new[] {"identifier"}}; + yield return new object[] {"simple.split", new[] {"simple", "split"}}; + yield return new object[] {"multi.simple.split", new[] {"multi", "simple", "split"}}; + yield return new object[] {"[escaped]", new[] {"escaped"}}; + yield return new object[] {"[escaped].[split]", new[] {"escaped", "split"}}; + yield return new object[] {"[multi].[escaped].[split]", new[] {"multi", "escaped", "split"}}; + yield return new object[] {"[escaped]]characters]", new[] {"escaped]characters"}}; + yield return new object[] {"[multi]]escaped]]chars]", new[] {"multi]escaped]chars"}}; + yield return new object[] {"[multi]]]]chars]", new[] {"multi]]chars"}}; + yield return new object[] {"unescaped]chars", new[] {"unescaped]chars"}}; + yield return new object[] {"multi]unescaped]chars", new[] {"multi]unescaped]chars"}}; + yield return new object[] {"multi]]chars", new[] {"multi]]chars"}}; + yield return new object[] {"[escaped.dot]", new[] {"escaped.dot"}}; + yield return new object[] {"mixed.[escaped]", new[] {"mixed", "escaped"}}; + yield return new object[] {"[escaped].mixed", new[] {"escaped", "mixed"}}; + yield return new object[] {"dbo.[[].weird", new[] {"dbo", "[", "weird"}}; + } + } + + [Theory] + [InlineData("[bracket]closed")] + [InlineData("[bracket][closed")] + [InlineData(".stuff")] + [InlineData(".")] + public void DecodeMultipartIdentifierFailTest(string input) + { + // If: I decode an invalid input + // Then: It should throw an exception + Assert.Throws(() => SqlScriptFormatter.DecodeMultipartIdenfitier(input)); + } + + #endregion + [Theory] [InlineData("(0)", "0")] [InlineData("((0))", "0")]