Skip to content

Commit c45b389

Browse files
committed
feat: enhance role membership syntax compatibility handling and add related tests
1 parent 6adc3b7 commit c45b389

5 files changed

Lines changed: 109 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/).
1212
- Match legacy non-canonical schema-less object filenames to the scripted object name when the canonical name requires percent escaping.
1313
- Allow bare `--object` selectors with dotted schema-less names (for example assemblies) to resolve correctly during targeted `status`, `diff`, and `pull`.
1414
- Treat equivalent queue option formatting, explicit default `ON [PRIMARY]`, and disabled default activation as compatible during comparison.
15+
- Treat equivalent role-membership statements written as `EXEC sp_addrolemember ...` or `ALTER ROLE ... ADD MEMBER ...` as compatible during comparison.
1516
- Treat legacy Service Broker message-type validation synonyms and equivalent contract/service body formatting and item ordering as compatible during comparison.
1617
- Treat legacy explicit `NULL` tokens on CLR table-valued function return columns as compatible during comparison and preserve them during compatibility reconciliation when the rest of the definition matches.
1718
- Trailing semicolon differences on `INSERT` statement lines in data scripts are now suppressed during comparison normalization; scripts emitted with and without statement terminators compare as compatible (#47).

specs/01-cli.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ Behavior:
199199
- Whitespace-only lines are normalized to empty lines during comparison so blank separators with spaces or tabs compare as compatible.
200200
- Trailing semicolons on `INSERT` statement lines are stripped during normalization; scripts emitted with and without statement terminators compare as compatible.
201201
- Equivalent `Queue` option spacing, line wrapping, explicit default `ON [PRIMARY]`, and disabled default activation compare as compatible.
202+
- Equivalent `Role` membership statements written as `EXEC sp_addrolemember ...` or `ALTER ROLE ... ADD MEMBER ...` compare as compatible.
202203
- Equivalent `MessageType` validation synonyms/spacing and equivalent `Contract` and `Service` body formatting and item ordering compare as compatible.
203204
- When `data.trackedTables` is configured, `status` also reports data-script differences for tracked tables.
204205
- Status output MUST report schema and data summaries separately.
@@ -223,6 +224,7 @@ Behavior:
223224
- Whitespace-only lines are normalized to empty lines during comparison so blank separators with spaces or tabs compare as compatible.
224225
- Trailing semicolons on `INSERT` statement lines are stripped during normalization; scripts emitted with and without statement terminators compare as compatible.
225226
- Equivalent `Queue` option spacing, line wrapping, explicit default `ON [PRIMARY]`, and disabled default activation compare as compatible.
227+
- Equivalent `Role` membership statements written as `EXEC sp_addrolemember ...` or `ALTER ROLE ... ADD MEMBER ...` compare as compatible.
226228
- Equivalent `MessageType` validation synonyms/spacing and equivalent `Contract` and `Service` body formatting and item ordering compare as compatible.
227229
- Diff output uses a chunked format: only changed lines and their surrounding context are shown, not the entire file.
228230
- `--context <N>` controls the number of unchanged context lines shown before and after each changed segment (default: 3). Negative values are treated as 0.

specs/04-scripting.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -805,6 +805,7 @@ When compatibility reference files are available, `sqlct` MAY apply reconciliati
805805
- For `TableData`, trailing semicolons on `SET IDENTITY_INSERT` lines MUST also be stripped by comparison normalization.
806806
- For `TableData`, comparison normalization MUST treat legacy top-level `N'...'` string literals inside single-line or multi-line `INSERT ... VALUES (...)` statements as compatible with canonical `'...'` literals; canonical script generation remains governed by Section 8.26.
807807
- For `Queue`, comparison normalization MUST treat equivalent single-line and multi-line queue option formatting as compatible, MAY treat explicit `ON [PRIMARY]` as equivalent to an omitted default primary filegroup, and MAY treat disabled activation containing only default owner execution context as equivalent to omitted activation.
808+
- For `Role`, comparison normalization MAY treat legacy `EXEC sp_addrolemember N'<role>', N'<member>'` statements as compatible with `ALTER ROLE [role] ADD MEMBER [member]` when the effective role-membership change is otherwise identical.
808809
- For `MessageType`, comparison normalization MAY treat legacy `VALIDATION = XML` as compatible with canonical `VALIDATION = WELL_FORMED_XML`, and MAY ignore equivalent spacing around the validation assignment.
809810
- For `Contract`, comparison normalization MAY treat equivalent single-line and multi-line body formatting as compatible and MAY compare message-usage item ordering as compatible when the emitted usage set is otherwise identical.
810811
- For `Service`, comparison normalization MAY treat equivalent single-line and multi-line contract-list formatting as compatible and MAY compare contract item ordering as compatible when the emitted contract set is otherwise identical.

src/SqlChangeTracker/Sync/SyncCommandService.cs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,12 @@ internal sealed class SyncCommandService : ISyncCommandService
5454
private static readonly Regex ClrTableValuedFunctionReturnColumnNullWithCloseParenRegex = new(
5555
@"^(?<prefix>\s*(?:\[[^\]]+\]|[A-Za-z_][\w@#$]*)\s+(?:(?:\[[^\]]+\]|[A-Za-z_][\w@#$]*)(?:\.(?:\[[^\]]+\]|[A-Za-z_][\w@#$]*))?)(?:\s*\([^)]*\))?)\s+NULL(?<suffix>\s*\)\s*)$",
5656
RegexOptions.IgnoreCase | RegexOptions.CultureInvariant | RegexOptions.Compiled);
57+
private static readonly Regex RoleMembershipLegacySyntaxRegex = new(
58+
@"^\s*EXEC(?:UTE)?\s+sp_addrolemember\s+N'(?<role>(?:''|[^'])*)'\s*,\s*N'(?<member>(?:''|[^'])*)'\s*;?\s*$",
59+
RegexOptions.IgnoreCase | RegexOptions.CultureInvariant | RegexOptions.Compiled);
60+
private static readonly Regex RoleMembershipAlterRoleSyntaxRegex = new(
61+
@"^\s*ALTER\s+ROLE\s+(?<role>\[[^\]]+(?:\]\])*\]|""(?:""""|[^""])+""|[^\s;]+)\s+ADD\s+MEMBER\s+(?<member>\[[^\]]+(?:\]\])*\]|""(?:""""|[^""])+""|[^\s;]+)\s*;?\s*$",
62+
RegexOptions.IgnoreCase | RegexOptions.CultureInvariant | RegexOptions.Compiled);
5763
private static readonly IReadOnlyList<SupportedSqlObjectType> ActiveObjectTypes = SupportedSqlObjectTypes.ActiveSync;
5864

5965
private readonly SqlctConfigReader _configReader;
@@ -1926,6 +1932,11 @@ internal static string NormalizeForComparison(string script, string? objectType)
19261932
return NormalizeQueueScriptForComparison(joined);
19271933
}
19281934

