Skip to content

Commit a2d3b29

Browse files
johljujohlju
authored
Add-SqlDscTraceFlag: Simplify trace flag addition uniqueness (#2301)
1 parent f6b06ee commit a2d3b29

File tree

5 files changed

+219
-61
lines changed

5 files changed

+219
-61
lines changed

‎CHANGELOG.md‎

Lines changed: 33 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -111,35 +111,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
111111
by dropping and recreating the audit with the new GUID. This parameter is
112112
required when changing the `AuditGuid` property because SQL Server does not
113113
allow direct modification of the audit GUID ([issue #2287](https://github.com/dsccommunity/SqlServerDsc/issues/2287)).
114-
115-
### Changed
116-
117-
- Refactored integration tests to remove `finally` blocks from `It`-blocks and
118-
use Pester `BeforeEach`/`AfterEach` blocks instead, following DSC Community
119-
coding guidelines. This improves test cleanup reliability and maintainability
120-
across the following test files ([issue #2288](https://github.com/dsccommunity/SqlServerDsc/issues/2288)):
121-
- `New-SqlDscLogin.Integration.Tests.ps1`
122-
- `New-SqlDscAudit.Integration.Tests.ps1`
123-
- `Get-SqlDscDatabasePermission.Integration.Tests.ps1`
124-
- `Get-SqlDscPreferredModule.Integration.Tests.ps1`
125-
- `New-SqlDscDatabase.Integration.Tests.ps1`
126-
- Refactored unit tests to remove `finally` blocks from `It`-blocks and
127-
use Pester `AfterEach` blocks instead, following DSC Community coding
128-
guidelines. This improves test cleanup reliability and maintainability
129-
across the following test files ([issue #2288](https://github.com/dsccommunity/SqlServerDsc/issues/2288)):
130-
- `Set-SqlDscConfigurationOption.Tests.ps1`
131-
- `Get-SqlDscConfigurationOption.Tests.ps1`
132-
- `Test-SqlDscConfigurationOption.Tests.ps1`
133-
- `Test-SqlDscIsDatabasePrincipal` and `Get-SqlDscDatabasePermission`
134-
- Added `Refresh` parameter to refresh SMO collections before checking
135-
database principals, addressing issues with custom database roles created
136-
via T-SQL that aren't immediately visible to SMO. The refresh logic is
137-
optimized to only refresh collections that will be used based on exclude
138-
parameters, improving performance on databases with large numbers of principals
139-
([issue #2221](https://github.com/dsccommunity/SqlServerDsc/issues/2221)).
140-
141-
### Fixed
142-
143114
- `Save-SqlDscSqlServerMediaFile`
144115
- Fixed the Force parameter to work correctly when the target ISO file already
145116
exists. The command now properly overwrites the target file when Force is
@@ -174,6 +145,39 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
174145
([issue #2239](https://github.com/dsccommunity/SqlServerDsc/issues/2239)).
175146
- Updated `.gitattributes` to enforce LF line endings for PowerShell files to
176147
ensure cross-platform compatibility.
148+
149+
### Changed
150+
151+
- `Add-SqlDscTraceFlag`
152+
- Improved de-duplication logic to normalize element types to `[System.UInt32]`
153+
before sorting and removing duplicates, ensuring proper handling of mixed
154+
numeric types ([issue #2277](https://github.com/dsccommunity/SqlServerDsc/issues/2277)).
155+
- Added idempotent behavior by comparing current and desired trace flags before
156+
calling `Set-SqlDscTraceFlag`, skipping unnecessary writes when there are no
157+
effective changes ([issue #2277](https://github.com/dsccommunity/SqlServerDsc/issues/2277)).
158+
- Refactored integration tests to remove `finally` blocks from `It`-blocks and
159+
use Pester `BeforeEach`/`AfterEach` blocks instead, following DSC Community
160+
coding guidelines. This improves test cleanup reliability and maintainability
161+
across the following test files ([issue #2288](https://github.com/dsccommunity/SqlServerDsc/issues/2288)):
162+
- `New-SqlDscLogin.Integration.Tests.ps1`
163+
- `New-SqlDscAudit.Integration.Tests.ps1`
164+
- `Get-SqlDscDatabasePermission.Integration.Tests.ps1`
165+
- `Get-SqlDscPreferredModule.Integration.Tests.ps1`
166+
- `New-SqlDscDatabase.Integration.Tests.ps1`
167+
- Refactored unit tests to remove `finally` blocks from `It`-blocks and
168+
use Pester `AfterEach` blocks instead, following DSC Community coding
169+
guidelines. This improves test cleanup reliability and maintainability
170+
across the following test files ([issue #2288](https://github.com/dsccommunity/SqlServerDsc/issues/2288)):
171+
- `Set-SqlDscConfigurationOption.Tests.ps1`
172+
- `Get-SqlDscConfigurationOption.Tests.ps1`
173+
- `Test-SqlDscConfigurationOption.Tests.ps1`
174+
- `Test-SqlDscIsDatabasePrincipal` and `Get-SqlDscDatabasePermission`
175+
- Added `Refresh` parameter to refresh SMO collections before checking
176+
database principals, addressing issues with custom database roles created
177+
via T-SQL that aren't immediately visible to SMO. The refresh logic is
178+
optimized to only refresh collections that will be used based on exclude
179+
parameters, improving performance on databases with large numbers of principals
180+
([issue #2221](https://github.com/dsccommunity/SqlServerDsc/issues/2221)).
177181
- Updated GitHub Copilot setup workflow to fix environment variable assignment
178182
in task.
179183
- Updated VS Code tasks configuration to use proper build and test commands

‎source/Public/Add-SqlDscTraceFlag.ps1‎

Lines changed: 41 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
Specifies the server name where the instance exist.
1414
1515
.PARAMETER InstanceName
16-
Specifies the instance name on which to remove the trace flags.
16+
Specifies the instance name on which to add the trace flags.
1717
1818
.PARAMETER TraceFlag
1919
Specifies the trace flags to add.
@@ -72,6 +72,7 @@ function Add-SqlDscTraceFlag
7272
$InstanceName = 'MSSQLSERVER',
7373

7474
[Parameter(Mandatory = $true)]
75+
[ValidateRange(1, [System.UInt32]::MaxValue)]
7576
[System.UInt32[]]
7677
$TraceFlag,
7778

@@ -112,35 +113,51 @@ function Add-SqlDscTraceFlag
112113

113114
$ErrorActionPreference = $originalErrorActionPreference
114115

115-
$desiredTraceFlags = [System.UInt32[]] $currentTraceFlags + @(
116-
$TraceFlag |
117-
ForEach-Object -Process {
118-
# Add only when it does not already exist.
119-
if ($_ -notin $currentTraceFlags)
120-
{
121-
$_
122-
}
123-
}
124-
)
125-
126-
$verboseDescriptionMessage = $script:localizedData.TraceFlag_Add_ShouldProcessVerboseDescription -f $InstanceName, ($TraceFlag -join ', ')
127-
$verboseWarningMessage = $script:localizedData.TraceFlag_Add_ShouldProcessVerboseWarning -f $InstanceName
128-
$captionMessage = $script:localizedData.TraceFlag_Add_ShouldProcessCaption
129-
130-
if ($PSCmdlet.ShouldProcess($verboseDescriptionMessage, $verboseWarningMessage, $captionMessage))
116+
# Normalize current trace flags: sort uniquely.
117+
# Get-SqlDscTraceFlag already filters out nulls and zeros, and returns [UInt32[]].
118+
$normalizedCurrentTraceFlags = $currentTraceFlags | Sort-Object -Unique
119+
120+
# Ensure $normalizedCurrentTraceFlags is an empty array if no trace flags exist.
121+
if ($null -eq $normalizedCurrentTraceFlags)
131122
{
132-
# Copy $PSBoundParameters to keep it intact.
133-
$setSqlDscTraceFlagParameters = Remove-CommonParameter -Hashtable $PSBoundParameters
123+
$normalizedCurrentTraceFlags = [System.UInt32[]] @()
124+
}
125+
126+
# Normalize input trace flags: sort uniquely.
127+
# Parameter type [System.UInt32[]] with ValidateRange already ensures non-null values and valid range.
128+
$normalizedInputTraceFlags = $TraceFlag | Sort-Object -Unique
134129

135-
$setSqlDscTraceFlagParameters.TraceFlag = $desiredTraceFlags
130+
# Combine normalized current and input trace flags to get the desired state
131+
$desiredTraceFlags = [System.UInt32[]] @(
132+
$normalizedCurrentTraceFlags
133+
$normalizedInputTraceFlags
134+
) |
135+
Sort-Object -Unique
136136

137-
$originalErrorActionPreference = $ErrorActionPreference
137+
# Compare normalized current and desired trace flags to determine if there's an effective change
138+
$compareResult = Compare-Object -ReferenceObject $normalizedCurrentTraceFlags -DifferenceObject $desiredTraceFlags
139+
140+
if ($compareResult)
141+
{
142+
$descriptionMessage = $script:localizedData.TraceFlag_Add_ShouldProcessVerboseDescription -f $InstanceName, ($desiredTraceFlags -join ', ')
143+
$confirmationMessage = $script:localizedData.TraceFlag_Add_ShouldProcessVerboseWarning -f $InstanceName
144+
$captionMessage = $script:localizedData.TraceFlag_Add_ShouldProcessCaption
138145

139-
$ErrorActionPreference = 'Stop'
146+
if ($PSCmdlet.ShouldProcess($descriptionMessage, $confirmationMessage, $captionMessage))
147+
{
148+
# Copy $PSBoundParameters to keep it intact.
149+
$setSqlDscTraceFlagParameters = Remove-CommonParameter -Hashtable $PSBoundParameters
140150

141-
Set-SqlDscTraceFlag @setSqlDscTraceFlagParameters -ErrorAction 'Stop'
151+
$setSqlDscTraceFlagParameters.TraceFlag = $desiredTraceFlags
142152

143-
$ErrorActionPreference = $originalErrorActionPreference
153+
$originalErrorActionPreference = $ErrorActionPreference
154+
155+
$ErrorActionPreference = 'Stop'
156+
157+
Set-SqlDscTraceFlag @setSqlDscTraceFlagParameters -ErrorAction 'Stop'
158+
159+
$ErrorActionPreference = $originalErrorActionPreference
160+
}
144161
}
145162
}
146163
}

‎source/Public/Get-SqlDscTraceFlag.ps1‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,12 @@ function Get-SqlDscTraceFlag
7373

7474
$traceFlags = [System.UInt32[]] @()
7575

76-
if ($startupParameter)
76+
if ($startupParameter -and $startupParameter.TraceFlag)
7777
{
78-
$traceFlags = $startupParameter.TraceFlag
78+
# Filter out null and zero values (nulls get converted to 0 in UInt32 arrays).
79+
# Valid trace flags start at 1.
80+
$traceFlags = $startupParameter.TraceFlag |
81+
Where-Object -FilterScript { $_ -ne 0 }
7982
}
8083

8184
Write-Debug -Message (

‎tests/Integration/Commands/Add-SqlDscTraceFlag.Integration.Tests.ps1‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ Describe 'Add-SqlDscTraceFlag' -Tag @('Integration_SQL2017', 'Integration_SQL201
8989
It 'Should not duplicate existing trace flags when adding them again' {
9090
# Arrange - Ensure a trace flag is already set
9191
Add-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -TraceFlag $script:singleTestTraceFlag -Force -ErrorAction 'Stop'
92-
92+
9393
$beforeAddTraceFlags = Get-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -ErrorAction 'Stop'
9494
$beforeCount = ($beforeAddTraceFlags | Where-Object { $_ -eq $script:singleTestTraceFlag }).Count
9595

@@ -101,7 +101,7 @@ Describe 'Add-SqlDscTraceFlag' -Tag @('Integration_SQL2017', 'Integration_SQL201
101101
# Assert - Verify no duplicate was created
102102
$afterAddTraceFlags = Get-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -ErrorAction 'Stop'
103103
$afterCount = ($afterAddTraceFlags | Where-Object { $_ -eq $script:singleTestTraceFlag }).Count
104-
104+
105105
$afterCount | Should -Be $beforeCount
106106
$afterAddTraceFlags | Should -Contain $script:singleTestTraceFlag
107107
}
@@ -120,6 +120,16 @@ Describe 'Add-SqlDscTraceFlag' -Tag @('Integration_SQL2017', 'Integration_SQL201
120120
$currentTraceFlags | Should -Contain $script:singleTestTraceFlag
121121
$currentTraceFlags | Should -Contain $script:additionalTestTraceFlag
122122
}
123+
124+
It 'Should de-duplicate trace flags provided in the input array' {
125+
# Act - Add trace flags with duplicates in the input array
126+
$null = Add-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -TraceFlag @($script:singleTestTraceFlag, $script:singleTestTraceFlag, $script:additionalTestTraceFlag, $script:additionalTestTraceFlag) -Force -ErrorAction 'Stop'
127+
128+
# Assert - Verify each trace flag appears only once
129+
$currentTraceFlags = Get-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -ErrorAction 'Stop'
130+
($currentTraceFlags | Where-Object { $_ -eq $script:singleTestTraceFlag }).Count | Should -Be 1
131+
($currentTraceFlags | Where-Object { $_ -eq $script:additionalTestTraceFlag }).Count | Should -Be 1
132+
}
123133
}
124134

125135
Context 'When adding trace flags using ServiceObject parameter' {
@@ -170,5 +180,15 @@ Describe 'Add-SqlDscTraceFlag' -Tag @('Integration_SQL2017', 'Integration_SQL201
170180
$currentTraceFlags | Should -Contain $traceFlag
171181
}
172182
}
183+
184+
It 'Should de-duplicate trace flags provided in the input array using ServiceObject parameter' {
185+
# Act - Add trace flags with duplicates in the input array
186+
$null = Add-SqlDscTraceFlag -ServiceObject $script:serviceObject -TraceFlag @($script:singleTestTraceFlag, $script:singleTestTraceFlag, $script:additionalTestTraceFlag, $script:additionalTestTraceFlag) -Force -ErrorAction 'Stop'
187+
188+
# Assert - Verify each trace flag appears only once
189+
$currentTraceFlags = Get-SqlDscTraceFlag -ServiceObject $script:serviceObject -ErrorAction 'Stop'
190+
($currentTraceFlags | Where-Object { $_ -eq $script:singleTestTraceFlag }).Count | Should -Be 1
191+
($currentTraceFlags | Where-Object { $_ -eq $script:additionalTestTraceFlag }).Count | Should -Be 1
192+
}
173193
}
174194
}

‎tests/Unit/Public/Add-SqlDscTraceFlag.Tests.ps1‎

Lines changed: 118 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,14 @@ Describe 'Add-SqlDscTraceFlag' -Tag 'Public' {
8787
}
8888
}
8989

90+
Context 'When passing $null in TraceFlag array' {
91+
It 'Should throw the correct error' {
92+
$mockErrorMessage = 'Cannot validate argument on parameter ''TraceFlag''*'
93+
94+
{ Add-SqlDscTraceFlag -TraceFlag @(4199, $null, 3226) -Force } | Should -Throw -ExpectedMessage $mockErrorMessage
95+
}
96+
}
97+
9098
Context 'When there are no existing trace flags' {
9199
BeforeAll {
92100
Mock -CommandName Set-SqlDscTraceFlag
@@ -195,10 +203,116 @@ Describe 'Add-SqlDscTraceFlag' -Tag 'Public' {
195203
It 'Should not add duplicate if it already exist' {
196204
{ Add-SqlDscTraceFlag -ServiceObject $mockServiceObject -TraceFlag 3226 -Force } | Should -Not -Throw
197205

198-
Should -Invoke -CommandName Set-SqlDscTraceFlag -ParameterFilter {
199-
$TraceFlag.Count -eq 1 -and
200-
$TraceFlag -contains 3226
201-
} -Exactly -Times 1 -Scope It
206+
# Should not call Set-SqlDscTraceFlag when the trace flag already exists (idempotent)
207+
Should -Invoke -CommandName Set-SqlDscTraceFlag -Exactly -Times 0 -Scope It
208+
}
209+
}
210+
211+
Context 'When duplicate trace flags are provided in the input' {
212+
BeforeAll {
213+
Mock -CommandName Set-SqlDscTraceFlag
214+
}
215+
216+
Context 'When there are no existing trace flags and duplicates are provided' {
217+
BeforeAll {
218+
Mock -CommandName Get-SqlDscTraceFlag -MockWith {
219+
return @()
220+
}
221+
222+
$mockServiceObject = [Microsoft.SqlServer.Management.Smo.Wmi.Service]::CreateTypeInstance()
223+
$mockServiceObject.Name = 'MSSQL$SQL2022'
224+
}
225+
226+
It 'Should de-duplicate trace flags when only duplicates are provided' {
227+
$null = Add-SqlDscTraceFlag -ServiceObject $mockServiceObject -TraceFlag 4199,4199 -Force
228+
229+
Should -Invoke -CommandName Set-SqlDscTraceFlag -ParameterFilter {
230+
$TraceFlag.Count -eq 1 -and
231+
$TraceFlag -contains 4199
232+
} -Exactly -Times 1 -Scope It
233+
}
234+
235+
It 'Should de-duplicate trace flags when mix of unique and duplicates are provided' {
236+
$null = Add-SqlDscTraceFlag -ServiceObject $mockServiceObject -TraceFlag 4199,3226,4199,3226 -Force
237+
238+
Should -Invoke -CommandName Set-SqlDscTraceFlag -ParameterFilter {
239+
$TraceFlag.Count -eq 2 -and
240+
$TraceFlag -contains 4199 -and
241+
$TraceFlag -contains 3226
242+
} -Exactly -Times 1 -Scope It
243+
}
244+
245+
It 'Should handle multiple duplicates of multiple trace flags' {
246+
$null = Add-SqlDscTraceFlag -ServiceObject $mockServiceObject -TraceFlag 4199,4199,3226,3226,3226,1222 -Force
247+
248+
Should -Invoke -CommandName Set-SqlDscTraceFlag -ParameterFilter {
249+
$TraceFlag.Count -eq 3 -and
250+
$TraceFlag -contains 4199 -and
251+
$TraceFlag -contains 3226 -and
252+
$TraceFlag -contains 1222
253+
} -Exactly -Times 1 -Scope It
254+
}
255+
}
256+
257+
Context 'When there are existing trace flags and duplicates are provided' {
258+
BeforeAll {
259+
Mock -CommandName Get-SqlDscTraceFlag -MockWith {
260+
return @(3226)
261+
}
262+
263+
$mockServiceObject = [Microsoft.SqlServer.Management.Smo.Wmi.Service]::CreateTypeInstance()
264+
$mockServiceObject.Name = 'MSSQL$SQL2022'
265+
}
266+
267+
It 'Should de-duplicate when adding trace flags that include duplicates and existing flag' {
268+
$null = Add-SqlDscTraceFlag -ServiceObject $mockServiceObject -TraceFlag 4199,3226,4199 -Force
269+
270+
Should -Invoke -CommandName Set-SqlDscTraceFlag -ParameterFilter {
271+
$TraceFlag.Count -eq 2 -and
272+
$TraceFlag -contains 4199 -and
273+
$TraceFlag -contains 3226
274+
} -Exactly -Times 1 -Scope It
275+
}
276+
277+
It 'Should de-duplicate when all provided trace flags are duplicates of existing flag' {
278+
$null = Add-SqlDscTraceFlag -ServiceObject $mockServiceObject -TraceFlag 3226,3226 -Force
279+
280+
# Should not call Set-SqlDscTraceFlag when all flags already exist (idempotent)
281+
Should -Invoke -CommandName Set-SqlDscTraceFlag -Exactly -Times 0 -Scope It
282+
}
283+
284+
It 'Should de-duplicate complex scenario with existing and new flags' {
285+
$null = Add-SqlDscTraceFlag -ServiceObject $mockServiceObject -TraceFlag 4199,4199,3226,1222,1222 -Force
286+
287+
Should -Invoke -CommandName Set-SqlDscTraceFlag -ParameterFilter {
288+
$TraceFlag.Count -eq 3 -and
289+
$TraceFlag -contains 4199 -and
290+
$TraceFlag -contains 3226 -and
291+
$TraceFlag -contains 1222
292+
} -Exactly -Times 1 -Scope It
293+
}
294+
}
295+
296+
Context 'When there are multiple existing trace flags and duplicates are provided' {
297+
BeforeAll {
298+
Mock -CommandName Get-SqlDscTraceFlag -MockWith {
299+
return @(3226, 1222)
300+
}
301+
302+
$mockServiceObject = [Microsoft.SqlServer.Management.Smo.Wmi.Service]::CreateTypeInstance()
303+
$mockServiceObject.Name = 'MSSQL$SQL2022'
304+
}
305+
306+
It 'Should de-duplicate and merge with multiple existing trace flags' {
307+
$null = Add-SqlDscTraceFlag -ServiceObject $mockServiceObject -TraceFlag 4199,4199,3226,1222 -Force
308+
309+
Should -Invoke -CommandName Set-SqlDscTraceFlag -ParameterFilter {
310+
$TraceFlag.Count -eq 3 -and
311+
$TraceFlag -contains 4199 -and
312+
$TraceFlag -contains 3226 -and
313+
$TraceFlag -contains 1222
314+
} -Exactly -Times 1 -Scope It
315+
}
202316
}
203317
}
204318
}

0 commit comments

Comments
 (0)