Skip to content

Cleanup, Tests | MultipartIdentifier #3891

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
edwardneal wants to merge 18 commits into
base: main
Choose a base branch
from
Open

Cleanup, Tests | MultipartIdentifier #3891

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
65b528e
Remove leftQuote and rightQuote parameters
edwardneal Jan 13, 2026
62988ae
Remove constant-value separator parameter
edwardneal Jan 13, 2026
3f12787
Remove constant-value removeQuotes parameter
edwardneal Jan 13, 2026
b7f564a
Remove unnecessary test case
edwardneal Jan 13, 2026
5d084e3
Minor comment cleanup
edwardneal Jan 13, 2026
f797d3b
Remove constant-value rightQuote parameter
edwardneal Jan 13, 2026
ae5551b
Remove constant-value leftQuote parameter
edwardneal Jan 13, 2026
2215652
Minor cleanup
edwardneal Jan 13, 2026
df55f47
Comment and syntax cleanup
edwardneal Jan 14, 2026
4f6bfd4
if-block cleanup
edwardneal Jan 14, 2026
daa76c3
Remove unnecessary test case
edwardneal Jan 14, 2026
8830d5c
Move MultipartIdentifierTests to the UnitTests project, remove condit…
edwardneal Jan 14, 2026
5c581fb
Cleanup warning in MultipartIdentifierTests
edwardneal Jan 14, 2026
c79ac4b
Remove now-unnecessary ParseMultipartIdentifier overload
edwardneal Jan 14, 2026
36239ad
Move debug assertions to comments on the constants
edwardneal Jan 14, 2026
149606f
Naming cleanup
edwardneal Jan 14, 2026
1f7cde9
Enable nullability annotations
edwardneal Jan 14, 2026
1b5acff
Sense-check names of tests
edwardneal Jan 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
200 changes: 95 additions & 105 deletions src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/MultipartIdentifier.cs
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ private string CreateInitialQuery()
string[] parts;
try
{
parts = MultipartIdentifier.ParseMultipartIdentifier(DestinationTableName, "[\"", "]\"", Strings.SQL_BulkCopyDestinationTableName, true);
parts = MultipartIdentifier.ParseMultipartIdentifier(DestinationTableName, Strings.SQL_BulkCopyDestinationTableName, true);
}
catch (Exception e)
{
Expand Down Expand Up @@ -542,7 +542,7 @@ private string AnalyzeTargetAndCreateUpdateBulkCommand(BulkCopySimpleResultSet i
throw SQL.BulkLoadNoCollation();
}

string[] parts = MultipartIdentifier.ParseMultipartIdentifier(DestinationTableName, "[\"", "]\"", Strings.SQL_BulkCopyDestinationTableName, true);
string[] parts = MultipartIdentifier.ParseMultipartIdentifier(DestinationTableName, Strings.SQL_BulkCopyDestinationTableName, true);
updateBulkCommandText.AppendFormat("insert bulk {0} (", ADP.BuildMultiPartName(parts));

// Throw if there is a transaction but no flag is set
Expand Down
4 changes: 0 additions & 4 deletions src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1292,8 +1292,6 @@ internal void DeriveParameters()
// Schema, ProcedureName
string[] parsedSProc = MultipartIdentifier.ParseMultipartIdentifier(
name: CommandText,
leftQuote: "[\"",
rightQuote: "]\"",
property: Strings.SQL_SqlCommandCommandText,
ThrowOnEmptyMultipartName: false);

Expand Down Expand Up @@ -2809,8 +2807,6 @@ private void SetUpRPCParameters(_SqlRPC rpc, bool inSchema, SqlParameterCollecti
{
string[] parts = MultipartIdentifier.ParseMultipartIdentifier(
parameter.TypeName,
leftQuote: "[\"",
rightQuote: "]\"",
property: Strings.SQL_TDSParserTableName,
ThrowOnEmptyMultipartName: false);
// @TODO: Combine this and inner if statement
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2514,7 +2514,7 @@ internal static string[] ParseTypeName(string typeName, bool isUdtTypeName)
try
{
string errorMsg = isUdtTypeName ? Strings.SQL_UDTTypeName : Strings.SQL_TypeName;
return MultipartIdentifier.ParseMultipartIdentifier(typeName, "[\"", "]\"", '.', 3, true, errorMsg, true);
return MultipartIdentifier.ParseMultipartIdentifier(typeName, errorMsg, true, limit: 3);
}
catch (ArgumentException)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserHelperClasses.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,7 @@ private void ParseMultipartName()
{
if (_multipartName != null)
{
string[] parts = MultipartIdentifier.ParseMultipartIdentifier(_multipartName, "[\"", "]\"", Strings.SQL_TDSParserTableName, false);
string[] parts = MultipartIdentifier.ParseMultipartIdentifier(_multipartName, Strings.SQL_TDSParserTableName, false);
_serverName = parts[0];
_catalogName = parts[1];
_schemaName = parts[2];
Expand Down
2 changes: 0 additions & 2 deletions ...soft.Data.SqlClient/tests/FunctionalTests/Microsoft.Data.SqlClient.FunctionalTests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
<Compile Include="DataCommon\SystemDataResourceManager.cs" />
<Compile Include="DataCommon\TestUtility.cs" />
<Compile Include="LocalizationTest.cs" />
<Compile Include="MultipartIdentifierTests.cs" />
<Compile Include="MultiplexerTests.cs" />
<Compile Include="SqlAuthenticationProviderTest.cs" />
<Compile Include="SqlClientLoggerTest.cs" />
Expand Down Expand Up @@ -64,7 +63,6 @@
<Compile Include="SerializeSqlTypesTest.cs" />
<Compile Include="TdsParserStateObject.TestHarness.cs" />
<Compile Include="SqlHelperTest.cs" />
<Compile Include="..\..\src\Microsoft\Data\Common\MultipartIdentifier.cs" />
<Compile Include="..\..\src\Microsoft\Data\SqlClient\TdsParserStateObject.Multiplexer.cs" />
<Compile Include="..\..\src\Microsoft\Data\SqlClient\Packet.cs" />
</ItemGroup>
Expand Down
142 changes: 49 additions & 93 deletions ...nctionalTests/MultipartIdentifierTests.cs → ...t/Data/Common/MultipartIdentifierTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,155 +3,133 @@
// See the LICENSE file in the project root for more information.

using System;
using Microsoft.Data.Common;
using Xunit;

namespace Microsoft.Data.SqlClient.Tests
namespace Microsoft.Data.Common.UnitTests
{
public class MultipartIdentifierTests
{
[Fact]
public void SingleUnquoted() => RunParse("foo", new[] { "foo" });
public void SingleUnquoted() => RunParse("foo", ["foo"]);

[Fact]
public void SingleUnquotedOvercount() => RunParse("foo", new[] { null, "foo" }, maxCount: 2);
public void SingleUnquotedOvercount() => RunParse("foo", [null, "foo"], maxCount: 2);

[Fact]
public void SingleUnquotedContainsWhitespace() => RunParse("foo bar", new[] { "foo bar" });
public void SingleUnquotedContainsWhitespace() => RunParse("foo bar", ["foo bar"]);

[Fact]
public void SingleUnquotedStartWithShitespace() => RunParse(" foo", new[] { "foo" });
public void SingleUnquotedStartWithShitespace() => RunParse(" foo", ["foo"]);

[Fact]
public void SingleUnquotedEndWithShitespace() => RunParse("foo ", new[] { "foo" });
public void SingleUnquotedEndWithShitespace() => RunParse("foo ", ["foo"]);

[Fact]
public void SingleQuotedRemoveQuote() => RunParse("[foo]", new[] { "[foo]" }, false);
public void SingleQuotedKeepQuote() => RunParse("[foo]", ["foo"]);
Copy link
Contributor

@paulmedynski paulmedynski Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the old tests were named backwards. This test shows that quotes are removed.

Copy link
Contributor Author

@edwardneal edwardneal Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've sanity-checked all of the test names. I think I've caught the typos, any backwards names and a few names which refer to ] as quotes.


[Fact]
public void SingleQuotedKeepQuote() => RunParse("[foo]", new[] { "foo" }, true);
public void SingleQuotedLeadingWhitespace() => RunParse("[ foo]", [" foo"]);

[Fact]
public void SingleQuotedLeadingWhitespace() => RunParse("[ foo]", new[] { " foo" }, true);
public void SingleQuotedTrailingWhitespace() => RunParse("[foo ]", ["foo "]);

[Fact]
public void SingleQuotedTrailingWhitespace() => RunParse("[foo ]", new[] { "foo " }, true);
public void QuotedContainsWhitespace() => RunParse("[foo bar]", ["foo bar"]);

[Fact]
public void QuotedContainsWhitespace() => RunParse("[foo bar]", new[] { "foo bar" }, true);
public void SingleQuotedContainsAndTrailingWhitespace() => RunParse("[foo bar ]", ["foo bar "]);

[Fact]
public void SingleQuotedContainsAndTrailingWhitespace() => RunParse("[foo bar ]", new[] { "foo bar " });
public void SingleQuotedInternalAndLeadingWhitespace() => RunParse("[ foo bar]", [" foo bar"]);

[Fact]
public void SingleQuotedInternalAndLeadingWhitespace() => RunParse("[ foo bar]", new[] { " foo bar" });
public void SingleQuotedContainsAndLeadingAndTrailingWhitespace() => RunParse("[ foo bar ]", [" foo bar "]);

[Fact]
public void SingleQuotedContainsAndLeadingAndTrailingWhitespace() => RunParse("[ foo bar ]", new[] { " foo bar " });
public void SingleQuotedEscapedQuote() => RunParse("[foo]]bar]", ["foo]bar"]);

[Fact]
public void SingleQuotedEscapedQuote() => RunParse("[foo]]bar]", new[] { "foo]bar" }, true);


[Fact]
public void DoubleUnquotedParts() => RunParse("foo.bar", new[] { "foo", "bar" });

[Fact]
public void DoubleUnquotedPartContainsTrailngWhitespace() => RunParse("foo .bar", new[] { "foo", "bar" });
public void DoubleUnquotedParts() => RunParse("foo.bar", ["foo", "bar"]);

[Fact]
public void DoubleUnquotedPartContainsLeadingWhitespace() => RunParse("foo. bar", new[] { "foo", "bar" });
public void DoubleUnquotedPartContainsTrailngWhitespace() => RunParse("foo .bar", ["foo", "bar"]);

[Fact]
public void DoubleUnquotedEmptyFirst() => RunParse(".bar", new[] { "", "bar" });
public void DoubleUnquotedPartContainsLeadingWhitespace() => RunParse("foo. bar", ["foo", "bar"]);

[Fact]
public void DoubleUnquotedEmptyLast() => RunParse("foo.", new[] { "foo", "" });
public void DoubleUnquotedEmptyFirst() => RunParse(".bar", ["", "bar"]);

[Fact]
public void DoubleQuotedParts() => RunParse("[foo].[bar]", new string[] { "foo", "bar" });
public void DoubleUnquotedEmptyLast() => RunParse("foo.", ["foo", ""]);

[Fact]
public void DoubleQuotedPartContainsLeadingWhitespace() => RunParse("[foo]. [bar]", new[] { "foo", "bar" });

[Fact]
public void DoubleQuotedPartContainsTrailngWhitespace() => RunParse("[foo] .[bar]", new[] { "foo", "bar" });

public void DoubleQuotedParts() => RunParse("[foo].[bar]", ["foo", "bar"]);

[Fact]
public void TripleUnquotedParts() => RunParse("foo.bar.ed", new[] { "foo", "bar", "ed" });
public void DoubleQuotedPartContainsLeadingWhitespace() => RunParse("[foo]. [bar]", ["foo", "bar"]);

[Fact]
public void TripleUnquotedMissingMiddle() => RunParse("foo..bar", new[] { "foo", "", "bar" });
public void DoubleQuotedPartContainsTrailngWhitespace() => RunParse("[foo] .[bar]", ["foo", "bar"]);

[Fact]
public void TripleUnquotedPartContainsTrailingWhitespace() => RunParse("foo .bar .ed", new[] { "foo", "bar", "ed" });

[Fact]
public void TripleUnquotedPartContainsEmptyAndTrailngWhitespace() => RunParse(" .bar .ed", new[] { "", "bar", "ed" });
public void TripleUnquotedParts() => RunParse("foo.bar.ed", ["foo", "bar", "ed"]);

[Fact]
public void TripleUnquotedPartContainsLeadingWhitespace() => RunParse("foo. bar.", new[] { "foo", "bar", "" });
public void TripleUnquotedMissingMiddle() => RunParse("foo..bar", ["foo", "", "bar"]);

[Fact]
public void TripleUnquotedEmptyPart() => RunParse(".bar", new[] { "", "bar" });
public void TripleUnquotedPartContainsTrailingWhitespace() => RunParse("foo .bar .ed", ["foo", "bar", "ed"]);

[Fact]
public void TripleQuotedParts() => RunParse("[foo].[bar]", new[] { "foo", "bar" });
public void TripleUnquotedPartContainsEmptyAndTrailngWhitespace() => RunParse(" .bar .ed", ["", "bar", "ed"]);

[Fact]
public void TripleQuotedPartContainsLeadingWhitespace() => RunParse("[foo]. [bar]", new[] { "foo", "bar" });
public void TripleUnquotedPartContainsLeadingWhitespace() => RunParse("foo. bar.", ["foo", "bar", ""]);

[Fact]
public void TripleQuotedPartContainsTrailngWhitespace() => RunParse("[foo] .[bar]", new[] { "foo", "bar" });
public void TripleUnquotedEmptyPart() => RunParse(".bar", ["", "bar"]);

[Fact]
public void InvalidUnquotedEmpty() => ThrowParse<ArgumentException>("", new[] { "" });
public void TripleQuotedParts() => RunParse("[foo].[bar]", ["foo", "bar"]);

[Fact]
public void InvalidContainsOpen() => ThrowParse<ArgumentException>("foo[bar", new[] { "foo[bar" });
public void TripleQuotedPartContainsLeadingWhitespace() => RunParse("[foo]. [bar]", ["foo", "bar"]);

[Fact]
public void InvalidContainsClose() => ThrowParse<ArgumentException>("foo]bar", new[] { "foo]bar" });
public void TripleQuotedPartContainsTrailngWhitespace() => RunParse("[foo] .[bar]", ["foo", "bar"]);

[Fact]
public void InvalidStartsWithClose() => ThrowParse<ArgumentException>("]bar", new[] { "]bar" });
public void InvalidUnquotedEmpty() => ThrowParse<ArgumentException>("", [""]);

[Fact]
public void InvalidEndsWithClose() => ThrowParse<ArgumentException>("bar]", new[] { "bar]" });
public void InvalidContainsOpen() => ThrowParse<ArgumentException>("foo[bar", ["foo[bar"]);

[Fact]
public void InvalidUnfinishedBraceOpen() => ThrowParse<ArgumentException>("[foo", new[] { "[foo" });
public void InvalidContainsClose() => ThrowParse<ArgumentException>("foo]bar", ["foo]bar"]);

[Fact]
public void InvalidUnfinishedQuoteOpen() => ThrowParse<ArgumentException>("\"foo", new[] { "\"foo" });
public void InvalidStartsWithClose() => ThrowParse<ArgumentException>("]bar", ["]bar"]);

[Fact]
public void InvalidCapacity()
{
ThrowParse<ArgumentException>("", Array.Empty<string>());
}
public void InvalidEndsWithClose() => ThrowParse<ArgumentException>("bar]", ["bar]"]);

[Fact]
public void InvalidLeftQuote()
{
ThrowParse<ArgumentException>("foo", new[] { "foo" }, leftQuotes: "[.");
}
public void InvalidUnfinishedBraceOpen() => ThrowParse<ArgumentException>("[foo", ["[foo"]);

[Fact]
public void InvalidRightQuote()
{
ThrowParse<ArgumentException>("foo", new[] { "foo" }, rightQuotes: "[.");
}
public void InvalidUnfinishedQuoteOpen() => ThrowParse<ArgumentException>("\"foo", ["\"foo"]);

[Fact]
public void InvalidQuotedPartContainsTrailngNonWhitespace() => ThrowParse<ArgumentException>("[foo]!.[bar]", new[] { "foo", "bar" });
public void InvalidQuotedPartContainsTrailngNonWhitespace() => ThrowParse<ArgumentException>("[foo]!.[bar]", ["foo", "bar"]);

[Fact]
public void InvalidQuotedPartContainsTrailngWhiteSpaceThenNonWhitespace() => ThrowParse<ArgumentException>("[foo] !.[bar]", new[] { "foo", "bar" });
public void InvalidQuotedPartContainsTrailngWhiteSpaceThenNonWhitespace() => ThrowParse<ArgumentException>("[foo] !.[bar]", ["foo", "bar"]);

[Fact]
public void InvalidTooManyParts_2to1() => ThrowParse<ArgumentException>("foo.bar", new[] { "foo" });
public void InvalidTooManyParts_2to1() => ThrowParse<ArgumentException>("foo.bar", ["foo"]);

[Fact]
public void InvalidTooManyPartsEndsInSeparator() => ThrowParse("a.", 1);
Expand All @@ -178,7 +156,7 @@ public void InvalidRightQuote()
public void InvalidTooManyPartsSeparatorAfterPart() => ThrowParse("a.b.c", 1);


private static void RunParse(string name, string[] expected, bool removeQuotes = true, int maxCount = 0)
private static void RunParse(string name, string?[] expected, int maxCount = 0)
{
if (maxCount == 0)
{
Expand All @@ -191,18 +169,18 @@ private static void RunParse(string name, string[] expected, bool removeQuotes =
}
}

string[] originalParts = MultipartIdentifier.ParseMultipartIdentifier(name, "[\"", "]\"", '.', maxCount, removeQuotes, "", true);
string?[] originalParts = MultipartIdentifier.ParseMultipartIdentifier(name, "", true, maxCount);

for (int index = 0; index < expected.Length; index++)
{
string expectedPart = expected[index];
string originalPart = originalParts[index];
string? expectedPart = expected[index];
string? originalPart = originalParts[index];

Assert.Equal(expectedPart, originalPart);
}
}

private static void ThrowParse<TException>(string name, string[] expected, bool removeQuotes = true, string leftQuotes = "[\"", string rightQuotes = "]\"", char separator = '.')
private static void ThrowParse<TException>(string name, string?[] expected)
where TException : Exception
{
int maxCount = 0;
Expand All @@ -215,46 +193,24 @@ private static void ThrowParse<TException>(string name, string[] expected, bool
}

Exception originalException = Assert.Throws<TException>(() =>
MultipartIdentifier.ParseMultipartIdentifier(name, leftQuotes, rightQuotes, separator, maxCount, removeQuotes, "", true)
MultipartIdentifier.ParseMultipartIdentifier(name, "", true, maxCount)
);

Assert.NotNull(originalException);
}



private static void ThrowParse(string name, int expectedLength, bool removeQuotes = true, string leftQuotes = "[\"", string rightQuotes = "]\"", char separator = '.')
private static void ThrowParse(string name, int expectedLength)
{
Exception originalException = Assert.Throws<ArgumentException>(
() =>
{
MultipartIdentifier.ParseMultipartIdentifier(name, leftQuotes, rightQuotes, separator, expectedLength, removeQuotes, "test", true);
MultipartIdentifier.ParseMultipartIdentifier(name, "test", true, expectedLength);
}
);
Assert.NotNull(originalException);
}

}
}

namespace Microsoft.Data.Common
{
// this is needed for the inclusion of MultipartIdentifier class
internal class ADP
{
internal static ArgumentException InvalidMultipartName(string property, string name)
{
return new ArgumentException();
}

internal static ArgumentException InvalidMultipartNameIncorrectUsageOfQuotes(string property, string name)
{
return new ArgumentException();
}

internal static ArgumentException InvalidMultipartNameToManyParts(string property, string name, int limit)
{
return new ArgumentException();
}
}
}
Loading