1935+
if (string.Equals(objectType, "Role", StringComparison.OrdinalIgnoreCase))
1936+
{
1937+
return NormalizeRoleScriptForComparison(joined);
1938+
}
1939+
19291940
if (string.Equals(objectType, "MessageType", StringComparison.OrdinalIgnoreCase))
19301941
{
19311942
return NormalizeServiceBrokerScriptForComparison(joined, NormalizeMessageTypeBaseBlockForComparison);
@@ -1991,6 +2002,44 @@ private static string NormalizeQueueScriptForComparison(string script)
19912002
return normalized;
19922003
}
19932004

2005+
private static string NormalizeRoleScriptForComparison(string script)
2006+
{
2007+
var lines = script.Split('\n');
2008+
for (var i = 0; i < lines.Length; i++)
2009+
{
2010+
lines[i] = NormalizeRoleMembershipLineForComparison(lines[i]);
2011+
}
2012+
2013+
return string.Join("\n", lines);
2014+
}
2015+
2016+
private static string NormalizeRoleMembershipLineForComparison(string line)
2017+
{
2018+
var legacyMatch = RoleMembershipLegacySyntaxRegex.Match(line);
2019+
if (legacyMatch.Success)
2020+
{
2021+
var roleName = UnescapeSqlStringLiteral(legacyMatch.Groups["role"].Value);
2022+
var memberName = UnescapeSqlStringLiteral(legacyMatch.Groups["member"].Value);
2023+
return $"ALTER ROLE {QuoteIdentifierForComparison(roleName)} ADD MEMBER {QuoteIdentifierForComparison(memberName)}";
2024+
}
2025+
2026+
var alterRoleMatch = RoleMembershipAlterRoleSyntaxRegex.Match(line);
2027+
if (alterRoleMatch.Success)
2028+
{
2029+
var roleName = UnquoteSqlIdentifier(alterRoleMatch.Groups["role"].Value);
2030+
var memberName = UnquoteSqlIdentifier(alterRoleMatch.Groups["member"].Value);
2031+
return $"ALTER ROLE {QuoteIdentifierForComparison(roleName)} ADD MEMBER {QuoteIdentifierForComparison(memberName)}";
2032+
}
2033+
2034+
return line;
2035+
}
2036+
2037+
private static string UnescapeSqlStringLiteral(string value)
2038+
=> value.Replace("''", "'", StringComparison.Ordinal);
2039+
2040+
private static string QuoteIdentifierForComparison(string value)
2041+
=> $"[{value.Replace("]", "]]", StringComparison.Ordinal)}]";
2042+
19942043
private static string NormalizeServiceBrokerScriptForComparison(
19952044
string script,
19962045
Func<string, string> normalizeBaseBlock)

