diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c920b39c6..b7221dd799 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -111,35 +111,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 by dropping and recreating the audit with the new GUID. This parameter is required when changing the `AuditGuid` property because SQL Server does not allow direct modification of the audit GUID ([issue #2287](https://github.com/dsccommunity/SqlServerDsc/issues/2287)). - -### Changed - -- Refactored integration tests to remove `finally` blocks from `It`-blocks and - use Pester `BeforeEach`/`AfterEach` blocks instead, following DSC Community - coding guidelines. This improves test cleanup reliability and maintainability - across the following test files ([issue #2288](https://github.com/dsccommunity/SqlServerDsc/issues/2288)): - - `New-SqlDscLogin.Integration.Tests.ps1` - - `New-SqlDscAudit.Integration.Tests.ps1` - - `Get-SqlDscDatabasePermission.Integration.Tests.ps1` - - `Get-SqlDscPreferredModule.Integration.Tests.ps1` - - `New-SqlDscDatabase.Integration.Tests.ps1` -- Refactored unit tests to remove `finally` blocks from `It`-blocks and - use Pester `AfterEach` blocks instead, following DSC Community coding - guidelines. This improves test cleanup reliability and maintainability - across the following test files ([issue #2288](https://github.com/dsccommunity/SqlServerDsc/issues/2288)): - - `Set-SqlDscConfigurationOption.Tests.ps1` - - `Get-SqlDscConfigurationOption.Tests.ps1` - - `Test-SqlDscConfigurationOption.Tests.ps1` -- `Test-SqlDscIsDatabasePrincipal` and `Get-SqlDscDatabasePermission` - - Added `Refresh` parameter to refresh SMO collections before checking - database principals, addressing issues with custom database roles created - via T-SQL that aren't immediately visible to SMO. The refresh logic is - optimized to only refresh collections that will be used based on exclude - parameters, improving performance on databases with large numbers of principals - ([issue #2221](https://github.com/dsccommunity/SqlServerDsc/issues/2221)). - -### Fixed - - `Save-SqlDscSqlServerMediaFile` - Fixed the Force parameter to work correctly when the target ISO file already 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 ([issue #2239](https://github.com/dsccommunity/SqlServerDsc/issues/2239)). - Updated `.gitattributes` to enforce LF line endings for PowerShell files to ensure cross-platform compatibility. + +### Changed + +- `Add-SqlDscTraceFlag` + - Improved de-duplication logic to normalize element types to `[System.UInt32]` + before sorting and removing duplicates, ensuring proper handling of mixed + numeric types ([issue #2277](https://github.com/dsccommunity/SqlServerDsc/issues/2277)). + - Added idempotent behavior by comparing current and desired trace flags before + calling `Set-SqlDscTraceFlag`, skipping unnecessary writes when there are no + effective changes ([issue #2277](https://github.com/dsccommunity/SqlServerDsc/issues/2277)). +- Refactored integration tests to remove `finally` blocks from `It`-blocks and + use Pester `BeforeEach`/`AfterEach` blocks instead, following DSC Community + coding guidelines. This improves test cleanup reliability and maintainability + across the following test files ([issue #2288](https://github.com/dsccommunity/SqlServerDsc/issues/2288)): + - `New-SqlDscLogin.Integration.Tests.ps1` + - `New-SqlDscAudit.Integration.Tests.ps1` + - `Get-SqlDscDatabasePermission.Integration.Tests.ps1` + - `Get-SqlDscPreferredModule.Integration.Tests.ps1` + - `New-SqlDscDatabase.Integration.Tests.ps1` +- Refactored unit tests to remove `finally` blocks from `It`-blocks and + use Pester `AfterEach` blocks instead, following DSC Community coding + guidelines. This improves test cleanup reliability and maintainability + across the following test files ([issue #2288](https://github.com/dsccommunity/SqlServerDsc/issues/2288)): + - `Set-SqlDscConfigurationOption.Tests.ps1` + - `Get-SqlDscConfigurationOption.Tests.ps1` + - `Test-SqlDscConfigurationOption.Tests.ps1` +- `Test-SqlDscIsDatabasePrincipal` and `Get-SqlDscDatabasePermission` + - Added `Refresh` parameter to refresh SMO collections before checking + database principals, addressing issues with custom database roles created + via T-SQL that aren't immediately visible to SMO. The refresh logic is + optimized to only refresh collections that will be used based on exclude + parameters, improving performance on databases with large numbers of principals + ([issue #2221](https://github.com/dsccommunity/SqlServerDsc/issues/2221)). - Updated GitHub Copilot setup workflow to fix environment variable assignment in task. - Updated VS Code tasks configuration to use proper build and test commands diff --git a/source/Public/Add-SqlDscTraceFlag.ps1 b/source/Public/Add-SqlDscTraceFlag.ps1 index 075737be44..083fc1f864 100644 --- a/source/Public/Add-SqlDscTraceFlag.ps1 +++ b/source/Public/Add-SqlDscTraceFlag.ps1 @@ -13,7 +13,7 @@ Specifies the server name where the instance exist. .PARAMETER InstanceName - Specifies the instance name on which to remove the trace flags. + Specifies the instance name on which to add the trace flags. .PARAMETER TraceFlag Specifies the trace flags to add. @@ -72,6 +72,7 @@ function Add-SqlDscTraceFlag $InstanceName = 'MSSQLSERVER', [Parameter(Mandatory = $true)] + [ValidateRange(1, [System.UInt32]::MaxValue)] [System.UInt32[]] $TraceFlag, @@ -112,35 +113,51 @@ function Add-SqlDscTraceFlag $ErrorActionPreference = $originalErrorActionPreference - $desiredTraceFlags = [System.UInt32[]] $currentTraceFlags + @( - $TraceFlag | - ForEach-Object -Process { - # Add only when it does not already exist. - if ($_ -notin $currentTraceFlags) - { - $_ - } - } - ) - - $verboseDescriptionMessage = $script:localizedData.TraceFlag_Add_ShouldProcessVerboseDescription -f $InstanceName, ($TraceFlag -join ', ') - $verboseWarningMessage = $script:localizedData.TraceFlag_Add_ShouldProcessVerboseWarning -f $InstanceName - $captionMessage = $script:localizedData.TraceFlag_Add_ShouldProcessCaption - - if ($PSCmdlet.ShouldProcess($verboseDescriptionMessage, $verboseWarningMessage, $captionMessage)) + # Normalize current trace flags: sort uniquely. + # Get-SqlDscTraceFlag already filters out nulls and zeros, and returns [UInt32[]]. + $normalizedCurrentTraceFlags = $currentTraceFlags | Sort-Object -Unique + + # Ensure $normalizedCurrentTraceFlags is an empty array if no trace flags exist. + if ($null -eq $normalizedCurrentTraceFlags) { - # Copy $PSBoundParameters to keep it intact. - $setSqlDscTraceFlagParameters = Remove-CommonParameter -Hashtable $PSBoundParameters + $normalizedCurrentTraceFlags = [System.UInt32[]] @() + } + + # Normalize input trace flags: sort uniquely. + # Parameter type [System.UInt32[]] with ValidateRange already ensures non-null values and valid range. + $normalizedInputTraceFlags = $TraceFlag | Sort-Object -Unique - $setSqlDscTraceFlagParameters.TraceFlag = $desiredTraceFlags + # Combine normalized current and input trace flags to get the desired state + $desiredTraceFlags = [System.UInt32[]] @( + $normalizedCurrentTraceFlags + $normalizedInputTraceFlags + ) | + Sort-Object -Unique - $originalErrorActionPreference = $ErrorActionPreference + # Compare normalized current and desired trace flags to determine if there's an effective change + $compareResult = Compare-Object -ReferenceObject $normalizedCurrentTraceFlags -DifferenceObject $desiredTraceFlags + + if ($compareResult) + { + $descriptionMessage = $script:localizedData.TraceFlag_Add_ShouldProcessVerboseDescription -f $InstanceName, ($desiredTraceFlags -join ', ') + $confirmationMessage = $script:localizedData.TraceFlag_Add_ShouldProcessVerboseWarning -f $InstanceName + $captionMessage = $script:localizedData.TraceFlag_Add_ShouldProcessCaption - $ErrorActionPreference = 'Stop' + if ($PSCmdlet.ShouldProcess($descriptionMessage, $confirmationMessage, $captionMessage)) + { + # Copy $PSBoundParameters to keep it intact. + $setSqlDscTraceFlagParameters = Remove-CommonParameter -Hashtable $PSBoundParameters - Set-SqlDscTraceFlag @setSqlDscTraceFlagParameters -ErrorAction 'Stop' + $setSqlDscTraceFlagParameters.TraceFlag = $desiredTraceFlags - $ErrorActionPreference = $originalErrorActionPreference + $originalErrorActionPreference = $ErrorActionPreference + + $ErrorActionPreference = 'Stop' + + Set-SqlDscTraceFlag @setSqlDscTraceFlagParameters -ErrorAction 'Stop' + + $ErrorActionPreference = $originalErrorActionPreference + } } } } diff --git a/source/Public/Get-SqlDscTraceFlag.ps1 b/source/Public/Get-SqlDscTraceFlag.ps1 index c3282f5382..15c1d4fb61 100644 --- a/source/Public/Get-SqlDscTraceFlag.ps1 +++ b/source/Public/Get-SqlDscTraceFlag.ps1 @@ -73,9 +73,12 @@ function Get-SqlDscTraceFlag $traceFlags = [System.UInt32[]] @() - if ($startupParameter) + if ($startupParameter -and $startupParameter.TraceFlag) { - $traceFlags = $startupParameter.TraceFlag + # Filter out null and zero values (nulls get converted to 0 in UInt32 arrays). + # Valid trace flags start at 1. + $traceFlags = $startupParameter.TraceFlag | + Where-Object -FilterScript { $_ -ne 0 } } Write-Debug -Message ( diff --git a/tests/Integration/Commands/Add-SqlDscTraceFlag.Integration.Tests.ps1 b/tests/Integration/Commands/Add-SqlDscTraceFlag.Integration.Tests.ps1 index c7e6f8a6d5..bcc984ce1a 100644 --- a/tests/Integration/Commands/Add-SqlDscTraceFlag.Integration.Tests.ps1 +++ b/tests/Integration/Commands/Add-SqlDscTraceFlag.Integration.Tests.ps1 @@ -89,7 +89,7 @@ Describe 'Add-SqlDscTraceFlag' -Tag @('Integration_SQL2017', 'Integration_SQL201 It 'Should not duplicate existing trace flags when adding them again' { # Arrange - Ensure a trace flag is already set Add-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -TraceFlag $script:singleTestTraceFlag -Force -ErrorAction 'Stop' - + $beforeAddTraceFlags = Get-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -ErrorAction 'Stop' $beforeCount = ($beforeAddTraceFlags | Where-Object { $_ -eq $script:singleTestTraceFlag }).Count @@ -101,7 +101,7 @@ Describe 'Add-SqlDscTraceFlag' -Tag @('Integration_SQL2017', 'Integration_SQL201 # Assert - Verify no duplicate was created $afterAddTraceFlags = Get-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -ErrorAction 'Stop' $afterCount = ($afterAddTraceFlags | Where-Object { $_ -eq $script:singleTestTraceFlag }).Count - + $afterCount | Should -Be $beforeCount $afterAddTraceFlags | Should -Contain $script:singleTestTraceFlag } @@ -120,6 +120,16 @@ Describe 'Add-SqlDscTraceFlag' -Tag @('Integration_SQL2017', 'Integration_SQL201 $currentTraceFlags | Should -Contain $script:singleTestTraceFlag $currentTraceFlags | Should -Contain $script:additionalTestTraceFlag } + + It 'Should de-duplicate trace flags provided in the input array' { + # Act - Add trace flags with duplicates in the input array + $null = Add-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -TraceFlag @($script:singleTestTraceFlag, $script:singleTestTraceFlag, $script:additionalTestTraceFlag, $script:additionalTestTraceFlag) -Force -ErrorAction 'Stop' + + # Assert - Verify each trace flag appears only once + $currentTraceFlags = Get-SqlDscTraceFlag -ServerName $script:mockComputerName -InstanceName $script:mockInstanceName -ErrorAction 'Stop' + ($currentTraceFlags | Where-Object { $_ -eq $script:singleTestTraceFlag }).Count | Should -Be 1 + ($currentTraceFlags | Where-Object { $_ -eq $script:additionalTestTraceFlag }).Count | Should -Be 1 + } } Context 'When adding trace flags using ServiceObject parameter' { @@ -170,5 +180,15 @@ Describe 'Add-SqlDscTraceFlag' -Tag @('Integration_SQL2017', 'Integration_SQL201 $currentTraceFlags | Should -Contain $traceFlag } } + + It 'Should de-duplicate trace flags provided in the input array using ServiceObject parameter' { + # Act - Add trace flags with duplicates in the input array + $null = Add-SqlDscTraceFlag -ServiceObject $script:serviceObject -TraceFlag @($script:singleTestTraceFlag, $script:singleTestTraceFlag, $script:additionalTestTraceFlag, $script:additionalTestTraceFlag) -Force -ErrorAction 'Stop' + + # Assert - Verify each trace flag appears only once + $currentTraceFlags = Get-SqlDscTraceFlag -ServiceObject $script:serviceObject -ErrorAction 'Stop' + ($currentTraceFlags | Where-Object { $_ -eq $script:singleTestTraceFlag }).Count | Should -Be 1 + ($currentTraceFlags | Where-Object { $_ -eq $script:additionalTestTraceFlag }).Count | Should -Be 1 + } } } diff --git a/tests/Unit/Public/Add-SqlDscTraceFlag.Tests.ps1 b/tests/Unit/Public/Add-SqlDscTraceFlag.Tests.ps1 index aa95217ac7..e6720b1ca9 100644 --- a/tests/Unit/Public/Add-SqlDscTraceFlag.Tests.ps1 +++ b/tests/Unit/Public/Add-SqlDscTraceFlag.Tests.ps1 @@ -87,6 +87,14 @@ Describe 'Add-SqlDscTraceFlag' -Tag 'Public' { } } + Context 'When passing $null in TraceFlag array' { + It 'Should throw the correct error' { + $mockErrorMessage = 'Cannot validate argument on parameter ''TraceFlag''*' + + { Add-SqlDscTraceFlag -TraceFlag @(4199, $null, 3226) -Force } | Should -Throw -ExpectedMessage $mockErrorMessage + } + } + Context 'When there are no existing trace flags' { BeforeAll { Mock -CommandName Set-SqlDscTraceFlag @@ -195,10 +203,116 @@ Describe 'Add-SqlDscTraceFlag' -Tag 'Public' { It 'Should not add duplicate if it already exist' { { Add-SqlDscTraceFlag -ServiceObject $mockServiceObject -TraceFlag 3226 -Force } | Should -Not -Throw - Should -Invoke -CommandName Set-SqlDscTraceFlag -ParameterFilter { - $TraceFlag.Count -eq 1 -and - $TraceFlag -contains 3226 - } -Exactly -Times 1 -Scope It + # Should not call Set-SqlDscTraceFlag when the trace flag already exists (idempotent) + Should -Invoke -CommandName Set-SqlDscTraceFlag -Exactly -Times 0 -Scope It + } + } + + Context 'When duplicate trace flags are provided in the input' { + BeforeAll { + Mock -CommandName Set-SqlDscTraceFlag + } + + Context 'When there are no existing trace flags and duplicates are provided' { + BeforeAll { + Mock -CommandName Get-SqlDscTraceFlag -MockWith { + return @() + } + + $mockServiceObject = [Microsoft.SqlServer.Management.Smo.Wmi.Service]::CreateTypeInstance() + $mockServiceObject.Name = 'MSSQL$SQL2022' + } + + It 'Should de-duplicate trace flags when only duplicates are provided' { + $null = Add-SqlDscTraceFlag -ServiceObject $mockServiceObject -TraceFlag 4199,4199 -Force + + Should -Invoke -CommandName Set-SqlDscTraceFlag -ParameterFilter { + $TraceFlag.Count -eq 1 -and + $TraceFlag -contains 4199 + } -Exactly -Times 1 -Scope It + } + + It 'Should de-duplicate trace flags when mix of unique and duplicates are provided' { + $null = Add-SqlDscTraceFlag -ServiceObject $mockServiceObject -TraceFlag 4199,3226,4199,3226 -Force + + Should -Invoke -CommandName Set-SqlDscTraceFlag -ParameterFilter { + $TraceFlag.Count -eq 2 -and + $TraceFlag -contains 4199 -and + $TraceFlag -contains 3226 + } -Exactly -Times 1 -Scope It + } + + It 'Should handle multiple duplicates of multiple trace flags' { + $null = Add-SqlDscTraceFlag -ServiceObject $mockServiceObject -TraceFlag 4199,4199,3226,3226,3226,1222 -Force + + Should -Invoke -CommandName Set-SqlDscTraceFlag -ParameterFilter { + $TraceFlag.Count -eq 3 -and + $TraceFlag -contains 4199 -and + $TraceFlag -contains 3226 -and + $TraceFlag -contains 1222 + } -Exactly -Times 1 -Scope It + } + } + + Context 'When there are existing trace flags and duplicates are provided' { + BeforeAll { + Mock -CommandName Get-SqlDscTraceFlag -MockWith { + return @(3226) + } + + $mockServiceObject = [Microsoft.SqlServer.Management.Smo.Wmi.Service]::CreateTypeInstance() + $mockServiceObject.Name = 'MSSQL$SQL2022' + } + + It 'Should de-duplicate when adding trace flags that include duplicates and existing flag' { + $null = Add-SqlDscTraceFlag -ServiceObject $mockServiceObject -TraceFlag 4199,3226,4199 -Force + + Should -Invoke -CommandName Set-SqlDscTraceFlag -ParameterFilter { + $TraceFlag.Count -eq 2 -and + $TraceFlag -contains 4199 -and + $TraceFlag -contains 3226 + } -Exactly -Times 1 -Scope It + } + + It 'Should de-duplicate when all provided trace flags are duplicates of existing flag' { + $null = Add-SqlDscTraceFlag -ServiceObject $mockServiceObject -TraceFlag 3226,3226 -Force + + # Should not call Set-SqlDscTraceFlag when all flags already exist (idempotent) + Should -Invoke -CommandName Set-SqlDscTraceFlag -Exactly -Times 0 -Scope It + } + + It 'Should de-duplicate complex scenario with existing and new flags' { + $null = Add-SqlDscTraceFlag -ServiceObject $mockServiceObject -TraceFlag 4199,4199,3226,1222,1222 -Force + + Should -Invoke -CommandName Set-SqlDscTraceFlag -ParameterFilter { + $TraceFlag.Count -eq 3 -and + $TraceFlag -contains 4199 -and + $TraceFlag -contains 3226 -and + $TraceFlag -contains 1222 + } -Exactly -Times 1 -Scope It + } + } + + Context 'When there are multiple existing trace flags and duplicates are provided' { + BeforeAll { + Mock -CommandName Get-SqlDscTraceFlag -MockWith { + return @(3226, 1222) + } + + $mockServiceObject = [Microsoft.SqlServer.Management.Smo.Wmi.Service]::CreateTypeInstance() + $mockServiceObject.Name = 'MSSQL$SQL2022' + } + + It 'Should de-duplicate and merge with multiple existing trace flags' { + $null = Add-SqlDscTraceFlag -ServiceObject $mockServiceObject -TraceFlag 4199,4199,3226,1222 -Force + + Should -Invoke -CommandName Set-SqlDscTraceFlag -ParameterFilter { + $TraceFlag.Count -eq 3 -and + $TraceFlag -contains 4199 -and + $TraceFlag -contains 3226 -and + $TraceFlag -contains 1222 + } -Exactly -Times 1 -Scope It + } } } }