Add-SqlDscTraceFlag: Simplify trace flag addition uniqueness

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

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
+            }
         }
     }
 }

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 (

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
+        }
     }
 }

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
+            }
         }
     }
 }