tests/SqlChangeTracker.Tests/Sync/SyncCommandServiceTests.cs

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -947,6 +947,62 @@ public void BuildUnifiedDiff_Queue_SuppressesEquivalentMultilineActivationFormat
947947
Assert.Empty(diff);
948948
}
949949

950+
[Fact]
951+
public void BuildUnifiedDiff_Role_SuppressesLegacyAndAlterRoleMembershipSyntaxDifferences_ForFixedRole()
952+
{
953+
var source =
954+
"EXEC sp_addrolemember N'db_datareader', N'ReadOnlyUser'\n" +
955+
"GO\n" +
956+
"EXEC sp_addrolemember N'db_datareader', N'ReportUser'\n" +
957+
"GO";
958+
var target =
959+
"ALTER ROLE [db_datareader] ADD MEMBER [ReadOnlyUser]\n" +
960+
"GO\n" +
961+
"ALTER ROLE [db_datareader] ADD MEMBER [ReportUser]\n" +
962+
"GO";
963+
964+
var diff = SyncCommandService.BuildUnifiedDiff("Role", "db", "folder", source, target);
965+
966+
Assert.Empty(diff);
967+
}
968+
969+
[Fact]
970+
public void BuildUnifiedDiff_Role_SuppressesLegacyAndAlterRoleMembershipSyntaxDifferences_ForUserDefinedRole()
971+
{
972+
var source =
973+
"CREATE ROLE [ReportingRole]\n" +
974+
"AUTHORIZATION [dbo]\n" +
975+
"GO\n" +
976+
"EXEC sp_addrolemember N'ReportingRole', N'ReportUser'\n" +
977+
"GO";
978+
var target =
979+
"CREATE ROLE [ReportingRole]\n" +
980+
"AUTHORIZATION [dbo]\n" +
981+
"GO\n" +
982+
"ALTER ROLE [ReportingRole] ADD MEMBER [ReportUser]\n" +
983+
"GO";
984+
985+
var diff = SyncCommandService.BuildUnifiedDiff("Role", "db", "folder", source, target);
986+
987+
Assert.Empty(diff);
988+
}
989+
990+
[Fact]
991+
public void BuildUnifiedDiff_Role_PreservesMembershipTargetDifferences()
992+
{
993+
var source =
994+
"EXEC sp_addrolemember N'db_datareader', N'ReadOnlyUser'\n" +
995+
"GO";
996+
var target =
997+
"ALTER ROLE [db_datareader] ADD MEMBER [ReportUser]\n" +
998+
"GO";
999+
1000+
var diff = SyncCommandService.BuildUnifiedDiff("Role", "db", "folder", source, target);
1001+
1002+
Assert.Contains("ALTER ROLE [db_datareader] ADD MEMBER [ReadOnlyUser]", diff);
1003+
Assert.Contains("ALTER ROLE [db_datareader] ADD MEMBER [ReportUser]", diff);
1004+
}
1005+
9501006
[Fact]
9511007
public void BuildUnifiedDiff_MessageType_SuppressesLegacyValidationXmlSynonymAndSpacing()
9521008
{

0 commit comments

Comments
 (0)